RustCrypto / RSA

RSA implementation in pure Rust
Apache License 2.0
550 stars 153 forks source link

Fix pss sign/verify when key length is multiple of 8 + 1 bits. #263

Closed dfabregat closed 1 year ago

dfabregat commented 1 year ago

In the unusual case of having a key of length k*8 + 1 bits, em_len will be 1 byte shorter than key_len. The call to raw_encryption_primitive requires a buffer key_len bytes long.

Vice versa, we would be generating a one-byte too-long salt in generate_salt.

Finally, emsa_pss_verify_pre would fail by shifting 1 byte left by 8 bits.

dignifiedquire commented 1 year ago

is there an actual use case for keys of that size, or should we just restrict the sizes?

dfabregat commented 1 year ago

I personally do not remember encountering any special use case of this type. I've generally seen multiples of 512. On the other hand, OpenSSL, for instance, seems to support all key lengths (within whichever range they allow).

In the end, it is a design decision. I personally would rather support this case, but it is just my opinion. Alternatively, one can restrict and see if anyone ever raises a request for this. Then, we learn in which context would one have such a need :)

dignifiedquire commented 1 year ago

@tarcieri any thoughts on this?

tarcieri commented 1 year ago

I think it'd be good to eliminate panics regardless of what invariants we choose to uphold, and if we have _unchecked APIs, that means we don't really have invariants.

All that said, I think the default (checked) APIs should probably at least check the key size is an even number.

dignifiedquire commented 1 year ago

@tarcieri agreed, merging this, as this in all cases improves the status quo

thanks for catching and fixing @dfabregat

dfabregat commented 1 year ago

Always glad to contribute. And thank you guys for your feedback :smiley: