From 8bfed75498fbeba0e131e96209dacbec4b111d8a Mon Sep 17 00:00:00 2001 From: Robert Brennan Date: Fri, 9 Jun 2023 14:35:36 -0400 Subject: [PATCH] Minor fixes for NSA checks (#952) * fix rbac checks * fix sensitive env var check * add test case * fix service account check * fix comment --------- Co-authored-by: Andrew Suderman --- checks/automountServiceAccountToken.yaml | 5 ++--- checks/clusterrolebindingClusterAdmin.yaml | 2 ++ checks/clusterrolebindingPodExecAttach.yaml | 2 ++ checks/sensitiveContainerEnvVar.yaml | 4 +++- pkg/config/schema.go | 8 ++++++-- .../success.no_serviceaccount.yaml | 12 ++++++++++++ test/checks/insecureCapabilities/failure.add.yaml | 14 ++++++++++++++ test/checks/insecureCapabilities/mutated.add.yaml | 14 ++++++++++++++ test/mutation_test.go | 1 + 9 files changed, 56 insertions(+), 6 deletions(-) create mode 100644 test/checks/automountServiceAccountToken/success.no_serviceaccount.yaml create mode 100644 test/checks/insecureCapabilities/failure.add.yaml create mode 100644 test/checks/insecureCapabilities/mutated.add.yaml diff --git a/checks/automountServiceAccountToken.yaml b/checks/automountServiceAccountToken.yaml index 69cc01c5a..e1a80283b 100644 --- a/checks/automountServiceAccountToken.yaml +++ b/checks/automountServiceAccountToken.yaml @@ -5,7 +5,6 @@ target: PodSpec schema: '$schema': http://json-schema.org/draft-07/schema type: object - required: ["serviceAccountName"] properties: serviceAccountName: type: string @@ -15,12 +14,11 @@ schema: const: true additionalSchemaStrings: ServiceAccount: | + {{ if not (eq .Polaris.PodSpec.automountServiceAccountToken false) }} type: object required: - metadata - {{ if not (eq .Polaris.PodSpec.automountServiceAccountToken false) }} - automountServiceAccountToken - {{ end }} properties: metadata: type: object @@ -34,3 +32,4 @@ additionalSchemaStrings: type: boolean const: false {{ end }} + {{ end }} diff --git a/checks/clusterrolebindingClusterAdmin.yaml b/checks/clusterrolebindingClusterAdmin.yaml index c4464ec70..86f1d91ab 100644 --- a/checks/clusterrolebindingClusterAdmin.yaml +++ b/checks/clusterrolebindingClusterAdmin.yaml @@ -39,6 +39,7 @@ additionalSchemaStrings: rbac.authorization.k8s.io/ClusterRole: | type: object # Do not alert on default ClusterRoleBindings. + {{ if (ne .roleRef.name "view") }} {{ if and (ne .metadata.name "cluster-admin") (not (hasPrefix .metadata.name "system:")) (ne .metadata.name "gce:podsecuritypolicy:calico-sa") }} required: ["metadata", "rules"] allOf: @@ -86,3 +87,4 @@ additionalSchemaStrings: - "patch" - "delete" {{ end }} + {{ end }} diff --git a/checks/clusterrolebindingPodExecAttach.yaml b/checks/clusterrolebindingPodExecAttach.yaml index cf5d510c2..de698b515 100644 --- a/checks/clusterrolebindingPodExecAttach.yaml +++ b/checks/clusterrolebindingPodExecAttach.yaml @@ -37,6 +37,7 @@ additionalSchemaStrings: rbac.authorization.k8s.io/ClusterRole: | type: object # Do not alert on default ClusterRoleBindings. + {{ if (ne .roleRef.name "view") }} {{ if and (ne .metadata.name "cluster-admin") (not (hasPrefix .metadata.name "system:")) (ne .metadata.name "gce:podsecuritypolicy:calico-sa") }} required: ["metadata", "rules"] allOf: @@ -80,3 +81,4 @@ additionalSchemaStrings: - const: 'get' - const: 'create' {{ end }} + {{ end }} diff --git a/checks/sensitiveContainerEnvVar.yaml b/checks/sensitiveContainerEnvVar.yaml index 8527a94cd..25e6219b6 100644 --- a/checks/sensitiveContainerEnvVar.yaml +++ b/checks/sensitiveContainerEnvVar.yaml @@ -10,7 +10,9 @@ schemaString: | type: array items: type: object - oneOf: + anyOf: + - not: + required: ["value"] - required: ["name", "value"] properties: name: diff --git a/pkg/config/schema.go b/pkg/config/schema.go index b72700426..87c5c750d 100644 --- a/pkg/config/schema.go +++ b/pkg/config/schema.go @@ -236,11 +236,15 @@ func (check SchemaCheck) TemplateForResource(res interface{}) (*SchemaCheck, err if err != nil { return nil, err } + templated := w.String() + if strings.TrimSpace(templated) == "" { + continue + } if kind == "" { - newCheck.SchemaString = w.String() + newCheck.SchemaString = templated } else { - newCheck.AdditionalSchemaStrings[kind] = w.String() + newCheck.AdditionalSchemaStrings[kind] = templated } } diff --git a/test/checks/automountServiceAccountToken/success.no_serviceaccount.yaml b/test/checks/automountServiceAccountToken/success.no_serviceaccount.yaml new file mode 100644 index 000000000..777b584f6 --- /dev/null +++ b/test/checks/automountServiceAccountToken/success.no_serviceaccount.yaml @@ -0,0 +1,12 @@ +# This succeeds because automounting is disabled on the pod, and there is no specified service account +apiVersion: v1 +kind: Pod +metadata: + name: test-pod +spec: + automountServiceAccountToken: false + containers: + - name: nginx-foo + image: nginx-foo + ports: + - containerPort: 80 diff --git a/test/checks/insecureCapabilities/failure.add.yaml b/test/checks/insecureCapabilities/failure.add.yaml new file mode 100644 index 000000000..c73c79e7e --- /dev/null +++ b/test/checks/insecureCapabilities/failure.add.yaml @@ -0,0 +1,14 @@ +apiVersion: v1 +kind: Pod +metadata: + name: nginx + labels: + env: test +spec: + containers: + - name: nginx + image: nginx + securityContext: + capabilities: + add: + - ALL diff --git a/test/checks/insecureCapabilities/mutated.add.yaml b/test/checks/insecureCapabilities/mutated.add.yaml new file mode 100644 index 000000000..b782362ea --- /dev/null +++ b/test/checks/insecureCapabilities/mutated.add.yaml @@ -0,0 +1,14 @@ +apiVersion: v1 +kind: Pod +metadata: + name: nginx + labels: + env: test +spec: + containers: + - name: nginx + image: nginx + securityContext: + capabilities: + drop: + - ALL diff --git a/test/mutation_test.go b/test/mutation_test.go index bd4aaa0c1..1e4fe8f21 100644 --- a/test/mutation_test.go +++ b/test/mutation_test.go @@ -27,6 +27,7 @@ checks: memoryLimitsMissing: warning readinessProbeMissing: warning livenessProbeMissing: warning + insecureCapabilities: warning ` func TestMutations(t *testing.T) {