From b6ee0336af3889ba8bc1eff4eb119a38ab026ee5 Mon Sep 17 00:00:00 2001 From: "Jose I. Paris" Date: Fri, 4 Sep 2026 09:51:06 +0200 Subject: [PATCH 1/3] fix(verifier): do not reject attestations when our pinned TSA chain is stale Upstream timestamp authorities rotate their responder certificates without notice. When that happens the pinned certChain no longer matches the signer of the RFC3161 response, and every attestation push fails even though the signature verified against a trusted certificate. Distinguish a timestamp failure caused by our own trust configuration from one caused by the attestation: verifyTimestamp now identifies the response's PKCS#7 signer up front and reports ErrTSASignerNotTrusted, naming the authority and the leaf it expected, when no configured authority signed it. SaveAttestation accepts such attestations with a warning instead of discarding the evidence; verification is recomputed on every read, so the result self-heals once the pinned chain catches up. Failures attributable to the attestation stay fatal. Assisted-by: Claude Code Signed-off-by: Jose I. Paris Chainloop-Trace-Sessions: 38ffe2d2-dec5-4c9f-9a68-abf837907af8 --- app/controlplane/pkg/biz/workflowrun.go | 22 +- go.mod | 4 +- pkg/attestation/verifier/timestamp.go | 93 ++++++- pkg/attestation/verifier/timestamp_test.go | 302 +++++++++++++++++++++ 4 files changed, 415 insertions(+), 6 deletions(-) create mode 100644 pkg/attestation/verifier/timestamp_test.go diff --git a/app/controlplane/pkg/biz/workflowrun.go b/app/controlplane/pkg/biz/workflowrun.go index 8b6dea686..619189151 100644 --- a/app/controlplane/pkg/biz/workflowrun.go +++ b/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 } diff --git a/go.mod b/go.mod index dc41db224..bed2a2fc2 100644 --- a/go.mod +++ b/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 diff --git a/pkg/attestation/verifier/timestamp.go b/pkg/attestation/verifier/timestamp.go index a3b4ff327..587bf40c9 100644 --- a/pkg/attestation/verifier/timestamp.go +++ b/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 +} diff --git a/pkg/attestation/verifier/timestamp_test.go b/pkg/attestation/verifier/timestamp_test.go new file mode 100644 index 000000000..2f29f0942 --- /dev/null +++ b/pkg/attestation/verifier/timestamp_test.go @@ -0,0 +1,302 @@ +// +// Copyright 2026 The Chainloop Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package verifier + +import ( + "crypto" + "crypto/rand" + "crypto/rsa" + "crypto/sha256" + "crypto/x509" + "crypto/x509/pkix" + "encoding/asn1" + "errors" + "math/big" + "testing" + "time" + + "github.com/digitorus/timestamp" + protobundle "github.com/sigstore/protobuf-specs/gen/pb-go/bundle/v1" + protocommon "github.com/sigstore/protobuf-specs/gen/pb-go/common/v1" + protodsse "github.com/sigstore/protobuf-specs/gen/pb-go/dsse" + sigstorebundle "github.com/sigstore/sigstore-go/pkg/bundle" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// testSignature is the DSSE signature the timestamps in these tests are taken +// over. It is deliberately not valid base64 so that VerifyTimestamps' legacy +// double-encoding workaround leaves it untouched. +var testSignature = []byte("!chainloop-test-signature!") + +func TestVerifyTimestamps_SignerClassification(t *testing.T) { + root, rootKey := newTestCA(t) + responder := newTestTSALeaf(t, "Chainloop Test Timestamp Responder 1", root, rootKey) + // The same authority after rotating its responder certificate: same root, + // different leaf. This is what a DigiCert cutover looks like to us. + rotated := newTestTSALeaf(t, "Chainloop Test Timestamp Responder 2", root, rootKey) + + otherRoot, otherRootKey := newTestCA(t) + unrelated := newTestTSALeaf(t, "Unrelated Timestamp Responder", otherRoot, otherRootKey) + + // A response signed by `responder` over the test signature. + tsr := createTestTimestamp(t, responder, testSignature) + // A response signed by `responder` but covering different content: the + // timestamp does not apply to this signature, which is an attestation fault. + tsrOverOtherContent := createTestTimestamp(t, responder, []byte("!other-content!")) + + cases := []struct { + name string + // authorities maps an authority name to its pinned chain, leaf first. + authorities map[string][]*x509.Certificate + tsr []byte + // expectSentinel nil means verification is expected to succeed. + expectSentinel error + // expectTrustConfig asserts the failure is attributed to our own trust + // configuration rather than to the attestation. + expectTrustConfig bool + }{ + { + name: "pinned chain matches the responder", + authorities: map[string][]*x509.Certificate{"test-tsa": {responder.cert, root.cert}}, + tsr: tsr, + }, + { + name: "upstream responder rotated ahead of our pinned chain", + authorities: map[string][]*x509.Certificate{"test-tsa": {rotated.cert, root.cert}}, + tsr: tsr, + expectSentinel: ErrTSASignerNotTrusted, + expectTrustConfig: true, + }, + { + name: "signer belongs to no configured authority", + authorities: map[string][]*x509.Certificate{"other-tsa": {unrelated.cert, otherRoot.cert}}, + tsr: tsr, + expectSentinel: ErrTSASignerNotTrusted, + expectTrustConfig: true, + }, + { + name: "one of several authorities matches the responder", + authorities: map[string][]*x509.Certificate{ + "rotated-tsa": {rotated.cert, root.cert}, + "other-tsa": {unrelated.cert, otherRoot.cert}, + "test-tsa": {responder.cert, root.cert}, + }, + tsr: tsr, + }, + { + name: "timestamp does not cover the signature", + authorities: map[string][]*x509.Certificate{"test-tsa": {responder.cert, root.cert}}, + tsr: tsrOverOtherContent, + // The signer is trusted, so the failure is the attestation's own. + expectSentinel: ErrTSAResponseInvalid, + expectTrustConfig: false, + }, + { + name: "unparseable response with a trusted authority", + authorities: map[string][]*x509.Certificate{"test-tsa": {responder.cert, root.cert}}, + tsr: []byte("not-a-valid-tsr"), + expectSentinel: ErrTSAResponseInvalid, + // We cannot attribute a malformed response to our configuration. + expectTrustConfig: false, + }, + { + name: "no authorities configured", + authorities: nil, + tsr: tsr, + expectSentinel: ErrNoTSARootsConfigured, + expectTrustConfig: true, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + err := VerifyTimestamps( + testBundleWithTimestamp(t, testSignature, tc.tsr), + &TrustedRoot{TimestampAuthorities: tc.authorities}, + ) + + if tc.expectSentinel == nil { + require.NoError(t, err) + return + } + + require.Error(t, err) + assert.True(t, errors.Is(err, tc.expectSentinel), "expected %v, got: %v", tc.expectSentinel, err) + assert.Equal(t, tc.expectTrustConfig, IsTrustConfigError(err), "unexpected fault attribution for: %v", err) + }) + } +} + +// TestVerifyTimestamps_RotatedSignerErrorNamesAuthority checks the rotation +// error is self-diagnosing: it must say which authority was tried and which +// leaf we expected, so the chain to rotate is identifiable from the message. +func TestVerifyTimestamps_RotatedSignerErrorNamesAuthority(t *testing.T) { + root, rootKey := newTestCA(t) + responder := newTestTSALeaf(t, "Chainloop Test Timestamp Responder 1", root, rootKey) + rotated := newTestTSALeaf(t, "Chainloop Test Timestamp Responder 2", root, rootKey) + + err := VerifyTimestamps( + testBundleWithTimestamp(t, testSignature, createTestTimestamp(t, responder, testSignature)), + &TrustedRoot{TimestampAuthorities: map[string][]*x509.Certificate{ + "prod-tsa": {rotated.cert, root.cert}, + }}, + ) + + require.Error(t, err) + assert.Contains(t, err.Error(), "prod-tsa") + assert.Contains(t, err.Error(), rotated.cert.Subject.CommonName) +} + +func TestIsTrustConfigError(t *testing.T) { + cases := []struct { + name string + err error + expect bool + }{ + {name: "nil", err: nil, expect: false}, + {name: "rotated signer", err: ErrTSASignerNotTrusted, expect: true}, + {name: "wrapped rotated signer", err: errors.Join(errors.New("boom"), ErrTSASignerNotTrusted), expect: true}, + {name: "no roots configured", err: ErrNoTSARootsConfigured, expect: true}, + {name: "invalid response", err: ErrTSAResponseInvalid, expect: false}, + {name: "timestamp outside TSA validity", err: ErrTimestampOutsideTSAValidity, expect: false}, + {name: "signing cert not valid at timestamp", err: ErrSigningCertNotValidAtTimestamp, expect: false}, + {name: "unrelated", err: errors.New("boom"), expect: false}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.expect, IsTrustConfigError(tc.err)) + }) + } +} + +// testKeyPair is a certificate together with the key that signs with it. +type testKeyPair struct { + cert *x509.Certificate + key *rsa.PrivateKey +} + +// newTestCA builds a self-signed CA to act as a timestamp authority root. +func newTestCA(t *testing.T) (*testKeyPair, *rsa.PrivateKey) { + t.Helper() + + key, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + + tmpl := &x509.Certificate{ + SerialNumber: newTestSerial(t), + Subject: pkix.Name{CommonName: "Chainloop Test Timestamp Root"}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(24 * time.Hour), + KeyUsage: x509.KeyUsageCertSign | x509.KeyUsageDigitalSignature, + BasicConstraintsValid: true, + IsCA: true, + } + + return &testKeyPair{cert: signTestCert(t, tmpl, tmpl, key.Public(), key), key: key}, key +} + +// newTestTSALeaf builds an end-entity timestamping certificate issued by the +// given CA. RFC 3161 requires the extended key usage extension to be critical +// and to carry timestamping only, which x509.CreateCertificate does not do on +// its own, so the extension is written explicitly. +func newTestTSALeaf(t *testing.T, commonName string, ca *testKeyPair, caKey *rsa.PrivateKey) *testKeyPair { + t.Helper() + + key, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + + // id-kp-timeStamping, per RFC 5280. + eku, err := asn1.Marshal([]asn1.ObjectIdentifier{{1, 3, 6, 1, 5, 5, 7, 3, 8}}) + require.NoError(t, err) + + tmpl := &x509.Certificate{ + SerialNumber: newTestSerial(t), + Subject: pkix.Name{CommonName: commonName}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(24 * time.Hour), + KeyUsage: x509.KeyUsageDigitalSignature, + ExtraExtensions: []pkix.Extension{{ + Id: asn1.ObjectIdentifier{2, 5, 29, 37}, // extended key usage + Critical: true, + Value: eku, + }}, + BasicConstraintsValid: true, + } + + return &testKeyPair{cert: signTestCert(t, tmpl, ca.cert, key.Public(), caKey), key: key} +} + +func signTestCert(t *testing.T, tmpl, parent *x509.Certificate, pub crypto.PublicKey, signer crypto.Signer) *x509.Certificate { + t.Helper() + + der, err := x509.CreateCertificate(rand.Reader, tmpl, parent, pub, signer) + require.NoError(t, err) + cert, err := x509.ParseCertificate(der) + require.NoError(t, err) + + return cert +} + +func newTestSerial(t *testing.T) *big.Int { + t.Helper() + + serial, err := rand.Int(rand.Reader, new(big.Int).Lsh(big.NewInt(1), 128)) + require.NoError(t, err) + + return serial +} + +// createTestTimestamp issues an RFC3161 response over content, signed by the +// given responder. Certificates are left out of the response, matching the +// timestamp requests Chainloop makes. +func createTestTimestamp(t *testing.T, responder *testKeyPair, content []byte) []byte { + t.Helper() + + digest := sha256.Sum256(content) + ts := ×tamp.Timestamp{ + HashAlgorithm: crypto.SHA256, + HashedMessage: digest[:], + Time: time.Now(), + Policy: asn1.ObjectIdentifier{1, 2, 3, 4, 1}, + } + + tsr, err := ts.CreateResponseWithOpts(responder.cert, responder.key, crypto.SHA256) + require.NoError(t, err) + + return tsr +} + +// testBundleWithTimestamp builds the minimum bundle VerifyTimestamps needs: a +// DSSE envelope carrying signature and one RFC3161 timestamp. +func testBundleWithTimestamp(t *testing.T, signature, tsr []byte) *sigstorebundle.Bundle { + t.Helper() + + return &sigstorebundle.Bundle{Bundle: &protobundle.Bundle{ + MediaType: "application/vnd.dev.sigstore.bundle+json;version=0.3", + VerificationMaterial: &protobundle.VerificationMaterial{ + TimestampVerificationData: &protobundle.TimestampVerificationData{ + Rfc3161Timestamps: []*protocommon.RFC3161SignedTimestamp{{SignedTimestamp: tsr}}, + }, + }, + Content: &protobundle.Bundle_DsseEnvelope{DsseEnvelope: &protodsse.Envelope{ + Payload: []byte(`{"_type":"https://in-toto.io/Statement/v1"}`), + PayloadType: "application/vnd.in-toto+json", + Signatures: []*protodsse.Signature{{Sig: signature}}, + }}, + }} +} From b0f0b0aab9a87e27dc2466672f52059651a76553 Mon Sep 17 00:00:00 2001 From: "Jose I. Paris" Date: Fri, 4 Sep 2026 12:29:17 +0200 Subject: [PATCH 2/3] fix(cli): keep a run readable when the control plane cannot verify its timestamp `wf run describe` aborted with "bundle verification failed" whenever the control plane served no timestamp authority for a bundle carrying a timestamp, hiding the run entirely. The gap is in the trust material the control plane publishes, not in the attestation, so report the attestation as unverified and carry on. Failures attributable to the attestation still abort. Assisted-by: Claude Code Signed-off-by: Jose I. Paris Chainloop-Trace-Sessions: 38ffe2d2-dec5-4c9f-9a68-abf837907af8 --- app/cli/pkg/action/attestation_verify.go | 19 ++++- app/cli/pkg/action/attestation_verify_test.go | 75 +++++++++++++++++++ 2 files changed, 92 insertions(+), 2 deletions(-) create mode 100644 app/cli/pkg/action/attestation_verify_test.go diff --git a/app/cli/pkg/action/attestation_verify.go b/app/cli/pkg/action/attestation_verify.go index b7c5feb06..fd2809998 100644 --- a/app/cli/pkg/action/attestation_verify.go +++ b/app/cli/pkg/action/attestation_verify.go @@ -1,5 +1,5 @@ // -// Copyright 2025 The Chainloop Authors. +// Copyright 2025-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -60,10 +60,15 @@ func verifyBundle(ctx context.Context, content []byte, opts *ActionsOpts) (bool, return false, fmt.Errorf("getting roots: %w", err) } if err = verifier.VerifyBundle(ctx, content, tr); err != nil { - if !errors.Is(err, verifier.ErrMissingVerificationMaterial) { + if verificationFailedFatally(err) { opts.Logger.Debug().Err(err).Msg("bundle verification failed") return false, errors.New("bundle verification failed") } + if verifier.IsTrustConfigError(err) { + // The attestation is reported as unverified, so say why: the + // gap is in the control plane's trust configuration. + opts.Logger.Warn().Err(err).Msg("attestation could not be verified") + } } else { return true, nil } @@ -71,3 +76,13 @@ func verifyBundle(ctx context.Context, content []byte, opts *ActionsOpts) (bool, return false, nil } + +// verificationFailedFatally reports whether a bundle verification error must +// abort the command instead of leaving the attestation reported as unverified. +// Verification relies on the trust material the control plane serves, so a gap +// there — no timestamp authorities configured, or a timestamp signed by an +// authority whose chain it does not publish — says nothing about the +// attestation and must not keep the evidence from being read. +func verificationFailedFatally(err error) bool { + return !errors.Is(err, verifier.ErrMissingVerificationMaterial) && !verifier.IsTrustConfigError(err) +} diff --git a/app/cli/pkg/action/attestation_verify_test.go b/app/cli/pkg/action/attestation_verify_test.go new file mode 100644 index 000000000..1b0bfda4b --- /dev/null +++ b/app/cli/pkg/action/attestation_verify_test.go @@ -0,0 +1,75 @@ +// +// Copyright 2026 The Chainloop Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package action + +import ( + "errors" + "fmt" + "testing" + + "github.com/chainloop-dev/chainloop/pkg/attestation/verifier" + "github.com/stretchr/testify/assert" +) + +func TestVerificationFailedFatally(t *testing.T) { + cases := []struct { + name string + err error + // expect reports whether the error must abort the command instead of + // leaving the attestation reported as unverified. + expect bool + }{ + { + name: "no verification material", + err: verifier.ErrMissingVerificationMaterial, + expect: false, + }, + { + // A control plane with no timestamp authorities configured cannot + // tell us whether the timestamp is good. That is not the + // attestation's fault and must not mask the evidence. + name: "control plane has no TSA roots configured", + err: fmt.Errorf("could not verify timestamps: %w", verifier.ErrNoTSARootsConfigured), + expect: false, + }, + { + name: "timestamp signed by an authority we do not have", + err: fmt.Errorf("could not verify timestamps: %w", verifier.ErrTSASignerNotTrusted), + expect: false, + }, + { + name: "timestamp does not verify against a trusted authority", + err: fmt.Errorf("could not verify timestamps: %w", verifier.ErrTSAResponseInvalid), + expect: true, + }, + { + name: "signing certificate not valid at timestamp time", + err: fmt.Errorf("could not verify timestamps: %w", verifier.ErrSigningCertNotValidAtTimestamp), + expect: true, + }, + { + name: "invalid signature", + err: errors.New("validating the DSSE envelope: signature not verified"), + expect: true, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.expect, verificationFailedFatally(tc.err)) + }) + } +} From b88dda4a3603a438f90eb5ba378ba1c0533212f4 Mon Sep 17 00:00:00 2001 From: "Jose I. Paris" Date: Fri, 4 Sep 2026 12:45:27 +0200 Subject: [PATCH 3/3] Revert "fix(cli): keep a run readable when the control plane cannot verify its timestamp" This reverts commit b0f0b0aab9a87e27dc2466672f52059651a76553. Failing the describe when the control plane cannot verify a timestamp is the intended behaviour on the read path. Assisted-by: Claude Code Signed-off-by: Jose I. Paris Chainloop-Trace-Sessions: 38ffe2d2-dec5-4c9f-9a68-abf837907af8 --- app/cli/pkg/action/attestation_verify.go | 19 +---- app/cli/pkg/action/attestation_verify_test.go | 75 ------------------- 2 files changed, 2 insertions(+), 92 deletions(-) delete mode 100644 app/cli/pkg/action/attestation_verify_test.go diff --git a/app/cli/pkg/action/attestation_verify.go b/app/cli/pkg/action/attestation_verify.go index fd2809998..b7c5feb06 100644 --- a/app/cli/pkg/action/attestation_verify.go +++ b/app/cli/pkg/action/attestation_verify.go @@ -1,5 +1,5 @@ // -// Copyright 2025-2026 The Chainloop Authors. +// Copyright 2025 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -60,15 +60,10 @@ func verifyBundle(ctx context.Context, content []byte, opts *ActionsOpts) (bool, return false, fmt.Errorf("getting roots: %w", err) } if err = verifier.VerifyBundle(ctx, content, tr); err != nil { - if verificationFailedFatally(err) { + if !errors.Is(err, verifier.ErrMissingVerificationMaterial) { opts.Logger.Debug().Err(err).Msg("bundle verification failed") return false, errors.New("bundle verification failed") } - if verifier.IsTrustConfigError(err) { - // The attestation is reported as unverified, so say why: the - // gap is in the control plane's trust configuration. - opts.Logger.Warn().Err(err).Msg("attestation could not be verified") - } } else { return true, nil } @@ -76,13 +71,3 @@ func verifyBundle(ctx context.Context, content []byte, opts *ActionsOpts) (bool, return false, nil } - -// verificationFailedFatally reports whether a bundle verification error must -// abort the command instead of leaving the attestation reported as unverified. -// Verification relies on the trust material the control plane serves, so a gap -// there — no timestamp authorities configured, or a timestamp signed by an -// authority whose chain it does not publish — says nothing about the -// attestation and must not keep the evidence from being read. -func verificationFailedFatally(err error) bool { - return !errors.Is(err, verifier.ErrMissingVerificationMaterial) && !verifier.IsTrustConfigError(err) -} diff --git a/app/cli/pkg/action/attestation_verify_test.go b/app/cli/pkg/action/attestation_verify_test.go deleted file mode 100644 index 1b0bfda4b..000000000 --- a/app/cli/pkg/action/attestation_verify_test.go +++ /dev/null @@ -1,75 +0,0 @@ -// -// Copyright 2026 The Chainloop Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package action - -import ( - "errors" - "fmt" - "testing" - - "github.com/chainloop-dev/chainloop/pkg/attestation/verifier" - "github.com/stretchr/testify/assert" -) - -func TestVerificationFailedFatally(t *testing.T) { - cases := []struct { - name string - err error - // expect reports whether the error must abort the command instead of - // leaving the attestation reported as unverified. - expect bool - }{ - { - name: "no verification material", - err: verifier.ErrMissingVerificationMaterial, - expect: false, - }, - { - // A control plane with no timestamp authorities configured cannot - // tell us whether the timestamp is good. That is not the - // attestation's fault and must not mask the evidence. - name: "control plane has no TSA roots configured", - err: fmt.Errorf("could not verify timestamps: %w", verifier.ErrNoTSARootsConfigured), - expect: false, - }, - { - name: "timestamp signed by an authority we do not have", - err: fmt.Errorf("could not verify timestamps: %w", verifier.ErrTSASignerNotTrusted), - expect: false, - }, - { - name: "timestamp does not verify against a trusted authority", - err: fmt.Errorf("could not verify timestamps: %w", verifier.ErrTSAResponseInvalid), - expect: true, - }, - { - name: "signing certificate not valid at timestamp time", - err: fmt.Errorf("could not verify timestamps: %w", verifier.ErrSigningCertNotValidAtTimestamp), - expect: true, - }, - { - name: "invalid signature", - err: errors.New("validating the DSSE envelope: signature not verified"), - expect: true, - }, - } - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - assert.Equal(t, tc.expect, verificationFailedFatally(tc.err)) - }) - } -}