fix(verifier): do not reject attestations when our pinned TSA chain is stale (#3396)

Author: jiparis

app/controlplane/pkg/biz/workflowrun.go

@@ -472,7 +472,17 @@ func (uc *WorkflowRunUseCase) SaveAttestation(ctx context.Context, id string, bu
 
 	// if it's verifiable, make sure it passed
 	if validation != nil && !validation.Result {
-		return nil, NewErrValidation(fmt.Errorf("attestation verification failed: %s", validation.FailureReason))
+		// A failure caused by our own TSA trust configuration — typically an
+		// upstream authority that rotated its responder certificate ahead of the
+		// chain we pin — must not discard the evidence. The signature has already
+		// been verified against a trusted certificate, and verification is
+		// recomputed on every read, so the result self-heals once the
+		// configuration catches up.
+		if !validation.TrustConfigFault {
+			return nil, NewErrValidation(fmt.Errorf("attestation verification failed: %s", validation.FailureReason))
+		}
+		uc.logger.Warnw("msg", "accepting attestation with an unverifiable timestamp, review the configured TSA certificate chains",
+			"workflowRunID", runID.String(), "reason", validation.FailureReason)
 	}
 
 	// Run some validations on the predicate
@@ -571,6 +581,10 @@ func (uc *WorkflowRunUseCase) GetByIDInOrg(ctx context.Context, orgID, runID str
 type VerificationResult struct {
 	Result        bool
 	FailureReason string
+	// TrustConfigFault is set when verification failed because of this server's
+	// TSA trust configuration rather than because of a fault in the attestation.
+	// Callers persisting an attestation must not reject it on such a failure.
+	TrustConfigFault bool
 }
 
 func (uc *WorkflowRunUseCase) VerifyRun(ctx context.Context, run *WorkflowRun) (*VerificationResult, error) {
@@ -604,7 +618,11 @@ func (uc *WorkflowRunUseCase) verifyBundle(ctx context.Context, bundle []byte) (
 			return nil, err
 		}
 
-		return &VerificationResult{Result: false, FailureReason: err.Error()}, nil
+		return &VerificationResult{
+			Result:           false,
+			FailureReason:    err.Error(),
+			TrustConfigFault: verifier.IsTrustConfigError(err),
+		}, nil
 	}
 	return &VerificationResult{Result: true}, nil
 }

go.mod

@@ -352,8 +352,8 @@ require (
 	github.com/cyberphone/json-canonicalization v0.0.0-20241213102144-19d51d7fe467 // indirect
 	github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
 	github.com/desertbit/timer v0.0.0-20180107155436-c41aec40b27f // indirect
-	github.com/digitorus/pkcs7 v0.0.0-20230818184609-3a137a874352 // indirect
-	github.com/digitorus/timestamp v0.0.0-20231217203849-220c5c2851b7 // indirect
+	github.com/digitorus/pkcs7 v0.0.0-20230818184609-3a137a874352
+	github.com/digitorus/timestamp v0.0.0-20231217203849-220c5c2851b7
 	github.com/docker/cli v29.7.2+incompatible // indirect
 	github.com/docker/docker-credential-helpers v0.9.5 // indirect
 	github.com/docker/go-units v0.5.0 // indirect

pkg/attestation/verifier/timestamp.go

@@ -21,7 +21,11 @@ import (
 	"encoding/base64"
 	"errors"
 	"fmt"
+	"math/big"
+	"strings"
 
+	"github.com/digitorus/pkcs7"
+	"github.com/digitorus/timestamp"
 	"github.com/sigstore/sigstore-go/pkg/bundle"
 	"github.com/sigstore/sigstore-go/pkg/verify"
 	"github.com/sigstore/timestamp-authority/v2/pkg/verification"
@@ -43,8 +47,23 @@ var (
 	// ErrNoTSARootsConfigured indicates the bundle contains signed timestamps
 	// but no TSA trust roots are configured on the server.
 	ErrNoTSARootsConfigured = errors.New("no TSA trust roots configured")
+
+	// ErrTSASignerNotTrusted indicates the timestamp was signed by a certificate
+	// that belongs to none of the configured timestamp authorities. Upstream TSAs
+	// rotate their responder certificates without notice, so this points at a
+	// pinned chain that has fallen behind, not at a faulty attestation.
+	ErrTSASignerNotTrusted = errors.New("TSA response signer is not a configured timestamp authority")
 )
 
+// IsTrustConfigError reports whether err is a timestamp verification failure
+// attributable to the TSA trust configuration rather than to the attestation
+// itself. Such a failure must not reject an incoming attestation: the signature
+// is verified independently, and verification is recomputed on every read, so
+// the outcome self-heals once the configuration catches up with the upstream TSA.
+func IsTrustConfigError(err error) bool {
+	return errors.Is(err, ErrNoTSARootsConfigured) || errors.Is(err, ErrTSASignerNotTrusted)
+}
+
 func VerifyTimestamps(sb *bundle.Bundle, tr *TrustedRoot) error {
 	signedTimestamps, err := sb.Timestamps()
 	if err != nil {
@@ -91,11 +110,25 @@ func VerifyTimestamps(sb *bundle.Bundle, tr *TrustedRoot) error {
 }
 
 // verifyTimestamp tries to verify a single signed timestamp against every
-// configured TSA. Returns the error from the last attempted TSA on failure.
+// configured TSA. Returns the error from the last attempted TSA on failure, or
+// ErrTSASignerNotTrusted when the response was signed by none of them.
 func verifyTimestamp(st []byte, sigBytes []byte, vc verify.VerificationContent, tr *TrustedRoot) error {
+	// Chainloop's timestamp requests do not ask for certificates, so the response
+	// carries none and the pinned leaf is injected as the only candidate signer.
+	// Knowing up front which authority actually signed keeps a rotated upstream
+	// responder distinguishable from a response we should reject.
+	signers := tsrSigners(st)
+
 	var lastErr error
-	for _, tsa := range tr.TimestampAuthorities {
+	var skipped []string
+	for name, tsa := range tr.TimestampAuthorities {
 		tsaCert := tsa[0]
+		if !signedByCert(signers, tsaCert) {
+			skipped = append(skipped, fmt.Sprintf("%q (expected leaf %q, serial %s)",
+				name, tsaCert.Subject.CommonName, tsaCert.SerialNumber))
+			continue
+		}
+
 		var roots []*x509.Certificate
 		var intermediates []*x509.Certificate
 		if len(tsa) > 1 {
@@ -127,5 +160,61 @@ func verifyTimestamp(st []byte, sigBytes []byte, vc verify.VerificationContent,
 
 		return nil
 	}
+
+	// No authority signed this response: our pinned chains are behind the
+	// upstream TSA rather than the response being at fault.
+	if lastErr == nil && len(skipped) > 0 {
+		return fmt.Errorf("%w: tried %s", ErrTSASignerNotTrusted, strings.Join(skipped, ", "))
+	}
+
 	return lastErr
 }
+
+// tsrSignerID identifies the certificate that signed an RFC3161 response, as
+// carried in the PKCS#7 SignerInfo.
+type tsrSignerID struct {
+	rawIssuer []byte
+	serial    *big.Int
+}
+
+// tsrSigners returns the identities of the certificates that signed the RFC3161
+// response. A nil result means the response could not be parsed; callers must
+// then treat every candidate as a possible signer so the underlying verifier
+// produces the authoritative error.
+func tsrSigners(st []byte) []tsrSignerID {
+	ts, err := timestamp.ParseResponse(st)
+	if err != nil {
+		return nil
+	}
+
+	p7, err := pkcs7.Parse(ts.RawToken)
+	if err != nil || len(p7.Signers) == 0 {
+		return nil
+	}
+
+	signers := make([]tsrSignerID, 0, len(p7.Signers))
+	for _, signer := range p7.Signers {
+		signers = append(signers, tsrSignerID{
+			rawIssuer: signer.IssuerAndSerialNumber.IssuerName.FullBytes,
+			serial:    signer.IssuerAndSerialNumber.SerialNumber,
+		})
+	}
+
+	return signers
+}
+
+// signedByCert reports whether cert is one of the given signers. An unknown
+// signer set (a response we could not parse) matches every certificate.
+func signedByCert(signers []tsrSignerID, cert *x509.Certificate) bool {
+	if signers == nil {
+		return true
+	}
+
+	for _, signer := range signers {
+		if signer.serial != nil && cert.SerialNumber.Cmp(signer.serial) == 0 && bytes.Equal(cert.RawIssuer, signer.rawIssuer) {
+			return true
+		}
+	}
+
+	return false
+}