diff --git a/pkg/controlplane/apiserver/options/validation.go b/pkg/controlplane/apiserver/options/validation.go index cca855be43f..4838d1f29ef 100644 --- a/pkg/controlplane/apiserver/options/validation.go +++ b/pkg/controlplane/apiserver/options/validation.go @@ -70,6 +70,13 @@ func validateAPIPriorityAndFairness(options *Options) []error { return nil } +func validateNodeSelectorAuthorizationFeature() []error { + if utilfeature.DefaultFeatureGate.Enabled(features.AuthorizeNodeWithSelectors) && !utilfeature.DefaultFeatureGate.Enabled(genericfeatures.AuthorizeWithSelectors) { + return []error{fmt.Errorf("AuthorizeNodeWithSelectors feature requires AuthorizeWithSelectors feature to be enabled")} + } + return nil +} + func validateUnknownVersionInteroperabilityProxyFeature() []error { if utilfeature.DefaultFeatureGate.Enabled(features.UnknownVersionInteroperabilityProxy) { if utilfeature.DefaultFeatureGate.Enabled(genericfeatures.StorageVersionAPI) { @@ -113,6 +120,7 @@ func (s *Options) Validate() []error { errs = append(errs, s.Metrics.Validate()...) errs = append(errs, validateUnknownVersionInteroperabilityProxyFeature()...) errs = append(errs, validateUnknownVersionInteroperabilityProxyFlags(s)...) + errs = append(errs, validateNodeSelectorAuthorizationFeature()...) return errs } diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 51eb8fed5e4..93c98a97cd5 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -74,6 +74,14 @@ const ( // beta: v1.30 AppArmorFields featuregate.Feature = "AppArmorFields" + // owner: @liggitt + // kep: + // alpha: v1.31 + // + // Make the Node authorizer use fine-grained selector authorization. + // Requires AuthorizeWithSelectors to be enabled. + AuthorizeNodeWithSelectors featuregate.Feature = "AuthorizeNodeWithSelectors" + // owner: @danwinship // alpha: v1.27 // beta: v1.29 @@ -1008,6 +1016,8 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS AppArmorFields: {Default: true, PreRelease: featuregate.Beta}, + AuthorizeNodeWithSelectors: {Default: false, PreRelease: featuregate.Alpha}, + CloudDualStackNodeIPs: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.32 ClusterTrustBundle: {Default: false, PreRelease: featuregate.Alpha}, diff --git a/plugin/pkg/auth/authorizer/node/node_authorizer.go b/plugin/pkg/auth/authorizer/node/node_authorizer.go index 28d76af2dca..aac48afb026 100644 --- a/plugin/pkg/auth/authorizer/node/node_authorizer.go +++ b/plugin/pkg/auth/authorizer/node/node_authorizer.go @@ -24,6 +24,7 @@ import ( rbacv1 "k8s.io/api/rbac/v1" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/selection" "k8s.io/apiserver/pkg/authentication/user" "k8s.io/apiserver/pkg/authorization/authorizer" utilfeature "k8s.io/apiserver/pkg/util/feature" @@ -33,6 +34,7 @@ import ( resourceapi "k8s.io/kubernetes/pkg/apis/resource" storageapi "k8s.io/kubernetes/pkg/apis/storage" "k8s.io/kubernetes/pkg/auth/nodeidentifier" + "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/plugin/pkg/auth/authorizer/rbac" "k8s.io/kubernetes/third_party/forked/gonum/graph" "k8s.io/kubernetes/third_party/forked/gonum/graph/traverse" @@ -81,6 +83,8 @@ func NewAuthorizer(graph *Graph, identifier nodeidentifier.NodeIdentifier, rules var ( configMapResource = api.Resource("configmaps") secretResource = api.Resource("secrets") + podResource = api.Resource("pods") + nodeResource = api.Resource("nodes") resourceSlice = resourceapi.Resource("resourceslices") pvcResource = api.Resource("persistentvolumeclaims") pvResource = api.Resource("persistentvolumes") @@ -138,8 +142,15 @@ func (r *NodeAuthorizer) Authorize(ctx context.Context, attrs authorizer.Attribu return r.authorizeCSINode(nodeName, attrs) case resourceSlice: return r.authorizeResourceSlice(nodeName, attrs) + case nodeResource: + if r.features.Enabled(features.AuthorizeNodeWithSelectors) { + return r.authorizeNode(nodeName, attrs) + } + case podResource: + if r.features.Enabled(features.AuthorizeNodeWithSelectors) { + return r.authorizePod(nodeName, attrs) + } } - } // Access to other resources is not subdivided, so just evaluate against the statically defined node rules @@ -323,22 +334,104 @@ func (r *NodeAuthorizer) authorizeResourceSlice(nodeName string, attrs authorize // is allowed by recording a graph edge. return r.authorize(nodeName, sliceVertexType, attrs) case "watch", "list", "deletecollection": - // Okay. The kubelet is trusted to use a filter for its own objects in watch and list. - // The NodeRestriction admission plugin (plugin/pkg/admission/noderestriction) - // ensures that the node is not deleting some ResourceSlice belonging to - // some other node. - // - // TODO (https://github.com/kubernetes/kubernetes/issues/125355): - // Once https://github.com/kubernetes/enhancements/pull/4600 is implemented, - // this code needs to be extended to verify that the node filter is indeed set. - // Then the admission check can be removed. - return authorizer.DecisionAllow, "", nil + if r.features.Enabled(features.AuthorizeNodeWithSelectors) { + // only allow a scoped fieldSelector + reqs, _ := attrs.GetFieldSelector() + for _, req := range reqs { + if req.Field == "nodeName" && req.Operator == selection.Equals && req.Value == nodeName { + return authorizer.DecisionAllow, "", nil + } + } + // deny otherwise + klog.V(2).Infof("NODE DENY: '%s' %#v", nodeName, attrs) + return authorizer.DecisionNoOpinion, "can only list/watch/deletecollection resourceslices with nodeName field selector", nil + } else { + // Allow broad list/watch access if AuthorizeNodeWithSelectors is not enabled. + // + // The NodeRestriction admission plugin (plugin/pkg/admission/noderestriction) + // ensures that the node is not deleting some ResourceSlice belonging to + // some other node. + return authorizer.DecisionAllow, "", nil + } default: klog.V(2).Infof("NODE DENY: '%s' %#v", nodeName, attrs) return authorizer.DecisionNoOpinion, "only the following verbs are allowed for a ResourceSlice: get, watch, list, create, update, patch, delete, deletecollection", nil } } +// authorizeNode authorizes node requests to Node API objects +func (r *NodeAuthorizer) authorizeNode(nodeName string, attrs authorizer.Attributes) (authorizer.Decision, string, error) { + switch attrs.GetSubresource() { + case "": + switch attrs.GetVerb() { + case "create", "update", "patch": + // Use the NodeRestriction admission plugin to limit a node to creating/updating its own API object. + return authorizer.DecisionAllow, "", nil + case "get", "list", "watch": + return r.authorize(nodeName, nodeVertexType, attrs) + } + case "status": + switch attrs.GetVerb() { + case "update", "patch": + // Use the NodeRestriction admission plugin to limit a node to updating its own Node status. + return authorizer.DecisionAllow, "", nil + } + } + + klog.V(2).Infof("NODE DENY: '%s' %#v", nodeName, attrs) + return authorizer.DecisionNoOpinion, "", nil +} + +// authorizePod authorizes node requests to Pod API objects +func (r *NodeAuthorizer) authorizePod(nodeName string, attrs authorizer.Attributes) (authorizer.Decision, string, error) { + switch attrs.GetSubresource() { + case "": + switch attrs.GetVerb() { + case "get": + return r.authorizeGet(nodeName, podVertexType, attrs) + + case "list", "watch": + // allow a scoped fieldSelector + reqs, _ := attrs.GetFieldSelector() + for _, req := range reqs { + if req.Field == "spec.nodeName" && req.Operator == selection.Equals && req.Value == nodeName { + return authorizer.DecisionAllow, "", nil + } + } + // allow a read of a single pod known to be related to the node + if attrs.GetName() != "" { + return r.authorize(nodeName, podVertexType, attrs) + } + // deny otherwise + klog.V(2).Infof("NODE DENY: '%s' %#v", nodeName, attrs) + return authorizer.DecisionNoOpinion, "can only list/watch pods with spec.nodeName field selector", nil + + case "create", "delete": + // Needed for the node to create/delete mirror pods. + // Use the NodeRestriction admission plugin to limit a node to creating/deleting mirror pods bound to itself. + return authorizer.DecisionAllow, "", nil + } + + case "status": + switch attrs.GetVerb() { + case "update", "patch": + // Needed for the node to report status of pods it is running. + // Use the NodeRestriction admission plugin to limit a node to updating status of pods bound to itself. + return authorizer.DecisionAllow, "", nil + } + + case "eviction": + if attrs.GetVerb() == "create" { + // Needed for the node to evict pods it is running. + // Use the NodeRestriction admission plugin to limit a node to evicting pods bound to itself. + return authorizer.DecisionAllow, "", nil + } + } + + klog.V(2).Infof("NODE DENY: '%s' %#v", nodeName, attrs) + return authorizer.DecisionNoOpinion, "", nil +} + // hasPathFrom returns true if there is a directed path from the specified type/namespace/name to the specified Node func (r *NodeAuthorizer) hasPathFrom(nodeName string, startingType vertexType, startingNamespace, startingName string) (bool, error) { r.graph.lock.RLock() diff --git a/plugin/pkg/auth/authorizer/node/node_authorizer_test.go b/plugin/pkg/auth/authorizer/node/node_authorizer_test.go index ac83df0b344..1f61b6ebd38 100644 --- a/plugin/pkg/auth/authorizer/node/node_authorizer_test.go +++ b/plugin/pkg/auth/authorizer/node/node_authorizer_test.go @@ -31,15 +31,19 @@ import ( resourcev1alpha2 "k8s.io/api/resource/v1alpha2" storagev1 "k8s.io/api/storage/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/fields" "k8s.io/apiserver/pkg/authentication/user" "k8s.io/apiserver/pkg/authorization/authorizer" + genericfeatures "k8s.io/apiserver/pkg/features" utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/component-base/featuregate" + featuregatetesting "k8s.io/component-base/featuregate/testing" "k8s.io/kubernetes/pkg/auth/nodeidentifier" + "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/plugin/pkg/auth/authorizer/rbac/bootstrappolicy" ) -func TestAuthorizer(t *testing.T) { +func TestNodeAuthorizer(t *testing.T) { g := NewGraph() opts := &sampleDataOpts{ @@ -66,6 +70,22 @@ func TestAuthorizer(t *testing.T) { node0 := &user.DefaultInfo{Name: "system:node:node0", Groups: []string{"system:nodes"}} + selectorAuthzDisabled := utilfeature.DefaultFeatureGate.DeepCopy() + featuregatetesting.SetFeatureGateDuringTest(t, selectorAuthzDisabled, genericfeatures.AuthorizeWithSelectors, false) + featuregatetesting.SetFeatureGateDuringTest(t, selectorAuthzDisabled, features.AuthorizeNodeWithSelectors, false) + + selectorAuthzEnabled := utilfeature.DefaultFeatureGate.DeepCopy() + featuregatetesting.SetFeatureGateDuringTest(t, selectorAuthzEnabled, genericfeatures.AuthorizeWithSelectors, true) + featuregatetesting.SetFeatureGateDuringTest(t, selectorAuthzEnabled, features.AuthorizeNodeWithSelectors, true) + + featureVariants := []struct { + suffix string + features featuregate.FeatureGate + }{ + {suffix: "selector_disabled", features: selectorAuthzDisabled}, + {suffix: "selector_enabled", features: selectorAuthzEnabled}, + } + tests := []struct { name string attrs authorizer.AttributesRecord @@ -365,15 +385,39 @@ func TestAuthorizer(t *testing.T) { expect: authorizer.DecisionNoOpinion, }, { - name: "allowed list ResourceSlices", - attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "resourceslices", APIGroup: "resource.k8s.io"}, + name: "allowed filtered list ResourceSlices", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "resourceslices", APIGroup: "resource.k8s.io", FieldSelectorRequirements: mustParseFields("nodeName==node0")}, expect: authorizer.DecisionAllow, }, { - name: "allowed watch ResourceSlices", - attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "resourceslices", APIGroup: "resource.k8s.io"}, + name: "allowed unfiltered list ResourceSlices - selector authz disabled", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "resourceslices", APIGroup: "resource.k8s.io"}, + expect: authorizer.DecisionAllow, + 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: "allowed filtered watch ResourceSlices", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "resourceslices", APIGroup: "resource.k8s.io", FieldSelectorRequirements: mustParseFields("nodeName==node0")}, expect: authorizer.DecisionAllow, }, + { + name: "allowed unfiltered watch ResourceSlices - selector authz disabled", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "resourceslices", APIGroup: "resource.k8s.io"}, + expect: authorizer.DecisionAllow, + 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: "allowed get ResourceSlice", attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "resourceslices", APIGroup: "resource.k8s.io", Name: "slice0-node0"}, @@ -399,24 +443,299 @@ func TestAuthorizer(t *testing.T) { attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "delete", Resource: "resourceslices", APIGroup: "resource.k8s.io", Name: "slice0-node0"}, expect: authorizer.DecisionAllow, }, + + // pods + // get pods + { + name: "get related pod", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "pods", APIGroup: "", Name: "pod0-node0", Namespace: "ns0"}, + expect: authorizer.DecisionAllow, + }, + { + name: "get unrelated pod - selector disabled", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "pods", APIGroup: "", Name: "pod0-node1", Namespace: "ns0"}, + expect: authorizer.DecisionAllow, + 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, + }, + // list pods + { + name: "list related pods", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "pods", APIGroup: "", FieldSelectorRequirements: mustParseFields("spec.nodeName=node0")}, + expect: authorizer.DecisionAllow, + }, + { + name: "list related pods - alternate selector", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "pods", APIGroup: "", FieldSelectorRequirements: mustParseFields("spec.nodeName==node0")}, + expect: authorizer.DecisionAllow, + }, + { + name: "list single related pod", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "pods", APIGroup: "", Name: "pod0-node0", Namespace: "ns0"}, + expect: authorizer.DecisionAllow, + }, + { + name: "list unrelated pods - selector disabled", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "pods", APIGroup: ""}, + expect: authorizer.DecisionAllow, + 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, + }, + // watch pods + { + name: "watch related pods", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "pods", APIGroup: "", FieldSelectorRequirements: mustParseFields("spec.nodeName=node0")}, + expect: authorizer.DecisionAllow, + }, + { + name: "watch related pods", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "pods", APIGroup: "", FieldSelectorRequirements: mustParseFields("spec.nodeName==node0")}, + expect: authorizer.DecisionAllow, + }, + { + name: "watch single related pod", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "pods", APIGroup: "", Name: "pod0-node0", Namespace: "ns0"}, + expect: authorizer.DecisionAllow, + }, + { + name: "watch unrelated pods - selector disabled", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "pods", APIGroup: ""}, + expect: authorizer.DecisionAllow, + 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, + }, + // create, delete pods + { + name: "create unnamed pod", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "create", Resource: "pods", APIGroup: "", Namespace: "ns0"}, + expect: authorizer.DecisionAllow, + }, + { + name: "create pod proxy subresource forbidden", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "create", Resource: "pods", Name: "", Subresource: "proxy", APIGroup: "", Namespace: "ns0"}, + expect: authorizer.DecisionNoOpinion, + }, + { + name: "delete related pod", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "delete", Resource: "pods", Name: "pod0-node0", APIGroup: "", Namespace: "ns0"}, + expect: authorizer.DecisionAllow, + }, + { + name: "delete unrelated pod", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "delete", Resource: "pods", Name: "pod0-node1", APIGroup: "", Namespace: "ns0"}, + expect: authorizer.DecisionAllow, // guarded by NodeRestriction admission + }, + { + name: "delete pod proxy subresource forbidden", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "delete", Resource: "pods", Name: "pod0-node0", Subresource: "proxy", APIGroup: "", Namespace: "ns0"}, + expect: authorizer.DecisionNoOpinion, + }, + // update/patch pod status + { + name: "update related pod status", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "update", Resource: "pods", Subresource: "status", Name: "pod0-node0", APIGroup: "", Namespace: "ns0"}, + expect: authorizer.DecisionAllow, + }, + { + name: "update unrelated pod status", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "update", Resource: "pods", Subresource: "status", Name: "pod0-node1", APIGroup: "", Namespace: "ns0"}, + expect: authorizer.DecisionAllow, // guarded by NodeRestriction admission + }, + { + name: "patch related pod status", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "patch", Resource: "pods", Subresource: "status", Name: "pod0-node0", APIGroup: "", Namespace: "ns0"}, + expect: authorizer.DecisionAllow, + }, + { + name: "patch unrelated pod status", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "patch", Resource: "pods", Subresource: "status", Name: "pod0-node1", APIGroup: "", Namespace: "ns0"}, + expect: authorizer.DecisionAllow, // guarded by NodeRestriction admission + }, + { + name: "get pod status", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "pods", Subresource: "status", Name: "pod0-node0", APIGroup: "", Namespace: "ns0"}, + expect: authorizer.DecisionNoOpinion, // get status not allowed before or after selector feature + }, + // create pod eviction + { + name: "create related pod eviction", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "create", Resource: "pods", Subresource: "eviction", Name: "pod0-node0", APIGroup: "", Namespace: "ns0"}, + expect: authorizer.DecisionAllow, + }, + { + name: "create unrelated pod eviction", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "create", Resource: "pods", Subresource: "eviction", Name: "pod0-node1", APIGroup: "", Namespace: "ns0"}, + expect: authorizer.DecisionAllow, // guarded by NodeRestriction admission + }, + + // nodes + // get nodes + { + name: "get related node", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "nodes", APIGroup: "", Name: "node0"}, + expect: authorizer.DecisionAllow, + }, + { + name: "get unrelated node - selector disabled", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "nodes", APIGroup: "", Name: "node1"}, + expect: authorizer.DecisionAllow, + 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, + }, + // list nodes + { + name: "list single related node", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "nodes", APIGroup: "", Name: "node0"}, + expect: authorizer.DecisionAllow, + }, + { + name: "list single unrelated node - selector disabled", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "nodes", APIGroup: "", Name: "node1"}, + expect: authorizer.DecisionAllow, + 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 all nodes - selector disabled", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "nodes", APIGroup: ""}, + expect: authorizer.DecisionAllow, + 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, + }, + // watch nodes + { + name: "watch single related node", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "nodes", APIGroup: "", Name: "node0"}, + expect: authorizer.DecisionAllow, + }, + { + name: "watch single unrelated node - selector disabled", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "nodes", APIGroup: "", Name: "node1"}, + expect: authorizer.DecisionAllow, + 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 all nodes - selector disabled", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "nodes", APIGroup: ""}, + expect: authorizer.DecisionAllow, + 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, + }, + // create nodes + { + name: "create node", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "create", Resource: "nodes", APIGroup: ""}, + expect: authorizer.DecisionAllow, + }, + // update/patch nodes + { + name: "update related node", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "update", Resource: "nodes", Name: "node0", APIGroup: ""}, + expect: authorizer.DecisionAllow, + }, + { + name: "update unrelated node", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "update", Resource: "nodes", Name: "node1", APIGroup: ""}, + expect: authorizer.DecisionAllow, // guarded by NodeRestriction admission + }, + { + name: "patch related node", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "patch", Resource: "nodes", Name: "node0", APIGroup: ""}, + expect: authorizer.DecisionAllow, + }, + { + name: "patch unrelated node", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "patch", Resource: "nodes", Name: "node1", APIGroup: ""}, + expect: authorizer.DecisionAllow, // guarded by NodeRestriction admission + }, + // update/patch node status + { + name: "update related node status", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "update", Resource: "nodes", Subresource: "status", Name: "node0", APIGroup: ""}, + expect: authorizer.DecisionAllow, + }, + { + name: "update unrelated node status", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "update", Resource: "nodes", Subresource: "status", Name: "node1", APIGroup: ""}, + expect: authorizer.DecisionAllow, // guarded by NodeRestriction admission + }, + { + name: "patch related node status", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "patch", Resource: "nodes", Subresource: "status", Name: "node0", APIGroup: ""}, + expect: authorizer.DecisionAllow, + }, + { + name: "patch unrelated node status", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "patch", Resource: "nodes", Subresource: "status", Name: "node1", APIGroup: ""}, + expect: authorizer.DecisionAllow, // guarded by NodeRestriction admission + }, } for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - if tc.features == nil { - authz.features = utilfeature.DefaultFeatureGate - } else { + if tc.features == nil { + for _, variant := range featureVariants { + t.Run(tc.name+"_"+variant.suffix, func(t *testing.T) { + authz.features = variant.features + decision, reason, _ := authz.Authorize(context.Background(), tc.attrs) + if decision != tc.expect { + t.Errorf("expected %v, got %v (%s)", tc.expect, decision, reason) + } + }) + } + } else { + t.Run(tc.name, func(t *testing.T) { authz.features = tc.features - } - decision, _, _ := authz.Authorize(context.Background(), tc.attrs) - if decision != tc.expect { - t.Errorf("expected %v, got %v", tc.expect, decision) - } - }) + decision, reason, _ := authz.Authorize(context.Background(), tc.attrs) + if decision != tc.expect { + t.Errorf("expected %v, got %v (%s)", tc.expect, decision, reason) + } + }) + } } } -func TestAuthorizerSharedResources(t *testing.T) { +func TestNodeAuthorizerSharedResources(t *testing.T) { g := NewGraph() g.destinationEdgeThreshold = 1 identifier := nodeidentifier.NewDefaultNodeIdentifier() @@ -564,6 +883,14 @@ type sampleDataOpts struct { nodeResourceCapacitiesPerNode int } +func mustParseFields(s string) fields.Requirements { + selector, err := fields.ParseSelector(s) + if err != nil { + panic(err) + } + return selector.Requirements() +} + func BenchmarkPopulationAllocation(b *testing.B) { opts := &sampleDataOpts{ nodes: 500, diff --git a/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/policy.go b/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/policy.go index 8e3f4090c2e..1e523352d3f 100644 --- a/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/policy.go +++ b/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/policy.go @@ -125,7 +125,7 @@ func NodeRules() []rbacv1.PolicyRule { // TODO: restrict to the bound node as creator in the NodeRestrictions admission plugin rbacv1helpers.NewRule("create", "update", "patch").Groups(legacyGroup).Resources("events").RuleOrDie(), - // TODO: restrict to pods scheduled on the bound node once field selectors are supported by list/watch authorization + // Use the Node authorizer to limit get to pods related to the node, and to limit list/watch to field selectors related to the node. rbacv1helpers.NewRule(Read...).Groups(legacyGroup).Resources("pods").RuleOrDie(), // Needed for the node to create/delete mirror pods.