Merge pull request #46884 from jcbsmpsn/autoapprover-kubelet-server-certificate

Automatic merge from submit-queue (batch tested with PRs 46884, 47557)

Auto approve kubelet server certificate signing requests.

Fixes https://github.com/kubernetes/kubernetes/issues/47208

**Release note**:
```release-note
Adds an approval work flow to the the certificate approver that will approve certificate signing requests from kubelets that meet all the criteria of kubelet server certificates.
```

cluster/addons/rbac/kubelet-certificate-management.yaml

@@ -57,5 +57,6 @@ rules:
   - "certificates.k8s.io"
   resources:
   - certificatesigningrequests/selfnodeclient
+  - certificatesigningrequests/selfnodeserver
   verbs:
   - "create"

pkg/controller/certificates/approver/BUILD

@@ -33,6 +33,8 @@ go_library(
         "//pkg/client/clientset_generated/clientset:go_default_library",
         "//pkg/client/informers/informers_generated/externalversions/certificates/v1beta1:go_default_library",
         "//pkg/controller/certificates:go_default_library",
+        "//pkg/features:go_default_library",
+        "//vendor/k8s.io/apiserver/pkg/util/feature:go_default_library",
     ],
 )
 

pkg/controller/certificates/approver/sarapprove.go

@@ -23,11 +23,13 @@ import (
 	"reflect"
 	"strings"
 
+	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	authorization "k8s.io/kubernetes/pkg/apis/authorization/v1beta1"
 	capi "k8s.io/kubernetes/pkg/apis/certificates/v1beta1"
 	"k8s.io/kubernetes/pkg/client/clientset_generated/clientset"
 	certificatesinformers "k8s.io/kubernetes/pkg/client/informers/informers_generated/externalversions/certificates/v1beta1"
 	"k8s.io/kubernetes/pkg/controller/certificates"
+	"k8s.io/kubernetes/pkg/features"
 )
 
 type csrRecognizer struct {
@@ -54,7 +56,7 @@ func NewCSRApprovingController(client clientset.Interface, csrInformer certifica
 }
 
 func recognizers() []csrRecognizer {
-	return []csrRecognizer{
+	recognizers := []csrRecognizer{
 		{
 			recognize:      isSelfNodeClientCert,
 			permission:     authorization.ResourceAttributes{Group: "certificates.k8s.io", Resource: "certificatesigningrequests", Verb: "create", Subresource: "selfnodeclient"},
@@ -65,12 +67,15 @@ func recognizers() []csrRecognizer {
 			permission:     authorization.ResourceAttributes{Group: "certificates.k8s.io", Resource: "certificatesigningrequests", Verb: "create", Subresource: "nodeclient"},
 			successMessage: "Auto approving kubelet client certificate after SubjectAccessReview.",
 		},
-		{
+	}
+	if utilfeature.DefaultFeatureGate.Enabled(features.RotateKubeletServerCertificate) {
+		recognizers = append(recognizers, csrRecognizer{
 			recognize:      isSelfNodeServerCert,
 			permission:     authorization.ResourceAttributes{Group: "certificates.k8s.io", Resource: "certificatesigningrequests", Verb: "create", Subresource: "selfnodeserver"},
 			successMessage: "Auto approving self kubelet server certificate after SubjectAccessReview.",
-		},
+		})
 	}
+	return recognizers
 }
 
 func (a *sarApprover) handle(csr *capi.CertificateSigningRequest) error {
@@ -192,9 +197,20 @@ var kubeletServerUsages = []capi.KeyUsage{
 }
 
 func isSelfNodeServerCert(csr *capi.CertificateSigningRequest, x509cr *x509.CertificateRequest) bool {
+	if !reflect.DeepEqual([]string{"system:nodes"}, x509cr.Subject.Organization) {
+		return false
+	}
+	if len(x509cr.DNSNames) == 0 || len(x509cr.IPAddresses) == 0 {
+		return false
+	}
 	if !hasExactUsages(csr, kubeletServerUsages) {
 		return false
 	}
-	//TODO(jcbsmpsn): implement the rest of this
+	if !strings.HasPrefix(x509cr.Subject.CommonName, "system:node:") {
-	return false
+		return false
+	}
+	if csr.Spec.Username != x509cr.Subject.CommonName {
+		return false
+	}
+	return true
 }

pkg/controller/certificates/approver/sarapprove_test.go

@@ -184,6 +184,77 @@ func TestHandle(t *testing.T) {
 	}
 }
 
+func TestSelfNodeServerCertRecognizer(t *testing.T) {
+	defaultCSR := csrBuilder{
+		cn:        "system:node:foo",
+		orgs:      []string{"system:nodes"},
+		requestor: "system:node:foo",
+		usages: []capi.KeyUsage{
+			capi.UsageKeyEncipherment,
+			capi.UsageDigitalSignature,
+			capi.UsageServerAuth,
+		},
+		dns: []string{"node"},
+		ips: []net.IP{net.ParseIP("192.168.0.1")},
+	}
+
+	testCases := []struct {
+		description     string
+		csrBuilder      csrBuilder
+		expectedOutcome bool
+	}{
+		{
+			description:     "Success - all requirements met",
+			csrBuilder:      defaultCSR,
+			expectedOutcome: true,
+		},
+		{
+			description: "No organization",
+			csrBuilder: func(b csrBuilder) csrBuilder {
+				b.orgs = []string{}
+				return b
+			}(defaultCSR),
+			expectedOutcome: false,
+		},
+		{
+			description: "Wrong organization",
+			csrBuilder: func(b csrBuilder) csrBuilder {
+				b.orgs = append(b.orgs, "new-org")
+				return b
+			}(defaultCSR),
+			expectedOutcome: false,
+		},
+		{
+			description: "Wrong usages",
+			csrBuilder: func(b csrBuilder) csrBuilder {
+				b.usages = []capi.KeyUsage{}
+				return b
+			}(defaultCSR),
+			expectedOutcome: false,
+		},
+		{
+			description: "Wrong common name",
+			csrBuilder: func(b csrBuilder) csrBuilder {
+				b.cn = "wrong-common-name"
+				return b
+			}(defaultCSR),
+			expectedOutcome: false,
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.description, func(t *testing.T) {
+			csr := makeFancyTestCsr(tc.csrBuilder)
+			x509cr, err := capi.ParseCSR(csr)
+			if err != nil {
+				t.Errorf("unexpected err: %v", err)
+			}
+			if isSelfNodeServerCert(csr, x509cr) != tc.expectedOutcome {
+				t.Errorf("expected recognized to be %v", tc.expectedOutcome)
+			}
+		})
+	}
+}
+
 func TestRecognizers(t *testing.T) {
 	goodCases := []func(b *csrBuilder){
 		func(b *csrBuilder) {

pkg/kubelet/kubelet.go

@@ -1134,16 +1134,25 @@ func initializeServerCertificateManager(kubeClient clientset.Interface, kubeCfg
 		CertificateSigningRequestClient: certSigningRequestClient,
 		Template: &x509.CertificateRequest{
 			Subject: pkix.Name{
-				CommonName:   string(nodeName),
+				CommonName:   fmt.Sprintf("system:node:%s", nodeName),
 				Organization: []string{"system:nodes"},
 			},
 			DNSNames:    hostnames,
 			IPAddresses: ips,
 		},
 		Usages: []certificates.KeyUsage{
+			// https://tools.ietf.org/html/rfc5280#section-4.2.1.3
+			//
+			// Digital signature allows the certificate to be used to verify
+			// digital signatures used during TLS negotiation.
+			certificates.UsageDigitalSignature,
+			// KeyEncipherment allows the cert/key pair to be used to encrypt
+			// keys, including the symetric keys negotiated during TLS setup
+			// and used for data transfer.
 			certificates.UsageKeyEncipherment,
+			// ServerAuth allows the cert to be used by a TLS server to
+			// authenticate itself to a TLS client.
 			certificates.UsageServerAuth,
-			certificates.UsageSigning,
 		},
 		CertificateStore: certificateStore,
 	})