Skip to content

Commit 2e1ffbd

Browse files
committed
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 <miguel@chainloop.dev> Chainloop-Trace-Sessions: 3ed0fe4b-8701-40f8-bb35-c85ce99f6fd1
1 parent 6d4b612 commit 2e1ffbd

2 files changed

Lines changed: 30 additions & 19 deletions

File tree

‎app/controlplane/pkg/biz/workflowrun.go‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -700,7 +700,12 @@ func (uc *WorkflowRunUseCase) verifyAttestationToStore(ctx context.Context, run
700700
return nil
701701
}
702702

703-
validation, err := uc.verifyRunBundle(ctx, run, bundle)
703+
// Bind the signing certificate to the organization that owns the run
704+
if run.Workflow == nil || run.Workflow.OrgID == uuid.Nil {
705+
return NewErrValidation(fmt.Errorf("attestation verification failed: workflow run %s has no organization", run.ID))
706+
}
707+
708+
validation, err := uc.verifyRunBundle(ctx, bundle, verifier.WithExpectedOrganization(run.Workflow.OrgID.String()))
704709
if err != nil {
705710
// only returned when verification is not forced
706711
if errors.Is(err, verifier.ErrInvalidBundle) {
@@ -729,16 +734,12 @@ func (uc *WorkflowRunUseCase) verifyAttestationToStore(ctx context.Context, run
729734
return NewErrValidation(fmt.Errorf("attestation verification failed: %s", validation.FailureReason))
730735
}
731736

732-
// verifyRunBundle verifies a bundle of the run, bound to the run's organization.
737+
// verifyRunBundle verifies a bundle and applies the enforcement policy.
733738
// When verification is forced, a bundle that can't be verified is reported as a
734739
// failed verification. Otherwise it is reported as not applicable (nil result),
735740
// or as an ErrInvalidBundle error for data that is not a bundle.
736-
func (uc *WorkflowRunUseCase) verifyRunBundle(ctx context.Context, run *WorkflowRun, bundle []byte) (*VerificationResult, error) {
737-
if run.Workflow == nil || run.Workflow.OrgID == uuid.Nil {
738-
return nil, fmt.Errorf("workflow run %s has no organization", run.ID)
739-
}
740-
741-
vr, err := uc.verifyBundle(ctx, bundle, verifier.WithExpectedOrganization(run.Workflow.OrgID.String()))
741+
func (uc *WorkflowRunUseCase) verifyRunBundle(ctx context.Context, bundle []byte, opts ...verifier.VerifyOption) (*VerificationResult, error) {
742+
vr, err := uc.verifyBundle(ctx, bundle, opts...)
742743
if !uc.signingUseCase.ForceVerification {
743744
return vr, err
744745
}
@@ -772,7 +773,10 @@ func (uc *WorkflowRunUseCase) VerifyRun(ctx context.Context, run *WorkflowRun) (
772773
return &VerificationResult{FailureReason: "the attestation bundle could not be retrieved"}, nil
773774
}
774775

775-
return uc.verifyRunBundle(ctx, run, run.Attestation.Bundle)
776+
// The organization binding is enforced at push time only. Stored runs
777+
// signed with certificates that do not carry the organization, for example
778+
// issued by an EJBCA profile that did not map it, must not turn into failures.
779+
return uc.verifyRunBundle(ctx, run.Attestation.Bundle)
776780
}
777781

778782
func (uc *WorkflowRunUseCase) verifyBundle(ctx context.Context, bundle []byte, opts ...verifier.VerifyOption) (*VerificationResult, error) {

‎app/controlplane/pkg/biz/workflowrun_verification_test.go‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -131,9 +131,18 @@ func TestValidateAttestationContractEnforcesKeylessVerification(t *testing.T) {
131131
name string
132132
signing *biz.SigningUseCase
133133
bundle []byte
134+
// the run has no organization
135+
noOrg bool
134136
// signature check is expected to reject the attestation
135137
wantRejected bool
136138
}{
139+
{
140+
name: "run without organization",
141+
signing: forced,
142+
bundle: newSignedTestBundle(t, forced, orgID.String(), bundleWithCert),
143+
noOrg: true,
144+
wantRejected: true,
145+
},
137146
{
138147
name: "keyless certificate issued to the run organization",
139148
signing: forced,
@@ -191,9 +200,11 @@ func TestValidateAttestationContractEnforcesKeylessVerification(t *testing.T) {
191200
runID := uuid.New()
192201
// the run has no contract revision, so an attestation that passes
193202
// the signature check is stopped at the contract check instead
194-
repo.On("FindByID", mock.Anything, runID).Return(&biz.WorkflowRun{
195-
ID: runID, Workflow: &biz.Workflow{OrgID: orgID},
196-
}, nil)
203+
run := &biz.WorkflowRun{ID: runID, Workflow: &biz.Workflow{OrgID: orgID}}
204+
if tc.noOrg {
205+
run.Workflow = nil
206+
}
207+
repo.On("FindByID", mock.Anything, runID).Return(run, nil)
197208

198209
err = uc.ValidateAttestationContract(context.Background(), runID.String(), tc.bundle)
199210
require.Error(t, err)
@@ -228,10 +239,12 @@ func TestVerifyRunKeyless(t *testing.T) {
228239
wantResult: true,
229240
},
230241
{
242+
// the organization is checked at push time only, so stored runs
243+
// signed before the certificate carried the organization still verify
231244
name: "keyless certificate issued to another organization",
232245
signing: forced,
233246
bundle: newSignedTestBundle(t, forced, uuid.NewString(), bundleWithCert),
234-
wantReason: "organization mismatch",
247+
wantResult: true,
235248
},
236249
{
237250
name: "keyless certificate issued to the run organization, with a tampered signature",
@@ -280,12 +293,6 @@ func TestVerifyRunKeyless(t *testing.T) {
280293
digest: "sha256:0f9b2a1c",
281294
wantNil: true,
282295
},
283-
{
284-
name: "opt-out: keyless certificate issued to another organization is not verified",
285-
signing: optOut,
286-
bundle: newSignedTestBundle(t, forced, uuid.NewString(), bundleWithCert),
287-
wantReason: "organization mismatch",
288-
},
289296
{
290297
name: "opt-out: keyless certificate with a tampered signature is not verified",
291298
signing: optOut,

0 commit comments

Comments
 (0)