Skip to content

Commit 3adc3d4

Browse files
authored
verifier: Fix TSA cert chain construction (#1482)
* verifier: Fix TSA cert chain construction Using add_root_certificate() multiple times looks like a bug: it usually works but in the case of timestamps with no embedded certs verification seems to fails with Certificates neither found in the answer or in the Verification Options Signed-off-by: Jussi Kukkonen <[email protected]> * verify: avoid modifying the cert list We don't need to modify the list so let's avoid it to make it easier to review. Signed-off-by: Jussi Kukkonen <[email protected]> * tests: Add regression test for timestamp without certs Signed-off-by: Jussi Kukkonen <[email protected]> --------- Signed-off-by: Jussi Kukkonen <[email protected]>
1 parent a65383b commit 3adc3d4

File tree

4 files changed

+27
-3
lines changed

4 files changed

+27
-3
lines changed

sigstore/verify/verifier.py

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -128,9 +128,18 @@ def _verify_signed_timestamp(
128128
for certificate_authority in cert_authorities:
129129
certificates = certificate_authority.certificates(allow_expired=True)
130130

131-
builder = VerifierBuilder()
132-
for certificate in certificates:
133-
builder.add_root_certificate(certificate)
131+
# We expect at least a signing cert and a root cert but there may be intermediates
132+
if len(certificates) < 2:
133+
_logger.debug("Unable to verify Timestamp: cert chain is incomplete")
134+
continue
135+
136+
builder = (
137+
VerifierBuilder()
138+
.tsa_certificate(certificates[0])
139+
.add_root_certificate(certificates[-1])
140+
)
141+
for certificate in certificates[1:-1]:
142+
builder = builder.add_intermediate_certificate(certificate)
134143

135144
verifier = builder.build()
136145
try:

test/assets/tsa/issue1482-message

72 Bytes
Binary file not shown.
728 Bytes
Binary file not shown.

test/unit/verify/test_verifier.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -377,3 +377,18 @@ def test_verifier_not_enough_timestamp(
377377
Bundle.from_json(asset("tsa/bundle.txt.sigstore").read_bytes()),
378378
null_policy,
379379
)
380+
381+
def test_verify_signed_timestamp_regression(self, asset):
382+
"""
383+
Ensure we correctly verify a timestamp with no embedded certs.
384+
385+
This is a regression test for # 1482
386+
"""
387+
verifier = Verifier.staging(offline=True)
388+
ts = rfc3161_client.decode_timestamp_response(
389+
asset("tsa/issue1482-timestamp-with-no-cert").read_bytes()
390+
)
391+
res = verifier._verify_signed_timestamp(
392+
ts, asset("tsa/issue1482-message").read_bytes()
393+
)
394+
assert res is not None

0 commit comments

Comments
 (0)