From 49b88f7e33345d5a2de1b2b76f0a5dfe2b1919f3 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Mon, 5 Oct 2026 17:32:25 +0200 Subject: [PATCH 1/5] fix(controlplane): enforce keyless verification of pushed attestations When keyless signing is configured, the control plane now requires every pushed attestation to be signed with a certificate issued by one of its certificate authorities to the organization that owns the workflow run. Attestations signed with other methods, or with no verification material, are rejected. Instances without keyless signing are not affected. Viewing a run with verification enabled now reports an attestation without verification material as not verified. Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino Chainloop-Trace-Sessions: 3ed0fe4b-8701-40f8-bb35-c85ce99f6fd1 --- app/controlplane/pkg/biz/signing.go | 6 + app/controlplane/pkg/biz/workflowrun.go | 120 ++++++--- .../pkg/biz/workflowrun_verification_test.go | 254 ++++++++++++++++++ pkg/attestation/verifier/verifier.go | 44 ++- pkg/attestation/verifier/verifier_test.go | 84 ++++++ 5 files changed, 476 insertions(+), 32 deletions(-) create mode 100644 app/controlplane/pkg/biz/workflowrun_verification_test.go diff --git a/app/controlplane/pkg/biz/signing.go b/app/controlplane/pkg/biz/signing.go index 266918188..94daee221 100644 --- a/app/controlplane/pkg/biz/signing.go +++ b/app/controlplane/pkg/biz/signing.go @@ -136,6 +136,12 @@ func parseTSA(tsaConf *conf.TSA) (*TimestampAuthority, error) { return tsa, nil } +// KeylessEnabled tells if keyless signing is configured, in other words, if +// there are certificate authorities to issue signing certificates. +func (s *SigningUseCase) KeylessEnabled() bool { + return s != nil && s.CAs != nil +} + func (s *SigningUseCase) GetCurrentTSA() *TimestampAuthority { for _, tsa := range s.TimestampAuthorities { if tsa.Issuer { diff --git a/app/controlplane/pkg/biz/workflowrun.go b/app/controlplane/pkg/biz/workflowrun.go index 1ad36826f..8fdf650ab 100644 --- a/app/controlplane/pkg/biz/workflowrun.go +++ b/app/controlplane/pkg/biz/workflowrun.go @@ -442,14 +442,13 @@ func (uc *WorkflowRunUseCase) orgBlocksReleasedVersions(ctx context.Context, run return org.BlockAttestationsOnReleasedVersions, nil } -// ValidateAttestationContract checks a bundle against the contract revision -// pinned on its workflow run without persisting anything. +// ValidateAttestationContract checks a bundle's signature and the contract +// revision pinned on its workflow run without persisting anything. // -// SaveAttestation runs the same check and is the authoritative one, since it +// SaveAttestation runs the same checks and is the authoritative one, since it // sits on the path every attestation takes. This entry point exists for callers // that push the bundle to a CAS backend before calling SaveAttestation: without -// it, an attestation rejected for violating its contract would already have left -// a blob behind in CAS. +// it, a rejected attestation would already have left a blob behind in CAS. func (uc *WorkflowRunUseCase) ValidateAttestationContract(ctx context.Context, runID string, bundle []byte) error { ctx, span := otelx.Start(ctx, workflowRunTracer, "WorkflowRunUseCase.ValidateAttestationContract") defer span.End() @@ -466,6 +465,10 @@ func (uc *WorkflowRunUseCase) ValidateAttestationContract(ctx context.Context, r return NewErrNotFound("workflow run") } + if err := uc.verifyAttestationToStore(ctx, run, bundle); err != nil { + return err + } + dsseEnv, err := attestation.DSSEEnvelopeFromBundleBytes(bundle) if err != nil { return fmt.Errorf("extracting DSSE envelope: %w", err) @@ -568,29 +571,8 @@ func (uc *WorkflowRunUseCase) SaveAttestation(ctx context.Context, id string, bu return nil, fmt.Errorf("extracting predicate: %w", err) } - // verify attestation (only if chainloop is the signer) - validation, err := uc.verifyBundle(ctx, bundle) - if err != nil { - if !errors.Is(err, verifier.ErrInvalidBundle) { - return nil, err - } - // invalid bundle is expected for old attestations so we skip validation - uc.logger.Warn("received an old attestation format, not a bundle: attestation verification skipped", "error", err) - } - - // if it's verifiable, make sure it passed - if validation != nil && !validation.Result { - // 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) + if err := uc.verifyAttestationToStore(ctx, run, bundle); err != nil { + return nil, err } // Run some validations on the predicate @@ -699,14 +681,90 @@ type VerificationResult struct { TrustConfigFault bool } +// verifyAttestationToStore checks the attestation signature before it is stored. +// +// When keyless signing is configured, the attestation MUST be signed with a +// certificate issued by one of the configured certificate authorities to the +// organization that owns the run. Any other signing method is rejected, since +// the control plane has nothing to verify it against. When keyless signing is +// not configured there is nothing to verify against, and no check runs. +func (uc *WorkflowRunUseCase) verifyAttestationToStore(ctx context.Context, run *WorkflowRun, bundle []byte) error { + if !uc.signingUseCase.KeylessEnabled() { + return nil + } + + opts, err := verifyOptionsForRun(run) + if err != nil { + return err + } + + validation, err := uc.verifyBundle(ctx, bundle, opts...) + if err != nil { + if errors.Is(err, verifier.ErrInvalidBundle) { + return NewErrValidation(fmt.Errorf("attestation verification failed: %w", err)) + } + return err + } + + if validation == nil { + return NewErrValidation(errors.New("attestation verification failed: the attestation must be signed with the keyless signer of this instance, other signing methods are not allowed")) + } + + if !validation.Result { + // 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 issued to the organization, + // and verification is recomputed on every read, so the result self-heals + // once the configuration catches up. + if !validation.TrustConfigFault { + return 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", run.ID.String(), "reason", validation.FailureReason) + } + + return nil +} + +// verifyOptionsForRun binds the verification to the organization that owns the run. +func verifyOptionsForRun(run *WorkflowRun) ([]verifier.VerifyOption, error) { + if run.Workflow == nil || run.Workflow.OrgID == uuid.Nil { + return nil, fmt.Errorf("workflow run %s has no organization", run.ID) + } + return []verifier.VerifyOption{verifier.WithExpectedOrganization(run.Workflow.OrgID.String())}, nil +} + func (uc *WorkflowRunUseCase) VerifyRun(ctx context.Context, run *WorkflowRun) (*VerificationResult, error) { ctx, span := otelx.Start(ctx, workflowRunTracer, "WorkflowRunUseCase.VerifyRun") defer span.End() - return uc.verifyBundle(ctx, run.Attestation.Bundle) + // Without keyless signing there is nothing to verify against, and a run + // that has no attestation yet has nothing to verify + if !uc.signingUseCase.KeylessEnabled() || run.Attestation == nil || len(run.Attestation.Bundle) == 0 { + return nil, nil + } + + opts, err := verifyOptionsForRun(run) + if err != nil { + return nil, err + } + + vr, err := uc.verifyBundle(ctx, run.Attestation.Bundle, opts...) + if err != nil { + return nil, err + } + + // Keyless signing is enforced, so an attestation that can't be verified + // must not be reported as if verification did not apply + if vr == nil { + return &VerificationResult{Result: false, FailureReason: "the attestation has no verification material"}, nil + } + + return vr, nil } -func (uc *WorkflowRunUseCase) verifyBundle(ctx context.Context, bundle []byte) (*VerificationResult, error) { +func (uc *WorkflowRunUseCase) verifyBundle(ctx context.Context, bundle []byte, opts ...verifier.VerifyOption) (*VerificationResult, error) { tr, err := uc.signingUseCase.GetTrustedRoot(ctx) if err != nil { if IsErrNotImplemented(err) { @@ -719,7 +777,7 @@ func (uc *WorkflowRunUseCase) verifyBundle(ctx context.Context, bundle []byte) ( if err != nil { return nil, fmt.Errorf("parsing roots: %w", err) } - err = verifier.VerifyBundle(ctx, bundle, verifierRoots) + err = verifier.VerifyBundle(ctx, bundle, verifierRoots, opts...) if err != nil { // if no verification material found, it's not verifiable if errors.Is(err, verifier.ErrMissingVerificationMaterial) { diff --git a/app/controlplane/pkg/biz/workflowrun_verification_test.go b/app/controlplane/pkg/biz/workflowrun_verification_test.go new file mode 100644 index 000000000..f414eb6a5 --- /dev/null +++ b/app/controlplane/pkg/biz/workflowrun_verification_test.go @@ -0,0 +1,254 @@ +// +// 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 biz_test + +import ( + "context" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/sha256" + "crypto/x509" + "crypto/x509/pkix" + "encoding/base64" + "encoding/json" + "encoding/pem" + "os" + "testing" + + "github.com/chainloop-dev/chainloop/app/controlplane/pkg/biz" + repoM "github.com/chainloop-dev/chainloop/app/controlplane/pkg/biz/mocks" + ca2 "github.com/chainloop-dev/chainloop/app/controlplane/pkg/ca" + "github.com/chainloop-dev/chainloop/pkg/attestation" + "github.com/google/uuid" + "github.com/secure-systems-lab/go-securesystemslib/dsse" + protobundle "github.com/sigstore/protobuf-specs/gen/pb-go/bundle/v1" + v1 "github.com/sigstore/protobuf-specs/gen/pb-go/common/v1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + "google.golang.org/protobuf/encoding/protojson" +) + +// keylessSigningUseCase returns a signing use case backed by an ephemeral CA, +// so keyless signing is enabled. +func keylessSigningUseCase(t *testing.T) *biz.SigningUseCase { + t.Helper() + ca, err := NewTestCA() + require.NoError(t, err) + return &biz.SigningUseCase{CAs: &ca2.CertificateAuthorities{CAs: []ca2.CertificateAuthority{ca}, SignerCA: ca}} +} + +type testBundleKind int + +const ( + // signed with a keyless certificate that carries the certificate + bundleWithCert testBundleKind = iota + // signed, but without verification material, as cosign key or SignServer signers do + bundleWithoutMaterial + // a raw DSSE envelope instead of a Sigstore bundle + bundleRawEnvelope +) + +// newSignedTestBundle signs the test attestation with a certificate issued by +// the signing use case to the given organization. +func newSignedTestBundle(t *testing.T, signing *biz.SigningUseCase, orgID string, kind testBundleKind) []byte { + t.Helper() + + raw, err := os.ReadFile("testdata/attestations/bundle.json") + require.NoError(t, err) + env, err := attestation.DSSEEnvelopeFromBundleBytes(raw) + require.NoError(t, err) + payload, err := base64.StdEncoding.DecodeString(env.Payload) + require.NoError(t, err) + + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + csrDER, err := x509.CreateCertificateRequest(rand.Reader, &x509.CertificateRequest{Subject: pkix.Name{CommonName: "ephemeral certificate"}}, key) + require.NoError(t, err) + chain, err := signing.CreateSigningCert(context.Background(), orgID, pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE REQUEST", Bytes: csrDER})) + require.NoError(t, err) + + digest := sha256.Sum256(dsse.PAE(env.PayloadType, payload)) + sig, err := ecdsa.SignASN1(rand.Reader, key, digest[:]) + require.NoError(t, err) + + signed := &dsse.Envelope{ + PayloadType: env.PayloadType, + Payload: env.Payload, + Signatures: []dsse.Signature{{Sig: base64.StdEncoding.EncodeToString(sig)}}, + } + + if kind == bundleRawEnvelope { + out, err := json.Marshal(signed) + require.NoError(t, err) + return out + } + + bundle, err := attestation.BundleFromDSSEEnvelope(signed) + require.NoError(t, err) + + if kind == bundleWithCert { + block, _ := pem.Decode([]byte(chain[0])) + require.NotNil(t, block) + bundle.VerificationMaterial.Content = &protobundle.VerificationMaterial_Certificate{ + Certificate: &v1.X509Certificate{RawBytes: block.Bytes}, + } + } + + out, err := protojson.Marshal(bundle) + require.NoError(t, err) + return out +} + +func TestValidateAttestationContractEnforcesKeylessVerification(t *testing.T) { + orgID := uuid.New() + otherOrgID := uuid.New() + signing := keylessSigningUseCase(t) + + cases := []struct { + name string + bundle []byte + // signature check is expected to reject the attestation + wantRejected bool + }{ + { + name: "keyless certificate issued to the run organization", + bundle: newSignedTestBundle(t, signing, orgID.String(), bundleWithCert), + }, + { + name: "keyless certificate issued to another organization", + bundle: newSignedTestBundle(t, signing, otherOrgID.String(), bundleWithCert), + wantRejected: true, + }, + { + name: "signed without verification material", + bundle: newSignedTestBundle(t, signing, orgID.String(), bundleWithoutMaterial), + wantRejected: true, + }, + { + name: "raw DSSE envelope", + bundle: newSignedTestBundle(t, signing, orgID.String(), bundleRawEnvelope), + wantRejected: true, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + repo := repoM.NewWorkflowRunRepo(t) + uc, err := biz.NewWorkflowRunUseCase(&biz.WorkflowRunUseCaseOpts{WfrRepo: repo, SigningUC: signing}) + require.NoError(t, err) + + runID := uuid.New() + // the run has no contract revision, so an attestation that passes + // the signature check is stopped at the contract check instead + repo.On("FindByID", mock.Anything, runID).Return(&biz.WorkflowRun{ + ID: runID, Workflow: &biz.Workflow{OrgID: orgID}, + }, nil) + + err = uc.ValidateAttestationContract(context.Background(), runID.String(), tc.bundle) + require.Error(t, err) + assert.True(t, biz.IsErrValidation(err), "unexpected error type: %v", err) + if tc.wantRejected { + assert.Contains(t, err.Error(), "attestation verification failed") + return + } + assert.Contains(t, err.Error(), "no contract revision") + }) + } +} + +func TestVerifyRunKeyless(t *testing.T) { + orgID := uuid.New() + signing := keylessSigningUseCase(t) + + cases := []struct { + name string + signing *biz.SigningUseCase + bundle []byte + wantNil bool + wantResult bool + wantReason string + }{ + { + name: "keyless certificate issued to the run organization", + signing: signing, + bundle: newSignedTestBundle(t, signing, orgID.String(), bundleWithCert), + wantResult: true, + }, + { + name: "keyless certificate issued to another organization", + signing: signing, + bundle: newSignedTestBundle(t, signing, uuid.NewString(), bundleWithCert), + wantReason: "organization mismatch", + }, + { + name: "no verification material with keyless signing enabled", + signing: signing, + bundle: newSignedTestBundle(t, signing, orgID.String(), bundleWithoutMaterial), + wantReason: "no verification material", + }, + { + name: "keyless signing not configured", + signing: &biz.SigningUseCase{}, + bundle: newSignedTestBundle(t, signing, orgID.String(), bundleWithoutMaterial), + wantNil: true, + }, + { + name: "run without attestation", + signing: signing, + wantNil: true, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + uc, err := biz.NewWorkflowRunUseCase(&biz.WorkflowRunUseCaseOpts{SigningUC: tc.signing}) + require.NoError(t, err) + + got, err := uc.VerifyRun(context.Background(), &biz.WorkflowRun{ + Workflow: &biz.Workflow{OrgID: orgID}, + Attestation: &biz.Attestation{Bundle: tc.bundle}, + }) + require.NoError(t, err) + if tc.wantNil { + assert.Nil(t, got) + return + } + require.NotNil(t, got) + assert.Equal(t, tc.wantResult, got.Result) + assert.Contains(t, got.FailureReason, tc.wantReason) + }) + } +} + +func TestSigningUseCaseKeylessEnabled(t *testing.T) { + cases := []struct { + name string + uc *biz.SigningUseCase + want bool + }{ + {name: "certificate authorities configured", uc: keylessSigningUseCase(t), want: true}, + {name: "no certificate authorities", uc: &biz.SigningUseCase{}, want: false}, + {name: "nil use case", uc: nil, want: false}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, tc.uc.KeylessEnabled()) + }) + } +} diff --git a/pkg/attestation/verifier/verifier.go b/pkg/attestation/verifier/verifier.go index 5d8ca6b0a..48054144d 100644 --- a/pkg/attestation/verifier/verifier.go +++ b/pkg/attestation/verifier/verifier.go @@ -46,7 +46,31 @@ var ErrInvalidBundle = errors.New("invalid bundle") // 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 { +// ErrOrganizationMismatch indicates the signing certificate was not issued to the +// expected organization, or does not identify a single organization. +var ErrOrganizationMismatch = errors.New("signing certificate organization mismatch") + +type verifyOptions struct { + expectedOrg string +} + +type VerifyOption func(*verifyOptions) + +// WithExpectedOrganization requires the signing certificate to be issued to the +// given organization. Chainloop keyless certificates carry it in the subject +// Organization field. +func WithExpectedOrganization(orgID string) VerifyOption { + return func(o *verifyOptions) { + o.expectedOrg = orgID + } +} + +func VerifyBundle(ctx context.Context, bundleBytes []byte, tr *TrustedRoot, opts ...VerifyOption) error { + options := &verifyOptions{} + for _, opt := range opts { + opt(options) + } + if bundleBytes == nil { return ErrMissingVerificationMaterial } @@ -74,6 +98,11 @@ func VerifyBundle(ctx context.Context, bundleBytes []byte, tr *TrustedRoot) erro if err := verifyCertSignature(ctx, bundle, vc.Certificate(), tr); err != nil { return err } + if options.expectedOrg != "" { + if err := checkCertOrganization(vc.Certificate(), options.expectedOrg); 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) @@ -92,6 +121,19 @@ func VerifyBundle(ctx context.Context, bundleBytes []byte, tr *TrustedRoot) erro return nil } +// checkCertOrganization makes sure the certificate identifies exactly one +// organization, and that it is the expected one. +func checkCertOrganization(cert *x509.Certificate, expected string) error { + orgs := cert.Subject.Organization + if len(orgs) != 1 { + return fmt.Errorf("%w: expected a single organization, found %d", ErrOrganizationMismatch, len(orgs)) + } + if orgs[0] != expected { + return fmt.Errorf("%w: certificate issued to %q", ErrOrganizationMismatch, orgs[0]) + } + return nil +} + // 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 { diff --git a/pkg/attestation/verifier/verifier_test.go b/pkg/attestation/verifier/verifier_test.go index 6da3d9d47..42a6f7c6b 100644 --- a/pkg/attestation/verifier/verifier_test.go +++ b/pkg/attestation/verifier/verifier_test.go @@ -19,6 +19,7 @@ import ( "bytes" "context" "crypto/x509" + "crypto/x509/pkix" "errors" "os" "testing" @@ -119,6 +120,89 @@ func TestVerifyBundle(t *testing.T) { } } +func TestVerifyBundleExpectedOrganization(t *testing.T) { + ca, err := os.ReadFile("testdata/ca.pub") + require.NoError(t, err) + certs, err := cryptoutils.LoadCertificatesFromPEM(bytes.NewReader(ca)) + require.NoError(t, err) + roots := &TrustedRoot{Keys: map[string][]*x509.Certificate{ + "2a522d9652e0933d2a1237c395bc116e012f86dffff13122da59f76e0d2abe27": certs, + }} + + // organization embedded in the signing certificate of bundle_valid.json + const certOrg = "18c3f782-4936-4630-ab8d-20b511366699" + + cases := []struct { + name string + bundle string + opts []VerifyOption + expectSentinel error + }{ + { + name: "matching organization", + bundle: "testdata/bundle_valid.json", + opts: []VerifyOption{WithExpectedOrganization(certOrg)}, + }, + { + name: "different organization", + bundle: "testdata/bundle_valid.json", + opts: []VerifyOption{WithExpectedOrganization("00000000-0000-0000-0000-000000000000")}, + expectSentinel: ErrOrganizationMismatch, + }, + { + name: "no expected organization keeps the previous behavior", + bundle: "testdata/bundle_valid.json", + }, + { + name: "missing material is still reported as such", + bundle: "testdata/bundle_valid_nomaterial.json", + opts: []VerifyOption{WithExpectedOrganization(certOrg)}, + expectSentinel: ErrMissingVerificationMaterial, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + bundleBytes, err := os.ReadFile(tc.bundle) + require.NoError(t, err) + err = VerifyBundle(context.TODO(), bundleBytes, roots, tc.opts...) + if tc.expectSentinel != nil { + require.Error(t, err) + assert.ErrorIs(t, err, tc.expectSentinel) + return + } + assert.NoError(t, err) + }) + } +} + +func TestCheckCertOrganization(t *testing.T) { + const expected = "org-a" + + cases := []struct { + name string + orgs []string + wantErr bool + }{ + {name: "match", orgs: []string{expected}}, + {name: "mismatch", orgs: []string{"org-b"}, wantErr: true}, + {name: "no organization in the certificate", orgs: nil, wantErr: true}, + {name: "several organizations are ambiguous", orgs: []string{expected, "org-b"}, wantErr: true}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + cert := &x509.Certificate{Subject: pkix.Name{Organization: tc.orgs}} + err := checkCertOrganization(cert, expected) + if tc.wantErr { + assert.ErrorIs(t, err, ErrOrganizationMismatch) + return + } + assert.NoError(t, err) + }) + } +} + func TestVerifyTimestamps_TypedErrors(t *testing.T) { ca, err := os.ReadFile("testdata/ca.pub") require.NoError(t, err) From dad3e0e1cf655cf64fc3eb57b45fe0a69ff554b4 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Mon, 5 Oct 2026 18:10:59 +0200 Subject: [PATCH 2/5] fix(controlplane): report an unretrievable attestation bundle as not verified When keyless signing is configured and a run has an attestation digest but its bundle cannot be loaded, the verification result is now a failure instead of no result. Also add tests for a valid keyless certificate with a signature that does not match the payload. Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino Chainloop-Trace-Sessions: 3ed0fe4b-8701-40f8-bb35-c85ce99f6fd1 --- app/controlplane/pkg/biz/workflowrun.go | 11 ++++++- .../pkg/biz/workflowrun_verification_test.go | 32 +++++++++++++++++-- 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/app/controlplane/pkg/biz/workflowrun.go b/app/controlplane/pkg/biz/workflowrun.go index 8fdf650ab..74317ed31 100644 --- a/app/controlplane/pkg/biz/workflowrun.go +++ b/app/controlplane/pkg/biz/workflowrun.go @@ -741,10 +741,19 @@ func (uc *WorkflowRunUseCase) VerifyRun(ctx context.Context, run *WorkflowRun) ( // Without keyless signing there is nothing to verify against, and a run // that has no attestation yet has nothing to verify - if !uc.signingUseCase.KeylessEnabled() || run.Attestation == nil || len(run.Attestation.Bundle) == 0 { + if !uc.signingUseCase.KeylessEnabled() || run.Attestation == nil { return nil, nil } + if len(run.Attestation.Bundle) == 0 { + if run.Attestation.Digest == "" { + return nil, nil + } + // The run has an attestation, but its bundle could not be loaded, for + // example because the CAS backend is unavailable + return &VerificationResult{Result: false, FailureReason: "the attestation bundle could not be retrieved"}, nil + } + opts, err := verifyOptionsForRun(run) if err != nil { return nil, err diff --git a/app/controlplane/pkg/biz/workflowrun_verification_test.go b/app/controlplane/pkg/biz/workflowrun_verification_test.go index f414eb6a5..72d951753 100644 --- a/app/controlplane/pkg/biz/workflowrun_verification_test.go +++ b/app/controlplane/pkg/biz/workflowrun_verification_test.go @@ -61,6 +61,8 @@ const ( bundleWithoutMaterial // a raw DSSE envelope instead of a Sigstore bundle bundleRawEnvelope + // carries a valid keyless certificate, but the signature does not match the payload + bundleWithCertTamperedSignature ) // newSignedTestBundle signs the test attestation with a certificate issued by @@ -82,7 +84,13 @@ func newSignedTestBundle(t *testing.T, signing *biz.SigningUseCase, orgID string chain, err := signing.CreateSigningCert(context.Background(), orgID, pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE REQUEST", Bytes: csrDER})) require.NoError(t, err) - digest := sha256.Sum256(dsse.PAE(env.PayloadType, payload)) + signedPayload := payload + if kind == bundleWithCertTamperedSignature { + // sign other content, so the signature does not match the payload in the envelope + signedPayload = append([]byte("tampered"), payload...) + } + + digest := sha256.Sum256(dsse.PAE(env.PayloadType, signedPayload)) sig, err := ecdsa.SignASN1(rand.Reader, key, digest[:]) require.NoError(t, err) @@ -101,7 +109,7 @@ func newSignedTestBundle(t *testing.T, signing *biz.SigningUseCase, orgID string bundle, err := attestation.BundleFromDSSEEnvelope(signed) require.NoError(t, err) - if kind == bundleWithCert { + if kind == bundleWithCert || kind == bundleWithCertTamperedSignature { block, _ := pem.Decode([]byte(chain[0])) require.NotNil(t, block) bundle.VerificationMaterial.Content = &protobundle.VerificationMaterial_Certificate{ @@ -134,6 +142,11 @@ func TestValidateAttestationContractEnforcesKeylessVerification(t *testing.T) { bundle: newSignedTestBundle(t, signing, otherOrgID.String(), bundleWithCert), wantRejected: true, }, + { + name: "keyless certificate issued to the run organization, with a tampered signature", + bundle: newSignedTestBundle(t, signing, orgID.String(), bundleWithCertTamperedSignature), + wantRejected: true, + }, { name: "signed without verification material", bundle: newSignedTestBundle(t, signing, orgID.String(), bundleWithoutMaterial), @@ -179,6 +192,7 @@ func TestVerifyRunKeyless(t *testing.T) { name string signing *biz.SigningUseCase bundle []byte + digest string wantNil bool wantResult bool wantReason string @@ -195,6 +209,18 @@ func TestVerifyRunKeyless(t *testing.T) { bundle: newSignedTestBundle(t, signing, uuid.NewString(), bundleWithCert), wantReason: "organization mismatch", }, + { + name: "keyless certificate issued to the run organization, with a tampered signature", + signing: signing, + bundle: newSignedTestBundle(t, signing, orgID.String(), bundleWithCertTamperedSignature), + wantReason: "validating the DSSE envelope", + }, + { + name: "attestation digest recorded, but its bundle could not be retrieved", + signing: signing, + digest: "sha256:0f9b2a1c", + wantReason: "could not be retrieved", + }, { name: "no verification material with keyless signing enabled", signing: signing, @@ -221,7 +247,7 @@ func TestVerifyRunKeyless(t *testing.T) { got, err := uc.VerifyRun(context.Background(), &biz.WorkflowRun{ Workflow: &biz.Workflow{OrgID: orgID}, - Attestation: &biz.Attestation{Bundle: tc.bundle}, + Attestation: &biz.Attestation{Bundle: tc.bundle, Digest: tc.digest}, }) require.NoError(t, err) if tc.wantNil { From de4a430b88f4cd66ece27bb1a2781ea168af5b22 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Wed, 7 Oct 2026 11:40:55 +0200 Subject: [PATCH 3/5] feat(controlplane): make forced keyless verification opt-out Add the attestations.force_verification setting, exposed in the Helm chart as controlplane.keylessSigning.forceVerification. It defaults to true. When it is set to false on an instance with keyless signing configured, attestations without verification material, for example signed with cosign keys or KMS, are accepted again. Attestations that carry a certificate must still pass verification. Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino Chainloop-Trace-Sessions: 3ed0fe4b-8701-40f8-bb35-c85ce99f6fd1 --- .../conf/controlplane/config/v1/conf.pb.go | 28 +++++- .../conf/controlplane/config/v1/conf.proto | 9 ++ app/controlplane/pkg/biz/signing.go | 21 +++- app/controlplane/pkg/biz/workflowrun.go | 51 +++++++--- .../pkg/biz/workflowrun_verification_test.go | 95 ++++++++++++++++--- deployment/chainloop/Chart.yaml | 2 +- deployment/chainloop/README.md | 1 + .../templates/controlplane/configmap.yaml | 1 + deployment/chainloop/values.yaml | 2 + 9 files changed, 180 insertions(+), 30 deletions(-) diff --git a/app/controlplane/internal/conf/controlplane/config/v1/conf.pb.go b/app/controlplane/internal/conf/controlplane/config/v1/conf.pb.go index 62b30eb2b..f83a219ba 100644 --- a/app/controlplane/internal/conf/controlplane/config/v1/conf.pb.go +++ b/app/controlplane/internal/conf/controlplane/config/v1/conf.pb.go @@ -343,8 +343,16 @@ type Attestations struct { // control plane defaults to 1 minute. // In YAML configuration, use seconds with an "s" suffix, for example "60s". WorkflowRunExpirationCheckInterval *durationpb.Duration `protobuf:"bytes,4,opt,name=workflow_run_expiration_check_interval,json=workflowRunExpirationCheckInterval,proto3" json:"workflow_run_expiration_check_interval,omitempty"` - unknownFields protoimpl.UnknownFields - sizeCache protoimpl.SizeCache + // Require every pushed attestation to be verified with keyless signing. + // It only applies when keyless signing is configured (certificate_authorities). + // When enabled, an attestation must be signed with a certificate issued by one + // of the configured certificate authorities to the organization that owns the + // workflow run, and other signing methods are rejected. Set it to false to + // also accept attestations signed with other methods, for example cosign keys + // or KMS. When unset, it defaults to true. + ForceVerification *bool `protobuf:"varint,5,opt,name=force_verification,json=forceVerification,proto3,oneof" json:"force_verification,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache } func (x *Attestations) Reset() { @@ -405,6 +413,13 @@ func (x *Attestations) GetWorkflowRunExpirationCheckInterval() *durationpb.Durat return nil } +func (x *Attestations) GetForceVerification() bool { + if x != nil && x.ForceVerification != nil { + return *x.ForceVerification + } + return false +} + type OperationAuthorizationProvider struct { state protoimpl.MessageState `protogen:"open.v1"` // URL of the authorization endpoint @@ -1525,7 +1540,7 @@ type Data_Database struct { state protoimpl.MessageState `protogen:"open.v1"` Driver string `protobuf:"bytes,1,opt,name=driver,proto3" json:"driver,omitempty"` Source string `protobuf:"bytes,2,opt,name=source,proto3" json:"source,omitempty"` - // default 0 + // default 0 MinOpenConns int32 `protobuf:"varint,3,opt,name=min_open_conns,json=minOpenConns,proto3" json:"min_open_conns,omitempty"` // default max(4, runtime.NumCPU()) MaxOpenConns int32 `protobuf:"varint,4,opt,name=max_open_conns,json=maxOpenConns,proto3" json:"max_open_conns,omitempty"` @@ -1881,12 +1896,14 @@ const file_controlplane_config_v1_conf_proto_rawDesc = "" + "\breplicas\x18\x03 \x01(\x05R\breplicasB\x10\n" + "\x0eauthenticationJ\x04\b\b\x10\tR\x15referrer_shared_index\"J\n" + "\x14PluginsNetworkPolicy\x122\n" + - "\x15block_private_targets\x18\x01 \x01(\bR\x13blockPrivateTargets\"\xe7\x02\n" + + "\x15block_private_targets\x18\x01 \x01(\bR\x13blockPrivateTargets\"\xb2\x03\n" + "\fAttestations\x12&\n" + "\x0fskip_db_storage\x18\x01 \x01(\bR\rskipDbStorage\x12L\n" + "#policy_evaluations_max_inline_bytes\x18\x02 \x01(\x03R\x1fpolicyEvaluationsMaxInlineBytes\x12h\n" + "\x1eworkflow_run_expiration_window\x18\x03 \x01(\v2\x19.google.protobuf.DurationB\b\xbaH\x05\xaa\x01\x02*\x00R\x1bworkflowRunExpirationWindow\x12w\n" + - "&workflow_run_expiration_check_interval\x18\x04 \x01(\v2\x19.google.protobuf.DurationB\b\xbaH\x05\xaa\x01\x02*\x00R\"workflowRunExpirationCheckInterval\"v\n" + + "&workflow_run_expiration_check_interval\x18\x04 \x01(\v2\x19.google.protobuf.DurationB\b\xbaH\x05\xaa\x01\x02*\x00R\"workflowRunExpirationCheckInterval\x122\n" + + "\x12force_verification\x18\x05 \x01(\bH\x00R\x11forceVerification\x88\x01\x01B\x15\n" + + "\x13_force_verification\"v\n" + "\x1eOperationAuthorizationProvider\x12\x1a\n" + "\x03url\x18\x01 \x01(\tB\b\xbaH\x05r\x03\x88\x01\x01R\x03url\x12\x18\n" + "\aenabled\x18\x02 \x01(\bR\aenabled\x12\x1e\n" + @@ -2058,6 +2075,7 @@ func file_controlplane_config_v1_conf_proto_init() { if File_controlplane_config_v1_conf_proto != nil { return } + file_controlplane_config_v1_conf_proto_msgTypes[2].OneofWrappers = []any{} file_controlplane_config_v1_conf_proto_msgTypes[10].OneofWrappers = []any{ (*CA_FileCa)(nil), (*CA_EjbcaCa)(nil), diff --git a/app/controlplane/internal/conf/controlplane/config/v1/conf.proto b/app/controlplane/internal/conf/controlplane/config/v1/conf.proto index 9cf52d74d..757856c67 100644 --- a/app/controlplane/internal/conf/controlplane/config/v1/conf.proto +++ b/app/controlplane/internal/conf/controlplane/config/v1/conf.proto @@ -170,6 +170,15 @@ message Attestations { // control plane defaults to 1 minute. // In YAML configuration, use seconds with an "s" suffix, for example "60s". google.protobuf.Duration workflow_run_expiration_check_interval = 4 [(buf.validate.field).duration.gt = {}]; + + // Require every pushed attestation to be verified with keyless signing. + // It only applies when keyless signing is configured (certificate_authorities). + // When enabled, an attestation must be signed with a certificate issued by one + // of the configured certificate authorities to the organization that owns the + // workflow run, and other signing methods are rejected. Set it to false to + // also accept attestations signed with other methods, for example cosign keys + // or KMS. When unset, it defaults to true. + optional bool force_verification = 5; } message OperationAuthorizationProvider { diff --git a/app/controlplane/pkg/biz/signing.go b/app/controlplane/pkg/biz/signing.go index 94daee221..68fc40c92 100644 --- a/app/controlplane/pkg/biz/signing.go +++ b/app/controlplane/pkg/biz/signing.go @@ -43,6 +43,9 @@ type SigningUseCase struct { logger *log.Helper CAs *ca.CertificateAuthorities TimestampAuthorities []*TimestampAuthority + // ForceVerification requires every attestation to be verified with keyless + // signing, when keyless signing is configured + ForceVerification bool } type TimestampAuthority struct { @@ -64,7 +67,17 @@ func NewChainloopSigningUseCase(config *conf.Bootstrap, l log.Logger) (*SigningU return nil, fmt.Errorf("failed to parse CA authorities: %w", err) } - return &SigningUseCase{CAs: cas, TimestampAuthorities: tsas, logger: logger}, nil + // Forced verification is opt-out, so it is enabled unless it is explicitly disabled + forceVerification := true + if a := config.GetAttestations(); a != nil && a.ForceVerification != nil { + forceVerification = *a.ForceVerification + } + + if cas != nil && !forceVerification { + logger.Warn("keyless signing verification is not forced, attestations signed with other methods are accepted") + } + + return &SigningUseCase{CAs: cas, TimestampAuthorities: tsas, logger: logger, ForceVerification: forceVerification}, nil } func parseTimestamps(config *conf.Bootstrap, logger *log.Helper) ([]*TimestampAuthority, error) { @@ -142,6 +155,12 @@ func (s *SigningUseCase) KeylessEnabled() bool { return s != nil && s.CAs != nil } +// VerificationEnforced tells if every attestation must be verified with +// keyless signing, so other signing methods are rejected. +func (s *SigningUseCase) VerificationEnforced() bool { + return s.KeylessEnabled() && s.ForceVerification +} + func (s *SigningUseCase) GetCurrentTSA() *TimestampAuthority { for _, tsa := range s.TimestampAuthorities { if tsa.Issuer { diff --git a/app/controlplane/pkg/biz/workflowrun.go b/app/controlplane/pkg/biz/workflowrun.go index 74317ed31..32daf9e1d 100644 --- a/app/controlplane/pkg/biz/workflowrun.go +++ b/app/controlplane/pkg/biz/workflowrun.go @@ -683,31 +683,52 @@ type VerificationResult struct { // verifyAttestationToStore checks the attestation signature before it is stored. // -// When keyless signing is configured, the attestation MUST be signed with a -// certificate issued by one of the configured certificate authorities to the -// organization that owns the run. Any other signing method is rejected, since -// the control plane has nothing to verify it against. When keyless signing is -// not configured there is nothing to verify against, and no check runs. +// When keyless signing is configured and verification is forced (the default), +// the attestation MUST be signed with a certificate issued by one of the +// configured certificate authorities to the organization that owns the run. +// Any other signing method is rejected, since the control plane has nothing to +// verify it against. +// +// When verification is not forced, an attestation that carries a certificate +// must still pass verification, but an attestation without verification +// material, for example signed with a cosign key or KMS, is accepted. +// +// When keyless signing is not configured there is nothing to verify against, +// and no check runs. func (uc *WorkflowRunUseCase) verifyAttestationToStore(ctx context.Context, run *WorkflowRun, bundle []byte) error { if !uc.signingUseCase.KeylessEnabled() { return nil } - opts, err := verifyOptionsForRun(run) - if err != nil { - return err + enforced := uc.signingUseCase.VerificationEnforced() + + var opts []verifier.VerifyOption + if enforced { + var err error + if opts, err = verifyOptionsForRun(run); err != nil { + return err + } } validation, err := uc.verifyBundle(ctx, bundle, opts...) if err != nil { - if errors.Is(err, verifier.ErrInvalidBundle) { + if !errors.Is(err, verifier.ErrInvalidBundle) { + return err + } + if enforced { return NewErrValidation(fmt.Errorf("attestation verification failed: %w", err)) } - return err + // invalid bundle is expected for old attestations so we skip validation + uc.logger.Warnw("msg", "received an old attestation format, not a bundle: attestation verification skipped", "error", err) + return nil } if validation == nil { - return NewErrValidation(errors.New("attestation verification failed: the attestation must be signed with the keyless signer of this instance, other signing methods are not allowed")) + if enforced { + return NewErrValidation(errors.New("attestation verification failed: the attestation must be signed with the keyless signer of this instance, other signing methods are not allowed")) + } + // not verifiable, and verification is not forced + return nil } if !validation.Result { @@ -745,6 +766,12 @@ func (uc *WorkflowRunUseCase) VerifyRun(ctx context.Context, run *WorkflowRun) ( return nil, nil } + // When verification is not forced, an attestation without verification + // material is reported as not applicable + if !uc.signingUseCase.VerificationEnforced() { + return uc.verifyBundle(ctx, run.Attestation.Bundle) + } + if len(run.Attestation.Bundle) == 0 { if run.Attestation.Digest == "" { return nil, nil @@ -764,7 +791,7 @@ func (uc *WorkflowRunUseCase) VerifyRun(ctx context.Context, run *WorkflowRun) ( return nil, err } - // Keyless signing is enforced, so an attestation that can't be verified + // Verification is enforced, so an attestation that can't be verified // must not be reported as if verification did not apply if vr == nil { return &VerificationResult{Result: false, FailureReason: "the attestation has no verification material"}, nil diff --git a/app/controlplane/pkg/biz/workflowrun_verification_test.go b/app/controlplane/pkg/biz/workflowrun_verification_test.go index 72d951753..a7261a7e2 100644 --- a/app/controlplane/pkg/biz/workflowrun_verification_test.go +++ b/app/controlplane/pkg/biz/workflowrun_verification_test.go @@ -26,13 +26,16 @@ import ( "encoding/base64" "encoding/json" "encoding/pem" + "io" "os" "testing" + conf "github.com/chainloop-dev/chainloop/app/controlplane/internal/conf/controlplane/config/v1" "github.com/chainloop-dev/chainloop/app/controlplane/pkg/biz" repoM "github.com/chainloop-dev/chainloop/app/controlplane/pkg/biz/mocks" ca2 "github.com/chainloop-dev/chainloop/app/controlplane/pkg/ca" "github.com/chainloop-dev/chainloop/pkg/attestation" + "github.com/go-kratos/kratos/v2/log" "github.com/google/uuid" "github.com/secure-systems-lab/go-securesystemslib/dsse" protobundle "github.com/sigstore/protobuf-specs/gen/pb-go/bundle/v1" @@ -41,15 +44,24 @@ import ( "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "google.golang.org/protobuf/encoding/protojson" + "google.golang.org/protobuf/proto" ) // keylessSigningUseCase returns a signing use case backed by an ephemeral CA, -// so keyless signing is enabled. +// so keyless signing is enabled, and verification is forced (the default). func keylessSigningUseCase(t *testing.T) *biz.SigningUseCase { t.Helper() ca, err := NewTestCA() require.NoError(t, err) - return &biz.SigningUseCase{CAs: &ca2.CertificateAuthorities{CAs: []ca2.CertificateAuthority{ca}, SignerCA: ca}} + return &biz.SigningUseCase{CAs: &ca2.CertificateAuthorities{CAs: []ca2.CertificateAuthority{ca}, SignerCA: ca}, ForceVerification: true} +} + +// withoutForcedVerification returns a copy of the signing use case, with the +// same certificate authorities, that does not force verification. +func withoutForcedVerification(uc *biz.SigningUseCase) *biz.SigningUseCase { + optOut := *uc + optOut.ForceVerification = false + return &optOut } type testBundleKind int @@ -127,9 +139,13 @@ func TestValidateAttestationContractEnforcesKeylessVerification(t *testing.T) { otherOrgID := uuid.New() signing := keylessSigningUseCase(t) + optOut := withoutForcedVerification(signing) + cases := []struct { name string bundle []byte + // verification not forced + optOut bool // signature check is expected to reject the attestation wantRejected bool }{ @@ -137,6 +153,22 @@ func TestValidateAttestationContractEnforcesKeylessVerification(t *testing.T) { name: "keyless certificate issued to the run organization", bundle: newSignedTestBundle(t, signing, orgID.String(), bundleWithCert), }, + { + name: "opt-out: signed without verification material is accepted", + bundle: newSignedTestBundle(t, signing, orgID.String(), bundleWithoutMaterial), + optOut: true, + }, + { + name: "opt-out: keyless certificate issued to another organization is accepted", + bundle: newSignedTestBundle(t, signing, otherOrgID.String(), bundleWithCert), + optOut: true, + }, + { + name: "opt-out: keyless certificate with a tampered signature is still rejected", + bundle: newSignedTestBundle(t, signing, orgID.String(), bundleWithCertTamperedSignature), + optOut: true, + wantRejected: true, + }, { name: "keyless certificate issued to another organization", bundle: newSignedTestBundle(t, signing, otherOrgID.String(), bundleWithCert), @@ -161,8 +193,13 @@ func TestValidateAttestationContractEnforcesKeylessVerification(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { + signingUC := signing + if tc.optOut { + signingUC = optOut + } + repo := repoM.NewWorkflowRunRepo(t) - uc, err := biz.NewWorkflowRunUseCase(&biz.WorkflowRunUseCaseOpts{WfrRepo: repo, SigningUC: signing}) + uc, err := biz.NewWorkflowRunUseCase(&biz.WorkflowRunUseCaseOpts{WfrRepo: repo, SigningUC: signingUC}) require.NoError(t, err) runID := uuid.New() @@ -238,6 +275,18 @@ func TestVerifyRunKeyless(t *testing.T) { signing: signing, wantNil: true, }, + { + name: "opt-out: no verification material means verification does not apply", + signing: withoutForcedVerification(signing), + bundle: newSignedTestBundle(t, signing, orgID.String(), bundleWithoutMaterial), + wantNil: true, + }, + { + name: "opt-out: keyless certificate with a tampered signature is not verified", + signing: withoutForcedVerification(signing), + bundle: newSignedTestBundle(t, signing, orgID.String(), bundleWithCertTamperedSignature), + wantReason: "validating the DSSE envelope", + }, } for _, tc := range cases { @@ -261,20 +310,44 @@ func TestVerifyRunKeyless(t *testing.T) { } } -func TestSigningUseCaseKeylessEnabled(t *testing.T) { +func TestSigningUseCaseVerificationEnforced(t *testing.T) { + cases := []struct { + name string + uc *biz.SigningUseCase + wantKeyless bool + wantForced bool + }{ + {name: "certificate authorities configured, verification forced", uc: keylessSigningUseCase(t), wantKeyless: true, wantForced: true}, + {name: "certificate authorities configured, opt-out", uc: withoutForcedVerification(keylessSigningUseCase(t)), wantKeyless: true}, + {name: "no certificate authorities", uc: &biz.SigningUseCase{ForceVerification: true}}, + {name: "nil use case", uc: nil}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.wantKeyless, tc.uc.KeylessEnabled()) + assert.Equal(t, tc.wantForced, tc.uc.VerificationEnforced()) + }) + } +} + +func TestNewChainloopSigningUseCaseForceVerification(t *testing.T) { cases := []struct { - name string - uc *biz.SigningUseCase - want bool + name string + config *conf.Bootstrap + want bool }{ - {name: "certificate authorities configured", uc: keylessSigningUseCase(t), want: true}, - {name: "no certificate authorities", uc: &biz.SigningUseCase{}, want: false}, - {name: "nil use case", uc: nil, want: false}, + {name: "defaults to true when unset", config: &conf.Bootstrap{}, want: true}, + {name: "defaults to true when the attestations section has no value", config: &conf.Bootstrap{Attestations: &conf.Attestations{}}, want: true}, + {name: "explicitly enabled", config: &conf.Bootstrap{Attestations: &conf.Attestations{ForceVerification: proto.Bool(true)}}, want: true}, + {name: "explicitly disabled", config: &conf.Bootstrap{Attestations: &conf.Attestations{ForceVerification: proto.Bool(false)}}, want: false}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - assert.Equal(t, tc.want, tc.uc.KeylessEnabled()) + uc, err := biz.NewChainloopSigningUseCase(tc.config, log.NewStdLogger(io.Discard)) + require.NoError(t, err) + assert.Equal(t, tc.want, uc.ForceVerification) }) } } diff --git a/deployment/chainloop/Chart.yaml b/deployment/chainloop/Chart.yaml index 99e883519..6b46d5048 100644 --- a/deployment/chainloop/Chart.yaml +++ b/deployment/chainloop/Chart.yaml @@ -7,7 +7,7 @@ description: Chainloop is an open source software supply chain control plane, a type: application # Bump the patch (not minor, not major) version on each change in the Chart Source code -version: 1.455.0 +version: 1.455.1 # Do not update appVersion, this is handled automatically by the release process appVersion: v1.115.0 diff --git a/deployment/chainloop/README.md b/deployment/chainloop/README.md index 4847257ed..25eb3392b 100644 --- a/deployment/chainloop/README.md +++ b/deployment/chainloop/README.md @@ -706,6 +706,7 @@ Once done, you can access with [two predefined users](https://github.com/chainlo | Name | Description | Value | | ---------------------------------------------------------------------- | --------------------------------------------------------------------------------------------- | ------- | | `controlplane.keylessSigning.enabled` | Activates or deactivates the feature | `false` | +| `controlplane.keylessSigning.forceVerification` | Requires every attestation to be signed with keyless signing and verified. Set it to false to also accept attestations signed with other methods, for example cosign keys or KMS | `true` | | `controlplane.keylessSigning.backends[0].issuer` | Whether this backend should be used to issue new certificates. Only one can be set at a time. | | | `controlplane.keylessSigning.backends[0].type` | backend type. Only "fileCA" and "ejbcaCA" are supported | | | `controlplane.keylessSigning.backends[0].fileCA.cert` | The PEM-encoded certificate of the file based CA | | diff --git a/deployment/chainloop/templates/controlplane/configmap.yaml b/deployment/chainloop/templates/controlplane/configmap.yaml index f1daddf7f..018742f03 100644 --- a/deployment/chainloop/templates/controlplane/configmap.yaml +++ b/deployment/chainloop/templates/controlplane/configmap.yaml @@ -53,6 +53,7 @@ data: attestations: workflow_run_expiration_window: {{ .Values.controlplane.attestations.workflowRunExpirationWindow | quote }} workflow_run_expiration_check_interval: {{ .Values.controlplane.attestations.workflowRunExpirationCheckInterval | quote }} + force_verification: {{ ne .Values.controlplane.keylessSigning.forceVerification false }} restrict_org_creation: {{ .Values.controlplane.restrictOrgCreation }} {{- if .Values.controlplane.uiDashboardURL }} ui_dashboard_url: {{ .Values.controlplane.uiDashboardURL | quote }} diff --git a/deployment/chainloop/values.yaml b/deployment/chainloop/values.yaml index 98b914390..df7b9048d 100644 --- a/deployment/chainloop/values.yaml +++ b/deployment/chainloop/values.yaml @@ -706,6 +706,7 @@ controlplane: ## Configuration for keyless signing using one of the supported providers ## @param controlplane.keylessSigning.enabled Activates or deactivates the feature + ## @param controlplane.keylessSigning.forceVerification Requires every attestation to be signed with keyless signing and verified. Set it to false to also accept attestations signed with other methods, for example cosign keys or KMS ## @extra controlplane.keylessSigning.backends[0].issuer Whether this backend should be used to issue new certificates. Only one can be set at a time. ## @extra controlplane.keylessSigning.backends[0].type backend type. Only "fileCA" and "ejbcaCA" are supported ## @extra controlplane.keylessSigning.backends[0].fileCA.cert The PEM-encoded certificate of the file based CA @@ -727,6 +728,7 @@ controlplane: ## @extra controlplane.keylessSigning.backends[1].ejbcaCA.caName Name of the CA issuer to use in EJBCA keylessSigning: enabled: false + forceVerification: true # backends: # - type: fileCA # fileCA: From 6d4b612de02130ab2e07a2a4dbdc928e880be702 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Wed, 7 Oct 2026 12:04:31 +0200 Subject: [PATCH 4/5] refactor(controlplane): keep the org check when verification is not forced When forced verification is turned off, an attestation that carries a keyless certificate must still be issued to the organization that owns the run. Also apply the enforcement policy in one place for the push and view paths. With forced verification, viewing an attestation that is not a valid bundle now reports it as not verified instead of failing the request. Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino Chainloop-Trace-Sessions: 3ed0fe4b-8701-40f8-bb35-c85ce99f6fd1 --- app/controlplane/pkg/biz/signing.go | 6 - app/controlplane/pkg/biz/signing_test.go | 18 +- app/controlplane/pkg/biz/workflowrun.go | 101 +++++------- .../pkg/biz/workflowrun_verification_test.go | 156 +++++++++--------- 4 files changed, 125 insertions(+), 156 deletions(-) diff --git a/app/controlplane/pkg/biz/signing.go b/app/controlplane/pkg/biz/signing.go index 68fc40c92..b1d13b48f 100644 --- a/app/controlplane/pkg/biz/signing.go +++ b/app/controlplane/pkg/biz/signing.go @@ -155,12 +155,6 @@ func (s *SigningUseCase) KeylessEnabled() bool { return s != nil && s.CAs != nil } -// VerificationEnforced tells if every attestation must be verified with -// keyless signing, so other signing methods are rejected. -func (s *SigningUseCase) VerificationEnforced() bool { - return s.KeylessEnabled() && s.ForceVerification -} - func (s *SigningUseCase) GetCurrentTSA() *TimestampAuthority { for _, tsa := range s.TimestampAuthorities { if tsa.Issuer { diff --git a/app/controlplane/pkg/biz/signing_test.go b/app/controlplane/pkg/biz/signing_test.go index d81a43523..22322d9f3 100644 --- a/app/controlplane/pkg/biz/signing_test.go +++ b/app/controlplane/pkg/biz/signing_test.go @@ -1,5 +1,5 @@ // -// Copyright 2024 The Chainloop Authors. +// Copyright 2024-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. @@ -27,7 +27,6 @@ import ( "testing" "github.com/chainloop-dev/chainloop/app/controlplane/pkg/biz" - ca2 "github.com/chainloop-dev/chainloop/app/controlplane/pkg/ca" fulcioca "github.com/sigstore/fulcio/pkg/ca" "github.com/sigstore/fulcio/pkg/ca/ephemeralca" "github.com/sigstore/fulcio/pkg/identity" @@ -113,24 +112,23 @@ func TestSuite(t *testing.T) { } func (s *signingUseCaseTestSuite) SetupTest() { - csr, err := createCSR() + _, csr, err := createCSR() s.Require().NoError(err) s.csr = csr - ca, err := NewTestCA() - s.Require().NoError(err) - s.uc = &biz.SigningUseCase{CAs: &ca2.CertificateAuthorities{CAs: []ca2.CertificateAuthority{ca}, SignerCA: ca}} + s.uc = keylessSigningUseCase(s.T()) } -func createCSR() ([]byte, error) { +// createCSR returns a PEM-encoded certificate request and the private key it was created with +func createCSR() (*ecdsa.PrivateKey, []byte, error) { priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) if err != nil { - return nil, fmt.Errorf("generating cert: %w", err) + return nil, nil, fmt.Errorf("generating cert: %w", err) } csrTmpl := &x509.CertificateRequest{Subject: pkix.Name{CommonName: "ephemeral certificate"}} derCSR, err := x509.CreateCertificateRequest(rand.Reader, csrTmpl, priv) if err != nil { - return nil, fmt.Errorf("generating certificate request: %w", err) + return nil, nil, fmt.Errorf("generating certificate request: %w", err) } // Encode CSR to PEM @@ -139,5 +137,5 @@ func createCSR() ([]byte, error) { Bytes: derCSR, }) - return pemCSR, nil + return priv, pemCSR, nil } diff --git a/app/controlplane/pkg/biz/workflowrun.go b/app/controlplane/pkg/biz/workflowrun.go index 32daf9e1d..ce4b995bf 100644 --- a/app/controlplane/pkg/biz/workflowrun.go +++ b/app/controlplane/pkg/biz/workflowrun.go @@ -700,60 +700,57 @@ func (uc *WorkflowRunUseCase) verifyAttestationToStore(ctx context.Context, run return nil } - enforced := uc.signingUseCase.VerificationEnforced() - - var opts []verifier.VerifyOption - if enforced { - var err error - if opts, err = verifyOptionsForRun(run); err != nil { - return err - } - } - - validation, err := uc.verifyBundle(ctx, bundle, opts...) + validation, err := uc.verifyRunBundle(ctx, run, bundle) if err != nil { - if !errors.Is(err, verifier.ErrInvalidBundle) { - return err - } - if enforced { - return NewErrValidation(fmt.Errorf("attestation verification failed: %w", err)) + // only returned when verification is not forced + if errors.Is(err, verifier.ErrInvalidBundle) { + uc.logger.Warnw("msg", "received an old attestation format, not a bundle: attestation verification skipped", "error", err) + return nil } - // invalid bundle is expected for old attestations so we skip validation - uc.logger.Warnw("msg", "received an old attestation format, not a bundle: attestation verification skipped", "error", err) - return nil + return err } - if validation == nil { - if enforced { - return NewErrValidation(errors.New("attestation verification failed: the attestation must be signed with the keyless signer of this instance, other signing methods are not allowed")) - } - // not verifiable, and verification is not forced + if validation == nil || validation.Result { return nil } - if !validation.Result { - // 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 issued to the organization, - // and verification is recomputed on every read, so the result self-heals - // once the configuration catches up. - if !validation.TrustConfigFault { - return 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 issued to the organization, + // and verification is recomputed on every read, so the result self-heals + // once the configuration catches up. + if validation.TrustConfigFault { uc.logger.Warnw("msg", "accepting attestation with an unverifiable timestamp, review the configured TSA certificate chains", "workflowRunID", run.ID.String(), "reason", validation.FailureReason) + return nil } - return nil + return NewErrValidation(fmt.Errorf("attestation verification failed: %s", validation.FailureReason)) } -// verifyOptionsForRun binds the verification to the organization that owns the run. -func verifyOptionsForRun(run *WorkflowRun) ([]verifier.VerifyOption, error) { +// verifyRunBundle verifies a bundle of the run, bound to the run's organization. +// When verification is forced, a bundle that can't be verified is reported as a +// failed verification. Otherwise it is reported as not applicable (nil result), +// or as an ErrInvalidBundle error for data that is not a bundle. +func (uc *WorkflowRunUseCase) verifyRunBundle(ctx context.Context, run *WorkflowRun, bundle []byte) (*VerificationResult, error) { if run.Workflow == nil || run.Workflow.OrgID == uuid.Nil { return nil, fmt.Errorf("workflow run %s has no organization", run.ID) } - return []verifier.VerifyOption{verifier.WithExpectedOrganization(run.Workflow.OrgID.String())}, nil + + vr, err := uc.verifyBundle(ctx, bundle, verifier.WithExpectedOrganization(run.Workflow.OrgID.String())) + if !uc.signingUseCase.ForceVerification { + return vr, err + } + + switch { + case errors.Is(err, verifier.ErrInvalidBundle): + return &VerificationResult{FailureReason: fmt.Sprintf("the attestation is not a valid bundle: %s", err)}, nil + case err == nil && vr == nil: + return &VerificationResult{FailureReason: "the attestation has no verification material, it must be signed with the keyless signer of this instance"}, nil + } + + return vr, err } func (uc *WorkflowRunUseCase) VerifyRun(ctx context.Context, run *WorkflowRun) (*VerificationResult, error) { @@ -766,38 +763,16 @@ func (uc *WorkflowRunUseCase) VerifyRun(ctx context.Context, run *WorkflowRun) ( return nil, nil } - // When verification is not forced, an attestation without verification - // material is reported as not applicable - if !uc.signingUseCase.VerificationEnforced() { - return uc.verifyBundle(ctx, run.Attestation.Bundle) - } - if len(run.Attestation.Bundle) == 0 { - if run.Attestation.Digest == "" { + if run.Attestation.Digest == "" || !uc.signingUseCase.ForceVerification { return nil, nil } // The run has an attestation, but its bundle could not be loaded, for // example because the CAS backend is unavailable - return &VerificationResult{Result: false, FailureReason: "the attestation bundle could not be retrieved"}, nil - } - - opts, err := verifyOptionsForRun(run) - if err != nil { - return nil, err - } - - vr, err := uc.verifyBundle(ctx, run.Attestation.Bundle, opts...) - if err != nil { - return nil, err - } - - // Verification is enforced, so an attestation that can't be verified - // must not be reported as if verification did not apply - if vr == nil { - return &VerificationResult{Result: false, FailureReason: "the attestation has no verification material"}, nil + return &VerificationResult{FailureReason: "the attestation bundle could not be retrieved"}, nil } - return vr, nil + return uc.verifyRunBundle(ctx, run, run.Attestation.Bundle) } func (uc *WorkflowRunUseCase) verifyBundle(ctx context.Context, bundle []byte, opts ...verifier.VerifyOption) (*VerificationResult, error) { diff --git a/app/controlplane/pkg/biz/workflowrun_verification_test.go b/app/controlplane/pkg/biz/workflowrun_verification_test.go index a7261a7e2..4a43bbc84 100644 --- a/app/controlplane/pkg/biz/workflowrun_verification_test.go +++ b/app/controlplane/pkg/biz/workflowrun_verification_test.go @@ -18,11 +18,8 @@ package biz_test import ( "context" "crypto/ecdsa" - "crypto/elliptic" "crypto/rand" "crypto/sha256" - "crypto/x509" - "crypto/x509/pkix" "encoding/base64" "encoding/json" "encoding/pem" @@ -56,14 +53,6 @@ func keylessSigningUseCase(t *testing.T) *biz.SigningUseCase { return &biz.SigningUseCase{CAs: &ca2.CertificateAuthorities{CAs: []ca2.CertificateAuthority{ca}, SignerCA: ca}, ForceVerification: true} } -// withoutForcedVerification returns a copy of the signing use case, with the -// same certificate authorities, that does not force verification. -func withoutForcedVerification(uc *biz.SigningUseCase) *biz.SigningUseCase { - optOut := *uc - optOut.ForceVerification = false - return &optOut -} - type testBundleKind int const ( @@ -89,11 +78,9 @@ func newSignedTestBundle(t *testing.T, signing *biz.SigningUseCase, orgID string payload, err := base64.StdEncoding.DecodeString(env.Payload) require.NoError(t, err) - key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + key, csr, err := createCSR() require.NoError(t, err) - csrDER, err := x509.CreateCertificateRequest(rand.Reader, &x509.CertificateRequest{Subject: pkix.Name{CommonName: "ephemeral certificate"}}, key) - require.NoError(t, err) - chain, err := signing.CreateSigningCert(context.Background(), orgID, pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE REQUEST", Bytes: csrDER})) + chain, err := signing.CreateSigningCert(context.Background(), orgID, csr) require.NoError(t, err) signedPayload := payload @@ -137,69 +124,68 @@ func newSignedTestBundle(t *testing.T, signing *biz.SigningUseCase, orgID string func TestValidateAttestationContractEnforcesKeylessVerification(t *testing.T) { orgID := uuid.New() otherOrgID := uuid.New() - signing := keylessSigningUseCase(t) - - optOut := withoutForcedVerification(signing) + forced := keylessSigningUseCase(t) + optOut := &biz.SigningUseCase{CAs: forced.CAs} cases := []struct { - name string - bundle []byte - // verification not forced - optOut bool + name string + signing *biz.SigningUseCase + bundle []byte // signature check is expected to reject the attestation wantRejected bool }{ { - name: "keyless certificate issued to the run organization", - bundle: newSignedTestBundle(t, signing, orgID.String(), bundleWithCert), + name: "keyless certificate issued to the run organization", + signing: forced, + bundle: newSignedTestBundle(t, forced, orgID.String(), bundleWithCert), }, { - name: "opt-out: signed without verification material is accepted", - bundle: newSignedTestBundle(t, signing, orgID.String(), bundleWithoutMaterial), - optOut: true, + name: "keyless certificate issued to another organization", + signing: forced, + bundle: newSignedTestBundle(t, forced, otherOrgID.String(), bundleWithCert), + wantRejected: true, }, { - name: "opt-out: keyless certificate issued to another organization is accepted", - bundle: newSignedTestBundle(t, signing, otherOrgID.String(), bundleWithCert), - optOut: true, + name: "keyless certificate issued to the run organization, with a tampered signature", + signing: forced, + bundle: newSignedTestBundle(t, forced, orgID.String(), bundleWithCertTamperedSignature), + wantRejected: true, }, { - name: "opt-out: keyless certificate with a tampered signature is still rejected", - bundle: newSignedTestBundle(t, signing, orgID.String(), bundleWithCertTamperedSignature), - optOut: true, + name: "signed without verification material", + signing: forced, + bundle: newSignedTestBundle(t, forced, orgID.String(), bundleWithoutMaterial), wantRejected: true, }, { - name: "keyless certificate issued to another organization", - bundle: newSignedTestBundle(t, signing, otherOrgID.String(), bundleWithCert), + name: "raw DSSE envelope", + signing: forced, + bundle: newSignedTestBundle(t, forced, orgID.String(), bundleRawEnvelope), wantRejected: true, }, { - name: "keyless certificate issued to the run organization, with a tampered signature", - bundle: newSignedTestBundle(t, signing, orgID.String(), bundleWithCertTamperedSignature), - wantRejected: true, + name: "opt-out: signed without verification material is accepted", + signing: optOut, + bundle: newSignedTestBundle(t, forced, orgID.String(), bundleWithoutMaterial), }, { - name: "signed without verification material", - bundle: newSignedTestBundle(t, signing, orgID.String(), bundleWithoutMaterial), + name: "opt-out: keyless certificate issued to another organization is still rejected", + signing: optOut, + bundle: newSignedTestBundle(t, forced, otherOrgID.String(), bundleWithCert), wantRejected: true, }, { - name: "raw DSSE envelope", - bundle: newSignedTestBundle(t, signing, orgID.String(), bundleRawEnvelope), + name: "opt-out: keyless certificate with a tampered signature is still rejected", + signing: optOut, + bundle: newSignedTestBundle(t, forced, orgID.String(), bundleWithCertTamperedSignature), wantRejected: true, }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - signingUC := signing - if tc.optOut { - signingUC = optOut - } - repo := repoM.NewWorkflowRunRepo(t) - uc, err := biz.NewWorkflowRunUseCase(&biz.WorkflowRunUseCaseOpts{WfrRepo: repo, SigningUC: signingUC}) + uc, err := biz.NewWorkflowRunUseCase(&biz.WorkflowRunUseCaseOpts{WfrRepo: repo, SigningUC: tc.signing}) require.NoError(t, err) runID := uuid.New() @@ -223,7 +209,8 @@ func TestValidateAttestationContractEnforcesKeylessVerification(t *testing.T) { func TestVerifyRunKeyless(t *testing.T) { orgID := uuid.New() - signing := keylessSigningUseCase(t) + forced := keylessSigningUseCase(t) + optOut := &biz.SigningUseCase{CAs: forced.CAs} cases := []struct { name string @@ -236,55 +223,73 @@ func TestVerifyRunKeyless(t *testing.T) { }{ { name: "keyless certificate issued to the run organization", - signing: signing, - bundle: newSignedTestBundle(t, signing, orgID.String(), bundleWithCert), + signing: forced, + bundle: newSignedTestBundle(t, forced, orgID.String(), bundleWithCert), wantResult: true, }, { name: "keyless certificate issued to another organization", - signing: signing, - bundle: newSignedTestBundle(t, signing, uuid.NewString(), bundleWithCert), + signing: forced, + bundle: newSignedTestBundle(t, forced, uuid.NewString(), bundleWithCert), wantReason: "organization mismatch", }, { name: "keyless certificate issued to the run organization, with a tampered signature", - signing: signing, - bundle: newSignedTestBundle(t, signing, orgID.String(), bundleWithCertTamperedSignature), + signing: forced, + bundle: newSignedTestBundle(t, forced, orgID.String(), bundleWithCertTamperedSignature), wantReason: "validating the DSSE envelope", }, { name: "attestation digest recorded, but its bundle could not be retrieved", - signing: signing, + signing: forced, digest: "sha256:0f9b2a1c", wantReason: "could not be retrieved", }, { - name: "no verification material with keyless signing enabled", - signing: signing, - bundle: newSignedTestBundle(t, signing, orgID.String(), bundleWithoutMaterial), + name: "no verification material", + signing: forced, + bundle: newSignedTestBundle(t, forced, orgID.String(), bundleWithoutMaterial), wantReason: "no verification material", }, { - name: "keyless signing not configured", - signing: &biz.SigningUseCase{}, - bundle: newSignedTestBundle(t, signing, orgID.String(), bundleWithoutMaterial), - wantNil: true, + name: "raw DSSE envelope", + signing: forced, + bundle: newSignedTestBundle(t, forced, orgID.String(), bundleRawEnvelope), + wantReason: "not a valid bundle", }, { name: "run without attestation", - signing: signing, + signing: forced, + wantNil: true, + }, + { + name: "keyless signing not configured", + signing: &biz.SigningUseCase{ForceVerification: true}, + bundle: newSignedTestBundle(t, forced, orgID.String(), bundleWithoutMaterial), wantNil: true, }, { name: "opt-out: no verification material means verification does not apply", - signing: withoutForcedVerification(signing), - bundle: newSignedTestBundle(t, signing, orgID.String(), bundleWithoutMaterial), + signing: optOut, + bundle: newSignedTestBundle(t, forced, orgID.String(), bundleWithoutMaterial), + wantNil: true, + }, + { + name: "opt-out: attestation bundle could not be retrieved", + signing: optOut, + digest: "sha256:0f9b2a1c", wantNil: true, }, + { + name: "opt-out: keyless certificate issued to another organization is not verified", + signing: optOut, + bundle: newSignedTestBundle(t, forced, uuid.NewString(), bundleWithCert), + wantReason: "organization mismatch", + }, { name: "opt-out: keyless certificate with a tampered signature is not verified", - signing: withoutForcedVerification(signing), - bundle: newSignedTestBundle(t, signing, orgID.String(), bundleWithCertTamperedSignature), + signing: optOut, + bundle: newSignedTestBundle(t, forced, orgID.String(), bundleWithCertTamperedSignature), wantReason: "validating the DSSE envelope", }, } @@ -310,23 +315,20 @@ func TestVerifyRunKeyless(t *testing.T) { } } -func TestSigningUseCaseVerificationEnforced(t *testing.T) { +func TestSigningUseCaseKeylessEnabled(t *testing.T) { cases := []struct { - name string - uc *biz.SigningUseCase - wantKeyless bool - wantForced bool + name string + uc *biz.SigningUseCase + want bool }{ - {name: "certificate authorities configured, verification forced", uc: keylessSigningUseCase(t), wantKeyless: true, wantForced: true}, - {name: "certificate authorities configured, opt-out", uc: withoutForcedVerification(keylessSigningUseCase(t)), wantKeyless: true}, + {name: "certificate authorities configured", uc: keylessSigningUseCase(t), want: true}, {name: "no certificate authorities", uc: &biz.SigningUseCase{ForceVerification: true}}, {name: "nil use case", uc: nil}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - assert.Equal(t, tc.wantKeyless, tc.uc.KeylessEnabled()) - assert.Equal(t, tc.wantForced, tc.uc.VerificationEnforced()) + assert.Equal(t, tc.want, tc.uc.KeylessEnabled()) }) } } From 2e1ffbd768cec6582f38f259d4416bee7f7fe64e Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Wed, 7 Oct 2026 12:32:02 +0200 Subject: [PATCH 5/5] fix(controlplane): check the certificate organization at push time only Viewing a stored run no longer checks that the signing certificate was issued to the run's organization. Runs signed with certificates that do not carry the organization, for example issued by an EJBCA profile that did not map it, do not turn into failed verifications. The check still runs on every push. A run without an organization now fails the push with a validation error. Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino Chainloop-Trace-Sessions: 3ed0fe4b-8701-40f8-bb35-c85ce99f6fd1 --- app/controlplane/pkg/biz/workflowrun.go | 22 ++++++++------- .../pkg/biz/workflowrun_verification_test.go | 27 ++++++++++++------- 2 files changed, 30 insertions(+), 19 deletions(-) diff --git a/app/controlplane/pkg/biz/workflowrun.go b/app/controlplane/pkg/biz/workflowrun.go index ce4b995bf..d21ca04cc 100644 --- a/app/controlplane/pkg/biz/workflowrun.go +++ b/app/controlplane/pkg/biz/workflowrun.go @@ -700,7 +700,12 @@ func (uc *WorkflowRunUseCase) verifyAttestationToStore(ctx context.Context, run return nil } - validation, err := uc.verifyRunBundle(ctx, run, bundle) + // Bind the signing certificate to the organization that owns the run + if run.Workflow == nil || run.Workflow.OrgID == uuid.Nil { + return NewErrValidation(fmt.Errorf("attestation verification failed: workflow run %s has no organization", run.ID)) + } + + validation, err := uc.verifyRunBundle(ctx, bundle, verifier.WithExpectedOrganization(run.Workflow.OrgID.String())) if err != nil { // only returned when verification is not forced if errors.Is(err, verifier.ErrInvalidBundle) { @@ -729,16 +734,12 @@ func (uc *WorkflowRunUseCase) verifyAttestationToStore(ctx context.Context, run return NewErrValidation(fmt.Errorf("attestation verification failed: %s", validation.FailureReason)) } -// verifyRunBundle verifies a bundle of the run, bound to the run's organization. +// verifyRunBundle verifies a bundle and applies the enforcement policy. // When verification is forced, a bundle that can't be verified is reported as a // failed verification. Otherwise it is reported as not applicable (nil result), // or as an ErrInvalidBundle error for data that is not a bundle. -func (uc *WorkflowRunUseCase) verifyRunBundle(ctx context.Context, run *WorkflowRun, bundle []byte) (*VerificationResult, error) { - if run.Workflow == nil || run.Workflow.OrgID == uuid.Nil { - return nil, fmt.Errorf("workflow run %s has no organization", run.ID) - } - - vr, err := uc.verifyBundle(ctx, bundle, verifier.WithExpectedOrganization(run.Workflow.OrgID.String())) +func (uc *WorkflowRunUseCase) verifyRunBundle(ctx context.Context, bundle []byte, opts ...verifier.VerifyOption) (*VerificationResult, error) { + vr, err := uc.verifyBundle(ctx, bundle, opts...) if !uc.signingUseCase.ForceVerification { return vr, err } @@ -772,7 +773,10 @@ func (uc *WorkflowRunUseCase) VerifyRun(ctx context.Context, run *WorkflowRun) ( return &VerificationResult{FailureReason: "the attestation bundle could not be retrieved"}, nil } - return uc.verifyRunBundle(ctx, run, run.Attestation.Bundle) + // The organization binding is enforced at push time only. Stored runs + // signed with certificates that do not carry the organization, for example + // issued by an EJBCA profile that did not map it, must not turn into failures. + return uc.verifyRunBundle(ctx, run.Attestation.Bundle) } func (uc *WorkflowRunUseCase) verifyBundle(ctx context.Context, bundle []byte, opts ...verifier.VerifyOption) (*VerificationResult, error) { diff --git a/app/controlplane/pkg/biz/workflowrun_verification_test.go b/app/controlplane/pkg/biz/workflowrun_verification_test.go index 4a43bbc84..77ea1c2bc 100644 --- a/app/controlplane/pkg/biz/workflowrun_verification_test.go +++ b/app/controlplane/pkg/biz/workflowrun_verification_test.go @@ -131,9 +131,18 @@ func TestValidateAttestationContractEnforcesKeylessVerification(t *testing.T) { name string signing *biz.SigningUseCase bundle []byte + // the run has no organization + noOrg bool // signature check is expected to reject the attestation wantRejected bool }{ + { + name: "run without organization", + signing: forced, + bundle: newSignedTestBundle(t, forced, orgID.String(), bundleWithCert), + noOrg: true, + wantRejected: true, + }, { name: "keyless certificate issued to the run organization", signing: forced, @@ -191,9 +200,11 @@ func TestValidateAttestationContractEnforcesKeylessVerification(t *testing.T) { runID := uuid.New() // the run has no contract revision, so an attestation that passes // the signature check is stopped at the contract check instead - repo.On("FindByID", mock.Anything, runID).Return(&biz.WorkflowRun{ - ID: runID, Workflow: &biz.Workflow{OrgID: orgID}, - }, nil) + run := &biz.WorkflowRun{ID: runID, Workflow: &biz.Workflow{OrgID: orgID}} + if tc.noOrg { + run.Workflow = nil + } + repo.On("FindByID", mock.Anything, runID).Return(run, nil) err = uc.ValidateAttestationContract(context.Background(), runID.String(), tc.bundle) require.Error(t, err) @@ -228,10 +239,12 @@ func TestVerifyRunKeyless(t *testing.T) { wantResult: true, }, { + // the organization is checked at push time only, so stored runs + // signed before the certificate carried the organization still verify name: "keyless certificate issued to another organization", signing: forced, bundle: newSignedTestBundle(t, forced, uuid.NewString(), bundleWithCert), - wantReason: "organization mismatch", + wantResult: true, }, { name: "keyless certificate issued to the run organization, with a tampered signature", @@ -280,12 +293,6 @@ func TestVerifyRunKeyless(t *testing.T) { digest: "sha256:0f9b2a1c", wantNil: true, }, - { - name: "opt-out: keyless certificate issued to another organization is not verified", - signing: optOut, - bundle: newSignedTestBundle(t, forced, uuid.NewString(), bundleWithCert), - wantReason: "organization mismatch", - }, { name: "opt-out: keyless certificate with a tampered signature is not verified", signing: optOut,