Refactor identity code and check for MSP ID

This change set refactors the code inside of the MSP that is called whenever
a serialized identity has to be turned into an identity instance. The code
was previously repeating the deserialization into a SerializedIdentity struct
twice - once by the MSPManager and once by the MSP. Now the MSP
implementation exposes a function that turns SerializedIdentity.IdBytes
into an identity. This method is called by the public method that takes the
full serialized identity, deserializes it into a SerializedIdentity struct
and calls the private method passing in SerializedIdentity.IdBytes.

Also, the code now checks that SerializedIdentity.Mspid mathces the ID of
the MSP that performs the deserialization.

Change-Id: Ie76c500e93b9acf1ea9c69ccc8db46f1e830d204
Signed-off-by: Alessandro Sorniotti <[email protected]>

common/cauthdsl/cauthdsl.go

@@ -30,7 +30,7 @@ import (
 var cauthdslLogger = logging.MustGetLogger("cauthdsl")
 
 // compile recursively builds a go evaluatable function corresponding to the policy specified
-func compile(policy *cb.SignaturePolicy, identities []*cb.MSPPrincipal, deserializer msp.Common) (func([]*cb.SignedData, []bool) bool, error) {
+func compile(policy *cb.SignaturePolicy, identities []*cb.MSPPrincipal, deserializer msp.IdentityDeserializer) (func([]*cb.SignedData, []bool) bool, error) {
 	switch t := policy.Type.(type) {
 	case *cb.SignaturePolicy_From:
 		policies := make([]func([]*cb.SignedData, []bool) bool, len(t.From.Policies))
@@ -159,7 +159,7 @@ func toSignedData(data [][]byte, identities [][]byte, signatures [][]byte) ([]*c
 type mockDeserializer struct {
 }
 
-func NewMockDeserializer() msp.Common {
+func NewMockDeserializer() msp.IdentityDeserializer {
 	return &mockDeserializer{}
 }
 

common/cauthdsl/policy.go

@@ -28,11 +28,11 @@ import (
 )
 
 type provider struct {
-	deserializer msp.Common
+	deserializer msp.IdentityDeserializer
 }
 
 // NewProviderImpl provides a policy generator for cauthdsl type policies
-func NewPolicyProvider(deserializer msp.Common) policies.Provider {
+func NewPolicyProvider(deserializer msp.IdentityDeserializer) policies.Provider {
 	return &provider{
 		deserializer: deserializer,
 	}

core/common/validation/msgvalidation.go

@@ -120,7 +120,7 @@ func checkSignatureFromCreator(creatorBytes []byte, sig []byte, msg []byte, Chai
 		return fmt.Errorf("Nil arguments")
 	}
 
-	mspObj := mspmgmt.GetMSPCommon(ChainID)
+	mspObj := mspmgmt.GetIdentityDeserializer(ChainID)
 	if mspObj == nil {
 		return fmt.Errorf("could not get msp for chain [%s]", ChainID)
 	}

msp/mgmt/mgmt.go

@@ -111,8 +111,8 @@ func GetLocalMSP() msp.MSP {
 	return lclMsp
 }
 
-//GetMSPCommon returns the common interface
-func GetMSPCommon(chainID string) msp.Common {
+// GetIdentityDeserializer returns the IdentityDeserializer for the given chain
+func GetIdentityDeserializer(chainID string) msp.IdentityDeserializer {
 	if chainID == "" {
 		return GetLocalMSP()
 	}

msp/msp.go

@@ -25,8 +25,8 @@ import (
 // FIXME: we need better comments on the interfaces!!
 // FIXME: we need better comments on the interfaces!!
 
-//Common is implemented by both MSPManger and MSP
-type Common interface {
+// IdentityDeserializer is implemented by both MSPManger and MSP
+type IdentityDeserializer interface {
 	// DeserializeIdentity deserializes an identity.
 	// Deserialization will fail if the identity is associated to
 	// an msp that is different from this one that is performing
@@ -55,8 +55,8 @@ type Common interface {
 // This object is immutable, it is initialized once and never changed.
 type MSPManager interface {
 
-	// Common interface needs to be implemented by MSPManager
-	Common
+	// IdentityDeserializer interface needs to be implemented by MSPManager
+	IdentityDeserializer
 
 	// Setup the MSP manager instance according to configuration information
 	Setup(msps []*msp.MSPConfig) error
@@ -69,8 +69,8 @@ type MSPManager interface {
 // to accommodate peer functionality
 type MSP interface {
 
-	// Common interface needs to be implemented by MSP
-	Common
+	// IdentityDeserializer interface needs to be implemented by MSP
+	IdentityDeserializer
 
 	// Setup the MSP instance according to configuration information
 	Setup(config *msp.MSPConfig) error

msp/msp_test.go

@@ -4,9 +4,13 @@ import (
 	"os"
 	"reflect"
 	"testing"
-)
 
-var localMsp MSP
+	"fmt"
+
+	"github.com/golang/protobuf/proto"
+	"github.com/hyperledger/fabric/protos/msp"
+	"github.com/stretchr/testify/assert"
+)
 
 func TestNoopMSP(t *testing.T) {
 	noopmsp := NewNoopMsp()
@@ -57,26 +61,6 @@ func TestMSPSetupBad(t *testing.T) {
 	}
 }
 
-func TestMSPSetupGood(t *testing.T) {
-	conf, err := GetLocalMspConfig("./sampleconfig/")
-	if err != nil {
-		t.Fatalf("Setup should have succeeded, got err %s instead", err)
-		return
-	}
-
-	localMsp, err = NewBccspMsp()
-	if err != nil {
-		t.Fatalf("Constructor for msp should have succeeded, got err %s instead", err)
-		return
-	}
-
-	err = localMsp.Setup(conf)
-	if err != nil {
-		t.Fatalf("Setup for msp should have succeeded, got err %s instead", err)
-		return
-	}
-}
-
 func TestGetIdentities(t *testing.T) {
 	_, err := localMsp.GetDefaultSigningIdentity()
 	if err != nil {
@@ -116,6 +100,61 @@ func TestSerializeIdentities(t *testing.T) {
 	}
 }
 
+func TestSerializeIdentitiesWithWrongMSP(t *testing.T) {
+	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 := &SerializedIdentity{}
+	err = proto.Unmarshal(serializedID, sid)
+	assert.NoError(t, err)
+
+	sid.Mspid += "BARF"
+
+	serializedID, err = proto.Marshal(sid)
+	assert.NoError(t, err)
+
+	_, err = localMsp.DeserializeIdentity(serializedID)
+	assert.Error(t, err)
+}
+
+func TestSerializeIdentitiesWithMSPManager(t *testing.T) {
+	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
+	}
+
+	_, err = mspMgr.DeserializeIdentity(serializedID)
+	assert.NoError(t, err)
+
+	sid := &SerializedIdentity{}
+	err = proto.Unmarshal(serializedID, sid)
+	assert.NoError(t, err)
+
+	sid.Mspid += "BARF"
+
+	serializedID, err = proto.Marshal(sid)
+	assert.NoError(t, err)
+
+	_, err = mspMgr.DeserializeIdentity(serializedID)
+	assert.Error(t, err)
+}
+
 func TestSignAndVerify(t *testing.T) {
 	id, err := localMsp.GetDefaultSigningIdentity()
 	if err != nil {
@@ -194,7 +233,37 @@ func TestSignAndVerify_longMessage(t *testing.T) {
 	}
 }
 
+var conf *msp.MSPConfig
+var localMsp MSP
+var mspMgr MSPManager
+
 func TestMain(m *testing.M) {
+	var err error
+	conf, err = GetLocalMspConfig("./sampleconfig/")
+	if err != nil {
+		fmt.Printf("Setup should have succeeded, got err %s instead", err)
+		os.Exit(-1)
+	}
+
+	localMsp, err = NewBccspMsp()
+	if err != nil {
+		fmt.Printf("Constructor for msp should have succeeded, got err %s instead", err)
+		os.Exit(-1)
+	}
+
+	err = localMsp.Setup(conf)
+	if err != nil {
+		fmt.Printf("Setup for msp should have succeeded, got err %s instead", err)
+		os.Exit(-1)
+	}
+
+	mspMgr = NewMSPManager()
+	err = mspMgr.Setup([]*msp.MSPConfig{conf})
+	if err != nil {
+		fmt.Printf("Setup for msp manager should have succeeded, got err %s instead", err)
+		os.Exit(-1)
+	}
+
 	retVal := m.Run()
 	os.Exit(retVal)
 }

msp/mspimpl.go

@@ -250,29 +250,29 @@ func (msp *bccspmsp) Validate(id Identity) error {
 	}
 }
 
-// DeserializeIdentity returns an Identity
-// instance that was marshalled to the supplied byte array
+// DeserializeIdentity returns an Identity given the byte-level
+// representation of a SerializedIdentity struct
 func (msp *bccspmsp) DeserializeIdentity(serializedID []byte) (Identity, error) {
 	mspLogger.Infof("Obtaining identity")
 
-	// FIXME: this is not ideal, because the manager already does this
-	// unmarshalling if we go through it; however the local MSP does
-	// not have a manager and in case it has to deserialize an identity,
-	// it will have to do the whole thing by itself; for now I've left
-	// it this way but we can introduce a local MSP manager and fix it
-	// more nicely
-
 	// We first deserialize to a SerializedIdentity to get the MSP ID
 	sId := &SerializedIdentity{}
 	err := proto.Unmarshal(serializedID, sId)
 	if err != nil {
 		return nil, fmt.Errorf("Could not deserialize a SerializedIdentity, err %s", err)
 	}
 
-	// TODO: check that sId.Mspid is equal to this msp'id as per contract of the interface.
+	if sId.Mspid != msp.name {
+		return nil, fmt.Errorf("Expected MSP ID %s, received %s", msp.name, sId.Mspid)
+	}
+
+	return msp.deserializeIdentityInternal(sId.IdBytes)
+}
 
+// deserializeIdentityInternal returns an identity given its byte-level representation
+func (msp *bccspmsp) deserializeIdentityInternal(serializedIdentity []byte) (Identity, error) {
 	// This MSP will always deserialize certs this way
-	bl, _ := pem.Decode(sId.IdBytes)
+	bl, _ := pem.Decode(serializedIdentity)
 	if bl == nil {
 		return nil, fmt.Errorf("Could not decode the PEM structure")
 	}
@@ -285,9 +285,6 @@ func (msp *bccspmsp) DeserializeIdentity(serializedID []byte) (Identity, error)
 	// (e.g. the Issuer.OU or the Subject.OU) match with the
 	// MSP id that this MSP has; otherwise it might be an attack
 	// TODO!
-	// TODO!
-	// TODO!
-	// TODO!
 	// We can't do it yet because there is no standardized way
 	// (yet) to encode the MSP ID into the x.509 body of a cert
 

msp/mspmgrimpl.go

@@ -116,6 +116,10 @@ func (mgr *mspManagerImpl) DeserializeIdentity(serializedID []byte) (Identity, e
 		return nil, fmt.Errorf("MSP %s is unknown", sId.Mspid)
 	}
 
-	// if we have this MSP, we ask it to deserialize
-	return msp.DeserializeIdentity(serializedID)
+	switch t := msp.(type) {
+	case *bccspmsp:
+		return t.deserializeIdentityInternal(sId.IdBytes)
+	default:
+		return t.DeserializeIdentity(serializedID)
+	}
 }