[msp] Fix cached nil identities (#78)

#### Type of change

- Bug fix

#### Description

The commit fixes a nil pointer dereference panic that occurred when
deserializing an identity using a CertificateId.

  Root cause: When DeserializeIdentity was called with an
  Identity_CertificateId, it called GetKnownDeserializedIdentity()
  to look up the identity from an internal map. If the identity wasn't
  found, the method returned nil. This nil identity then reached
  the cache layer (cachedMSP.DeserializeIdentity), which attempted
  id.(msp.Identity) on the nil interface value, causing a panic.

#### Related issues

 - resolves #77

Signed-off-by: Senthilnathan <cendhu@gmail.com>

Author: cendhu

msp/cache/cache.go

@@ -74,14 +74,19 @@ func (c *cachedMSP) DeserializeIdentity(identity *msppb.Identity) (msp.Identity,
 	}
 
 	id, err := c.MSP.DeserializeIdentity(identity)
-	if err == nil {
-		c.deserializeIdentityCache.add(identity.String(), id)
-		return &cachedIdentity{
-			cache:    c,
-			Identity: id.(msp.Identity),
-		}, nil
+	if err != nil {
+		return nil, err
+	}
+
+	c.deserializeIdentityCache.add(identity.String(), id)
+	mspIdentity, ok := id.(msp.Identity)
+	if !ok {
+		return nil, errors.New("internal error: DeserializeIdentity returned unexpected type")
 	}
-	return nil, err
+	return &cachedIdentity{
+		cache:    c,
+		Identity: mspIdentity,
+	}, nil
 }
 
 func (c *cachedMSP) Setup(config *pmsp.MSPConfig) error {

msp/msp_test.go

@@ -236,6 +236,35 @@ func TestDeserializeIdentityFails(t *testing.T) {
 	require.Error(t, err)
 }
 
+func TestDeserializeIdentityWithUnknownCertificateId(t *testing.T) {
+	t.Parallel()
+
+	// Deserializing with a CertificateId that is not in the known identities
+	// map should return an error instead of a nil identity.
+	tests := []struct {
+		name        string
+		deserialize func(*msppb.Identity) (Identity, error)
+	}{
+		{
+			name:        "via MSP",
+			deserialize: localMsp.DeserializeIdentity,
+		},
+		{
+			name:        "via MSP manager",
+			deserialize: mspMgr.DeserializeIdentity,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			id, err := tt.deserialize(msppb.NewIdentityWithIDOfCert("SampleOrg", "nonexistent-cert-id"))
+			require.Nil(t, id)
+			require.EqualError(t, err, "identity is unknown")
+		})
+	}
+}
+
 func TestGetSigningIdentityFromVerifyingMSP(t *testing.T) {
 	cryptoProvider, err := sw.NewDefaultSecurityLevelWithKeystore(sw.NewDummyKeyStore())
 	require.NoError(t, err)

msp/mspimpl.go

@@ -406,8 +406,12 @@ func (msp *bccspmsp) DeserializeIdentity(identity *msppb.Identity) (Identity, er
 	case *msppb.Identity_Certificate:
 		return msp.deserializeIdentityInternal(identity.GetCertificate())
 	case *msppb.Identity_CertificateId:
-		return msp.GetKnownDeserializedIdentity(
-			IdentityIdentifier{Mspid: identity.MspId, Id: identity.GetCertificateId()}), nil
+		id := msp.GetKnownDeserializedIdentity(
+			IdentityIdentifier{Mspid: identity.MspId, Id: identity.GetCertificateId()})
+		if id == nil {
+			return nil, errors.New("identity is unknown")
+		}
+		return id, nil
 	default:
 		return nil, errors.New("unknown creator type in the identity")
 	}

msp/mspmgrimpl.go

@@ -89,8 +89,12 @@ func (mgr *mspManagerImpl) DeserializeIdentity(sID *msppb.Identity) (Identity, e
 		case *msppb.Identity_Certificate:
 			return t.deserializeIdentityInternal(sID.GetCertificate())
 		case *msppb.Identity_CertificateId:
-			return msp.GetKnownDeserializedIdentity(
-				IdentityIdentifier{Mspid: sID.MspId, Id: sID.GetCertificateId()}), nil
+			id := msp.GetKnownDeserializedIdentity(
+				IdentityIdentifier{Mspid: sID.MspId, Id: sID.GetCertificateId()})
+			if id == nil {
+				return nil, errors.New("identity is unknown")
+			}
+			return id, nil
 		default:
 			return nil, errors.New("unknown creator type")
 		}