Switch to pointer to policy rule, visit and short circuit during authorization

This commit is contained in:
Jordan Liggitt 2017-04-13 10:33:28 -04:00
parent 2c6fbc95c4
commit 67360883bc
No known key found for this signature in database
GPG Key ID: 24E7ADF9A3B42012
7 changed files with 176 additions and 32 deletions

View File

@ -29,7 +29,7 @@ func RoleRefGroupKind(roleRef RoleRef) schema.GroupKind {
return schema.GroupKind{Group: roleRef.APIGroup, Kind: roleRef.Kind}
}
func VerbMatches(rule PolicyRule, requestedVerb string) bool {
func VerbMatches(rule *PolicyRule, requestedVerb string) bool {
for _, ruleVerb := range rule.Verbs {
if ruleVerb == VerbAll {
return true
@ -42,7 +42,7 @@ func VerbMatches(rule PolicyRule, requestedVerb string) bool {
return false
}
func APIGroupMatches(rule PolicyRule, requestedGroup string) bool {
func APIGroupMatches(rule *PolicyRule, requestedGroup string) bool {
for _, ruleGroup := range rule.APIGroups {
if ruleGroup == APIGroupAll {
return true
@ -55,7 +55,7 @@ func APIGroupMatches(rule PolicyRule, requestedGroup string) bool {
return false
}
func ResourceMatches(rule PolicyRule, requestedResource string) bool {
func ResourceMatches(rule *PolicyRule, requestedResource string) bool {
for _, ruleResource := range rule.Resources {
if ruleResource == ResourceAll {
return true
@ -68,7 +68,7 @@ func ResourceMatches(rule PolicyRule, requestedResource string) bool {
return false
}
func ResourceNameMatches(rule PolicyRule, requestedName string) bool {
func ResourceNameMatches(rule *PolicyRule, requestedName string) bool {
if len(rule.ResourceNames) == 0 {
return true
}
@ -82,7 +82,7 @@ func ResourceNameMatches(rule PolicyRule, requestedName string) bool {
return false
}
func NonResourceURLMatches(rule PolicyRule, requestedURL string) bool {
func NonResourceURLMatches(rule *PolicyRule, requestedURL string) bool {
for _, ruleURL := range rule.NonResourceURLs {
if ruleURL == NonResourceAll {
return true

View File

@ -39,6 +39,10 @@ type AuthorizationRuleResolver interface {
// 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.
RulesFor(user user.Info, namespace string) ([]rbac.PolicyRule, error)
// VisitRulesFor invokes visitor() with each rule that applies to a given user in a given namespace, and each error encountered resolving those rules.
// If visitor() returns false, visiting is short-circuited.
VisitRulesFor(user user.Info, namespace string, visitor func(rule *rbac.PolicyRule, err error) bool)
}
// ConfirmNoEscalation determines if the roles for a given user in a given namespace encompass the provided role.
@ -93,12 +97,31 @@ type ClusterRoleBindingLister interface {
}
func (r *DefaultRuleResolver) RulesFor(user user.Info, namespace string) ([]rbac.PolicyRule, error) {
policyRules := []rbac.PolicyRule{}
errorlist := []error{}
visitor := &ruleAccumulator{}
r.VisitRulesFor(user, namespace, visitor.visit)
return visitor.rules, utilerrors.NewAggregate(visitor.errors)
}
type ruleAccumulator struct {
rules []rbac.PolicyRule
errors []error
}
func (r *ruleAccumulator) visit(rule *rbac.PolicyRule, err error) bool {
if rule != nil {
r.rules = append(r.rules, *rule)
}
if err != nil {
r.errors = append(r.errors, err)
}
return true
}
func (r *DefaultRuleResolver) VisitRulesFor(user user.Info, namespace string, visitor func(rule *rbac.PolicyRule, err error) bool) {
if clusterRoleBindings, err := r.clusterRoleBindingLister.ListClusterRoleBindings(); err != nil {
errorlist = append(errorlist, err)
if !visitor(nil, err) {
return
}
} else {
for _, clusterRoleBinding := range clusterRoleBindings {
if !appliesTo(user, clusterRoleBinding.Subjects, "") {
@ -106,17 +129,24 @@ func (r *DefaultRuleResolver) RulesFor(user user.Info, namespace string) ([]rbac
}
rules, err := r.GetRoleReferenceRules(clusterRoleBinding.RoleRef, "")
if err != nil {
errorlist = append(errorlist, err)
if !visitor(nil, err) {
return
}
continue
}
policyRules = append(policyRules, rules...)
for i := range rules {
if !visitor(&rules[i], nil) {
return
}
}
}
}
if len(namespace) > 0 {
if roleBindings, err := r.roleBindingLister.ListRoleBindings(namespace); err != nil {
errorlist = append(errorlist, err)
if !visitor(nil, err) {
return
}
} else {
for _, roleBinding := range roleBindings {
if !appliesTo(user, roleBinding.Subjects, namespace) {
@ -124,15 +154,19 @@ func (r *DefaultRuleResolver) RulesFor(user user.Info, namespace string) ([]rbac
}
rules, err := r.GetRoleReferenceRules(roleBinding.RoleRef, namespace)
if err != nil {
errorlist = append(errorlist, err)
if !visitor(nil, err) {
return
}
continue
}
policyRules = append(policyRules, rules...)
for i := range rules {
if !visitor(&rules[i], nil) {
return
}
}
}
}
}
return policyRules, utilerrors.NewAggregate(errorlist)
}
// GetRoleReferenceRules attempts to resolve the RoleBinding or ClusterRoleBinding.

View File

@ -128,7 +128,7 @@ func TestDefaultRuleResolver(t *testing.T) {
StaticRoles: staticRoles1,
user: &user.DefaultInfo{Name: "foobar"},
namespace: "namespace2",
effectiveRules: []rbac.PolicyRule{},
effectiveRules: nil,
},
{
StaticRoles: staticRoles1,
@ -139,7 +139,7 @@ func TestDefaultRuleResolver(t *testing.T) {
{
StaticRoles: staticRoles1,
user: &user.DefaultInfo{},
effectiveRules: []rbac.PolicyRule{},
effectiveRules: nil,
},
}

View File

@ -36,6 +36,7 @@ go_test(
deps = [
"//pkg/apis/rbac:go_default_library",
"//pkg/registry/rbac/validation:go_default_library",
"//plugin/pkg/auth/authorizer/rbac/bootstrappolicy:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//vendor/k8s.io/apiserver/pkg/authentication/user:go_default_library",
"//vendor/k8s.io/apiserver/pkg/authorization/authorizer:go_default_library",

View File

@ -24,6 +24,7 @@ import (
"bytes"
utilerrors "k8s.io/apimachinery/pkg/util/errors"
"k8s.io/apiserver/pkg/authentication/user"
"k8s.io/apiserver/pkg/authorization/authorizer"
"k8s.io/kubernetes/pkg/apis/rbac"
@ -36,15 +37,41 @@ type RequestToRuleMapper interface {
// 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)
// VisitRulesFor invokes visitor() with each rule that applies to a given user in a given namespace,
// and each error encountered resolving those rules. Rule may be nil if err is non-nil.
// If visitor() returns false, visiting is short-circuited.
VisitRulesFor(user user.Info, namespace string, visitor func(rule *rbac.PolicyRule, err error) bool)
}
type RBACAuthorizer struct {
authorizationRuleResolver RequestToRuleMapper
}
// authorizingVisitor short-circuits once allowed, and collects any resolution errors encountered
type authorizingVisitor struct {
requestAttributes authorizer.Attributes
allowed bool
errors []error
}
func (v *authorizingVisitor) visit(rule *rbac.PolicyRule, err error) bool {
if rule != nil && RuleAllows(v.requestAttributes, rule) {
v.allowed = true
return false
}
if err != nil {
v.errors = append(v.errors, err)
}
return true
}
func (r *RBACAuthorizer) Authorize(requestAttributes authorizer.Attributes) (bool, string, error) {
rules, ruleResolutionError := r.authorizationRuleResolver.RulesFor(requestAttributes.GetUser(), requestAttributes.GetNamespace())
if RulesAllow(requestAttributes, rules...) {
ruleCheckingVisitor := &authorizingVisitor{requestAttributes: requestAttributes}
r.authorizationRuleResolver.VisitRulesFor(requestAttributes.GetUser(), requestAttributes.GetNamespace(), ruleCheckingVisitor.visit)
if ruleCheckingVisitor.allowed {
return true, "", nil
}
@ -88,8 +115,8 @@ func (r *RBACAuthorizer) Authorize(requestAttributes authorizer.Attributes) (boo
}
reason := ""
if ruleResolutionError != nil {
reason = fmt.Sprintf("%v", ruleResolutionError)
if len(ruleCheckingVisitor.errors) > 0 {
reason = fmt.Sprintf("%v", utilerrors.NewAggregate(ruleCheckingVisitor.errors))
}
return false, reason, nil
}
@ -104,8 +131,8 @@ func New(roles rbacregistryvalidation.RoleGetter, roleBindings rbacregistryvalid
}
func RulesAllow(requestAttributes authorizer.Attributes, rules ...rbac.PolicyRule) bool {
for _, rule := range rules {
if RuleAllows(requestAttributes, rule) {
for i := range rules {
if RuleAllows(requestAttributes, &rules[i]) {
return true
}
}
@ -113,7 +140,7 @@ func RulesAllow(requestAttributes authorizer.Attributes, rules ...rbac.PolicyRul
return false
}
func RuleAllows(requestAttributes authorizer.Attributes, rule rbac.PolicyRule) bool {
func RuleAllows(requestAttributes authorizer.Attributes, rule *rbac.PolicyRule) bool {
if requestAttributes.IsResourceRequest() {
resource := requestAttributes.GetResource()
if len(requestAttributes.GetSubresource()) > 0 {

View File

@ -26,6 +26,7 @@ import (
"k8s.io/apiserver/pkg/authorization/authorizer"
"k8s.io/kubernetes/pkg/apis/rbac"
rbacregistryvalidation "k8s.io/kubernetes/pkg/registry/rbac/validation"
"k8s.io/kubernetes/plugin/pkg/auth/authorizer/rbac/bootstrappolicy"
)
func newRule(verbs, apiGroups, resources, nonResourceURLs string) rbac.PolicyRule {
@ -381,7 +382,7 @@ func TestRuleMatches(t *testing.T) {
}
for _, tc := range tests {
for request, expected := range tc.requestsToExpected {
if e, a := expected, RuleAllows(request, tc.rule); e != a {
if e, a := expected, RuleAllows(request, &tc.rule); e != a {
t.Errorf("%q: expected %v, got %v for %v", tc.name, e, a, request)
}
}
@ -432,3 +433,84 @@ func (r *requestAttributeBuilder) URL(url string) *requestAttributeBuilder {
func (r *requestAttributeBuilder) New() authorizer.AttributesRecord {
return r.request
}
func BenchmarkAuthorize(b *testing.B) {
bootstrapRoles := []rbac.ClusterRole{}
bootstrapRoles = append(bootstrapRoles, bootstrappolicy.ControllerRoles()...)
bootstrapRoles = append(bootstrapRoles, bootstrappolicy.ClusterRoles()...)
bootstrapBindings := []rbac.ClusterRoleBinding{}
bootstrapBindings = append(bootstrapBindings, bootstrappolicy.ClusterRoleBindings()...)
bootstrapBindings = append(bootstrapBindings, bootstrappolicy.ControllerRoleBindings()...)
clusterRoles := []*rbac.ClusterRole{}
for i := range bootstrapRoles {
clusterRoles = append(clusterRoles, &bootstrapRoles[i])
}
clusterRoleBindings := []*rbac.ClusterRoleBinding{}
for i := range bootstrapBindings {
clusterRoleBindings = append(clusterRoleBindings, &bootstrapBindings[i])
}
_, resolver := rbacregistryvalidation.NewTestRuleResolver(nil, nil, clusterRoles, clusterRoleBindings)
authz := New(resolver, resolver, resolver, resolver)
nodeUser := &user.DefaultInfo{Name: "system:node:node1", Groups: []string{"system:nodes", "system:authenticated"}}
requests := []struct {
name string
attrs authorizer.Attributes
}{
{
"allow list pods",
authorizer.AttributesRecord{
ResourceRequest: true,
User: nodeUser,
Verb: "list",
Resource: "pods",
Subresource: "",
Name: "",
Namespace: "",
APIGroup: "",
APIVersion: "v1",
},
},
{
"allow update pods/status",
authorizer.AttributesRecord{
ResourceRequest: true,
User: nodeUser,
Verb: "update",
Resource: "pods",
Subresource: "status",
Name: "mypods",
Namespace: "myns",
APIGroup: "",
APIVersion: "v1",
},
},
{
"forbid educate dolphins",
authorizer.AttributesRecord{
ResourceRequest: true,
User: nodeUser,
Verb: "educate",
Resource: "dolphins",
Subresource: "",
Name: "",
Namespace: "",
APIGroup: "",
APIVersion: "v1",
},
},
}
b.ResetTimer()
for _, request := range requests {
b.Run(request.name, func(b *testing.B) {
for i := 0; i < b.N; i++ {
authz.Authorize(request.attrs)
}
})
}
}

View File

@ -29,7 +29,7 @@ func RoleRefGroupKind(roleRef RoleRef) schema.GroupKind {
return schema.GroupKind{Group: roleRef.APIGroup, Kind: roleRef.Kind}
}
func VerbMatches(rule PolicyRule, requestedVerb string) bool {
func VerbMatches(rule *PolicyRule, requestedVerb string) bool {
for _, ruleVerb := range rule.Verbs {
if ruleVerb == VerbAll {
return true
@ -42,7 +42,7 @@ func VerbMatches(rule PolicyRule, requestedVerb string) bool {
return false
}
func APIGroupMatches(rule PolicyRule, requestedGroup string) bool {
func APIGroupMatches(rule *PolicyRule, requestedGroup string) bool {
for _, ruleGroup := range rule.APIGroups {
if ruleGroup == APIGroupAll {
return true
@ -55,7 +55,7 @@ func APIGroupMatches(rule PolicyRule, requestedGroup string) bool {
return false
}
func ResourceMatches(rule PolicyRule, requestedResource string) bool {
func ResourceMatches(rule *PolicyRule, requestedResource string) bool {
for _, ruleResource := range rule.Resources {
if ruleResource == ResourceAll {
return true
@ -68,7 +68,7 @@ func ResourceMatches(rule PolicyRule, requestedResource string) bool {
return false
}
func ResourceNameMatches(rule PolicyRule, requestedName string) bool {
func ResourceNameMatches(rule *PolicyRule, requestedName string) bool {
if len(rule.ResourceNames) == 0 {
return true
}
@ -82,7 +82,7 @@ func ResourceNameMatches(rule PolicyRule, requestedName string) bool {
return false
}
func NonResourceURLMatches(rule PolicyRule, requestedURL string) bool {
func NonResourceURLMatches(rule *PolicyRule, requestedURL string) bool {
for _, ruleURL := range rule.NonResourceURLs {
if ruleURL == NonResourceAll {
return true