Skip to content

Conversation

@EthanHeilman
Copy link
Contributor

A risk exists where a malicious DNS seeder eclipses a node by returning an enormous number of IP addresses. In this commit we mitigate this risk by limiting the number of IP addresses addrman learns to 256 per DNS seeder.

As discussed with @theuni

A risk exists where a malicious DNS seeder eclipses a node by returning an enormous number of IP addresses. In this commit we mitigate this risk by limiting the number of IP addresses addrman learns to 256 per DNS seeder.
@fanquake fanquake added the P2P label Mar 6, 2018
Copy link
Contributor

@randolf randolf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a very good first step in mitigating this type of DoS attack, and 256 seems me to be an extremely generous default.

@sipa
Copy link
Member

sipa commented Mar 7, 2018

Since DNS responses generally are sent over UDP, all of them need to fit in a single IP packet (I believe), which puts a natural limit regardless. Having some explicit limit sounds good though.

@randolf
Copy link
Contributor

randolf commented Mar 7, 2018

@sipa Packets larger than 512 bytes are supported with the introduction of EDNS (see RFC 6891 dated April 2013; earlier RFCs that reference EDNS0 that may also be of interest) that uses an unsigned 16-bit integer to specify RDLEN (Record Data Length). Also, while UDP is a MUST for DNS services, TCP is a SHOULD, and both of these transport layer protocols can, for the most part, support EDNS's larger packet size options.

In summary, the natural limit that is more well-known has effectively been extended (IP packet fragmentation and reassembly make it possible to venture beyond the MSU, which is commonly set to 1,500 bytes).

@practicalswift
Copy link
Contributor

utACK 46e7f80

@sipa
Copy link
Member

sipa commented Mar 7, 2018

@randolf Thanks, TIL.

@sdaftuar
Copy link
Member

sdaftuar commented Mar 7, 2018

utACK 46e7f80

Copy link
Member

@theuni theuni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK 46e7f80

@EthanHeilman
Copy link
Contributor Author

Three years ago I tested the number of DNS entries I could get into Bitcoin for the eclipse attack paper. My test setup was Ubuntu Linux running Bitcoind querying a custom DNS server on localhost. We didn't end up using this attack so I wrote up a blog entry about the general question without mentioning bitcoin: How many IP addresses can a DNS query return?

@fanquake
Copy link
Member

fanquake commented Mar 7, 2018

utACK 46e7f80

@laanwj laanwj added this to the 0.16.1 milestone Mar 7, 2018
@laanwj laanwj merged commit 46e7f80 into bitcoin:master Mar 7, 2018
laanwj added a commit that referenced this pull request Mar 7, 2018
…eder

46e7f80 Limit the number of IPs we use from each DNS seeder (e0)

Pull request description:

  A risk exists where a malicious DNS seeder eclipses a node by returning an enormous number of IP addresses. In this commit we mitigate this risk by limiting the number of IP addresses addrman learns to 256 per DNS seeder.

  As discussed with @theuni

Tree-SHA512: 949e870765b1470200f2c650341d9e3308a973a7d1a6e557b944b0a2b8ccda49226fc8c4ff7d2a05e5854c4014ec0b67e37a3f2287556fe7dfa2048ede1f2e6f
@fanquake fanquake mentioned this pull request Apr 12, 2018
fanquake pushed a commit to fanquake/bitcoin that referenced this pull request Apr 26, 2018
A risk exists where a malicious DNS seeder eclipses a node by returning an enormous number of IP addresses. In this commit we mitigate this risk by limiting the number of IP addresses addrman learns to 256 per DNS seeder.

GitHub-Pull: bitcoin#12626
Rebased-From: 46e7f80
laanwj added a commit that referenced this pull request May 16, 2018
8fca086 List support for BIP173 in bips.md (Pieter Wuille)
9645aa6 Remove blockmaxsize option from init.cpp (fanquake)
7847b92 Default to defining endian-conversion DECLs in compat w/o config (Matt Corallo)
1720eb3 qt:Show the entire Window when double clicking on taskbar (Chun Kuan Lee)
e055bc0 depends: Fix Qt build with XCode 9.3 (fanquake)
0684cf9 Avoid launching as admin when NSIS installer ends. (JeremyRand)
e802c22 [config] Remove blockmaxsize option (John Newbery)
f118a7a Fix illegal default `addProxy` and `addrSeparateProxyTor` settings. (251)
f60e84d Limit the number of IPs we use from each DNS seeder (e0)

Pull request description:

  Backports:
  - #12626 Limit the number of IPs addrman learns from each DNS seeder
  - #12650 gui: Fix issue: "default port not shown correctly in settings dialog"
  - #12756 [config] Remove blockmaxsize option
  - #12985 Windows: Avoid launching as admin when NSIS installer ends.
  - #12946 depends: Fix Qt build with XCode 9.3
  - #12998 Default to defining endian-conversion DECLs in compat w/o config
  - #12999 qt: Show the Window when double clicking the taskbar icon
  - #13064 List support for BIP173 in bips.md

  to the 0.16 branch.

Tree-SHA512: 3e6b47c54b2cd2bdd81fbc6176cb31e46423f6e05988984d3a09b3535e3cee101ffb071cf753a4beff3c9f0521eb5de4b7c0424a3e97da801d56b4015847ac0f
@fanquake
Copy link
Member

Backported in #12967

HashUnlimited pushed a commit to chaincoin/chaincoin that referenced this pull request Jun 29, 2018
A risk exists where a malicious DNS seeder eclipses a node by returning an enormous number of IP addresses. In this commit we mitigate this risk by limiting the number of IP addresses addrman learns to 256 per DNS seeder.

GitHub-Pull: bitcoin#12626
Rebased-From: 46e7f80
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 27, 2020
… DNS seeder

46e7f80 Limit the number of IPs we use from each DNS seeder (e0)

Pull request description:

  A risk exists where a malicious DNS seeder eclipses a node by returning an enormous number of IP addresses. In this commit we mitigate this risk by limiting the number of IP addresses addrman learns to 256 per DNS seeder.

  As discussed with @theuni

Tree-SHA512: 949e870765b1470200f2c650341d9e3308a973a7d1a6e557b944b0a2b8ccda49226fc8c4ff7d2a05e5854c4014ec0b67e37a3f2287556fe7dfa2048ede1f2e6f
@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.

8 participants