Skip to content

K
k3s
Commit 4b738a7b authored by Cao Shufeng's avatar Cao Shufeng
Browse files
Options

[PSP] always check validated policy first for update operation

When update a pod with `kubernetes.io/psp` annotation set, we should check this policy first. Because this saved policy is `usually` the one we are looking for.
parent 637cd7d0
Hide whitespace changes
Inline Side-by-side
Showing with 130 additions and 11 deletions
plugin/pkg/admission/security/podsecuritypolicy/BUILD
View file @ 4b738a7b
......@@ -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",
],
)
......
plugin/pkg/admission/security/podsecuritypolicy/admission.go
View file @ 4b738a7b
......@@ -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
......
plugin/pkg/admission/security/podsecuritypolicy/admission_test.go
View file @ 4b738a7b
......@@ -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{
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment