Backport verified time changes (#1492)

Author: jku

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

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

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,15 +333,21 @@ 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
         "tsa/bundle.txt.late_timestamp.sigstore" was generated by adding `time.sleep(12*60)`
         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()),