[FAB-3359] Mutual TLS support in gossip handshake

This commit enforces mutual TLS in the gossip handshake, and
removes the flag that was used to skip mutual TLS.

It also adds more test scenarios for the handshake test that
give 100% code coverage for the handshake method.

Signed-off-by: Yacov Manevich <[email protected]>

Change-Id: I69d6e482a1b594b392121b92b437a1ad644c590e
Signed-off-by: Yacov Manevich <[email protected]>

Author: yacovm

core/comm/connection.go

@@ -102,9 +102,11 @@ func (cas *CASupport) GetDeliverServiceCredentials() credentials.TransportCreden
 
 // GetPeerCredentials returns GRPC transport credentials for use by GRPC
 // clients which communicate with remote peer endpoints.
-func (cas *CASupport) GetPeerCredentials() credentials.TransportCredentials {
+func (cas *CASupport) GetPeerCredentials(tlsCert tls.Certificate) credentials.TransportCredentials {
 	var creds credentials.TransportCredentials
-	var tlsConfig = &tls.Config{}
+	var tlsConfig = &tls.Config{
+		Certificates: []tls.Certificate{tlsCert},
+	}
 	var certPool = x509.NewCertPool()
 	// loop through the orderer CAs
 	roots, _ := cas.GetServerRootCAs()

core/comm/connection_test.go

@@ -24,6 +24,8 @@ import (
 
 	"github.com/spf13/viper"
 
+	"crypto/tls"
+
 	"github.com/hyperledger/fabric/core/testutil"
 	"github.com/stretchr/testify/assert"
 	"google.golang.org/grpc"
@@ -142,7 +144,7 @@ func TestCASupport(t *testing.T) {
 	creds := cas.GetDeliverServiceCredentials()
 	assert.Equal(t, "1.2", creds.Info().SecurityVersion,
 		"Expected Security version to be 1.2")
-	creds = cas.GetPeerCredentials()
+	creds = cas.GetPeerCredentials(tls.Certificate{})
 	assert.Equal(t, "1.2", creds.Info().SecurityVersion,
 		"Expected Security version to be 1.2")
 
@@ -152,7 +154,7 @@ func TestCASupport(t *testing.T) {
 	creds = cas.GetDeliverServiceCredentials()
 	assert.Equal(t, "1.2", creds.Info().SecurityVersion,
 		"Expected Security version to be 1.2")
-	creds = cas.GetPeerCredentials()
+	creds = cas.GetPeerCredentials(tls.Certificate{})
 	assert.Equal(t, "1.2", creds.Info().SecurityVersion,
 		"Expected Security version to be 1.2")
 

core/comm/server.go

@@ -140,6 +140,7 @@ func NewGRPCServerFromListener(listener net.Listener, secureConfig SecureServerC
 				Certificates:           certificates,
 				SessionTicketsDisabled: true,
 			}
+			grpcServer.tlsConfig.ClientAuth = tls.RequestClientCert
 			//checkif client authentication is required
 			if secureConfig.RequireClientCert {
 				//require TLS client auth

examples/cluster/config/core.yaml

@@ -155,9 +155,6 @@ peer:
         # This is an endpoint that is published to peers outside of the organization.
         # If this isn't set, the peer will not be known to other organizations.
         externalEndpoint:
-        # Makes gossip skip verification of remote peer signature when performing
-        # the authentication handshake with remote peers
-        skipHandshake: true
 
         # Leader election service configuration
         election:

examples/e2e_cli/base/peer-base.yaml

@@ -19,9 +19,6 @@ services:
       - CORE_PEER_ENDORSER_ENABLED=true
       - CORE_PEER_GOSSIP_USELEADERELECTION=true
       - CORE_PEER_GOSSIP_ORGLEADER=false
-      # The following setting skips the gossip handshake since we are
-      # are not doing mutual TLS
-      - CORE_PEER_GOSSIP_SKIPHANDSHAKE=true
       - CORE_PEER_PROFILE_ENABLED=true
       - CORE_PEER_TLS_CERT_FILE=/etc/hyperledger/fabric/tls/server.crt
       - CORE_PEER_TLS_KEY_FILE=/etc/hyperledger/fabric/tls/server.key

gossip/comm/comm_impl.go

@@ -108,10 +108,6 @@ func NewCommInstanceWithServer(port int, idMapper identity.Mapper, peerIdentity
 		proto.RegisterGossipServer(s, commInst)
 	}
 
-	if viper.GetBool("peer.gossip.skipHandshake") {
-		commInst.skipHandshake = true
-	}
-
 	return commInst, nil
 }
 
@@ -416,10 +412,11 @@ func (c *commImpl) authenticateRemotePeer(stream stream) (*proto.ConnectionInfo,
 	var err error
 	var cMsg *proto.SignedGossipMessage
 	var signer proto.Signer
+	useTLS := c.selfCertHash != nil
 
-	// If TLS is detected, sign the hash of our cert to bind our TLS cert
-	// to the gRPC session
-	if remoteCertHash != nil && c.selfCertHash != nil && !c.skipHandshake {
+	// If TLS is enabled, sign the connection message in order to bind
+	// the TLS session to the peer's identity
+	if useTLS {
 		signer = func(msg []byte) ([]byte, error) {
 			return c.idMapper.Sign(msg)
 		}
@@ -430,50 +427,57 @@ func (c *commImpl) authenticateRemotePeer(stream stream) (*proto.ConnectionInfo,
 		}
 	}
 
+	// TLS enabled but not detected on other side
+	if useTLS && len(remoteCertHash) == 0 {
+		c.logger.Warningf("%s didn't send TLS certificate", remoteAddress)
+		return nil, errors.New("No TLS certificate")
+	}
+
 	cMsg = c.createConnectionMsg(c.PKIID, c.selfCertHash, c.peerIdentity, signer)
 
 	c.logger.Debug("Sending", cMsg, "to", remoteAddress)
 	stream.Send(cMsg.Envelope)
 	m, err := readWithTimeout(stream, util.GetDurationOrDefault("peer.gossip.connTimeout", defConnTimeout), remoteAddress)
 	if err != nil {
-		err := fmt.Errorf("Failed reading messge from %s, reason: %v", remoteAddress, err)
-		c.logger.Warning(err)
+		c.logger.Warningf("Failed reading messge from %s, reason: %v", remoteAddress, err)
 		return nil, err
 	}
 	receivedMsg := m.GetConn()
 	if receivedMsg == nil {
-		c.logger.Warning("Expected connection message but got", receivedMsg)
+		c.logger.Warning("Expected connection message from", remoteAddress, "but got", receivedMsg)
 		return nil, errors.New("Wrong type")
 	}
 
 	if receivedMsg.PkiId == nil {
-		c.logger.Warning("%s didn't send a pkiID")
-		return nil, fmt.Errorf("%s didn't send a pkiID", remoteAddress)
+		c.logger.Warning("%s didn't send a pkiID", remoteAddress)
+		return nil, errors.New("No PKI-ID")
 	}
 
 	c.logger.Debug("Received", receivedMsg, "from", remoteAddress)
-	err = c.idMapper.Put(receivedMsg.PkiId, receivedMsg.Cert)
+	err = c.idMapper.Put(receivedMsg.PkiId, receivedMsg.Identity)
 	if err != nil {
 		c.logger.Warning("Identity store rejected", remoteAddress, ":", err)
 		return nil, err
 	}
 
 	connInfo := &proto.ConnectionInfo{
 		ID:       receivedMsg.PkiId,
-		Identity: receivedMsg.Cert,
+		Identity: receivedMsg.Identity,
 		Endpoint: remoteAddress,
 	}
 
 	// if TLS is enabled and detected, verify remote peer
-	if remoteCertHash != nil && c.selfCertHash != nil && !c.skipHandshake {
-		if !bytes.Equal(remoteCertHash, receivedMsg.Hash) {
-			return nil, fmt.Errorf("Expected %v in remote hash, but got %v", remoteCertHash, receivedMsg.Hash)
+	if useTLS {
+		// If the remote peer sent its TLS certificate, make sure it actually matches the TLS cert
+		// that the peer used.
+		if !bytes.Equal(remoteCertHash, receivedMsg.TlsCertHash) {
+			return nil, fmt.Errorf("Expected %v in remote hash of TLS cert, but got %v", remoteCertHash, receivedMsg.TlsCertHash)
 		}
 		verifier := func(peerIdentity []byte, signature, message []byte) error {
 			pkiID := c.idMapper.GetPKIidOfCert(api.PeerIdentityType(peerIdentity))
 			return c.idMapper.Verify(pkiID, signature, message)
 		}
-		err = m.Verify(receivedMsg.Cert, verifier)
+		err = m.Verify(receivedMsg.Identity, verifier)
 		if err != nil {
 			c.logger.Error("Failed verifying signature from", remoteAddress, ":", err)
 			return nil, err
@@ -484,13 +488,6 @@ func (c *commImpl) authenticateRemotePeer(stream stream) (*proto.ConnectionInfo,
 		}
 	}
 
-	// TLS enabled but not detected on other side, and we're not configured to skip handshake verification
-	if remoteCertHash == nil && c.selfCertHash != nil && !c.skipHandshake {
-		err = fmt.Errorf("Remote peer %s didn't send TLS certificate", remoteAddress)
-		c.logger.Warning(err)
-		return nil, err
-	}
-
 	c.logger.Debug("Authenticated", remoteAddress)
 
 	return connInfo, nil
@@ -583,15 +580,15 @@ func readWithTimeout(stream interface{}, timeout time.Duration, address string)
 	}
 }
 
-func (c *commImpl) createConnectionMsg(pkiID common.PKIidType, hash []byte, cert api.PeerIdentityType, signer proto.Signer) *proto.SignedGossipMessage {
+func (c *commImpl) createConnectionMsg(pkiID common.PKIidType, certHash []byte, cert api.PeerIdentityType, signer proto.Signer) *proto.SignedGossipMessage {
 	m := &proto.GossipMessage{
 		Tag:   proto.GossipMessage_EMPTY,
 		Nonce: 0,
 		Content: &proto.GossipMessage_Conn{
 			Conn: &proto.ConnEstablish{
-				Hash:  hash,
-				Cert:  cert,
-				PkiId: pkiID,
+				TlsCertHash: certHash,
+				Identity:    cert,
+				PkiId:       pkiID,
 			},
 		},
 	}