diff --git a/eth/downloader/downloader.go b/eth/downloader/downloader.go index d817b223cc..f3a8664414 100644 --- a/eth/downloader/downloader.go +++ b/eth/downloader/downloader.go @@ -28,10 +28,11 @@ var ( ) var ( - errLowTd = errors.New("peer's TD is too low") + errLowTd = errors.New("peers TD is too low") ErrBusy = errors.New("busy") - errUnknownPeer = errors.New("peer's unknown or unhealthy") + errUnknownPeer = errors.New("peer is unknown or unhealthy") ErrBadPeer = errors.New("action from bad peer ignored") + ErrStallingPeer = errors.New("peer is stalling") errNoPeers = errors.New("no peers to keep download active") ErrPendingQueue = errors.New("pending items in queue") ErrTimeout = errors.New("timeout") @@ -283,15 +284,18 @@ func (d *Downloader) fetchHashes(p *peer, h common.Hash) error { return ErrBadPeer } if !done { + // Check that the peer is not stalling the sync + if len(inserts) < maxHashFetch { + return ErrStallingPeer + } // Try and fetch a random block to verify the hash batch // Skip the last hash as the cross check races with the next hash fetch - if len(inserts) > 1 { - cross := inserts[rand.Intn(len(inserts)-1)] - glog.V(logger.Detail).Infof("Cross checking (%s) with %x", active.id, cross) + cross := inserts[rand.Intn(len(inserts)-1)] + glog.V(logger.Detail).Infof("Cross checking (%s) with %x", active.id, cross) + + d.checks[cross] = time.Now().Add(blockTTL) + active.getBlocks([]common.Hash{cross}) - d.checks[cross] = time.Now().Add(blockTTL) - active.getBlocks([]common.Hash{cross}) - } // Also fetch a fresh active.getHashes(head) continue diff --git a/eth/downloader/downloader_test.go b/eth/downloader/downloader_test.go index 19d64ac67f..8ed3289c69 100644 --- a/eth/downloader/downloader_test.go +++ b/eth/downloader/downloader_test.go @@ -53,6 +53,8 @@ type downloadTester struct { blocks map[common.Hash]*types.Block // Blocks associated with the hashes chain []common.Hash // Block-chain being constructed + maxHashFetch int // Overrides the maximum number of retrieved hashes + t *testing.T pcount int done chan bool @@ -133,8 +135,12 @@ func (dl *downloadTester) getBlock(hash common.Hash) *types.Block { // getHashes retrieves a batch of hashes for reconstructing the chain. func (dl *downloadTester) getHashes(head common.Hash) error { + limit := maxHashFetch + if dl.maxHashFetch > 0 { + limit = dl.maxHashFetch + } // Gather the next batch of hashes - hashes := make([]common.Hash, 0, maxHashFetch) + hashes := make([]common.Hash, 0, limit) for i, hash := range dl.hashes { if hash == head { i++ @@ -469,6 +475,23 @@ func TestMadeupHashChainAttack(t *testing.T) { } } +// Tests that if a malicious peer makes up a random hash chain, and tries to push +// indefinitely, one hash at a time, it actually gets caught with it. The reason +// this is separate from the classical made up chain attack is that sending hashes +// one by one prevents reliable block/parent verification. +func TestMadeupHashChainDrippingAttack(t *testing.T) { + // Create a random chain of hashes to drip + hashes := createHashes(0, 16*blockCacheLimit) + tester := newTester(t, hashes, nil) + + // Try and sync with the attacker, one hash at a time + tester.maxHashFetch = 1 + tester.newPeer("attack", big.NewInt(10000), hashes[0]) + if _, err := tester.syncTake("attack", hashes[0]); err != ErrStallingPeer { + t.Fatalf("synchronisation error mismatch: have %v, want %v", err, ErrStallingPeer) + } +} + // Tests that if a malicious peer makes up a random block chain, and tried to // push indefinitely, it actually gets caught with it. func TestMadeupBlockChainAttack(t *testing.T) {