Skip to content

Conversation

lidel
Copy link
Member

@lidel lidel commented Sep 26, 2025

This PR moves hardcoded DNS resolver defaults from gateway.NewDNSResolver to autoconf.FallbackDNSResolvers so we have them in one place, while preserving existing behavior for users.

Changes

  • Move hardcoded DNS resolver defaults to autoconf.FallbackDNSResolvers
  • NewDNSResolver(nil) uses autoconf.FallbackDNSResolvers (preserves .eth resolution)
  • NewDNSResolver(map[string]string{}) explicitly uses system DNS only
  • Remove autoconf.Client.ExpandDNSResolvers() method (no use case yet)
  • Update test to use .foo instead of .eth TLD
  • No network calls at gateway initialization (privacy-preserving)

Migration

No migration needed. For those wanting custom DNS configuration, existing code (Kubo, Rainbow) continues to work.

References

remove implicit DNS-over-HTTPS resolvers that were difficult to
discover, override, or disable, improving user agency and configuration
transparency

- remove defaultResolvers map from gateway.NewDNSResolver
- NewDNSResolver(nil) now uses system DNS only (no implicit DoH)
- add autoconf.Client.ExpandDNSResolvers() convenience method for
  applications needing network-specific DNS resolver expansion
- update test to use .foo instead of .eth TLD

users needing DoH for non-ICANN TLDs should either pass explicit
resolvers or use autoconf.ExpandDNSResolvers() to merge network
defaults with custom resolvers

Closes #771
Closes #772
@lidel lidel changed the title feat(gateway)!: remove hardcoded defaultResolvers for .eth and .crypto feat(gateway)!: switch to autoconf and remove hardcoded defaultResolvers Sep 26, 2025
@lidel lidel changed the title feat(gateway)!: switch to autoconf and remove hardcoded defaultResolvers feat(gateway)!: remove hardcoded defaultResolvers + autoconf guidance Sep 26, 2025
Copy link

codecov bot commented Sep 26, 2025

Codecov Report

❌ Patch coverage is 66.66667% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.76%. Comparing base (4c0aa3a) to head (b3238d9).

Files with missing lines Patch % Lines
gateway/dns.go 66.66% 4 Missing and 2 partials ⚠️

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1044      +/-   ##
==========================================
- Coverage   60.76%   60.76%   -0.01%     
==========================================
  Files         268      268              
  Lines       33593    33595       +2     
==========================================
- Hits        20414    20413       -1     
+ Misses      11508    11507       -1     
- Partials     1671     1675       +4     
Files with missing lines Coverage Δ
gateway/dns.go 57.44% <66.66%> (-2.56%) ⬇️

... and 13 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

CHANGELOG.md Outdated
Comment on lines 50 to 51
- This change removes implicit defaults that were difficult to discover, override, or disable, improving user agency and configuration transparency
- `NewDNSResolver(nil)` now uses system DNS only (no implicit DoH resolvers)
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it make sense to make this a breaking change for all users? We had centralized / hardcoded endpoints in here beforehand why not just use autoconf by default?

e.g. if ENS is highly visible / expected within the ecosystem and we make it so that by default it doesn't show up we're going to be hoisting a whole bunch of bug reports onto users who'll have to report this to their implementations. As a bonus this type of breakage doesn't even show up in Go tooling so most users will just upgrade (it seems like nobody reads release notes until maybe they see Go yelling at them), think everything is fine, and then rely on users to report bugs.

Copy link
Member Author

@lidel lidel Sep 26, 2025

Choose a reason for hiding this comment

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

Fair point – we do not want to triage extra bugs – this is legacy debt we can pay off some other time.

I've restored the old behavior (b3238d9) to not break existing users, but did not add full autoconf, just replaced defaultResolvers with code that reads defaults from autoconf.FallbackDNSResolvers. This way we still remove copy of the defaults from boxo/gateway and have single source of truth for fallback defaults in autoconf package, but avoid a can of worms described below.

(There are many reasons to not use autoconf here: real autoconf comes with extra network I/O and potential delay in startup/initialization. We would also add a ton of code to ensure user can pass own client, and initialize new implicit default client if user did not pass any. Then, how do we know if user is ok with sending extra HTTP Request every time their app with boxo/gateway starts? Is user ok with leaking their IP to some http server when they were using boxo/gateway only in private contexts? Can of worms.)

Copy link
Contributor

Choose a reason for hiding this comment

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

There are many reasons to not use autoconf here: real autoconf comes with extra network I/O and potential delay in startup/initialization

Agree on some of the can of worms and that we can defer at the library layer if we want, but in most ways it seems like using autoconf still ends up better than the status quo.

real autoconf comes with extra network I/O and potential delay in startup/initialization

Delays and I/O don't have to happen, they can happen on first use of a non-ICANN domain.

Then, how do we know if user is ok with sending extra HTTP Request every time their app with boxo/gateway starts

Mostly fixed by above, unless the user has very short-lived gateways (a real possibility, but other caching capabilities like those that are DoH related are impacted here as well).

Is user ok with leaking their IP to some http server when they were using boxo/gateway only in private contexts?

We already have this if someone uses a .eth or .crypto domain

We would also add a ton of code

Maybe, I'm not sure that's true. We have overrideable defaults like this in a bunch of places (even if they don't rely on I/O since we'd prefer to avoid that if possible).


IIUC the win here from using the default autoconf endpoint is basically that users can rely on the boxo maintainers to give them sane default DNSLink resolvers rather than relying on externally run hard coded ones to be good indefinitely (i.e. the status quo we're keeping here). This could have been done by the maintainers instead running proxies for each default DNSLink resolver, but using autoconf makes the extra operational burden on the maintainers minimal while allowing for extra resiliency with having longer cache time so that even if the default autoconf endpoint goes down for a bit that many users will still be able to resolve their domain names. In theory multiple endpoints for a given eTLD could be configured, but at the moment even ENS in practice only has one public DNSLink resolver 😬.

…defaults

- moves hardcoded DNS resolver defaults to autoconf.FallbackDNSResolvers
- NewDNSResolver(nil) uses autoconf fallback, preserving behavior
- empty map NewDNSResolver({}) explicitly uses system DNS only
- removes autoconf.Client.ExpandDNSResolvers method (no use case yet)
- addresses privacy concerns by avoiding network calls at gateway init
@lidel lidel marked this pull request as ready for review September 26, 2025 20:18
@lidel lidel requested a review from a team as a code owner September 26, 2025 20:18
@lidel lidel changed the title feat(gateway)!: remove hardcoded defaultResolvers + autoconf guidance feat(gateway)!: switch defaults to autoconf.FallbackDNSResolvers Sep 26, 2025
@lidel lidel changed the title feat(gateway)!: switch defaults to autoconf.FallbackDNSResolvers refactor(gateway): switch defaults to autoconf.FallbackDNSResolvers Sep 26, 2025

var defaultResolvers = map[string]string{
"eth.": "https://dns.eth.limo/dns-query",
"crypto.": "https://resolver.unstoppable.io/dns-query",
Copy link
Contributor

Choose a reason for hiding this comment

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

We should flag that this is dead / removed now (it's not in the default autoconf resolver or in the fallbacks)

@guillaumemichel guillaumemichel marked this pull request as draft October 7, 2025 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: all "Unstoppable Domains" (.crypto) DoH resolvers are down

3 participants