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 diff --git a/sigstore/verify/verifier.py b/sigstore/verify/verifier.py index b782f969c..488364b8a 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 @@ -331,13 +324,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..9383b36ef 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()), @@ -241,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()), @@ -260,15 +272,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 +301,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 +333,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 +341,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 +364,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()),