From 829e6f6cfb0f5ae080dcb76ff0bb60920569fb0d Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Wed, 1 Mar 2017 16:21:40 -0500 Subject: [PATCH] Include pod namespace in PSP 'use' authorization check --- .../security/podsecuritypolicy/admission.go | 22 ++--- .../podsecuritypolicy/admission_test.go | 94 +++++++++++-------- 2 files changed, 67 insertions(+), 49 deletions(-) diff --git a/plugin/pkg/admission/security/podsecuritypolicy/admission.go b/plugin/pkg/admission/security/podsecuritypolicy/admission.go index ac03ec20ddf..87b81d6da0d 100644 --- a/plugin/pkg/admission/security/podsecuritypolicy/admission.go +++ b/plugin/pkg/admission/security/podsecuritypolicy/admission.go @@ -52,7 +52,7 @@ func init() { } // PSPMatchFn allows plugging in how PSPs are matched against user information. -type PSPMatchFn func(lister extensionslisters.PodSecurityPolicyLister, user user.Info, sa user.Info, authz authorizer.Authorizer) ([]*extensions.PodSecurityPolicy, error) +type PSPMatchFn func(lister extensionslisters.PodSecurityPolicyLister, user user.Info, sa user.Info, authz authorizer.Authorizer, namespace string) ([]*extensions.PodSecurityPolicy, error) // podSecurityPolicyPlugin holds state for and implements the admission plugin. type podSecurityPolicyPlugin struct { @@ -130,7 +130,7 @@ func (c *podSecurityPolicyPlugin) Admit(a admission.Attributes) error { saInfo = serviceaccount.UserInfo(a.GetNamespace(), pod.Spec.ServiceAccountName, "") } - matchedPolicies, err := c.pspMatcher(c.lister, a.GetUserInfo(), saInfo, c.authz) + matchedPolicies, err := c.pspMatcher(c.lister, a.GetUserInfo(), saInfo, c.authz, a.GetNamespace()) if err != nil { return admission.NewForbidden(a, err) } @@ -279,7 +279,7 @@ func (c *podSecurityPolicyPlugin) createProvidersFromPolicies(psps []*extensions // TODO: this will likely need optimization since the initial implementation will // always query for authorization. Needs scale testing and possibly checking against // a cache. -func getMatchingPolicies(lister extensionslisters.PodSecurityPolicyLister, user user.Info, sa user.Info, authz authorizer.Authorizer) ([]*extensions.PodSecurityPolicy, error) { +func getMatchingPolicies(lister extensionslisters.PodSecurityPolicyLister, user user.Info, sa user.Info, authz authorizer.Authorizer, namespace string) ([]*extensions.PodSecurityPolicy, error) { matchedPolicies := make([]*extensions.PodSecurityPolicy, 0) list, err := lister.List(labels.Everything()) @@ -289,7 +289,7 @@ func getMatchingPolicies(lister extensionslisters.PodSecurityPolicyLister, user for _, constraint := range list { // if no user info exists then the API is being hit via the unsecured port. In this case authorize the request. - if user == nil || authorizedForPolicy(user, constraint, authz) || authorizedForPolicy(sa, constraint, authz) { + if user == nil || authorizedForPolicy(user, namespace, constraint, authz) || authorizedForPolicy(sa, namespace, constraint, authz) { matchedPolicies = append(matchedPolicies, constraint) } } @@ -297,26 +297,26 @@ func getMatchingPolicies(lister extensionslisters.PodSecurityPolicyLister, user return matchedPolicies, nil } -// authorizedForPolicy returns true if info is authorized to perform a "get" on policy. -func authorizedForPolicy(info user.Info, policy *extensions.PodSecurityPolicy, authz authorizer.Authorizer) bool { +// authorizedForPolicy returns true if info is authorized to perform the "use" verb on the policy resource. +func authorizedForPolicy(info user.Info, namespace string, policy *extensions.PodSecurityPolicy, authz authorizer.Authorizer) bool { if info == nil { return false } - attr := buildAttributes(info, policy) + attr := buildAttributes(info, namespace, policy) allowed, reason, err := authz.Authorize(attr) if err != nil { - glog.V(5).Infof("cannot authorized for policy: %v,%v", reason, err) + glog.V(5).Infof("cannot authorize for policy: %v,%v", reason, err) } return allowed } // buildAttributes builds an attributes record for a SAR based on the user info and policy. -func buildAttributes(info user.Info, policy *extensions.PodSecurityPolicy) authorizer.Attributes { - // TODO consider checking against the namespace that the pod is being - // created in to allow per-namespace PSP definitions. +func buildAttributes(info user.Info, namespace string, policy *extensions.PodSecurityPolicy) authorizer.Attributes { + // check against the namespace that the pod is being created in to allow per-namespace PSP grants. attr := authorizer.AttributesRecord{ User: info, Verb: "use", + Namespace: namespace, Name: policy.Name, APIGroup: extensions.GroupName, Resource: "podsecuritypolicies", diff --git a/plugin/pkg/admission/security/podsecuritypolicy/admission_test.go b/plugin/pkg/admission/security/podsecuritypolicy/admission_test.go index be13e8cac26..80a233cad23 100644 --- a/plugin/pkg/admission/security/podsecuritypolicy/admission_test.go +++ b/plugin/pkg/admission/security/podsecuritypolicy/admission_test.go @@ -57,18 +57,18 @@ func NewTestAdmission(lister extensionslisters.PodSecurityPolicyLister) kadmissi // TestAlwaysAllowedAuthorizer is a testing struct for testing that fulfills the authorizer interface. type TestAuthorizer struct { - // disallowed contains names of disallowed policies. Map is keyed by user.Info.GetName() - disallowed map[string][]string + // usernameToNamespaceToAllowedPSPs contains the map of allowed PSPs. + // if nil, all PSPs are allowed. + usernameToNamespaceToAllowedPSPs map[string]map[string]map[string]bool } func (t *TestAuthorizer) Authorize(a authorizer.Attributes) (authorized bool, reason string, err error) { - disallowedForUser, _ := t.disallowed[a.GetUser().GetName()] - for _, name := range disallowedForUser { - if a.GetName() == name { - return false, "", nil - } + if t.usernameToNamespaceToAllowedPSPs == nil { + return true, "", nil } - return true, "", nil + allowedInNamespace := t.usernameToNamespaceToAllowedPSPs[a.GetUser().GetName()][a.GetNamespace()][a.GetName()] + allowedClusterWide := t.usernameToNamespaceToAllowedPSPs[a.GetUser().GetName()][""][a.GetName()] + return (allowedInNamespace || allowedClusterWide), "", nil } var _ authorizer.Authorizer = &TestAuthorizer{} @@ -1546,17 +1546,21 @@ func TestGetMatchingPolicies(t *testing.T) { } tests := map[string]struct { - user user.Info - sa user.Info - expectedPolicies sets.String - inPolicies []*extensions.PodSecurityPolicy - disallowedPolicies map[string][]string + user user.Info + sa user.Info + ns string + expectedPolicies sets.String + inPolicies []*extensions.PodSecurityPolicy + allowed map[string]map[string]map[string]bool }{ "policy allowed by user": { user: &user.DefaultInfo{Name: "user"}, sa: &user.DefaultInfo{Name: "sa"}, - disallowedPolicies: map[string][]string{ - "sa": {"policy"}, + ns: "test", + allowed: map[string]map[string]map[string]bool{ + "user": { + "test": {"policy": true}, + }, }, inPolicies: []*extensions.PodSecurityPolicy{policyWithName("policy")}, expectedPolicies: sets.NewString("policy"), @@ -1564,43 +1568,55 @@ func TestGetMatchingPolicies(t *testing.T) { "policy allowed by sa": { user: &user.DefaultInfo{Name: "user"}, sa: &user.DefaultInfo{Name: "sa"}, - disallowedPolicies: map[string][]string{ - "user": {"policy"}, + ns: "test", + allowed: map[string]map[string]map[string]bool{ + "sa": { + "test": {"policy": true}, + }, }, inPolicies: []*extensions.PodSecurityPolicy{policyWithName("policy")}, expectedPolicies: sets.NewString("policy"), }, "no policies allowed": { - user: &user.DefaultInfo{Name: "user"}, - sa: &user.DefaultInfo{Name: "sa"}, - disallowedPolicies: map[string][]string{ - "user": {"policy"}, - "sa": {"policy"}, - }, + user: &user.DefaultInfo{Name: "user"}, + sa: &user.DefaultInfo{Name: "sa"}, + ns: "test", + allowed: map[string]map[string]map[string]bool{}, inPolicies: []*extensions.PodSecurityPolicy{policyWithName("policy")}, expectedPolicies: sets.NewString(), }, "multiple policies allowed": { user: &user.DefaultInfo{Name: "user"}, sa: &user.DefaultInfo{Name: "sa"}, - disallowedPolicies: map[string][]string{ - "user": {"policy1", "policy3"}, - "sa": {"policy2", "policy3"}, + ns: "test", + allowed: map[string]map[string]map[string]bool{ + "sa": { + "test": {"policy1": true}, + "": {"policy4": true}, + "other": {"policy6": true}, + }, + "user": { + "test": {"policy2": true}, + "": {"policy5": true}, + "other": {"policy7": true}, + }, }, inPolicies: []*extensions.PodSecurityPolicy{ policyWithName("policy1"), // allowed by sa policyWithName("policy2"), // allowed by user policyWithName("policy3"), // not allowed + policyWithName("policy4"), // allowed by sa at cluster level + policyWithName("policy5"), // allowed by user at cluster level + policyWithName("policy6"), // not allowed in this namespace + policyWithName("policy7"), // not allowed in this namespace }, - expectedPolicies: sets.NewString("policy1", "policy2"), + expectedPolicies: sets.NewString("policy1", "policy2", "policy4", "policy5"), }, "policies are allowed for nil user info": { - user: nil, - sa: &user.DefaultInfo{Name: "sa"}, - disallowedPolicies: map[string][]string{ - "user": {"policy1", "policy3"}, - "sa": {"policy2", "policy3"}, - }, + user: nil, + sa: &user.DefaultInfo{Name: "sa"}, + ns: "test", + allowed: map[string]map[string]map[string]bool{}, // authorizer not consulted inPolicies: []*extensions.PodSecurityPolicy{ policyWithName("policy1"), policyWithName("policy2"), @@ -1613,9 +1629,11 @@ func TestGetMatchingPolicies(t *testing.T) { "policies are not allowed for nil sa info": { user: &user.DefaultInfo{Name: "user"}, sa: nil, - disallowedPolicies: map[string][]string{ - "user": {"policy1", "policy3"}, - "sa": {"policy2", "policy3"}, + ns: "test", + allowed: map[string]map[string]map[string]bool{ + "user": { + "test": {"policy2": true}, + }, }, inPolicies: []*extensions.PodSecurityPolicy{ policyWithName("policy1"), @@ -1634,8 +1652,8 @@ func TestGetMatchingPolicies(t *testing.T) { store.Add(psp) } - authz := &TestAuthorizer{disallowed: v.disallowedPolicies} - allowedPolicies, err := getMatchingPolicies(pspInformer.Lister(), v.user, v.sa, authz) + authz := &TestAuthorizer{usernameToNamespaceToAllowedPSPs: v.allowed} + allowedPolicies, err := getMatchingPolicies(pspInformer.Lister(), v.user, v.sa, authz, v.ns) if err != nil { t.Errorf("%s got unexpected error %#v", k, err) continue