[FAB-6526] Collection membership policy checks

Perform validation of collection configuration supplied at deploy/upgrade time
to ensure that the member signature policy is meaningful (only consists
of ORs).

Also perform input sanitization at LSCC deploy/upgrade time to check
that the member signature policy orgs are part of the channel.

Change-Id: I71f82cf7757c9f360a789832effa68d0df59166a
Signed-off-by: Matthias Neugschwandtner <eug@zurich.ibm.com>

Author: Matthias Neugschwandtner

core/handlers/validation/builtin/validation_logic.go

@@ -174,34 +174,59 @@ func validateNewCollectionConfigs(newCollectionConfigs []*common.CollectionConfi
 
 		newCollection := newCollectionConfig.GetStaticCollectionConfig()
 		if newCollection == nil {
-			return policyErr(errors.New("unknown collection configuration type"))
+			return errors.New("unknown collection configuration type")
 		}
 
 		// Ensure that there are no duplicate collection names
 		collectionName := newCollection.GetName()
 
 		if err := validateCollectionName(collectionName); err != nil {
-			return policyErr(err)
+			return err
 		}
 
 		if _, ok := newCollectionsMap[collectionName]; !ok {
 			newCollectionsMap[collectionName] = true
 		} else {
-			return policyErr(fmt.Errorf("collection-name: %s -- found duplicate collection configuration", collectionName))
+			return fmt.Errorf("collection-name: %s -- found duplicate collection configuration", collectionName)
 		}
 
 		// Validate gossip related parameters present in the collection config
 		maximumPeerCount := newCollection.GetMaximumPeerCount()
 		requiredPeerCount := newCollection.GetRequiredPeerCount()
 		if maximumPeerCount < requiredPeerCount {
-			return policyErr(fmt.Errorf("collection-name: %s -- maximum peer count (%d) cannot be greater than the required peer count (%d)",
-				collectionName, maximumPeerCount, requiredPeerCount))
+			return fmt.Errorf("collection-name: %s -- maximum peer count (%d) cannot be greater than the required peer count (%d)",
+				collectionName, maximumPeerCount, requiredPeerCount)
 
 		}
 		if requiredPeerCount < 0 {
-			return policyErr(fmt.Errorf("collection-name: %s -- requiredPeerCount (%d) cannot be lesser than zero (%d)",
-				collectionName, maximumPeerCount, requiredPeerCount))
+			return fmt.Errorf("collection-name: %s -- requiredPeerCount (%d) cannot be lesser than zero (%d)",
+				collectionName, maximumPeerCount, requiredPeerCount)
+
+		}
+
+		// make sure that the signature policy is meaningful (only consists of ORs)
+		err := validateSpOrConcat(newCollection.MemberOrgsPolicy.GetSignaturePolicy().Rule)
+		if err != nil {
+			return errors.WithMessage(err, fmt.Sprintf("collection-name: %s -- error in member org policy", collectionName))
+		}
+	}
+	return nil
+}
 
+// validateSpOrConcat checks if the supplied signature policy is just an OR-concatenation of identities
+func validateSpOrConcat(sp *common.SignaturePolicy) error {
+	if sp.GetNOutOf() == nil {
+		return nil
+	}
+	// check if N == 1 (OR concatenation)
+	if sp.GetNOutOf().N != 1 {
+		return errors.New(fmt.Sprintf("signature policy is not an OR concatenation, NOutOf %d", sp.GetNOutOf().N))
+	}
+	// recurse into all sub-rules
+	for _, rule := range sp.GetNOutOf().Rules {
+		err := validateSpOrConcat(rule)
+		if err != nil {
+			return err
 		}
 	}
 	return nil
@@ -382,8 +407,6 @@ func (vscc *ValidatorOneValidSignature) validateRWSetAndCollection(
 		}
 	}
 
-	// TODO: FAB-6526 - to add validation of the collections object
-
 	return nil
 }
 

core/handlers/validation/builtin/validation_logic_test.go

@@ -1830,6 +1830,14 @@ func TestValidateRWSetAndCollectionForDeploy(t *testing.T) {
 	err = testValidateCollection(t, v, []*common.CollectionConfig{coll1, coll2, coll3}, cdRWSet, lsccFunc, ac, chid)
 	assert.Errorf(t, err, "collection-name: %s -- maximum peer count (%d) cannot be greater than the required peer count (%d)",
 		collName3, maximumPeerCount, requiredPeerCount)
+
+	// Test 12: AND concatenation of orgs in access policy -> error
+	requiredPeerCount = 1
+	maximumPeerCount = 2
+	policyEnvelope = cauthdsl.Envelope(cauthdsl.And(cauthdsl.SignedBy(0), cauthdsl.SignedBy(1)), signers)
+	coll3 = createCollectionConfig(collName3, policyEnvelope, requiredPeerCount, maximumPeerCount, blockToLive)
+	err = testValidateCollection(t, v, []*common.CollectionConfig{coll3}, cdRWSet, lsccFunc, ac, chid)
+	assert.EqualError(t, err, "collection-name: mycollection3 -- error in member org policy: signature policy is not an OR concatenation, NOutOf 2")
 }
 
 func TestValidateRWSetAndCollectionForUpgrade(t *testing.T) {

core/mocks/scc/lscc/support.go

@@ -8,6 +8,7 @@ package lscc
 
 import (
 	"github.com/hyperledger/fabric/core/common/ccprovider"
+	"github.com/hyperledger/fabric/protos/common"
 	"github.com/hyperledger/fabric/protos/peer"
 )
 
@@ -22,6 +23,7 @@ type MockSupport struct {
 	CheckInstantiationPolicyErr      error
 	GetInstantiationPolicyMap        map[string][]byte
 	CheckInstantiationPolicyMap      map[string]error
+	CheckCollectionConfigErr         error
 }
 
 func (s *MockSupport) PutChaincodeToLocalStorage(ccpack ccprovider.CCPackage) error {
@@ -51,3 +53,7 @@ func (s *MockSupport) CheckInstantiationPolicy(signedProp *peer.SignedProposal,
 	}
 	return s.CheckInstantiationPolicyErr
 }
+
+func (s *MockSupport) CheckCollectionConfig(collectionConfig *common.CollectionConfig, channelName string) error {
+	return s.CheckCollectionConfigErr
+}

core/scc/lscc/lscc.go

@@ -24,8 +24,10 @@ import (
 	"github.com/hyperledger/fabric/core/peer"
 	"github.com/hyperledger/fabric/core/policy"
 	"github.com/hyperledger/fabric/core/policyprovider"
+	"github.com/hyperledger/fabric/msp"
 	"github.com/hyperledger/fabric/msp/mgmt"
 	"github.com/hyperledger/fabric/protos/common"
+	mb "github.com/hyperledger/fabric/protos/msp"
 	pb "github.com/hyperledger/fabric/protos/peer"
 	"github.com/hyperledger/fabric/protos/utils"
 	"github.com/pkg/errors"
@@ -142,6 +144,87 @@ func (lscc *lifeCycleSysCC) putChaincodeData(stub shim.ChaincodeStubInterface, c
 	return err
 }
 
+// checkCollectionMemberPolicy checks whether the supplied collection configuration
+// complies to the given msp configuration
+func checkCollectionMemberPolicy(collectionConfig *common.CollectionConfig, mspmgr msp.MSPManager) error {
+	if mspmgr == nil {
+		return fmt.Errorf("msp manager not set")
+	}
+	msps, err := mspmgr.GetMSPs()
+	if err != nil {
+		return errors.Wrapf(err, "error getting channel msp")
+	}
+	if collectionConfig == nil {
+		return fmt.Errorf("collection configuration is not set")
+	}
+	coll := collectionConfig.GetStaticCollectionConfig()
+	if coll == nil {
+		return fmt.Errorf("collection configuration is empty")
+	}
+	if coll.MemberOrgsPolicy == nil {
+		return fmt.Errorf("collection member policy is not set")
+	}
+	if coll.MemberOrgsPolicy.GetSignaturePolicy() == nil {
+		return fmt.Errorf("collection member org policy is empty")
+	}
+	// make sure that the orgs listed are actually part of the channel
+	// check all principals in the signature policy
+	for _, principal := range coll.MemberOrgsPolicy.GetSignaturePolicy().Identities {
+		found := false
+		var orgID string
+		// the member org policy only supports certain principal types
+		switch principal.PrincipalClassification {
+
+		case mb.MSPPrincipal_ROLE:
+			msprole := &mb.MSPRole{}
+			err := proto.Unmarshal(principal.Principal, msprole)
+			if err != nil {
+				return errors.Wrapf(err, "collection-name: %s -- cannot unmarshal identities", coll.GetName())
+			}
+			orgID = msprole.MspIdentifier
+			// the msp map is indexed using msp IDs - this behavior is implementation specific, making the following check a bit of a hack
+			for mspid := range msps {
+				if mspid == orgID {
+					found = true
+					break
+				}
+			}
+
+		case mb.MSPPrincipal_ORGANIZATION_UNIT:
+			mspou := &mb.OrganizationUnit{}
+			err := proto.Unmarshal(principal.Principal, mspou)
+			if err != nil {
+				return errors.Wrapf(err, "collection-name: %s -- cannot unmarshal identities", coll.GetName())
+			}
+			orgID = mspou.MspIdentifier
+			// the msp map is indexed using msp IDs - this behavior is implementation specific, making the following check a bit of a hack
+			for mspid := range msps {
+				if mspid == orgID {
+					found = true
+					break
+				}
+			}
+
+		case mb.MSPPrincipal_IDENTITY:
+			orgID = "identity principal"
+			for _, msp := range msps {
+				_, err := msp.DeserializeIdentity(principal.Principal)
+				if err == nil {
+					found = true
+					break
+				}
+			}
+		default:
+			return fmt.Errorf("collection-name: %s -- principal type %v is not supported", coll.GetName(), principal.PrincipalClassification)
+		}
+		if !found {
+			logger.Warningf("collection-name: %s collection member %s is not part of the channel", coll.GetName(), orgID)
+		}
+	}
+
+	return nil
+}
+
 // putChaincodeCollectionData adds collection data for the chaincode
 func (lscc *lifeCycleSysCC) putChaincodeCollectionData(stub shim.ChaincodeStubInterface, cd *ccprovider.ChaincodeData, collectionConfigBytes []byte) error {
 	if cd == nil {
@@ -159,7 +242,16 @@ func (lscc *lifeCycleSysCC) putChaincodeCollectionData(stub shim.ChaincodeStubIn
 		return errors.Errorf("invalid collection configuration supplied for chaincode %s:%s", cd.Name, cd.Version)
 	}
 
-	// TODO: FAB-6526 - to add validation of the collections object
+	mspmgr := mgmt.GetManagerForChain(stub.GetChannelID())
+	if mspmgr == nil {
+		return fmt.Errorf("could not get MSP manager for channel %s", stub.GetChannelID())
+	}
+	for _, collectionConfig := range collections.Config {
+		err = checkCollectionMemberPolicy(collectionConfig, mspmgr)
+		if err != nil {
+			return errors.Wrapf(err, "collection member policy check failed")
+		}
+	}
 
 	key := privdata.BuildCollectionKVSKey(cd.Name)