From a0afda4f0a8140b3f5da5e30d7307ae42c22f41d Mon Sep 17 00:00:00 2001 From: Vishal Choudhary Date: Mon, 22 Jan 2024 12:19:22 +0530 Subject: [PATCH] fix: allow multiple keys in verifyImages.attestations.attestors.entries (#8880) * fix: allow multiple keys in verifyImages.attestations.attestors.entries Signed-off-by: Vishal Choudhary * fix: tests Signed-off-by: Vishal Choudhary --------- Signed-off-by: Vishal Choudhary --- pkg/engine/internal/imageverifier.go | 31 +++++++++----- .../README.md | 11 +++++ .../chainsaw-test.yaml | 21 ++++++++++ .../pod-assert.yaml | 5 +++ .../pod.yaml | 9 +++++ .../policy-assert.yaml | 7 ++++ .../policy.yaml | 40 +++++++++++++++++++ .../keyless-image-invalid-attestor/README.md | 11 +++++ .../chainsaw-test.yaml | 21 ++++++++++ .../pod-assert.yaml | 5 +++ .../keyless-image-invalid-attestor/pod.yaml | 9 +++++ .../policy-assert.yaml | 7 ++++ .../policy.yaml | 38 ++++++++++++++++++ 13 files changed, 204 insertions(+), 11 deletions(-) create mode 100644 test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-attestation-invalid-attestor/README.md create mode 100644 test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-attestation-invalid-attestor/chainsaw-test.yaml create mode 100644 test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-attestation-invalid-attestor/pod-assert.yaml create mode 100644 test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-attestation-invalid-attestor/pod.yaml create mode 100644 test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-attestation-invalid-attestor/policy-assert.yaml create mode 100644 test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-attestation-invalid-attestor/policy.yaml create mode 100644 test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-image-invalid-attestor/README.md create mode 100644 test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-image-invalid-attestor/chainsaw-test.yaml create mode 100644 test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-image-invalid-attestor/pod-assert.yaml create mode 100644 test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-image-invalid-attestor/pod.yaml create mode 100644 test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-image-invalid-attestor/policy-assert.yaml create mode 100644 test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-image-invalid-attestor/policy.yaml diff --git a/pkg/engine/internal/imageverifier.go b/pkg/engine/internal/imageverifier.go index e9bd2a130ad6..92fe60490d37 100644 --- a/pkg/engine/internal/imageverifier.go +++ b/pkg/engine/internal/imageverifier.go @@ -385,7 +385,7 @@ func (iv *ImageVerifier) verifyAttestations( ) (*engineapi.RuleResponse, string) { image := imageInfo.String() for i, attestation := range imageVerify.Attestations { - var attestationError error + var errorList []error path := fmt.Sprintf(".attestations[%d]", i) iv.logger.V(2).Info(fmt.Sprintf("attestation %+v", attestation)) @@ -408,12 +408,14 @@ func (iv *ImageVerifier) verifyAttestations( verifiedCount := 0 for _, a := range attestor.Entries { + var attestationError error entryPath := fmt.Sprintf("%s.entries[%d]", attestorPath, i) v, opts, subPath := iv.buildVerifier(a, imageVerify, image, &imageVerify.Attestations[i]) cosignResp, err := v.FetchAttestations(ctx, *opts) if err != nil { iv.logger.Error(err, "failed to fetch attestations") - return iv.handleRegistryErrors(image, err), "" + errorList = append(errorList, err) + continue } if imageInfo.Digest == "" { @@ -422,20 +424,27 @@ func (iv *ImageVerifier) verifyAttestations( } attestationError = iv.verifyAttestation(cosignResp.Statements, attestation, imageInfo) - if attestationError != nil { - attestationError = fmt.Errorf("%s: %w", entryPath+subPath, attestationError) - return engineapi.RuleFail(iv.rule.Name, engineapi.ImageVerify, attestationError.Error()), "" - } - verifiedCount++ - if verifiedCount >= requiredCount { - iv.logger.V(2).Info("image attestations verification succeeded", "verifiedCount", verifiedCount, "requiredCount", requiredCount) - break + if attestationError == nil { + verifiedCount++ + if verifiedCount >= requiredCount { + iv.logger.V(2).Info("image attestations verification succeeded", "verifiedCount", verifiedCount, "requiredCount", requiredCount) + break + } + } else { + attestationError = fmt.Errorf("%s: %w", entryPath+subPath, attestationError) + iv.logger.Error(attestationError, "image attestation verification failed") + errorList = append(errorList, attestationError) } } + err := multierr.Combine(errorList...) + errMsg := "attestations verification failed" + if err != nil { + errMsg = err.Error() + } if verifiedCount < requiredCount { - msg := fmt.Sprintf("image attestations verification failed, verifiedCount: %v, requiredCount: %v", verifiedCount, requiredCount) + msg := fmt.Sprintf("image attestations verification failed, verifiedCount: %v, requiredCount: %v, error: %s", verifiedCount, requiredCount, errMsg) return engineapi.RuleFail(iv.rule.Name, engineapi.ImageVerify, msg), "" } } diff --git a/test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-attestation-invalid-attestor/README.md b/test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-attestation-invalid-attestor/README.md new file mode 100644 index 000000000000..7a603fd5cd7d --- /dev/null +++ b/test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-attestation-invalid-attestor/README.md @@ -0,0 +1,11 @@ +## Description + +Verify image attestations with the given predicateType and attestors. The rule has multiple attestors with an invalid attestor + +## Expected Behavior + +The rule has two attestors, first attestor is invalid, second attestor is valid. Since count is set to 1, the pod creation should pass. + +## Reference Issue(s) + +https://github.com/kyverno/kyverno/issues/8842 diff --git a/test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-attestation-invalid-attestor/chainsaw-test.yaml b/test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-attestation-invalid-attestor/chainsaw-test.yaml new file mode 100644 index 000000000000..29271af436b1 --- /dev/null +++ b/test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-attestation-invalid-attestor/chainsaw-test.yaml @@ -0,0 +1,21 @@ +apiVersion: chainsaw.kyverno.io/v1alpha1 +kind: Test +metadata: + creationTimestamp: null + name: keyless-attestation-invalid-attestor +spec: + timeouts: + delete: 2m + steps: + - name: step-01 + try: + - apply: + file: policy.yaml + - assert: + file: policy-assert.yaml + - name: step-02 + try: + - apply: + file: pod.yaml + - assert: + file: pod-assert.yaml \ No newline at end of file diff --git a/test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-attestation-invalid-attestor/pod-assert.yaml b/test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-attestation-invalid-attestor/pod-assert.yaml new file mode 100644 index 000000000000..e47387521843 --- /dev/null +++ b/test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-attestation-invalid-attestor/pod-assert.yaml @@ -0,0 +1,5 @@ +apiVersion: v1 +kind: Pod +metadata: + name: zulu + namespace: default diff --git a/test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-attestation-invalid-attestor/pod.yaml b/test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-attestation-invalid-attestor/pod.yaml new file mode 100644 index 000000000000..921f8ee74797 --- /dev/null +++ b/test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-attestation-invalid-attestor/pod.yaml @@ -0,0 +1,9 @@ +apiVersion: v1 +kind: Pod +metadata: + name: zulu + namespace: default +spec: + containers: + - image: ghcr.io/chipzoller/zulu:v0.0.14 + name: zulu diff --git a/test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-attestation-invalid-attestor/policy-assert.yaml b/test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-attestation-invalid-attestor/policy-assert.yaml new file mode 100644 index 000000000000..0556cd0c33b3 --- /dev/null +++ b/test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-attestation-invalid-attestor/policy-assert.yaml @@ -0,0 +1,7 @@ +--- +apiVersion: kyverno.io/v1 +kind: ClusterPolicy +metadata: + name: check-invalid-attestors-in-attestations + annotations: + pod-policies.kyverno.io/autogen-controllers: none diff --git a/test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-attestation-invalid-attestor/policy.yaml b/test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-attestation-invalid-attestor/policy.yaml new file mode 100644 index 000000000000..8d65e30c395f --- /dev/null +++ b/test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-attestation-invalid-attestor/policy.yaml @@ -0,0 +1,40 @@ +--- +apiVersion: kyverno.io/v1 +kind: ClusterPolicy +metadata: + name: check-invalid-attestors-in-attestations + annotations: + pod-policies.kyverno.io/autogen-controllers: none +spec: + validationFailureAction: Enforce + webhookTimeoutSeconds: 30 + background: false + rules: + - name: check-invalid-attestation-attestor + match: + any: + - resources: + kinds: + - Pod + verifyImages: + - imageReferences: + - "ghcr.io/chipzoller/zulu*" + attestations: + - type: https://slsa.dev/provenance/v0.2 + attestors: + - count: 1 + entries: + - keyless: + subject: "invalid subject" + issuer: "https://token.actions.githubusercontent.com" + rekor: + url: https://rekor.sigstore.dev + ctlog: + ignoreSCT: true + - keyless: + subject: "https://github.com/slsa-framework/slsa-github-generator/.github/workflows/generator_container_slsa3.yml@refs/heads/main" + issuer: "https://token.actions.githubusercontent.com" + rekor: + url: https://rekor.sigstore.dev + ctlog: + ignoreSCT: true diff --git a/test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-image-invalid-attestor/README.md b/test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-image-invalid-attestor/README.md new file mode 100644 index 000000000000..707d74fff727 --- /dev/null +++ b/test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-image-invalid-attestor/README.md @@ -0,0 +1,11 @@ +## Description + +Verify image signature with the given predicateType and attestors. The rule has multiple attestors with an invalid attestor + +## Expected Behavior + +The rule has two attestors, first attestor is invalid, second attestor is valid. Since count is set to 1, the pod creation should pass. + +## Reference Issue(s) + +https://github.com/kyverno/kyverno/issues/8842 diff --git a/test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-image-invalid-attestor/chainsaw-test.yaml b/test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-image-invalid-attestor/chainsaw-test.yaml new file mode 100644 index 000000000000..a369c8db3799 --- /dev/null +++ b/test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-image-invalid-attestor/chainsaw-test.yaml @@ -0,0 +1,21 @@ +apiVersion: chainsaw.kyverno.io/v1alpha1 +kind: Test +metadata: + creationTimestamp: null + name: keyless-image-invalid-attestor +spec: + timeouts: + delete: 2m + steps: + - name: step-01 + try: + - apply: + file: policy.yaml + - assert: + file: policy-assert.yaml + - name: step-02 + try: + - apply: + file: pod.yaml + - assert: + file: pod-assert.yaml \ No newline at end of file diff --git a/test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-image-invalid-attestor/pod-assert.yaml b/test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-image-invalid-attestor/pod-assert.yaml new file mode 100644 index 000000000000..e47387521843 --- /dev/null +++ b/test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-image-invalid-attestor/pod-assert.yaml @@ -0,0 +1,5 @@ +apiVersion: v1 +kind: Pod +metadata: + name: zulu + namespace: default diff --git a/test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-image-invalid-attestor/pod.yaml b/test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-image-invalid-attestor/pod.yaml new file mode 100644 index 000000000000..921f8ee74797 --- /dev/null +++ b/test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-image-invalid-attestor/pod.yaml @@ -0,0 +1,9 @@ +apiVersion: v1 +kind: Pod +metadata: + name: zulu + namespace: default +spec: + containers: + - image: ghcr.io/chipzoller/zulu:v0.0.14 + name: zulu diff --git a/test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-image-invalid-attestor/policy-assert.yaml b/test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-image-invalid-attestor/policy-assert.yaml new file mode 100644 index 000000000000..8d90e075d1b9 --- /dev/null +++ b/test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-image-invalid-attestor/policy-assert.yaml @@ -0,0 +1,7 @@ +--- +apiVersion: kyverno.io/v1 +kind: ClusterPolicy +metadata: + name: check-invalid-attestors + annotations: + pod-policies.kyverno.io/autogen-controllers: none diff --git a/test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-image-invalid-attestor/policy.yaml b/test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-image-invalid-attestor/policy.yaml new file mode 100644 index 000000000000..450d4c7034a4 --- /dev/null +++ b/test/conformance/chainsaw/verifyImages/clusterpolicy/standard/keyless-image-invalid-attestor/policy.yaml @@ -0,0 +1,38 @@ +--- +apiVersion: kyverno.io/v1 +kind: ClusterPolicy +metadata: + name: check-invalid-attestors + annotations: + pod-policies.kyverno.io/autogen-controllers: none +spec: + validationFailureAction: Enforce + webhookTimeoutSeconds: 30 + background: false + rules: + - name: check-invalid-attestors + match: + any: + - resources: + kinds: + - Pod + verifyImages: + - imageReferences: + - "ghcr.io/chipzoller/zulu:*" + attestors: + - count: 1 + entries: + - keyless: + subject: "invalid subject" + issuer: "https://token.actions.githubusercontent.com" + rekor: + url: https://rekor.sigstore.dev + ctlog: + ignoreSCT: true + - keyless: + subject: "https://github.com/chipzoller/zulu/.github/workflows/slsa-generic-keyless.yaml@refs/tags/v*" + issuer: "https://token.actions.githubusercontent.com" + rekor: + url: https://rekor.sigstore.dev + ctlog: + ignoreSCT: true