From f638e6d7f97ad427b67cb39c2533d8c81e14d281 Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Wed, 30 Jul 2025 16:34:50 +0300 Subject: [PATCH 1/2] Clean verified time handling Try to handle TSA timestamps and rekor v1 integrated time in a sensible manner: * no special cases for when TSA timestamps are present * require one verified time by default * Only allow integrated time to be a verified time if entry is from rekor v1 * VERIFY_TIMESTAMP_THRESHOLD now refers to "number of verified times", not just TSA timestamps * Tests use a rekor v1 bundle but expect it to be invalid if the timestamp is invalid -- but the integrated time is enough. Fix this by monkeypatching VERIFY_TIMESTAMP_THRESHOLD Signed-off-by: Jussi Kukkonen --- sigstore/verify/verifier.py | 22 ++++++++-------- test/unit/verify/test_verifier.py | 42 +++++++++++++++++++++++++------ 2 files changed, 44 insertions(+), 20 deletions(-) diff --git a/sigstore/verify/verifier.py b/sigstore/verify/verifier.py index 336ddb7a..00e6a8cd 100644 --- a/sigstore/verify/verifier.py +++ b/sigstore/verify/verifier.py @@ -215,13 +215,6 @@ def _establish_time(self, bundle: Bundle) -> list[TimestampVerificationResult]: raise VerificationError(msg) timestamp_from_tsa = self._verify_timestamp_authority(bundle) - if len(timestamp_from_tsa) < VERIFY_TIMESTAMP_THRESHOLD: - msg = ( - f"not enough timestamps validated to meet the validation " - f"threshold ({len(timestamp_from_tsa)}/{VERIFY_TIMESTAMP_THRESHOLD})" - ) - raise VerificationError(msg) - verified_timestamps.extend(timestamp_from_tsa) # If a timestamp from the Transparency Service is available, the Verifier MUST @@ -232,6 +225,12 @@ def _establish_time(self, bundle: Bundle) -> list[TimestampVerificationResult]: if ( timestamp := bundle.log_entry.integrated_time ) and bundle.log_entry.inclusion_promise: + kv = bundle.log_entry._kind_version + if not (kv.kind in ["dsse", "hashedrekord"] and kv.version == "0.0.1"): + raise VerificationError( + "Integrated time only supported for dsse/hashedrekord 0.0.1 types" + ) + verified_timestamps.append( TimestampVerificationResult( source=TimestampSource.TRANSPARENCY_SERVICE, @@ -320,13 +319,12 @@ def _verify_common_signing_cert( store.add_cert(parent_cert_ossl) # (0): Establishing a Time for the Signature - # First, establish a time for the signature. This timestamp is required to + # First, establish a time for the signature. This is required to # validate the certificate chain, so this step comes first. - # While this step is optional and only performed if timestamp data has been - # provided within the bundle, providing a signed timestamp without a TSA to - # verify it result in a VerificationError. + # These include TSA timestamps and (in the case of rekor v1 entries) + # rekor log integrated time. verified_timestamps = self._establish_time(bundle) - if not verified_timestamps: + if len(verified_timestamps) < VERIFY_TIMESTAMP_THRESHOLD: raise VerificationError("not enough sources of verified time") # (1): verify that the signing certificate is signed by the root diff --git a/test/unit/verify/test_verifier.py b/test/unit/verify/test_verifier.py index 7d1ebda9..802518aa 100644 --- a/test/unit/verify/test_verifier.py +++ b/test/unit/verify/test_verifier.py @@ -219,7 +219,11 @@ def verifier(self, asset) -> Verifier: verifier._trusted_root._inner.timestamp_authorities = [authority._inner] return verifier - def test_verifier_verify_timestamp(self, verifier, asset, null_policy): + def test_verifier_verify_timestamp(self, verifier, asset, null_policy, monkeypatch): + # asset is a rekor v1 bundle: set threshold to 2 so both integrated time and the + # TSA timestamp are required + monkeypatch.setattr("sigstore.verify.verifier.VERIFY_TIMESTAMP_THRESHOLD", 2) + verifier.verify_artifact( asset("tsa/bundle.txt").read_bytes(), Bundle.from_json(asset("tsa/bundle.txt.sigstore").read_bytes()), @@ -296,15 +300,21 @@ def test_verifier_duplicate_timestamp(self, verifier, asset, null_policy): ) def test_verifier_outside_validity_range( - self, caplog, verifier, asset, null_policy + self, caplog, verifier, asset, null_policy, monkeypatch ): + # asset is a rekor v1 bundle: set threshold to 2 so both integrated time and the + # TSA timestamp are required + monkeypatch.setattr("sigstore.verify.verifier.VERIFY_TIMESTAMP_THRESHOLD", 2) + # Set a date before the timestamp range verifier._trusted_root.get_timestamp_authorities()[ 0 ]._inner.valid_for.end = datetime(2024, 10, 31, tzinfo=timezone.utc) with caplog.at_level(logging.DEBUG, logger="sigstore.verify.verifier"): - with pytest.raises(VerificationError, match="not enough timestamps"): + with pytest.raises( + VerificationError, match="not enough sources of verified time" + ): verifier.verify_artifact( asset("tsa/bundle.txt").read_bytes(), Bundle.from_json(asset("tsa/bundle.txt.sigstore").read_bytes()), @@ -319,13 +329,19 @@ def test_verifier_outside_validity_range( def test_verifier_rfc3161_error( self, verifier, asset, null_policy, caplog, monkeypatch ): + # asset is a rekor v1 bundle: set threshold to 2 so both integrated time and the + # TSA timestamp are required + monkeypatch.setattr("sigstore.verify.verifier.VERIFY_TIMESTAMP_THRESHOLD", 2) + def verify_function(*args): raise rfc3161_client.VerificationError() monkeypatch.setattr(rfc3161_client.verify._Verifier, "verify", verify_function) with caplog.at_level(logging.DEBUG, logger="sigstore.verify.verifier"): - with pytest.raises(VerificationError, match="not enough timestamps"): + with pytest.raises( + VerificationError, match="not enough sources of verified time" + ): verifier.verify_artifact( asset("tsa/bundle.txt").read_bytes(), Bundle.from_json(asset("tsa/bundle.txt.sigstore").read_bytes()), @@ -345,7 +361,7 @@ def test_verifier_no_authorities(self, asset, null_policy): null_policy, ) - def test_late_timestamp(self, caplog, verifier, asset, null_policy): + def test_late_timestamp(self, caplog, verifier, asset, null_policy, monkeypatch): """ Ensures that verifying the signing certificate fails because the timestamp is outside the certificate's validity window. The sample bundle @@ -353,7 +369,13 @@ def test_late_timestamp(self, caplog, verifier, asset, null_policy): into `sigstore.sign.Signer._finalize_sign()`, just after the entry is posted to Rekor but before the timestamp is requested. """ - with pytest.raises(VerificationError, match="not enough timestamps"): + # asset is a rekor v1 bundle: set threshold to 2 so both integrated time and the + # TSA timestamp are required + monkeypatch.setattr("sigstore.verify.verifier.VERIFY_TIMESTAMP_THRESHOLD", 2) + + with pytest.raises( + VerificationError, match="not enough sources of verified time" + ): verifier.verify_artifact( asset("tsa/bundle.txt").read_bytes(), Bundle.from_json( @@ -370,8 +392,12 @@ def test_late_timestamp(self, caplog, verifier, asset, null_policy): def test_verifier_not_enough_timestamp( self, verifier, asset, null_policy, monkeypatch ): - monkeypatch.setattr("sigstore.verify.verifier.VERIFY_TIMESTAMP_THRESHOLD", 2) - with pytest.raises(VerificationError, match="not enough timestamps"): + # asset is a rekor v1 bundle: set threshold to 3 so integrated time and one + # TSA timestamp are not enough + monkeypatch.setattr("sigstore.verify.verifier.VERIFY_TIMESTAMP_THRESHOLD", 3) + with pytest.raises( + VerificationError, match="not enough sources of verified time" + ): verifier.verify_artifact( asset("tsa/bundle.txt").read_bytes(), Bundle.from_json(asset("tsa/bundle.txt.sigstore").read_bytes()), From 99ef4b80e8956c47e6dcda7626b173418112194c Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Tue, 5 Aug 2025 11:13:37 +0300 Subject: [PATCH 2/2] verify: Rename VERIFY_TIMESTAMP_THRESHOLD VERIFIED_TIME_THRESHOLD makes more sense since integrated time is also in this threshold. Strictly speaking this is an API change but since the meaning has (slightly) changed already that makes sense. Signed-off-by: Jussi Kukkonen --- sigstore/verify/verifier.py | 10 +++++----- test/unit/verify/test_verifier.py | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/sigstore/verify/verifier.py b/sigstore/verify/verifier.py index 00e6a8cd..9347e77b 100644 --- a/sigstore/verify/verifier.py +++ b/sigstore/verify/verifier.py @@ -62,9 +62,9 @@ # From https://github.com/sigstore/sigstore-go/blob/e92142f0734064ebf6001f188b7330a1212245fe/pkg/verify/tsa.go#L29 MAX_ALLOWED_TIMESTAMP: int = 32 -# When verifying a timestamp, this threshold represents the minimum number of required -# timestamps to consider a signature valid. -VERIFY_TIMESTAMP_THRESHOLD: int = 1 +# When verifying an entry, this threshold represents the minimum number of required +# verified times to consider a signature valid. +VERIFIED_TIME_THRESHOLD: int = 1 class Verifier: @@ -319,12 +319,12 @@ def _verify_common_signing_cert( store.add_cert(parent_cert_ossl) # (0): Establishing a Time for the Signature - # First, establish a time for the signature. This is required to + # First, establish verified times for the signature. This is required to # validate the certificate chain, so this step comes first. # These include TSA timestamps and (in the case of rekor v1 entries) # rekor log integrated time. verified_timestamps = self._establish_time(bundle) - if len(verified_timestamps) < VERIFY_TIMESTAMP_THRESHOLD: + if len(verified_timestamps) < VERIFIED_TIME_THRESHOLD: raise VerificationError("not enough sources of verified time") # (1): verify that the signing certificate is signed by the root diff --git a/test/unit/verify/test_verifier.py b/test/unit/verify/test_verifier.py index 802518aa..16df1d88 100644 --- a/test/unit/verify/test_verifier.py +++ b/test/unit/verify/test_verifier.py @@ -222,7 +222,7 @@ def verifier(self, asset) -> Verifier: def test_verifier_verify_timestamp(self, verifier, asset, null_policy, monkeypatch): # asset is a rekor v1 bundle: set threshold to 2 so both integrated time and the # TSA timestamp are required - monkeypatch.setattr("sigstore.verify.verifier.VERIFY_TIMESTAMP_THRESHOLD", 2) + monkeypatch.setattr("sigstore.verify.verifier.VERIFIED_TIME_THRESHOLD", 2) verifier.verify_artifact( asset("tsa/bundle.txt").read_bytes(), @@ -304,7 +304,7 @@ def test_verifier_outside_validity_range( ): # asset is a rekor v1 bundle: set threshold to 2 so both integrated time and the # TSA timestamp are required - monkeypatch.setattr("sigstore.verify.verifier.VERIFY_TIMESTAMP_THRESHOLD", 2) + monkeypatch.setattr("sigstore.verify.verifier.VERIFIED_TIME_THRESHOLD", 2) # Set a date before the timestamp range verifier._trusted_root.get_timestamp_authorities()[ @@ -331,7 +331,7 @@ def test_verifier_rfc3161_error( ): # asset is a rekor v1 bundle: set threshold to 2 so both integrated time and the # TSA timestamp are required - monkeypatch.setattr("sigstore.verify.verifier.VERIFY_TIMESTAMP_THRESHOLD", 2) + monkeypatch.setattr("sigstore.verify.verifier.VERIFIED_TIME_THRESHOLD", 2) def verify_function(*args): raise rfc3161_client.VerificationError() @@ -371,7 +371,7 @@ def test_late_timestamp(self, caplog, verifier, asset, null_policy, monkeypatch) """ # asset is a rekor v1 bundle: set threshold to 2 so both integrated time and the # TSA timestamp are required - monkeypatch.setattr("sigstore.verify.verifier.VERIFY_TIMESTAMP_THRESHOLD", 2) + monkeypatch.setattr("sigstore.verify.verifier.VERIFIED_TIME_THRESHOLD", 2) with pytest.raises( VerificationError, match="not enough sources of verified time" @@ -394,7 +394,7 @@ def test_verifier_not_enough_timestamp( ): # asset is a rekor v1 bundle: set threshold to 3 so integrated time and one # TSA timestamp are not enough - monkeypatch.setattr("sigstore.verify.verifier.VERIFY_TIMESTAMP_THRESHOLD", 3) + monkeypatch.setattr("sigstore.verify.verifier.VERIFIED_TIME_THRESHOLD", 3) with pytest.raises( VerificationError, match="not enough sources of verified time" ):