From 10d340cc88a37c84a06e14d3c84e942d38a7a0eb Mon Sep 17 00:00:00 2001 From: Jason Yellick Date: Mon, 28 Aug 2017 15:57:28 -0400 Subject: [PATCH] [FAB-5944] Restore disabled config update checks In order to refactor the channel config to be immutable, checks which depended upon the previous state had to be removed (in particular, checks for the consensus type changing and for the org's MSPID changing were temporarily removed). This CR adds those checks back in a step which allows the previous config to validate the new one via 'ValidateNew'. Change-Id: Ic6c51f4b09f5aaaa8d176d850728287a1381b5e4 Signed-off-by: Jason Yellick --- common/channelconfig/api.go | 6 + common/channelconfig/bundle.go | 71 +++++++ common/channelconfig/bundle_test.go | 202 ++++++++++++++++++++ common/channelconfig/bundlesource.go | 5 + common/channelconfig/consortium.go | 6 +- common/channelconfig/orderer.go | 6 - common/channelconfig/orderer_test.go | 5 - common/channelconfig/organization.go | 2 - common/mocks/config/resources.go | 8 + core/peer/peer.go | 4 + orderer/common/multichannel/chainsupport.go | 10 +- 11 files changed, 308 insertions(+), 17 deletions(-) create mode 100644 common/channelconfig/bundle_test.go diff --git a/common/channelconfig/api.go b/common/channelconfig/api.go index 3382b00b4ef..d18cea82f2d 100644 --- a/common/channelconfig/api.go +++ b/common/channelconfig/api.go @@ -64,6 +64,9 @@ type Consortiums interface { type Consortium interface { // ChannelCreationPolicy returns the policy to check when instantiating a channel for this consortium ChannelCreationPolicy() *cb.Policy + + // Organizations returns the organizations for this consortium + Organizations() map[string]Org } // Orderer stores the common shared orderer config @@ -116,4 +119,7 @@ type Resources interface { // MSPManager returns the msp.MSPManager for the chain MSPManager() msp.MSPManager + + // ValidateNew should return an error if a new set of configuration resources is incompatible with the current one + ValidateNew(resources Resources) error } diff --git a/common/channelconfig/bundle.go b/common/channelconfig/bundle.go index d6b563c7192..db1c06b44a8 100644 --- a/common/channelconfig/bundle.go +++ b/common/channelconfig/bundle.go @@ -80,6 +80,77 @@ func (b *Bundle) ConfigtxManager() configtxapi.Manager { return b.configtxManager } +// ValidateNew checks if a new bundle's contained configuration is valid to be derived from the current bundle. +// This allows checks of the nature "Make sure that the consensus type did not change." which is otherwise +func (b *Bundle) ValidateNew(nb Resources) error { + if oc, ok := b.OrdererConfig(); ok { + noc, ok := nb.OrdererConfig() + if !ok { + return fmt.Errorf("Current config has orderer section, but new config does not") + } + + if oc.ConsensusType() != noc.ConsensusType() { + return fmt.Errorf("Attempted to change consensus type from %s to %s", oc.ConsensusType(), noc.ConsensusType()) + } + + for orgName, org := range oc.Organizations() { + norg, ok := noc.Organizations()[orgName] + if !ok { + continue + } + mspID := org.MSPID() + if mspID != norg.MSPID() { + return fmt.Errorf("Orderer org %s attempted to change MSP ID from %s to %s", orgName, mspID, norg.MSPID()) + } + } + } + + if ac, ok := b.ApplicationConfig(); ok { + nac, ok := nb.ApplicationConfig() + if !ok { + return fmt.Errorf("Current config has consortiums section, but new config does not") + } + + for orgName, org := range ac.Organizations() { + norg, ok := nac.Organizations()[orgName] + if !ok { + continue + } + mspID := org.MSPID() + if mspID != norg.MSPID() { + return fmt.Errorf("Application org %s attempted to change MSP ID from %s to %s", orgName, mspID, norg.MSPID()) + } + } + } + + if cc, ok := b.ConsortiumsConfig(); ok { + ncc, ok := nb.ConsortiumsConfig() + if !ok { + return fmt.Errorf("Current config has consortiums section, but new config does not") + } + + for consortiumName, consortium := range cc.Consortiums() { + nconsortium, ok := ncc.Consortiums()[consortiumName] + if !ok { + continue + } + + for orgName, org := range consortium.Organizations() { + norg, ok := nconsortium.Organizations()[orgName] + if !ok { + continue + } + mspID := org.MSPID() + if mspID != norg.MSPID() { + return fmt.Errorf("Consortium %s org %s attempted to change MSP ID from %s to %s", consortiumName, orgName, mspID, norg.MSPID()) + } + } + } + } + + return nil +} + type simpleProposer struct { policyManager policies.Manager } diff --git a/common/channelconfig/bundle_test.go b/common/channelconfig/bundle_test.go new file mode 100644 index 00000000000..b8064e4f3cb --- /dev/null +++ b/common/channelconfig/bundle_test.go @@ -0,0 +1,202 @@ +/* +Copyright IBM Corp. All Rights Reserved. + +SPDX-License-Identifier: Apache-2.0 +*/ + +package channelconfig + +import ( + "testing" + + ab "github.com/hyperledger/fabric/protos/orderer" + + "github.com/stretchr/testify/assert" +) + +func TestValidateNew(t *testing.T) { + t.Run("DisappearingOrdererConfig", func(t *testing.T) { + cb := &Bundle{ + channelConfig: &ChannelConfig{ + ordererConfig: &OrdererConfig{}, + }, + } + + nb := &Bundle{ + channelConfig: &ChannelConfig{}, + } + + err := cb.ValidateNew(nb) + assert.Error(t, err) + assert.Regexp(t, "Current config has orderer section, but new config does not", err.Error()) + }) + + t.Run("DisappearingApplicationConfig", func(t *testing.T) { + cb := &Bundle{ + channelConfig: &ChannelConfig{ + appConfig: &ApplicationConfig{}, + }, + } + + nb := &Bundle{ + channelConfig: &ChannelConfig{}, + } + + err := cb.ValidateNew(nb) + assert.Error(t, err) + assert.Regexp(t, "Current config has consortiums section, but new config does not", err.Error()) + }) + + t.Run("DisappearingConsortiumsConfig", func(t *testing.T) { + cb := &Bundle{ + channelConfig: &ChannelConfig{ + consortiumsConfig: &ConsortiumsConfig{}, + }, + } + + nb := &Bundle{ + channelConfig: &ChannelConfig{}, + } + + err := cb.ValidateNew(nb) + assert.Error(t, err) + assert.Regexp(t, "Current config has consortiums section, but new config does not", err.Error()) + }) + + t.Run("ConsensusTypeChange", func(t *testing.T) { + cb := &Bundle{ + channelConfig: &ChannelConfig{ + ordererConfig: &OrdererConfig{ + protos: &OrdererProtos{ + ConsensusType: &ab.ConsensusType{ + Type: "type1", + }, + }, + }, + }, + } + + nb := &Bundle{ + channelConfig: &ChannelConfig{ + ordererConfig: &OrdererConfig{ + protos: &OrdererProtos{ + ConsensusType: &ab.ConsensusType{ + Type: "type2", + }, + }, + }, + }, + } + + err := cb.ValidateNew(nb) + assert.Error(t, err) + assert.Regexp(t, "Attempted to change consensus type from", err.Error()) + }) + + t.Run("OrdererOrgMSPIDChange", func(t *testing.T) { + cb := &Bundle{ + channelConfig: &ChannelConfig{ + ordererConfig: &OrdererConfig{ + protos: &OrdererProtos{ + ConsensusType: &ab.ConsensusType{ + Type: "type1", + }, + }, + orgs: map[string]Org{ + "org1": &OrganizationConfig{mspID: "org1msp"}, + "org2": &OrganizationConfig{mspID: "org2msp"}, + "org3": &OrganizationConfig{mspID: "org3msp"}, + }, + }, + }, + } + + nb := &Bundle{ + channelConfig: &ChannelConfig{ + ordererConfig: &OrdererConfig{ + protos: &OrdererProtos{ + ConsensusType: &ab.ConsensusType{ + Type: "type1", + }, + }, + orgs: map[string]Org{ + "org1": &OrganizationConfig{mspID: "org1msp"}, + "org3": &OrganizationConfig{mspID: "org2msp"}, + }, + }, + }, + } + + err := cb.ValidateNew(nb) + assert.Error(t, err) + assert.Regexp(t, "Orderer org org3 attempted to change MSP ID from", err.Error()) + }) + + t.Run("ApplicationOrgMSPIDChange", func(t *testing.T) { + cb := &Bundle{ + channelConfig: &ChannelConfig{ + appConfig: &ApplicationConfig{ + applicationOrgs: map[string]ApplicationOrg{ + "org1": &ApplicationOrgConfig{OrganizationConfig: &OrganizationConfig{mspID: "org1msp"}}, + "org2": &ApplicationOrgConfig{OrganizationConfig: &OrganizationConfig{mspID: "org2msp"}}, + "org3": &ApplicationOrgConfig{OrganizationConfig: &OrganizationConfig{mspID: "org3msp"}}, + }, + }, + }, + } + + nb := &Bundle{ + channelConfig: &ChannelConfig{ + appConfig: &ApplicationConfig{ + applicationOrgs: map[string]ApplicationOrg{ + "org1": &ApplicationOrgConfig{OrganizationConfig: &OrganizationConfig{mspID: "org1msp"}}, + "org3": &ApplicationOrgConfig{OrganizationConfig: &OrganizationConfig{mspID: "org2msp"}}, + }, + }, + }, + } + + err := cb.ValidateNew(nb) + assert.Error(t, err) + assert.Regexp(t, "Application org org3 attempted to change MSP ID from", err.Error()) + }) + + t.Run("ConsortiumOrgMSPIDChange", func(t *testing.T) { + cb := &Bundle{ + channelConfig: &ChannelConfig{ + consortiumsConfig: &ConsortiumsConfig{ + consortiums: map[string]Consortium{ + "consortium1": &ConsortiumConfig{ + orgs: map[string]Org{ + "org1": &OrganizationConfig{mspID: "org1msp"}, + "org2": &OrganizationConfig{mspID: "org2msp"}, + "org3": &OrganizationConfig{mspID: "org3msp"}, + }, + }, + "consortium2": &ConsortiumConfig{}, + "consortium3": &ConsortiumConfig{}, + }, + }, + }, + } + + nb := &Bundle{ + channelConfig: &ChannelConfig{ + consortiumsConfig: &ConsortiumsConfig{ + consortiums: map[string]Consortium{ + "consortium1": &ConsortiumConfig{ + orgs: map[string]Org{ + "org1": &OrganizationConfig{mspID: "org1msp"}, + "org3": &OrganizationConfig{mspID: "org2msp"}, + }, + }, + }, + }, + }, + } + + err := cb.ValidateNew(nb) + assert.Error(t, err) + assert.Regexp(t, "Consortium consortium1 org org3 attempted to change MSP ID from", err.Error()) + }) +} diff --git a/common/channelconfig/bundlesource.go b/common/channelconfig/bundlesource.go index 2782a97dc1f..c2b6dff2e05 100644 --- a/common/channelconfig/bundlesource.go +++ b/common/channelconfig/bundlesource.go @@ -93,3 +93,8 @@ func (bs *BundleSource) ApplicationConfig() (Application, bool) { func (bs *BundleSource) ConfigtxManager() configtxapi.Manager { return bs.StableBundle().ConfigtxManager() } + +// ValidateNew passes through to the current bundle +func (bs *BundleSource) ValidateNew(resources Resources) error { + return bs.StableBundle().ValidateNew(resources) +} diff --git a/common/channelconfig/consortium.go b/common/channelconfig/consortium.go index 901e8ac32fc..6a130b75771 100644 --- a/common/channelconfig/consortium.go +++ b/common/channelconfig/consortium.go @@ -20,14 +20,14 @@ type ConsortiumProtos struct { // ConsortiumConfig holds the consoritums configuration information type ConsortiumConfig struct { protos *ConsortiumProtos - orgs map[string]*OrganizationConfig + orgs map[string]Org } // NewConsortiumConfig creates a new instance of the consoritums config func NewConsortiumConfig(consortiumGroup *cb.ConfigGroup, mspConfig *MSPConfigHandler) (*ConsortiumConfig, error) { cc := &ConsortiumConfig{ protos: &ConsortiumProtos{}, - orgs: make(map[string]*OrganizationConfig), + orgs: make(map[string]Org), } if err := DeserializeProtoValuesFromGroup(consortiumGroup, cc.protos); err != nil { @@ -45,7 +45,7 @@ func NewConsortiumConfig(consortiumGroup *cb.ConfigGroup, mspConfig *MSPConfigHa } // Organizations returns the set of organizations in the consortium -func (cc *ConsortiumConfig) Organizations() map[string]*OrganizationConfig { +func (cc *ConsortiumConfig) Organizations() map[string]Org { return cc.orgs } diff --git a/common/channelconfig/orderer.go b/common/channelconfig/orderer.go index f646a0bcf23..afb62d96fe7 100644 --- a/common/channelconfig/orderer.go +++ b/common/channelconfig/orderer.go @@ -116,7 +116,6 @@ func (oc *OrdererConfig) Organizations() map[string]Org { func (oc *OrdererConfig) Validate() error { for _, validator := range []func() error{ - oc.validateConsensusType, oc.validateBatchSize, oc.validateBatchTimeout, oc.validateKafkaBrokers, @@ -129,11 +128,6 @@ func (oc *OrdererConfig) Validate() error { return nil } -func (oc *OrdererConfig) validateConsensusType() error { - // XXX TODO add check to prevent changing consensus type back - return nil -} - func (oc *OrdererConfig) validateBatchSize() error { if oc.protos.BatchSize.MaxMessageCount == 0 { return fmt.Errorf("Attempted to set the batch size max message count to an invalid value: 0") diff --git a/common/channelconfig/orderer_test.go b/common/channelconfig/orderer_test.go index 0123ae44629..743e0efa4d6 100644 --- a/common/channelconfig/orderer_test.go +++ b/common/channelconfig/orderer_test.go @@ -19,11 +19,6 @@ func init() { logging.SetLevel(logging.DEBUG, "") } -func TestConsensusType(t *testing.T) { - oc := &OrdererConfig{protos: &OrdererProtos{ConsensusType: &ab.ConsensusType{Type: "foo"}}} - assert.NoError(t, oc.validateConsensusType(), "Should have validly set new consensus type") -} - func TestBatchSize(t *testing.T) { validMaxMessageCount := uint32(10) validAbsoluteMaxBytes := uint32(1000) diff --git a/common/channelconfig/organization.go b/common/channelconfig/organization.go index f6582618dd9..ef2426da0c7 100644 --- a/common/channelconfig/organization.go +++ b/common/channelconfig/organization.go @@ -84,7 +84,5 @@ func (oc *OrganizationConfig) validateMSP() error { return fmt.Errorf("MSP for org %s has empty MSP ID", oc.name) } - // XXX TODO Add back a check for MSP ID modification - return nil } diff --git a/common/mocks/config/resources.go b/common/mocks/config/resources.go index 5b358a05eff..fc3f3e26b8d 100644 --- a/common/mocks/config/resources.go +++ b/common/mocks/config/resources.go @@ -34,6 +34,9 @@ type Resources struct { // MSPManagerVal is returned as the result of MSPManager() MSPManagerVal msp.MSPManager + + // ValidateNewErr is returned as the result of ValidateNew + ValidateNewErr error } // ConfigtxMangaer returns ConfigtxManagerVal @@ -69,3 +72,8 @@ func (r *Resources) ConsortiumsConfig() (channelconfig.Consortiums, bool) { func (r *Resources) MSPManager() msp.MSPManager { return r.MSPManagerVal } + +// ValidateNew returns ValidateNewErr +func (r *Resources) ValidateNew(res channelconfig.Resources) error { + return r.ValidateNewErr +} diff --git a/core/peer/peer.go b/core/peer/peer.go index 8040ecc94a0..34900f62cee 100644 --- a/core/peer/peer.go +++ b/core/peer/peer.go @@ -84,6 +84,10 @@ func (cs *chainSupport) Apply(configtx *common.ConfigEnvelope) error { if err != nil { return err } + err = cs.bundleSource.ValidateNew(bundle) + if err != nil { + return err + } cs.bundleSource.Update(bundle) } return nil diff --git a/orderer/common/multichannel/chainsupport.go b/orderer/common/multichannel/chainsupport.go index 16881dc89b7..591c6f4d739 100644 --- a/orderer/common/multichannel/chainsupport.go +++ b/orderer/common/multichannel/chainsupport.go @@ -97,7 +97,15 @@ func (cs *ChainSupport) Validate(configEnv *cb.ConfigEnvelope) error { // ProposeConfigUpdate passes through to the underlying configtxapi.Manager func (cs *ChainSupport) ProposeConfigUpdate(configtx *cb.Envelope) (*cb.ConfigEnvelope, error) { - return cs.ConfigtxManager().ProposeConfigUpdate(configtx) + env, err := cs.ConfigtxManager().ProposeConfigUpdate(configtx) + if err != nil { + return nil, err + } + bundle, err := cs.CreateBundle(cs.ChainID(), env.Config) + if err != nil { + return nil, err + } + return env, cs.ValidateNew(bundle) } // ChainID passes through to the underlying configtxapi.Manager