From 53efa1904c3184d1eadbf742124a8aff57d81831 Mon Sep 17 00:00:00 2001 From: Jason Yellick Date: Wed, 7 Jun 2017 10:38:58 -0400 Subject: [PATCH] [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 --- common/configtx/template.go | 40 ++++++++++++--------- common/configtx/template_test.go | 46 ++++++++++++++++++++++++ common/configtx/tool/configtxgen/main.go | 8 +---- 3 files changed, 71 insertions(+), 23 deletions(-) diff --git a/common/configtx/template.go b/common/configtx/template.go index 1c53286d14b..38b62b32ca5 100644 --- a/common/configtx/template.go +++ b/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 diff --git a/common/configtx/template_test.go b/common/configtx/template_test.go index 531f672668b..6ac5ae52693 100644 --- a/common/configtx/template_test.go +++ b/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") +} diff --git a/common/configtx/tool/configtxgen/main.go b/common/configtx/tool/configtxgen/main.go index a1f329e1148..c9a960db03d 100644 --- a/common/configtx/tool/configtxgen/main.go +++ b/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) }