Skip to content

Commit

Permalink
[FAB-5309] Set mod_policy for new channel policies
Browse files Browse the repository at this point in the history
The current configtxgen creates a channel creation tx with policies of
Reader/Writer/Admin, where these each do not have a mod_policy set.
This makes it difficult to change these policies, because the policy
does not exist.

It is possible through a roundabout way to change these policies (by
removing and re-adding the policy with a different mod_policy), but this
is certainly not a painless procedure and requires assistance of the
orderer org.

This CR updates to configtx processing to validate mod_policy values
such that they are valid names.  This means that the policy is not
empty, and if the policy is path specified (ie using '/'), that each
component of the path can be described as a valid config element (does
not violate the config element naming rules).

This CR additionally sets the mod_policy on the policies generated by
configtxgen to "Admins".

For users of the old v1.0.0 configtxgen, channel creation will fail with
an error for v1.0.1 orderers.  However, v1.0.1 configtxgen users will
have their transaction appropriately consumed by v1.0.0 orderers.

So in summary, upgrading configtxgen without fabric is okay.  Upgrading
fabric without configtxgen will cause breakage for new channel creation.

An additional bug in the configtxgen output was discovered which was not
setting a mod_policy on the anchor peers element.  There was also an
error in the bddtests which mimic the missing policies for the
application level policies.

Change-Id: Ic2bc120cfb6170f3e4e6cbeac5be145363a64861
Signed-off-by: Jason Yellick <[email protected]>
  • Loading branch information
Jason Yellick committed Jul 17, 2017
1 parent 8737eba commit 3a2dd8e
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 6 deletions.
2 changes: 1 addition & 1 deletion bddtests/steps/bootstrap_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -1102,7 +1102,7 @@ def create_channel_config_update(system_channel_version, channel_id, consortium_
Policy(type=typeImplicitMeta, value=IMP(
rule=ruleMajority, sub_policy=BootstrapHelper.KEY_POLICY_ADMINS).SerializeToString()))
write_set.groups[ApplicationGroup].mod_policy = "Admins"
for k, v in write_set.groups[ApplicationGroup].groups.iteritems():
for k, v in write_set.groups[ApplicationGroup].policies.iteritems():
v.mod_policy=BootstrapHelper.KEY_POLICY_ADMINS
config_update = common_dot_configtx_pb2.ConfigUpdate(channel_id=channel_id,
read_set=read_set,
Expand Down
3 changes: 3 additions & 0 deletions common/configtx/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -229,8 +229,11 @@ func (cct *channelCreationTemplate) Envelope(channelID string) (*cb.ConfigUpdate

wSet.Groups[config.ApplicationGroupKey].ModPolicy = configmsp.AdminsPolicyKey
wSet.Groups[config.ApplicationGroupKey].Policies[configmsp.AdminsPolicyKey] = policies.ImplicitMetaPolicyWithSubPolicy(configmsp.AdminsPolicyKey, cb.ImplicitMetaPolicy_MAJORITY)
wSet.Groups[config.ApplicationGroupKey].Policies[configmsp.AdminsPolicyKey].ModPolicy = configmsp.AdminsPolicyKey
wSet.Groups[config.ApplicationGroupKey].Policies[configmsp.WritersPolicyKey] = policies.ImplicitMetaPolicyWithSubPolicy(configmsp.WritersPolicyKey, cb.ImplicitMetaPolicy_ANY)
wSet.Groups[config.ApplicationGroupKey].Policies[configmsp.WritersPolicyKey].ModPolicy = configmsp.AdminsPolicyKey
wSet.Groups[config.ApplicationGroupKey].Policies[configmsp.ReadersPolicyKey] = policies.ImplicitMetaPolicyWithSubPolicy(configmsp.ReadersPolicyKey, cb.ImplicitMetaPolicy_ANY)
wSet.Groups[config.ApplicationGroupKey].Policies[configmsp.ReadersPolicyKey].ModPolicy = configmsp.AdminsPolicyKey
wSet.Groups[config.ApplicationGroupKey].Version = 1

return &cb.ConfigUpdateEnvelope{
Expand Down
6 changes: 5 additions & 1 deletion common/configtx/template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"testing"

"github.com/hyperledger/fabric/common/config"
configmsp "github.com/hyperledger/fabric/common/config/msp"
mmsp "github.com/hyperledger/fabric/common/mocks/msp"
cb "github.com/hyperledger/fabric/protos/common"
"github.com/hyperledger/fabric/protos/utils"
Expand Down Expand Up @@ -144,8 +145,11 @@ func TestNewChainTemplate(t *testing.T) {
assert.Len(t, configUpdate.WriteSet.Groups[config.ApplicationGroupKey].Groups, len(orgs))

for _, org := range orgs {
_, ok := configUpdate.WriteSet.Groups[config.ApplicationGroupKey].Groups[org]
group, ok := configUpdate.WriteSet.Groups[config.ApplicationGroupKey].Groups[org]
assert.True(t, ok, "Expected to find %s but did not", org)
for _, policy := range group.Policies {
assert.Equal(t, configmsp.AdminsPolicyKey, policy.ModPolicy)
}
}
}

Expand Down
4 changes: 4 additions & 0 deletions common/configtx/tool/configtxgen/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ func doOutputAnchorPeersUpdate(conf *genesisconfig.Profile, channelID string, ou
}

configGroup := config.TemplateAnchorPeers(org.Name, anchorPeers)
configGroup.Groups[config.ApplicationGroupKey].Groups[org.Name].Values[config.AnchorPeersKey].ModPolicy = mspconfig.AdminsPolicyKey
configUpdate := &cb.ConfigUpdate{
ChannelId: channelID,
WriteSet: configGroup,
Expand All @@ -132,6 +133,7 @@ func doOutputAnchorPeersUpdate(conf *genesisconfig.Profile, channelID string, ou
// Add all the existing config to the readset
configUpdate.ReadSet.Groups[config.ApplicationGroupKey] = cb.NewConfigGroup()
configUpdate.ReadSet.Groups[config.ApplicationGroupKey].Version = 1
configUpdate.ReadSet.Groups[config.ApplicationGroupKey].ModPolicy = mspconfig.AdminsPolicyKey
configUpdate.ReadSet.Groups[config.ApplicationGroupKey].Groups[org.Name] = cb.NewConfigGroup()
configUpdate.ReadSet.Groups[config.ApplicationGroupKey].Groups[org.Name].Values[config.MSPKey] = &cb.ConfigValue{}
configUpdate.ReadSet.Groups[config.ApplicationGroupKey].Groups[org.Name].Policies[mspconfig.ReadersPolicyKey] = &cb.ConfigPolicy{}
Expand All @@ -140,7 +142,9 @@ func doOutputAnchorPeersUpdate(conf *genesisconfig.Profile, channelID string, ou

// Add all the existing at the same versions to the writeset
configUpdate.WriteSet.Groups[config.ApplicationGroupKey].Version = 1
configUpdate.WriteSet.Groups[config.ApplicationGroupKey].ModPolicy = mspconfig.AdminsPolicyKey
configUpdate.WriteSet.Groups[config.ApplicationGroupKey].Groups[org.Name].Version = 1
configUpdate.WriteSet.Groups[config.ApplicationGroupKey].Groups[org.Name].ModPolicy = mspconfig.AdminsPolicyKey
configUpdate.WriteSet.Groups[config.ApplicationGroupKey].Groups[org.Name].Values[config.MSPKey] = &cb.ConfigValue{}
configUpdate.WriteSet.Groups[config.ApplicationGroupKey].Groups[org.Name].Policies[mspconfig.ReadersPolicyKey] = &cb.ConfigPolicy{}
configUpdate.WriteSet.Groups[config.ApplicationGroupKey].Groups[org.Name].Policies[mspconfig.WritersPolicyKey] = &cb.ConfigPolicy{}
Expand Down
25 changes: 25 additions & 0 deletions common/configtx/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package configtx

import (
"fmt"
"strings"

"github.com/hyperledger/fabric/common/policies"
cb "github.com/hyperledger/fabric/protos/common"
Expand Down Expand Up @@ -54,12 +55,36 @@ func ComputeDeltaSet(readSet, writeSet map[string]comparable) map[string]compara
return result
}

func validateModPolicy(modPolicy string) error {
if modPolicy == "" {
return fmt.Errorf("mod_policy not set")
}

trimmed := modPolicy
if modPolicy[0] == '/' {
trimmed = modPolicy[1:]
}

for i, pathElement := range strings.Split(trimmed, PathSeparator) {
err := validateConfigID(pathElement)
if err != nil {
return fmt.Errorf("path element at %d is invalid: %s", i, err)
}
}
return nil

}

func (cm *configManager) verifyDeltaSet(deltaSet map[string]comparable, signedData []*cb.SignedData) error {
if len(deltaSet) == 0 {
return fmt.Errorf("Delta set was empty. Update would have no effect.")
}

for key, value := range deltaSet {
if err := validateModPolicy(value.modPolicy()); err != nil {
return fmt.Errorf("invalid mod_policy for element %s: %s", key, err)
}

existing, ok := cm.current.configMap[key]
if !ok {
if value.version() != 0 {
Expand Down
34 changes: 30 additions & 4 deletions common/configtx/update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,31 +89,39 @@ func TestVerifyDeltaSet(t *testing.T) {
t.Run("Green path", func(t *testing.T) {
deltaSet := make(map[string]comparable)

deltaSet["foo"] = comparable{ConfigValue: &cb.ConfigValue{Version: 1}}
deltaSet["foo"] = comparable{ConfigValue: &cb.ConfigValue{Version: 1, ModPolicy: "foo"}}

assert.NoError(t, cm.verifyDeltaSet(deltaSet, nil), "Good update")
})

t.Run("Bad mod policy", func(t *testing.T) {
deltaSet := make(map[string]comparable)

deltaSet["foo"] = comparable{ConfigValue: &cb.ConfigValue{Version: 1}}

assert.Regexp(t, "invalid mod_policy for element", cm.verifyDeltaSet(deltaSet, nil))
})

t.Run("Big Skip", func(t *testing.T) {
deltaSet := make(map[string]comparable)

deltaSet["foo"] = comparable{ConfigValue: &cb.ConfigValue{Version: 2}}
deltaSet["foo"] = comparable{ConfigValue: &cb.ConfigValue{Version: 2, ModPolicy: "foo"}}

assert.Error(t, cm.verifyDeltaSet(deltaSet, nil), "Version skip from 0 to 2")
})

t.Run("New item high version", func(t *testing.T) {
deltaSet := make(map[string]comparable)

deltaSet["bar"] = comparable{ConfigValue: &cb.ConfigValue{Version: 1}}
deltaSet["bar"] = comparable{ConfigValue: &cb.ConfigValue{Version: 1, ModPolicy: "foo"}}

assert.Error(t, cm.verifyDeltaSet(deltaSet, nil), "New key not at version 0")
})

t.Run("Policy evalaution to false", func(t *testing.T) {
deltaSet := make(map[string]comparable)

deltaSet["foo"] = comparable{ConfigValue: &cb.ConfigValue{Version: 1}}
deltaSet["foo"] = comparable{ConfigValue: &cb.ConfigValue{Version: 1, ModPolicy: "foo"}}
cm.Resources.(*mockconfigtx.Resources).PolicyManagerVal.Policy = &mockpolicies.Policy{Err: fmt.Errorf("Err")}

assert.Error(t, cm.verifyDeltaSet(deltaSet, nil), "Policy evaluation should have failed")
Expand Down Expand Up @@ -182,3 +190,21 @@ func TestPolicyForItem(t *testing.T) {
assert.True(t, ok)
assert.Equal(t, policy, fooPolicy, "Should have found relative foo policy for foo group")
}

func TestValidateModPolicy(t *testing.T) {
t.Run("Valid", func(t *testing.T) {
assert.Nil(t, validateModPolicy("/foo/bar"))
})
t.Run("Empty", func(t *testing.T) {
assert.Regexp(t, "mod_policy not set", validateModPolicy(""))
})
t.Run("InvalidFirstChar", func(t *testing.T) {
assert.Regexp(t, "path element at 0 is invalid", validateModPolicy("^foo"))
})
t.Run("InvalidRootPath", func(t *testing.T) {
assert.Regexp(t, "path element at 0 is invalid", validateModPolicy("/"))
})
t.Run("InvalidSubPath", func(t *testing.T) {
assert.Regexp(t, "path element at 1 is invalid", validateModPolicy("foo//bar"))
})
}

0 comments on commit 3a2dd8e

Please sign in to comment.