AllowPrivilegeEscalation: add validations for caps and privileged

Signed-off-by: Jess Frazelle <acidburn@microsoft.com>

pkg/api/validation/validation.go

@@ -4388,6 +4388,21 @@ func ValidateSecurityContext(sc *api.SecurityContext, fldPath *field.Path) field
 			allErrs = append(allErrs, field.Invalid(fldPath.Child("runAsUser"), *sc.RunAsUser, isNegativeErrorMsg))
 		}
 	}
+
+	if sc.AllowPrivilegeEscalation != nil && !*sc.AllowPrivilegeEscalation {
+		if sc.Privileged != nil && *sc.Privileged {
+			allErrs = append(allErrs, field.Invalid(fldPath, sc, "cannot set `allowPrivilegeEscalation` to false and `privileged` to true"))
+		}
+
+		if sc.Capabilities != nil {
+			for _, cap := range sc.Capabilities.Add {
+				if string(cap) == "CAP_SYS_ADMIN" {
+					allErrs = append(allErrs, field.Invalid(fldPath, sc, "cannot set `allowPrivilegeEscalation` to false and `capabilities.Add` CAP_SYS_ADMIN"))
+				}
+			}
+		}
+	}
+
 	return allErrs
 }
 

pkg/api/validation/validation_test.go

@@ -4704,6 +4704,38 @@ func TestValidatePodSpec(t *testing.T) {
 			DNSPolicy:         api.DNSClusterFirst,
 			PriorityClassName: "InvalidName",
 		},
+		"with privileged and allowPrivilegeEscalation false": {
+			Containers: []api.Container{
+				{
+					Name:            "ctr",
+					Image:           "image",
+					ImagePullPolicy: "IfNotPresent",
+					Ports: []api.ContainerPort{
+						{HostPort: 8080, ContainerPort: 2600, Protocol: "TCP"}},
+					SecurityContext: &api.SecurityContext{
+						Privileged:               boolPtr(true),
+						AllowPrivilegeEscalation: boolPtr(false),
+					},
+				},
+			},
+		},
+		"with CAP_SYS_ADMIN and allowPrivilegeEscalation false": {
+			Containers: []api.Container{
+				{
+					Name:            "ctr",
+					Image:           "image",
+					ImagePullPolicy: "IfNotPresent",
+					Ports: []api.ContainerPort{
+						{HostPort: 8080, ContainerPort: 2600, Protocol: "TCP"}},
+					SecurityContext: &api.SecurityContext{
+						Capabilities: &api.Capabilities{
+							Add: []api.Capability{"CAP_SYS_ADMIN"},
+						},
+						AllowPrivilegeEscalation: boolPtr(false),
+					},
+				},
+			},
+		},
 	}
 	for k, v := range failureCases {
 		if errs := ValidatePodSpec(&v, field.NewPath("field")); len(errs) == 0 {
@@ -11082,3 +11114,7 @@ func TestValidateOrSetClientIPAffinityConfig(t *testing.T) {
 		}
 	}
 }
+
+func boolPtr(b bool) *bool {
+	return &b
+}

pkg/securitycontext/util.go

@@ -242,32 +242,12 @@ func internalSecurityContextFromPodSecurityContext(pod *api.Pod) *api.SecurityCo
 	return synthesized
 }
 
-// AddNoNewPrivileges returns if we should add the no_new_privs option. This will return true if:
-// 1) the container is not privileged
-// 2) CAP_SYS_ADMIN is not being added
-// 3) if podSecurityPolicy.DefaultAllowPrivilegeEscalation is:
-//		- nil, then return false
-//		- true, then return false
-//		- false, then return true
+// AddNoNewPrivileges returns if we should add the no_new_privs option.
 func AddNoNewPrivileges(sc *v1.SecurityContext) bool {
 	if sc == nil {
 		return false
 	}
 
-	// handle the case where the container is privileged
-	if sc.Privileged != nil && *sc.Privileged {
-		return false
-	}
-
-	// handle the case where we are adding CAP_SYS_ADMIN
-	if sc.Capabilities != nil {
-		for _, cap := range sc.Capabilities.Add {
-			if string(cap) == "CAP_SYS_ADMIN" {
-				return false
-			}
-		}
-	}
-
 	// handle the case where the user did not set the default and did not explicitly set allowPrivilegeEscalation
 	if sc.AllowPrivilegeEscalation == nil {
 		return false

pkg/securitycontext/util_test.go

@@ -188,18 +188,6 @@ func TestAddNoNewPrivileges(t *testing.T) {
 		expect bool
 	}{
 		"allowPrivilegeEscalation nil security context nil": {},
-		"allowPrivilegeEscalation nil capAddSysadmin": {
-			sc: v1.SecurityContext{
-				Capabilities: &v1.Capabilities{
-					Add: []v1.Capability{"CAP_SYS_ADMIN"},
-				},
-			},
-		},
-		"allowPrivilegeEscalation nil privileged": {
-			sc: v1.SecurityContext{
-				Privileged: &ptrue,
-			},
-		},
 		"allowPrivilegeEscalation nil nonRoot": {
 			sc: v1.SecurityContext{
 				RunAsUser: &nonRoot,
@@ -210,20 +198,6 @@ func TestAddNoNewPrivileges(t *testing.T) {
 				RunAsUser: &root,
 			},
 		},
-		"allowPrivilegeEscalation false capAddSysadmin": {
-			sc: v1.SecurityContext{
-				Capabilities: &v1.Capabilities{
-					Add: []v1.Capability{"CAP_SYS_ADMIN"},
-				},
-				AllowPrivilegeEscalation: &pfalse,
-			},
-		},
-		"allowPrivilegeEscalation false privileged": {
-			sc: v1.SecurityContext{
-				Privileged:               &ptrue,
-				AllowPrivilegeEscalation: &pfalse,
-			},
-		},
 		"allowPrivilegeEscalation false nonRoot": {
 			sc: v1.SecurityContext{
 				RunAsUser:                &nonRoot,
@@ -238,20 +212,6 @@ func TestAddNoNewPrivileges(t *testing.T) {
 			},
 			expect: true,
 		},
-		"allowPrivilegeEscalation true capAddSysadmin": {
-			sc: v1.SecurityContext{
-				Capabilities: &v1.Capabilities{
-					Add: []v1.Capability{"CAP_SYS_ADMIN"},
-				},
-				AllowPrivilegeEscalation: &ptrue,
-			},
-		},
-		"allowPrivilegeEscalation true privileged": {
-			sc: v1.SecurityContext{
-				Privileged:               &ptrue,
-				AllowPrivilegeEscalation: &ptrue,
-			},
-		},
 		"allowPrivilegeEscalation true nonRoot": {
 			sc: v1.SecurityContext{
 				RunAsUser:                &nonRoot,