diff --git a/plugin/pkg/admission/security/podsecuritypolicy/BUILD b/plugin/pkg/admission/security/podsecuritypolicy/BUILD index 6d96b49e456..5c256b008b2 100644 --- a/plugin/pkg/admission/security/podsecuritypolicy/BUILD +++ b/plugin/pkg/admission/security/podsecuritypolicy/BUILD @@ -56,6 +56,7 @@ go_test( "//vendor/k8s.io/apiserver/pkg/authentication/serviceaccount:go_default_library", "//vendor/k8s.io/apiserver/pkg/authentication/user:go_default_library", "//vendor/k8s.io/apiserver/pkg/authorization/authorizer:go_default_library", + "//vendor/k8s.io/apiserver/pkg/authorization/authorizerfactory:go_default_library", ], ) diff --git a/plugin/pkg/admission/security/podsecuritypolicy/admission.go b/plugin/pkg/admission/security/podsecuritypolicy/admission.go index a8d80515535..380cf2b63a0 100644 --- a/plugin/pkg/admission/security/podsecuritypolicy/admission.go +++ b/plugin/pkg/admission/security/podsecuritypolicy/admission.go @@ -122,8 +122,8 @@ func (c *PodSecurityPolicyPlugin) Admit(a admission.Attributes) error { pod := a.GetObject().(*api.Pod) - // compute the context - allowedPod, pspName, validationErrs, err := c.computeSecurityContext(a, pod, true) + // compute the context. Mutation is allowed. ValidatedPSPAnnotation is not taken into account. + allowedPod, pspName, validationErrs, err := c.computeSecurityContext(a, pod, true, "") if err != nil { return admission.NewForbidden(a, err) } @@ -152,8 +152,8 @@ func (c *PodSecurityPolicyPlugin) Validate(a admission.Attributes) error { pod := a.GetObject().(*api.Pod) - // compute the context. Mutation is not allowed. - allowedPod, _, validationErrs, err := c.computeSecurityContext(a, pod, false) + // compute the context. Mutation is not allowed. ValidatedPSPAnnotation is used as a hint to gain same speed-up. + allowedPod, _, validationErrs, err := c.computeSecurityContext(a, pod, false, pod.ObjectMeta.Annotations[psputil.ValidatedPSPAnnotation]) if err != nil { return admission.NewForbidden(a, err) } @@ -193,8 +193,9 @@ func shouldIgnore(a admission.Attributes) (bool, error) { // computeSecurityContext derives a valid security context while trying to avoid any changes to the given pod. I.e. // if there is a matching policy with the same security context as given, it will be reused. If there is no -// matching policy the returned pod will be nil and the pspName empty. -func (c *PodSecurityPolicyPlugin) computeSecurityContext(a admission.Attributes, pod *api.Pod, specMutationAllowed bool) (*api.Pod, string, field.ErrorList, error) { +// matching policy the returned pod will be nil and the pspName empty. validatedPSPHint is the validated psp name +// saved in kubernetes.io/psp annotation. This psp is usually the one we are looking for. +func (c *PodSecurityPolicyPlugin) computeSecurityContext(a admission.Attributes, pod *api.Pod, specMutationAllowed bool, validatedPSPHint string) (*api.Pod, string, field.ErrorList, error) { // get all constraints that are usable by the user glog.V(4).Infof("getting pod security policies for pod %s (generate: %s)", pod.Name, pod.GenerateName) var saInfo user.Info @@ -213,9 +214,18 @@ func (c *PodSecurityPolicyPlugin) computeSecurityContext(a admission.Attributes, return pod, "", nil, nil } - // sort by name to make order deterministic + // sort policies by name to make order deterministic + // If mutation is not allowed and validatedPSPHint is provided, check the validated policy first. // TODO(liggitt): add priority field to allow admins to bucket differently sort.SliceStable(policies, func(i, j int) bool { + if !specMutationAllowed { + if policies[i].Name == validatedPSPHint { + return true + } + if policies[j].Name == validatedPSPHint { + return false + } + } return strings.Compare(policies[i].Name, policies[j].Name) < 0 }) @@ -244,7 +254,6 @@ func (c *PodSecurityPolicyPlugin) computeSecurityContext(a admission.Attributes, } // the entire pod validated - mutated := !apiequality.Semantic.DeepEqual(pod, podCopy) if mutated && !specMutationAllowed { continue diff --git a/plugin/pkg/admission/security/podsecuritypolicy/admission_test.go b/plugin/pkg/admission/security/podsecuritypolicy/admission_test.go index 8b2c2b701ed..48febd89769 100644 --- a/plugin/pkg/admission/security/podsecuritypolicy/admission_test.go +++ b/plugin/pkg/admission/security/podsecuritypolicy/admission_test.go @@ -32,6 +32,7 @@ import ( "k8s.io/apiserver/pkg/authentication/serviceaccount" "k8s.io/apiserver/pkg/authentication/user" "k8s.io/apiserver/pkg/authorization/authorizer" + "k8s.io/apiserver/pkg/authorization/authorizerfactory" "k8s.io/kubernetes/pkg/api/legacyscheme" kapi "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/core/helper" @@ -65,7 +66,7 @@ func NewTestAdmission(psps []*extensions.PodSecurityPolicy, authz authorizer.Aut } } -// TestAlwaysAllowedAuthorizer is a testing struct for testing that fulfills the authorizer interface. +// TestAuthorizer is a testing struct for testing that fulfills the authorizer interface. type TestAuthorizer struct { // usernameToNamespaceToAllowedPSPs contains the map of allowed PSPs. // if nil, all PSPs are allowed. @@ -344,6 +345,13 @@ func TestAdmitPreferNonmutating(t *testing.T) { gcChangedPod.OwnerReferences = []metav1.OwnerReference{{Kind: "Foo", Name: "bar"}} gcChangedPod.Finalizers = []string{"foo"} + podWithAnnotation := unprivilegedRunAsAnyPod.DeepCopy() + podWithAnnotation.ObjectMeta.Annotations = map[string]string{ + // "mutating2" is lexicographically behind "mutating1", so "mutating1" should be + // chosen because it's the canonical PSP order. + psputil.ValidatedPSPAnnotation: mutating2.Name, + } + tests := map[string]struct { operation kadmission.Operation pod *kapi.Pod @@ -381,6 +389,15 @@ func TestAdmitPreferNonmutating(t *testing.T) { expectedContainerUser: &mutating1.Spec.RunAsUser.Ranges[0].Min, expectedPSP: mutating1.Name, }, + "pod should use deterministic mutating PSP on create even if ValidatedPSPAnnotation is set": { + operation: kadmission.Create, + pod: podWithAnnotation, + psps: []*extensions.PodSecurityPolicy{mutating2, mutating1}, + shouldPassValidate: true, + expectMutation: true, + expectedContainerUser: &mutating1.Spec.RunAsUser.Ranges[0].Min, + expectedPSP: mutating1.Name, + }, "pod should prefer non-mutating PSP on update": { operation: kadmission.Update, pod: changedPodWithSC.DeepCopy(), @@ -2130,7 +2147,6 @@ func TestPolicyAuthorizationErrors(t *testing.T) { ) tests := map[string]struct { - priviliged bool inPolicies []*extensions.PodSecurityPolicy allowed map[string]map[string]map[string]bool expectValidationErrs int @@ -2197,7 +2213,7 @@ func TestPolicyAuthorizationErrors(t *testing.T) { plugin := NewTestAdmission(tc.inPolicies, authz) attrs := kadmission.NewAttributesRecord(pod, nil, kapi.Kind("Pod").WithVersion("version"), ns, "", kapi.Resource("pods").WithVersion("version"), "", kadmission.Create, &user.DefaultInfo{Name: userName}) - allowedPod, _, validationErrs, err := plugin.computeSecurityContext(attrs, pod, true) + allowedPod, _, validationErrs, err := plugin.computeSecurityContext(attrs, pod, true, "") assert.Nil(t, allowedPod) assert.NoError(t, err) assert.Len(t, validationErrs, tc.expectValidationErrs) @@ -2205,6 +2221,99 @@ func TestPolicyAuthorizationErrors(t *testing.T) { } } +func TestPreferValidatedPSP(t *testing.T) { + restrictivePSPWithName := func(name string) *extensions.PodSecurityPolicy { + p := restrictivePSP() + p.Name = name + return p + } + + permissivePSPWithName := func(name string) *extensions.PodSecurityPolicy { + p := permissivePSP() + p.Name = name + return p + } + + tests := map[string]struct { + inPolicies []*extensions.PodSecurityPolicy + expectValidationErrs int + validatedPSPHint string + expectedPSP string + }{ + "no policy saved in annotations, PSPs are ordered lexicographically": { + inPolicies: []*extensions.PodSecurityPolicy{ + restrictivePSPWithName("001restrictive"), + restrictivePSPWithName("002restrictive"), + permissivePSPWithName("002permissive"), + permissivePSPWithName("001permissive"), + permissivePSPWithName("003permissive"), + }, + expectValidationErrs: 0, + validatedPSPHint: "", + expectedPSP: "001permissive", + }, + "policy saved in annotations is prefered": { + inPolicies: []*extensions.PodSecurityPolicy{ + restrictivePSPWithName("001restrictive"), + restrictivePSPWithName("002restrictive"), + permissivePSPWithName("001permissive"), + permissivePSPWithName("002permissive"), + permissivePSPWithName("003permissive"), + }, + expectValidationErrs: 0, + validatedPSPHint: "002permissive", + expectedPSP: "002permissive", + }, + "policy saved in annotations is invalid": { + inPolicies: []*extensions.PodSecurityPolicy{ + restrictivePSPWithName("001restrictive"), + restrictivePSPWithName("002restrictive"), + }, + expectValidationErrs: 2, + validatedPSPHint: "foo", + expectedPSP: "", + }, + "policy saved in annotations is disallowed anymore": { + inPolicies: []*extensions.PodSecurityPolicy{ + restrictivePSPWithName("001restrictive"), + restrictivePSPWithName("002restrictive"), + }, + expectValidationErrs: 2, + validatedPSPHint: "001restrictive", + expectedPSP: "", + }, + "policy saved in annotations is disallowed anymore, but find another one": { + inPolicies: []*extensions.PodSecurityPolicy{ + restrictivePSPWithName("001restrictive"), + restrictivePSPWithName("002restrictive"), + permissivePSPWithName("002permissive"), + permissivePSPWithName("001permissive"), + }, + expectValidationErrs: 0, + validatedPSPHint: "001restrictive", + expectedPSP: "001permissive", + }, + } + for desc, tc := range tests { + t.Run(desc, func(t *testing.T) { + authz := authorizerfactory.NewAlwaysAllowAuthorizer() + allowPrivilegeEscalation := true + pod := goodPod() + pod.Namespace = "ns" + pod.Spec.ServiceAccountName = "sa" + pod.Spec.Containers[0].SecurityContext.AllowPrivilegeEscalation = &allowPrivilegeEscalation + + plugin := NewTestAdmission(tc.inPolicies, authz) + attrs := kadmission.NewAttributesRecord(pod, nil, kapi.Kind("Pod").WithVersion("version"), "ns", "", kapi.Resource("pods").WithVersion("version"), "", kadmission.Update, &user.DefaultInfo{Name: "test"}) + + _, pspName, validationErrs, err := plugin.computeSecurityContext(attrs, pod, false, tc.validatedPSPHint) + assert.NoError(t, err) + assert.Len(t, validationErrs, tc.expectValidationErrs) + assert.Equal(t, pspName, tc.expectedPSP) + }) + } +} + func restrictivePSP() *extensions.PodSecurityPolicy { return &extensions.PodSecurityPolicy{ ObjectMeta: metav1.ObjectMeta{