[FAB-2080] - peer enforces ACLs on proposals

This change set enables the peer to validate proposal messages against channel
policies.

Change-Id: Ie60a9deefa5e0002dbf8125e512f3f43db78102e
Signed-off-by: Alessandro Sorniotti <[email protected]>

Author: ale-linux

common/policies/policy.go

@@ -37,6 +37,9 @@ const (
 
 	// ChannelApplicationReaders is the label for the channel's application readers policy
 	ChannelApplicationReaders = "/" + ChannelPrefix + "/" + ApplicationPrefix + "/Readers"
+
+	// ChannelApplicationWriters is the label for the channel's application writers policy
+	ChannelApplicationWriters = "/" + ChannelPrefix + "/" + ApplicationPrefix + "/Writers"
 )
 
 var logger = logging.MustGetLogger("common/policies")
@@ -248,7 +251,7 @@ func (pm *ManagerImpl) CommitProposals() {
 	if pm.parent == nil && pm.basePath == ChannelPrefix {
 		if _, ok := pm.config.managers[ApplicationPrefix]; ok {
 			// Check for default application policies if the application component is defined
-			for _, policyName := range []string{ChannelApplicationReaders} {
+			for _, policyName := range []string{ChannelApplicationReaders, ChannelApplicationWriters} {
 				_, ok := pm.GetPolicy(policyName)
 				if !ok {
 					logger.Warningf("Current configuration has no policy '%s', this will likely cause problems in production systems", policyName)

core/common/validation/msgvalidation.go

@@ -90,8 +90,6 @@ func ValidateProposalMessage(signedProp *pb.SignedProposal) (*pb.Proposal, *comm
 		return nil, nil, nil, err
 	}
 
-	// TODO: ensure that creator can transact with us (some ACLs?) which set of APIs is supposed to give us this info?
-
 	// Verify that the transaction ID has been computed properly.
 	// This check is needed to ensure that the lookup into the ledger
 	// for the same TxID catches duplicates.

core/endorser/endorser.go

@@ -25,6 +25,7 @@ import (
 
 	"errors"
 
+	"github.com/hyperledger/fabric/common/policies"
 	"github.com/hyperledger/fabric/common/util"
 	"github.com/hyperledger/fabric/core/chaincode"
 	"github.com/hyperledger/fabric/core/chaincode/shim"
@@ -54,8 +55,29 @@ func NewEndorserServer() pb.EndorserServer {
 	return e
 }
 
-//TODO - what would Endorser's ACL be ?
-func (*Endorser) checkACL(signedProp *pb.SignedProposal, prop *pb.Proposal) error {
+func (*Endorser) checkACL(signedProp *pb.SignedProposal, chdr *common.ChannelHeader, shdr *common.SignatureHeader) error {
+	// get policy manager to check ACLs
+	pm := peer.GetPolicyManager(chdr.ChannelId)
+	if pm == nil {
+		return fmt.Errorf("No policy manager available for chain %s", chdr.ChannelId)
+	}
+
+	// access the writers policy
+	policy, _ := pm.GetPolicy(policies.ChannelApplicationWriters)
+
+	// evaluate that this proposal complies with the writers
+	err := policy.Evaluate(
+		[]*common.SignedData{{
+			Data:      signedProp.ProposalBytes,
+			Identity:  shdr.Creator,
+			Signature: signedProp.Signature,
+		}})
+	if err != nil {
+		return fmt.Errorf("The proposal does not comply with the channel writers for channel %s, error %s",
+			chdr.ChannelId,
+			err)
+	}
+
 	return nil
 }
 
@@ -146,12 +168,8 @@ func (e *Endorser) simulateProposal(ctx context.Context, chainID string, txid st
 	if err != nil {
 		return nil, nil, nil, nil, err
 	}
-	//---1. check ACL
-	if err = e.checkACL(signedProp, prop); err != nil {
-		return nil, nil, nil, nil, err
-	}
 
-	//---2. check ESCC and VSCC for the chaincode
+	//---1. check ESCC and VSCC for the chaincode
 	if err = e.checkEsccAndVscc(prop); err != nil {
 		return nil, nil, nil, nil, err
 	}
@@ -290,13 +308,6 @@ func (e *Endorser) ProcessProposal(ctx context.Context, signedProp *pb.SignedPro
 
 	chainID := chdr.ChannelId
 
-	//chainless MSPs have "" chain name
-	ischainless := false
-
-	if chainID == "" {
-		ischainless = true
-	}
-
 	// Check for uniqueness of prop.TxID with ledger
 	// Notice that ValidateProposalMessage has already verified
 	// that TxID is computed propertly
@@ -308,14 +319,20 @@ func (e *Endorser) ProcessProposal(ctx context.Context, signedProp *pb.SignedPro
 
 	//chainless proposals do not/cannot affect ledger and cannot be submitted as transactions
 	//ignore uniqueness checks
-	if !ischainless {
+	if chainID != "" {
 		lgr := peer.GetLedger(chainID)
 		if lgr == nil {
 			return nil, errors.New(fmt.Sprintf("Failure while looking up the ledger %s", chainID))
 		}
 		if _, err := lgr.GetTransactionByID(txid); err == nil {
 			return nil, fmt.Errorf("Duplicate transaction found [%s]. Creator [%x]. [%s]", txid, shdr.Creator, err)
 		}
+
+		// check ACL - we verify that this proposal
+		// complies with the "writers" policy of the chain
+		if err = e.checkACL(signedProp, chdr, shdr); err != nil {
+			return &pb.ProposalResponse{Response: &pb.Response{Status: 500, Message: err.Error()}}, err
+		}
 	}
 
 	// obtaining once the tx simulator for this proposal. This will be nil
@@ -355,7 +372,7 @@ func (e *Endorser) ProcessProposal(ctx context.Context, signedProp *pb.SignedPro
 
 	//TODO till we implement global ESCC, CSCC for system chaincodes
 	//chainless proposals (such as CSCC) don't have to be endorsed
-	if ischainless {
+	if chainID == "" {
 		pResp = &pb.ProposalResponse{Response: res}
 	} else {
 		pResp, err = e.endorseProposal(ctx, chainID, txid, signedProp, prop, res, simulationResult, ccevent, hdrExt.PayloadVisibility, hdrExt.ChaincodeId, txsim, cd)

core/endorser/endorser_test.go

@@ -26,7 +26,11 @@ import (
 
 	"path/filepath"
 
+	"errors"
+
 	"github.com/golang/protobuf/proto"
+	mockpolicies "github.com/hyperledger/fabric/common/mocks/policies"
+	"github.com/hyperledger/fabric/common/policies"
 	"github.com/hyperledger/fabric/common/util"
 	"github.com/hyperledger/fabric/core/chaincode"
 	"github.com/hyperledger/fabric/core/common/ccprovider"
@@ -505,6 +509,70 @@ func TestDeployAndUpgrade(t *testing.T) {
 	chaincode.GetChain().Stop(ctxt, cccid2, &pb.ChaincodeDeploymentSpec{ChaincodeSpec: &pb.ChaincodeSpec{ChaincodeId: chaincodeID2}})
 }
 
+// TestACLFail deploys a chaincode and then tries to invoke it;
+// however we inject a special policy for writers to simulate
+// the scenario in which the creator of this proposal is not among
+// the writers for the chain
+func TestACLFail(t *testing.T) {
+	chainID := util.GetTestChainID()
+	var ctxt = context.Background()
+
+	url := "github.com/hyperledger/fabric/examples/chaincode/go/chaincode_example01"
+	chaincodeID := &pb.ChaincodeID{Path: url, Name: "ex01-fail", Version: "0"}
+
+	defer deleteChaincodeOnDisk("ex01-fail.0")
+
+	args := []string{"10"}
+
+	f := "init"
+	argsDeploy := util.ToChaincodeArgs(f, "a", "100", "b", "200")
+	spec := &pb.ChaincodeSpec{Type: 1, ChaincodeId: chaincodeID, Input: &pb.ChaincodeInput{Args: argsDeploy}}
+
+	cccid := ccprovider.NewCCContext(chainID, "ex01-fail", "0", "", false, nil, nil)
+
+	resp, prop, err := deploy(endorserServer, chainID, spec, nil)
+	chaincodeID1 := spec.ChaincodeId.Name
+	if err != nil {
+		t.Fail()
+		t.Logf("Error deploying <%s>: %s", chaincodeID1, err)
+		chaincode.GetChain().Stop(ctxt, cccid, &pb.ChaincodeDeploymentSpec{ChaincodeSpec: &pb.ChaincodeSpec{ChaincodeId: chaincodeID}})
+		return
+	}
+	var nextBlockNumber uint64 = 3 // The tests that ran before this test created blocks 0-2
+	err = endorserServer.(*Endorser).commitTxSimulation(prop, chainID, signer, resp, nextBlockNumber)
+	if err != nil {
+		t.Fail()
+		t.Logf("Error committing deploy <%s>: %s", chaincodeID1, err)
+		chaincode.GetChain().Stop(ctxt, cccid, &pb.ChaincodeDeploymentSpec{ChaincodeSpec: &pb.ChaincodeSpec{ChaincodeId: chaincodeID}})
+		return
+	}
+
+	// here we inject a reject policy for writers
+	// to simulate the scenario in which the invoker
+	// is not authorized to issue this proposal
+	rejectpolicy := &mockpolicies.Policy{
+		Err: errors.New("The creator of this proposal does not fulfil the writers policy of this chain"),
+	}
+	pm := peer.GetPolicyManager(chainID)
+	pm.(*mockpolicies.Manager).PolicyMap = map[string]*mockpolicies.Policy{policies.ChannelApplicationWriters: rejectpolicy}
+
+	f = "invoke"
+	invokeArgs := append([]string{f}, args...)
+	spec = &pb.ChaincodeSpec{Type: 1, ChaincodeId: chaincodeID, Input: &pb.ChaincodeInput{Args: util.ToChaincodeArgs(invokeArgs...)}}
+	prop, resp, _, _, err = invoke(chainID, spec)
+	if err == nil {
+		t.Fail()
+		t.Logf("Invocation should have failed")
+		chaincode.GetChain().Stop(ctxt, cccid, &pb.ChaincodeDeploymentSpec{ChaincodeSpec: &pb.ChaincodeSpec{ChaincodeId: chaincodeID}})
+		return
+	}
+
+	fmt.Println("TestACLFail passed")
+	t.Logf("TestACLFail passed")
+
+	chaincode.GetChain().Stop(ctxt, cccid, &pb.ChaincodeDeploymentSpec{ChaincodeSpec: &pb.ChaincodeSpec{ChaincodeId: chaincodeID}})
+}
+
 func newTempDir() string {
 	tempDir, err := ioutil.TempDir("", "fabric-")
 	if err != nil {

core/peer/peer.go

@@ -25,14 +25,15 @@ import (
 	"github.com/hyperledger/fabric/common/configtx"
 	configtxapi "github.com/hyperledger/fabric/common/configtx/api"
 	configvaluesapi "github.com/hyperledger/fabric/common/configvalues"
+	mockconfigtx "github.com/hyperledger/fabric/common/mocks/configtx"
+	mockpolicies "github.com/hyperledger/fabric/common/mocks/policies"
 	"github.com/hyperledger/fabric/common/policies"
 	"github.com/hyperledger/fabric/core/comm"
 	"github.com/hyperledger/fabric/core/committer"
 	"github.com/hyperledger/fabric/core/committer/txvalidator"
 	"github.com/hyperledger/fabric/core/ledger"
 	"github.com/hyperledger/fabric/core/ledger/ledgermgmt"
 	"github.com/hyperledger/fabric/gossip/service"
-	"github.com/hyperledger/fabric/msp"
 	mspmgmt "github.com/hyperledger/fabric/msp/mgmt"
 	"github.com/hyperledger/fabric/protos/common"
 	"github.com/hyperledger/fabric/protos/utils"
@@ -236,9 +237,22 @@ func MockCreateChain(cid string) error {
 		return err
 	}
 
+	i := mockconfigtx.Initializer{
+		Resources: mockconfigtx.Resources{
+			PolicyManagerVal: &mockpolicies.Manager{
+				Policy: &mockpolicies.Policy{},
+			},
+		},
+	}
+
 	chains.Lock()
 	defer chains.Unlock()
-	chains.list[cid] = &chain{cs: &chainSupport{ledger: ledger}}
+	chains.list[cid] = &chain{
+		cs: &chainSupport{
+			ledger:  ledger,
+			Manager: &mockconfigtx.Manager{Initializer: i},
+		},
+	}
 
 	return nil
 }
@@ -265,28 +279,6 @@ func GetPolicyManager(cid string) policies.Manager {
 	return nil
 }
 
-// GetMSPMgr returns the MSP manager of the chain with chain ID.
-// Note that this call returns nil if chain cid has not been created.
-func GetMSPMgr(cid string) msp.MSPManager {
-	chains.RLock()
-	defer chains.RUnlock()
-	if c, ok := chains.list[cid]; ok {
-		return c.cs.MSPManager()
-	}
-	return nil
-}
-
-// GetCommitter returns the committer of the chain with chain ID. Note that this
-// call returns nil if chain cid has not been created.
-func GetCommitter(cid string) committer.Committer {
-	chains.RLock()
-	defer chains.RUnlock()
-	if c, ok := chains.list[cid]; ok {
-		return c.committer
-	}
-	return nil
-}
-
 // GetCurrConfigBlock returns the cached config block of the specified chain.
 // Note that this call returns nil if chain cid has not been created.
 func GetCurrConfigBlock(cid string) *common.Block {

peer/node/start.go

@@ -27,8 +27,12 @@ import (
 	"syscall"
 	"time"
 
+	"github.com/hyperledger/fabric/common/configtx"
 	"github.com/hyperledger/fabric/common/configtx/test"
+	"github.com/hyperledger/fabric/common/configvalues/channel/application"
+	"github.com/hyperledger/fabric/common/configvalues/msp"
 	"github.com/hyperledger/fabric/common/genesis"
+	"github.com/hyperledger/fabric/common/policies"
 	"github.com/hyperledger/fabric/common/util"
 	"github.com/hyperledger/fabric/core"
 	"github.com/hyperledger/fabric/core/chaincode"
@@ -161,9 +165,19 @@ func serve(args []string) error {
 	if peerDefaultChain {
 		chainID := util.GetTestChainID()
 
+		// add readers, writers and admin policies for the default chain
+		policyTemplate := configtx.NewSimpleTemplate(
+			policies.TemplateImplicitMetaAnyPolicy([]string{application.GroupKey}, msp.ReadersPolicyKey),
+			policies.TemplateImplicitMetaAnyPolicy([]string{application.GroupKey}, msp.WritersPolicyKey),
+			policies.TemplateImplicitMetaMajorityPolicy([]string{application.GroupKey}, msp.AdminsPolicyKey),
+		)
+
 		// We create a genesis block for the test
 		// chain with its MSP so that we can transact
-		block, err := genesis.NewFactoryImpl(test.ApplicationOrgTemplate()).Block(chainID)
+		block, err := genesis.NewFactoryImpl(
+			configtx.NewCompositeTemplate(
+				test.ApplicationOrgTemplate(),
+				policyTemplate)).Block(chainID)
 		if nil != err {
 			panic(fmt.Sprintf("Unable to create genesis block for [%s] due to [%s]", chainID, err))
 		}