Skip to content

Commit 6defd8c

Browse files
aramasearamase
committed
node authorizer changes to allow read on svcaccounts
Signed-off-by: Anish Ramasekar <[email protected]>
1 parent d398de2 commit 6defd8c

File tree

3 files changed

+129
-57
lines changed

3 files changed

+129
-57
lines changed

‎plugin/pkg/auth/authorizer/node/node_authorizer.go‎

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ func (r *NodeAuthorizer) Authorize(ctx context.Context, attrs authorizer.Attribu
135135
case vaResource:
136136
return r.authorizeGet(nodeName, vaVertexType, attrs)
137137
case svcAcctResource:
138-
return r.authorizeCreateToken(nodeName, serviceAccountVertexType, attrs)
138+
return r.authorizeServiceAccount(nodeName, attrs)
139139
case leaseResource:
140140
return r.authorizeLease(nodeName, attrs)
141141
case csiNodeResource:
@@ -196,7 +196,7 @@ func (r *NodeAuthorizer) authorizeGet(nodeName string, startingType vertexType,
196196
func (r *NodeAuthorizer) authorizeReadNamespacedObject(nodeName string, startingType vertexType, attrs authorizer.Attributes) (authorizer.Decision, string, error) {
197197
switch attrs.GetVerb() {
198198
case "get", "list", "watch":
199-
//ok
199+
// ok
200200
default:
201201
klog.V(2).Infof("NODE DENY: '%s' %#v", nodeName, attrs)
202202
return authorizer.DecisionNoOpinion, "can only read resources of this type", nil
@@ -231,6 +231,23 @@ func (r *NodeAuthorizer) authorize(nodeName string, startingType vertexType, att
231231
return authorizer.DecisionAllow, "", nil
232232
}
233233

234+
// authorizeServiceAccount authorizes
235+
// - "get" requests to serviceaccounts when KubeletServiceAccountTokenForCredentialProviders feature is enabled
236+
// - "create" requests to serviceaccounts 'token' subresource of pods running on a node
237+
func (r *NodeAuthorizer) authorizeServiceAccount(nodeName string, attrs authorizer.Attributes) (authorizer.Decision, string, error) {
238+
verb := attrs.GetVerb()
239+
240+
if verb == "get" && attrs.GetSubresource() == "" {
241+
if !r.features.Enabled(features.KubeletServiceAccountTokenForCredentialProviders) {
242+
klog.V(2).Infof("NODE DENY: '%s' %#v", nodeName, attrs)
243+
return authorizer.DecisionNoOpinion, "not allowed to get service accounts", nil
244+
}
245+
return r.authorizeReadNamespacedObject(nodeName, serviceAccountVertexType, attrs)
246+
}
247+
248+
return r.authorizeCreateToken(nodeName, serviceAccountVertexType, attrs)
249+
}
250+
234251
// authorizeCreateToken authorizes "create" requests to serviceaccounts 'token'
235252
// subresource of pods running on a node
236253
func (r *NodeAuthorizer) authorizeCreateToken(nodeName string, startingType vertexType, attrs authorizer.Attributes) (authorizer.Decision, string, error) {
@@ -262,7 +279,7 @@ func (r *NodeAuthorizer) authorizeLease(nodeName string, attrs authorizer.Attrib
262279
verb := attrs.GetVerb()
263280
switch verb {
264281
case "get", "create", "update", "patch", "delete":
265-
//ok
282+
// ok
266283
default:
267284
klog.V(2).Infof("NODE DENY: '%s' %#v", nodeName, attrs)
268285
return authorizer.DecisionNoOpinion, "can only get, create, update, patch, or delete a node lease", nil
@@ -291,7 +308,7 @@ func (r *NodeAuthorizer) authorizeCSINode(nodeName string, attrs authorizer.Attr
291308
verb := attrs.GetVerb()
292309
switch verb {
293310
case "get", "create", "update", "patch", "delete":
294-
//ok
311+
// ok
295312
default:
296313
klog.V(2).Infof("NODE DENY: '%s' %#v", nodeName, attrs)
297314
return authorizer.DecisionNoOpinion, "can only get, create, update, patch, or delete a CSINode", nil

‎plugin/pkg/auth/authorizer/node/node_authorizer_test.go‎

Lines changed: 103 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,12 @@ func TestNodeAuthorizer(t *testing.T) {
8080
featuregatetesting.SetFeatureGateDuringTest(t, selectorAuthzEnabled, genericfeatures.AuthorizeWithSelectors, true)
8181
featuregatetesting.SetFeatureGateDuringTest(t, selectorAuthzEnabled, features.AuthorizeNodeWithSelectors, true)
8282

83+
serviceAccountTokenForCredentialProvidersDisabled := utilfeature.DefaultFeatureGate.DeepCopy()
84+
featuregatetesting.SetFeatureGateDuringTest(t, serviceAccountTokenForCredentialProvidersDisabled, features.KubeletServiceAccountTokenForCredentialProviders, false)
85+
86+
serviceAccountTokenForCredentialProvidersEnabled := utilfeature.DefaultFeatureGate.DeepCopy()
87+
featuregatetesting.SetFeatureGateDuringTest(t, serviceAccountTokenForCredentialProvidersEnabled, features.KubeletServiceAccountTokenForCredentialProviders, true)
88+
8389
featureVariants := []struct {
8490
suffix string
8591
features featuregate.FeatureGate
@@ -89,10 +95,11 @@ func TestNodeAuthorizer(t *testing.T) {
8995
}
9096

9197
tests := []struct {
92-
name string
93-
attrs authorizer.AttributesRecord
94-
expect authorizer.Decision
95-
features featuregate.FeatureGate
98+
name string
99+
attrs authorizer.AttributesRecord
100+
expect authorizer.Decision
101+
expectReason string
102+
features featuregate.FeatureGate
96103
}{
97104
{
98105
name: "allowed configmap",
@@ -115,19 +122,22 @@ func TestNodeAuthorizer(t *testing.T) {
115122
expect: authorizer.DecisionAllow,
116123
},
117124
{
118-
name: "disallowed list many secrets",
119-
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "secrets", Name: "", Namespace: "ns0"},
120-
expect: authorizer.DecisionNoOpinion,
125+
name: "disallowed list many secrets",
126+
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "secrets", Name: "", Namespace: "ns0"},
127+
expect: authorizer.DecisionNoOpinion,
128+
expectReason: "No Object name found,",
121129
},
122130
{
123-
name: "disallowed watch many secrets",
124-
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "secrets", Name: "", Namespace: "ns0"},
125-
expect: authorizer.DecisionNoOpinion,
131+
name: "disallowed watch many secrets",
132+
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "secrets", Name: "", Namespace: "ns0"},
133+
expect: authorizer.DecisionNoOpinion,
134+
expectReason: "No Object name found,",
126135
},
127136
{
128-
name: "disallowed list secrets from all namespaces with name",
129-
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "secrets", Name: "secret0-pod0-node0", Namespace: ""},
130-
expect: authorizer.DecisionNoOpinion,
137+
name: "disallowed list secrets from all namespaces with name",
138+
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "secrets", Name: "secret0-pod0-node0", Namespace: ""},
139+
expect: authorizer.DecisionNoOpinion,
140+
expectReason: "can only read namespaced object of this type",
131141
},
132142
{
133143
name: "allowed shared secret via pod",
@@ -219,6 +229,33 @@ func TestNodeAuthorizer(t *testing.T) {
219229
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "update", Resource: "serviceaccounts", Subresource: "token", Name: "svcacct0-node0", Namespace: "ns0"},
220230
expect: authorizer.DecisionNoOpinion,
221231
},
232+
{
233+
name: "get allowed svcacct via pod - feature enabled",
234+
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "serviceaccounts", Name: "svcacct0-node0", Namespace: "ns0"},
235+
expect: authorizer.DecisionAllow,
236+
features: serviceAccountTokenForCredentialProvidersEnabled,
237+
},
238+
{
239+
name: "disallowed get svcacct via pod - feature disabled",
240+
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "serviceaccounts", Name: "svcacct0-node0", Namespace: "ns0"},
241+
expect: authorizer.DecisionNoOpinion,
242+
features: serviceAccountTokenForCredentialProvidersDisabled,
243+
expectReason: "not allowed to get service accounts",
244+
},
245+
{
246+
name: "disallowed list svcacct via pod - feature disabled",
247+
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "serviceaccounts", Name: "svcacct0-node0", Namespace: "ns0"},
248+
expect: authorizer.DecisionNoOpinion,
249+
features: serviceAccountTokenForCredentialProvidersDisabled,
250+
expectReason: "can only create tokens for individual service accounts",
251+
},
252+
{
253+
name: "disallowed watch svcacct via pod - feature disabled",
254+
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "serviceaccounts", Name: "svcacct0-node0", Namespace: "ns0"},
255+
expect: authorizer.DecisionNoOpinion,
256+
features: serviceAccountTokenForCredentialProvidersDisabled,
257+
expectReason: "can only create tokens for individual service accounts",
258+
},
222259
{
223260
name: "disallowed get lease in namespace other than kube-node-lease - feature enabled",
224261
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "leases", APIGroup: "coordination.k8s.io", Name: "node0", Namespace: "foo"},
@@ -398,10 +435,11 @@ func TestNodeAuthorizer(t *testing.T) {
398435
features: selectorAuthzDisabled,
399436
},
400437
{
401-
name: "disallowed unfiltered list ResourceSlices - selector authz enabled",
402-
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "resourceslices", APIGroup: "resource.k8s.io"},
403-
expect: authorizer.DecisionNoOpinion,
404-
features: selectorAuthzEnabled,
438+
name: "disallowed unfiltered list ResourceSlices - selector authz enabled",
439+
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "resourceslices", APIGroup: "resource.k8s.io"},
440+
expect: authorizer.DecisionNoOpinion,
441+
features: selectorAuthzEnabled,
442+
expectReason: "can only list/watch/deletecollection resourceslices with nodeName field selector",
405443
},
406444
{
407445
name: "allowed filtered watch ResourceSlices",
@@ -415,10 +453,11 @@ func TestNodeAuthorizer(t *testing.T) {
415453
features: selectorAuthzDisabled,
416454
},
417455
{
418-
name: "disallowed unfiltered watch ResourceSlices - selector authz enabled",
419-
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "resourceslices", APIGroup: "resource.k8s.io"},
420-
expect: authorizer.DecisionNoOpinion,
421-
features: selectorAuthzEnabled,
456+
name: "disallowed unfiltered watch ResourceSlices - selector authz enabled",
457+
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "resourceslices", APIGroup: "resource.k8s.io"},
458+
expect: authorizer.DecisionNoOpinion,
459+
features: selectorAuthzEnabled,
460+
expectReason: "can only list/watch/deletecollection resourceslices with nodeName field selector",
422461
},
423462
{
424463
name: "allowed get ResourceSlice",
@@ -460,10 +499,11 @@ func TestNodeAuthorizer(t *testing.T) {
460499
features: selectorAuthzDisabled,
461500
},
462501
{
463-
name: "get unrelated pod - selector enabled",
464-
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "pods", APIGroup: "", Name: "pod0-node1", Namespace: "ns0"},
465-
expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled
466-
features: selectorAuthzEnabled,
502+
name: "get unrelated pod - selector enabled",
503+
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "pods", APIGroup: "", Name: "pod0-node1", Namespace: "ns0"},
504+
expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled
505+
features: selectorAuthzEnabled,
506+
expectReason: "no relationship found between node 'node0' and this object",
467507
},
468508
// list pods
469509
{
@@ -488,10 +528,11 @@ func TestNodeAuthorizer(t *testing.T) {
488528
features: selectorAuthzDisabled,
489529
},
490530
{
491-
name: "list unrelated pods - selector enabled",
492-
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "pods", APIGroup: ""},
493-
expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled
494-
features: selectorAuthzEnabled,
531+
name: "list unrelated pods - selector enabled",
532+
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "pods", APIGroup: ""},
533+
expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled
534+
features: selectorAuthzEnabled,
535+
expectReason: "can only list/watch pods with spec.nodeName field selector",
495536
},
496537
// watch pods
497538
{
@@ -516,10 +557,11 @@ func TestNodeAuthorizer(t *testing.T) {
516557
features: selectorAuthzDisabled,
517558
},
518559
{
519-
name: "watch unrelated pods - selector enabled",
520-
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "pods", APIGroup: ""},
521-
expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled
522-
features: selectorAuthzEnabled,
560+
name: "watch unrelated pods - selector enabled",
561+
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "pods", APIGroup: ""},
562+
expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled
563+
features: selectorAuthzEnabled,
564+
expectReason: "can only list/watch pods with spec.nodeName field selector",
523565
},
524566
// create, delete pods
525567
{
@@ -604,10 +646,11 @@ func TestNodeAuthorizer(t *testing.T) {
604646
features: selectorAuthzDisabled,
605647
},
606648
{
607-
name: "get unrelated pod - selector enabled",
608-
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "nodes", APIGroup: "", Name: "node1"},
609-
expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled
610-
features: selectorAuthzEnabled,
649+
name: "get unrelated pod - selector enabled",
650+
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "nodes", APIGroup: "", Name: "node1"},
651+
expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled
652+
features: selectorAuthzEnabled,
653+
expectReason: "node 'node0' cannot read 'node1', only its own Node object",
611654
},
612655
// list nodes
613656
{
@@ -622,10 +665,11 @@ func TestNodeAuthorizer(t *testing.T) {
622665
features: selectorAuthzDisabled,
623666
},
624667
{
625-
name: "list single unrelated node - selector enabled",
626-
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "nodes", APIGroup: "", Name: "node1"},
627-
expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled
628-
features: selectorAuthzEnabled,
668+
name: "list single unrelated node - selector enabled",
669+
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "nodes", APIGroup: "", Name: "node1"},
670+
expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled
671+
features: selectorAuthzEnabled,
672+
expectReason: "node 'node0' cannot read 'node1', only its own Node object",
629673
},
630674
{
631675
name: "list all nodes - selector disabled",
@@ -634,10 +678,11 @@ func TestNodeAuthorizer(t *testing.T) {
634678
features: selectorAuthzDisabled,
635679
},
636680
{
637-
name: "list all nodes - selector enabled",
638-
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "nodes", APIGroup: ""},
639-
expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled
640-
features: selectorAuthzEnabled,
681+
name: "list all nodes - selector enabled",
682+
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "nodes", APIGroup: ""},
683+
expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled
684+
features: selectorAuthzEnabled,
685+
expectReason: "node 'node0' cannot read all nodes, only its own Node object",
641686
},
642687
// watch nodes
643688
{
@@ -652,10 +697,11 @@ func TestNodeAuthorizer(t *testing.T) {
652697
features: selectorAuthzDisabled,
653698
},
654699
{
655-
name: "watch single unrelated node - selector enabled",
656-
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "nodes", APIGroup: "", Name: "node1"},
657-
expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled
658-
features: selectorAuthzEnabled,
700+
name: "watch single unrelated node - selector enabled",
701+
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "nodes", APIGroup: "", Name: "node1"},
702+
expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled
703+
features: selectorAuthzEnabled,
704+
expectReason: "node 'node0' cannot read 'node1', only its own Node object",
659705
},
660706
{
661707
name: "watch all nodes - selector disabled",
@@ -664,10 +710,11 @@ func TestNodeAuthorizer(t *testing.T) {
664710
features: selectorAuthzDisabled,
665711
},
666712
{
667-
name: "watch all nodes - selector enabled",
668-
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "nodes", APIGroup: ""},
669-
expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled
670-
features: selectorAuthzEnabled,
713+
name: "watch all nodes - selector enabled",
714+
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "nodes", APIGroup: ""},
715+
expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled
716+
features: selectorAuthzEnabled,
717+
expectReason: "node 'node0' cannot read all nodes, only its own Node object",
671718
},
672719
// create nodes
673720
{
@@ -737,6 +784,9 @@ func TestNodeAuthorizer(t *testing.T) {
737784
if decision != tc.expect {
738785
t.Errorf("expected %v, got %v (%s)", tc.expect, decision, reason)
739786
}
787+
if reason != tc.expectReason {
788+
t.Errorf("expected reason %q, got %q", tc.expectReason, reason)
789+
}
740790
})
741791
}
742792
}

‎plugin/pkg/auth/authorizer/rbac/bootstrappolicy/policy.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,11 @@ func NodeRules() []rbacv1.PolicyRule {
189189
if utilfeature.DefaultFeatureGate.Enabled(features.ClusterTrustBundle) {
190190
nodePolicyRules = append(nodePolicyRules, rbacv1helpers.NewRule("get", "list", "watch").Groups(certificatesGroup).Resources("clustertrustbundles").RuleOrDie())
191191
}
192+
// Kubelet needs access to ServiceAccounts to support sending service account tokens to the credential provider.
193+
// Use the Node authorizer to limit get to service accounts related to the node.
194+
if utilfeature.DefaultFeatureGate.Enabled(features.KubeletServiceAccountTokenForCredentialProviders) {
195+
nodePolicyRules = append(nodePolicyRules, rbacv1helpers.NewRule("get").Groups(legacyGroup).Resources("serviceaccounts").RuleOrDie())
196+
}
192197

193198
return nodePolicyRules
194199
}

0 commit comments

Comments
 (0)