allow SPKI RSA keys to be parsed even if they have an incorrect delimiter (#10248) (#10251)

reaperhulk · web-flow · commit 56255de6b2d1 · 2024-01-25T02:02:37.000Z
* allow SPKI RSA keys to be parsed even if they have an incorrect delimiter

This allows RSA SPKI keys (typically delimited with PUBLIC KEY) to be parsed
even if they are using the RSA PUBLIC KEY delimiter.

* formatting

* use original error if nothing parses, don't let it parse non-RSA
diff --git a/docs/development/test-vectors.rst b/docs/development/test-vectors.rst
@@ -72,12 +72,18 @@ Custom asymmetric vectors
 * ``asymmetric/PEM_Serialization/ec_public_key.pem`` and
   ``asymmetric/DER_Serialization/ec_public_key.der``- Contains the public key
   corresponding to ``ec_private_key.pem``, generated using OpenSSL.
+* ``asymmetric/PEM_Serialization/ec_public_key_rsa_delimiter.pem`` - Contains
+  the public key corresponding to ``ec_private_key.pem``, but with the wrong PEM
+  delimiter (``RSA PUBLIC KEY`` when it should be ``PUBLIC KEY``).
 * ``asymmetric/PEM_Serialization/rsa_private_key.pem`` - Contains an RSA 2048
   bit key generated using OpenSSL, protected by the secret "123456" with DES3
   encryption.
 * ``asymmetric/PEM_Serialization/rsa_public_key.pem`` and
   ``asymmetric/DER_Serialization/rsa_public_key.der``- Contains an RSA 2048
   bit public generated using OpenSSL from ``rsa_private_key.pem``.
+* ``asymmetric/PEM_Serialization/rsa_wrong_delimiter_public_key.pem`` - Contains
+  an RSA 2048 bit public key generated from ``rsa_private_key.pem``, but with
+  the wrong PEM delimiter (``RSA PUBLIC KEY`` when it should be ``PUBLIC KEY``).
 * ``asymmetric/PEM_Serialization/dsa_4096.pem`` - Contains a 4096-bit DSA
   private key generated using OpenSSL.
 * ``asymmetric/PEM_Serialization/dsaparam.pem`` - Contains 2048-bit DSA
diff --git a/src/rust/src/backend/keys.rs b/src/rust/src/backend/keys.rs
@@ -165,7 +165,26 @@ fn load_pem_public_key(
     let _ = backend;
     let p = pem::parse(data.as_bytes())?;
     let pkey = match p.tag() {
-        "RSA PUBLIC KEY" => cryptography_key_parsing::rsa::parse_pkcs1_public_key(p.contents())?,
+        "RSA PUBLIC KEY" => {
+            // We try to parse it as a PKCS1 first since that's the PEM delimiter, and if
+            // that fails we try to parse it as an SPKI. This is to match the permissiveness
+            // of OpenSSL, which doesn't care about the delimiter.
+            match cryptography_key_parsing::rsa::parse_pkcs1_public_key(p.contents()) {
+                Ok(pkey) => pkey,
+                Err(err) => {
+                    let pkey = cryptography_key_parsing::spki::parse_public_key(p.contents())
+                        .map_err(|_| err)?;
+                    if pkey.id() != openssl::pkey::Id::RSA {
+                        return Err(CryptographyError::from(
+                            pyo3::exceptions::PyValueError::new_err(
+                                "Incorrect PEM delimiter for key type.",
+                            ),
+                        ));
+                    }
+                    pkey
+                }
+            }
+        }
         "PUBLIC KEY" => cryptography_key_parsing::spki::parse_public_key(p.contents())?,
         _ => return Err(CryptographyError::from(pem::PemError::MalformedFraming)),
     };
diff --git a/tests/hazmat/primitives/test_serialization.py b/tests/hazmat/primitives/test_serialization.py
@@ -506,6 +506,11 @@ def test_load_pem_ec_private_key(self, key_path, password, backend):
                 "asymmetric", "PEM_Serialization", "rsa_public_key.pem"
             ),
             os.path.join("asymmetric", "public", "PKCS1", "rsa.pub.pem"),
+            os.path.join(
+                "asymmetric",
+                "PEM_Serialization",
+                "rsa_wrong_delimiter_public_key.pem",
+            ),
         ],
     )
     def test_load_pem_rsa_public_key(self, key_file, backend):
@@ -520,6 +525,17 @@ def test_load_pem_rsa_public_key(self, key_file, backend):
         numbers = key.public_numbers()
         assert numbers.e == 65537
 
+    def test_load_pem_public_fails_with_ec_key_with_rsa_delimiter(self):
+        with pytest.raises(ValueError):
+            load_vectors_from_file(
+                os.path.join(
+                    "asymmetric",
+                    "PEM_Serialization",
+                    "ec_public_key_rsa_delimiter.pem",
+                ),
+                lambda pemfile: load_pem_public_key(pemfile.read().encode()),
+            )
+
     def test_load_priv_key_with_public_key_api_fails(
         self, rsa_key_2048, backend
     ):
diff --git a/vectors/cryptography_vectors/asymmetric/PEM_Serialization/ec_public_key_rsa_delimiter.pem b/vectors/cryptography_vectors/asymmetric/PEM_Serialization/ec_public_key_rsa_delimiter.pem
@@ -0,0 +1,4 @@
+-----BEGIN RSA PUBLIC KEY-----
+MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEJLzzbuz2tRnLFlOL+6bTX6giVavA
+sc6NDFFT0IMCd2ibTTNUDDkFGsgq0cH5JYPg/6xUlMBFKrWYe3yQ4has9w==
+-----END RSA PUBLIC KEY-----
diff --git a/vectors/cryptography_vectors/asymmetric/PEM_Serialization/rsa_wrong_delimiter_public_key.pem b/vectors/cryptography_vectors/asymmetric/PEM_Serialization/rsa_wrong_delimiter_public_key.pem
@@ -0,0 +1,9 @@
+-----BEGIN RSA PUBLIC KEY-----
+MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAnR4AZ+tgWYql+S3MaTQ6
+zeIO1fKzFIoau9Q0zGuv/1oCAewXwxeDSSxw+/Z3GL1NpuuS9CpbR5EQ3d71bD0v
+0G+Sf+mShSl0oljG7YqnNSPzKl+EQ3/KE+eEButcwas6KGof2BA4bFNCw/fPbuhk
+u/d8sIIEgdzBMiGRMdW33uci3rsdOenMZQA7uWsM/q/pu85YLAVOxq6wlUCzP4FM
+Tw/RKzayrPkn3Jfbqcy1aM2HDlFVx24vaN+RRbPSnVoQbo5EQYkUMXE8WmadSyHl
+pXGRnWsJSV9AdGyDrbU+6tcFwcIwnW22jb/OJy8swHdqKGkuR1kQ0XqokK1yGKFZ
+8wIDAQAB
+-----END RSA PUBLIC KEY-----
