From efc1d5127f0e037baea0189f8be3f86013788831 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Fri, 16 Oct 2015 13:05:08 +0300 Subject: [PATCH] eth/downloader: assume stale delivery vs. attack --- eth/downloader/downloader.go | 5 ----- eth/downloader/downloader_test.go | 3 ++- eth/downloader/queue.go | 25 +++++++++++++------------ 3 files changed, 15 insertions(+), 18 deletions(-) diff --git a/eth/downloader/downloader.go b/eth/downloader/downloader.go index a435e8f171..3e53aec35e 100644 --- a/eth/downloader/downloader.go +++ b/eth/downloader/downloader.go @@ -1372,11 +1372,6 @@ func (d *Downloader) fetchParts(errCancel error, deliveryCh chan dataPack, deliv // The hash chain is invalid (blocks are not ordered properly), abort return err - case errInvalidBody, errInvalidReceipt: - // The peer delivered something very bad, drop immediately - glog.V(logger.Error).Infof("%s: delivered invalid %s, dropping", peer, strings.ToLower(kind)) - d.dropPeer(peer.id) - case errNoFetchesPending: // Peer probably timed out with its delivery but came through // in the end, demote, but allow to to pull from this peer. diff --git a/eth/downloader/downloader_test.go b/eth/downloader/downloader_test.go index a1754933cb..f0af4cbaee 100644 --- a/eth/downloader/downloader_test.go +++ b/eth/downloader/downloader_test.go @@ -1127,6 +1127,7 @@ func testInvalidHeaderRollback(t *testing.T, protocol int, mode SyncMode) { assertOwnChain(t, tester, targetBlocks+1) } +/* // Tests that if a peer sends an invalid block piece (body or receipt) for a // requested block, it gets dropped immediately by the downloader. func TestInvalidContentAttack62(t *testing.T) { testInvalidContentAttack(t, 62, FullSync) } @@ -1171,7 +1172,7 @@ func testInvalidContentAttack(t *testing.T, protocol int, mode SyncMode) { if _, ok := tester.peerHashes["receipt attack"]; ok { t.Fatalf("receipt attacker not detected/dropped") } -} +}*/ // Tests that a peer advertising an high TD doesn't get to stall the downloader // afterwards by not sending any useful hashes. diff --git a/eth/downloader/queue.go b/eth/downloader/queue.go index 3eee4f7b67..e09f90107b 100644 --- a/eth/downloader/queue.go +++ b/eth/downloader/queue.go @@ -859,7 +859,10 @@ func (q *queue) deliver(id string, taskPool map[common.Hash]*types.Header, taskQ } } // Assemble each of the results with their headers and retrieved data parts - errs := make([]error, 0) + var ( + failure error + useful bool + ) for i, header := range request.Headers { // Short circuit assembly if no more fetch results are found if i >= results { @@ -868,15 +871,16 @@ func (q *queue) deliver(id string, taskPool map[common.Hash]*types.Header, taskQ // Reconstruct the next result if contents match up index := int(header.Number.Int64() - int64(q.resultOffset)) if index >= len(q.resultCache) || index < 0 || q.resultCache[index] == nil { - errs = []error{errInvalidChain} + failure = errInvalidChain break } if err := reconstruct(header, i, q.resultCache[index]); err != nil { - errs = []error{err} + failure = err break } donePool[header.Hash()] = struct{}{} q.resultCache[index].Pending-- + useful = true // Clean up a successful fetch request.Headers[i] = nil @@ -888,19 +892,16 @@ func (q *queue) deliver(id string, taskPool map[common.Hash]*types.Header, taskQ taskQueue.Push(header, -float32(header.Number.Uint64())) } } - // If none of the blocks were good, it's a stale delivery + // If none of the data was good, it's a stale delivery switch { - case len(errs) == 0: - return nil + case failure == nil || failure == errInvalidChain: + return failure - case len(errs) == 1 && (errs[0] == errInvalidChain || errs[0] == errInvalidBody || errs[0] == errInvalidReceipt): - return errs[0] - - case len(errs) == results: - return errStaleDelivery + case useful: + return fmt.Errorf("partial failure: %v", failure) default: - return fmt.Errorf("multiple failures: %v", errs) + return errStaleDelivery } }