fix(verifier): require signature verification, reject timestamp-only bundles

pkg/attestation/verifier/testdata/bundle_with_publickey.json

@@ -0,0 +1,17 @@
+{
+   "mediaType": "application/vnd.dev.sigstore.bundle+json;version=0.3",
+   "verificationMaterial": {
+      "publicKey": {
+         "hint": "some-key-hint"
+      }
+   },
+   "dsseEnvelope": {
+      "payload": "eyJfdHlwZSI6Imh0dHBzOi8vaW4tdG90by5pby9TdGF0ZW1lbnQvdjEifQ==",
+      "payloadType": "application/vnd.in-toto+json",
+      "signatures": [
+         {
+            "sig": "TUVVQ0lRQ1JjOFVmeFVSMnBkL2UxVW1pVmhkRE5BZUxVRi83ODZ2WGxCT1VWM0dJcUFJZ0NqOTROczZwbzFQSzJjTW95MzBpOVB3Smx5c1E0R1RmTGI4TnZ5WlB1ckk9"
+         }
+      ]
+   }
+}

pkg/attestation/verifier/verifier.go

@@ -41,6 +41,11 @@ type TrustedRoot struct {
 var ErrMissingVerificationMaterial = errors.New("missing material")
 var ErrInvalidBundle = errors.New("invalid bundle")
 
+// ErrUnsupportedVerificationMaterial indicates the bundle carries verification
+// material we cannot verify a signature against (e.g. a bare public key with no
+// trusted key set). It is treated as a verification failure, never ignored.
+var ErrUnsupportedVerificationMaterial = errors.New("unsupported verification material")
+
 func VerifyBundle(ctx context.Context, bundleBytes []byte, tr *TrustedRoot) error {
 	if bundleBytes == nil {
 		return ErrMissingVerificationMaterial
@@ -55,7 +60,6 @@ func VerifyBundle(ctx context.Context, bundleBytes []byte, tr *TrustedRoot) erro
 	// fix for old attestations
 	attestation.FixSignatureInBundle(bundle)
 
-	hasVerificationMaterial := false
 	sb := &sigstorebundle.Bundle{Bundle: bundle}
 	vc, err := sb.VerificationContent()
 	if err != nil {
@@ -64,44 +68,52 @@ func VerifyBundle(ctx context.Context, bundleBytes []byte, tr *TrustedRoot) erro
 		}
 	}
 
-	if vc != nil && vc.Certificate() != nil {
-		hasVerificationMaterial = true
-		signingCert := vc.Certificate()
-
-		akiSum := sha256.Sum256(signingCert.AuthorityKeyId)
-		aki := hex.EncodeToString(akiSum[:])
-		chain, ok := tr.Keys[aki]
-		if !ok {
-			return fmt.Errorf("trusted root not found for signing key with AKI %s", aki)
+	// Signature verification is MANDATORY
+	switch {
+	case vc != nil && vc.Certificate() != nil:
+		if err := verifyCertSignature(ctx, bundle, vc.Certificate(), tr); err != nil {
+			return err
 		}
+	case bundle.GetVerificationMaterial().GetPublicKey() != nil:
+		// Public-key bundles are not supported at this time
+		return fmt.Errorf("%w: public key verification material", ErrUnsupportedVerificationMaterial)
+	default:
+		// No certificate and no public key: nothing to verify the signature against.
+		return ErrMissingVerificationMaterial
+	}
 
-		verifier, err := cosign.ValidateAndUnpackCertWithChain(signingCert, chain, &cosign.CheckOpts{IgnoreSCT: true})
-		if err != nil {
-			return fmt.Errorf("validating the certificate: %w", err)
-		}
+	// The signature has been verified against a trusted certificate. The timestamp
+	// (if present) only validates the signing window; it can never be the sole
+	// verification material.
+	if err := VerifyTimestamps(sb, tr); err != nil && !errors.Is(err, ErrMissingVerificationMaterial) {
+		return fmt.Errorf("could not verify timestamps: %w", err)
+	}
 
-		dsseVerifier, err := dsse.NewEnvelopeVerifier(&sigdsee.VerifierAdapter{SignatureVerifier: verifier})
-		if err != nil {
-			return fmt.Errorf("creating DSSE verifier: %w", err)
-		}
+	return nil
+}
 
-		_, err = dsseVerifier.Verify(ctx, attestation.DSSEEnvelopeFromBundle(bundle))
-		if err != nil {
-			return fmt.Errorf("validating the DSSE envelope: %w", err)
-		}
+// verifyCertSignature validates the signing certificate against the trusted root
+// chain and verifies the DSSE envelope signature with the certificate's key.
+func verifyCertSignature(ctx context.Context, bundle *protobundle.Bundle, signingCert *x509.Certificate, tr *TrustedRoot) error {
+	akiSum := sha256.Sum256(signingCert.AuthorityKeyId)
+	aki := hex.EncodeToString(akiSum[:])
+	chain, ok := tr.Keys[aki]
+	if !ok {
+		return fmt.Errorf("trusted root not found for signing key with AKI %s", aki)
 	}
 
-	// Even with no cert (using a local key), we can still validate the timestamp
-	if err = VerifyTimestamps(sb, tr); err != nil {
-		if !errors.Is(err, ErrMissingVerificationMaterial) {
-			return fmt.Errorf("could not verify timestamps: %w", err)
-		}
-	} else {
-		hasVerificationMaterial = true
+	verifier, err := cosign.ValidateAndUnpackCertWithChain(signingCert, chain, &cosign.CheckOpts{IgnoreSCT: true})
+	if err != nil {
+		return fmt.Errorf("validating the certificate: %w", err)
 	}
 
-	if !hasVerificationMaterial {
-		return ErrMissingVerificationMaterial
+	dsseVerifier, err := dsse.NewEnvelopeVerifier(&sigdsee.VerifierAdapter{SignatureVerifier: verifier})
+	if err != nil {
+		return fmt.Errorf("creating DSSE verifier: %w", err)
+	}
+
+	if _, err := dsseVerifier.Verify(ctx, attestation.DSSEEnvelopeFromBundle(bundle)); err != nil {
+		return fmt.Errorf("validating the DSSE envelope: %w", err)
 	}
 
 	return nil

pkg/attestation/verifier/verifier_test.go

@@ -23,9 +23,12 @@ import (
 	"os"
 	"testing"
 
+	protobundle "github.com/sigstore/protobuf-specs/gen/pb-go/bundle/v1"
+	sigstorebundle "github.com/sigstore/sigstore-go/pkg/bundle"
 	"github.com/sigstore/sigstore/pkg/cryptoutils"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"google.golang.org/protobuf/encoding/protojson"
 )
 
 func TestVerifyBundle(t *testing.T) {
@@ -42,6 +45,9 @@ func TestVerifyBundle(t *testing.T) {
 		roots     *TrustedRoot
 		bundle    string
 		expectErr string
+		// expectSentinel, when set, is asserted with errors.Is in addition to
+		// (or instead of) the substring match.
+		expectSentinel error
 	}{
 		{
 			name:   "invalid bundle, but still verifiable",
@@ -54,10 +60,11 @@ func TestVerifyBundle(t *testing.T) {
 			bundle: "testdata/bundle_valid.json",
 		},
 		{
-			name:      "valid bundle without verification material",
-			roots:     roots,
-			bundle:    "testdata/bundle_valid_nomaterial.json",
-			expectErr: "missing material",
+			name:           "valid bundle without verification material",
+			roots:          roots,
+			bundle:         "testdata/bundle_valid_nomaterial.json",
+			expectErr:      "missing material",
+			expectSentinel: ErrMissingVerificationMaterial,
 		},
 		{
 			name:      "corrupted bundle",
@@ -71,6 +78,26 @@ func TestVerifyBundle(t *testing.T) {
 			bundle:    "testdata/dsse_envelope.json",
 			expectErr: "invalid bundle",
 		},
+		{
+			// a cert-less bundle carrying only a timestamp must never be reported as verified.
+			// It is rejected at the mandatory-signature gate before timestamp validation runs,
+			// so the timestamp can never be the deciding factor.
+			name:           "timestamp-only bundle (no signing key) is rejected",
+			roots:          roots,
+			bundle:         "testdata/bundle_with_bad_timestamp.json",
+			expectErr:      "missing material",
+			expectSentinel: ErrMissingVerificationMaterial,
+		},
+		{
+			// public-key bundles have no trusted key
+			// set to verify against and must fail rather than fall through to
+			// the timestamp-only path.
+			name:           "public key bundle is rejected as unsupported",
+			roots:          roots,
+			bundle:         "testdata/bundle_with_publickey.json",
+			expectErr:      "unsupported verification material",
+			expectSentinel: ErrUnsupportedVerificationMaterial,
+		},
 	}
 
 	for _, tc := range cases {
@@ -81,6 +108,10 @@ func TestVerifyBundle(t *testing.T) {
 			if tc.expectErr != "" {
 				assert.Error(t, err)
 				assert.Contains(t, err.Error(), tc.expectErr)
+				if tc.expectSentinel != nil {
+					assert.True(t, errors.Is(err, tc.expectSentinel),
+						"expected %v, got: %v", tc.expectSentinel, err)
+				}
 				return
 			}
 			assert.NoError(t, err)
@@ -118,9 +149,16 @@ func TestVerifyTimestamps_TypedErrors(t *testing.T) {
 	bundleBytes, err := os.ReadFile("testdata/bundle_with_bad_timestamp.json")
 	require.NoError(t, err)
 
+	// VerifyTimestamps is exercised directly: the bad-timestamp fixture is a
+	// cert-less bundle, which VerifyBundle now rejects at the mandatory-signature
+	// gate before timestamp validation would run.
+	bundle := new(protobundle.Bundle)
+	require.NoError(t, protojson.Unmarshal(bundleBytes, bundle))
+	sb := &sigstorebundle.Bundle{Bundle: bundle}
+
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
-			err := VerifyBundle(context.TODO(), bundleBytes, tc.roots)
+			err := VerifyTimestamps(sb, tc.roots)
 			require.Error(t, err)
 			assert.True(t, errors.Is(err, tc.expectSentinel),
 				"expected %v, got: %v", tc.expectSentinel, err)