From b43cd7307dfe81476795bbaecc409238d9bf1669 Mon Sep 17 00:00:00 2001 From: Mike Danese Date: Fri, 23 Feb 2018 11:24:43 -0800 Subject: [PATCH] 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 3a528a04b38..4e83ee51578 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 d958dad3762..333506e6657 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 fb0b2543f87..93dec714618 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 a17e9605072..929f8828d84 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