-
Notifications
You must be signed in to change notification settings - Fork 38.7k
Limit the number of IPs addrman learns from each DNS seeder #12626
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
Conversation
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.
randolf
left a comment
There was a problem hiding this 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.
|
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. |
|
@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). |
|
utACK 46e7f80 |
|
@randolf Thanks, TIL. |
|
utACK 46e7f80 |
theuni
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK 46e7f80
|
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? |
|
utACK 46e7f80 |
…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
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
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
|
Backported in #12967 |
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
… 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
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