From befcd73ea1b453a66922bf15ec149ec94db03aaf Mon Sep 17 00:00:00 2001 From: Jim Bugwadia Date: Mon, 11 Mar 2024 02:32:05 -0700 Subject: [PATCH] add control names and images to PSS results (#9869) * add control names and images to PSS results Signed-off-by: Jim Bugwadia * remove init Signed-off-by: Jim Bugwadia * fix tets Signed-off-by: Jim Bugwadia * update chainsaw tests Signed-off-by: Jim Bugwadia * add unit test Signed-off-by: Jim Bugwadia --------- Signed-off-by: Jim Bugwadia Co-authored-by: shuting --- .../kubectl-kyverno/commands/apply/command.go | 19 +++-- .../handlers/validation/validate_image.go | 6 +- .../handlers/validation/validate_pss.go | 69 ++++++++++++++++--- .../handlers/validation/validate_resource.go | 2 +- pkg/pss/evaluate.go | 4 +- pkg/pss/utils/mapping.go | 16 ++++- pkg/pss/utils/types.go | 1 + pkg/utils/report/metadata.go | 3 +- pkg/utils/report/results.go | 67 ++++++++++++------ .../report-assert.yaml | 1 + 10 files changed, 139 insertions(+), 49 deletions(-) diff --git a/cmd/cli/kubectl-kyverno/commands/apply/command.go b/cmd/cli/kubectl-kyverno/commands/apply/command.go index 0d041cac0f..ee6a22f3a7 100644 --- a/cmd/cli/kubectl-kyverno/commands/apply/command.go +++ b/cmd/cli/kubectl-kyverno/commands/apply/command.go @@ -89,7 +89,7 @@ func Command() *cobra.Command { cmd.SilenceErrors = true printSkippedAndInvalidPolicies(out, skipInvalidPolicies) if applyCommandConfig.PolicyReport { - printReport(out, responses, applyCommandConfig.AuditWarn) + printReports(out, responses, applyCommandConfig.AuditWarn) } else if table { printTable(out, detailedResults, applyCommandConfig.AuditWarn, responses...) } else { @@ -167,7 +167,7 @@ func (c *ApplyCommandConfig) applyCommandHelper(out io.Writer) (*processor.Resul if err != nil { return rc, resources1, skipInvalidPolicies, responses1, fmt.Errorf("Error: failed to load exceptions (%s)", err) } - if !c.Stdin { + if !c.Stdin && !c.PolicyReport { var policyRulesCount int for _, policy := range policies { policyRulesCount += len(autogen.ComputeRules(policy)) @@ -446,18 +446,17 @@ func printSkippedAndInvalidPolicies(out io.Writer, skipInvalidPolicies SkippedIn } } -func printReport(out io.Writer, engineResponses []engineapi.EngineResponse, auditWarn bool) { +func printReports(out io.Writer, engineResponses []engineapi.EngineResponse, auditWarn bool) { clustered, namespaced := report.ComputePolicyReports(auditWarn, engineResponses...) - if len(clustered) > 0 || len(namespaced) > 0 { - fmt.Fprintln(out, divider) - fmt.Fprintln(out, "POLICY REPORT:") - fmt.Fprintln(out, divider) + if len(clustered) > 0 { report := report.MergeClusterReports(clustered) yamlReport, _ := yaml.Marshal(report) fmt.Fprintln(out, string(yamlReport)) - } else { - fmt.Fprintln(out, divider) - fmt.Fprintln(out, "POLICY REPORT: skip generating policy report (no validate policy found/resource skipped)") + } + for _, r := range namespaced { + fmt.Fprintln(out, string("---")) + yamlReport, _ := yaml.Marshal(r) + fmt.Fprintln(out, string(yamlReport)) } } diff --git a/pkg/engine/handlers/validation/validate_image.go b/pkg/engine/handlers/validation/validate_image.go index 66ba799cca..ee609e3fa9 100644 --- a/pkg/engine/handlers/validation/validate_image.go +++ b/pkg/engine/handlers/validation/validate_image.go @@ -67,7 +67,7 @@ func (h validateImageHandler) Process( for _, v := range rule.VerifyImages { imageVerify := v.Convert() for _, infoMap := range policyContext.JSONContext().ImageInfo() { - for name, imageInfo := range infoMap { + for _, imageInfo := range infoMap { image := imageInfo.String() if !engineutils.ImageMatches(image, imageVerify.ImageReferences) { @@ -76,7 +76,7 @@ func (h validateImageHandler) Process( } logger.V(4).Info("validating image", "image", image) - if v, err := validateImage(policyContext, imageVerify, name, imageInfo, logger); err != nil { + if v, err := validateImage(policyContext, imageVerify, imageInfo, logger); err != nil { return resource, handlers.WithFail(rule, engineapi.ImageVerify, err.Error()) } else if v == engineapi.ImageVerificationSkip { skippedImages = append(skippedImages, image) @@ -98,7 +98,7 @@ func (h validateImageHandler) Process( } } -func validateImage(ctx engineapi.PolicyContext, imageVerify *kyvernov1.ImageVerification, name string, imageInfo apiutils.ImageInfo, log logr.Logger) (engineapi.ImageVerificationMetadataStatus, error) { +func validateImage(ctx engineapi.PolicyContext, imageVerify *kyvernov1.ImageVerification, imageInfo apiutils.ImageInfo, log logr.Logger) (engineapi.ImageVerificationMetadataStatus, error) { var verified engineapi.ImageVerificationMetadataStatus var err error image := imageInfo.String() diff --git a/pkg/engine/handlers/validation/validate_pss.go b/pkg/engine/handlers/validation/validate_pss.go index 78d5bf7a01..fa5574cdd7 100644 --- a/pkg/engine/handlers/validation/validate_pss.go +++ b/pkg/engine/handlers/validation/validate_pss.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "regexp" "strings" "github.com/go-logr/logr" @@ -14,6 +15,7 @@ import ( engineutils "github.com/kyverno/kyverno/pkg/engine/utils" "github.com/kyverno/kyverno/pkg/pss" pssutils "github.com/kyverno/kyverno/pkg/pss/utils" + "github.com/kyverno/kyverno/pkg/utils/api" appsv1 "k8s.io/api/apps/v1" batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" @@ -76,6 +78,7 @@ func (h validatePssHandler) Process( } allowed, pssChecks := pss.EvaluatePod(levelVersion, podSecurity.Exclude, pod) pssChecks = convertChecks(pssChecks, resource.GetKind()) + pssChecks = addImages(pssChecks, policyContext.JSONContext().ImageInfo()) podSecurityChecks := engineapi.PodSecurityChecks{ Level: podSecurity.Level, Version: podSecurity.Version, @@ -134,12 +137,65 @@ func convertChecks(checks []pssutils.PSSCheckResult, kind string) (newChecks []p return checks } +// Extract container names from PSS error details. Here are some example inputs: +// - "containers \"nginx\", \"busybox\" must set securityContext.allowPrivilegeEscalation=false" +// - "containers \"nginx\", \"busybox\" must set securityContext.capabilities.drop=[\"ALL\"]" +// - "pod or containers \"nginx\", \"busybox\" must set securityContext.runAsNonRoot=true" +// - "pod or containers \"nginx\", \"busybox\" must set securityContext.seccompProfile.type to \"RuntimeDefault\" or \"Localhost\"" +// - "pod or container \"nginx\" must set securityContext.seccompProfile.type to \"RuntimeDefault\" or \"Localhost\"" +// - "container \"nginx\" must set securityContext.allowPrivilegeEscalation=false" +var regexContainerNames = regexp.MustCompile(`container(?:s)?\s*(.*?)\s*must`) + +func addImages(checks []pssutils.PSSCheckResult, imageInfos map[string]map[string]api.ImageInfo) []pssutils.PSSCheckResult { + for i, check := range checks { + text := check.CheckResult.ForbiddenDetail + matches := regexContainerNames.FindAllStringSubmatch(text, -1) + if len(matches) > 0 { + s := strings.ReplaceAll(matches[0][1], "\"", "") + s = strings.ReplaceAll(s, " ", "") + containerNames := strings.Split(s, ",") + checks[i].Images = getImages(containerNames, imageInfos) + } + } + return checks +} + +// return image references for containers +func getImages(containerNames []string, imageInfos map[string]map[string]api.ImageInfo) []string { + var images []string + for _, cn := range containerNames { + image := getImageReference(cn, imageInfos) + images = append(images, image) + } + return images +} + +// return an image references for a container name +// if the image is not found, the name is returned +func getImageReference(name string, imageInfos map[string]map[string]api.ImageInfo) string { + if containers, ok := imageInfos["containers"]; ok { + if imageInfo, ok := containers[name]; ok { + return imageInfo.String() + } + } + if initContainers, ok := imageInfos["initContainers"]; ok { + if imageInfo, ok := initContainers[name]; ok { + return imageInfo.String() + } + } + if ephemeralContainers, ok := imageInfos["ephemeralContainers"]; ok { + if imageInfo, ok := ephemeralContainers[name]; ok { + return imageInfo.String() + } + } + return name +} + func getSpec(resource unstructured.Unstructured) (podSpec *corev1.PodSpec, metadata *metav1.ObjectMeta, err error) { kind := resource.GetKind() if kind == "DaemonSet" || kind == "Deployment" || kind == "Job" || kind == "StatefulSet" || kind == "ReplicaSet" || kind == "ReplicationController" { var deployment appsv1.Deployment - resourceBytes, err := resource.MarshalJSON() if err != nil { return nil, nil, err @@ -153,7 +209,6 @@ func getSpec(resource unstructured.Unstructured) (podSpec *corev1.PodSpec, metad return podSpec, metadata, nil } else if kind == "CronJob" { var cronJob batchv1.CronJob - resourceBytes, err := resource.MarshalJSON() if err != nil { return nil, nil, err @@ -164,9 +219,9 @@ func getSpec(resource unstructured.Unstructured) (podSpec *corev1.PodSpec, metad } podSpec = &cronJob.Spec.JobTemplate.Spec.Template.Spec metadata = &cronJob.Spec.JobTemplate.ObjectMeta + return podSpec, metadata, nil } else if kind == "Pod" { var pod corev1.Pod - resourceBytes, err := resource.MarshalJSON() if err != nil { return nil, nil, err @@ -178,11 +233,7 @@ func getSpec(resource unstructured.Unstructured) (podSpec *corev1.PodSpec, metad podSpec = &pod.Spec metadata = &pod.ObjectMeta return podSpec, metadata, nil - } else { - return nil, nil, fmt.Errorf("could not find correct resource type") } - if err != nil { - return nil, nil, err - } - return podSpec, metadata, err + + return nil, nil, fmt.Errorf("could not find correct resource type") } diff --git a/pkg/engine/handlers/validation/validate_resource.go b/pkg/engine/handlers/validation/validate_resource.go index ef952ed8cc..bcd2e2071c 100644 --- a/pkg/engine/handlers/validation/validate_resource.go +++ b/pkg/engine/handlers/validation/validate_resource.go @@ -331,7 +331,7 @@ func (v *validator) validatePatterns(resource unstructured.Unstructured) *engine return engineapi.RuleFail(v.rule.Name, engineapi.Validation, v.buildErrorMessage(err, pe.Path)) } - return engineapi.RuleError(v.rule.Name, engineapi.Validation, v.buildErrorMessage(err, pe.Path), nil) + return engineapi.RuleError(v.rule.Name, engineapi.Validation, v.buildErrorMessage(err, ""), nil) } v.log.V(4).Info("successfully processed rule") diff --git a/pkg/pss/evaluate.go b/pkg/pss/evaluate.go index a1fd22ccba..9fea6bd41d 100644 --- a/pkg/pss/evaluate.go +++ b/pkg/pss/evaluate.go @@ -77,7 +77,7 @@ func exemptExclusions(defaultCheckResults, excludeCheckResults []pssutils.PSSChe } for _, excludeResult := range excludeCheckResults { - for _, checkID := range pssutils.PSS_controls_to_check_id[exclude.ControlName] { + for _, checkID := range pssutils.PSS_control_name_to_ids[exclude.ControlName] { if excludeResult.ID == checkID { for _, excludeFieldErr := range *excludeResult.CheckResult.ErrList { var excludeField, excludeContainerType string @@ -313,7 +313,7 @@ func GetPodWithMatchingContainers(exclude kyvernov1.PodSecurityStandard, pod *co // Get restrictedFields from Check.ID func GetRestrictedFields(check policy.Check) []pssutils.RestrictedField { - for _, control := range pssutils.PSS_controls_to_check_id { + for _, control := range pssutils.PSS_control_name_to_ids { for _, checkID := range control { if string(check.ID) == checkID { return pssutils.PSS_controls[checkID] diff --git a/pkg/pss/utils/mapping.go b/pkg/pss/utils/mapping.go index f1dded830c..0f0e8e07b7 100644 --- a/pkg/pss/utils/mapping.go +++ b/pkg/pss/utils/mapping.go @@ -42,7 +42,7 @@ var PSS_container_level_control = []string{ // Translate PSS control to CheckResult.ID so that we can use PSS control in Kyverno policy // For PSS controls see: https://kubernetes.io/docs/concepts/security/pod-security-standards/ // For CheckResult.ID see: https://github.com/kubernetes/pod-security-admission/tree/master/policy -var PSS_controls_to_check_id = map[string][]string{ +var PSS_control_name_to_ids = map[string][]string{ // Controls with 2 different controls for each level // container-level control "Capabilities": { @@ -110,6 +110,20 @@ var PSS_controls_to_check_id = map[string][]string{ }, } +// reverse mapping of PSS_control_name_to_ids +var pss_control_id_to_name = map[string]string{} + +func PSSControlIDToName(id string) string { + if len(pss_control_id_to_name) == 0 { + for name, ids := range PSS_control_name_to_ids { + for _, id := range ids { + pss_control_id_to_name[id] = name + } + } + } + return pss_control_id_to_name[id] +} + var PSS_controls = map[string][]RestrictedField{ // Control name as key, same as ID field in CheckResult diff --git a/pkg/pss/utils/types.go b/pkg/pss/utils/types.go index da2b40827a..0a1e76c39b 100644 --- a/pkg/pss/utils/types.go +++ b/pkg/pss/utils/types.go @@ -13,4 +13,5 @@ type PSSCheckResult struct { ID string CheckResult policy.CheckResult RestrictedFields []RestrictedField + Images []string } diff --git a/pkg/utils/report/metadata.go b/pkg/utils/report/metadata.go index e4f8c29b0c..5af083329f 100644 --- a/pkg/utils/report/metadata.go +++ b/pkg/utils/report/metadata.go @@ -13,7 +13,6 @@ import ( kyvernov2beta1 "github.com/kyverno/kyverno/api/kyverno/v2beta1" engineapi "github.com/kyverno/kyverno/pkg/engine/api" controllerutils "github.com/kyverno/kyverno/pkg/utils/controller" - "k8s.io/api/admissionregistration/v1alpha1" admissionregistrationv1alpha1 "k8s.io/api/admissionregistration/v1alpha1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -169,7 +168,7 @@ func SetPolicyExceptionLabel(report kyvernov1alpha2.ReportInterface, exception k controllerutils.SetLabel(report, PolicyExceptionLabel(exception), exception.GetResourceVersion()) } -func SetValidatingAdmissionPolicyBindingLabel(report kyvernov1alpha2.ReportInterface, binding v1alpha1.ValidatingAdmissionPolicyBinding) { +func SetValidatingAdmissionPolicyBindingLabel(report kyvernov1alpha2.ReportInterface, binding admissionregistrationv1alpha1.ValidatingAdmissionPolicyBinding) { controllerutils.SetLabel(report, ValidatingAdmissionPolicyBindingLabel(binding), binding.GetResourceVersion()) } diff --git a/pkg/utils/report/results.go b/pkg/utils/report/results.go index 38bf51722a..062dc6421f 100644 --- a/pkg/utils/report/results.go +++ b/pkg/utils/report/results.go @@ -2,8 +2,8 @@ package report import ( "cmp" + "encoding/json" "slices" - "sort" "strings" "time" @@ -12,6 +12,7 @@ import ( kyvernov1alpha2 "github.com/kyverno/kyverno/api/kyverno/v1alpha2" policyreportv1alpha2 "github.com/kyverno/kyverno/api/policyreport/v1alpha2" engineapi "github.com/kyverno/kyverno/pkg/engine/api" + "github.com/kyverno/kyverno/pkg/pss/utils" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/tools/cache" @@ -87,14 +88,6 @@ func SeverityFromString(severity string) policyreportv1alpha2.PolicySeverity { return "" } -func addProperty(k, v string, result *policyreportv1alpha2.PolicyReportResult) { - if result.Properties == nil { - result.Properties = map[string]string{} - } - - result.Properties[k] = v -} - func ToPolicyReportResult(policyType engineapi.PolicyType, policyName string, ruleResult engineapi.RuleResponse, annotations map[string]string, resource *corev1.ObjectReference) policyreportv1alpha2.PolicyReportResult { result := policyreportv1alpha2.PolicyReportResult{ Source: kyverno.ValueKyvernoApp, @@ -122,18 +115,7 @@ func ToPolicyReportResult(policyType engineapi.PolicyType, policyName string, ru } pss := ruleResult.PodSecurityChecks() if pss != nil && len(pss.Checks) > 0 { - var controls []string - for _, check := range pss.Checks { - if !check.CheckResult.Allowed { - controls = append(controls, check.ID) - } - } - if len(controls) > 0 { - sort.Strings(controls) - addProperty("standard", string(pss.Level), &result) - addProperty("version", pss.Version, &result) - addProperty("controls", strings.Join(controls, ","), &result) - } + addPodSecurityProperties(pss, &result) } if policyType == engineapi.ValidatingAdmissionPolicyType { result.Source = "ValidatingAdmissionPolicy" @@ -145,6 +127,49 @@ func ToPolicyReportResult(policyType engineapi.PolicyType, policyName string, ru return result } +func addProperty(k, v string, result *policyreportv1alpha2.PolicyReportResult) { + if result.Properties == nil { + result.Properties = map[string]string{} + } + + result.Properties[k] = v +} + +type Control struct { + ID string + Name string + Images []string +} + +func addPodSecurityProperties(pss *engineapi.PodSecurityChecks, result *policyreportv1alpha2.PolicyReportResult) { + if pss == nil { + return + } + if result.Properties == nil { + result.Properties = map[string]string{} + } + var controls []Control + var controlIDs []string + for _, check := range pss.Checks { + if !check.CheckResult.Allowed { + controlName := utils.PSSControlIDToName(check.ID) + controlIDs = append(controlIDs, check.ID) + controls = append(controls, Control{ + ID: check.ID, + Name: controlName, + Images: check.Images, + }) + } + } + if len(controls) > 0 { + controlsJson, _ := json.Marshal(controls) + result.Properties["standard"] = string(pss.Level) + result.Properties["version"] = pss.Version + result.Properties["controls"] = strings.Join(controlIDs, ",") + result.Properties["controlsJSON"] = string(controlsJson) + } +} + func EngineResponseToReportResults(response engineapi.EngineResponse) []policyreportv1alpha2.PolicyReportResult { pol := response.Policy() policyName, _ := cache.MetaNamespaceKeyFunc(pol.AsKyvernoPolicy()) diff --git a/test/conformance/chainsaw/reports/background/test-report-background-mode/report-assert.yaml b/test/conformance/chainsaw/reports/background/test-report-background-mode/report-assert.yaml index a35451bd2f..3a8ed42bf8 100644 --- a/test/conformance/chainsaw/reports/background/test-report-background-mode/report-assert.yaml +++ b/test/conformance/chainsaw/reports/background/test-report-background-mode/report-assert.yaml @@ -19,6 +19,7 @@ results: policy: podsecurity-subrule-restricted properties: controls: capabilities_restricted + controlsJSON: '[{"ID":"capabilities_restricted","Name":"Capabilities","Images":["docker.io/dummyimagename:latest"]}]' standard: restricted version: latest result: fail