From b45b809f4cd0cd9d128cc7bcfec956b6c64e02a8 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Thu, 5 Oct 2017 15:55:53 -0400 Subject: [PATCH] PodSecurityPolicy: Do not mutate nil privileged field to false --- pkg/security/podsecuritypolicy/provider.go | 7 +--- .../podsecuritypolicy/admission_test.go | 41 ++++++++++++++----- 2 files changed, 31 insertions(+), 17 deletions(-) diff --git a/pkg/security/podsecuritypolicy/provider.go b/pkg/security/podsecuritypolicy/provider.go index 1c608ef81da..c659addf4ec 100644 --- a/pkg/security/podsecuritypolicy/provider.go +++ b/pkg/security/podsecuritypolicy/provider.go @@ -157,11 +157,6 @@ func (s *simpleProvider) CreateContainerSecurityContext(pod *api.Pod, container return nil, nil, err } - if sc.Privileged == nil { - priv := false - sc.Privileged = &priv - } - // if we're using the non-root strategy set the marker that this container should not be // run as root which will signal to the kubelet to do a final check either on the runAsUser // or, if runAsUser is not set, the image UID will be checked. @@ -284,7 +279,7 @@ func (s *simpleProvider) ValidateContainerSecurityContext(pod *api.Pod, containe allErrs = append(allErrs, s.strategies.AppArmorStrategy.Validate(pod, container)...) allErrs = append(allErrs, s.strategies.SeccompStrategy.ValidateContainer(pod, container)...) - if !s.psp.Spec.Privileged && *sc.Privileged { + if !s.psp.Spec.Privileged && sc.Privileged != nil && *sc.Privileged { allErrs = append(allErrs, field.Invalid(fldPath.Child("privileged"), *sc.Privileged, "Privileged containers are not allowed")) } diff --git a/plugin/pkg/admission/security/podsecuritypolicy/admission_test.go b/plugin/pkg/admission/security/podsecuritypolicy/admission_test.go index b88df3deefd..b6bafdbbb82 100644 --- a/plugin/pkg/admission/security/podsecuritypolicy/admission_test.go +++ b/plugin/pkg/admission/security/podsecuritypolicy/admission_test.go @@ -204,37 +204,54 @@ func TestAdmitPrivileged(t *testing.T) { privilegedPSP.Name = "priv" privilegedPSP.Spec.Privileged = true + trueValue := true + falseValue := false + tests := map[string]struct { pod *kapi.Pod psps []*extensions.PodSecurityPolicy shouldPass bool - expectedPriv bool + expectedPriv *bool expectedPSP string }{ - "pod without priv request allowed under non priv PSP": { + "pod with priv=nil allowed under non priv PSP": { pod: goodPod(), psps: []*extensions.PodSecurityPolicy{nonPrivilegedPSP}, shouldPass: true, - expectedPriv: false, + expectedPriv: nil, expectedPSP: nonPrivilegedPSP.Name, }, - "pod without priv request allowed under priv PSP": { + "pod with priv=nil allowed under priv PSP": { pod: goodPod(), psps: []*extensions.PodSecurityPolicy{privilegedPSP}, shouldPass: true, - expectedPriv: false, + expectedPriv: nil, expectedPSP: privilegedPSP.Name, }, - "pod with priv request denied by non priv PSP": { + "pod with priv=false allowed under non priv PSP": { + pod: createPodWithPriv(false), + psps: []*extensions.PodSecurityPolicy{nonPrivilegedPSP}, + shouldPass: true, + expectedPriv: &falseValue, + expectedPSP: nonPrivilegedPSP.Name, + }, + "pod with priv=false allowed under priv PSP": { + pod: createPodWithPriv(false), + psps: []*extensions.PodSecurityPolicy{privilegedPSP}, + shouldPass: true, + expectedPriv: &falseValue, + expectedPSP: privilegedPSP.Name, + }, + "pod with priv=true denied by non priv PSP": { pod: createPodWithPriv(true), psps: []*extensions.PodSecurityPolicy{nonPrivilegedPSP}, shouldPass: false, }, - "pod with priv request allowed by priv PSP": { + "pod with priv=true allowed by priv PSP": { pod: createPodWithPriv(true), psps: []*extensions.PodSecurityPolicy{nonPrivilegedPSP, privilegedPSP}, shouldPass: true, - expectedPriv: true, + expectedPriv: &trueValue, expectedPSP: privilegedPSP.Name, }, } @@ -243,9 +260,11 @@ func TestAdmitPrivileged(t *testing.T) { testPSPAdmit(k, v.psps, v.pod, v.shouldPass, v.expectedPSP, t) if v.shouldPass { - if v.pod.Spec.Containers[0].SecurityContext.Privileged == nil || - *v.pod.Spec.Containers[0].SecurityContext.Privileged != v.expectedPriv { - t.Errorf("%s expected privileged to be %t", k, v.expectedPriv) + priv := v.pod.Spec.Containers[0].SecurityContext.Privileged + if (priv == nil) != (v.expectedPriv == nil) { + t.Errorf("%s expected privileged to be %v, got %v", k, v.expectedPriv, priv) + } else if priv != nil && *priv != *v.expectedPriv { + t.Errorf("%s expected privileged to be %v, got %v", k, *v.expectedPriv, *priv) } } }