From ef71102b224bfbd77db40d04646ae2ac82cc72fb Mon Sep 17 00:00:00 2001 From: Jim Bugwadia Date: Sun, 1 May 2022 16:02:49 -0700 Subject: [PATCH] Fix verify all images (#3748) Co-authored-by: Sambhav Kothari --- pkg/engine/imageVerify.go | 84 +++++++++++++++++++----------- pkg/engine/imageVerifyValidate.go | 57 +++++++++++++++----- pkg/engine/imageVerify_test.go | 14 +++-- pkg/webhookconfig/configmanager.go | 2 +- 4 files changed, 107 insertions(+), 50 deletions(-) diff --git a/pkg/engine/imageVerify.go b/pkg/engine/imageVerify.go index e0d6fa27af..8c19ac4d50 100644 --- a/pkg/engine/imageVerify.go +++ b/pkg/engine/imageVerify.go @@ -4,7 +4,6 @@ import ( "encoding/json" "fmt" "reflect" - "strconv" "strings" "time" @@ -171,8 +170,8 @@ func (iv *imageVerifier) verify(imageVerify v1.ImageVerification, images map[str } } - if imageVerify.MutateDigest && imageInfo.Digest != "" { - patch, err := iv.handleDigest(digest, imageInfo) + if imageVerify.MutateDigest && imageInfo.Digest == "" { + patch, err := iv.handleMutateDigest(digest, imageInfo) if err != nil { ruleResp = ruleError(iv.rule, response.ImageVerify, "failed to update digest", err) } @@ -194,7 +193,7 @@ func (iv *imageVerifier) verify(imageVerify v1.ImageVerification, images map[str } } -func (iv *imageVerifier) handleDigest(digest string, imageInfo kubeutils.ImageInfo) ([]byte, error) { +func (iv *imageVerifier) handleMutateDigest(digest string, imageInfo kubeutils.ImageInfo) ([]byte, error) { if digest == "" { digest, err := fetchImageDigest(imageInfo.String()) if err != nil { @@ -220,11 +219,12 @@ func (iv *imageVerifier) markImageVerified(imageVerify v1.ImageVerification, rul if imageVerify.Required { isImageVerified := ruleResp.Status == response.RuleStatusPass - patch, err := makeImageVerifiedPatch(imageInfo, digest, isImageVerified) - if err == nil { - ruleResp.Patches = [][]byte{patch} - } else { + 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...) } } @@ -232,42 +232,64 @@ func (iv *imageVerifier) markImageVerified(imageVerify v1.ImageVerification, rul } func hasImageVerifiedAnnotationChanged(ctx *PolicyContext, name, digest string) bool { - if reflect.DeepEqual(ctx.OldResource, &unstructured.Unstructured{}) || - reflect.DeepEqual(ctx.NewResource, &unstructured.Unstructured{}) { + if reflect.DeepEqual(ctx.NewResource, &unstructured.Unstructured{}) || + reflect.DeepEqual(ctx.OldResource, &unstructured.Unstructured{}) { return false } - key := makeAnnotationKey(name, digest) + key := makeAnnotationKey(name) newValue := ctx.NewResource.GetAnnotations()[key] oldValue := ctx.OldResource.GetAnnotations()[key] return newValue != oldValue } -func makeImageVerifiedPatch(imageInfo kubeutils.ImageInfo, digest string, verified bool) ([]byte, error) { - var patch = make(map[string]interface{}) - annotationKey := makeAnnotationKeyForJSONPatch(imageInfo.Name, digest) - patch["op"] = "add" - patch["path"] = annotationKey - patch["value"] = strconv.FormatBool(verified) - return json.Marshal(patch) +func makeImageVerifiedPatches(imageInfo kubeutils.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 makeAnnotationKeyForJMESPath(imageName, imageDigest string) string { - key := makeAnnotationKey(imageName, imageDigest) - return "request.object.metadata.annotations." + `"` + key + `"` -} - -func makeAnnotationKeyForJSONPatch(imageName, imageDigest string) string { - key := makeAnnotationKey(imageName, imageDigest) +func makeAnnotationKeyForJSONPatch(imageName string) string { + key := makeAnnotationKey(imageName) return "/metadata/annotations/" + strings.ReplaceAll(key, "/", "~1") } -func makeAnnotationKey(imageName, imageDigest string) string { - if imageDigest == "" { - return fmt.Sprintf("images.kyverno.io/%s", imageName) - } - - return fmt.Sprintf("images.kyverno.io/%s/%s/%s", imageName, imageDigest[0:6], imageDigest[7:]) +func makeAnnotationKey(imageName string) string { + return fmt.Sprintf("images.kyverno.io/%s", imageName) } func fetchImageDigest(ref string) (string, error) { diff --git a/pkg/engine/imageVerifyValidate.go b/pkg/engine/imageVerifyValidate.go index f32ba5bc32..dc901fbd4a 100644 --- a/pkg/engine/imageVerifyValidate.go +++ b/pkg/engine/imageVerifyValidate.go @@ -1,7 +1,13 @@ 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" @@ -58,10 +64,11 @@ func processImageValidationRule(log logr.Logger, ctx *PolicyContext, rule *kyver func validateImage(ctx *PolicyContext, imageVerify *kyverno.ImageVerification, imageInfo kubeutils.ImageInfo) error { image := imageInfo.String() if imageVerify.VerifyDigest && imageInfo.Digest == "" { + log.Log.Info("missing digest", "request.object", ctx.NewResource.UnstructuredContent()) return fmt.Errorf("missing digest for %s", image) } - if imageVerify.Required { + if imageVerify.Required && !reflect.DeepEqual(ctx.NewResource, unstructured.Unstructured{}) { verified, err := isImageVerified(ctx, imageInfo) if err != nil { return err @@ -75,17 +82,39 @@ func validateImage(ctx *PolicyContext, imageVerify *kyverno.ImageVerification, i return nil } -func isImageVerified(ctx *PolicyContext, imageInfo kubeutils.ImageInfo) (bool, error) { - key := makeAnnotationKeyForJMESPath(imageInfo.Name, imageInfo.Digest) - data, err := ctx.JSONContext.Query(key) - if err != nil { - return false, errors.Wrapf(err, "failed to query annotation for %s", key) - } - - result, ok := data.(string) - if !ok { - return false, errors.Wrapf(err, "failed to convert data %s", key) - } - - return result == "true", nil +type ImageVerificationMetadata struct { + Verified bool `json:"verified,omitempty"` + Digest string `json:"digest,omitempty"` +} + +func isImageVerified(ctx *PolicyContext, imageInfo kubeutils.ImageInfo) (bool, error) { + if reflect.DeepEqual(ctx.NewResource, unstructured.Unstructured{}) { + return false, errors.Errorf("resource does not exist") + } + + annotations := ctx.NewResource.GetAnnotations() + if len(annotations) == 0 { + return false, nil + } + + key := makeAnnotationKey(imageInfo.Name) + data, ok := annotations[key] + if !ok { + 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") + } + + if !ivm.Verified { + return false, nil + } + + if imageInfo.Digest != ivm.Digest { + return false, errors.Errorf("failed to verify image; digest mismatch") + } + + return true, nil } diff --git a/pkg/engine/imageVerify_test.go b/pkg/engine/imageVerify_test.go index 158f515cd1..0e895c1d97 100644 --- a/pkg/engine/imageVerify_test.go +++ b/pkg/engine/imageVerify_test.go @@ -502,7 +502,7 @@ func createStaticKeyAttestorSet(s string) kyverno.AttestorSet { func Test_ChangedAnnotation(t *testing.T) { name := "nginx" digest := "sha256:859ab6768a6f26a79bc42b231664111317d095a4f04e4b6fe79ce37b3d199097" - annotationKey := makeAnnotationKey(name, digest) + annotationKey := makeAnnotationKey(name) annotationNew := fmt.Sprintf("\"annotations\": {\"%s\": \"%s\"}", annotationKey, "true") newResource := strings.ReplaceAll(testResource, "\"annotations\": {}", annotationNew) @@ -537,12 +537,18 @@ func Test_MarkImageVerified(t *testing.T) { imageInfo.Name = "nginx" iv.markImageVerified(imageVerifyRule, ruleResp, digest, imageInfo) - assert.Equal(t, len(ruleResp.Patches), 1) + assert.Equal(t, len(ruleResp.Patches), 2) u := applyPatches(t, ruleResp) - key := makeAnnotationKey(imageInfo.Name, digest) + key := makeAnnotationKey(imageInfo.Name) value := u.GetAnnotations()[key] - assert.Equal(t, value, "true") + + var ivm ImageVerificationMetadata + err := json.Unmarshal([]byte(value), &ivm) + assert.NilError(t, err) + + assert.Equal(t, ivm.Verified, true) + assert.Equal(t, ivm.Digest, digest) ruleResp.Patches = nil imageVerifyRule = kyverno.ImageVerification{Required: false} diff --git a/pkg/webhookconfig/configmanager.go b/pkg/webhookconfig/configmanager.go index ab36075f62..a5ef6dbca6 100644 --- a/pkg/webhookconfig/configmanager.go +++ b/pkg/webhookconfig/configmanager.go @@ -747,7 +747,7 @@ func (m *webhookConfigManager) mergeWebhook(dst *webhook, policy kyverno.PolicyI continue } - if (updateValidate && rule.HasValidate()) || + if (updateValidate && rule.HasValidate() || rule.HasImagesValidationChecks()) || (updateValidate && rule.HasMutate() && rule.IsMutateExisting()) || (!updateValidate && rule.HasMutate()) && !rule.IsMutateExisting() || (!updateValidate && rule.HasVerifyImages()) {