Skip to content

Commit 9fe8c60

Browse files
committed
[FAB-1934] admin validation for chain-scoped syscc
This change set validates proposals for chain-scoped system chaincodes against the admin policy of the chain. Change-Id: I1dc2211fd3e2170b31a62a22b4b95287d7b1acf4 Signed-off-by: Alessandro Sorniotti <[email protected]>
1 parent 3e0481b commit 9fe8c60

File tree

3 files changed

+76
-12
lines changed

3 files changed

+76
-12
lines changed

common/policies/policy.go

+7-1
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,9 @@ const (
4040

4141
// ChannelApplicationWriters is the label for the channel's application writers policy
4242
ChannelApplicationWriters = "/" + ChannelPrefix + "/" + ApplicationPrefix + "/Writers"
43+
44+
// ChannelApplicationAdmins is the label for the channel's application admin policy
45+
ChannelApplicationAdmins = "/" + ChannelPrefix + "/" + ApplicationPrefix + "/Admins"
4346
)
4447

4548
var logger = logging.MustGetLogger("common/policies")
@@ -251,7 +254,10 @@ func (pm *ManagerImpl) CommitProposals() {
251254
if pm.parent == nil && pm.basePath == ChannelPrefix {
252255
if _, ok := pm.config.managers[ApplicationPrefix]; ok {
253256
// Check for default application policies if the application component is defined
254-
for _, policyName := range []string{ChannelApplicationReaders, ChannelApplicationWriters} {
257+
for _, policyName := range []string{
258+
ChannelApplicationReaders,
259+
ChannelApplicationWriters,
260+
ChannelApplicationAdmins} {
255261
_, ok := pm.GetPolicy(policyName)
256262
if !ok {
257263
logger.Warningf("Current configuration has no policy '%s', this will likely cause problems in production systems", policyName)

core/endorser/endorser.go

+22-7
Original file line numberDiff line numberDiff line change
@@ -55,15 +55,26 @@ func NewEndorserServer() pb.EndorserServer {
5555
return e
5656
}
5757

58-
func (*Endorser) checkACL(signedProp *pb.SignedProposal, chdr *common.ChannelHeader, shdr *common.SignatureHeader) error {
58+
// checkACL checks that the supplied proposal complies
59+
// with the policies of the chain; for a system chaincode
60+
// we use the admins policy, whereas for normal chaincodes
61+
// we use the writers policy
62+
func (*Endorser) checkACL(signedProp *pb.SignedProposal, chdr *common.ChannelHeader, shdr *common.SignatureHeader, hdrext *pb.ChaincodeHeaderExtension) error {
5963
// get policy manager to check ACLs
6064
pm := peer.GetPolicyManager(chdr.ChannelId)
6165
if pm == nil {
6266
return fmt.Errorf("No policy manager available for chain %s", chdr.ChannelId)
6367
}
6468

65-
// access the writers policy
66-
policy, _ := pm.GetPolicy(policies.ChannelApplicationWriters)
69+
// access the policy to use to validate this proposal
70+
var policy policies.Policy
71+
if syscc.IsSysCC(hdrext.ChaincodeId.Name) {
72+
// in the case of a system chaincode, we use the admin policy
73+
policy, _ = pm.GetPolicy(policies.ChannelApplicationAdmins)
74+
} else {
75+
// in the case of a normal chaincode, we use the writers policy
76+
policy, _ = pm.GetPolicy(policies.ChannelApplicationWriters)
77+
}
6778

6879
// evaluate that this proposal complies with the writers
6980
err := policy.Evaluate(
@@ -317,9 +328,8 @@ func (e *Endorser) ProcessProposal(ctx context.Context, signedProp *pb.SignedPro
317328
return &pb.ProposalResponse{Response: &pb.Response{Status: 500, Message: err.Error()}}, err
318329
}
319330

320-
//chainless proposals do not/cannot affect ledger and cannot be submitted as transactions
321-
//ignore uniqueness checks
322331
if chainID != "" {
332+
// here we handle uniqueness check and ACLs for proposals targeting a chain
323333
lgr := peer.GetLedger(chainID)
324334
if lgr == nil {
325335
return nil, errors.New(fmt.Sprintf("Failure while looking up the ledger %s", chainID))
@@ -329,10 +339,15 @@ func (e *Endorser) ProcessProposal(ctx context.Context, signedProp *pb.SignedPro
329339
}
330340

331341
// check ACL - we verify that this proposal
332-
// complies with the "writers" policy of the chain
333-
if err = e.checkACL(signedProp, chdr, shdr); err != nil {
342+
// complies with the policy of the chain
343+
if err = e.checkACL(signedProp, chdr, shdr, hdrExt); err != nil {
334344
return &pb.ProposalResponse{Response: &pb.Response{Status: 500, Message: err.Error()}}, err
335345
}
346+
} else {
347+
// chainless proposals do not/cannot affect ledger and cannot be submitted as transactions
348+
// ignore uniqueness checks; also, chainless proposals are not validated using the policies
349+
// of the chain since by definition there is no chain; they are validated against the local
350+
// MSP of the peer instead by the call to ValidateProposalMessage above
336351
}
337352

338353
// obtaining once the tx simulator for this proposal. This will be nil

core/endorser/endorser_test.go

+47-4
Original file line numberDiff line numberDiff line change
@@ -509,11 +509,11 @@ func TestDeployAndUpgrade(t *testing.T) {
509509
chaincode.GetChain().Stop(ctxt, cccid2, &pb.ChaincodeDeploymentSpec{ChaincodeSpec: &pb.ChaincodeSpec{ChaincodeId: chaincodeID2}})
510510
}
511511

512-
// TestACLFail deploys a chaincode and then tries to invoke it;
512+
// TestWritersACLFail deploys a chaincode and then tries to invoke it;
513513
// however we inject a special policy for writers to simulate
514514
// the scenario in which the creator of this proposal is not among
515515
// the writers for the chain
516-
func TestACLFail(t *testing.T) {
516+
func TestWritersACLFail(t *testing.T) {
517517
chainID := util.GetTestChainID()
518518
var ctxt = context.Background()
519519

@@ -567,8 +567,51 @@ func TestACLFail(t *testing.T) {
567567
return
568568
}
569569

570-
fmt.Println("TestACLFail passed")
571-
t.Logf("TestACLFail passed")
570+
fmt.Println("TestWritersACLFail passed")
571+
t.Logf("TestWritersACLFail passed")
572+
573+
chaincode.GetChain().Stop(ctxt, cccid, &pb.ChaincodeDeploymentSpec{ChaincodeSpec: &pb.ChaincodeSpec{ChaincodeId: chaincodeID}})
574+
}
575+
576+
// TestAdminACLFail deploys tried to deploy a chaincode;
577+
// however we inject a special policy for admins to simulate
578+
// the scenario in which the creator of this proposal is not among
579+
// the admins for the chain
580+
func TestAdminACLFail(t *testing.T) {
581+
chainID := util.GetTestChainID()
582+
583+
// here we inject a reject policy for admins
584+
// to simulate the scenario in which the invoker
585+
// is not authorized to issue this proposal
586+
rejectpolicy := &mockpolicies.Policy{
587+
Err: errors.New("The creator of this proposal does not fulfil the writers policy of this chain"),
588+
}
589+
pm := peer.GetPolicyManager(chainID)
590+
pm.(*mockpolicies.Manager).PolicyMap = map[string]*mockpolicies.Policy{policies.ChannelApplicationAdmins: rejectpolicy}
591+
592+
var ctxt = context.Background()
593+
594+
url := "github.com/hyperledger/fabric/examples/chaincode/go/chaincode_example01"
595+
chaincodeID := &pb.ChaincodeID{Path: url, Name: "ex01-fail1", Version: "0"}
596+
597+
defer deleteChaincodeOnDisk("ex01-fail1.0")
598+
599+
f := "init"
600+
argsDeploy := util.ToChaincodeArgs(f, "a", "100", "b", "200")
601+
spec := &pb.ChaincodeSpec{Type: 1, ChaincodeId: chaincodeID, Input: &pb.ChaincodeInput{Args: argsDeploy}}
602+
603+
cccid := ccprovider.NewCCContext(chainID, "ex01-fail1", "0", "", false, nil, nil)
604+
605+
_, _, err := deploy(endorserServer, chainID, spec, nil)
606+
if err == nil {
607+
t.Fail()
608+
t.Logf("Deploying chaincode should have failed!")
609+
chaincode.GetChain().Stop(ctxt, cccid, &pb.ChaincodeDeploymentSpec{ChaincodeSpec: &pb.ChaincodeSpec{ChaincodeId: chaincodeID}})
610+
return
611+
}
612+
613+
fmt.Println("TestAdminACLFail passed")
614+
t.Logf("TestATestAdminACLFailCLFail passed")
572615

573616
chaincode.GetChain().Stop(ctxt, cccid, &pb.ChaincodeDeploymentSpec{ChaincodeSpec: &pb.ChaincodeSpec{ChaincodeId: chaincodeID}})
574617
}

0 commit comments

Comments
 (0)