Skip to content

Commit 5736314

Browse files
authored
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]>
1 parent 7f041da commit 5736314

File tree

4 files changed

+126
-9
lines changed

4 files changed

+126
-9
lines changed

CHANGELOG.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2121
`--offline` flag enforces that the distribution and provenance file arguments
2222
must be local file paths.
2323

24+
### Fixed
25+
26+
- Fixed a bug where `GitHubPublisher` policy verification would fail
27+
if the `Source Repository Ref` or `Source Repository Digest` claim
28+
was missing from the attestation's certificate. We require at least
29+
one of the two claims, but not necessarily both
30+
([#109](https://github.com/trailofbits/pypi-attestations/pull/109))
31+
2432
## [0.0.22]
2533

2634
### Changed

src/pypi_attestations/_impl.py

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -491,18 +491,31 @@ def verify(self, cert: Certificate) -> None:
491491
raw_build_config_uri = _der_decode_utf8string(build_config_uri.value.public_bytes())
492492

493493
# (2) Extract the source repo digest and ref.
494-
source_repo_digest = cert.extensions.get_extension_for_oid(
495-
policy._OIDC_SOURCE_REPOSITORY_DIGEST_OID # noqa: SLF001
496-
)
497-
sha = _der_decode_utf8string(source_repo_digest.value.public_bytes())
494+
# We require at least one of these to be present.
495+
suffixes = []
496+
try:
497+
source_repo_digest = cert.extensions.get_extension_for_oid(
498+
policy._OIDC_SOURCE_REPOSITORY_DIGEST_OID # noqa: SLF001
499+
)
500+
suffixes.append(_der_decode_utf8string(source_repo_digest.value.public_bytes()))
501+
except x509.ExtensionNotFound:
502+
pass
498503

499-
source_repo_ref = cert.extensions.get_extension_for_oid(
500-
policy._OIDC_SOURCE_REPOSITORY_REF_OID # noqa: SLF001
501-
)
502-
ref = _der_decode_utf8string(source_repo_ref.value.public_bytes())
504+
try:
505+
source_repo_ref = cert.extensions.get_extension_for_oid(
506+
policy._OIDC_SOURCE_REPOSITORY_REF_OID # noqa: SLF001
507+
)
508+
suffixes.append(_der_decode_utf8string(source_repo_ref.value.public_bytes()))
509+
except x509.ExtensionNotFound:
510+
pass
511+
512+
if not suffixes:
513+
raise sigstore.errors.VerificationError(
514+
"Certificate must contain either Source Repository Digest or Source Repository Ref"
515+
)
503516

504517
# (3)-(4): Build the expected URIs and compare them
505-
for suffix in [sha, ref]:
518+
for suffix in suffixes:
506519
expected = (
507520
f"https://github.com/{self._repository}/.github/workflows/{self._workflow}@{suffix}"
508521
)
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
See: https://search.sigstore.dev/?logIndex=191974551
2+
3+
-----BEGIN CERTIFICATE-----
4+
MIIG7DCCBnKgAwIBAgIUJrNb177y4TSfo1t8/miMosKaxpwwCgYIKoZIzj0EAwMw
5+
NzEVMBMGA1UEChMMc2lnc3RvcmUuZGV2MR4wHAYDVQQDExVzaWdzdG9yZS1pbnRl
6+
cm1lZGlhdGUwHhcNMjUwNDAzMTUxMzA0WhcNMjUwNDAzMTUyMzA0WjAAMFkwEwYH
7+
KoZIzj0CAQYIKoZIzj0DAQcDQgAEgQxA014C9CF5Zi/SXcIEVLO/EX5UkvR6yL4s
8+
sHUCXGA3YHvLtTMm8DmJlezko3fWtA+91qDuVGt5cA4BMoowSKOCBZEwggWNMA4G
9+
A1UdDwEB/wQEAwIHgDATBgNVHSUEDDAKBggrBgEFBQcDAzAdBgNVHQ4EFgQUwWgy
10+
8G1LtCB3aaiQPvkewDCCvBEwHwYDVR0jBBgwFoAU39Ppz1YkEZb5qNjpKFWixi4Y
11+
ZD8wgYMGA1UdEQEB/wR5MHeGdWh0dHBzOi8vZ2l0aHViLmNvbS9TV0lGVFNJTS9z
12+
d2lmdGdhbGF4eS8uZ2l0aHViL3dvcmtmbG93cy9weXRob24tcHVibGlzaC55bWxA
13+
YTI2ZWIwMDZkMDRkZDBjNDFhNmUzOGRjMTVkZGFmN2I0NjE0MzUwNTA5BgorBgEE
14+
AYO/MAEBBCtodHRwczovL3Rva2VuLmFjdGlvbnMuZ2l0aHVidXNlcmNvbnRlbnQu
15+
Y29tMBUGCisGAQQBg78wAQIEB3JlbGVhc2UwNgYKKwYBBAGDvzABAwQoYTI2ZWIw
16+
MDZkMDRkZDBjNDFhNmUzOGRjMTVkZGFmN2I0NjE0MzUwNTAjBgorBgEEAYO/MAEE
17+
BBVVcGxvYWQgUHl0aG9uIFBhY2thZ2UwIgYKKwYBBAGDvzABBQQUU1dJRlRTSU0v
18+
c3dpZnRnYWxheHkwOwYKKwYBBAGDvzABCAQtDCtodHRwczovL3Rva2VuLmFjdGlv
19+
bnMuZ2l0aHVidXNlcmNvbnRlbnQuY29tMIGFBgorBgEEAYO/MAEJBHcMdWh0dHBz
20+
Oi8vZ2l0aHViLmNvbS9TV0lGVFNJTS9zd2lmdGdhbGF4eS8uZ2l0aHViL3dvcmtm
21+
bG93cy9weXRob24tcHVibGlzaC55bWxAYTI2ZWIwMDZkMDRkZDBjNDFhNmUzOGRj
22+
MTVkZGFmN2I0NjE0MzUwNTA4BgorBgEEAYO/MAEKBCoMKGEyNmViMDA2ZDA0ZGQw
23+
YzQxYTZlMzhkYzE1ZGRhZjdiNDYxNDM1MDUwHQYKKwYBBAGDvzABCwQPDA1naXRo
24+
dWItaG9zdGVkMDcGCisGAQQBg78wAQwEKQwnaHR0cHM6Ly9naXRodWIuY29tL1NX
25+
SUZUU0lNL3N3aWZ0Z2FsYXh5MDgGCisGAQQBg78wAQ0EKgwoYTI2ZWIwMDZkMDRk
26+
ZDBjNDFhNmUzOGRjMTVkZGFmN2I0NjE0MzUwNTAZBgorBgEEAYO/MAEPBAsMCTQ4
27+
ODI3MTc5NTArBgorBgEEAYO/MAEQBB0MG2h0dHBzOi8vZ2l0aHViLmNvbS9TV0lG
28+
VFNJTTAYBgorBgEEAYO/MAERBAoMCDM3NTQxMzA5MIGFBgorBgEEAYO/MAESBHcM
29+
dWh0dHBzOi8vZ2l0aHViLmNvbS9TV0lGVFNJTS9zd2lmdGdhbGF4eS8uZ2l0aHVi
30+
L3dvcmtmbG93cy9weXRob24tcHVibGlzaC55bWxAYTI2ZWIwMDZkMDRkZDBjNDFh
31+
NmUzOGRjMTVkZGFmN2I0NjE0MzUwNTA4BgorBgEEAYO/MAETBCoMKGEyNmViMDA2
32+
ZDA0ZGQwYzQxYTZlMzhkYzE1ZGRhZjdiNDYxNDM1MDUwFwYKKwYBBAGDvzABFAQJ
33+
DAdyZWxlYXNlMFsGCisGAQQBg78wARUETQxLaHR0cHM6Ly9naXRodWIuY29tL1NX
34+
SUZUU0lNL3N3aWZ0Z2FsYXh5L2FjdGlvbnMvcnVucy8xNDI0NTc0MzgxMi9hdHRl
35+
bXB0cy8yMBYGCisGAQQBg78wARYECAwGcHVibGljMIGJBgorBgEEAdZ5AgQCBHsE
36+
eQB3AHUA3T0wasbHETJjGR4cmWc3AqJKXrjePK3/h4pygC8p7o4AAAGV/DZ++wAA
37+
BAMARjBEAiAGvZn6Blv/M5qoz++/bkL2y5yM1Wad5aGNwMPRY+FoUwIgFiETf+Pp
38+
sbLE1Jm6TzHxgPoq9zBM24kO3DPFzG3tWrYwCgYIKoZIzj0EAwMDaAAwZQIwby+C
39+
egP77F5410AdZvLwHz+7qOBUmVJxr/c7e+4qvL1v8jMHs0+z+gEVOaiNAfr5AjEA
40+
ntFI0uzl76l0QslAefj7HiFMt1nn3Qryz1wA17U1l25rRVz8H4kfDcJ5/6i2mH5O
41+
-----END CERTIFICATE-----

test/test_impl.py

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@
99
import pretend
1010
import pytest
1111
import sigstore
12+
import sigstore.errors
13+
from cryptography import x509
14+
from cryptography.hazmat.primitives import hashes
15+
from cryptography.hazmat.primitives.asymmetric import ec
1216
from pydantic import Base64Bytes, BaseModel, TypeAdapter, ValidationError
1317
from sigstore.dsse import DigestSet, StatementBuilder, Subject
1418
from sigstore.models import Bundle
@@ -662,3 +666,54 @@ class TestBase64Bytes:
662666
def test_encoding(self) -> None:
663667
model = DummyModel(base64_bytes=b"aaaa" * 76)
664668
assert "\\n" not in model.model_dump_json()
669+
670+
671+
class TestGitHubublisher:
672+
def test_verifies_cert_with_missing_ref(self) -> None:
673+
cert_path = _ASSETS / "no-source-repository-ref-extension.pem"
674+
cert = x509.load_pem_x509_certificate(cert_path.read_bytes())
675+
676+
publisher = impl.GitHubPublisher(
677+
repository="SWIFTSIM/swiftgalaxy",
678+
workflow="python-publish.yml",
679+
)
680+
681+
publisher._as_policy().verify(cert)
682+
683+
def test_fails_cert_with_no_digest_or_ref(self) -> None:
684+
# To test this, we manually mangle a certificate
685+
# to remove the digest extension. This ends up not being a valid
686+
# certificate from an attestation perspective (since we replace
687+
# the signature as well), but it's sufficient for the policy test.
688+
689+
cert_path = _ASSETS / "no-source-repository-ref-extension.pem"
690+
orig_cert = x509.load_pem_x509_certificate(cert_path.read_bytes())
691+
692+
# Rebuild the certificate, but with the digest extension removed
693+
builder = (
694+
x509.CertificateBuilder()
695+
.subject_name(orig_cert.subject)
696+
.issuer_name(orig_cert.issuer)
697+
.public_key(orig_cert.public_key())
698+
.serial_number(orig_cert.serial_number)
699+
.not_valid_before(orig_cert.not_valid_before)
700+
.not_valid_after(orig_cert.not_valid_after)
701+
)
702+
703+
for ext in orig_cert.extensions:
704+
if ext.oid != policy._OIDC_SOURCE_REPOSITORY_DIGEST_OID:
705+
builder = builder.add_extension(ext.value, ext.critical)
706+
707+
cert = builder.sign(ec.generate_private_key(ec.SECP256R1()), hashes.SHA256())
708+
709+
publisher = impl.GitHubPublisher(
710+
repository="SWIFTSIM/swiftgalaxy",
711+
workflow="python-publish.yml",
712+
)
713+
with pytest.raises(
714+
sigstore.errors.VerificationError,
715+
match=(
716+
"Certificate must contain either Source Repository Digest or Source Repository Ref"
717+
),
718+
):
719+
publisher._as_policy().verify(cert)

0 commit comments

Comments
 (0)