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 <[email protected]>

* 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 <[email protected]>

---------

Signed-off-by: Jussi Kukkonen <[email protected]>

Author: jku

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:
@@ -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 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

@@ -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.VERIFIED_TIME_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.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()),
@@ -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.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()),
@@ -345,15 +361,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(
@@ -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.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()),