diff --git a/cmd/kube-apiserver/app/options/options_test.go b/cmd/kube-apiserver/app/options/options_test.go index f90c333a0a5..fda8e294947 100644 --- a/cmd/kube-apiserver/app/options/options_test.go +++ b/cmd/kube-apiserver/app/options/options_test.go @@ -73,7 +73,7 @@ func TestAddFlags(t *testing.T) { "--audit-webhook-initial-backoff=2s", "--authentication-token-webhook-cache-ttl=3m", "--authentication-token-webhook-config-file=/token-webhook-config", - "--authorization-mode=AlwaysDeny", + "--authorization-mode=AlwaysDeny,RBAC", "--authorization-policy-file=/policy", "--authorization-webhook-cache-authorized-ttl=3m", "--authorization-webhook-cache-unauthorized-ttl=1m", @@ -248,7 +248,7 @@ func TestAddFlags(t *testing.T) { TokenFailureCacheTTL: 0, }, Authorization: &kubeoptions.BuiltInAuthorizationOptions{ - Mode: "AlwaysDeny", + Modes: []string{"AlwaysDeny", "RBAC"}, PolicyFile: "/policy", WebhookConfigFile: "/webhook-config", WebhookCacheAuthorizedTTL: 180000000000, diff --git a/cmd/kube-apiserver/app/options/validation.go b/cmd/kube-apiserver/app/options/validation.go index 79d13bb0887..b69cd84ea4f 100644 --- a/cmd/kube-apiserver/app/options/validation.go +++ b/cmd/kube-apiserver/app/options/validation.go @@ -67,6 +67,9 @@ func (s *ServerRunOptions) Validate() []error { if errs := s.Authentication.Validate(); len(errs) > 0 { errors = append(errors, errs...) } + if errs := s.Authorization.Validate(); len(errs) > 0 { + errors = append(errors, errs...) + } if errs := s.Audit.Validate(); len(errs) > 0 { errors = append(errors, errs...) } diff --git a/cmd/kube-apiserver/app/server.go b/cmd/kube-apiserver/app/server.go index 3b51fc6fa42..b9d80dfe5be 100644 --- a/cmd/kube-apiserver/app/server.go +++ b/cmd/kube-apiserver/app/server.go @@ -485,7 +485,7 @@ func BuildGenericConfig(s *options.ServerRunOptions, proxyTransport *http.Transp if err != nil { return nil, nil, nil, nil, nil, fmt.Errorf("invalid authorization config: %v", err) } - if !sets.NewString(s.Authorization.Modes()...).Has(modes.ModeRBAC) { + if !sets.NewString(s.Authorization.Modes...).Has(modes.ModeRBAC) { genericConfig.DisabledPostStartHooks.Insert(rbacrest.PostStartHookName) } diff --git a/pkg/kubeapiserver/authorizer/BUILD b/pkg/kubeapiserver/authorizer/BUILD index 59bd54a40cf..de78b616f8b 100644 --- a/pkg/kubeapiserver/authorizer/BUILD +++ b/pkg/kubeapiserver/authorizer/BUILD @@ -3,17 +3,6 @@ package(default_visibility = ["//visibility:public"]) load( "@io_bazel_rules_go//go:def.bzl", "go_library", - "go_test", -) - -go_test( - name = "go_default_test", - srcs = ["config_test.go"], - data = [ - "//pkg/auth/authorizer/abac:example_policy", - ], - embed = [":go_default_library"], - deps = ["//pkg/kubeapiserver/authorizer/modes:go_default_library"], ) go_library( diff --git a/pkg/kubeapiserver/authorizer/config.go b/pkg/kubeapiserver/authorizer/config.go index 30661bc14fc..35e61489cf6 100644 --- a/pkg/kubeapiserver/authorizer/config.go +++ b/pkg/kubeapiserver/authorizer/config.go @@ -17,7 +17,6 @@ limitations under the License. package authorizer import ( - "errors" "fmt" "time" @@ -60,20 +59,15 @@ type AuthorizationConfig struct { // based on the authorizationMode or an error. func (config AuthorizationConfig) New() (authorizer.Authorizer, authorizer.RuleResolver, error) { if len(config.AuthorizationModes) == 0 { - return nil, nil, errors.New("At least one authorization mode should be passed") + return nil, nil, fmt.Errorf("at least one authorization mode must be passed") } var ( authorizers []authorizer.Authorizer ruleResolvers []authorizer.RuleResolver ) - authorizerMap := make(map[string]bool) for _, authorizationMode := range config.AuthorizationModes { - if authorizerMap[authorizationMode] { - return nil, nil, fmt.Errorf("Authorization mode %s specified more than once", authorizationMode) - } - // Keep cases in sync with constant list above. switch authorizationMode { case modes.ModeNode: @@ -96,9 +90,6 @@ func (config AuthorizationConfig) New() (authorizer.Authorizer, authorizer.RuleR authorizers = append(authorizers, alwaysDenyAuthorizer) ruleResolvers = append(ruleResolvers, alwaysDenyAuthorizer) case modes.ModeABAC: - if config.PolicyFile == "" { - return nil, nil, errors.New("ABAC's authorization policy file not passed") - } abacAuthorizer, err := abac.NewFromFile(config.PolicyFile) if err != nil { return nil, nil, err @@ -106,9 +97,6 @@ func (config AuthorizationConfig) New() (authorizer.Authorizer, authorizer.RuleR authorizers = append(authorizers, abacAuthorizer) ruleResolvers = append(ruleResolvers, abacAuthorizer) case modes.ModeWebhook: - if config.WebhookConfigFile == "" { - return nil, nil, errors.New("Webhook's configuration file not passed") - } webhookAuthorizer, err := webhook.New(config.WebhookConfigFile, config.WebhookCacheAuthorizedTTL, config.WebhookCacheUnauthorizedTTL) @@ -127,16 +115,8 @@ func (config AuthorizationConfig) New() (authorizer.Authorizer, authorizer.RuleR authorizers = append(authorizers, rbacAuthorizer) ruleResolvers = append(ruleResolvers, rbacAuthorizer) default: - return nil, nil, fmt.Errorf("Unknown authorization mode %s specified", authorizationMode) + return nil, nil, fmt.Errorf("unknown authorization mode %s specified", authorizationMode) } - authorizerMap[authorizationMode] = true - } - - if !authorizerMap[modes.ModeABAC] && config.PolicyFile != "" { - return nil, nil, errors.New("Cannot specify --authorization-policy-file without mode ABAC") - } - if !authorizerMap[modes.ModeWebhook] && config.WebhookConfigFile != "" { - return nil, nil, errors.New("Cannot specify --authorization-webhook-config-file without mode Webhook") } return union.New(authorizers...), union.NewRuleResolvers(ruleResolvers...), nil diff --git a/pkg/kubeapiserver/authorizer/config_test.go b/pkg/kubeapiserver/authorizer/config_test.go deleted file mode 100644 index 42cee988900..00000000000 --- a/pkg/kubeapiserver/authorizer/config_test.go +++ /dev/null @@ -1,101 +0,0 @@ -/* -Copyright 2016 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package authorizer - -import ( - "k8s.io/kubernetes/pkg/kubeapiserver/authorizer/modes" - "testing" -) - -// New has multiple return possibilities. This test -// validates that errors are returned only when proper. -func TestNew(t *testing.T) { - examplePolicyFile := "../../auth/authorizer/abac/example_policy_file.jsonl" - - tests := []struct { - config AuthorizationConfig - wantErr bool - msg string - }{ - { - // Unknown modes should return errors - config: AuthorizationConfig{AuthorizationModes: []string{"DoesNotExist"}}, - wantErr: true, - msg: "using a fake mode should have returned an error", - }, - { - // ModeAlwaysAllow and ModeAlwaysDeny should return without authorizationPolicyFile - // but error if one is given - config: AuthorizationConfig{AuthorizationModes: []string{modes.ModeAlwaysAllow, modes.ModeAlwaysDeny}}, - msg: "returned an error for valid config", - }, - { - // ModeABAC requires a policy file - config: AuthorizationConfig{AuthorizationModes: []string{modes.ModeAlwaysAllow, modes.ModeAlwaysDeny, modes.ModeABAC}}, - wantErr: true, - msg: "specifying ABAC with no policy file should return an error", - }, - { - // ModeABAC should not error if a valid policy path is provided - config: AuthorizationConfig{ - AuthorizationModes: []string{modes.ModeAlwaysAllow, modes.ModeAlwaysDeny, modes.ModeABAC}, - PolicyFile: examplePolicyFile, - }, - msg: "errored while using a valid policy file", - }, - { - - // Authorization Policy file cannot be used without ModeABAC - config: AuthorizationConfig{ - AuthorizationModes: []string{modes.ModeAlwaysAllow, modes.ModeAlwaysDeny}, - PolicyFile: examplePolicyFile, - }, - wantErr: true, - msg: "should have errored when Authorization Policy File is used without ModeABAC", - }, - { - // At least one authorizationMode is necessary - config: AuthorizationConfig{PolicyFile: examplePolicyFile}, - wantErr: true, - msg: "should have errored when no authorization modes are passed", - }, - { - // ModeWebhook requires at minimum a target. - config: AuthorizationConfig{AuthorizationModes: []string{modes.ModeWebhook}}, - wantErr: true, - msg: "should have errored when config was empty with ModeWebhook", - }, - { - // Cannot provide webhook flags without ModeWebhook - config: AuthorizationConfig{ - AuthorizationModes: []string{modes.ModeAlwaysAllow}, - WebhookConfigFile: "authz_webhook_config.yml", - }, - wantErr: true, - msg: "should have errored when Webhook config file is used without ModeWebhook", - }, - } - - for _, tt := range tests { - _, _, err := tt.config.New() - if tt.wantErr && (err == nil) { - t.Errorf("New %s", tt.msg) - } else if !tt.wantErr && (err != nil) { - t.Errorf("New %s: %v", tt.msg, err) - } - } -} diff --git a/pkg/kubeapiserver/options/BUILD b/pkg/kubeapiserver/options/BUILD index 6e411d157a6..b3b6db8c678 100644 --- a/pkg/kubeapiserver/options/BUILD +++ b/pkg/kubeapiserver/options/BUILD @@ -90,8 +90,12 @@ go_test( name = "go_default_test", srcs = [ "admission_test.go", + "authorization_test.go", "storage_versions_test.go", ], embed = [":go_default_library"], - deps = ["//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library"], + deps = [ + "//pkg/kubeapiserver/authorizer/modes:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", + ], ) diff --git a/pkg/kubeapiserver/options/authentication.go b/pkg/kubeapiserver/options/authentication.go index 1a71fb17734..171f31141de 100644 --- a/pkg/kubeapiserver/options/authentication.go +++ b/pkg/kubeapiserver/options/authentication.go @@ -25,6 +25,7 @@ import ( "github.com/golang/glog" "github.com/spf13/pflag" + "k8s.io/apimachinery/pkg/util/sets" genericapiserver "k8s.io/apiserver/pkg/server" genericoptions "k8s.io/apiserver/pkg/server/options" "k8s.io/kubernetes/pkg/kubeapiserver/authenticator" @@ -365,17 +366,8 @@ func (o *BuiltInAuthenticationOptions) ApplyAuthorization(authorization *BuiltIn // authorization ModeAlwaysAllow cannot be combined with AnonymousAuth. // in such a case the AnonymousAuth is stomped to false and you get a message - if o.Anonymous.Allow { - found := false - for _, mode := range strings.Split(authorization.Mode, ",") { - if mode == authzmodes.ModeAlwaysAllow { - found = true - break - } - } - if found { - glog.Warningf("AnonymousAuth is not allowed with the AllowAll authorizer. Resetting AnonymousAuth to false. You should use a different authorizer") - o.Anonymous.Allow = false - } + if o.Anonymous.Allow && sets.NewString(authorization.Modes...).Has(authzmodes.ModeAlwaysAllow) { + glog.Warningf("AnonymousAuth is not allowed with the AlwaysAllow authorizer. Resetting AnonymousAuth to false. You should use a different authorizer") + o.Anonymous.Allow = false } } diff --git a/pkg/kubeapiserver/options/authorization.go b/pkg/kubeapiserver/options/authorization.go index 44b378a4a65..dd483c5e4c8 100644 --- a/pkg/kubeapiserver/options/authorization.go +++ b/pkg/kubeapiserver/options/authorization.go @@ -17,11 +17,13 @@ limitations under the License. package options import ( + "fmt" "strings" "time" "github.com/spf13/pflag" + "k8s.io/apimachinery/pkg/util/sets" versionedinformers "k8s.io/client-go/informers" informers "k8s.io/kubernetes/pkg/client/informers/informers_generated/internalversion" "k8s.io/kubernetes/pkg/kubeapiserver/authorizer" @@ -29,7 +31,7 @@ import ( ) type BuiltInAuthorizationOptions struct { - Mode string + Modes []string PolicyFile string WebhookConfigFile string WebhookCacheAuthorizedTTL time.Duration @@ -38,19 +40,57 @@ type BuiltInAuthorizationOptions struct { func NewBuiltInAuthorizationOptions() *BuiltInAuthorizationOptions { return &BuiltInAuthorizationOptions{ - Mode: authzmodes.ModeAlwaysAllow, + Modes: []string{authzmodes.ModeAlwaysAllow}, WebhookCacheAuthorizedTTL: 5 * time.Minute, WebhookCacheUnauthorizedTTL: 30 * time.Second, } } func (s *BuiltInAuthorizationOptions) Validate() []error { + if s == nil { + return nil + } allErrors := []error{} + + if len(s.Modes) == 0 { + allErrors = append(allErrors, fmt.Errorf("at least one authorization-mode must be passed")) + } + + allowedModes := sets.NewString(authzmodes.AuthorizationModeChoices...) + modes := sets.NewString(s.Modes...) + for _, mode := range s.Modes { + if !allowedModes.Has(mode) { + allErrors = append(allErrors, fmt.Errorf("authorization-mode %q is not a valid mode", mode)) + } + if mode == authzmodes.ModeABAC { + if s.PolicyFile == "" { + allErrors = append(allErrors, fmt.Errorf("authorization-mode ABAC's authorization policy file not passed")) + } + } + if mode == authzmodes.ModeWebhook { + if s.WebhookConfigFile == "" { + allErrors = append(allErrors, fmt.Errorf("authorization-mode Webhook's authorization config file not passed")) + } + } + } + + if s.PolicyFile != "" && !modes.Has(authzmodes.ModeABAC) { + allErrors = append(allErrors, fmt.Errorf("cannot specify --authorization-policy-file without mode ABAC")) + } + + if s.WebhookConfigFile != "" && !modes.Has(authzmodes.ModeWebhook) { + allErrors = append(allErrors, fmt.Errorf("cannot specify --authorization-webhook-config-file without mode Webhook")) + } + + if len(s.Modes) != len(modes.List()) { + allErrors = append(allErrors, fmt.Errorf("authorization-mode %q has mode specified more than once", s.Modes)) + } + return allErrors } func (s *BuiltInAuthorizationOptions) AddFlags(fs *pflag.FlagSet) { - fs.StringVar(&s.Mode, "authorization-mode", s.Mode, ""+ + fs.StringSliceVar(&s.Modes, "authorization-mode", s.Modes, ""+ "Ordered list of plug-ins to do authorization on secure port. Comma-delimited list of: "+ strings.Join(authzmodes.AuthorizationModeChoices, ",")+".") @@ -70,17 +110,9 @@ func (s *BuiltInAuthorizationOptions) AddFlags(fs *pflag.FlagSet) { "The duration to cache 'unauthorized' responses from the webhook authorizer.") } -func (s *BuiltInAuthorizationOptions) Modes() []string { - modes := []string{} - if len(s.Mode) > 0 { - modes = strings.Split(s.Mode, ",") - } - return modes -} - func (s *BuiltInAuthorizationOptions) ToAuthorizationConfig(informerFactory informers.SharedInformerFactory, versionedInformerFactory versionedinformers.SharedInformerFactory) authorizer.AuthorizationConfig { return authorizer.AuthorizationConfig{ - AuthorizationModes: s.Modes(), + AuthorizationModes: s.Modes, PolicyFile: s.PolicyFile, WebhookConfigFile: s.WebhookConfigFile, WebhookCacheAuthorizedTTL: s.WebhookCacheAuthorizedTTL, diff --git a/pkg/kubeapiserver/options/authorization_test.go b/pkg/kubeapiserver/options/authorization_test.go new file mode 100644 index 00000000000..4794de700cb --- /dev/null +++ b/pkg/kubeapiserver/options/authorization_test.go @@ -0,0 +1,104 @@ +/* +Copyright 2018 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package options + +import ( + "testing" + + "k8s.io/kubernetes/pkg/kubeapiserver/authorizer/modes" +) + +func TestAuthzValidate(t *testing.T) { + examplePolicyFile := "../../auth/authorizer/abac/example_policy_file.jsonl" + + testCases := []struct { + name string + modes []string + policyFile string + webhookConfigFile string + expectErr bool + }{ + { + name: "Unknown modes should return errors", + modes: []string{"DoesNotExist"}, + expectErr: true, + }, + { + name: "At least one authorizationMode is necessary", + modes: []string{}, + expectErr: true, + }, + { + name: "ModeAlwaysAllow and ModeAlwaysDeny should return without authorizationPolicyFile", + modes: []string{modes.ModeAlwaysAllow, modes.ModeAlwaysDeny}, + expectErr: false, + }, + { + name: "ModeABAC requires a policy file", + modes: []string{modes.ModeAlwaysAllow, modes.ModeAlwaysDeny, modes.ModeABAC}, + expectErr: true, + }, + { + name: "Authorization Policy file cannot be used without ModeABAC", + modes: []string{modes.ModeAlwaysAllow, modes.ModeAlwaysDeny}, + policyFile: examplePolicyFile, + webhookConfigFile: "", + expectErr: true, + }, + { + name: "ModeABAC should not error if a valid policy path is provided", + modes: []string{modes.ModeAlwaysAllow, modes.ModeAlwaysDeny, modes.ModeABAC}, + policyFile: examplePolicyFile, + webhookConfigFile: "", + expectErr: false, + }, + { + name: "ModeWebhook requires a config file", + modes: []string{modes.ModeWebhook}, + expectErr: true, + }, + { + name: "Cannot provide webhook config file without ModeWebhook", + modes: []string{modes.ModeAlwaysAllow}, + webhookConfigFile: "authz_webhook_config.yaml", + expectErr: true, + }, + { + name: "ModeWebhook should not error if a valid config file is provided", + modes: []string{modes.ModeWebhook}, + webhookConfigFile: "authz_webhook_config.yaml", + expectErr: false, + }, + } + + for _, testcase := range testCases { + t.Run(testcase.name, func(t *testing.T) { + options := NewBuiltInAuthorizationOptions() + options.Modes = testcase.modes + options.WebhookConfigFile = testcase.webhookConfigFile + options.PolicyFile = testcase.policyFile + + errs := options.Validate() + if len(errs) > 0 && !testcase.expectErr { + t.Errorf("got unexpected err %v", errs) + } + if testcase.expectErr && len(errs) == 0 { + t.Errorf("should return an error") + } + }) + } +} diff --git a/test/integration/etcd/etcd_storage_path_test.go b/test/integration/etcd/etcd_storage_path_test.go index 6be9ac3c617..3c87d257098 100644 --- a/test/integration/etcd/etcd_storage_path_test.go +++ b/test/integration/etcd/etcd_storage_path_test.go @@ -733,7 +733,7 @@ func startRealMasterOrDie(t *testing.T, certDir string) (*allClient, clientv3.KV kubeAPIServerOptions.Etcd.StorageConfig.ServerList = []string{framework.GetEtcdURL()} kubeAPIServerOptions.Etcd.DefaultStorageMediaType = runtime.ContentTypeJSON // TODO use protobuf? kubeAPIServerOptions.ServiceClusterIPRange = *defaultServiceClusterIPRange - kubeAPIServerOptions.Authorization.Mode = "RBAC" + kubeAPIServerOptions.Authorization.Modes = []string{"RBAC"} kubeAPIServerOptions.Admission.GenericAdmission.DisablePlugins = []string{"ServiceAccount"} tunneler, proxyTransport, err := app.CreateNodeDialer(kubeAPIServerOptions) diff --git a/test/integration/examples/apiserver_test.go b/test/integration/examples/apiserver_test.go index d24ff797f22..496e799f6e4 100644 --- a/test/integration/examples/apiserver_test.go +++ b/test/integration/examples/apiserver_test.go @@ -109,7 +109,7 @@ func TestAggregatedAPIServer(t *testing.T) { kubeAPIServerOptions.Authentication.RequestHeader.AllowedNames = []string{"kube-aggregator"} kubeAPIServerOptions.Authentication.RequestHeader.ClientCAFile = proxyCACertFile.Name() kubeAPIServerOptions.Authentication.ClientCert.ClientCA = clientCACertFile.Name() - kubeAPIServerOptions.Authorization.Mode = "RBAC" + kubeAPIServerOptions.Authorization.Modes = []string{"RBAC"} tunneler, proxyTransport, err := app.CreateNodeDialer(kubeAPIServerOptions) if err != nil {