[FAB-1444] Move signature to top level in gossip msg

A gossip.GossipMessage is a oneof of different types of messages,
and all messages in gossip are under that message.
Messages that are signed have their signature in the inner field of the message.
To make things more secure, and also more uniform, I'm moving the signature
to the top level of the GossipMessage.

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

Author: yacovm

gossip/comm/comm_impl.go

@@ -388,20 +388,25 @@ func (c *commImpl) authenticateRemotePeer(stream stream) (common.PKIidType, erro
 	ctx := stream.Context()
 	remoteAddress := extractRemoteAddress(stream)
 	remoteCertHash := extractCertificateHashFromContext(ctx)
-	var sig []byte
 	var err error
+	var cMsg *proto.GossipMessage
+	var signer proto.Signer
 
 	// 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 {
-		sig, err = c.idMapper.Sign(c.selfCertHash)
-		if err != nil {
-			c.logger.Error("Failed signing self certificate hash:", err)
-			return nil, err
+		signer = func(msg []byte) ([]byte, error) {
+			return c.idMapper.Sign(msg)
+		}
+	} else { // If we don't use TLS, we have no unique text to sign,
+		//  so don't sign anything
+		signer = func(msg []byte) ([]byte, error) {
+			return msg, nil
 		}
 	}
 
-	cMsg := createConnectionMsg(c.PKIID, sig, c.peerIdentity)
+	cMsg = createConnectionMsg(c.PKIID, c.selfCertHash, c.peerIdentity, signer)
+
 	c.logger.Debug("Sending", cMsg, "to", remoteAddress)
 	stream.Send(cMsg)
 	m := readWithTimeout(stream, defConnTimeout)
@@ -433,7 +438,14 @@ func (c *commImpl) authenticateRemotePeer(stream stream) (common.PKIidType, erro
 
 	// if TLS is detected, verify remote peer
 	if remoteCertHash != nil && c.selfCertHash != nil {
-		err = c.idMapper.Verify(receivedMsg.PkiID, receivedMsg.Sig, remoteCertHash)
+		if !bytes.Equal(remoteCertHash, receivedMsg.Hash) {
+			return nil, fmt.Errorf("Expected %v in remote hash, but got %v", remoteCertHash, receivedMsg.Hash)
+		}
+		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)
 		if err != nil {
 			c.logger.Error("Failed verifying signature from", remoteAddress, ":", err)
 			return nil, err
@@ -516,18 +528,20 @@ func readWithTimeout(stream interface{}, timeout time.Duration) *proto.GossipMes
 	}
 }
 
-func createConnectionMsg(pkiID common.PKIidType, sig []byte, cert api.PeerIdentityType) *proto.GossipMessage {
-	return &proto.GossipMessage{
+func createConnectionMsg(pkiID common.PKIidType, hash []byte, cert api.PeerIdentityType, signer proto.Signer) *proto.GossipMessage {
+	m := &proto.GossipMessage{
 		Tag:   proto.GossipMessage_EMPTY,
 		Nonce: 0,
 		Content: &proto.GossipMessage_Conn{
 			Conn: &proto.ConnEstablish{
+				Hash:  hash,
 				Cert:  cert,
 				PkiID: pkiID,
-				Sig:   sig,
 			},
 		},
 	}
+	m.Sign(signer)
+	return m
 }
 
 type stream interface {

gossip/comm/comm_test.go

@@ -116,22 +116,29 @@ func handshaker(endpoint string, comm Comm, t *testing.T, sigMutator func([]byte
 		return nil
 	}
 	clientCertHash := certHashFromRawCert(cert.Certificate[0])
-	sig, err := naiveSec.Sign(clientCertHash)
-	if sigMutator != nil {
-		sig = sigMutator(sig)
-	}
+
 	pkiID := common.PKIidType(endpoint)
 	if pkiIDmutator != nil {
 		pkiID = common.PKIidType(pkiIDmutator([]byte(endpoint)))
 	}
-
 	assert.NoError(t, err, "%v", err)
-	msg := createConnectionMsg(pkiID, sig, []byte(endpoint))
+	msg := createConnectionMsg(pkiID, clientCertHash, []byte(endpoint), func(msg []byte) ([]byte, error) {
+		return msg, nil
+	})
+
+	if sigMutator != nil {
+		msg.Signature = sigMutator(msg.Signature)
+	}
+
 	stream.Send(msg)
 	msg, err = stream.Recv()
 	assert.NoError(t, err, "%v", err)
 	if sigMutator == nil {
-		assert.Equal(t, extractCertificateHashFromContext(stream.Context()), msg.GetConn().Sig)
+		hash := extractCertificateHashFromContext(stream.Context())
+		expectedMsg := createConnectionMsg(common.PKIidType("localhost:9611"), hash, []byte("localhost:9611"), func(msg []byte) ([]byte, error) {
+			return msg, nil
+		})
+		assert.Equal(t, expectedMsg.Signature, msg.Signature)
 	}
 	assert.Equal(t, []byte("localhost:9611"), msg.GetConn().PkiID)
 	msg2Send := createGossipMsg()

gossip/discovery/discovery.go

@@ -23,11 +23,11 @@ import (
 
 // CryptoService is an interface that the discovery expects to be implemented and passed on creation
 type CryptoService interface {
-	// validateAliveMsg validates that an Alive message is authentic
-	ValidateAliveMsg(*proto.AliveMessage) bool
+	// ValidateAliveMsg validates that an Alive message is authentic
+	ValidateAliveMsg(*proto.GossipMessage) bool
 
-	// SignMessage signs an AliveMessage and updates its signature field
-	SignMessage(*proto.AliveMessage) *proto.AliveMessage
+	// SignMessage signs a message
+	SignMessage(m *proto.GossipMessage) *proto.GossipMessage
 }
 
 // CommService is an interface that the discovery expects to be implemented and passed on creation