[FAB-7237] Specify TLS client key pair for peer

Prior to this change, the peer used the
TLS server key pair when making client
TLS connections.  In many cases, this
will be fine, but in cases where the
TLS server key pair is issued by
a public CA (e.g. Verisign), this is
an issue as now any client certificate
issued by Versign will be valid.

So this change simply adds an optional
TLS client key pair to the peer config.

Change-Id: I0195407ef0ecef58059e726a6c9eb6e473926cc8
Signed-off-by: Gari Singh <gari.r.singh@gmail.com>

Author: mastersingh24

core/comm/connection.go

@@ -44,7 +44,7 @@ type CASupport struct {
 // CredentialSupport type manages credentials used for gRPC client connections
 type CredentialSupport struct {
 	*CASupport
-	ClientCert tls.Certificate
+	clientCert tls.Certificate
 }
 
 // GetCredentialSupport returns the singleton CredentialSupport instance
@@ -109,6 +109,12 @@ func (cas *CASupport) GetClientRootCAs() (appRootCAs, ordererRootCAs [][]byte) {
 	return appRootCAs, ordererRootCAs
 }
 
+// SetClientCertificate sets the tls.Certificate to use for gRPC client
+// connections
+func (cs *CredentialSupport) SetClientCertificate(cert tls.Certificate) {
+	cs.clientCert = cert
+}
+
 // GetDeliverServiceCredentials returns GRPC transport credentials for given channel to be used by GRPC
 // clients which communicate with ordering service endpoints.
 // If the channel isn't found, error is returned.
@@ -118,7 +124,7 @@ func (cs *CredentialSupport) GetDeliverServiceCredentials(channelID string) (cre
 
 	var creds credentials.TransportCredentials
 	tlsConfig := &tls.Config{
-		Certificates: []tls.Certificate{cs.ClientCert},
+		Certificates: []tls.Certificate{cs.clientCert},
 	}
 	certPool := x509.NewCertPool()
 
@@ -151,7 +157,7 @@ func (cs *CredentialSupport) GetDeliverServiceCredentials(channelID string) (cre
 func (cs *CredentialSupport) GetPeerCredentials() credentials.TransportCredentials {
 	var creds credentials.TransportCredentials
 	tlsConfig := &tls.Config{
-		Certificates: []tls.Certificate{cs.ClientCert},
+		Certificates: []tls.Certificate{cs.clientCert},
 	}
 	certPool := x509.NewCertPool()
 	// loop through the server root CAs

core/comm/connection_test.go

@@ -195,7 +195,10 @@ func TestCredentialSupport(t *testing.T) {
 	}
 
 	cs := GetCredentialSupport()
-	cs.ClientCert = tls.Certificate{}
+	cert := tls.Certificate{Certificate: [][]byte{}}
+	cs.SetClientCertificate(cert)
+	assert.Equal(t, cert, cs.clientCert)
+
 	cs.AppRootCAsByChain["channel1"] = [][]byte{rootCAs[0]}
 	cs.AppRootCAsByChain["channel2"] = [][]byte{rootCAs[1]}
 	cs.AppRootCAsByChain["channel3"] = [][]byte{rootCAs[2]}

core/peer/config.go

@@ -20,6 +20,7 @@ SPDX-License-Identifier: Apache-2.0
 package peer
 
 import (
+	"crypto/tls"
 	"fmt"
 	"io/ioutil"
 	"net"
@@ -172,3 +173,59 @@ func GetServerConfig() (comm.ServerConfig, error) {
 	}
 	return serverConfig, nil
 }
+
+// GetClientCertificate returns the TLS certificate to use for gRPC client
+// connections
+func GetClientCertificate() (tls.Certificate, error) {
+	cert := tls.Certificate{}
+
+	keyPath := viper.GetString("peer.tls.clientKey.file")
+	certPath := viper.GetString("peer.tls.clientCert.file")
+
+	if keyPath != "" || certPath != "" {
+		// need both keyPath and certPath to be set
+		if keyPath == "" || certPath == "" {
+			return cert, errors.New("peer.tls.clientKey.file and " +
+				"peer.tls.clientCert.file must both be set or must both be empty")
+		}
+		keyPath = config.GetPath("peer.tls.clientKey.file")
+		certPath = config.GetPath("peer.tls.clientCert.file")
+
+	} else {
+		// use the TLS server keypair
+		keyPath = viper.GetString("peer.tls.key.file")
+		certPath = viper.GetString("peer.tls.key.file")
+
+		if keyPath != "" || certPath != "" {
+			// need both keyPath and certPath to be set
+			if keyPath == "" || certPath == "" {
+				return cert, errors.New("peer.tls.key.file and " +
+					"peer.tls.cert.file must both be set or must both be empty")
+			}
+			keyPath = config.GetPath("peer.tls.key.file")
+			certPath = config.GetPath("peer.tls.cert.file")
+		} else {
+			return cert, errors.New("must set either " +
+				"[peer.tls.key.file and peer.tls.cert.file] or " +
+				"[peer.tls.clientKey.file and peer.tls.clientCert.file]" +
+				"when peer.tls.clientAuthEnabled is set to true")
+		}
+	}
+	// get the keypair from the file system
+	clientKey, err := ioutil.ReadFile(keyPath)
+	if err != nil {
+		return cert, errors.WithMessage(err,
+			"error loading client TLS key")
+	}
+	clientCert, err := ioutil.ReadFile(certPath)
+	if err != nil {
+		return cert, errors.WithMessage(err,
+			"error loading client TLS certificate")
+	}
+	cert, err = tls.X509KeyPair(clientCert, clientKey)
+	if err != nil {
+		return cert, errors.WithMessage(err,
+			"error parsing client TLS key pair")
+	}
+	return cert, nil
+}

core/peer/config_test.go

@@ -6,6 +6,7 @@ SPDX-License-Identifier: Apache-2.0
 package peer
 
 import (
+	"crypto/tls"
 	"fmt"
 	"net"
 	"path/filepath"
@@ -176,3 +177,75 @@ func TestGetServerConfig(t *testing.T) {
 	viper.Set("peer.tls.clientAuthRequired", false)
 
 }
+
+func TestGetClientCertificate(t *testing.T) {
+	viper.Set("peer.tls.key.file", "")
+	viper.Set("peer.tls.cert.file", "")
+	viper.Set("peer.tls.clientKey.file", "")
+	viper.Set("peer.tls.clientCert.file", "")
+
+	// neither client nor server key pairs set - expect error
+	_, err := GetClientCertificate()
+	assert.Error(t, err)
+
+	viper.Set("peer.tls.key.file", "")
+	viper.Set("peer.tls.cert.file",
+		filepath.Join("testdata", "Org1-server1-cert.pem"))
+	// missing server key file - expect error
+	_, err = GetClientCertificate()
+	assert.Error(t, err)
+
+	viper.Set("peer.tls.key.file",
+		filepath.Join("testdata", "Org1-server1-key.pem"))
+	viper.Set("peer.tls.cert.file", "")
+	// missing server cert file - expect error
+	_, err = GetClientCertificate()
+	assert.Error(t, err)
+
+	// set server TLS settings to ensure we get the client TLS settings
+	// when they are set properly
+	viper.Set("peer.tls.key.file",
+		filepath.Join("testdata", "Org1-server1-key.pem"))
+	viper.Set("peer.tls.cert.file",
+		filepath.Join("testdata", "Org1-server1-cert.pem"))
+
+	// peer.tls.clientCert.file not set - expect error
+	viper.Set("peer.tls.clientKey.file",
+		filepath.Join("testdata", "Org2-server1-key.pem"))
+	_, err = GetClientCertificate()
+	assert.Error(t, err)
+
+	// peer.tls.clientKey.file not set - expect error
+	viper.Set("peer.tls.clientKey.file", "")
+	viper.Set("peer.tls.clientCert.file",
+		filepath.Join("testdata", "Org2-server1-cert.pem"))
+	_, err = GetClientCertificate()
+	assert.Error(t, err)
+
+	// client auth required and clientKey/clientCert set
+	expected, err := tls.LoadX509KeyPair(
+		filepath.Join("testdata", "Org2-server1-cert.pem"),
+		filepath.Join("testdata", "Org2-server1-key.pem"))
+	if err != nil {
+		t.Fatalf("Failed to load test certificate (%s)", err)
+	}
+	viper.Set("peer.tls.clientKey.file",
+		filepath.Join("testdata", "Org2-server1-key.pem"))
+	cert, err := GetClientCertificate()
+	assert.NoError(t, err)
+	assert.Equal(t, expected, cert)
+
+	// client auth required and clientKey/clientCert not set - expect
+	// client cert to be the server cert
+	viper.Set("peer.tls.clientKey.file", "")
+	viper.Set("peer.tls.clientCert.file", "")
+	expected, err = tls.LoadX509KeyPair(
+		filepath.Join("testdata", "Org1-server1-cert.pem"),
+		filepath.Join("testdata", "Org1-server1-key.pem"))
+	if err != nil {
+		t.Fatalf("Failed to load test certificate (%s)", err)
+	}
+	cert, err = GetClientCertificate()
+	assert.NoError(t, err)
+	assert.Equal(t, expected, cert)
+}

peer/node/start.go

@@ -142,6 +142,13 @@ func serve(args []string) error {
 		// set up credential support
 		cs := comm.GetCredentialSupport()
 		cs.ServerRootCAs = serverConfig.SecOpts.ServerRootCAs
+
+		// set the cert to use if client auth is requested by remote endpoints
+		clientCert, err := peer.GetClientCertificate()
+		if err != nil {
+			logger.Fatalf("Failed to set TLS client certficate (%s)", err)
+		}
+		comm.GetCredentialSupport().SetClientCertificate(clientCert)
 	}
 
 	//TODO - do we need different SSL material for events ?
@@ -215,7 +222,6 @@ func serve(args []string) error {
 		dialOpts = append(dialOpts, comm.ClientKeepaliveOptions(kaOpts)...)
 
 		if comm.TLSEnabled() {
-			comm.GetCredentialSupport().ClientCert = peerServer.ServerCertificate()
 			dialOpts = append(dialOpts, grpc.WithTransportCredentials(comm.GetCredentialSupport().GetPeerCredentials()))
 		} else {
 			dialOpts = append(dialOpts, grpc.WithInsecure())

sampleconfig/core.yaml

@@ -263,28 +263,34 @@ peer:
     # Note that peer-chaincode connections through chaincodeListenAddress is
     # not mutual TLS auth. See comments on chaincodeListenAddress for more info
     tls:
-        # require server-side TLS
+        # Require server-side TLS
         enabled:  false
-        # require client certificates / mutual TLS.
-        # note that clients that are not configured to use a certificate will
+        # Require client certificates / mutual TLS.
+        # Note that clients that are not configured to use a certificate will
         # fail to connect to the peer.
         clientAuthRequired: false
-        # X.509 certificate used for TLS server (and client if clientAuthEnabled
-        # is set to true
+        # X.509 certificate used for TLS server
         cert:
             file: tls/server.crt
-        # private key used for TLS server (and client if clientAuthEnabled
+        # Private key used for TLS server (and client if clientAuthEnabled
         # is set to true
         key:
             file: tls/server.key
-        # trusted root certificate chain for tls.cert
+        # Trusted root certificate chain for tls.cert
         rootcert:
             file: tls/ca.crt
-        # set of root certificate authorities used to verify client certificates
+        # Set of root certificate authorities used to verify client certificates
         clientRootCAs:
             files:
               - tls/ca.crt
-
+        # Private key used for TLS when making client connections.  If
+        # not set, peer.tls.key.file will be used instead
+        clientKey:
+            file:
+        # X.509 certificate used for TLS when making client connections.
+        # If not set, peer.tls.cert.file will be used instead
+        clientCert:
+            file:
         # The server name use to verify the hostname returned by TLS handshake
         serverhostoverride: