security validation fixes and more thorough tests

FairwindsOps · Apr 5, 2019 · c171384 · c171384
1 parent 31fc6f6
commit c171384
Show file tree

Hide file tree

Showing 2 changed files with 97 additions and 4 deletions.
diff --git a/pkg/validator/container.go b/pkg/validator/container.go
@@ -226,12 +226,13 @@ func (cv *ContainerValidation) validateCapabilities(confLists conf.SecurityCapab
 	}
 
 	if len(confLists.IfAnyNotDropped) > 0 {
-		intersectDrops := capIntersection(capabilities.Drop, confLists.IfAnyNotDropped)
-		if len(intersectDrops) > 0 && !capContains(capabilities.Drop, "ALL") {
-			capsString := commaSeparatedCapabilities(intersectDrops)
+		missingDrops := capDifference(confLists.IfAnyNotDropped, capabilities.Drop)
+
+		if len(missingDrops) > 0 && !capContains(capabilities.Drop, "ALL") {
+			capsString := commaSeparatedCapabilities(missingDrops)
 			cv.addFailure(fmt.Sprintf("Security capabilities not dropped from %v list: %v", severity, capsString), severity)
 		} else {
-			cv.addSuccess(fmt.Sprintf("All security capabilities dropped from %v list", severity))
+			cv.addSuccess(fmt.Sprintf("Required security capabilities dropped from %v list", severity))
 		}
 	}
 }

diff --git a/pkg/validator/container_test.go b/pkg/validator/container_test.go
@@ -352,6 +352,21 @@ func TestValidateSecurity(t *testing.T) {
 			},
 		},
 	}
+	strongConf := conf.Security{
+		RunAsRootAllowed:           conf.SeverityError,
+		RunAsPrivileged:            conf.SeverityError,
+		NotReadOnlyRootFileSystem:  conf.SeverityError,
+		PrivilegeEscalationAllowed: conf.SeverityError,
+		Capabilities: conf.SecurityCapabilities{
+			Error: conf.SecurityCapabilityLists{
+				IfAnyAdded:      []corev1.Capability{"ALL", "SYS_ADMIN", "NET_ADMIN"},
+				IfAnyNotDropped: []corev1.Capability{"NET_BIND_SERVICE", "DAC_OVERRIDE", "SYS_CHROOT"},
+			},
+			Warning: conf.SecurityCapabilityLists{
+				IfAnyAddedBeyond: []corev1.Capability{"NONE"},
+			},
+		},
+	}
 
 	emptyCV := ContainerValidation{
 		Container: &corev1.Container{Name: ""},
@@ -376,6 +391,21 @@ func TestValidateSecurity(t *testing.T) {
 	}
 
 	goodCV := ContainerValidation{
+		Container: &corev1.Container{Name: "", SecurityContext: &corev1.SecurityContext{
+			RunAsNonRoot:             &trueVar,
+			ReadOnlyRootFilesystem:   &trueVar,
+			Privileged:               &falseVar,
+			AllowPrivilegeEscalation: &falseVar,
+			Capabilities: &corev1.Capabilities{
+				Drop: []corev1.Capability{"NET_BIND_SERVICE", "FOWNER"},
+			},
+		}},
+		ResourceValidation: &ResourceValidation{
+			Summary: &ResultSummary{},
+		},
+	}
+
+	strongCV := ContainerValidation{
 		Container: &corev1.Container{Name: "", SecurityContext: &corev1.SecurityContext{
 			RunAsNonRoot:             &trueVar,
 			ReadOnlyRootFilesystem:   &trueVar,
@@ -474,13 +504,75 @@ func TestValidateSecurity(t *testing.T) {
 				Type:    "success",
 			}},
 		},
+		{
+			name:         "good security context + strong validation config",
+			securityConf: strongConf,
+			cv:           goodCV,
+			expectedMessages: []*ResultMessage{{
+				Message: "Security capabilities not dropped from error list: DAC_OVERRIDE, SYS_CHROOT",
+				Type:    "error",
+			}, {
+				Message: "Container is not allowed to run as root",
+				Type:    "success",
+			}, {
+				Message: "Container is running with a read only filesystem",
+				Type:    "success",
+			}, {
+				Message: "Container is not running as privileged",
+				Type:    "success",
+			}, {
+				Message: "Container does not allow privilege escalation",
+				Type:    "success",
+			}, {
+				Message: "No security capabilities added from error list",
+				Type:    "success",
+			}, {
+				Message: "No security capabilities added beyond warning list",
+				Type:    "success",
+			}},
+		},
+		{
+			name:         "strong security context + strong validation config",
+			securityConf: strongConf,
+			cv:           strongCV,
+			expectedMessages: []*ResultMessage{{
+				Message: "Container is not allowed to run as root",
+				Type:    "success",
+			}, {
+				Message: "Container is running with a read only filesystem",
+				Type:    "success",
+			}, {
+				Message: "Container is not running as privileged",
+				Type:    "success",
+			}, {
+				Message: "Container does not allow privilege escalation",
+				Type:    "success",
+			}, {
+				Message: "No security capabilities added from error list",
+				Type:    "success",
+			}, {
+				Message: "Required security capabilities dropped from error list",
+				Type:    "success",
+			}, {
+				Message: "No security capabilities added beyond warning list",
+				Type:    "success",
+			}},
+		},
 	}
 
 	for _, tt := range testCases {
 		t.Run(tt.name, func(t *testing.T) {
+			tt.cv = resetCV(tt.cv)
 			tt.cv.validateSecurity(&tt.securityConf)
 			assert.Len(t, tt.cv.messages(), len(tt.expectedMessages))
 			assert.ElementsMatch(t, tt.cv.messages(), tt.expectedMessages)
 		})
 	}
 }
+
+func resetCV(cv ContainerValidation) ContainerValidation {
+	cv.Errors = []*ResultMessage{}
+	cv.Successes = []*ResultMessage{}
+	cv.Warnings = []*ResultMessage{}
+	return cv
+}