bitcoin: DNS seed node lookup is ambiguous in some (rare) scenarios

DNS seed node lookup ambiguous in some (rare) scenarios.

To reproduce run:

$ RES_OPTIONS="options ndots:15" LOCALDOMAIN="localtest.me" src/bitcoind

Context from https://github.com/bitcoin/bitcoin/pull/22098#issuecomment-890227550:

It is outside of the scope of this PR but I think it would make sense to err on the safe side and specify all seed domain names with an ending dot to make them unambiguous.

In other words …

    vSeeds.emplace_back("seed.bitcoin.sipa.be."); // Pieter Wuille, only supports x1, x5, x9, and xd

… instead of the current …

    vSeeds.emplace_back("seed.bitcoin.sipa.be"); // Pieter Wuille, only supports x1, x5, x9, and xd

The former is guaranteed to be interpreted as seed.bitcoin.sipa.be. whereas the latter may be interpreted as sayseed.bitcoin.sipa.be.megacorp.com. depending on the content of the user’s /etc/resolv.conf.

The case I’m thinking about is if search megacorp.com is specified in /etc/resolv.conf and say options ndots:5 is used.

Looking up seed.bitcoin.sipa.be would then result in the following DNS traffic:

IP <src> > <resolver>.53: A? seed.bitcoin.sipa.be.megacorp.com.
IP <resolver>.53 > <src>: NXDomain
IP <src> > <resolver>.53: A? seed.bitcoin.sipa.be.
IP <resolver>.53 > <src>: Some valid response.

In other words seed.bitcoin.sipa.be.megacorp.com. would be tried before seed.bitcoin.sipa.be. 😦

I can think of three different approaches to fixing this:

  • Add a bool allow_non_fqdn argument to Lookup* functions, and automatically add a trailing . before passing the hostname string to getaddrinfo if !allow_non_fqdn. This approach would make the choice between FQDN vs “potentially non-FQDN” explicit. I think this is my preferred approach since the caller would explicitly signal his/her intent.
  • Specify seed node strings as FQDNs by appending . to the seed hostname strings in src/chainparams.cpp’s vSeed.
  • Change from strprintf("x%x.%s", requiredServiceBits, seed) to strprintf("x%x.%s.", requiredServiceBits, seed) when building the seed hostname in CConnman::ThreadDNSAddressSeed().

Background: https://github.com/bitcoin/bitcoin/pull/22098#issuecomment-890227550 https://github.com/bitcoin/bitcoin/pull/22098#issuecomment-890582857 https://github.com/bitcoin/bitcoin/pull/22098#issuecomment-890763573

About this issue

  • Original URL
  • State: closed
  • Created 3 years ago
  • Comments: 24 (14 by maintainers)

Most upvoted comments

I think we should just use FQDNs for the DNS seeds.

Even though you have removed the systemd stub resolver form your /etc/resolv.conf, it’s possible that getaddrinfo is still using systemd if /etc/nsswitch.conf contains “resolve” in the host directive. This is the default in fedora, but not ubuntu (not sure about debian).

https://www.freedesktop.org/software/systemd/man/nss-resolve.html

@prayank23 Consider submitting that PR (https://github.com/bitcoinknots/bitcoin/pull/40) upstreams too 😃 I’d be glad to review.