Prevent peer flooding inv request queue (redux) #6306

1. 
   dgenr8 commented at 6:42 am on June 19, 2015: contributor

   Removes a network attacker node’s ability to indefinitely blind its peers to a block or transaction new to them, such as a double-spend generated by attacker. The possible blinding interval is reduced to the getdata timeout (currently 2 minutes).

   This vulnerability is discussed in Tampering with the Delivery of Blocks and Transactions in Bitcoin [1] and was described earlier in Discovering Bitcoin’s Public Topology and Influential Nodes [2].

   This is a lighter implementation of #4547. Attention is paid to the result of the insert into the existing collection setInventoryKnown, rather than introducing a new collection. #4547 was closed to focus on a wider solution that has been delayed.

2. 
   dgenr8 commented at 4:25 pm on June 19, 2015: contributor

   The targeted 1-conf attack “Double-Spending of 1-Confirmation Transactions” described in [1] appears to be a real risk. The vulnerability is reduced to a known level per attacker node by this bug fix.

   Attacker blinds both funds receiver and a small miner to a 1-conf double spend, and the block containing it (attacker must be the first to inv the block to these nodes). If the small miner produces a block, funds receiver believes he has 1 confirmation.

3. laanwj added the label Bug on Jun 19, 2015
4. laanwj added the label Priority Medium on Jun 19, 2015
5. 
   petertodd commented at 2:08 am on June 30, 2015: contributor

   It’d be good to add a demo script and unittest of the attack to this patch to make it easier to test it. The qa/rpc-tests has a mininode implementation that you could use.
6. Prevent peer flooding inv request queue (redux)

   This is a lighter implementation of dcd7ef782bd8105c3427f932206e33fa07ddeded
   from @kazcw.  The effect is the same.  Here is the original description:
   
   mapAlreadyAskedFor does not keep track of which peer has a request queued for a
   particular tx. As a result, a peer can blind a node to a tx indefinitely by
   sending many invs for the same tx, and then never replying to getdatas for it.
   Each inv received will be placed 2 minutes farther back in mapAlreadyAskedFor,
   so a short message containing 10 invs would render that tx unavailable for 20
   minutes.
   
   This is fixed by disallowing a peer from having more than one entry for a
   particular inv in mapAlreadyAskedFor at a time.

   66a7146ac2
7. dgenr8 force-pushed on Jul 29, 2015
8. 
   dgenr8 commented at 1:10 am on July 29, 2015: contributor

   One of the tests in p2p-acceptblock.py was relying on being able to push a duplicate block from a single peer. Updated that test to use another node.
9. 
   jgarzik commented at 1:47 pm on September 16, 2015: contributor

   ut ACK - travis is unhappy
10. 
    morcos commented at 7:03 pm on September 16, 2015: member

    There was discussion in #4547 against following this approach because it doesn’t fully solve the problem. I think a quick fix might be ok while we’re waiting for something more substantial (P2P rewrite or #4831) but I’m not convinced there is no downside to this pull. For example this might hinder a peer trying to resend a wallet transaction to you that you don’t currently have because of your mempool was full or it was evicted.
11. 
    laanwj commented at 9:53 am on November 24, 2015: member

    Closing in favor of #7079 which continues this
12. laanwj closed this on Nov 24, 2015

    ---

13. MarcoFalke locked this on Sep 8, 2021

Contributors
dgenr8 petertodd jgarzik morcos laanwj

Labels
Bug