4 ms·
Edit: The comment below is incorrect. I'm leaving it for transparency, but I misread where the padding is being applied. I could be missing something here, but
by woodruffw 3y ago
Edit: The comment below is incorrect. I'm leaving it for transparency, but I misread where the padding is being applied.
I could be missing something here, but I think this is vulnerable to DO'1985, a/k/a Desmedt-Odlyzko:
https://github.com/rpgp/rpgp/blob/8e67756ebce780c91b8c2ffc7db1f6230f8a9419/src/crypto/rsa.rs#L172-L178 https://github.com/rpgp/rpgp/blob/8e67756ebce780c91b8c2ffc7d...
In particular, in the presence of an insufficiently wide hash, the absence of padding here means that RSA signature validation is not secure under EUF-CMA. Matt Green has a great post on why and when EUF-CMA matters[1].
(This isn't necessarily this implementation's fault, since PGP seemingly (!) encourages the stripping of padding from signatures. But I can't find another source for whether this is actually encouraged by OpenPGP, or whether implementations just widely allow it.)
[1]: https://blog.cryptographyengineering.com/euf-cma-and-suf-cma/ https://blog.cryptographyengineering.com/euf-cma-and-suf-cma...
- dignifiedquire 3y agoI am not sure if this is an actual issue, all auditors that looked at this so far haven’t mentioned this being a problem. But I will have to investigate what the exact state is.
- woodruffw 3y agoAccording to `git blame`, this was introduced June 2023, i.e. after your audit in 2019. But maybe it was moved from an older piece of the codebase, I didn't dig too deep. (Looks like the IncludeSec folks did a decent job in 2019. Hi Eric!)
- dignifiedquire 3y agoThis was allowed in the rust-rsa crate directly before, which is why it was introduced in that commit.
- woodruffw 3y agoYep, I saw the upstream[1]. However, I misread this: I thought the padding was being done on the cleartext signing side, but this is padding of the signature itself. So there's some malleability here, but it isn't susceptible to DO'1985. I'll update my top-level comment. [1]: https://github.com/RustCrypto/RSA/issues/272 https://github.com/RustCrypto/RSA/issues/272
- dignifiedquire 3y agoThanks, appreciate the careful check!
- junon 3y agoGlad people care to look, that's what matters.
- deleted 3y ago[deleted]