Skip to content

Warn about local destinations named by host, not just by address - #707

Merged
kasnder merged 1 commit into
claude/trackercontrol-issue-701-9x335ifrom
fix/local-network-observed-destinations
Aug 5, 2026
Merged

Warn about local destinations named by host, not just by address#707
kasnder merged 1 commit into
claude/trackercontrol-issue-701-9x335ifrom
fix/local-network-observed-destinations

Conversation

@kasnder

@kasnder kasnder commented Aug 5, 2026

Copy link
Copy Markdown
Member

Stacked on #704 — base is claude/trackercontrol-issue-701-9x335i, so review that first. Merge #704 and this retargets to master on its own.

The gap

LocalNetworkAccess.isConfigured() in #704 classifies the stored configuration, and deliberately only looks at address literals: resolving a name there would mean a blocking lookup on whichever thread asked, including the main one. That leaves a hole:

  • a Secure DNS endpoint at https://pi.hole/dns-query
  • a WireGuard peer on a dynamic DNS name that points home

Both resolve to the local network, both are blocked on Android 17 without ACCESS_LOCAL_NETWORK, and both read as remote. Those users get no banner and no prompt — just a resolver that quietly stopped answering, which is the exact failure #701 reported.

The change

Take the address from the code that has already resolved it, so nothing new is looked up and no thread blocks:

  • DoH — OkHttp resolves the endpoint regardless, so the client gets a Dns that delegates to Dns.SYSTEM and notes what came back.
  • WireGuardWgEgress.resolveEndpoint() already holds the peer's address when it builds ip:port.

Both call LocalNetworkAccess.reportDestination(address), which latches when the address is local. isMissing() ors that in beside isConfigured(), so the banner and the prompt behave identically whether the configuration named an address or a host.

Two properties worth stating:

  • The destination is what matters, not whether the connection failed. If it is local and the permission is missing, that traffic is blocked — no need to wait for a timeout to say so.
  • The latch is in-memory only. A stale observation dies with the process, and anything still pointing at the LAN re-reports itself on next use. Changing a relevant setting clears it, since the new value may point somewhere else entirely.

reportDestination() is deliberately not gated on isEnforced()isMissing() applies that gate anyway, and leaving the branch out makes the latch testable under Robolectric, which runs below the enforcement level.

Testing

Four new unit tests: a LAN destination latches, a public one and a host name do not, reset clears it, and an observation never leaks into isConfigured(). 27 tests in LocalNetworkAccessTest, all passing.

Verified on a Pixel 8 (Android 17, API 37) with ACCESS_LOCAL_NETWORK denied, using a DoH endpoint at https://MAC-G0WPC6L7Y4.fritz.box:8443/dns-query — a host name, with no address literal anywhere in the configuration:

  • Before any DoH traffic: no banner on ActivityMain. isConfigured() cannot classify a host name, which is the gap this PR closes.
  • After the tunnel came up and DoH resolved the endpoint: banner present on the next onResume.
  • Negative control — same flow with https://dns.quad9.net/dns-query in a fresh process: no banner. The latch fires on the address, not on the mere fact that DoH ran.

One observation from testing, not a defect in this PR but worth recording: the FRITZ!Box returned both a private A record (192.168.178.128) and a global AAAA (2a02:a470:...), because it delegates a public IPv6 prefix to LAN hosts. The latch fired on the IPv4. A LAN host reachable only over a global IPv6 address would not be classified as local by isLocalAddress(), which goes by address range. Whether Android's own local-network enforcement is purely range-based or also same-link aware is untested here — if it is same-link aware, that is a gap in isLocalAddress() itself, predating this PR and worth its own investigation.

isConfigured() classifies the stored configuration, so it only sees
addresses: a DoH endpoint at https://pi.hole/dns-query, or a WireGuard
peer on a dynamic DNS name, resolves to the LAN but reads as remote.
Those users get no banner and no prompt — just a resolver that stopped
answering. Classifying them up front would mean resolving a name on
whichever thread asked, including the main one, which is exactly what
the address-only rule exists to avoid.

Take the address from the code that already resolved it instead. OkHttp
resolves the DoH endpoint anyway, so hang a Dns off the client and read
what it got; WgEgress.resolveEndpoint() likewise has the peer's address
in hand. Both hand it to LocalNetworkAccess.reportDestination(), which
latches if it is local, and isMissing() ors that in.

The destination is what matters, not whether the connection failed: if
it is local and the permission is missing, that traffic is blocked. The
latch is in-memory only — a stale observation dies with the process, and
anything still pointing at the LAN re-reports on next use. Changing a
relevant setting clears it, since the new value may point elsewhere.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@kasnder
kasnder force-pushed the fix/local-network-observed-destinations branch from bbf7313 to bbda98b Compare August 5, 2026 09:13
@kasnder
kasnder merged commit 9d4bcfc into master Aug 5, 2026
4 checks passed
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.

1 participant