Skip to content

Commit

Permalink
fix: allow multiple keys in verifyImages.attestations.attestors.entri…
Browse files Browse the repository at this point in the history
…es (kyverno#8880)

* fix: allow multiple keys in verifyImages.attestations.attestors.entries

Signed-off-by: Vishal Choudhary <[email protected]>

* fix: tests

Signed-off-by: Vishal Choudhary <[email protected]>

---------

Signed-off-by: Vishal Choudhary <[email protected]>
  • Loading branch information
vishal-chdhry authored Jan 22, 2024
1 parent 566db3a commit a0afda4
Show file tree
Hide file tree
Showing 13 changed files with 204 additions and 11 deletions.
31 changes: 20 additions & 11 deletions pkg/engine/internal/imageverifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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 == "" {
Expand All @@ -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), ""
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
apiVersion: v1
kind: Pod
metadata:
name: zulu
namespace: default
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
apiVersion: v1
kind: Pod
metadata:
name: zulu
namespace: default
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
name: check-invalid-attestors
annotations:
pod-policies.kyverno.io/autogen-controllers: none
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit a0afda4

Please sign in to comment.