diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ff870d0..db697efb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ All versions prior to 1.0.0 are untracked. - Standardized CLI flags to use hyphens (e.g., `--trust-config` instead of `--trust_config`). Underscore variants are still accepted for backwards compatibility via token normalization. ### Fixed +- Fixed certificate verification accepting a signing certificate whose extended key usage does not permit code signing (for example a TLS `serverAuth` certificate), as long as the digitalSignature key usage bit was set. ([#648](https://github.com/sigstore/model-transparency/pull/648)) - Fixed a bug where reusing a single `verifying.Config` across models let the ignore paths and guessed hashing configuration from one verification carry over into later ones. A file that a later model's signature never excluded could be silently skipped instead of reported as unsigned. ([#650](https://github.com/sigstore/model-transparency/pull/650)) - Fixed a bug where installing from the sdist produced an empty wheel with zero Python modules. The hatch `packages` directive was scoped to all build targets instead of the wheel target only, causing the sdist's flattened layout to not match the expected `src/` path. ([#636](https://github.com/sigstore/model-transparency/issues/636)) - Fixed a bug where ignored symlinks could raise `ValueError`s if allow_symlinks was unset, even though they were skipped during serialization. ([#550](https://github.com/sigstore/model-transparency/pull/550)) diff --git a/src/model_signing/_signing/sign_certificate.py b/src/model_signing/_signing/sign_certificate.py index 74b017f5..2101e4a2 100644 --- a/src/model_signing/_signing/sign_certificate.py +++ b/src/model_signing/_signing/sign_certificate.py @@ -256,6 +256,23 @@ def _to_openssl_certificate(certificate_bytes, log_fingerprints): "Certificate does not specify 'ExtendedKeyUsage'." ) + # An extended key usage, when present, restricts the certificate to the + # listed purposes (RFC 5280 4.2.1.12). A certificate not marked for code + # signing must be rejected even when the digitalSignature key usage bit + # is set, otherwise a TLS (serverAuth) certificate chaining to a trusted + # root would be accepted for model signing. + try: + eku = extensions.get_extension_for_class( + x509.ExtendedKeyUsage + ).value + if ( + oid.ExtendedKeyUsageOID.CODE_SIGNING not in eku + and oid.ExtendedKeyUsageOID.ANY_EXTENDED_KEY_USAGE not in eku + ): + can_use_for_signing = False + except x509.ExtensionNotFound: + pass + if not can_use_for_signing: raise ValueError("Signing certificate cannot be used for signing") diff --git a/tests/_signing/certificate_test.py b/tests/_signing/certificate_test.py index 3d1c1c2d..08940101 100644 --- a/tests/_signing/certificate_test.py +++ b/tests/_signing/certificate_test.py @@ -12,8 +12,9 @@ # See the License for the specific language governing permissions and # limitations under the License. -"""Tests for SubjectAltName identity pinning in the certificate verifier.""" +"""Tests for the certificate verifier.""" +import base64 import datetime import pathlib @@ -23,10 +24,13 @@ from cryptography.hazmat.primitives.asymmetric import ec from cryptography.x509 import oid import pytest +from sigstore_models.bundle import v1 as bundle_pb +from sigstore_models.common import v1 as common_pb from model_signing import hashing from model_signing import signing from model_signing import verifying +from model_signing._signing import sign_certificate as certificate _SAN_URI = "spiffe://demo.example.com/signer/demo" @@ -199,3 +203,107 @@ def test_cross_signer_attack_rejected(self, tmp_path): ).set_hashing_config( hashing.Config().set_ignored_paths(paths=[sig]) ).verify(model, sig) + + +def _name(common_name): + return x509.Name([x509.NameAttribute(oid.NameOID.COMMON_NAME, common_name)]) + + +def _mint(extended_key_usages): + """Mints a private root and a leaf carrying the given extended key usages. + + The leaf always sets the digitalSignature key usage bit. Passing `None` for + the extended key usages omits the ExtendedKeyUsage extension entirely. + + Returns: + A tuple of the root certificate and the leaf certificate. + """ + now = datetime.datetime.now(datetime.timezone.utc) + root_key = ec.generate_private_key(ec.SECP256R1()) + root = ( + x509.CertificateBuilder() + .subject_name(_name("test-root")) + .issuer_name(_name("test-root")) + .public_key(root_key.public_key()) + .serial_number(x509.random_serial_number()) + .not_valid_before(now - datetime.timedelta(days=1)) + .not_valid_after(now + datetime.timedelta(days=3650)) + .add_extension( + x509.BasicConstraints(ca=True, path_length=None), critical=True + ) + .sign(root_key, hashes.SHA256()) + ) + + leaf_key = ec.generate_private_key(ec.SECP256R1()) + builder = ( + x509.CertificateBuilder() + .subject_name(_name("test-leaf")) + .issuer_name(root.subject) + .public_key(leaf_key.public_key()) + .serial_number(x509.random_serial_number()) + .not_valid_before(now - datetime.timedelta(days=1)) + .not_valid_after(now + datetime.timedelta(days=365)) + .add_extension( + x509.BasicConstraints(ca=False, path_length=None), critical=True + ) + .add_extension( + x509.KeyUsage( + digital_signature=True, + content_commitment=False, + key_encipherment=False, + data_encipherment=False, + key_agreement=False, + key_cert_sign=False, + crl_sign=False, + encipher_only=False, + decipher_only=False, + ), + critical=True, + ) + ) + if extended_key_usages is not None: + builder = builder.add_extension( + x509.ExtendedKeyUsage(extended_key_usages), critical=False + ) + leaf = builder.sign(root_key, hashes.SHA256()) + return root, leaf + + +def _material(leaf): + der = leaf.public_bytes(serialization.Encoding.DER) + return bundle_pb.VerificationMaterial( + x509_certificate_chain=common_pb.X509CertificateChain( + certificates=[ + common_pb.X509Certificate(raw_bytes=base64.b64encode(der)) + ] + ), + tlog_entries=[], + ) + + +def _verifier(root, tmp_path): + root_pem = tmp_path / "root.pem" + root_pem.write_bytes(root.public_bytes(serialization.Encoding.PEM)) + return certificate.Verifier(certificate_chain_paths=[root_pem]) + + +class TestCertificateExtendedKeyUsage: + def test_rejects_non_code_signing_eku(self, tmp_path): + # A TLS (serverAuth) certificate must not be usable for model signing, + # even though it carries the digitalSignature key usage bit. + root, leaf = _mint([oid.ExtendedKeyUsageOID.SERVER_AUTH]) + verifier = _verifier(root, tmp_path) + with pytest.raises(ValueError, match="cannot be used for signing"): + verifier._verify_certificates(_material(leaf)) + + def test_accepts_code_signing_eku(self, tmp_path): + root, leaf = _mint([oid.ExtendedKeyUsageOID.CODE_SIGNING]) + verifier = _verifier(root, tmp_path) + public_key = verifier._verify_certificates(_material(leaf)) + assert isinstance(public_key, ec.EllipticCurvePublicKey) + + def test_accepts_missing_eku(self, tmp_path): + root, leaf = _mint(None) + verifier = _verifier(root, tmp_path) + public_key = verifier._verify_certificates(_material(leaf)) + assert isinstance(public_key, ec.EllipticCurvePublicKey)