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/$* 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)