Merge pull request #42360 from liggitt/psp-namespaced-use-check

Automatic merge from submit-queue (batch tested with PRs 42360, 43109, 43737, 43853)

Include pod namespace in PSP 'use' authorization check

Follow up to https://github.com/kubernetes/kubernetes/pull/33080/files#diff-291b8dd7d08cc034975ddb3925dbb08fR341

Prior to this PR, when PodSecurityPolicy admission is active, you must be authorized to use a covering PodSecurityPolicy cluster-wide in order to create a pod. This PR changes that to only require a covering PodSecurityPolicy within the pod's namespace.

When used in concert with mechanisms that limits pods within a namespace to a particular set of nodes, this can be used to allow users to create privileged pods within specific namespaces only.

```release-note
Permission to use a PodSecurityPolicy can now be granted within a single namespace by allowing the `use` verb on the `podsecuritypolicies` resource within the namespace.
```

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",

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