refactor(iroh-dns): Replace hickory with a simpledns based DNS resolver#4036
Closed
dignifiedquire wants to merge 88 commits into
Closed
refactor(iroh-dns): Replace hickory with a simpledns based DNS resolver#4036dignifiedquire wants to merge 88 commits into
dignifiedquire wants to merge 88 commits into
Conversation
Contributor
|
I don't remember the exact detalis, but there was something that hickory could do that simple-dns can't. @Frando ? |
dignifiedquire
force-pushed
the
refactor-hickory
branch
from
March 23, 2026 09:14
4f62d53 to
f6c6c09
Compare
|
Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh/pr/4036/docs/iroh/ Last updated: 2026-06-29T11:23:51Z |
Recursive resolvers commonly return CNAME records when a queried name
is an alias. The previous code only looked for the exact queried record
type (A/AAAA/TXT) in the answer section, silently returning empty
results for any name behind a CNAME (common with CDNs, cloud LBs).
This adds two levels of CNAME following:
(a) In-response: resolve_cname_chain() walks CNAME records within a
single response packet to find the canonical name, then collects
records matching either the original or canonical name.
(b) Recursive: send_query_following_cnames() detects when a response
contains only a CNAME with no target records, and issues a new
query for the CNAME target. Limited to 8 hops to prevent loops.
Without EDNS(0), well-behaved DNS servers limit UDP responses to 512 bytes (RFC 1035). Iroh endpoint TXT records with multiple addresses can easily exceed this, forcing a TCP fallback round-trip on every endpoint discovery query. Add an OPT pseudo-record to all outgoing queries advertising 1232-byte UDP payload support (the recommended safe value per RFC 6891 and DNS flag day 2020). This avoids unnecessary TCP fallbacks while staying under common path MTU limits.
Previously, send_query tried nameservers sequentially with a 5-second per-nameserver timeout. Since the outer DNS_TIMEOUT is 3 seconds, only the first nameserver was ever reached. A single UDP packet loss meant immediate failure. Now: - All nameservers are queried in parallel via FuturesUnorderedBounded, with staggered starts (100ms between each) so the preferred nameserver gets a head start. - UDP queries retry once per nameserver (2 attempts total) before giving up, matching hickory-resolver's default attempts:2 behavior. - Per-nameserver timeout reduced to 2s so individual attempts complete quickly and don't block the overall query. - First successful response from any nameserver wins. Also fixes CNAME name-matching to gracefully handle responses without a question section (accept all records of the target type).
Use recv_from instead of recv to verify that the DNS response came from the expected nameserver. Without this check, a local network attacker could race a spoofed response before the real one arrives. The random 16-bit query ID provides some defense, but source address validation is standard defense-in-depth against cache poisoning.
DNS-over-TLS and DNS-over-HTTPS currently derive the TLS server name from the IP address. This works for providers with IP SANs in their certificates (Google, Cloudflare) but will fail for servers with hostname-only certificates. Document this as a known limitation.
TxtRecordData was changed from Box<[Box<[u8]>]> to Box<[String]>, which is lossy for non-UTF-8 TXT record content and breaks the public API. Restore the original bytes representation to preserve binary TXT record fidelity. Display still uses from_utf8_lossy for rendering. Keep From<Vec<String>> for convenience at construction sites.
The previous extract_txt_record_data used TXT::attributes() which returns a HashMap, losing ordering and deduplicating keys. This is destructive for iroh's endpoint records which publish multiple addr= entries as separate TXT records. Replace with String::try_from(txt) which preserves the raw concatenated content of each TXT record faithfully.
The resolv.conf parser previously only extracted nameserver lines, ignoring search and domain directives. This means search domain completion for short hostnames was silently broken. Parse both directives per resolv.conf(5) semantics: search and domain are mutually exclusive, last one wins. Introduce SystemDnsConfig struct to carry both nameservers and search domains. The resolver does not yet apply search domains to queries -- this commit just ensures the configuration is read and available.
Some setups (Docker, VPNs, custom resolvers) use non-standard DNS ports. Previously, entries like "nameserver 8.8.8.8:5353" would silently fail to parse as IpAddr and be skipped. Now try parsing as SocketAddr first (which supports port), falling back to IpAddr with the default port 53.
NXDOMAIN (domain doesn't exist) and SERVFAIL (server error) were lumped into the same ServerError variant. Add a dedicated NxDomain variant so callers can distinguish "this domain doesn't exist" from "DNS is broken" and skip retries for definitive NXDOMAIN responses.
dedup_by_key only removes consecutive duplicates, so if the same DNS server appears on non-adjacent network adapters, it would survive deduplication. Use a HashSet to properly deduplicate regardless of ordering.
Document two known limitations: 1. No negative caching: NXDOMAIN/NODATA responses are never cached, which can cause thundering herd under high concurrency for non-existent domains. This matches the old hickory-resolver config. 2. No TCP/TLS connection reuse: each query opens a fresh connection, adding a full TLS handshake per DoT query. Only affects non-default DoT/DoH configurations.
The field is intentionally parsed from resolv.conf but not yet consumed by the resolver. Add allow(dead_code) with a comment explaining why.
Implement resolv.conf search domain semantics per resolv.conf(5): - Short hostnames (fewer dots than ndots, default 1) try each search domain suffix first, then the bare name. - Names with enough dots try the bare name first, then search domains. - FQDNs (trailing dot) bypass search domains entirely. - NXDOMAIN responses advance to the next candidate name rather than failing immediately. This makes the custom resolver behave like system resolvers for short hostnames, which matters for Docker, Kubernetes, and corporate network setups where search domains are commonly configured.
- Collapse nested if/if-let into if-let chains (resolve_cname_chain) - Use .then() and .ok()? for cname_target - Use is_ok_and for is_truncated - Use matches! for record type checking - Use bytes() instead of chars() for dot counting - Extract with_timeout helper to deduplicate timeout wrapping - Simplify stagger logging in send_query - Use elapsed() instead of manual Instant arithmetic in cache - Remove dead system_nameservers() wrapper - Remove stale #[allow(dead_code)] on search_domains
- with_timeout: use `?` on Elapsed to get DnsError::Timeout directly instead of wrapping in a fake io::Error -> DnsError::Transport - InvalidPacket: use `?` on SimpleDnsError directly (from_sources generates the From impl) - UDP source validation: use `?` on io::Error instead of e!() wrapper - TLS config missing: use io::Error::other for brevity - Remove unused n0_error::e import from transport.rs
Frando
changed the title
A major network change rebuilds the resolver via `reset()` to pick up new nameservers. It used to start with an empty cache, so lookups went cold exactly during the transition (e.g. WiFi to 5G) and reconnects stranded while DNS was still in flux. Make `DnsCache` Arc-backed and carry it into the rebuilt resolver, so cached records keep serving until the new nameservers settle. Closes #4037
Plain-TCP and DNS-over-TLS queries previously opened a fresh connection every time, paying the TCP (and for DoT, TLS) handshake on each lookup. Add a small per-nameserver connection pool that checks a connection out for exclusive use and returns it on success, so the handshake is amortized across repeated queries to the same server. Idle connections are dropped on checkout once older than IDLE_TIMEOUT and capped per nameserver; a background task sweeps connections that no checkout touches, so a nameserver queried once does not pin a socket open. UDP stays unpooled (a fresh source port per query helps prevent cache poisoning) and DoH reuses connections inside reqwest already.
The reaper now holds a Weak reference to the pool and exits once the pool is dropped, instead of holding the connections alive behind an AbortOnDropHandle. Dropping the pool (e.g. when a network change rebuilds the resolver) now closes its idle sockets immediately rather than waiting for the task abort to propagate, and there is no join handle to store.
The simple-dns resolver only special-cased localhost and otherwise went straight to the network, dropping the hosts-file overrides the old hickory-backed resolver honored through its use_hosts_file default. An operator pinning a relay or discovery origin to a fixed address in /etc/hosts (self-hosted relays, CI and Docker setups) saw the name go to real DNS and resolve to nothing or to the wrong address. Parse the hosts file once at construction when system defaults are in use, and check it ahead of the cache for IPv4 and IPv6 lookups. Reading it at construction mirrors reading /etc/resolv.conf and performs no network IO, so it is safe on the reset path.
macOS does not reliably keep /etc/resolv.conf in sync with the live resolver configuration, so the generic Unix reader could miss the nameservers the system is actually using. The old hickory-resolver path read the primary resolver from the SystemConfiguration framework on Apple targets; the rewrite routed macOS through the resolv.conf reader and lost that. Add an apple module that reads ServerAddresses and SearchDomains from the dynamic store key State:/Network/Global/DNS, and dispatch macOS to it. This restores the previous source of truth for the default resolver. Scoped per-domain resolvers (VPN split-DNS) live under other keys and were not read by the old path either, so they remain out of scope.
`name_matches` accepted every answer record when the parsed response carried no question section, so a spoofed reply that guessed the 16-bit transaction ID could inject records for an arbitrary name. A well-formed response always echoes the query's question (RFC 1035 Section 4.1.2). Reject question-less responses in `check_response` and tighten the now-unreachable `name_matches` fallback to match nothing.
The Unix resolv.conf parser recognizes nameserver, search, domain, and options ndots, and silently ignores the rest. Record that timeout, attempts, rotate, and sortlist are dropped on purpose: iroh imposes its own timeouts and attempt budget, orders nameservers by measured RTT, and selects addresses itself, so honoring them would not change behavior.
The mock UDP/TCP servers returned responses with no question section, which a real server never does. Now that the resolver rejects question-less responses, the mocks must echo the question to stay realistic. Pairs with the question-section validation change.
Gating the apple reader on target_os = "macos" left iOS, tvOS, and watchOS on the generic Unix reader, which parses /etc/resolv.conf. On iOS the sandbox hides that file, so the resolver fell back to the public nameservers. hickory routed every Apple target through SystemConfiguration. Gate on target_vendor = "apple" to match, and exclude the same vendor from the Unix reader.
The response parser only checked the transaction ID, the QR bit, the presence of a question, and the RCODE. It then took the queried name from the response's own question section, so a forged reply that guessed the 16-bit ID and supplied its own question could have its answers accepted for an arbitrary name. CNAME-following decisions were taken on a packet whose ID was never verified, since only the final hop was checked. Validate every hop in send_query_following_cnames: check_response now takes the queried name and type and rejects any response whose single question does not echo them, alongside the existing ID, QR, and RCODE checks. The parse_* functions become pure extractors that run on an already-validated response, anchoring CNAME resolution on the verified question.
send_query returned the first transport-level response regardless of its RCODE, so a SERVFAIL or REFUSED from the fastest nameserver became the result for the whole lookup and the slower servers that might have answered were never consulted. This hurts hosts whose primary resolver refuses a name a secondary resolver would answer (split-horizon, captive or misconfigured resolvers). Treat a SERVFAIL or REFUSED response like a transport failure: penalize the server, record the error, and let the race continue to the next nameserver. A definitive NXDOMAIN is still trusted and returned, matching hickory's default. The RCODE is read straight from the response header, so a spoofed code at worst makes the race try another server and the eventual answer is still validated.
is_truncated hand-decoded the TC bit out of the third header byte. Use simple_dns::header_buffer::has_flags with PacketFlag::TRUNCATION, which reads the same header flags without parsing the body, so we no longer maintain a bespoke copy of the wire layout.
This was referenced Jul 14, 2026
Member
|
I moved the actual resolver to a new crate: https://github.com/n0-computer/n0-dns-resolver/ Thus closing this PR. Leaving the branch in place still for a while in case we want to consult its history. |
18 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaces the hickory stack with a simpler DNS resolver based on
simple-dns.Reduces binary size of a size-optimized build by around 600KB (or about 10%).
We also can react and adapt faster this way if we want to adjust things further for specific platforms.
This will need to get good real-world testing across platforms though.