Skip to content

Commit

Permalink
Security Context Constraints: prefer non-mutating SCC on update.
Browse files Browse the repository at this point in the history
  • Loading branch information
@php-coder
php-coder committed Dec 19, 2017
1 parent f4d81e2 commit de94214
Show file tree
Hide file tree
Showing 6 changed files with 154 additions and 114 deletions.
43 changes: 39 additions & 4 deletions pkg/security/admission/admission.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"github.com/golang/glog"

apiequality "k8s.io/apimachinery/pkg/api/equality"
"k8s.io/apimachinery/pkg/util/validation/field"
kapihelper "k8s.io/kubernetes/pkg/apis/core/helper"
rbacregistry "k8s.io/kubernetes/pkg/registry/rbac"
Expand Down Expand Up @@ -115,20 +116,54 @@ func (c *constraint) Admit(a admission.Attributes) error {
return admission.NewForbidden(a, fmt.Errorf("no providers available to validate pod request"))
}

// TODO(liggitt): allow spec mutation during initializing updates?
specMutationAllowed := a.GetOperation() == admission.Create

// all containers in a single pod must validate under a single provider or we will reject the request
validationErrs := field.ErrorList{}
var (
allowedPod *kapi.Pod
allowingProvider scc.SecurityContextConstraintsProvider
)

loop:
for _, provider := range providers {
if errs := scc.AssignSecurityContext(provider, pod, field.NewPath(fmt.Sprintf("provider %s: ", provider.GetSCCName()))); len(errs) > 0 {
podCopy := pod.DeepCopy()

if errs := scc.AssignSecurityContext(provider, podCopy, field.NewPath(fmt.Sprintf("provider %s: ", provider.GetSCCName()))); len(errs) > 0 {
validationErrs = append(validationErrs, errs...)
continue
}

// the entire pod validated, annotate and accept the pod
glog.V(4).Infof("pod %s (generate: %s) validated against provider %s", pod.Name, pod.GenerateName, provider.GetSCCName())
// the entire pod validated
switch {
case specMutationAllowed:
// if mutation is allowed, use the first found SCC that allows the pod.
// This behavior is different from Kubernetes which tries to search a non-mutating provider
// even on creating. We prefer most restrictive SCC in this case even if it mutates a pod.
allowedPod = podCopy
allowingProvider = provider
glog.V(6).Infof("pod %s (generate: %s) validated against provider %s with mutation", pod.Name, pod.GenerateName, provider.GetSCCName())
break loop
case apiequality.Semantic.DeepEqual(pod, podCopy):
// if we don't allow mutation, only use the validated pod if it didn't require any spec changes
allowedPod = podCopy
allowingProvider = provider
glog.V(6).Infof("pod %s (generate: %s) validated against provider %s without mutation", pod.Name, pod.GenerateName, provider.GetSCCName())
break loop
default:
glog.V(6).Infof("pod %s (generate: %s) validated against provider %s, but required mutation, skipping", pod.Name, pod.GenerateName, provider.GetSCCName())
}
}

if allowedPod != nil {
*pod = *allowedPod
// annotate and accept the pod
glog.V(4).Infof("pod %s (generate: %s) validated against provider %s", pod.Name, pod.GenerateName, allowingProvider.GetSCCName())
if pod.ObjectMeta.Annotations == nil {
pod.ObjectMeta.Annotations = map[string]string{}
}
pod.ObjectMeta.Annotations[allocator.ValidatedSCCAnnotation] = provider.GetSCCName()
pod.ObjectMeta.Annotations[allocator.ValidatedSCCAnnotation] = allowingProvider.GetSCCName()
return nil
}

Expand Down
122 changes: 102 additions & 20 deletions pkg/security/admission/admission_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,16 +140,17 @@ func TestAdmitCaps(t *testing.T) {
for i := 0; i < 2; i++ {
for k, v := range tc {
v.pod.Spec.Containers, v.pod.Spec.InitContainers = v.pod.Spec.InitContainers, v.pod.Spec.Containers

testSCCAdmit(k, v.sccs, v.pod, v.shouldPass, t)

containers := v.pod.Spec.Containers
if i == 0 {
containers = v.pod.Spec.InitContainers
}

testSCCAdmit(k, v.sccs, v.pod, v.shouldPass, t)

if v.expectedCapabilities != nil {
if !reflect.DeepEqual(v.expectedCapabilities, containers[0].SecurityContext.Capabilities) {
t.Errorf("%s resulted in caps that were not expected - expected: %v, received: %v", k, v.expectedCapabilities, containers[0].SecurityContext.Capabilities)
t.Errorf("%s resulted in caps that were not expected - expected: %#v, received: %#v", k, v.expectedCapabilities, containers[0].SecurityContext.Capabilities)
}
}
}
Expand Down Expand Up @@ -239,43 +240,44 @@ func TestAdmitSuccess(t *testing.T) {
"specifyUIDInRange": {
pod: specifyUIDInRange,
expectedPodSC: podSC(seLinuxLevelFromNamespace, defaultGroup, defaultGroup),
expectedContainerSC: containerSC(seLinuxLevelFromNamespace, goodUID),
expectedContainerSC: containerSC(nil, goodUID),
},
"specifyLabels": {
pod: specifyLabels,
expectedPodSC: podSC(seLinuxLevelFromNamespace, defaultGroup, defaultGroup),
expectedContainerSC: containerSC(seLinuxLevelFromNamespace, 1),
expectedContainerSC: containerSC(&seLinuxLevelFromNamespace, 1),
},
"specifyFSGroup": {
pod: specifyFSGroupInRange,
expectedPodSC: podSC(seLinuxLevelFromNamespace, goodFSGroup, defaultGroup),
expectedContainerSC: containerSC(seLinuxLevelFromNamespace, 1),
expectedContainerSC: containerSC(nil, 1),
},
"specifySupGroup": {
pod: specifySupGroup,
expectedPodSC: podSC(seLinuxLevelFromNamespace, defaultGroup, 3),
expectedContainerSC: containerSC(seLinuxLevelFromNamespace, 1),
expectedContainerSC: containerSC(nil, 1),
},
"specifyPodLevelSELinuxLevel": {
pod: specifyPodLevelSELinux,
expectedPodSC: podSC(seLinuxLevelFromNamespace, defaultGroup, defaultGroup),
expectedContainerSC: containerSC(seLinuxLevelFromNamespace, 1),
expectedContainerSC: containerSC(nil, 1),
},
}

for i := 0; i < 2; i++ {
for k, v := range testCases {
v.pod.Spec.Containers, v.pod.Spec.InitContainers = v.pod.Spec.InitContainers, v.pod.Spec.Containers
containers := v.pod.Spec.Containers
if i == 0 {
containers = v.pod.Spec.InitContainers
}

hasErrors := testSCCAdmission(v.pod, p, saSCC.Name, k, t)
if hasErrors {
continue
}

containers := v.pod.Spec.Containers
if i == 0 {
containers = v.pod.Spec.InitContainers
}

if !reflect.DeepEqual(v.expectedPodSC, v.pod.Spec.SecurityContext) {
t.Errorf("%s unexpected pod SecurityContext diff:\n%s", k, diff.ObjectGoPrintSideBySide(v.expectedPodSC, v.pod.Spec.SecurityContext))
}
Expand Down Expand Up @@ -934,6 +936,85 @@ func TestAdmitSeccomp(t *testing.T) {

}

func TestAdmitPreferNonmutatingWhenPossible(t *testing.T) {

mutatingSCC := restrictiveSCC()
mutatingSCC.Name = "mutating-scc"

nonMutatingSCC := laxSCC()
nonMutatingSCC.Name = "non-mutating-scc"

simplePod := goodPod()
simplePod.Spec.Containers[0].Name = "simple-pod"
simplePod.Spec.Containers[0].Image = "test-image:0.1"

modifiedPod := simplePod.DeepCopy()
modifiedPod.Spec.Containers[0].Image = "test-image:0.2"

tests := map[string]struct {
oldPod *kapi.Pod
newPod *kapi.Pod
operation kadmission.Operation
sccs []*securityapi.SecurityContextConstraints
shouldPass bool
expectedSCC string
}{
"creation: the first SCC (even if it mutates) should be used": {
newPod: simplePod.DeepCopy(),
operation: kadmission.Create,
sccs: []*securityapi.SecurityContextConstraints{mutatingSCC, nonMutatingSCC},
shouldPass: true,
expectedSCC: mutatingSCC.Name,
},
"updating: the first non-mutating SCC should be used": {
oldPod: simplePod.DeepCopy(),
newPod: modifiedPod.DeepCopy(),
operation: kadmission.Update,
sccs: []*securityapi.SecurityContextConstraints{mutatingSCC, nonMutatingSCC},
shouldPass: true,
expectedSCC: nonMutatingSCC.Name,
},
"updating: a pod should be rejected when there are only mutating SCCs": {
oldPod: simplePod.DeepCopy(),
newPod: modifiedPod.DeepCopy(),
operation: kadmission.Update,
sccs: []*securityapi.SecurityContextConstraints{mutatingSCC},
shouldPass: false,
},
}

for testCaseName, testCase := range tests {
// We can't use testSCCAdmission() here because it doesn't support Update operation.
// We can't use testSCCAdmit() here because it doesn't support Update operation and doesn't check for the SCC annotation.

tc := setupClientSet()
lister := createSCCLister(t, testCase.sccs)
plugin := NewTestAdmission(lister, tc)

attrs := kadmission.NewAttributesRecord(testCase.newPod, testCase.oldPod, kapi.Kind("Pod").WithVersion("version"), testCase.newPod.Namespace, testCase.newPod.Name, kapi.Resource("pods").WithVersion("version"), "", testCase.operation, &user.DefaultInfo{})
err := plugin.(kadmission.MutationInterface).Admit(attrs)

if testCase.shouldPass {
if err != nil {
t.Errorf("%s expected no errors but received %v", testCaseName, err)
} else {
validatedSCC, ok := testCase.newPod.Annotations[allocator.ValidatedSCCAnnotation]
if !ok {
t.Errorf("expected %q to find the validated annotation on the pod for the scc but found none", testCaseName)

} else if validatedSCC != testCase.expectedSCC {
t.Errorf("%q should have validated against %q but found %q", testCaseName, testCase.expectedSCC, validatedSCC)
}
}
} else {
if err == nil {
t.Errorf("%s expected errors but received none", testCaseName)
}
}
}

}

// testSCCAdmission is a helper to admit the pod and ensure it was validated against the expected
// SCC. Returns true when errors have been encountered.
func testSCCAdmission(pod *kapi.Pod, plugin kadmission.Interface, expectedSCC, testName string, t *testing.T) bool {
Expand Down Expand Up @@ -1087,15 +1168,16 @@ func goodPod() *kapi.Pod {
}
}

func containerSC(seLinuxLevel string, uid int64) *kapi.SecurityContext {
no := false
return &kapi.SecurityContext{
Privileged: &no,
RunAsUser: &uid,
SELinuxOptions: &kapi.SELinuxOptions{
Level: seLinuxLevel,
},
func containerSC(seLinuxLevel *string, uid int64) *kapi.SecurityContext {
sc := &kapi.SecurityContext{
RunAsUser: &uid,
}
if seLinuxLevel != nil {
sc.SELinuxOptions = &kapi.SELinuxOptions{
Level: *seLinuxLevel,
}
}
return sc
}

func podSC(seLinuxLevel string, fsGroup, supGroup int64) *kapi.PodSecurityContext {
Expand Down
70 changes: 13 additions & 57 deletions pkg/security/securitycontextconstraints/matcher.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
"k8s.io/apiserver/pkg/authentication/user"
kapi "k8s.io/kubernetes/pkg/apis/core"
clientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
sc "k8s.io/kubernetes/pkg/securitycontext"

allocator "github.com/openshift/origin/pkg/security"
securityapi "github.com/openshift/origin/pkg/security/apis/security"
Expand Down Expand Up @@ -71,87 +70,44 @@ func ConstraintAppliesTo(constraint *securityapi.SecurityContextConstraints, use
// and validates that the sc falls within the scc constraints. All containers must validate against
// the same scc or is not considered valid.
func AssignSecurityContext(provider SecurityContextConstraintsProvider, pod *kapi.Pod, fldPath *field.Path) field.ErrorList {
generatedSCs := make([]*kapi.SecurityContext, len(pod.Spec.InitContainers)+len(pod.Spec.Containers))

errs := field.ErrorList{}

psc, generatedAnnotations, err := provider.CreatePodSecurityContext(pod)
if err != nil {
errs = append(errs, field.Invalid(fldPath.Child("spec", "securityContext"), pod.Spec.SecurityContext, err.Error()))
}

// save the original PSC and validate the generated PSC. Leave the generated PSC
// set for container generation/validation. We will reset to original post container
// validation.
originalPSC := pod.Spec.SecurityContext
originalAnnotations := pod.Annotations

pod.Spec.SecurityContext = psc
pod.Annotations = generatedAnnotations
errs = append(errs, provider.ValidatePodSecurityContext(pod, fldPath.Child("spec", "securityContext"))...)

// Note: this is not changing the original container, we will set container SCs later so long
// as all containers validated under the same SCC.
containerPath := fldPath.Child("spec", "initContainers")
for i, containerCopy := range pod.Spec.InitContainers {
csc, resolutionErrs := resolveContainerSecurityContext(provider, pod, &containerCopy, containerPath.Index(i))
errs = append(errs, resolutionErrs...)
if len(resolutionErrs) > 0 {
for i := range pod.Spec.InitContainers {
sc, err := provider.CreateContainerSecurityContext(pod, &pod.Spec.InitContainers[i])
if err != nil {
errs = append(errs, field.Invalid(field.NewPath("spec", "initContainers").Index(i).Child("securityContext"), "", err.Error()))
continue
}
generatedSCs[i] = csc
pod.Spec.InitContainers[i].SecurityContext = sc
errs = append(errs, provider.ValidateContainerSecurityContext(pod, &pod.Spec.InitContainers[i], field.NewPath("spec", "initContainers").Index(i).Child("securityContext"))...)
}

base := len(pod.Spec.InitContainers)

// Note: this is not changing the original container, we will set container SCs later so long
// as all containers validated under the same SCC.
containerPath = fldPath.Child("spec", "containers")
for i, containerCopy := range pod.Spec.Containers {
csc, resolutionErrs := resolveContainerSecurityContext(provider, pod, &containerCopy, containerPath.Index(i))
errs = append(errs, resolutionErrs...)
if len(resolutionErrs) > 0 {
for i := range pod.Spec.Containers {
sc, err := provider.CreateContainerSecurityContext(pod, &pod.Spec.Containers[i])
if err != nil {
errs = append(errs, field.Invalid(field.NewPath("spec", "containers").Index(i).Child("securityContext"), "", err.Error()))
continue
}
generatedSCs[i+base] = csc
pod.Spec.Containers[i].SecurityContext = sc
errs = append(errs, provider.ValidateContainerSecurityContext(pod, &pod.Spec.Containers[i], field.NewPath("spec", "containers").Index(i).Child("securityContext"))...)
}

if len(errs) > 0 {
// ensure psc is not mutated if there are errors
pod.Spec.SecurityContext = originalPSC
pod.Annotations = originalAnnotations
return errs
}

// if we've reached this code then we've generated and validated an SC for every container in the
// pod so let's apply what we generated. Note: the psc is already applied.
for i := range pod.Spec.InitContainers {
pod.Spec.InitContainers[i].SecurityContext = generatedSCs[i]
}
for i := range pod.Spec.Containers {
pod.Spec.Containers[i].SecurityContext = generatedSCs[i+base]
}
return nil
}

// resolveContainerSecurityContext checks the provided container against the provider, returning any
// validation errors encountered on the resulting security context, or the security context that was
// resolved. The SecurityContext field of the container is updated, so ensure that a copy of the original
// container is passed here if you wish to preserve the original input.
func resolveContainerSecurityContext(provider SecurityContextConstraintsProvider, pod *kapi.Pod, container *kapi.Container, path *field.Path) (*kapi.SecurityContext, field.ErrorList) {
// We will determine the effective security context for the container and validate against that
// since that is how the sc provider will eventually apply settings in the runtime.
// This results in an SC that is based on the Pod's PSC with the set fields from the container
// overriding pod level settings.
container.SecurityContext = sc.InternalDetermineEffectiveSecurityContext(pod, container)

csc, err := provider.CreateContainerSecurityContext(pod, container)
if err != nil {
return nil, field.ErrorList{field.Invalid(path.Child("securityContext"), "", err.Error())}
}
container.SecurityContext = csc
return csc, provider.ValidateContainerSecurityContext(pod, container, path.Child("securityContext"))
}

// constraintSupportsGroup checks that group is in constraintGroups.
func constraintSupportsGroup(group string, constraintGroups []string) bool {
for _, g := range constraintGroups {
Expand Down
Loading

0 comments on commit de94214

Please sign in to comment.