From 2923d090915ca64361f3cb90e7cb9001fca6435b Mon Sep 17 00:00:00 2001 From: deads2k Date: Mon, 5 Dec 2016 11:28:59 -0500 Subject: [PATCH] remove rbac super user --- cmd/kube-apiserver/app/server.go | 1 - .../cmd/federation-apiserver/app/server.go | 1 - pkg/genericapiserver/authorizer/builtin.go | 1 - pkg/genericapiserver/config.go | 7 ------ pkg/genericapiserver/options/authorization.go | 5 ++-- pkg/master/master.go | 2 +- .../rbac/clusterrole/policybased/storage.go | 11 +++----- .../clusterrolebinding/policybased/storage.go | 11 +++----- pkg/registry/rbac/escalation_check.go | 6 +---- pkg/registry/rbac/rest/storage_rbac.go | 12 ++++----- pkg/registry/rbac/role/policybased/storage.go | 11 +++----- .../rbac/rolebinding/policybased/storage.go | 11 +++----- plugin/pkg/auth/authorizer/rbac/rbac.go | 9 +------ plugin/pkg/auth/authorizer/rbac/rbac_test.go | 4 +-- test/integration/auth/rbac_test.go | 25 ++++++------------- 15 files changed, 35 insertions(+), 82 deletions(-) diff --git a/cmd/kube-apiserver/app/server.go b/cmd/kube-apiserver/app/server.go index 8d34964def..4d4470d9e6 100644 --- a/cmd/kube-apiserver/app/server.go +++ b/cmd/kube-apiserver/app/server.go @@ -100,7 +100,6 @@ func Run(s *options.ServerRunOptions) error { ApplyOptions(s.GenericServerRunOptions). // apply the options selected ApplyInsecureServingOptions(s.InsecureServing). ApplyAuthenticationOptions(s.Authentication). - ApplyRBACSuperUser(s.Authorization.RBACSuperUser). ApplySecureServingOptions(s.SecureServing) if err != nil { return fmt.Errorf("failed to configure https: %s", err) diff --git a/federation/cmd/federation-apiserver/app/server.go b/federation/cmd/federation-apiserver/app/server.go index ee99369965..d05e0a2c29 100644 --- a/federation/cmd/federation-apiserver/app/server.go +++ b/federation/cmd/federation-apiserver/app/server.go @@ -82,7 +82,6 @@ func Run(s *options.ServerRunOptions) error { ApplyOptions(s.GenericServerRunOptions). // apply the options selected ApplyInsecureServingOptions(s.InsecureServing). ApplyAuthenticationOptions(s.Authentication). - ApplyRBACSuperUser(s.Authorization.RBACSuperUser). ApplySecureServingOptions(s.SecureServing) if err != nil { return fmt.Errorf("failed to configure https: %s", err) diff --git a/pkg/genericapiserver/authorizer/builtin.go b/pkg/genericapiserver/authorizer/builtin.go index 86fc05f2b0..63c6db61ad 100644 --- a/pkg/genericapiserver/authorizer/builtin.go +++ b/pkg/genericapiserver/authorizer/builtin.go @@ -173,7 +173,6 @@ func NewAuthorizerFromAuthorizationConfig(config AuthorizationConfig) (authorize config.InformerFactory.RoleBindings().Lister(), config.InformerFactory.ClusterRoles().Lister(), config.InformerFactory.ClusterRoleBindings().Lister(), - config.RBACSuperUser, ) authorizers = append(authorizers, rbacAuthorizer) default: diff --git a/pkg/genericapiserver/config.go b/pkg/genericapiserver/config.go index 06e8f7dbc5..fe3fdcada4 100644 --- a/pkg/genericapiserver/config.go +++ b/pkg/genericapiserver/config.go @@ -100,8 +100,6 @@ type Config struct { SupportsBasicAuth bool Authorizer authorizer.Authorizer AdmissionControl admission.Interface - // TODO(ericchiang): Determine if policy escalation checks should be an admission controller. - AuthorizerRBACSuperUser string // LoopbackClientConfig is a config for a privileged loopback connection to the API server LoopbackClientConfig *restclient.Config @@ -311,11 +309,6 @@ func (c *Config) ApplyAuthenticationOptions(o *options.BuiltInAuthenticationOpti return c } -func (c *Config) ApplyRBACSuperUser(rbacSuperUser string) *Config { - c.AuthorizerRBACSuperUser = rbacSuperUser - return c -} - // ApplyOptions applies the run options to the method receiver and returns self func (c *Config) ApplyOptions(options *options.ServerRunOptions) *Config { if len(options.AuditLogPath) != 0 { diff --git a/pkg/genericapiserver/options/authorization.go b/pkg/genericapiserver/options/authorization.go index 869e511b5c..f43d30b969 100644 --- a/pkg/genericapiserver/options/authorization.go +++ b/pkg/genericapiserver/options/authorization.go @@ -36,7 +36,6 @@ type BuiltInAuthorizationOptions struct { WebhookConfigFile string WebhookCacheAuthorizedTTL time.Duration WebhookCacheUnauthorizedTTL time.Duration - RBACSuperUser string } func NewBuiltInAuthorizationOptions() *BuiltInAuthorizationOptions { @@ -72,9 +71,10 @@ func (s *BuiltInAuthorizationOptions) AddFlags(fs *pflag.FlagSet) { "authorization-webhook-cache-unauthorized-ttl", s.WebhookCacheUnauthorizedTTL, "The duration to cache 'unauthorized' responses from the webhook authorizer. Default is 30s.") - fs.StringVar(&s.RBACSuperUser, "authorization-rbac-super-user", s.RBACSuperUser, ""+ + fs.String("authorization-rbac-super-user", "", ""+ "If specified, a username which avoids RBAC authorization checks and role binding "+ "privilege escalation checks, to be used with --authorization-mode=RBAC.") + fs.MarkDeprecated("authorization-rbac-super-user", "Removed during alpha to beta. The 'system:masters' group has privileged access.") } @@ -90,7 +90,6 @@ func (s *BuiltInAuthorizationOptions) ToAuthorizationConfig(informerFactory info WebhookConfigFile: s.WebhookConfigFile, WebhookCacheAuthorizedTTL: s.WebhookCacheAuthorizedTTL, WebhookCacheUnauthorizedTTL: s.WebhookCacheUnauthorizedTTL, - RBACSuperUser: s.RBACSuperUser, InformerFactory: informerFactory, } } diff --git a/pkg/master/master.go b/pkg/master/master.go index c92a80e32b..1dfa946714 100644 --- a/pkg/master/master.go +++ b/pkg/master/master.go @@ -253,7 +253,7 @@ func (c completedConfig) New() (*Master, error) { certificatesrest.RESTStorageProvider{}, extensionsrest.RESTStorageProvider{ResourceInterface: thirdparty.NewThirdPartyResourceServer(s, c.StorageFactory)}, policyrest.RESTStorageProvider{}, - rbacrest.RESTStorageProvider{AuthorizerRBACSuperUser: c.GenericConfig.AuthorizerRBACSuperUser}, + rbacrest.RESTStorageProvider{}, storagerest.RESTStorageProvider{}, } m.InstallAPIs(c.Config.GenericConfig.APIResourceConfigSource, restOptionsFactory.NewFor, restStorageProviders...) diff --git a/pkg/registry/rbac/clusterrole/policybased/storage.go b/pkg/registry/rbac/clusterrole/policybased/storage.go index a4909a7da0..17cc672269 100644 --- a/pkg/registry/rbac/clusterrole/policybased/storage.go +++ b/pkg/registry/rbac/clusterrole/policybased/storage.go @@ -33,17 +33,14 @@ type Storage struct { rest.StandardStorage ruleResolver validation.AuthorizationRuleResolver - - // user which skips privilege escalation checks - superUser string } -func NewStorage(s rest.StandardStorage, ruleResolver validation.AuthorizationRuleResolver, superUser string) *Storage { - return &Storage{s, ruleResolver, superUser} +func NewStorage(s rest.StandardStorage, ruleResolver validation.AuthorizationRuleResolver) *Storage { + return &Storage{s, ruleResolver} } func (s *Storage) Create(ctx api.Context, obj runtime.Object) (runtime.Object, error) { - if rbacregistry.EscalationAllowed(ctx, s.superUser) { + if rbacregistry.EscalationAllowed(ctx) { return s.StandardStorage.Create(ctx, obj) } @@ -56,7 +53,7 @@ func (s *Storage) Create(ctx api.Context, obj runtime.Object) (runtime.Object, e } func (s *Storage) Update(ctx api.Context, name string, obj rest.UpdatedObjectInfo) (runtime.Object, bool, error) { - if rbacregistry.EscalationAllowed(ctx, s.superUser) { + if rbacregistry.EscalationAllowed(ctx) { return s.StandardStorage.Update(ctx, name, obj) } diff --git a/pkg/registry/rbac/clusterrolebinding/policybased/storage.go b/pkg/registry/rbac/clusterrolebinding/policybased/storage.go index 80b9374ec3..0190a0baaf 100644 --- a/pkg/registry/rbac/clusterrolebinding/policybased/storage.go +++ b/pkg/registry/rbac/clusterrolebinding/policybased/storage.go @@ -33,17 +33,14 @@ type Storage struct { rest.StandardStorage ruleResolver validation.AuthorizationRuleResolver - - // user which skips privilege escalation checks - superUser string } -func NewStorage(s rest.StandardStorage, ruleResolver validation.AuthorizationRuleResolver, superUser string) *Storage { - return &Storage{s, ruleResolver, superUser} +func NewStorage(s rest.StandardStorage, ruleResolver validation.AuthorizationRuleResolver) *Storage { + return &Storage{s, ruleResolver} } func (s *Storage) Create(ctx api.Context, obj runtime.Object) (runtime.Object, error) { - if rbacregistry.EscalationAllowed(ctx, s.superUser) { + if rbacregistry.EscalationAllowed(ctx) { return s.StandardStorage.Create(ctx, obj) } @@ -59,7 +56,7 @@ func (s *Storage) Create(ctx api.Context, obj runtime.Object) (runtime.Object, e } func (s *Storage) Update(ctx api.Context, name string, obj rest.UpdatedObjectInfo) (runtime.Object, bool, error) { - if rbacregistry.EscalationAllowed(ctx, s.superUser) { + if rbacregistry.EscalationAllowed(ctx) { return s.StandardStorage.Update(ctx, name, obj) } diff --git a/pkg/registry/rbac/escalation_check.go b/pkg/registry/rbac/escalation_check.go index c5ba65a1f4..e40edad454 100644 --- a/pkg/registry/rbac/escalation_check.go +++ b/pkg/registry/rbac/escalation_check.go @@ -21,7 +21,7 @@ import ( "k8s.io/kubernetes/pkg/auth/user" ) -func EscalationAllowed(ctx api.Context, superUser string) bool { +func EscalationAllowed(ctx api.Context) bool { u, ok := api.UserFrom(ctx) if !ok { // the only way to be without a user is to either have no authenticators by explicitly saying that's your preference @@ -29,10 +29,6 @@ func EscalationAllowed(ctx api.Context, superUser string) bool { return true } - // check to see if this subject is allowed to escalate - if len(superUser) != 0 && u.GetName() == superUser { - return true - } // system:masters is special because the API server uses it for privileged loopback connections // therefore we know that a member of system:masters can always do anything for _, group := range u.GetGroups() { diff --git a/pkg/registry/rbac/rest/storage_rbac.go b/pkg/registry/rbac/rest/storage_rbac.go index 83eb36a02f..898f0aa262 100644 --- a/pkg/registry/rbac/rest/storage_rbac.go +++ b/pkg/registry/rbac/rest/storage_rbac.go @@ -46,9 +46,7 @@ import ( "k8s.io/kubernetes/plugin/pkg/auth/authorizer/rbac/bootstrappolicy" ) -type RESTStorageProvider struct { - AuthorizerRBACSuperUser string -} +type RESTStorageProvider struct{} var _ genericapiserver.PostStartHookProvider = RESTStorageProvider{} @@ -83,19 +81,19 @@ func (p RESTStorageProvider) v1alpha1Storage(apiResourceConfigSource genericapis storage := map[string]rest.Storage{} if apiResourceConfigSource.ResourceEnabled(version.WithResource("roles")) { rolesStorage := roleetcd.NewREST(restOptionsGetter(rbac.Resource("roles"))) - storage["roles"] = rolepolicybased.NewStorage(rolesStorage, newRuleValidator(), p.AuthorizerRBACSuperUser) + storage["roles"] = rolepolicybased.NewStorage(rolesStorage, newRuleValidator()) } if apiResourceConfigSource.ResourceEnabled(version.WithResource("rolebindings")) { roleBindingsStorage := rolebindingetcd.NewREST(restOptionsGetter(rbac.Resource("rolebindings"))) - storage["rolebindings"] = rolebindingpolicybased.NewStorage(roleBindingsStorage, newRuleValidator(), p.AuthorizerRBACSuperUser) + storage["rolebindings"] = rolebindingpolicybased.NewStorage(roleBindingsStorage, newRuleValidator()) } if apiResourceConfigSource.ResourceEnabled(version.WithResource("clusterroles")) { clusterRolesStorage := clusterroleetcd.NewREST(restOptionsGetter(rbac.Resource("clusterroles"))) - storage["clusterroles"] = clusterrolepolicybased.NewStorage(clusterRolesStorage, newRuleValidator(), p.AuthorizerRBACSuperUser) + storage["clusterroles"] = clusterrolepolicybased.NewStorage(clusterRolesStorage, newRuleValidator()) } if apiResourceConfigSource.ResourceEnabled(version.WithResource("clusterrolebindings")) { clusterRoleBindingsStorage := clusterrolebindingetcd.NewREST(restOptionsGetter(rbac.Resource("clusterrolebindings"))) - storage["clusterrolebindings"] = clusterrolebindingpolicybased.NewStorage(clusterRoleBindingsStorage, newRuleValidator(), p.AuthorizerRBACSuperUser) + storage["clusterrolebindings"] = clusterrolebindingpolicybased.NewStorage(clusterRoleBindingsStorage, newRuleValidator()) } return storage } diff --git a/pkg/registry/rbac/role/policybased/storage.go b/pkg/registry/rbac/role/policybased/storage.go index c7976b9def..23e1f8734b 100644 --- a/pkg/registry/rbac/role/policybased/storage.go +++ b/pkg/registry/rbac/role/policybased/storage.go @@ -33,17 +33,14 @@ type Storage struct { rest.StandardStorage ruleResolver validation.AuthorizationRuleResolver - - // user which skips privilege escalation checks - superUser string } -func NewStorage(s rest.StandardStorage, ruleResolver validation.AuthorizationRuleResolver, superUser string) *Storage { - return &Storage{s, ruleResolver, superUser} +func NewStorage(s rest.StandardStorage, ruleResolver validation.AuthorizationRuleResolver) *Storage { + return &Storage{s, ruleResolver} } func (s *Storage) Create(ctx api.Context, obj runtime.Object) (runtime.Object, error) { - if rbacregistry.EscalationAllowed(ctx, s.superUser) { + if rbacregistry.EscalationAllowed(ctx) { return s.StandardStorage.Create(ctx, obj) } @@ -56,7 +53,7 @@ func (s *Storage) Create(ctx api.Context, obj runtime.Object) (runtime.Object, e } func (s *Storage) Update(ctx api.Context, name string, obj rest.UpdatedObjectInfo) (runtime.Object, bool, error) { - if rbacregistry.EscalationAllowed(ctx, s.superUser) { + if rbacregistry.EscalationAllowed(ctx) { return s.StandardStorage.Update(ctx, name, obj) } diff --git a/pkg/registry/rbac/rolebinding/policybased/storage.go b/pkg/registry/rbac/rolebinding/policybased/storage.go index 9622b6451f..db9261fe74 100644 --- a/pkg/registry/rbac/rolebinding/policybased/storage.go +++ b/pkg/registry/rbac/rolebinding/policybased/storage.go @@ -33,17 +33,14 @@ type Storage struct { rest.StandardStorage ruleResolver validation.AuthorizationRuleResolver - - // user which skips privilege escalation checks - superUser string } -func NewStorage(s rest.StandardStorage, ruleResolver validation.AuthorizationRuleResolver, superUser string) *Storage { - return &Storage{s, ruleResolver, superUser} +func NewStorage(s rest.StandardStorage, ruleResolver validation.AuthorizationRuleResolver) *Storage { + return &Storage{s, ruleResolver} } func (s *Storage) Create(ctx api.Context, obj runtime.Object) (runtime.Object, error) { - if rbacregistry.EscalationAllowed(ctx, s.superUser) { + if rbacregistry.EscalationAllowed(ctx) { return s.StandardStorage.Create(ctx, obj) } @@ -59,7 +56,7 @@ func (s *Storage) Create(ctx api.Context, obj runtime.Object) (runtime.Object, e } func (s *Storage) Update(ctx api.Context, name string, obj rest.UpdatedObjectInfo) (runtime.Object, bool, error) { - if rbacregistry.EscalationAllowed(ctx, s.superUser) { + if rbacregistry.EscalationAllowed(ctx) { return s.StandardStorage.Update(ctx, name, obj) } diff --git a/plugin/pkg/auth/authorizer/rbac/rbac.go b/plugin/pkg/auth/authorizer/rbac/rbac.go index 04fdf45752..64ad3707b5 100644 --- a/plugin/pkg/auth/authorizer/rbac/rbac.go +++ b/plugin/pkg/auth/authorizer/rbac/rbac.go @@ -33,16 +33,10 @@ type RequestToRuleMapper interface { } type RBACAuthorizer struct { - superUser string - authorizationRuleResolver RequestToRuleMapper } func (r *RBACAuthorizer) Authorize(requestAttributes authorizer.Attributes) (bool, string, error) { - if r.superUser != "" && requestAttributes.GetUser() != nil && requestAttributes.GetUser().GetName() == r.superUser { - return true, "", nil - } - rules, ruleResolutionError := r.authorizationRuleResolver.RulesFor(requestAttributes.GetUser(), requestAttributes.GetNamespace()) if RulesAllow(requestAttributes, rules...) { return true, "", nil @@ -51,9 +45,8 @@ func (r *RBACAuthorizer) Authorize(requestAttributes authorizer.Attributes) (boo return false, "", ruleResolutionError } -func New(roles validation.RoleGetter, roleBindings validation.RoleBindingLister, clusterRoles validation.ClusterRoleGetter, clusterRoleBindings validation.ClusterRoleBindingLister, superUser string) *RBACAuthorizer { +func New(roles validation.RoleGetter, roleBindings validation.RoleBindingLister, clusterRoles validation.ClusterRoleGetter, clusterRoleBindings validation.ClusterRoleBindingLister) *RBACAuthorizer { authorizer := &RBACAuthorizer{ - superUser: superUser, authorizationRuleResolver: validation.NewDefaultRuleResolver( roles, roleBindings, clusterRoles, clusterRoleBindings, ), diff --git a/plugin/pkg/auth/authorizer/rbac/rbac_test.go b/plugin/pkg/auth/authorizer/rbac/rbac_test.go index a0721e562a..f781cea3ae 100644 --- a/plugin/pkg/auth/authorizer/rbac/rbac_test.go +++ b/plugin/pkg/auth/authorizer/rbac/rbac_test.go @@ -122,8 +122,6 @@ func TestAuthorizer(t *testing.T) { clusterRoles []*rbac.ClusterRole clusterRoleBindings []*rbac.ClusterRoleBinding - superUser string - shouldPass []authorizer.Attributes shouldFail []authorizer.Attributes }{ @@ -222,7 +220,7 @@ func TestAuthorizer(t *testing.T) { } for i, tt := range tests { ruleResolver, _ := validation.NewTestRuleResolver(tt.roles, tt.roleBindings, tt.clusterRoles, tt.clusterRoleBindings) - a := RBACAuthorizer{tt.superUser, ruleResolver} + a := RBACAuthorizer{ruleResolver} for _, attr := range tt.shouldPass { if authorized, _, _ := a.Authorize(attr); !authorized { t.Errorf("case %d: incorrectly restricted %s", i, attr) diff --git a/test/integration/auth/rbac_test.go b/test/integration/auth/rbac_test.go index 750dcc4337..91e49fc290 100644 --- a/test/integration/auth/rbac_test.go +++ b/test/integration/auth/rbac_test.go @@ -19,7 +19,6 @@ limitations under the License. package auth import ( - "errors" "fmt" "io" "io/ioutil" @@ -38,7 +37,6 @@ import ( "k8s.io/kubernetes/pkg/auth/authenticator" "k8s.io/kubernetes/pkg/auth/authenticator/bearertoken" "k8s.io/kubernetes/pkg/auth/authorizer" - "k8s.io/kubernetes/pkg/auth/user" clientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset" "k8s.io/kubernetes/pkg/client/restclient" "k8s.io/kubernetes/pkg/client/transport" @@ -53,18 +51,13 @@ import ( "k8s.io/kubernetes/pkg/registry/rbac/rolebinding" rolebindingetcd "k8s.io/kubernetes/pkg/registry/rbac/rolebinding/etcd" "k8s.io/kubernetes/pkg/watch" + "k8s.io/kubernetes/plugin/pkg/auth/authenticator/token/anytoken" "k8s.io/kubernetes/plugin/pkg/auth/authorizer/rbac" "k8s.io/kubernetes/test/integration/framework" ) func newFakeAuthenticator() authenticator.Request { - return bearertoken.New(authenticator.TokenFunc(func(token string) (user.Info, bool, error) { - if token == "" { - return nil, false, errors.New("no bearer token found") - } - // Set the bearer token as the user name. - return &user.DefaultInfo{Name: token, UID: token}, true, nil - })) + return bearertoken.New(anytoken.AnyTokenAuthenticator{}) } func clientForUser(user string) *http.Client { @@ -82,7 +75,7 @@ func clientsetForUser(user string, config *restclient.Config) clientset.Interfac return clientset.NewForConfigOrDie(&configCopy) } -func newRBACAuthorizer(t *testing.T, superUser string, config *master.Config) authorizer.Authorizer { +func newRBACAuthorizer(t *testing.T, config *master.Config) authorizer.Authorizer { newRESTOptions := func(resource string) generic.RESTOptions { storageConfig, err := config.StorageFactory.NewConfig(rbacapi.Resource(resource)) if err != nil { @@ -95,7 +88,7 @@ func newRBACAuthorizer(t *testing.T, superUser string, config *master.Config) au roleBindingRegistry := rolebinding.AuthorizerAdapter{Registry: rolebinding.NewRegistry(rolebindingetcd.NewREST(newRESTOptions("rolebindings")))} clusterRoleRegistry := clusterrole.AuthorizerAdapter{Registry: clusterrole.NewRegistry(clusterroleetcd.NewREST(newRESTOptions("clusterroles")))} clusterRoleBindingRegistry := clusterrolebinding.AuthorizerAdapter{Registry: clusterrolebinding.NewRegistry(clusterrolebindingetcd.NewREST(newRESTOptions("clusterrolebindings")))} - return rbac.New(roleRegistry, roleBindingRegistry, clusterRoleRegistry, clusterRoleBindingRegistry, superUser) + return rbac.New(roleRegistry, roleBindingRegistry, clusterRoleRegistry, clusterRoleBindingRegistry) } // bootstrapRoles are a set of RBAC roles which will be populated before the test. @@ -240,7 +233,7 @@ var ( ) func TestRBAC(t *testing.T) { - superUser := "admin" + superUser := "admin/system:masters" tests := []struct { bootstrapRoles bootstrapRoles @@ -339,9 +332,8 @@ func TestRBAC(t *testing.T) { for i, tc := range tests { // Create an API Server. masterConfig := framework.NewIntegrationTestMasterConfig() - masterConfig.GenericConfig.Authorizer = newRBACAuthorizer(t, superUser, masterConfig) + masterConfig.GenericConfig.Authorizer = newRBACAuthorizer(t, masterConfig) masterConfig.GenericConfig.Authenticator = newFakeAuthenticator() - masterConfig.GenericConfig.AuthorizerRBACSuperUser = superUser _, s := framework.RunAMaster(masterConfig) defer s.Close() @@ -435,12 +427,11 @@ func TestRBAC(t *testing.T) { } func TestBootstrapping(t *testing.T) { - superUser := "admin" + superUser := "admin/system:masters" masterConfig := framework.NewIntegrationTestMasterConfig() - masterConfig.GenericConfig.Authorizer = newRBACAuthorizer(t, superUser, masterConfig) + masterConfig.GenericConfig.Authorizer = newRBACAuthorizer(t, masterConfig) masterConfig.GenericConfig.Authenticator = newFakeAuthenticator() - masterConfig.GenericConfig.AuthorizerRBACSuperUser = superUser _, s := framework.RunAMaster(masterConfig) defer s.Close()