[FAB-6462] Enforce PEM types for gossip identities

yacovm · yacovm · commit 64faaaed7499 · 2017-10-13T12:17:55.000+03:00
The MSP support for gossip (MessageCryptoService) takes into account
The entire PEM of an x509 based identity, which is too lenient.

Instead of enforcing explicit sanitation of input for GetPKIidOfCert(),
We will just make identities that have the wrong PEM type - invalid.

This is much more efficient because the GetPKIidOfCert is called many times
in the code, and sanitizing its input is therefore costly,
while if we just make identities that have the wrong type - invalid,
it would implicitly force sanitation of identities.
Another argument for doing so, would be that the Validate() is costly
anyway and is rarely performed in the peer's life cycle, therefore -
adding an additional slight overhead to it doesn't affect much the cost
of validating an identity.

The BCCSP-MSP Serialize() method already puts the right PEM type as of
an earlier commit that is targeted for v1.1 and isn't in v1.0.x

This change set makes the MessageCryptoService consider x509 based
identities valid only if they have a PEM type of either CERTIFICATE
or a blank one.

Change-Id: Ic332f592cf2ebfe364f6c99ae762b2789d9c2b51
Signed-off-by: yacovm &lt;yacovm@il.ibm.com&gt;
diff --git a/common/cauthdsl/cauthdsl_test.go b/common/cauthdsl/cauthdsl_test.go
@@ -86,6 +86,10 @@ type mockDeserializer struct {
 	fail error
 }
 
+func (md *mockDeserializer) IsWellFormed(_ *mb.SerializedIdentity) error {
+	return nil
+}
+
 func (md *mockDeserializer) DeserializeIdentity(serializedIdentity []byte) (msp.Identity, error) {
 	if md.fail != nil {
 		return nil, md.fail
diff --git a/common/mocks/msp/noopmsp.go b/common/mocks/msp/noopmsp.go
@@ -86,6 +86,11 @@ func (msp *noopmsp) SatisfiesPrincipal(id m.Identity, principal *msp.MSPPrincipa
 	return nil
 }
 
+// IsWellFormed checks if the given identity can be deserialized into its provider-specific form
+func (msp *noopmsp) IsWellFormed(_ *msp.SerializedIdentity) error {
+	return nil
+}
+
 type noopidentity struct {
 }
 
diff --git a/core/common/privdata/simplecollection_test.go b/core/common/privdata/simplecollection_test.go
@@ -82,6 +82,10 @@ func (md *mockDeserializer) DeserializeIdentity(serializedIdentity []byte) (msp.
 	return &mockIdentity{idBytes: serializedIdentity}, nil
 }
 
+func (md *mockDeserializer) IsWellFormed(_ *mb.SerializedIdentity) error {
+	return nil
+}
+
 func TestSetupBadConfig(t *testing.T) {
 	// set up simple collection with invalid data
 	var sc SimpleCollection
diff --git a/core/policy/mocks/mocks.go b/core/policy/mocks/mocks.go
@@ -78,6 +78,10 @@ func (d *MockIdentityDeserializer) DeserializeIdentity(serializedIdentity []byte
 	return nil, errors.New("Invalid Identity")
 }
 
+func (d *MockIdentityDeserializer) IsWellFormed(_ *mspproto.SerializedIdentity) error {
+	return nil
+}
+
 type MockIdentity struct {
 	identity []byte
 	msg      []byte
diff --git a/msp/idemixmsp.go b/msp/idemixmsp.go
@@ -328,6 +328,18 @@ func (msp *idemixmsp) SatisfiesPrincipal(id Identity, principal *m.MSPPrincipal)
 	}
 }
 
+// IsWellFormed checks if the given identity can be deserialized into its provider-specific .
+// In this MSP implementation, an identity is considered well formed if it contains a
+// marshaled SerializedIdemixIdentity protobuf message.
+func (id *idemixmsp) IsWellFormed(identity *m.SerializedIdentity) error {
+	sId := new(m.SerializedIdemixIdentity)
+	err := proto.Unmarshal(identity.IdBytes, sId)
+	if err != nil {
+		return errors.Wrap(err, "not an idemix identity")
+	}
+	return nil
+}
+
 func (msp *idemixmsp) GetTLSRootCerts() [][]byte {
 	// TODO
 	return nil
diff --git a/msp/idemixmsp_test.go b/msp/idemixmsp_test.go
@@ -285,6 +285,26 @@ func TestPrincipalIdentityBadIdentity(t *testing.T) {
 	}
 }
 
+func TestIdemixIsWellFormed(t *testing.T) {
+	idemixMSP, err := setup("testdata/idemix/MSP1OU1")
+	assert.NoError(t, err)
+
+	id, err := getDefaultSigner(idemixMSP)
+	assert.NoError(t, err)
+	rawId, err := id.Serialize()
+	assert.NoError(t, err)
+	sId := &msp.SerializedIdentity{}
+	err = proto.Unmarshal(rawId, sId)
+	assert.NoError(t, err)
+	err = idemixMSP.IsWellFormed(sId)
+	assert.NoError(t, err)
+	// Corrupt the identity bytes
+	sId.IdBytes = append(sId.IdBytes, 1)
+	err = idemixMSP.IsWellFormed(sId)
+	assert.Error(t, err)
+	assert.Contains(t, err.Error(), "not an idemix identity")
+}
+
 func TestPrincipalOU(t *testing.T) {
 	msp1, err := setup("testdata/idemix/MSP1OU1")
 	assert.NoError(t, err)
diff --git a/msp/mgmt/deserializer.go b/msp/mgmt/deserializer.go
@@ -26,6 +26,9 @@ import (
 // DeserializersManager is a support interface to
 // access the local and channel deserializers
 type DeserializersManager interface {
+
+	// Deserialize receives SerializedIdentity bytes and returns the unmarshaled form
+	// of the SerializedIdentity, or error on failure
 	Deserialize(raw []byte) (*mspproto.SerializedIdentity, error)
 
 	// GetLocalMSPIdentifier returns the local MSP identifier
diff --git a/msp/mocks/mocks.go b/msp/mocks/mocks.go
@@ -18,6 +18,10 @@ type MockMSP struct {
 	mock.Mock
 }
 
+func (m *MockMSP) IsWellFormed(_ *pmsp.SerializedIdentity) error {
+	return nil
+}
+
 func (m *MockMSP) DeserializeIdentity(serializedIdentity []byte) (msp.Identity, error) {
 	args := m.Called(serializedIdentity)
 	return args.Get(0).(msp.Identity), args.Error(1)
diff --git a/msp/msp.go b/msp/msp.go
@@ -19,6 +19,9 @@ type IdentityDeserializer interface {
 	// an msp that is different from this one that is performing
 	// the deserialization.
 	DeserializeIdentity(serializedIdentity []byte) (Identity, error)
+
+	// IsWellFormed checks if the given identity can be deserialized into its provider-specific form
+	IsWellFormed(identity *msp.SerializedIdentity) error
 }
 
 // Membership service provider APIs for Hyperledger Fabric:
diff --git a/msp/msp_test.go b/msp/msp_test.go
@@ -258,6 +258,64 @@ func TestSerializeIdentities(t *testing.T) {
 	}
 }
 
+func TestIsWellFormed(t *testing.T) {
+	mspMgr := NewMSPManager()
+
+	id, err := localMsp.GetDefaultSigningIdentity()
+	if err != nil {
+		t.Fatalf("GetSigningIdentity should have succeeded, got err %s", err)
+		return
+	}
+
+	serializedID, err := id.Serialize()
+	if err != nil {
+		t.Fatalf("Serialize should have succeeded, got err %s", err)
+		return
+	}
+
+	sId := &msp.SerializedIdentity{}
+	err = proto.Unmarshal(serializedID, sId)
+	assert.NoError(t, err)
+
+	// An MSP Manager without any MSPs should not recognize the identity since
+	// not providers are registered
+	err = mspMgr.IsWellFormed(sId)
+	assert.Error(t, err)
+	assert.Equal(t, "no MSP provider recognizes the identity", err.Error())
+
+	// Add the MSP to the MSP Manager
+	mspMgr.Setup([]MSP{localMsp})
+
+	err = localMsp.IsWellFormed(sId)
+	assert.NoError(t, err)
+	err = mspMgr.IsWellFormed(sId)
+	assert.NoError(t, err)
+
+	bl, _ := pem.Decode(sId.IdBytes)
+	assert.Equal(t, "CERTIFICATE", bl.Type)
+
+	// Now, strip off the type from the PEM block. It should still be valid
+	bl.Type = ""
+	sId.IdBytes = pem.EncodeToMemory(bl)
+
+	err = localMsp.IsWellFormed(sId)
+	assert.NoError(t, err)
+
+	// Now, corrupt the type of the PEM block.
+	// make sure it isn't considered well formed by both an MSP and an MSP Manager
+	bl.Type = "foo"
+	sId.IdBytes = pem.EncodeToMemory(bl)
+	err = localMsp.IsWellFormed(sId)
+
+	assert.Error(t, err)
+	assert.Contains(t, err.Error(), "pem type is")
+	assert.Contains(t, err.Error(), "should be 'CERTIFICATE' or missing")
+
+	err = mspMgr.IsWellFormed(sId)
+	assert.Error(t, err)
+	assert.Equal(t, "no MSP provider recognizes the identity", err.Error())
+}
+
 func TestValidateCAIdentity(t *testing.T) {
 	caID := getIdentity(t, cacerts)
 
diff --git a/msp/mspimpl.go b/msp/mspimpl.go
@@ -1112,3 +1112,23 @@ func (msp *bccspmsp) getValidityOptsForCert(cert *x509.Certificate) x509.VerifyO
 
 	return tempOpts
 }
+
+// IsWellFormed checks if the given identity can be deserialized into its provider-specific form.
+// In this MSP implementation, well formed means that the PEM has a Type which is either
+// the string 'CERTIFICATE' or the Type is missing altogether.
+func (msp *bccspmsp) IsWellFormed(identity *m.SerializedIdentity) error {
+	bl, _ := pem.Decode(identity.IdBytes)
+	if bl == nil {
+		return errors.New("PEM decoding resulted in an empty block")
+	}
+	// Important: This method looks very similar to getCertFromPem(idBytes []byte) (*x509.Certificate, error)
+	// But we:
+	// 1) Must ensure PEM block is of type CERTIFICATE or is empty
+	// 2) Must not replace getCertFromPem with this method otherwise we will introduce
+	//    a change in validation logic which will result in a chain fork.
+	if bl.Type != "CERTIFICATE" && bl.Type != "" {
+		return errors.Errorf("pem type is %s, should be 'CERTIFICATE' or missing", bl.Type)
+	}
+	_, err := x509.ParseCertificate(bl.Bytes)
+	return err
+}
diff --git a/msp/mspmgrimpl.go b/msp/mspmgrimpl.go
@@ -29,6 +29,9 @@ type mspManagerImpl struct {
 	// map that contains all MSPs that we have setup or otherwise added
 	mspsMap map[string]MSP
 
+	// map that maps MSPs by their provider types
+	mspsByProviders map[ProviderType][]MSP
+
 	// error that might have occurred at startup
 	up bool
 }
@@ -52,13 +55,18 @@ func (mgr *mspManagerImpl) Setup(msps []MSP) error {
 	// create the map that assigns MSP IDs to their manager instance - once
 	mgr.mspsMap = make(map[string]MSP)
 
+	// create the map that sorts MSPs by their provider types
+	mgr.mspsByProviders = make(map[ProviderType][]MSP)
+
 	for _, msp := range msps {
 		// add the MSP to the map of active MSPs
 		mspID, err := msp.GetIdentifier()
 		if err != nil {
 			return errors.WithMessage(err, "could not extract msp identifier")
 		}
 		mgr.mspsMap[mspID] = msp
+		providerType := msp.GetType()
+		mgr.mspsByProviders[providerType] = append(mgr.mspsByProviders[providerType], msp)
 	}
 
 	mgr.up = true
@@ -97,3 +105,16 @@ func (mgr *mspManagerImpl) DeserializeIdentity(serializedID []byte) (Identity, e
 		return t.DeserializeIdentity(serializedID)
 	}
 }
+
+func (mgr *mspManagerImpl) IsWellFormed(identity *msp.SerializedIdentity) error {
+	// Iterate over all the MSPs by their providers, and find at least 1 MSP that can attest
+	// that this identity is well formed
+	for _, mspList := range mgr.mspsByProviders {
+		// We are guaranteed to have at least 1 MSP in each list from the initialization at Setup()
+		msp := mspList[0]
+		if err := msp.IsWellFormed(identity); err == nil {
+			return nil
+		}
+	}
+	return errors.New("no MSP provider recognizes the identity")
+}
diff --git a/peer/gossip/mcs.go b/peer/gossip/mcs.go
@@ -67,8 +67,8 @@ func NewMCS(channelPolicyManagerGetter policies.ChannelPolicyManagerGetter, loca
 // If the identity is invalid, revoked, expired it returns an error.
 // Else, returns nil
 func (s *mspMessageCryptoService) ValidateIdentity(peerIdentity api.PeerIdentityType) error {
-	// As prescibed by the contract of method,
-	// here we check only that peerIdentity is not
+	// As prescribed by the contract of method,
+	// below we check only that peerIdentity is not
 	// invalid, revoked or expired.
 
 	_, _, err := s.getValidatedIdentity(peerIdentity)
@@ -271,18 +271,27 @@ func (s *mspMessageCryptoService) getValidatedIdentity(peerIdentity api.PeerIden
 		return nil, nil, errors.New("Invalid Peer Identity. It must be different from nil.")
 	}
 
+	sId, err := s.deserializer.Deserialize(peerIdentity)
+	if err != nil {
+		mcsLogger.Error("failed deserializing identity", err)
+		return nil, nil, err
+	}
+
 	// Notice that peerIdentity is assumed to be the serialization of an identity.
 	// So, first step is the identity deserialization and then verify it.
 
 	// First check against the local MSP.
 	// If the peerIdentity is in the same organization of this node then
 	// the local MSP is required to take the final decision on the validity
 	// of the signature.
-	identity, err := s.deserializer.GetLocalDeserializer().DeserializeIdentity([]byte(peerIdentity))
+	lDes := s.deserializer.GetLocalDeserializer()
+	identity, err := lDes.DeserializeIdentity([]byte(peerIdentity))
 	if err == nil {
 		// No error means that the local MSP successfully deserialized the identity.
 		// We now check additional properties.
-
+		if err := lDes.IsWellFormed(sId); err != nil {
+			return nil, nil, errors.Wrap(err, "identity is not well formed")
+		}
 		// TODO: The following check will be replaced by a check on the organizational units
 		// when we allow the gossip network to have organization unit (MSP subdivisions)
 		// scoped messages.
@@ -310,6 +319,11 @@ func (s *mspMessageCryptoService) getValidatedIdentity(peerIdentity api.PeerIden
 			continue
 		}
 
+		// We managed deserializing the identity with this MSP manager. Now we check if it's well formed.
+		if err := mspManager.IsWellFormed(sId); err != nil {
+			return nil, nil, errors.Wrap(err, "identity is not well formed")
+		}
+
 		// Check identity validity
 		// Notice that at this stage we don't have to check the identity
 		// against any channel's policies.
diff --git a/peer/gossip/mcs_test.go b/peer/gossip/mcs_test.go
diff --git a/peer/gossip/mocks/mocks.go b/peer/gossip/mocks/mocks.go
diff --git a/peer/gossip/sa_test.go b/peer/gossip/sa_test.go

Original file line number	Diff line number	Diff line change
`@@ -86,6 +86,11 @@ func (msp noopmsp) SatisfiesPrincipal(id m.Identity, principal msp.MSPPrincipa`
`86`	`86`	`return nil`
`87`	`87`	`}`
`88`	`88`
	`89`	`+// IsWellFormed checks if the given identity can be deserialized into its provider-specific form`
	`90`	`+func (msp noopmsp) IsWellFormed(_ msp.SerializedIdentity) error {`
	`91`	`+ return nil`
	`92`	`+}`
	`93`	`+`
`89`	`94`	`type noopidentity struct {`
`90`	`95`	`}`
`91`	`96`
Original file line number	Diff line number	Diff line change
`@@ -19,6 +19,9 @@ type IdentityDeserializer interface {`
`19`	`19`	`// an msp that is different from this one that is performing`
`20`	`20`	`// the deserialization.`
`21`	`21`	`DeserializeIdentity(serializedIdentity []byte) (Identity, error)`
	`22`	`+`
	`23`	`+ // IsWellFormed checks if the given identity can be deserialized into its provider-specific form`
	`24`	`+ IsWellFormed(identity *msp.SerializedIdentity) error`
`22`	`25`	`}`
`23`	`26`
`24`	`27`	`// Membership service provider APIs for Hyperledger Fabric:`