Extend client-go csr package to invalidate CSRs based on signerName

Author: James Munnelly

staging/src/k8s.io/client-go/util/certificate/csr/BUILD

@@ -1,6 +1,6 @@
 package(default_visibility = ["//visibility:public"])
 
-load("@io_bazel_rules_go//go:def.bzl", "go_library")
+load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
 
 go_library(
     name = "go_default_library",
@@ -35,3 +35,13 @@ filegroup(
     srcs = [":package-srcs"],
     tags = ["automanaged"],
 )
+
+go_test(
+    name = "go_default_test",
+    srcs = ["csr_test.go"],
+    embed = [":go_default_library"],
+    deps = [
+        "//staging/src/k8s.io/api/certificates/v1beta1:go_default_library",
+        "//vendor/k8s.io/utils/pointer:go_default_library",
+    ],
+)

staging/src/k8s.io/client-go/util/certificate/csr/csr.go

@@ -150,6 +150,9 @@ func ensureCompatible(new, orig *certificates.CertificateSigningRequest, private
 	if !reflect.DeepEqual(newCSR.Subject, origCSR.Subject) {
 		return fmt.Errorf("csr subjects differ: new: %#v, orig: %#v", newCSR.Subject, origCSR.Subject)
 	}
+	if new.Spec.SignerName != nil && orig.Spec.SignerName != nil && *new.Spec.SignerName != *orig.Spec.SignerName {
+		return fmt.Errorf("csr signerNames differ: new %q, orig: %q", *new.Spec.SignerName, *orig.Spec.SignerName)
+	}
 	signer, ok := privateKey.(crypto.Signer)
 	if !ok {
 		return fmt.Errorf("privateKey is not a signer")

staging/src/k8s.io/client-go/util/certificate/csr/csr_test.go

@@ -0,0 +1,146 @@
+/*
+Copyright 2020 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package csr
+
+import (
+	"crypto"
+	"crypto/rand"
+	"crypto/rsa"
+	"crypto/x509"
+	"crypto/x509/pkix"
+	"encoding/pem"
+	"testing"
+
+	certificates "k8s.io/api/certificates/v1beta1"
+	"k8s.io/utils/pointer"
+)
+
+func TestEnsureCompatible(t *testing.T) {
+	privateKey, err := rsa.GenerateKey(rand.Reader, 2048)
+	if err != nil {
+		t.Fatal(err)
+	}
+	req := pemWithPrivateKey(privateKey)
+
+	tests := map[string]struct {
+		new, orig  *certificates.CertificateSigningRequest
+		privateKey interface{}
+		err        string
+	}{
+		"nil signerName on 'new' matches any signerName on 'orig'": {
+			new: &certificates.CertificateSigningRequest{
+				Spec: certificates.CertificateSigningRequestSpec{
+					Request: req,
+				},
+			},
+			orig: &certificates.CertificateSigningRequest{
+				Spec: certificates.CertificateSigningRequestSpec{
+					Request:    req,
+					SignerName: pointer.StringPtr("example.com/test"),
+				},
+			},
+			privateKey: privateKey,
+		},
+		"nil signerName on 'orig' matches any signerName on 'new'": {
+			new: &certificates.CertificateSigningRequest{
+				Spec: certificates.CertificateSigningRequestSpec{
+					Request:    req,
+					SignerName: pointer.StringPtr("example.com/test"),
+				},
+			},
+			orig: &certificates.CertificateSigningRequest{
+				Spec: certificates.CertificateSigningRequestSpec{
+					Request: req,
+				},
+			},
+			privateKey: privateKey,
+		},
+		"signerName on 'orig' matches signerName on 'new'": {
+			new: &certificates.CertificateSigningRequest{
+				Spec: certificates.CertificateSigningRequestSpec{
+					Request:    req,
+					SignerName: pointer.StringPtr("example.com/test"),
+				},
+			},
+			orig: &certificates.CertificateSigningRequest{
+				Spec: certificates.CertificateSigningRequestSpec{
+					Request:    req,
+					SignerName: pointer.StringPtr("example.com/test"),
+				},
+			},
+			privateKey: privateKey,
+		},
+		"signerName on 'orig' does not match signerName on 'new'": {
+			new: &certificates.CertificateSigningRequest{
+				Spec: certificates.CertificateSigningRequestSpec{
+					Request:    req,
+					SignerName: pointer.StringPtr("example.com/test"),
+				},
+			},
+			orig: &certificates.CertificateSigningRequest{
+				Spec: certificates.CertificateSigningRequestSpec{
+					Request:    req,
+					SignerName: pointer.StringPtr("example.com/not-test"),
+				},
+			},
+			privateKey: privateKey,
+			err:        `csr signerNames differ: new "example.com/test", orig: "example.com/not-test"`,
+		},
+	}
+	for name, test := range tests {
+		t.Run(name, func(t *testing.T) {
+			err := ensureCompatible(test.new, test.orig, test.privateKey)
+			if err != nil && test.err == "" {
+				t.Errorf("expected no error, but got: %v", err)
+			} else if err != nil && test.err != err.Error() {
+				t.Errorf("error did not match as expected, got=%v, exp=%s", err, test.err)
+			}
+			if err == nil && test.err != "" {
+				t.Errorf("expected to get an error but got none")
+			}
+		})
+	}
+}
+
+func pemWithPrivateKey(pk crypto.PrivateKey) []byte {
+	template := &x509.CertificateRequest{
+		Subject: pkix.Name{
+			CommonName:   "something",
+			Organization: []string{"test"},
+		},
+	}
+	return pemWithTemplate(template, pk)
+}
+
+func pemWithTemplate(template *x509.CertificateRequest, key crypto.PrivateKey) []byte {
+	csrDER, err := x509.CreateCertificateRequest(rand.Reader, template, key)
+	if err != nil {
+		panic(err)
+	}
+
+	csrPemBlock := &pem.Block{
+		Type:  "CERTIFICATE REQUEST",
+		Bytes: csrDER,
+	}
+
+	p := pem.EncodeToMemory(csrPemBlock)
+	if p == nil {
+		panic("invalid pem block")
+	}
+
+	return p
+}