[FAB-17059] Change collection membership eligibility checks to

only compare mspID instead of Evaluating the certificate against the
access policy

* This handles the issue when ca certs are rolled and access policy
evalutaion for eligible private data was failing due to policy
evaluation checking certs prior to the config update block that added
the new cert

Signed-off-by: Danny Cao <dcao@us.ibm.com>
(cherry picked from commit b52075b)

core/common/privdata/collection.go

@@ -30,7 +30,7 @@ type Collection interface {
 
 	// MemberOrgs returns the collection's members as MSP IDs. This serves as
 	// a human-readable way of quickly identifying who is part of a collection.
-	MemberOrgs() []string
+	MemberOrgs() map[string]struct{}
 }
 
 // CollectionAccessPolicy encapsulates functions for the access policy of a collection
@@ -49,7 +49,7 @@ type CollectionAccessPolicy interface {
 
 	// MemberOrgs returns the collection's members as MSP IDs. This serves as
 	// a human-readable way of quickly identifying who is part of a collection.
-	MemberOrgs() []string
+	MemberOrgs() map[string]struct{}
 
 	// IsMemberOnlyRead returns a true if only collection members can read
 	// the private data

core/common/privdata/membershipinfo.go

@@ -17,19 +17,33 @@ var logger = flogging.MustGetLogger("common.privdata")
 
 // MembershipProvider can be used to check whether a peer is eligible to a collection or not
 type MembershipProvider struct {
+	mspID                       string
 	selfSignedData              protoutil.SignedData
 	IdentityDeserializerFactory func(chainID string) msp.IdentityDeserializer
 }
 
 // NewMembershipInfoProvider returns MembershipProvider
-func NewMembershipInfoProvider(selfSignedData protoutil.SignedData, identityDeserializerFunc func(chainID string) msp.IdentityDeserializer) *MembershipProvider {
+func NewMembershipInfoProvider(mspID string, selfSignedData protoutil.SignedData, identityDeserializerFunc func(chainID string) msp.IdentityDeserializer) *MembershipProvider {
 	return &MembershipProvider{selfSignedData: selfSignedData, IdentityDeserializerFactory: identityDeserializerFunc}
 }
 
 // AmMemberOf checks whether the current peer is a member of the given collection config.
 // If getPolicy returns an error, it will drop the error and return false - same as a RejectAll policy.
+// It is used when a chaincode is upgraded to see if the peer's org has become eligible after	a collection
+// change.
 func (m *MembershipProvider) AmMemberOf(channelName string, collectionPolicyConfig *peer.CollectionPolicyConfig) (bool, error) {
 	deserializer := m.IdentityDeserializerFactory(channelName)
+
+	// Do a simple check to see if the mspid matches any principal identities in the SignaturePolicy - FAB-17059
+	if collectionPolicyConfig.GetSignaturePolicy() != nil {
+		memberOrgs := getMemberOrgs(collectionPolicyConfig.GetSignaturePolicy().GetIdentities(), deserializer)
+
+		if _, ok := memberOrgs[m.mspID]; ok {
+			return true, nil
+		}
+	}
+
+	// Fall back to default access policy evaluation otherwise
 	accessPolicy, err := getPolicy(collectionPolicyConfig, deserializer)
 	if err != nil {
 		// drop the error and return false - same as reject all policy
@@ -39,5 +53,6 @@ func (m *MembershipProvider) AmMemberOf(channelName string, collectionPolicyConf
 	if err := accessPolicy.EvaluateSignedData([]*protoutil.SignedData{&m.selfSignedData}); err != nil {
 		return false, nil
 	}
+
 	return true, nil
 }

core/common/privdata/membershipinfo_test.go

@@ -17,6 +17,7 @@ import (
 )
 
 func TestMembershipInfoProvider(t *testing.T) {
+	mspID := "peer0"
 	peerSelfSignedData := protoutil.SignedData{
 		Identity:  []byte("peer0"),
 		Signature: []byte{1, 2, 3},
@@ -28,7 +29,7 @@ func TestMembershipInfoProvider(t *testing.T) {
 	}
 
 	// verify membership provider returns true
-	membershipProvider := NewMembershipInfoProvider(peerSelfSignedData, identityDeserializer)
+	membershipProvider := NewMembershipInfoProvider(mspID, peerSelfSignedData, identityDeserializer)
 	res, err := membershipProvider.AmMemberOf("test1", getAccessPolicy([]string{"peer0", "peer1"}))
 	assert.True(t, res)
 	assert.Nil(t, err)

core/common/privdata/simplecollection.go

@@ -7,10 +7,6 @@ SPDX-License-Identifier: Apache-2.0
 package privdata
 
 import (
-	"fmt"
-
-	"github.com/golang/protobuf/proto"
-	m "github.com/hyperledger/fabric-protos-go/msp"
 	"github.com/hyperledger/fabric-protos-go/peer"
 	"github.com/hyperledger/fabric/common/policies"
 	"github.com/hyperledger/fabric/msp"
@@ -23,7 +19,7 @@ import (
 type SimpleCollection struct {
 	name         string
 	accessPolicy policies.Policy
-	memberOrgs   []string
+	memberOrgs   map[string]struct{}
 	conf         peer.StaticCollectionConfig
 }
 
@@ -45,7 +41,7 @@ func (sc *SimpleCollection) CollectionID() string {
 }
 
 // MemberOrgs returns the MSP IDs that are part of this collection
-func (sc *SimpleCollection) MemberOrgs() []string {
+func (sc *SimpleCollection) MemberOrgs() map[string]struct{} {
 	return sc.memberOrgs
 }
 
@@ -108,34 +104,8 @@ func (sc *SimpleCollection) Setup(collectionConfig *peer.StaticCollectionConfig,
 		return err
 	}
 
-	// get member org MSP IDs from the envelope
-	for _, principal := range accessPolicyEnvelope.Identities {
-		switch principal.PrincipalClassification {
-		case m.MSPPrincipal_ROLE:
-			// Principal contains the msp role
-			mspRole := &m.MSPRole{}
-			err := proto.Unmarshal(principal.Principal, mspRole)
-			if err != nil {
-				return errors.Wrap(err, "Could not unmarshal MSPRole from principal")
-			}
-			sc.memberOrgs = append(sc.memberOrgs, mspRole.MspIdentifier)
-		case m.MSPPrincipal_IDENTITY:
-			principalId, err := deserializer.DeserializeIdentity(principal.Principal)
-			if err != nil {
-				return errors.Wrap(err, "Invalid identity principal, not a certificate")
-			}
-			sc.memberOrgs = append(sc.memberOrgs, principalId.GetMSPIdentifier())
-		case m.MSPPrincipal_ORGANIZATION_UNIT:
-			OU := &m.OrganizationUnit{}
-			err := proto.Unmarshal(principal.Principal, OU)
-			if err != nil {
-				return errors.Wrap(err, "Could not unmarshal OrganizationUnit from principal")
-			}
-			sc.memberOrgs = append(sc.memberOrgs, OU.MspIdentifier)
-		default:
-			return errors.New(fmt.Sprintf("Invalid principal type %d", int32(principal.PrincipalClassification)))
-		}
-	}
+	// get member org MSP IDs from the envelope, identities that fail to deserialize will not be returned
+	sc.memberOrgs = getMemberOrgs(accessPolicyEnvelope.Identities, deserializer)
 
 	return nil
 }

core/common/privdata/simplecollection_test.go

@@ -130,8 +130,8 @@ func TestNewSimpleCollectionWithGoodConfig(t *testing.T) {
 
 	// check members
 	members := sc.MemberOrgs()
-	assert.True(t, members[0] == "signer0")
-	assert.True(t, members[1] == "signer1")
+	assert.Contains(t, members, "signer0")
+	assert.Contains(t, members, "signer1")
 
 	// check required peer count
 	assert.True(t, sc.RequiredPeerCount() == 1)
@@ -177,8 +177,8 @@ func TestSetupGoodConfigCollection(t *testing.T) {
 
 	// check members
 	members := sc.MemberOrgs()
-	assert.True(t, members[0] == "signer0")
-	assert.True(t, members[1] == "signer1")
+	assert.Contains(t, members, "signer0")
+	assert.Contains(t, members, "signer1")
 
 	// check required peer count
 	assert.True(t, sc.RequiredPeerCount() == 1)

core/common/privdata/util.go

@@ -7,6 +7,8 @@ SPDX-License-Identifier: Apache-2.0
 package privdata
 
 import (
+	"github.com/gogo/protobuf/proto"
+	mspp "github.com/hyperledger/fabric-protos-go/msp"
 	"github.com/hyperledger/fabric-protos-go/peer"
 	"github.com/hyperledger/fabric/common/cauthdsl"
 	"github.com/hyperledger/fabric/common/policies"
@@ -35,3 +37,34 @@ func getPolicy(collectionPolicyConfig *peer.CollectionPolicyConfig, deserializer
 
 	return accessPolicy, nil
 }
+
+// getMemberOrgs returns a map containing member orgs from a list of MSPPrincipals,
+// it will skip identities it fails to process
+func getMemberOrgs(identities []*mspp.MSPPrincipal, deserializer msp.IdentityDeserializer) map[string]struct{} {
+	memberOrgs := map[string]struct{}{}
+
+	// get member org MSP IDs from the envelope
+	for _, principal := range identities {
+		switch principal.PrincipalClassification {
+		case mspp.MSPPrincipal_ROLE:
+			// Principal contains the msp role
+			mspRole := &mspp.MSPRole{}
+			err := proto.Unmarshal(principal.Principal, mspRole)
+			if err == nil {
+				memberOrgs[mspRole.MspIdentifier] = struct{}{}
+			}
+		case mspp.MSPPrincipal_IDENTITY:
+			principalId, err := deserializer.DeserializeIdentity(principal.Principal)
+			if err == nil {
+				memberOrgs[principalId.GetMSPIdentifier()] = struct{}{}
+			}
+		case mspp.MSPPrincipal_ORGANIZATION_UNIT:
+			OU := &mspp.OrganizationUnit{}
+			err := proto.Unmarshal(principal.Principal, OU)
+			if err == nil {
+				memberOrgs[OU.MspIdentifier] = struct{}{}
+			}
+		}
+	}
+	return memberOrgs
+}

core/ledger/kvledger/tests/env.go

@@ -217,7 +217,8 @@ func populateMissingsWithTestDefaults(t *testing.T, initializer *ledgermgmt.Init
 		}
 		cryptoProvider, err := sw.NewDefaultSecurityLevelWithKeystore(sw.NewDummyKeyStore())
 		assert.NoError(t, err)
-		membershipInfoProvider := privdata.NewMembershipInfoProvider(createSelfSignedData(cryptoProvider), identityDeserializerFactory)
+		mspID := "test-mspid"
+		membershipInfoProvider := privdata.NewMembershipInfoProvider(mspID, createSelfSignedData(cryptoProvider), identityDeserializerFactory)
 		initializer.MembershipInfoProvider = membershipInfoProvider
 	}
 

gossip/privdata/coordinator.go

@@ -119,6 +119,7 @@ type CoordinatorConfig struct {
 }
 
 type coordinator struct {
+	mspID          string
 	selfSignedData protoutil.SignedData
 	Support
 	store                          *transientstore.Store
@@ -130,9 +131,10 @@ type coordinator struct {
 }
 
 // NewCoordinator creates a new instance of coordinator
-func NewCoordinator(support Support, store *transientstore.Store, selfSignedData protoutil.SignedData, metrics *metrics.PrivdataMetrics,
+func NewCoordinator(mspID string, support Support, store *transientstore.Store, selfSignedData protoutil.SignedData, metrics *metrics.PrivdataMetrics,
 	config CoordinatorConfig, idDeserializerFactory IdentityDeserializerFactory) Coordinator {
 	return &coordinator{Support: support,
+		mspID:                          mspID,
 		store:                          store,
 		selfSignedData:                 selfSignedData,
 		transientBlockRetention:        config.TransientBlockRetention,
@@ -183,6 +185,7 @@ func (c *coordinator) StoreBlock(block *common.Block, privateDataSets util.PvtDa
 	fetchDurationHistogram := c.metrics.FetchDuration.With("channel", c.ChainID)
 	purgeDurationHistogram := c.metrics.PurgeDuration.With("channel", c.ChainID)
 	pdp := &PvtdataProvider{
+		mspID:                                   c.mspID,
 		selfSignedData:                          c.selfSignedData,
 		logger:                                  logger.With("channel", c.ChainID),
 		listMissingPrivateDataDurationHistogram: listMissingPrivateDataDurationHistogram,