Gossip:Add option to skip handshake verification

This commit adds a configuration option to make gossip
skip verification of signature upon handshake.

It will be (hopefully!) removed once mutual TLS will be configured
in the peers and integrated with gossip.

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

Author: yacovm

gossip/comm/comm_impl.go

@@ -115,6 +115,10 @@ func NewCommInstanceWithServer(port int, idMapper identity.Mapper, peerIdentity
 		proto.RegisterGossipServer(s, commInst)
 	}
 
+	if viper.GetBool("peer.gossip.skipHandshake") {
+		commInst.skipHandshake = true
+	}
+
 	return commInst, nil
 }
 
@@ -140,6 +144,7 @@ func NewCommInstance(s *grpc.Server, cert *tls.Certificate, idStore identity.Map
 }
 
 type commImpl struct {
+	skipHandshake     bool
 	selfCertHash      []byte
 	peerIdentity      api.PeerIdentityType
 	idMapper          identity.Mapper
@@ -424,7 +429,7 @@ func (c *commImpl) authenticateRemotePeer(stream stream) (*proto.ConnectionInfo,
 
 	// 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 {
+	if remoteCertHash != nil && c.selfCertHash != nil && !c.skipHandshake {
 		signer = func(msg []byte) ([]byte, error) {
 			return c.idMapper.Sign(msg)
 		}
@@ -472,8 +477,8 @@ func (c *commImpl) authenticateRemotePeer(stream stream) (*proto.ConnectionInfo,
 		Identity: receivedMsg.Cert,
 	}
 
-	// if TLS is detected, verify remote peer
-	if remoteCertHash != nil && c.selfCertHash != nil {
+	// 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)
 		}
@@ -492,6 +497,13 @@ 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

gossip/comm/comm_test.go

@@ -18,6 +18,8 @@ package comm
 
 import (
 	"bytes"
+	"crypto/hmac"
+	"crypto/sha256"
 	"crypto/tls"
 	"fmt"
 	"math/rand"
@@ -49,7 +51,10 @@ func acceptAll(msg interface{}) bool {
 	return true
 }
 
-var naiveSec = &naiveSecProvider{}
+var (
+	naiveSec = &naiveSecProvider{}
+	hmacKey  = []byte{0, 0, 0}
+)
 
 type naiveSecProvider struct {
 }
@@ -72,15 +77,19 @@ func (*naiveSecProvider) VerifyBlock(chainID common.ChainID, signedBlock []byte)
 // Sign signs msg with this peer's signing key and outputs
 // the signature if no error occurred.
 func (*naiveSecProvider) Sign(msg []byte) ([]byte, error) {
-	return msg, nil
+	mac := hmac.New(sha256.New, hmacKey)
+	mac.Write(msg)
+	return mac.Sum(nil), nil
 }
 
 // Verify checks that signature is a valid signature of message under a peer's verification key.
 // If the verification succeeded, Verify returns nil meaning no error occurred.
 // If peerCert is nil, then the signature is verified against this peer's verification key.
 func (*naiveSecProvider) Verify(peerIdentity api.PeerIdentityType, signature, message []byte) error {
-	equal := bytes.Equal(signature, message)
-	if !equal {
+	mac := hmac.New(sha256.New, hmacKey)
+	mac.Write(message)
+	expected := mac.Sum(nil)
+	if !bytes.Equal(signature, expected) {
 		return fmt.Errorf("Wrong certificate:%v, %v", signature, message)
 	}
 	return nil
@@ -98,16 +107,22 @@ func newCommInstance(port int, sec api.MessageCryptoService) (Comm, error) {
 	return inst, err
 }
 
-func handshaker(endpoint string, comm Comm, t *testing.T, sigMutator func([]byte) []byte, pkiIDmutator func([]byte) []byte) <-chan proto.ReceivedMessage {
+func handshaker(endpoint string, comm Comm, t *testing.T, sigMutator func([]byte) []byte, pkiIDmutator func([]byte) []byte, mutualTLS bool) <-chan proto.ReceivedMessage {
 	c := &commImpl{}
 	err := generateCertificates("key.pem", "cert.pem")
 	defer os.Remove("cert.pem")
 	defer os.Remove("key.pem")
 	cert, err := tls.LoadX509KeyPair("cert.pem", "key.pem")
-	ta := credentials.NewTLS(&tls.Config{
+	tlsCfg := &tls.Config{
 		InsecureSkipVerify: true,
-		Certificates:       []tls.Certificate{cert},
-	})
+	}
+
+	if mutualTLS {
+		tlsCfg.Certificates = []tls.Certificate{cert}
+	}
+
+	ta := credentials.NewTLS(tlsCfg)
+
 	acceptChan := comm.Accept(acceptAll)
 	conn, err := grpc.Dial("localhost:9611", grpc.WithTransportCredentials(&authCreds{tlsCreds: ta}), grpc.WithBlock(), grpc.WithTimeout(time.Second))
 	assert.NoError(t, err, "%v", err)
@@ -119,16 +134,25 @@ func handshaker(endpoint string, comm Comm, t *testing.T, sigMutator func([]byte
 	assert.NoError(t, err, "%v", err)
 	if err != nil {
 		return nil
+	} // cert.Certificate[0]
+
+	var clientCertHash []byte
+	if mutualTLS {
+		clientCertHash = certHashFromRawCert(tlsCfg.Certificates[0].Certificate[0])
 	}
-	clientCertHash := certHashFromRawCert(cert.Certificate[0])
 
 	pkiID := common.PKIidType(endpoint)
 	if pkiIDmutator != nil {
 		pkiID = common.PKIidType(pkiIDmutator([]byte(endpoint)))
 	}
 	assert.NoError(t, err, "%v", err)
 	msg := c.createConnectionMsg(pkiID, clientCertHash, []byte(endpoint), func(msg []byte) ([]byte, error) {
-		return msg, nil
+		if !mutualTLS {
+			return msg, nil
+		}
+		mac := hmac.New(sha256.New, hmacKey)
+		mac.Write(msg)
+		return mac.Sum(nil), nil
 	})
 
 	if sigMutator != nil {
@@ -143,9 +167,14 @@ func handshaker(endpoint string, comm Comm, t *testing.T, sigMutator func([]byte
 	if sigMutator == nil {
 		hash := extractCertificateHashFromContext(stream.Context())
 		expectedMsg := c.createConnectionMsg(common.PKIidType("localhost:9611"), hash, []byte("localhost:9611"), func(msg []byte) ([]byte, error) {
-			return msg, nil
+			mac := hmac.New(sha256.New, hmacKey)
+			mac.Write(msg)
+			return mac.Sum(nil), nil
 		})
-		assert.Equal(t, expectedMsg.Envelope.Signature, msg.Envelope.Signature)
+		if mutualTLS {
+			assert.Equal(t, expectedMsg.Envelope.Signature, msg.Envelope.Signature)
+		}
+
 	}
 	assert.Equal(t, []byte("localhost:9611"), msg.GetConn().PkiId)
 	msg2Send := createGossipMsg()
@@ -177,7 +206,7 @@ func TestHandshake(t *testing.T) {
 	comm, _ := newCommInstance(9611, naiveSec)
 	defer comm.Stop()
 
-	acceptChan := handshaker("localhost:9610", comm, t, nil, nil)
+	acceptChan := handshaker("localhost:9610", comm, t, nil, nil, true)
 	time.Sleep(2 * time.Second)
 	assert.Equal(t, 1, len(acceptChan))
 	msg := <-acceptChan
@@ -186,7 +215,8 @@ func TestHandshake(t *testing.T) {
 	assert.Equal(t, api.PeerIdentityType("localhost:9610"), msg.GetConnectionInfo().Identity)
 	assert.NotNil(t, msg.GetConnectionInfo().Auth)
 	assert.True(t, msg.GetConnectionInfo().IsAuthenticated())
-	assert.Equal(t, msg.GetConnectionInfo().Auth.Signature, msg.GetConnectionInfo().Auth.SignedData)
+	sig, _ := (&naiveSecProvider{}).Sign(msg.GetConnectionInfo().Auth.SignedData)
+	assert.Equal(t, sig, msg.GetConnectionInfo().Auth.Signature)
 	// negative path, nothing should be read from the channel because the signature is wrong
 	mutateSig := func(b []byte) []byte {
 		if b[0] == 0 {
@@ -196,17 +226,36 @@ func TestHandshake(t *testing.T) {
 		}
 		return b
 	}
-	acceptChan = handshaker("localhost:9612", comm, t, mutateSig, nil)
+	acceptChan = handshaker("localhost:9612", comm, t, mutateSig, nil, true)
 	time.Sleep(time.Second)
 	assert.Equal(t, 0, len(acceptChan))
 
 	// negative path, nothing should be read from the channel because the PKIid doesn't match the identity
 	mutatePKIID := func(b []byte) []byte {
 		return []byte("localhost:9650")
 	}
-	acceptChan = handshaker("localhost:9613", comm, t, nil, mutatePKIID)
+	acceptChan = handshaker("localhost:9613", comm, t, nil, mutatePKIID, true)
 	time.Sleep(time.Second)
 	assert.Equal(t, 0, len(acceptChan))
+
+	// Now we test for a handshake without mutual TLS
+	// The first time should fail
+	acceptChan = handshaker("localhost:9614", comm, t, nil, nil, false)
+	select {
+	case <-acceptChan:
+		assert.Fail(t, "Should not have successfully authenticated to remote peer")
+	case <-time.After(time.Second):
+	}
+
+	// And the second time should succeed
+	comm.(*commImpl).skipHandshake = true
+	acceptChan = handshaker("localhost:9615", comm, t, nil, nil, false)
+	select {
+	case <-acceptChan:
+	case <-time.After(time.Second * 10):
+		assert.Fail(t, "skipHandshake flag should have authorized the authentication")
+	}
+
 }
 
 func TestBasic(t *testing.T) {

peer/core.yaml

@@ -126,6 +126,9 @@ 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: false
 
         # Leader election service configuration
         election: