Decreasing nMaxIPs when learning from DNS seeds #16070

issue dongcarl openend this issue on May 21, 2019
  1. dongcarl commented at 9:03 pm on May 21, 2019: contributor

    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 by default, which would mean that obtaining 256 IPs is entirely possible.

  2. fanquake added the label P2P on May 22, 2019
  3. dongcarl commented at 10:17 pm on June 13, 2019: contributor

    wrote a little python 3 script that will list number of answers with the same getaddrinfo call as what we use:

     0import socket
     1
     2seeds = ["seed.bitcoin.sipa.be",
     3         "dnsseed.bluematt.me",
     4         "dnsseed.bitcoin.dashjr.org",
     5         "seed.bitcoinstats.com",
     6         "seed.bitcoin.jonasschnelli.ch",
     7         "seed.btc.petertodd.org",
     8         "seed.bitcoin.sprovoost.nl",
     9         "dnsseed.emzy.de"]
    10
    11for seed in seeds:
    12    result = len(socket.getaddrinfo(seed, None, family=socket.AF_UNSPEC, type=socket.SOCK_STREAM, proto=socket.IPPROTO_TCP, flags=socket.AI_ADDRCONFIG))
    13    print("{:>4}\t{}".format(result, seed))
    

    Here’s the result:

    0  25    seed.bitcoin.sipa.be
    1  33    dnsseed.bluematt.me
    2  38    dnsseed.bitcoin.dashjr.org
    3  50    seed.bitcoinstats.com
    4  38    seed.bitcoin.jonasschnelli.ch
    5  23    seed.btc.petertodd.org
    6  35    seed.bitcoin.sprovoost.nl
    7  41    dnsseed.emzy.de
    
  4. dongcarl commented at 10:19 pm on June 13, 2019: contributor
    Given the range of # of results (23-50), I’d be interested in what people think a good nMaxIPs choice would be.
  5. pstratem commented at 10:51 pm on June 13, 2019: contributor
    Evenly divided by the limit on whichever bucket DNS seeds go into?
  6. dongcarl commented at 3:30 pm on June 14, 2019: contributor

    Evenly divided by the limit on whichever bucket DNS seeds go into? @pstratem Not 100% sure what you mean, could you elaborate a little?

  7. EthanHeilman commented at 8:37 pm on June 19, 2019: contributor

    @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.

  8. sipa commented at 8:41 pm on June 19, 2019: member
    @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.
  9. EthanHeilman commented at 2:27 pm on June 20, 2019: contributor
    @sipa Do recursive revolvers transform DNS responses except for censorship, billing and malware purposes?
  10. TheBlueMatt commented at 0:05 am on November 28, 2019: contributor

    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…

  11. TheBlueMatt commented at 0:42 am on November 28, 2019: contributor

    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.

    0$  dig +nodnssec +noedns [@ns](/bitcoin-bitcoin/contributor/ns/).as397444.net dnsseed.bluematt.me
    1...
    2dnsseed.bluematt.me.	3600	IN	CNAME	x1.dnsseed.bluematt.me.
    3...
    4;; MSG SIZE  rcvd: 486
    
    0$  dig +nodnssec +noedns [@ns](/bitcoin-bitcoin/contributor/ns/).as397444.net dnsseed.bluematt.me AAAA
    1...
    2dnsseed.bluematt.me.	3600	IN	CNAME	x1.dnsseed.bluematt.me.
    3...
    4;; MSG SIZE  rcvd: 486
    
  12. cdecker commented at 3:51 pm on November 28, 2019: contributor

    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

  13. willcl-ark commented at 4:13 pm on September 21, 2023: member
  14. willcl-ark assigned pinheadmz on Apr 10, 2024
  15. laanwj referenced this in commit 2ee2ef44b9 on Apr 11, 2024
  16. laanwj referenced this in commit f2e3662e57 on Apr 11, 2024
  17. achow101 closed this on Apr 22, 2024

  18. achow101 referenced this in commit 3310a965bd on Apr 22, 2024

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-12-25 03:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me