From fb91ef7462b421349a56c32733724d920fce3ad4 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Thu, 1 Nov 2018 07:07:18 +0100 Subject: [PATCH] validate reactor messages (#2711) * validate reactor messages Refs #2683 * validate blockchain messages Refs #2683 * validate evidence messages Refs #2683 * todo * check ProposalPOL and signature sizes * add a changelog entry * check addr is valid when we add it to the addrbook * validate incoming netAddr (not just nil check!) * fixes after Bucky's review * check timestamps * beef up block#ValidateBasic * move some checks into bcBlockResponseMessage * update Gopkg.lock Fix ``` grouped write of manifest, lock and vendor: failed to export github.com/tendermint/go-amino: fatal: failed to unpack tree object 6dcc6ddc143e116455c94b25c1004c99e0d0ca12 ``` by running `dep ensure -update` * bump year since now we check it * generate test/p2p/data on the fly using tendermint testnet * allow sync chains older than 1 year * use full path when creating a testnet * move testnet gen to test/docker/Dockerfile * relax LastCommitRound check Refs #2737 * fix conflicts after merge * add small comment * some ValidateBasic updates * fixes * AppHash length is not fixed --- CHANGELOG_PENDING.md | 4 + blockchain/reactor.go | 53 +++++- config/toml.go | 2 +- consensus/common_test.go | 2 - consensus/mempool_test.go | 1 - consensus/reactor.go | 161 ++++++++++++++++-- consensus/replay.go | 8 +- consensus/types/round_state.go | 7 + crypto/crypto.go | 1 + evidence/reactor.go | 23 ++- p2p/pex/addrbook.go | 4 + p2p/pex/errors.go | 8 + p2p/pex/pex_reactor.go | 28 ++- state/validation.go | 50 +++--- test/docker/Dockerfile | 11 +- test/p2p/README.md | 9 +- test/p2p/data/mach1/core/config/genesis.json | 39 ----- test/p2p/data/mach1/core/config/node_key.json | 6 - .../mach1/core/config/priv_validator.json | 14 -- test/p2p/data/mach2/core/config/genesis.json | 39 ----- test/p2p/data/mach2/core/config/node_key.json | 6 - .../mach2/core/config/priv_validator.json | 14 -- test/p2p/data/mach3/core/config/genesis.json | 39 ----- test/p2p/data/mach3/core/config/node_key.json | 6 - .../mach3/core/config/priv_validator.json | 14 -- test/p2p/data/mach4/core/config/genesis.json | 39 ----- test/p2p/data/mach4/core/config/node_key.json | 6 - .../mach4/core/config/priv_validator.json | 14 -- test/p2p/ip_plus_id.sh | 2 +- test/p2p/peer.sh | 6 +- test/p2p/pex/test_addrbook.sh | 6 +- types/block.go | 115 ++++++++++--- types/block_test.go | 6 +- types/evidence.go | 21 +++ types/heartbeat.go | 31 ++++ types/part_set.go | 26 ++- types/proposal.go | 29 ++++ types/signable.go | 12 ++ types/signed_msg_type.go | 9 +- types/validation.go | 40 +++++ types/vote.go | 38 ++++- 41 files changed, 613 insertions(+), 336 deletions(-) delete mode 100644 test/p2p/data/mach1/core/config/genesis.json delete mode 100644 test/p2p/data/mach1/core/config/node_key.json delete mode 100644 test/p2p/data/mach1/core/config/priv_validator.json delete mode 100644 test/p2p/data/mach2/core/config/genesis.json delete mode 100644 test/p2p/data/mach2/core/config/node_key.json delete mode 100644 test/p2p/data/mach2/core/config/priv_validator.json delete mode 100644 test/p2p/data/mach3/core/config/genesis.json delete mode 100644 test/p2p/data/mach3/core/config/node_key.json delete mode 100644 test/p2p/data/mach3/core/config/priv_validator.json delete mode 100644 test/p2p/data/mach4/core/config/genesis.json delete mode 100644 test/p2p/data/mach4/core/config/node_key.json delete mode 100644 test/p2p/data/mach4/core/config/priv_validator.json create mode 100644 types/validation.go diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 2d970e407..cad2f444a 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -92,6 +92,10 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi github.com/tendermint/crypto - [tools] [\#2238](https://github.com/tendermint/tendermint/issues/2238) Binary dependencies are now locked to a specific git commit - [libs/log] [\#2706](https://github.com/tendermint/tendermint/issues/2706) Add year to log format +- [consensus] [\#2683] validate all incoming messages +- [evidence] [\#2683] validate all incoming messages +- [blockchain] [\#2683] validate all incoming messages +- [p2p/pex] [\#2683] validate pexAddrsMessage addresses ### BUG FIXES: - [autofile] [\#2428](https://github.com/tendermint/tendermint/issues/2428) Group.RotateFile need call Flush() before rename (@goolAdapter) diff --git a/blockchain/reactor.go b/blockchain/reactor.go index fc1b1f4d3..59318dcc5 100644 --- a/blockchain/reactor.go +++ b/blockchain/reactor.go @@ -1,6 +1,7 @@ package blockchain import ( + "errors" "fmt" "reflect" "time" @@ -180,6 +181,12 @@ func (bcR *BlockchainReactor) Receive(chID byte, src p2p.Peer, msgBytes []byte) return } + if err = msg.ValidateBasic(); err != nil { + bcR.Logger.Error("Peer sent us invalid msg", "peer", src, "msg", msg, "err", err) + bcR.Switch.StopPeerForError(src, err) + return + } + bcR.Logger.Debug("Receive", "src", src, "chID", chID, "msg", msg) switch msg := msg.(type) { @@ -188,7 +195,6 @@ func (bcR *BlockchainReactor) Receive(chID byte, src p2p.Peer, msgBytes []byte) // Unfortunately not queued since the queue is full. } case *bcBlockResponseMessage: - // Got a block. bcR.pool.AddBlock(src.ID(), msg.Block, len(msgBytes)) case *bcStatusRequestMessage: // Send peer our state. @@ -352,7 +358,9 @@ func (bcR *BlockchainReactor) BroadcastStatusRequest() error { // Messages // BlockchainMessage is a generic message for this reactor. -type BlockchainMessage interface{} +type BlockchainMessage interface { + ValidateBasic() error +} func RegisterBlockchainMessages(cdc *amino.Codec) { cdc.RegisterInterface((*BlockchainMessage)(nil), nil) @@ -377,6 +385,14 @@ type bcBlockRequestMessage struct { Height int64 } +// ValidateBasic performs basic validation. +func (m *bcBlockRequestMessage) ValidateBasic() error { + if m.Height < 0 { + return errors.New("Negative Height") + } + return nil +} + func (m *bcBlockRequestMessage) String() string { return fmt.Sprintf("[bcBlockRequestMessage %v]", m.Height) } @@ -385,6 +401,14 @@ type bcNoBlockResponseMessage struct { Height int64 } +// ValidateBasic performs basic validation. +func (m *bcNoBlockResponseMessage) ValidateBasic() error { + if m.Height < 0 { + return errors.New("Negative Height") + } + return nil +} + func (brm *bcNoBlockResponseMessage) String() string { return fmt.Sprintf("[bcNoBlockResponseMessage %d]", brm.Height) } @@ -395,6 +419,15 @@ type bcBlockResponseMessage struct { Block *types.Block } +// ValidateBasic performs basic validation. +func (m *bcBlockResponseMessage) ValidateBasic() error { + if err := m.Block.ValidateBasic(); err != nil { + return err + } + + return nil +} + func (m *bcBlockResponseMessage) String() string { return fmt.Sprintf("[bcBlockResponseMessage %v]", m.Block.Height) } @@ -405,6 +438,14 @@ type bcStatusRequestMessage struct { Height int64 } +// ValidateBasic performs basic validation. +func (m *bcStatusRequestMessage) ValidateBasic() error { + if m.Height < 0 { + return errors.New("Negative Height") + } + return nil +} + func (m *bcStatusRequestMessage) String() string { return fmt.Sprintf("[bcStatusRequestMessage %v]", m.Height) } @@ -415,6 +456,14 @@ type bcStatusResponseMessage struct { Height int64 } +// ValidateBasic performs basic validation. +func (m *bcStatusResponseMessage) ValidateBasic() error { + if m.Height < 0 { + return errors.New("Negative Height") + } + return nil +} + func (m *bcStatusResponseMessage) String() string { return fmt.Sprintf("[bcStatusResponseMessage %v]", m.Height) } diff --git a/config/toml.go b/config/toml.go index 62e5fa978..d73b9c81d 100644 --- a/config/toml.go +++ b/config/toml.go @@ -342,7 +342,7 @@ func ResetTestRoot(testName string) *Config { } var testGenesis = `{ - "genesis_time": "2017-10-10T08:20:13.695936996Z", + "genesis_time": "2018-10-10T08:20:13.695936996Z", "chain_id": "tendermint_test", "validators": [ { diff --git a/consensus/common_test.go b/consensus/common_test.go index ae8bb6bfe..4f48f4424 100644 --- a/consensus/common_test.go +++ b/consensus/common_test.go @@ -615,8 +615,6 @@ func randGenesisDoc(numValidators int, randPower bool, minPower int64) (*types.G func randGenesisState(numValidators int, randPower bool, minPower int64) (sm.State, []types.PrivValidator) { genDoc, privValidators := randGenesisDoc(numValidators, randPower, minPower) s0, _ := sm.MakeGenesisState(genDoc) - db := dbm.NewMemDB() // remove this ? - sm.SaveState(db, s0) return s0, privValidators } diff --git a/consensus/mempool_test.go b/consensus/mempool_test.go index ed97ae681..3dc1cd5ff 100644 --- a/consensus/mempool_test.go +++ b/consensus/mempool_test.go @@ -10,7 +10,6 @@ import ( "github.com/tendermint/tendermint/abci/example/code" abci "github.com/tendermint/tendermint/abci/types" - "github.com/tendermint/tendermint/types" ) diff --git a/consensus/reactor.go b/consensus/reactor.go index bf6f7ba77..fc41e5734 100644 --- a/consensus/reactor.go +++ b/consensus/reactor.go @@ -8,8 +8,7 @@ import ( "github.com/pkg/errors" - "github.com/tendermint/go-amino" - + amino "github.com/tendermint/go-amino" cstypes "github.com/tendermint/tendermint/consensus/types" cmn "github.com/tendermint/tendermint/libs/common" tmevents "github.com/tendermint/tendermint/libs/events" @@ -205,6 +204,13 @@ func (conR *ConsensusReactor) Receive(chID byte, src p2p.Peer, msgBytes []byte) conR.Switch.StopPeerForError(src, err) return } + + if err = msg.ValidateBasic(); err != nil { + conR.Logger.Error("Peer sent us invalid msg", "peer", src, "msg", msg, "err", err) + conR.Switch.StopPeerForError(src, err) + return + } + conR.Logger.Debug("Receive", "src", src, "chId", chID, "msg", msg) // Get peer states @@ -242,8 +248,7 @@ func (conR *ConsensusReactor) Receive(chID byte, src p2p.Peer, msgBytes []byte) case types.PrecommitType: ourVotes = votes.Precommits(msg.Round).BitArrayByBlockID(msg.BlockID) default: - conR.Logger.Error("Bad VoteSetBitsMessage field Type") - return + panic("Bad VoteSetBitsMessage field Type. Forgot to add a check in ValidateBasic?") } src.TrySend(VoteSetBitsChannel, cdc.MustMarshalBinaryBare(&VoteSetBitsMessage{ Height: msg.Height, @@ -322,8 +327,7 @@ func (conR *ConsensusReactor) Receive(chID byte, src p2p.Peer, msgBytes []byte) case types.PrecommitType: ourVotes = votes.Precommits(msg.Round).BitArrayByBlockID(msg.BlockID) default: - conR.Logger.Error("Bad VoteSetBitsMessage field Type") - return + panic("Bad VoteSetBitsMessage field Type. Forgot to add a check in ValidateBasic?") } ps.ApplyVoteSetBitsMessage(msg, ourVotes) } else { @@ -440,9 +444,9 @@ func (conR *ConsensusReactor) broadcastHasVoteMessage(vote *types.Vote) { func makeRoundStepMessage(rs *cstypes.RoundState) (nrsMsg *NewRoundStepMessage) { nrsMsg = &NewRoundStepMessage{ - Height: rs.Height, - Round: rs.Round, - Step: rs.Step, + Height: rs.Height, + Round: rs.Round, + Step: rs.Step, SecondsSinceStartTime: int(time.Since(rs.StartTime).Seconds()), LastCommitRound: rs.LastCommit.Round(), } @@ -1349,7 +1353,9 @@ func (ps *PeerState) StringIndented(indent string) string { // Messages // ConsensusMessage is a message that can be sent and received on the ConsensusReactor -type ConsensusMessage interface{} +type ConsensusMessage interface { + ValidateBasic() error +} func RegisterConsensusMessages(cdc *amino.Codec) { cdc.RegisterInterface((*ConsensusMessage)(nil), nil) @@ -1385,6 +1391,27 @@ type NewRoundStepMessage struct { LastCommitRound int } +// ValidateBasic performs basic validation. +func (m *NewRoundStepMessage) ValidateBasic() error { + if m.Height < 0 { + return errors.New("Negative Height") + } + if m.Round < 0 { + return errors.New("Negative Round") + } + if !m.Step.IsValid() { + return errors.New("Invalid Step") + } + + // NOTE: SecondsSinceStartTime may be negative + + if (m.Height == 1 && m.LastCommitRound != -1) || + (m.Height > 1 && m.LastCommitRound < -1) { // TODO: #2737 LastCommitRound should always be >= 0 for heights > 1 + return errors.New("Invalid LastCommitRound (for 1st block: -1, for others: >= 0)") + } + return nil +} + // String returns a string representation. func (m *NewRoundStepMessage) String() string { return fmt.Sprintf("[NewRoundStep H:%v R:%v S:%v LCR:%v]", @@ -1404,6 +1431,25 @@ type NewValidBlockMessage struct { IsCommit bool } +// ValidateBasic performs basic validation. +func (m *NewValidBlockMessage) ValidateBasic() error { + if m.Height < 0 { + return errors.New("Negative Height") + } + if m.Round < 0 { + return errors.New("Negative Round") + } + if err := m.BlockPartsHeader.ValidateBasic(); err != nil { + return fmt.Errorf("Wrong BlockPartsHeader: %v", err) + } + if m.BlockParts.Size() != m.BlockPartsHeader.Total { + return fmt.Errorf("BlockParts bit array size %d not equal to BlockPartsHeader.Total %d", + m.BlockParts.Size(), + m.BlockPartsHeader.Total) + } + return nil +} + // String returns a string representation. func (m *NewValidBlockMessage) String() string { return fmt.Sprintf("[ValidBlockMessage H:%v R:%v BP:%v BA:%v IsCommit:%v]", @@ -1417,6 +1463,11 @@ type ProposalMessage struct { Proposal *types.Proposal } +// ValidateBasic performs basic validation. +func (m *ProposalMessage) ValidateBasic() error { + return m.Proposal.ValidateBasic() +} + // String returns a string representation. func (m *ProposalMessage) String() string { return fmt.Sprintf("[Proposal %v]", m.Proposal) @@ -1431,6 +1482,20 @@ type ProposalPOLMessage struct { ProposalPOL *cmn.BitArray } +// ValidateBasic performs basic validation. +func (m *ProposalPOLMessage) ValidateBasic() error { + if m.Height < 0 { + return errors.New("Negative Height") + } + if m.ProposalPOLRound < 0 { + return errors.New("Negative ProposalPOLRound") + } + if m.ProposalPOL.Size() == 0 { + return errors.New("Empty ProposalPOL bit array") + } + return nil +} + // String returns a string representation. func (m *ProposalPOLMessage) String() string { return fmt.Sprintf("[ProposalPOL H:%v POLR:%v POL:%v]", m.Height, m.ProposalPOLRound, m.ProposalPOL) @@ -1445,6 +1510,20 @@ type BlockPartMessage struct { Part *types.Part } +// ValidateBasic performs basic validation. +func (m *BlockPartMessage) ValidateBasic() error { + if m.Height < 0 { + return errors.New("Negative Height") + } + if m.Round < 0 { + return errors.New("Negative Round") + } + if err := m.Part.ValidateBasic(); err != nil { + return fmt.Errorf("Wrong Part: %v", err) + } + return nil +} + // String returns a string representation. func (m *BlockPartMessage) String() string { return fmt.Sprintf("[BlockPart H:%v R:%v P:%v]", m.Height, m.Round, m.Part) @@ -1457,6 +1536,11 @@ type VoteMessage struct { Vote *types.Vote } +// ValidateBasic performs basic validation. +func (m *VoteMessage) ValidateBasic() error { + return m.Vote.ValidateBasic() +} + // String returns a string representation. func (m *VoteMessage) String() string { return fmt.Sprintf("[Vote %v]", m.Vote) @@ -1472,6 +1556,23 @@ type HasVoteMessage struct { Index int } +// ValidateBasic performs basic validation. +func (m *HasVoteMessage) ValidateBasic() error { + if m.Height < 0 { + return errors.New("Negative Height") + } + if m.Round < 0 { + return errors.New("Negative Round") + } + if !types.IsVoteTypeValid(m.Type) { + return errors.New("Invalid Type") + } + if m.Index < 0 { + return errors.New("Negative Index") + } + return nil +} + // String returns a string representation. func (m *HasVoteMessage) String() string { return fmt.Sprintf("[HasVote VI:%v V:{%v/%02d/%v}]", m.Index, m.Height, m.Round, m.Type) @@ -1487,6 +1588,23 @@ type VoteSetMaj23Message struct { BlockID types.BlockID } +// ValidateBasic performs basic validation. +func (m *VoteSetMaj23Message) ValidateBasic() error { + if m.Height < 0 { + return errors.New("Negative Height") + } + if m.Round < 0 { + return errors.New("Negative Round") + } + if !types.IsVoteTypeValid(m.Type) { + return errors.New("Invalid Type") + } + if err := m.BlockID.ValidateBasic(); err != nil { + return fmt.Errorf("Wrong BlockID: %v", err) + } + return nil +} + // String returns a string representation. func (m *VoteSetMaj23Message) String() string { return fmt.Sprintf("[VSM23 %v/%02d/%v %v]", m.Height, m.Round, m.Type, m.BlockID) @@ -1503,6 +1621,24 @@ type VoteSetBitsMessage struct { Votes *cmn.BitArray } +// ValidateBasic performs basic validation. +func (m *VoteSetBitsMessage) ValidateBasic() error { + if m.Height < 0 { + return errors.New("Negative Height") + } + if m.Round < 0 { + return errors.New("Negative Round") + } + if !types.IsVoteTypeValid(m.Type) { + return errors.New("Invalid Type") + } + if err := m.BlockID.ValidateBasic(); err != nil { + return fmt.Errorf("Wrong BlockID: %v", err) + } + // NOTE: Votes.Size() can be zero if the node does not have any + return nil +} + // String returns a string representation. func (m *VoteSetBitsMessage) String() string { return fmt.Sprintf("[VSB %v/%02d/%v %v %v]", m.Height, m.Round, m.Type, m.BlockID, m.Votes) @@ -1515,6 +1651,11 @@ type ProposalHeartbeatMessage struct { Heartbeat *types.Heartbeat } +// ValidateBasic performs basic validation. +func (m *ProposalHeartbeatMessage) ValidateBasic() error { + return m.Heartbeat.ValidateBasic() +} + // String returns a string representation. func (m *ProposalHeartbeatMessage) String() string { return fmt.Sprintf("[HEARTBEAT %v]", m.Heartbeat) diff --git a/consensus/replay.go b/consensus/replay.go index fcff877fd..abc43eb57 100644 --- a/consensus/replay.go +++ b/consensus/replay.go @@ -264,8 +264,12 @@ func (h *Handshaker) Handshake(proxyApp proxy.AppConns) error { // Replay all blocks since appBlockHeight and ensure the result matches the current state. // Returns the final AppHash or an error. -func (h *Handshaker) ReplayBlocks(state sm.State, appHash []byte, appBlockHeight int64, proxyApp proxy.AppConns) ([]byte, error) { - +func (h *Handshaker) ReplayBlocks( + state sm.State, + appHash []byte, + appBlockHeight int64, + proxyApp proxy.AppConns, +) ([]byte, error) { storeBlockHeight := h.store.Height() stateBlockHeight := state.LastBlockHeight h.logger.Info("ABCI Replay Blocks", "appHeight", appBlockHeight, "storeHeight", storeBlockHeight, "stateHeight", stateBlockHeight) diff --git a/consensus/types/round_state.go b/consensus/types/round_state.go index d3f6468bf..ef4236118 100644 --- a/consensus/types/round_state.go +++ b/consensus/types/round_state.go @@ -26,8 +26,15 @@ const ( RoundStepPrecommitWait = RoundStepType(0x07) // Did receive any +2/3 precommits, start timeout RoundStepCommit = RoundStepType(0x08) // Entered commit state machine // NOTE: RoundStepNewHeight acts as RoundStepCommitWait. + + // NOTE: Update IsValid method if you change this! ) +// IsValid returns true if the step is valid, false if unknown/undefined. +func (rs RoundStepType) IsValid() bool { + return uint8(rs) >= 0x01 && uint8(rs) <= 0x08 +} + // String returns a string func (rs RoundStepType) String() string { switch rs { diff --git a/crypto/crypto.go b/crypto/crypto.go index 2462b0a98..b3526f881 100644 --- a/crypto/crypto.go +++ b/crypto/crypto.go @@ -6,6 +6,7 @@ import ( ) const ( + // AddressSize is the size of a pubkey address. AddressSize = tmhash.TruncatedSize ) diff --git a/evidence/reactor.go b/evidence/reactor.go index 52eb4a56f..32753b2b9 100644 --- a/evidence/reactor.go +++ b/evidence/reactor.go @@ -74,6 +74,13 @@ func (evR *EvidenceReactor) Receive(chID byte, src p2p.Peer, msgBytes []byte) { evR.Switch.StopPeerForError(src, err) return } + + if err = msg.ValidateBasic(); err != nil { + evR.Logger.Error("Peer sent us invalid msg", "peer", src, "msg", msg, "err", err) + evR.Switch.StopPeerForError(src, err) + return + } + evR.Logger.Debug("Receive", "src", src, "chId", chID, "msg", msg) switch msg := msg.(type) { @@ -191,7 +198,9 @@ type PeerState interface { // Messages // EvidenceMessage is a message sent or received by the EvidenceReactor. -type EvidenceMessage interface{} +type EvidenceMessage interface { + ValidateBasic() error +} func RegisterEvidenceMessages(cdc *amino.Codec) { cdc.RegisterInterface((*EvidenceMessage)(nil), nil) @@ -209,11 +218,21 @@ func decodeMsg(bz []byte) (msg EvidenceMessage, err error) { //------------------------------------- -// EvidenceMessage contains a list of evidence. +// EvidenceListMessage contains a list of evidence. type EvidenceListMessage struct { Evidence []types.Evidence } +// ValidateBasic performs basic validation. +func (m *EvidenceListMessage) ValidateBasic() error { + for i, ev := range m.Evidence { + if err := ev.ValidateBasic(); err != nil { + return fmt.Errorf("Invalid evidence (#%d): %v", i, err) + } + } + return nil +} + // String returns a string representation of the EvidenceListMessage. func (m *EvidenceListMessage) String() string { return fmt.Sprintf("[EvidenceListMessage %v]", m.Evidence) diff --git a/p2p/pex/addrbook.go b/p2p/pex/addrbook.go index e0c0e0b9c..61710bbf2 100644 --- a/p2p/pex/addrbook.go +++ b/p2p/pex/addrbook.go @@ -648,6 +648,10 @@ func (a *addrBook) addAddress(addr, src *p2p.NetAddress) error { return ErrAddrBookNonRoutable{addr} } + if !addr.Valid() { + return ErrAddrBookInvalidAddr{addr} + } + // TODO: we should track ourAddrs by ID and by IP:PORT and refuse both. if _, ok := a.ourAddrs[addr.String()]; ok { return ErrAddrBookSelf{addr} diff --git a/p2p/pex/errors.go b/p2p/pex/errors.go index 7f660bdc5..fbee748ac 100644 --- a/p2p/pex/errors.go +++ b/p2p/pex/errors.go @@ -46,3 +46,11 @@ type ErrAddrBookNilAddr struct { func (err ErrAddrBookNilAddr) Error() string { return fmt.Sprintf("Cannot add a nil address. Got (addr, src) = (%v, %v)", err.Addr, err.Src) } + +type ErrAddrBookInvalidAddr struct { + Addr *p2p.NetAddress +} + +func (err ErrAddrBookInvalidAddr) Error() string { + return fmt.Sprintf("Cannot add invalid address %v", err.Addr) +} diff --git a/p2p/pex/pex_reactor.go b/p2p/pex/pex_reactor.go index c919794ab..46a12c488 100644 --- a/p2p/pex/pex_reactor.go +++ b/p2p/pex/pex_reactor.go @@ -288,21 +288,37 @@ func (r *PEXReactor) RequestAddrs(p Peer) { func (r *PEXReactor) ReceiveAddrs(addrs []*p2p.NetAddress, src Peer) error { id := string(src.ID()) if !r.requestsSent.Has(id) { - return cmn.NewError("Received unsolicited pexAddrsMessage") + return errors.New("Unsolicited pexAddrsMessage") } r.requestsSent.Delete(id) srcAddr := src.NodeInfo().NetAddress() for _, netAddr := range addrs { - // NOTE: GetSelection methods should never return nil addrs + // Validate netAddr. Disconnect from a peer if it sends us invalid data. if netAddr == nil { - return cmn.NewError("received nil addr") + return errors.New("nil address in pexAddrsMessage") + } + // TODO: extract validating logic from NewNetAddressStringWithOptionalID + // and put it in netAddr#Valid (#2722) + na, err := p2p.NewNetAddressString(netAddr.String()) + if err != nil { + return fmt.Errorf("%s address in pexAddrsMessage is invalid: %v", + netAddr.String(), + err, + ) } - err := r.book.AddAddress(netAddr, srcAddr) - r.logErrAddrBook(err) + // NOTE: we check netAddr validity and routability in book#AddAddress. + err = r.book.AddAddress(na, srcAddr) + if err != nil { + r.logErrAddrBook(err) + // XXX: should we be strict about incoming data and disconnect from a + // peer here too? + continue + } - // If this address came from a seed node, try to connect to it without waiting. + // If this address came from a seed node, try to connect to it without + // waiting. for _, seedAddr := range r.seedAddrs { if seedAddr.Equals(srcAddr) { r.ensurePeers() diff --git a/state/validation.go b/state/validation.go index 345224843..e28d40e8b 100644 --- a/state/validation.go +++ b/state/validation.go @@ -21,22 +21,19 @@ func validateBlock(stateDB dbm.DB, state State, block *types.Block) error { // Validate basic info. if block.Version != state.Version.Consensus { - return fmt.Errorf( - "Wrong Block.Header.Version. Expected %v, got %v", + return fmt.Errorf("Wrong Block.Header.Version. Expected %v, got %v", state.Version.Consensus, block.Version, ) } if block.ChainID != state.ChainID { - return fmt.Errorf( - "Wrong Block.Header.ChainID. Expected %v, got %v", + return fmt.Errorf("Wrong Block.Header.ChainID. Expected %v, got %v", state.ChainID, block.ChainID, ) } if block.Height != state.LastBlockHeight+1 { - return fmt.Errorf( - "Wrong Block.Header.Height. Expected %v, got %v", + return fmt.Errorf("Wrong Block.Header.Height. Expected %v, got %v", state.LastBlockHeight+1, block.Height, ) @@ -44,16 +41,15 @@ func validateBlock(stateDB dbm.DB, state State, block *types.Block) error { // Validate prev block info. if !block.LastBlockID.Equals(state.LastBlockID) { - return fmt.Errorf( - "Wrong Block.Header.LastBlockID. Expected %v, got %v", + return fmt.Errorf("Wrong Block.Header.LastBlockID. Expected %v, got %v", state.LastBlockID, block.LastBlockID, ) } + newTxs := int64(len(block.Data.Txs)) if block.TotalTxs != state.LastBlockTotalTx+newTxs { - return fmt.Errorf( - "Wrong Block.Header.TotalTxs. Expected %v, got %v", + return fmt.Errorf("Wrong Block.Header.TotalTxs. Expected %v, got %v", state.LastBlockTotalTx+newTxs, block.TotalTxs, ) @@ -61,46 +57,44 @@ func validateBlock(stateDB dbm.DB, state State, block *types.Block) error { // Validate app info if !bytes.Equal(block.AppHash, state.AppHash) { - return fmt.Errorf( - "Wrong Block.Header.AppHash. Expected %X, got %v", + return fmt.Errorf("Wrong Block.Header.AppHash. Expected %X, got %v", state.AppHash, block.AppHash, ) } if !bytes.Equal(block.ConsensusHash, state.ConsensusParams.Hash()) { - return fmt.Errorf( - "Wrong Block.Header.ConsensusHash. Expected %X, got %v", + return fmt.Errorf("Wrong Block.Header.ConsensusHash. Expected %X, got %v", state.ConsensusParams.Hash(), block.ConsensusHash, ) } if !bytes.Equal(block.LastResultsHash, state.LastResultsHash) { - return fmt.Errorf( - "Wrong Block.Header.LastResultsHash. Expected %X, got %v", + return fmt.Errorf("Wrong Block.Header.LastResultsHash. Expected %X, got %v", state.LastResultsHash, block.LastResultsHash, ) } if !bytes.Equal(block.ValidatorsHash, state.Validators.Hash()) { - return fmt.Errorf( - "Wrong Block.Header.ValidatorsHash. Expected %X, got %v", + return fmt.Errorf("Wrong Block.Header.ValidatorsHash. Expected %X, got %v", state.Validators.Hash(), block.ValidatorsHash, ) } if !bytes.Equal(block.NextValidatorsHash, state.NextValidators.Hash()) { - return fmt.Errorf("Wrong Block.Header.NextValidatorsHash. Expected %X, got %v", state.NextValidators.Hash(), block.NextValidatorsHash) + return fmt.Errorf("Wrong Block.Header.NextValidatorsHash. Expected %X, got %v", + state.NextValidators.Hash(), + block.NextValidatorsHash, + ) } // Validate block LastCommit. if block.Height == 1 { if len(block.LastCommit.Precommits) != 0 { - return errors.New("Block at height 1 (first block) should have no LastCommit precommits") + return errors.New("Block at height 1 can't have LastCommit precommits") } } else { if len(block.LastCommit.Precommits) != state.LastValidators.Size() { - return fmt.Errorf( - "Invalid block commit size. Expected %v, got %v", + return fmt.Errorf("Invalid block commit size. Expected %v, got %v", state.LastValidators.Size(), len(block.LastCommit.Precommits), ) @@ -115,8 +109,7 @@ func validateBlock(stateDB dbm.DB, state State, block *types.Block) error { // Validate block Time if block.Height > 1 { if !block.Time.After(state.LastBlockTime) { - return fmt.Errorf( - "Block time %v not greater than last block time %v", + return fmt.Errorf("Block time %v not greater than last block time %v", block.Time, state.LastBlockTime, ) @@ -124,8 +117,7 @@ func validateBlock(stateDB dbm.DB, state State, block *types.Block) error { medianTime := MedianTime(block.LastCommit, state.LastValidators) if !block.Time.Equal(medianTime) { - return fmt.Errorf( - "Invalid block time. Expected %v, got %v", + return fmt.Errorf("Invalid block time. Expected %v, got %v", medianTime, block.Time, ) @@ -133,8 +125,7 @@ func validateBlock(stateDB dbm.DB, state State, block *types.Block) error { } else if block.Height == 1 { genesisTime := state.LastBlockTime if !block.Time.Equal(genesisTime) { - return fmt.Errorf( - "Block time %v is not equal to genesis time %v", + return fmt.Errorf("Block time %v is not equal to genesis time %v", block.Time, genesisTime, ) @@ -160,8 +151,7 @@ func validateBlock(stateDB dbm.DB, state State, block *types.Block) error { // a legit address and a known validator. if len(block.ProposerAddress) != crypto.AddressSize || !state.Validators.HasAddress(block.ProposerAddress) { - return fmt.Errorf( - "Block.Header.ProposerAddress, %X, is not a validator", + return fmt.Errorf("Block.Header.ProposerAddress, %X, is not a validator", block.ProposerAddress, ) } diff --git a/test/docker/Dockerfile b/test/docker/Dockerfile index 6bb320be8..1a64d4173 100644 --- a/test/docker/Dockerfile +++ b/test/docker/Dockerfile @@ -14,6 +14,7 @@ ENV GOBIN $GOPATH/bin WORKDIR $REPO # Copy in the code +# TODO: rewrite to only copy Makefile & other files? COPY . $REPO # Install the vendored dependencies @@ -21,16 +22,18 @@ COPY . $REPO RUN make get_tools RUN make get_vendor_deps -# Now copy in the code -# NOTE: this will overwrite whatever is in vendor/ -COPY . $REPO - # install ABCI CLI RUN make install_abci # install Tendermint RUN make install +RUN tendermint testnet --node-dir-prefix="mach" --v=4 --populate-persistent-peers=false --o=$REPO/test/p2p/data + +# Now copy in the code +# NOTE: this will overwrite whatever is in vendor/ +COPY . $REPO + # expose the volume for debugging VOLUME $REPO diff --git a/test/p2p/README.md b/test/p2p/README.md index 4ee3690af..956ce906c 100644 --- a/test/p2p/README.md +++ b/test/p2p/README.md @@ -19,7 +19,7 @@ docker network create --driver bridge --subnet 172.57.0.0/16 my_testnet ``` This gives us a new network with IP addresses in the rage `172.57.0.0 - 172.57.255.255`. -Peers on the network can have any IP address in this range. +Peers on the network can have any IP address in this range. For our four node network, let's pick `172.57.0.101 - 172.57.0.104`. Since we use Tendermint's default listening port of 26656, our list of seed nodes will look like: @@ -37,7 +37,7 @@ for i in $(seq 1 4); do --ip="172.57.0.$((100 + $i))" \ --name local_testnet_$i \ --entrypoint tendermint \ - -e TMHOME=/go/src/github.com/tendermint/tendermint/test/p2p/data/mach$i/core \ + -e TMHOME=/go/src/github.com/tendermint/tendermint/test/p2p/data/mach$((i-1)) \ tendermint_tester node --p2p.persistent_peers 172.57.0.101:26656,172.57.0.102:26656,172.57.0.103:26656,172.57.0.104:26656 --proxy_app=kvstore done ``` @@ -47,8 +47,5 @@ If you now run `docker ps`, you'll see your containers! We can confirm they are making blocks by checking the `/status` message using `curl` and `jq` to pretty print the output json: ``` -curl 172.57.0.101:26657/status | jq . +curl 172.57.0.101:26657/status | jq . ``` - - - diff --git a/test/p2p/data/mach1/core/config/genesis.json b/test/p2p/data/mach1/core/config/genesis.json deleted file mode 100644 index 515c10714..000000000 --- a/test/p2p/data/mach1/core/config/genesis.json +++ /dev/null @@ -1,39 +0,0 @@ -{ - "genesis_time": "2016-06-24T20:01:19.322Z", - "chain_id": "chain-9ujDWI", - "validators": [ - { - "pub_key": { - "type": "tendermint/PubKeyEd25519", - "value": "vokz3/FgDAJuNHGPF4Wkzeq5DDVpizlOOLaUeukd4RY=" - }, - "power": "1", - "name": "mach1" - }, - { - "pub_key": { - "type": "tendermint/PubKeyEd25519", - "value": "bcU0RlMjEmWH0qKpO1nWibcXBzsd6WiiWm7xPVlTGK0=" - }, - "power": "1", - "name": "mach2" - }, - { - "pub_key": { - "type": "tendermint/PubKeyEd25519", - "value": "rmesaX0TWqC0YB6lfqqz/r9Lqk8inEWlmMKYWxL80aE=" - }, - "power": "1", - "name": "mach3" - }, - { - "pub_key": { - "type": "tendermint/PubKeyEd25519", - "value": "nryPWM7UtG3NWrirpZHdJTzXy1A3Jz/aMrwLZGHE79k=" - }, - "power": "1", - "name": "mach4" - } - ], - "app_hash": "" -} diff --git a/test/p2p/data/mach1/core/config/node_key.json b/test/p2p/data/mach1/core/config/node_key.json deleted file mode 100644 index 4fa960850..000000000 --- a/test/p2p/data/mach1/core/config/node_key.json +++ /dev/null @@ -1,6 +0,0 @@ -{ - "priv_key": { - "type": "tendermint/PrivKeyEd25519", - "value": "BpYtFp8xSrudBa5aBLRuSPD72PGDAUm0dJORDL3Kd5YJbluUzRefVFrjwoHZv1yeDj2P9xkEi2L3hJCUz/qFkQ==" - } -} diff --git a/test/p2p/data/mach1/core/config/priv_validator.json b/test/p2p/data/mach1/core/config/priv_validator.json deleted file mode 100644 index ea2a01f5c..000000000 --- a/test/p2p/data/mach1/core/config/priv_validator.json +++ /dev/null @@ -1,14 +0,0 @@ -{ - "address": "AE47BBD4B3ACD80BFE17F6E0C66C5B8492A81AE4", - "pub_key": { - "type": "tendermint/PubKeyEd25519", - "value": "vokz3/FgDAJuNHGPF4Wkzeq5DDVpizlOOLaUeukd4RY=" - }, - "last_height": "0", - "last_round": "0", - "last_step": 0, - "priv_key": { - "type": "tendermint/PrivKeyEd25519", - "value": "VHqgfHqM4WxcsqQMbCbRWwoylgQQqfHqblC2NvGrOJq+iTPf8WAMAm40cY8XhaTN6rkMNWmLOU44tpR66R3hFg==" - } -} diff --git a/test/p2p/data/mach2/core/config/genesis.json b/test/p2p/data/mach2/core/config/genesis.json deleted file mode 100644 index 515c10714..000000000 --- a/test/p2p/data/mach2/core/config/genesis.json +++ /dev/null @@ -1,39 +0,0 @@ -{ - "genesis_time": "2016-06-24T20:01:19.322Z", - "chain_id": "chain-9ujDWI", - "validators": [ - { - "pub_key": { - "type": "tendermint/PubKeyEd25519", - "value": "vokz3/FgDAJuNHGPF4Wkzeq5DDVpizlOOLaUeukd4RY=" - }, - "power": "1", - "name": "mach1" - }, - { - "pub_key": { - "type": "tendermint/PubKeyEd25519", - "value": "bcU0RlMjEmWH0qKpO1nWibcXBzsd6WiiWm7xPVlTGK0=" - }, - "power": "1", - "name": "mach2" - }, - { - "pub_key": { - "type": "tendermint/PubKeyEd25519", - "value": "rmesaX0TWqC0YB6lfqqz/r9Lqk8inEWlmMKYWxL80aE=" - }, - "power": "1", - "name": "mach3" - }, - { - "pub_key": { - "type": "tendermint/PubKeyEd25519", - "value": "nryPWM7UtG3NWrirpZHdJTzXy1A3Jz/aMrwLZGHE79k=" - }, - "power": "1", - "name": "mach4" - } - ], - "app_hash": "" -} diff --git a/test/p2p/data/mach2/core/config/node_key.json b/test/p2p/data/mach2/core/config/node_key.json deleted file mode 100644 index 6eb151106..000000000 --- a/test/p2p/data/mach2/core/config/node_key.json +++ /dev/null @@ -1,6 +0,0 @@ -{ - "priv_key": { - "type": "tendermint/PrivKeyEd25519", - "value": "uM6LDVE4wQIIUmq9rc6RxzX8zEGG4G4Jcuw15klzQopF68YfJM4bkbPSavurEcJ4nvBMusKBg2GcARFrZqnFKA==" - } -} diff --git a/test/p2p/data/mach2/core/config/priv_validator.json b/test/p2p/data/mach2/core/config/priv_validator.json deleted file mode 100644 index 6e0cd7f8f..000000000 --- a/test/p2p/data/mach2/core/config/priv_validator.json +++ /dev/null @@ -1,14 +0,0 @@ -{ - "address": "5D61EE46CCE91F579086522D7FD8CEC3F854E946", - "pub_key": { - "type": "tendermint/PubKeyEd25519", - "value": "bcU0RlMjEmWH0qKpO1nWibcXBzsd6WiiWm7xPVlTGK0=" - }, - "last_height": "0", - "last_round": "0", - "last_step": 0, - "priv_key": { - "type": "tendermint/PrivKeyEd25519", - "value": "0EeInmBQL8MSnQq38zSxg47Z7R7Nmcu5a3GtWr9agUNtxTRGUyMSZYfSoqk7WdaJtxcHOx3paKJabvE9WVMYrQ==" - } -} diff --git a/test/p2p/data/mach3/core/config/genesis.json b/test/p2p/data/mach3/core/config/genesis.json deleted file mode 100644 index 515c10714..000000000 --- a/test/p2p/data/mach3/core/config/genesis.json +++ /dev/null @@ -1,39 +0,0 @@ -{ - "genesis_time": "2016-06-24T20:01:19.322Z", - "chain_id": "chain-9ujDWI", - "validators": [ - { - "pub_key": { - "type": "tendermint/PubKeyEd25519", - "value": "vokz3/FgDAJuNHGPF4Wkzeq5DDVpizlOOLaUeukd4RY=" - }, - "power": "1", - "name": "mach1" - }, - { - "pub_key": { - "type": "tendermint/PubKeyEd25519", - "value": "bcU0RlMjEmWH0qKpO1nWibcXBzsd6WiiWm7xPVlTGK0=" - }, - "power": "1", - "name": "mach2" - }, - { - "pub_key": { - "type": "tendermint/PubKeyEd25519", - "value": "rmesaX0TWqC0YB6lfqqz/r9Lqk8inEWlmMKYWxL80aE=" - }, - "power": "1", - "name": "mach3" - }, - { - "pub_key": { - "type": "tendermint/PubKeyEd25519", - "value": "nryPWM7UtG3NWrirpZHdJTzXy1A3Jz/aMrwLZGHE79k=" - }, - "power": "1", - "name": "mach4" - } - ], - "app_hash": "" -} diff --git a/test/p2p/data/mach3/core/config/node_key.json b/test/p2p/data/mach3/core/config/node_key.json deleted file mode 100644 index 0885bcf9c..000000000 --- a/test/p2p/data/mach3/core/config/node_key.json +++ /dev/null @@ -1,6 +0,0 @@ -{ - "priv_key": { - "type": "tendermint/PrivKeyEd25519", - "value": "kT3orG0YkipT9rAZbvAjtGk/7Pu1ZeCE8LSUF2jz2uiSs1rdlUVi/gccRlvCRLKvrtSicOyEkmk0FHPOGS3mgg==" - } -} diff --git a/test/p2p/data/mach3/core/config/priv_validator.json b/test/p2p/data/mach3/core/config/priv_validator.json deleted file mode 100644 index ec68ca7bb..000000000 --- a/test/p2p/data/mach3/core/config/priv_validator.json +++ /dev/null @@ -1,14 +0,0 @@ -{ - "address": "705F9DA2CC7D7AF5F4519455ED99622E40E439A1", - "pub_key": { - "type": "tendermint/PubKeyEd25519", - "value": "rmesaX0TWqC0YB6lfqqz/r9Lqk8inEWlmMKYWxL80aE=" - }, - "last_height": "0", - "last_round": "0", - "last_step": 0, - "priv_key": { - "type": "tendermint/PrivKeyEd25519", - "value": "waTkfzSfxfVW9Kmie6d2uUQkwxK6ps9u5EuGc0jXw/KuZ6xpfRNaoLRgHqV+qrP+v0uqTyKcRaWYwphbEvzRoQ==" - } -} diff --git a/test/p2p/data/mach4/core/config/genesis.json b/test/p2p/data/mach4/core/config/genesis.json deleted file mode 100644 index 515c10714..000000000 --- a/test/p2p/data/mach4/core/config/genesis.json +++ /dev/null @@ -1,39 +0,0 @@ -{ - "genesis_time": "2016-06-24T20:01:19.322Z", - "chain_id": "chain-9ujDWI", - "validators": [ - { - "pub_key": { - "type": "tendermint/PubKeyEd25519", - "value": "vokz3/FgDAJuNHGPF4Wkzeq5DDVpizlOOLaUeukd4RY=" - }, - "power": "1", - "name": "mach1" - }, - { - "pub_key": { - "type": "tendermint/PubKeyEd25519", - "value": "bcU0RlMjEmWH0qKpO1nWibcXBzsd6WiiWm7xPVlTGK0=" - }, - "power": "1", - "name": "mach2" - }, - { - "pub_key": { - "type": "tendermint/PubKeyEd25519", - "value": "rmesaX0TWqC0YB6lfqqz/r9Lqk8inEWlmMKYWxL80aE=" - }, - "power": "1", - "name": "mach3" - }, - { - "pub_key": { - "type": "tendermint/PubKeyEd25519", - "value": "nryPWM7UtG3NWrirpZHdJTzXy1A3Jz/aMrwLZGHE79k=" - }, - "power": "1", - "name": "mach4" - } - ], - "app_hash": "" -} diff --git a/test/p2p/data/mach4/core/config/node_key.json b/test/p2p/data/mach4/core/config/node_key.json deleted file mode 100644 index d6a5d79c2..000000000 --- a/test/p2p/data/mach4/core/config/node_key.json +++ /dev/null @@ -1,6 +0,0 @@ -{ - "priv_key": { - "type": "tendermint/PrivKeyEd25519", - "value": "QIIm8/QEEawiJi3Zozv+J9b+1CufCEkGs3lxGMlRy4L4FVIXCoXJTwYIrotZtwoMqLYEqQV1hbKKJmFA3GFelw==" - } -} diff --git a/test/p2p/data/mach4/core/config/priv_validator.json b/test/p2p/data/mach4/core/config/priv_validator.json deleted file mode 100644 index 468550ea8..000000000 --- a/test/p2p/data/mach4/core/config/priv_validator.json +++ /dev/null @@ -1,14 +0,0 @@ -{ - "address": "D1054266EC9EEA511ED9A76DEFD520BBE1B5E850", - "pub_key": { - "type": "tendermint/PubKeyEd25519", - "value": "nryPWM7UtG3NWrirpZHdJTzXy1A3Jz/aMrwLZGHE79k=" - }, - "last_height": "0", - "last_round": "0", - "last_step": 0, - "priv_key": { - "type": "tendermint/PrivKeyEd25519", - "value": "xMw+0o8CDC29qYvNvwjDztNwRw508l6TjV0pXo49KwyevI9YztS0bc1auKulkd0lPNfLUDcnP9oyvAtkYcTv2Q==" - } -} diff --git a/test/p2p/ip_plus_id.sh b/test/p2p/ip_plus_id.sh index 0d2248fe0..95871d3f1 100755 --- a/test/p2p/ip_plus_id.sh +++ b/test/p2p/ip_plus_id.sh @@ -3,5 +3,5 @@ set -eu ID=$1 DOCKER_IMAGE=$2 -NODEID="$(docker run --rm -e TMHOME=/go/src/github.com/tendermint/tendermint/test/p2p/data/mach$ID/core $DOCKER_IMAGE tendermint show_node_id)" +NODEID="$(docker run --rm -e TMHOME=/go/src/github.com/tendermint/tendermint/test/p2p/data/mach$((ID-1)) $DOCKER_IMAGE tendermint show_node_id)" echo "$NODEID@172.57.0.$((100+$ID))" diff --git a/test/p2p/peer.sh b/test/p2p/peer.sh index ad04d000f..63d46f8d5 100644 --- a/test/p2p/peer.sh +++ b/test/p2p/peer.sh @@ -15,13 +15,15 @@ echo "starting tendermint peer ID=$ID" # NOTE: $NODE_FLAGS should be unescaped (no quotes). otherwise it will be # treated as one flag. +# test/p2p/data/mach$((ID-1)) data is generated in test/docker/Dockerfile using +# the tendermint testnet command. if [[ "$ID" == "x" ]]; then # Set "x" to "1" to print to console. docker run \ --net="$NETWORK_NAME" \ --ip=$(test/p2p/ip.sh "$ID") \ --name "local_testnet_$ID" \ --entrypoint tendermint \ - -e TMHOME="/go/src/github.com/tendermint/tendermint/test/p2p/data/mach$ID/core" \ + -e TMHOME="/go/src/github.com/tendermint/tendermint/test/p2p/data/mach$((ID-1))" \ -e GOMAXPROCS=1 \ --log-driver=syslog \ --log-opt syslog-address=udp://127.0.0.1:5514 \ @@ -34,7 +36,7 @@ else --ip=$(test/p2p/ip.sh "$ID") \ --name "local_testnet_$ID" \ --entrypoint tendermint \ - -e TMHOME="/go/src/github.com/tendermint/tendermint/test/p2p/data/mach$ID/core" \ + -e TMHOME="/go/src/github.com/tendermint/tendermint/test/p2p/data/mach$((ID-1))" \ -e GOMAXPROCS=1 \ --log-driver=syslog \ --log-opt syslog-address=udp://127.0.0.1:5514 \ diff --git a/test/p2p/pex/test_addrbook.sh b/test/p2p/pex/test_addrbook.sh index 9c58db30c..06f9212fd 100644 --- a/test/p2p/pex/test_addrbook.sh +++ b/test/p2p/pex/test_addrbook.sh @@ -18,7 +18,7 @@ echo "1. restart peer $ID" docker stop "local_testnet_$ID" echo "stopped local_testnet_$ID" # preserve addrbook.json -docker cp "local_testnet_$ID:/go/src/github.com/tendermint/tendermint/test/p2p/data/mach1/core/config/addrbook.json" "/tmp/addrbook.json" +docker cp "local_testnet_$ID:/go/src/github.com/tendermint/tendermint/test/p2p/data/mach0/config/addrbook.json" "/tmp/addrbook.json" set +e #CIRCLE docker rm -vf "local_testnet_$ID" set -e @@ -32,11 +32,11 @@ bash test/p2p/client.sh "$DOCKER_IMAGE" "$NETWORK_NAME" "$CLIENT_NAME" "test/p2p # Now we know that the node is up. -docker cp "/tmp/addrbook.json" "local_testnet_$ID:/go/src/github.com/tendermint/tendermint/test/p2p/data/mach1/core/config/addrbook.json" +docker cp "/tmp/addrbook.json" "local_testnet_$ID:/go/src/github.com/tendermint/tendermint/test/p2p/data/mach0/config/addrbook.json" echo "with the following addrbook:" cat /tmp/addrbook.json # exec doesn't work on circle -# docker exec "local_testnet_$ID" cat "/go/src/github.com/tendermint/tendermint/test/p2p/data/mach1/core/config/addrbook.json" +# docker exec "local_testnet_$ID" cat "/go/src/github.com/tendermint/tendermint/test/p2p/data/mach0/config/addrbook.json" echo "" echo "----------------------------------------------------------------------" diff --git a/types/block.go b/types/block.go index 46ad73a71..4ae51d4df 100644 --- a/types/block.go +++ b/types/block.go @@ -2,12 +2,14 @@ package types import ( "bytes" - "errors" "fmt" "strings" "sync" "time" + "github.com/pkg/errors" + + "github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/crypto/merkle" cmn "github.com/tendermint/tendermint/libs/common" "github.com/tendermint/tendermint/version" @@ -57,54 +59,117 @@ func MakeBlock(height int64, txs []Tx, lastCommit *Commit, evidence []Evidence) // ValidateBasic performs basic validation that doesn't involve state data. // It checks the internal consistency of the block. +// Further validation is done using state#ValidateBlock. func (b *Block) ValidateBasic() error { if b == nil { - return errors.New("Nil blocks are invalid") + return errors.New("nil block") } b.mtx.Lock() defer b.mtx.Unlock() + if len(b.ChainID) > MaxChainIDLen { + return fmt.Errorf("ChainID is too long. Max is %d, got %d", MaxChainIDLen, len(b.ChainID)) + } + if b.Height < 0 { - return fmt.Errorf( - "Negative Block.Header.Height: %v", - b.Height, - ) + return errors.New("Negative Header.Height") + } else if b.Height == 0 { + return errors.New("Zero Header.Height") } + // NOTE: Timestamp validation is subtle and handled elsewhere. + newTxs := int64(len(b.Data.Txs)) if b.NumTxs != newTxs { - return fmt.Errorf( - "Wrong Block.Header.NumTxs. Expected %v, got %v", + return fmt.Errorf("Wrong Header.NumTxs. Expected %v, got %v", newTxs, b.NumTxs, ) } + + // TODO: fix tests so we can do this + /*if b.TotalTxs < b.NumTxs { + return fmt.Errorf("Header.TotalTxs (%d) is less than Header.NumTxs (%d)", b.TotalTxs, b.NumTxs) + }*/ + if b.TotalTxs < 0 { + return errors.New("Negative Header.TotalTxs") + } + + if err := b.LastBlockID.ValidateBasic(); err != nil { + return fmt.Errorf("Wrong Header.LastBlockID: %v", err) + } + + // Validate the last commit and its hash. + if b.Header.Height > 1 { + if b.LastCommit == nil { + return errors.New("nil LastCommit") + } + if err := b.LastCommit.ValidateBasic(); err != nil { + return fmt.Errorf("Wrong LastCommit") + } + } + if err := ValidateHash(b.LastCommitHash); err != nil { + return fmt.Errorf("Wrong Header.LastCommitHash: %v", err) + } if !bytes.Equal(b.LastCommitHash, b.LastCommit.Hash()) { - return fmt.Errorf( - "Wrong Block.Header.LastCommitHash. Expected %v, got %v", - b.LastCommitHash, + return fmt.Errorf("Wrong Header.LastCommitHash. Expected %v, got %v", b.LastCommit.Hash(), + b.LastCommitHash, ) } - if b.Header.Height != 1 { - if err := b.LastCommit.ValidateBasic(); err != nil { - return err - } + + // Validate the hash of the transactions. + // NOTE: b.Data.Txs may be nil, but b.Data.Hash() + // still works fine + if err := ValidateHash(b.DataHash); err != nil { + return fmt.Errorf("Wrong Header.DataHash: %v", err) } if !bytes.Equal(b.DataHash, b.Data.Hash()) { return fmt.Errorf( - "Wrong Block.Header.DataHash. Expected %v, got %v", - b.DataHash, + "Wrong Header.DataHash. Expected %v, got %v", b.Data.Hash(), + b.DataHash, ) } + + // Basic validation of hashes related to application data. + // Will validate fully against state in state#ValidateBlock. + if err := ValidateHash(b.ValidatorsHash); err != nil { + return fmt.Errorf("Wrong Header.ValidatorsHash: %v", err) + } + if err := ValidateHash(b.NextValidatorsHash); err != nil { + return fmt.Errorf("Wrong Header.NextValidatorsHash: %v", err) + } + if err := ValidateHash(b.ConsensusHash); err != nil { + return fmt.Errorf("Wrong Header.ConsensusHash: %v", err) + } + // NOTE: AppHash is arbitrary length + if err := ValidateHash(b.LastResultsHash); err != nil { + return fmt.Errorf("Wrong Header.LastResultsHash: %v", err) + } + + // Validate evidence and its hash. + if err := ValidateHash(b.EvidenceHash); err != nil { + return fmt.Errorf("Wrong Header.EvidenceHash: %v", err) + } + // NOTE: b.Evidence.Evidence may be nil, but we're just looping. + for i, ev := range b.Evidence.Evidence { + if err := ev.ValidateBasic(); err != nil { + return fmt.Errorf("Invalid evidence (#%d): %v", i, err) + } + } if !bytes.Equal(b.EvidenceHash, b.Evidence.Hash()) { - return fmt.Errorf( - "Wrong Block.Header.EvidenceHash. Expected %v, got %v", + return fmt.Errorf("Wrong Header.EvidenceHash. Expected %v, got %v", b.EvidenceHash, b.Evidence.Hash(), ) } + + if len(b.ProposerAddress) != crypto.AddressSize { + return fmt.Errorf("Expected len(Header.ProposerAddress) to be %d, got %d", + crypto.AddressSize, len(b.ProposerAddress)) + } + return nil } @@ -719,6 +784,18 @@ func (blockID BlockID) Key() string { return string(blockID.Hash) + string(bz) } +// ValidateBasic performs basic validation. +func (blockID BlockID) ValidateBasic() error { + // Hash can be empty in case of POLBlockID in Proposal. + if err := ValidateHash(blockID.Hash); err != nil { + return fmt.Errorf("Wrong Hash") + } + if err := blockID.PartsHeader.ValidateBasic(); err != nil { + return fmt.Errorf("Wrong PartsHeader: %v", err) + } + return nil +} + // String returns a human readable string representation of the BlockID func (blockID BlockID) String() string { return fmt.Sprintf(`%v:%v`, blockID.Hash, blockID.PartsHeader) diff --git a/types/block_test.go b/types/block_test.go index 46881a099..cdea293f0 100644 --- a/types/block_test.go +++ b/types/block_test.go @@ -80,11 +80,13 @@ func TestBlockValidateBasic(t *testing.T) { blk.EvidenceHash = []byte("something else") }, true}, } - for _, tc := range testCases { + for i, tc := range testCases { t.Run(tc.testName, func(t *testing.T) { block := MakeBlock(h, txs, commit, evList) + block.ProposerAddress = valSet.GetProposer().Address tc.malleateBlock(block) - assert.Equal(t, tc.expErr, block.ValidateBasic() != nil, "ValidateBasic had an unexpected result") + err = block.ValidateBasic() + assert.Equal(t, tc.expErr, err != nil, "#%d: %v", i, err) }) } } diff --git a/types/evidence.go b/types/evidence.go index d1e15c819..fb2423458 100644 --- a/types/evidence.go +++ b/types/evidence.go @@ -4,6 +4,7 @@ import ( "bytes" "fmt" + "github.com/pkg/errors" "github.com/tendermint/tendermint/crypto/tmhash" amino "github.com/tendermint/go-amino" @@ -60,6 +61,7 @@ type Evidence interface { Verify(chainID string, pubKey crypto.PubKey) error // verify the evidence Equal(Evidence) bool // check equality of evidence + ValidateBasic() error String() string } @@ -172,6 +174,23 @@ func (dve *DuplicateVoteEvidence) Equal(ev Evidence) bool { return bytes.Equal(dveHash, evHash) } +// ValidateBasic performs basic validation. +func (dve *DuplicateVoteEvidence) ValidateBasic() error { + if len(dve.PubKey.Bytes()) == 0 { + return errors.New("Empty PubKey") + } + if dve.VoteA == nil || dve.VoteB == nil { + return fmt.Errorf("One or both of the votes are empty %v, %v", dve.VoteA, dve.VoteB) + } + if err := dve.VoteA.ValidateBasic(); err != nil { + return fmt.Errorf("Invalid VoteA: %v", err) + } + if err := dve.VoteB.ValidateBasic(); err != nil { + return fmt.Errorf("Invalid VoteB: %v", err) + } + return nil +} + //----------------------------------------------------------------- // UNSTABLE @@ -201,6 +220,7 @@ func (e MockGoodEvidence) Equal(ev Evidence) bool { return e.Height_ == e2.Height_ && bytes.Equal(e.Address_, e2.Address_) } +func (e MockGoodEvidence) ValidateBasic() error { return nil } func (e MockGoodEvidence) String() string { return fmt.Sprintf("GoodEvidence: %d/%s", e.Height_, e.Address_) } @@ -218,6 +238,7 @@ func (e MockBadEvidence) Equal(ev Evidence) bool { return e.Height_ == e2.Height_ && bytes.Equal(e.Address_, e2.Address_) } +func (e MockBadEvidence) ValidateBasic() error { return nil } func (e MockBadEvidence) String() string { return fmt.Sprintf("BadEvidence: %d/%s", e.Height_, e.Address_) } diff --git a/types/heartbeat.go b/types/heartbeat.go index 9dea039e0..986e9384f 100644 --- a/types/heartbeat.go +++ b/types/heartbeat.go @@ -3,6 +3,8 @@ package types import ( "fmt" + "github.com/pkg/errors" + "github.com/tendermint/tendermint/crypto" cmn "github.com/tendermint/tendermint/libs/common" ) @@ -50,3 +52,32 @@ func (heartbeat *Heartbeat) String() string { heartbeat.Height, heartbeat.Round, heartbeat.Sequence, fmt.Sprintf("/%X.../", cmn.Fingerprint(heartbeat.Signature[:]))) } + +// ValidateBasic performs basic validation. +func (heartbeat *Heartbeat) ValidateBasic() error { + if len(heartbeat.ValidatorAddress) != crypto.AddressSize { + return fmt.Errorf("Expected ValidatorAddress size to be %d bytes, got %d bytes", + crypto.AddressSize, + len(heartbeat.ValidatorAddress), + ) + } + if heartbeat.ValidatorIndex < 0 { + return errors.New("Negative ValidatorIndex") + } + if heartbeat.Height < 0 { + return errors.New("Negative Height") + } + if heartbeat.Round < 0 { + return errors.New("Negative Round") + } + if heartbeat.Sequence < 0 { + return errors.New("Negative Sequence") + } + if len(heartbeat.Signature) == 0 { + return errors.New("Signature is missing") + } + if len(heartbeat.Signature) > MaxSignatureSize { + return fmt.Errorf("Signature is too big (max: %d)", MaxSignatureSize) + } + return nil +} diff --git a/types/part_set.go b/types/part_set.go index 812b1c2fd..af59851c9 100644 --- a/types/part_set.go +++ b/types/part_set.go @@ -2,11 +2,12 @@ package types import ( "bytes" - "errors" "fmt" "io" "sync" + "github.com/pkg/errors" + "github.com/tendermint/tendermint/crypto/merkle" "github.com/tendermint/tendermint/crypto/tmhash" cmn "github.com/tendermint/tendermint/libs/common" @@ -36,6 +37,17 @@ func (part *Part) Hash() []byte { return part.hash } +// ValidateBasic performs basic validation. +func (part *Part) ValidateBasic() error { + if part.Index < 0 { + return errors.New("Negative Index") + } + if len(part.Bytes) > BlockPartSizeBytes { + return fmt.Errorf("Too big (max: %d)", BlockPartSizeBytes) + } + return nil +} + func (part *Part) String() string { return part.StringIndented("") } @@ -70,6 +82,18 @@ func (psh PartSetHeader) Equals(other PartSetHeader) bool { return psh.Total == other.Total && bytes.Equal(psh.Hash, other.Hash) } +// ValidateBasic performs basic validation. +func (psh PartSetHeader) ValidateBasic() error { + if psh.Total < 0 { + return errors.New("Negative Total") + } + // Hash can be empty in case of POLBlockID.PartsHeader in Proposal. + if err := ValidateHash(psh.Hash); err != nil { + return errors.Wrap(err, "Wrong Hash") + } + return nil +} + //------------------------------------- type PartSet struct { diff --git a/types/proposal.go b/types/proposal.go index 09cfd1967..f3b62aae7 100644 --- a/types/proposal.go +++ b/types/proposal.go @@ -43,6 +43,35 @@ func NewProposal(height int64, round int, polRound int, blockID BlockID) *Propos } } +// ValidateBasic performs basic validation. +func (p *Proposal) ValidateBasic() error { + if p.Type != ProposalType { + return errors.New("Invalid Type") + } + if p.Height < 0 { + return errors.New("Negative Height") + } + if p.Round < 0 { + return errors.New("Negative Round") + } + if p.POLRound < -1 { + return errors.New("Negative POLRound (exception: -1)") + } + if err := p.BlockID.ValidateBasic(); err != nil { + return fmt.Errorf("Wrong BlockID: %v", err) + } + + // NOTE: Timestamp validation is subtle and handled elsewhere. + + if len(p.Signature) == 0 { + return errors.New("Signature is missing") + } + if len(p.Signature) > MaxSignatureSize { + return fmt.Errorf("Signature is too big (max: %d)", MaxSignatureSize) + } + return nil +} + // String returns a string representation of the Proposal. func (p *Proposal) String() string { return fmt.Sprintf("Proposal{%v/%v (%v, %v) %X @ %s}", diff --git a/types/signable.go b/types/signable.go index cc6498882..baabdff08 100644 --- a/types/signable.go +++ b/types/signable.go @@ -1,5 +1,17 @@ package types +import ( + "github.com/tendermint/tendermint/crypto/ed25519" + cmn "github.com/tendermint/tendermint/libs/common" +) + +var ( + // MaxSignatureSize is a maximum allowed signature size for the Heartbeat, + // Proposal and Vote. + // XXX: secp256k1 does not have Size nor MaxSize defined. + MaxSignatureSize = cmn.MaxInt(ed25519.SignatureSize, 64) +) + // Signable is an interface for all signable things. // It typically removes signatures before serializing. // SignBytes returns the bytes to be signed diff --git a/types/signed_msg_type.go b/types/signed_msg_type.go index cc3ddbdc1..10e7c70c0 100644 --- a/types/signed_msg_type.go +++ b/types/signed_msg_type.go @@ -15,11 +15,10 @@ const ( HeartbeatType SignedMsgType = 0x30 ) -func IsVoteTypeValid(type_ SignedMsgType) bool { - switch type_ { - case PrevoteType: - return true - case PrecommitType: +// IsVoteTypeValid returns true if t is a valid vote type. +func IsVoteTypeValid(t SignedMsgType) bool { + switch t { + case PrevoteType, PrecommitType: return true default: return false diff --git a/types/validation.go b/types/validation.go new file mode 100644 index 000000000..1271cfd94 --- /dev/null +++ b/types/validation.go @@ -0,0 +1,40 @@ +package types + +import ( + "fmt" + "time" + + "github.com/tendermint/tendermint/crypto/tmhash" + tmtime "github.com/tendermint/tendermint/types/time" +) + +// ValidateTime does a basic time validation ensuring time does not drift too +// much: +/- one year. +// TODO: reduce this to eg 1 day +// NOTE: DO NOT USE in ValidateBasic methods in this package. This function +// can only be used for real time validation, like on proposals and votes +// in the consensus. If consensus is stuck, and rounds increase for more than a day, +// having only a 1-day band here could break things... +// Can't use for validating blocks because we may be syncing years worth of history. +func ValidateTime(t time.Time) error { + var ( + now = tmtime.Now() + oneYear = 8766 * time.Hour + ) + if t.Before(now.Add(-oneYear)) || t.After(now.Add(oneYear)) { + return fmt.Errorf("Time drifted too much. Expected: -1 < %v < 1 year", now) + } + return nil +} + +// ValidateHash returns an error if the hash is not empty, but its +// size != tmhash.Size. +func ValidateHash(h []byte) error { + if len(h) > 0 && len(h) != tmhash.Size { + return fmt.Errorf("Expected size to be %d bytes, got %d bytes", + tmhash.Size, + len(h), + ) + } + return nil +} diff --git a/types/vote.go b/types/vote.go index 1d7e9cf6f..bf14d403b 100644 --- a/types/vote.go +++ b/types/vote.go @@ -46,7 +46,8 @@ func NewConflictingVoteError(val *Validator, voteA, voteB *Vote) *ErrVoteConflic // Address is hex bytes. type Address = crypto.Address -// Represents a prevote, precommit, or commit vote from validators for consensus. +// Vote represents a prevote, precommit, or commit vote from validators for +// consensus. type Vote struct { Type SignedMsgType `json:"type"` Height int64 `json:"height"` @@ -108,3 +109,38 @@ func (vote *Vote) Verify(chainID string, pubKey crypto.PubKey) error { } return nil } + +// ValidateBasic performs basic validation. +func (vote *Vote) ValidateBasic() error { + if !IsVoteTypeValid(vote.Type) { + return errors.New("Invalid Type") + } + if vote.Height < 0 { + return errors.New("Negative Height") + } + if vote.Round < 0 { + return errors.New("Negative Round") + } + + // NOTE: Timestamp validation is subtle and handled elsewhere. + + if err := vote.BlockID.ValidateBasic(); err != nil { + return fmt.Errorf("Wrong BlockID: %v", err) + } + if len(vote.ValidatorAddress) != crypto.AddressSize { + return fmt.Errorf("Expected ValidatorAddress size to be %d bytes, got %d bytes", + crypto.AddressSize, + len(vote.ValidatorAddress), + ) + } + if vote.ValidatorIndex < 0 { + return errors.New("Negative ValidatorIndex") + } + if len(vote.Signature) == 0 { + return errors.New("Signature is missing") + } + if len(vote.Signature) > MaxSignatureSize { + return fmt.Errorf("Signature is too big (max: %d)", MaxSignatureSize) + } + return nil +}