From 4b00af89b106183f55a8e2bb137525714ecb3257 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Thu, 3 Apr 2025 13:58:14 -0400 Subject: [PATCH 1/3] bugfix: impl: require at least one of the source ref/sha extensions Like with Trusted Publishing we need at least one of these, but not necessarily both. We need to gracefully handle the scenario where at least one (both not both) is missing. xref https://github.com/SWIFTSIM/swiftgalaxy/issues/26 Signed-off-by: William Woodruff --- src/pypi_attestations/_impl.py | 31 ++++++++++---- .../no-source-repository-ref-extension.pem | 41 +++++++++++++++++++ test/test_impl.py | 14 +++++++ 3 files changed, 77 insertions(+), 9 deletions(-) create mode 100644 test/assets/no-source-repository-ref-extension.pem diff --git a/src/pypi_attestations/_impl.py b/src/pypi_attestations/_impl.py index 2933ee5..0fd0f53 100644 --- a/src/pypi_attestations/_impl.py +++ b/src/pypi_attestations/_impl.py @@ -491,18 +491,31 @@ def verify(self, cert: Certificate) -> None: raw_build_config_uri = _der_decode_utf8string(build_config_uri.value.public_bytes()) # (2) Extract the source repo digest and ref. - source_repo_digest = cert.extensions.get_extension_for_oid( - policy._OIDC_SOURCE_REPOSITORY_DIGEST_OID # noqa: SLF001 - ) - sha = _der_decode_utf8string(source_repo_digest.value.public_bytes()) + # We require at least one of these to be present. + suffixes = [] + try: + source_repo_digest = cert.extensions.get_extension_for_oid( + policy._OIDC_SOURCE_REPOSITORY_DIGEST_OID # noqa: SLF001 + ) + suffixes.append(_der_decode_utf8string(source_repo_digest.value.public_bytes())) + except x509.ExtensionNotFound: + pass - source_repo_ref = cert.extensions.get_extension_for_oid( - policy._OIDC_SOURCE_REPOSITORY_REF_OID # noqa: SLF001 - ) - ref = _der_decode_utf8string(source_repo_ref.value.public_bytes()) + try: + source_repo_ref = cert.extensions.get_extension_for_oid( + policy._OIDC_SOURCE_REPOSITORY_REF_OID # noqa: SLF001 + ) + suffixes.append(_der_decode_utf8string(source_repo_ref.value.public_bytes())) + except x509.ExtensionNotFound: + pass + + if not suffixes: + raise sigstore.errors.VerificationError( + "Certificate must contain either Source Repository Digest or Source Repository Ref" + ) # (3)-(4): Build the expected URIs and compare them - for suffix in [sha, ref]: + for suffix in suffixes: expected = ( f"https://github.com/{self._repository}/.github/workflows/{self._workflow}@{suffix}" ) diff --git a/test/assets/no-source-repository-ref-extension.pem b/test/assets/no-source-repository-ref-extension.pem new file mode 100644 index 0000000..c44b6ad --- /dev/null +++ b/test/assets/no-source-repository-ref-extension.pem @@ -0,0 +1,41 @@ +See: https://search.sigstore.dev/?logIndex=191974551 + +-----BEGIN CERTIFICATE----- +MIIG7DCCBnKgAwIBAgIUJrNb177y4TSfo1t8/miMosKaxpwwCgYIKoZIzj0EAwMw +NzEVMBMGA1UEChMMc2lnc3RvcmUuZGV2MR4wHAYDVQQDExVzaWdzdG9yZS1pbnRl +cm1lZGlhdGUwHhcNMjUwNDAzMTUxMzA0WhcNMjUwNDAzMTUyMzA0WjAAMFkwEwYH +KoZIzj0CAQYIKoZIzj0DAQcDQgAEgQxA014C9CF5Zi/SXcIEVLO/EX5UkvR6yL4s +sHUCXGA3YHvLtTMm8DmJlezko3fWtA+91qDuVGt5cA4BMoowSKOCBZEwggWNMA4G +A1UdDwEB/wQEAwIHgDATBgNVHSUEDDAKBggrBgEFBQcDAzAdBgNVHQ4EFgQUwWgy +8G1LtCB3aaiQPvkewDCCvBEwHwYDVR0jBBgwFoAU39Ppz1YkEZb5qNjpKFWixi4Y +ZD8wgYMGA1UdEQEB/wR5MHeGdWh0dHBzOi8vZ2l0aHViLmNvbS9TV0lGVFNJTS9z +d2lmdGdhbGF4eS8uZ2l0aHViL3dvcmtmbG93cy9weXRob24tcHVibGlzaC55bWxA +YTI2ZWIwMDZkMDRkZDBjNDFhNmUzOGRjMTVkZGFmN2I0NjE0MzUwNTA5BgorBgEE +AYO/MAEBBCtodHRwczovL3Rva2VuLmFjdGlvbnMuZ2l0aHVidXNlcmNvbnRlbnQu +Y29tMBUGCisGAQQBg78wAQIEB3JlbGVhc2UwNgYKKwYBBAGDvzABAwQoYTI2ZWIw +MDZkMDRkZDBjNDFhNmUzOGRjMTVkZGFmN2I0NjE0MzUwNTAjBgorBgEEAYO/MAEE +BBVVcGxvYWQgUHl0aG9uIFBhY2thZ2UwIgYKKwYBBAGDvzABBQQUU1dJRlRTSU0v +c3dpZnRnYWxheHkwOwYKKwYBBAGDvzABCAQtDCtodHRwczovL3Rva2VuLmFjdGlv +bnMuZ2l0aHVidXNlcmNvbnRlbnQuY29tMIGFBgorBgEEAYO/MAEJBHcMdWh0dHBz +Oi8vZ2l0aHViLmNvbS9TV0lGVFNJTS9zd2lmdGdhbGF4eS8uZ2l0aHViL3dvcmtm +bG93cy9weXRob24tcHVibGlzaC55bWxAYTI2ZWIwMDZkMDRkZDBjNDFhNmUzOGRj +MTVkZGFmN2I0NjE0MzUwNTA4BgorBgEEAYO/MAEKBCoMKGEyNmViMDA2ZDA0ZGQw +YzQxYTZlMzhkYzE1ZGRhZjdiNDYxNDM1MDUwHQYKKwYBBAGDvzABCwQPDA1naXRo +dWItaG9zdGVkMDcGCisGAQQBg78wAQwEKQwnaHR0cHM6Ly9naXRodWIuY29tL1NX +SUZUU0lNL3N3aWZ0Z2FsYXh5MDgGCisGAQQBg78wAQ0EKgwoYTI2ZWIwMDZkMDRk +ZDBjNDFhNmUzOGRjMTVkZGFmN2I0NjE0MzUwNTAZBgorBgEEAYO/MAEPBAsMCTQ4 +ODI3MTc5NTArBgorBgEEAYO/MAEQBB0MG2h0dHBzOi8vZ2l0aHViLmNvbS9TV0lG +VFNJTTAYBgorBgEEAYO/MAERBAoMCDM3NTQxMzA5MIGFBgorBgEEAYO/MAESBHcM +dWh0dHBzOi8vZ2l0aHViLmNvbS9TV0lGVFNJTS9zd2lmdGdhbGF4eS8uZ2l0aHVi +L3dvcmtmbG93cy9weXRob24tcHVibGlzaC55bWxAYTI2ZWIwMDZkMDRkZDBjNDFh +NmUzOGRjMTVkZGFmN2I0NjE0MzUwNTA4BgorBgEEAYO/MAETBCoMKGEyNmViMDA2 +ZDA0ZGQwYzQxYTZlMzhkYzE1ZGRhZjdiNDYxNDM1MDUwFwYKKwYBBAGDvzABFAQJ +DAdyZWxlYXNlMFsGCisGAQQBg78wARUETQxLaHR0cHM6Ly9naXRodWIuY29tL1NX +SUZUU0lNL3N3aWZ0Z2FsYXh5L2FjdGlvbnMvcnVucy8xNDI0NTc0MzgxMi9hdHRl +bXB0cy8yMBYGCisGAQQBg78wARYECAwGcHVibGljMIGJBgorBgEEAdZ5AgQCBHsE +eQB3AHUA3T0wasbHETJjGR4cmWc3AqJKXrjePK3/h4pygC8p7o4AAAGV/DZ++wAA +BAMARjBEAiAGvZn6Blv/M5qoz++/bkL2y5yM1Wad5aGNwMPRY+FoUwIgFiETf+Pp +sbLE1Jm6TzHxgPoq9zBM24kO3DPFzG3tWrYwCgYIKoZIzj0EAwMDaAAwZQIwby+C +egP77F5410AdZvLwHz+7qOBUmVJxr/c7e+4qvL1v8jMHs0+z+gEVOaiNAfr5AjEA +ntFI0uzl76l0QslAefj7HiFMt1nn3Qryz1wA17U1l25rRVz8H4kfDcJ5/6i2mH5O +-----END CERTIFICATE----- diff --git a/test/test_impl.py b/test/test_impl.py index d501ac1..30633dc 100644 --- a/test/test_impl.py +++ b/test/test_impl.py @@ -9,6 +9,7 @@ import pretend import pytest import sigstore +from cryptography import x509 from pydantic import Base64Bytes, BaseModel, TypeAdapter, ValidationError from sigstore.dsse import DigestSet, StatementBuilder, Subject from sigstore.models import Bundle @@ -662,3 +663,16 @@ class TestBase64Bytes: def test_encoding(self) -> None: model = DummyModel(base64_bytes=b"aaaa" * 76) assert "\\n" not in model.model_dump_json() + + +class TestGitHubublisher: + def test_verifies_cert_with_missing_ref(self) -> None: + cert_path = _ASSETS / "no-source-repository-ref-extension.pem" + cert = x509.load_pem_x509_certificate(cert_path.read_bytes()) + + publisher = impl.GitHubPublisher( + repository="SWIFTSIM/swiftgalaxy", + workflow="python-publish.yml", + ) + + publisher._as_policy().verify(cert) From 883c29a9c794d66036a96d3ab3cbbb45cf2fd813 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Thu, 3 Apr 2025 14:03:08 -0400 Subject: [PATCH 2/3] CHANGELOG: record changes Signed-off-by: William Woodruff --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6084374..b19219d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `--offline` flag enforces that the distribution and provenance file arguments must be local file paths. +### Fixed + +- Fixed a bug where `GitHubPublisher` policy verification would fail + if the `Source Repository Ref` or `Source Repository Digest` claim + was missing from the attestation's certificate. We require at least + one of the two claims, but not necessarily both + ([#109](https://github.com/trailofbits/pypi-attestations/pull/109)) + ## [0.0.22] ### Changed From 9f087fb98ed724f77fbabf9b7140387672c5b523 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Thu, 3 Apr 2025 14:18:56 -0400 Subject: [PATCH 3/3] add a negative test Signed-off-by: William Woodruff --- test/test_impl.py | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/test/test_impl.py b/test/test_impl.py index 30633dc..3dc2787 100644 --- a/test/test_impl.py +++ b/test/test_impl.py @@ -9,7 +9,10 @@ import pretend import pytest import sigstore +import sigstore.errors from cryptography import x509 +from cryptography.hazmat.primitives import hashes +from cryptography.hazmat.primitives.asymmetric import ec from pydantic import Base64Bytes, BaseModel, TypeAdapter, ValidationError from sigstore.dsse import DigestSet, StatementBuilder, Subject from sigstore.models import Bundle @@ -676,3 +679,41 @@ def test_verifies_cert_with_missing_ref(self) -> None: ) publisher._as_policy().verify(cert) + + def test_fails_cert_with_no_digest_or_ref(self) -> None: + # To test this, we manually mangle a certificate + # to remove the digest extension. This ends up not being a valid + # certificate from an attestation perspective (since we replace + # the signature as well), but it's sufficient for the policy test. + + cert_path = _ASSETS / "no-source-repository-ref-extension.pem" + orig_cert = x509.load_pem_x509_certificate(cert_path.read_bytes()) + + # Rebuild the certificate, but with the digest extension removed + builder = ( + x509.CertificateBuilder() + .subject_name(orig_cert.subject) + .issuer_name(orig_cert.issuer) + .public_key(orig_cert.public_key()) + .serial_number(orig_cert.serial_number) + .not_valid_before(orig_cert.not_valid_before) + .not_valid_after(orig_cert.not_valid_after) + ) + + for ext in orig_cert.extensions: + if ext.oid != policy._OIDC_SOURCE_REPOSITORY_DIGEST_OID: + builder = builder.add_extension(ext.value, ext.critical) + + cert = builder.sign(ec.generate_private_key(ec.SECP256R1()), hashes.SHA256()) + + publisher = impl.GitHubPublisher( + repository="SWIFTSIM/swiftgalaxy", + workflow="python-publish.yml", + ) + with pytest.raises( + sigstore.errors.VerificationError, + match=( + "Certificate must contain either Source Repository Digest or Source Repository Ref" + ), + ): + publisher._as_policy().verify(cert)