From a2670d3b9dd1d85f81415037020122fb9254229e Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Mon, 9 Jan 2017 09:36:17 -0500 Subject: [PATCH] Allow rolebinding/clusterrolebinding with explicit bind permission check --- pkg/master/master.go | 2 +- pkg/registry/rbac/BUILD | 3 + .../rbac/clusterrolebinding/policybased/BUILD | 1 + .../clusterrolebinding/policybased/storage.go | 17 ++++- pkg/registry/rbac/escalation_check.go | 51 +++++++++++++++ pkg/registry/rbac/rest/BUILD | 1 + pkg/registry/rbac/rest/storage_rbac.go | 9 ++- .../rbac/rolebinding/policybased/BUILD | 1 + .../rbac/rolebinding/policybased/storage.go | 17 ++++- test/integration/auth/rbac_test.go | 65 +++++++++++++++++++ 10 files changed, 159 insertions(+), 8 deletions(-) diff --git a/pkg/master/master.go b/pkg/master/master.go index 6ea364d016..c232363550 100644 --- a/pkg/master/master.go +++ b/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...) diff --git a/pkg/registry/rbac/BUILD b/pkg/registry/rbac/BUILD index 8d597c6146..8367642c30 100644 --- a/pkg/registry/rbac/BUILD +++ b/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", ], ) diff --git a/pkg/registry/rbac/clusterrolebinding/policybased/BUILD b/pkg/registry/rbac/clusterrolebinding/policybased/BUILD index 42cc90df16..6ee5ca3efe 100644 --- a/pkg/registry/rbac/clusterrolebinding/policybased/BUILD +++ b/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", ], ) diff --git a/pkg/registry/rbac/clusterrolebinding/policybased/storage.go b/pkg/registry/rbac/clusterrolebinding/policybased/storage.go index 9b2f2b6809..3413907d70 100644 --- a/pkg/registry/rbac/clusterrolebinding/policybased/storage.go +++ b/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 diff --git a/pkg/registry/rbac/escalation_check.go b/pkg/registry/rbac/escalation_check.go index 7b0657a277..5e07568ec0 100644 --- a/pkg/registry/rbac/escalation_check.go +++ b/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 +} diff --git a/pkg/registry/rbac/rest/BUILD b/pkg/registry/rbac/rest/BUILD index 218c23f57f..c57f686cb5 100644 --- a/pkg/registry/rbac/rest/BUILD +++ b/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", ], ) diff --git a/pkg/registry/rbac/rest/storage_rbac.go b/pkg/registry/rbac/rest/storage_rbac.go index c318ad5e2e..cc6f71bb85 100644 --- a/pkg/registry/rbac/rest/storage_rbac.go +++ b/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 } diff --git a/pkg/registry/rbac/rolebinding/policybased/BUILD b/pkg/registry/rbac/rolebinding/policybased/BUILD index 42cc90df16..6ee5ca3efe 100644 --- a/pkg/registry/rbac/rolebinding/policybased/BUILD +++ b/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", ], ) diff --git a/pkg/registry/rbac/rolebinding/policybased/storage.go b/pkg/registry/rbac/rolebinding/policybased/storage.go index bb9e971e48..4f86b543fd 100644 --- a/pkg/registry/rbac/rolebinding/policybased/storage.go +++ b/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 diff --git a/test/integration/auth/rbac_test.go b/test/integration/auth/rbac_test.go index 4fef04d254..a068d261d5 100644 --- a/test/integration/auth/rbac_test.go +++ b/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}, }, }, }