Prevent peer flooding inv request queue (redux)#6306
Prevent peer flooding inv request queue (redux)#6306dgenr8 wants to merge 1 commit intobitcoin:masterfrom
Conversation
|
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. |
|
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. |
This is a lighter implementation of dcd7ef7 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.
d12b019 to
66a7146
Compare
|
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. |
|
ut ACK - travis is unhappy |
|
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. |
|
Closing in favor of #7079 which continues this |
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.