From 118b40c64437c9bdbf8302997c96f77023ebacc0 Mon Sep 17 00:00:00 2001 From: Mohan B E <54951236+b-entangled@users.noreply.github.com> Date: Thu, 3 Sep 2020 22:14:54 +0530 Subject: [PATCH] added invalid field validation for policy (#1094) --- pkg/generate/cleanup/controller.go | 6 ++--- pkg/generate/controller.go | 6 ++--- pkg/generate/generate.go | 12 +++++----- pkg/policy/validate.go | 36 +++++++++++++++++++++++++++++- pkg/webhooks/generation.go | 4 ++-- 5 files changed, 48 insertions(+), 16 deletions(-) diff --git a/pkg/generate/cleanup/controller.go b/pkg/generate/cleanup/controller.go index e5f7c7e3bc..95f5e3c60e 100644 --- a/pkg/generate/cleanup/controller.go +++ b/pkg/generate/cleanup/controller.go @@ -172,14 +172,14 @@ func (c *Controller) deleteGR(obj interface{}) { return } } - for _,resource := range gr.Status.GeneratedResources { - r,err := c.client.GetResource(resource.APIVersion,resource.Kind,resource.Namespace,resource.Name) + for _, resource := range gr.Status.GeneratedResources { + r, err := c.client.GetResource(resource.APIVersion, resource.Kind, resource.Namespace, resource.Name) if err != nil { logger.Error(err, "Generated resource is not deleted", "Resource", r.GetName()) } labels := r.GetLabels() if labels["policy.kyverno.io/synchronize"] == "enable" { - if err := c.client.DeleteResource(r.GetAPIVersion(), r.GetKind(),r.GetNamespace(), r.GetName(), false); err != nil { + if err := c.client.DeleteResource(r.GetAPIVersion(), r.GetKind(), r.GetNamespace(), r.GetName(), false); err != nil { logger.Error(err, "Generated resource is not deleted", "Resource", r.GetName()) } } diff --git a/pkg/generate/controller.go b/pkg/generate/controller.go index 0b502e483e..4bbdcf2ffa 100644 --- a/pkg/generate/controller.go +++ b/pkg/generate/controller.go @@ -201,14 +201,14 @@ func (c *Controller) deleteGR(obj interface{}) { return } } - for _,resource := range gr.Status.GeneratedResources { - r,err := c.client.GetResource(resource.APIVersion,resource.Kind,resource.Namespace,resource.Name) + for _, resource := range gr.Status.GeneratedResources { + r, err := c.client.GetResource(resource.APIVersion, resource.Kind, resource.Namespace, resource.Name) if err != nil { logger.Error(err, "Generated resource is not deleted", "Resource", r.GetName()) } labels := r.GetLabels() if labels["policy.kyverno.io/synchronize"] == "enable" { - if err := c.client.DeleteResource(r.GetAPIVersion(), r.GetKind(),r.GetNamespace(), r.GetName(), false); err != nil { + if err := c.client.DeleteResource(r.GetAPIVersion(), r.GetKind(), r.GetNamespace(), r.GetName(), false); err != nil { logger.Error(err, "Generated resource is not deleted", "Resource", r.GetName()) } } diff --git a/pkg/generate/generate.go b/pkg/generate/generate.go index 6d62a4f07f..03f3fd80aa 100644 --- a/pkg/generate/generate.go +++ b/pkg/generate/generate.go @@ -116,8 +116,8 @@ func (c *Controller) applyGenerate(resource unstructured.Unstructured, gr kyvern continue } for _, v := range grList.Items { - if engineResponse.PolicyResponse.Policy == v.Spec.Policy && engineResponse.PolicyResponse.Resource.Name == v.Spec.Resource.Name && engineResponse.PolicyResponse.Resource.Kind == v.Spec.Resource.Kind && engineResponse.PolicyResponse.Resource.Namespace == v.Spec.Resource.Namespace{ - err :=c.kyvernoClient.KyvernoV1().GenerateRequests(config.KubePolicyNamespace).Delete(v.GetName(),&metav1.DeleteOptions{}) + if engineResponse.PolicyResponse.Policy == v.Spec.Policy && engineResponse.PolicyResponse.Resource.Name == v.Spec.Resource.Name && engineResponse.PolicyResponse.Resource.Kind == v.Spec.Resource.Kind && engineResponse.PolicyResponse.Resource.Namespace == v.Spec.Resource.Namespace { + err := c.kyvernoClient.KyvernoV1().GenerateRequests(config.KubePolicyNamespace).Delete(v.GetName(), &metav1.DeleteOptions{}) if err != nil { logger.Error(err, " failed to delete generate request") } @@ -126,7 +126,7 @@ func (c *Controller) applyGenerate(resource unstructured.Unstructured, gr kyvern if len(engineResponse.PolicyResponse.Rules) > 1 { engineResponse.PolicyResponse.Rules = append(engineResponse.PolicyResponse.Rules[:i], engineResponse.PolicyResponse.Rules[i+1:]...) continue - }else if len(engineResponse.PolicyResponse.Rules) == 1 { + } else if len(engineResponse.PolicyResponse.Rules) == 1 { engineResponse.PolicyResponse.Rules = []response.RuleResponse{} } } @@ -160,7 +160,7 @@ func (c *Controller) applyGeneratePolicy(log logr.Logger, policyContext engine.P continue } startTime := time.Now() - genResource, err := applyRule(log, c.client, rule, resource, ctx, policy.Name,gr) + genResource, err := applyRule(log, c.client, rule, resource, ctx, policy.Name, gr) if err != nil { return nil, err } @@ -217,7 +217,7 @@ func updateGenerateExecutionTime(newTime time.Duration, oldAverageTimeString str return time.Duration(newAverageTimeInNanoSeconds) * time.Nanosecond } -func applyRule(log logr.Logger, client *dclient.Client, rule kyverno.Rule, resource unstructured.Unstructured, ctx context.EvalInterface, policy string,gr kyverno.GenerateRequest) (kyverno.ResourceSpec, error) { +func applyRule(log logr.Logger, client *dclient.Client, rule kyverno.Rule, resource unstructured.Unstructured, ctx context.EvalInterface, policy string, gr kyverno.GenerateRequest) (kyverno.ResourceSpec, error) { var rdata map[string]interface{} var err error var mode ResourceMode @@ -287,10 +287,8 @@ func applyRule(log logr.Logger, client *dclient.Client, rule kyverno.Rule, resou return newGenResource, nil } - logger := log.WithValues("genKind", genKind, "genAPIVersion", genAPIVersion, "genNamespace", genNamespace, "genName", genName) - // build the resource template newResource := &unstructured.Unstructured{} newResource.SetUnstructuredContent(rdata) diff --git a/pkg/policy/validate.go b/pkg/policy/validate.go index 3d157b13c3..136ca94705 100644 --- a/pkg/policy/validate.go +++ b/pkg/policy/validate.go @@ -22,8 +22,14 @@ import ( // - One operation per rule // - ResourceDescription mandatory checks func Validate(policyRaw []byte, client *dclient.Client, mock bool, openAPIController *openapi.Controller) error { + // check for invalid fields + err := checkInvalidFields(policyRaw) + if err != nil { + return err + } + var p kyverno.ClusterPolicy - err := json.Unmarshal(policyRaw, &p) + err = json.Unmarshal(policyRaw, &p) if err != nil { return fmt.Errorf("failed to unmarshal policy admission request err %v", err) } @@ -119,6 +125,34 @@ func Validate(policyRaw []byte, client *dclient.Client, mock bool, openAPIContro return nil } +// checkInvalidFields - checks invalid fields in webhook policy request +// policy supports 5 json fields in types.go i.e. "apiVersion", "kind", "metadata", "spec", "status" +// If the webhook request policy contains new fields then block creation of policy +func checkInvalidFields(policyRaw []byte) error { + // hardcoded supported fields by policy + var allowedKeys = []string{"apiVersion", "kind", "metadata", "spec", "status"} + var data interface{} + err := json.Unmarshal(policyRaw, &data) + if err != nil { + return fmt.Errorf("failed to unmarshal policy admission request err %v", err) + } + mapData := data.(map[string]interface{}) + // validate any new fields in the admission request against the supported fields and block the request with any new fields + for requestField, _ := range mapData { + ok := false + for _, allowedField := range allowedKeys { + if requestField == allowedField { + ok = true + break + } + } + if !ok { + return fmt.Errorf("unknown field \"%s\" in policy admission request", requestField) + } + } + return nil +} + // doesMatchAndExcludeConflict checks if the resultant // of match and exclude block is not an empty set func doesMatchAndExcludeConflict(rule kyverno.Rule) bool { diff --git a/pkg/webhooks/generation.go b/pkg/webhooks/generation.go index c56f256a0d..dab3ef07eb 100644 --- a/pkg/webhooks/generation.go +++ b/pkg/webhooks/generation.go @@ -58,7 +58,7 @@ func (ws *WebhookServer) HandleGenerate(request *v1beta1.AdmissionRequest, polic } for _, v := range grList.Items { if engineResponse.PolicyResponse.Policy == v.Spec.Policy && engineResponse.PolicyResponse.Resource.Name == v.Spec.Resource.Name && engineResponse.PolicyResponse.Resource.Kind == v.Spec.Resource.Kind && engineResponse.PolicyResponse.Resource.Namespace == v.Spec.Resource.Namespace { - err := ws.kyvernoClient.KyvernoV1().GenerateRequests(config.KubePolicyNamespace).Delete(v.GetName(),&metav1.DeleteOptions{}) + err := ws.kyvernoClient.KyvernoV1().GenerateRequests(config.KubePolicyNamespace).Delete(v.GetName(), &metav1.DeleteOptions{}) if err != nil { logger.Error(err, "failed to update gr") } @@ -67,7 +67,7 @@ func (ws *WebhookServer) HandleGenerate(request *v1beta1.AdmissionRequest, polic if len(engineResponse.PolicyResponse.Rules) > 1 { engineResponse.PolicyResponse.Rules = append(engineResponse.PolicyResponse.Rules[:i], engineResponse.PolicyResponse.Rules[i+1:]...) continue - }else if len(engineResponse.PolicyResponse.Rules) == 1 { + } else if len(engineResponse.PolicyResponse.Rules) == 1 { engineResponse.PolicyResponse.Rules = []response.RuleResponse{} } }