Repository: bitcoin/bitcoin (language: cpp)

Issue:
Decreasing nMaxIPs when learning from DNS seeds
Currently, our `nMaxIPs` when learning from DNS seeds is 256. @TheBlueMatt pointed out to me that if one of the seeds decided to actually return that many addresses, it would bias the addresses we add to `AddrMan` and consequently choose as our initial outbound peers. A quick `drill -t` of all the current seeds show that none yielded more than 26 results, meaning that if any of the seeds decided to return all 256 results (perhaps malicious ones), results from that seed would take over more than half of `AddrMan`.

If there were no TCP-fallback, this wouldn't be too much of a problem as at maximum we could fit around 31 IPs in a UDP packet. However, it seems that `glibc` [uses the TCP-fallback](https://bugs.launchpad.net/ubuntu/+source/systemd/+bug/1811471) by default, which would mean that obtaining 256 IPs is entirely possible.


Discussion / hints:
wrote a little python 3 script that will list number of answers with the same `getaddrinfo` call as what we use:
```python3
import socket

seeds = ["seed.bitcoin.sipa.be",
         "dnsseed.bluematt.me",
         "dnsseed.bitcoin.dashjr.org",
         "seed.bitcoinstats.com",
         "seed.bitcoin.jonasschnelli.ch",
         "seed.btc.petertodd.org",
         "seed.bitcoin.sprovoost.nl",
         "dnsseed.emzy.de"]

for seed in seeds:
    result = len(socket.getaddrinfo(seed, None, family=socket.AF_UNSPEC, type=socket.SOCK_STREAM, proto=socket.IPPROTO_TCP, flags=socket.AI_ADDRCONFIG))
    print("{:>4}\t{}".format(result, seed))
```
-----
Here's the result:
```
  25    seed.bitcoin.sipa.be
  33    dnsseed.bluematt.me
  38    dnsseed.bitcoin.dashjr.org
  50    seed.bitcoinstats.com
  38    seed.bitcoin.jonasschnelli.ch
  23    seed.btc.petertodd.org
  35    seed.bitcoin.sprovoost.nl
  41    dnsseed.emzy.de
```
Given the range of # of results (23-50), I'd be interested in what people think a good `nMaxIPs` choice would be.
Evenly divided by the limit on whichever bucket DNS seeds go into?
> Evenly divided by the limit on whichever bucket DNS seeds go into?

@pstratem Not 100% sure what you mean, could you elaborate a little?
@dongcarl Given the range of responses `nMaxIPs=32` sounds like a good number.

How easy would it be to update the DNS seeders to always return up to 32 addresses and then in bitcoin-core only accept 32 addresses from a DNS seeder. 
@EthanHeilman That's not actually up to the seeder implementation, as recursive resolving may transform the response in various ways. And it's also restricted to being able to fit in a UDP packet.
@sipa Do recursive revolvers transform DNS responses except for censorship, billing and malware purposes?
> Do recursive revolvers transform DNS responses except for censorship, billing and malware purposes?

Nope, they largely do not. If the response has been modified, something has gone wrong...
32, indeed, seems like a good number. With my hostname, 33 across v4+v6 fits neatly in the two 500 byte payload responses with an authority section, though if the server doesn't have an authority/additional section you could go a bit higher. To avoid biasing, we probably want a limit that is the ~minimum of all the seeds.

@petertodd and @sipa looks like y'all should be able to increase the number of responses your seeds provide?

While we're at it @cdecker looks like your seed violates the DNS spec for non-EDNS requests (it sends back 739 bytes if you query for v6 addresses, though your limit for v4 looks OK).

Of course that said, we could also probably look at the number of requests that use EDNS, and use (and provide) a larger limit if thats common enough (TCP fallback isn't sooo expensive). I don't, however, know if @sipa's seeder supports EDNS which would be a blocker.

```
$  dig +nodnssec +noedns @ns.as397444.net dnsseed.bluematt.me
...
dnsseed.bluematt.me.	3600	IN	CNAME	x1.dnsseed.bluematt.me.
...
;; MSG SIZE  rcvd: 486
```

```
$  dig +nodnssec +noedns @ns.as397444.net dnsseed.bluematt.me AAAA
...
dnsseed.bluematt.me.	3600	IN	CNAME	x1.dnsseed.bluematt.me.
...
;; MSG SIZE  rcvd: 486
```
> While we're at it @cdecker looks like your seed violates the DNS spec for non-EDNS requests (it sends back 739 bytes if you query for v6 addresses, though your limit for v4 looks OK).

I'll fix that asap, thanks for the heads up
@sipa 

Fix patch:
diff --git a/src/net.cpp b/src/net.cpp
index e388f05b037ad..3e959c187c2c7 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -2256,7 +2256,11 @@ void CConnman::ThreadDNSAddressSeed()
             if (!resolveSource.SetInternal(host)) {
                 continue;
             }
-            unsigned int nMaxIPs = 256; // Limits number of IPs learned from a DNS seed
+            // Limit number of IPs learned from a single DNS seed. This limit exists to prevent the results from
+            // one DNS seed from dominating AddrMan. Note that the number of results from a UDP DNS query is
+            // bounded to 33 already, but it is possible for it to use TCP where a larger number of results can be
+            // returned.
+            unsigned int nMaxIPs = 32;
             const auto addresses{LookupHost(host, nMaxIPs, true)};
             if (!addresses.empty()) {
                 for (const CNetAddr& ip : addresses) {


Test patch: