From 876d9f14768acd17aabfce71ddeca8eae15da9c1 Mon Sep 17 00:00:00 2001
From: Damien Claisse <d.claisse@criteo.com>
Date: Mon, 3 Oct 2022 15:30:18 +0000
Subject: [PATCH] Connect: re-add common name on secondary CA intermediate
 certificate

https://github.com/hashicorp/consul/pull/10424 introduced a regression,
because it removed common name also for secondary CA intermediate
certificate, which used the same code path. This violates RFC 5280,
which mandates that Issuer CN must not be empty and equal to Subject CN
of subordinate certificate. Partially revert this patch to fix consul
provider, and add a test case to prevent future regressions.
---
 agent/connect/ca/provider_consul.go      |  8 +++++++-
 agent/connect/ca/provider_consul_test.go |  5 +++++
 agent/connect/csr.go                     | 22 ++++++++++++++++++++--
 3 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/agent/connect/ca/provider_consul.go b/agent/connect/ca/provider_consul.go
index 40eb2f44570e..26e5ecc9c6bd 100644
--- a/agent/connect/ca/provider_consul.go
+++ b/agent/connect/ca/provider_consul.go
@@ -222,7 +222,13 @@ func (c *ConsulProvider) GenerateIntermediateCSR() (string, error) {
 		return "", err
 	}
 
-	csr, err := connect.CreateCACSR(c.spiffeID, signer)
+	uid, err := connect.CompactUID()
+	if err != nil {
+		return "", err
+	}
+	cn := connect.CACN("consul", uid, c.clusterID, c.isPrimary)
+
+	csr, err := connect.CreateCACSR(c.spiffeID, cn, signer)
 	if err != nil {
 		return "", err
 	}
diff --git a/agent/connect/ca/provider_consul_test.go b/agent/connect/ca/provider_consul_test.go
index a4df05b76729..df2fa1962c3e 100644
--- a/agent/connect/ca/provider_consul_test.go
+++ b/agent/connect/ca/provider_consul_test.go
@@ -425,6 +425,9 @@ func testSignIntermediateCrossDC(t *testing.T, provider1, provider2 Provider) {
 	// Sign the CSR with provider1.
 	intermediatePEM, err := provider1.SignIntermediate(csr)
 	require.NoError(t, err)
+	intermediateCert, err := connect.ParseCert(intermediatePEM)
+	require.NoError(t, err)
+	require.NotEmpty(t, intermediateCert.Subject.CommonName)
 	root, err := provider1.GenerateRoot()
 	require.NoError(t, err)
 	rootPEM := root.PEM
@@ -451,6 +454,8 @@ func testSignIntermediateCrossDC(t *testing.T, provider1, provider2 Provider) {
 	require.NoError(t, err)
 	requireNotEncoded(t, cert.SubjectKeyId)
 	requireNotEncoded(t, cert.AuthorityKeyId)
+	require.NotEmpty(t, cert.Issuer.CommonName)
+	require.Equal(t, cert.Issuer.CommonName, intermediateCert.Subject.CommonName)
 
 	// Check that the leaf signed by the new cert can be verified using the
 	// returned cert chain (signed intermediate + remote root).
diff --git a/agent/connect/csr.go b/agent/connect/csr.go
index f699a5879298..73f09c726e78 100644
--- a/agent/connect/csr.go
+++ b/agent/connect/csr.go
@@ -73,13 +73,31 @@ func CreateCSR(uri CertURI, privateKey crypto.Signer,
 
 // CreateCSR returns a CA CSR to sign the given service along with the PEM-encoded
 // private key for this certificate.
-func CreateCACSR(uri CertURI, privateKey crypto.Signer) (string, error) {
+func CreateCACSR(uri CertURI, commonName string, privateKey crypto.Signer) (string, error) {
 	ext, err := CreateCAExtension()
 	if err != nil {
 		return "", err
 	}
+	template := &x509.CertificateRequest{
+		URIs:               []*url.URL{uri.URI()},
+		SignatureAlgorithm: SigAlgoForKey(privateKey),
+		ExtraExtensions:    []pkix.Extension{ext},
+		Subject:            pkix.Name{CommonName: commonName},
+	}
 
-	return CreateCSR(uri, privateKey, nil, nil, ext)
+	// Create the CSR itself
+	var csrBuf bytes.Buffer
+	bs, err := x509.CreateCertificateRequest(rand.Reader, template, privateKey)
+	if err != nil {
+		return "", err
+	}
+
+	err = pem.Encode(&csrBuf, &pem.Block{Type: "CERTIFICATE REQUEST", Bytes: bs})
+	if err != nil {
+		return "", err
+	}
+
+	return csrBuf.String(), nil
 }
 
 // CreateCAExtension creates a pkix.Extension for the x509 Basic Constraints