Improve SSlibKey validation in _verify

Added checks as requrested in secure-systems-lab#766:

- keytype matches scheme
- scheme matches deserialized key type (only for pem formatted keyvals;
  for ed25519 the check happens implicitly on deserialization)
- scheme matches deserialized key curve (only for ecdsa)

Note that this PR does not move the checks to the constructor as
suggested in secure-systems-lab#766. This may or may not be addressed in a follow-up PR.

failing tests fixed in secure-systems-lab#794

Signed-off-by: Lukas Puehringer <[email protected]>

securesystemslib/signer/_key.py

@@ -341,9 +341,18 @@ def _verify_ed25519_fallback(self, signature: bytes, data: bytes) -> None:
 
     def _verify(self, signature: bytes, data: bytes) -> None:
         """Helper to verify signature using pyca/cryptography (default)."""
+
+        def _validate_type(key, type_):
+            if not isinstance(key, type_):
+                raise ValueError(f"bad key {key} for {self.scheme}")
+
+        def _validate_curve(key, curve):
+            if not isinstance(key.curve, curve):
+                raise ValueError(f"bad curve {key.curve} for {self.scheme}")
+
         try:
             key: PublicKeyTypes
-            if self.scheme in [
+            if self.keytype == "rsa" and self.scheme in [
                 "rsassa-pss-sha224",
                 "rsassa-pss-sha256",
                 "rsassa-pss-sha384",
@@ -354,28 +363,39 @@ def _verify(self, signature: bytes, data: bytes) -> None:
                 "rsa-pkcs1v15-sha512",
             ]:
                 key = cast(RSAPublicKey, self._crypto_key())
+                _validate_type(key, RSAPublicKey)
                 padding_name, hash_name = self.scheme.split("-")[1:]
                 hash_algorithm = self._get_hash_algorithm(hash_name)
                 padding = self._get_rsa_padding(padding_name, hash_algorithm)
                 key.verify(signature, data, padding, hash_algorithm)
 
-            elif self.scheme in [
-                "ecdsa-sha2-nistp256",
-                "ecdsa-sha2-nistp384",
-            ]:
+            elif (
+                self.keytype in ["ecdsa", "ecdsa-sha2-nistp256"]
+                and self.scheme == "ecdsa-sha2-nistp256"
+            ):
                 key = cast(EllipticCurvePublicKey, self._crypto_key())
-                hash_name = f"sha{self.scheme[-3:]}"
-                hash_algorithm = self._get_hash_algorithm(hash_name)
-                signature_algorithm = ECDSA(hash_algorithm)
-                key.verify(signature, data, signature_algorithm)
+                _validate_type(key, EllipticCurvePublicKey)
+                _validate_curve(key, SECP256R1)
+                key.verify(signature, data, ECDSA(SHA256()))
+
+            elif (
+                self.keytype in ["ecdsa", "ecdsa-sha2-nistp384"]
+                and self.scheme == "ecdsa-sha2-nistp384"
+            ):
+                key = cast(EllipticCurvePublicKey, self._crypto_key())
+                _validate_type(key, EllipticCurvePublicKey)
+                _validate_curve(key, SECP384R1)
+                key.verify(signature, data, ECDSA(SHA384()))
 
-            elif self.scheme in ["ed25519"]:
+            elif self.keytype == "ed25519" and self.scheme == "ed25519":
                 public_bytes = bytes.fromhex(self.keyval["public"])
                 key = Ed25519PublicKey.from_public_bytes(public_bytes)
                 key.verify(signature, data)
 
             else:
-                raise ValueError(f"unknown scheme '{self.scheme}'")
+                raise ValueError(
+                    f"Unsupported public key {self.keytype}/{self.scheme}"
+                )
 
         except InvalidSignature as e:
             raise UnverifiedSignatureError from e

tests/test_signer.py

@@ -339,6 +339,34 @@ def _from_file(path):
         self.assertEqual(key.scheme, "rsa-pkcs1v15-sha224")
         self.assertEqual(key.keyid, "abcdef")
 
+    def test_verify_invalid_keytype_scheme(self):
+
+        rsa = "-----BEGIN PUBLIC KEY-----\nMIIBojANBgkqhkiG9w0BAQEFAAOCAY8AMIIBigKCAYEAsDqUoiFJZX+5gm5pyI1l\nVc/N3yjJVOIl9GyiK0mRyzV3IzUQzhjq8nhk0eLfzXw2XwIAYOJC6dR/tGRG4JDx\nJkez5FFH4zLosr/XzT7CG5zxJ3kKICLD1v9rZQr5ZgARQDOpkxzPz46rGnE0sHd7\nMpnpPMScA1pMIzwM1RoPS4ntZipI1cl9M7HMQ6mkBp8/DNKCqaDWixJqaGgWrhhK\nhI/1mzBliMKriNxPKSCGVlOk/QpZft+y1fs42s0DMd5BOFBo+ZcoXLYRncg9S3A2\nxx/jT69Bt3ceiAZqnp7f6M+ZzoUifSelaoL7QIYg/GkEl+0oxTD0yRphGiCKwn9c\npSbn7NgnbjqSgIMeEtlf/5Coyrs26pyFf/9GbusddPSxxxwIJ/7IJuF7P1Yy0WpZ\nkMeY83h9n2IdnEYi+rpdbLJPQd7Fpu2xrdA3Fokj8AvCpcmxn8NIXZuK++r8/xsE\nAUL30HH7dgVn50AvdPaJnqAORT3OlabW0DK9prcwKnyzAgMBAAE=\n-----END PUBLIC KEY-----"
+        ed25519 = (
+            "50a5768a7a577483c28e57a6742b4d2170b9be628a961355ef127c45f2aefdc5"
+        )
+        ecdsa_nistp256 = "-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEsYJfSlYU3UlYbGOZfE/yOHkayWWq\nLPR/NeCa83szZmnJGc9wwCRPvJS87K+eDGIhhhKueTyrLqXQqmyHioQbOQ==\n-----END PUBLIC KEY-----\n"
+        ecdsa_nistp384 = "-----BEGIN PUBLIC KEY-----\nMHYwEAYHKoZIzj0CAQYFK4EEACIDYgAEksAG80nLUksODTEUBTPJJPYN0bfxhkrr\n2hlyokfRG4kDYsRRN86vWwxDTW7qhWNZPFhJMJxHmvHsCbLz/IF7hdo8Xv/vRO4M\nVHbwq0fiWznUvkZowHC5fH2EEvNF1R5t\n-----END PUBLIC KEY-----\n"
+
+        test_data = [
+            # bad keytype / scheme
+            ("ed25519", "rsassa-pss-sha256", rsa),
+            ("ecdsa-sha2-nistp384", "ecdsa-sha2-nistp256", ecdsa_nistp256),
+            ("ecdsa-sha2-nistp256", "ecdsa-sha2-nistp384", ecdsa_nistp384),
+            ("rsa", "ed25519", ed25519),
+            # bad key type (pem formatted keys only)
+            ("rsa", "rsassa-pss-sha256", ecdsa_nistp256),
+            ("ecdsa", "ecdsa-sha2-nistp256", rsa),
+            # bad curve (ecdsa keys only)
+            ("ecdsa", "ecdsa-sha2-nistp256", ecdsa_nistp384),
+            ("ecdsa", "ecdsa-sha2-nistp384", ecdsa_nistp256),
+        ]
+
+        for keytype, scheme, val in test_data:
+            key = SSlibKey("fake", keytype, scheme, {"public": val})
+            with self.assertRaises(ValueError) as ctx:
+                key._verify(b"fakesig", b"fakedata")
+
 
 class TestSigner(unittest.TestCase):
     """Test Signer and SSlibSigner functionality"""