diff --git a/common/configtx/manager.go b/common/configtx/manager.go index 4783ac6d5d5..a3a40f780c5 100644 --- a/common/configtx/manager.go +++ b/common/configtx/manager.go @@ -17,8 +17,8 @@ limitations under the License. package configtx import ( - "bytes" "fmt" + "reflect" "github.com/hyperledger/fabric/common/policies" cb "github.com/hyperledger/fabric/protos/common" @@ -205,33 +205,12 @@ func (cm *configurationManager) processConfig(configtx *cb.ConfigurationEnvelope return nil, fmt.Errorf("Error unmarshaling ConfigurationItem: %s", err) } - // Get the modification policy for this config item if one was previously specified - // or the default if this is a new config item - var policy policies.Policy - oldItem, ok := cm.configuration[config.Type][config.Key] - if ok { - policy, _ = cm.pm.GetPolicy(oldItem.ModificationPolicy) - } else { - policy = defaultModificationPolicy - } - - // Get signatures - signedData, err := entry.AsSignedData() - if err != nil { - return nil, err - } - - // Ensure the policy is satisfied - if err = policy.Evaluate(signedData); err != nil { - return nil, err - } - // Ensure the config sequence numbers are correct to prevent replay attacks isModified := false if val, ok := cm.configuration[config.Type][config.Key]; ok { - // Config was modified if the LastModified or the Data contents changed - isModified = (val.LastModified != config.LastModified) || !bytes.Equal(config.Value, val.Value) + // Config was modified if any of the contents changed + isModified = !reflect.DeepEqual(val, config) } else { if config.LastModified != seq { return nil, fmt.Errorf("Key %v for type %v was new, but had an older Sequence %d set", config.Key, config.Type, config.LastModified) @@ -239,11 +218,32 @@ func (cm *configurationManager) processConfig(configtx *cb.ConfigurationEnvelope isModified = true } - // If a config item was modified, its LastModified must be set correctly + // If a config item was modified, its LastModified must be set correctly, and it must satisfy the modification policy if isModified { if config.LastModified != seq { return nil, fmt.Errorf("Key %v for type %v was modified, but its LastModified %d does not equal current configtx Sequence %d", config.Key, config.Type, config.LastModified, seq) } + + // Get the modification policy for this config item if one was previously specified + // or the default if this is a new config item + var policy policies.Policy + oldItem, ok := cm.configuration[config.Type][config.Key] + if ok { + policy, _ = cm.pm.GetPolicy(oldItem.ModificationPolicy) + } else { + policy = defaultModificationPolicy + } + + // Get signatures + signedData, err := entry.AsSignedData() + if err != nil { + return nil, err + } + + // Ensure the policy is satisfied + if err = policy.Evaluate(signedData); err != nil { + return nil, err + } } // Ensure the type handler agrees the config is well formed diff --git a/common/configtx/manager_test.go b/common/configtx/manager_test.go index 26cc3afe218..93dd21f2672 100644 --- a/common/configtx/manager_test.go +++ b/common/configtx/manager_test.go @@ -23,8 +23,8 @@ import ( "github.com/hyperledger/fabric/common/policies" cb "github.com/hyperledger/fabric/protos/common" - "github.com/golang/protobuf/proto" "errors" + "github.com/golang/protobuf/proto" ) var defaultChain = "DefaultChainID" @@ -40,12 +40,19 @@ func defaultHandlers() map[cb.ConfigurationItem_ConfigurationType]Handler { // mockPolicy always returns the error set as policyResult type mockPolicy struct { policyResult error + // validReplies is the number of times to successfully validate before returning policyResult + // it is decremented at each invocation + validReplies int } func (mp *mockPolicy) Evaluate(signedData []*cb.SignedData) error { if mp == nil { return errors.New("Invoked nil policy") } + if mp.validReplies > 0 { + return nil + } + mp.validReplies-- return mp.policyResult } @@ -328,7 +335,7 @@ func TestSilentConfigModification(t *testing.T) { func TestInvalidInitialConfigByPolicy(t *testing.T) { _, err := NewConfigurationManager(&cb.ConfigurationEnvelope{ Items: []*cb.SignedConfigurationItem{makeSignedConfigurationItem("foo", "foo", 0, []byte("foo"), defaultChain)}, - }, &mockPolicyManager{&mockPolicy{fmt.Errorf("err")}}, defaultHandlers()) + }, &mockPolicyManager{&mockPolicy{policyResult: fmt.Errorf("err")}}, defaultHandlers()) // mockPolicyManager will return non-validating defualt policy if err == nil { @@ -348,7 +355,7 @@ func TestConfigChangeViolatesPolicy(t *testing.T) { t.Fatalf("Error constructing configuration manager: %s", err) } // Set the mock policy to error - mpm.policy = &mockPolicy{fmt.Errorf("err")} + mpm.policy = &mockPolicy{policyResult: fmt.Errorf("err")} newConfig := &cb.ConfigurationEnvelope{ Items: []*cb.SignedConfigurationItem{makeSignedConfigurationItem("foo", "foo", 1, []byte("foo"), defaultChain)}, @@ -365,6 +372,41 @@ func TestConfigChangeViolatesPolicy(t *testing.T) { } } +// TestUnchangedConfigViolatesPolicy checks to make sure that existing config items are not revalidated against their modification policies +// as the policy may have changed, certs revoked, etc. since the config was adopted. +func TestUnchangedConfigViolatesPolicy(t *testing.T) { + mpm := &mockPolicyManager{} + cm, err := NewConfigurationManager(&cb.ConfigurationEnvelope{ + Items: []*cb.SignedConfigurationItem{makeSignedConfigurationItem("foo", "foo", 0, []byte("foo"), defaultChain)}, + }, mpm, defaultHandlers()) + + if err != nil { + t.Fatalf("Error constructing configuration manager: %s", err) + } + // Set the mock policy to error + mpm.policy = &mockPolicy{ + policyResult: fmt.Errorf("err"), + validReplies: 1, + } + + newConfig := &cb.ConfigurationEnvelope{ + Items: []*cb.SignedConfigurationItem{ + makeSignedConfigurationItem("foo", "foo", 0, []byte("foo"), defaultChain), + makeSignedConfigurationItem("bar", "bar", 1, []byte("foo"), defaultChain), + }, + } + + err = cm.Validate(newConfig) + if err != nil { + t.Errorf("Should not have errored validating config, but got %s", err) + } + + err = cm.Apply(newConfig) + if err != nil { + t.Errorf("Should not have errored applying config, but got %s", err) + } +} + type failHandler struct{} func (fh failHandler) BeginConfig() {}