feat(verification): Add PublicKeyIdentifier hash matching and old-sty…

CHANGELOG.md

@@ -17,8 +17,11 @@ All versions prior to 1.0.0 are untracked.
 - Fix and test the sharded file hasher
 - Added tests for verifying signatures created with v0.3.1
 - cli: `model_signing sign` now supports the `--oauth_force_oob` option (default: False)
-- Added support for specifying `--client_id` and `--client_secret` for OIDC authentication.
+- Added support for specifying `--client_id` and `--client_secret` for OIDC authentication
 - cli: Added support for `--allow_symlinks` option
+- Fix Bundle deserialization error caused by null keyid in DSSE signatures; keyid now serializes as an empty string
+- Implemented public key identifier hash matching for bundle verification
+- Add warning for older verification material formats (e.g., raw public key bytes) during verification, recommending re-signing
 
 ## [1.0.1] - 2024-04-18
 

src/model_signing/_signing/sign_ec_key.py

@@ -14,6 +14,7 @@
 
 """Signers and verifiers using elliptic curve keys."""
 
+import hashlib
 import pathlib
 from typing import Optional
 
@@ -125,26 +126,16 @@ def sign(self, payload: signing.Payload) -> signing.Signature:
     def _get_verification_material(self) -> bundle_pb.VerificationMaterial:
         """Returns the verification material to include in the bundle."""
         public_key = self._private_key.public_key()
-        key_size = public_key.curve.key_size
-
-        # TODO: Once Python 3.9 support is deprecated revert to using `match`
-        if key_size == 256:
-            key_details = common_pb.PublicKeyDetails.PKIX_ECDSA_P256_SHA_256
-        elif key_size == 384:
-            key_details = common_pb.PublicKeyDetails.PKIX_ECDSA_P384_SHA_384
-        elif key_size == 521:
-            key_details = common_pb.PublicKeyDetails.PKIX_ECDSA_P521_SHA_512
-        else:
-            raise ValueError(f"Unexpected key size {key_size}")
+
+        raw_bytes = public_key.public_bytes(
+            encoding=serialization.Encoding.PEM,
+            format=serialization.PublicFormat.SubjectPublicKeyInfo,
+        )
+
+        hash_bytes = hashlib.sha256(raw_bytes).digest().hex()
 
         return bundle_pb.VerificationMaterial(
-            public_key=common_pb.PublicKey(
-                raw_bytes=public_key.public_bytes(
-                    encoding=serialization.Encoding.PEM,
-                    format=serialization.PublicFormat.SubjectPublicKeyInfo,
-                ),
-                key_details=key_details,
-            )
+            public_key=common_pb.PublicKeyIdentifier(hint=hash_bytes)
         )
 
 
@@ -165,9 +156,28 @@ def __init__(self, public_key_path: pathlib.Path):
 
     @override
     def _verify_bundle(self, bundle: bundle_pb.Bundle) -> tuple[str, bytes]:
-        # TODO: This method currently does not verify that the public key
-        # matches what is included in the signature's verification material.
-        # Instead, the verification material is completely ignored right now.
+        raw_bytes = self._public_key.public_bytes(
+            encoding=serialization.Encoding.PEM,
+            format=serialization.PublicFormat.SubjectPublicKeyInfo,
+        )
+
+        hash_bytes = hashlib.sha256(raw_bytes).digest().hex()
+
+        if bundle.verification_material.public_key.hint:
+            key_hint = bundle.verification_material.public_key.hint
+            if key_hint != hash_bytes:
+                raise ValueError(
+                    "Key mismatch: The public key hash in the signature's "
+                    "verification material does not match the provided "
+                    "public key. "
+                )
+        else:
+            print(
+                "WARNING: This model's signature uses an older "
+                "verification material format. Please re-sign "
+                "with an updated signer to use a public key "
+                "identifier hash. "
+            )
 
         envelope = bundle.dsse_envelope
         try:

src/model_signing/_signing/sign_pkcs11.py

@@ -13,6 +13,7 @@
 # limitations under the License.
 
 from collections.abc import Iterable
+import hashlib
 import pathlib
 from typing import Optional
 
@@ -191,26 +192,16 @@ def sign(self, payload: signing.Payload) -> signing.Signature:
     def _get_verification_material(self) -> bundle_pb.VerificationMaterial:
         """Returns the verification material to include in the bundle."""
         public_key = self._public_key
-        key_size = public_key.curve.key_size
-
-        # TODO: Once Python 3.9 support is deprecated revert to using `match`
-        if key_size == 256:
-            key_details = common_pb.PublicKeyDetails.PKIX_ECDSA_P256_SHA_256
-        elif key_size == 384:
-            key_details = common_pb.PublicKeyDetails.PKIX_ECDSA_P384_SHA_384
-        elif key_size == 521:
-            key_details = common_pb.PublicKeyDetails.PKIX_ECDSA_P521_SHA_512
-        else:
-            raise ValueError(f"Unexpected key size {key_size}")
+
+        raw_bytes = public_key.public_bytes(
+            encoding=serialization.Encoding.PEM,
+            format=serialization.PublicFormat.SubjectPublicKeyInfo,
+        )
+
+        hash_bytes = hashlib.sha256(raw_bytes).digest().hex()
 
         return bundle_pb.VerificationMaterial(
-            public_key=common_pb.PublicKey(
-                raw_bytes=public_key.public_bytes(
-                    encoding=serialization.Encoding.PEM,
-                    format=serialization.PublicFormat.SubjectPublicKeyInfo,
-                ),
-                key_details=key_details,
-            )
+            public_key=common_pb.PublicKeyIdentifier(hint=hash_bytes)
         )
 
 

src/model_signing/_signing/sign_sigstore_pb.py

@@ -22,6 +22,7 @@
 """
 
 import abc
+import json
 import pathlib
 import sys
 from typing import cast
@@ -110,7 +111,8 @@ def write(self, path: pathlib.Path) -> None:
     @override
     def read(cls, path: pathlib.Path) -> Self:
         content = path.read_text()
-        return cls(bundle_pb.Bundle().from_json(content))
+        parsed_dict = json.loads(content)
+        return cls(bundle_pb.Bundle().from_dict(parsed_dict))
 
 
 class Signer(signing.Signer):

tests/api_test.py

@@ -165,7 +165,7 @@ def test_sign_and_verify(
             use_staging=True,
         ).verify(sample_model_folder, signature_path)
 
-        assert [
+        assert get_signed_files(signature_path) == [
             "d0/f00",
             "d0/f01",
             "d0/f02",
@@ -176,7 +176,7 @@ def test_sign_and_verify(
             "f1",
             "f2",
             "f3",
-        ] == get_signed_files(signature_path)
+        ]
         check_ignore_paths(signature_path, True, [])
         assert get_model_name(signature_path) == os.path.basename(
             sample_model_folder
@@ -214,9 +214,11 @@ def test_sign_and_verify(self, base_path, populate_tmpdir):
             )
         ).verify(model_path, signature)
 
-        assert [".gitignore", "signme-1", "signme-2"] == get_signed_files(
-            signature
-        )
+        assert get_signed_files(signature) == [
+            ".gitignore",
+            "signme-1",
+            "signme-2",
+        ]
         check_ignore_paths(signature, ignore_git_paths, ["model.sig"])
         assert get_model_name(signature) == os.path.basename(model_path)
 
@@ -233,7 +235,7 @@ def test_sign_and_verify(self, base_path, populate_tmpdir):
             )
         ).sign(model_path, signature)
 
-        assert ["signme-1", "signme-2"] == get_signed_files(signature)
+        assert get_signed_files(signature) == ["signme-1", "signme-2"]
         check_ignore_paths(
             signature, ignore_git_paths, ["model.sig", "ignored"]
         )
@@ -280,9 +282,11 @@ def test_sign_and_verify(self, base_path, populate_tmpdir):
             )
         ).verify(model_path, signature)
 
-        assert [".gitignore", "signme-1", "signme-2"] == get_signed_files(
-            signature
-        )
+        assert get_signed_files(signature) == [
+            ".gitignore",
+            "signme-1",
+            "signme-2",
+        ]
         check_ignore_paths(signature, ignore_git_paths, ["model.sig"])
         assert get_model_name(signature) == os.path.basename(model_path)
 
@@ -301,7 +305,7 @@ def test_sign_and_verify(self, base_path, populate_tmpdir):
             )
         ).sign(model_path, signature)
 
-        assert ["signme-1", "signme-2"] == get_signed_files(signature)
+        assert get_signed_files(signature) == ["signme-1", "signme-2"]
         check_ignore_paths(
             signature, ignore_git_paths, ["model.sig", "ignored"]
         )
@@ -349,11 +353,11 @@ def test_sign_and_verify_sharded(self, base_path, populate_tmpdir):
         )
         # .verify(model_path, signature)
 
-        assert [
+        assert get_signed_files(signature) == [
             ".gitignore:0:4",
             "signme-1:0:8",
             "signme-2:0:8",
-        ] == get_signed_files(signature)
+        ]
         check_ignore_paths(signature, ignore_git_paths, ["model.sig"])
         assert get_model_name(signature) == os.path.basename(model_path)
 
@@ -374,7 +378,7 @@ def test_sign_and_verify_sharded(self, base_path, populate_tmpdir):
             .use_shard_serialization()
         ).sign(model_path, signature)
 
-        assert ["signme-1:0:8", "signme-2:0:8"] == get_signed_files(signature)
+        assert get_signed_files(signature) == ["signme-1:0:8", "signme-2:0:8"]
         check_ignore_paths(
             signature, ignore_git_paths, ["model.sig", "ignored"]
         )