RustCrypto / RSA

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

RSA signatures larger than the modulus are accepted #272

Closed randombit closed 1 year ago

randombit commented 1 year ago

raw_encryption_primitive is used for verifying signatures. It does not check that the decoded integer value is less than the modulus, thus if s is a valid signature it will also accept s+k*n where n is the public modulus and k is any positive integer.

This introduces signature malleability, which is probably not an enormous problem in most applications, but neither does it seem desirable.

A related issue is that signatures are decoded as big integers but it is never checked that the length of the signature is equal to the public modulus length. So if sig is the binary encoding of a valid signature, prefixing that signature with any number of zero bytes will also be accepted as valid.

This may affect ciphertext decryption as well, but I haven't checked this.

The following patch to the tests demonstrates the issues

diff --git a/src/pkcs1v15.rs b/src/pkcs1v15.rs
index 5d9bac4..50b6dc1 100644
--- a/src/pkcs1v15.rs
+++ b/src/pkcs1v15.rs
@@ -1152,15 +1152,23 @@ mod tests {
             (
                 "Test.\n",
                 hex!(
-                    "a4f3fa6ea93bcdd0c57be020c1193ecbfd6f200a3d95c409769b029578fa0e33"
+                    "00a4f3fa6ea93bcdd0c57be020c1193ecbfd6f200a3d95c409769b029578fa0e33"
                     "6ad9a347600e40d3ae823b8c7e6bad88cc07c1d54c3a1523cbbb6d58efc362ae"
                 ),
                 true,
             ),
             (
                 "Test.\n",
+                // signature + modulus
                 hex!(
-                    "a4f3fa6ea93bcdd0c57be020c1193ecbfd6f200a3d95c409769b029578fa0e33"
+                    "01578d09b86db9c85d997c8e8b0e34c9076782843561884f09b2969a0e83d8a88033052e71e182beb26f0d9e2363a5b64bfd7040c7a88609b7375d2e3ef3e1ffc3"
+                ),
+                true,
+            ),
+            (
+                "Test.\n",
+                hex!(
+                    "00a4f3fa6ea93bcdd0c57be020c1193ecbfd6f200a3d95c409769b029578fa0e33"
                     "6ad9a347600e40d3ae823b8c7e6bad88cc07c1d54c3a1523cbbb6d58efc362af"
                 ),
                 false,
tarcieri commented 1 year ago

A related issue is that signatures are decoded as big integers but it is never checked that the length of the signature is equal to the public modulus length.

I think this only applies to RSASSA-PKCS#1v1.5. It seems like the PSS implementation has always had checks for the signature length:

https://github.com/RustCrypto/RSA/commit/5d28baf32a30173ebef0ab0c5d9cd2e98c522074#diff-22b3e2164bd21d723f22be3c3381f4f58ba9c360f2079d76b93bd50b37e43611R21

Anyway, will get them added to PKCS#1v1.5 too.

tarcieri commented 1 year ago

I believe #306 should address these concerns