From af0dd2d04d3ae4c35113168bd4449e26111e7375 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Fri, 11 Oct 2024 10:09:38 +0100 Subject: [PATCH 01/10] remove Verifier param from verify() API This removes a foreign type by instead constructing the Verifier under the hood within verify(). Signed-off-by: William Woodruff --- pyproject.toml | 2 +- src/pypi_attestations/_cli.py | 5 +- src/pypi_attestations/_impl.py | 10 ++- test/test_impl.py | 116 ++++++++++++++++++--------------- 4 files changed, 75 insertions(+), 58 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 650510b..b4b0587 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -13,7 +13,7 @@ dependencies = [ "cryptography", "packaging", "pydantic", - "sigstore~=3.3", + "sigstore~=3.4", "sigstore-protobuf-specs", ] requires-python = ">=3.11" diff --git a/src/pypi_attestations/_cli.py b/src/pypi_attestations/_cli.py index cf0172f..56b04ed 100644 --- a/src/pypi_attestations/_cli.py +++ b/src/pypi_attestations/_cli.py @@ -11,7 +11,7 @@ from pydantic import ValidationError from sigstore.oidc import IdentityError, IdentityToken, Issuer from sigstore.sign import SigningContext -from sigstore.verify import Verifier, policy +from sigstore.verify import policy from pypi_attestations import Attestation, AttestationError, VerificationError, __version__ from pypi_attestations._impl import Distribution @@ -256,7 +256,6 @@ def _inspect(args: argparse.Namespace) -> None: def _verify(args: argparse.Namespace) -> None: """Verify the files passed as argument.""" - verifier: Verifier = Verifier.staging() if args.staging else Verifier.production() pol = policy.Identity(identity=args.identity) # Validate that both the attestations and files exists @@ -291,7 +290,7 @@ def _verify(args: argparse.Namespace) -> None: _die(f"Invalid Python package distribution: {e}") try: - attestation.verify(verifier, pol, dist) + attestation.verify(pol, dist, staging=args.staging) except VerificationError as verification_error: _die(f"Verification failed for {input}: {verification_error}") diff --git a/src/pypi_attestations/_impl.py b/src/pypi_attestations/_impl.py index e050061..7a3a75f 100644 --- a/src/pypi_attestations/_impl.py +++ b/src/pypi_attestations/_impl.py @@ -23,6 +23,7 @@ from sigstore.dsse import Error as DsseError from sigstore.models import Bundle, LogEntry from sigstore.sign import ExpiredCertificate, ExpiredIdentity +from sigstore.verify import Verifier # pragma: no cover from sigstore_protobuf_specs.io.intoto import Envelope as _Envelope from sigstore_protobuf_specs.io.intoto import Signature as _Signature @@ -30,7 +31,6 @@ from pathlib import Path # pragma: no cover from sigstore.sign import Signer # pragma: no cover - from sigstore.verify import Verifier # pragma: no cover from sigstore.verify.policy import VerificationPolicy # pragma: no cover @@ -180,14 +180,20 @@ def sign(cls, signer: Signer, dist: Distribution) -> Attestation: def verify( self, - verifier: Verifier, policy: VerificationPolicy, dist: Distribution, + *, + staging: bool = False, ) -> tuple[str, dict[str, Any] | None]: """Verify against an existing Python distribution. On failure, raises an appropriate subclass of `AttestationError`. """ + if staging: + verifier = Verifier.staging() + else: + verifier = Verifier.production() + bundle = self.to_bundle() try: type_, payload = verifier.verify_dsse(bundle, policy) diff --git a/test/test_impl.py b/test/test_impl.py index 79fc8ee..ad07864 100644 --- a/test/test_impl.py +++ b/test/test_impl.py @@ -59,20 +59,19 @@ class TestAttestation: @online def test_roundtrip(self, id_token: IdentityToken) -> None: sign_ctx = SigningContext.staging() - verifier = Verifier.staging() with sign_ctx.signer(id_token) as signer: attestation = impl.Attestation.sign(signer, dist) - attestation.verify(verifier, policy.UnsafeNoOp(), dist) + attestation.verify(policy.UnsafeNoOp(), dist, staging=True) # converting to a bundle and verifying as a bundle also works bundle = attestation.to_bundle() - verifier.verify_dsse(bundle, policy.UnsafeNoOp()) + Verifier.staging().verify_dsse(bundle, policy.UnsafeNoOp()) # converting back also works roundtripped_attestation = impl.Attestation.from_bundle(bundle) - roundtripped_attestation.verify(verifier, policy.UnsafeNoOp(), dist) + roundtripped_attestation.verify(policy.UnsafeNoOp(), dist, staging=True) def test_wrong_predicate_raises_exception(self, monkeypatch: pytest.MonkeyPatch) -> None: def dummy_predicate(self_: StatementBuilder, _: str) -> StatementBuilder: @@ -117,7 +116,6 @@ def get_bundle(*_: Any) -> Bundle: impl.Attestation.sign(signer, dist) def test_verify_github_attested(self) -> None: - verifier = Verifier.production() pol = policy.AllOf( [ policy.OIDCSourceRepositoryURI( @@ -130,29 +128,27 @@ def test_verify_github_attested(self) -> None: bundle = Bundle.from_json(gh_signed_dist_bundle_path.read_bytes()) attestation = impl.Attestation.from_bundle(bundle) - predicate_type, predicate = attestation.verify(verifier, pol, gh_signed_dist) + predicate_type, predicate = attestation.verify(pol, gh_signed_dist) assert predicate_type == "https://docs.pypi.org/attestations/publish/v1" assert predicate == {} def test_verify(self) -> None: - verifier = Verifier.staging() # Our checked-in asset has this identity. pol = policy.Identity( identity="william@yossarian.net", issuer="https://github.com/login/oauth" ) attestation = impl.Attestation.model_validate_json(dist_attestation_path.read_text()) - predicate_type, predicate = attestation.verify(verifier, pol, dist) + predicate_type, predicate = attestation.verify(pol, dist, staging=True) assert predicate_type == "https://docs.pypi.org/attestations/publish/v1" assert predicate is None # convert the attestation to a bundle and verify it that way too bundle = attestation.to_bundle() - verifier.verify_dsse(bundle, policy.UnsafeNoOp()) + Verifier.staging().verify_dsse(bundle, policy.UnsafeNoOp()) def test_verify_digest_mismatch(self, tmp_path: Path) -> None: - verifier = Verifier.staging() # Our checked-in asset has this identity. pol = policy.Identity( identity="william@yossarian.net", issuer="https://github.com/login/oauth" @@ -169,10 +165,9 @@ def test_verify_digest_mismatch(self, tmp_path: Path) -> None: with pytest.raises( impl.VerificationError, match="subject does not match distribution digest" ): - attestation.verify(verifier, pol, modified_dist) + attestation.verify(pol, modified_dist, staging=True) def test_verify_filename_mismatch(self, tmp_path: Path) -> None: - verifier = Verifier.staging() # Our checked-in asset has this identity. pol = policy.Identity( identity="william@yossarian.net", issuer="https://github.com/login/oauth" @@ -189,43 +184,48 @@ def test_verify_filename_mismatch(self, tmp_path: Path) -> None: with pytest.raises( impl.VerificationError, match="subject does not match distribution name" ): - attestation.verify(verifier, pol, different_name_dist) + attestation.verify(pol, different_name_dist, staging=True) def test_verify_policy_mismatch(self) -> None: - verifier = Verifier.staging() # Wrong identity. pol = policy.Identity(identity="fake@example.com", issuer="https://github.com/login/oauth") attestation = impl.Attestation.model_validate_json(dist_attestation_path.read_text()) with pytest.raises(impl.VerificationError, match=r"Certificate's SANs do not match"): - attestation.verify(verifier, pol, dist) + attestation.verify(pol, dist, staging=True) - def test_verify_wrong_envelope(self) -> None: - verifier = pretend.stub( - verify_dsse=pretend.call_recorder(lambda bundle, policy: ("fake-type", None)) + def test_verify_wrong_envelope(self, monkeypatch: pytest.MonkeyPatch) -> None: + staging = pretend.call_recorder( + lambda: pretend.stub( + verify_dsse=pretend.call_recorder(lambda bundle, policy: ("fake-type", None)) + ) ) + monkeypatch.setattr(impl.Verifier, "staging", staging) pol = pretend.stub() attestation = impl.Attestation.model_validate_json(dist_attestation_path.read_text()) with pytest.raises(impl.VerificationError, match="expected JSON envelope, got fake-type"): - attestation.verify(verifier, pol, dist) + attestation.verify(pol, dist, staging=True) - def test_verify_bad_payload(self) -> None: - verifier = pretend.stub( - verify_dsse=pretend.call_recorder( - lambda bundle, policy: ("application/vnd.in-toto+json", b"invalid json") + def test_verify_bad_payload(self, monkeypatch: pytest.MonkeyPatch) -> None: + staging = pretend.call_recorder( + lambda: pretend.stub( + verify_dsse=pretend.call_recorder( + lambda bundle, policy: ("application/vnd.in-toto+json", b"invalid json") + ) ) ) + monkeypatch.setattr(impl.Verifier, "staging", staging) pol = pretend.stub() attestation = impl.Attestation.model_validate_json(dist_attestation_path.read_text()) with pytest.raises(impl.VerificationError, match="invalid statement"): - attestation.verify(verifier, pol, dist) + attestation.verify(pol, dist, staging=True) - def test_verify_too_many_subjects(self) -> None: + def test_verify_too_many_subjects(self, monkeypatch: pytest.MonkeyPatch) -> None: statement = ( StatementBuilder() # noqa: SLF001 .subjects( @@ -239,22 +239,25 @@ def test_verify_too_many_subjects(self) -> None: ._inner.model_dump_json() ) - verifier = pretend.stub( - verify_dsse=pretend.call_recorder( - lambda bundle, policy: ( - "application/vnd.in-toto+json", - statement.encode(), + staging = pretend.call_recorder( + lambda: pretend.stub( + verify_dsse=pretend.call_recorder( + lambda bundle, policy: ( + "application/vnd.in-toto+json", + statement.encode(), + ) ) ) ) + monkeypatch.setattr(impl.Verifier, "staging", staging) pol = pretend.stub() attestation = impl.Attestation.model_validate_json(dist_attestation_path.read_text()) with pytest.raises(impl.VerificationError, match="too many subjects in statement"): - attestation.verify(verifier, pol, dist) + attestation.verify(pol, dist, staging=True) - def test_verify_subject_missing_name(self) -> None: + def test_verify_subject_missing_name(self, monkeypatch: pytest.MonkeyPatch) -> None: statement = ( StatementBuilder() # noqa: SLF001 .subjects( @@ -267,22 +270,25 @@ def test_verify_subject_missing_name(self) -> None: ._inner.model_dump_json() ) - verifier = pretend.stub( - verify_dsse=pretend.call_recorder( - lambda bundle, policy: ( - "application/vnd.in-toto+json", - statement.encode(), + staging = pretend.call_recorder( + lambda: pretend.stub( + verify_dsse=pretend.call_recorder( + lambda bundle, policy: ( + "application/vnd.in-toto+json", + statement.encode(), + ) ) ) ) + monkeypatch.setattr(impl.Verifier, "staging", staging) pol = pretend.stub() attestation = impl.Attestation.model_validate_json(dist_attestation_path.read_text()) with pytest.raises(impl.VerificationError, match="invalid subject: missing name"): - attestation.verify(verifier, pol, dist) + attestation.verify(pol, dist, staging=True) - def test_verify_subject_invalid_name(self) -> None: + def test_verify_subject_invalid_name(self, monkeypatch: pytest.MonkeyPatch) -> None: statement = ( StatementBuilder() # noqa: SLF001 .subjects( @@ -298,22 +304,25 @@ def test_verify_subject_invalid_name(self) -> None: ._inner.model_dump_json() ) - verifier = pretend.stub( - verify_dsse=pretend.call_recorder( - lambda bundle, policy: ( - "application/vnd.in-toto+json", - statement.encode(), + staging = pretend.call_recorder( + lambda: pretend.stub( + verify_dsse=pretend.call_recorder( + lambda bundle, policy: ( + "application/vnd.in-toto+json", + statement.encode(), + ) ) ) ) + monkeypatch.setattr(impl.Verifier, "staging", staging) pol = pretend.stub() attestation = impl.Attestation.model_validate_json(dist_attestation_path.read_text()) with pytest.raises(impl.VerificationError, match="invalid subject: Invalid wheel filename"): - attestation.verify(verifier, pol, dist) + attestation.verify(pol, dist, staging=True) - def test_verify_unknown_attestation_type(self) -> None: + def test_verify_unknown_attestation_type(self, monkeypatch: pytest.MonkeyPatch) -> None: statement = ( StatementBuilder() # noqa: SLF001 .subjects( @@ -335,20 +344,23 @@ def test_verify_unknown_attestation_type(self) -> None: ._inner.model_dump_json() ) - verifier = pretend.stub( - verify_dsse=pretend.call_recorder( - lambda bundle, policy: ( - "application/vnd.in-toto+json", - statement.encode(), + staging = pretend.call_recorder( + lambda: pretend.stub( + verify_dsse=pretend.call_recorder( + lambda bundle, policy: ( + "application/vnd.in-toto+json", + statement.encode(), + ) ) ) ) + monkeypatch.setattr(impl.Verifier, "staging", staging) pol = pretend.stub() attestation = impl.Attestation.model_validate_json(dist_attestation_path.read_text()) with pytest.raises(impl.VerificationError, match="unknown attestation type: foo"): - attestation.verify(verifier, pol, dist) + attestation.verify(pol, dist, staging=True) def test_from_bundle_missing_signatures() -> None: From 5401277064c3f70fee73030a55fc9c9365e934f2 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Fri, 11 Oct 2024 11:20:47 +0100 Subject: [PATCH 02/10] add Publisher._as_policy WIP. Signed-off-by: William Woodruff --- src/pypi_attestations/_impl.py | 74 ++++++++++++++++++++++++++++++++-- 1 file changed, 71 insertions(+), 3 deletions(-) diff --git a/src/pypi_attestations/_impl.py b/src/pypi_attestations/_impl.py index 7a3a75f..1b5220e 100644 --- a/src/pypi_attestations/_impl.py +++ b/src/pypi_attestations/_impl.py @@ -23,7 +23,7 @@ from sigstore.dsse import Error as DsseError from sigstore.models import Bundle, LogEntry from sigstore.sign import ExpiredCertificate, ExpiredIdentity -from sigstore.verify import Verifier # pragma: no cover +from sigstore.verify import Verifier, policy from sigstore_protobuf_specs.io.intoto import Envelope as _Envelope from sigstore_protobuf_specs.io.intoto import Signature as _Signature @@ -180,15 +180,31 @@ def sign(cls, signer: Signer, dist: Distribution) -> Attestation: def verify( self, - policy: VerificationPolicy, + identity: VerificationPolicy | Publisher, dist: Distribution, *, staging: bool = False, ) -> tuple[str, dict[str, Any] | None]: """Verify against an existing Python distribution. + The `identity` can be an object confirming to + `sigstore.policy.VerificationPolicy` or a `Publisher`, which will be + transformed into an appropriate verification policy. + + By default, Sigstore's production verifier will be used. The + `staging` parameter can be toggled to enable the staging verifier + instead. + On failure, raises an appropriate subclass of `AttestationError`. """ + # NOTE: Can't do `isinstance` with `Publisher` since it's + # a `_GenericAlias`; instead we punch through to the inner + # `_Publisher` union. + if isinstance(identity, _Publisher): + policy = identity._as_policy() # noqa: SLF001 + else: + policy = identity + if staging: verifier = Verifier.staging() else: @@ -370,6 +386,10 @@ class _PublisherBase(BaseModel): kind: str claims: dict[str, Any] | None = None + def _as_policy(self) -> VerificationPolicy: + """Return an appropriate `sigstore.policy.VerificationPolicy` for this publisher.""" + raise NotImplementedError + class GitHubPublisher(_PublisherBase): """A GitHub-based Trusted Publisher.""" @@ -394,6 +414,33 @@ class GitHubPublisher(_PublisherBase): action was performed from. """ + def _as_policy(self) -> VerificationPolicy: + policies: list[VerificationPolicy] = [ + policy.OIDCIssuerV2("https://token.actions.githubusercontent.com"), + policy.OIDCSourceRepositoryURI(f"https://github.com/{self.repository}"), + ] + + if not self.claims: + raise VerificationError("refusing to build a policy without claims") + + sha = self.claims.get("sha") + ref = self.claims.get("ref") + if not (sha or ref): + # This should never happen, since we should always _at least_ have + # the `sha` claim. + raise VerificationError("refusing to build a policy without a sha or ref claim") + + expected_build_configs: list[VerificationPolicy] = [ + policy.OIDCBuildConfigURI( + f"https://github.com/{self.repository}/.github/workflows/{self.workflow}@{claim}" + ) + for claim in [ref, sha] + if claim is not None + ] + policies.append(policy.AnyOf(expected_build_configs)) + + return policy.AllOf(policies) + class GitLabPublisher(_PublisherBase): """A GitLab-based Trusted Publisher.""" @@ -412,8 +459,29 @@ class GitLabPublisher(_PublisherBase): The optional environment that the publishing action was performed from. """ + def _as_policy(self) -> VerificationPolicy: + policies: list[VerificationPolicy] = [ + policy.OIDCIssuerV2("https://gitlab.com"), + policy.OIDCSourceRepositoryURI(f"https://gitlab.com/{self.repository}"), + ] + + if not self.claims: + raise VerificationError("refusing to build a policy without claims") + + if ref := self.claims.get("ref"): + policies.append( + policy.OIDCBuildConfigURI( + f"https://gitlab.com/{self.repository}//.gitlab-ci.yml@{ref}" + ) + ) + else: + raise VerificationError("refusing to build a policy without a ref claim") + + return policy.AllOf(policies) + -Publisher = Annotated[GitHubPublisher | GitLabPublisher, Field(discriminator="kind")] +_Publisher = GitHubPublisher | GitLabPublisher +Publisher = Annotated[_Publisher, Field(discriminator="kind")] class AttestationBundle(BaseModel): From 9bbda10af49231bf10abfa4ac0b53a842bcd335c Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Fri, 11 Oct 2024 11:41:18 +0100 Subject: [PATCH 03/10] begin filling in coverage Signed-off-by: William Woodruff --- src/pypi_attestations/_impl.py | 2 +- test/test_impl.py | 38 ++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/src/pypi_attestations/_impl.py b/src/pypi_attestations/_impl.py index 1b5220e..80cd693 100644 --- a/src/pypi_attestations/_impl.py +++ b/src/pypi_attestations/_impl.py @@ -388,7 +388,7 @@ class _PublisherBase(BaseModel): def _as_policy(self) -> VerificationPolicy: """Return an appropriate `sigstore.policy.VerificationPolicy` for this publisher.""" - raise NotImplementedError + raise NotImplementedError # pragma: no cover class GitHubPublisher(_PublisherBase): diff --git a/test/test_impl.py b/test/test_impl.py index ad07864..25ca3bd 100644 --- a/test/test_impl.py +++ b/test/test_impl.py @@ -132,6 +132,44 @@ def test_verify_github_attested(self) -> None: assert predicate_type == "https://docs.pypi.org/attestations/publish/v1" assert predicate == {} + def test_verify_from_github_publisher(self) -> None: + publisher = impl.GitHubPublisher( + repository="trailofbits/pypi-attestation-models", + workflow="release.yml", + claims={"ref": "refs/tags/v0.0.4a2"}, + ) + + bundle = Bundle.from_json(gh_signed_dist_bundle_path.read_bytes()) + attestation = impl.Attestation.from_bundle(bundle) + + predicate_type, predicate = attestation.verify(publisher, gh_signed_dist) + assert predicate_type == "https://docs.pypi.org/attestations/publish/v1" + assert predicate == {} + + @pytest.mark.parametrize( + "claims", + ( + None, + {}, + {"something": "other"}, + {"ref": None}, + {"sha": None}, + {"ref": None, "sha": None}, + ), + ) + def test_verify_from_github_publisher_invalid_claims(self, claims: dict | None) -> None: + publisher = impl.GitHubPublisher( + repository="trailofbits/pypi-attestation-models", + workflow="release.yml", + claims=claims, + ) + + bundle = Bundle.from_json(gh_signed_dist_bundle_path.read_bytes()) + attestation = impl.Attestation.from_bundle(bundle) + + with pytest.raises(impl.VerificationError, match="refusing to build a policy"): + attestation.verify(publisher, gh_signed_dist) + def test_verify(self) -> None: # Our checked-in asset has this identity. pol = policy.Identity( From 3ef52733e8fa486fa1fa38ca923049c6ce3cb2bf Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Fri, 11 Oct 2024 11:49:40 +0100 Subject: [PATCH 04/10] test: fill in coverage Signed-off-by: William Woodruff --- test/test_impl.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/test_impl.py b/test/test_impl.py index 25ca3bd..831dd1d 100644 --- a/test/test_impl.py +++ b/test/test_impl.py @@ -559,6 +559,21 @@ def test_claims(self) -> None: } +class TestGitLabPublisher: + def test_as_policy(self) -> None: + publisher = impl.GitLabPublisher(repository="fake/fake", claims={"ref": "refs/heads/main"}) + pol: policy.AllOf = publisher._as_policy() # type: ignore[assignment] + + assert len(pol._children) == 3 + + @pytest.mark.parametrize("claims", [None, {}, {"something": "unrelated"}, {"ref": None}]) + def test_as_policy_invalid(self, claims: dict | None) -> None: + publisher = impl.GitLabPublisher(repository="fake/fake", claims=claims) + + with pytest.raises(impl.VerificationError, match="refusing to build a policy"): + publisher._as_policy() + + class TestProvenance: def test_version(self) -> None: attestation = impl.Attestation.model_validate_json(dist_attestation_path.read_bytes()) From 6d4fdf6da8a6b26ced0f6b131b6c09106dc15ca2 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Fri, 11 Oct 2024 16:42:24 +0100 Subject: [PATCH 05/10] do things the annoying way Signed-off-by: William Woodruff --- pyproject.toml | 1 + src/pypi_attestations/_impl.py | 85 ++++++++++++++++++++++++++-------- test/test_impl.py | 29 ++---------- 3 files changed, 70 insertions(+), 45 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index b4b0587..ff7310e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -12,6 +12,7 @@ classifiers = [ dependencies = [ "cryptography", "packaging", + "pyasn1 ~= 0.6", "pydantic", "sigstore~=3.4", "sigstore-protobuf-specs", diff --git a/src/pypi_attestations/_impl.py b/src/pypi_attestations/_impl.py index 80cd693..168265b 100644 --- a/src/pypi_attestations/_impl.py +++ b/src/pypi_attestations/_impl.py @@ -14,6 +14,8 @@ from cryptography import x509 from cryptography.hazmat.primitives import serialization from packaging.utils import parse_sdist_filename, parse_wheel_filename +from pyasn1.codec.der.decoder import decode as der_decode +from pyasn1.type.char import UTF8String from pydantic import Base64Encoder, BaseModel, ConfigDict, EncodedBytes, Field, field_validator from pydantic.alias_generators import to_snake from pydantic_core import ValidationError @@ -30,6 +32,7 @@ if TYPE_CHECKING: from pathlib import Path # pragma: no cover + from cryptography.x509 import Certificate from sigstore.sign import Signer # pragma: no cover from sigstore.verify.policy import VerificationPolicy # pragma: no cover @@ -391,6 +394,68 @@ def _as_policy(self) -> VerificationPolicy: raise NotImplementedError # pragma: no cover +class _GitHubTrustedPublisherPolicy: + """A custom sigstore-python policy for verifying against a GitHub-based Trusted Publisher.""" + + def __init__(self, repository: str, workflow: str) -> None: + self._repository = repository + self._workflow = workflow + + @classmethod + def _der_decode_utf8string(cls, der: bytes) -> str: + """Decode a DER-encoded UTF8String.""" + return der_decode(der, UTF8String)[0].decode() + + def verify(self, cert: Certificate) -> None: + """Foo.""" + + # This process has a few annoying steps, since a Trusted Publisher + # isn't aware of the commit or ref it runs on, while Sigstore's + # leaf certificate claims (like GitHub Actions' OIDC claims) only + # ever encode the workflow filename (which we need to check) next + # to the ref/sha (which we can't check). + # + # To get around this, we: + # (1) extract the `Build Config URI` extension; + # (2) extract the `Source Repository Digest` and + # `Source Repository Ref` extensions; + # (3) build the *expected* URI with the user-controlled + # Trusted Publisher identity *with* (2) + # (4) compare (1) with (3) + + # (1) Extract the build config URI, which looks like this: + # https://github.com/OWNER/REPO/.github/workflows/WORKFLOW@REF + # where OWNER/REPO and WORKFLOW are controlled by the TP identity, + # and REF is controlled by the certificate's own claims. + build_config_uri = cert.extensions.get_extension_for_oid(policy._OIDC_BUILD_CONFIG_URI_OID) # noqa: SLF001 + raw_build_config_uri = self._der_decode_utf8string(build_config_uri.value.value) + + # (2) Extract the source repo digest and ref. + source_repo_digest = cert.extensions.get_extension_for_oid( + policy._OIDC_BUILD_SIGNER_DIGEST_OID # noqa: SLF001 + ) + sha = self._der_decode_utf8string(source_repo_digest.value.value) + + source_repo_ref = cert.extensions.get_extension_for_oid( + policy._OIDC_SOURCE_REPOSITORY_REF_OID # noqa: SLF001 + ) + ref = self._der_decode_utf8string(source_repo_ref.value.value) + + # (3)-(4): Build the expected URIs and compare them + for suffix in [sha, ref]: + expected = ( + f"https://github.com/{self._repository}/.github/workflows/{self._workflow}@{suffix}" + ) + if raw_build_config_uri == expected: + return + + # If none of the expected URIs matched, the policy fails. + raise ValidationError( + f"Certificate's Build Config URI ({build_config_uri}) does not match expected " + f"Trusted Publisher ({self._workflow} @ {self._repository})" + ) + + class GitHubPublisher(_PublisherBase): """A GitHub-based Trusted Publisher.""" @@ -418,27 +483,9 @@ def _as_policy(self) -> VerificationPolicy: policies: list[VerificationPolicy] = [ policy.OIDCIssuerV2("https://token.actions.githubusercontent.com"), policy.OIDCSourceRepositoryURI(f"https://github.com/{self.repository}"), + _GitHubTrustedPublisherPolicy(self.repository, self.workflow), ] - if not self.claims: - raise VerificationError("refusing to build a policy without claims") - - sha = self.claims.get("sha") - ref = self.claims.get("ref") - if not (sha or ref): - # This should never happen, since we should always _at least_ have - # the `sha` claim. - raise VerificationError("refusing to build a policy without a sha or ref claim") - - expected_build_configs: list[VerificationPolicy] = [ - policy.OIDCBuildConfigURI( - f"https://github.com/{self.repository}/.github/workflows/{self.workflow}@{claim}" - ) - for claim in [ref, sha] - if claim is not None - ] - policies.append(policy.AnyOf(expected_build_configs)) - return policy.AllOf(policies) diff --git a/test/test_impl.py b/test/test_impl.py index 831dd1d..262d216 100644 --- a/test/test_impl.py +++ b/test/test_impl.py @@ -132,11 +132,12 @@ def test_verify_github_attested(self) -> None: assert predicate_type == "https://docs.pypi.org/attestations/publish/v1" assert predicate == {} - def test_verify_from_github_publisher(self) -> None: + @pytest.mark.parametrize("claims", (None, {}, {"ref": "refs/tags/v0.0.4a2"})) + def test_verify_from_github_publisher(self, claims: dict | None) -> None: publisher = impl.GitHubPublisher( repository="trailofbits/pypi-attestation-models", workflow="release.yml", - claims={"ref": "refs/tags/v0.0.4a2"}, + claims=claims, ) bundle = Bundle.from_json(gh_signed_dist_bundle_path.read_bytes()) @@ -146,30 +147,6 @@ def test_verify_from_github_publisher(self) -> None: assert predicate_type == "https://docs.pypi.org/attestations/publish/v1" assert predicate == {} - @pytest.mark.parametrize( - "claims", - ( - None, - {}, - {"something": "other"}, - {"ref": None}, - {"sha": None}, - {"ref": None, "sha": None}, - ), - ) - def test_verify_from_github_publisher_invalid_claims(self, claims: dict | None) -> None: - publisher = impl.GitHubPublisher( - repository="trailofbits/pypi-attestation-models", - workflow="release.yml", - claims=claims, - ) - - bundle = Bundle.from_json(gh_signed_dist_bundle_path.read_bytes()) - attestation = impl.Attestation.from_bundle(bundle) - - with pytest.raises(impl.VerificationError, match="refusing to build a policy"): - attestation.verify(publisher, gh_signed_dist) - def test_verify(self) -> None: # Our checked-in asset has this identity. pol = policy.Identity( From 2f1f15d3720d4e4624cc88254ebfe13808c64492 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Fri, 11 Oct 2024 16:44:04 +0100 Subject: [PATCH 06/10] _impl: cleanup Signed-off-by: William Woodruff --- src/pypi_attestations/_impl.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/pypi_attestations/_impl.py b/src/pypi_attestations/_impl.py index 168265b..84fc939 100644 --- a/src/pypi_attestations/_impl.py +++ b/src/pypi_attestations/_impl.py @@ -404,11 +404,10 @@ def __init__(self, repository: str, workflow: str) -> None: @classmethod def _der_decode_utf8string(cls, der: bytes) -> str: """Decode a DER-encoded UTF8String.""" - return der_decode(der, UTF8String)[0].decode() + return der_decode(der, UTF8String)[0].decode() # type: ignore[no-any-return] def verify(self, cert: Certificate) -> None: - """Foo.""" - + """Verify the certificate against the Trusted Publisher identity.""" # This process has a few annoying steps, since a Trusted Publisher # isn't aware of the commit or ref it runs on, while Sigstore's # leaf certificate claims (like GitHub Actions' OIDC claims) only @@ -428,18 +427,18 @@ def verify(self, cert: Certificate) -> None: # where OWNER/REPO and WORKFLOW are controlled by the TP identity, # and REF is controlled by the certificate's own claims. build_config_uri = cert.extensions.get_extension_for_oid(policy._OIDC_BUILD_CONFIG_URI_OID) # noqa: SLF001 - raw_build_config_uri = self._der_decode_utf8string(build_config_uri.value.value) + raw_build_config_uri = self._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_BUILD_SIGNER_DIGEST_OID # noqa: SLF001 ) - sha = self._der_decode_utf8string(source_repo_digest.value.value) + sha = self._der_decode_utf8string(source_repo_digest.value.public_bytes()) source_repo_ref = cert.extensions.get_extension_for_oid( policy._OIDC_SOURCE_REPOSITORY_REF_OID # noqa: SLF001 ) - ref = self._der_decode_utf8string(source_repo_ref.value.value) + ref = self._der_decode_utf8string(source_repo_ref.value.public_bytes()) # (3)-(4): Build the expected URIs and compare them for suffix in [sha, ref]: From 07c79113fc94db965ffc830781ba9241b4472928 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Fri, 11 Oct 2024 16:50:08 +0100 Subject: [PATCH 07/10] test error case Signed-off-by: William Woodruff --- src/pypi_attestations/_impl.py | 10 +++++----- test/test_impl.py | 12 ++++++++++++ 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/pypi_attestations/_impl.py b/src/pypi_attestations/_impl.py index 84fc939..9c05736 100644 --- a/src/pypi_attestations/_impl.py +++ b/src/pypi_attestations/_impl.py @@ -29,12 +29,12 @@ from sigstore_protobuf_specs.io.intoto import Envelope as _Envelope from sigstore_protobuf_specs.io.intoto import Signature as _Signature -if TYPE_CHECKING: - from pathlib import Path # pragma: no cover +if TYPE_CHECKING: # pragma: no cover + from pathlib import Path from cryptography.x509 import Certificate - from sigstore.sign import Signer # pragma: no cover - from sigstore.verify.policy import VerificationPolicy # pragma: no cover + from sigstore.sign import Signer + from sigstore.verify.policy import VerificationPolicy class Base64EncoderSansNewline(Base64Encoder): @@ -449,7 +449,7 @@ def verify(self, cert: Certificate) -> None: return # If none of the expected URIs matched, the policy fails. - raise ValidationError( + raise sigstore.errors.VerificationError( f"Certificate's Build Config URI ({build_config_uri}) does not match expected " f"Trusted Publisher ({self._workflow} @ {self._repository})" ) diff --git a/test/test_impl.py b/test/test_impl.py index 262d216..5fefdd5 100644 --- a/test/test_impl.py +++ b/test/test_impl.py @@ -147,6 +147,18 @@ def test_verify_from_github_publisher(self, claims: dict | None) -> None: assert predicate_type == "https://docs.pypi.org/attestations/publish/v1" assert predicate == {} + def test_verify_from_github_publisher_wrong(self) -> None: + publisher = impl.GitHubPublisher( + repository="trailofbits/pypi-attestation-models", + workflow="wrong.yml", + ) + + bundle = Bundle.from_json(gh_signed_dist_bundle_path.read_bytes()) + attestation = impl.Attestation.from_bundle(bundle) + + with pytest.raises(impl.VerificationError, match=r"Build Config URI .+ does not match"): + attestation.verify(publisher, gh_signed_dist) + def test_verify(self) -> None: # Our checked-in asset has this identity. pol = policy.Identity( From ae8d10986f9991fb42f6e1e94c9759075fa396b5 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Mon, 21 Oct 2024 16:53:23 -0400 Subject: [PATCH 08/10] Update src/pypi_attestations/_impl.py Co-authored-by: Facundo Tuesca --- src/pypi_attestations/_impl.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pypi_attestations/_impl.py b/src/pypi_attestations/_impl.py index 9c05736..7af1ca6 100644 --- a/src/pypi_attestations/_impl.py +++ b/src/pypi_attestations/_impl.py @@ -431,7 +431,7 @@ def verify(self, cert: Certificate) -> None: # (2) Extract the source repo digest and ref. source_repo_digest = cert.extensions.get_extension_for_oid( - policy._OIDC_BUILD_SIGNER_DIGEST_OID # noqa: SLF001 + policy._OIDC_SOURCE_REPOSITORY_DIGEST_OID # noqa: SLF001 ) sha = self._der_decode_utf8string(source_repo_digest.value.public_bytes()) From f133676ea94845ff6b21225a25480bc69c192e23 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Tue, 22 Oct 2024 08:55:26 -0400 Subject: [PATCH 09/10] move AllOf into _GitHubTrustedPublisherPolicy Signed-off-by: William Woodruff --- src/pypi_attestations/_impl.py | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/pypi_attestations/_impl.py b/src/pypi_attestations/_impl.py index 7af1ca6..0b94f60 100644 --- a/src/pypi_attestations/_impl.py +++ b/src/pypi_attestations/_impl.py @@ -400,6 +400,15 @@ class _GitHubTrustedPublisherPolicy: def __init__(self, repository: str, workflow: str) -> None: self._repository = repository self._workflow = workflow + # This policy must also satisfy some baseline underlying policies: + # the issuer must be GitHub Actions, and the repo must be the one + # we expect. + self._subpolicy = policy.AllOf( + [ + policy.OIDCIssuerV2("https://token.actions.githubusercontent.com"), + policy.OIDCSourceRepositoryURI(f"https://github.com/{self.repository}"), + ] + ) @classmethod def _der_decode_utf8string(cls, der: bytes) -> str: @@ -408,6 +417,8 @@ def _der_decode_utf8string(cls, der: bytes) -> str: def verify(self, cert: Certificate) -> None: """Verify the certificate against the Trusted Publisher identity.""" + self._subpolicy.verify() + # This process has a few annoying steps, since a Trusted Publisher # isn't aware of the commit or ref it runs on, while Sigstore's # leaf certificate claims (like GitHub Actions' OIDC claims) only @@ -479,13 +490,7 @@ class GitHubPublisher(_PublisherBase): """ def _as_policy(self) -> VerificationPolicy: - policies: list[VerificationPolicy] = [ - policy.OIDCIssuerV2("https://token.actions.githubusercontent.com"), - policy.OIDCSourceRepositoryURI(f"https://github.com/{self.repository}"), - _GitHubTrustedPublisherPolicy(self.repository, self.workflow), - ] - - return policy.AllOf(policies) + return _GitHubTrustedPublisherPolicy(self.repository, self.workflow) class GitLabPublisher(_PublisherBase): From c0d53504931c5bf8ea5024b549afdf9b8ddfa574 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Tue, 22 Oct 2024 08:57:35 -0400 Subject: [PATCH 10/10] fix tests Signed-off-by: William Woodruff --- src/pypi_attestations/_impl.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pypi_attestations/_impl.py b/src/pypi_attestations/_impl.py index 0b94f60..962c3db 100644 --- a/src/pypi_attestations/_impl.py +++ b/src/pypi_attestations/_impl.py @@ -406,7 +406,7 @@ def __init__(self, repository: str, workflow: str) -> None: self._subpolicy = policy.AllOf( [ policy.OIDCIssuerV2("https://token.actions.githubusercontent.com"), - policy.OIDCSourceRepositoryURI(f"https://github.com/{self.repository}"), + policy.OIDCSourceRepositoryURI(f"https://github.com/{self._repository}"), ] ) @@ -417,7 +417,7 @@ def _der_decode_utf8string(cls, der: bytes) -> str: def verify(self, cert: Certificate) -> None: """Verify the certificate against the Trusted Publisher identity.""" - self._subpolicy.verify() + self._subpolicy.verify(cert) # This process has a few annoying steps, since a Trusted Publisher # isn't aware of the commit or ref it runs on, while Sigstore's