Skip to content

Conversation

@dgenr8
Copy link
Contributor

@dgenr8 dgenr8 commented Jun 19, 2015

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.

@dgenr8
Copy link
Contributor Author

dgenr8 commented Jun 19, 2015

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.

@petertodd
Copy link
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.

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.
@dgenr8 dgenr8 force-pushed the no_duplicate_askfor branch from d12b019 to 66a7146 Compare July 29, 2015 01:05
@dgenr8
Copy link
Contributor Author

dgenr8 commented Jul 29, 2015

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.

@jgarzik
Copy link
Contributor

jgarzik commented Sep 16, 2015

ut ACK - travis is unhappy

@morcos
Copy link
Contributor

morcos commented Sep 16, 2015

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.

@laanwj
Copy link
Member

laanwj commented Nov 24, 2015

Closing in favor of #7079 which continues this

@laanwj laanwj closed this Nov 24, 2015
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants