diff --git a/pkg/apis/rbac/helpers.go b/pkg/apis/rbac/helpers.go index ff805d3180d..e225d3f74b8 100644 --- a/pkg/apis/rbac/helpers.go +++ b/pkg/apis/rbac/helpers.go @@ -17,9 +17,80 @@ limitations under the License. package rbac import ( + "strings" + "k8s.io/kubernetes/pkg/api/unversioned" ) func RoleRefGroupKind(roleRef RoleRef) unversioned.GroupKind { return unversioned.GroupKind{Group: roleRef.APIGroup, Kind: roleRef.Kind} } + +func VerbMatches(rule PolicyRule, requestedVerb string) bool { + for _, ruleVerb := range rule.Verbs { + if ruleVerb == VerbAll { + return true + } + if ruleVerb == requestedVerb { + return true + } + } + + return false +} + +func APIGroupMatches(rule PolicyRule, requestedGroup string) bool { + for _, ruleGroup := range rule.APIGroups { + if ruleGroup == APIGroupAll { + return true + } + if ruleGroup == requestedGroup { + return true + } + } + + return false +} + +func ResourceMatches(rule PolicyRule, requestedResource string) bool { + for _, ruleResource := range rule.Resources { + if ruleResource == ResourceAll { + return true + } + if ruleResource == requestedResource { + return true + } + } + + return false +} + +func ResourceNameMatches(rule PolicyRule, requestedName string) bool { + if len(rule.ResourceNames) == 0 { + return true + } + + for _, ruleName := range rule.ResourceNames { + if ruleName == requestedName { + return true + } + } + + return false +} + +func NonResourceURLMatches(rule PolicyRule, requestedURL string) bool { + for _, ruleURL := range rule.NonResourceURLs { + if ruleURL == NonResourceAll { + return true + } + if ruleURL == requestedURL { + return true + } + if strings.HasSuffix(ruleURL, "*") && strings.HasPrefix(requestedURL, strings.TrimRight(ruleURL, "*")) { + return true + } + } + + return false +} diff --git a/pkg/apis/rbac/validation/policy_comparator.go b/pkg/apis/rbac/validation/policy_comparator.go index 53dcba5bc60..5e4117fe14b 100644 --- a/pkg/apis/rbac/validation/policy_comparator.go +++ b/pkg/apis/rbac/validation/policy_comparator.go @@ -131,7 +131,6 @@ func nonResourceURLCovers(ownerPath, subPath string) bool { // ruleCovers determines whether the ownerRule (which may have multiple verbs, resources, and resourceNames) covers // the subrule (which may only contain at most one verb, resource, and resourceName) func ruleCovers(ownerRule, subRule rbac.PolicyRule) bool { - verbMatches := has(ownerRule.Verbs, rbac.VerbAll) || hasAll(ownerRule.Verbs, subRule.Verbs) groupMatches := has(ownerRule.APIGroups, rbac.APIGroupAll) || hasAll(ownerRule.APIGroups, subRule.APIGroups) resourceMatches := has(ownerRule.Resources, rbac.ResourceAll) || hasAll(ownerRule.Resources, subRule.Resources) diff --git a/pkg/apis/rbac/validation/rulevalidation.go b/pkg/apis/rbac/validation/rulevalidation.go index acf90d1c580..fbb910a6f44 100644 --- a/pkg/apis/rbac/validation/rulevalidation.go +++ b/pkg/apis/rbac/validation/rulevalidation.go @@ -34,37 +34,29 @@ type AuthorizationRuleResolver interface { // of the role binding, the empty string if a cluster role binding. GetRoleReferenceRules(ctx api.Context, roleRef rbac.RoleRef, namespace string) ([]rbac.PolicyRule, error) - // GetEffectivePolicyRules returns the list of rules that apply to a given user in a given namespace and error. If an error is returned, the slice of + // RulesFor returns the list of rules that apply to a given user in a given namespace and error. If an error is returned, the slice of // PolicyRules may not be complete, but it contains all retrievable rules. This is done because policy rules are purely additive and policy determinations // can be made on the basis of those rules that are found. - GetEffectivePolicyRules(ctx api.Context) ([]rbac.PolicyRule, error) + RulesFor(user user.Info, namespace string) ([]rbac.PolicyRule, error) } // ConfirmNoEscalation determines if the roles for a given user in a given namespace encompass the provided role. func ConfirmNoEscalation(ctx api.Context, ruleResolver AuthorizationRuleResolver, rules []rbac.PolicyRule) error { ruleResolutionErrors := []error{} - ownerLocalRules, err := ruleResolver.GetEffectivePolicyRules(ctx) + user, ok := api.UserFrom(ctx) + if !ok { + return fmt.Errorf("no user on context") + } + namespace, _ := api.NamespaceFrom(ctx) + + ownerRules, err := ruleResolver.RulesFor(user, namespace) if err != nil { // As per AuthorizationRuleResolver contract, this may return a non fatal error with an incomplete list of policies. Log the error and continue. - user, _ := api.UserFrom(ctx) glog.V(1).Infof("non-fatal error getting local rules for %v: %v", user, err) ruleResolutionErrors = append(ruleResolutionErrors, err) } - masterContext := api.WithNamespace(ctx, "") - ownerGlobalRules, err := ruleResolver.GetEffectivePolicyRules(masterContext) - if err != nil { - // Same case as above. Log error, don't fail. - user, _ := api.UserFrom(ctx) - glog.V(1).Infof("non-fatal error getting global rules for %v: %v", user, err) - ruleResolutionErrors = append(ruleResolutionErrors, err) - } - - ownerRules := make([]rbac.PolicyRule, 0, len(ownerGlobalRules)+len(ownerLocalRules)) - ownerRules = append(ownerRules, ownerLocalRules...) - ownerRules = append(ownerRules, ownerGlobalRules...) - ownerRightsCover, missingRights := Covers(ownerRules, rules) if !ownerRightsCover { user, _ := api.UserFrom(ctx) @@ -100,7 +92,53 @@ type ClusterRoleBindingLister interface { ListClusterRoleBindings(ctx api.Context, options *api.ListOptions) (*rbac.ClusterRoleBindingList, error) } -// GetRoleReferenceRules attempts resolve the RoleBinding or ClusterRoleBinding. +func (r *DefaultRuleResolver) RulesFor(user user.Info, namespace string) ([]rbac.PolicyRule, error) { + policyRules := []rbac.PolicyRule{} + errorlist := []error{} + + ctx := api.NewContext() + if clusterRoleBindings, err := r.clusterRoleBindingLister.ListClusterRoleBindings(ctx, &api.ListOptions{}); err != nil { + errorlist = append(errorlist, err) + + } else { + for _, clusterRoleBinding := range clusterRoleBindings.Items { + if !appliesTo(user, clusterRoleBinding.Subjects, "") { + continue + } + rules, err := r.GetRoleReferenceRules(ctx, clusterRoleBinding.RoleRef, "") + if err != nil { + errorlist = append(errorlist, err) + continue + } + policyRules = append(policyRules, rules...) + } + } + + if len(namespace) > 0 { + ctx := api.WithNamespace(api.NewContext(), namespace) + + if roleBindings, err := r.roleBindingLister.ListRoleBindings(ctx, &api.ListOptions{}); err != nil { + errorlist = append(errorlist, err) + + } else { + for _, roleBinding := range roleBindings.Items { + if !appliesTo(user, roleBinding.Subjects, namespace) { + continue + } + rules, err := r.GetRoleReferenceRules(ctx, roleBinding.RoleRef, namespace) + if err != nil { + errorlist = append(errorlist, err) + continue + } + policyRules = append(policyRules, rules...) + } + } + } + + return policyRules, utilerrors.NewAggregate(errorlist) +} + +// GetRoleReferenceRules attempts to resolve the RoleBinding or ClusterRoleBinding. func (r *DefaultRuleResolver) GetRoleReferenceRules(ctx api.Context, roleRef rbac.RoleRef, bindingNamespace string) ([]rbac.PolicyRule, error) { switch kind := rbac.RoleRefGroupKind(roleRef); kind { case rbac.Kind("Role"): @@ -121,83 +159,36 @@ func (r *DefaultRuleResolver) GetRoleReferenceRules(ctx api.Context, roleRef rba return nil, fmt.Errorf("unsupported role reference kind: %q", kind) } } - -func (r *DefaultRuleResolver) GetEffectivePolicyRules(ctx api.Context) ([]rbac.PolicyRule, error) { - policyRules := []rbac.PolicyRule{} - errorlist := []error{} - - if namespace := api.NamespaceValue(ctx); len(namespace) == 0 { - clusterRoleBindings, err := r.clusterRoleBindingLister.ListClusterRoleBindings(ctx, &api.ListOptions{}) - if err != nil { - return nil, err - } - - for _, clusterRoleBinding := range clusterRoleBindings.Items { - if ok, err := appliesTo(ctx, clusterRoleBinding.Subjects); err != nil { - errorlist = append(errorlist, err) - } else if !ok { - continue - } - rules, err := r.GetRoleReferenceRules(ctx, clusterRoleBinding.RoleRef, namespace) - if err != nil { - errorlist = append(errorlist, err) - continue - } - policyRules = append(policyRules, rules...) - } - } else { - roleBindings, err := r.roleBindingLister.ListRoleBindings(ctx, &api.ListOptions{}) - if err != nil { - return nil, err - } - - for _, roleBinding := range roleBindings.Items { - if ok, err := appliesTo(ctx, roleBinding.Subjects); err != nil { - errorlist = append(errorlist, err) - } else if !ok { - continue - } - rules, err := r.GetRoleReferenceRules(ctx, roleBinding.RoleRef, namespace) - if err != nil { - errorlist = append(errorlist, err) - continue - } - policyRules = append(policyRules, rules...) +func appliesTo(user user.Info, bindingSubjects []rbac.Subject, namespace string) bool { + for _, bindingSubject := range bindingSubjects { + if appliesToUser(user, bindingSubject, namespace) { + return true } } - - if len(errorlist) != 0 { - return policyRules, utilerrors.NewAggregate(errorlist) - } - return policyRules, nil + return false } -func appliesTo(ctx api.Context, subjects []rbac.Subject) (bool, error) { - user, ok := api.UserFrom(ctx) - if !ok { - return false, fmt.Errorf("no user data associated with context") - } - for _, subject := range subjects { - if ok, err := appliesToUser(user, subject); err != nil || ok { - return ok, err - } - } - return false, nil -} - -func appliesToUser(user user.Info, subject rbac.Subject) (bool, error) { +func appliesToUser(user user.Info, subject rbac.Subject, namespace string) bool { switch subject.Kind { case rbac.UserKind: - return subject.Name == rbac.UserAll || user.GetName() == subject.Name, nil + return subject.Name == rbac.UserAll || user.GetName() == subject.Name + case rbac.GroupKind: - return has(user.GetGroups(), subject.Name), nil + return has(user.GetGroups(), subject.Name) + case rbac.ServiceAccountKind: - if subject.Namespace == "" { - return false, fmt.Errorf("subject of kind service account without specified namespace") + // default the namespace to namespace we're working in if its available. This allows rolebindings that reference + // SAs in th local namespace to avoid having to qualify them. + saNamespace := namespace + if len(subject.Namespace) > 0 { + saNamespace = subject.Namespace } - return serviceaccount.MakeUsername(subject.Namespace, subject.Name) == user.GetName(), nil + if len(saNamespace) == 0 { + return false + } + return serviceaccount.MakeUsername(saNamespace, subject.Name) == user.GetName() default: - return false, fmt.Errorf("unknown subject kind: %s", subject.Kind) + return false } } diff --git a/pkg/apis/rbac/validation/rulevalidation_test.go b/pkg/apis/rbac/validation/rulevalidation_test.go index 34321608059..a972171edff 100644 --- a/pkg/apis/rbac/validation/rulevalidation_test.go +++ b/pkg/apis/rbac/validation/rulevalidation_test.go @@ -114,52 +114,38 @@ func TestDefaultRuleResolver(t *testing.T) { staticRoles // For a given context, what are the rules that apply? - ctx api.Context + user user.Info + namespace string effectiveRules []rbac.PolicyRule }{ { - staticRoles: staticRoles1, - ctx: api.WithNamespace( - api.WithUser(api.NewContext(), &user.DefaultInfo{Name: "foobar"}), "namespace1", - ), + staticRoles: staticRoles1, + user: &user.DefaultInfo{Name: "foobar"}, + namespace: "namespace1", effectiveRules: []rbac.PolicyRule{ruleReadPods, ruleReadServices}, }, { - staticRoles: staticRoles1, - ctx: api.WithNamespace( - // Same as above but diffrerent namespace. Should return no rules. - api.WithUser(api.NewContext(), &user.DefaultInfo{Name: "foobar"}), "namespace2", - ), + staticRoles: staticRoles1, + user: &user.DefaultInfo{Name: "foobar"}, + namespace: "namespace2", effectiveRules: []rbac.PolicyRule{}, }, - { - staticRoles: staticRoles1, - // GetEffectivePolicyRules only returns the policies for the namespace, not the master namespace. - ctx: api.WithNamespace( - api.WithUser(api.NewContext(), &user.DefaultInfo{ - Name: "foobar", Groups: []string{"admin"}, - }), "namespace1", - ), - effectiveRules: []rbac.PolicyRule{ruleReadPods, ruleReadServices}, - }, { staticRoles: staticRoles1, // Same as above but without a namespace. Only cluster rules should apply. - ctx: api.WithUser(api.NewContext(), &user.DefaultInfo{ - Name: "foobar", Groups: []string{"admin"}, - }), + user: &user.DefaultInfo{Name: "foobar", Groups: []string{"admin"}}, effectiveRules: []rbac.PolicyRule{ruleAdmin}, }, { staticRoles: staticRoles1, - ctx: api.WithUser(api.NewContext(), &user.DefaultInfo{}), + user: &user.DefaultInfo{}, effectiveRules: []rbac.PolicyRule{}, }, } for i, tc := range tests { ruleResolver := newMockRuleResolver(&tc.staticRoles) - rules, err := ruleResolver.GetEffectivePolicyRules(tc.ctx) + rules, err := ruleResolver.RulesFor(tc.user, tc.namespace) if err != nil { t.Errorf("case %d: GetEffectivePolicyRules(context)=%v", i, err) continue @@ -179,7 +165,8 @@ func TestDefaultRuleResolver(t *testing.T) { func TestAppliesTo(t *testing.T) { tests := []struct { subjects []rbac.Subject - ctx api.Context + user user.Info + namespace string appliesTo bool testCase string }{ @@ -187,7 +174,7 @@ func TestAppliesTo(t *testing.T) { subjects: []rbac.Subject{ {Kind: rbac.UserKind, Name: "foobar"}, }, - ctx: api.WithUser(api.NewContext(), &user.DefaultInfo{Name: "foobar"}), + user: &user.DefaultInfo{Name: "foobar"}, appliesTo: true, testCase: "single subject that matches username", }, @@ -196,7 +183,7 @@ func TestAppliesTo(t *testing.T) { {Kind: rbac.UserKind, Name: "barfoo"}, {Kind: rbac.UserKind, Name: "foobar"}, }, - ctx: api.WithUser(api.NewContext(), &user.DefaultInfo{Name: "foobar"}), + user: &user.DefaultInfo{Name: "foobar"}, appliesTo: true, testCase: "multiple subjects, one that matches username", }, @@ -205,7 +192,7 @@ func TestAppliesTo(t *testing.T) { {Kind: rbac.UserKind, Name: "barfoo"}, {Kind: rbac.UserKind, Name: "foobar"}, }, - ctx: api.WithUser(api.NewContext(), &user.DefaultInfo{Name: "zimzam"}), + user: &user.DefaultInfo{Name: "zimzam"}, appliesTo: false, testCase: "multiple subjects, none that match username", }, @@ -214,7 +201,7 @@ func TestAppliesTo(t *testing.T) { {Kind: rbac.UserKind, Name: "barfoo"}, {Kind: rbac.GroupKind, Name: "foobar"}, }, - ctx: api.WithUser(api.NewContext(), &user.DefaultInfo{Name: "zimzam", Groups: []string{"foobar"}}), + user: &user.DefaultInfo{Name: "zimzam", Groups: []string{"foobar"}}, appliesTo: true, testCase: "multiple subjects, one that match group", }, @@ -223,10 +210,8 @@ func TestAppliesTo(t *testing.T) { {Kind: rbac.UserKind, Name: "barfoo"}, {Kind: rbac.GroupKind, Name: "foobar"}, }, - ctx: api.WithNamespace( - api.WithUser(api.NewContext(), &user.DefaultInfo{Name: "zimzam", Groups: []string{"foobar"}}), - "namespace1", - ), + user: &user.DefaultInfo{Name: "zimzam", Groups: []string{"foobar"}}, + namespace: "namespace1", appliesTo: true, testCase: "multiple subjects, one that match group, should ignore namespace", }, @@ -236,10 +221,8 @@ func TestAppliesTo(t *testing.T) { {Kind: rbac.GroupKind, Name: "foobar"}, {Kind: rbac.ServiceAccountKind, Namespace: "kube-system", Name: "default"}, }, - ctx: api.WithNamespace( - api.WithUser(api.NewContext(), &user.DefaultInfo{Name: "system:serviceaccount:kube-system:default"}), - "default", - ), + user: &user.DefaultInfo{Name: "system:serviceaccount:kube-system:default"}, + namespace: "default", appliesTo: true, testCase: "multiple subjects with a service account that matches", }, @@ -247,21 +230,15 @@ func TestAppliesTo(t *testing.T) { subjects: []rbac.Subject{ {Kind: rbac.UserKind, Name: "*"}, }, - ctx: api.WithNamespace( - api.WithUser(api.NewContext(), &user.DefaultInfo{Name: "foobar"}), - "default", - ), + user: &user.DefaultInfo{Name: "foobar"}, + namespace: "default", appliesTo: true, testCase: "multiple subjects with a service account that matches", }, } for _, tc := range tests { - got, err := appliesTo(tc.ctx, tc.subjects) - if err != nil { - t.Errorf("case %q %v", tc.testCase, err) - continue - } + got := appliesTo(tc.user, tc.subjects, tc.namespace) if got != tc.appliesTo { t.Errorf("case %q want appliesTo=%t, got appliesTo=%t", tc.testCase, tc.appliesTo, got) } diff --git a/plugin/pkg/auth/authorizer/rbac/rbac.go b/plugin/pkg/auth/authorizer/rbac/rbac.go index 30e6eb5339d..215bfbd600d 100644 --- a/plugin/pkg/auth/authorizer/rbac/rbac.go +++ b/plugin/pkg/auth/authorizer/rbac/rbac.go @@ -18,56 +18,41 @@ limitations under the License. package rbac import ( - "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/apis/rbac" "k8s.io/kubernetes/pkg/apis/rbac/validation" "k8s.io/kubernetes/pkg/auth/authorizer" + "k8s.io/kubernetes/pkg/auth/user" "k8s.io/kubernetes/pkg/registry/clusterrole" "k8s.io/kubernetes/pkg/registry/clusterrolebinding" "k8s.io/kubernetes/pkg/registry/role" "k8s.io/kubernetes/pkg/registry/rolebinding" ) +type RequestToRuleMapper interface { + // RulesFor returns all known PolicyRules and any errors that happened while locating those rules. + // Any rule returned is still valid, since rules are deny by default. If you can pass with the rules + // supplied, you do not have to fail the request. If you cannot, you should indicate the error along + // with your denial. + RulesFor(subject user.Info, namespace string) ([]rbac.PolicyRule, error) +} + type RBACAuthorizer struct { superUser string - authorizationRuleResolver validation.AuthorizationRuleResolver + authorizationRuleResolver RequestToRuleMapper } -func (r *RBACAuthorizer) Authorize(attr authorizer.Attributes) (bool, string, error) { - if r.superUser != "" && attr.GetUser() != nil && attr.GetUser().GetName() == r.superUser { +func (r *RBACAuthorizer) Authorize(requestAttributes authorizer.Attributes) (bool, string, error) { + if r.superUser != "" && requestAttributes.GetUser() != nil && requestAttributes.GetUser().GetName() == r.superUser { return true, "", nil } - ctx := api.WithNamespace(api.WithUser(api.NewContext(), attr.GetUser()), attr.GetNamespace()) - - // Frame the authorization request as a privilege escalation check. - var requestedRule rbac.PolicyRule - if attr.IsResourceRequest() { - resource := attr.GetResource() - if len(attr.GetSubresource()) > 0 { - resource = attr.GetResource() + "/" + attr.GetSubresource() - } - requestedRule = rbac.PolicyRule{ - Verbs: []string{attr.GetVerb()}, - APIGroups: []string{attr.GetAPIGroup()}, // TODO(ericchiang): add api version here too? - Resources: []string{resource}, - ResourceNames: []string{attr.GetName()}, - } - } else { - requestedRule = rbac.PolicyRule{ - Verbs: []string{attr.GetVerb()}, - NonResourceURLs: []string{attr.GetPath()}, - } + rules, ruleResolutionError := r.authorizationRuleResolver.RulesFor(requestAttributes.GetUser(), requestAttributes.GetNamespace()) + if RulesAllow(requestAttributes, rules...) { + return true, "", nil } - // TODO(nhlfr): Try to find more lightweight way to check attributes than escalation checks. - err := validation.ConfirmNoEscalation(ctx, r.authorizationRuleResolver, []rbac.PolicyRule{requestedRule}) - if err != nil { - return false, err.Error(), nil - } - - return true, "", nil + return false, "", ruleResolutionError } func New(roleRegistry role.Registry, roleBindingRegistry rolebinding.Registry, clusterRoleRegistry clusterrole.Registry, clusterRoleBindingRegistry clusterrolebinding.Registry, superUser string) *RBACAuthorizer { @@ -82,3 +67,30 @@ func New(roleRegistry role.Registry, roleBindingRegistry rolebinding.Registry, c } return authorizer } + +func RulesAllow(requestAttributes authorizer.Attributes, rules ...rbac.PolicyRule) bool { + for _, rule := range rules { + if RuleAllows(requestAttributes, rule) { + return true + } + } + + return false +} + +func RuleAllows(requestAttributes authorizer.Attributes, rule rbac.PolicyRule) bool { + if requestAttributes.IsResourceRequest() { + resource := requestAttributes.GetResource() + if len(requestAttributes.GetSubresource()) > 0 { + resource = requestAttributes.GetResource() + "/" + requestAttributes.GetSubresource() + } + + return rbac.VerbMatches(rule, requestAttributes.GetVerb()) && + rbac.APIGroupMatches(rule, requestAttributes.GetAPIGroup()) && + rbac.ResourceMatches(rule, resource) && + rbac.ResourceNameMatches(rule, requestAttributes.GetName()) + } + + return rbac.VerbMatches(rule, requestAttributes.GetVerb()) && + rbac.NonResourceURLMatches(rule, requestAttributes.GetPath()) +}