[FAB-4443] Do not sign configtx with Noop MSP

Because it has no crypto identity, configtxgen uses the noop signer as
the parameter to the channel creation helpers which require it.
Unfortunately, this creates an unreadable garbage signature in the
output of the channel creation transaction.

This behavior does not prevent channel creation from occurring, because
of the multi-sig nature of the messages, but, it does cause nasty error
messages to appear in the log.

This CR removes the use of the noop MSP and instead fixes the method
which depended on having a non-nil signer passed in to accept a nil
signer and omit signatures instead.

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

Author: Jason Yellick

common/configtx/template.go

@@ -244,36 +244,44 @@ func (cct *channelCreationTemplate) Envelope(channelID string) (*cb.ConfigUpdate
 
 // MakeChainCreationTransaction is a handy utility function for creating new chain transactions using the underlying Template framework
 func MakeChainCreationTransaction(channelID string, consortium string, signer msp.SigningIdentity, orgs ...string) (*cb.Envelope, error) {
-	sSigner, err := signer.Serialize()
-	if err != nil {
-		return nil, fmt.Errorf("Serialization of identity failed, err %s", err)
-	}
-
 	newChainTemplate := NewChainCreationTemplate(consortium, orgs)
 	newConfigUpdateEnv, err := newChainTemplate.Envelope(channelID)
 	if err != nil {
 		return nil, err
 	}
-	newConfigUpdateEnv.Signatures = []*cb.ConfigSignature{&cb.ConfigSignature{
-		SignatureHeader: utils.MarshalOrPanic(utils.MakeSignatureHeader(sSigner, utils.CreateNonceOrPanic())),
-	}}
 
-	newConfigUpdateEnv.Signatures[0].Signature, err = signer.Sign(util.ConcatenateBytes(newConfigUpdateEnv.Signatures[0].SignatureHeader, newConfigUpdateEnv.ConfigUpdate))
-	if err != nil {
-		return nil, err
+	payloadSignatureHeader := &cb.SignatureHeader{}
+	if signer != nil {
+		sSigner, err := signer.Serialize()
+		if err != nil {
+			return nil, fmt.Errorf("Serialization of identity failed, err %s", err)
+		}
+
+		newConfigUpdateEnv.Signatures = []*cb.ConfigSignature{&cb.ConfigSignature{
+			SignatureHeader: utils.MarshalOrPanic(utils.MakeSignatureHeader(sSigner, utils.CreateNonceOrPanic())),
+		}}
+
+		newConfigUpdateEnv.Signatures[0].Signature, err = signer.Sign(util.ConcatenateBytes(newConfigUpdateEnv.Signatures[0].SignatureHeader, newConfigUpdateEnv.ConfigUpdate))
+		if err != nil {
+			return nil, err
+		}
+
+		payloadSignatureHeader = utils.MakeSignatureHeader(sSigner, utils.CreateNonceOrPanic())
 	}
 
 	payloadChannelHeader := utils.MakeChannelHeader(cb.HeaderType_CONFIG_UPDATE, msgVersion, channelID, epoch)
-	payloadSignatureHeader := utils.MakeSignatureHeader(sSigner, utils.CreateNonceOrPanic())
 	utils.SetTxID(payloadChannelHeader, payloadSignatureHeader)
 	payloadHeader := utils.MakePayloadHeader(payloadChannelHeader, payloadSignatureHeader)
 	payload := &cb.Payload{Header: payloadHeader, Data: utils.MarshalOrPanic(newConfigUpdateEnv)}
 	paylBytes := utils.MarshalOrPanic(payload)
 
-	// sign the payload
-	sig, err := signer.Sign(paylBytes)
-	if err != nil {
-		return nil, err
+	var sig []byte
+	if signer != nil {
+		// sign the payload
+		sig, err = signer.Sign(paylBytes)
+		if err != nil {
+			return nil, err
+		}
 	}
 
 	return &cb.Envelope{Payload: paylBytes, Signature: sig}, nil

common/configtx/template_test.go

@@ -21,7 +21,9 @@ import (
 	"testing"
 
 	"github.com/hyperledger/fabric/common/config"
+	mmsp "github.com/hyperledger/fabric/common/mocks/msp"
 	cb "github.com/hyperledger/fabric/protos/common"
+	"github.com/hyperledger/fabric/protos/utils"
 
 	"github.com/golang/protobuf/proto"
 	"github.com/stretchr/testify/assert"
@@ -146,3 +148,47 @@ func TestNewChainTemplate(t *testing.T) {
 		assert.True(t, ok, "Expected to find %s but did not", org)
 	}
 }
+
+func TestMakeChainCreationTransactionWithSigner(t *testing.T) {
+	channelID := "foo"
+
+	signer, err := mmsp.NewNoopMsp().GetDefaultSigningIdentity()
+	assert.NoError(t, err, "Creating noop MSP")
+
+	cct, err := MakeChainCreationTransaction(channelID, "test", signer)
+	assert.NoError(t, err, "Making chain creation tx")
+
+	assert.NotEmpty(t, cct.Signature, "Should have signature")
+
+	payload, err := utils.UnmarshalPayload(cct.Payload)
+	assert.NoError(t, err, "Unmarshaling payload")
+
+	configUpdateEnv, err := UnmarshalConfigUpdateEnvelope(payload.Data)
+	assert.NoError(t, err, "Unmarshaling ConfigUpdateEnvelope")
+
+	assert.NotEmpty(t, configUpdateEnv.Signatures, "Should have config env sigs")
+
+	sigHeader, err := utils.GetSignatureHeader(payload.Header.SignatureHeader)
+	assert.NoError(t, err, "Unmarshaling SignatureHeader")
+	assert.NotEmpty(t, sigHeader.Creator, "Creator specified")
+}
+
+func TestMakeChainCreationTransactionNoSigner(t *testing.T) {
+	channelID := "foo"
+	cct, err := MakeChainCreationTransaction(channelID, "test", nil)
+	assert.NoError(t, err, "Making chain creation tx")
+
+	assert.Empty(t, cct.Signature, "Should have empty signature")
+
+	payload, err := utils.UnmarshalPayload(cct.Payload)
+	assert.NoError(t, err, "Unmarshaling payload")
+
+	configUpdateEnv, err := UnmarshalConfigUpdateEnvelope(payload.Data)
+	assert.NoError(t, err, "Unmarshaling ConfigUpdateEnvelope")
+
+	assert.Empty(t, configUpdateEnv.Signatures, "Should have no config env sigs")
+
+	sigHeader, err := utils.GetSignatureHeader(payload.Header.SignatureHeader)
+	assert.NoError(t, err, "Unmarshaling SignatureHeader")
+	assert.Empty(t, sigHeader.Creator, "No creator specified")
+}

common/configtx/tool/configtxgen/main.go

@@ -35,7 +35,6 @@ import (
 	"github.com/hyperledger/fabric/protos/utils"
 
 	"github.com/golang/protobuf/proto"
-	mmsp "github.com/hyperledger/fabric/common/mocks/msp"
 	logging "github.com/op/go-logging"
 )
 
@@ -61,11 +60,6 @@ func doOutputBlock(config *genesisconfig.Profile, channelID string, outputBlock
 
 func doOutputChannelCreateTx(conf *genesisconfig.Profile, channelID string, outputChannelCreateTx string) error {
 	logger.Info("Generating new channel configtx")
-	// TODO, use actual MSP eventually
-	signer, err := mmsp.NewNoopMsp().GetDefaultSigningIdentity()
-	if err != nil {
-		return fmt.Errorf("Error getting signing identity: %s", err)
-	}
 
 	if conf.Application == nil {
 		return fmt.Errorf("Cannot define a new channel with no Application section")
@@ -82,7 +76,7 @@ func doOutputChannelCreateTx(conf *genesisconfig.Profile, channelID string, outp
 	for _, org := range conf.Application.Organizations {
 		orgNames = append(orgNames, org.Name)
 	}
-	configtx, err := configtx.MakeChainCreationTransaction(channelID, conf.Consortium, signer, orgNames...)
+	configtx, err := configtx.MakeChainCreationTransaction(channelID, conf.Consortium, nil, orgNames...)
 	if err != nil {
 		return fmt.Errorf("Error generating configtx: %s", err)
 	}