[FAB-15593] Extra validation of collection policies

This change set adds extra validation for collection policies during
lscc chaincode deploy/upgrade. It calls the constructor for
SignaturePolicyEnvelope evaluators to perform extra semantic validation.
Among other things, this validation catches any out-of-range references
to the identities array.

No capability is needed because the validation is done at endorsement.

Change-Id: I16d74f16c66dc3533ea7638c252a390df1bfae3b
Signed-off-by: Wenjian Qiao <wenjianq@gmail.com>

common/cauthdsl/cauthdsl.go

@@ -82,7 +82,7 @@ func compile(policy *cb.SignaturePolicy, identities []*mb.MSPPrincipal, deserial
 		}, nil
 	case *cb.SignaturePolicy_SignedBy:
 		if t.SignedBy < 0 || t.SignedBy >= int32(len(identities)) {
-			return nil, fmt.Errorf("identity index out of range, requested %v, but identies length is %d", t.SignedBy, len(identities))
+			return nil, fmt.Errorf("identity index out of range, requested %v, but identities length is %d", t.SignedBy, len(identities))
 		}
 		signedByID := identities[t.SignedBy]
 		return func(signedData []*cb.SignedData, used []bool) bool {

common/cauthdsl/cauthdsl_test.go

@@ -276,7 +276,7 @@ func TestReturnNil(t *testing.T) {
 
 	spe, err := compile(policy.Rule, policy.Identities, &mockDeserializer{})
 	assert.Nil(t, spe)
-	assert.EqualError(t, err, "identity index out of range, requested -1, but identies length is 2")
+	assert.EqualError(t, err, "identity index out of range, requested -1, but identities length is 2")
 }
 
 func TestDeserializeIdentityError(t *testing.T) {

common/cauthdsl/policy.go

@@ -51,6 +51,28 @@ func (pr *provider) NewPolicy(data []byte) (policies.Policy, proto.Message, erro
 
 }
 
+// EnvelopeBasedPolicyProvider allows to create a new policy from SignaturePolicyEnvelope struct instead of []byte
+type EnvelopeBasedPolicyProvider struct {
+	Deserializer msp.IdentityDeserializer
+}
+
+// NewPolicy creates a new policy from the policy envelope
+func (pp *EnvelopeBasedPolicyProvider) NewPolicy(sigPolicy *cb.SignaturePolicyEnvelope) (policies.Policy, error) {
+	if sigPolicy == nil {
+		return nil, errors.New("invalid arguments")
+	}
+
+	compiled, err := compile(sigPolicy.Rule, sigPolicy.Identities, pp.Deserializer)
+	if err != nil {
+		return nil, err
+	}
+
+	return &policy{
+		evaluator:    compiled,
+		deserializer: pp.Deserializer,
+	}, nil
+}
+
 type policy struct {
 	evaluator    func([]*cb.SignedData, []bool) bool
 	deserializer msp.IdentityDeserializer

common/cauthdsl/policy_test.go

@@ -118,3 +118,18 @@ func TestNewPolicyErrorCase(t *testing.T) {
 	err4 := pol4.Evaluate([]*cb.SignedData{})
 	assert.EqualError(t, err4, "No such policy")
 }
+
+func TestEnvelopeBasedPolicyProvider(t *testing.T) {
+	pp := &EnvelopeBasedPolicyProvider{Deserializer: &mockDeserializer{}}
+	p, err := pp.NewPolicy(nil)
+	assert.Nil(t, p)
+	assert.Error(t, err, "invalid arguments")
+
+	p, err = pp.NewPolicy(&cb.SignaturePolicyEnvelope{})
+	assert.Nil(t, p)
+	assert.Error(t, err, "Empty policy element")
+
+	p, err = pp.NewPolicy(SignedByMspPeer("primus inter pares"))
+	assert.NotNil(t, p)
+	assert.NoError(t, err)
+}

core/scc/lscc/lscc.go

@@ -218,7 +218,11 @@ func (lscc *LifeCycleSysCC) putChaincodeData(stub shim.ChaincodeStubInterface, c
 }
 
 // checkCollectionMemberPolicy checks whether the supplied collection configuration
-// complies to the given msp configuration
+// complies to the given msp configuration and performs semantic validation.
+// Channel config may change afterwards (i.e., after endorsement or commit of this transaction).
+// Fabric will deal with the situation where some collection configs are no longer meaningful.
+// Therefore, the use of channel config for verifying during endorsement is more
+// towards catching manual errors in the config as oppose to any attempt of serializability.
 func checkCollectionMemberPolicy(collectionConfig *common.CollectionConfig, mspmgr msp.MSPManager) error {
 	if mspmgr == nil {
 		return fmt.Errorf("msp manager not set")
@@ -295,6 +299,14 @@ func checkCollectionMemberPolicy(collectionConfig *common.CollectionConfig, mspm
 		}
 	}
 
+	// Call the constructor for SignaturePolicyEnvelope evaluators to perform extra semantic validation.
+	// Among other things, this validation catches any out-of-range references to the identities array.
+	policyProvider := &cauthdsl.EnvelopeBasedPolicyProvider{Deserializer: mspmgr}
+	if _, err := policyProvider.NewPolicy(coll.MemberOrgsPolicy.GetSignaturePolicy()); err != nil {
+		logger.Errorf("Invalid member org policy for collection '%s', error: %s", coll.Name, err)
+		return errors.WithMessage(err, fmt.Sprintf("invalid member org policy for collection '%s'", coll.Name))
+	}
+
 	return nil
 }
 

core/scc/lscc/lscc_test.go

@@ -35,7 +35,7 @@ import (
 	"github.com/hyperledger/fabric/core/scc/lscc/mock"
 	"github.com/hyperledger/fabric/msp"
 	mspmgmt "github.com/hyperledger/fabric/msp/mgmt"
-	"github.com/hyperledger/fabric/msp/mgmt/testtools"
+	msptesttools "github.com/hyperledger/fabric/msp/mgmt/testtools"
 	mspmocks "github.com/hyperledger/fabric/msp/mocks"
 	"github.com/hyperledger/fabric/protos/common"
 	"github.com/hyperledger/fabric/protos/ledger/queryresult"
@@ -57,6 +57,18 @@ type stateQueryIterator interface {
 	shim.StateQueryIteratorInterface
 }
 
+// create a valid SignaturePolicyEnvelope to be used in tests
+var testPolicyEnvelope = &common.SignaturePolicyEnvelope{
+	Version: 0,
+	Rule:    cauthdsl.NOutOf(1, []*common.SignaturePolicy{cauthdsl.SignedBy(0)}),
+	Identities: []*mb.MSPPrincipal{
+		{
+			PrincipalClassification: mb.MSPPrincipal_ORGANIZATION_UNIT,
+			Principal:               utils.MarshalOrPanic(&mb.OrganizationUnit{MspIdentifier: "Org1"}),
+		},
+	},
+}
+
 func constructDeploymentSpec(name string, path string, version string, initArgs [][]byte, createInvalidIndex bool, createFS bool, scc *LifeCycleSysCC) (*pb.ChaincodeDeploymentSpec, error) {
 	spec := &pb.ChaincodeSpec{Type: pb.ChaincodeSpec_GOLANG, ChaincodeId: &pb.ChaincodeID{Name: name, Path: path, Version: version}, Input: &pb.ChaincodeInput{Args: initArgs}}
 
@@ -511,11 +523,10 @@ func TestUpgrade(t *testing.T) {
 	scc.Support.(*lscc.MockSupport).GetInstantiationPolicyRv = []byte("instantiation policy")
 
 	collName1 := "mycollection1"
-	policyEnvelope := &common.SignaturePolicyEnvelope{}
 	var requiredPeerCount, maximumPeerCount int32
 	requiredPeerCount = 1
 	maximumPeerCount = 2
-	coll1 := createCollectionConfig(collName1, policyEnvelope, requiredPeerCount, maximumPeerCount)
+	coll1 := createCollectionConfig(collName1, testPolicyEnvelope, requiredPeerCount, maximumPeerCount)
 
 	ccp := &common.CollectionConfigPackage{Config: []*common.CollectionConfig{coll1}}
 	ccpBytes, err := proto.Marshal(ccp)
@@ -873,8 +884,7 @@ func TestPutChaincodeCollectionData(t *testing.T) {
 	assert.NoError(t, err)
 
 	collName1 := "mycollection1"
-	policyEnvelope := &common.SignaturePolicyEnvelope{}
-	coll1 := createCollectionConfig(collName1, policyEnvelope, 1, 2)
+	coll1 := createCollectionConfig(collName1, testPolicyEnvelope, 1, 2)
 	ccp := &common.CollectionConfigPackage{Config: []*common.CollectionConfig{coll1}}
 	ccpBytes, err := proto.Marshal(ccp)
 	assert.NoError(t, err)
@@ -900,8 +910,7 @@ func TestGetChaincodeCollectionData(t *testing.T) {
 	cd := &ccprovider.ChaincodeData{Name: "foo"}
 
 	collName1 := "mycollection1"
-	policyEnvelope := &common.SignaturePolicyEnvelope{}
-	coll1 := createCollectionConfig(collName1, policyEnvelope, 1, 2)
+	coll1 := createCollectionConfig(collName1, testPolicyEnvelope, 1, 2)
 	ccp := &common.CollectionConfigPackage{Config: []*common.CollectionConfig{coll1}}
 	ccpBytes, err := proto.Marshal(ccp)
 	assert.NoError(t, err)
@@ -960,25 +969,25 @@ func TestCheckCollectionMemberPolicy(t *testing.T) {
 	mgr := mspmgmt.GetManagerForChain("foochannel")
 
 	// error case: msp manager not set up, no collection config set
-	err = checkCollectionMemberPolicy(nil, mgr)
-	assert.Error(t, err)
+	err = checkCollectionMemberPolicy(nil, nil)
+	assert.EqualError(t, err, "msp manager not set")
 
 	// set up msp manager
 	mgr.Setup([]msp.MSP{mockmsp})
 
 	// error case: no collection config set
 	err = checkCollectionMemberPolicy(nil, mgr)
-	assert.Error(t, err)
+	assert.EqualError(t, err, "collection configuration is not set")
 
 	// error case: empty collection config
 	cc := &common.CollectionConfig{}
 	err = checkCollectionMemberPolicy(cc, mgr)
-	assert.Error(t, err)
+	assert.EqualError(t, err, "collection configuration is empty")
 
 	// error case: no static collection config
 	cc = &common.CollectionConfig{Payload: &common.CollectionConfig_StaticCollectionConfig{}}
 	err = checkCollectionMemberPolicy(cc, mgr)
-	assert.Error(t, err)
+	assert.EqualError(t, err, "collection configuration is empty")
 
 	// error case: member org policy not set
 	cc = &common.CollectionConfig{
@@ -987,7 +996,7 @@ func TestCheckCollectionMemberPolicy(t *testing.T) {
 		},
 	}
 	err = checkCollectionMemberPolicy(cc, mgr)
-	assert.Error(t, err)
+	assert.EqualError(t, err, "collection member policy is not set")
 
 	// error case: member org policy config empty
 	cc = &common.CollectionConfig{
@@ -1001,16 +1010,28 @@ func TestCheckCollectionMemberPolicy(t *testing.T) {
 		},
 	}
 	err = checkCollectionMemberPolicy(cc, mgr)
-	assert.Error(t, err)
+	assert.EqualError(t, err, "collection member org policy is empty")
 
-	// valid case: member org policy empty
+	// error case: signd-by index is out of range of signers
+	cc = &common.CollectionConfig{
+		Payload: &common.CollectionConfig_StaticCollectionConfig{
+			StaticCollectionConfig: &common.StaticCollectionConfig{
+				Name:             "mycollection",
+				MemberOrgsPolicy: getBadAccessPolicy([]string{"signer0"}, 1),
+			},
+		},
+	}
+	err = checkCollectionMemberPolicy(cc, mgr)
+	assert.EqualError(t, err, "invalid member org policy for collection 'mycollection': identity index out of range, requested 1, but identities length is 1")
+
+	// valid case: well-formed collection policy config
 	cc = &common.CollectionConfig{
 		Payload: &common.CollectionConfig_StaticCollectionConfig{
 			StaticCollectionConfig: &common.StaticCollectionConfig{
 				Name: "mycollection",
 				MemberOrgsPolicy: &common.CollectionPolicyConfig{
 					Payload: &common.CollectionPolicyConfig_SignaturePolicy{
-						SignaturePolicy: &common.SignaturePolicyEnvelope{},
+						SignaturePolicy: testPolicyEnvelope,
 					},
 				},
 			},
@@ -1038,7 +1059,7 @@ func TestCheckCollectionMemberPolicy(t *testing.T) {
 	}
 	err = checkCollectionMemberPolicy(cc, mgr)
 	assert.NoError(t, err)
-	mockmsp.AssertNumberOfCalls(t, "DeserializeIdentity", 2)
+	mockmsp.AssertNumberOfCalls(t, "DeserializeIdentity", 3)
 
 	// check MSPPrincipal_ROLE type
 	signaturePolicyEnvelope = cauthdsl.SignedByAnyMember([]string{"Org1"})
@@ -1150,3 +1171,18 @@ func TestMain(m *testing.M) {
 
 	os.Exit(m.Run())
 }
+
+// getBadAccessPolicy creates a bad CollectionPolicyConfig with signedby index out of range of signers
+func getBadAccessPolicy(signers []string, badIndex int32) *common.CollectionPolicyConfig {
+	var data [][]byte
+	for _, signer := range signers {
+		data = append(data, []byte(signer))
+	}
+	// use a out of range index to trigger error
+	policyEnvelope := cauthdsl.Envelope(cauthdsl.Or(cauthdsl.SignedBy(0), cauthdsl.SignedBy(badIndex)), data)
+	return &common.CollectionPolicyConfig{
+		Payload: &common.CollectionPolicyConfig_SignaturePolicy{
+			SignaturePolicy: policyEnvelope,
+		},
+	}
+}