From 0ad51ed7638209be116ce6cb2cfce52ed29144bf Mon Sep 17 00:00:00 2001 From: Jess Frazelle Date: Thu, 21 Sep 2017 18:35:06 -0400 Subject: [PATCH] AllowPrivilegeEscalation: add validations for caps and privileged Signed-off-by: Jess Frazelle --- pkg/api/validation/validation.go | 15 ++++++++++ pkg/api/validation/validation_test.go | 36 ++++++++++++++++++++++++ pkg/securitycontext/util.go | 22 +-------------- pkg/securitycontext/util_test.go | 40 --------------------------- 4 files changed, 52 insertions(+), 61 deletions(-) diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index 55830e26f4f1..3d08bbe51ee4 100644 --- a/pkg/api/validation/validation.go +++ b/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 } diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index 70168696b8fd..1df78dc1d9fa 100644 --- a/pkg/api/validation/validation_test.go +++ b/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 +} diff --git a/pkg/securitycontext/util.go b/pkg/securitycontext/util.go index 60ea8dd544ff..dee1564d0622 100644 --- a/pkg/securitycontext/util.go +++ 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 diff --git a/pkg/securitycontext/util_test.go b/pkg/securitycontext/util_test.go index 91cf64054f41..1b89adcd893f 100644 --- a/pkg/securitycontext/util_test.go +++ b/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,