RustCrypto / RSA

RSA implementation in pure Rust
Apache License 2.0
536 stars 146 forks source link

OAEP hash input limitation #434

Closed Bl4omArchie closed 2 months ago

Bl4omArchie commented 3 months ago

Hello, recently I saw in the oaep algorithm a todo comment about the input limitation of hash other than sha1. I made my research and I came out with a solution:

You can find the answers in NIST FIPS PUB 180-4 Secure Hash Standard that specify the hash input limit for every sha families. For sha1 and sha2 the limit is 2^64 bits and for sha3 the limit is 2^128 bits. See figure 1 for the complete table.

For blake2 the limit is specified in the original paper appendix A as 2^128. Md5 doesn't seem to have a specified limit.

I forked the project to make a pull request with the solution but as I'm new to the project and not enough comfortable with Rust.

How can I found out which hash is used ? According to the Digest's craft documentation it don't seem to be possible to see which hash was selected. How can we handle this case ? I thought about two possible options:

Thank you for taking the time to read my issue. With your help I'll be glad to finish this task!

tarcieri commented 3 months ago

I can confirm that the label length is correctly limited by the input size of the hash function: https://datatracker.ietf.org/doc/html/rfc8017#section-7.1.1

If the length of L is greater than the input limitation for the hash function (2^61 - 1 octets for SHA-1), output "label too long" and stop.

I don't believe we have anything in the extant digest traits for querying this information dynamically, certainly not in the DynDigest API which is used by e.g. oaep_encrypt.

All that said, I'm not sure how much that really matters in practice versus a static sanity limit as we have now. OAEP labels are typically empty or if not that, relatively short strings.

Bl4omArchie commented 3 months ago

I also thought of a static limit which is more simple of course. Because of the todo I wanted to clarify this point but if you see no need to change then everything's fine!

tarcieri commented 3 months ago

We are in the midst of another breaking release cycle, so it would be possible to add another method to digest::DynDigest like max_input_size(&self) -> Option<u64> or thereabouts.

If you're interested in that, it would be good to open a separate issue on https://github.com/rustcrypto/traits for the digest crate

Bl4omArchie commented 3 months ago

great! I'll do that

newpavlov commented 2 months ago

After discussion in https://github.com/RustCrypto/traits/issues/1590 we decide that it's probably fine to keep the current limit for all hash functions.