From 59a4bfa2b4deea28a0d0ce4447bca0997c2ad936 Mon Sep 17 00:00:00 2001 From: Alessandro Sorniotti Date: Tue, 29 Oct 2019 19:46:05 +0100 Subject: [PATCH] [FAB-16169] Separate unmarshalling from validation Change-Id: I842af4e11f33fdea62da59033e889e832fe02e53 Signed-off-by: Alessandro Sorniotti --- core/tx/endorser/parser.go | 141 ++++++++++++++++---------------- core/tx/endorser/parser_test.go | 54 ++++++------ 2 files changed, 98 insertions(+), 97 deletions(-) diff --git a/core/tx/endorser/parser.go b/core/tx/endorser/parser.go index 5568a24b7ca..ffa8ca3a279 100644 --- a/core/tx/endorser/parser.go +++ b/core/tx/endorser/parser.go @@ -9,7 +9,6 @@ package endorsertx import ( "regexp" - "github.com/hyperledger/fabric-protos-go/common" "github.com/hyperledger/fabric-protos-go/peer" "github.com/hyperledger/fabric/pkg/tx" @@ -27,67 +26,20 @@ var ( // EndorserTx represents a parsed common.Envelope protobuf type EndorserTx struct { ComputedTxID string - ChID string - CCName string + ChannelID string + ChaincodeID *peer.ChaincodeID Creator []byte Response *peer.Response Events []byte Results []byte Endorsements []*peer.Endorsement + Type int32 + Version int32 + Epoch uint64 + Nonce []byte } -func validateHeaders( - cHdr *common.ChannelHeader, - sHdr *common.SignatureHeader, - hdrExt *peer.ChaincodeHeaderExtension, -) error { - /******************************************/ - /*****VALIDATION OF THE CHANNEL HEADER*****/ - /******************************************/ - - if cHdr.Epoch != 0 { - return errors.Errorf("invalid Epoch in ChannelHeader. Expected 0, got [%d]", cHdr.Epoch) - } - - if cHdr.Version != 0 { - return errors.Errorf("invalid version in ChannelHeader. Expected 0, got [%d]", cHdr.Version) - } - - if err := ValidateChannelID(cHdr.ChannelId); err != nil { - return err - } - - /********************************************/ - /*****VALIDATION OF THE SIGNATURE HEADER*****/ - /********************************************/ - - if len(sHdr.Nonce) == 0 { - return errors.New("empty nonce") - } - - if len(sHdr.Creator) == 0 { - return errors.New("empty creator") - } - - /********************************************/ - /*****VALIDATION OF THE HEADER EXTENSION*****/ - /********************************************/ - - if hdrExt.ChaincodeId == nil { - return errors.New("nil ChaincodeId") - } - - if hdrExt.ChaincodeId.Name == "" { - return errors.New("empty chaincode name in chaincode id") - } - - return nil -} - -// NewEndorserTx receives a tx.Envelope containing a partially -// unmarshalled endorser transaction and returns an EndorserTx -// instance (or an error) -func NewEndorserTx(txenv *tx.Envelope) (*EndorserTx, error) { +func unmarshalEndorserTx(txenv *tx.Envelope) (*EndorserTx, error) { if len(txenv.ChannelHeader.Extension) == 0 { return nil, errors.New("empty header extension") @@ -100,15 +52,6 @@ func NewEndorserTx(txenv *tx.Envelope) (*EndorserTx, error) { return nil, err } - err = validateHeaders( - txenv.ChannelHeader, - txenv.SignatureHeader, - hdrExt, - ) - if err != nil { - return nil, err - } - if len(txenv.Data) == 0 { return nil, errors.New("nil payload data") } @@ -128,8 +71,6 @@ func NewEndorserTx(txenv *tx.Envelope) (*EndorserTx, error) { return nil, errors.New("nil action") } - // TODO FAB-16170: check that header in the tx action and channel header match bitwise - if len(txAction.Payload) == 0 { return nil, errors.New("empty ChaincodeActionPayload") } @@ -163,27 +104,83 @@ func NewEndorserTx(txenv *tx.Envelope) (*EndorserTx, error) { return nil, err } - // TODO FAB-16170: check proposal hash - computedTxID := protoutil.ComputeTxID( txenv.SignatureHeader.Nonce, txenv.SignatureHeader.Creator, ) - // TODO FAB-16170: verify that txid matches the one in the header - return &EndorserTx{ ComputedTxID: computedTxID, - ChID: txenv.ChannelHeader.ChannelId, + ChannelID: txenv.ChannelHeader.ChannelId, Creator: txenv.SignatureHeader.Creator, Response: ccAction.Response, Events: ccAction.Events, Results: ccAction.Results, Endorsements: ccActionPayload.Action.Endorsements, - CCName: hdrExt.ChaincodeId.Name, // FIXME: we might have to get the ccid from the CIS + ChaincodeID: hdrExt.ChaincodeId, + Type: txenv.ChannelHeader.Type, + Version: txenv.ChannelHeader.Version, + Epoch: txenv.ChannelHeader.Epoch, + Nonce: txenv.SignatureHeader.Nonce, }, nil } +func (e *EndorserTx) validate() error { + + if e.Epoch != 0 { + return errors.Errorf("invalid epoch in ChannelHeader. Expected 0, got [%d]", e.Epoch) + } + + if e.Version != 0 { + return errors.Errorf("invalid version in ChannelHeader. Expected 0, got [%d]", e.Version) + } + + if err := ValidateChannelID(e.ChannelID); err != nil { + return err + } + + if len(e.Nonce) == 0 { + return errors.New("empty nonce") + } + + if len(e.Creator) == 0 { + return errors.New("empty creator") + } + + if e.ChaincodeID == nil { + return errors.New("nil ChaincodeId") + } + + if e.ChaincodeID.Name == "" { + return errors.New("empty chaincode name in chaincode id") + } + + // TODO FAB-16170: check proposal hash + + // TODO FAB-16170: verify that txid matches the one in the header + + // TODO FAB-16170: check that header in the tx action and channel header match bitwise + + return nil +} + +// UnmarshalEndorserTxAndValidate receives a tx.Envelope containing +// a partially unmarshalled endorser transaction and returns an EndorserTx +// instance (or an error) +func UnmarshalEndorserTxAndValidate(txenv *tx.Envelope) (*EndorserTx, error) { + etx, err := unmarshalEndorserTx(txenv) + if err != nil { + return nil, err + } + + err = etx.validate() + if err != nil { + return nil, err + } + + return etx, nil +} + // ValidateChannelID makes sure that proposed channel IDs comply with the // following restrictions: // 1. Contain only lower case ASCII alphanumerics, dots '.', and dashes '-' diff --git a/core/tx/endorser/parser_test.go b/core/tx/endorser/parser_test.go index d7e884090d2..09b5bdfbbaa 100644 --- a/core/tx/endorser/parser_test.go +++ b/core/tx/endorser/parser_test.go @@ -44,7 +44,7 @@ var _ = Describe("Parser", func() { }) It("returns an instance of EndorserTx", func() { - pe, err := endorsertx.NewEndorserTx(txenv) + pe, err := endorsertx.UnmarshalEndorserTxAndValidate(txenv) Expect(err).NotTo(HaveOccurred()) Expect(pe).To(Equal(&endorsertx.EndorserTx{ Response: &peer.Response{ @@ -59,9 +59,13 @@ var _ = Describe("Parser", func() { Signature: []byte("signature"), }, }, - ChID: "my-channel", - Creator: []byte("creator"), - CCName: "my-called-cc", + ChannelID: "my-channel", + Creator: []byte("creator"), + ChaincodeID: &peer.ChaincodeID{Name: "my-called-cc"}, + Type: 0, + Version: 0, + Epoch: 0, + Nonce: []byte("1234"), })) }) }) @@ -90,7 +94,7 @@ var _ = Describe("Parser", func() { }) It("returns an error", func() { - pe, err := endorsertx.NewEndorserTx(txenv) + pe, err := endorsertx.UnmarshalEndorserTxAndValidate(txenv) Expect(err).To(MatchError("nil payload data")) Expect(pe).To(BeNil()) }) @@ -102,7 +106,7 @@ var _ = Describe("Parser", func() { }) It("returns an error", func() { - pe, err := endorsertx.NewEndorserTx(txenv) + pe, err := endorsertx.UnmarshalEndorserTxAndValidate(txenv) Expect(err).To(MatchError("error unmarshaling Transaction: unexpected EOF")) Expect(pe).To(BeNil()) }) @@ -116,7 +120,7 @@ var _ = Describe("Parser", func() { }) It("returns an error", func() { - pe, err := endorsertx.NewEndorserTx(txenv) + pe, err := endorsertx.UnmarshalEndorserTxAndValidate(txenv) Expect(err).To(MatchError("only one transaction action is supported, 2 were present")) Expect(pe).To(BeNil()) }) @@ -130,7 +134,7 @@ var _ = Describe("Parser", func() { }) It("returns an error", func() { - pe, err := endorsertx.NewEndorserTx(txenv) + pe, err := endorsertx.UnmarshalEndorserTxAndValidate(txenv) Expect(err).To(MatchError("empty ChaincodeActionPayload")) Expect(pe).To(BeNil()) }) @@ -144,7 +148,7 @@ var _ = Describe("Parser", func() { }) It("returns an error", func() { - pe, err := endorsertx.NewEndorserTx(txenv) + pe, err := endorsertx.UnmarshalEndorserTxAndValidate(txenv) Expect(err).To(MatchError("error unmarshaling ChaincodeActionPayload: unexpected EOF")) Expect(pe).To(BeNil()) }) @@ -167,7 +171,7 @@ var _ = Describe("Parser", func() { }) It("returns an error", func() { - pe, err := endorsertx.NewEndorserTx(txenv) + pe, err := endorsertx.UnmarshalEndorserTxAndValidate(txenv) Expect(err).To(MatchError("nil ChaincodeEndorsedAction")) Expect(pe).To(BeNil()) }) @@ -179,7 +183,7 @@ var _ = Describe("Parser", func() { }) It("returns an error", func() { - pe, err := endorsertx.NewEndorserTx(txenv) + pe, err := endorsertx.UnmarshalEndorserTxAndValidate(txenv) Expect(err).To(MatchError("empty header extension")) Expect(pe).To(BeNil()) }) @@ -191,7 +195,7 @@ var _ = Describe("Parser", func() { }) It("returns an error", func() { - pe, err := endorsertx.NewEndorserTx(txenv) + pe, err := endorsertx.UnmarshalEndorserTxAndValidate(txenv) Expect(err).To(MatchError("error unmarshaling ChaincodeHeaderExtension: unexpected EOF")) Expect(pe).To(BeNil()) }) @@ -203,7 +207,7 @@ var _ = Describe("Parser", func() { }) It("returns an error", func() { - pe, err := endorsertx.NewEndorserTx(txenv) + pe, err := endorsertx.UnmarshalEndorserTxAndValidate(txenv) Expect(err).To(MatchError("empty ProposalResponsePayload")) Expect(pe).To(BeNil()) }) @@ -215,7 +219,7 @@ var _ = Describe("Parser", func() { }) It("returns an error", func() { - pe, err := endorsertx.NewEndorserTx(txenv) + pe, err := endorsertx.UnmarshalEndorserTxAndValidate(txenv) Expect(err).To(MatchError("error unmarshaling ProposalResponsePayload: unexpected EOF")) Expect(pe).To(BeNil()) }) @@ -227,7 +231,7 @@ var _ = Describe("Parser", func() { }) It("returns an error", func() { - pe, err := endorsertx.NewEndorserTx(txenv) + pe, err := endorsertx.UnmarshalEndorserTxAndValidate(txenv) Expect(err).To(MatchError("nil Extension")) Expect(pe).To(BeNil()) }) @@ -239,7 +243,7 @@ var _ = Describe("Parser", func() { }) It("returns an error", func() { - pe, err := endorsertx.NewEndorserTx(txenv) + pe, err := endorsertx.UnmarshalEndorserTxAndValidate(txenv) Expect(err).To(MatchError("error unmarshaling ChaincodeAction: unexpected EOF")) Expect(pe).To(BeNil()) }) @@ -254,8 +258,8 @@ var _ = Describe("Parser", func() { }) It("returns an error", func() { - pe, err := endorsertx.NewEndorserTx(txenv) - Expect(err).To(MatchError("invalid Epoch in ChannelHeader. Expected 0, got [35]")) + pe, err := endorsertx.UnmarshalEndorserTxAndValidate(txenv) + Expect(err).To(MatchError("invalid epoch in ChannelHeader. Expected 0, got [35]")) Expect(pe).To(BeNil()) }) }) @@ -269,7 +273,7 @@ var _ = Describe("Parser", func() { }) It("returns an error", func() { - pe, err := endorsertx.NewEndorserTx(txenv) + pe, err := endorsertx.UnmarshalEndorserTxAndValidate(txenv) Expect(err).To(MatchError("invalid version in ChannelHeader. Expected 0, got [35]")) Expect(pe).To(BeNil()) }) @@ -283,7 +287,7 @@ var _ = Describe("Parser", func() { }) It("returns an error", func() { - pe, err := endorsertx.NewEndorserTx(txenv) + pe, err := endorsertx.UnmarshalEndorserTxAndValidate(txenv) Expect(err).To(MatchError("channel ID illegal, cannot be empty")) Expect(pe).To(BeNil()) }) @@ -297,7 +301,7 @@ var _ = Describe("Parser", func() { }) It("returns an error", func() { - pe, err := endorsertx.NewEndorserTx(txenv) + pe, err := endorsertx.UnmarshalEndorserTxAndValidate(txenv) Expect(err).To(MatchError("'.foo' contains illegal characters")) Expect(pe).To(BeNil()) }) @@ -311,7 +315,7 @@ var _ = Describe("Parser", func() { }) It("returns an error", func() { - pe, err := endorsertx.NewEndorserTx(txenv) + pe, err := endorsertx.UnmarshalEndorserTxAndValidate(txenv) Expect(err).To(MatchError("empty nonce")) Expect(pe).To(BeNil()) }) @@ -325,7 +329,7 @@ var _ = Describe("Parser", func() { }) It("returns an error", func() { - pe, err := endorsertx.NewEndorserTx(txenv) + pe, err := endorsertx.UnmarshalEndorserTxAndValidate(txenv) Expect(err).To(MatchError("empty creator")) Expect(pe).To(BeNil()) }) @@ -346,7 +350,7 @@ var _ = Describe("Parser", func() { }) It("returns an error", func() { - pe, err := endorsertx.NewEndorserTx(txenv) + pe, err := endorsertx.UnmarshalEndorserTxAndValidate(txenv) Expect(err).To(MatchError("nil ChaincodeId")) Expect(pe).To(BeNil()) }) @@ -362,7 +366,7 @@ var _ = Describe("Parser", func() { }) It("returns an error", func() { - pe, err := endorsertx.NewEndorserTx(txenv) + pe, err := endorsertx.UnmarshalEndorserTxAndValidate(txenv) Expect(err).To(MatchError("empty chaincode name in chaincode id")) Expect(pe).To(BeNil()) })