[FAB-6018] Make handshake sign even when no TLS

The private data pull needs to be able to determine if a peer
is eligible of receiving a private RW set of a certain collection
by evaluating a policy.
The policy needs a signature and some signed message.

If TLS is used, the peer can use the Authentication info from the handshake,
but if the peer doesn't use TLS - the authentication info isn't populated.

This commit makes the handshake sign anyway the connection established
message, and removes the "IsAuthenticated" method.

Note: This doesn't mean, of course - that not using TLS is secure now.
It only means that the handshake message is signed, but that doesn't
imply that an attacker cannot do a MITM if TLS isn't used.

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

Author: yacovm

gossip/comm/comm_impl.go

@@ -405,20 +405,10 @@ func (c *commImpl) authenticateRemotePeer(stream stream) (*proto.ConnectionInfo,
 	remoteCertHash := extractCertificateHashFromContext(ctx)
 	var err error
 	var cMsg *proto.SignedGossipMessage
-	var signer proto.Signer
 	useTLS := c.selfCertHash != nil
 
-	// 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)
-		}
-	} 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
-		}
+	signer := func(msg []byte) ([]byte, error) {
+		return c.idMapper.Sign(msg)
 	}
 
 	// TLS enabled but not detected on other side
@@ -461,6 +451,10 @@ func (c *commImpl) authenticateRemotePeer(stream stream) (*proto.ConnectionInfo,
 		ID:       receivedMsg.PkiId,
 		Identity: receivedMsg.Identity,
 		Endpoint: remoteAddress,
+		Auth: &proto.AuthInfo{
+			Signature:  m.Signature,
+			SignedData: m.Payload,
+		},
 	}
 
 	// if TLS is enabled and detected, verify remote peer
@@ -470,19 +464,16 @@ func (c *commImpl) authenticateRemotePeer(stream stream) (*proto.ConnectionInfo,
 		if !bytes.Equal(remoteCertHash, receivedMsg.TlsCertHash) {
 			return nil, errors.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.Identity, verifier)
-		if err != nil {
-			c.logger.Errorf("Failed verifying signature from %s : %v", remoteAddress, err)
-			return nil, err
-		}
-		connInfo.Auth = &proto.AuthInfo{
-			Signature:  m.Signature,
-			SignedData: m.Payload,
-		}
+	}
+	// Final step - verify the signature on the connection message itself
+	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.Identity, verifier)
+	if err != nil {
+		c.logger.Errorf("Failed verifying signature from %s : %v", remoteAddress, err)
+		return nil, err
 	}
 
 	c.logger.Debug("Authenticated", remoteAddress)

gossip/comm/comm_test.go

@@ -203,7 +203,6 @@ func TestHandshake(t *testing.T) {
 		assert.Equal(t, expectedPKIID, msg.GetConnectionInfo().ID)
 		assert.Equal(t, api.PeerIdentityType(endpoint), msg.GetConnectionInfo().Identity)
 		assert.NotNil(t, msg.GetConnectionInfo().Auth)
-		assert.True(t, msg.GetConnectionInfo().IsAuthenticated())
 		sig, _ := (&naiveSecProvider{}).Sign(msg.GetConnectionInfo().Auth.SignedData)
 		assert.Equal(t, sig, msg.GetConnectionInfo().Auth.Signature)
 	}
@@ -230,8 +229,8 @@ func TestHandshake(t *testing.T) {
 	}
 	assert.Equal(t, common.PKIidType("localhost:9608"), msg.GetConnectionInfo().ID)
 	assert.Equal(t, api.PeerIdentityType("localhost:9608"), msg.GetConnectionInfo().Identity)
-	assert.Nil(t, msg.GetConnectionInfo().Auth)
-	assert.False(t, msg.GetConnectionInfo().IsAuthenticated())
+	sig, _ := (&naiveSecProvider{}).Sign(msg.GetConnectionInfo().Auth.SignedData)
+	assert.Equal(t, sig, msg.GetConnectionInfo().Auth.Signature)
 
 	inst.Stop()
 	s.Stop()

gossip/state/state.go

@@ -175,11 +175,6 @@ func NewGossipStateProvider(chainID string, services *ServicesMediator, ledger l
 		if !msg.IsRemoteStateMessage() {
 			return false
 		}
-		// If we're not running with authentication, no point
-		// in enforcing access control
-		if !receivedMsg.GetConnectionInfo().IsAuthenticated() {
-			return true
-		}
 		connInfo := receivedMsg.GetConnectionInfo()
 		authErr := services.VerifyByChannel(msg.Channel, connInfo.Identity, connInfo.Auth.Signature, connInfo.Auth.SignedData)
 		if authErr != nil {

protos/gossip/extensions.go

@@ -345,12 +345,6 @@ func (c *ConnectionInfo) String() string {
 	return fmt.Sprintf("%s %v", c.Endpoint, c.ID)
 }
 
-// IsAuthenticated returns whether the connection to the remote peer
-// was authenticated when the handshake took place
-func (c *ConnectionInfo) IsAuthenticated() bool {
-	return c.Auth != nil
-}
-
 // AuthInfo represents the authentication
 // data that was provided by the remote peer
 // at the connection time

protos/gossip/extensions_test.go

@@ -673,21 +673,6 @@ func TestGossipMessageLeadershipMessageTagType(t *testing.T) {
 	assert.Error(t, msg.IsTagLegal())
 }
 
-func TestConnectionInfo_IsAuthenticated(t *testing.T) {
-	connInfo := &ConnectionInfo{
-		ID: common.PKIidType("peerID"),
-	}
-
-	assert.False(t, connInfo.IsAuthenticated())
-
-	connInfo = &ConnectionInfo{
-		ID:   common.PKIidType("peerID"),
-		Auth: &AuthInfo{},
-	}
-
-	assert.True(t, connInfo.IsAuthenticated())
-}
-
 func TestGossipMessageSign(t *testing.T) {
 	idSigner := func(msg []byte) ([]byte, error) {
 		return msg, nil