bugfix: impl: require at least one of the source ref/sha extensions (#109)

* 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 SWIFTSIM/swiftgalaxy#26

Signed-off-by: William Woodruff <[email protected]>

* CHANGELOG: record changes

Signed-off-by: William Woodruff <[email protected]>

* add a negative test

Signed-off-by: William Woodruff <[email protected]>

---------

Signed-off-by: William Woodruff <[email protected]>

Author: woodruffw

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

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}"
             )

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-----

test/test_impl.py

@@ -9,6 +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
@@ -662,3 +666,54 @@ 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)
+
+    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)