From 000691dc9e0ec6c6f5e2323280d2b595a035f345 Mon Sep 17 00:00:00 2001 From: Dave Collins Date: Tue, 8 Jul 2014 22:01:13 -0500 Subject: [PATCH] Implement BIP0061 reject handling (pver 70002). This commit implements reject handling as defined by BIP0061 and bumps the maximum supported protocol version to 70002 accordingly. As a part of supporting this a new error type named RuleError has been introduced which encapsulates and underlying error which could be one of the existing TxRuleError or btcchain.RuleError types. This allows a single high level type assertion to be used to determine if the block or transaction was rejected due to a rule error or due to an unexpected error. Meanwhile, an appropriate reject error can be created from the error by pulling the underlying error out and using it. Also, a check for minimum protocol version of 209 has been added. Closes #133. --- blockmanager.go | 24 +++++++-- log.go | 43 ++++++++++++++++ mempool.go | 128 ++++++++++++++++++++++++++++----------------- mempoolerror.go | 134 ++++++++++++++++++++++++++++++++++++++++++++++++ peer.go | 110 +++++++++++++++++++++++++++++++++++---- rpcserver.go | 2 +- 6 files changed, 379 insertions(+), 62 deletions(-) create mode 100644 mempoolerror.go diff --git a/blockmanager.go b/blockmanager.go index e22622f3..6cee488e 100644 --- a/blockmanager.go +++ b/blockmanager.go @@ -499,12 +499,19 @@ func (b *blockManager) handleTxMsg(tmsg *txMsg) { // simply rejected as opposed to something actually going wrong, // so log it as such. Otherwise, something really did go wrong, // so log it as an actual error. - if _, ok := err.(TxRuleError); ok { - bmgrLog.Debugf("Rejected transaction %v from %s: %v", txHash, - tmsg.peer, err) + if _, ok := err.(RuleError); ok { + bmgrLog.Debugf("Rejected transaction %v from %s: %v", + txHash, tmsg.peer, err) } else { - bmgrLog.Errorf("Failed to process transaction %v: %v", txHash, err) + bmgrLog.Errorf("Failed to process transaction %v: %v", + txHash, err) } + + // Convert the error into an appropriate reject message and + // send it. + code, reason := errToRejectErr(err) + tmsg.peer.PushRejectMsg(btcwire.CmdBlock, code, reason, txHash, + false) return } } @@ -594,8 +601,15 @@ func (b *blockManager) handleBlockMsg(bmsg *blockMsg) { bmgrLog.Infof("Rejected block %v from %s: %v", blockSha, bmsg.peer, err) } else { - bmgrLog.Errorf("Failed to process block %v: %v", blockSha, err) + bmgrLog.Errorf("Failed to process block %v: %v", + blockSha, err) } + + // Convert the error into an appropriate reject message and + // send it. + code, reason := errToRejectErr(err) + bmsg.peer.PushRejectMsg(btcwire.CmdBlock, code, reason, + blockSha, false) return } diff --git a/log.go b/log.go index 44d6a4c2..6825e8b3 100644 --- a/log.go +++ b/log.go @@ -7,6 +7,7 @@ package main import ( "fmt" "os" + "strings" "time" "github.com/conformal/btcd/addrmgr" @@ -26,6 +27,10 @@ const ( // years. However, if the field is interpreted as a timestamp, given // the lock time is a uint32, the max is sometime around 2106. lockTimeThreshold uint32 = 5e8 // Tue Nov 5 00:53:20 1985 UTC + + // maxRejectReasonLen is the maximum length of a sanitized reject reason + // that will be logged. + maxRejectReasonLen = 200 ) // Loggers per subsytem. Note that backendLog is a seelog logger that all of @@ -250,6 +255,30 @@ func locatorSummary(locator []*btcwire.ShaHash, stopHash *btcwire.ShaHash) strin } +// sanitizeString strips any characters which are even remotely dangerous, such +// as html control characters, from the passed string. It also limits it to +// the passed maximum size, which can be 0 for unlimited. When the string is +// limited, it will also add "..." to the string to indicate it was truncated. +func sanitizeString(str string, maxLength uint) string { + const safeChars = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXY" + + "Z01234567890 .,;_/:?@" + + // Strip any characters not in the safeChars string removed. + str = strings.Map(func(r rune) rune { + if strings.IndexRune(safeChars, r) >= 0 { + return r + } + return -1 + }, str) + + // Limit the string to the max allowed length. + if maxLength > 0 && uint(len(str)) > maxLength { + str = str[:maxLength] + str = str + "..." + } + return str +} + // messageSummary returns a human-readable string which summarizes a message. // Not all messages have or need a summary. This is used for debug logging. func messageSummary(msg btcwire.Message) string { @@ -308,6 +337,20 @@ func messageSummary(msg btcwire.Message) string { case *btcwire.MsgHeaders: return fmt.Sprintf("num %d", len(msg.Headers)) + + case *btcwire.MsgReject: + // Ensure the variable length strings don't contain any + // characters which are even remotely dangerous such as HTML + // control characters, etc. Also limit them to sane length for + // logging. + rejCommand := sanitizeString(msg.Cmd, btcwire.CommandSize) + rejReason := sanitizeString(msg.Reason, maxRejectReasonLen) + summary := fmt.Sprintf("cmd %v, code %v, reason %v", rejCommand, + msg.Code, rejReason) + if rejCommand == btcwire.CmdBlock || rejCommand == btcwire.CmdTx { + summary += fmt.Sprintf(", hash %v", msg.Hash) + } + return summary } // No summary for other messages. diff --git a/mempool.go b/mempool.go index cc1ab3bb..e8b75c56 100644 --- a/mempool.go +++ b/mempool.go @@ -20,17 +20,6 @@ import ( "github.com/conformal/btcwire" ) -// TxRuleError identifies a rule violation. It is used to indicate that -// processing of a transaction failed due to one of the many validation -// rules. The caller can use type assertions to determine if a failure was -// specifically due to a rule violation. -type TxRuleError string - -// Error satisfies the error interface to print human-readable errors. -func (e TxRuleError) Error() string { - return string(e) -} - const ( // mempoolHeight is the height used for the "block" height field of the // contextual transaction information provided in a transaction store. @@ -182,36 +171,41 @@ func checkPkScriptStandard(pkScript []byte, scriptClass btcscript.ScriptClass) e case btcscript.MultiSigTy: numPubKeys, numSigs, err := btcscript.CalcMultiSigStats(pkScript) if err != nil { - return err + str := fmt.Sprintf("multi-signature script parse "+ + "failure: %v", err) + return txRuleError(btcwire.RejectNonstandard, str) } // A standard multi-signature public key script must contain // from 1 to maxStandardMultiSigKeys public keys. if numPubKeys < 1 { - return TxRuleError("multi-signature script with no pubkeys") + str := "multi-signature script with no pubkeys" + return txRuleError(btcwire.RejectNonstandard, str) } if numPubKeys > maxStandardMultiSigKeys { str := fmt.Sprintf("multi-signature script with %d "+ "public keys which is more than the allowed "+ "max of %d", numPubKeys, maxStandardMultiSigKeys) - return TxRuleError(str) + return txRuleError(btcwire.RejectNonstandard, str) } // A standard multi-signature public key script must have at // least 1 signature and no more signatures than available // public keys. if numSigs < 1 { - return TxRuleError("multi-signature script with no signatures") + return txRuleError(btcwire.RejectNonstandard, + "multi-signature script with no signatures") } if numSigs > numPubKeys { str := fmt.Sprintf("multi-signature script with %d "+ "signatures which is more than the available "+ "%d public keys", numSigs, numPubKeys) - return TxRuleError(str) + return txRuleError(btcwire.RejectNonstandard, str) } case btcscript.NonStandardTy: - return TxRuleError("non-standard script form") + return txRuleError(btcwire.RejectNonstandard, + "non-standard script form") } return nil @@ -232,13 +226,14 @@ func checkTransactionStandard(tx *btcutil.Tx, height int64) error { str := fmt.Sprintf("transaction version %d is not in the "+ "valid range of %d-%d", msgTx.Version, 1, btcwire.TxVersion) - return TxRuleError(str) + return txRuleError(btcwire.RejectNonstandard, str) } // The transaction must be finalized to be standard and therefore // considered for inclusion in a block. if !btcchain.IsFinalizedTransaction(tx, height, time.Now()) { - return TxRuleError("transaction is not finalized") + return txRuleError(btcwire.RejectNonstandard, + "transaction is not finalized") } // Since extremely large transactions with a lot of inputs can cost @@ -249,7 +244,7 @@ func checkTransactionStandard(tx *btcutil.Tx, height int64) error { if serializedLen > maxStandardTxSize { str := fmt.Sprintf("transaction size of %v is larger than max "+ "allowed size of %v", serializedLen, maxStandardTxSize) - return TxRuleError(str) + return txRuleError(btcwire.RejectNonstandard, str) } for i, txIn := range msgTx.TxIn { @@ -262,7 +257,7 @@ func checkTransactionStandard(tx *btcutil.Tx, height int64) error { "script size of %d bytes is large than max "+ "allowed size of %d bytes", i, sigScriptLen, maxStandardSigScriptSize) - return TxRuleError(str) + return txRuleError(btcwire.RejectNonstandard, str) } // Each transaction input signature script must only contain @@ -270,7 +265,7 @@ func checkTransactionStandard(tx *btcutil.Tx, height int64) error { if !btcscript.IsPushOnlyScript(txIn.SignatureScript) { str := fmt.Sprintf("transaction input %d: signature "+ "script is not push only", i) - return TxRuleError(str) + return txRuleError(btcwire.RejectNonstandard, str) } // Each transaction input signature script must only contain @@ -280,7 +275,7 @@ func checkTransactionStandard(tx *btcutil.Tx, height int64) error { if !btcscript.HasCanonicalPushes(txIn.SignatureScript) { str := fmt.Sprintf("transaction input %d: signature "+ "script has a non-canonical data push", i) - return TxRuleError(str) + return txRuleError(btcwire.RejectNonstandard, str) } } @@ -291,8 +286,15 @@ func checkTransactionStandard(tx *btcutil.Tx, height int64) error { scriptClass := btcscript.GetScriptClass(txOut.PkScript) err := checkPkScriptStandard(txOut.PkScript, scriptClass) if err != nil { + // Attempt to extract a reject code from the error so + // it can be retained. When not possible, fall back to + // a non standard error. + rejectCode, found := extractRejectCode(err) + if !found { + rejectCode = btcwire.RejectNonstandard + } str := fmt.Sprintf("transaction output %d: %v", i, err) - return TxRuleError(str) + return txRuleError(rejectCode, str) } // Accumulate the number of outputs which only carry data. @@ -303,15 +305,15 @@ func checkTransactionStandard(tx *btcutil.Tx, height int64) error { if isDust(txOut) { str := fmt.Sprintf("transaction output %d: payment "+ "of %d is dust", i, txOut.Value) - return TxRuleError(str) + return txRuleError(btcwire.RejectDust, str) } } // A standard transaction must not have more than one output script that // only carries data. if numNullDataOutputs > 1 { - return TxRuleError("more than one transaction output is a " + - "nulldata script") + str := "more than one transaction output in a nulldata script" + return txRuleError(btcwire.RejectNonstandard, str) } return nil @@ -341,7 +343,9 @@ func checkInputsStandard(tx *btcutil.Tx, txStore btcchain.TxStore) error { scriptInfo, err := btcscript.CalcScriptInfo(txIn.SignatureScript, originPkScript, true) if err != nil { - return err + str := fmt.Sprintf("transaction input #%d script parse "+ + "failure: %v", i, err) + return txRuleError(btcwire.RejectNonstandard, str) } // A negative value for expected inputs indicates the script is @@ -349,7 +353,7 @@ func checkInputsStandard(tx *btcutil.Tx, txStore btcchain.TxStore) error { if scriptInfo.ExpectedInputs < 0 { str := fmt.Sprintf("transaction input #%d expects %d "+ "inputs", i, scriptInfo.ExpectedInputs) - return TxRuleError(str) + return txRuleError(btcwire.RejectNonstandard, str) } // The script pair is non-standard if the number of available @@ -359,7 +363,7 @@ func checkInputsStandard(tx *btcutil.Tx, txStore btcchain.TxStore) error { "inputs, but referenced output script provides "+ "%d", i, scriptInfo.ExpectedInputs, scriptInfo.NumInputs) - return TxRuleError(str) + return txRuleError(btcwire.RejectNonstandard, str) } } @@ -511,7 +515,7 @@ func (mp *txMemPool) maybeAddOrphan(tx *btcutil.Tx) error { str := fmt.Sprintf("orphan transaction size of %d bytes is "+ "larger than max allowed size of %d bytes", serializedLen, maxOrphanTxSize) - return TxRuleError(str) + return txRuleError(btcwire.RejectNonstandard, str) } // Add the orphan if the none of the above disqualified it. @@ -677,7 +681,7 @@ func (mp *txMemPool) checkPoolDoubleSpend(tx *btcutil.Tx) error { if txR, exists := mp.outpoints[txIn.PreviousOutpoint]; exists { str := fmt.Sprintf("transaction %v in the pool "+ "already spends the same coins", txR.Sha()) - return TxRuleError(str) + return txRuleError(btcwire.RejectDuplicate, str) } } @@ -744,7 +748,7 @@ func (mp *txMemPool) maybeAcceptTransaction(tx *btcutil.Tx, isOrphan *bool, isNe // be a quick check to weed out duplicates. if mp.haveTransaction(txHash) { str := fmt.Sprintf("already have transaction %v", txHash) - return TxRuleError(str) + return txRuleError(btcwire.RejectDuplicate, str) } // Perform preliminary sanity checks on the transaction. This makes @@ -752,8 +756,8 @@ func (mp *txMemPool) maybeAcceptTransaction(tx *btcutil.Tx, isOrphan *bool, isNe // transactions are allowed into blocks. err := btcchain.CheckTransactionSanity(tx) if err != nil { - if _, ok := err.(btcchain.RuleError); ok { - return TxRuleError(err.Error()) + if cerr, ok := err.(btcchain.RuleError); ok { + return chainRuleError(cerr) } return err } @@ -762,7 +766,7 @@ func (mp *txMemPool) maybeAcceptTransaction(tx *btcutil.Tx, isOrphan *bool, isNe if btcchain.IsCoinBase(tx) { str := fmt.Sprintf("transaction %v is an individual coinbase", txHash) - return TxRuleError(str) + return txRuleError(btcwire.RejectInvalid, str) } // Don't accept transactions with a lock time after the maximum int32 @@ -772,7 +776,7 @@ func (mp *txMemPool) maybeAcceptTransaction(tx *btcutil.Tx, isOrphan *bool, isNe if tx.MsgTx().LockTime > math.MaxInt32 { str := fmt.Sprintf("transaction %v has a lock time after "+ "2038 which is not accepted yet", txHash) - return TxRuleError(str) + return txRuleError(btcwire.RejectNonstandard, str) } // Get the current height of the main chain. A standalone transaction @@ -780,6 +784,8 @@ func (mp *txMemPool) maybeAcceptTransaction(tx *btcutil.Tx, isOrphan *bool, isNe // one more than the current height. _, curHeight, err := mp.server.db.NewestSha() if err != nil { + // This is an unexpected error so don't turn it into a rule + // error. return err } nextBlockHeight := curHeight + 1 @@ -789,9 +795,16 @@ func (mp *txMemPool) maybeAcceptTransaction(tx *btcutil.Tx, isOrphan *bool, isNe if !activeNetParams.RelayNonStdTxs { err := checkTransactionStandard(tx, nextBlockHeight) if err != nil { - str := fmt.Sprintf("transaction %v is not a standard "+ - "transaction: %v", txHash, err) - return TxRuleError(str) + // Attempt to extract a reject code from the error so + // it can be retained. When not possible, fall back to + // a non standard error. + rejectCode, found := extractRejectCode(err) + if !found { + rejectCode = btcwire.RejectNonstandard + } + str := fmt.Sprintf("transaction %v is not standard: %v", + txHash, err) + return txRuleError(rejectCode, str) } } @@ -814,6 +827,9 @@ func (mp *txMemPool) maybeAcceptTransaction(tx *btcutil.Tx, isOrphan *bool, isNe // needing to do a separate lookup. txStore, err := mp.fetchInputTransactions(tx) if err != nil { + if cerr, ok := err.(btcchain.RuleError); ok { + return chainRuleError(cerr) + } return err } @@ -822,7 +838,8 @@ func (mp *txMemPool) maybeAcceptTransaction(tx *btcutil.Tx, isOrphan *bool, isNe if txD, exists := txStore[*txHash]; exists && txD.Err == nil { for _, isOutputSpent := range txD.Spent { if !isOutputSpent { - return TxRuleError("transaction already exists") + return txRuleError(btcwire.RejectDuplicate, + "transaction already exists") } } } @@ -844,8 +861,8 @@ func (mp *txMemPool) maybeAcceptTransaction(tx *btcutil.Tx, isOrphan *bool, isNe // used later. txFee, err := btcchain.CheckTransactionInputs(tx, nextBlockHeight, txStore) if err != nil { - if _, ok := err.(btcchain.RuleError); ok { - return TxRuleError(err.Error()) + if cerr, ok := err.(btcchain.RuleError); ok { + return chainRuleError(cerr) } return err } @@ -855,9 +872,16 @@ func (mp *txMemPool) maybeAcceptTransaction(tx *btcutil.Tx, isOrphan *bool, isNe if !activeNetParams.RelayNonStdTxs { err := checkInputsStandard(tx, txStore) if err != nil { + // Attempt to extract a reject code from the error so + // it can be retained. When not possible, fall back to + // a non standard error. + rejectCode, found := extractRejectCode(err) + if !found { + rejectCode = btcwire.RejectNonstandard + } str := fmt.Sprintf("transaction %v has a non-standard "+ "input: %v", txHash, err) - return TxRuleError(str) + return txRuleError(rejectCode, str) } } @@ -871,7 +895,7 @@ func (mp *txMemPool) maybeAcceptTransaction(tx *btcutil.Tx, isOrphan *bool, isNe str := fmt.Sprintf("transaction %v has %d fees which is under "+ "the required amount of %d", txHash, txFee, minRequiredFee) - return TxRuleError(str) + return txRuleError(btcwire.RejectInsufficientFee, str) } // Free-to-relay transactions are rate limited here to prevent @@ -888,7 +912,7 @@ func (mp *txMemPool) maybeAcceptTransaction(tx *btcutil.Tx, isOrphan *bool, isNe if mp.pennyTotal >= cfg.FreeTxRelayLimit*10*1000 { str := fmt.Sprintf("transaction %v has 0 fees and has "+ "been rejected by the rate limiter", txHash) - return TxRuleError(str) + return txRuleError(btcwire.RejectInsufficientFee, str) } oldTotal := mp.pennyTotal @@ -903,6 +927,9 @@ func (mp *txMemPool) maybeAcceptTransaction(tx *btcutil.Tx, isOrphan *bool, isNe err = btcchain.ValidateTransactionScripts(tx, txStore, standardScriptVerifyFlags) if err != nil { + if cerr, ok := err.(btcchain.RuleError); ok { + return chainRuleError(cerr) + } return err } @@ -1036,7 +1063,14 @@ func (mp *txMemPool) ProcessTransaction(tx *btcutil.Tx, allowOrphan, rateLimit b // The transaction is an orphan (has inputs missing). Reject // it if the flag to allow orphans is not set. if !allowOrphan { - return TxRuleError("transaction spends unknown inputs") + // NOTE: RejectDuplicate is really not an accurate + // reject code here, but it matches the reference + // implementation and there isn't a better choice due + // to the limited number of reject codes. Missing + // inputs is assumed to mean they are already spent + // which is not really always the case. + return txRuleError(btcwire.RejectDuplicate, + "transaction spends unknown inputs") } // Potentially add the orphan transaction to the orphan pool. diff --git a/mempoolerror.go b/mempoolerror.go new file mode 100644 index 00000000..338548f4 --- /dev/null +++ b/mempoolerror.go @@ -0,0 +1,134 @@ +// Copyright (c) 2014 Conformal Systems LLC. +// Use of this source code is governed by an ISC +// license that can be found in the LICENSE file. + +package main + +import ( + "github.com/conformal/btcchain" + "github.com/conformal/btcwire" +) + +// RuleError identifies a rule violation. It is used to indicate that +// processing of a transaction failed due to one of the many validation +// rules. The caller can use type assertions to determine if a failure was +// specifically due to a rule violation and use the Err field to access the +// underlying error, which will be either a TxRuleError or a btcchain.RuleError. +type RuleError struct { + Err error +} + +// Error satisfies the error interface and prints human-readable errors. +func (e RuleError) Error() string { + if e.Err == nil { + return "" + } + return e.Err.Error() +} + +// TxRuleError identifies a rule violation. It is used to indicate that +// processing of a transaction failed due to one of the many validation +// rules. The caller can use type assertions to determine if a failure was +// specifically due to a rule violation and access the ErrorCode field to +// ascertain the specific reason for the rule violation. +type TxRuleError struct { + RejectCode btcwire.RejectCode // The code to send with reject messages + Description string // Human readable description of the issue +} + +// Error satisfies the error interface and prints human-readable errors. +func (e TxRuleError) Error() string { + return e.Description +} + +// txRuleError creates an underlying TxRuleError with the given a set of +// arguments and returns a RuleError that encapsulates it. +func txRuleError(c btcwire.RejectCode, desc string) RuleError { + return RuleError{ + Err: TxRuleError{RejectCode: c, Description: desc}, + } +} + +// chainRuleError returns a RuleError that encapsulates the given +// btcchain.RuleError. +func chainRuleError(chainErr btcchain.RuleError) RuleError { + return RuleError{ + Err: chainErr, + } +} + +// extractRejectCode attempts to return a relevant reject code for a given error +// by examining the error for known types. It will return true if a code +// was successfully extracted. +func extractRejectCode(err error) (btcwire.RejectCode, bool) { + // Pull the underlying error out of a RuleError. + if rerr, ok := err.(RuleError); ok { + err = rerr.Err + } + + switch err := err.(type) { + case btcchain.RuleError: + // Convert the chain error to a reject code. + var code btcwire.RejectCode + switch err.ErrorCode { + // Rejected due to duplicate. + case btcchain.ErrDuplicateBlock: + fallthrough + case btcchain.ErrDoubleSpend: + code = btcwire.RejectDuplicate + + // Rejected due to obsolete version. + case btcchain.ErrBlockVersionTooOld: + code = btcwire.RejectObsolete + + // Rejected due to checkpoint. + case btcchain.ErrCheckpointTimeTooOld: + fallthrough + case btcchain.ErrDifficultyTooLow: + fallthrough + case btcchain.ErrBadCheckpoint: + fallthrough + case btcchain.ErrForkTooOld: + code = btcwire.RejectCheckpoint + + // Everything else is due to the block or transaction being invalid. + default: + code = btcwire.RejectInvalid + } + + return code, true + + case TxRuleError: + return err.RejectCode, true + + case nil: + return btcwire.RejectInvalid, false + } + + return btcwire.RejectInvalid, false +} + +// errToRejectErr examines the underlying type of the error and returns a reject +// code and string appropriate to be sent in a btcwire.MsgReject message. +func errToRejectErr(err error) (btcwire.RejectCode, string) { + // Return the reject code along with the error text if it can be + // extracted from the error. + rejectCode, found := extractRejectCode(err) + if found { + return rejectCode, err.Error() + } + + // Return a generic rejected string if there is no error. This really + // should not happen unless the code elsewhere is not setting an error + // as it should be, but it's best to be safe and simply return a generic + // string rather than allowing the following code that derferences the + // err to panic. + if err == nil { + return btcwire.RejectInvalid, "rejected" + } + + // When the underlying error is not one of the above cases, just return + // btcwire.RejectInvalid with a generic rejected string plus the error + // text. + return btcwire.RejectInvalid, "rejected: " + err.Error() +} diff --git a/peer.go b/peer.go index ea37f9e7..7cf8c01b 100644 --- a/peer.go +++ b/peer.go @@ -8,6 +8,7 @@ import ( "bytes" "container/list" "fmt" + "io" "net" "strconv" "sync" @@ -26,7 +27,7 @@ import ( const ( // maxProtocolVersion is the max protocol version the peer supports. - maxProtocolVersion = 70001 + maxProtocolVersion = 70002 // outputBufferSize is the number of elements the output channels use. outputBufferSize = 50 @@ -353,12 +354,35 @@ func (p *peer) handleVersionMsg(msg *btcwire.MsgVersion) { return } - p.StatsMtx.Lock() // Updating a bunch of stats. + // Notify and disconnect clients that have a protocol version that is + // too old. + if msg.ProtocolVersion < int32(btcwire.MultipleAddressVersion) { + // Send a reject message indicating the protocol version is + // obsolete and wait for the message to be sent before + // disconnecting. + reason := fmt.Sprintf("protocol version must be %d or greater", + btcwire.MultipleAddressVersion) + p.PushRejectMsg(msg.Command(), btcwire.RejectObsolete, reason, + nil, true) + p.Disconnect() + return + } + + // Updating a bunch of stats. + p.StatsMtx.Lock() + // Limit to one version message per peer. if p.versionKnown { p.logError("Only one version message per peer is allowed %s.", p) p.StatsMtx.Unlock() + + // Send an reject message indicating the version message was + // incorrectly sent twice and wait for the message to be sent + // before disconnecting. + p.PushRejectMsg(msg.Command(), btcwire.RejectDuplicate, + "duplicate version message", nil, true) + p.Disconnect() return } @@ -653,6 +677,40 @@ func (p *peer) PushGetHeadersMsg(locator btcchain.BlockLocator, stopHash *btcwir return nil } +// PushRejectMsg sends a reject message for the provided command, reject code, +// and reject reason, and hash. The hash will only be used when the command +// is a tx or block and should be nil in other cases. The wait parameter will +// cause the function to block until the reject message has actually been sent. +func (p *peer) PushRejectMsg(command string, code btcwire.RejectCode, reason string, hash *btcwire.ShaHash, wait bool) { + // Don't bother sending the reject message if the protocol version + // is too low. + if p.VersionKnown() && p.ProtocolVersion() < btcwire.RejectVersion { + return + } + + msg := btcwire.NewMsgReject(command, code, reason) + if command == btcwire.CmdTx || command == btcwire.CmdBlock { + if hash == nil { + peerLog.Warnf("Sending a reject message for command "+ + "type %v which should have specified a hash "+ + "but does not", command) + hash = &zeroHash + } + msg.Hash = *hash + } + + // Send the message without waiting if the caller has not requested it. + if !wait { + p.QueueMessage(msg, nil) + return + } + + // Send the message and block until it has been sent before returning. + doneChan := make(chan struct{}, 1) + p.QueueMessage(msg, doneChan) + <-doneChan +} + // handleMemPoolMsg is invoked when a peer receives a mempool bitcoin message. // It creates and sends an inventory message with the contents of the memory // pool up to the maximum inventory allowed per message. When the peer has a @@ -1231,9 +1289,11 @@ func (p *peer) writeMessage(msg btcwire.Message) { switch msg.(type) { case *btcwire.MsgVersion: // This is OK. + case *btcwire.MsgReject: + // This is OK. default: - // We drop all messages other than version if we - // haven't done the handshake already. + // Drop all messages other than version and reject if + // the handshake has not already been done. return } } @@ -1332,10 +1392,32 @@ out: continue } - // Only log the error if we're not forcibly disconnecting. + // Only log the error and possibly send reject message + // if we're not forcibly disconnecting. if atomic.LoadInt32(&p.disconnect) == 0 { - p.logError("Can't read message from %s: %v", - p, err) + errMsg := fmt.Sprintf("Can't read message "+ + "from %s: %v", p, err) + p.logError(errMsg) + + // Only send the reject message if it's not + // because the remote client disconnected. + if err != io.EOF { + // Push a reject message for the + // malformed message and wait for the + // message to be sent before + // disconnecting. + // + // NOTE: Ideally this would include the + // command in the header if at least + // that much of the message was valid, + // but that is not currently exposed by + // btcwire, so just used malformed for + // the command. + p.PushRejectMsg("malformed", + btcwire.RejectMalformed, errMsg, + nil, true) + } + } break out } @@ -1344,8 +1426,14 @@ out: p.StatsMtx.Unlock() // Ensure version message comes first. - if _, ok := rmsg.(*btcwire.MsgVersion); !ok && !p.VersionKnown() { - p.logError("A version message must precede all others") + if vmsg, ok := rmsg.(*btcwire.MsgVersion); !ok && !p.VersionKnown() { + errStr := "A version message must precede all others" + p.logError(errStr) + + // Push a reject message and wait for the message to be + // sent before disconnecting. + p.PushRejectMsg(vmsg.Command(), btcwire.RejectMalformed, + errStr, nil, true) break out } @@ -1417,6 +1505,10 @@ out: case *btcwire.MsgFilterLoad: p.handleFilterLoadMsg(msg) + case *btcwire.MsgReject: + // Nothing to do currently. Logging of the rejected + // message is handled already in readMessage. + default: peerLog.Debugf("Received unhandled message of type %v: Fix Me", rmsg.Command()) diff --git a/rpcserver.go b/rpcserver.go index 22133d00..1281a29b 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -2868,7 +2868,7 @@ func handleSendRawTransaction(s *rpcServer, cmd btcjson.Cmd, closeChan <-chan st // so log it as an actual error. In both cases, a JSON-RPC // error is returned to the client with the deserialization // error code (to match bitcoind behavior). - if _, ok := err.(TxRuleError); ok { + if _, ok := err.(RuleError); ok { rpcsLog.Debugf("Rejected transaction %v: %v", tx.Sha(), err) } else {