Add signerName field to CSR resource spec

Signed-off-by: James Munnelly <james.munnelly@jetstack.io>

pkg/apis/certificates/fuzzer/fuzzer.go

@@ -29,6 +29,7 @@ var Funcs = func(codecs runtimeserializer.CodecFactory) []interface{} {
 		func(obj *certificates.CertificateSigningRequestSpec, c fuzz.Continue) {
 			c.FuzzNoCustom(obj) // fuzz self without calling this function again
 			obj.Usages = []certificates.KeyUsage{certificates.UsageKeyEncipherment}
+			obj.SignerName = "example.com/custom-sample-signer"
 		},
 	}
 }

pkg/apis/certificates/types.go

@@ -42,6 +42,12 @@ type CertificateSigningRequestSpec struct {
 	// Base64-encoded PKCS#10 CSR data
 	Request []byte
 
+	// Requested signer for the request. It is a qualified name in the form:
+	// `scope-hostname.io/name`.
+	// Distribution of trust for signers happens out of band.
+	// You can select on this field using `spec.signerName`.
+	SignerName string
+
 	// usages specifies a set of usage contexts the key will be
 	// valid for.
 	// See: https://tools.ietf.org/html/rfc5280#section-4.2.1.3

pkg/apis/certificates/v1beta1/BUILD

@@ -3,11 +3,13 @@ package(default_visibility = ["//visibility:public"])
 load(
     "@io_bazel_rules_go//go:def.bzl",
     "go_library",
+    "go_test",
 )
 
 go_library(
     name = "go_default_library",
     srcs = [
+        "conversion.go",
         "defaults.go",
         "doc.go",
         "helpers.go",
@@ -19,9 +21,11 @@ go_library(
     deps = [
         "//pkg/apis/certificates:go_default_library",
         "//staging/src/k8s.io/api/certificates/v1beta1:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/conversion:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
     ],
 )
 
@@ -37,3 +41,10 @@ filegroup(
     srcs = [":package-srcs"],
     tags = ["automanaged"],
 )
+
+go_test(
+    name = "go_default_test",
+    srcs = ["defaults_test.go"],
+    embed = [":go_default_library"],
+    deps = ["//staging/src/k8s.io/api/certificates/v1beta1:go_default_library"],
+)

pkg/apis/certificates/v1beta1/conversion.go

@@ -0,0 +1,38 @@
+/*
+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 v1beta1
+
+import (
+	"fmt"
+
+	"k8s.io/apimachinery/pkg/runtime"
+)
+
+func addConversionFuncs(scheme *runtime.Scheme) error {
+	// Add field conversion funcs.
+	return scheme.AddFieldLabelConversionFunc(SchemeGroupVersion.WithKind("CertificateSigningRequest"),
+		func(label, value string) (string, string, error) {
+			switch label {
+			case "metadata.name",
+				"spec.signerName":
+				return label, value, nil
+			default:
+				return "", "", fmt.Errorf("field label not supported: %s", label)
+			}
+		},
+	)
+}

pkg/apis/certificates/v1beta1/defaults.go

@@ -17,15 +17,113 @@ limitations under the License.
 package v1beta1
 
 import (
+	"crypto/x509"
+	"reflect"
+	"strings"
+
 	certificatesv1beta1 "k8s.io/api/certificates/v1beta1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/sets"
 )
 
 func addDefaultingFuncs(scheme *runtime.Scheme) error {
 	return RegisterDefaults(scheme)
 }
+
 func SetDefaults_CertificateSigningRequestSpec(obj *certificatesv1beta1.CertificateSigningRequestSpec) {
 	if obj.Usages == nil {
 		obj.Usages = []certificatesv1beta1.KeyUsage{certificatesv1beta1.UsageDigitalSignature, certificatesv1beta1.UsageKeyEncipherment}
 	}
+
+	if obj.SignerName == nil {
+		signerName := DefaultSignerNameFromSpec(obj)
+		obj.SignerName = &signerName
+	}
+}
+
+// DefaultSignerNameFromSpec will determine the signerName that should be set
+// by attempting to inspect the 'request' content and the spec options.
+func DefaultSignerNameFromSpec(obj *certificatesv1beta1.CertificateSigningRequestSpec) string {
+	csr, err := ParseCSR(obj.Request)
+	switch {
+	case err != nil:
+		// Set the signerName to 'legacy-unknown' as the CSR could not be
+		// recognised.
+		return certificatesv1beta1.LegacyUnknownSignerName
+	case IsKubeletClientCSR(csr, obj.Usages):
+		return certificatesv1beta1.KubeAPIServerClientKubeletSignerName
+	case IsKubeletServingCSR(csr, obj.Usages):
+		return certificatesv1beta1.KubeletServingSignerName
+	default:
+		return certificatesv1beta1.LegacyUnknownSignerName
+	}
+}
+
+func IsKubeletServingCSR(req *x509.CertificateRequest, usages []certificatesv1beta1.KeyUsage) bool {
+	if !reflect.DeepEqual([]string{"system:nodes"}, req.Subject.Organization) {
+		return false
+	}
+
+	// at least one of dnsNames or ipAddresses must be specified
+	if len(req.DNSNames) == 0 && len(req.IPAddresses) == 0 {
+		return false
+	}
+
+	if len(req.EmailAddresses) > 0 || len(req.URIs) > 0 {
+		return false
+	}
+
+	requiredUsages := []certificatesv1beta1.KeyUsage{
+		certificatesv1beta1.UsageDigitalSignature,
+		certificatesv1beta1.UsageKeyEncipherment,
+		certificatesv1beta1.UsageServerAuth,
+	}
+	if !equalUnsorted(requiredUsages, usages) {
+		return false
+	}
+
+	if !strings.HasPrefix(req.Subject.CommonName, "system:node:") {
+		return false
+	}
+
+	return true
+}
+
+func IsKubeletClientCSR(req *x509.CertificateRequest, usages []certificatesv1beta1.KeyUsage) bool {
+	if !reflect.DeepEqual([]string{"system:nodes"}, req.Subject.Organization) {
+		return false
+	}
+
+	if len(req.DNSNames) > 0 || len(req.EmailAddresses) > 0 || len(req.IPAddresses) > 0 || len(req.URIs) > 0 {
+		return false
+	}
+
+	if !strings.HasPrefix(req.Subject.CommonName, "system:node:") {
+		return false
+	}
+
+	requiredUsages := []certificatesv1beta1.KeyUsage{
+		certificatesv1beta1.UsageDigitalSignature,
+		certificatesv1beta1.UsageKeyEncipherment,
+		certificatesv1beta1.UsageClientAuth,
+	}
+	if !equalUnsorted(requiredUsages, usages) {
+		return false
+	}
+
+	return true
+}
+
+// equalUnsorted compares two []string for equality of contents regardless of
+// the order of the elements
+func equalUnsorted(left, right []certificatesv1beta1.KeyUsage) bool {
+	l := sets.NewString()
+	for _, s := range left {
+		l.Insert(string(s))
+	}
+	r := sets.NewString()
+	for _, s := range right {
+		r.Insert(string(s))
+	}
+	return l.Equal(r)
 }

pkg/apis/certificates/v1beta1/defaults_test.go

@@ -0,0 +1,353 @@
+/*
+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 v1beta1
+
+import (
+	"crypto/ed25519"
+	"crypto/rand"
+	"crypto/x509"
+	"crypto/x509/pkix"
+	"encoding/pem"
+	"net"
+	"net/url"
+	"reflect"
+	"testing"
+
+	capi "k8s.io/api/certificates/v1beta1"
+)
+
+var (
+	kubeletClientUsages = []capi.KeyUsage{
+		capi.UsageDigitalSignature,
+		capi.UsageKeyEncipherment,
+		capi.UsageClientAuth,
+	}
+	kubeletClientPEMOptions = pemOptions{
+		cn:  "system:node:nodename",
+		org: "system:nodes",
+	}
+
+	kubeletServerUsages = []capi.KeyUsage{
+		capi.UsageDigitalSignature,
+		capi.UsageKeyEncipherment,
+		capi.UsageServerAuth,
+	}
+	kubeletServerPEMOptions = pemOptions{
+		cn:          "system:node:requester-name",
+		org:         "system:nodes",
+		dnsNames:    []string{"node-server-name"},
+		ipAddresses: []net.IP{{0, 0, 0, 0}},
+	}
+)
+
+func TestSetDefaults_CertificateSigningRequestSpec(t *testing.T) {
+	strPtr := func(s string) *string { return &s }
+	tests := map[string]struct {
+		csr                capi.CertificateSigningRequestSpec
+		expectedSignerName string
+		expectedUsages     []capi.KeyUsage
+	}{
+		"defaults to legacy-unknown if request is not a CSR": {
+			csr: capi.CertificateSigningRequestSpec{
+				Request: []byte("invalid data"),
+				Usages:  kubeletServerUsages,
+			},
+			expectedSignerName: capi.LegacyUnknownSignerName,
+		},
+		"does not default signerName if signerName is already set": {
+			csr: capi.CertificateSigningRequestSpec{
+				Request:    csrWithOpts(kubeletServerPEMOptions),
+				Usages:     kubeletServerUsages,
+				SignerName: strPtr("example.com/not-kubelet-serving"),
+			},
+			expectedSignerName: "example.com/not-kubelet-serving",
+		},
+		"defaults usages if not set": {
+			csr: capi.CertificateSigningRequestSpec{
+				Request:    csrWithOpts(kubeletServerPEMOptions),
+				SignerName: strPtr("example.com/test"),
+			},
+			expectedSignerName: "example.com/test",
+			expectedUsages:     []capi.KeyUsage{capi.UsageDigitalSignature, capi.UsageKeyEncipherment},
+		},
+	}
+	for name, test := range tests {
+		t.Run(name, func(t *testing.T) {
+			// create a deepcopy to be sure we don't modify anything in-place
+			csrSpec := test.csr.DeepCopy()
+			SetDefaults_CertificateSigningRequestSpec(csrSpec)
+			if *csrSpec.SignerName != test.expectedSignerName {
+				t.Errorf("expected signerName to be defaulted to %q but it is %q", test.expectedSignerName, *csrSpec.SignerName)
+			}
+
+			// only check expectedUsages if it is non-nil
+			if test.expectedUsages != nil {
+				if !reflect.DeepEqual(test.expectedUsages, csrSpec.Usages) {
+					t.Errorf("expected usages to be defaulted to %v but it is %v", test.expectedUsages, csrSpec.Usages)
+				}
+			}
+		})
+	}
+}
+
+func TestSetDefaults_CertificateSigningRequestSpec_KubeletServing(t *testing.T) {
+	tests := map[string]struct {
+		csr                capi.CertificateSigningRequestSpec
+		expectedSignerName string
+	}{
+		"defaults for kubelet-serving": {
+			csr: capi.CertificateSigningRequestSpec{
+				Request:  csrWithOpts(kubeletServerPEMOptions),
+				Usages:   kubeletServerUsages,
+				Username: kubeletServerPEMOptions.cn,
+			},
+			expectedSignerName: capi.KubeletServingSignerName,
+		},
+		"does not default to kube-apiserver-client-kubelet if org is not 'system:nodes'": {
+			csr: capi.CertificateSigningRequestSpec{
+				Request:  csrWithOpts(kubeletServerPEMOptions, pemOptions{org: "not-system:nodes"}),
+				Usages:   kubeletServerUsages,
+				Username: "system:node:not-requester-name",
+			},
+			expectedSignerName: capi.LegacyUnknownSignerName,
+		},
+		"does not default to kubelet-serving if CN does not have system:node: prefix": {
+			csr: capi.CertificateSigningRequestSpec{
+				Request:  csrWithOpts(kubeletServerPEMOptions, pemOptions{cn: "notprefixed"}),
+				Usages:   kubeletServerUsages,
+				Username: "notprefixed",
+			},
+			expectedSignerName: capi.LegacyUnknownSignerName,
+		},
+		"does not default to kubelet-serving if it has an unexpected usage": {
+			csr: capi.CertificateSigningRequestSpec{
+				Request:  csrWithOpts(kubeletServerPEMOptions),
+				Usages:   append(kubeletServerUsages, capi.UsageClientAuth),
+				Username: kubeletServerPEMOptions.cn,
+			},
+			expectedSignerName: capi.LegacyUnknownSignerName,
+		},
+		"does not default to kubelet-serving if it is missing an expected usage": {
+			csr: capi.CertificateSigningRequestSpec{
+				Request: csrWithOpts(kubeletServerPEMOptions),
+				// Remove the first usage in 'kubeletServerUsages'
+				Usages:   kubeletServerUsages[1:],
+				Username: kubeletServerPEMOptions.cn,
+			},
+			expectedSignerName: capi.LegacyUnknownSignerName,
+		},
+		"does not default to kubelet-serving if it does not specify any dnsNames or ipAddresses": {
+			csr: capi.CertificateSigningRequestSpec{
+				Request:  csrWithOpts(kubeletServerPEMOptions, pemOptions{ipAddresses: []net.IP{}, dnsNames: []string{}}),
+				Usages:   kubeletServerUsages,
+				Username: kubeletServerPEMOptions.cn,
+			},
+			expectedSignerName: capi.LegacyUnknownSignerName,
+		},
+		"does not default to kubelet-serving if it specifies a URI SAN": {
+			csr: capi.CertificateSigningRequestSpec{
+				Request:  csrWithOpts(kubeletServerPEMOptions, pemOptions{uris: []string{"http://something"}}),
+				Usages:   kubeletServerUsages,
+				Username: kubeletServerPEMOptions.cn,
+			},
+			expectedSignerName: capi.LegacyUnknownSignerName,
+		},
+		"does not default to kubelet-serving if it specifies an emailAddress SAN": {
+			csr: capi.CertificateSigningRequestSpec{
+				Request:  csrWithOpts(kubeletServerPEMOptions, pemOptions{emailAddresses: []string{"something"}}),
+				Usages:   kubeletServerUsages,
+				Username: kubeletServerPEMOptions.cn,
+			},
+			expectedSignerName: capi.LegacyUnknownSignerName,
+		},
+	}
+	for name, test := range tests {
+		t.Run(name, func(t *testing.T) {
+			// create a deepcopy to be sure we don't modify anything in-place
+			csrSpec := test.csr.DeepCopy()
+			SetDefaults_CertificateSigningRequestSpec(csrSpec)
+			if *csrSpec.SignerName != test.expectedSignerName {
+				t.Errorf("expected signerName to be defaulted to %q but it is %q", test.expectedSignerName, *csrSpec.SignerName)
+			}
+		})
+	}
+}
+
+func TestSetDefaults_CertificateSigningRequestSpec_KubeletClient(t *testing.T) {
+	tests := map[string]struct {
+		csr                capi.CertificateSigningRequestSpec
+		expectedSignerName string
+	}{
+		"defaults for kube-apiserver-client-kubelet": {
+			csr: capi.CertificateSigningRequestSpec{
+				Request: csrWithOpts(kubeletClientPEMOptions),
+				Usages:  kubeletClientUsages,
+			},
+			expectedSignerName: capi.KubeAPIServerClientKubeletSignerName,
+		},
+		"does not default to kube-apiserver-client-kubelet if org is not 'system:nodes'": {
+			csr: capi.CertificateSigningRequestSpec{
+				Request: csrWithOpts(kubeletClientPEMOptions, pemOptions{org: "not-system:nodes"}),
+				Usages:  kubeletClientUsages,
+			},
+			expectedSignerName: capi.LegacyUnknownSignerName,
+		},
+		"does not default to kube-apiserver-client-kubelet if a dnsName is set": {
+			csr: capi.CertificateSigningRequestSpec{
+				Request: csrWithOpts(kubeletClientPEMOptions, pemOptions{dnsNames: []string{"something"}}),
+				Usages:  kubeletClientUsages,
+			},
+			expectedSignerName: capi.LegacyUnknownSignerName,
+		},
+		"does not default to kube-apiserver-client-kubelet if an emailAddress is set": {
+			csr: capi.CertificateSigningRequestSpec{
+				Request: csrWithOpts(kubeletClientPEMOptions, pemOptions{emailAddresses: []string{"something"}}),
+				Usages:  kubeletClientUsages,
+			},
+			expectedSignerName: capi.LegacyUnknownSignerName,
+		},
+		"does not default to kube-apiserver-client-kubelet if a uri SAN is set": {
+			csr: capi.CertificateSigningRequestSpec{
+				Request: csrWithOpts(kubeletClientPEMOptions, pemOptions{uris: []string{"http://something"}}),
+				Usages:  kubeletClientUsages,
+			},
+			expectedSignerName: capi.LegacyUnknownSignerName,
+		},
+		"does not default to kube-apiserver-client-kubelet if an ipAddress is set": {
+			csr: capi.CertificateSigningRequestSpec{
+				Request: csrWithOpts(kubeletClientPEMOptions, pemOptions{ipAddresses: []net.IP{{0, 0, 0, 0}}}),
+				Usages:  kubeletClientUsages,
+			},
+			expectedSignerName: capi.LegacyUnknownSignerName,
+		},
+		"does not default to kube-apiserver-client-kubelet if CN does not have 'system:node:' prefix": {
+			csr: capi.CertificateSigningRequestSpec{
+				Request: csrWithOpts(kubeletClientPEMOptions, pemOptions{cn: "not-prefixed"}),
+				Usages:  kubeletClientUsages,
+			},
+			expectedSignerName: capi.LegacyUnknownSignerName,
+		},
+		"does not default to kube-apiserver-client-kubelet if it has an unexpected usage": {
+			csr: capi.CertificateSigningRequestSpec{
+				Request: csrWithOpts(kubeletClientPEMOptions),
+				Usages:  append(kubeletClientUsages, capi.UsageServerAuth),
+			},
+			expectedSignerName: capi.LegacyUnknownSignerName,
+		},
+		"does not default to kube-apiserver-client-kubelet if it is missing an expected usage": {
+			csr: capi.CertificateSigningRequestSpec{
+				Request: csrWithOpts(kubeletClientPEMOptions),
+				// Remove the first usage in 'kubeletClientUsages'
+				Usages: kubeletClientUsages[1:],
+			},
+			expectedSignerName: capi.LegacyUnknownSignerName,
+		},
+	}
+	for name, test := range tests {
+		t.Run(name, func(t *testing.T) {
+			// create a deepcopy to be sure we don't modify anything in-place
+			csrSpec := test.csr.DeepCopy()
+			SetDefaults_CertificateSigningRequestSpec(csrSpec)
+			if *csrSpec.SignerName != test.expectedSignerName {
+				t.Errorf("expected signerName to be defaulted to %q but it is %q", test.expectedSignerName, *csrSpec.SignerName)
+			}
+		})
+	}
+}
+
+type pemOptions struct {
+	cn             string
+	org            string
+	ipAddresses    []net.IP
+	dnsNames       []string
+	emailAddresses []string
+	uris           []string
+}
+
+// overlayPEMOptions overlays one set of pemOptions on top of another to allow
+// for easily overriding a single field in the options
+func overlayPEMOptions(opts ...pemOptions) pemOptions {
+	if len(opts) == 0 {
+		return pemOptions{}
+	}
+	base := opts[0]
+	for _, opt := range opts[1:] {
+		if opt.cn != "" {
+			base.cn = opt.cn
+		}
+		if opt.org != "" {
+			base.org = opt.org
+		}
+		if opt.ipAddresses != nil {
+			base.ipAddresses = opt.ipAddresses
+		}
+		if opt.dnsNames != nil {
+			base.dnsNames = opt.dnsNames
+		}
+		if opt.emailAddresses != nil {
+			base.emailAddresses = opt.emailAddresses
+		}
+		if opt.uris != nil {
+			base.uris = opt.uris
+		}
+	}
+	return base
+}
+
+func csrWithOpts(base pemOptions, overlays ...pemOptions) []byte {
+	opts := overlayPEMOptions(append([]pemOptions{base}, overlays...)...)
+	uris := make([]*url.URL, len(opts.uris))
+	for i, s := range opts.uris {
+		u, err := url.ParseRequestURI(s)
+		if err != nil {
+			panic(err)
+		}
+		uris[i] = u
+	}
+	template := &x509.CertificateRequest{
+		Subject: pkix.Name{
+			CommonName:   opts.cn,
+			Organization: []string{opts.org},
+		},
+		IPAddresses:    opts.ipAddresses,
+		DNSNames:       opts.dnsNames,
+		EmailAddresses: opts.emailAddresses,
+		URIs:           uris,
+	}
+
+	_, key, err := ed25519.GenerateKey(rand.Reader)
+	if err != nil {
+		panic(err)
+	}
+
+	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
+}

pkg/apis/certificates/v1beta1/helpers.go

@@ -20,14 +20,11 @@ import (
 	"crypto/x509"
 	"encoding/pem"
 	"errors"
-
-	certificatesv1beta1 "k8s.io/api/certificates/v1beta1"
 )
 
-// ParseCSR extracts the CSR from the API object and decodes it.
-func ParseCSR(obj *certificatesv1beta1.CertificateSigningRequest) (*x509.CertificateRequest, error) {
+// ParseCSR decodes a PEM encoded CSR
+func ParseCSR(pemBytes []byte) (*x509.CertificateRequest, error) {
 	// extract PEM from request object
-	pemBytes := obj.Spec.Request
 	block, _ := pem.Decode(pemBytes)
 	if block == nil || block.Type != "CERTIFICATE REQUEST" {
 		return nil, errors.New("PEM block type must be CERTIFICATE REQUEST")

pkg/apis/certificates/v1beta1/register.go

@@ -46,5 +46,5 @@ func init() {
 	// We only register manually written functions here. The registration of the
 	// generated functions takes place in the generated files. The separation
 	// makes the code compile even when the generated files are missing.
-	localSchemeBuilder.Register(addDefaultingFuncs)
+	localSchemeBuilder.Register(addDefaultingFuncs, addConversionFuncs)
 }

pkg/apis/certificates/v1beta1/zz_generated.conversion.go

@@ -24,6 +24,7 @@ import (
 	unsafe "unsafe"
 
 	v1beta1 "k8s.io/api/certificates/v1beta1"
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	conversion "k8s.io/apimachinery/pkg/conversion"
 	runtime "k8s.io/apimachinery/pkg/runtime"
 	certificates "k8s.io/kubernetes/pkg/apis/certificates"
@@ -149,7 +150,17 @@ func Convert_certificates_CertificateSigningRequestCondition_To_v1beta1_Certific
 
 func autoConvert_v1beta1_CertificateSigningRequestList_To_certificates_CertificateSigningRequestList(in *v1beta1.CertificateSigningRequestList, out *certificates.CertificateSigningRequestList, s conversion.Scope) error {
 	out.ListMeta = in.ListMeta
-	out.Items = *(*[]certificates.CertificateSigningRequest)(unsafe.Pointer(&in.Items))
+	if in.Items != nil {
+		in, out := &in.Items, &out.Items
+		*out = make([]certificates.CertificateSigningRequest, len(*in))
+		for i := range *in {
+			if err := Convert_v1beta1_CertificateSigningRequest_To_certificates_CertificateSigningRequest(&(*in)[i], &(*out)[i], s); err != nil {
+				return err
+			}
+		}
+	} else {
+		out.Items = nil
+	}
 	return nil
 }
 
@@ -160,7 +171,17 @@ func Convert_v1beta1_CertificateSigningRequestList_To_certificates_CertificateSi
 
 func autoConvert_certificates_CertificateSigningRequestList_To_v1beta1_CertificateSigningRequestList(in *certificates.CertificateSigningRequestList, out *v1beta1.CertificateSigningRequestList, s conversion.Scope) error {
 	out.ListMeta = in.ListMeta
-	out.Items = *(*[]v1beta1.CertificateSigningRequest)(unsafe.Pointer(&in.Items))
+	if in.Items != nil {
+		in, out := &in.Items, &out.Items
+		*out = make([]v1beta1.CertificateSigningRequest, len(*in))
+		for i := range *in {
+			if err := Convert_certificates_CertificateSigningRequest_To_v1beta1_CertificateSigningRequest(&(*in)[i], &(*out)[i], s); err != nil {
+				return err
+			}
+		}
+	} else {
+		out.Items = nil
+	}
 	return nil
 }
 
@@ -171,6 +192,9 @@ func Convert_certificates_CertificateSigningRequestList_To_v1beta1_CertificateSi
 
 func autoConvert_v1beta1_CertificateSigningRequestSpec_To_certificates_CertificateSigningRequestSpec(in *v1beta1.CertificateSigningRequestSpec, out *certificates.CertificateSigningRequestSpec, s conversion.Scope) error {
 	out.Request = *(*[]byte)(unsafe.Pointer(&in.Request))
+	if err := v1.Convert_Pointer_string_To_string(&in.SignerName, &out.SignerName, s); err != nil {
+		return err
+	}
 	out.Usages = *(*[]certificates.KeyUsage)(unsafe.Pointer(&in.Usages))
 	out.Username = in.Username
 	out.UID = in.UID
@@ -186,6 +210,9 @@ func Convert_v1beta1_CertificateSigningRequestSpec_To_certificates_CertificateSi
 
 func autoConvert_certificates_CertificateSigningRequestSpec_To_v1beta1_CertificateSigningRequestSpec(in *certificates.CertificateSigningRequestSpec, out *v1beta1.CertificateSigningRequestSpec, s conversion.Scope) error {
 	out.Request = *(*[]byte)(unsafe.Pointer(&in.Request))
+	if err := v1.Convert_string_To_Pointer_string(&in.SignerName, &out.SignerName, s); err != nil {
+		return err
+	}
 	out.Usages = *(*[]v1beta1.KeyUsage)(unsafe.Pointer(&in.Usages))
 	out.Username = in.Username
 	out.UID = in.UID

pkg/apis/certificates/validation/BUILD

@@ -3,6 +3,7 @@ package(default_visibility = ["//visibility:public"])
 load(
     "@io_bazel_rules_go//go:def.bzl",
     "go_library",
+    "go_test",
 )
 
 go_library(
@@ -12,6 +13,7 @@ go_library(
     deps = [
         "//pkg/apis/certificates:go_default_library",
         "//pkg/apis/core/validation:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/util/validation:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
     ],
 )
@@ -28,3 +30,14 @@ filegroup(
     srcs = [":package-srcs"],
     tags = ["automanaged"],
 )
+
+go_test(
+    name = "go_default_test",
+    srcs = ["validation_test.go"],
+    embed = [":go_default_library"],
+    deps = [
+        "//pkg/apis/certificates:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
+    ],
+)

pkg/apis/certificates/validation/validation.go

@@ -18,8 +18,11 @@ package validation
 
 import (
 	"fmt"
+	"strings"
 
+	utilvalidation "k8s.io/apimachinery/pkg/util/validation"
 	"k8s.io/apimachinery/pkg/util/validation/field"
+
 	"k8s.io/kubernetes/pkg/apis/certificates"
 	apivalidation "k8s.io/kubernetes/pkg/apis/core/validation"
 )
@@ -48,19 +51,95 @@ func ValidateCertificateRequestName(name string, prefix bool) []string {
 func ValidateCertificateSigningRequest(csr *certificates.CertificateSigningRequest) field.ErrorList {
 	isNamespaced := false
 	allErrs := apivalidation.ValidateObjectMeta(&csr.ObjectMeta, isNamespaced, ValidateCertificateRequestName, field.NewPath("metadata"))
-	err := validateCSR(csr)
 
 	specPath := field.NewPath("spec")
-
+	err := validateCSR(csr)
 	if err != nil {
 		allErrs = append(allErrs, field.Invalid(specPath.Child("request"), csr.Spec.Request, fmt.Sprintf("%v", err)))
 	}
 	if len(csr.Spec.Usages) == 0 {
 		allErrs = append(allErrs, field.Required(specPath.Child("usages"), "usages must be provided"))
 	}
+	allErrs = append(allErrs, ValidateCertificateSigningRequestSignerName(specPath.Child("signerName"), csr.Spec.SignerName)...)
 	return allErrs
 }
 
+// ensure signerName is of the form domain.com/something and up to 571 characters.
+// This length and format is specified to accommodate signerNames like:
+// <fqdn>/<resource-namespace>.<resource-name>.
+// The max length of a FQDN is 253 characters (DNS1123Subdomain max length)
+// The max length of a namespace name is 63 characters (DNS1123Label max length)
+// The max length of a resource name is 253 characters (DNS1123Subdomain max length)
+// We then add an additional 2 characters to account for the one '.' and one '/'.
+func ValidateCertificateSigningRequestSignerName(fldPath *field.Path, signerName string) field.ErrorList {
+	var el field.ErrorList
+	if len(signerName) == 0 {
+		el = append(el, field.Required(fldPath, "signerName must be provided"))
+		return el
+	}
+
+	segments := strings.Split(signerName, "/")
+	// validate that there is one '/' in the signerName.
+	// we do this after validating the domain segment to provide more info to the user.
+	if len(segments) != 2 {
+		el = append(el, field.Invalid(fldPath, signerName, "must be a fully qualified domain and path of the form 'example.com/signer-name'"))
+		// return early here as we should not continue attempting to validate a missing or malformed path segment
+		// (i.e. one containing multiple or zero `/`)
+		return el
+	}
+
+	// validate that segments[0] is less than 253 characters altogether
+	maxDomainSegmentLength := utilvalidation.DNS1123SubdomainMaxLength
+	if len(segments[0]) > maxDomainSegmentLength {
+		el = append(el, field.TooLong(fldPath, segments[0], maxDomainSegmentLength))
+	}
+	// validate that segments[0] consists of valid DNS1123 labels separated by '.'
+	domainLabels := strings.Split(segments[0], ".")
+	for _, lbl := range domainLabels {
+		// use IsDNS1123Label as we want to ensure the max length of any single label in the domain
+		// is 63 characters
+		if errs := utilvalidation.IsDNS1123Label(lbl); len(errs) > 0 {
+			for _, err := range errs {
+				el = append(el, field.Invalid(fldPath, segments[0], fmt.Sprintf("validating label %q: %s", lbl, err)))
+			}
+			// if we encounter any errors whilst parsing the domain segment, break from
+			// validation as any further error messages will be duplicates, and non-distinguishable
+			// from each other, confusing users.
+			break
+		}
+	}
+
+	// validate that there is at least one '.' in segments[0]
+	if len(domainLabels) < 2 {
+		el = append(el, field.Invalid(fldPath, segments[0], "should be a domain with at least two segments separated by dots"))
+	}
+
+	// validate that segments[1] consists of valid DNS1123 subdomains separated by '.'.
+	pathLabels := strings.Split(segments[1], ".")
+	for _, lbl := range pathLabels {
+		// use IsDNS1123Subdomain because it enforces a length restriction of 253 characters
+		// which is required in order to fit a full resource name into a single 'label'
+		if errs := utilvalidation.IsDNS1123Subdomain(lbl); len(errs) > 0 {
+			for _, err := range errs {
+				el = append(el, field.Invalid(fldPath, segments[1], fmt.Sprintf("validating label %q: %s", lbl, err)))
+			}
+			// if we encounter any errors whilst parsing the path segment, break from
+			// validation as any further error messages will be duplicates, and non-distinguishable
+			// from each other, confusing users.
+			break
+		}
+	}
+
+	// ensure that segments[1] can accommodate a dns label + dns subdomain + '.'
+	maxPathSegmentLength := utilvalidation.DNS1123SubdomainMaxLength + utilvalidation.DNS1123LabelMaxLength + 1
+	maxSignerNameLength := maxDomainSegmentLength + maxPathSegmentLength + 1
+	if len(signerName) > maxSignerNameLength {
+		el = append(el, field.TooLong(fldPath, signerName, maxSignerNameLength))
+	}
+
+	return el
+}
+
 func ValidateCertificateSigningRequestUpdate(newCSR, oldCSR *certificates.CertificateSigningRequest) field.ErrorList {
 	validationErrorList := ValidateCertificateSigningRequest(newCSR)
 	metaUpdateErrorList := apivalidation.ValidateObjectMetaUpdate(&newCSR.ObjectMeta, &oldCSR.ObjectMeta, field.NewPath("metadata"))

pkg/apis/certificates/validation/validation_test.go

@@ -0,0 +1,308 @@
+/*
+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 validation
+
+import (
+	"crypto/ed25519"
+	"crypto/rand"
+	"crypto/x509"
+	"crypto/x509/pkix"
+	"encoding/pem"
+	"fmt"
+	"reflect"
+	"strings"
+	"testing"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/validation/field"
+
+	capi "k8s.io/kubernetes/pkg/apis/certificates"
+)
+
+var (
+	validObjectMeta = metav1.ObjectMeta{Name: "testcsr"}
+	validSignerName = "example.com/valid-name"
+	validUsages     = []capi.KeyUsage{capi.UsageKeyEncipherment}
+)
+
+func TestValidateCertificateSigningRequest(t *testing.T) {
+	specPath := field.NewPath("spec")
+	// maxLengthSignerName is a signerName that is of maximum length, utilising
+	// the max length specifications defined in validation.go.
+	// It is of the form <fqdn(253)>/<resource-namespace(63)>.<resource-name(253)>
+	maxLengthFQDN := fmt.Sprintf("%s.%s.%s.%s", repeatString("a", 63), repeatString("a", 63), repeatString("a", 63), repeatString("a", 61))
+	maxLengthSignerName := fmt.Sprintf("%s/%s.%s", maxLengthFQDN, repeatString("a", 63), repeatString("a", 253))
+	tests := map[string]struct {
+		csr  capi.CertificateSigningRequest
+		errs field.ErrorList
+	}{
+		"CSR with empty request data should fail": {
+			csr: capi.CertificateSigningRequest{
+				ObjectMeta: validObjectMeta,
+				Spec: capi.CertificateSigningRequestSpec{
+					Usages:     validUsages,
+					SignerName: validSignerName,
+				},
+			},
+			errs: field.ErrorList{
+				field.Invalid(specPath.Child("request"), []byte(nil), "PEM block type must be CERTIFICATE REQUEST"),
+			},
+		},
+		"CSR with invalid request data should fail": {
+			csr: capi.CertificateSigningRequest{
+				ObjectMeta: validObjectMeta,
+				Spec: capi.CertificateSigningRequestSpec{
+					Usages:     validUsages,
+					SignerName: validSignerName,
+					Request:    []byte("invalid data"),
+				},
+			},
+			errs: field.ErrorList{
+				field.Invalid(specPath.Child("request"), []byte("invalid data"), "PEM block type must be CERTIFICATE REQUEST"),
+			},
+		},
+		"CSR with no usages should fail": {
+			csr: capi.CertificateSigningRequest{
+				ObjectMeta: validObjectMeta,
+				Spec: capi.CertificateSigningRequestSpec{
+					SignerName: validSignerName,
+					Request:    newCSRPEM(t),
+				},
+			},
+			errs: field.ErrorList{
+				field.Required(specPath.Child("usages"), "usages must be provided"),
+			},
+		},
+		"CSR with no signerName set should fail": {
+			csr: capi.CertificateSigningRequest{
+				ObjectMeta: validObjectMeta,
+				Spec: capi.CertificateSigningRequestSpec{
+					Usages:  validUsages,
+					Request: newCSRPEM(t),
+				},
+			},
+			errs: field.ErrorList{
+				field.Required(specPath.Child("signerName"), "signerName must be provided"),
+			},
+		},
+		"signerName contains no '/'": {
+			csr: capi.CertificateSigningRequest{
+				ObjectMeta: validObjectMeta,
+				Spec: capi.CertificateSigningRequestSpec{
+					Usages:     validUsages,
+					Request:    newCSRPEM(t),
+					SignerName: "an-invalid-signer-name",
+				},
+			},
+			errs: field.ErrorList{
+				field.Invalid(specPath.Child("signerName"), "an-invalid-signer-name", "must be a fully qualified domain and path of the form 'example.com/signer-name'"),
+			},
+		},
+		"signerName contains two '/'": {
+			csr: capi.CertificateSigningRequest{
+				ObjectMeta: validObjectMeta,
+				Spec: capi.CertificateSigningRequestSpec{
+					Usages:     validUsages,
+					Request:    newCSRPEM(t),
+					SignerName: "an-invalid-signer-name.com/something/else",
+				},
+			},
+			errs: field.ErrorList{
+				field.Invalid(specPath.Child("signerName"), "an-invalid-signer-name.com/something/else", "must be a fully qualified domain and path of the form 'example.com/signer-name'"),
+			},
+		},
+		"signerName domain component is not fully qualified": {
+			csr: capi.CertificateSigningRequest{
+				ObjectMeta: validObjectMeta,
+				Spec: capi.CertificateSigningRequestSpec{
+					Usages:     validUsages,
+					Request:    newCSRPEM(t),
+					SignerName: "example/some-signer-name",
+				},
+			},
+			errs: field.ErrorList{
+				field.Invalid(specPath.Child("signerName"), "example", "should be a domain with at least two segments separated by dots"),
+			},
+		},
+		"signerName path component is empty": {
+			csr: capi.CertificateSigningRequest{
+				ObjectMeta: validObjectMeta,
+				Spec: capi.CertificateSigningRequestSpec{
+					Usages:     validUsages,
+					Request:    newCSRPEM(t),
+					SignerName: "example.com/",
+				},
+			},
+			errs: field.ErrorList{
+				field.Invalid(specPath.Child("signerName"), "", `validating label "": a DNS-1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')`),
+			},
+		},
+		"signerName path component ends with a symbol": {
+			csr: capi.CertificateSigningRequest{
+				ObjectMeta: validObjectMeta,
+				Spec: capi.CertificateSigningRequestSpec{
+					Usages:     validUsages,
+					Request:    newCSRPEM(t),
+					SignerName: "example.com/something-",
+				},
+			},
+			errs: field.ErrorList{
+				field.Invalid(specPath.Child("signerName"), "something-", `validating label "something-": a DNS-1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')`),
+			},
+		},
+		"signerName path component is a symbol": {
+			csr: capi.CertificateSigningRequest{
+				ObjectMeta: validObjectMeta,
+				Spec: capi.CertificateSigningRequestSpec{
+					Usages:     validUsages,
+					Request:    newCSRPEM(t),
+					SignerName: "example.com/-",
+				},
+			},
+			errs: field.ErrorList{
+				field.Invalid(specPath.Child("signerName"), "-", `validating label "-": a DNS-1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')`),
+			},
+		},
+		"signerName path component contains no '.' but is valid": {
+			csr: capi.CertificateSigningRequest{
+				ObjectMeta: validObjectMeta,
+				Spec: capi.CertificateSigningRequestSpec{
+					Usages:     validUsages,
+					Request:    newCSRPEM(t),
+					SignerName: "example.com/some-signer-name",
+				},
+			},
+			errs: field.ErrorList{},
+		},
+		"signerName with a total length greater than 571 characters should be rejected": {
+			csr: capi.CertificateSigningRequest{
+				ObjectMeta: validObjectMeta,
+				Spec: capi.CertificateSigningRequestSpec{
+					Usages:  validUsages,
+					Request: newCSRPEM(t),
+					// this string is longer than the max signerName limit (635 chars)
+					SignerName: maxLengthSignerName + ".toolong",
+				},
+			},
+			errs: field.ErrorList{
+				field.TooLong(specPath.Child("signerName"), maxLengthSignerName+".toolong", len(maxLengthSignerName)),
+			},
+		},
+		"signerName with a fqdn greater than 253 characters should be rejected": {
+			csr: capi.CertificateSigningRequest{
+				ObjectMeta: validObjectMeta,
+				Spec: capi.CertificateSigningRequestSpec{
+					Usages:  validUsages,
+					Request: newCSRPEM(t),
+					// this string is longer than the max signerName limit (635 chars)
+					SignerName: fmt.Sprintf("%s.extra/valid-path", maxLengthFQDN),
+				},
+			},
+			errs: field.ErrorList{
+				field.TooLong(specPath.Child("signerName"), fmt.Sprintf("%s.extra", maxLengthFQDN), len(maxLengthFQDN)),
+			},
+		},
+		"signerName can have a longer path if the domain component is less than the max length": {
+			csr: capi.CertificateSigningRequest{
+				ObjectMeta: validObjectMeta,
+				Spec: capi.CertificateSigningRequestSpec{
+					Usages:     validUsages,
+					Request:    newCSRPEM(t),
+					SignerName: fmt.Sprintf("abc.io/%s.%s", repeatString("a", 253), repeatString("a", 253)),
+				},
+			},
+			errs: field.ErrorList{},
+		},
+		"signerName with a domain label greater than 63 characters will fail": {
+			csr: capi.CertificateSigningRequest{
+				ObjectMeta: validObjectMeta,
+				Spec: capi.CertificateSigningRequestSpec{
+					Usages:     validUsages,
+					Request:    newCSRPEM(t),
+					SignerName: fmt.Sprintf("%s.example.io/valid-path", repeatString("a", 66)),
+				},
+			},
+			errs: field.ErrorList{
+				field.Invalid(specPath.Child("signerName"), fmt.Sprintf("%s.example.io", repeatString("a", 66)), fmt.Sprintf(`validating label "%s": must be no more than 63 characters`, repeatString("a", 66))),
+			},
+		},
+		"signerName of max length in format <fully-qualified-domain-name>/<resource-namespace>.<resource-name> is valid": {
+			// ensure signerName is of the form domain.com/something and up to 571 characters.
+			// This length and format is specified to accommodate signerNames like:
+			// <fqdn>/<resource-namespace>.<resource-name>.
+			// The max length of a FQDN is 253 characters (DNS1123Subdomain max length)
+			// The max length of a namespace name is 63 characters (DNS1123Label max length)
+			// The max length of a resource name is 253 characters (DNS1123Subdomain max length)
+			// We then add an additional 2 characters to account for the one '.' and one '/'.
+			csr: capi.CertificateSigningRequest{
+				ObjectMeta: validObjectMeta,
+				Spec: capi.CertificateSigningRequestSpec{
+					Usages:     validUsages,
+					Request:    newCSRPEM(t),
+					SignerName: maxLengthSignerName,
+				},
+			},
+			errs: field.ErrorList{},
+		},
+	}
+	for name, test := range tests {
+		t.Run(name, func(t *testing.T) {
+			el := ValidateCertificateSigningRequest(&test.csr)
+			if !reflect.DeepEqual(el, test.errs) {
+				t.Errorf("returned and expected errors did not match - expected %v but got %v", test.errs.ToAggregate(), el.ToAggregate())
+			}
+		})
+	}
+}
+
+func repeatString(s string, num int) string {
+	l := make([]string, num)
+	for i := 0; i < num; i++ {
+		l[i] = s
+	}
+	return strings.Join(l, "")
+}
+
+func newCSRPEM(t *testing.T) []byte {
+	template := &x509.CertificateRequest{
+		Subject: pkix.Name{
+			Organization: []string{"testing-org"},
+		},
+	}
+
+	_, key, err := ed25519.GenerateKey(rand.Reader)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	csrDER, err := x509.CreateCertificateRequest(rand.Reader, template, key)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	csrPemBlock := &pem.Block{
+		Type:  "CERTIFICATE REQUEST",
+		Bytes: csrDER,
+	}
+
+	p := pem.EncodeToMemory(csrPemBlock)
+	if p == nil {
+		t.Fatal("invalid pem block")
+	}
+
+	return p
+}