[FAB-10238] Filter collections on peers, not principals

The collection support in the discovery service supports collections by
filtering out principal sets that aren't authorized by the principals
derived from the policy in the collection config.

This is problematic in cases when the collection config is defined
with principals which have no obvious IsA relation with the
endorsement policy:
Consider a case where the endorsement policy is defined with principals
of Org1.peer or and org2.peer, and the collection is
defined with OUs of Org.

Any principal based policy can't accept nor reject such a scenario,
since a peer role is based on an OU that is hidden from the principal
itself.

This change set moves the filtering to the peer identities instead of
filtering the principal sets of the combinations of the endorsement
policy.

It also raises the code coverage from 97% to 99%

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

Author: yacovm

common/policies/inquire/compare.go

@@ -181,18 +181,3 @@ func (cps ComparablePrincipalSet) Clone() ComparablePrincipalSet {
 	}
 	return res
 }
-
-// IsCoveredBy returns whether the ComparablePrincipalSet is covered by the given ComparablePrincipalSet.
-// A ComparablePrincipalSet X is covered by another ComparablePrincipalSet Y if:
-// for each ComparablePrincipal x in X there is a ComparablePrincipal y in Y,
-// such that x.IsA(y) is true.
-// This method is useful for determining if the all elements of a ComparablePrincipalSet
-// are accepted by another ComparablePrincipalSet.
-func (cps ComparablePrincipalSet) IsCoveredBy(set ComparablePrincipalSet) bool {
-	for _, cp := range cps {
-		if !cp.IsFound(set...) {
-			return false
-		}
-	}
-	return true
-}

common/policies/inquire/compare_test.go

@@ -174,32 +174,6 @@ func TestNewComparablePrincipalSet(t *testing.T) {
 	})
 }
 
-func TestIsCoveredBy(t *testing.T) {
-	t.Run("Covered", func(t *testing.T) {
-		ou1 := NewComparablePrincipal(ou("Org1MSP"))
-		peer1 := NewComparablePrincipal(peer("Org1MSP"))
-		member1 := NewComparablePrincipal(member("Org1MSP"))
-		member2 := NewComparablePrincipal(member("Org2MSP"))
-
-		covered := ComparablePrincipalSet([]*ComparablePrincipal{ou1, peer1})
-		cover := ComparablePrincipalSet([]*ComparablePrincipal{member1, member2})
-
-		assert.True(t, covered.IsCoveredBy(cover))
-		assert.False(t, cover.IsCoveredBy(covered))
-	})
-
-	t.Run("Not Covered", func(t *testing.T) {
-		peer1 := NewComparablePrincipal(peer("Org1MSP"))
-		peer2 := NewComparablePrincipal(peer("Org1MSP"))
-		member1 := NewComparablePrincipal(member("Org1MSP"))
-
-		covered := ComparablePrincipalSet([]*ComparablePrincipal{peer1, peer2})
-		cover := ComparablePrincipalSet([]*ComparablePrincipal{member1})
-
-		assert.False(t, cover.IsCoveredBy(covered))
-	})
-}
-
 func member(orgName string) *msp.MSPPrincipal {
 	return &msp.MSPPrincipal{
 		PrincipalClassification: msp.MSPPrincipal_ROLE,

discovery/endorsement/collection.go

@@ -8,42 +8,27 @@ package endorsement
 
 import (
 	"github.com/hyperledger/fabric/common/policies"
-	"github.com/hyperledger/fabric/common/policies/inquire"
 	"github.com/hyperledger/fabric/core/common/privdata"
+	"github.com/hyperledger/fabric/gossip/api"
+	. "github.com/hyperledger/fabric/protos/discovery"
 	"github.com/pkg/errors"
 )
 
-type filterPrincipalSets func(collectionName string, principalSets policies.PrincipalSets) (policies.PrincipalSets, error)
-
-func (f filterPrincipalSets) forCollections(ccName string, collections ...string) filterFunc {
-	return func(principalSets policies.PrincipalSets) (policies.PrincipalSets, error) {
-		var err error
-		for _, col := range collections {
-			principalSets, err = f(col, principalSets)
-			if err != nil {
-				logger.Warningf("Failed filtering collection for chaincode %s, collection %s: %v", ccName, col, err)
-				return nil, err
-			}
-		}
-		return principalSets, nil
-	}
-}
-
-func newCollectionFilter(configBytes []byte) (filterPrincipalSets, error) {
-	mapFilter := make(principalSetsByCollectionName)
+func principalsFromCollectionConfig(configBytes []byte) (principalSetsByCollectionName, error) {
+	principalSetsByCollections := make(principalSetsByCollectionName)
 	if len(configBytes) == 0 {
-		return mapFilter.filter, nil
+		return principalSetsByCollections, nil
 	}
 	ccp, err := privdata.ParseCollectionConfig(configBytes)
 	if err != nil {
 		return nil, errors.Wrapf(err, "invalid collection bytes")
 	}
-	for _, cfg := range ccp.Config {
-		staticCol := cfg.GetStaticCollectionConfig()
+	for _, colConfig := range ccp.Config {
+		staticCol := colConfig.GetStaticCollectionConfig()
 		if staticCol == nil {
 			// Right now we only support static collections, so if we got something else
 			// we should refuse to process further
-			return nil, errors.Errorf("expected a static collection but got %v instead", cfg)
+			return nil, errors.Errorf("expected a static collection but got %v instead", colConfig)
 		}
 		if staticCol.MemberOrgsPolicy == nil {
 			return nil, errors.Errorf("MemberOrgsPolicy of %s is nil", staticCol.Name)
@@ -57,31 +42,56 @@ func newCollectionFilter(configBytes []byte) (filterPrincipalSets, error) {
 		for _, principal := range pol.Identities {
 			principals = append(principals, principal)
 		}
-		principalSet := inquire.NewComparablePrincipalSet(principals)
-		if principalSet == nil {
-			return nil, errors.Errorf("failed constructing principal set for %s: principals given are %v", staticCol.Name, pol.Identities)
-		}
-		mapFilter[staticCol.Name] = principalSet
+		principalSetsByCollections[staticCol.Name] = principals
 	}
-	return mapFilter.filter, nil
+	return principalSetsByCollections, nil
 }
 
-type principalSetsByCollectionName map[string]inquire.ComparablePrincipalSet
+type principalSetsByCollectionName map[string]policies.PrincipalSet
 
-func (psbc principalSetsByCollectionName) filter(collectionName string, principalSets policies.PrincipalSets) (policies.PrincipalSets, error) {
-	collectionPrincipals, exists := psbc[collectionName]
-	if !exists {
-		return nil, errors.Errorf("collection %s wasn't found in configuration", collectionName)
+// toIdentityFilter converts this principalSetsByCollectionName mapping to a filter
+// which accepts or rejects identities of peers.
+func (psbc principalSetsByCollectionName) toIdentityFilter(channel string, evaluator principalEvaluator, cc *ChaincodeCall) (identityFilter, error) {
+	var principalSets policies.PrincipalSets
+	for _, col := range cc.CollectionNames {
+		// Each collection we're interested in should exist in the principalSetsByCollectionName mapping.
+		// Otherwise, we have no way of computing a filter because we can't locate the principals the peer identities
+		// need to satisfy.
+		principalSet, exists := psbc[col]
+		if !exists {
+			return nil, errors.Errorf("collection %s doesn't exist in collection config for chaincode %s", col, cc.Name)
+		}
+		principalSets = append(principalSets, principalSet)
 	}
-	var res policies.PrincipalSets
-	for _, ps := range principalSets {
-		comparablePS := inquire.NewComparablePrincipalSet(ps)
-		if comparablePS == nil {
-			return nil, errors.Errorf("principal set %v is invalid", ps)
+	return filterForPrincipalSets(channel, evaluator, principalSets), nil
+}
+
+// filterForPrincipalSets creates a filter of peer identities out of the given PrincipalSets
+func filterForPrincipalSets(channel string, evaluator principalEvaluator, sets policies.PrincipalSets) identityFilter {
+	return func(identity api.PeerIdentityType) bool {
+		// Iterate over all principal sets and ensure each principal set
+		// authorizes the identity.
+		for _, principalSet := range sets {
+			if !isIdentityAuthorizedByPrincipalSet(channel, evaluator, principalSet, identity) {
+				return false
+			}
 		}
-		if comparablePS.IsCoveredBy(collectionPrincipals) {
-			res = append(res, ps)
+		return true
+	}
+}
+
+// isIdentityAuthorizedByPrincipalSet returns whether the given identity satisfies some principal out of the given PrincipalSet
+func isIdentityAuthorizedByPrincipalSet(channel string, evaluator principalEvaluator, principalSet policies.PrincipalSet, identity api.PeerIdentityType) bool {
+	// We look for a principal which authorizes the identity
+	// among all principals in the principalSet
+	for _, principal := range principalSet {
+		err := evaluator.SatisfiesPrincipal(channel, identity, principal)
+		if err != nil {
+			continue
 		}
+		// Else, err is nil, so we found a principal which authorized
+		// the given identity.
+		return true
 	}
-	return res, nil
+	return false
 }