New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Countermeasures against eclipse attacks #5941

Merged
merged 6 commits into from Apr 1, 2015

Conversation

Projects
None yet
7 participants
@sipa
Member

sipa commented Mar 24, 2015

This implements countermeasures 1, 2, and 6 from the http://cs-people.bu.edu/heilman/eclipse/ paper.

sipa added some commits Mar 8, 2015

Make addrman's bucket placement deterministic.
Give each address a single fixed location in the new and tried tables,
which become simple fixed-size arrays instead of sets and vectors.

This prevents attackers from having an advantages by inserting an
address multiple times.

This change was suggested as Countermeasure 1 in
Eclipse Attacks on Bitcoin’s Peer-to-Peer Network, Ethan Heilman,
Alison Kendler, Aviv Zohar, Sharon Goldberg. ePrint Archive Report
2015/263. March 2015.

It is also more efficient.
Do not bias outgoing connections towards fresh addresses
This change was suggested as Countermeasure 2 in
Eclipse Attacks on Bitcoin’s Peer-to-Peer Network, Ethan Heilman,
Alison Kendler, Aviv Zohar, Sharon Goldberg. ePrint Archive Report
2015/263. March 2015.
Always use a 50% chance to choose between tried and new entries
This change was suggested as Countermeasure 2 in
Eclipse Attacks on Bitcoin’s Peer-to-Peer Network, Ethan Heilman,
Alison Kendler, Aviv Zohar, Sharon Goldberg. ePrint Archive Report
2015/263. March 2015.
Scale up addrman
This change was suggested as Countermeasure 6 in
Eclipse Attacks on Bitcoin’s Peer-to-Peer Network, Ethan Heilman,
Alison Kendler, Aviv Zohar, Sharon Goldberg. ePrint Archive Report
2015/263. March 2015.

@laanwj laanwj added this to the 0.10.0 milestone Mar 24, 2015

@sipa

This comment has been minimized.

Show comment
Hide comment
@sipa

sipa Mar 24, 2015

Member

Someone should test how long it takes to find peers with this, when DNS seeding and the built-in seeds are disabled. I'm not able to test the next ~3 days or so.

Member

sipa commented Mar 24, 2015

Someone should test how long it takes to find peers with this, when DNS seeding and the built-in seeds are disabled. I'm not able to test the next ~3 days or so.

@sipa

This comment has been minimized.

Show comment
Hide comment
@sipa

sipa Mar 24, 2015

Member

This is running on bitcoin.sipa.be, with -DDEBUG_ADDRMAN enabled.

Member

sipa commented Mar 24, 2015

This is running on bitcoin.sipa.be, with -DDEBUG_ADDRMAN enabled.

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Mar 24, 2015

Member

Going to test this.

Someone should test how long it takes to find peers with this, when DNS seeding and the built-in seeds are disabled.

OK, disabling those.

Member

laanwj commented Mar 24, 2015

Going to test this.

Someone should test how long it takes to find peers with this, when DNS seeding and the built-in seeds are disabled.

OK, disabling those.

@jgarzik

This comment has been minimized.

Show comment
Hide comment
@jgarzik

jgarzik Mar 24, 2015

Contributor

Typically, (a) less than 11 seconds if you have a fresh addrman, or (b) sometimes hours or longer, if not.

Current code skips DNS seeds & built in seeds automatically, if it manages to connect to some peers within a short amount of time.

Contributor

jgarzik commented Mar 24, 2015

Typically, (a) less than 11 seconds if you have a fresh addrman, or (b) sometimes hours or longer, if not.

Current code skips DNS seeds & built in seeds automatically, if it manages to connect to some peers within a short amount of time.

@sipa

This comment has been minimized.

Show comment
Hide comment
@sipa

sipa Mar 24, 2015

Member

@jgarzik I mean after this patch. Reason is the bias changes in peer selection. If you have very few tried peers, the new code will likely very actively keep retrying those (as there is a 50% chance to pick a previously tried peer), and there is no bias towards more recently learned addresses anymore. Both probably slow down initial connection, but likely don't have that much impact in a working system.

Member

sipa commented Mar 24, 2015

@jgarzik I mean after this patch. Reason is the bias changes in peer selection. If you have very few tried peers, the new code will likely very actively keep retrying those (as there is a 50% chance to pick a previously tried peer), and there is no bias towards more recently learned addresses anymore. Both probably slow down initial connection, but likely don't have that much impact in a working system.

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Mar 24, 2015

Member

It instantly found a peer on start:

2015-03-24 13:55:51 init message: Loading addresses...
2015-03-24 13:55:51 Loaded 14669 addresses from peers.dat  664ms
...
2015-03-24 13:55:51 receive version message: /Satoshi:0.9.2/: version 70002, blocks=346129, ...
Member

laanwj commented Mar 24, 2015

It instantly found a peer on start:

2015-03-24 13:55:51 init message: Loading addresses...
2015-03-24 13:55:51 Loaded 14669 addresses from peers.dat  664ms
...
2015-03-24 13:55:51 receive version message: /Satoshi:0.9.2/: version 70002, blocks=346129, ...
@gavinandresen

This comment has been minimized.

Show comment
Hide comment
@gavinandresen

gavinandresen Mar 24, 2015

Contributor

Ran this with -dnsseed=0 and a peers.dat from a bitcoind that I stopped 23 hours ago:

2015-03-24 20:15:18 Loaded 12904 addresses from peers.dat  512ms
   ...
2015-03-24 20:15:30 connect() to 93.143.11.182:8333 failed after select(): Connection refused (61)
2015-03-24 20:16:15 connect() to 85.199.110.252:8333 failed after select(): Connection refused (61)
2015-03-24 20:16:16 receive version message: /Satoshi:0.10.0/: version 70002, blocks=349059, us=...., peer=1
 ...
2015-03-24 20:16:28 receive version message: /Satoshi:0.9.1/: version 70002, blocks=349059, us=..., peer=2
...
2015-03-24 20:17:09 receive version message: /Marilyn:0.9.99/: version 70002, blocks=30160, us=..., peer=3
...
2015-03-24 20:17:15 connect() to 82.130.103.162:8333 failed after select(): Connection refused (61)
...
2015-03-24 20:17:44 receive version message: /Satoshi:0.10.0/: version 70002, blocks=349059, us=..., peer=4

... so just under one minute to connect to first peer, and it looks like it is finding about a peer a minute.

Contributor

gavinandresen commented Mar 24, 2015

Ran this with -dnsseed=0 and a peers.dat from a bitcoind that I stopped 23 hours ago:

2015-03-24 20:15:18 Loaded 12904 addresses from peers.dat  512ms
   ...
2015-03-24 20:15:30 connect() to 93.143.11.182:8333 failed after select(): Connection refused (61)
2015-03-24 20:16:15 connect() to 85.199.110.252:8333 failed after select(): Connection refused (61)
2015-03-24 20:16:16 receive version message: /Satoshi:0.10.0/: version 70002, blocks=349059, us=...., peer=1
 ...
2015-03-24 20:16:28 receive version message: /Satoshi:0.9.1/: version 70002, blocks=349059, us=..., peer=2
...
2015-03-24 20:17:09 receive version message: /Marilyn:0.9.99/: version 70002, blocks=30160, us=..., peer=3
...
2015-03-24 20:17:15 connect() to 82.130.103.162:8333 failed after select(): Connection refused (61)
...
2015-03-24 20:17:44 receive version message: /Satoshi:0.10.0/: version 70002, blocks=349059, us=..., peer=4

... so just under one minute to connect to first peer, and it looks like it is finding about a peer a minute.

@21E14

This comment has been minimized.

Show comment
Hide comment
@21E14

21E14 Mar 24, 2015

Contributor

utAck. The paper does not specify a countermeasure "6" in Section 7.

We've still got a bit to go (the symptoms being #5886, #5397, #5352, #5299), but great to see the attention on P2P. Let it remind us that P2P is at the (decentralized) core of Bitcoin Core as much as transactions are; arguably more so.

Contributor

21E14 commented Mar 24, 2015

utAck. The paper does not specify a countermeasure "6" in Section 7.

We've still got a bit to go (the symptoms being #5886, #5397, #5352, #5299), but great to see the attention on P2P. Let it remind us that P2P is at the (decentralized) core of Bitcoin Core as much as transactions are; arguably more so.

@sipa

This comment has been minimized.

Show comment
Hide comment
@sipa

sipa Mar 24, 2015

Member
Member

sipa commented Mar 24, 2015

@21E14

This comment has been minimized.

Show comment
Hide comment
@21E14

21E14 Mar 25, 2015

Contributor

It's worth pointing out.

This may also be a good opportunity to tilt the balance of preprocessor directives vs "magic" values, e.g. ADDRMAN_TRIED_BUCKETS_PER_GROUP (2 occurrences) vs 24 * 60 * 60 & 60 * 10.

Interestingly, P2P is not very well reflected in the ongoing libification. As it stands, libbitcoin_server is its closest anchor, but the two are arguably distinct. A large subset of the protocol is arguably entirely P2P (ver, verack, ping, pong, inv, getheaders, ...).

Contributor

21E14 commented Mar 25, 2015

It's worth pointing out.

This may also be a good opportunity to tilt the balance of preprocessor directives vs "magic" values, e.g. ADDRMAN_TRIED_BUCKETS_PER_GROUP (2 occurrences) vs 24 * 60 * 60 & 60 * 10.

Interestingly, P2P is not very well reflected in the ongoing libification. As it stands, libbitcoin_server is its closest anchor, but the two are arguably distinct. A large subset of the protocol is arguably entirely P2P (ver, verack, ping, pong, inv, getheaders, ...).

@fanquake

This comment has been minimized.

Show comment
Hide comment
@fanquake

fanquake Mar 26, 2015

Member

Running with -dnsseed=0

2015-03-26 03:21:39 init message: Loading addresses...
2015-03-26 03:21:39 Loaded 12984 addresses from peers.dat  94ms
2015-03-26 03:21:39 DNS seeding disabled
2015-03-26 03:21:40 connect() to 199.167.100.226:8333 failed after select(): Connection refused (61)
2015-03-26 03:21:52 receive version message: /Satoshi:0.9.3/: version 70002, blocks=349268, us=..., peer=1
2015-03-26 03:21:58 receive version message: /Satoshi:0.10.0/: version 70002, blocks=349268, us=..., peer=2
2015-03-26 03:22:05 receive version message: /Satoshi:0.9.2.1/: version 70002, blocks=349268, us=..., peer=3
2015-03-26 03:22:16 receive version message: /Satoshi:0.10.0/: version 70002, blocks=349268, us=..., peer=4

Removed peers.dat

2015-03-26 03:28:42 init message: Loading addresses...
2015-03-26 03:28:42 Loaded 0 addresses from peers.dat  1ms
2015-03-26 03:28:42 dnsseed thread start
2015-03-26 03:28:44 receive version message: /Satoshi:0.10.0/: version 70002, blocks=349269, us=..., peer=1
2015-03-26 03:28:45 72 addresses found from DNS seeds
2015-03-26 03:28:45 dnsseed thread exit
2015-03-26 03:28:46 receive version message: /Satoshi:0.8.6/: version 70001, blocks=349269, us=..., peer=2
2015-03-26 03:28:47 receive version message: /Satoshi:0.10.0/: version 70002, blocks=349269, us=..., peer=3
2015-03-26 03:28:56 receive version message: /Satoshi:0.9.3/: version 70002, blocks=349269, us=..., peer=4
Member

fanquake commented Mar 26, 2015

Running with -dnsseed=0

2015-03-26 03:21:39 init message: Loading addresses...
2015-03-26 03:21:39 Loaded 12984 addresses from peers.dat  94ms
2015-03-26 03:21:39 DNS seeding disabled
2015-03-26 03:21:40 connect() to 199.167.100.226:8333 failed after select(): Connection refused (61)
2015-03-26 03:21:52 receive version message: /Satoshi:0.9.3/: version 70002, blocks=349268, us=..., peer=1
2015-03-26 03:21:58 receive version message: /Satoshi:0.10.0/: version 70002, blocks=349268, us=..., peer=2
2015-03-26 03:22:05 receive version message: /Satoshi:0.9.2.1/: version 70002, blocks=349268, us=..., peer=3
2015-03-26 03:22:16 receive version message: /Satoshi:0.10.0/: version 70002, blocks=349268, us=..., peer=4

Removed peers.dat

2015-03-26 03:28:42 init message: Loading addresses...
2015-03-26 03:28:42 Loaded 0 addresses from peers.dat  1ms
2015-03-26 03:28:42 dnsseed thread start
2015-03-26 03:28:44 receive version message: /Satoshi:0.10.0/: version 70002, blocks=349269, us=..., peer=1
2015-03-26 03:28:45 72 addresses found from DNS seeds
2015-03-26 03:28:45 dnsseed thread exit
2015-03-26 03:28:46 receive version message: /Satoshi:0.8.6/: version 70001, blocks=349269, us=..., peer=2
2015-03-26 03:28:47 receive version message: /Satoshi:0.10.0/: version 70002, blocks=349269, us=..., peer=3
2015-03-26 03:28:56 receive version message: /Satoshi:0.9.3/: version 70002, blocks=349269, us=..., peer=4
@sipa

This comment has been minimized.

Show comment
Hide comment
@sipa

sipa Mar 27, 2015

Member

I tried a few times with -dnsseed=0 and a copy of bitcoin.sipa.be's peers.dat, and always had an initial connection between 15 and 30 seconds later.

Member

sipa commented Mar 27, 2015

I tried a few times with -dnsseed=0 and a copy of bitcoin.sipa.be's peers.dat, and always had an initial connection between 15 and 30 seconds later.

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Mar 30, 2015

Member

Works for me - tested ACK

Member

laanwj commented Mar 30, 2015

Works for me - tested ACK

@gmaxwell

This comment has been minimized.

Show comment
Hide comment
@gmaxwell

gmaxwell Mar 30, 2015

Member

ACK.

Member

gmaxwell commented Mar 30, 2015

ACK.

@jgarzik

This comment has been minimized.

Show comment
Hide comment
@jgarzik

jgarzik Mar 30, 2015

Contributor

ut ACK

My only nit: I wonder if we could replace that '%' with a shift. Could be, if the powers [of two] are aligned.

Contributor

jgarzik commented Mar 30, 2015

ut ACK

My only nit: I wonder if we could replace that '%' with a shift. Could be, if the powers [of two] are aligned.

@sipa

This comment has been minimized.

Show comment
Hide comment
@sipa

sipa Mar 30, 2015

Member
Member

sipa commented Mar 30, 2015

@jgarzik

This comment has been minimized.

Show comment
Hide comment
@jgarzik

jgarzik Mar 30, 2015

Contributor

@sipa should, agreed. With our 256 bit C++ implemented integer type, I wonder if that is true?

Contributor

jgarzik commented Mar 30, 2015

@sipa should, agreed. With our 256 bit C++ implemented integer type, I wonder if that is true?

@sipa

This comment has been minimized.

Show comment
Hide comment
@sipa

sipa Mar 30, 2015

Member
Member

sipa commented Mar 30, 2015

@laanwj laanwj merged commit 1d21ba2 into bitcoin:master Apr 1, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Apr 1, 2015

Merge pull request #5941
1d21ba2 Scale up addrman (Pieter Wuille)
c6a63ce Always use a 50% chance to choose between tried and new entries (Pieter Wuille)
f68ba3f Do not bias outgoing connections towards fresh addresses (Pieter Wuille)
a8ff7c6 Simplify hashing code (Pieter Wuille)
e6b343d Make addrman's bucket placement deterministic. (Pieter Wuille)
b23add5 Switch addrman key from vector to uint256 (Pieter Wuille)

sipa added a commit that referenced this pull request Apr 1, 2015

Switch addrman key from vector to uint256
Conflicts:
	src/addrman.cpp

Rebased-From: b23add5
Github-Pull: #5941

sipa added a commit that referenced this pull request Apr 1, 2015

Make addrman's bucket placement deterministic.
Give each address a single fixed location in the new and tried tables,
which become simple fixed-size arrays instead of sets and vectors.

This prevents attackers from having an advantages by inserting an
address multiple times.

This change was suggested as Countermeasure 1 in
Eclipse Attacks on Bitcoin’s Peer-to-Peer Network, Ethan Heilman,
Alison Kendler, Aviv Zohar, Sharon Goldberg. ePrint Archive Report
2015/263. March 2015.

It is also more efficient.

Conflicts:
	src/addrman.cpp
	src/addrman.h

Rebased-From: e6b343d
Github-Pull: #5941

sipa added a commit that referenced this pull request Apr 1, 2015

Simplify hashing code
Conflicts:
	src/addrman.cpp

Rebased-From: a8ff7c6
Github-Pull: #5941

sipa added a commit that referenced this pull request Apr 1, 2015

Do not bias outgoing connections towards fresh addresses
This change was suggested as Countermeasure 2 in
Eclipse Attacks on Bitcoin’s Peer-to-Peer Network, Ethan Heilman,
Alison Kendler, Aviv Zohar, Sharon Goldberg. ePrint Archive Report
2015/263. March 2015.

Rebased-From: 68ba3f67bd500a64fb8932c6b41924ddc31d76f
Github-Pull: #5941

sipa added a commit that referenced this pull request Apr 1, 2015

Always use a 50% chance to choose between tried and new entries
This change was suggested as Countermeasure 2 in
Eclipse Attacks on Bitcoin’s Peer-to-Peer Network, Ethan Heilman,
Alison Kendler, Aviv Zohar, Sharon Goldberg. ePrint Archive Report
2015/263. March 2015.

Rebased-From: c6a63ce
Github-Pull: #5941

sipa added a commit that referenced this pull request Apr 1, 2015

Scale up addrman
This change was suggested as Countermeasure 6 in
Eclipse Attacks on Bitcoin’s Peer-to-Peer Network, Ethan Heilman,
Alison Kendler, Aviv Zohar, Sharon Goldberg. ePrint Archive Report
2015/263. March 2015.

Rebased-From: 1d21ba2
Github-Pull: #5941
@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Apr 1, 2015

Member

Cherry-picked to 0.10 as aa587d4 0c6f334 214154e 2218d4b cf0218f b788994
Some conflicts encountered with uint256 usage, but was easy to backport.

Member

laanwj commented Apr 1, 2015

Cherry-picked to 0.10 as aa587d4 0c6f334 214154e 2218d4b cf0218f b788994
Some conflicts encountered with uint256 usage, but was easy to backport.

laanwj added a commit that referenced this pull request Apr 1, 2015

theuni pushed a commit to theuni/bitcoin that referenced this pull request Apr 23, 2015

addrman: update comments
nUnkBias was removed in bitcoin#5941

dexX7 added a commit to dexX7/bitcoin that referenced this pull request Apr 27, 2015

Merge upstream branch bitcoin/0.10
d8ac901 doc: improve credits in release notes (Gregory Maxwell)
bf8ad0d update release notes for 0.10.1rc3 (Wladimir J. van der Laan)
139cd81 Cap nAttempts penalty at 8 and switch to pow instead of a division loop. (Gregory Maxwell)
bac6fca Set nSequenceId when a block is fully linked (Suhas Daftuar)
323de27 Initialization: setup environment before starting QT tests (dexX7)
7494e09 Initialization: setup environment before starting tests (dexX7)
df45564 Initialization: set fallback locale as environment variable (dexX7)
57d1f46 Fix CheckBlockIndex for reindex. (mrbandrews)
eae305f Fix missing lock in submitblock (Matt Corallo)
34127c7 doc: update release notes pre rc2 (Wladimir J. van der Laan)
1c62e84 Keep mempool consistent during block-reorgs (Gavin Andresen)
149c1d8 doc: Credit Jonas Nick in release notes (Wladimir J. van der Laan)
bf1cc80 Docs: Use new Bitcoin.org download URLs (David A. Harding)
9e1cc16 doc: add historical release notes for 0.10.0 (Wladimir J. van der Laan)
fe31225 update release notes for #5953/#5900 (Wladimir J. van der Laan)
a1f425b Add a consistency check for the block chain data structures (Pieter Wuille)
ae1479a update release notes after #5941 (Wladimir J. van der Laan)
aa587d4 Scale up addrman (Pieter Wuille)
0c6f334 Always use a 50% chance to choose between tried and new entries (Pieter Wuille)
214154e Do not bias outgoing connections towards fresh addresses (Pieter Wuille)
2218d4b Simplify hashing code (Pieter Wuille)
cf0218f Make addrman's bucket placement deterministic. (Pieter Wuille)
b788994 Switch addrman key from vector to uint256 (Pieter Wuille)
90bef66 No notable changes for minor release (Wladimir J. van der Laan)
4635a4c Translations update from transifex (Wladimir J. van der Laan)
0eccf0a Add commits (up to now) to release notes (Wladimir J. van der Laan)
78f64ef don't trickle for whitelisted nodes (Ruben de Vries)
a316622 Clean out release notes for 0.10.1 (Wladimir J. van der Laan)

theuni added a commit to theuni/bitcoin that referenced this pull request May 14, 2015

addrman: update comments
nUnkBias was removed in bitcoin#5941

dexX7 added a commit to OmniLayer/omnicore that referenced this pull request May 16, 2015

Merge pull request #36
16f4560 doc: small amandment to release notes (Wladimir J. van der Laan)
ff32503 Release notes 0.10.2 (Wladimir J. van der Laan)
da65606 Avoid crash on start in TestBlockValidity with gen=1. (Gregory Maxwell)
49e4d14 Translations update (Wladimir J. van der Laan)
d7e7727 Preparations for 0.10.2 release (Wladimir J. van der Laan)
424ae66 don't imbue boost::filesystem::path with locale "C" on windows (Jonas Schnelli)
824c011 wallet: fix boost::get usage with boost 1.58 (Cory Fields)
ebc0e41 qt: translation update for next 0.10 point release (Wladimir J. van der Laan)
d8ac901 doc: improve credits in release notes (Gregory Maxwell)
bf8ad0d update release notes for 0.10.1rc3 (Wladimir J. van der Laan)
139cd81 Cap nAttempts penalty at 8 and switch to pow instead of a division loop. (Gregory Maxwell)
bac6fca Set nSequenceId when a block is fully linked (Suhas Daftuar)
323de27 Initialization: setup environment before starting QT tests (dexX7)
7494e09 Initialization: setup environment before starting tests (dexX7)
df45564 Initialization: set fallback locale as environment variable (dexX7)
57d1f46 Fix CheckBlockIndex for reindex. (mrbandrews)
eae305f Fix missing lock in submitblock (Matt Corallo)
34127c7 doc: update release notes pre rc2 (Wladimir J. van der Laan)
1c62e84 Keep mempool consistent during block-reorgs (Gavin Andresen)
149c1d8 doc: Credit Jonas Nick in release notes (Wladimir J. van der Laan)
bf1cc80 Docs: Use new Bitcoin.org download URLs (David A. Harding)
9e1cc16 doc: add historical release notes for 0.10.0 (Wladimir J. van der Laan)
fe31225 update release notes for #5953/#5900 (Wladimir J. van der Laan)
a1f425b Add a consistency check for the block chain data structures (Pieter Wuille)
ae1479a update release notes after #5941 (Wladimir J. van der Laan)
aa587d4 Scale up addrman (Pieter Wuille)
0c6f334 Always use a 50% chance to choose between tried and new entries (Pieter Wuille)
214154e Do not bias outgoing connections towards fresh addresses (Pieter Wuille)
2218d4b Simplify hashing code (Pieter Wuille)
cf0218f Make addrman's bucket placement deterministic. (Pieter Wuille)
b788994 Switch addrman key from vector to uint256 (Pieter Wuille)
90bef66 No notable changes for minor release (Wladimir J. van der Laan)
4635a4c Translations update from transifex (Wladimir J. van der Laan)
0eccf0a Add commits (up to now) to release notes (Wladimir J. van der Laan)
78f64ef don't trickle for whitelisted nodes (Ruben de Vries)
a316622 Clean out release notes for 0.10.1 (Wladimir J. van der Laan)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment