[FAB-17869] Allow TLS CAs with overlapping issuers (#1229)

The client root TLS CA certificate pool construction doesn't allow different issuers
with the same subject name to exist in the CA cert pool.

This prevents use cases where different organizations have issued CAs with the same subject name.

There is little sense to prevent this in the first place, as there can be only 2 possible cases:
I) The certificate chain validation of a TLS handshake allows this: In this case, Fabric should not
   artificially prevent supporting such scenarios.
II) The certificate chain validation of a TLS handshake disallows this: In this case, there is little
    gained in skipping pluralities in subject name during TLS CA cert pool construction, as the
    chain validation would prevent it anyway. In fact, the way it is done in the current code
    in Fabric is not only un-necessary but also non-deterministic, as map iteration isn't
    deterministic in Go, therefore the behavior might not be the same across different nodes.

This change set removes the artificial constraint, and adjust one of the integration tests
that was built to adjust to the said constraint.

Change-Id: Ie93c34997ad3e134a0a04b805fb007cd036d9683
Signed-off-by: yacovm <yacovm@il.ibm.com>

integration/raft/config_test.go

@@ -8,10 +8,6 @@ package raft
 
 import (
 	"bytes"
-	"crypto/ecdsa"
-	"crypto/rand"
-	"crypto/x509"
-	"encoding/pem"
 	"fmt"
 	"io/ioutil"
 	"os"
@@ -338,22 +334,15 @@ var _ = Describe("EndToEnd reconfiguration and onboarding", func() {
 			nameDomain := fmt.Sprintf("%s.%s", name, domain)
 			ordererTLSPath := filepath.Join(tmpDir, "ordererOrganizations", domain, "orderers", nameDomain, "tls")
 
-			caKeyPath := filepath.Join(tmpDir, "ordererOrganizations", domain, "tlsca", "priv_sk")
 			caCertPath := filepath.Join(tmpDir, "ordererOrganizations", domain, "tlsca", fmt.Sprintf("tlsca.%s-cert.pem", domain))
 
-			caKey, err := ioutil.ReadFile(caKeyPath)
-			Expect(err).NotTo(HaveOccurred())
-
 			caCert, err := ioutil.ReadFile(caCertPath)
 			Expect(err).NotTo(HaveOccurred())
 
 			thirdOrdererCertificatePath := filepath.Join(ordererTLSPath, "server.crt")
 			thirdOrdererCertificate, err := ioutil.ReadFile(thirdOrdererCertificatePath)
 			Expect(err).NotTo(HaveOccurred())
 
-			By("Changing its subject name")
-			caCert, thirdOrdererCertificate = changeSubjectName(caCert, caKey, thirdOrdererCertificate, "tlsca2")
-
 			By("Updating it on the file system")
 			err = ioutil.WriteFile(caCertPath, caCert, 0644)
 			Expect(err).NotTo(HaveOccurred())
@@ -417,7 +406,7 @@ var _ = Describe("EndToEnd reconfiguration and onboarding", func() {
 			By("Ensuring TLS handshakes fail with the other orderers")
 			for i, oRunner := range ordererRunners {
 				if i < 2 {
-					Eventually(oRunner.Err(), network.EventuallyTimeout).Should(gbytes.Say("TLS handshake failed with error tls: client didn't provide a certificate"))
+					Eventually(oRunner.Err(), network.EventuallyTimeout).Should(gbytes.Say("TLS handshake failed with error tls: failed to verify client certificate"))
 					continue
 				}
 				Eventually(oRunner.Err(), network.EventuallyTimeout).Should(gbytes.Say("TLS handshake failed with error remote error: tls: bad certificate"))
@@ -1741,44 +1730,3 @@ func updateEtcdRaftMetadata(network *nwo.Network, peer *nwo.Peer, orderer *nwo.O
 		return newMetadata
 	})
 }
-
-func changeSubjectName(caCertPEM, caKeyPEM, leafPEM []byte, newSubjectName string) (newCA, newLeaf []byte) {
-	keyAsDER, _ := pem.Decode(caKeyPEM)
-	caKeyWithoutType, err := x509.ParsePKCS8PrivateKey(keyAsDER.Bytes)
-	Expect(err).NotTo(HaveOccurred())
-	caKey := caKeyWithoutType.(*ecdsa.PrivateKey)
-
-	caCertAsDER, _ := pem.Decode(caCertPEM)
-	caCert, err := x509.ParseCertificate(caCertAsDER.Bytes)
-	Expect(err).NotTo(HaveOccurred())
-
-	// Change its subject name
-	caCert.Subject.CommonName = newSubjectName
-	caCert.Issuer.CommonName = newSubjectName
-	caCert.RawTBSCertificate = nil
-	caCert.RawSubjectPublicKeyInfo = nil
-	caCert.Raw = nil
-	caCert.RawSubject = nil
-	caCert.RawIssuer = nil
-
-	// The CA signs its own certificate
-	caCertBytes, err := x509.CreateCertificate(rand.Reader, caCert, caCert, caCert.PublicKey, caKey)
-	Expect(err).NotTo(HaveOccurred())
-
-	// Now it's the turn of the leaf certificate
-	leafAsDER, _ := pem.Decode(leafPEM)
-	leafCert, err := x509.ParseCertificate(leafAsDER.Bytes)
-	Expect(err).NotTo(HaveOccurred())
-
-	leafCert.Raw = nil
-	leafCert.RawIssuer = nil
-	leafCert.RawTBSCertificate = nil
-
-	// The CA signs the leaf cert
-	leafCertBytes, err := x509.CreateCertificate(rand.Reader, leafCert, caCert, leafCert.PublicKey, caKey)
-	Expect(err).NotTo(HaveOccurred())
-
-	newCA = pem.EncodeToMemory(&pem.Block{Bytes: caCertBytes, Type: "CERTIFICATE"})
-	newLeaf = pem.EncodeToMemory(&pem.Block{Bytes: leafCertBytes, Type: "CERTIFICATE"})
-	return
-}

internal/pkg/comm/server.go

@@ -33,9 +33,6 @@ type GRPCServer struct {
 	serverCertificate atomic.Value
 	// lock to protect concurrent access to append / remove
 	lock *sync.Mutex
-	// Set of PEM-encoded X509 certificate authorities used to populate
-	// the tlsConfig.ClientCAs indexed by subject
-	clientRootCAs map[string]*x509.Certificate
 	// TLS configuration used by the grpc server
 	tls *TLSConfig
 	// Server for gRPC Health Check Protocol.
@@ -110,8 +107,6 @@ func NewGRPCServerFromListener(listener net.Listener, serverConfig ServerConfig)
 				grpcServer.tls.config.ClientAuth = tls.RequireAndVerifyClientCert
 				//if we have client root CAs, create a certPool
 				if len(secureConfig.ClientRootCAs) > 0 {
-					grpcServer.clientRootCAs = make(map[string]*x509.Certificate)
-
 					grpcServer.tls.config.ClientCAs = x509.NewCertPool()
 					for _, clientRootCA := range secureConfig.ClientRootCAs {
 						err = grpcServer.appendClientRootCA(clientRootCA)
@@ -234,7 +229,7 @@ func (gServer *GRPCServer) Stop() {
 
 // internal function to add a PEM-encoded clientRootCA
 func (gServer *GRPCServer) appendClientRootCA(clientRoot []byte) error {
-	certs, subjects, err := pemToX509Certs(clientRoot)
+	certs, err := pemToX509Certs(clientRoot)
 	if err != nil {
 		return errors.WithMessage(err, "failed to append client root certificate(s)")
 	}
@@ -243,11 +238,8 @@ func (gServer *GRPCServer) appendClientRootCA(clientRoot []byte) error {
 		return errors.New("no client root certificates found")
 	}
 
-	for i, cert := range certs {
-		//first add to the ClientCAs
+	for _, cert := range certs {
 		gServer.tls.AddClientRootCA(cert)
-		//add it to our clientRootCAs map using subject as key
-		gServer.clientRootCAs[subjects[i]] = cert
 	}
 
 	return nil
@@ -259,26 +251,18 @@ func (gServer *GRPCServer) SetClientRootCAs(clientRoots [][]byte) error {
 	gServer.lock.Lock()
 	defer gServer.lock.Unlock()
 
-	//create a new map and CertPool
-	clientRootCAs := make(map[string]*x509.Certificate)
+	certPool := x509.NewCertPool()
+
 	for _, clientRoot := range clientRoots {
-		certs, subjects, err := pemToX509Certs(clientRoot)
+		certs, err := pemToX509Certs(clientRoot)
 		if err != nil {
 			return errors.WithMessage(err, "failed to set client root certificate(s)")
 		}
 
-		for i, cert := range certs {
-			clientRootCAs[subjects[i]] = cert
+		for _, cert := range certs {
+			certPool.AddCert(cert)
 		}
 	}
-
-	//create a new CertPool and populate with the new clientRootCAs
-	certPool := x509.NewCertPool()
-	for _, clientRoot := range clientRootCAs {
-		certPool.AddCert(clientRoot)
-	}
-
-	gServer.clientRootCAs = clientRootCAs
 	gServer.tls.SetClientCAs(certPool)
 	return nil
 }

internal/pkg/comm/util.go

@@ -22,7 +22,7 @@ import (
 
 // AddPemToCertPool adds PEM-encoded certs to a cert pool
 func AddPemToCertPool(pemCerts []byte, pool *x509.CertPool) error {
-	certs, _, err := pemToX509Certs(pemCerts)
+	certs, err := pemToX509Certs(pemCerts)
 	if err != nil {
 		return err
 	}
@@ -33,7 +33,7 @@ func AddPemToCertPool(pemCerts []byte, pool *x509.CertPool) error {
 }
 
 // parse PEM-encoded certs
-func pemToX509Certs(pemCerts []byte) ([]*x509.Certificate, []string, error) {
+func pemToX509Certs(pemCerts []byte) ([]*x509.Certificate, error) {
 	var certs []*x509.Certificate
 	var subjects []string
 
@@ -47,14 +47,14 @@ func pemToX509Certs(pemCerts []byte) ([]*x509.Certificate, []string, error) {
 
 		cert, err := x509.ParseCertificate(block.Bytes)
 		if err != nil {
-			return nil, []string{}, err
+			return nil, err
 		}
 
 		certs = append(certs, cert)
 		subjects = append(subjects, string(cert.RawSubject))
 	}
 
-	return certs, subjects, nil
+	return certs, nil
 }
 
 // BindingInspector receives as parameters a gRPC context and an Envelope,