Repository navigation
Fix resolver list getting stuck after a partial bootstrap result - #215
Merged
Merged
Conversation
While the network is down, the periodic bootstrap lookup can still succeed with only one address family: c-ares (query cache on by default since 1.31) answers from whichever of the A/AAAA records has not expired yet, while the other query times out. doh_proxy_handle_resolver_update() installed that partial list, but once the network came back the full list was treated as unchanged, because addr_list_reduced() was only checked in one direction (old list contained in the new one). The proxy kept the shrunken list forever. On an IPv4-only host left with an IPv6-only list, every request then failed at once with CURLE_COULDNT_CONNECT, nothing was logged at the default level, and the proxy stopped answering until it was restarted. - Compare the old and the new list in both directions, and match addresses as whole list elements so that "192.0.2.1" does not match the start of "192.0.2.10". - Disable the c-ares query cache in the poller. The poller re-resolves on its own schedule, and cached partial answers only produce bogus updates while the network is down. - Add unit tests for the resolver update logic and the poller options. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
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.
The problem we ran into
We run https_dns_proxy on an OpenWrt 25.12.5 router (aarch64) with an IPv4-only uplink. The default route goes through a VPN tunnel. After a short tunnel flap, both instances started by the OpenWrt init script (Cloudflare and Google) stopped answering and stayed that way for 37 hours, until we killed them. The whole LAN had no DNS. Meanwhile the tunnel, the routes, plain UDP DNS to 1.1.1.1 and
curl https://1.1.1.1/dns-query?...from the same router all worked. Both processes were alive and idle, had no upstream sockets open, their UDP listeners drained the queue with no drops, and nothing was logged at the default verbosity.OpenWrt ships 0ba0525 as package 2026.05.06. Current master f8e13aa behaves the same way.
Root cause
Two things combine:
ares_getaddrinfo()withAF_UNSPECsucceeds with whichever of the A/AAAA records is still cached, while the other query times out.doh_proxy_handle_resolver_update()installs that partial list.addr_list_reduced(addr_list, old_addr_list)only checks that every old address is in the new list. When the network is back, the full list arrives, the shrunken old list is a subset of it, and the update is reported as unchanged and dropped. The list never grows back.If the AAAA records are the ones that survive, an IPv4-only host is left with IPv6-only addresses. Every request then fails with
CURLE_COULDNT_CONNECTbefore a connection is even attempted. No reply is sent, the reset timer is never armed, and nothing is logged at the default level.Log from the router with the OpenWrt build, while all outbound traffic of the proxy was dropped with nftables for 6 minutes (times UTC+3):
What this PR changes
src/doh_proxy.c: compare the old and the new address list in both directions, and match addresses as whole list elements, so192.0.2.1no longer matches the start of192.0.2.10. The prefix case only matters once the check is two-way: a reordered list would otherwise trigger a needless client reset.src/dns_poller.c: disable the c-ares query cache in the poller (ARES_OPT_QUERY_CACHEwithqcache_max_ttl = 0, behind#ifdeffor older c-ares). The poller re-resolves on its own schedule, so the cache only serves stale partial answers during an outage.tests/unit/test_resolver_update.ccovers the update logic, andtest_dns_poller.cchecks that the query cache is off.Reproduction
The bug is deterministic with a fake bootstrap DNS server that gives A records a 5 s TTL and AAAA records 3600 s, and then goes silent for a while. The poll at t=10 s is then answered from the c-ares cache with the AAAA records only. No real outage is needed, only an IPv4-only host: a Docker container on the default bridge network is one.
BOOTmirrors the bootstrap list that OpenWrt passes: two reachable IPv4 servers and two IPv6 ones that fail at once. Output on master:repro.sh
fake_bootstrap_dns.py
Five builds side by side in the same setup:
-4With this PR the list was not touched during the outage. One lookup right after it returned only the A records, without any cache involved, and the full list was back 10 s later. That is why both parts are needed: disabling the cache removes stale answers during an outage, but a live lookup can still come back partial at its edge, and only the two-way comparison lets the list grow back.
Testing
resolver_updateand the query-cache case indns_poller_watchers. With themctestpasses, and valgrind reports no errors and no leaks.Relation to #214
#214 also makes the comparison two-way, with the same whole-element matching, but keeps the query cache. This PR is small and independent, so it can be merged on its own, and OpenWrt can pick it up without waiting for the larger refactor. If #214 lands first, I will rebase this down to the query-cache change and the tests.
Notes for users on affected versions
-4avoids the problem: only A records are queried, so the list cannot shrink to the wrong family./etc/init.d/https-dns-proxy restartdoes not help, because it is a reload and procd keeps an instance whose command line did not change. Killing the process does.c-ares needed more IO event handler, than the number of provided nameservers: 1when the bootstrap server stops answering. That is already fixed on master by 63c49b4, but OpenWrt still ships 0ba0525.🤖 Generated with Claude Code