From 9b7d7132b750f3ee0e824179b7fe2ea0cb0916ed Mon Sep 17 00:00:00 2001 From: simar7 <1254783+simar7@users.noreply.github.com> Date: Thu, 18 Apr 2024 19:17:43 -0600 Subject: [PATCH] fix(misconf): Parse JSON k8s manifests properly (#6490) --- go.mod | 2 +- pkg/iac/scanners/kubernetes/parser/parser.go | 7 +- pkg/iac/scanners/kubernetes/scanner_test.go | 319 +++++++++---------- 3 files changed, 160 insertions(+), 168 deletions(-) diff --git a/go.mod b/go.mod index 5ab8f34700c..93089feee02 100644 --- a/go.mod +++ b/go.mod @@ -138,6 +138,7 @@ require ( github.com/zclconf/go-cty-yaml v1.0.3 golang.org/x/crypto v0.19.0 helm.sh/helm/v3 v3.14.2 + sigs.k8s.io/yaml v1.4.0 ) require ( @@ -433,5 +434,4 @@ require ( sigs.k8s.io/kustomize/api v0.13.5-0.20230601165947-6ce0bf390ce3 // indirect sigs.k8s.io/kustomize/kyaml v0.14.3-0.20230601165947-6ce0bf390ce3 // indirect sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect - sigs.k8s.io/yaml v1.4.0 // indirect ) diff --git a/pkg/iac/scanners/kubernetes/parser/parser.go b/pkg/iac/scanners/kubernetes/parser/parser.go index fd2c12ecb32..658d7a2e6cf 100644 --- a/pkg/iac/scanners/kubernetes/parser/parser.go +++ b/pkg/iac/scanners/kubernetes/parser/parser.go @@ -12,6 +12,7 @@ import ( "strings" "gopkg.in/yaml.v3" + kyaml "sigs.k8s.io/yaml" "github.com/aquasecurity/trivy/pkg/iac/debug" "github.com/aquasecurity/trivy/pkg/iac/detection" @@ -113,7 +114,11 @@ func (p *Parser) Parse(r io.Reader, path string) ([]interface{}, error) { if err := json.Unmarshal(contents, &target); err != nil { return nil, err } - return []interface{}{target}, nil + + contents, err = kyaml.JSONToYAML(contents) // convert into yaml to reuse file parsing logic + if err != nil { + return nil, err + } } var results []interface{} diff --git a/pkg/iac/scanners/kubernetes/scanner_test.go b/pkg/iac/scanners/kubernetes/scanner_test.go index 931b2a09b8f..11d58b3ce30 100644 --- a/pkg/iac/scanners/kubernetes/scanner_test.go +++ b/pkg/iac/scanners/kubernetes/scanner_test.go @@ -14,7 +14,7 @@ import ( "github.com/stretchr/testify/require" ) -func Test_BasicScan(t *testing.T) { +func Test_BasicScan_YAML(t *testing.T) { fs := testutil.CreateFS(t, map[string]string{ "/code/example.yaml": ` @@ -28,182 +28,166 @@ spec: image: busybox name: hello `, - "/rules/lib.k8s.rego": ` - package lib.kubernetes - - default is_gatekeeper = false - - is_gatekeeper { - has_field(input, "review") - has_field(input.review, "object") - } - - object = input { - not is_gatekeeper - } + "/rules/rule.rego": ` +package builtin.kubernetes.KSV011 - object = input.review.object { - is_gatekeeper - } +import data.lib.kubernetes +import data.lib.utils - format(msg) = gatekeeper_format { - is_gatekeeper - gatekeeper_format = {"msg": msg} - } +default failLimitsCPU = false - format(msg) = msg { - not is_gatekeeper - } +__rego_metadata__ := { + "id": "KSV011", + "avd_id": "AVD-KSV-0011", + "title": "CPU not limited", + "short_code": "limit-cpu", + "version": "v1.0.0", + "severity": "LOW", + "type": "Kubernetes Security Check", + "description": "Enforcing CPU limits prevents DoS via resource exhaustion.", + "recommended_actions": "Set a limit value under 'containers[].resources.limits.cpu'.", + "url": "https://cloud.google.com/blog/products/containers-kubernetes/kubernetes-best-practices-resource-requests-and-limits", +} - name = object.metadata.name +__rego_input__ := { + "combine": false, + "selector": [{"type": "kubernetes"}], +} - default namespace = "default" +# getLimitsCPUContainers returns all containers which have set resources.limits.cpu +getLimitsCPUContainers[container] { + allContainers := kubernetes.containers[_] + utils.has_key(allContainers.resources.limits, "cpu") + container := allContainers.name +} - namespace = object.metadata.namespace +# getNoLimitsCPUContainers returns all containers which have not set +# resources.limits.cpu +getNoLimitsCPUContainers[container] { + container := kubernetes.containers[_].name + not getLimitsCPUContainers[container] +} - #annotations = object.metadata.annotations +# failLimitsCPU is true if containers[].resources.limits.cpu is not set +# for ANY container +failLimitsCPU { + count(getNoLimitsCPUContainers) > 0 +} - kind = object.kind +deny[res] { + failLimitsCPU - is_pod { - kind = "Pod" - } + msg := kubernetes.format(sprintf("Container '%s' of %s '%s' should set 'resources.limits.cpu'", [getNoLimitsCPUContainers[_], kubernetes.kind, kubernetes.name])) - is_cronjob { - kind = "CronJob" - } + res := { + "msg": msg, + "id": __rego_metadata__.id, + "title": __rego_metadata__.title, + "severity": __rego_metadata__.severity, + "type": __rego_metadata__.type, + "startline": 6, + "endline": 10, + } +} +`, + }) - default is_controller = false + scanner := NewScanner( + options.ScannerWithPolicyDirs("rules"), + options.ScannerWithEmbeddedLibraries(true), + ) - is_controller { - kind = "Deployment" - } + results, err := scanner.ScanFS(context.TODO(), fs, "code") + require.NoError(t, err) - is_controller { - kind = "StatefulSet" - } + require.Len(t, results.GetFailed(), 1) - is_controller { - kind = "DaemonSet" - } + assert.Equal(t, scan.Rule{ + AVDID: "AVD-KSV-0011", + Aliases: []string{"KSV011"}, + ShortCode: "limit-cpu", + Summary: "CPU not limited", + Explanation: "Enforcing CPU limits prevents DoS via resource exhaustion.", + Impact: "", + Resolution: "Set a limit value under 'containers[].resources.limits.cpu'.", + Provider: "kubernetes", + Service: "general", + Links: []string{"https://cloud.google.com/blog/products/containers-kubernetes/kubernetes-best-practices-resource-requests-and-limits"}, + Severity: "LOW", + Terraform: &scan.EngineMetadata{}, + CloudFormation: &scan.EngineMetadata{}, + CustomChecks: scan.CustomChecks{Terraform: (*scan.TerraformCustomCheck)(nil)}, + RegoPackage: "data.builtin.kubernetes.KSV011", + Frameworks: map[framework.Framework][]string{}, + }, results.GetFailed()[0].Rule()) - is_controller { - kind = "ReplicaSet" - } + failure := results.GetFailed()[0] + actualCode, err := failure.GetCode() + require.NoError(t, err) + for i := range actualCode.Lines { + actualCode.Lines[i].Highlighted = "" + } + assert.Equal(t, []scan.Line{ + { + Number: 6, + Content: "spec: ", + IsCause: true, + FirstCause: true, + Annotation: "", + }, + { + Number: 7, + Content: " containers: ", + IsCause: true, + Annotation: "", + }, + { + Number: 8, + Content: " - command: [\"sh\", \"-c\", \"echo 'Hello' && sleep 1h\"]", + IsCause: true, + Annotation: "", + }, + { + Number: 9, + Content: " image: busybox", + IsCause: true, + Annotation: "", + }, + { + Number: 10, + Content: " name: hello", + IsCause: true, + LastCause: true, + Annotation: "", + }, + }, actualCode.Lines) +} - is_controller { - kind = "ReplicationController" - } +func Test_BasicScan_JSON(t *testing.T) { - is_controller { - kind = "Job" - } - - split_image(image) = [image, "latest"] { - not contains(image, ":") - } - - split_image(image) = [image_name, tag] { - [image_name, tag] = split(image, ":") - } - - pod_containers(pod) = all_containers { - keys = {"containers", "initContainers"} - all_containers = [c | keys[k]; c = pod.spec[k][_]] - } - - containers[container] { - pods[pod] - all_containers = pod_containers(pod) - container = all_containers[_] - } - - containers[container] { - all_containers = pod_containers(object) - container = all_containers[_] - } - - pods[pod] { - is_pod - pod = object - } - - pods[pod] { - is_controller - pod = object.spec.template - } - - pods[pod] { - is_cronjob - pod = object.spec.jobTemplate.spec.template - } - - volumes[volume] { - pods[pod] - volume = pod.spec.volumes[_] - } - - dropped_capability(container, cap) { - container.securityContext.capabilities.drop[_] == cap - } - - added_capability(container, cap) { - container.securityContext.capabilities.add[_] == cap - } - - has_field(obj, field) { - obj[field] - } - - no_read_only_filesystem(c) { - not has_field(c, "securityContext") - } - - no_read_only_filesystem(c) { - has_field(c, "securityContext") - not has_field(c.securityContext, "readOnlyRootFilesystem") - } - - privilege_escalation_allowed(c) { - not has_field(c, "securityContext") - } - - privilege_escalation_allowed(c) { - has_field(c, "securityContext") - has_field(c.securityContext, "allowPrivilegeEscalation") - } - - annotations[annotation] { - pods[pod] - annotation = pod.metadata.annotations - } - - host_ipcs[host_ipc] { - pods[pod] - host_ipc = pod.spec.hostIPC - } - - host_networks[host_network] { - pods[pod] - host_network = pod.spec.hostNetwork - } - - host_pids[host_pid] { - pods[pod] - host_pid = pod.spec.hostPID - } - - host_aliases[host_alias] { - pods[pod] - host_alias = pod.spec - } - `, - "/rules/lib.util.rego": ` - package lib.utils - - has_key(x, k) { - _ = x[k] - }`, + fs := testutil.CreateFS(t, map[string]string{ + "/code/example.json": ` +{ + "apiVersion": "v1", + "kind": "Pod", + "metadata": { + "name": "hello-cpu-limit" + }, + "spec": { + "containers": [ + { + "command": [ + "sh", + "-c", + "echo 'Hello' && sleep 1h" + ], + "image": "busybox", + "name": "hello" + } + ] + } +} +`, "/rules/rule.rego": ` package builtin.kubernetes.KSV011 @@ -268,7 +252,10 @@ deny[res] { `, }) - scanner := NewScanner(options.ScannerWithPolicyDirs("rules")) + scanner := NewScanner( + options.ScannerWithPolicyDirs("rules"), + options.ScannerWithEmbeddedLibraries(true), + ) results, err := scanner.ScanFS(context.TODO(), fs, "code") require.NoError(t, err) @@ -303,32 +290,32 @@ deny[res] { assert.Equal(t, []scan.Line{ { Number: 6, - Content: "spec: ", + Content: ` "name": "hello-cpu-limit"`, IsCause: true, FirstCause: true, Annotation: "", }, { Number: 7, - Content: " containers: ", + Content: ` },`, IsCause: true, Annotation: "", }, { Number: 8, - Content: " - command: [\"sh\", \"-c\", \"echo 'Hello' && sleep 1h\"]", + Content: ` "spec": {`, IsCause: true, Annotation: "", }, { Number: 9, - Content: " image: busybox", + Content: ` "containers": [`, IsCause: true, Annotation: "", }, { Number: 10, - Content: " name: hello", + Content: ` {`, IsCause: true, LastCause: true, Annotation: "",