[FAB-1992] Move configtx signatures to envelope

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

The current configtx scheme is a collection of signed configuration
items. This has the benefit of being very flexible, allowing a submitter
to collect signatures from one set of parties for one set of items, a
second set of signatures from a second set of parties for a second set
of items, and then glue them together, and submit a valid
reconfiguration.

However, this flexibility comes at the expense of requiring more
signatures, and the added complexity does not seem worth the added
flexibility. This CR moves the signatures to the envelope level, but
maintains the per item policy evaluation.

This is a step on the way towards:

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

Which will improve the administrative process even further.

Change-Id: Iae35ed8ac5577603666de9d79a50d51509ad2d45
Signed-off-by: Jason Yellick <[email protected]>

common/configtx/inspector/common_types.go

@@ -26,35 +26,23 @@ import (
 func viewableConfigurationEnvelope(name string, configEnvelope *cb.ConfigurationEnvelope) Viewable {
 	return &field{
 		name:   name,
-		values: []Viewable{viewableSignedConfigurationItemSlice("Items", configEnvelope.Items)},
+		values: []Viewable{viewableConfig("Config", configEnvelope.Config), viewableConfigurationSignatureSlice("Signatures", configEnvelope.Signatures)},
 	}
 }
 
-func viewableSignedConfigurationItemSlice(name string, signedConfigItems []*cb.SignedConfigurationItem) Viewable {
-	values := make([]Viewable, len(signedConfigItems))
-	for i, item := range signedConfigItems {
-		values[i] = viewableSignedConfigurationItem(fmt.Sprintf("Element %d", i), item)
+func viewableConfig(name string, configBytes []byte) Viewable {
+	config := &cb.Config{}
+	err := proto.Unmarshal(configBytes, config)
+	if err != nil {
+		return viewableError(name, err)
 	}
-	return &field{
-		name:   name,
-		values: values,
+	values := make([]Viewable, len(config.Items))
+	for i, item := range config.Items {
+		values[i] = viewableConfigurationItem(fmt.Sprintf("Element %d", i), item)
 	}
-}
-
-func viewableSignedConfigurationItem(name string, signedConfigItem *cb.SignedConfigurationItem) Viewable {
-	var viewableConfigItem Viewable
-
-	configItem := &cb.ConfigurationItem{}
-
-	if err := proto.Unmarshal(signedConfigItem.ConfigurationItem, configItem); err != nil {
-		viewableConfigItem = viewableError(name, err)
-	} else {
-		viewableConfigItem = viewableConfigurationItem("ConfigurationItem", configItem)
-	}
-
 	return &field{
 		name:   name,
-		values: []Viewable{viewableConfigItem, viewableConfigurationSignatureSlice("Signatures", signedConfigItem.Signatures)},
+		values: values,
 	}
 }
 

common/configtx/inspector/inspector_test.go

@@ -20,17 +20,13 @@ import (
 	"testing"
 
 	configtxtest "github.com/hyperledger/fabric/common/configtx/test"
-	cb "github.com/hyperledger/fabric/protos/common"
 )
 
 func TestFromTemplate(t *testing.T) {
 	ordererTemplate := configtxtest.OrdererTemplate()
-	signedItems, err := ordererTemplate.Items("SampleChainID")
+	configEnvelope, err := ordererTemplate.Envelope("SampleChainID")
 	if err != nil {
 		t.Fatalf("Error creating signed items: %s", err)
 	}
-	configEnvelope := &cb.ConfigurationEnvelope{
-		Items: signedItems,
-	}
 	PrintConfiguration(configEnvelope)
 }

common/configtx/inspector/orderer_types.go

@@ -132,7 +132,6 @@ func viewableCreationPolicy(name string, creationPolicy *ab.CreationPolicy) View
 		name: name,
 		values: []Viewable{
 			viewableString("Policy", creationPolicy.Policy),
-			viewableBytes("Digest", creationPolicy.Digest),
 		},
 	}
 }

common/configtx/manager.go

@@ -17,16 +17,14 @@ limitations under the License.
 package configtx
 
 import (
+	"errors"
 	"fmt"
 	"reflect"
 	"regexp"
 
 	"github.com/hyperledger/fabric/common/policies"
 	cb "github.com/hyperledger/fabric/protos/common"
 
-	"errors"
-
-	"github.com/golang/protobuf/proto"
 	logging "github.com/op/go-logging"
 )
 
@@ -93,33 +91,28 @@ type configurationManager struct {
 
 // computeChainIDAndSequence returns the chain id and the sequence number for a configuration envelope
 // or an error if there is a problem with the configuration envelope
-func computeChainIDAndSequence(configtx *cb.ConfigurationEnvelope) (string, uint64, error) {
-	if len(configtx.Items) == 0 {
+func computeChainIDAndSequence(config *cb.Config) (string, uint64, error) {
+	if len(config.Items) == 0 {
 		return "", 0, errors.New("Empty envelope unsupported")
 	}
 
 	m := uint64(0)
 
-	if configtx.Header == nil {
+	if config.Header == nil {
 		return "", 0, fmt.Errorf("Header not set")
 	}
 
-	if configtx.Header.ChainID == "" {
+	if config.Header.ChainID == "" {
 		return "", 0, fmt.Errorf("Header chainID was not set")
 	}
 
-	chainID := configtx.Header.ChainID
+	chainID := config.Header.ChainID
 
 	if err := validateChainID(chainID); err != nil {
 		return "", 0, err
 	}
 
-	for _, signedItem := range configtx.Items {
-		item := &cb.ConfigurationItem{}
-		if err := proto.Unmarshal(signedItem.ConfigurationItem, item); err != nil {
-			return "", 0, fmt.Errorf("Error unmarshaling signedItem.ConfigurationItem: %s", err)
-		}
-
+	for _, item := range config.Items {
 		if item.LastModified > m {
 			m = item.LastModified
 		}
@@ -167,7 +160,12 @@ func NewManagerImpl(configtx *cb.ConfigurationEnvelope, initializer Initializer,
 		}
 	}
 
-	chainID, seq, err := computeChainIDAndSequence(configtx)
+	config, err := UnmarshalConfig(configtx.Config)
+	if err != nil {
+		return nil, err
+	}
+
+	chainID, seq, err := computeChainIDAndSequence(config)
 	if err != nil {
 		return nil, fmt.Errorf("Error computing chain ID and sequence: %s", err)
 	}
@@ -222,7 +220,17 @@ func (cm *configurationManager) commitHandlers() {
 }
 
 func (cm *configurationManager) processConfig(configtx *cb.ConfigurationEnvelope) (configMap map[cb.ConfigurationItem_ConfigurationType]map[string]*cb.ConfigurationItem, err error) {
-	chainID, seq, err := computeChainIDAndSequence(configtx)
+	config, err := UnmarshalConfig(configtx.Config)
+	if err != nil {
+		return nil, err
+	}
+
+	chainID, seq, err := computeChainIDAndSequence(config)
+	if err != nil {
+		return nil, err
+	}
+
+	signedData, err := configtx.AsSignedData()
 	if err != nil {
 		return nil, err
 	}
@@ -246,66 +254,58 @@ func (cm *configurationManager) processConfig(configtx *cb.ConfigurationEnvelope
 
 	configMap = makeConfigMap()
 
-	for _, entry := range configtx.Items {
-		// Verify every entry is well formed
-		config := &cb.ConfigurationItem{}
-		err = proto.Unmarshal(entry.ConfigurationItem, config)
-		if err != nil {
-			// Note that this is not reachable by test coverage because the unmarshal error would have already been found when computing the chainID and seqNo
-			return nil, fmt.Errorf("Error unmarshaling ConfigurationItem: %s", err)
-		}
-
+	for _, item := range config.Items {
 		// Ensure the config sequence numbers are correct to prevent replay attacks
 		var isModified bool
 
-		if val, ok := cm.configuration[config.Type][config.Key]; ok {
+		if val, ok := cm.configuration[item.Type][item.Key]; ok {
 			// Config was modified if any of the contents changed
-			isModified = !reflect.DeepEqual(val, config)
+			isModified = !reflect.DeepEqual(val, item)
 		} 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)
+			if item.LastModified != seq {
+				return nil, fmt.Errorf("Key %v for type %v was new, but had an older Sequence %d set", item.Key, item.Type, item.LastModified)
 			}
 			isModified = true
 		}
 
 		// If a config item was modified, its LastModified must be set correctly, and it must satisfy the modification policy
 		if isModified {
-			logger.Debugf("Proposed configuration item of type %v and key %s on chain %s has been modified", config.Type, config.Key, cm.chainID)
+			logger.Debugf("Proposed configuration item of type %v and key %s on chain %s has been modified", item.Type, item.Key, chainID)
 
-			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)
+			if item.LastModified != seq {
+				return nil, fmt.Errorf("Key %v for type %v was modified, but its LastModified %d does not equal current configtx Sequence %d", item.Key, item.Type, item.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]
+			oldItem, ok := cm.configuration[item.Type][item.Key]
 			if ok {
 				policy, _ = cm.PolicyManager().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
-		logger.Debugf("Proposing configuration item of type %v for key %s on chain %s", config.Type, config.Key, cm.chainID)
-		err = cm.Initializer.Handlers()[config.Type].ProposeConfig(config)
+		logger.Debugf("Proposing configuration item of type %v for key %s on chain %s", item.Type, item.Key, cm.chainID)
+		err = cm.Initializer.Handlers()[item.Type].ProposeConfig(item)
 		if err != nil {
-			return nil, fmt.Errorf("Error proposing configuration item of type %v for key %s on chain %s: %s", config.Type, config.Key, chainID, err)
+			return nil, fmt.Errorf("Error proposing configuration item of type %v for key %s on chain %s: %s", item.Type, item.Key, chainID, err)
+		}
+
+		// Ensure the same key has not been specified multiple times
+		_, ok := configMap[item.Type][item.Key]
+		if ok {
+			return nil, fmt.Errorf("Specified config item of type %v for key %s for chain %s multiple times", item.Type, item.Key, chainID)
 		}
 
-		configMap[config.Type][config.Key] = config
+		configMap[item.Type][item.Key] = item
 	}
 
 	// Ensure that any config items which used to exist still exist, to prevent implicit deletion