Skip to content

Commit 7fa775c

Browse files
committed
Wire node authorizer
1 parent a5b2890 commit 7fa775c

File tree

7 files changed

+414
-32
lines changed

7 files changed

+414
-32
lines changed

pkg/cmd/server/bootstrappolicy/dead.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import (
99

1010
var (
1111
deadClusterRoles = []rbac.ClusterRole{}
12+
13+
deadClusterRoleBindings = []rbac.ClusterRoleBinding{}
1214
)
1315

1416
func addDeadClusterRole(name string) {
@@ -25,12 +27,33 @@ func addDeadClusterRole(name string) {
2527
)
2628
}
2729

30+
func addDeadClusterRoleBinding(name, roleName string) {
31+
for _, existing := range deadClusterRoleBindings {
32+
if name == existing.Name {
33+
glog.Fatalf("%q was already registered", name)
34+
}
35+
}
36+
37+
deadClusterRoleBindings = append(deadClusterRoleBindings,
38+
rbac.ClusterRoleBinding{
39+
ObjectMeta: metav1.ObjectMeta{Name: name},
40+
RoleRef: rbac.RoleRef{APIGroup: rbac.GroupName, Kind: "ClusterRole", Name: roleName},
41+
},
42+
)
43+
}
44+
2845
// GetDeadClusterRoles returns cluster roles which should no longer have any permissions.
2946
// These are enumerated so that a reconcile that tightens permissions will properly.
3047
func GetDeadClusterRoles() []rbac.ClusterRole {
3148
return deadClusterRoles
3249
}
3350

51+
// GetDeadClusterRoleBindings returns cluster role bindings which should no longer have any subjects.
52+
// These are enumerated so that a reconcile that tightens permissions will properly remove them.
53+
func GetDeadClusterRoleBindings() []rbac.ClusterRoleBinding {
54+
return deadClusterRoleBindings
55+
}
56+
3457
func init() {
3558
// these were replaced by kube controller roles
3659
addDeadClusterRole("system:replication-controller")
@@ -50,4 +73,7 @@ func init() {
5073
addDeadClusterRole("system:build-controller")
5174
addDeadClusterRole("system:deploymentconfig-controller")
5275
addDeadClusterRole("system:deployment-controller")
76+
77+
// this was replaced by the node authorizer
78+
addDeadClusterRoleBinding("system:nodes", "system:node")
5379
}

pkg/cmd/server/bootstrappolicy/policy.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -696,13 +696,13 @@ func GetOpenshiftBootstrapClusterRoles() []rbac.ClusterRole {
696696
Rules: []rbac.PolicyRule{
697697
// Needed to check API access. These creates are non-mutating
698698
rbac.NewRule("create").Groups(kAuthnGroup).Resources("tokenreviews").RuleOrDie(),
699-
rbac.NewRule("create").Groups(authzGroup, legacyAuthzGroup).Resources("subjectaccessreviews", "localsubjectaccessreviews").RuleOrDie(),
700699
rbac.NewRule("create").Groups(kAuthzGroup).Resources("subjectaccessreviews", "localsubjectaccessreviews").RuleOrDie(),
701700
// Needed to build serviceLister, to populate env vars for services
702701
rbac.NewRule(read...).Groups(kapiGroup).Resources("services").RuleOrDie(),
703702
// Nodes can register themselves
704-
// TODO: restrict to creating a node with the same name they announce
703+
// Use the NodeRestriction admission plugin to limit a node to creating/updating its own API object.
705704
rbac.NewRule("create", "get", "list", "watch").Groups(kapiGroup).Resources("nodes").RuleOrDie(),
705+
rbac.NewRule("update", "patch", "delete").Groups(kapiGroup).Resources("nodes").RuleOrDie(),
706706
// TODO: restrict to the bound node once supported
707707
rbac.NewRule("update", "patch").Groups(kapiGroup).Resources("nodes/status").RuleOrDie(),
708708

@@ -731,7 +731,7 @@ func GetOpenshiftBootstrapClusterRoles() []rbac.ClusterRole {
731731
// Needed for glusterfs volumes
732732
rbac.NewRule("get").Groups(kapiGroup).Resources("endpoints").RuleOrDie(),
733733
// Nodes are allowed to request CSRs (specifically, request serving certs)
734-
rbac.NewRule("get", "create").Groups(certificates.GroupName).Resources("certificatesigningrequests").RuleOrDie(),
734+
rbac.NewRule("get", "create", "list", "watch").Groups(certificates.GroupName).Resources("certificatesigningrequests").RuleOrDie(),
735735
},
736736
},
737737

@@ -966,9 +966,6 @@ func GetOpenshiftBootstrapClusterRoleBindings() []rbac.ClusterRoleBinding {
966966
newOriginClusterBinding(StatusCheckerRoleBindingName, StatusCheckerRoleName).
967967
Groups(AuthenticatedGroup, UnauthenticatedGroup).
968968
BindingOrDie(),
969-
newOriginClusterBinding(NodeRoleBindingName, NodeRoleName).
970-
Groups(NodesGroup).
971-
BindingOrDie(),
972969
newOriginClusterBinding(NodeProxierRoleBindingName, NodeProxierRoleName).
973970
// Allow node identities to run node proxies
974971
Groups(NodesGroup).
@@ -1009,6 +1006,10 @@ func GetOpenshiftBootstrapClusterRoleBindings() []rbac.ClusterRoleBinding {
10091006

10101007
func GetBootstrapClusterRoleBindings() []rbac.ClusterRoleBinding {
10111008
openshiftClusterRoleBindings := GetOpenshiftBootstrapClusterRoleBindings()
1009+
// dead cluster roles need to be checked for conflicts (in case something new comes up)
1010+
// so add them to this list.
1011+
openshiftClusterRoleBindings = append(openshiftClusterRoleBindings, GetDeadClusterRoleBindings()...)
1012+
10121013
kubeClusterRoleBindings := bootstrappolicy.ClusterRoleBindings()
10131014
kubeControllerClusterRoleBindings := bootstrappolicy.ControllerRoleBindings()
10141015
openshiftControllerClusterRoleBindings := ControllerRoleBindings()

pkg/cmd/server/bootstrappolicy/policy_test.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"k8s.io/kubernetes/pkg/apis/rbac"
1515
"k8s.io/kubernetes/pkg/apis/rbac/v1beta1"
1616
rulevalidation "k8s.io/kubernetes/pkg/registry/rbac/validation"
17+
kbootstrappolicy "k8s.io/kubernetes/plugin/pkg/auth/authorizer/rbac/bootstrappolicy"
1718

1819
"github.com/openshift/origin/pkg/api/v1"
1920
"github.com/openshift/origin/pkg/cmd/server/bootstrappolicy"
@@ -109,6 +110,7 @@ func TestCovers(t *testing.T) {
109110
var clusterAdmin *rbac.ClusterRole
110111
var storageAdmin *rbac.ClusterRole
111112
var imageBuilder *rbac.ClusterRole
113+
var nodeRole *rbac.ClusterRole
112114

113115
for i := range allRoles {
114116
role := allRoles[i]
@@ -135,6 +137,8 @@ func TestCovers(t *testing.T) {
135137
storageAdmin = &role
136138
case bootstrappolicy.ImageBuilderRoleName:
137139
imageBuilder = &role
140+
case bootstrappolicy.NodeRoleName:
141+
nodeRole = &role
138142
}
139143
}
140144

@@ -176,4 +180,19 @@ func TestCovers(t *testing.T) {
176180
if covers, miss := rulevalidation.Covers(systemMaster.Rules, clusterAdmin.Rules); !covers {
177181
t.Errorf("failed to cover: %#v", miss)
178182
}
183+
184+
// Make sure our node role covers upstream node rules
185+
if covers, miss := rulevalidation.Covers(nodeRole.Rules, kbootstrappolicy.NodeRules()); !covers {
186+
t.Errorf("upstream node role has extra permissions:")
187+
for _, r := range miss {
188+
t.Logf("\t%s", r.CompactString())
189+
}
190+
}
191+
// Make sure our node role doesn't have any extra permissions
192+
if covers, miss := rulevalidation.Covers(kbootstrappolicy.NodeRules(), nodeRole.Rules); !covers {
193+
t.Errorf("openshift node role has extra permissions:")
194+
for _, r := range miss {
195+
t.Logf("\t%s", r.CompactString())
196+
}
197+
}
179198
}

pkg/cmd/server/origin/master_config.go

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,12 @@ import (
4343
restclient "k8s.io/client-go/rest"
4444
"k8s.io/client-go/tools/cache"
4545
kapi "k8s.io/kubernetes/pkg/api"
46+
"k8s.io/kubernetes/pkg/auth/nodeidentifier"
4647
kclientsetexternal "k8s.io/kubernetes/pkg/client/clientset_generated/clientset"
4748
kclientsetinternal "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
4849
kinformers "k8s.io/kubernetes/pkg/client/informers/informers_generated/externalversions"
4950
kinternalinformers "k8s.io/kubernetes/pkg/client/informers/informers_generated/internalversion"
51+
coreinformers "k8s.io/kubernetes/pkg/client/informers/informers_generated/internalversion/core/internalversion"
5052
rbacinformers "k8s.io/kubernetes/pkg/client/informers/informers_generated/internalversion/rbac/internalversion"
5153
rbaclisters "k8s.io/kubernetes/pkg/client/listers/rbac/internalversion"
5254
sacontroller "k8s.io/kubernetes/pkg/controller/serviceaccount"
@@ -57,7 +59,9 @@ import (
5759
noderestriction "k8s.io/kubernetes/plugin/pkg/admission/noderestriction"
5860
saadmit "k8s.io/kubernetes/plugin/pkg/admission/serviceaccount"
5961
storageclassdefaultadmission "k8s.io/kubernetes/plugin/pkg/admission/storageclass/setdefault"
62+
"k8s.io/kubernetes/plugin/pkg/auth/authorizer/node"
6063
rbacauthorizer "k8s.io/kubernetes/plugin/pkg/auth/authorizer/rbac"
64+
kbootstrappolicy "k8s.io/kubernetes/plugin/pkg/auth/authorizer/rbac/bootstrappolicy"
6165

6266
"github.com/openshift/origin/pkg/auth/authenticator/request/paramtoken"
6367
authnregistry "github.com/openshift/origin/pkg/auth/oauth/registry"
@@ -228,6 +232,8 @@ func BuildMasterConfig(options configapi.MasterConfig, informers InformerAccess)
228232
kubeAuthorizer,
229233
kubeSubjectLocator,
230234
informers.GetInternalKubeInformers().Rbac().InternalVersion().ClusterRoles().Lister(),
235+
informers.GetInternalKubeInformers().Core().InternalVersion().Pods(),
236+
informers.GetInternalKubeInformers().Core().InternalVersion().PersistentVolumes(),
231237
options.ProjectConfig.ProjectRequestMessage,
232238
)
233239

@@ -780,14 +786,27 @@ func buildKubeAuth(r rbacinformers.Interface) (kauthorizer.Authorizer, rbacregis
780786
return kubeAuthorizer, ruleResolver, kubeSubjectLocator
781787
}
782788

783-
func newAuthorizer(kubeAuthorizer kauthorizer.Authorizer, kubeSubjectLocator rbacauthorizer.SubjectLocator, clusterRoleGetter rbaclisters.ClusterRoleLister, projectRequestDenyMessage string) (kauthorizer.Authorizer, authorizer.SubjectLocator) {
789+
func newAuthorizer(
790+
kubeAuthorizer kauthorizer.Authorizer,
791+
kubeSubjectLocator rbacauthorizer.SubjectLocator,
792+
clusterRoleGetter rbaclisters.ClusterRoleLister,
793+
podInformer coreinformers.PodInformer,
794+
pvInformer coreinformers.PersistentVolumeInformer,
795+
projectRequestDenyMessage string,
796+
) (kauthorizer.Authorizer, authorizer.SubjectLocator) {
784797
messageMaker := authorizer.NewForbiddenMessageResolver(projectRequestDenyMessage)
785798
roleBasedAuthorizer := authorizer.NewAuthorizer(kubeAuthorizer, messageMaker)
786799
subjectLocator := authorizer.NewSubjectLocator(kubeSubjectLocator)
800+
787801
scopeLimitedAuthorizer := scope.NewAuthorizer(roleBasedAuthorizer, clusterRoleGetter, messageMaker)
788802

803+
graph := node.NewGraph()
804+
node.AddGraphEventHandlers(graph, podInformer, pvInformer)
805+
nodeAuthorizer := node.NewAuthorizer(graph, nodeidentifier.NewDefaultNodeIdentifier(), kbootstrappolicy.NodeRules())
806+
789807
authorizer := authorizerunion.New(
790808
authorizerfactory.NewPrivilegedGroups(user.SystemPrivilegedGroup), // authorizes system:masters to do anything, just like upstream
809+
nodeAuthorizer,
791810
scopeLimitedAuthorizer)
792811

793812
return authorizer, subjectLocator

0 commit comments

Comments
 (0)