From 600c5fecf621915f54b52c49b0dafe38ae250c80 Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Wed, 6 Aug 2025 14:45:12 +0300 Subject: [PATCH 1/4] Clean up verified time handling (#1489) * 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 * 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 --------- Signed-off-by: Jussi Kukkonen --- sigstore/verify/verifier.py | 28 ++++++++++----------- test/unit/verify/test_verifier.py | 42 +++++++++++++++++++++++++------ 2 files changed, 47 insertions(+), 23 deletions(-) diff --git a/sigstore/verify/verifier.py b/sigstore/verify/verifier.py index b782f969c..331277a62 100644 --- a/sigstore/verify/verifier.py +++ b/sigstore/verify/verifier.py @@ -59,9 +59,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: @@ -226,13 +226,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 @@ -243,6 +236,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, @@ -331,13 +330,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 verified times 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) < 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 a02217811..8d3491af5 100644 --- a/test/unit/verify/test_verifier.py +++ b/test/unit/verify/test_verifier.py @@ -205,7 +205,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.VERIFIED_TIME_THRESHOLD", 2) + verifier.verify_artifact( asset("tsa/bundle.txt").read_bytes(), Bundle.from_json(asset("tsa/bundle.txt.sigstore").read_bytes()), @@ -260,15 +264,21 @@ def test_verifier_no_validity(self, caplog, 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.VERIFIED_TIME_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()), @@ -283,13 +293,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.VERIFIED_TIME_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()), @@ -309,7 +325,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 @@ -317,7 +333,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.VERIFIED_TIME_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( @@ -334,8 +356,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.VERIFIED_TIME_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 d416cbdb29a40714fd94328cd90a231abdb0d3c4 Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Wed, 6 Aug 2025 14:49:15 +0300 Subject: [PATCH 2/4] verifier: Remove unnecessary check This check came with the backport of verified time fix, but it is not useful here since we only support 0.0.1 entry types (and is problematic since the LogEntry does not have the required fields here) Signed-off-by: Jussi Kukkonen --- sigstore/verify/verifier.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/sigstore/verify/verifier.py b/sigstore/verify/verifier.py index 331277a62..488364b8a 100644 --- a/sigstore/verify/verifier.py +++ b/sigstore/verify/verifier.py @@ -236,12 +236,6 @@ 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, From db9b81e54b8e2d234c84fdcdf7d7b2aad7141d45 Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Wed, 6 Aug 2025 14:58:50 +0300 Subject: [PATCH 3/4] tests: Update test for verified time changes This test no longer exists in main branch and the expected result has changed (valid_for.end is optional). In 3.6.x we want to keep testing the same thing we used to, so set VERIFIED_TIME_THRESHOLD = 2, meaning both integrated time and timestamp are needed (but expect timestamp to not be used since valid_for.end is not set) Signed-off-by: Jussi Kukkonen --- test/unit/verify/test_verifier.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/test/unit/verify/test_verifier.py b/test/unit/verify/test_verifier.py index 8d3491af5..9383b36ef 100644 --- a/test/unit/verify/test_verifier.py +++ b/test/unit/verify/test_verifier.py @@ -245,13 +245,21 @@ def test_verifier_duplicate_timestamp(self, verifier, asset, null_policy): null_policy, ) - def test_verifier_no_validity(self, caplog, verifier, asset, null_policy): + def test_verifier_no_validity( + 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.VERIFIED_TIME_THRESHOLD", 2) + verifier._trusted_root.get_timestamp_authorities()[ 0 ]._inner.valid_for.end = None 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()), From fbbbe5cef0025d575527876f50df21b16a529663 Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Wed, 6 Aug 2025 15:15:23 +0300 Subject: [PATCH 4/4] CHANGELOG: Add entry for verified time fixes Signed-off-by: Jussi Kukkonen --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e04bab02b..ff953acaf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,11 @@ All versions prior to 0.9.0 are untracked. ## [Unreleased] +### Fixed + +* Fixed verified time handling so that additional timestamps cannot break + otherwise valid signature bundles ([#1492](https://github.com/sigstore/sigstore-python/pull/1492)) + ## [3.6.4] ### Fixed