From 05c7b17d60abbd308efbf84039112b4d5dce391a Mon Sep 17 00:00:00 2001 From: Simon Baird Date: Wed, 3 Jun 2026 15:33:01 -0400 Subject: [PATCH 1/2] Restore deleted snaps after make scenario_% We're going this already for make feature_%. Use the same technique. Co-authored-by: Claude Code --- Makefile | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Makefile b/Makefile index 9225d44d3..5ece74cfb 100644 --- a/Makefile +++ b/Makefile @@ -177,6 +177,10 @@ feature_%: ## Run acceptance tests for a single feature file, e.g. make feature_ # (Replace spaces with underscores in the scenario name.) scenario_%: build ## Run acceptance tests for a single scenario, e.g. make scenario_inline_policy @cd acceptance && go test -test.run 'TestFeatures/$*' + @# With UPDATE_SNAPS=true all the other snap files will be deleted. Let's put them back. + @if [ -n "$$UPDATE_SNAPS" ]; then \ + git ls-files --deleted -- '../features/__snapshots__/*.snap' | xargs -r git checkout --; \ + fi benchmark/%/data.tar.gz: @cd benchmark/$* From c3eb9eee36c5c0aa559b7300ae167c8098f28218 Mon Sep 17 00:00:00 2001 From: Simon Baird Date: Thu, 23 Apr 2026 16:25:55 -0400 Subject: [PATCH 2/2] Add --skip-att-sig-check flag Implement --skip-att-sig-check flag to skip attestation signature validation checks, mirroring the existing --skip-image-sig-check flag. When enabled, attestation signature verification is bypassed during image validation. Why? Often I'm debugging/troubleshooting something and I get given an image ref to look at. We can use cosign download attestation to inspect the attestation, which is very useful, but if we want to try running Conforma against it, we must either guess, find, or ask to be provided with the public key. Sometimes that's not so difficult, but other times it may be very difficult or even impossible. (Consider for example if the image was built on an ephemeral cluster and the signing secret used is gone forever.) Now we can use --skip-image-sig-check and --skip-att-sig-check and carry on with the debugging. Note that we added the --skip-image-sig-check recently for other reasons, see https://redhat.atlassian.net/browse/EC-1647. The --skip-att-sig-check is a little more complicated because we needed to add a new function that can download the attestation without verifying it. The argument against this is that it may encourage less secure practices, but I would say it's acceptable because we're not changing the default behavior, which is always to require signature verification. Ref: https://redhat.atlassian.net/browse/EC-1815 Co-Authored-By: Claude Sonnet 4.5 --- cmd/validate/image.go | 5 + .../modules/ROOT/pages/ec_validate_image.adoc | 1 + features/__snapshots__/validate_image.snap | 136 ++++++++++++++ features/validate_image.feature | 59 ++++++ .../application_snapshot_image.go | 32 +++- .../application_snapshot_image_test.go | 150 +++++++++++++++ internal/image/validate.go | 22 ++- internal/image/validate_test.go | 177 +++++++++++------- internal/policy/policy.go | 8 + internal/policy/policy_test.go | 35 ++++ internal/utils/oci/client.go | 29 +++ internal/utils/oci/fake/client.go | 9 + 12 files changed, 592 insertions(+), 71 deletions(-) diff --git a/cmd/validate/image.go b/cmd/validate/image.go index ac3d12009..8fb17e8f2 100644 --- a/cmd/validate/image.go +++ b/cmd/validate/image.go @@ -211,6 +211,7 @@ func validateImageCmd(validate imageValidationFunc) *cobra.Command { }, IgnoreRekor: data.ignoreRekor, SkipImageSigCheck: data.skipImageSigCheck, + SkipAttSigCheck: data.skipAttSigCheck, PolicyRef: data.policyConfiguration, PublicKey: data.publicKey, RekorURL: data.rekorURL, @@ -499,6 +500,9 @@ func validateImageCmd(validate imageValidationFunc) *cobra.Command { cmd.Flags().BoolVar(&data.skipImageSigCheck, "skip-image-sig-check", data.skipImageSigCheck, "Skip image signature validation checks.") + cmd.Flags().BoolVar(&data.skipAttSigCheck, "skip-att-sig-check", data.skipAttSigCheck, + "Skip attestation signature validation checks.") + cmd.Flags().StringVar(&data.certificateIdentity, "certificate-identity", data.certificateIdentity, "URL of the certificate identity for keyless verification") @@ -637,6 +641,7 @@ type imageData struct { input string ignoreRekor bool skipImageSigCheck bool + skipAttSigCheck bool output []string outputFile string policy policy.Policy diff --git a/docs/modules/ROOT/pages/ec_validate_image.adoc b/docs/modules/ROOT/pages/ec_validate_image.adoc index d2d67263f..2eadb4315 100644 --- a/docs/modules/ROOT/pages/ec_validate_image.adoc +++ b/docs/modules/ROOT/pages/ec_validate_image.adoc @@ -151,6 +151,7 @@ mark (?) sign, for example: --output text=output.txt?show-successes=false * inline JSON ('{sources: {...}, identity: {...}}')") -k, --public-key:: path to the public key. Overrides publicKey from EnterpriseContractPolicy -r, --rekor-url:: Rekor URL. Overrides rekorURL from EnterpriseContractPolicy +--skip-att-sig-check:: Skip attestation signature validation checks. (Default: false) --skip-image-sig-check:: Skip image signature validation checks. (Default: false) --snapshot:: Provide the AppStudio Snapshot as a source of the images to validate, as inline JSON of the "spec" or a reference to a Kubernetes object [/] diff --git a/features/__snapshots__/validate_image.snap b/features/__snapshots__/validate_image.snap index 8aca1ad7d..e5bc417ef 100644 --- a/features/__snapshots__/validate_image.snap +++ b/features/__snapshots__/validate_image.snap @@ -5529,6 +5529,7 @@ Error: success criteria not met --- [TestFeatures/invalid image signature with valid att sig and skip-image-sig-check flag:stderr - 1] +time="${TIMESTAMP}" level=warning msg="Image signature check skipped" --- @@ -5594,6 +5595,7 @@ Error: success criteria not met --- [TestFeatures/happy day with skip-image-sig-check flag:stderr - 1] +time="${TIMESTAMP}" level=warning msg="Image signature check skipped" --- @@ -5780,3 +5782,137 @@ Error: success criteria not met [TestFeatures/discover artifact referrers via OCI Referrers API:stderr - 1] --- + +[TestFeatures/invalid att signature with valid image sig and skip-att-sig-check flag:stdout - 1] +{ + "success": true, + "components": [ + { + "name": "Unnamed", + "containerImage": "${REGISTRY}/acceptance/invalid-att-signature@sha256:${REGISTRY_acceptance/invalid-att-signature:latest_DIGEST}", + "source": {}, + "successes": [ + { + "msg": "Pass", + "metadata": { + "code": "builtin.attestation.syntax_check" + } + }, + { + "msg": "Pass", + "metadata": { + "code": "builtin.image.signature_check" + } + }, + { + "msg": "Pass", + "metadata": { + "code": "main.acceptor" + } + } + ], + "success": true, + "signatures": [ + { + "keyid": "", + "sig": "${IMAGE_SIGNATURE_acceptance/invalid-att-signature}" + } + ], + "attestations": [ + { + "type": "https://in-toto.io/Statement/v0.1", + "predicateType": "https://slsa.dev/provenance/v0.2", + "predicateBuildType": "https://tekton.dev/attestations/chains/pipelinerun@v2", + "signatures": [ + { + "keyid": "", + "sig": "${ATTESTATION_SIGNATURE_acceptance/invalid-att-signature}" + } + ] + } + ] + } + ], + "key": "${known_PUBLIC_KEY_JSON}", + "policy": { + "sources": [ + { + "policy": [ + "git::${GITHOST}/git/invalid-att-signature.git?ref=${LATEST_COMMIT}" + ] + } + ], + "rekorUrl": "${REKOR}", + "publicKey": "${known_PUBLIC_KEY}" + }, + "ec-version": "${EC_VERSION}", + "effective-time": "${TIMESTAMP}" +} +--- + +[TestFeatures/invalid att signature with valid image sig and skip-att-sig-check flag:stderr - 1] +time="${TIMESTAMP}" level=warning msg="Attestation signature check skipped, fetching attestations without verification" + +--- + +[TestFeatures/skip both image and att sig checks for debugging:stdout - 1] +{ + "success": true, + "components": [ + { + "name": "Unnamed", + "containerImage": "${REGISTRY}/acceptance/skip-both-sig-checks@sha256:${REGISTRY_acceptance/skip-both-sig-checks:latest_DIGEST}", + "source": {}, + "successes": [ + { + "msg": "Pass", + "metadata": { + "code": "builtin.attestation.syntax_check" + } + }, + { + "msg": "Pass", + "metadata": { + "code": "main.acceptor" + } + } + ], + "success": true, + "attestations": [ + { + "type": "https://in-toto.io/Statement/v0.1", + "predicateType": "https://slsa.dev/provenance/v0.2", + "predicateBuildType": "https://tekton.dev/attestations/chains/pipelinerun@v2", + "signatures": [ + { + "keyid": "", + "sig": "${ATTESTATION_SIGNATURE_acceptance/skip-both-sig-checks}" + } + ] + } + ] + } + ], + "key": "${known_PUBLIC_KEY_JSON}", + "policy": { + "sources": [ + { + "policy": [ + "git::${GITHOST}/git/skip-both-sig-checks.git?ref=${LATEST_COMMIT}" + ] + } + ], + "rekorUrl": "${REKOR}", + "publicKey": "${known_PUBLIC_KEY}" + }, + "ec-version": "${EC_VERSION}", + "effective-time": "${TIMESTAMP}" +} +--- + +[TestFeatures/skip both image and att sig checks for debugging:stderr - 1] +time="${TIMESTAMP}" level=warning msg="Image signature check skipped" +time="${TIMESTAMP}" level=warning msg="Attestation signature check skipped, fetching attestations without verification" +time="${TIMESTAMP}" level=warning msg="Both --skip-image-sig-check and --skip-att-sig-check are active, all cryptographic verification is disabled" + +--- diff --git a/features/validate_image.feature b/features/validate_image.feature index 373fcb5c1..05899bb12 100644 --- a/features/validate_image.feature +++ b/features/validate_image.feature @@ -56,6 +56,65 @@ Feature: evaluate enterprise contract # builtin.attestation.image_check is not found in the success output Then the output should match the snapshot + Scenario: invalid att signature with valid image sig and skip-att-sig-check flag + Given a key pair named "known" + Given a key pair named "unknown" + Given an image named "acceptance/invalid-att-signature" + + # We're going to use the "known" public key when validating, so the image + # sig check should pass but the att sig check should fail (if it wasn't skipped). + Given a valid image signature of "acceptance/invalid-att-signature" image signed by the "known" key + Given a valid attestation of "acceptance/invalid-att-signature" signed by the "unknown" key + + Given a git repository named "invalid-att-signature" with + | main.rego | examples/happy_day.rego | + Given policy configuration named "invalid-att-signature-policy" with specification + """ + { + "sources": [ + { + "policy": [ + "git::https://${GITHOST}/git/invalid-att-signature.git" + ] + } + ] + } + """ + # Notice we're using the --skip-att-sig-check flag, which is the reason this is + # green even though attestation is signed with a different key to the one we're providing + When ec command is run with "validate image --image ${REGISTRY}/acceptance/invalid-att-signature --policy acceptance/invalid-att-signature-policy --public-key ${known_PUBLIC_KEY} --skip-att-sig-check --rekor-url ${REKOR} --show-successes --output json" + Then the exit status should be 0 + Then the output should match the snapshot + + Scenario: skip both image and att sig checks for debugging + Given a key pair named "known" + Given a key pair named "unknown" + Given an image named "acceptance/skip-both-sig-checks" + + # Both signatures use the wrong key. Without the skip flags this would fail. + Given a valid image signature of "acceptance/skip-both-sig-checks" image signed by the "unknown" key + Given a valid attestation of "acceptance/skip-both-sig-checks" signed by the "unknown" key + + Given a git repository named "skip-both-sig-checks" with + | main.rego | examples/happy_day.rego | + Given policy configuration named "skip-both-sig-checks-policy" with specification + """ + { + "sources": [ + { + "policy": [ + "git::https://${GITHOST}/git/skip-both-sig-checks.git" + ] + } + ] + } + """ + # This is the expected real-world debugging usage: skip all signature checks + # to focus on policy evaluation when the signing key is unavailable + When ec command is run with "validate image --image ${REGISTRY}/acceptance/skip-both-sig-checks --policy acceptance/skip-both-sig-checks-policy --public-key ${known_PUBLIC_KEY} --skip-image-sig-check --skip-att-sig-check --rekor-url ${REKOR} --show-successes --output json" + Then the exit status should be 0 + Then the output should match the snapshot + Scenario: happy day with git config and yaml Given a key pair named "known" Given an image named "acceptance/ec-happy-day" diff --git a/internal/evaluation_target/application_snapshot_image/application_snapshot_image.go b/internal/evaluation_target/application_snapshot_image/application_snapshot_image.go index b96714fc1..7cb95d11a 100644 --- a/internal/evaluation_target/application_snapshot_image/application_snapshot_image.go +++ b/internal/evaluation_target/application_snapshot_image/application_snapshot_image.go @@ -201,9 +201,32 @@ func (a *ApplicationSnapshotImage) ValidateAttestationSignature(ctx context.Cont return a.parseAttestationsFromBundles(layers) } - // Extract the signatures from the attestations here in order to also validate that - // the signatures do exist in the expected format. - for _, sig := range layers { + return a.parseAttestationsFromSignatures(layers) +} + +// FetchAttestationsWithoutVerification fetches attestations from the registry +// without performing signature verification. This is used when --skip-att-sig-check +// is enabled but we still need the attestation data for policy evaluation. +func (a *ApplicationSnapshotImage) FetchAttestationsWithoutVerification(ctx context.Context) error { + if trace.IsEnabled() { + region := trace.StartRegion(ctx, "ec:fetch-attestations-without-verification") + defer region.End() + } + + sigs, err := oci.NewClient(ctx).FetchAttestationLayers(a.reference) + if err != nil { + return err + } + + if a.hasBundles(ctx) { + return a.parseAttestationsFromBundles(sigs) + } + + return a.parseAttestationsFromSignatures(sigs) +} + +func (a *ApplicationSnapshotImage) parseAttestationsFromSignatures(sigs []cosignOCI.Signature) error { + for _, sig := range sigs { att, err := attestation.ProvenanceFromSignature(sig) if err != nil { return fmt.Errorf("unable to parse untyped provenance: %w", err) @@ -283,7 +306,8 @@ func (a *ApplicationSnapshotImage) parseAttestationsFromBundles(layers []cosignO // ValidateAttestationSyntax validates the attestations against known JSON // schemas, errors out if there are no attestations to check to prevent // successful syntax check of no inputs, must invoke -// [ValidateAttestationSignature] to prefill the attestations. +// [ValidateAttestationSignature] or [FetchAttestationsWithoutVerification] +// to prefill the attestations. func (a ApplicationSnapshotImage) ValidateAttestationSyntax(ctx context.Context) error { if trace.IsEnabled() { region := trace.StartRegion(ctx, "ec:validate-attestation-syntax") diff --git a/internal/evaluation_target/application_snapshot_image/application_snapshot_image_test.go b/internal/evaluation_target/application_snapshot_image/application_snapshot_image_test.go index e53a95baf..361bc3a91 100644 --- a/internal/evaluation_target/application_snapshot_image/application_snapshot_image_test.go +++ b/internal/evaluation_target/application_snapshot_image/application_snapshot_image_test.go @@ -1175,6 +1175,156 @@ func createBundleDSSESignature(t *testing.T, statement any) oci.Signature { } // createDSSESignature creates a test signature with a DSSE envelope containing the given statement +func TestParseAttestationsFromSignatures(t *testing.T) { + ref := name.MustParseReference("registry.io/repository/image:tag") + + //nolint:staticcheck + slsaV02Statement := in_toto.ProvenanceStatementSLSA02{ + StatementHeader: in_toto.StatementHeader{ + Type: in_toto.StatementInTotoV01, + PredicateType: v02.PredicateSLSAProvenance, + Subject: []in_toto.Subject{ + { + Name: "test-image", + Digest: common.DigestSet{"sha256": "abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789"}, + }, + }, + }, + Predicate: v02.ProvenancePredicate{ + BuildType: pipelineRunBuildType, + Builder: common.ProvenanceBuilder{ID: "https://tekton.dev/chains/v2"}, + }, + } + + //nolint:staticcheck + slsaV1Statement := in_toto.ProvenanceStatementSLSA1{ + StatementHeader: in_toto.StatementHeader{ + Type: in_toto.StatementInTotoV01, + PredicateType: "https://slsa.dev/provenance/v1", + Subject: []in_toto.Subject{ + { + Name: "test-image", + Digest: common.DigestSet{"sha256": "abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789"}, + }, + }, + }, + Predicate: slsav1.ProvenancePredicate{ + BuildDefinition: slsav1.ProvenanceBuildDefinition{ + BuildType: "https://tekton.dev/attestations/chains/pipelinerun@v2", + ExternalParameters: json.RawMessage(`{}`), + }, + RunDetails: slsav1.ProvenanceRunDetails{ + Builder: slsav1.Builder{ID: "https://tekton.dev/chains/v2"}, + }, + }, + } + + //nolint:staticcheck + spdxStatement := in_toto.Statement{ + StatementHeader: in_toto.StatementHeader{ + Type: in_toto.StatementInTotoV01, + PredicateType: "https://spdx.dev/Document", + Subject: []in_toto.Subject{ + { + Name: "test-image", + Digest: common.DigestSet{"sha256": "abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789"}, + }, + }, + }, + Predicate: json.RawMessage(`{"spdxVersion":"SPDX-2.3"}`), + } + + //nolint:staticcheck + unknownStatement := in_toto.Statement{ + StatementHeader: in_toto.StatementHeader{ + Type: in_toto.StatementInTotoV01, + PredicateType: "https://example.com/unknown/v1", + Subject: []in_toto.Subject{ + { + Name: "test-image", + Digest: common.DigestSet{"sha256": "abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789"}, + }, + }, + }, + Predicate: json.RawMessage(`{"custom":"data"}`), + } + + cases := []struct { + name string + signatures []oci.Signature + expectErr bool + expectedAttCount int + expectedPredTypes []string + }{ + { + name: "no signatures", + signatures: []oci.Signature{}, + expectedAttCount: 0, + }, + { + name: "SLSA v0.2", + signatures: []oci.Signature{createDSSESignature(t, slsaV02Statement)}, + expectedAttCount: 1, + expectedPredTypes: []string{v02.PredicateSLSAProvenance}, + }, + { + name: "SLSA v1.0", + signatures: []oci.Signature{createDSSESignature(t, slsaV1Statement)}, + expectedAttCount: 1, + expectedPredTypes: []string{"https://slsa.dev/provenance/v1"}, + }, + { + name: "SPDX document", + signatures: []oci.Signature{createDSSESignature(t, spdxStatement)}, + expectedAttCount: 1, + expectedPredTypes: []string{"https://spdx.dev/Document"}, + }, + { + name: "unknown predicate type", + signatures: []oci.Signature{createDSSESignature(t, unknownStatement)}, + expectedAttCount: 1, + expectedPredTypes: []string{"https://example.com/unknown/v1"}, + }, + { + name: "multiple mixed types", + signatures: []oci.Signature{ + createDSSESignature(t, slsaV02Statement), + createDSSESignature(t, slsaV1Statement), + createDSSESignature(t, spdxStatement), + createDSSESignature(t, unknownStatement), + }, + expectedAttCount: 4, + expectedPredTypes: []string{ + v02.PredicateSLSAProvenance, + "https://slsa.dev/provenance/v1", + "https://spdx.dev/Document", + "https://example.com/unknown/v1", + }, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + a := ApplicationSnapshotImage{reference: ref} + + err := a.parseAttestationsFromSignatures(tc.signatures) + + if tc.expectErr { + require.Error(t, err) + } else { + require.NoError(t, err) + assert.Equal(t, tc.expectedAttCount, len(a.attestations)) + + if tc.expectedPredTypes != nil { + for i, expectedType := range tc.expectedPredTypes { + assert.Equal(t, expectedType, a.attestations[i].PredicateType()) + } + } + } + }) + } +} + func createDSSESignature(t *testing.T, statement any) oci.Signature { t.Helper() diff --git a/internal/image/validate.go b/internal/image/validate.go index 998b9e0f6..bce4747f7 100644 --- a/internal/image/validate.go +++ b/internal/image/validate.go @@ -19,6 +19,7 @@ package image import ( "context" "encoding/json" + "fmt" "runtime/trace" "sort" "time" @@ -76,14 +77,27 @@ func ValidateImage(ctx context.Context, comp app.SnapshotComponent, snap *app.Sn // Handle image signature validation if p.SkipImageSigCheck() { - log.Debug("Image signature check skipped") + log.Warn("Image signature check skipped") } else { out.SetImageSignatureCheckFromError(a.ValidateImageSignature(ctx)) } - out.SetAttestationSignatureCheckFromError(a.ValidateAttestationSignature(ctx)) - if !out.AttestationSignatureCheck.Passed { - return out, nil + // Handle attestation signature validation + if p.SkipAttSigCheck() { + log.Warn("Attestation signature check skipped, fetching attestations without verification") + if p.SkipImageSigCheck() { + log.Warn("Both --skip-image-sig-check and --skip-att-sig-check are active, all cryptographic verification is disabled") + } + if err := a.FetchAttestationsWithoutVerification(ctx); err != nil { + log.Warnf("Failed to fetch attestations without verification: %v", err) + out.SetAttestationSignatureCheckFromError(fmt.Errorf("failed to fetch attestations (signature check skipped): %w", err)) + return out, nil + } + } else { + out.SetAttestationSignatureCheckFromError(a.ValidateAttestationSignature(ctx)) + if !out.AttestationSignatureCheck.Passed { + return out, nil + } } out.Signatures = a.Signatures() diff --git a/internal/image/validate_test.go b/internal/image/validate_test.go index 1c49077cf..e6df85841 100644 --- a/internal/image/validate_test.go +++ b/internal/image/validate_test.go @@ -574,25 +574,19 @@ func TestValidateImageWithVSACheck_FlagCombinations(t *testing.T) { func TestValidateImageSkipImageSigCheck(t *testing.T) { tests := []struct { - name string - skipImageSigCheck bool - expectImageSigCheckCall bool - expectImageSigResult bool - expectAttSigCheckCall bool + name string + skipImageSigCheck bool + expectImageSigResult bool }{ { - name: "skip image signature check disabled (default)", - skipImageSigCheck: false, - expectImageSigCheckCall: true, - expectImageSigResult: true, - expectAttSigCheckCall: true, + name: "skip image signature check disabled (default)", + skipImageSigCheck: false, + expectImageSigResult: true, }, { - name: "skip image signature check enabled", - skipImageSigCheck: true, - expectImageSigCheckCall: false, - expectImageSigResult: false, - expectAttSigCheckCall: true, + name: "skip image signature check enabled", + skipImageSigCheck: true, + expectImageSigResult: false, }, } @@ -645,66 +639,123 @@ func TestValidateImageSkipImageSigCheck(t *testing.T) { require.NoError(t, err) require.NotNil(t, output) - // Check if ImageSignatureCheck result was set based on expectations if tt.expectImageSigResult { - // When image signature check is NOT skipped, it should have a result assert.NotNil(t, output.ImageSignatureCheck.Result, "Expected ImageSignatureCheck to have a result") } else { - // When image signature check IS skipped, it should not have a result - assert.Nil(t, output.ImageSignatureCheck.Result, "Expected ImageSignatureCheck to not have a result when skipped") + assert.Nil(t, output.ImageSignatureCheck.Result, "Expected no ImageSignatureCheck result when skipped") } - // AttestationSignatureCheck should always have a result (not affected by the flag) assert.NotNil(t, output.AttestationSignatureCheck.Result, "AttestationSignatureCheck should always have a result") - // Verify that skipped checks don't appear in violations or successes - violations := output.Violations() - successes := output.Successes() - if tt.skipImageSigCheck { - // When skipped, image signature check should not appear in violations or successes - for _, violation := range violations { - if violation.Metadata != nil { - code, ok := violation.Metadata["code"].(string) - if ok { - assert.NotEqual(t, "builtin.image.signature_check", code, - "Skipped image signature check should not appear in violations") - } - } - } - - for _, success := range successes { - if success.Metadata != nil { - code, ok := success.Metadata["code"].(string) - if ok { - assert.NotEqual(t, "builtin.image.signature_check", code, - "Skipped image signature check should not appear in successes") - } - } - } + fakeClient.AssertNotCalled(t, "VerifyImageSignatures", mock.Anything, mock.Anything) + } else { + fakeClient.AssertCalled(t, "VerifyImageSignatures", refNoTag, mock.Anything) + } + }) + } +} + +func TestValidateImageSkipAttSigCheck(t *testing.T) { + tests := []struct { + name string + skipAttSigCheck bool + skipImageSigCheck bool + fetchAttErr error + expectAttSigResult bool + expectImgSigResult bool + }{ + { + name: "skip attestation signature check disabled (default)", + skipAttSigCheck: false, + expectAttSigResult: true, + expectImgSigResult: true, + }, + { + name: "skip attestation signature check enabled", + skipAttSigCheck: true, + expectAttSigResult: false, + expectImgSigResult: true, + }, + { + name: "skip attestation signature check with fetch error", + skipAttSigCheck: true, + fetchAttErr: fmt.Errorf("registry unavailable"), + expectAttSigResult: true, + expectImgSigResult: true, + }, + { + name: "skip both image and attestation signature checks", + skipAttSigCheck: true, + skipImageSigCheck: true, + expectAttSigResult: false, + expectImgSigResult: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fs := afero.NewMemMapFs() + ctx := utils.WithFS(context.Background(), fs) + + comp := app.SnapshotComponent{ + ContainerImage: imageRef, + } + + ctx = withImageConfig(ctx, comp.ContainerImage) + client := ecoci.NewClient(ctx) + fakeClient := client.(*fake.FakeClient) + + fakeClient.On("Head", ref).Return(&v1.Descriptor{MediaType: types.OCIManifestSchema1}, nil) + fakeClient.On("HasBundles", mock.Anything, refNoTag).Return(false, nil) + fakeClient.On("VerifyImageSignatures", refNoTag, mock.Anything).Return([]oci.Signature{}, false, fmt.Errorf("no signatures found")) + fakeClient.On("VerifyImageAttestations", refNoTag, mock.Anything).Return([]oci.Signature{}, false, fmt.Errorf("no attestations found")) + if tt.fetchAttErr != nil { + fakeClient.On("FetchAttestationLayers", refNoTag).Return([]oci.Signature(nil), tt.fetchAttErr) + } else { + fakeClient.On("FetchAttestationLayers", refNoTag).Return([]oci.Signature{validAttestation}, nil) + } + + opts := policy.Options{ + EffectiveTime: policy.Now, + SkipAttSigCheck: tt.skipAttSigCheck, + SkipImageSigCheck: tt.skipImageSigCheck, + Identity: cosign.Identity{ + Issuer: "https://example.com/oidc", + Subject: "test@example.com", + }, + } + + updatedPolicy, _, err := policy.PreProcessPolicy(ctx, opts) + require.NoError(t, err) + + snap := &app.SnapshotSpec{} + evaluators := []evaluator.Evaluator{} + + output, err := ValidateImage(ctx, comp, snap, updatedPolicy, evaluators, false) + + require.NoError(t, err) + require.NotNil(t, output) + + if tt.expectAttSigResult { + assert.NotNil(t, output.AttestationSignatureCheck.Result, "Expected AttestationSignatureCheck to have a result") + } else { + assert.Nil(t, output.AttestationSignatureCheck.Result, "Expected no AttestationSignatureCheck result when skipped") } - // Attestation signature check should always appear in results (when it has a result) - foundAttestationCheck := false - for _, violation := range violations { - if violation.Metadata != nil { - code, ok := violation.Metadata["code"].(string) - if ok && code == "builtin.attestation.signature_check" { - foundAttestationCheck = true - break - } - } + if tt.expectImgSigResult { + assert.NotNil(t, output.ImageSignatureCheck.Result, "Expected ImageSignatureCheck to have a result") + } else { + assert.Nil(t, output.ImageSignatureCheck.Result, "Expected no ImageSignatureCheck result when skipped") } - for _, success := range successes { - if success.Metadata != nil { - code, ok := success.Metadata["code"].(string) - if ok && code == "builtin.attestation.signature_check" { - foundAttestationCheck = true - break - } - } + + if tt.skipAttSigCheck { + fakeClient.AssertCalled(t, "FetchAttestationLayers", refNoTag) + fakeClient.AssertNotCalled(t, "VerifyImageAttestations", mock.Anything, mock.Anything) + } else { + fakeClient.AssertCalled(t, "VerifyImageAttestations", refNoTag, mock.Anything) + fakeClient.AssertNotCalled(t, "FetchAttestationLayers", mock.Anything) } - assert.True(t, foundAttestationCheck, "AttestationSignatureCheck should appear in results") }) } } diff --git a/internal/policy/policy.go b/internal/policy/policy.go index b8122bf6c..b76fbd260 100644 --- a/internal/policy/policy.go +++ b/internal/policy/policy.go @@ -76,6 +76,7 @@ type Policy interface { Spec() ecc.EnterpriseContractPolicySpec EffectiveTime() time.Time SkipImageSigCheck() bool + SkipAttSigCheck() bool AttestationTime(time.Time) Identity() cosign.Identity Keyless() bool @@ -91,6 +92,7 @@ type policy struct { identity cosign.Identity ignoreRekor bool skipImageSigCheck bool + skipAttSigCheck bool } // PublicKeyPEM returns the PublicKey in PEM format. When SigVerifier is not @@ -169,6 +171,7 @@ type Options struct { Identity cosign.Identity IgnoreRekor bool SkipImageSigCheck bool + SkipAttSigCheck bool PolicyRef string PublicKey string RekorURL string @@ -266,6 +269,7 @@ func NewPolicy(ctx context.Context, opts Options) (Policy, error) { p.ignoreRekor = opts.IgnoreRekor p.skipImageSigCheck = opts.SkipImageSigCheck + p.skipAttSigCheck = opts.SkipAttSigCheck if opts.PublicKey != "" && opts.PublicKey != p.PublicKey { p.PublicKey = opts.PublicKey @@ -409,6 +413,10 @@ func (p policy) SkipImageSigCheck() bool { return p.skipImageSigCheck } +func (p policy) SkipAttSigCheck() bool { + return p.skipAttSigCheck +} + func isNow(choosenTime string) bool { return strings.EqualFold(choosenTime, Now) } diff --git a/internal/policy/policy_test.go b/internal/policy/policy_test.go index d1a8c3a76..c528ffb31 100644 --- a/internal/policy/policy_test.go +++ b/internal/policy/policy_test.go @@ -1615,6 +1615,41 @@ type FakeCosignClient struct { publicKey string } +func TestSkipAttSigCheck(t *testing.T) { + ctx := context.Background() + ctx = withSignatureClient(ctx, &FakeCosignClient{publicKey: utils.TestPublicKey}) + utils.SetTestRekorPublicKey(t) + utils.SetTestFulcioRoots(t) + utils.SetTestCTLogPublicKey(t) + + t.Run("disabled by default", func(t *testing.T) { + p, err := NewPolicy(ctx, Options{ + PolicyRef: toJson(&ecc.EnterpriseContractPolicySpec{PublicKey: utils.TestPublicKey}), + EffectiveTime: Now, + Identity: cosign.Identity{ + Issuer: "https://example.com/oidc", + Subject: "test@example.com", + }, + }) + require.NoError(t, err) + assert.False(t, p.SkipAttSigCheck()) + }) + + t.Run("enabled when set", func(t *testing.T) { + p, err := NewPolicy(ctx, Options{ + PolicyRef: toJson(&ecc.EnterpriseContractPolicySpec{PublicKey: utils.TestPublicKey}), + EffectiveTime: Now, + SkipAttSigCheck: true, + Identity: cosign.Identity{ + Issuer: "https://example.com/oidc", + Subject: "test@example.com", + }, + }) + require.NoError(t, err) + assert.True(t, p.SkipAttSigCheck()) + }) +} + func (c *FakeCosignClient) publicKeyFromKeyRef(ctx context.Context, publicKey string) (sigstoreSig.Verifier, error) { if strings.Contains(publicKey, "invalid:") { return nil, fmt.Errorf("invalid public key reference format") diff --git a/internal/utils/oci/client.go b/internal/utils/oci/client.go index 3b9f97c66..651f5f510 100644 --- a/internal/utils/oci/client.go +++ b/internal/utils/oci/client.go @@ -76,6 +76,7 @@ func CreateRemoteOptions(ctx context.Context) []remote.Option { type Client interface { VerifyImageSignatures(name.Reference, *cosign.CheckOpts) ([]oci.Signature, bool, error) VerifyImageAttestations(name.Reference, *cosign.CheckOpts) ([]oci.Signature, bool, error) + FetchAttestationLayers(name.Reference) ([]oci.Signature, error) HasBundles(context.Context, name.Reference) (bool, error) Head(name.Reference) (*v1.Descriptor, error) ResolveDigest(name.Reference) (string, error) @@ -130,6 +131,34 @@ func (c *defaultClient) VerifyImageAttestations(ref name.Reference, opts *cosign return cosign.VerifyImageAttestations(c.ctx, ref, opts) } +func (c *defaultClient) FetchAttestationLayers(ref name.Reference) ([]oci.Signature, error) { + if trace.IsEnabled() { + region := trace.StartRegion(c.ctx, "ec:fetch-attestation-layers") + defer region.End() + trace.Logf(c.ctx, "", "image=%q", ref) + } + + signedEntity, err := ociremote.SignedEntity(ref, ociremote.WithRemoteOptions(c.opts...)) + if err != nil { + return nil, fmt.Errorf("failed to fetch signed entity: %w", err) + } + + layers, err := signedEntity.Attestations() + if err != nil { + return nil, fmt.Errorf("failed to fetch attestations: %w", err) + } + if layers == nil { + return nil, nil + } + + sigs, err := layers.Get() + if err != nil { + return nil, fmt.Errorf("failed to get attestation signatures: %w", err) + } + + return sigs, nil +} + func (c *defaultClient) HasBundles(ctx context.Context, ref name.Reference) (bool, error) { regOpts := []ociremote.Option{ociremote.WithRemoteOptions(c.opts...)} bundles, _, err := cosign.GetBundles(ctx, ref, regOpts) diff --git a/internal/utils/oci/fake/client.go b/internal/utils/oci/fake/client.go index b18bcf891..7056aa157 100644 --- a/internal/utils/oci/fake/client.go +++ b/internal/utils/oci/fake/client.go @@ -101,6 +101,15 @@ func (m *FakeClient) VerifyImageAttestations(ref name.Reference, opts *cosign.Ch return sigs, args.Bool(1), args.Error(2) } +func (m *FakeClient) FetchAttestationLayers(ref name.Reference) ([]cosignoci.Signature, error) { + args := m.Called(ref) + var sigs []cosignoci.Signature + if maybeSigs, ok := args.Get(0).([]cosignoci.Signature); ok { + sigs = maybeSigs + } + return sigs, args.Error(1) +} + func (m *FakeClient) HasBundles(ctx context.Context, ref name.Reference) (bool, error) { args := m.Called(ctx, ref) return args.Bool(0), args.Error(1)