Add a Chain Configuration Manager

In order to specify genesis configuration, it must be encoded in the
block in a consumable way.  This configuration must then be stored
in a way accessible to any other components which wish to consume this
configuration.  Further, when new configuration is proposed it must be
validated, and applied or rejected depending on its validity.

This changeset creates a configuration manager which handles all these
tasks.  It allows for registering arbitrary configuration handlers for
different configuration types, and has been designed in concert with the
policy manager to allow the policy manager to be plugged in as a
configuration handler so that policy both enforces what configuration
modifications are allowed and is itself a type of configuration.

Because of the complicated nature of verifying that a configuration is
valid (well formed, not forged or replayed, conforms to existing policy,
and satisfies the assorted handlers) this changeset is unfortunately
large, but is needed to achieve complete code and test coverage of
invalid configurations.

This resolves:

https://jira.hyperledger.org/browse/FAB-724

Change-Id: I44cbc3f2e5850d8c22977c923c044d353a9bc453
Signed-off-by: Jason Yellick <jyellick@us.ibm.com>

Author: Jason Yellick

orderer/common/configtx/bytes_handler.go

@@ -0,0 +1,67 @@
+/*
+Copyright IBM Corp. 2016 All Rights Reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+                 http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package configtx
+
+import (
+	ab "github.com/hyperledger/fabric/orderer/atomicbroadcast"
+)
+
+// BytesHandler is a trivial ConfigHandler which simpy tracks the bytes stores in a config
+type BytesHandler struct {
+	config   map[string][]byte
+	proposed map[string][]byte
+}
+
+// NewBytesHandler creates a new BytesHandler
+func NewBytesHandler() *BytesHandler {
+	return &BytesHandler{
+		config: make(map[string][]byte),
+	}
+}
+
+// BeginConfig called when a config proposal is begun
+func (bh *BytesHandler) BeginConfig() {
+	if bh.proposed != nil {
+		panic("Programming error, called BeginConfig while a proposal was in process")
+	}
+	bh.proposed = make(map[string][]byte)
+}
+
+// RollbackConfig called when a config proposal is abandoned
+func (bh *BytesHandler) RollbackConfig() {
+	bh.proposed = nil
+}
+
+// CommitConfig called when a config proposal is committed
+func (bh *BytesHandler) CommitConfig() {
+	if bh.proposed == nil {
+		panic("Programming error, called CommitConfig with no proposal in process")
+	}
+	bh.config = bh.proposed
+	bh.proposed = nil
+}
+
+// ProposeConfig called when config is added to a proposal
+func (bh *BytesHandler) ProposeConfig(configItem *ab.Configuration) error {
+	bh.proposed[configItem.ID] = configItem.Data
+	return nil
+}
+
+// GetBytes allows the caller to retrieve the bytes for a config
+func (bh *BytesHandler) GetBytes(id string) []byte {
+	return bh.config[id]
+}

orderer/common/configtx/configtx.go

@@ -0,0 +1,235 @@
+/*
+Copyright IBM Corp. 2016 All Rights Reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+                 http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package configtx
+
+import (
+	"bytes"
+	"fmt"
+
+	ab "github.com/hyperledger/fabric/orderer/atomicbroadcast"
+	"github.com/hyperledger/fabric/orderer/common/policies"
+
+	"github.com/golang/protobuf/proto"
+)
+
+// Handler provides a hook which allows other pieces of code to participate in config proposals
+type Handler interface {
+	// BeginConfig called when a config proposal is begun
+	BeginConfig()
+
+	// RollbackConfig called when a config proposal is abandoned
+	RollbackConfig()
+
+	// CommitConfig called when a config proposal is committed
+	CommitConfig()
+
+	// ProposeConfig called when config is added to a proposal
+	ProposeConfig(configItem *ab.Configuration) error
+}
+
+// Manager provides a mechanism to query and update configuration
+type Manager interface {
+	// Apply attempts to apply a configtx to become the new configuration
+	Apply(configtx *ab.ConfigurationEnvelope) error
+
+	// Validate attempts to validate a new configtx against the current config state
+	Validate(configtx *ab.ConfigurationEnvelope) error
+}
+
+// DefaultModificationPolicyID is the ID of the policy used when no other policy can be resolved, for instance when attempting to create a new config item
+const DefaultModificationPolicyID = "DefaultModificationPolicy"
+
+type acceptAllPolicy struct{}
+
+func (ap *acceptAllPolicy) Evaluate(msg []byte, sigs []*ab.SignedData) error {
+	return nil
+}
+
+type configurationManager struct {
+	sequence      uint64
+	chainID       []byte
+	pm            policies.Manager
+	configuration map[ab.Configuration_ConfigurationType]map[string]*ab.Configuration
+	handlers      map[ab.Configuration_ConfigurationType]Handler
+}
+
+// NewConfigurationManager creates a new Manager unless an error is encountered
+func NewConfigurationManager(configtx *ab.ConfigurationEnvelope, pm policies.Manager, handlers map[ab.Configuration_ConfigurationType]Handler) (Manager, error) {
+	for ctype := range ab.Configuration_ConfigurationType_name {
+		if _, ok := handlers[ab.Configuration_ConfigurationType(ctype)]; !ok {
+			return nil, fmt.Errorf("Must supply a handler for all known types")
+		}
+	}
+
+	cm := &configurationManager{
+		sequence:      configtx.Sequence - 1,
+		chainID:       configtx.ChainID,
+		pm:            pm,
+		handlers:      handlers,
+		configuration: makeConfigMap(),
+	}
+
+	err := cm.Apply(configtx)
+
+	if err != nil {
+		return nil, err
+	}
+
+	return cm, nil
+}
+
+func makeConfigMap() map[ab.Configuration_ConfigurationType]map[string]*ab.Configuration {
+	configMap := make(map[ab.Configuration_ConfigurationType]map[string]*ab.Configuration)
+	for ctype := range ab.Configuration_ConfigurationType_name {
+		configMap[ab.Configuration_ConfigurationType(ctype)] = make(map[string]*ab.Configuration)
+	}
+	return configMap
+}
+
+func (cm *configurationManager) beginHandlers() {
+	for ctype := range ab.Configuration_ConfigurationType_name {
+		cm.handlers[ab.Configuration_ConfigurationType(ctype)].BeginConfig()
+	}
+}
+
+func (cm *configurationManager) rollbackHandlers() {
+	for ctype := range ab.Configuration_ConfigurationType_name {
+		cm.handlers[ab.Configuration_ConfigurationType(ctype)].RollbackConfig()
+	}
+}
+
+func (cm *configurationManager) commitHandlers() {
+	for ctype := range ab.Configuration_ConfigurationType_name {
+		cm.handlers[ab.Configuration_ConfigurationType(ctype)].CommitConfig()
+	}
+}
+
+func (cm *configurationManager) processConfig(configtx *ab.ConfigurationEnvelope) (configMap map[ab.Configuration_ConfigurationType]map[string]*ab.Configuration, err error) {
+	// Verify config is a sequential update to prevent exhausting sequence numbers
+	if configtx.Sequence != cm.sequence+1 {
+		return nil, fmt.Errorf("Config sequence number jumped from %d to %d", cm.sequence, configtx.Sequence)
+	}
+
+	// Verify config is intended for this globally unique chain ID
+	if !bytes.Equal(configtx.ChainID, cm.chainID) {
+		return nil, fmt.Errorf("Config is for the wrong chain, expected %x, got %x", cm.chainID, configtx.ChainID)
+	}
+
+	defaultModificationPolicy, defaultPolicySet := cm.pm.GetPolicy(DefaultModificationPolicyID)
+
+	// If the default modification policy is not set, it indicates this is an uninitialized chain, so be permissive of modification
+	if !defaultPolicySet {
+		defaultModificationPolicy = &acceptAllPolicy{}
+	}
+
+	configMap = makeConfigMap()
+
+	for _, entry := range configtx.Entries {
+		// Verify every entry is well formed
+		config := &ab.Configuration{}
+		err = proto.Unmarshal(entry.Configuration, config)
+		if err != nil {
+			return nil, err
+		}
+
+		// Ensure this configuration was intended for this chain
+		if !bytes.Equal(config.ChainID, cm.chainID) {
+			return nil, fmt.Errorf("Config item %v for type %v was not meant for a different chain %x", config.ID, config.Type, config.ChainID)
+		}
+
+		// 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.ID]
+		if ok {
+			policy, _ = cm.pm.GetPolicy(oldItem.ModificationPolicy)
+		} else {
+			policy = defaultModificationPolicy
+		}
+
+		// Ensure the policy is satisfied
+		if err = policy.Evaluate(entry.Configuration, entry.Signatures); 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.ID]; ok {
+			// Config was modified if the LastModified or the Data contents changed
+			isModified = (val.LastModified != config.LastModified) || !bytes.Equal(config.Data, val.Data)
+		} else {
+			if config.LastModified != configtx.Sequence {
+				return nil, fmt.Errorf("Key %v for type %v was new, but had an older Sequence %d set", config.ID, config.Type, config.LastModified)
+			}
+			isModified = true
+		}
+
+		// If a config item was modified, its LastModified must be set correctly
+		if isModified {
+			if config.LastModified != configtx.Sequence {
+				return nil, fmt.Errorf("Key %v for type %v was modified, but its LastModified %d does not equal current configtx Sequence %d", config.ID, config.Type, config.LastModified, configtx.Sequence)
+			}
+		}
+
+		// Ensure the type handler agrees the config is well formed
+		err = cm.handlers[config.Type].ProposeConfig(config)
+		if err != nil {
+			return nil, err
+		}
+
+		configMap[config.Type][config.ID] = config
+	}
+
+	// Ensure that any config items which used to exist still exist, to prevent implicit deletion
+	for ctype := range ab.Configuration_ConfigurationType_name {
+		curMap := cm.configuration[ab.Configuration_ConfigurationType(ctype)]
+		newMap := configMap[ab.Configuration_ConfigurationType(ctype)]
+		for id := range curMap {
+			_, ok := newMap[id]
+			if !ok {
+				return nil, fmt.Errorf("Missing key %v for type %v in new configuration", id, ctype)
+			}
+
+		}
+	}
+
+	return configMap, nil
+
+}
+
+// Validate attempts to validate a new configtx against the current config state
+func (cm *configurationManager) Validate(configtx *ab.ConfigurationEnvelope) error {
+	cm.beginHandlers()
+	_, err := cm.processConfig(configtx)
+	cm.rollbackHandlers()
+	return err
+}
+
+// Apply attempts to apply a configtx to become the new configuration
+func (cm *configurationManager) Apply(configtx *ab.ConfigurationEnvelope) error {
+	cm.beginHandlers()
+	configMap, err := cm.processConfig(configtx)
+	if err != nil {
+		cm.rollbackHandlers()
+		return err
+	}
+	cm.configuration = configMap
+	cm.sequence = configtx.Sequence
+	cm.commitHandlers()
+	return nil
+}

orderer/common/configtx/configtx_test.go

@@ -0,0 +1,499 @@
+/*
+Copyright IBM Corp. 2016 All Rights Reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+                 http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package configtx
+
+import (
+	"fmt"
+	"testing"
+
+	ab "github.com/hyperledger/fabric/orderer/atomicbroadcast"
+	"github.com/hyperledger/fabric/orderer/common/policies"
+
+	"github.com/golang/protobuf/proto"
+)
+
+var defaultChain = []byte("DefaultChainID")
+
+func defaultHandlers() map[ab.Configuration_ConfigurationType]Handler {
+	handlers := make(map[ab.Configuration_ConfigurationType]Handler)
+	for ctype := range ab.Configuration_ConfigurationType_name {
+		handlers[ab.Configuration_ConfigurationType(ctype)] = NewBytesHandler()
+	}
+	return handlers
+}
+
+// mockPolicy always returns the error set as policyResult
+type mockPolicy struct {
+	policyResult error
+}
+
+func (mp *mockPolicy) Evaluate(msg []byte, sigs []*ab.SignedData) error {
+	if mp == nil {
+		return fmt.Errorf("Invoked nil policy")
+	}
+	return mp.policyResult
+}
+
+// mockPolicyManager always returns the policy set as policy, note that if unset, the default policy always returns error when evaluated
+type mockPolicyManager struct {
+	policy *mockPolicy
+}
+
+func (mpm *mockPolicyManager) GetPolicy(id string) (policies.Policy, bool) {
+	return mpm.policy, (mpm.policy != nil)
+}
+
+func makeConfiguration(id, modificationPolicy string, lastModified uint64, data []byte) *ab.Configuration {
+	return &ab.Configuration{
+		ChainID:            defaultChain,
+		ID:                 id,
+		Data:               data,
+		ModificationPolicy: modificationPolicy,
+		LastModified:       lastModified,
+	}
+}
+
+func makeConfigurationEntry(id, modificationPolicy string, lastModified uint64, data []byte) *ab.ConfigurationEntry {
+	config := makeConfiguration(id, modificationPolicy, lastModified, data)
+	marshaledConfig, err := proto.Marshal(config)
+	if err != nil {
+		panic(err)
+	}
+	return &ab.ConfigurationEntry{
+		Configuration: marshaledConfig,
+	}
+}
+
+// TestOmittedHandler tests that startup fails if not all configuration types have an associated handler
+func TestOmittedHandler(t *testing.T) {
+	_, err := NewConfigurationManager(&ab.ConfigurationEnvelope{
+		Sequence: 0,
+		ChainID:  defaultChain,
+	}, &mockPolicyManager{&mockPolicy{}}, map[ab.Configuration_ConfigurationType]Handler{})
+
+	if err == nil {
+		t.Fatalf("Should have failed to construct manager because handlers were missing")
+	}
+}
+
+// TestWrongChainID tests that a configuration update for a different chain ID fails
+func TestWrongChainID(t *testing.T) {
+	cm, err := NewConfigurationManager(&ab.ConfigurationEnvelope{
+		Sequence: 0,
+		ChainID:  defaultChain,
+	}, &mockPolicyManager{&mockPolicy{}}, defaultHandlers())
+
+	if err != nil {
+		t.Fatalf("Error constructing configuration manager: %s", err)
+	}
+
+	newConfig := &ab.ConfigurationEnvelope{
+		Sequence: 1,
+		ChainID:  []byte("wrongChain"),
+	}
+
+	err = cm.Validate(newConfig)
+	if err == nil {
+		t.Errorf("Should have errored when validating a new configuration set the wrong chain ID")
+	}
+
+	err = cm.Apply(newConfig)
+	if err == nil {
+		t.Errorf("Should have errored when applying a new configuration with the wrong chain ID")
+	}
+}
+
+// TestOldConfigReplay tests that resubmitting a config for a sequence number which is not newer is ignored
+func TestOldConfigReplay(t *testing.T) {
+	cm, err := NewConfigurationManager(&ab.ConfigurationEnvelope{
+		Sequence: 0,
+		ChainID:  defaultChain,
+	}, &mockPolicyManager{&mockPolicy{}}, defaultHandlers())
+
+	if err != nil {
+		t.Fatalf("Error constructing configuration manager: %s", err)
+	}
+
+	newConfig := &ab.ConfigurationEnvelope{
+		Sequence: 0,
+		ChainID:  defaultChain,
+	}
+
+	err = cm.Validate(newConfig)
+	if err == nil {
+		t.Errorf("Should have errored when validating a configuration that is not a newer sequence number")
+	}
+
+	err = cm.Apply(newConfig)
+	if err == nil {
+		t.Errorf("Should have errored when applying a configuration that is not a newer sequence number")
+	}
+}
+
+// TestInvalidInitialConfigByStructure tests to make sure that if the config contains corrupted configuration that construction results in error
+func TestInvalidInitialConfigByStructure(t *testing.T) {
+	entries := []*ab.ConfigurationEntry{makeConfigurationEntry("foo", "foo", 0, []byte("foo"))}
+	entries[0].Configuration = []byte("Corrupted")
+	_, err := NewConfigurationManager(&ab.ConfigurationEnvelope{
+		Sequence: 0,
+		ChainID:  defaultChain,
+		Entries:  entries,
+	}, &mockPolicyManager{&mockPolicy{}}, defaultHandlers())
+
+	if err == nil {
+		t.Fatalf("Should have failed to construct configuration by policy")
+	}
+}
+
+// TestValidConfigChange tests the happy path of updating a configuration value with no defaultModificationPolicy
+func TestValidConfigChange(t *testing.T) {
+	cm, err := NewConfigurationManager(&ab.ConfigurationEnvelope{
+		Sequence: 0,
+		ChainID:  defaultChain,
+	}, &mockPolicyManager{}, defaultHandlers())
+
+	if err != nil {
+		t.Fatalf("Error constructing configuration manager: %s", err)
+	}
+
+	newConfig := &ab.ConfigurationEnvelope{
+		Sequence: 1,
+		ChainID:  defaultChain,
+		Entries:  []*ab.ConfigurationEntry{makeConfigurationEntry("foo", "foo", 1, []byte("foo"))},
+	}
+
+	err = cm.Validate(newConfig)
+	if err != nil {
+		t.Errorf("Should not have errored validating config: %s", err)
+	}
+
+	err = cm.Apply(newConfig)
+	if err != nil {
+		t.Errorf("Should not have errored applying config: %s", err)
+	}
+}
+
+// TestConfigChangeNoUpdatedSequence tests that a new submitted config is rejected if it increments the
+// sequence number without a corresponding config item with that sequence number
+func TestConfigChangeNoUpdatedSequence(t *testing.T) {
+	cm, err := NewConfigurationManager(&ab.ConfigurationEnvelope{
+		Sequence: 0,
+		ChainID:  defaultChain,
+	}, &mockPolicyManager{&mockPolicy{}}, defaultHandlers())
+
+	if err != nil {
+		t.Fatalf("Error constructing configuration manager: %s", err)
+	}
+
+	newConfig := &ab.ConfigurationEnvelope{
+		Sequence: 1,
+		ChainID:  defaultChain,
+		// Note that the entries do not contain any config items with seqNo=1, so this is invalid
+		Entries: []*ab.ConfigurationEntry{makeConfigurationEntry("foo", "foo", 0, []byte("foo"))},
+	}
+
+	err = cm.Validate(newConfig)
+	if err == nil {
+		t.Errorf("Should have errored validating config because no new sequence number matches")
+	}
+
+	err = cm.Apply(newConfig)
+	if err == nil {
+		t.Errorf("Should have errored applying config because no new sequence number matches")
+	}
+}
+
+// TestConfigChangeRegressedSequence tests to make sure that a new config cannot roll back one of the
+// config values while advancing another
+func TestConfigChangeRegressedSequence(t *testing.T) {
+	cm, err := NewConfigurationManager(&ab.ConfigurationEnvelope{
+		Sequence: 1,
+		ChainID:  defaultChain,
+		Entries:  []*ab.ConfigurationEntry{makeConfigurationEntry("foo", "foo", 1, []byte("foo"))},
+	}, &mockPolicyManager{&mockPolicy{}}, defaultHandlers())
+
+	if err != nil {
+		t.Fatalf("Error constructing configuration manager: %s", err)
+	}
+
+	newConfig := &ab.ConfigurationEnvelope{
+		Sequence: 2,
+		ChainID:  defaultChain,
+		Entries: []*ab.ConfigurationEntry{
+			makeConfigurationEntry("foo", "foo", 0, []byte("foo")),
+			makeConfigurationEntry("bar", "bar", 2, []byte("bar")),
+		},
+	}
+
+	err = cm.Validate(newConfig)
+	if err == nil {
+		t.Errorf("Should have errored validating config because foo's sequence number regressed")
+	}
+
+	err = cm.Apply(newConfig)
+	if err == nil {
+		t.Errorf("Should have errored applying config because foo's sequence number regressed")
+	}
+}
+
+// TestConfigImplicitDelete tests to make sure that a new config does not implicitly delete config items
+// by omitting them in the new config
+func TestConfigImplicitDelete(t *testing.T) {
+	cm, err := NewConfigurationManager(&ab.ConfigurationEnvelope{
+		Sequence: 0,
+		ChainID:  defaultChain,
+		Entries: []*ab.ConfigurationEntry{
+			makeConfigurationEntry("foo", "foo", 0, []byte("foo")),
+			makeConfigurationEntry("bar", "bar", 0, []byte("bar")),
+		},
+	}, &mockPolicyManager{&mockPolicy{}}, defaultHandlers())
+
+	if err != nil {
+		t.Fatalf("Error constructing configuration manager: %s", err)
+	}
+
+	newConfig := &ab.ConfigurationEnvelope{
+		Sequence: 1,
+		ChainID:  defaultChain,
+		Entries: []*ab.ConfigurationEntry{
+			makeConfigurationEntry("bar", "bar", 1, []byte("bar")),
+		},
+	}
+
+	err = cm.Validate(newConfig)
+	if err == nil {
+		t.Errorf("Should have errored validating config because foo was implicitly deleted")
+	}
+
+	err = cm.Apply(newConfig)
+	if err == nil {
+		t.Errorf("Should have errored applying config because foo was implicitly deleted")
+	}
+}
+
+// TestConfigModifyWithoutFullIncrease tests to make sure that if an item is modified in a config change
+// that it not only increments its LastModified, but also increments it to the current sequence number
+func TestConfigModifyWithoutFullIncrease(t *testing.T) {
+	cm, err := NewConfigurationManager(&ab.ConfigurationEnvelope{
+		Sequence: 0,
+		ChainID:  defaultChain,
+		Entries: []*ab.ConfigurationEntry{
+			makeConfigurationEntry("foo", "foo", 0, []byte("foo")),
+			makeConfigurationEntry("bar", "bar", 0, []byte("bar")),
+		},
+	}, &mockPolicyManager{&mockPolicy{}}, defaultHandlers())
+
+	if err != nil {
+		t.Fatalf("Error constructing configuration manager: %s", err)
+	}
+
+	newConfig := &ab.ConfigurationEnvelope{
+		Sequence: 1,
+		ChainID:  defaultChain,
+		Entries: []*ab.ConfigurationEntry{
+			makeConfigurationEntry("foo", "foo", 0, []byte("foo")),
+			makeConfigurationEntry("bar", "bar", 1, []byte("bar")),
+		},
+	}
+
+	err = cm.Validate(newConfig)
+	if err != nil {
+		t.Errorf("Should not have errored validating config: %s", err)
+	}
+
+	err = cm.Apply(newConfig)
+	if err != nil {
+		t.Errorf("Should not have errored applying config: %s", err)
+	}
+
+	newConfig = &ab.ConfigurationEnvelope{
+		Sequence: 2,
+		ChainID:  defaultChain,
+		Entries: []*ab.ConfigurationEntry{
+			makeConfigurationEntry("foo", "foo", 1, []byte("foo")),
+			makeConfigurationEntry("bar", "bar", 2, []byte("bar")),
+		},
+	}
+
+	err = cm.Validate(newConfig)
+	if err == nil {
+		t.Errorf("Should have errored validating config because foo was modified, but its lastModifiedSeqNo did not increase to the current seqNo")
+	}
+
+	err = cm.Apply(newConfig)
+	if err == nil {
+		t.Errorf("Should have errored applying config because foo was modified, but its lastModifiedSeqNo did not increase to the current seqNo")
+	}
+}
+
+// TestSilentConfigModification tests to make sure that even if a validly signed new configuration for an existing sequence number
+// is substituted into an otherwise valid new config, that the new config is rejected for attempting a modification without
+// increasing the config item's LastModified
+func TestSilentConfigModification(t *testing.T) {
+	cm, err := NewConfigurationManager(&ab.ConfigurationEnvelope{
+		Sequence: 0,
+		ChainID:  defaultChain,
+		Entries: []*ab.ConfigurationEntry{
+			makeConfigurationEntry("foo", "foo", 0, []byte("foo")),
+			makeConfigurationEntry("bar", "bar", 0, []byte("bar")),
+		},
+	}, &mockPolicyManager{&mockPolicy{}}, defaultHandlers())
+
+	if err != nil {
+		t.Fatalf("Error constructing configuration manager: %s", err)
+	}
+
+	newConfig := &ab.ConfigurationEnvelope{
+		Sequence: 1,
+		ChainID:  defaultChain,
+		Entries: []*ab.ConfigurationEntry{
+			makeConfigurationEntry("foo", "foo", 0, []byte("different")),
+			makeConfigurationEntry("bar", "bar", 1, []byte("bar")),
+		},
+	}
+
+	err = cm.Validate(newConfig)
+	if err == nil {
+		t.Errorf("Should not errored validating config because foo was silently modified (despite modification allowed by policy)")
+	}
+
+	err = cm.Apply(newConfig)
+	if err == nil {
+		t.Errorf("Should not errored applying config because foo was silently modified (despite modification allowed by policy)")
+	}
+}
+
+// TestInvalidInitialConfigByPolicy tests to make sure that if an existing policies does not validate the config that
+// even construction fails
+func TestInvalidInitialConfigByPolicy(t *testing.T) {
+	_, err := NewConfigurationManager(&ab.ConfigurationEnvelope{
+		Sequence: 0,
+		ChainID:  defaultChain,
+		Entries:  []*ab.ConfigurationEntry{makeConfigurationEntry("foo", "foo", 0, []byte("foo"))},
+	}, &mockPolicyManager{&mockPolicy{fmt.Errorf("err")}}, defaultHandlers())
+	// mockPolicyManager will return non-validating defualt policy
+
+	if err == nil {
+		t.Fatalf("Should have failed to construct configuration by policy")
+	}
+}
+
+// TestConfigChangeViolatesPolicy checks to make sure that if policy rejects the validation of a config item that
+// it is rejected in a config update
+func TestConfigChangeViolatesPolicy(t *testing.T) {
+	mpm := &mockPolicyManager{}
+	cm, err := NewConfigurationManager(&ab.ConfigurationEnvelope{
+		Sequence: 0,
+		ChainID:  defaultChain,
+	}, mpm, defaultHandlers())
+
+	if err != nil {
+		t.Fatalf("Error constructing configuration manager: %s", err)
+	}
+	// Set the mock policy to error
+	mpm.policy = &mockPolicy{fmt.Errorf("err")}
+
+	newConfig := &ab.ConfigurationEnvelope{
+		Sequence: 1,
+		ChainID:  defaultChain,
+		Entries:  []*ab.ConfigurationEntry{makeConfigurationEntry("foo", "foo", 1, []byte("foo"))},
+	}
+
+	err = cm.Validate(newConfig)
+	if err == nil {
+		t.Errorf("Should have errored validating config because policy rejected modification")
+	}
+
+	err = cm.Apply(newConfig)
+	if err == nil {
+		t.Errorf("Should have errored applying config because policy rejected modification")
+	}
+}
+
+type failHandler struct{}
+
+func (fh failHandler) BeginConfig()    {}
+func (fh failHandler) RollbackConfig() {}
+func (fh failHandler) CommitConfig()   {}
+func (fh failHandler) ProposeConfig(item *ab.Configuration) error {
+	return fmt.Errorf("Fail")
+}
+
+// TestInvalidProposal checks that even if the policy allows the transaction and the sequence etc. is well formed,
+// that if the handler does not accept the config, it is rejected
+func TestInvalidProposal(t *testing.T) {
+	handlers := defaultHandlers()
+	handlers[ab.Configuration_ConfigurationType(0)] = failHandler{}
+	cm, err := NewConfigurationManager(&ab.ConfigurationEnvelope{
+		Sequence: 0,
+		ChainID:  defaultChain,
+	}, &mockPolicyManager{&mockPolicy{}}, handlers)
+
+	if err != nil {
+		t.Fatalf("Error constructing configuration manager: %s", err)
+	}
+
+	newConfig := &ab.ConfigurationEnvelope{
+		Sequence: 1,
+		ChainID:  defaultChain,
+		Entries:  []*ab.ConfigurationEntry{makeConfigurationEntry("foo", "foo", 1, []byte("foo"))},
+	}
+
+	err = cm.Validate(newConfig)
+	if err == nil {
+		t.Errorf("Should have errored validating config because the handler rejected it")
+	}
+
+	err = cm.Apply(newConfig)
+	if err == nil {
+		t.Errorf("Should have errored applying config because the handler rejected it")
+	}
+}
+
+// TestConfigItemOnWrongChain tests to make sure that a config is rejected if it contains an item for the wrong chain
+func TestConfigItemOnWrongChain(t *testing.T) {
+	cm, err := NewConfigurationManager(&ab.ConfigurationEnvelope{
+		Sequence: 0,
+		ChainID:  defaultChain,
+	}, &mockPolicyManager{&mockPolicy{}}, defaultHandlers())
+
+	if err != nil {
+		t.Fatalf("Error constructing configuration manager: %s", err)
+	}
+
+	config := makeConfiguration("foo", "foo", 1, []byte("foo"))
+	config.ChainID = []byte("Wrong")
+	marshaledConfig, err := proto.Marshal(config)
+	if err != nil {
+		t.Fatalf("Should have been able marshal config: %s", err)
+	}
+	newConfig := &ab.ConfigurationEnvelope{
+		Sequence: 1,
+		ChainID:  defaultChain,
+		Entries:  []*ab.ConfigurationEntry{&ab.ConfigurationEntry{Configuration: marshaledConfig}},
+	}
+
+	err = cm.Validate(newConfig)
+	if err == nil {
+		t.Errorf("Should have errored validating config because new config item is for a different chain")
+	}
+
+	err = cm.Apply(newConfig)
+	if err == nil {
+		t.Errorf("Should have errored applying config because new config item is for a different chain")
+	}
+}