Merge pull request #39383 from liggitt/bind-check

Automatic merge from submit-queue (batch tested with PRs 39694, 39383, 39651, 39691, 39497)

Allow rolebinding/clusterrolebinding with explicit bind permission check

Fixes https://github.com/kubernetes/kubernetes/issues/39176
Fixes https://github.com/kubernetes/kubernetes/issues/39258

Allows creating/updating a rolebinding/clusterrolebinding if the user has explicitly been granted permission to perform the "bind" verb against the referenced role/clusterrole (previously, they could only bind if they already had all the permissions in the referenced role via an RBAC role themselves)

```release-note
To create or update an RBAC RoleBinding or ClusterRoleBinding object, a user must:
1. Be authorized to make the create or update API request
2. Be allowed to bind the referenced role, either by already having all of the permissions contained in the referenced role, or by having the "bind" permission on the referenced role.
```

cluster/addons/e2e-rbac-bindings/e2e-user-binding.yaml

@@ -1,6 +1,6 @@
 # This is the main user for the e2e tests.  This is ok to leave long term
 # since the first user in the test can reasonably be high power
-# its kubecfg in gce and kubekins in gke
+# its kubecfg in gce
 # TODO consider provisioning each test its namespace and giving it an
 # admin user.  This still has to exist, but e2e wouldn't normally use it
 apiVersion: rbac.authorization.k8s.io/v1alpha1
@@ -17,6 +17,3 @@ subjects:
 - apiVersion: rbac/v1alpha1
   kind: User
   name: kubecfg
-- apiVersion: rbac/v1alpha1
-  kind: User
-  name: kubekins@kubernetes-jenkins.iam.gserviceaccount.com

pkg/master/master.go

@@ -252,7 +252,7 @@ func (c completedConfig) New() (*Master, error) {
 		certificatesrest.RESTStorageProvider{},
 		extensionsrest.RESTStorageProvider{ResourceInterface: thirdparty.NewThirdPartyResourceServer(s, c.StorageFactory)},
 		policyrest.RESTStorageProvider{},
-		rbacrest.RESTStorageProvider{},
+		rbacrest.RESTStorageProvider{Authorizer: c.GenericConfig.Authorizer},
 		storagerest.RESTStorageProvider{},
 	}
 	m.InstallAPIs(c.Config.APIResourceConfigSource, restOptionsFactory, restStorageProviders...)

pkg/registry/rbac/BUILD

@@ -12,8 +12,11 @@ go_library(
     srcs = ["escalation_check.go"],
     tags = ["automanaged"],
     deps = [
+        "//pkg/apis/rbac:go_default_library",
         "//pkg/genericapiserver/api/request:go_default_library",
+        "//pkg/util/runtime:go_default_library",
         "//vendor:k8s.io/apiserver/pkg/authentication/user",
+        "//vendor:k8s.io/apiserver/pkg/authorization/authorizer",
     ],
 )
 

pkg/registry/rbac/clusterrolebinding/policybased/BUILD

@@ -19,6 +19,7 @@ go_library(
         "//pkg/genericapiserver/api/request:go_default_library",
         "//pkg/registry/rbac:go_default_library",
         "//pkg/runtime:go_default_library",
+        "//vendor:k8s.io/apiserver/pkg/authorization/authorizer",
     ],
 )
 

pkg/registry/rbac/clusterrolebinding/policybased/storage.go

@@ -18,6 +18,7 @@ limitations under the License.
 package policybased
 
 import (
+	"k8s.io/apiserver/pkg/authorization/authorizer"
 	"k8s.io/kubernetes/pkg/api/errors"
 	"k8s.io/kubernetes/pkg/api/rest"
 	"k8s.io/kubernetes/pkg/apis/rbac"
@@ -32,11 +33,13 @@ var groupResource = rbac.Resource("clusterrolebindings")
 type Storage struct {
 	rest.StandardStorage
 
+	authorizer authorizer.Authorizer
+
 	ruleResolver validation.AuthorizationRuleResolver
 }
 
-func NewStorage(s rest.StandardStorage, ruleResolver validation.AuthorizationRuleResolver) *Storage {
-	return &Storage{s, ruleResolver}
+func NewStorage(s rest.StandardStorage, authorizer authorizer.Authorizer, ruleResolver validation.AuthorizationRuleResolver) *Storage {
+	return &Storage{s, authorizer, ruleResolver}
 }
 
 func (s *Storage) Create(ctx genericapirequest.Context, obj runtime.Object) (runtime.Object, error) {
@@ -45,6 +48,10 @@ func (s *Storage) Create(ctx genericapirequest.Context, obj runtime.Object) (run
 	}
 
 	clusterRoleBinding := obj.(*rbac.ClusterRoleBinding)
+	if rbacregistry.BindingAuthorized(ctx, clusterRoleBinding.RoleRef, clusterRoleBinding.Namespace, s.authorizer) {
+		return s.StandardStorage.Create(ctx, obj)
+	}
+
 	rules, err := s.ruleResolver.GetRoleReferenceRules(clusterRoleBinding.RoleRef, clusterRoleBinding.Namespace)
 	if err != nil {
 		return nil, err
@@ -63,6 +70,12 @@ func (s *Storage) Update(ctx genericapirequest.Context, name string, obj rest.Up
 	nonEscalatingInfo := rest.WrapUpdatedObjectInfo(obj, func(ctx genericapirequest.Context, obj runtime.Object, oldObj runtime.Object) (runtime.Object, error) {
 		clusterRoleBinding := obj.(*rbac.ClusterRoleBinding)
 
+		// if we're explicitly authorized to bind this clusterrole, return
+		if rbacregistry.BindingAuthorized(ctx, clusterRoleBinding.RoleRef, clusterRoleBinding.Namespace, s.authorizer) {
+			return obj, nil
+		}
+
+		// Otherwise, see if we already have all the permissions contained in the referenced clusterrole
 		rules, err := s.ruleResolver.GetRoleReferenceRules(clusterRoleBinding.RoleRef, clusterRoleBinding.Namespace)
 		if err != nil {
 			return nil, err

pkg/registry/rbac/escalation_check.go

@@ -17,8 +17,13 @@ limitations under the License.
 package rbac
 
 import (
+	"fmt"
+
 	"k8s.io/apiserver/pkg/authentication/user"
+	"k8s.io/apiserver/pkg/authorization/authorizer"
+	"k8s.io/kubernetes/pkg/apis/rbac"
 	genericapirequest "k8s.io/kubernetes/pkg/genericapiserver/api/request"
+	utilruntime "k8s.io/kubernetes/pkg/util/runtime"
 )
 
 func EscalationAllowed(ctx genericapirequest.Context) bool {
@@ -39,3 +44,49 @@ func EscalationAllowed(ctx genericapirequest.Context) bool {
 
 	return false
 }
+
+// BindingAuthorized returns true if the user associated with the context is explicitly authorized to bind the specified roleRef
+func BindingAuthorized(ctx genericapirequest.Context, roleRef rbac.RoleRef, bindingNamespace string, a authorizer.Authorizer) bool {
+	if a == nil {
+		return false
+	}
+
+	user, ok := genericapirequest.UserFrom(ctx)
+	if !ok {
+		return false
+	}
+
+	attrs := authorizer.AttributesRecord{
+		User: user,
+		Verb: "bind",
+		// check against the namespace where the binding is being created (or the empty namespace for clusterrolebindings).
+		// this allows delegation to bind particular clusterroles in rolebindings within particular namespaces,
+		// and to authorize binding a clusterrole across all namespaces in a clusterrolebinding.
+		Namespace:       bindingNamespace,
+		ResourceRequest: true,
+	}
+
+	// This occurs after defaulting and conversion, so values pulled from the roleRef won't change
+	// Invalid APIGroup or Name values will fail validation
+	switch roleRef.Kind {
+	case "ClusterRole":
+		attrs.APIGroup = roleRef.APIGroup
+		attrs.Resource = "clusterroles"
+		attrs.Name = roleRef.Name
+	case "Role":
+		attrs.APIGroup = roleRef.APIGroup
+		attrs.Resource = "roles"
+		attrs.Name = roleRef.Name
+	default:
+		return false
+	}
+
+	ok, _, err := a.Authorize(attrs)
+	if err != nil {
+		utilruntime.HandleError(fmt.Errorf(
+			"error authorizing user %#v to bind %#v in namespace %s: %v",
+			roleRef, bindingNamespace, user, err,
+		))
+	}
+	return ok
+}

pkg/registry/rbac/rest/BUILD

@@ -36,6 +36,7 @@ go_library(
         "//pkg/util/wait:go_default_library",
         "//plugin/pkg/auth/authorizer/rbac/bootstrappolicy:go_default_library",
         "//vendor:github.com/golang/glog",
+        "//vendor:k8s.io/apiserver/pkg/authorization/authorizer",
     ],
 )
 

pkg/registry/rbac/rest/storage_rbac.go

@@ -23,6 +23,7 @@ import (
 
 	"github.com/golang/glog"
 
+	"k8s.io/apiserver/pkg/authorization/authorizer"
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/api/rest"
 	"k8s.io/kubernetes/pkg/apis/rbac"
@@ -48,7 +49,9 @@ import (
 	"k8s.io/kubernetes/plugin/pkg/auth/authorizer/rbac/bootstrappolicy"
 )
 
-type RESTStorageProvider struct{}
+type RESTStorageProvider struct {
+	Authorizer authorizer.Authorizer
+}
 
 var _ genericapiserver.PostStartHookProvider = RESTStorageProvider{}
 
@@ -98,7 +101,7 @@ func (p RESTStorageProvider) v1alpha1Storage(apiResourceConfigSource genericapis
 	}
 	if apiResourceConfigSource.ResourceEnabled(version.WithResource("rolebindings")) {
 		initializeStorage()
-		storage["rolebindings"] = rolebindingpolicybased.NewStorage(roleBindingsStorage, authorizationRuleResolver)
+		storage["rolebindings"] = rolebindingpolicybased.NewStorage(roleBindingsStorage, p.Authorizer, authorizationRuleResolver)
 	}
 	if apiResourceConfigSource.ResourceEnabled(version.WithResource("clusterroles")) {
 		initializeStorage()
@@ -106,7 +109,7 @@ func (p RESTStorageProvider) v1alpha1Storage(apiResourceConfigSource genericapis
 	}
 	if apiResourceConfigSource.ResourceEnabled(version.WithResource("clusterrolebindings")) {
 		initializeStorage()
-		storage["clusterrolebindings"] = clusterrolebindingpolicybased.NewStorage(clusterRoleBindingsStorage, authorizationRuleResolver)
+		storage["clusterrolebindings"] = clusterrolebindingpolicybased.NewStorage(clusterRoleBindingsStorage, p.Authorizer, authorizationRuleResolver)
 	}
 	return storage
 }

pkg/registry/rbac/rolebinding/policybased/BUILD

@@ -19,6 +19,7 @@ go_library(
         "//pkg/genericapiserver/api/request:go_default_library",
         "//pkg/registry/rbac:go_default_library",
         "//pkg/runtime:go_default_library",
+        "//vendor:k8s.io/apiserver/pkg/authorization/authorizer",
     ],
 )
 

pkg/registry/rbac/rolebinding/policybased/storage.go

@@ -18,6 +18,7 @@ limitations under the License.
 package policybased
 
 import (
+	"k8s.io/apiserver/pkg/authorization/authorizer"
 	"k8s.io/kubernetes/pkg/api/errors"
 	"k8s.io/kubernetes/pkg/api/rest"
 	"k8s.io/kubernetes/pkg/apis/rbac"
@@ -32,11 +33,13 @@ var groupResource = rbac.Resource("rolebindings")
 type Storage struct {
 	rest.StandardStorage
 
+	authorizer authorizer.Authorizer
+
 	ruleResolver validation.AuthorizationRuleResolver
 }
 
-func NewStorage(s rest.StandardStorage, ruleResolver validation.AuthorizationRuleResolver) *Storage {
-	return &Storage{s, ruleResolver}
+func NewStorage(s rest.StandardStorage, authorizer authorizer.Authorizer, ruleResolver validation.AuthorizationRuleResolver) *Storage {
+	return &Storage{s, authorizer, ruleResolver}
 }
 
 func (s *Storage) Create(ctx genericapirequest.Context, obj runtime.Object) (runtime.Object, error) {
@@ -45,6 +48,10 @@ func (s *Storage) Create(ctx genericapirequest.Context, obj runtime.Object) (run
 	}
 
 	roleBinding := obj.(*rbac.RoleBinding)
+	if rbacregistry.BindingAuthorized(ctx, roleBinding.RoleRef, roleBinding.Namespace, s.authorizer) {
+		return s.StandardStorage.Create(ctx, obj)
+	}
+
 	rules, err := s.ruleResolver.GetRoleReferenceRules(roleBinding.RoleRef, roleBinding.Namespace)
 	if err != nil {
 		return nil, err
@@ -63,6 +70,12 @@ func (s *Storage) Update(ctx genericapirequest.Context, name string, obj rest.Up
 	nonEscalatingInfo := rest.WrapUpdatedObjectInfo(obj, func(ctx genericapirequest.Context, obj runtime.Object, oldObj runtime.Object) (runtime.Object, error) {
 		roleBinding := obj.(*rbac.RoleBinding)
 
+		// if we're explicitly authorized to bind this role, return
+		if rbacregistry.BindingAuthorized(ctx, roleBinding.RoleRef, roleBinding.Namespace, s.authorizer) {
+			return obj, nil
+		}
+
+		// Otherwise, see if we already have all the permissions contained in the referenced role
 		rules, err := s.ruleResolver.GetRoleReferenceRules(roleBinding.RoleRef, roleBinding.Namespace)
 		if err != nil {
 			return nil, err

test/integration/auth/rbac_test.go

@@ -168,6 +168,25 @@ func (s statusCode) String() string {
 
 // Declare a set of raw objects to use.
 var (
+	writeJobsRoleBinding = `
+{
+  "apiVersion": "rbac.authorization.k8s.io/v1alpha1",
+  "kind": "RoleBinding",
+  "metadata": {
+    "name": "pi"%s
+  },
+  "roleRef": {
+    "apiGroup": "rbac.authorization.k8s.io",
+    "kind": "ClusterRole",
+    "name": "write-jobs"
+  },
+  "subjects": [{
+    "apiVersion": "rbac/v1alpha1",
+    "kind": "User",
+    "name": "admin"
+  }]
+}`
+
 	aJob = `
 {
   "apiVersion": "batch/v1",
@@ -291,6 +310,18 @@ func TestRBAC(t *testing.T) {
 						ObjectMeta: api.ObjectMeta{Name: "write-jobs"},
 						Rules:      []rbacapi.PolicyRule{ruleWriteJobs},
 					},
+					{
+						ObjectMeta: api.ObjectMeta{Name: "create-rolebindings"},
+						Rules: []rbacapi.PolicyRule{
+							rbacapi.NewRule("create").Groups("rbac.authorization.k8s.io").Resources("rolebindings").RuleOrDie(),
+						},
+					},
+					{
+						ObjectMeta: api.ObjectMeta{Name: "bind-any-clusterrole"},
+						Rules: []rbacapi.PolicyRule{
+							rbacapi.NewRule("bind").Groups("rbac.authorization.k8s.io").Resources("clusterroles").RuleOrDie(),
+						},
+					},
 				},
 				clusterRoleBindings: []rbacapi.ClusterRoleBinding{
 					{
@@ -298,6 +329,20 @@ func TestRBAC(t *testing.T) {
 						Subjects:   []rbacapi.Subject{{Kind: "User", Name: "job-writer"}},
 						RoleRef:    rbacapi.RoleRef{Kind: "ClusterRole", Name: "write-jobs"},
 					},
+					{
+						ObjectMeta: api.ObjectMeta{Name: "create-rolebindings"},
+						Subjects: []rbacapi.Subject{
+							{Kind: "User", Name: "job-writer"},
+							{Kind: "User", Name: "nonescalating-rolebinding-writer"},
+							{Kind: "User", Name: "any-rolebinding-writer"},
+						},
+						RoleRef: rbacapi.RoleRef{Kind: "ClusterRole", Name: "create-rolebindings"},
+					},
+					{
+						ObjectMeta: api.ObjectMeta{Name: "bind-any-clusterrole"},
+						Subjects:   []rbacapi.Subject{{Kind: "User", Name: "any-rolebinding-writer"}},
+						RoleRef:    rbacapi.RoleRef{Kind: "ClusterRole", Name: "bind-any-clusterrole"},
+					},
 				},
 				roleBindings: []rbacapi.RoleBinding{
 					{
@@ -305,6 +350,11 @@ func TestRBAC(t *testing.T) {
 						Subjects:   []rbacapi.Subject{{Kind: "User", Name: "job-writer-namespace"}},
 						RoleRef:    rbacapi.RoleRef{Kind: "ClusterRole", Name: "write-jobs"},
 					},
+					{
+						ObjectMeta: api.ObjectMeta{Name: "create-rolebindings", Namespace: "job-namespace"},
+						Subjects:   []rbacapi.Subject{{Kind: "User", Name: "job-writer-namespace"}},
+						RoleRef:    rbacapi.RoleRef{Kind: "ClusterRole", Name: "create-rolebindings"},
+					},
 				},
 			},
 			requests: []request{
@@ -331,6 +381,21 @@ func TestRBAC(t *testing.T) {
 				{"job-writer-namespace", "GET", "batch", "jobs", "job-namespace", "pi", "", http.StatusNotFound},
 				{"job-writer-namespace", "POST", "batch", "jobs", "job-namespace", "", aJob, http.StatusCreated},
 				{"job-writer-namespace", "GET", "batch", "jobs", "job-namespace", "pi", "", http.StatusOK},
+
+				// cannot bind role anywhere
+				{"user-with-no-permissions", "POST", "rbac.authorization.k8s.io", "rolebindings", "job-namespace", "", writeJobsRoleBinding, http.StatusForbidden},
+				// can only bind role in namespace where they have covering permissions
+				{"job-writer-namespace", "POST", "rbac.authorization.k8s.io", "rolebindings", "forbidden-namespace", "", writeJobsRoleBinding, http.StatusForbidden},
+				{"job-writer-namespace", "POST", "rbac.authorization.k8s.io", "rolebindings", "job-namespace", "", writeJobsRoleBinding, http.StatusCreated},
+				{superUser, "DELETE", "rbac.authorization.k8s.io", "rolebindings", "job-namespace", "pi", "", http.StatusOK},
+				// can bind role in any namespace where they have covering permissions
+				{"job-writer", "POST", "rbac.authorization.k8s.io", "rolebindings", "forbidden-namespace", "", writeJobsRoleBinding, http.StatusCreated},
+				{superUser, "DELETE", "rbac.authorization.k8s.io", "rolebindings", "forbidden-namespace", "pi", "", http.StatusOK},
+				// cannot bind role because they don't have covering permissions
+				{"nonescalating-rolebinding-writer", "POST", "rbac.authorization.k8s.io", "rolebindings", "job-namespace", "", writeJobsRoleBinding, http.StatusForbidden},
+				// can bind role because they have explicit bind permission
+				{"any-rolebinding-writer", "POST", "rbac.authorization.k8s.io", "rolebindings", "job-namespace", "", writeJobsRoleBinding, http.StatusCreated},
+				{superUser, "DELETE", "rbac.authorization.k8s.io", "rolebindings", "job-namespace", "pi", "", http.StatusOK},
 			},
 		},
 	}