Fail timestamp verification if no root is provided (#3224)

* Fail timestamp verification if no root is provided The bug was that we would conditionally check a timestamp if a root was provided. If no root was provided even if a timestamp was provided, then signature verification would succeed. The good news is this will not show a successful signature if the transparency log does not contain the entry too, for timestamp verification. Signed-off-by: Hayden Blauzvern <[email protected]> * Switch to switch Signed-off-by: Hayden Blauzvern <[email protected]> * Fix e2e test Signed-off-by: Hayden Blauzvern <[email protected]> * Fix e2e test Signed-off-by: Hayden Blauzvern <[email protected]> --------- Signed-off-by: Hayden Blauzvern <[email protected]>
sigstore · Sep 8, 2023 · 0d48aa1 · 0d48aa1
1 parent 29396d6
commit 0d48aa1
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 14 deletions.
diff --git a/pkg/cosign/verify.go b/pkg/cosign/verify.go
@@ -648,14 +648,12 @@ func verifyInternal(ctx context.Context, sig oci.Signature, h v1.Hash,
 	bundleVerified bool, err error) {
 	var acceptableRFC3161Time, acceptableRekorBundleTime *time.Time // Timestamps for the signature we accept, or nil if not applicable.
 
-	if co.TSARootCertificates != nil {
-		acceptableRFC3161Timestamp, err := VerifyRFC3161Timestamp(sig, co)
-		if err != nil {
-			return false, fmt.Errorf("unable to verify RFC3161 timestamp bundle: %w", err)
-		}
-		if acceptableRFC3161Timestamp != nil {
-			acceptableRFC3161Time = &acceptableRFC3161Timestamp.Time
-		}
+	acceptableRFC3161Timestamp, err := VerifyRFC3161Timestamp(sig, co)
+	if err != nil {
+		return false, fmt.Errorf("unable to verify RFC3161 timestamp bundle: %w", err)
+	}
+	if acceptableRFC3161Timestamp != nil {
+		acceptableRFC3161Time = &acceptableRFC3161Timestamp.Time
 	}
 
 	if !co.IgnoreTlog {
@@ -1099,13 +1097,17 @@ func VerifyBundle(sig oci.Signature, co *CheckOpts) (bool, error) {
 
 // VerifyRFC3161Timestamp verifies that the timestamp in sig is correctly signed, and if so,
 // returns the timestamp value.
-// It returns (nil, nil) if there is no timestamp, or (nil, err) if there is an invalid timestamp.
+// It returns (nil, nil) if there is no timestamp, or (nil, err) if there is an invalid timestamp or if
+// no root is provided with a timestamp.
 func VerifyRFC3161Timestamp(sig oci.Signature, co *CheckOpts) (*timestamp.Timestamp, error) {
 	ts, err := sig.RFC3161Timestamp()
-	if err != nil {
+	switch {
+	case err != nil:
 		return nil, err
-	} else if ts == nil {
+	case ts == nil:
 		return nil, nil
+	case co.TSARootCertificates == nil:
+		return nil, errors.New("no TSA root certificate(s) provided to verify timestamp")
 	}
 
 	b64Sig, err := sig.Base64Signature()

diff --git a/pkg/cosign/verify_test.go b/pkg/cosign/verify_test.go
@@ -1420,4 +1420,13 @@ func TestVerifyRFC3161Timestamp(t *testing.T) {
 	if err == nil || !strings.Contains(err.Error(), "hashed messages don't match") {
 		t.Fatalf("expected error verifying mismatched signatures, got: %v", err)
 	}
+
+	// failure without root certificate
+	_, err = VerifyRFC3161Timestamp(ociSig, &CheckOpts{
+		TSACertificate:              leaves[0],
+		TSAIntermediateCertificates: intermediates,
+	})
+	if err == nil || !strings.Contains(err.Error(), "no TSA root certificate(s) provided to verify timestamp") {
+		t.Fatalf("expected error verifying without a root certificate, got: %v", err)
+	}
 }
diff --git a/test/e2e_tsa_mtls.sh b/test/e2e_tsa_mtls.sh
@@ -31,6 +31,7 @@ TIMESTAMP_SERVER_CERT=$CERT_BASE/tsa-mtls-server.crt
 TIMESTAMP_SERVER_KEY=$CERT_BASE/tsa-mtls-server.key
 TIMESTAMP_SERVER_NAME="server.example.com"
 TIMESTAMP_SERVER_URL=https://localhost:3000/api/v1/timestamp
+TIMESTAMP_CHAIN_FILE="timestamp-chain"
 
 set +e
 COSIGN_CLI=./cosign
@@ -50,6 +51,12 @@ timestamp-server serve --disable-ntp-monitoring --tls-host 0.0.0.0 --tls-port 30
 		--scheme https --tls-ca $TIMESTAMP_CACERT --tls-key $TIMESTAMP_SERVER_KEY \
 		--tls-certificate $TIMESTAMP_SERVER_CERT &
 
+sleep 1
+curl -k -s --key test/testdata/tsa-mtls-client.key \
+    --cert test/testdata/tsa-mtls-client.crt \
+    --cacert test/testdata/tsa-mtls-ca.crt https://localhost:3000/api/v1/timestamp/certchain \
+    > $TIMESTAMP_CHAIN_FILE
+echo "DONE: $(ls -l $TIMESTAMP_CHAIN_FILE)"
 
 IMG=${IMAGE_URI_DIGEST:-}
 if [[ "$#" -ge 1 ]]; then
@@ -67,7 +74,6 @@ echo "IMG (IMAGE_URI_DIGEST): $IMG, TIMESTAMP_SERVER_URL: $TIMESTAMP_SERVER_URL"
 
 rm -f *.pem import-cosign.* key.pem
 
-
 # use gencert to generate CA, keys and certificates
 echo "generate keys and certificates with gencert"
 
@@ -85,8 +91,8 @@ rm -f key.pem import-cosign.*
 echo "cosign verify:"
 $COSIGN_CLI verify --insecure-ignore-tlog --insecure-ignore-sct --check-claims=true \
 	--certificate-identity-regexp '[email protected]' --certificate-oidc-issuer-regexp '.*' \
-	--certificate-chain cacert.pem $IMG
+	--certificate-chain cacert.pem --timestamp-certificate-chain $TIMESTAMP_CHAIN_FILE $IMG
 
 # cleanup
-rm -fr ca-key.pem cacert.pem cert.pem /tmp/timestamp-authority
+rm -fr ca-key.pem cacert.pem cert.pem timestamp-chain /tmp/timestamp-authority
 pkill -f 'timestamp-server'