From e3604e25903cd45ed0ad33e3526be2d8bd956264 Mon Sep 17 00:00:00 2001 From: Eric Chiang Date: Wed, 25 May 2016 14:19:04 -0700 Subject: [PATCH] add validation to rbac group and apply small cleanups --- pkg/apis/rbac/doc.go | 18 + pkg/apis/rbac/install/install_test.go | 114 ++++++ pkg/apis/rbac/types.go | 12 +- pkg/apis/rbac/v1alpha1/doc.go | 1 + pkg/apis/rbac/v1alpha1/generated.proto | 2 +- pkg/apis/rbac/v1alpha1/register.go | 2 +- pkg/apis/rbac/v1alpha1/types.go | 10 +- .../v1alpha1/types_swagger_doc_generated.go | 2 +- pkg/apis/rbac/validation/policy_comparator.go | 120 ++++++ .../rbac/validation/policy_comparator_test.go | 371 ++++++++++++++++++ pkg/apis/rbac/validation/rulevalidation.go | 208 ++++++++++ .../rbac/validation/rulevalidation_test.go | 335 ++++++++++++++++ pkg/apis/rbac/validation/validation.go | 54 ++- pkg/apis/rbac/validation/validation_test.go | 12 +- 14 files changed, 1219 insertions(+), 42 deletions(-) create mode 100644 pkg/apis/rbac/doc.go create mode 100644 pkg/apis/rbac/install/install_test.go create mode 100644 pkg/apis/rbac/validation/policy_comparator.go create mode 100644 pkg/apis/rbac/validation/policy_comparator_test.go create mode 100644 pkg/apis/rbac/validation/rulevalidation.go create mode 100644 pkg/apis/rbac/validation/rulevalidation_test.go diff --git a/pkg/apis/rbac/doc.go b/pkg/apis/rbac/doc.go new file mode 100644 index 00000000000..15f91da2c38 --- /dev/null +++ b/pkg/apis/rbac/doc.go @@ -0,0 +1,18 @@ +/* +Copyright 2016 The Kubernetes Authors All rights reserved. + +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. +*/ + +// +groupName=rbac.authorization.k8s.io +package rbac diff --git a/pkg/apis/rbac/install/install_test.go b/pkg/apis/rbac/install/install_test.go new file mode 100644 index 00000000000..4f80a090dcf --- /dev/null +++ b/pkg/apis/rbac/install/install_test.go @@ -0,0 +1,114 @@ +/* +Copyright 2016 The Kubernetes Authors All rights reserved. + +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 install + +import ( + "encoding/json" + "testing" + + "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/api/unversioned" + "k8s.io/kubernetes/pkg/apimachinery/registered" + "k8s.io/kubernetes/pkg/apis/rbac" + "k8s.io/kubernetes/pkg/apis/rbac/v1alpha1" + "k8s.io/kubernetes/pkg/runtime" +) + +func TestResourceVersioner(t *testing.T) { + roleBinding := rbac.RoleBinding{ObjectMeta: api.ObjectMeta{ResourceVersion: "10"}} + version, err := accessor.ResourceVersion(&roleBinding) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if version != "10" { + t.Errorf("unexpected version %v", version) + } + + roleBindingList := rbac.RoleBindingList{ListMeta: unversioned.ListMeta{ResourceVersion: "10"}} + version, err = accessor.ResourceVersion(&roleBindingList) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if version != "10" { + t.Errorf("unexpected version %v", version) + } +} + +func TestCodec(t *testing.T) { + roleBinding := rbac.RoleBinding{} + // We do want to use package registered rather than testapi here, because we + // want to test if the package install and package registered work as expected. + data, err := runtime.Encode(api.Codecs.LegacyCodec(registered.GroupOrDie(rbac.GroupName).GroupVersion), &roleBinding) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + other := rbac.RoleBinding{} + if err := json.Unmarshal(data, &other); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if other.APIVersion != registered.GroupOrDie(rbac.GroupName).GroupVersion.String() || other.Kind != "RoleBinding" { + t.Errorf("unexpected unmarshalled object %#v", other) + } +} + +func TestInterfacesFor(t *testing.T) { + if _, err := registered.GroupOrDie(rbac.GroupName).InterfacesFor(rbac.SchemeGroupVersion); err == nil { + t.Fatalf("unexpected non-error: %v", err) + } + for i, version := range registered.GroupOrDie(rbac.GroupName).GroupVersions { + if vi, err := registered.GroupOrDie(rbac.GroupName).InterfacesFor(version); err != nil || vi == nil { + t.Fatalf("%d: unexpected result: %v", i, err) + } + } +} + +func TestRESTMapper(t *testing.T) { + gv := v1alpha1.SchemeGroupVersion + roleBindingGVK := gv.WithKind("RoleBinding") + + if gvk, err := registered.GroupOrDie(rbac.GroupName).RESTMapper.KindFor(gv.WithResource("rolebindings")); err != nil || gvk != roleBindingGVK { + t.Errorf("unexpected version mapping: %v %v", gvk, err) + } + + for _, version := range registered.GroupOrDie(rbac.GroupName).GroupVersions { + mapping, err := registered.GroupOrDie(rbac.GroupName).RESTMapper.RESTMapping(roleBindingGVK.GroupKind(), version.Version) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + + if mapping.Resource != "rolebindings" { + t.Errorf("incorrect resource name: %#v", mapping) + } + if mapping.GroupVersionKind.GroupVersion() != version { + t.Errorf("incorrect groupVersion: %v", mapping) + } + + interfaces, _ := registered.GroupOrDie(rbac.GroupName).InterfacesFor(version) + if mapping.ObjectConvertor != interfaces.ObjectConvertor { + t.Errorf("unexpected: %#v, expected: %#v", mapping, interfaces) + } + + roleBinding := &rbac.RoleBinding{ObjectMeta: api.ObjectMeta{Name: "foo"}} + name, err := mapping.MetadataAccessor.Name(roleBinding) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if name != "foo" { + t.Errorf("unable to retrieve object meta with: %v", mapping.MetadataAccessor) + } + } +} diff --git a/pkg/apis/rbac/types.go b/pkg/apis/rbac/types.go index 454f6b9bb8e..a35eb7db957 100644 --- a/pkg/apis/rbac/types.go +++ b/pkg/apis/rbac/types.go @@ -36,6 +36,8 @@ const ( GroupKind = "Group" ServiceAccountKind = "ServiceAccount" UserKind = "User" + + UserAll = "*" ) // PolicyRule holds information that describes a policy rule, but does not contain information @@ -66,7 +68,7 @@ type Subject struct { // If the Authorizer does not recognized the kind value, the Authorizer should report an error. Kind string // APIVersion holds the API group and version of the referenced object. For non-object references such as "Group" and "User" this is - // expected to be API version of this API group. For example "rbac.authorization.k8s.io/v1alpha1". + // expected to be API version of this API group. For example "rbac/v1alpha1". APIVersion string // Name of the object being referenced. Name string @@ -75,6 +77,8 @@ type Subject struct { Namespace string } +// +genclient=true + // Role is a namespaced, logical grouping of PolicyRules that can be referenced as a unit by a RoleBinding. type Role struct { unversioned.TypeMeta @@ -85,6 +89,8 @@ type Role struct { Rules []PolicyRule } +// +genclient=true + // RoleBinding references a role, but does not contain it. It can reference a Role in the same namespace or a ClusterRole in the global namespace. // It adds who information via Subjects and namespace information by which namespace it exists in. RoleBindings in a given // namespace only have effect in that namespace. @@ -120,6 +126,8 @@ type RoleList struct { Items []Role } +// +genclient=true,nonNamespaced=true + // ClusterRole is a cluster level, logical grouping of PolicyRules that can be referenced as a unit by a RoleBinding or ClusterRoleBinding. type ClusterRole struct { unversioned.TypeMeta @@ -130,6 +138,8 @@ type ClusterRole struct { Rules []PolicyRule } +// +genclient=true,nonNamespaced=true + // ClusterRoleBinding references a ClusterRole, but not contain it. It can reference a ClusterRole in the global namespace, // and adds who information via Subject. type ClusterRoleBinding struct { diff --git a/pkg/apis/rbac/v1alpha1/doc.go b/pkg/apis/rbac/v1alpha1/doc.go index 65a03a2093d..6873ebb101f 100644 --- a/pkg/apis/rbac/v1alpha1/doc.go +++ b/pkg/apis/rbac/v1alpha1/doc.go @@ -14,5 +14,6 @@ See the License for the specific language governing permissions and limitations under the License. */ +// +groupName=rbac.authorization.k8s.io // +genconversion=true package v1alpha1 diff --git a/pkg/apis/rbac/v1alpha1/generated.proto b/pkg/apis/rbac/v1alpha1/generated.proto index 0e713926626..71a2f612f14 100644 --- a/pkg/apis/rbac/v1alpha1/generated.proto +++ b/pkg/apis/rbac/v1alpha1/generated.proto @@ -147,7 +147,7 @@ message Subject { optional string kind = 1; // APIVersion holds the API group and version of the referenced object. For non-object references such as "Group" and "User" this is - // expected to be API version of this API group. For example "rbac.authorization.k8s.io/v1alpha1". + // expected to be API version of this API group. For example "rbac/v1alpha1". optional string apiVersion = 2; // Name of the object being referenced. diff --git a/pkg/apis/rbac/v1alpha1/register.go b/pkg/apis/rbac/v1alpha1/register.go index 53c69b36a1b..41cbe420e24 100644 --- a/pkg/apis/rbac/v1alpha1/register.go +++ b/pkg/apis/rbac/v1alpha1/register.go @@ -24,7 +24,7 @@ import ( ) // SchemeGroupVersion is group version used to register these objects -var SchemeGroupVersion = unversioned.GroupVersion{Group: rbac.GroupName, Version: "v1alpha"} +var SchemeGroupVersion = unversioned.GroupVersion{Group: rbac.GroupName, Version: "v1alpha1"} func AddToScheme(scheme *runtime.Scheme) { addKnownTypes(scheme) diff --git a/pkg/apis/rbac/v1alpha1/types.go b/pkg/apis/rbac/v1alpha1/types.go index 7c785ee3580..0863dfbf588 100644 --- a/pkg/apis/rbac/v1alpha1/types.go +++ b/pkg/apis/rbac/v1alpha1/types.go @@ -55,7 +55,7 @@ type Subject struct { // If the Authorizer does not recognized the kind value, the Authorizer should report an error. Kind string `json:"kind" protobuf:"bytes,1,opt,name=kind"` // APIVersion holds the API group and version of the referenced object. For non-object references such as "Group" and "User" this is - // expected to be API version of this API group. For example "rbac.authorization.k8s.io/v1alpha1". + // expected to be API version of this API group. For example "rbac/v1alpha1". APIVersion string `json:"apiVersion" protobuf:"bytes,2,opt.name=apiVersion"` // Name of the object being referenced. Name string `json:"name" protobuf:"bytes,3,opt,name=name"` @@ -64,6 +64,8 @@ type Subject struct { Namespace string `json:"namespace,omitempty" protobuf:"bytes,4,opt,name=namespace"` } +// +genclient=true + // Role is a namespaced, logical grouping of PolicyRules that can be referenced as a unit by a RoleBinding. type Role struct { unversioned.TypeMeta `json:",inline"` @@ -74,6 +76,8 @@ type Role struct { Rules []PolicyRule `json:"rules" protobuf:"bytes,2,rep,name=rules"` } +// +genclient=true + // RoleBinding references a role, but does not contain it. It can reference a Role in the same namespace or a ClusterRole in the global namespace. // It adds who information via Subjects and namespace information by which namespace it exists in. RoleBindings in a given // namespace only have effect in that namespace. @@ -110,6 +114,8 @@ type RoleList struct { Items []Role `json:"items" protobuf:"bytes,2,rep,name=items"` } +// +genclient=true,nonNamespaced=true + // ClusterRole is a cluster level, logical grouping of PolicyRules that can be referenced as a unit by a RoleBinding or ClusterRoleBinding. type ClusterRole struct { unversioned.TypeMeta `json:",inline"` @@ -120,6 +126,8 @@ type ClusterRole struct { Rules []PolicyRule `json:"rules" protobuf:"bytes,2,rep,name=rules"` } +// +genclient=true,nonNamespaced=true + // ClusterRoleBinding references a ClusterRole, but not contain it. It can reference a ClusterRole in the global namespace, // and adds who information via Subject. type ClusterRoleBinding struct { diff --git a/pkg/apis/rbac/v1alpha1/types_swagger_doc_generated.go b/pkg/apis/rbac/v1alpha1/types_swagger_doc_generated.go index 864acad2a8e..c9d723469df 100644 --- a/pkg/apis/rbac/v1alpha1/types_swagger_doc_generated.go +++ b/pkg/apis/rbac/v1alpha1/types_swagger_doc_generated.go @@ -126,7 +126,7 @@ func (RoleList) SwaggerDoc() map[string]string { var map_Subject = map[string]string{ "": "Subject contains a reference to the object or user identities a role binding applies to. This can either hold a direct API object reference, or a value for non-objects such as user and group names.", "kind": "Kind of object being referenced. Values defined by this API group are \"User\", \"Group\", and \"ServiceAccount\". If the Authorizer does not recognized the kind value, the Authorizer should report an error.", - "apiVersion": "APIVersion holds the API group and version of the referenced object. For non-object references such as \"Group\" and \"User\" this is expected to be API version of this API group. For example \"rbac.authorization.k8s.io/v1alpha1\".", + "apiVersion": "APIVersion holds the API group and version of the referenced object. For non-object references such as \"Group\" and \"User\" this is expected to be API version of this API group. For example \"rbac/v1alpha1\".", "name": "Name of the object being referenced.", "namespace": "Namespace of the referenced object. If the object kind is non-namespace, such as \"User\" or \"Group\", and this value is not empty the Authorizer should report an error.", } diff --git a/pkg/apis/rbac/validation/policy_comparator.go b/pkg/apis/rbac/validation/policy_comparator.go new file mode 100644 index 00000000000..1e5e74faa17 --- /dev/null +++ b/pkg/apis/rbac/validation/policy_comparator.go @@ -0,0 +1,120 @@ +/* +Copyright 2016 The Kubernetes Authors All rights reserved. + +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 validation + +import "k8s.io/kubernetes/pkg/apis/rbac" + +// Covers determines whether or not the ownerRules cover the servantRules in terms of allowed actions. +// It returns whether or not the ownerRules cover and a list of the rules that the ownerRules do not cover. +func Covers(ownerRules, servantRules []rbac.PolicyRule) (bool, []rbac.PolicyRule) { + // 1. Break every servantRule into individual rule tuples: group, verb, resource, resourceName + // 2. Compare the mini-rules against each owner rule. Because the breakdown is down to the most atomic level, we're guaranteed that each mini-servant rule will be either fully covered or not covered by a single owner rule + // 3. Any left over mini-rules means that we are not covered and we have a nice list of them. + // TODO: it might be nice to collapse the list down into something more human readable + + subrules := []rbac.PolicyRule{} + for _, servantRule := range servantRules { + subrules = append(subrules, breakdownRule(servantRule)...) + } + + uncoveredRules := []rbac.PolicyRule{} + for _, subrule := range subrules { + covered := false + for _, ownerRule := range ownerRules { + if ruleCovers(ownerRule, subrule) { + covered = true + break + } + } + + if !covered { + uncoveredRules = append(uncoveredRules, subrule) + } + } + + return (len(uncoveredRules) == 0), uncoveredRules +} + +// breadownRule takes a rule and builds an equivalent list of rules that each have at most one verb, one +// resource, and one resource name +func breakdownRule(rule rbac.PolicyRule) []rbac.PolicyRule { + subrules := []rbac.PolicyRule{} + for _, group := range rule.APIGroups { + for _, resource := range rule.Resources { + for _, verb := range rule.Verbs { + if len(rule.ResourceNames) > 0 { + for _, resourceName := range rule.ResourceNames { + subrules = append(subrules, rbac.PolicyRule{APIGroups: []string{group}, Resources: []string{resource}, Verbs: []string{verb}, ResourceNames: []string{resourceName}}) + } + + } else { + subrules = append(subrules, rbac.PolicyRule{APIGroups: []string{group}, Resources: []string{resource}, Verbs: []string{verb}}) + } + + } + } + } + + // Non-resource URLs are unique because they don't combine with other policy rule fields. + for _, nonResourceURL := range rule.NonResourceURLs { + subrules = append(subrules, rbac.PolicyRule{NonResourceURLs: []string{nonResourceURL}}) + } + + return subrules +} + +func has(set []string, ele string) bool { + for _, s := range set { + if s == ele { + return true + } + } + return false +} + +func hasAll(set, contains []string) bool { + owning := make(map[string]struct{}, len(set)) + for _, ele := range set { + owning[ele] = struct{}{} + } + for _, ele := range contains { + if _, ok := owning[ele]; !ok { + return false + } + } + return true +} + +// 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) + nonResourceURLMatches := has(ownerRule.NonResourceURLs, rbac.NonResourceAll) || hasAll(ownerRule.NonResourceURLs, subRule.NonResourceURLs) + + resourceNameMatches := false + + if len(subRule.ResourceNames) == 0 { + resourceNameMatches = (len(ownerRule.ResourceNames) == 0) + } else { + resourceNameMatches = (len(ownerRule.ResourceNames) == 0) || hasAll(ownerRule.ResourceNames, subRule.ResourceNames) + } + + return verbMatches && groupMatches && resourceMatches && resourceNameMatches && nonResourceURLMatches +} diff --git a/pkg/apis/rbac/validation/policy_comparator_test.go b/pkg/apis/rbac/validation/policy_comparator_test.go new file mode 100644 index 00000000000..da2f910a4e6 --- /dev/null +++ b/pkg/apis/rbac/validation/policy_comparator_test.go @@ -0,0 +1,371 @@ +/* +Copyright 2016 The Kubernetes Authors All rights reserved. + +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 validation + +import ( + "reflect" + "testing" + + "k8s.io/kubernetes/pkg/apis/rbac" +) + +type escalationTest struct { + ownerRules []rbac.PolicyRule + servantRules []rbac.PolicyRule + + expectedCovered bool + expectedUncoveredRules []rbac.PolicyRule +} + +func TestCoversExactMatch(t *testing.T) { + escalationTest{ + ownerRules: []rbac.PolicyRule{ + {APIGroups: []string{"v1"}, Verbs: []string{"get"}, Resources: []string{"builds"}}, + }, + servantRules: []rbac.PolicyRule{ + {APIGroups: []string{"v1"}, Verbs: []string{"get"}, Resources: []string{"builds"}}, + }, + + expectedCovered: true, + expectedUncoveredRules: []rbac.PolicyRule{}, + }.test(t) +} + +func TestCoversMultipleRulesCoveringSingleRule(t *testing.T) { + escalationTest{ + ownerRules: []rbac.PolicyRule{ + {APIGroups: []string{"v1"}, Verbs: []string{"delete"}, Resources: []string{"deployments"}}, + {APIGroups: []string{"v1"}, Verbs: []string{"delete"}, Resources: []string{"builds"}}, + {APIGroups: []string{"v1"}, Verbs: []string{"update"}, Resources: []string{"builds", "deployments"}}, + }, + servantRules: []rbac.PolicyRule{ + {APIGroups: []string{"v1"}, Verbs: []string{"delete", "update"}, Resources: []string{"builds", "deployments"}}, + }, + + expectedCovered: true, + expectedUncoveredRules: []rbac.PolicyRule{}, + }.test(t) + +} + +func TestCoversMultipleAPIGroupsCoveringSingleRule(t *testing.T) { + escalationTest{ + ownerRules: []rbac.PolicyRule{ + {APIGroups: []string{"group1"}, Verbs: []string{"delete"}, Resources: []string{"deployments"}}, + {APIGroups: []string{"group1"}, Verbs: []string{"delete"}, Resources: []string{"builds"}}, + {APIGroups: []string{"group1"}, Verbs: []string{"update"}, Resources: []string{"builds", "deployments"}}, + {APIGroups: []string{"group2"}, Verbs: []string{"delete"}, Resources: []string{"deployments"}}, + {APIGroups: []string{"group2"}, Verbs: []string{"delete"}, Resources: []string{"builds"}}, + {APIGroups: []string{"group2"}, Verbs: []string{"update"}, Resources: []string{"builds", "deployments"}}, + }, + servantRules: []rbac.PolicyRule{ + {APIGroups: []string{"group1", "group2"}, Verbs: []string{"delete", "update"}, Resources: []string{"builds", "deployments"}}, + }, + + expectedCovered: true, + expectedUncoveredRules: []rbac.PolicyRule{}, + }.test(t) + +} + +func TestCoversSingleAPIGroupsCoveringMultiple(t *testing.T) { + escalationTest{ + ownerRules: []rbac.PolicyRule{ + {APIGroups: []string{"group1", "group2"}, Verbs: []string{"delete", "update"}, Resources: []string{"builds", "deployments"}}, + }, + servantRules: []rbac.PolicyRule{ + {APIGroups: []string{"group1"}, Verbs: []string{"delete"}, Resources: []string{"deployments"}}, + {APIGroups: []string{"group1"}, Verbs: []string{"delete"}, Resources: []string{"builds"}}, + {APIGroups: []string{"group1"}, Verbs: []string{"update"}, Resources: []string{"builds", "deployments"}}, + {APIGroups: []string{"group2"}, Verbs: []string{"delete"}, Resources: []string{"deployments"}}, + {APIGroups: []string{"group2"}, Verbs: []string{"delete"}, Resources: []string{"builds"}}, + {APIGroups: []string{"group2"}, Verbs: []string{"update"}, Resources: []string{"builds", "deployments"}}, + }, + + expectedCovered: true, + expectedUncoveredRules: []rbac.PolicyRule{}, + }.test(t) + +} + +func TestCoversMultipleRulesMissingSingleVerbResourceCombination(t *testing.T) { + escalationTest{ + ownerRules: []rbac.PolicyRule{ + {APIGroups: []string{"v1"}, Verbs: []string{"delete", "update"}, Resources: []string{"builds", "deployments"}}, + {APIGroups: []string{"v1"}, Verbs: []string{"delete"}, Resources: []string{"pods"}}, + }, + servantRules: []rbac.PolicyRule{ + {APIGroups: []string{"v1"}, Verbs: []string{"delete", "update"}, Resources: []string{"builds", "deployments", "pods"}}, + }, + + expectedCovered: false, + expectedUncoveredRules: []rbac.PolicyRule{ + {APIGroups: []string{"v1"}, Verbs: []string{"update"}, Resources: []string{"pods"}}, + }, + }.test(t) +} + +func TestCoversAPIGroupStarCoveringMultiple(t *testing.T) { + escalationTest{ + ownerRules: []rbac.PolicyRule{ + {APIGroups: []string{"*"}, Verbs: []string{"get"}, Resources: []string{"roles"}}, + }, + servantRules: []rbac.PolicyRule{ + {APIGroups: []string{"group1", "group2"}, Verbs: []string{"get"}, Resources: []string{"roles"}}, + }, + + expectedCovered: true, + expectedUncoveredRules: []rbac.PolicyRule{}, + }.test(t) +} + +func TestCoversEnumerationNotCoveringAPIGroupStar(t *testing.T) { + escalationTest{ + ownerRules: []rbac.PolicyRule{ + {APIGroups: []string{"dummy-group"}, Verbs: []string{"get"}, Resources: []string{"roles"}}, + }, + servantRules: []rbac.PolicyRule{ + {APIGroups: []string{"*"}, Verbs: []string{"get"}, Resources: []string{"roles"}}, + }, + + expectedCovered: false, + expectedUncoveredRules: []rbac.PolicyRule{ + {APIGroups: []string{"*"}, Verbs: []string{"get"}, Resources: []string{"roles"}}, + }, + }.test(t) +} + +func TestCoversAPIGroupStarCoveringStar(t *testing.T) { + escalationTest{ + ownerRules: []rbac.PolicyRule{ + {APIGroups: []string{"*"}, Verbs: []string{"get"}, Resources: []string{"roles"}}, + }, + servantRules: []rbac.PolicyRule{ + {APIGroups: []string{"*"}, Verbs: []string{"get"}, Resources: []string{"roles"}}, + }, + + expectedCovered: true, + expectedUncoveredRules: []rbac.PolicyRule{}, + }.test(t) +} + +func TestCoversVerbStarCoveringMultiple(t *testing.T) { + escalationTest{ + ownerRules: []rbac.PolicyRule{ + {APIGroups: []string{"v1"}, Verbs: []string{"*"}, Resources: []string{"roles"}}, + }, + servantRules: []rbac.PolicyRule{ + {APIGroups: []string{"v1"}, Verbs: []string{"watch", "list"}, Resources: []string{"roles"}}, + }, + + expectedCovered: true, + expectedUncoveredRules: []rbac.PolicyRule{}, + }.test(t) +} + +func TestCoversEnumerationNotCoveringVerbStar(t *testing.T) { + escalationTest{ + ownerRules: []rbac.PolicyRule{ + {APIGroups: []string{"v1"}, Verbs: []string{"get", "list", "watch", "create", "update", "delete", "exec"}, Resources: []string{"roles"}}, + }, + servantRules: []rbac.PolicyRule{ + {APIGroups: []string{"v1"}, Verbs: []string{"*"}, Resources: []string{"roles"}}, + }, + + expectedCovered: false, + expectedUncoveredRules: []rbac.PolicyRule{ + {APIGroups: []string{"v1"}, Verbs: []string{"*"}, Resources: []string{"roles"}}, + }, + }.test(t) +} + +func TestCoversVerbStarCoveringStar(t *testing.T) { + escalationTest{ + ownerRules: []rbac.PolicyRule{ + {APIGroups: []string{"v1"}, Verbs: []string{"*"}, Resources: []string{"roles"}}, + }, + servantRules: []rbac.PolicyRule{ + {APIGroups: []string{"v1"}, Verbs: []string{"*"}, Resources: []string{"roles"}}, + }, + + expectedCovered: true, + expectedUncoveredRules: []rbac.PolicyRule{}, + }.test(t) +} + +func TestCoversResourceStarCoveringMultiple(t *testing.T) { + escalationTest{ + ownerRules: []rbac.PolicyRule{ + {APIGroups: []string{"v1"}, Verbs: []string{"get"}, Resources: []string{"*"}}, + }, + servantRules: []rbac.PolicyRule{ + {APIGroups: []string{"v1"}, Verbs: []string{"get"}, Resources: []string{"resourcegroup:deployments"}}, + }, + + expectedCovered: true, + expectedUncoveredRules: []rbac.PolicyRule{}, + }.test(t) +} + +func TestCoversEnumerationNotCoveringResourceStar(t *testing.T) { + escalationTest{ + ownerRules: []rbac.PolicyRule{ + {APIGroups: []string{"v1"}, Verbs: []string{"get"}, Resources: []string{"roles", "resourcegroup:deployments"}}, + }, + servantRules: []rbac.PolicyRule{ + {APIGroups: []string{"v1"}, Verbs: []string{"get"}, Resources: []string{"*"}}, + }, + + expectedCovered: false, + expectedUncoveredRules: []rbac.PolicyRule{ + {APIGroups: []string{"v1"}, Verbs: []string{"get"}, Resources: []string{"*"}}, + }, + }.test(t) +} + +func TestCoversResourceStarCoveringStar(t *testing.T) { + escalationTest{ + ownerRules: []rbac.PolicyRule{ + {APIGroups: []string{"v1"}, Verbs: []string{"get"}, Resources: []string{"*"}}, + }, + servantRules: []rbac.PolicyRule{ + {APIGroups: []string{"v1"}, Verbs: []string{"get"}, Resources: []string{"*"}}, + }, + + expectedCovered: true, + expectedUncoveredRules: []rbac.PolicyRule{}, + }.test(t) +} + +func TestCoversResourceNameEmptyCoveringMultiple(t *testing.T) { + escalationTest{ + ownerRules: []rbac.PolicyRule{ + {APIGroups: []string{"v1"}, Verbs: []string{"get"}, Resources: []string{"pods"}, ResourceNames: []string{}}, + }, + servantRules: []rbac.PolicyRule{ + {APIGroups: []string{"v1"}, Verbs: []string{"get"}, Resources: []string{"pods"}, ResourceNames: []string{"foo", "bar"}}, + }, + + expectedCovered: true, + expectedUncoveredRules: []rbac.PolicyRule{}, + }.test(t) +} + +func TestCoversEnumerationNotCoveringResourceNameEmpty(t *testing.T) { + escalationTest{ + ownerRules: []rbac.PolicyRule{ + {APIGroups: []string{"v1"}, Verbs: []string{"get"}, Resources: []string{"pods"}, ResourceNames: []string{"foo", "bar"}}, + }, + servantRules: []rbac.PolicyRule{ + {APIGroups: []string{"v1"}, Verbs: []string{"get"}, Resources: []string{"pods"}, ResourceNames: []string{}}, + }, + + expectedCovered: false, + expectedUncoveredRules: []rbac.PolicyRule{ + {APIGroups: []string{"v1"}, Verbs: []string{"get"}, Resources: []string{"pods"}}, + }, + }.test(t) +} + +func TestCoversNonResourceURLs(t *testing.T) { + escalationTest{ + ownerRules: []rbac.PolicyRule{ + {NonResourceURLs: []string{"/apis"}}, + }, + servantRules: []rbac.PolicyRule{ + {NonResourceURLs: []string{"/apis"}}, + }, + + expectedCovered: true, + expectedUncoveredRules: []rbac.PolicyRule{}, + }.test(t) +} + +func TestCoversNonResourceURLsStar(t *testing.T) { + escalationTest{ + ownerRules: []rbac.PolicyRule{ + {NonResourceURLs: []string{"*"}}, + }, + servantRules: []rbac.PolicyRule{ + {NonResourceURLs: []string{"/apis", "/apis/v1", "/"}}, + }, + + expectedCovered: true, + expectedUncoveredRules: []rbac.PolicyRule{}, + }.test(t) +} + +func TestCoversNonResourceURLsWithOtherFields(t *testing.T) { + escalationTest{ + ownerRules: []rbac.PolicyRule{ + {APIGroups: []string{"v1"}, Verbs: []string{"get"}, Resources: []string{"builds"}, NonResourceURLs: []string{"/apis"}}, + }, + servantRules: []rbac.PolicyRule{ + {APIGroups: []string{"v1"}, Verbs: []string{"get"}, Resources: []string{"builds"}, NonResourceURLs: []string{"/apis"}}, + }, + + expectedCovered: true, + expectedUncoveredRules: []rbac.PolicyRule{}, + }.test(t) +} + +func TestCoversNonResourceURLsWithOtherFieldsFailure(t *testing.T) { + escalationTest{ + ownerRules: []rbac.PolicyRule{ + {APIGroups: []string{"v1"}, Verbs: []string{"get"}, Resources: []string{"builds"}}, + }, + servantRules: []rbac.PolicyRule{ + {APIGroups: []string{"v1"}, Verbs: []string{"get"}, Resources: []string{"builds"}, NonResourceURLs: []string{"/apis"}}, + }, + + expectedCovered: false, + expectedUncoveredRules: []rbac.PolicyRule{{NonResourceURLs: []string{"/apis"}}}, + }.test(t) +} + +func (test escalationTest) test(t *testing.T) { + actualCovered, actualUncoveredRules := Covers(test.ownerRules, test.servantRules) + + if actualCovered != test.expectedCovered { + t.Errorf("expected %v, but got %v", test.expectedCovered, actualCovered) + } + + if !rulesMatch(test.expectedUncoveredRules, actualUncoveredRules) { + t.Errorf("expected %v, but got %v", test.expectedUncoveredRules, actualUncoveredRules) + } +} + +func rulesMatch(expectedRules, actualRules []rbac.PolicyRule) bool { + if len(expectedRules) != len(actualRules) { + return false + } + + for _, expectedRule := range expectedRules { + found := false + for _, actualRule := range actualRules { + if reflect.DeepEqual(expectedRule, actualRule) { + found = true + } + } + + if !found { + return false + } + } + + return true +} diff --git a/pkg/apis/rbac/validation/rulevalidation.go b/pkg/apis/rbac/validation/rulevalidation.go new file mode 100644 index 00000000000..04b15622dc4 --- /dev/null +++ b/pkg/apis/rbac/validation/rulevalidation.go @@ -0,0 +1,208 @@ +/* +Copyright 2016 The Kubernetes Authors All rights reserved. + +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 validation + +import ( + "fmt" + + "github.com/golang/glog" + "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/api/errors" + "k8s.io/kubernetes/pkg/apis/rbac" + "k8s.io/kubernetes/pkg/auth/user" + utilerrors "k8s.io/kubernetes/pkg/util/errors" +) + +type AuthorizationRuleResolver interface { + // GetRoleReferenceRules attempts to resolve the role reference of a RoleBinding or ClusterRoleBinding. The passed namespace should be the namepsace + // of the role binding, the empty string if a cluster role binding. + GetRoleReferenceRules(ctx api.Context, roleRef api.ObjectReference, 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 + // 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) +} + +// 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) + 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) + return errors.NewUnauthorized(fmt.Sprintf("attempt to grant extra privileges: %v user=%v ownerrules=%v ruleResolutionErrors=%v", missingRights, user, ownerRules, ruleResolutionErrors)) + } + return nil +} + +type DefaultRuleResolver struct { + roleGetter RoleGetter + roleBindingLister RoleBindingLister + clusterRoleGetter ClusterRoleGetter + clusterRoleBindingLister ClusterRoleBindingLister +} + +func NewDefaultRuleResolver(roleGetter RoleGetter, roleBindingLister RoleBindingLister, clusterRoleGetter ClusterRoleGetter, clusterRoleBindingLister ClusterRoleBindingLister) *DefaultRuleResolver { + return &DefaultRuleResolver{roleGetter, roleBindingLister, clusterRoleGetter, clusterRoleBindingLister} +} + +type RoleGetter interface { + GetRole(ctx api.Context, id string) (*rbac.Role, error) +} + +type RoleBindingLister interface { + ListRoleBindings(ctx api.Context, options *api.ListOptions) (*rbac.RoleBindingList, error) +} + +type ClusterRoleGetter interface { + GetClusterRole(ctx api.Context, id string) (*rbac.ClusterRole, error) +} + +type ClusterRoleBindingLister interface { + ListClusterRoleBindings(ctx api.Context, options *api.ListOptions) (*rbac.ClusterRoleBindingList, error) +} + +// GetRoleReferenceRules attempts resolve the RoleBinding or ClusterRoleBinding. +func (r *DefaultRuleResolver) GetRoleReferenceRules(ctx api.Context, roleRef api.ObjectReference, bindingNamespace string) ([]rbac.PolicyRule, error) { + switch roleRef.Kind { + case "Role": + // Roles can only be referenced by RoleBindings within the same namespace. + if len(bindingNamespace) == 0 { + return nil, fmt.Errorf("cluster role binding references role %q in namespace %q", roleRef.Name, roleRef.Namespace) + } else { + if bindingNamespace != roleRef.Namespace { + return nil, fmt.Errorf("role binding in namespace %q references role %q in namespace %q", bindingNamespace, roleRef.Name, roleRef.Namespace) + } + } + + role, err := r.roleGetter.GetRole(api.WithNamespace(ctx, roleRef.Namespace), roleRef.Name) + if err != nil { + return nil, err + } + return role.Rules, nil + case "ClusterRole": + clusterRole, err := r.clusterRoleGetter.GetClusterRole(api.WithNamespace(ctx, ""), roleRef.Name) + if err != nil { + return nil, err + } + return clusterRole.Rules, nil + default: + return nil, fmt.Errorf("unsupported role reference kind: %q", roleRef.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...) + } + } + + if len(errorlist) != 0 { + return policyRules, utilerrors.NewAggregate(errorlist) + } + return policyRules, nil +} + +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) { + switch subject.Kind { + case rbac.UserKind: + return subject.Name == rbac.UserAll || user.GetName() == subject.Name, nil + case rbac.GroupKind: + return has(user.GetGroups(), subject.Name), nil + case rbac.ServiceAccountKind: + if subject.Namespace == "" { + return false, fmt.Errorf("subject of kind service account without specified namespace") + } + // TODO(ericchiang): Is there a better way of matching a service account name? + return "system:serviceaccount:"+subject.Name+":"+subject.Namespace == user.GetName(), nil + default: + return false, fmt.Errorf("unknown subject kind: %s", subject.Kind) + } +} diff --git a/pkg/apis/rbac/validation/rulevalidation_test.go b/pkg/apis/rbac/validation/rulevalidation_test.go new file mode 100644 index 00000000000..ef2b2254052 --- /dev/null +++ b/pkg/apis/rbac/validation/rulevalidation_test.go @@ -0,0 +1,335 @@ +/* +Copyright 2016 The Kubernetes Authors All rights reserved. + +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 validation + +import ( + "errors" + "hash/fnv" + "io" + "reflect" + "sort" + "testing" + + "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/apis/rbac" + "k8s.io/kubernetes/pkg/auth/user" + "k8s.io/kubernetes/pkg/util/diff" +) + +func newMockRuleResolver(r *staticRoles) AuthorizationRuleResolver { + return NewDefaultRuleResolver(r, r, r, r) +} + +type staticRoles struct { + roles []rbac.Role + roleBindings []rbac.RoleBinding + clusterRoles []rbac.ClusterRole + clusterRoleBindings []rbac.ClusterRoleBinding +} + +func (r *staticRoles) GetRole(ctx api.Context, id string) (*rbac.Role, error) { + namespace, ok := api.NamespaceFrom(ctx) + if !ok || namespace == "" { + return nil, errors.New("must provide namespace when getting role") + } + for _, role := range r.roles { + if role.Namespace == namespace && role.Name == id { + return &role, nil + } + } + return nil, errors.New("role not found") +} + +func (r *staticRoles) GetClusterRole(ctx api.Context, id string) (*rbac.ClusterRole, error) { + namespace, ok := api.NamespaceFrom(ctx) + if ok && namespace != "" { + return nil, errors.New("cannot provide namespace when getting cluster role") + } + for _, clusterRole := range r.clusterRoles { + if clusterRole.Namespace == namespace && clusterRole.Name == id { + return &clusterRole, nil + } + } + return nil, errors.New("role not found") +} + +func (r *staticRoles) ListRoleBindings(ctx api.Context, options *api.ListOptions) (*rbac.RoleBindingList, error) { + namespace, ok := api.NamespaceFrom(ctx) + if !ok || namespace == "" { + return nil, errors.New("must provide namespace when listing role bindings") + } + + roleBindingList := new(rbac.RoleBindingList) + for _, roleBinding := range r.roleBindings { + if roleBinding.Namespace != namespace { + continue + } + // TODO(ericchiang): need to implement label selectors? + roleBindingList.Items = append(roleBindingList.Items, roleBinding) + } + return roleBindingList, nil +} + +func (r *staticRoles) ListClusterRoleBindings(ctx api.Context, options *api.ListOptions) (*rbac.ClusterRoleBindingList, error) { + namespace, ok := api.NamespaceFrom(ctx) + if ok && namespace != "" { + return nil, errors.New("cannot list cluster role bindings from within a namespace") + } + clusterRoleBindings := new(rbac.ClusterRoleBindingList) + clusterRoleBindings.Items = make([]rbac.ClusterRoleBinding, len(r.clusterRoleBindings)) + copy(clusterRoleBindings.Items, r.clusterRoleBindings) + return clusterRoleBindings, nil +} + +// compute a hash of a policy rule so we can sort in a deterministic order +func hashOf(p rbac.PolicyRule) string { + hash := fnv.New32() + writeStrings := func(slis ...[]string) { + for _, sli := range slis { + for _, s := range sli { + io.WriteString(hash, s) + } + } + } + writeStrings(p.Verbs, p.APIGroups, p.Resources, p.ResourceNames, p.NonResourceURLs) + return string(hash.Sum(nil)) +} + +// byHash sorts a set of policy rules by a hash of its fields +type byHash []rbac.PolicyRule + +func (b byHash) Len() int { return len(b) } +func (b byHash) Less(i, j int) bool { return hashOf(b[i]) < hashOf(b[j]) } +func (b byHash) Swap(i, j int) { b[i], b[j] = b[j], b[i] } + +func TestDefaultRuleResolver(t *testing.T) { + ruleReadPods := rbac.PolicyRule{ + Verbs: []string{"GET", "WATCH"}, + APIGroups: []string{"v1"}, + Resources: []string{"pods"}, + } + ruleReadServices := rbac.PolicyRule{ + Verbs: []string{"GET", "WATCH"}, + APIGroups: []string{"v1"}, + Resources: []string{"services"}, + } + ruleWriteNodes := rbac.PolicyRule{ + Verbs: []string{"PUT", "CREATE", "UPDATE"}, + APIGroups: []string{"v1"}, + Resources: []string{"nodes"}, + } + ruleAdmin := rbac.PolicyRule{ + Verbs: []string{"*"}, + APIGroups: []string{"*"}, + Resources: []string{"*"}, + } + + staticRoles1 := staticRoles{ + roles: []rbac.Role{ + { + ObjectMeta: api.ObjectMeta{Namespace: "namespace1", Name: "readthings"}, + Rules: []rbac.PolicyRule{ruleReadPods, ruleReadServices}, + }, + }, + clusterRoles: []rbac.ClusterRole{ + { + ObjectMeta: api.ObjectMeta{Name: "cluster-admin"}, + Rules: []rbac.PolicyRule{ruleAdmin}, + }, + { + ObjectMeta: api.ObjectMeta{Name: "write-nodes"}, + Rules: []rbac.PolicyRule{ruleWriteNodes}, + }, + }, + roleBindings: []rbac.RoleBinding{ + { + ObjectMeta: api.ObjectMeta{Namespace: "namespace1"}, + Subjects: []rbac.Subject{ + {Kind: rbac.UserKind, Name: "foobar"}, + {Kind: rbac.GroupKind, Name: "group1"}, + }, + RoleRef: api.ObjectReference{Kind: "Role", Namespace: "namespace1", Name: "readthings"}, + }, + }, + clusterRoleBindings: []rbac.ClusterRoleBinding{ + { + Subjects: []rbac.Subject{ + {Kind: rbac.UserKind, Name: "admin"}, + {Kind: rbac.GroupKind, Name: "admin"}, + }, + RoleRef: api.ObjectReference{Kind: "ClusterRole", Name: "cluster-admin"}, + }, + }, + } + + tests := []struct { + staticRoles + + // For a given context, what are the rules that apply? + ctx api.Context + effectiveRules []rbac.PolicyRule + }{ + { + staticRoles: staticRoles1, + ctx: api.WithNamespace( + api.WithUser(api.NewContext(), &user.DefaultInfo{Name: "foobar"}), "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", + ), + 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"}, + }), + effectiveRules: []rbac.PolicyRule{ruleAdmin}, + }, + { + staticRoles: staticRoles1, + ctx: api.WithUser(api.NewContext(), &user.DefaultInfo{}), + effectiveRules: []rbac.PolicyRule{}, + }, + } + + for i, tc := range tests { + ruleResolver := newMockRuleResolver(&tc.staticRoles) + rules, err := ruleResolver.GetEffectivePolicyRules(tc.ctx) + if err != nil { + t.Errorf("case %d: GetEffectivePolicyRules(context)=%v", i, err) + continue + } + + // Sort for deep equals + sort.Sort(byHash(rules)) + sort.Sort(byHash(tc.effectiveRules)) + + if !reflect.DeepEqual(rules, tc.effectiveRules) { + ruleDiff := diff.ObjectDiff(rules, tc.effectiveRules) + t.Errorf("case %d: %s", i, ruleDiff) + } + } +} + +func TestAppliesTo(t *testing.T) { + tests := []struct { + subjects []rbac.Subject + ctx api.Context + appliesTo bool + testCase string + }{ + { + subjects: []rbac.Subject{ + {Kind: rbac.UserKind, Name: "foobar"}, + }, + ctx: api.WithUser(api.NewContext(), &user.DefaultInfo{Name: "foobar"}), + appliesTo: true, + testCase: "single subject that matches username", + }, + { + subjects: []rbac.Subject{ + {Kind: rbac.UserKind, Name: "barfoo"}, + {Kind: rbac.UserKind, Name: "foobar"}, + }, + ctx: api.WithUser(api.NewContext(), &user.DefaultInfo{Name: "foobar"}), + appliesTo: true, + testCase: "multiple subjects, one that matches username", + }, + { + subjects: []rbac.Subject{ + {Kind: rbac.UserKind, Name: "barfoo"}, + {Kind: rbac.UserKind, Name: "foobar"}, + }, + ctx: api.WithUser(api.NewContext(), &user.DefaultInfo{Name: "zimzam"}), + appliesTo: false, + testCase: "multiple subjects, none that match username", + }, + { + subjects: []rbac.Subject{ + {Kind: rbac.UserKind, Name: "barfoo"}, + {Kind: rbac.GroupKind, Name: "foobar"}, + }, + ctx: api.WithUser(api.NewContext(), &user.DefaultInfo{Name: "zimzam", Groups: []string{"foobar"}}), + appliesTo: true, + testCase: "multiple subjects, one that match group", + }, + { + subjects: []rbac.Subject{ + {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", + ), + appliesTo: true, + testCase: "multiple subjects, one that match group, should ignore namespace", + }, + { + subjects: []rbac.Subject{ + {Kind: rbac.UserKind, Name: "barfoo"}, + {Kind: rbac.GroupKind, Name: "foobar"}, + {Kind: rbac.ServiceAccountKind, Name: "kube-system", Namespace: "default"}, + }, + ctx: api.WithNamespace( + api.WithUser(api.NewContext(), &user.DefaultInfo{Name: "system:serviceaccount:kube-system:default"}), + "default", + ), + appliesTo: true, + testCase: "multiple subjects with a service account that matches", + }, + { + subjects: []rbac.Subject{ + {Kind: rbac.UserKind, Name: "*"}, + }, + ctx: api.WithNamespace( + api.WithUser(api.NewContext(), &user.DefaultInfo{Name: "foobar"}), + "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 + } + if got != tc.appliesTo { + t.Errorf("case %q want appliesTo=%t, got appliesTo=%t", tc.testCase, tc.appliesTo, got) + } + } +} diff --git a/pkg/apis/rbac/validation/validation.go b/pkg/apis/rbac/validation/validation.go index 69481a1fb62..4a384e65ee4 100644 --- a/pkg/apis/rbac/validation/validation.go +++ b/pkg/apis/rbac/validation/validation.go @@ -29,73 +29,65 @@ func minimalNameRequirements(name string, prefix bool) []string { return validation.IsValidPathSegmentName(name) } -func ValidateLocalRole(policy *rbac.Role) field.ErrorList { - return ValidateRole(policy, true) +func ValidateRole(policy *rbac.Role) field.ErrorList { + return validateRole(policy, true) } -func ValidateLocalRoleUpdate(policy *rbac.Role, oldRole *rbac.Role) field.ErrorList { - return ValidateRoleUpdate(policy, oldRole, true) +func ValidateRoleUpdate(policy *rbac.Role, oldRole *rbac.Role) field.ErrorList { + return validateRoleUpdate(policy, oldRole, true) } func ValidateClusterRole(policy *rbac.ClusterRole) field.ErrorList { - return ValidateRole(toRole(policy), false) + return validateRole(toRole(policy), false) } func ValidateClusterRoleUpdate(policy *rbac.ClusterRole, oldRole *rbac.ClusterRole) field.ErrorList { - return ValidateRoleUpdate(toRole(policy), toRole(oldRole), false) + return validateRoleUpdate(toRole(policy), toRole(oldRole), false) } -func ValidateRole(role *rbac.Role, isNamespaced bool) field.ErrorList { - return validateRole(role, isNamespaced, nil) +func validateRole(role *rbac.Role, isNamespaced bool) field.ErrorList { + return validation.ValidateObjectMeta(&role.ObjectMeta, isNamespaced, minimalNameRequirements, field.NewPath("metadata")) } -func validateRole(role *rbac.Role, isNamespaced bool, fldPath *field.Path) field.ErrorList { - return validation.ValidateObjectMeta(&role.ObjectMeta, isNamespaced, minimalNameRequirements, fldPath.Child("metadata")) -} - -func ValidateRoleUpdate(role *rbac.Role, oldRole *rbac.Role, isNamespaced bool) field.ErrorList { - allErrs := ValidateRole(role, isNamespaced) +func validateRoleUpdate(role *rbac.Role, oldRole *rbac.Role, isNamespaced bool) field.ErrorList { + allErrs := validateRole(role, isNamespaced) allErrs = append(allErrs, validation.ValidateObjectMetaUpdate(&role.ObjectMeta, &oldRole.ObjectMeta, field.NewPath("metadata"))...) return allErrs } -func ValidateLocalRoleBinding(policy *rbac.RoleBinding) field.ErrorList { - return ValidateRoleBinding(policy, true) +func ValidateRoleBinding(policy *rbac.RoleBinding) field.ErrorList { + return validateRoleBinding(policy, true) } -func ValidateLocalRoleBindingUpdate(policy *rbac.RoleBinding, oldRoleBinding *rbac.RoleBinding) field.ErrorList { - return ValidateRoleBindingUpdate(policy, oldRoleBinding, true) +func ValidateRoleBindingUpdate(policy *rbac.RoleBinding, oldRoleBinding *rbac.RoleBinding) field.ErrorList { + return validateRoleBindingUpdate(policy, oldRoleBinding, true) } func ValidateClusterRoleBinding(policy *rbac.ClusterRoleBinding) field.ErrorList { - return ValidateRoleBinding(toRoleBinding(policy), false) + return validateRoleBinding(toRoleBinding(policy), false) } func ValidateClusterRoleBindingUpdate(policy *rbac.ClusterRoleBinding, oldRoleBinding *rbac.ClusterRoleBinding) field.ErrorList { - return ValidateRoleBindingUpdate(toRoleBinding(policy), toRoleBinding(oldRoleBinding), false) + return validateRoleBindingUpdate(toRoleBinding(policy), toRoleBinding(oldRoleBinding), false) } -func ValidateRoleBinding(roleBinding *rbac.RoleBinding, isNamespaced bool) field.ErrorList { - return validateRoleBinding(roleBinding, isNamespaced, nil) -} - -func validateRoleBinding(roleBinding *rbac.RoleBinding, isNamespaced bool, fldPath *field.Path) field.ErrorList { +func validateRoleBinding(roleBinding *rbac.RoleBinding, isNamespaced bool) field.ErrorList { allErrs := field.ErrorList{} - allErrs = append(allErrs, validation.ValidateObjectMeta(&roleBinding.ObjectMeta, isNamespaced, minimalNameRequirements, fldPath.Child("metadata"))...) + allErrs = append(allErrs, validation.ValidateObjectMeta(&roleBinding.ObjectMeta, isNamespaced, minimalNameRequirements, field.NewPath("metadata"))...) // roleRef namespace is empty when referring to global policy. if len(roleBinding.RoleRef.Namespace) > 0 { for _, msg := range validation.ValidateNamespaceName(roleBinding.RoleRef.Namespace, false) { - allErrs = append(allErrs, field.Invalid(fldPath.Child("roleRef", "namespace"), roleBinding.RoleRef.Namespace, msg)) + allErrs = append(allErrs, field.Invalid(field.NewPath("roleRef", "namespace"), roleBinding.RoleRef.Namespace, msg)) } } if len(roleBinding.RoleRef.Name) == 0 { - allErrs = append(allErrs, field.Required(fldPath.Child("roleRef", "name"), "")) + allErrs = append(allErrs, field.Required(field.NewPath("roleRef", "name"), "")) } else { for _, msg := range minimalNameRequirements(roleBinding.RoleRef.Name, false) { - allErrs = append(allErrs, field.Invalid(fldPath.Child("roleRef", "name"), roleBinding.RoleRef.Name, msg)) + allErrs = append(allErrs, field.Invalid(field.NewPath("roleRef", "name"), roleBinding.RoleRef.Name, msg)) } } @@ -147,8 +139,8 @@ func validateRoleBindingSubject(subject rbac.Subject, isNamespaced bool, fldPath return allErrs } -func ValidateRoleBindingUpdate(roleBinding *rbac.RoleBinding, oldRoleBinding *rbac.RoleBinding, isNamespaced bool) field.ErrorList { - allErrs := ValidateRoleBinding(roleBinding, isNamespaced) +func validateRoleBindingUpdate(roleBinding *rbac.RoleBinding, oldRoleBinding *rbac.RoleBinding, isNamespaced bool) field.ErrorList { + allErrs := validateRoleBinding(roleBinding, isNamespaced) allErrs = append(allErrs, validation.ValidateObjectMetaUpdate(&roleBinding.ObjectMeta, &oldRoleBinding.ObjectMeta, field.NewPath("metadata"))...) if oldRoleBinding.RoleRef != roleBinding.RoleRef { diff --git a/pkg/apis/rbac/validation/validation_test.go b/pkg/apis/rbac/validation/validation_test.go index 3a6ecb65556..c513729a7b5 100644 --- a/pkg/apis/rbac/validation/validation_test.go +++ b/pkg/apis/rbac/validation/validation_test.go @@ -25,7 +25,7 @@ import ( ) func TestValidateRoleBinding(t *testing.T) { - errs := ValidateRoleBinding( + errs := validateRoleBinding( &rbac.RoleBinding{ ObjectMeta: api.ObjectMeta{Namespace: api.NamespaceDefault, Name: "master"}, RoleRef: api.ObjectReference{Namespace: "master", Name: "valid"}, @@ -116,7 +116,7 @@ func TestValidateRoleBinding(t *testing.T) { }, } for k, v := range errorCases { - errs := ValidateRoleBinding(&v.A, true) + errs := validateRoleBinding(&v.A, true) if len(errs) == 0 { t.Errorf("expected failure %s for %v", k, v.A) continue @@ -138,7 +138,7 @@ func TestValidateRoleBindingUpdate(t *testing.T) { RoleRef: api.ObjectReference{Namespace: "master", Name: "valid"}, } - errs := ValidateRoleBindingUpdate( + errs := validateRoleBindingUpdate( &rbac.RoleBinding{ ObjectMeta: api.ObjectMeta{Namespace: api.NamespaceDefault, Name: "master", ResourceVersion: "1"}, RoleRef: api.ObjectReference{Namespace: "master", Name: "valid"}, @@ -165,7 +165,7 @@ func TestValidateRoleBindingUpdate(t *testing.T) { }, } for k, v := range errorCases { - errs := ValidateRoleBindingUpdate(&v.A, old, true) + errs := validateRoleBindingUpdate(&v.A, old, true) if len(errs) == 0 { t.Errorf("expected failure %s for %v", k, v.A) continue @@ -182,7 +182,7 @@ func TestValidateRoleBindingUpdate(t *testing.T) { } func TestValidateRole(t *testing.T) { - errs := ValidateRole( + errs := validateRole( &rbac.Role{ ObjectMeta: api.ObjectMeta{Namespace: api.NamespaceDefault, Name: "master"}, }, @@ -213,7 +213,7 @@ func TestValidateRole(t *testing.T) { }, } for k, v := range errorCases { - errs := ValidateRole(&v.A, true) + errs := validateRole(&v.A, true) if len(errs) == 0 { t.Errorf("expected failure %s for %v", k, v.A) continue