Merge runAsUser and runAsNonRoot checks in a single testcase/check. (#…

…2654)

* Merge runAsUser and runAsNonRoot checks in a single testcase/check.

These two test cases have been merged:
access-control-security-context-run-as-non-root-user-check
access-control-security-context-non-root-user-id-check

As a result, both securityConstraint fields "runAsNonRoot" and
"runAsUser" are checked inside the existing test case
access-control-security-context-run-as-non-root-user-check

To pass the test case, either:
a) runAsUser must be `!= 0` at pod or container level.
b) runAsNonRoot must be `true` at pod or container level.

* Remove access-control-security-context-non-root-user-id-check from expected_results.yaml file.

* QE branch ref set to rename_access_control_test

QE PR
redhat-best-practices-for-k8s/certsuite-qe#1034
is pending to merge but the branch can be used here.

* Use access-control-security-context-non-root-user-id-check.

* Update expected_results.yaml file

CATALOG.md

@@ -7,13 +7,13 @@ Depending on the workload type, not all tests are required to pass to satisfy be
 
 ## Test cases summary
 
-### Total test cases: 118
+### Total test cases: 117
 
 ### Total suites: 10
 
 |Suite|Tests per suite|
 |---|---|
-|access-control|29|
+|access-control|28|
 |affiliated-certification|4|
 |lifecycle|18|
 |manageability|2|
@@ -36,11 +36,11 @@ Depending on the workload type, not all tests are required to pass to satisfy be
 |---|---|
 |8|1|
 
-### Non-Telco specific tests only: 70
+### Non-Telco specific tests only: 69
 
 |Mandatory|Optional|
 |---|---|
-|44|26|
+|43|26|
 
 ### Telco specific tests only: 27
 
@@ -379,8 +379,8 @@ Tags|extended,access-control
 Property|Description
 ---|---
 Unique ID|access-control-security-context-non-root-user-id-check
-Description|Checks the security context runAsUser parameter in pods and containers to make sure it is not set to uid root(0). Pods and containers should not run as root (runAsUser is not set to uid0).
-Suggested Remediation|Change the pod and containers "runAsUser" uid to something other than root(0)
+Description|Checks securityContext's runAsNonRoot and runAsUser fields at pod and container level to make sure containers are not run as root.
+Suggested Remediation|Set the securityContext.runAsNonRoot field to true either at pod or container level. Alternatively, set a non-zero value to securityContext.runAsUser field either at pod or container level.
 Best Practice Reference|https://redhat-best-practices-for-k8s.github.io/guide/#redhat-best-practices-for-k8s-cnf-security
 Exception Process|No exceptions - will only be considered under special circumstances. Must identify which container needs access and document why with details.
 Tags|common,access-control
@@ -422,22 +422,6 @@ Tags|common,access-control
 |Non-Telco|Optional|
 |Telco|Optional|
 
-#### access-control-security-context-run-as-non-root-user-check
-
-Property|Description
----|---
-Unique ID|access-control-security-context-run-as-non-root-user-check
-Description|Checks the security context runAsNonRoot parameter in pods and containers to make sure it is not set to false. Pods and containers should not be able to run as root..
-Suggested Remediation|Set the the pod and containers "runAsNonRoot" to true.
-Best Practice Reference|https://redhat-best-practices-for-k8s.github.io/guide/#redhat-best-practices-for-k8s-cnf-security
-Exception Process|No exceptions - will only be considered under special circumstances. Must identify which container needs access and document why with details.
-Tags|common,access-control
-|**Scenario**|**Optional/Mandatory**|
-|Extended|Mandatory|
-|Far-Edge|Mandatory|
-|Non-Telco|Mandatory|
-|Telco|Mandatory|
-
 #### access-control-service-type
 
 Property|Description

expected_results.yaml

@@ -22,7 +22,6 @@ testCases:
     - access-control-security-context-non-root-user-id-check
     - access-control-security-context-privilege-escalation
     - access-control-security-context-read-only-file-system
-    - access-control-security-context-run-as-non-root-user-check
     - access-control-security-context
     - access-control-service-type
     - access-control-ssh-daemons

pkg/provider/containers.go

@@ -197,26 +197,26 @@ func (c *Container) IsReadOnlyRootFilesystem(logger *log.Logger) bool {
 
 func (c *Container) IsContainerRunAsNonRoot(podRunAsNonRoot *bool) (isContainerRunAsNonRoot bool, reason string) {
 	if c.SecurityContext != nil && c.SecurityContext.RunAsNonRoot != nil {
-		return *c.SecurityContext.RunAsNonRoot, fmt.Sprintf("RunAsNonRoot is set to %t at the container level, overriding a %v value defined at pod level.",
+		return *c.SecurityContext.RunAsNonRoot, fmt.Sprintf("RunAsNonRoot is set to %t at the container level, overriding a %v value defined at pod level",
 			*c.SecurityContext.RunAsNonRoot, stringhelper.PointerToString(podRunAsNonRoot))
 	}
 
 	if podRunAsNonRoot != nil {
-		return *podRunAsNonRoot, fmt.Sprintf("RunAsNonRoot is set to nil at container level and inheriting a %t value from the pod level RunAsNonRoot setting.", *podRunAsNonRoot)
+		return *podRunAsNonRoot, fmt.Sprintf("RunAsNonRoot is set to nil at container level and inheriting a %t value from the pod level RunAsNonRoot setting", *podRunAsNonRoot)
 	}
 
-	return false, "RunAsNonRoot set to nil at pod and container level"
+	return false, "RunAsNonRoot is set to nil at pod and container level"
 }
 
 func (c *Container) IsContainerRunAsNonRootUserID(podRunAsNonRootUserID *int64) (isContainerRunAsNonRootUserID bool, reason string) {
 	if c.SecurityContext != nil && c.SecurityContext.RunAsUser != nil {
-		return *c.SecurityContext.RunAsUser != 0, fmt.Sprintf("RunAsUser is set to %v at the container level, overriding a %s value defined at pod level.",
+		return *c.SecurityContext.RunAsUser != 0, fmt.Sprintf("RunAsUser is set to %v at the container level, overriding a %s value defined at pod level",
 			*c.SecurityContext.RunAsUser, stringhelper.PointerToString(podRunAsNonRootUserID))
 	}
 
 	if podRunAsNonRootUserID != nil {
-		return *podRunAsNonRootUserID != 0, fmt.Sprintf("RunAsUser is set to nil at container level and inheriting a %v value from the pod level RunAsUser setting.", *podRunAsNonRootUserID)
+		return *podRunAsNonRootUserID != 0, fmt.Sprintf("RunAsUser is set to nil at container level and inheriting a %v value from the pod level RunAsUser setting", *podRunAsNonRootUserID)
 	}
 
-	return false, "RunAsUser set to nil at pod and container level"
+	return false, "RunAsUser is set to nil at pod and container level"
 }

pkg/provider/containers_test.go

@@ -266,7 +266,7 @@ func TestIsContainerRunAsNonRoot(t *testing.T) {
 			},
 			podDefault:     &falseVal,
 			expected:       true,
-			expectedReason: "RunAsNonRoot is set to true at the container level, overriding a false value defined at pod level.",
+			expectedReason: "RunAsNonRoot is set to true at the container level, overriding a false value defined at pod level",
 		},
 		{
 			name: "Container set to not run as non-root",
@@ -279,7 +279,7 @@ func TestIsContainerRunAsNonRoot(t *testing.T) {
 			},
 			podDefault:     &trueVal,
 			expected:       false,
-			expectedReason: "RunAsNonRoot is set to false at the container level, overriding a true value defined at pod level.",
+			expectedReason: "RunAsNonRoot is set to false at the container level, overriding a true value defined at pod level",
 		},
 		{
 			name: "Container set to not run as non-root",
@@ -292,7 +292,7 @@ func TestIsContainerRunAsNonRoot(t *testing.T) {
 			},
 			podDefault:     &falseVal,
 			expected:       false,
-			expectedReason: "RunAsNonRoot is set to nil at container level and inheriting a false value from the pod level RunAsNonRoot setting.",
+			expectedReason: "RunAsNonRoot is set to nil at container level and inheriting a false value from the pod level RunAsNonRoot setting",
 		},
 		{
 			name: "nil at pod and true at container",
@@ -305,7 +305,7 @@ func TestIsContainerRunAsNonRoot(t *testing.T) {
 			},
 			podDefault:     nil,
 			expected:       true,
-			expectedReason: "RunAsNonRoot is set to true at the container level, overriding a nil value defined at pod level.",
+			expectedReason: "RunAsNonRoot is set to true at the container level, overriding a nil value defined at pod level",
 		},
 	}
 

pkg/provider/pods.go

@@ -520,58 +520,42 @@ func (p *Pod) IsRunAsUserID(uid int64) bool {
 	return *p.Pod.Spec.SecurityContext.RunAsUser == uid
 }
 
-// Returns the list of containers that have the RunAsNonRoot SCC parameter set to false
-// The RunAsNonRoot parameter is checked first at the pod level and acts as a default value
+// Returns the list of containers that have the securityContext.runAsNonRoot set to false and securityContext.runAsUser set to zero.
+// Both parameteters are checked first at the pod level and acts as a default value
 // for the container configuration, if it is not present.
-// The RunAsNonRoot parameter is checked next at the container level.
 // See: https://kubernetes.io/docs/tasks/configure-pod-container/security-context/#set-the-security-context-for-a-container
-func (p *Pod) GetRunAsNonRootFalseContainers(knownContainersToSkip map[string]bool) (nonCompliantContainers []*Container, nonComplianceReason []string) {
+func (p *Pod) GetRunAsNonRootFalseContainers(knownContainersToSkip map[string]bool) (nonCompliantContainers []*Container, nonComplianceReasons []string) {
 	// Check pod-level security context this will be set by default for containers
 	// If not already configured at the container level
 	var podRunAsNonRoot *bool
 	if p.Pod.Spec.SecurityContext != nil && p.Pod.Spec.SecurityContext.RunAsNonRoot != nil {
 		podRunAsNonRoot = p.Pod.Spec.SecurityContext.RunAsNonRoot
 	}
-	// Check each container for the RunAsNonRoot parameter.
-	// If it is not present, the pod value applies
-	for _, cut := range p.Containers {
-		if knownContainersToSkip[cut.Name] {
-			continue
-		}
-		if isRunAsNonRoot, reason := cut.IsContainerRunAsNonRoot(podRunAsNonRoot); !isRunAsNonRoot {
-			// found a container with RunAsNonRoot set to false
-			nonCompliantContainers = append(nonCompliantContainers, cut)
-			nonComplianceReason = append(nonComplianceReason, reason)
-		}
-	}
-	return nonCompliantContainers, nonComplianceReason
-}
 
-// Returns the list of containers that have the RunAsUser SCC parameter set to 0 (root)
-// The RunAsUser parameter is checked first at the pod level and acts as a default value
-// for the container configuration, if it is not present.
-// The RunAsUser parameter is checked next at the container level.
-// See: https://kubernetes.io/docs/tasks/configure-pod-container/security-context/#set-the-security-context-for-a-container
-func (p *Pod) GetRunAsNonRootUserIDContainers(knownContainersToSkip map[string]bool) (nonCompliantContainers []*Container, nonComplianceReason []string) {
-	// Check pod-level security context this will be set by default for containers
-	// If not already configured at the container level
 	var podRunAsUserID *int64
 	if p.Pod.Spec.SecurityContext != nil && p.Pod.Spec.SecurityContext.RunAsUser != nil {
 		podRunAsUserID = p.Pod.Spec.SecurityContext.RunAsUser
 	}
-	// Check each container for the RunAsUser parameter.
+
+	// Check each container for the RunAsNonRoot parameter.
 	// If it is not present, the pod value applies
 	for _, cut := range p.Containers {
 		if knownContainersToSkip[cut.Name] {
 			continue
 		}
-		if isRunAsNonRootUserID, reason := cut.IsContainerRunAsNonRootUserID(podRunAsUserID); !isRunAsNonRootUserID {
-			// found a container with RunAsNonRoot set to false
-			nonCompliantContainers = append(nonCompliantContainers, cut)
-			nonComplianceReason = append(nonComplianceReason, reason)
+
+		isRunAsNonRoot, isRunAsNonRootReason := cut.IsContainerRunAsNonRoot(podRunAsNonRoot)
+		isRunAsNonRootUserID, isRunAsNonRootUserIDReason := cut.IsContainerRunAsNonRootUserID(podRunAsUserID)
+
+		if isRunAsNonRoot || isRunAsNonRootUserID {
+			continue
 		}
+
+		nonCompliantContainers = append(nonCompliantContainers, cut)
+		nonComplianceReasons = append(nonComplianceReasons, isRunAsNonRootReason+", "+isRunAsNonRootUserIDReason)
 	}
-	return nonCompliantContainers, nonComplianceReason
+
+	return nonCompliantContainers, nonComplianceReasons
 }
 
 // Get the list of top owners of pods

pkg/provider/pods_test.go

@@ -719,7 +719,7 @@ func TestIsRunAsNonRoot(t *testing.T) {
 				},
 			},
 			wantNonComplianceReason: []string{
-				"RunAsNonRoot is set to false at the container level, overriding a true value defined at pod level.",
+				"RunAsNonRoot is set to false at the container level, overriding a true value defined at pod level, RunAsUser is set to nil at pod and container level",
 			},
 		},
 		{
@@ -771,7 +771,7 @@ func TestIsRunAsNonRoot(t *testing.T) {
 				},
 			},
 			wantNonComplianceReason: []string{
-				"RunAsNonRoot is set to false at the container level, overriding a nil value defined at pod level.",
+				"RunAsNonRoot is set to false at the container level, overriding a nil value defined at pod level, RunAsUser is set to nil at pod and container level",
 			},
 		},
 		{

tests/accesscontrol/suite.go

@@ -119,13 +119,6 @@ func LoadChecks() {
 		}))
 
 	checksGroup.Add(checksdb.NewCheck(identifiers.GetTestIDAndLabels(identifiers.TestSecConNonRootUserIDIdentifier)).
-		WithSkipCheckFn(testhelper.GetNoContainersUnderTestSkipFn(&env)).
-		WithCheckFn(func(c *checksdb.Check) error {
-			testSecConRootUserID(c, &env)
-			return nil
-		}))
-
-	checksGroup.Add(checksdb.NewCheck(identifiers.GetTestIDAndLabels(identifiers.TestSecConRunAsNonRootIdentifier)).
 		WithSkipCheckFn(testhelper.GetNoContainersUnderTestSkipFn(&env)).
 		WithCheckFn(func(c *checksdb.Check) error {
 			testSecConRunAsNonRoot(c, &env)
@@ -351,51 +344,27 @@ func testBpfCapability(check *checksdb.Check, env *provider.TestEnvironment) {
 	check.SetResult(compliantObjects, nonCompliantObjects)
 }
 
-// testSecConRootUserID verifies that the container is not running as root
-func testSecConRootUserID(check *checksdb.Check, env *provider.TestEnvironment) {
-	var compliantObjects []*testhelper.ReportObject
-	var nonCompliantObjects []*testhelper.ReportObject
-
-	for _, put := range env.Pods {
-		check.LogInfo("Testing Pod %q in namespace %q", put.Name, put.Namespace)
-		nonCompliantContainers, nonComplianceReason := put.GetRunAsNonRootUserIDContainers(knownContainersToSkip)
-		if len(nonCompliantContainers) == 0 {
-			check.LogInfo("Pod %q is configured with RunAsUser SCC parameter different than 0 for all of its containers", put.Name)
-			compliantObjects = append(compliantObjects, testhelper.NewPodReportObject(put.Namespace, put.Name, "Pod is configured with RunAsUser SCC parameter different than 0 for all of its containers", true))
-		} else {
-			check.LogError("Pod %q is configured with RunAsUser SCC parameter equal to 0 for some of its containers", put.Name)
-			nonCompliantObjects = append(nonCompliantObjects, testhelper.NewPodReportObject(put.Namespace, put.Name, "Pod is configured with RunAsNonRoot SCC parameter set to false for some of its containers", false))
-			for index := range nonCompliantContainers {
-				check.LogError("In Container %q of Pod %q, %s", nonCompliantContainers[index].Name, put.Name, nonComplianceReason[index])
-				nonCompliantObjects = append(nonCompliantObjects, testhelper.NewContainerReportObject(put.Namespace, put.Name,
-					nonCompliantContainers[index].Name, fmt.Sprintf("In Container %q of Pod %q, %s", nonCompliantContainers[index].Name, put.Name, nonComplianceReason[index]), false))
-			}
-		}
-	}
-	check.SetResult(compliantObjects, nonCompliantObjects)
-}
-
 // testSecConRunAsNonRoot verifies that containers are not allowed to run as root.
 func testSecConRunAsNonRoot(check *checksdb.Check, env *provider.TestEnvironment) {
 	var compliantObjects []*testhelper.ReportObject
 	var nonCompliantObjects []*testhelper.ReportObject
 
 	for _, put := range env.Pods {
-		check.LogInfo("Testing Pod %q in namespace %q", put.Name, put.Namespace)
+		check.LogInfo("Testing pod %s/%s", put.Namespace, put.Name)
 		nonCompliantContainers, nonComplianceReason := put.GetRunAsNonRootFalseContainers(knownContainersToSkip)
 		if len(nonCompliantContainers) == 0 {
-			check.LogInfo("Pod %q is configured with RunAsNonRoot SCC parameter set to true for all of its containers", put.Name)
-			compliantObjects = append(compliantObjects, testhelper.NewPodReportObject(put.Namespace, put.Name, "Pod is configured with RunAsNonRoot SCC parameter set to true for all of its containers", true))
+			compliantObjects = append(compliantObjects, testhelper.NewPodReportObject(put.Namespace, put.Name, "Pod is configured with RunAsNonRoot=true or RunAsUser!=0 at pod or container level.", true))
 		} else {
-			check.LogError("Pod %q is configured with RunAsNonRoot SCC parameter set to false for some of its containers", put.Name)
-			nonCompliantObjects = append(nonCompliantObjects, testhelper.NewPodReportObject(put.Namespace, put.Name, "Pod is configured with RunAsNonRoot SCC parameter set to false for some of its containers", false))
+			nonCompliantObjects = append(nonCompliantObjects, testhelper.NewPodReportObject(put.Namespace, put.Name, "One or more containers of the pod are running with root user", false))
 			for index := range nonCompliantContainers {
-				check.LogError("In Container %q of Pod %q, %s", nonCompliantContainers[index].Name, put.Name, nonComplianceReason[index])
-				nonCompliantObjects = append(nonCompliantObjects, testhelper.NewContainerReportObject(put.Namespace, put.Name,
-					nonCompliantContainers[index].Name, fmt.Sprintf("In Container %q of Pod %q, %s", nonCompliantContainers[index].Name, put.Name, nonComplianceReason[index]), false))
+				check.LogError("Pod %s/%s, container %q is not compliant: %s", put.Namespace, put.Name, nonCompliantContainers[index].Name, nonComplianceReason[index])
+
+				nonCompliantObjects = append(nonCompliantObjects, testhelper.NewContainerReportObject(put.Namespace, put.Name, nonCompliantContainers[index].Name,
+					nonComplianceReason[index], false))
 			}
 		}
 	}
+
 	check.SetResult(compliantObjects, nonCompliantObjects)
 }
 

tests/identifiers/identifiers.go

@@ -102,7 +102,6 @@ var (
 	TestRtAppNoExecProbes                             claim.Identifier
 	TestRestartOnRebootLabelOnPodsUsingSRIOV          claim.Identifier
 	TestSecConNonRootUserIDIdentifier                 claim.Identifier
-	TestSecConRunAsNonRootIdentifier                  claim.Identifier
 	TestNetworkAttachmentDefinitionSRIOVUsingMTU      claim.Identifier
 	TestSecContextIdentifier                          claim.Identifier
 	TestSecConPrivilegeEscalation                     claim.Identifier
@@ -603,23 +602,7 @@ func InitCatalog() map[claim.Identifier]claim.TestCaseDescription {
 	TestSecConNonRootUserIDIdentifier = AddCatalogEntry(
 		"security-context-non-root-user-id-check",
 		common.AccessControlTestKey,
-		`Checks the security context runAsUser parameter in pods and containers to make sure it is not set to uid root(0). Pods and containers should not run as root (runAsUser is not set to uid0).`,
-		SecConNonRootUserRemediation,
-		SecConNonRootUserExceptionProcess,
-		TestSecConNonRootUserIdentifierDocLink,
-		true,
-		map[string]string{
-			FarEdge:  Mandatory,
-			Telco:    Mandatory,
-			NonTelco: Mandatory,
-			Extended: Mandatory,
-		},
-		TagCommon)
-
-	TestSecConRunAsNonRootIdentifier = AddCatalogEntry(
-		"security-context-run-as-non-root-user-check",
-		common.AccessControlTestKey,
-		`Checks the security context runAsNonRoot parameter in pods and containers to make sure it is not set to false. Pods and containers should not be able to run as root..`,
+		`Checks securityContext's runAsNonRoot and runAsUser fields at pod and container level to make sure containers are not run as root.`,
 		SecConRunAsNonRootUserRemediation,
 		SecConNonRootUserExceptionProcess,
 		TestSecConNonRootUserIdentifierDocLink,

tests/identifiers/remediation.go

@@ -26,7 +26,7 @@ const (
 
 	SecConNonRootUserRemediation = `Change the pod and containers "runAsUser" uid to something other than root(0)`
 
-	SecConRunAsNonRootUserRemediation = `Set the the pod and containers "runAsNonRoot" to true.`
+	SecConRunAsNonRootUserRemediation = `Set the securityContext.runAsNonRoot field to true either at pod or container level. Alternatively, set a non-zero value to securityContext.runAsUser field either at pod or container level.`
 
 	SecConRemediation = `Exception possible if a workload uses mlock(), mlockall(), shmctl(), mmap(); exception will be considered for DPDK applications. Must identify which container requires the capability and detail why.`