Merge pull request #126015 from micahhausler/kubelet-cert-validation

Enhance node admission to validate kubelet CSR's CN

pkg/features/kube_features.go

@@ -216,6 +216,13 @@ const (
 	// Disable in-tree functionality in kubelet to authenticate to cloud provider container registries for image pull credentials.
 	DisableKubeletCloudCredentialProviders featuregate.Feature = "DisableKubeletCloudCredentialProviders"
 
+	// owner: @micahhausler
+	// Deprecated: v1.31
+	//
+	// Disable Node Admission plugin validation of CSRs for kubelet signers where CN=system:node:$nodeName.
+	// Remove in v1.33
+	DisableKubeletCSRAdmissionValidation featuregate.Feature = "DisableKubeletCSRAdmissionValidation"
+
 	// owner: @HirazawaUi
 	// kep: http://kep.k8s.io/4004
 	// alpha: v1.29
@@ -1326,6 +1333,8 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
 	// ...
 	HPAScaleToZero: {Default: false, PreRelease: featuregate.Alpha},
 
+	DisableKubeletCSRAdmissionValidation: {Default: false, PreRelease: featuregate.Deprecated}, // remove in 1.33
+
 	StorageNamespaceIndex: {Default: true, PreRelease: featuregate.Beta},
 
 	RecursiveReadOnlyMounts: {Default: true, PreRelease: featuregate.Beta},

plugin/pkg/admission/noderestriction/admission.go

@@ -38,6 +38,7 @@ import (
 	kubeletapis "k8s.io/kubelet/pkg/apis"
 	podutil "k8s.io/kubernetes/pkg/api/pod"
 	authenticationapi "k8s.io/kubernetes/pkg/apis/authentication"
+	certapi "k8s.io/kubernetes/pkg/apis/certificates"
 	coordapi "k8s.io/kubernetes/pkg/apis/coordination"
 	api "k8s.io/kubernetes/pkg/apis/core"
 	"k8s.io/kubernetes/pkg/apis/policy"
@@ -73,8 +74,9 @@ type Plugin struct {
 	podsGetter     corev1lister.PodLister
 	nodesGetter    corev1lister.NodeLister
 
-	expansionRecoveryEnabled         bool
+	expansionRecoveryEnabled              bool
-	dynamicResourceAllocationEnabled bool
+	dynamicResourceAllocationEnabled      bool
+	kubeletCSRAdmissionValidationDisabled bool
 }
 
 var (
@@ -87,6 +89,7 @@ var (
 func (p *Plugin) InspectFeatureGates(featureGates featuregate.FeatureGate) {
 	p.expansionRecoveryEnabled = featureGates.Enabled(features.RecoverVolumeExpansionFailure)
 	p.dynamicResourceAllocationEnabled = featureGates.Enabled(features.DynamicResourceAllocation)
+	p.kubeletCSRAdmissionValidationDisabled = featureGates.Enabled(features.DisableKubeletCSRAdmissionValidation)
 }
 
 // SetExternalKubeInformerFactory registers an informer factory into Plugin
@@ -117,6 +120,7 @@ var (
 	leaseResource         = coordapi.Resource("leases")
 	csiNodeResource       = storage.Resource("csinodes")
 	resourceSliceResource = resource.Resource("resourceslices")
+	csrResource           = certapi.Resource("certificatesigningrequests")
 )
 
 // Admit checks the admission policy and triggers corresponding actions
@@ -171,6 +175,11 @@ func (p *Plugin) Admit(ctx context.Context, a admission.Attributes, o admission.
 	case resourceSliceResource:
 		return p.admitResourceSlice(nodeName, a)
 
+	case csrResource:
+		if p.kubeletCSRAdmissionValidationDisabled {
+			return nil
+		}
+		return p.admitCSR(nodeName, a)
 	default:
 		return nil
 	}
@@ -670,3 +679,31 @@ func (p *Plugin) admitResourceSlice(nodeName string, a admission.Attributes) err
 
 	return nil
 }
+
+func (p *Plugin) admitCSR(nodeName string, a admission.Attributes) error {
+	// Create requests for Kubelet serving signer and Kube API server client
+	// kubelet signer with a CN that begins with "system:node:" must have a CN
+	// that is exactly the node's name.
+	// Other CSR attributes get checked in CSR validation by the signer.
+	if a.GetOperation() != admission.Create {
+		return nil
+	}
+
+	csr, ok := a.GetObject().(*certapi.CertificateSigningRequest)
+	if !ok {
+		return admission.NewForbidden(a, fmt.Errorf("unexpected type %T", a.GetObject()))
+	}
+	if csr.Spec.SignerName != certapi.KubeletServingSignerName && csr.Spec.SignerName != certapi.KubeAPIServerClientKubeletSignerName {
+		return nil
+	}
+
+	x509cr, err := certapi.ParseCSR(csr.Spec.Request)
+	if err != nil {
+		return admission.NewForbidden(a, fmt.Errorf("unable to parse csr: %w", err))
+	}
+	if x509cr.Subject.CommonName != fmt.Sprintf("system:node:%s", nodeName) {
+		return admission.NewForbidden(a, fmt.Errorf("can only create a node CSR with CN=system:node:%s", nodeName))
+	}
+
+	return nil
+}

plugin/pkg/admission/noderestriction/admission_test.go

@@ -18,6 +18,11 @@ package noderestriction
 
 import (
 	"context"
+	"crypto/rand"
+	"crypto/rsa"
+	"crypto/x509"
+	"crypto/x509/pkix"
+	"encoding/pem"
 	"reflect"
 	"strings"
 	"testing"
@@ -41,6 +46,7 @@ import (
 	"k8s.io/component-base/featuregate"
 	kubeletapis "k8s.io/kubelet/pkg/apis"
 	authenticationapi "k8s.io/kubernetes/pkg/apis/authentication"
+	certificatesapi "k8s.io/kubernetes/pkg/apis/certificates"
 	"k8s.io/kubernetes/pkg/apis/coordination"
 	api "k8s.io/kubernetes/pkg/apis/core"
 	"k8s.io/kubernetes/pkg/apis/policy"
@@ -213,11 +219,15 @@ type admitTestCase struct {
 	nodesGetter corev1lister.NodeLister
 	attributes  admission.Attributes
 	features    featuregate.FeatureGate
+	setupFunc   func(t *testing.T)
 	err         string
 }
 
 func (a *admitTestCase) run(t *testing.T) {
 	t.Run(a.name, func(t *testing.T) {
+		if a.setupFunc != nil {
+			a.setupFunc(t)
+		}
 		c := NewPlugin(nodeidentifier.NewDefaultNodeIdentifier())
 		if a.features != nil {
 			c.InspectFeatureGates(a.features)
@@ -375,6 +385,8 @@ func Test_nodePlugin_Admit(t *testing.T) {
 		}
 		aLabeledPod  = withLabels(coremypod, labelsA)
 		abLabeledPod = withLabels(coremypod, labelsAB)
+
+		privKey, _ = rsa.GenerateKey(rand.Reader, 2048)
 	)
 
 	existingPodsIndex.Add(v1mymirrorpod)
@@ -1238,6 +1250,42 @@ func Test_nodePlugin_Admit(t *testing.T) {
 			attributes: admission.NewAttributesRecord(nil, nil, csiNodeKind, nodeInfo.Namespace, nodeInfo.Name, csiNodeResource, "", admission.Delete, &metav1.UpdateOptions{}, false, mynode),
 			err:        "",
 		},
+		// CSR
+		{
+			name:       "allowed CSR create correct node serving",
+			attributes: createCSRAttributes("system:node:mynode", certificatesapi.KubeletServingSignerName, true, privKey, mynode),
+			err:        "",
+		},
+		{
+			name:       "allowed CSR create correct node client",
+			attributes: createCSRAttributes("system:node:mynode", certificatesapi.KubeAPIServerClientKubeletSignerName, true, privKey, mynode),
+			err:        "",
+		},
+		{
+			name:       "allowed CSR create non-node CSR",
+			attributes: createCSRAttributes("some-other-identity", certificatesapi.KubeAPIServerClientSignerName, true, privKey, mynode),
+			err:        "",
+		},
+		{
+			name:       "deny CSR create incorrect node",
+			attributes: createCSRAttributes("system:node:othernode", certificatesapi.KubeletServingSignerName, true, privKey, mynode),
+			err:        "forbidden: can only create a node CSR with CN=system:node:mynode",
+		},
+		{
+			name:       "allow CSR create incorrect node with feature gate disabled",
+			attributes: createCSRAttributes("system:node:othernode", certificatesapi.KubeletServingSignerName, true, privKey, mynode),
+			err:        "",
+			features:   feature.DefaultFeatureGate,
+			setupFunc: func(t *testing.T) {
+				t.Helper()
+				featuregatetesting.SetFeatureGateDuringTest(t, feature.DefaultFeatureGate, features.DisableKubeletCSRAdmissionValidation, true)
+			},
+		},
+		{
+			name:       "deny CSR create invalid",
+			attributes: createCSRAttributes("system:node:mynode", certificatesapi.KubeletServingSignerName, false, privKey, mynode),
+			err:        "unable to parse csr: asn1: syntax error: sequence truncated",
+		},
 	}
 	for _, tt := range tests {
 		tt.nodesGetter = existingNodes
@@ -1603,6 +1651,31 @@ func createPodAttributes(pod *api.Pod, user user.Info) admission.Attributes {
 	return admission.NewAttributesRecord(pod, nil, podKind, pod.Namespace, pod.Name, podResource, "", admission.Create, &metav1.CreateOptions{}, false, user)
 }
 
+func createCSRAttributes(cn, signer string, validCsr bool, key any, user user.Info) admission.Attributes {
+	csrResource := certificatesapi.Resource("certificatesigningrequests").WithVersion("v1")
+	csrKind := certificatesapi.Kind("CertificateSigningRequest").WithVersion("v1")
+
+	csrPem := []byte("-----BEGIN CERTIFICATE REQUEST-----\n-----END CERTIFICATE REQUEST-----")
+	if validCsr {
+		structuredCsr := x509.CertificateRequest{
+			Subject: pkix.Name{
+				CommonName: cn,
+			},
+		}
+		csrDer, _ := x509.CreateCertificateRequest(rand.Reader, &structuredCsr, key)
+		csrPem = pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE REQUEST", Bytes: csrDer})
+	}
+
+	csreq := &certificatesapi.CertificateSigningRequest{
+		Spec: certificatesapi.CertificateSigningRequestSpec{
+			Request:    csrPem,
+			SignerName: signer,
+		},
+	}
+	return admission.NewAttributesRecord(csreq, nil, csrKind, "", "", csrResource, "", admission.Create, &metav1.CreateOptions{}, false, user)
+
+}
+
 func TestAdmitResourceSlice(t *testing.T) {
 	apiResource := resourceapi.SchemeGroupVersion.WithResource("resourceslices")
 	nodename := "mynode"