From dbc442b9e10ba20196f796b32404cb2747144280 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Charles-Edouard=20Br=C3=A9t=C3=A9ch=C3=A9?= Date: Mon, 27 Mar 2023 17:11:27 +0200 Subject: [PATCH] refactor: introduce image validation handler (#6697) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor: factorise rule handler invocation code Signed-off-by: Charles-Edouard Brétéché * fix Signed-off-by: Charles-Edouard Brétéché * fix Signed-off-by: Charles-Edouard Brétéché * refactor: introduce validation handler Signed-off-by: Charles-Edouard Brétéché * fix Signed-off-by: Charles-Edouard Brétéché * refactor: introduce image validation handler Signed-off-by: Charles-Edouard Brétéché * fix Signed-off-by: Charles-Edouard Brétéché --------- Signed-off-by: Charles-Edouard Brétéché --- pkg/engine/engine.go | 2 + .../handlers/validation/validateimage.go | 117 ++++++++++++++++++ pkg/engine/imageVerify.go | 58 ++------- pkg/engine/imageVerifyValidate.go | 116 ----------------- pkg/engine/imageVerify_test.go | 3 +- pkg/engine/utils/image.go | 83 +++++++++++++ pkg/engine/validation.go | 9 +- 7 files changed, 219 insertions(+), 169 deletions(-) create mode 100644 pkg/engine/handlers/validation/validateimage.go delete mode 100644 pkg/engine/imageVerifyValidate.go create mode 100644 pkg/engine/utils/image.go diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index fe7969b870..ff2ad07007 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -35,6 +35,7 @@ type engine struct { mutateHandler handlers.Handler mutateExistingHandler handlers.Handler validateHandler handlers.Handler + validateImageHandler handlers.Handler } func NewEngine( @@ -55,6 +56,7 @@ func NewEngine( e.mutateHandler = mutation.NewHandler(configuration, e.ContextLoader) e.mutateExistingHandler = mutation.NewMutateExistingHandler(configuration, client, e.ContextLoader) e.validateHandler = validation.NewHandler(e.ContextLoader) + e.validateImageHandler = validation.NewValidateImageHandler(configuration, e.ContextLoader) return e } diff --git a/pkg/engine/handlers/validation/validateimage.go b/pkg/engine/handlers/validation/validateimage.go new file mode 100644 index 0000000000..c7832728ba --- /dev/null +++ b/pkg/engine/handlers/validation/validateimage.go @@ -0,0 +1,117 @@ +package validation + +import ( + "context" + "fmt" + + "github.com/go-logr/logr" + gojmespath "github.com/jmespath/go-jmespath" + kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" + "github.com/kyverno/kyverno/pkg/config" + engineapi "github.com/kyverno/kyverno/pkg/engine/api" + "github.com/kyverno/kyverno/pkg/engine/handlers" + "github.com/kyverno/kyverno/pkg/engine/internal" + engineutils "github.com/kyverno/kyverno/pkg/engine/utils" + apiutils "github.com/kyverno/kyverno/pkg/utils/api" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" +) + +type validateImageHandler struct { + configuration config.Configuration + contextLoader func(kyvernov1.PolicyInterface, kyvernov1.Rule) engineapi.EngineContextLoader +} + +func NewValidateImageHandler( + configuration config.Configuration, + contextLoader func(kyvernov1.PolicyInterface, kyvernov1.Rule) engineapi.EngineContextLoader, +) handlers.Handler { + return validateImageHandler{ + configuration: configuration, + contextLoader: contextLoader, + } +} + +func (h validateImageHandler) Process( + ctx context.Context, + logger logr.Logger, + policyContext engineapi.PolicyContext, + resource unstructured.Unstructured, + rule kyvernov1.Rule, +) (unstructured.Unstructured, []engineapi.RuleResponse) { + if engineutils.IsDeleteRequest(policyContext) { + return resource, nil + } + policy := policyContext.Policy() + contextLoader := h.contextLoader(policy, rule) + matchingImages, _, err := engineutils.ExtractMatchingImages( + policyContext.NewResource(), + policyContext.JSONContext(), + rule, + h.configuration, + ) + if err != nil { + return resource, handlers.RuleResponses(internal.RuleError(&rule, engineapi.Validation, "", err)) + } + if len(matchingImages) == 0 { + return resource, handlers.RuleResponses(internal.RuleSkip(&rule, engineapi.Validation, "image verified")) + } + if err := contextLoader(ctx, rule.Context, policyContext.JSONContext()); err != nil { + if _, ok := err.(gojmespath.NotFoundError); ok { + logger.V(3).Info("failed to load context", "reason", err.Error()) + } else { + logger.Error(err, "failed to load context") + } + + return resource, handlers.RuleResponses(internal.RuleError(&rule, engineapi.Validation, "failed to load context", err)) + } + preconditionsPassed, err := internal.CheckPreconditions(logger, policyContext, rule.RawAnyAllConditions) + if err != nil { + return resource, handlers.RuleResponses(internal.RuleError(&rule, engineapi.Validation, "failed to evaluate preconditions", err)) + } + if !preconditionsPassed { + if policyContext.Policy().GetSpec().ValidationFailureAction.Audit() { + return resource, nil + } + + return resource, handlers.RuleResponses(internal.RuleSkip(&rule, engineapi.Validation, "preconditions not met")) + } + for _, v := range rule.VerifyImages { + imageVerify := v.Convert() + for _, infoMap := range policyContext.JSONContext().ImageInfo() { + for name, imageInfo := range infoMap { + image := imageInfo.String() + + if !engineutils.ImageMatches(image, imageVerify.ImageReferences) { + logger.V(4).Info("image does not match", "imageReferences", imageVerify.ImageReferences) + return resource, nil + } + + logger.V(4).Info("validating image", "image", image) + if err := validateImage(policyContext, imageVerify, name, imageInfo, logger); err != nil { + return resource, handlers.RuleResponses(internal.RuleResponse(rule, engineapi.ImageVerify, err.Error(), engineapi.RuleStatusFail)) + } + } + } + } + logger.V(4).Info("validated image", "rule", rule.Name) + return resource, handlers.RuleResponses(internal.RulePass(&rule, engineapi.Validation, "image verified")) +} + +func validateImage(ctx engineapi.PolicyContext, imageVerify *kyvernov1.ImageVerification, name string, imageInfo apiutils.ImageInfo, log logr.Logger) error { + image := imageInfo.String() + if imageVerify.VerifyDigest && imageInfo.Digest == "" { + log.V(2).Info("missing digest", "image", imageInfo.String()) + return fmt.Errorf("missing digest for %s", image) + } + newResource := ctx.NewResource() + if imageVerify.Required && newResource.Object != nil { + verified, err := engineutils.IsImageVerified(newResource, image, log) + if err != nil { + return err + } + if !verified { + return fmt.Errorf("unverified image %s", image) + } + } + return nil +} diff --git a/pkg/engine/imageVerify.go b/pkg/engine/imageVerify.go index f96b43ce25..bbdc44d138 100644 --- a/pkg/engine/imageVerify.go +++ b/pkg/engine/imageVerify.go @@ -3,7 +3,6 @@ package engine import ( "context" "fmt" - "strings" "time" "github.com/go-logr/logr" @@ -12,10 +11,9 @@ import ( engineapi "github.com/kyverno/kyverno/pkg/engine/api" enginecontext "github.com/kyverno/kyverno/pkg/engine/context" "github.com/kyverno/kyverno/pkg/engine/internal" + engineutils "github.com/kyverno/kyverno/pkg/engine/utils" "github.com/kyverno/kyverno/pkg/engine/variables" "github.com/kyverno/kyverno/pkg/tracing" - apiutils "github.com/kyverno/kyverno/pkg/utils/api" - "github.com/kyverno/kyverno/pkg/utils/wildcard" "go.opentelemetry.io/otel/trace" ) @@ -92,7 +90,12 @@ func (e *engine) doVerifyAndPatch( logger.V(3).Info("processing image verification rule") - ruleImages, imageRefs, err := e.extractMatchingImages(policyContext, rule) + ruleImages, imageRefs, err := engineutils.ExtractMatchingImages( + policyContext.NewResource(), + policyContext.JSONContext(), + *rule, + e.configuration, + ) if err != nil { internal.AddRuleResponse( &resp.PolicyResponse, @@ -145,53 +148,6 @@ func (e *engine) doVerifyAndPatch( } } -func getMatchingImages(images map[string]map[string]apiutils.ImageInfo, rule *kyvernov1.Rule) ([]apiutils.ImageInfo, string) { - imageInfos := []apiutils.ImageInfo{} - imageRefs := []string{} - for _, infoMap := range images { - for _, imageInfo := range infoMap { - image := imageInfo.String() - for _, verifyImage := range rule.VerifyImages { - verifyImage = *verifyImage.Convert() - imageRefs = append(imageRefs, verifyImage.ImageReferences...) - if imageMatches(image, verifyImage.ImageReferences) { - imageInfos = append(imageInfos, imageInfo) - } - } - } - } - return imageInfos, strings.Join(imageRefs, ",") -} - -func imageMatches(image string, imagePatterns []string) bool { - for _, imagePattern := range imagePatterns { - if wildcard.Match(imagePattern, image) { - return true - } - } - - return false -} - -func (e *engine) extractMatchingImages(policyContext engineapi.PolicyContext, rule *kyvernov1.Rule) ([]apiutils.ImageInfo, string, error) { - var ( - images map[string]map[string]apiutils.ImageInfo - err error - ) - newResource := policyContext.NewResource() - images = policyContext.JSONContext().ImageInfo() - if rule.ImageExtractors != nil { - images, err = policyContext.JSONContext().GenerateCustomImageInfo(&newResource, rule.ImageExtractors, e.configuration) - if err != nil { - // if we get an error while generating custom images from image extractors, - // don't check for matching images in imageExtractors - return nil, "", err - } - } - matchingImages, imageRefs := getMatchingImages(images, rule) - return matchingImages, imageRefs, nil -} - func substituteVariables(rule *kyvernov1.Rule, ctx enginecontext.EvalInterface, logger logr.Logger) (*kyvernov1.Rule, error) { // remove attestations as variables are not substituted in them ruleCopy := *rule.DeepCopy() diff --git a/pkg/engine/imageVerifyValidate.go b/pkg/engine/imageVerifyValidate.go deleted file mode 100644 index bb49b00b43..0000000000 --- a/pkg/engine/imageVerifyValidate.go +++ /dev/null @@ -1,116 +0,0 @@ -package engine - -import ( - "context" - "fmt" - - "github.com/go-logr/logr" - gojmespath "github.com/jmespath/go-jmespath" - kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" - engineapi "github.com/kyverno/kyverno/pkg/engine/api" - "github.com/kyverno/kyverno/pkg/engine/internal" - engineutils "github.com/kyverno/kyverno/pkg/engine/utils" - apiutils "github.com/kyverno/kyverno/pkg/utils/api" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" -) - -func (e *engine) processImageValidationRule( - ctx context.Context, - log logr.Logger, - enginectx engineapi.PolicyContext, - rule *kyvernov1.Rule, -) *engineapi.RuleResponse { - if engineutils.IsDeleteRequest(enginectx) { - return nil - } - - log = log.WithValues("rule", rule.Name) - matchingImages, _, err := e.extractMatchingImages(enginectx, rule) - if err != nil { - return internal.RuleError(rule, engineapi.Validation, "", err) - } - if len(matchingImages) == 0 { - return internal.RuleSkip(rule, engineapi.Validation, "image verified") - } - if err := internal.LoadContext(ctx, e, enginectx, *rule); err != nil { - if _, ok := err.(gojmespath.NotFoundError); ok { - log.V(3).Info("failed to load context", "reason", err.Error()) - } else { - log.Error(err, "failed to load context") - } - - return internal.RuleError(rule, engineapi.Validation, "failed to load context", err) - } - - preconditionsPassed, err := internal.CheckPreconditions(log, enginectx, rule.RawAnyAllConditions) - if err != nil { - return internal.RuleError(rule, engineapi.Validation, "failed to evaluate preconditions", err) - } - - if !preconditionsPassed { - if enginectx.Policy().GetSpec().ValidationFailureAction.Audit() { - return nil - } - - return internal.RuleSkip(rule, engineapi.Validation, "preconditions not met") - } - - for _, v := range rule.VerifyImages { - imageVerify := v.Convert() - for _, infoMap := range enginectx.JSONContext().ImageInfo() { - 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", "imageReferences", imageVerify.ImageReferences) - return nil - } - - log.V(4).Info("validating image", "image", image) - if err := validateImage(enginectx, imageVerify, name, imageInfo, log); err != nil { - return internal.RuleResponse(*rule, engineapi.ImageVerify, err.Error(), engineapi.RuleStatusFail) - } - } - } - } - - log.V(4).Info("validated image", "rule", rule.Name) - return internal.RulePass(rule, engineapi.Validation, "image verified") -} - -func validateImage(ctx engineapi.PolicyContext, imageVerify *kyvernov1.ImageVerification, name string, imageInfo apiutils.ImageInfo, log logr.Logger) error { - image := imageInfo.String() - if imageVerify.VerifyDigest && imageInfo.Digest == "" { - log.V(2).Info("missing digest", "image", imageInfo.String()) - return fmt.Errorf("missing digest for %s", image) - } - newResource := ctx.NewResource() - if imageVerify.Required && newResource.Object != nil { - verified, err := isImageVerified(newResource, image, log) - if err != nil { - return err - } - if !verified { - return fmt.Errorf("unverified image %s", image) - } - } - return nil -} - -func isImageVerified(resource unstructured.Unstructured, image string, log logr.Logger) (bool, error) { - if resource.Object == nil { - return false, fmt.Errorf("nil resource") - } - if annotations := resource.GetAnnotations(); len(annotations) == 0 { - return false, nil - } else if data, ok := annotations[engineapi.ImageVerifyAnnotationKey]; !ok { - log.V(2).Info("missing image metadata in annotation", "key", engineapi.ImageVerifyAnnotationKey) - return false, fmt.Errorf("image is not verified") - } else if ivm, err := engineapi.ParseImageMetadata(data); err != nil { - log.Error(err, "failed to parse image verification metadata", "data", data) - return false, fmt.Errorf("failed to parse image metadata: %w", err) - } else { - return ivm.IsVerified(image), nil - } -} diff --git a/pkg/engine/imageVerify_test.go b/pkg/engine/imageVerify_test.go index 13e681f7ee..2a4dea5f5b 100644 --- a/pkg/engine/imageVerify_test.go +++ b/pkg/engine/imageVerify_test.go @@ -17,6 +17,7 @@ import ( "github.com/kyverno/kyverno/pkg/engine/internal" "github.com/kyverno/kyverno/pkg/engine/policycontext" "github.com/kyverno/kyverno/pkg/engine/utils" + engineutils "github.com/kyverno/kyverno/pkg/engine/utils" "github.com/kyverno/kyverno/pkg/registryclient" kubeutils "github.com/kyverno/kyverno/pkg/utils/kube" "gotest.tools/assert" @@ -786,7 +787,7 @@ func Test_MarkImageVerified(t *testing.T) { json := patchedAnnotations[engineapi.ImageVerifyAnnotationKey] assert.Assert(t, json != "") - verified, err := isImageVerified(resource, image, logr.Discard()) + verified, err := engineutils.IsImageVerified(resource, image, logr.Discard()) assert.NilError(t, err) assert.Equal(t, verified, true) } diff --git a/pkg/engine/utils/image.go b/pkg/engine/utils/image.go new file mode 100644 index 0000000000..50964fed8b --- /dev/null +++ b/pkg/engine/utils/image.go @@ -0,0 +1,83 @@ +package utils + +import ( + "fmt" + "strings" + + "github.com/go-logr/logr" + kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" + "github.com/kyverno/kyverno/pkg/config" + engineapi "github.com/kyverno/kyverno/pkg/engine/api" + enginecontext "github.com/kyverno/kyverno/pkg/engine/context" + apiutils "github.com/kyverno/kyverno/pkg/utils/api" + "github.com/kyverno/kyverno/pkg/utils/wildcard" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" +) + +func ImageMatches(image string, imagePatterns []string) bool { + for _, imagePattern := range imagePatterns { + if wildcard.Match(imagePattern, image) { + return true + } + } + + return false +} + +func GetMatchingImages(images map[string]map[string]apiutils.ImageInfo, rule kyvernov1.Rule) ([]apiutils.ImageInfo, string) { + imageInfos := []apiutils.ImageInfo{} + imageRefs := []string{} + for _, infoMap := range images { + for _, imageInfo := range infoMap { + image := imageInfo.String() + for _, verifyImage := range rule.VerifyImages { + verifyImage = *verifyImage.Convert() + imageRefs = append(imageRefs, verifyImage.ImageReferences...) + if ImageMatches(image, verifyImage.ImageReferences) { + imageInfos = append(imageInfos, imageInfo) + } + } + } + } + return imageInfos, strings.Join(imageRefs, ",") +} + +func ExtractMatchingImages( + resource unstructured.Unstructured, + context enginecontext.Interface, + rule kyvernov1.Rule, + cfg config.Configuration, +) ([]apiutils.ImageInfo, string, error) { + var ( + images map[string]map[string]apiutils.ImageInfo + err error + ) + images = context.ImageInfo() + if rule.ImageExtractors != nil { + images, err = context.GenerateCustomImageInfo(&resource, rule.ImageExtractors, cfg) + if err != nil { + // if we get an error while generating custom images from image extractors, + // don't check for matching images in imageExtractors + return nil, "", err + } + } + matchingImages, imageRefs := GetMatchingImages(images, rule) + return matchingImages, imageRefs, nil +} + +func IsImageVerified(resource unstructured.Unstructured, image string, log logr.Logger) (bool, error) { + if resource.Object == nil { + return false, fmt.Errorf("nil resource") + } + if annotations := resource.GetAnnotations(); len(annotations) == 0 { + return false, nil + } else if data, ok := annotations[engineapi.ImageVerifyAnnotationKey]; !ok { + log.V(2).Info("missing image metadata in annotation", "key", engineapi.ImageVerifyAnnotationKey) + return false, fmt.Errorf("image is not verified") + } else if ivm, err := engineapi.ParseImageMetadata(data); err != nil { + log.Error(err, "failed to parse image verification metadata", "data", data) + return false, fmt.Errorf("failed to parse image metadata: %w", err) + } else { + return ivm.IsVerified(image), nil + } +} diff --git a/pkg/engine/validation.go b/pkg/engine/validation.go index 936e13b10b..0da2df050a 100644 --- a/pkg/engine/validation.go +++ b/pkg/engine/validation.go @@ -81,7 +81,14 @@ func (e *engine) validateResource( ) return rr } else if hasValidateImage { - return handlers.RuleResponses(e.processImageValidationRule(ctx, logger, policyContext, rule)) + _, rr := e.validateImageHandler.Process( + ctx, + logger, + policyContext, + policyContext.NewResource(), + *rule, + ) + return rr } else if hasYAMLSignatureVerify { _, rr := e.verifyManifestHandler.Process( ctx,