node authorizer changes to allow read on svcaccounts

Signed-off-by: Anish Ramasekar <anish.ramasekar@gmail.com>
This commit is contained in:
Anish Ramasekar 2024-11-05 11:04:45 -08:00
parent d398de294d
commit 6defd8c0bd
No known key found for this signature in database
GPG Key ID: E96F745A34A409C2
3 changed files with 129 additions and 57 deletions

View File

@ -135,7 +135,7 @@ func (r *NodeAuthorizer) Authorize(ctx context.Context, attrs authorizer.Attribu
case vaResource: case vaResource:
return r.authorizeGet(nodeName, vaVertexType, attrs) return r.authorizeGet(nodeName, vaVertexType, attrs)
case svcAcctResource: case svcAcctResource:
return r.authorizeCreateToken(nodeName, serviceAccountVertexType, attrs) return r.authorizeServiceAccount(nodeName, attrs)
case leaseResource: case leaseResource:
return r.authorizeLease(nodeName, attrs) return r.authorizeLease(nodeName, attrs)
case csiNodeResource: 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) { func (r *NodeAuthorizer) authorizeReadNamespacedObject(nodeName string, startingType vertexType, attrs authorizer.Attributes) (authorizer.Decision, string, error) {
switch attrs.GetVerb() { switch attrs.GetVerb() {
case "get", "list", "watch": case "get", "list", "watch":
//ok // ok
default: default:
klog.V(2).Infof("NODE DENY: '%s' %#v", nodeName, attrs) klog.V(2).Infof("NODE DENY: '%s' %#v", nodeName, attrs)
return authorizer.DecisionNoOpinion, "can only read resources of this type", nil 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 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' // authorizeCreateToken authorizes "create" requests to serviceaccounts 'token'
// subresource of pods running on a node // subresource of pods running on a node
func (r *NodeAuthorizer) authorizeCreateToken(nodeName string, startingType vertexType, attrs authorizer.Attributes) (authorizer.Decision, string, error) { 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() verb := attrs.GetVerb()
switch verb { switch verb {
case "get", "create", "update", "patch", "delete": case "get", "create", "update", "patch", "delete":
//ok // ok
default: default:
klog.V(2).Infof("NODE DENY: '%s' %#v", nodeName, attrs) 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 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() verb := attrs.GetVerb()
switch verb { switch verb {
case "get", "create", "update", "patch", "delete": case "get", "create", "update", "patch", "delete":
//ok // ok
default: default:
klog.V(2).Infof("NODE DENY: '%s' %#v", nodeName, attrs) klog.V(2).Infof("NODE DENY: '%s' %#v", nodeName, attrs)
return authorizer.DecisionNoOpinion, "can only get, create, update, patch, or delete a CSINode", nil return authorizer.DecisionNoOpinion, "can only get, create, update, patch, or delete a CSINode", nil

View File

@ -80,6 +80,12 @@ func TestNodeAuthorizer(t *testing.T) {
featuregatetesting.SetFeatureGateDuringTest(t, selectorAuthzEnabled, genericfeatures.AuthorizeWithSelectors, true) featuregatetesting.SetFeatureGateDuringTest(t, selectorAuthzEnabled, genericfeatures.AuthorizeWithSelectors, true)
featuregatetesting.SetFeatureGateDuringTest(t, selectorAuthzEnabled, features.AuthorizeNodeWithSelectors, 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 { featureVariants := []struct {
suffix string suffix string
features featuregate.FeatureGate features featuregate.FeatureGate
@ -89,10 +95,11 @@ func TestNodeAuthorizer(t *testing.T) {
} }
tests := []struct { tests := []struct {
name string name string
attrs authorizer.AttributesRecord attrs authorizer.AttributesRecord
expect authorizer.Decision expect authorizer.Decision
features featuregate.FeatureGate expectReason string
features featuregate.FeatureGate
}{ }{
{ {
name: "allowed configmap", name: "allowed configmap",
@ -115,19 +122,22 @@ func TestNodeAuthorizer(t *testing.T) {
expect: authorizer.DecisionAllow, expect: authorizer.DecisionAllow,
}, },
{ {
name: "disallowed list many secrets", name: "disallowed list many secrets",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "secrets", Name: "", Namespace: "ns0"}, attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "secrets", Name: "", Namespace: "ns0"},
expect: authorizer.DecisionNoOpinion, expect: authorizer.DecisionNoOpinion,
expectReason: "No Object name found,",
}, },
{ {
name: "disallowed watch many secrets", name: "disallowed watch many secrets",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "secrets", Name: "", Namespace: "ns0"}, attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "secrets", Name: "", Namespace: "ns0"},
expect: authorizer.DecisionNoOpinion, expect: authorizer.DecisionNoOpinion,
expectReason: "No Object name found,",
}, },
{ {
name: "disallowed list secrets from all namespaces with name", 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: ""}, attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "secrets", Name: "secret0-pod0-node0", Namespace: ""},
expect: authorizer.DecisionNoOpinion, expect: authorizer.DecisionNoOpinion,
expectReason: "can only read namespaced object of this type",
}, },
{ {
name: "allowed shared secret via pod", 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"}, attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "update", Resource: "serviceaccounts", Subresource: "token", Name: "svcacct0-node0", Namespace: "ns0"},
expect: authorizer.DecisionNoOpinion, 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", 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"}, 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, features: selectorAuthzDisabled,
}, },
{ {
name: "disallowed unfiltered list ResourceSlices - selector authz enabled", name: "disallowed unfiltered list ResourceSlices - selector authz enabled",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "resourceslices", APIGroup: "resource.k8s.io"}, attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "resourceslices", APIGroup: "resource.k8s.io"},
expect: authorizer.DecisionNoOpinion, expect: authorizer.DecisionNoOpinion,
features: selectorAuthzEnabled, features: selectorAuthzEnabled,
expectReason: "can only list/watch/deletecollection resourceslices with nodeName field selector",
}, },
{ {
name: "allowed filtered watch ResourceSlices", name: "allowed filtered watch ResourceSlices",
@ -415,10 +453,11 @@ func TestNodeAuthorizer(t *testing.T) {
features: selectorAuthzDisabled, features: selectorAuthzDisabled,
}, },
{ {
name: "disallowed unfiltered watch ResourceSlices - selector authz enabled", name: "disallowed unfiltered watch ResourceSlices - selector authz enabled",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "resourceslices", APIGroup: "resource.k8s.io"}, attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "resourceslices", APIGroup: "resource.k8s.io"},
expect: authorizer.DecisionNoOpinion, expect: authorizer.DecisionNoOpinion,
features: selectorAuthzEnabled, features: selectorAuthzEnabled,
expectReason: "can only list/watch/deletecollection resourceslices with nodeName field selector",
}, },
{ {
name: "allowed get ResourceSlice", name: "allowed get ResourceSlice",
@ -460,10 +499,11 @@ func TestNodeAuthorizer(t *testing.T) {
features: selectorAuthzDisabled, features: selectorAuthzDisabled,
}, },
{ {
name: "get unrelated pod - selector enabled", name: "get unrelated pod - selector enabled",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "pods", APIGroup: "", Name: "pod0-node1", Namespace: "ns0"}, 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 expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled
features: selectorAuthzEnabled, features: selectorAuthzEnabled,
expectReason: "no relationship found between node 'node0' and this object",
}, },
// list pods // list pods
{ {
@ -488,10 +528,11 @@ func TestNodeAuthorizer(t *testing.T) {
features: selectorAuthzDisabled, features: selectorAuthzDisabled,
}, },
{ {
name: "list unrelated pods - selector enabled", name: "list unrelated pods - selector enabled",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "pods", APIGroup: ""}, attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "pods", APIGroup: ""},
expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled
features: selectorAuthzEnabled, features: selectorAuthzEnabled,
expectReason: "can only list/watch pods with spec.nodeName field selector",
}, },
// watch pods // watch pods
{ {
@ -516,10 +557,11 @@ func TestNodeAuthorizer(t *testing.T) {
features: selectorAuthzDisabled, features: selectorAuthzDisabled,
}, },
{ {
name: "watch unrelated pods - selector enabled", name: "watch unrelated pods - selector enabled",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "pods", APIGroup: ""}, attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "pods", APIGroup: ""},
expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled
features: selectorAuthzEnabled, features: selectorAuthzEnabled,
expectReason: "can only list/watch pods with spec.nodeName field selector",
}, },
// create, delete pods // create, delete pods
{ {
@ -604,10 +646,11 @@ func TestNodeAuthorizer(t *testing.T) {
features: selectorAuthzDisabled, features: selectorAuthzDisabled,
}, },
{ {
name: "get unrelated pod - selector enabled", name: "get unrelated pod - selector enabled",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "nodes", APIGroup: "", Name: "node1"}, attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "nodes", APIGroup: "", Name: "node1"},
expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled
features: selectorAuthzEnabled, features: selectorAuthzEnabled,
expectReason: "node 'node0' cannot read 'node1', only its own Node object",
}, },
// list nodes // list nodes
{ {
@ -622,10 +665,11 @@ func TestNodeAuthorizer(t *testing.T) {
features: selectorAuthzDisabled, features: selectorAuthzDisabled,
}, },
{ {
name: "list single unrelated node - selector enabled", name: "list single unrelated node - selector enabled",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "nodes", APIGroup: "", Name: "node1"}, attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "nodes", APIGroup: "", Name: "node1"},
expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled
features: selectorAuthzEnabled, features: selectorAuthzEnabled,
expectReason: "node 'node0' cannot read 'node1', only its own Node object",
}, },
{ {
name: "list all nodes - selector disabled", name: "list all nodes - selector disabled",
@ -634,10 +678,11 @@ func TestNodeAuthorizer(t *testing.T) {
features: selectorAuthzDisabled, features: selectorAuthzDisabled,
}, },
{ {
name: "list all nodes - selector enabled", name: "list all nodes - selector enabled",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "nodes", APIGroup: ""}, attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "nodes", APIGroup: ""},
expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled
features: selectorAuthzEnabled, features: selectorAuthzEnabled,
expectReason: "node 'node0' cannot read all nodes, only its own Node object",
}, },
// watch nodes // watch nodes
{ {
@ -652,10 +697,11 @@ func TestNodeAuthorizer(t *testing.T) {
features: selectorAuthzDisabled, features: selectorAuthzDisabled,
}, },
{ {
name: "watch single unrelated node - selector enabled", name: "watch single unrelated node - selector enabled",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "nodes", APIGroup: "", Name: "node1"}, attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "nodes", APIGroup: "", Name: "node1"},
expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled
features: selectorAuthzEnabled, features: selectorAuthzEnabled,
expectReason: "node 'node0' cannot read 'node1', only its own Node object",
}, },
{ {
name: "watch all nodes - selector disabled", name: "watch all nodes - selector disabled",
@ -664,10 +710,11 @@ func TestNodeAuthorizer(t *testing.T) {
features: selectorAuthzDisabled, features: selectorAuthzDisabled,
}, },
{ {
name: "watch all nodes - selector enabled", name: "watch all nodes - selector enabled",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "nodes", APIGroup: ""}, attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "nodes", APIGroup: ""},
expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled expect: authorizer.DecisionNoOpinion, // stricter with selector authz enabled
features: selectorAuthzEnabled, features: selectorAuthzEnabled,
expectReason: "node 'node0' cannot read all nodes, only its own Node object",
}, },
// create nodes // create nodes
{ {
@ -737,6 +784,9 @@ func TestNodeAuthorizer(t *testing.T) {
if decision != tc.expect { if decision != tc.expect {
t.Errorf("expected %v, got %v (%s)", tc.expect, decision, reason) 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)
}
}) })
} }
} }

View File

@ -189,6 +189,11 @@ func NodeRules() []rbacv1.PolicyRule {
if utilfeature.DefaultFeatureGate.Enabled(features.ClusterTrustBundle) { if utilfeature.DefaultFeatureGate.Enabled(features.ClusterTrustBundle) {
nodePolicyRules = append(nodePolicyRules, rbacv1helpers.NewRule("get", "list", "watch").Groups(certificatesGroup).Resources("clustertrustbundles").RuleOrDie()) 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 return nodePolicyRules
} }