Merge #326

326: score/security: added optional, splitted probes as future replacemnt … r=zegl a=markuslackner

…for security context probe

Fixes #325

```
RELNOTE: container-security-context now deprecated and added optional probes container-security-context-user-group-id, container-security-context-privileged and container-security-context-readonlyrootfilesystem as replacement
```


Co-authored-by: Markus Lackner <[email protected]>

Author: bors[bot]

README.md

@@ -38,7 +38,7 @@ For a full list of checks, see [README_CHECKS.md](README_CHECKS.md).
 * Deployments and StatefulSets should have a `PodDisruptionPolicy`
 * Deployments and StatefulSets should have host PodAntiAffinity configured
 * Container probes, a readiness should be configured, and should not be identical to the liveness probe. Read more in  [README_PROBES.md](README_PROBES.md).
-* Container securityContext, run as high number user/group, do not run as root or with privileged root fs
+* Container securityContext, run as high number user/group, do not run as root or with privileged root fs. Read more in [README_SECURITYCONTEXT.md](README_SECURITYCONTEXT.md).
 * Stable APIs, use a stable API if available (supported: Deployments, StatefulSets, DaemonSet)
 
 ## Example output

README_CHECKS.md

@@ -14,7 +14,10 @@
 | pod-networkpolicy | Pod | Makes sure that all Pods are targeted by a NetworkPolicy | default |
 | networkpolicy-targets-pod | NetworkPolicy | Makes sure that all NetworkPolicies targets at least one Pod | default |
 | pod-probes | Pod | Makes sure that all Pods have safe probe configurations | default |
-| container-security-context | Pod | Makes sure that all pods have good securityContexts configured | default |
+| container-security-context | Pod | Makes sure that all pods have good securityContexts configured (*deprecated*, see [README_SECURITYCONTEXT.md](README_SECURITYCONTEXT.md) | default |
+| container-security-context-user-group-id | Pod | Makes sure that user and group ID are set and > 10000 | optional |
+| container-security-context-privileged | Pod | Makes sure that no Containers run in privileged mode | optional |
+| container-security-context-readonlyrootfilesystem | Pod | Makes sure that all Containers have a read only root filesystem | optional |
 | container-seccomp-profile | Pod | Makes sure that all pods have at a seccomp policy configured. | optional |
 | service-targets-pod | Service | Makes sure that all Services targets a Pod | default |
 | service-type | Service | Makes sure that the Service type is not NodePort | default |

README_SECURITYCONTEXT.md

@@ -0,0 +1,21 @@
+# Security Context
+
+The default `container-security-context` check checks the `SecurityContext`
+for 
+
+* Containers with writeable root filesystems
+* Containers that run with user ID or group ID < 10000
+* Privileged containers
+
+If you do not want all of this checks you can disable `container-security-context`
+and enable one or more of the following optional checks:
+
+* `container-security-context-user-group-id`
+* `container-security-context-privileged`
+* `container-security-context-readonlyrootfilesystem`
+
+## Release Timeline
+
+* v1.10: Introduce the three new checks.
+* v1.11: Make `container-security-context` optional, and make the three new checks run by default.
+* v1.12: Remove `container-security-context`.

score/security/security.go

@@ -10,10 +10,110 @@ import (
 
 func Register(allChecks *checks.Checks) {
 	allChecks.RegisterPodCheck("Container Security Context", `Makes sure that all pods have good securityContexts configured`, containerSecurityContext)
+
+	allChecks.RegisterOptionalPodCheck("Container Security Context User Group ID", `Makes sure that all pods have a security context with valid UID and GID set `, containerSecurityContextUserGroupID)
+	allChecks.RegisterOptionalPodCheck("Container Security Context Privileged", "Makes sure that all pods have a unprivileged security context set", containerSecurityContextPrivileged)
+	allChecks.RegisterOptionalPodCheck("Container Security Context ReadOnlyRootFilesystem", "Makes sure that all pods have a security context with read only filesystem set", containerSecurityContextReadOnlyRootFilesystem)
+
 	allChecks.RegisterOptionalPodCheck("Container Seccomp Profile", `Makes sure that all pods have at a seccomp policy configured.`, podSeccompProfile)
 }
 
+// containerSecurityContextReadOnlyRootFilesystem checks for pods using writeable root filesystems
+func containerSecurityContextReadOnlyRootFilesystem(podTemplate corev1.PodTemplateSpec, typeMeta metav1.TypeMeta) (score scorecard.TestScore) {
+	allContainers := podTemplate.Spec.InitContainers
+	allContainers = append(allContainers, podTemplate.Spec.Containers...)
+
+	noContextSet := false
+	hasWritableRootFS := false
+
+	for _, container := range allContainers {
+		if container.SecurityContext == nil {
+			noContextSet = true
+			score.AddComment(container.Name, "Container has no configured security context", "Set securityContext to run the container in a more secure context.")
+			continue
+		}
+		sec := container.SecurityContext
+		if sec.ReadOnlyRootFilesystem == nil || *sec.ReadOnlyRootFilesystem == false {
+			hasWritableRootFS = true
+			score.AddComment(container.Name, "The pod has a container with a writable root filesystem", "Set securityContext.readOnlyRootFilesystem to true")
+		}
+	}
+
+	if noContextSet || hasWritableRootFS {
+		score.Grade = scorecard.GradeCritical
+	} else {
+		score.Grade = scorecard.GradeAllOK
+	}
+
+	return
+}
+
+// containerSecurityContextPrivileged checks for privileged containers
+func containerSecurityContextPrivileged(podTemplate corev1.PodTemplateSpec, typeMeta metav1.TypeMeta) (score scorecard.TestScore) {
+	allContainers := podTemplate.Spec.InitContainers
+	allContainers = append(allContainers, podTemplate.Spec.Containers...)
+	hasPrivileged := false
+	for _, container := range allContainers {
+		if container.SecurityContext != nil && container.SecurityContext.Privileged != nil && *container.SecurityContext.Privileged {
+			hasPrivileged = true
+			score.AddComment(container.Name, "The container is privileged", "Set securityContext.privileged to false. Privileged containers can access all devices on the host, and grants almost the same access as non-containerized processes on the host.")
+		}
+	}
+	if hasPrivileged {
+		score.Grade = scorecard.GradeCritical
+	} else {
+		score.Grade = scorecard.GradeAllOK
+	}
+	return
+}
+
+// containerSecurityContextUserGroupID checks that the user and group are valid ( > 10000) in the security context
+func containerSecurityContextUserGroupID(podTemplate corev1.PodTemplateSpec, typeMeta metav1.TypeMeta) (score scorecard.TestScore) {
+	allContainers := podTemplate.Spec.InitContainers
+	allContainers = append(allContainers, podTemplate.Spec.Containers...)
+	podSecurityContext := podTemplate.Spec.SecurityContext
+	noContextSet := false
+	hasLowUserID := false
+	hasLowGroupID := false
+	for _, container := range allContainers {
+		if container.SecurityContext == nil && podSecurityContext == nil {
+			noContextSet = true
+			score.AddComment(container.Name, "Container has no configured security context", "Set securityContext to run the container in a more secure context.")
+			continue
+		}
+		sec := container.SecurityContext
+		if sec == nil {
+			sec = &corev1.SecurityContext{}
+		}
+		// Forward values from PodSecurityContext to the (container level) SecurityContext if not set
+		if podSecurityContext != nil {
+			if sec.RunAsGroup == nil {
+				sec.RunAsGroup = podSecurityContext.RunAsGroup
+			}
+			if sec.RunAsUser == nil {
+				sec.RunAsUser = podSecurityContext.RunAsUser
+			}
+		}
+		if sec.RunAsUser == nil || *sec.RunAsUser < 10000 {
+			hasLowUserID = true
+			score.AddComment(container.Name, "The container is running with a low user ID", "A userid above 10 000 is recommended to avoid conflicts with the host. Set securityContext.runAsUser to a value > 10000")
+		}
+
+		if sec.RunAsGroup == nil || *sec.RunAsGroup < 10000 {
+			hasLowGroupID = true
+			score.AddComment(container.Name, "The container running with a low group ID", "A groupid above 10 000 is recommended to avoid conflicts with the host. Set securityContext.runAsGroup to a value > 10000")
+		}
+	}
+	if noContextSet || hasLowUserID || hasLowGroupID {
+		score.Grade = scorecard.GradeCritical
+	} else {
+		score.Grade = scorecard.GradeAllOK
+	}
+	return
+}
+
 // containerSecurityContext checks that the recommended securityPolicy options are set
+// Deprecated: will be replaced with "Container Security Context User Group ID", "Container Security Context Privileged" and "Container Security Context ReadOnlyRootFilesystem" in future versions
 func containerSecurityContext(podTemplate corev1.PodTemplateSpec, typeMeta metav1.TypeMeta) (score scorecard.TestScore) {
 	allContainers := podTemplate.Spec.InitContainers
 	allContainers = append(allContainers, podTemplate.Spec.Containers...)

score/security_test.go

@@ -217,9 +217,9 @@ func TestPodSecurityContext(test *testing.T) {
 	}
 }
 
-func TestContainerSecurityContextPrivilegied(t *testing.T) {
+func TestContainerSecurityContextPrivileged(t *testing.T) {
 	t.Parallel()
-	testExpectedScore(t, "pod-security-context-privilegied.yaml", "Container Security Context", scorecard.GradeCritical)
+	testExpectedScore(t, "pod-security-context-privileged.yaml", "Container Security Context", scorecard.GradeCritical)
 }
 
 func TestContainerSecurityContextLowUser(t *testing.T) {
@@ -266,3 +266,126 @@ func TestContainerSeccompAllGood(t *testing.T) {
 		EnabledOptionalTests: structMap,
 	}, "Container Seccomp Profile", scorecard.GradeAllOK)
 }
+
+func TestContainerSecurityContextUserGroupIDAllGood(t *testing.T) {
+	t.Parallel()
+	structMap := make(map[string]struct{})
+	structMap["container-security-context-user-group-id"] = struct{}{}
+	c := testExpectedScoreWithConfig(t, config.Configuration{
+		AllFiles:             []ks.NamedReader{testFile("pod-security-context-all-good.yaml")},
+		EnabledOptionalTests: structMap,
+	}, "Container Security Context User Group ID", scorecard.GradeAllOK)
+	assert.Empty(t, c)
+}
+
+func TestContainerSecurityContextUserGroupIDLowGroup(t *testing.T) {
+	t.Parallel()
+	optionalChecks := make(map[string]struct{})
+	optionalChecks["container-security-context-user-group-id"] = struct{}{}
+	comments := testExpectedScoreWithConfig(t, config.Configuration{
+		AllFiles:             []ks.NamedReader{testFile("pod-security-context-low-group-id.yaml")},
+		EnabledOptionalTests: optionalChecks,
+	}, "Container Security Context User Group ID", scorecard.GradeCritical)
+	assert.Contains(t, comments, scorecard.TestScoreComment{
+		Path:        "foobar",
+		Summary:     "The container running with a low group ID",
+		Description: "A groupid above 10 000 is recommended to avoid conflicts with the host. Set securityContext.runAsGroup to a value > 10000",
+	})
+}
+
+func TestContainerSecurityContextUserGroupIDLowUser(t *testing.T) {
+	t.Parallel()
+	optionalChecks := make(map[string]struct{})
+	optionalChecks["container-security-context-user-group-id"] = struct{}{}
+	comments := testExpectedScoreWithConfig(t, config.Configuration{
+		AllFiles:             []ks.NamedReader{testFile("pod-security-context-low-user-id.yaml")},
+		EnabledOptionalTests: optionalChecks,
+	}, "Container Security Context User Group ID", scorecard.GradeCritical)
+	assert.Contains(t, comments, scorecard.TestScoreComment{
+		Path:        "foobar",
+		Summary:     "The container is running with a low user ID",
+		Description: "A userid above 10 000 is recommended to avoid conflicts with the host. Set securityContext.runAsUser to a value > 10000",
+	})
+}
+
+func TestContainerSecurityContextUserGroupIDNoSecurityContext(t *testing.T) {
+	t.Parallel()
+	optionalChecks := make(map[string]struct{})
+	optionalChecks["container-security-context-user-group-id"] = struct{}{}
+	comments := testExpectedScoreWithConfig(t, config.Configuration{
+		AllFiles:             []ks.NamedReader{testFile("pod-security-context-nosecuritycontext.yaml")},
+		EnabledOptionalTests: optionalChecks,
+	}, "Container Security Context User Group ID", scorecard.GradeCritical)
+	assert.Contains(t, comments, scorecard.TestScoreComment{
+		Path:        "foobar",
+		Summary:     "Container has no configured security context",
+		Description: "Set securityContext to run the container in a more secure context.",
+	})
+}
+
+func TestContainerSecurityContextPrivilegedAllGood(t *testing.T) {
+	t.Parallel()
+	structMap := make(map[string]struct{})
+	structMap["container-security-context-privileged"] = struct{}{}
+	c := testExpectedScoreWithConfig(t, config.Configuration{
+		AllFiles:             []ks.NamedReader{testFile("pod-security-context-all-good.yaml")},
+		EnabledOptionalTests: structMap,
+	}, "Container Security Context Privileged", scorecard.GradeAllOK)
+	assert.Empty(t, c)
+}
+
+func TestContainerSecurityContextPrivilegedPrivileged(t *testing.T) {
+	t.Parallel()
+	optionalChecks := make(map[string]struct{})
+	optionalChecks["container-security-context-privileged"] = struct{}{}
+	comments := testExpectedScoreWithConfig(t, config.Configuration{
+		AllFiles:             []ks.NamedReader{testFile("pod-security-context-privileged.yaml")},
+		EnabledOptionalTests: optionalChecks,
+	}, "Container Security Context Privileged", scorecard.GradeCritical)
+	assert.Contains(t, comments, scorecard.TestScoreComment{
+		Path:        "foobar",
+		Summary:     "The container is privileged",
+		Description: "Set securityContext.privileged to false. Privileged containers can access all devices on the host, and grants almost the same access as non-containerized processes on the host.",
+	})
+}
+
+func TestContainerSecurityContextReadOnlyRootFilesystemAllGood(t *testing.T) {
+	t.Parallel()
+	structMap := make(map[string]struct{})
+	structMap["container-security-context-readonlyrootfilesystem"] = struct{}{}
+	c := testExpectedScoreWithConfig(t, config.Configuration{
+		AllFiles:             []ks.NamedReader{testFile("pod-security-context-all-good.yaml")},
+		EnabledOptionalTests: structMap,
+	}, "Container Security Context ReadOnlyRootFilesystem", scorecard.GradeAllOK)
+	assert.Empty(t, c)
+}
+
+func TestContainerSecurityContextReadOnlyRootFilesystemWriteable(t *testing.T) {
+	t.Parallel()
+	optionalChecks := make(map[string]struct{})
+	optionalChecks["container-security-context-readonlyrootfilesystem"] = struct{}{}
+	comments := testExpectedScoreWithConfig(t, config.Configuration{
+		AllFiles:             []ks.NamedReader{testFile("pod-security-context-writeablerootfilesystem.yaml")},
+		EnabledOptionalTests: optionalChecks,
+	}, "Container Security Context ReadOnlyRootFilesystem", scorecard.GradeCritical)
+	assert.Contains(t, comments, scorecard.TestScoreComment{
+		Path:        "foobar",
+		Summary:     "The pod has a container with a writable root filesystem",
+		Description: "Set securityContext.readOnlyRootFilesystem to true",
+	})
+}
+
+func TestContainerSecurityContextReadOnlyRootFilesystemNoSecurityContext(t *testing.T) {
+	t.Parallel()
+	optionalChecks := make(map[string]struct{})
+	optionalChecks["container-security-context-readonlyrootfilesystem"] = struct{}{}
+	comments := testExpectedScoreWithConfig(t, config.Configuration{
+		AllFiles:             []ks.NamedReader{testFile("pod-security-context-nosecuritycontext.yaml")},
+		EnabledOptionalTests: optionalChecks,
+	}, "Container Security Context ReadOnlyRootFilesystem", scorecard.GradeCritical)
+	assert.Contains(t, comments, scorecard.TestScoreComment{
+		Path:        "foobar",
+		Summary:     "Container has no configured security context",
+		Description: "Set securityContext to run the container in a more secure context.",
+	})
+}

score/testdata/pod-security-context-nosecuritycontext.yaml

@@ -0,0 +1,8 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  name: pod-test-1
+spec:
+  containers:
+  - name: foobar
+    image: foo/bar:latest

score/testdata/pod-security-context-privilegied.yaml score/testdata/pod-security-context-privileged.yaml

@@ -0,0 +1,10 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  name: pod-test-1
+spec:
+  containers:
+  - name: foobar
+    image: foo/bar:latest
+    securityContext:
+      privileged: True