Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

AllowPrivilegeEscalation: add validations #52803

Merged
merged 1 commit into from Sep 25, 2017
Merged

AllowPrivilegeEscalation: add validations #52803

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 15 additions & 0 deletions pkg/api/validation/validation.go
View file
Open in desktop
Expand Up @@ -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
}

Expand Down
36 changes: 36 additions & 0 deletions pkg/api/validation/validation_test.go
View file
Open in desktop
Expand Up @@ -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 {
Expand Down Expand Up @@ -11082,3 +11114,7 @@ func TestValidateOrSetClientIPAffinityConfig(t *testing.T) {
}
}
}

func boolPtr(b bool) *bool {
return &b
}
22 changes: 1 addition & 21 deletions pkg/securitycontext/util.go
View file
Open in desktop
Expand Up @@ -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
Expand Down
40 changes: 0 additions & 40 deletions pkg/securitycontext/util_test.go
View file
Open in desktop
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down