From 76608e315ec9b379409d561763dff7dcf5cc1138 Mon Sep 17 00:00:00 2001 From: Jim Bugwadia Date: Thu, 5 May 2022 14:06:18 -0700 Subject: [PATCH] handle duplicate images; use container name as key (#3779) * handle duplicate images; use container name as key Signed-off-by: Jim Bugwadia * use OldObject for modify requests Signed-off-by: Jim Bugwadia * use unique image names Signed-off-by: Jim Bugwadia * merge main Signed-off-by: Jim Bugwadia * create a single annotation patch across rules and images Signed-off-by: Jim Bugwadia * fmt and change annotation key name Signed-off-by: Jim Bugwadia * fix linter issues Signed-off-by: Jim Bugwadia Co-authored-by: Vyankatesh Kudtarkar Co-authored-by: Sambhav Kothari --- .../kubectl-kyverno/utils/common/common.go | 2 +- pkg/engine/context/context.go | 4 - pkg/engine/imageVerify.go | 138 +++++++----------- pkg/engine/imageVerifyMetadata.go | 97 ++++++++++++ pkg/engine/imageVerifyValidate.go | 57 ++++---- pkg/engine/imageVerify_test.go | 99 ++++++------- pkg/engine/validation.go | 2 +- pkg/policyreport/builder.go | 2 +- pkg/webhooks/server.go | 36 +++-- pkg/webhooks/validate_audit.go | 2 +- pkg/webhooks/verify_images.go | 22 ++- 11 files changed, 269 insertions(+), 192 deletions(-) create mode 100644 pkg/engine/imageVerifyMetadata.go diff --git a/cmd/cli/kubectl-kyverno/utils/common/common.go b/cmd/cli/kubectl-kyverno/utils/common/common.go index 02aa6ab7de..450a55f57c 100644 --- a/cmd/cli/kubectl-kyverno/utils/common/common.go +++ b/cmd/cli/kubectl-kyverno/utils/common/common.go @@ -521,7 +521,7 @@ OuterLoop: engineResponses = append(engineResponses, validateResponse) } - verifyImageResponse := engine.VerifyAndPatchImages(policyContext) + verifyImageResponse, _ := engine.VerifyAndPatchImages(policyContext) if verifyImageResponse != nil && !verifyImageResponse.IsEmpty() { engineResponses = append(engineResponses, verifyImageResponse) info = ProcessValidateEngineResponse(policy, verifyImageResponse, resPath, rc, policyReport) diff --git a/pkg/engine/context/context.go b/pkg/engine/context/context.go index 0e9b45b507..74025997fe 100644 --- a/pkg/engine/context/context.go +++ b/pkg/engine/context/context.go @@ -245,9 +245,6 @@ func (ctx *context) AddImageInfo(info apiutils.ImageInfo) error { } func (ctx *context) AddImageInfos(resource *unstructured.Unstructured) error { - - log.Log.V(4).Info("extracting image info", "obj", resource.UnstructuredContent()) - images, err := apiutils.ExtractImagesFromResource(*resource, nil) if err != nil { return err @@ -258,7 +255,6 @@ func (ctx *context) AddImageInfos(resource *unstructured.Unstructured) error { ctx.images = images log.Log.V(4).Info("updated image info", "images", images) - return addToContext(ctx, images, "images") } diff --git a/pkg/engine/imageVerify.go b/pkg/engine/imageVerify.go index 56c8abc9e3..cb01e20c11 100644 --- a/pkg/engine/imageVerify.go +++ b/pkg/engine/imageVerify.go @@ -26,7 +26,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log" ) -func VerifyAndPatchImages(policyContext *PolicyContext) *response.EngineResponse { +func VerifyAndPatchImages(policyContext *PolicyContext) (*response.EngineResponse, *ImageVerificationMetadata) { resp := &response.EngineResponse{} images := policyContext.JSONContext.ImageInfo() @@ -38,7 +38,9 @@ func VerifyAndPatchImages(policyContext *PolicyContext) *response.EngineResponse startTime := time.Now() defer func() { buildResponse(policyContext, resp, startTime) - logger.V(4).Info("finished policy processing", "processingTime", resp.PolicyResponse.ProcessingTime.String(), "rulesApplied", resp.PolicyResponse.RulesAppliedCount) + logger.V(4).Info("processed image verification rules", + "time", resp.PolicyResponse.ProcessingTime.String(), + "applied", resp.PolicyResponse.RulesAppliedCount, "successful", resp.IsSuccessful()) }() policyContext.JSONContext.Checkpoint() @@ -52,6 +54,7 @@ func VerifyAndPatchImages(policyContext *PolicyContext) *response.EngineResponse } } + ivm := &ImageVerificationMetadata{} rules := autogen.ComputeRules(policyContext.Policy) for i := range rules { rule := &rules[i] @@ -94,6 +97,7 @@ func VerifyAndPatchImages(policyContext *PolicyContext) *response.EngineResponse policyContext: policyContext, rule: ruleCopy, resp: resp, + ivm: ivm, } for _, imageVerify := range ruleCopy.VerifyImages { @@ -101,7 +105,7 @@ func VerifyAndPatchImages(policyContext *PolicyContext) *response.EngineResponse } } - return resp + return resp, ivm } func appendError(resp *response.EngineResponse, rule *v1.Rule, msg string, status response.RuleStatus) { @@ -137,8 +141,11 @@ type imageVerifier struct { policyContext *PolicyContext rule *v1.Rule resp *response.EngineResponse + ivm *ImageVerificationMetadata } +// verify applies policy rules to each matching image. The policy rule results and annotation patches are +// added to tme imageVerifier `resp` and `ivm` fields. func (iv *imageVerifier) verify(imageVerify v1.ImageVerification, images map[string]map[string]apiutils.ImageInfo) { // for backward compatibility imageVerify = *imageVerify.Convert() @@ -152,6 +159,15 @@ func (iv *imageVerifier) verify(imageVerify v1.ImageVerification, images map[str continue } + if hasImageVerifiedAnnotationChanged(iv.policyContext) { + msg := imageVerifyAnnotationKey + " annotation cannot be changed" + iv.logger.Info("image verification error", "reason", msg) + ruleResp := ruleResponse(*iv.rule, response.ImageVerify, msg, response.RuleStatusFail, nil) + iv.resp.PolicyResponse.Rules = append(iv.resp.PolicyResponse.Rules, *ruleResp) + incrementAppliedCount(iv.resp) + continue + } + jmespath := engineUtils.JsonPointerToJMESPath(imageInfo.Pointer) changed, err := iv.policyContext.JSONContext.HasChanged(jmespath) if err == nil && !changed { @@ -170,20 +186,26 @@ func (iv *imageVerifier) verify(imageVerify v1.ImageVerification, images map[str } } - if imageVerify.MutateDigest && imageInfo.Digest == "" { - patch, err := iv.handleMutateDigest(digest, imageInfo) + if imageVerify.MutateDigest { + patch, retrievedDigest, err := iv.handleMutateDigest(digest, imageInfo) if err != nil { ruleResp = ruleError(iv.rule, response.ImageVerify, "failed to update digest", err) - } + } else if patch != nil { + if ruleResp == nil { + ruleResp = ruleResponse(*iv.rule, response.ImageVerify, "mutated image digest", response.RuleStatusPass, nil) + } - if ruleResp != nil { ruleResp.Patches = append(ruleResp.Patches, patch) + imageInfo.Digest = retrievedDigest + image = imageInfo.String() + digest = retrievedDigest } } if ruleResp != nil { - if ruleResp.Status == response.RuleStatusPass { - ruleResp = iv.markImageVerified(imageVerify, ruleResp, digest, imageInfo) + if len(imageVerify.Attestors) > 0 || len(imageVerify.Attestations) > 0 { + verified := ruleResp.Status == response.RuleStatusPass + iv.ivm.add(image, verified) } iv.resp.PolicyResponse.Rules = append(iv.resp.PolicyResponse.Rules, *ruleResp) @@ -193,114 +215,52 @@ func (iv *imageVerifier) verify(imageVerify v1.ImageVerification, images map[str } } -func (iv *imageVerifier) handleMutateDigest(digest string, imageInfo apiutils.ImageInfo) ([]byte, error) { - if digest == "" { - digest, err := fetchImageDigest(imageInfo.String()) - if err != nil { - return nil, err - } +func (iv *imageVerifier) handleMutateDigest(digest string, imageInfo apiutils.ImageInfo) ([]byte, string, error) { + if imageInfo.Digest != "" { + return nil, "", nil + } - imageInfo.Digest = digest + if digest == "" { + var err error + digest, err = fetchImageDigest(imageInfo.String()) + if err != nil { + return nil, "", err + } } patch, err := makeAddDigestPatch(imageInfo, digest) if err != nil { - return nil, err + return nil, "", errors.Wrapf(err, "failed to create image digest patch") } - return patch, nil + iv.logger.V(4).Info("adding digest patch", "image", imageInfo.String(), "patch", string(patch)) + + return patch, digest, nil } -func (iv *imageVerifier) markImageVerified(imageVerify v1.ImageVerification, ruleResp *response.RuleResponse, digest string, imageInfo apiutils.ImageInfo) *response.RuleResponse { - if hasImageVerifiedAnnotationChanged(iv.policyContext, imageInfo.Name, digest) { - msg := "changes to `images.kyverno.io` annotation are not allowed" - return ruleResponse(*iv.rule, response.ImageVerify, msg, response.RuleStatusFail, nil) - } - - if imageVerify.Required { - isImageVerified := ruleResp.Status == response.RuleStatusPass - hasAnnotations := len(iv.policyContext.NewResource.GetAnnotations()) > 0 - patches, err := makeImageVerifiedPatches(imageInfo, digest, isImageVerified, hasAnnotations) - if err != nil { - iv.logger.Error(err, "failed to create patch", "image", imageInfo.String()) - } else { - ruleResp.Patches = append(ruleResp.Patches, patches...) - } - } - - return ruleResp -} - -func hasImageVerifiedAnnotationChanged(ctx *PolicyContext, name, digest string) bool { +func hasImageVerifiedAnnotationChanged(ctx *PolicyContext) bool { if reflect.DeepEqual(ctx.NewResource, &unstructured.Unstructured{}) || reflect.DeepEqual(ctx.OldResource, &unstructured.Unstructured{}) { return false } - key := makeAnnotationKey(name) + key := imageVerifyAnnotationKey newValue := ctx.NewResource.GetAnnotations()[key] oldValue := ctx.OldResource.GetAnnotations()[key] return newValue != oldValue } -func makeImageVerifiedPatches(imageInfo apiutils.ImageInfo, digest string, verified, hasAnnotations bool) ([][]byte, error) { - var patches [][]byte - if !hasAnnotations { - var addAnnotationsPatch = make(map[string]interface{}) - addAnnotationsPatch["op"] = "add" - addAnnotationsPatch["path"] = "/metadata/annotations" - addAnnotationsPatch["value"] = map[string]string{} - patchBytes, err := json.Marshal(addAnnotationsPatch) - if err != nil { - return nil, err - } - - patches = append(patches, patchBytes) - } - - imageData := &ImageVerificationMetadata{ - Verified: verified, - Digest: digest, - } - - data, err := json.Marshal(imageData) - if err != nil { - return nil, errors.Wrapf(err, "failed to marshal metadata value: %v", imageData) - } - - var addKeyPatch = make(map[string]interface{}) - annotationKey := makeAnnotationKeyForJSONPatch(imageInfo.Name) - addKeyPatch["op"] = "add" - addKeyPatch["path"] = annotationKey - addKeyPatch["value"] = string(data) - - patchBytes, err := json.Marshal(addKeyPatch) - if err != nil { - return nil, err - } - - patches = append(patches, patchBytes) - return patches, err -} - -func makeAnnotationKeyForJSONPatch(imageName string) string { - key := makeAnnotationKey(imageName) - return "/metadata/annotations/" + strings.ReplaceAll(key, "/", "~1") -} - -func makeAnnotationKey(imageName string) string { - return fmt.Sprintf("images.kyverno.io/%s", imageName) -} - func fetchImageDigest(ref string) (string, error) { parsedRef, err := name.ParseReference(ref) if err != nil { return "", fmt.Errorf("failed to parse image reference: %s, error: %v", ref, err) } + desc, err := remote.Get(parsedRef, remote.WithAuthFromKeychain(registryclient.DefaultKeychain)) if err != nil { return "", fmt.Errorf("failed to fetch image reference: %s, error: %v", ref, err) } + return desc.Digest.String(), nil } diff --git a/pkg/engine/imageVerifyMetadata.go b/pkg/engine/imageVerifyMetadata.go new file mode 100644 index 0000000000..bc8a97c19d --- /dev/null +++ b/pkg/engine/imageVerifyMetadata.go @@ -0,0 +1,97 @@ +package engine + +import ( + "encoding/json" + "strings" + + "github.com/go-logr/logr" + "github.com/pkg/errors" +) + +const imageVerifyAnnotationKey = "kyverno.io/verify-images" + +type ImageVerificationMetadata struct { + Data map[string]bool `json:"data"` +} + +func (ivm *ImageVerificationMetadata) add(image string, verified bool) { + if ivm.Data == nil { + ivm.Data = make(map[string]bool) + } + + ivm.Data[image] = verified +} + +func (ivm *ImageVerificationMetadata) isVerified(image string) bool { + if ivm.Data == nil { + return false + } + + verified, ok := ivm.Data[image] + if !ok { + return false + } + + return verified +} + +func parseImageMetadata(jsonData string) (*ImageVerificationMetadata, error) { + var data map[string]bool + if err := json.Unmarshal([]byte(jsonData), &data); err != nil { + return nil, err + } + + return &ImageVerificationMetadata{ + Data: data, + }, nil +} + +func (ivm *ImageVerificationMetadata) Patches(hasAnnotations bool, log logr.Logger) ([][]byte, error) { + var patches [][]byte + if !hasAnnotations { + var addAnnotationsPatch = make(map[string]interface{}) + addAnnotationsPatch["op"] = "add" + addAnnotationsPatch["path"] = "/metadata/annotations" + addAnnotationsPatch["value"] = map[string]string{} + patchBytes, err := json.Marshal(addAnnotationsPatch) + if err != nil { + return nil, err + } + + log.V(4).Info("adding annotation patch", "patch", string(patchBytes)) + patches = append(patches, patchBytes) + } + + data, err := json.Marshal(ivm.Data) + if err != nil { + return nil, errors.Wrapf(err, "failed to marshal metadata value: %v", data) + } + + var addKeyPatch = make(map[string]interface{}) + addKeyPatch["op"] = "add" + addKeyPatch["path"] = makeAnnotationKeyForJSONPatch() + addKeyPatch["value"] = string(data) + + patchBytes, err := json.Marshal(addKeyPatch) + if err != nil { + return nil, err + } + + log.V(4).Info("adding image verification patch", "patch", string(patchBytes)) + patches = append(patches, patchBytes) + return patches, nil +} + +func (ivm *ImageVerificationMetadata) Merge(other *ImageVerificationMetadata) { + for k, v := range other.Data { + ivm.add(k, v) + } +} + +func (ivm *ImageVerificationMetadata) IsEmpty() bool { + return len(ivm.Data) == 0 +} + +func makeAnnotationKeyForJSONPatch() string { + return "/metadata/annotations/" + strings.ReplaceAll(imageVerifyAnnotationKey, "/", "~1") +} diff --git a/pkg/engine/imageVerifyValidate.go b/pkg/engine/imageVerifyValidate.go index 917edc311e..b252555cab 100644 --- a/pkg/engine/imageVerifyValidate.go +++ b/pkg/engine/imageVerifyValidate.go @@ -1,14 +1,11 @@ package engine import ( - "encoding/json" "fmt" "reflect" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "sigs.k8s.io/controller-runtime/pkg/log" - "github.com/go-logr/logr" gojmespath "github.com/jmespath/go-jmespath" kyverno "github.com/kyverno/kyverno/api/kyverno/v1" @@ -18,6 +15,11 @@ import ( ) func processImageValidationRule(log logr.Logger, ctx *PolicyContext, rule *kyverno.Rule) *response.RuleResponse { + if isDeleteRequest(ctx) { + return nil + } + + log = log.WithValues("rule", rule.Name) if err := LoadContext(log, rule.Context, ctx, rule.Name); err != nil { if _, ok := err.(gojmespath.NotFoundError); ok { log.V(3).Info("failed to load context", "reason", err.Error()) @@ -44,32 +46,36 @@ func processImageValidationRule(log logr.Logger, ctx *PolicyContext, rule *kyver for _, v := range rule.VerifyImages { imageVerify := v.Convert() for _, infoMap := range ctx.JSONContext.ImageInfo() { - for _, imageInfo := range infoMap { + for name, imageInfo := range infoMap { image := imageInfo.String() + log = log.WithValues("rule", rule.Name) + if !imageMatches(image, imageVerify.ImageReferences) { - log.V(4).Info("image does not match pattern", "image", image, "patterns", imageVerify.ImageReferences) + log.V(4).Info("image does not match", "imageReferences", imageVerify.ImageReferences) return nil } - if err := validateImage(ctx, imageVerify, imageInfo); err != nil { + log.V(4).Info("validating image", "image", image) + if err := validateImage(ctx, imageVerify, name, imageInfo, log); err != nil { return ruleResponse(*rule, response.ImageVerify, err.Error(), response.RuleStatusFail, nil) } } } } + log.V(4).Info("validated image", "rule", rule.Name) return ruleResponse(*rule, response.Validation, "image verified", response.RuleStatusPass, nil) } -func validateImage(ctx *PolicyContext, imageVerify *kyverno.ImageVerification, imageInfo apiutils.ImageInfo) error { +func validateImage(ctx *PolicyContext, imageVerify *kyverno.ImageVerification, name string, imageInfo apiutils.ImageInfo, log logr.Logger) error { image := imageInfo.String() if imageVerify.VerifyDigest && imageInfo.Digest == "" { - log.Log.Info("missing digest", "request.object", ctx.NewResource.UnstructuredContent()) + log.Info("missing digest", "image", imageInfo.String()) return fmt.Errorf("missing digest for %s", image) } if imageVerify.Required && !reflect.DeepEqual(ctx.NewResource, unstructured.Unstructured{}) { - verified, err := isImageVerified(ctx, imageInfo) + verified, err := isImageVerified(ctx.NewResource, image, log) if err != nil { return err } @@ -82,39 +88,28 @@ func validateImage(ctx *PolicyContext, imageVerify *kyverno.ImageVerification, i return nil } -type ImageVerificationMetadata struct { - Verified bool `json:"verified,omitempty"` - Digest string `json:"digest,omitempty"` -} - -func isImageVerified(ctx *PolicyContext, imageInfo apiutils.ImageInfo) (bool, error) { - if reflect.DeepEqual(ctx.NewResource, unstructured.Unstructured{}) { - return false, errors.Errorf("resource does not exist") +func isImageVerified(resource unstructured.Unstructured, image string, log logr.Logger) (bool, error) { + if reflect.DeepEqual(resource, unstructured.Unstructured{}) { + return false, errors.Errorf("nil resource") } - annotations := ctx.NewResource.GetAnnotations() + annotations := resource.GetAnnotations() if len(annotations) == 0 { return false, nil } - key := makeAnnotationKey(imageInfo.Name) + key := imageVerifyAnnotationKey data, ok := annotations[key] if !ok { + log.V(2).Info("missing image metadata in annotation", "key", key) return false, errors.Errorf("image is not verified") } - var ivm ImageVerificationMetadata - if err := json.Unmarshal([]byte(data), &ivm); err != nil { - return false, errors.Wrapf(err, "failed to extract image metadata") + ivm, err := parseImageMetadata(data) + if err != nil { + log.Error(err, "failed to parse image verification metadata", "data", data) + return false, errors.Wrapf(err, "failed to parse image metadata") } - if !ivm.Verified { - return false, nil - } - - if imageInfo.Digest != ivm.Digest { - return false, errors.Errorf("failed to verify image; digest mismatch") - } - - return true, nil + return ivm.isVerified(image), nil } diff --git a/pkg/engine/imageVerify_test.go b/pkg/engine/imageVerify_test.go index 7c8a744f1e..7cc2d6f007 100644 --- a/pkg/engine/imageVerify_test.go +++ b/pkg/engine/imageVerify_test.go @@ -14,7 +14,6 @@ import ( "github.com/kyverno/kyverno/pkg/engine/context" "github.com/kyverno/kyverno/pkg/engine/response" "github.com/kyverno/kyverno/pkg/engine/utils" - apiutils "github.com/kyverno/kyverno/pkg/utils/api" "gotest.tools/assert" ) @@ -141,13 +140,14 @@ var payloads = [][]byte{ func Test_CosignMockAttest(t *testing.T) { policyContext := buildContext(t, testPolicyGood, testResource, "") - err := cosign.SetMock("ghcr.io/jimbugwadia/pause2:latest", payloads) assert.NilError(t, err) - er := VerifyAndPatchImages(policyContext) + er, ivm := VerifyAndPatchImages(policyContext) assert.Equal(t, len(er.PolicyResponse.Rules), 1) assert.Equal(t, er.PolicyResponse.Rules[0].Status, response.RuleStatusPass) + assert.Equal(t, ivm.IsEmpty(), false) + assert.Equal(t, ivm.isVerified("ghcr.io/jimbugwadia/pause2:latest"), true) } func Test_CosignMockAttest_fail(t *testing.T) { @@ -155,7 +155,7 @@ func Test_CosignMockAttest_fail(t *testing.T) { err := cosign.SetMock("ghcr.io/jimbugwadia/pause2:latest", payloads) assert.NilError(t, err) - er := VerifyAndPatchImages(policyContext) + er, _ := VerifyAndPatchImages(policyContext) assert.Equal(t, len(er.PolicyResponse.Rules), 1) assert.Equal(t, er.PolicyResponse.Rules[0].Status, response.RuleStatusFail) } @@ -321,7 +321,7 @@ var testOtherKey = `-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcD func Test_SignatureGoodSigned(t *testing.T) { policyContext := buildContext(t, testSampleSingleKeyPolicy, testSampleResource, "") cosign.ClearMock() - err := VerifyAndPatchImages(policyContext) + err, _ := VerifyAndPatchImages(policyContext) assert.Equal(t, len(err.PolicyResponse.Rules), 1) assert.Equal(t, err.PolicyResponse.Rules[0].Status, response.RuleStatusPass, err.PolicyResponse.Rules[0].Message) } @@ -330,7 +330,7 @@ func Test_SignatureUnsigned(t *testing.T) { cosign.ClearMock() unsigned := strings.Replace(testSampleResource, ":signed", ":unsigned", -1) policyContext := buildContext(t, testSampleSingleKeyPolicy, unsigned, "") - err := VerifyAndPatchImages(policyContext) + err, _ := VerifyAndPatchImages(policyContext) assert.Equal(t, len(err.PolicyResponse.Rules), 1) assert.Equal(t, err.PolicyResponse.Rules[0].Status, response.RuleStatusFail, err.PolicyResponse.Rules[0].Message) } @@ -339,7 +339,7 @@ func Test_SignatureWrongKey(t *testing.T) { cosign.ClearMock() otherKey := strings.Replace(testSampleResource, ":signed", ":signed-by-someone-else", -1) policyContext := buildContext(t, testSampleSingleKeyPolicy, otherKey, "") - err := VerifyAndPatchImages(policyContext) + err, _ := VerifyAndPatchImages(policyContext) assert.Equal(t, len(err.PolicyResponse.Rules), 1) assert.Equal(t, err.PolicyResponse.Rules[0].Status, response.RuleStatusFail, err.PolicyResponse.Rules[0].Message) } @@ -350,7 +350,7 @@ func Test_SignaturesMultiKey(t *testing.T) { policy = strings.Replace(policy, "KEY2", testVerifyImageKey, -1) policy = strings.Replace(policy, "COUNT", "0", -1) policyContext := buildContext(t, policy, testSampleResource, "") - err := VerifyAndPatchImages(policyContext) + err, _ := VerifyAndPatchImages(policyContext) assert.Equal(t, len(err.PolicyResponse.Rules), 1) assert.Equal(t, err.PolicyResponse.Rules[0].Status, response.RuleStatusPass, err.PolicyResponse.Rules[0].Message) } @@ -360,7 +360,7 @@ func Test_SignaturesMultiKeyFail(t *testing.T) { policy := strings.Replace(testSampleMultipleKeyPolicy, "KEY1", testVerifyImageKey, -1) policy = strings.Replace(policy, "COUNT", "0", -1) policyContext := buildContext(t, policy, testSampleResource, "") - err := VerifyAndPatchImages(policyContext) + err, _ := VerifyAndPatchImages(policyContext) assert.Equal(t, len(err.PolicyResponse.Rules), 1) assert.Equal(t, err.PolicyResponse.Rules[0].Status, response.RuleStatusFail, err.PolicyResponse.Rules[0].Message) } @@ -371,7 +371,7 @@ func Test_SignaturesMultiKeyOneGoodKey(t *testing.T) { policy = strings.Replace(policy, "KEY2", testOtherKey, -1) policy = strings.Replace(policy, "COUNT", "1", -1) policyContext := buildContext(t, policy, testSampleResource, "") - err := VerifyAndPatchImages(policyContext) + err, _ := VerifyAndPatchImages(policyContext) assert.Equal(t, len(err.PolicyResponse.Rules), 1) assert.Equal(t, err.PolicyResponse.Rules[0].Status, response.RuleStatusPass, err.PolicyResponse.Rules[0].Message) } @@ -382,7 +382,7 @@ func Test_SignaturesMultiKeyZeroGoodKey(t *testing.T) { policy = strings.Replace(policy, "KEY2", testOtherKey, -1) policy = strings.Replace(policy, "COUNT", "1", -1) policyContext := buildContext(t, policy, testSampleResource, "") - err := VerifyAndPatchImages(policyContext) + err, _ := VerifyAndPatchImages(policyContext) assert.Equal(t, len(err.PolicyResponse.Rules), 1) assert.Equal(t, err.PolicyResponse.Rules[0].Status, response.RuleStatusFail, err.PolicyResponse.Rules[0].Message) } @@ -455,7 +455,7 @@ func Test_NestedAttestors(t *testing.T) { policy = strings.Replace(policy, "KEY2", testVerifyImageKey, -1) policy = strings.Replace(policy, "COUNT", "0", -1) policyContext := buildContext(t, policy, testSampleResource, "") - err := VerifyAndPatchImages(policyContext) + err, _ := VerifyAndPatchImages(policyContext) assert.Equal(t, len(err.PolicyResponse.Rules), 1) assert.Equal(t, err.PolicyResponse.Rules[0].Status, response.RuleStatusPass) @@ -463,7 +463,7 @@ func Test_NestedAttestors(t *testing.T) { policy = strings.Replace(policy, "KEY2", testOtherKey, -1) policy = strings.Replace(policy, "COUNT", "0", -1) policyContext = buildContext(t, policy, testSampleResource, "") - err = VerifyAndPatchImages(policyContext) + err, _ = VerifyAndPatchImages(policyContext) assert.Equal(t, len(err.PolicyResponse.Rules), 1) assert.Equal(t, err.PolicyResponse.Rules[0].Status, response.RuleStatusFail) @@ -471,7 +471,7 @@ func Test_NestedAttestors(t *testing.T) { policy = strings.Replace(policy, "KEY2", testOtherKey, -1) policy = strings.Replace(policy, "COUNT", "1", -1) policyContext = buildContext(t, policy, testSampleResource, "") - err = VerifyAndPatchImages(policyContext) + err, _ = VerifyAndPatchImages(policyContext) assert.Equal(t, len(err.PolicyResponse.Rules), 1) assert.Equal(t, err.PolicyResponse.Rules[0].Status, response.RuleStatusPass) } @@ -500,65 +500,62 @@ func createStaticKeyAttestorSet(s string) kyverno.AttestorSet { } func Test_ChangedAnnotation(t *testing.T) { - name := "nginx" - digest := "sha256:859ab6768a6f26a79bc42b231664111317d095a4f04e4b6fe79ce37b3d199097" - annotationKey := makeAnnotationKey(name) + annotationKey := imageVerifyAnnotationKey annotationNew := fmt.Sprintf("\"annotations\": {\"%s\": \"%s\"}", annotationKey, "true") newResource := strings.ReplaceAll(testResource, "\"annotations\": {}", annotationNew) policyContext := buildContext(t, testPolicyGood, testResource, testResource) - hasChanged := hasImageVerifiedAnnotationChanged(policyContext, name, digest) + + hasChanged := hasImageVerifiedAnnotationChanged(policyContext) assert.Equal(t, hasChanged, false) policyContext = buildContext(t, testPolicyGood, newResource, testResource) - hasChanged = hasImageVerifiedAnnotationChanged(policyContext, name, digest) + hasChanged = hasImageVerifiedAnnotationChanged(policyContext) assert.Equal(t, hasChanged, true) annotationOld := fmt.Sprintf("\"annotations\": {\"%s\": \"%s\"}", annotationKey, "false") oldResource := strings.ReplaceAll(testResource, "\"annotations\": {}", annotationOld) policyContext = buildContext(t, testPolicyGood, newResource, oldResource) - hasChanged = hasImageVerifiedAnnotationChanged(policyContext, name, digest) + hasChanged = hasImageVerifiedAnnotationChanged(policyContext) assert.Equal(t, hasChanged, true) } func Test_MarkImageVerified(t *testing.T) { - imageVerifyRule := kyverno.ImageVerification{Required: true} - iv := &imageVerifier{ - logger: log.Log, - policyContext: buildContext(t, testPolicyGood, testResource, ""), - rule: &kyverno.Rule{VerifyImages: []kyverno.ImageVerification{imageVerifyRule}}, - resp: &response.EngineResponse{}, - } - - ruleResp := &response.RuleResponse{Status: response.RuleStatusPass} - digest := "sha256:859ab6768a6f26a79bc42b231664111317d095a4f04e4b6fe79ce37b3d199097" - imageInfo := apiutils.ImageInfo{} - imageInfo.Name = "nginx" - - iv.markImageVerified(imageVerifyRule, ruleResp, digest, imageInfo) - assert.Equal(t, len(ruleResp.Patches), 2) - - u := applyPatches(t, ruleResp) - key := makeAnnotationKey(imageInfo.Name) - value := u.GetAnnotations()[key] - - var ivm ImageVerificationMetadata - err := json.Unmarshal([]byte(value), &ivm) + image := "ghcr.io/jimbugwadia/pause2:latest" + cosign.ClearMock() + policyContext := buildContext(t, testPolicyGood, testResource, "") + err := cosign.SetMock(image, payloads) assert.NilError(t, err) - assert.Equal(t, ivm.Verified, true) - assert.Equal(t, ivm.Digest, digest) + engineResponse, verifiedImages := VerifyAndPatchImages(policyContext) + assert.Assert(t, engineResponse != nil) + assert.Equal(t, len(engineResponse.PolicyResponse.Rules), 1) + assert.Equal(t, engineResponse.PolicyResponse.Rules[0].Status, response.RuleStatusPass) - ruleResp.Patches = nil - imageVerifyRule = kyverno.ImageVerification{Required: false} - iv.rule = &kyverno.Rule{VerifyImages: []kyverno.ImageVerification{imageVerifyRule}} - iv.markImageVerified(imageVerifyRule, ruleResp, digest, imageInfo) - assert.Equal(t, len(ruleResp.Patches), 0) + assert.Assert(t, verifiedImages != nil) + assert.Assert(t, verifiedImages.Data != nil) + assert.Equal(t, len(verifiedImages.Data), 1) + assert.Equal(t, verifiedImages.isVerified(image), true) + + patches, err := verifiedImages.Patches(false, log.Log) + assert.NilError(t, err) + assert.Equal(t, len(patches), 2) + + resource := applyPatches(t, patches) + patchedAnnotations := resource.GetAnnotations() + assert.Equal(t, len(patchedAnnotations), 1) + + json := patchedAnnotations[imageVerifyAnnotationKey] + assert.Assert(t, json != "") + + verified, err := isImageVerified(resource, image, log.Log) + assert.NilError(t, err) + assert.Equal(t, verified, true) } -func applyPatches(t *testing.T, ruleResp *response.RuleResponse) unstructured.Unstructured { - patchedResource, err := utils.ApplyPatches([]byte(testResource), ruleResp.Patches) +func applyPatches(t *testing.T, patches [][]byte) unstructured.Unstructured { + patchedResource, err := utils.ApplyPatches([]byte(testResource), patches) assert.NilError(t, err) assert.Assert(t, patchedResource != nil) diff --git a/pkg/engine/validation.go b/pkg/engine/validation.go index 3d96f96a57..3251155734 100644 --- a/pkg/engine/validation.go +++ b/pkg/engine/validation.go @@ -462,7 +462,7 @@ func matches(logger logr.Logger, rule *kyverno.Rule, ctx *PolicyContext) bool { } } - logger.V(4).Info("resource does not match rule", "reason", err.Error()) + logger.V(5).Info("resource does not match rule", "reason", err.Error()) return false } diff --git a/pkg/policyreport/builder.go b/pkg/policyreport/builder.go index b80123b73a..5b8738fbc4 100755 --- a/pkg/policyreport/builder.go +++ b/pkg/policyreport/builder.go @@ -58,7 +58,7 @@ func GeneratePRsFromEngineResponse(ers []*response.EngineResponse, log logr.Logg for _, er := range ers { // ignore creation of PV for resources that are yet to be assigned a name if er.PolicyResponse.Resource.Name == "" { - log.V(4).Info("resource does no have a name assigned yet, not creating a policy violation", "resource", er.PolicyResponse.Resource) + log.V(4).Info("skipping resource with no name", "resource", er.PolicyResponse.Resource) continue } diff --git a/pkg/webhooks/server.go b/pkg/webhooks/server.go index b54d5e93fa..e57af9c2ed 100644 --- a/pkg/webhooks/server.go +++ b/pkg/webhooks/server.go @@ -7,6 +7,8 @@ import ( "sync" "time" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "github.com/go-logr/logr" "github.com/julienschmidt/httprouter" "github.com/kyverno/kyverno/api/kyverno/v1beta1" @@ -183,23 +185,15 @@ func (ws *WebhookServer) buildPolicyContext(request *admissionv1.AdmissionReques return nil, errors.Wrap(err, "failed to create policy rule context") } - // convert RAW to unstructured - resource, err := utils.ConvertResource(request.Object.Raw, request.Kind.Group, request.Kind.Version, request.Kind.Kind, request.Namespace) + resource, err := convertResource(request, request.Object.Raw) if err != nil { - return nil, errors.Wrap(err, "failed to convert raw resource to unstructured format") + return nil, err } if err := ctx.AddImageInfos(&resource); err != nil { return nil, errors.Wrap(err, "failed to add image information to the policy rule context") } - if request.Kind.Kind == "Secret" && request.Operation == admissionv1.Update { - resource, err = utils.NormalizeSecret(&resource) - if err != nil { - return nil, errors.Wrap(err, "failed to convert secret to unstructured format") - } - } - policyContext := &engine.PolicyContext{ NewResource: resource, AdmissionInfo: userRequestInfo, @@ -211,12 +205,32 @@ func (ws *WebhookServer) buildPolicyContext(request *admissionv1.AdmissionReques } if request.Operation == admissionv1.Update { - policyContext.OldResource = resource + policyContext.OldResource, err = convertResource(request, request.OldObject.Raw) + if err != nil { + return nil, err + } } return policyContext, nil } +// convertResource converts RAW to unstructured +func convertResource(request *admissionv1.AdmissionRequest, resourceRaw []byte) (unstructured.Unstructured, error) { + resource, err := utils.ConvertResource(resourceRaw, request.Kind.Group, request.Kind.Version, request.Kind.Kind, request.Namespace) + if err != nil { + return unstructured.Unstructured{}, errors.Wrap(err, "failed to convert raw resource to unstructured format") + } + + if request.Kind.Kind == "Secret" && request.Operation == admissionv1.Update { + resource, err = utils.NormalizeSecret(&resource) + if err != nil { + return unstructured.Unstructured{}, errors.Wrap(err, "failed to convert secret to unstructured format") + } + } + + return resource, nil +} + // RunAsync TLS server in separate thread and returns control immediately func (ws *WebhookServer) RunAsync(stopCh <-chan struct{}) { go func() { diff --git a/pkg/webhooks/validate_audit.go b/pkg/webhooks/validate_audit.go index bb429861de..a69a80d308 100644 --- a/pkg/webhooks/validate_audit.go +++ b/pkg/webhooks/validate_audit.go @@ -168,7 +168,7 @@ func (h *auditHandler) process(request *admissionv1.AdmissionRequest) error { } if err := ctx.AddImageInfos(&newResource); err != nil { - return errors.Wrap(err, "failed add image information to policy rule context\"") + return errors.Wrap(err, "failed add image information to policy rule context") } policyContext := &engine.PolicyContext{ diff --git a/pkg/webhooks/verify_images.go b/pkg/webhooks/verify_images.go index ee922f04f7..56e2fa2797 100644 --- a/pkg/webhooks/verify_images.go +++ b/pkg/webhooks/verify_images.go @@ -36,18 +36,20 @@ func (ws *WebhookServer) handleVerifyImages(request *admissionv1.AdmissionReques var engineResponses []*response.EngineResponse var patches [][]byte + verifiedImageData := &engine.ImageVerificationMetadata{} for _, p := range policies { policyContext.Policy = p - resp := engine.VerifyAndPatchImages(policyContext) + resp, ivm := engine.VerifyAndPatchImages(policyContext) + engineResponses = append(engineResponses, resp) patches = append(patches, resp.GetPatches()...) + verifiedImageData.Merge(ivm) } prInfos := policyreport.GeneratePRsFromEngineResponse(engineResponses, logger) ws.prGenerator.Add(prInfos...) blocked := toBlockResource(engineResponses, logger) - events := generateEvents(engineResponses, blocked, logger) ws.eventGen.Add(events...) @@ -56,5 +58,21 @@ func (ws *WebhookServer) handleVerifyImages(request *admissionv1.AdmissionReques return false, getEnforceFailureErrorMsg(engineResponses), nil } + if !verifiedImageData.IsEmpty() { + hasAnnotations := hasAnnotations(policyContext) + annotationPatches, err := verifiedImageData.Patches(hasAnnotations, logger) + if err != nil { + logger.Error(err, "failed to create image verification annotation patches") + } else { + // add annotation patches first + patches = append(annotationPatches, patches...) + } + } + return true, "", jsonutils.JoinPatches(patches...) } + +func hasAnnotations(context *engine.PolicyContext) bool { + annotations := context.NewResource.GetAnnotations() + return len(annotations) != 0 +}