diff --git a/plugin/pkg/auth/authorizer/node/node_authorizer.go b/plugin/pkg/auth/authorizer/node/node_authorizer.go index 2a1be5b56cc..29c480fc88f 100644 --- a/plugin/pkg/auth/authorizer/node/node_authorizer.go +++ b/plugin/pkg/auth/authorizer/node/node_authorizer.go @@ -135,7 +135,7 @@ func (r *NodeAuthorizer) Authorize(ctx context.Context, attrs authorizer.Attribu case vaResource: return r.authorizeGet(nodeName, vaVertexType, attrs) case svcAcctResource: - return r.authorizeCreateToken(nodeName, serviceAccountVertexType, attrs) + return r.authorizeServiceAccount(nodeName, attrs) case leaseResource: return r.authorizeLease(nodeName, attrs) case csiNodeResource: @@ -196,7 +196,7 @@ func (r *NodeAuthorizer) authorizeGet(nodeName string, startingType vertexType, func (r *NodeAuthorizer) authorizeReadNamespacedObject(nodeName string, startingType vertexType, attrs authorizer.Attributes) (authorizer.Decision, string, error) { switch attrs.GetVerb() { case "get", "list", "watch": - //ok + // ok default: klog.V(2).Infof("NODE DENY: '%s' %#v", nodeName, attrs) return authorizer.DecisionNoOpinion, "can only read resources of this type", nil @@ -231,6 +231,23 @@ func (r *NodeAuthorizer) authorize(nodeName string, startingType vertexType, att return authorizer.DecisionAllow, "", nil } +// authorizeServiceAccount authorizes +// - "get" requests to serviceaccounts when KubeletServiceAccountTokenForCredentialProviders feature is enabled +// - "create" requests to serviceaccounts 'token' subresource of pods running on a node +func (r *NodeAuthorizer) authorizeServiceAccount(nodeName string, attrs authorizer.Attributes) (authorizer.Decision, string, error) { + verb := attrs.GetVerb() + + if verb == "get" && attrs.GetSubresource() == "" { + if !r.features.Enabled(features.KubeletServiceAccountTokenForCredentialProviders) { + klog.V(2).Infof("NODE DENY: '%s' %#v", nodeName, attrs) + return authorizer.DecisionNoOpinion, "not allowed to get service accounts", nil + } + return r.authorizeReadNamespacedObject(nodeName, serviceAccountVertexType, attrs) + } + + return r.authorizeCreateToken(nodeName, serviceAccountVertexType, attrs) +} + // authorizeCreateToken authorizes "create" requests to serviceaccounts 'token' // subresource of pods running on a node 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 verb := attrs.GetVerb() switch verb { case "get", "create", "update", "patch", "delete": - //ok + // ok default: klog.V(2).Infof("NODE DENY: '%s' %#v", nodeName, attrs) 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 verb := attrs.GetVerb() switch verb { case "get", "create", "update", "patch", "delete": - //ok + // ok default: klog.V(2).Infof("NODE DENY: '%s' %#v", nodeName, attrs) return authorizer.DecisionNoOpinion, "can only get, create, update, patch, or delete a CSINode", nil diff --git a/plugin/pkg/auth/authorizer/node/node_authorizer_test.go b/plugin/pkg/auth/authorizer/node/node_authorizer_test.go index ba3fe9e7b6c..d8495f4c0c4 100644 --- a/plugin/pkg/auth/authorizer/node/node_authorizer_test.go +++ b/plugin/pkg/auth/authorizer/node/node_authorizer_test.go @@ -80,6 +80,12 @@ func TestNodeAuthorizer(t *testing.T) { featuregatetesting.SetFeatureGateDuringTest(t, selectorAuthzEnabled, genericfeatures.AuthorizeWithSelectors, true) featuregatetesting.SetFeatureGateDuringTest(t, selectorAuthzEnabled, features.AuthorizeNodeWithSelectors, true) + serviceAccountTokenForCredentialProvidersDisabled := utilfeature.DefaultFeatureGate.DeepCopy() + featuregatetesting.SetFeatureGateDuringTest(t, serviceAccountTokenForCredentialProvidersDisabled, features.KubeletServiceAccountTokenForCredentialProviders, false) + + serviceAccountTokenForCredentialProvidersEnabled := utilfeature.DefaultFeatureGate.DeepCopy() + featuregatetesting.SetFeatureGateDuringTest(t, serviceAccountTokenForCredentialProvidersEnabled, features.KubeletServiceAccountTokenForCredentialProviders, true) + featureVariants := []struct { suffix string features featuregate.FeatureGate @@ -89,10 +95,11 @@ func TestNodeAuthorizer(t *testing.T) { } tests := []struct { - name string - attrs authorizer.AttributesRecord - expect authorizer.Decision - features featuregate.FeatureGate + name string + attrs authorizer.AttributesRecord + expect authorizer.Decision + expectReason string + features featuregate.FeatureGate }{ { name: "allowed configmap", @@ -115,19 +122,22 @@ func TestNodeAuthorizer(t *testing.T) { expect: authorizer.DecisionAllow, }, { - name: "disallowed list many secrets", - attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "secrets", Name: "", Namespace: "ns0"}, - expect: authorizer.DecisionNoOpinion, + name: "disallowed list many secrets", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "secrets", Name: "", Namespace: "ns0"}, + expect: authorizer.DecisionNoOpinion, + expectReason: "No Object name found,", }, { - name: "disallowed watch many secrets", - attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "secrets", Name: "", Namespace: "ns0"}, - expect: authorizer.DecisionNoOpinion, + name: "disallowed watch many secrets", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "secrets", Name: "", Namespace: "ns0"}, + expect: authorizer.DecisionNoOpinion, + expectReason: "No Object name found,", }, { - name: "disallowed list secrets from all namespaces with name", - attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "secrets", Name: "secret0-pod0-node0", Namespace: ""}, - expect: authorizer.DecisionNoOpinion, + name: "disallowed list secrets from all namespaces with name", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "secrets", Name: "secret0-pod0-node0", Namespace: ""}, + expect: authorizer.DecisionNoOpinion, + expectReason: "can only read namespaced object of this type", }, { name: "allowed shared secret via pod", @@ -219,6 +229,33 @@ func TestNodeAuthorizer(t *testing.T) { attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "update", Resource: "serviceaccounts", Subresource: "token", Name: "svcacct0-node0", Namespace: "ns0"}, expect: authorizer.DecisionNoOpinion, }, + { + name: "get allowed svcacct via pod - feature enabled", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "serviceaccounts", Name: "svcacct0-node0", Namespace: "ns0"}, + expect: authorizer.DecisionAllow, + features: serviceAccountTokenForCredentialProvidersEnabled, + }, + { + name: "disallowed get svcacct via pod - feature disabled", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "serviceaccounts", Name: "svcacct0-node0", Namespace: "ns0"}, + expect: authorizer.DecisionNoOpinion, + features: serviceAccountTokenForCredentialProvidersDisabled, + expectReason: "not allowed to get service accounts", + }, + { + name: "disallowed list svcacct via pod - feature disabled", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "serviceaccounts", Name: "svcacct0-node0", Namespace: "ns0"}, + expect: authorizer.DecisionNoOpinion, + features: serviceAccountTokenForCredentialProvidersDisabled, + expectReason: "can only create tokens for individual service accounts", + }, + { + name: "disallowed watch svcacct via pod - feature disabled", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "serviceaccounts", Name: "svcacct0-node0", Namespace: "ns0"}, + expect: authorizer.DecisionNoOpinion, + features: serviceAccountTokenForCredentialProvidersDisabled, + expectReason: "can only create tokens for individual service accounts", + }, { name: "disallowed get lease in namespace other than kube-node-lease - feature enabled", 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) { features: selectorAuthzDisabled, }, { - name: "disallowed unfiltered list ResourceSlices - selector authz enabled", - attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "resourceslices", APIGroup: "resource.k8s.io"}, - expect: authorizer.DecisionNoOpinion, - features: selectorAuthzEnabled, + name: "disallowed unfiltered list ResourceSlices - selector authz enabled", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "resourceslices", APIGroup: "resource.k8s.io"}, + expect: authorizer.DecisionNoOpinion, + features: selectorAuthzEnabled, + expectReason: "can only list/watch/deletecollection resourceslices with nodeName field selector", }, { name: "allowed filtered watch ResourceSlices", @@ -415,10 +453,11 @@ func TestNodeAuthorizer(t *testing.T) { features: selectorAuthzDisabled, }, { - name: "disallowed unfiltered watch ResourceSlices - selector authz enabled", - attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "resourceslices", APIGroup: "resource.k8s.io"}, - expect: authorizer.DecisionNoOpinion, - features: selectorAuthzEnabled, + name: "disallowed unfiltered watch ResourceSlices - selector authz enabled", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "resourceslices", APIGroup: "resource.k8s.io"}, + expect: authorizer.DecisionNoOpinion, + features: selectorAuthzEnabled, + expectReason: "can only list/watch/deletecollection resourceslices with nodeName field selector", }, { name: "allowed get ResourceSlice", @@ -460,10 +499,11 @@ func TestNodeAuthorizer(t *testing.T) { features: selectorAuthzDisabled, }, { - name: "get unrelated pod - selector enabled", - attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "pods", APIGroup: "", Name: "pod0-node1", Namespace: "ns0"}, - expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled - features: selectorAuthzEnabled, + name: "get unrelated pod - selector enabled", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "pods", APIGroup: "", Name: "pod0-node1", Namespace: "ns0"}, + expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled + features: selectorAuthzEnabled, + expectReason: "no relationship found between node 'node0' and this object", }, // list pods { @@ -488,10 +528,11 @@ func TestNodeAuthorizer(t *testing.T) { features: selectorAuthzDisabled, }, { - name: "list unrelated pods - selector enabled", - attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "pods", APIGroup: ""}, - expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled - features: selectorAuthzEnabled, + name: "list unrelated pods - selector enabled", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "pods", APIGroup: ""}, + expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled + features: selectorAuthzEnabled, + expectReason: "can only list/watch pods with spec.nodeName field selector", }, // watch pods { @@ -516,10 +557,11 @@ func TestNodeAuthorizer(t *testing.T) { features: selectorAuthzDisabled, }, { - name: "watch unrelated pods - selector enabled", - attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "pods", APIGroup: ""}, - expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled - features: selectorAuthzEnabled, + name: "watch unrelated pods - selector enabled", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "pods", APIGroup: ""}, + expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled + features: selectorAuthzEnabled, + expectReason: "can only list/watch pods with spec.nodeName field selector", }, // create, delete pods { @@ -604,10 +646,11 @@ func TestNodeAuthorizer(t *testing.T) { features: selectorAuthzDisabled, }, { - name: "get unrelated pod - selector enabled", - attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "nodes", APIGroup: "", Name: "node1"}, - expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled - features: selectorAuthzEnabled, + name: "get unrelated pod - selector enabled", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "nodes", APIGroup: "", Name: "node1"}, + expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled + features: selectorAuthzEnabled, + expectReason: "node 'node0' cannot read 'node1', only its own Node object", }, // list nodes { @@ -622,10 +665,11 @@ func TestNodeAuthorizer(t *testing.T) { features: selectorAuthzDisabled, }, { - name: "list single unrelated node - selector enabled", - attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "nodes", APIGroup: "", Name: "node1"}, - expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled - features: selectorAuthzEnabled, + name: "list single unrelated node - selector enabled", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "nodes", APIGroup: "", Name: "node1"}, + expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled + features: selectorAuthzEnabled, + expectReason: "node 'node0' cannot read 'node1', only its own Node object", }, { name: "list all nodes - selector disabled", @@ -634,10 +678,11 @@ func TestNodeAuthorizer(t *testing.T) { features: selectorAuthzDisabled, }, { - name: "list all nodes - selector enabled", - attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "nodes", APIGroup: ""}, - expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled - features: selectorAuthzEnabled, + name: "list all nodes - selector enabled", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "nodes", APIGroup: ""}, + expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled + features: selectorAuthzEnabled, + expectReason: "node 'node0' cannot read all nodes, only its own Node object", }, // watch nodes { @@ -652,10 +697,11 @@ func TestNodeAuthorizer(t *testing.T) { features: selectorAuthzDisabled, }, { - name: "watch single unrelated node - selector enabled", - attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "nodes", APIGroup: "", Name: "node1"}, - expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled - features: selectorAuthzEnabled, + name: "watch single unrelated node - selector enabled", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "nodes", APIGroup: "", Name: "node1"}, + expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled + features: selectorAuthzEnabled, + expectReason: "node 'node0' cannot read 'node1', only its own Node object", }, { name: "watch all nodes - selector disabled", @@ -664,10 +710,11 @@ func TestNodeAuthorizer(t *testing.T) { features: selectorAuthzDisabled, }, { - name: "watch all nodes - selector enabled", - attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "nodes", APIGroup: ""}, - expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled - features: selectorAuthzEnabled, + name: "watch all nodes - selector enabled", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "nodes", APIGroup: ""}, + expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled + features: selectorAuthzEnabled, + expectReason: "node 'node0' cannot read all nodes, only its own Node object", }, // create nodes { @@ -737,6 +784,9 @@ func TestNodeAuthorizer(t *testing.T) { if decision != tc.expect { t.Errorf("expected %v, got %v (%s)", tc.expect, decision, reason) } + if reason != tc.expectReason { + t.Errorf("expected reason %q, got %q", tc.expectReason, reason) + } }) } } diff --git a/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/policy.go b/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/policy.go index 947d043de46..87a60e37efe 100644 --- a/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/policy.go +++ b/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/policy.go @@ -189,6 +189,11 @@ func NodeRules() []rbacv1.PolicyRule { if utilfeature.DefaultFeatureGate.Enabled(features.ClusterTrustBundle) { nodePolicyRules = append(nodePolicyRules, rbacv1helpers.NewRule("get", "list", "watch").Groups(certificatesGroup).Resources("clustertrustbundles").RuleOrDie()) } + // Kubelet needs access to ServiceAccounts to support sending service account tokens to the credential provider. + // Use the Node authorizer to limit get to service accounts related to the node. + if utilfeature.DefaultFeatureGate.Enabled(features.KubeletServiceAccountTokenForCredentialProviders) { + nodePolicyRules = append(nodePolicyRules, rbacv1helpers.NewRule("get").Groups(legacyGroup).Resources("serviceaccounts").RuleOrDie()) + } return nodePolicyRules }