From 2cc75f0a5a90208bf3914c3d1791dcb14723ebda Mon Sep 17 00:00:00 2001 From: Mike Danese Date: Thu, 2 Nov 2017 10:26:04 -0700 Subject: [PATCH 1/3] auth: allow nodes to create tokones for svcaccts of pods running on them. --- plugin/pkg/auth/authorizer/node/graph.go | 25 ++++++++---- .../auth/authorizer/node/node_authorizer.go | 31 +++++++++++++++ .../authorizer/node/node_authorizer_test.go | 39 +++++++++++++++++++ 3 files changed, 88 insertions(+), 7 deletions(-) diff --git a/plugin/pkg/auth/authorizer/node/graph.go b/plugin/pkg/auth/authorizer/node/graph.go index ec3188371a..38fea6d032 100644 --- a/plugin/pkg/auth/authorizer/node/graph.go +++ b/plugin/pkg/auth/authorizer/node/graph.go @@ -107,16 +107,18 @@ const ( pvVertexType secretVertexType vaVertexType + serviceAccountVertexType ) var vertexTypes = map[vertexType]string{ - configMapVertexType: "configmap", - nodeVertexType: "node", - podVertexType: "pod", - pvcVertexType: "pvc", - pvVertexType: "pv", - secretVertexType: "secret", - vaVertexType: "volumeattachment", + configMapVertexType: "configmap", + nodeVertexType: "node", + podVertexType: "pod", + pvcVertexType: "pvc", + pvVertexType: "pv", + secretVertexType: "secret", + vaVertexType: "volumeattachment", + serviceAccountVertexType: "serviceAccount", } // must be called under a write lock @@ -204,6 +206,7 @@ func (g *Graph) deleteVertex_locked(vertexType vertexType, namespace, name strin // secret -> pod // configmap -> pod // pvc -> pod +// svcacct -> pod func (g *Graph) AddPod(pod *api.Pod) { g.lock.Lock() defer g.lock.Unlock() @@ -213,6 +216,14 @@ func (g *Graph) AddPod(pod *api.Pod) { nodeVertex := g.getOrCreateVertex_locked(nodeVertexType, "", pod.Spec.NodeName) g.graph.SetEdge(newDestinationEdge(podVertex, nodeVertex, nodeVertex)) + // TODO(mikedanese): If the pod doesn't mount the service account secrets, + // should the node still get access to the service account? + // + // ref https://github.com/kubernetes/kubernetes/issues/58790 + if len(pod.Spec.ServiceAccountName) > 0 { + g.graph.SetEdge(newDestinationEdge(g.getOrCreateVertex_locked(serviceAccountVertexType, pod.Namespace, pod.Spec.ServiceAccountName), podVertex, nodeVertex)) + } + podutil.VisitPodSecretNames(pod, func(secret string) bool { g.graph.SetEdge(newDestinationEdge(g.getOrCreateVertex_locked(secretVertexType, pod.Namespace, secret), podVertex, nodeVertex)) return true diff --git a/plugin/pkg/auth/authorizer/node/node_authorizer.go b/plugin/pkg/auth/authorizer/node/node_authorizer.go index b283eb0c73..b2078d2c6d 100644 --- a/plugin/pkg/auth/authorizer/node/node_authorizer.go +++ b/plugin/pkg/auth/authorizer/node/node_authorizer.go @@ -70,6 +70,7 @@ var ( pvcResource = api.Resource("persistentvolumeclaims") pvResource = api.Resource("persistentvolumes") vaResource = storageapi.Resource("volumeattachments") + svcAcctResource = api.Resource("serviceaccounts") ) func (r *NodeAuthorizer) Authorize(attrs authorizer.Attributes) (authorizer.Decision, string, error) { @@ -106,6 +107,11 @@ func (r *NodeAuthorizer) Authorize(attrs authorizer.Attributes) (authorizer.Deci return r.authorizeGet(nodeName, vaVertexType, attrs) } return authorizer.DecisionNoOpinion, fmt.Sprintf("disabled by feature gate %s", features.CSIPersistentVolume), nil + case svcAcctResource: + if r.features.Enabled(features.TokenRequest) { + return r.authorizeCreateToken(nodeName, serviceAccountVertexType, attrs) + } + return authorizer.DecisionNoOpinion, fmt.Sprintf("disabled by feature gate %s", features.TokenRequest), nil } } @@ -165,6 +171,31 @@ func (r *NodeAuthorizer) authorize(nodeName string, startingType vertexType, att return authorizer.DecisionAllow, "", nil } +// 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) { + if attrs.GetVerb() != "create" || len(attrs.GetName()) == 0 { + glog.V(2).Infof("NODE DENY: %s %#v", nodeName, attrs) + return authorizer.DecisionNoOpinion, "can only create tokens for individual service accounts", nil + } + + if attrs.GetSubresource() != "token" { + glog.V(2).Infof("NODE DENY: %s %#v", nodeName, attrs) + return authorizer.DecisionNoOpinion, "can only create token subresource of serviceaccount", nil + } + + ok, err := r.hasPathFrom(nodeName, startingType, attrs.GetNamespace(), attrs.GetName()) + if err != nil { + glog.V(2).Infof("NODE DENY: %v", err) + return authorizer.DecisionNoOpinion, "no path found to object", nil + } + if !ok { + glog.V(2).Infof("NODE DENY: %q %#v", nodeName, attrs) + return authorizer.DecisionNoOpinion, "no path found to object", nil + } + return authorizer.DecisionAllow, "", 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 669e41f020..19eb32fdfc 100644 --- a/plugin/pkg/auth/authorizer/node/node_authorizer_test.go +++ b/plugin/pkg/auth/authorizer/node/node_authorizer_test.go @@ -38,6 +38,8 @@ import ( var ( csiEnabledFeature = utilfeature.NewFeatureGate() csiDisabledFeature = utilfeature.NewFeatureGate() + trEnabledFeature = utilfeature.NewFeatureGate() + trDisabledFeature = utilfeature.NewFeatureGate() ) func init() { @@ -47,6 +49,12 @@ func init() { if err := csiDisabledFeature.Add(map[utilfeature.Feature]utilfeature.FeatureSpec{features.CSIPersistentVolume: {Default: false}}); err != nil { panic(err) } + if err := trEnabledFeature.Add(map[utilfeature.Feature]utilfeature.FeatureSpec{features.TokenRequest: {Default: true}}); err != nil { + panic(err) + } + if err := trDisabledFeature.Add(map[utilfeature.Feature]utilfeature.FeatureSpec{features.TokenRequest: {Default: false}}); err != nil { + panic(err) + } } func TestAuthorizer(t *testing.T) { @@ -152,6 +160,36 @@ func TestAuthorizer(t *testing.T) { features: csiEnabledFeature, expect: authorizer.DecisionAllow, }, + { + name: "allowed svcacct token create - feature enabled", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "create", Resource: "serviceaccounts", Subresource: "token", Name: "svcacct0-node0", Namespace: "ns0"}, + features: trEnabledFeature, + expect: authorizer.DecisionAllow, + }, + { + name: "disallowed svcacct token create - serviceaccount not attached to node", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "create", Resource: "serviceaccounts", Subresource: "token", Name: "svcacct0-node1", Namespace: "ns0"}, + features: trEnabledFeature, + expect: authorizer.DecisionNoOpinion, + }, + { + name: "disallowed svcacct token create - feature disabled", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "create", Resource: "serviceaccounts", Subresource: "token", Name: "svcacct0-node0", Namespace: "ns0"}, + features: trDisabledFeature, + expect: authorizer.DecisionNoOpinion, + }, + { + name: "disallowed svcacct token create - no subresource", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "create", Resource: "serviceaccounts", Name: "svcacct0-node0", Namespace: "ns0"}, + features: trEnabledFeature, + expect: authorizer.DecisionNoOpinion, + }, + { + name: "disallowed svcacct token create - non create", + attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "update", Resource: "serviceaccounts", Subresource: "token", Name: "svcacct0-node0", Namespace: "ns0"}, + features: trEnabledFeature, + expect: authorizer.DecisionNoOpinion, + }, } for _, tc := range tests { @@ -459,6 +497,7 @@ func generate(opts sampleDataOpts) ([]*api.Pod, []*api.PersistentVolume, []*stor pod.Namespace = fmt.Sprintf("ns%d", p%opts.namespaces) pod.Name = fmt.Sprintf("pod%d-%s", p, nodeName) pod.Spec.NodeName = nodeName + pod.Spec.ServiceAccountName = fmt.Sprintf("svcacct%d-%s", p, nodeName) for i := 0; i < opts.uniqueSecretsPerPod; i++ { pod.Spec.Volumes = append(pod.Spec.Volumes, api.Volume{VolumeSource: api.VolumeSource{ From b43cd7307dfe81476795bbaecc409238d9bf1669 Mon Sep 17 00:00:00 2001 From: Mike Danese Date: Fri, 23 Feb 2018 11:24:43 -0800 Subject: [PATCH 2/3] noderestriction: restrict nodes TokenRequest permission nodes should only be able to create TokenRequests if: * token is bound to a pod * binding has uid and name * the pod exists * the pod is running on that node --- plugin/pkg/admission/noderestriction/BUILD | 5 ++ .../admission/noderestriction/admission.go | 60 +++++++++++++- .../noderestriction/admission_test.go | 79 +++++++++++++++++++ test/integration/auth/node_test.go | 2 + 4 files changed, 142 insertions(+), 4 deletions(-) diff --git a/plugin/pkg/admission/noderestriction/BUILD b/plugin/pkg/admission/noderestriction/BUILD index 3a528a04b3..4e83ee5157 100644 --- a/plugin/pkg/admission/noderestriction/BUILD +++ b/plugin/pkg/admission/noderestriction/BUILD @@ -12,6 +12,7 @@ go_library( importpath = "k8s.io/kubernetes/plugin/pkg/admission/noderestriction", deps = [ "//pkg/api/pod:go_default_library", + "//pkg/apis/authentication:go_default_library", "//pkg/apis/core:go_default_library", "//pkg/apis/policy:go_default_library", "//pkg/auth/nodeidentifier:go_default_library", @@ -33,14 +34,18 @@ go_test( srcs = ["admission_test.go"], embed = [":go_default_library"], deps = [ + "//pkg/apis/authentication:go_default_library", "//pkg/apis/core:go_default_library", "//pkg/apis/policy:go_default_library", "//pkg/auth/nodeidentifier:go_default_library", "//pkg/client/clientset_generated/internalclientset/fake:go_default_library", "//pkg/client/clientset_generated/internalclientset/typed/core/internalversion:go_default_library", + "//pkg/features:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/types:go_default_library", "//vendor/k8s.io/apiserver/pkg/admission:go_default_library", "//vendor/k8s.io/apiserver/pkg/authentication/user:go_default_library", + "//vendor/k8s.io/apiserver/pkg/util/feature:go_default_library", ], ) diff --git a/plugin/pkg/admission/noderestriction/admission.go b/plugin/pkg/admission/noderestriction/admission.go index d958dad376..333506e665 100644 --- a/plugin/pkg/admission/noderestriction/admission.go +++ b/plugin/pkg/admission/noderestriction/admission.go @@ -27,6 +27,7 @@ import ( "k8s.io/apiserver/pkg/admission" utilfeature "k8s.io/apiserver/pkg/util/feature" podutil "k8s.io/kubernetes/pkg/api/pod" + authenticationapi "k8s.io/kubernetes/pkg/apis/authentication" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/policy" "k8s.io/kubernetes/pkg/auth/nodeidentifier" @@ -53,6 +54,7 @@ func NewPlugin(nodeIdentifier nodeidentifier.NodeIdentifier) *nodePlugin { return &nodePlugin{ Handler: admission.NewHandler(admission.Create, admission.Update, admission.Delete), nodeIdentifier: nodeIdentifier, + features: utilfeature.DefaultFeatureGate, } } @@ -61,6 +63,8 @@ type nodePlugin struct { *admission.Handler nodeIdentifier nodeidentifier.NodeIdentifier podsGetter coreinternalversion.PodsGetter + // allows overriding for testing + features utilfeature.FeatureGate } var ( @@ -83,9 +87,10 @@ func (p *nodePlugin) ValidateInitialization() error { } var ( - podResource = api.Resource("pods") - nodeResource = api.Resource("nodes") - pvcResource = api.Resource("persistentvolumeclaims") + podResource = api.Resource("pods") + nodeResource = api.Resource("nodes") + pvcResource = api.Resource("persistentvolumeclaims") + svcacctResource = api.Resource("serviceaccounts") ) func (c *nodePlugin) Admit(a admission.Attributes) error { @@ -125,6 +130,12 @@ func (c *nodePlugin) Admit(a admission.Attributes) error { return admission.NewForbidden(a, fmt.Errorf("may only update PVC status")) } + case svcacctResource: + if c.features.Enabled(features.TokenRequest) { + return c.admitServiceAccount(nodeName, a) + } + return nil + default: return nil } @@ -256,7 +267,7 @@ func (c *nodePlugin) admitPodEviction(nodeName string, a admission.Attributes) e func (c *nodePlugin) admitPVCStatus(nodeName string, a admission.Attributes) error { switch a.GetOperation() { case admission.Update: - if !utilfeature.DefaultFeatureGate.Enabled(features.ExpandPersistentVolumes) { + if !c.features.Enabled(features.ExpandPersistentVolumes) { return admission.NewForbidden(a, fmt.Errorf("node %q may not update persistentvolumeclaim metadata", nodeName)) } @@ -340,3 +351,44 @@ func (c *nodePlugin) admitNode(nodeName string, a admission.Attributes) error { return nil } + +func (c *nodePlugin) admitServiceAccount(nodeName string, a admission.Attributes) error { + if a.GetOperation() != admission.Create { + return nil + } + if a.GetSubresource() != "token" { + return nil + } + tr, ok := a.GetObject().(*authenticationapi.TokenRequest) + if !ok { + return admission.NewForbidden(a, fmt.Errorf("unexpected type %T", a.GetObject())) + } + + // TokenRequests from a node must have a pod binding. That pod must be + // scheduled on the node. + ref := tr.Spec.BoundObjectRef + if ref == nil || + ref.APIVersion != "v1" || + ref.Kind != "Pod" || + ref.Name == "" { + return admission.NewForbidden(a, fmt.Errorf("node requested token not bound to a pod")) + } + if ref.UID == "" { + return admission.NewForbidden(a, fmt.Errorf("node requested token with a pod binding without a uid")) + } + pod, err := c.podsGetter.Pods(a.GetNamespace()).Get(ref.Name, v1.GetOptions{}) + if errors.IsNotFound(err) { + return err + } + if err != nil { + return admission.NewForbidden(a, err) + } + if ref.UID != pod.UID { + return admission.NewForbidden(a, fmt.Errorf("the UID in the bound object reference (%s) does not match the UID in record (%s). The object might have been deleted and then recreated", ref.UID, pod.UID)) + } + if pod.Spec.NodeName != nodeName { + return admission.NewForbidden(a, fmt.Errorf("node requested token bound to a pod scheduled on a different node")) + } + + return nil +} diff --git a/plugin/pkg/admission/noderestriction/admission_test.go b/plugin/pkg/admission/noderestriction/admission_test.go index fb0b2543f8..93dec71461 100644 --- a/plugin/pkg/admission/noderestriction/admission_test.go +++ b/plugin/pkg/admission/noderestriction/admission_test.go @@ -21,18 +21,37 @@ import ( "testing" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" "k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/authentication/user" + utilfeature "k8s.io/apiserver/pkg/util/feature" + authenticationapi "k8s.io/kubernetes/pkg/apis/authentication" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/policy" "k8s.io/kubernetes/pkg/auth/nodeidentifier" "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake" coreinternalversion "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/core/internalversion" + "k8s.io/kubernetes/pkg/features" ) +var ( + trEnabledFeature = utilfeature.NewFeatureGate() + trDisabledFeature = utilfeature.NewFeatureGate() +) + +func init() { + if err := trEnabledFeature.Add(map[utilfeature.Feature]utilfeature.FeatureSpec{features.TokenRequest: {Default: true}}); err != nil { + panic(err) + } + if err := trDisabledFeature.Add(map[utilfeature.Feature]utilfeature.FeatureSpec{features.TokenRequest: {Default: false}}); err != nil { + panic(err) + } +} + func makeTestPod(namespace, name, node string, mirror bool) *api.Pod { pod := &api.Pod{} pod.Namespace = namespace + pod.UID = types.UID("pod-uid") pod.Name = name pod.Spec.NodeName = node if mirror { @@ -47,6 +66,23 @@ func makeTestPodEviction(name string) *policy.Eviction { return eviction } +func makeTokenRequest(podname string, poduid types.UID) *authenticationapi.TokenRequest { + tr := &authenticationapi.TokenRequest{ + Spec: authenticationapi.TokenRequestSpec{ + Audiences: []string{"foo"}, + }, + } + if podname != "" { + tr.Spec.BoundObjectRef = &authenticationapi.BoundObjectReference{ + Kind: "Pod", + APIVersion: "v1", + Name: podname, + UID: poduid, + } + } + return tr +} + func Test_nodePlugin_Admit(t *testing.T) { var ( mynode = &user.DefaultInfo{Name: "system:node:mynode", Groups: []string{"system:nodes"}} @@ -86,6 +122,9 @@ func Test_nodePlugin_Admit(t *testing.T) { nodeResource = api.Resource("nodes").WithVersion("v1") nodeKind = api.Kind("Node").WithVersion("v1") + svcacctResource = api.Resource("serviceaccounts").WithVersion("v1") + tokenrequestKind = api.Kind("TokenRequest").WithVersion("v1") + noExistingPods = fake.NewSimpleClientset().Core() existingPods = fake.NewSimpleClientset(mymirrorpod, othermirrorpod, unboundmirrorpod, mypod, otherpod, unboundpod).Core() ) @@ -106,6 +145,7 @@ func Test_nodePlugin_Admit(t *testing.T) { name string podsGetter coreinternalversion.PodsGetter attributes admission.Attributes + features utilfeature.FeatureGate err string }{ // Mirror pods bound to us @@ -653,6 +693,42 @@ func Test_nodePlugin_Admit(t *testing.T) { err: "cannot modify node", }, + // Service accounts + { + name: "forbid create of unbound token", + podsGetter: noExistingPods, + features: trEnabledFeature, + attributes: admission.NewAttributesRecord(makeTokenRequest("", ""), nil, tokenrequestKind, "ns", "mysa", svcacctResource, "token", admission.Create, mynode), + err: "not bound to a pod", + }, + { + name: "forbid create of token bound to nonexistant pod", + podsGetter: noExistingPods, + features: trEnabledFeature, + attributes: admission.NewAttributesRecord(makeTokenRequest("nopod", "someuid"), nil, tokenrequestKind, "ns", "mysa", svcacctResource, "token", admission.Create, mynode), + err: "not found", + }, + { + name: "forbid create of token bound to pod without uid", + podsGetter: existingPods, + features: trEnabledFeature, + attributes: admission.NewAttributesRecord(makeTokenRequest(mypod.Name, ""), nil, tokenrequestKind, "ns", "mysa", svcacctResource, "token", admission.Create, mynode), + err: "pod binding without a uid", + }, + { + name: "forbid create of token bound to pod scheduled on another node", + podsGetter: existingPods, + features: trEnabledFeature, + attributes: admission.NewAttributesRecord(makeTokenRequest(otherpod.Name, otherpod.UID), nil, tokenrequestKind, otherpod.Namespace, "mysa", svcacctResource, "token", admission.Create, mynode), + err: "pod scheduled on a different node", + }, + { + name: "allow create of token bound to pod scheduled this node", + podsGetter: existingPods, + features: trEnabledFeature, + attributes: admission.NewAttributesRecord(makeTokenRequest(mypod.Name, mypod.UID), nil, tokenrequestKind, mypod.Namespace, "mysa", svcacctResource, "token", admission.Create, mynode), + }, + // Unrelated objects { name: "allow create of unrelated object", @@ -714,6 +790,9 @@ func Test_nodePlugin_Admit(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { c := NewPlugin(nodeidentifier.NewDefaultNodeIdentifier()) + if tt.features != nil { + c.features = tt.features + } c.podsGetter = tt.podsGetter err := c.Admit(tt.attributes) if (err == nil) != (len(tt.err) == 0) { diff --git a/test/integration/auth/node_test.go b/test/integration/auth/node_test.go index a17e960507..929f8828d8 100644 --- a/test/integration/auth/node_test.go +++ b/test/integration/auth/node_test.go @@ -448,6 +448,8 @@ func TestNodeAuthorizer(t *testing.T) { defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIPersistentVolume, true)() expectForbidden(t, getVolumeAttachment(node1ClientExternal)) expectAllowed(t, getVolumeAttachment(node2ClientExternal)) + + //TODO(mikedanese): integration test node restriction of TokenRequest } // expect executes a function a set number of times until it either returns the From 363e861aeb19b362e0ef0fa7fcd8f034d0dce17c Mon Sep 17 00:00:00 2001 From: Mike Danese Date: Fri, 23 Feb 2018 13:15:33 -0800 Subject: [PATCH 3/3] rbac: allow system:node role to make TokenRequests for all service accounts --- plugin/pkg/auth/authorizer/rbac/bootstrappolicy/policy.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/policy.go b/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/policy.go index 3f5f54e3b6..7daa7b2d71 100644 --- a/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/policy.go +++ b/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/policy.go @@ -146,6 +146,13 @@ func NodeRules() []rbac.PolicyRule { nodePolicyRules = append(nodePolicyRules, pvcStatusPolicyRule) } + if utilfeature.DefaultFeatureGate.Enabled(features.TokenRequest) { + // Use the Node authorization to limit a node to create tokens for service accounts running on that node + // Use the NodeRestriction admission plugin to limit a node to create tokens bound to pods on that node + tokenRequestRule := rbac.NewRule("create").Groups(legacyGroup).Resources("serviceaccounts/token").RuleOrDie() + nodePolicyRules = append(nodePolicyRules, tokenRequestRule) + } + // CSI if utilfeature.DefaultFeatureGate.Enabled(features.CSIPersistentVolume) { volAttachRule := rbac.NewRule("get").Groups(storageGroup).Resources("volumeattachments").RuleOrDie()