Fix common name length limitation issue (#822)

* Hash and restrict the length of common names when they exceed the length limitation

pkg/pki/certmanagerpki/certmanager_pki.go

@@ -219,7 +219,7 @@ func caCertForCluster(cluster *v1beta1.KafkaCluster) *certv1.Certificate {
 		},
 		Spec: certv1.CertificateSpec{
 			SecretName: fmt.Sprintf(pkicommon.BrokerCACertTemplate, cluster.Name),
-			CommonName: fmt.Sprintf(pkicommon.CAFQDNTemplate, cluster.Name, cluster.Namespace),
+			CommonName: pkicommon.EnsureValidCommonNameLen(fmt.Sprintf(pkicommon.CAFQDNTemplate, cluster.Name, cluster.Namespace)),
 			IsCA:       true,
 			IssuerRef: certmeta.ObjectReference{
 				Name: fmt.Sprintf(pkicommon.BrokerSelfSignerTemplate, cluster.Name),

pkg/util/cert/certutil.go

@@ -178,12 +178,12 @@ func GenerateJKSFromByte(certByte []byte, privateKey []byte, caCert []byte) (out
 func GenerateJKS(certs []*x509.Certificate, privateKey []byte) (out, passw []byte, err error) {
 	pKeyRaw, err := DecodePrivateKeyBytes(privateKey)
 	if err != nil {
-		return
+		return nil, nil, err
 	}
 
 	pKeyPKCS8, err := x509.MarshalPKCS8PrivateKey(pKeyRaw)
 	if err != nil {
-		return
+		return nil, nil, err
 	}
 
 	certCABundle := make([]keystore.Certificate, 0, len(certs))

pkg/util/pki/common.go

@@ -16,19 +16,20 @@ package pki
 
 import (
 	"context"
+	"crypto/sha256"
 	"crypto/tls"
 	"fmt"
 	"sort"
 	"strings"
 
-	"github.com/banzaicloud/koperator/pkg/errorfactory"
-	"github.com/banzaicloud/koperator/pkg/k8sutil"
-
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
+	"github.com/banzaicloud/koperator/pkg/errorfactory"
+	"github.com/banzaicloud/koperator/pkg/k8sutil"
+
 	"github.com/banzaicloud/koperator/api/v1alpha1"
 	"github.com/banzaicloud/koperator/api/v1beta1"
 	"github.com/banzaicloud/koperator/pkg/resources/templates"
@@ -57,6 +58,8 @@ const (
 	// KafkaUserAnnotationName used in case of PKIbackend is k8s-csr to find the appropriate kafkauser in case of
 	// signing request event
 	KafkaUserAnnotationName = "banzaicloud.io/owner"
+	// MaxCNLen specifies the number of chars that the longest common name can have
+	MaxCNLen = 64
 )
 
 // Manager is the main interface for objects performing PKI operations
@@ -184,7 +187,7 @@ func BrokerUserForCluster(cluster *v1beta1.KafkaCluster, extListenerStatuses map
 	}
 	additionalHosts = sortAndDedupe(additionalHosts)
 	return &v1alpha1.KafkaUser{
-		ObjectMeta: templates.ObjectMeta(GetCommonName(cluster), LabelsForKafkaPKI(cluster.Name, cluster.Namespace), cluster),
+		ObjectMeta: templates.ObjectMeta(EnsureValidCommonNameLen(GetCommonName(cluster)), LabelsForKafkaPKI(cluster.Name, cluster.Namespace), cluster),
 		Spec: v1alpha1.KafkaUserSpec{
 			SecretName: fmt.Sprintf(BrokerServerCertTemplate, cluster.Name),
 			DNSNames:   append(GetInternalDNSNames(cluster), additionalHosts...),
@@ -215,9 +218,9 @@ func sortAndDedupe(hosts []string) []string {
 func ControllerUserForCluster(cluster *v1beta1.KafkaCluster) *v1alpha1.KafkaUser {
 	return &v1alpha1.KafkaUser{
 		ObjectMeta: templates.ObjectMeta(
-			fmt.Sprintf(BrokerControllerFQDNTemplate,
-				fmt.Sprintf(BrokerControllerTemplate, cluster.Name), cluster.Namespace, cluster.Spec.GetKubernetesClusterDomain()),
-			LabelsForKafkaPKI(cluster.Name, cluster.Namespace), cluster,
+			EnsureValidCommonNameLen(fmt.Sprintf(BrokerControllerFQDNTemplate, fmt.Sprintf(BrokerControllerTemplate, cluster.Name), cluster.Namespace, cluster.Spec.GetKubernetesClusterDomain())),
+			LabelsForKafkaPKI(cluster.Name, cluster.Namespace),
+			cluster,
 		),
 		Spec: v1alpha1.KafkaUserSpec{
 			SecretName: fmt.Sprintf(BrokerControllerTemplate, cluster.Name),
@@ -243,3 +246,14 @@ func EnsureControllerReference(ctx context.Context, user *v1alpha1.KafkaUser,
 	}
 	return nil
 }
+
+// EnsureValidCommonNameLen ensures that the passed-in common name doesn't exceed the longest supported length
+func EnsureValidCommonNameLen(s string) string {
+	if len(s) > MaxCNLen {
+		first := sha256.New()
+		first.Write([]byte(s))
+		// encode the hash to make sure it only consists of lower case alphanumeric characters to enforce RFC 1123
+		return fmt.Sprintf("%x", first.Sum(nil))
+	}
+	return s
+}

pkg/util/pki/pki_common_test.go

@@ -175,3 +175,42 @@ func TestControllerUserForCluster(t *testing.T) {
 		t.Errorf("Expected %+v\nGot %+v", expected, user)
 	}
 }
+
+func TestTruncatedCommonName(t *testing.T) {
+	testCases := []struct {
+		testName             string
+		commonName           string
+		expectedHashedString string
+		expectedLen          int
+	}{
+		{
+			testName:    "Common name with length exceeded limitation",
+			commonName:  "kafka-test-controller-exceeded-length.test-namespace.mgt.cluster.local",
+			expectedLen: MaxCNLen,
+		},
+		{
+			testName:             "Common name with length within limitation",
+			commonName:           "kafka-test-controller.test-namespace.mgt.cluster.local",
+			expectedHashedString: "kafka-test-controller.test-namespace.mgt.cluster.local",
+		},
+	}
+
+	t.Parallel()
+
+	for _, test := range testCases {
+		test := test
+		t.Run(test.testName, func(t *testing.T) {
+			get := EnsureValidCommonNameLen(test.commonName)
+
+			if test.expectedHashedString == "" {
+				if len(get) != test.expectedLen {
+					t.Errorf("Common name with exceeded lenghth after hashing doesnt match expected length, got:%d, expected:%d", len(get), test.expectedLen)
+				}
+			} else {
+				if get != test.expectedHashedString {
+					t.Errorf("Common name with valid length shouldn't be changed, got:%s, expected:%s", get, test.expectedHashedString)
+				}
+			}
+		})
+	}
+}