feat(networking): add captive-portal helper for tailnet-owned DNS - #449
feat(networking): add captive-portal helper for tailnet-owned DNS#449Bad3r wants to merge 63 commits into
Conversation
tailscaled registers a resolvconf entry that supersedes NetworkManager's, so /etc/resolv.conf on tpnix carries only
100.100.100.100 and the dnsmasq at 127.0.0.1 configured by modules/hosts/common/private-dns-hosts.nix is shadowed. With
MagicDNS and split DNS both off, the tailnet pushes global resolvers 194.242.2.2 and 1.1.1.1, so every query leaves for
the public internet. A captive portal drops those upstreams before authentication, so lookups time out instead of being
hijacked: the sign-in redirect never fires and NetworkManager's connectivity check reports limited rather than portal.
Observed on SSID AC-WIFI, where no portal page was reachable at all.
captive-portal releases DNS back to dnsmasq, locates the portal by querying the access point's resolver directly with
dig, opens it, and replays the saved CorpDNS and WantRunning values on --restore. Restore invokes tailscale up only when
--down stopped a running node, because tailscale up resets every pref it is not passed and would otherwise clear the
node's RunSSH setting.
LibreWolf is the default browser and ships network.captive-portal-service.enabled false, so it raises no sign-in bar of
its own; the helper opens the portal URL directly.
The script carries a shellcheck shell=bash directive instead of a shebang because writeShellApplication supplies the
interpreter line.
Validation:
nix fmt (0 changed)
nix build --impure --expr '(import <nixpkgs> {}).callPackage ./packages/captive-portal { }'
nix eval path:.#nixosConfigurations.tpnix.config.environment.systemPackages (captive-portal present once)
nix flake check --accept-flake-config --no-build path:. (exit 0)
end-to-end run: /etc/resolv.conf 100.100.100.100 -> 127.0.0.1 -> 100.100.100.100 with CorpDNS/WantRunning/RunSSH intact
Bad3r
added
type(enhancement)
|
Claude finished @Bad3r's task in 7m 32s —— View job Review complete
1 finding, posted inline.
The Checked and found clean: |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Multiple logic bugs in the helper script regarding state preservation and portal detection.
-
packages/captive-portal/captive-portal.sh:save_prefsoverwrites the state file on every execution. If the script is run while DNS is already released (e.g., a second attempt), it saves the modified state as the "original," preventing automatic restoration of Tailscale DNS.
Fix: Add[ -f "$state_file" ] && return 0at the start ofsave_prefs. -
packages/captive-portal/captive-portal.sh:is_private_v4uses flawed shell globs (e.g.,10.*,172.1[6-9].*) that incorrectly match public or CGNAT ranges (e.g.,100.64.0.1matches10.*, and172.160.0.1matches172.1[6-9].*).
Fix: Use more restrictive patterns like10.[0-9]*or validate the character following the prefix is a dot. -
packages/captive-portal/captive-portal.sh:probe_portalinterprets a200 OKresponse for the gstaticgenerate_204probe as a successful "clean" connection. This probe is defined to return204; a200response typically indicates a portal hijack.
Fix: Specifically verify that the gstatic probe returns204. -
packages/captive-portal/captive-portal.sh: The script callstailscaleandnmcliwhich require root privileges, but it is designed to run in a user context (usingXDG_RUNTIME_DIRandxdg-open). Running the entire script withsudowill causexdg-opento fail or behave unsafely.
Fix: Usesudoselectively for networking commands or document the required PolKit/sudoers configuration.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a88bd1008
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…eadable state file save_prefs re-snapshotted on every login-mode run, so a retry after a failed sign-in recorded CorpDNS=false as the original value; the following --restore then read corp_dns=false, skipped `tailscale set --accept-dns=true`, and still printed "DNS returned to Tailscale". Tailscale DNS stayed off with a success message on screen. The snapshot now stands until --restore consumes it, and it is written through a temporary so a failing `tailscale debug prefs` cannot leave the 0-byte file the old `>"$state_file"` redirect truncated before the pipeline's status was known. restore_dns read that file with a bare `read -r corp_dns want_running <"$state_file"` inside an if body, where errexit is not suppressed: on a 0-byte file read returned 1 and killed the script before `tailscale set --accept-dns=true`, `rm -f "$state_file"`, and `nmcli general reload dns-full`, so every later --restore repeated the same silent abort. Reproduced against the previous script with a stubbed tailscale: exit 1, no tailscale invocation, state file left in place. Both fields now fall back to restoring Tailscale when the file is empty, short, or holds anything other than true/false. `grep '^nameserver' /etc/resolv.conf` ran unguarded at the end of restore_dns and after release_dns. A network that hands out no DNS before authentication leaves resolv.conf with no nameserver line, and grep's exit 1 then killed the run before probing, with DNS released and no restore attempted. show_resolvers reports the empty case instead, and names the 127.0.0.53 or 127.0.0.1 stub for what it is, since on a systemd-resolved host resolv.conf never shows the change. No trap existed for INT or TERM, and probing three canaries takes up to ~24s. A Ctrl-C in that window returned the user to the shell with Tailscale DNS off and nothing saying --restore was needed; the handler prints that and re-raises. Validation: nix fmt (0 changed) shellcheck -s bash packages/captive-portal/captive-portal.sh stubbed tailscale/nmcli: --restore against empty, garbage, and honest false state files
The generate_204 canary carried an empty expected string, and the 200|204 arm only inspected the body when that string was non-empty, so any 200 from connectivity-check.gstatic.com counted as clean. An inline proxy that answers /generate_204 with its sign-in page instead of redirecting is a common portal shape, and it was read as an open network: with the other two canaries unreachable the run reported "no portal detected" and, in login mode, handed DNS back to Tailscale while the user was still walled off. The status is the whole answer for that URL, so 204 is clean and 200 is the portal. Reproduced against 7a88bd1 with stubbed dig/curl: exit 1 and no URL, against exit 0 and http://portal.lan/login now. The is_private_v4 recheck then ran even when the same iteration had already matched the expected payload, so a canary that answered "success" from a private address was reported as a hijack anyway. Same fixture: 7a88bd1 printed http://192.168.1.50 for a correct success.txt, and the per-host verdict now short-circuits that check. is_private_v4 itself missed 169.254.0.0/16 and 100.64.0.0/10, which is where a portal answers when it has issued no lease and on carrier or guest networks respectively. device_gateway read $3 out of `ip -4 route show default`, which is the gateway only when the route has a via. An on-link default route ("default dev wlan0 scope link") put the device name there, so the fallback URL became http://wlan0 and went to xdg-open. It now reads the field after via and validates the dotted quad, leaving no gateway when there is none. Device selection took whichever connected wifi or ethernet device nmcli listed first, so a docked laptop probed an arbitrary one of its two links. The candidate list is sorted wifi-first and then by name, and the devices passed over are named on stderr. The per-probe mktemp moved to a single file removed by an EXIT trap, so an interrupted run no longer leaks it. Validation: nix fmt (0 changed) shellcheck -s bash packages/captive-portal/captive-portal.sh stubbed dig/curl/ip: 204-clean, inline-200 portal, private-address-clean canary, CGNAT hijack, on-link default route
…strand a release --probe could not report what it found. "No portal detected" and "portal at $portal" both exited 0, so `if captive-portal --probe` could not tell a clean network from one holding a sign-in page, and the URL was buried among the diagnostics on stdout. Exit status now names the outcome (0 portal, 1 clean, 2 usage, 3 inconclusive, 4 uninspectable), stdout carries the URL alone, and every diagnostic goes to stderr. usage() documents the table. The inconclusive-with-no-gateway path exited 1 on the spot. In login mode release_dns had already turned Tailscale DNS off, so that exit left the host with DNS released, no portal found, no restore, and no reminder that --restore was needed: unlike every other exit in the script. Reproduced against 7a88bd1 with stubbed dig/curl returning nothing: tailscale set --accept-dns=false with no matching --accept-dns=true and the state file left in $XDG_RUNTIME_DIR. That path now restores before exiting 3. The gateway fallback is a guess, not a detection: on a network that merely blocks the canaries it is the user's own router. It was announced with the same "portal at $portal" line a confirmed hijack produced, so the two were indistinguishable once the browser opened. It now says no portal was confirmed and what the address actually is. `captive-portal --device --probe` consumed --probe as the device name, stayed in login mode, and released real DNS for a request that asked to change nothing, then failed with "has no DHCP-provided resolver". A value starting with - is rejected as usage. save_prefs failing now exits 4 explicitly instead of relying on errexit to unwind release_dns. Validation: nix fmt (0 changed) shellcheck -s bash packages/captive-portal/captive-portal.sh stubbed tailscale/nmcli/ip/dig/curl: --device --probe rejected; probe clean exits 1 with empty stdout; probe with a portal exits 0 with the URL alone; login clean releases then restores; login inconclusive restores and exits 3
The "Why the portal never appears" section presented the NetworkManager dnsmasq chain as a property of the hosts in this
repository. modules/hosts/common/private-dns-hosts.nix only sets networking.networkmanager.dns = "dnsmasq" and disables
services.resolved for hosts that declare privateDnsHostsSecretKeys, and modules/tpnix/policy.nix is the only declaration
("signalx_hosts"). Evaluating the other host contradicts the doc: system76 reports networkmanager.dns
"systemd-resolved", services.resolved.enable true, and environment.etc."resolv.conf".source
/run/systemd/resolve/stub-resolv.conf, a static symlink whose 127.0.0.53 stub reads identically before and after
captive-portal releases DNS. An operator there followed "Verify both facts" and saw none of the promised output. The
section now separates the two resolvers, opens with readlink -f /etc/resolv.conf to tell them apart, and sends the
systemd-resolved host to resolvectl status, where tailscaled installs the tailnet resolvers against the tailscale0 link.
The by-hand fallback hardcoded `nmcli -t -f IP4.DNS device show wifi0`. wifi0 is the name modules/tpnix/networking.nix
pins through a .link file; system76 keeps the kernel's wlan0, so the command matched no device, resolver came out empty,
and the following dig ran with an empty @ argument. It discovers the connected wireless device instead and says why.
Also documents what the helper now guarantees: the URL alone on stdout, the exit-status table, that status 3 is the
gateway rather than a confirmed portal, and which paths restore DNS on their way out.
Validation:
nix fmt (1 changed, table reflow)
nix eval path:.#nixosConfigurations.system76.config.networking.networkmanager.dns
nix eval path:.#nixosConfigurations.system76.config.services.resolved.enable
nix eval path:.#nixosConfigurations.system76.config.environment.etc."resolv.conf".source
nix eval path:.#nixosConfigurations.tpnix.config.networking.networkmanager.dns
…stubbed network tools captive-portal is instantiated only through the overlay in modules/custom-overlays/captive-portal.nix, so a host build lints the script with shellcheck and stops there. Every decision it makes depends on what dig, curl, nmcli, ip and tailscale answer, none of which exists in a build, so nothing had ever executed it. The eleven defects fixed in 7cee48b, 1a45cb3 and 45e7d1a all lived in that gap. All five tools are package arguments, so callPackage substitutes stubs and writeShellApplication's PATH prefix puts them ahead of the sandbox: the packaged script runs unmodified against a scripted network. Each scenario pins one failure that shipped, verified by building this check against 7a88bd1: --device --probe consumed the flag as a device name and released real DNS a 200 from generate_204 counted as a clean network instead of an inline portal a canary answering "success" from a private address was reported as a hijack 100.64.0.0/10 and 169.254.0.0/16 answers were not recognised as hijacks an inconclusive probe with no gateway exited leaving Tailscale DNS off an on-link default route printed http://wifi0 as the portal URL a retry re-snapshotted the released prefs, so --restore left DNS off and said otherwise an empty or truncated state file aborted --restore before it restored anything the device pick followed nmcli's listing order rather than preferring wifi /etc/resolv.conf does not exist in the sandbox, which is also what a portal that hands out no lease produces, so the login scenarios additionally cover the unguarded grep that used to abort between releasing DNS and probing. Validation: nix fmt (0 changed) nix build path:.#checks.x86_64-linux."packages/captive-portal-runtime" (exit 0) same check against 7a88bd1's script: fails on the first scenario per-finding comparison of both scripts' helpers under stubs: is_private_v4, device_gateway, device pick
probe_portal returned 2 for "every probe was inconclusive" and the caller passed that value straight to exit, so a run that fell back to the gateway exited 2, which usage() and docs/networking/README.md both define as invalid usage. The empty-gateway branch was the only inconclusive path that reached the documented 3, because that one is written out literally. probe_portal now returns the status the script exits with, and the check gained the case that missed it: inconclusive probes with a gateway present exit 3, print http://192.168.1.1 on stdout, and say no portal was confirmed. Validation: nix fmt (0 changed) shellcheck -s bash packages/captive-portal/captive-portal.sh nix build path:.#checks.x86_64-linux."packages/captive-portal-runtime" (exit 0) stubbed dig/curl/ip: --probe with unreachable canaries exits 3 with the gateway URL and touches no tailscale state
…aces The file header presented NetworkManager's dnsmasq at 127.0.0.1 as the resolver tailscaled shadows. That holds only for hosts selected by modules/hosts/common/private-dns-hosts.nix, which is tpnix alone; system76 evaluates to networkmanager.dns "systemd-resolved" with services.resolved enabled, and tailscaled installs its configuration there rather than through resolvconf. The behaviour the script implements is the same in both cases, so the header now names both resolvers instead of one, matching the correction made to docs/networking/README.md in dfad85e. Validation: nix fmt (0 changed) shellcheck -s bash packages/captive-portal/captive-portal.sh nix build path:.#checks.x86_64-linux."packages/captive-portal-runtime" (exit 0)
The exit-3 outcome means no probe confirmed a portal and the address printed is whatever answers as the default gateway, which on a network that merely blocks detectportal.firefox.com, captive.apple.com and connectivity-check.gstatic.com is the user's own router. Handing that to xdg-open opened the router admin page unasked. Only a confirmed detection now reaches the browser; the guess is printed, and the closing text says it was not opened and that DNS stays released until --restore runs. The runtime check gained the case. It runs the guess first and the confirmed detection second, so the wait for the detection's xdg-open doubles as the settle time the guess had to produce one: the script backgrounds that call, so a log read taken the instant it exits proves nothing. Verified by ungating the launch, which fails the check with "a gateway guess must not be opened in a browser". Validation: nix fmt (0 changed) shellcheck -s bash packages/captive-portal/captive-portal.sh nix build path:.#checks.x86_64-linux."packages/captive-portal-runtime" (exit 0; exit 1 with the launch ungated) stubbed run: exit 3 prints http://192.168.1.1 on stdout with no xdg-open call
…ract 4043c3a pinned curl's --resolve because curlStub answers from the URL alone and nothing else would catch its loss. digStub has the identical shape: it answers from the host name alone via "${!#}" and never inspected "@$resolver", so deleting "@$resolver" from the dig call in probe_portal (packages/captive-portal/captive-portal.sh) left every scenario green while production queried the ambient resolver instead, 100.100.100.100 under --probe (docs/networking/README.md). $answer would stop describing anything the access point returned, and the hijack arm plus both sinkhole guards would grade an address no portal chose. The extraction loop matches curlStub's own shape, a `for arg in "$@"` scan rather than a whole-command-line substring match, so a failure names just the resolver value it got, not the full argument list. Confirmed non-vacuous: dropping "@$resolver" from the dig call fails the check with the stub's own "must query the access-point resolver" message. Validation: nix build path:.#checks.x86_64-linux."packages/captive-portal-runtime" (exit 0), plus that mutation (exit 1).
|
Both automated-review findings from the latest Implemented
Rejected: none. Both findings verified accurate against current code with no false claims. Decisions escalated to the user: none. Both fixes were fully derivable from the repo (the existing Consequential fixes beyond the literal comments: updated the PR body's "Details worth review" and "Test plan" Reliability notes: both fixes land inside existing, already-proven patterns rather than new machinery: the Validation: |
…his machine probe_portal checked the scheme of a 30x Location and of $found (the value extract_url reads from formaction/action, url, or href), but never the host. curl --resolve only pins where the request goes: a portal or a filtering resolver in front of it chooses Location and the page body freely, so Location: http://127.0.0.1/ or <form action="http://127.0.0.1/"> reached xdg-open unfiltered, the exact harm the sinkholed-canary guard on $answer (127.0.0.0/8, 0.0.0.0) exists to stop, on the two paths it does not cover. is_loopback_host applies that same exclusion, plus localhost and ::1 which is_private_v4 never had to name since $answer only ever carries a dotted-quad, to both $redirect and $found. A rejected $redirect falls through to the hijack check the way an unmatched status already does; a rejected $found falls back to http://$answer the way an empty one already does.
…orm target extract_url's only exclusion was w3.org, so a relative <form action="/login"> left tier 1 and tier 2 empty and tier 3 took the first absolute href on the page. A stylesheet or script sitting ahead of the form in <head>, the same shape 28eb90a's absolute-action scenario already covers, then won: the run printed the asset URL as the portal and xdg-open'd it, and the sign-in form was never shown. The two existing relative-action scenarios both happened to use a w3.org href, so the existing filter hid the gap. An asset extension is never a sign-in target in any of the three tiers, so the filter sits next to the w3.org one in the shared pipeline rather than in tier 3 alone. Finding nothing still falls back to http://$answer, the host that served the page, the same as the existing "no usable link" case.
…ss exists probe_portal returns 3 with nothing on stdout when device_gateway found no via (an on-link default route, or no default route at all), and the caller already exits 3 on an empty $portal without printing one. usage() and the status table in docs/networking/README.md both stated unconditionally that status 3 prints a gateway guess, so a wrapper written against either contract would read an empty string as a URL. The in-source comment above probe_portal already had the qualifier right; the two documented contracts did not. No behavior changes: probe_portal's own comment already named "when there is one", and the two runtime-check scenarios for the no-gateway path (empty stdout, exit 3) already pass unchanged.
…acted link 7df8b4c added is_loopback_host so a Location or an extracted link naming this machine, localhost, or ::1 is dropped the same way a canary sinkholed to one already was, but this section still described only the scheme check that predated it: "Nor is a Location header trusted for its scheme: only http and https targets are ever printed or opened." That understated the current guard and left a reader unaware the host is checked too.
Sixteenth review round resolvedThree threads, four commits. All three held; two were implemented past the letter of the suggested diff to close a gap the comment itself named but the diff didn't cover. Implemented
Rejected: none. All three findings verified accurate against current code. Decisions escalated to the user: none. All three fixes were fully derivable from the repo (the sinkhole-guard precedent already on Consequential fixes beyond the literal comments:
Reliability notes: both Validation: |
…symlinked state directory save_prefs (ca8e324) refuses to write through a symlinked state_dir, but restore_dns reaches the same path with no such check: it reads $state_file and later rm -f's it. sudo captive-portal --restore is the documented invocation (docs/networking/README.md), and under sudo, state_dir falls back to /run/user/$SUDO_UID/captive-portal, a name the invoking user controls. state_dir being a symlink is a parent-directory path component, dereferenced by the kernel on every access through it, so a symlink planted there has root's read and root's rm -f both operate inside whatever directory the invoking user named, on the half of this script that runs as root more often than save_prefs does. The check exits 4 rather than returning, matching restore_dns's other failure paths: its only call site does restore_dns; exit 0 with no rc capture, so a bare return would be swallowed into a false success.
…racted link against this machine
is_loopback_host tested ${1#*//} directly, which is the whole authority including userinfo: http://x@127.0.0.1/
strips to x@127.0.0.1/, matches no arm, and the 30x branch prints it and hands it to xdg-open exactly as the bare
form would. Same at the extraction site: <form action="http://a@localhost/pwn"> was printed as the sign-in URL.
Userinfo is a field of the authority curl's redirect_url and extract_url's regex both leave untouched.
The path is stripped first (${authority%%/*}) so the trailing patterns only ever see a bare host or host:port, and
userinfo is stripped with ##*@ rather than #*@: a parser splits on the last @ in a multi-userinfo authority
(http://a@b@127.0.0.1/ resolves host=127.0.0.1 in both the WHATWG URL algorithm and urllib.parse), and #*@ takes
the first, leaving that shape unguarded.
…he body 85082fd's extension filter only catches an asset URL ending in one, so the two commonest <link> shapes still won tier 3: a preconnect hint (<link rel="preconnect" href="https://fonts.gstatic.com">) has no path at all, and a stylesheet behind a font-service query string (.../css2?family=Inter) has no dot before its extension. On the relative-form-action page shape 85082fd already covers, tier 3 returned the font host, and the run printed and opened it instead of falling back to the address that answered the hijacked lookup. A <link> element is metadata: never a submit target, never a refresh, so its href is dropped before any tier reads the body rather than guessed at by extension. The extension filter stays for an <a href> or <form action> pointing at an asset directly, which the strip does not touch.
Seventeenth review round resolvedThree threads, three commits. All three held, all against code from the two immediately preceding rounds: two on Implemented
Rejected: none. All three findings verified accurate against current code. Decisions escalated to the user: none. All three fixes were fully derivable from the repo and from cross-checking against standard URL-parsing behavior (Node's WHATWG Consequential fixes beyond the literal comments:
Reliability notes: the userinfo fix is the one place this round improved on its own suggested diff rather than just applying it, verified independently against two different URL-parsing implementations before committing to Validation: |
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
The runtime check covered single-line link stripping and lowercase loopback spellings, but it did not constrain wrapped metadata attributes or case-insensitive and unspecified IPv6 loopback targets. Add those scenarios and document the existing URL flattening, host normalization, and symlink-safe ownership behavior.
Network-controlled form, href, meta-refresh, and Location values reach diagnostics and xdg-open. GNU grep also treats a NUL-bearing response as binary and can suppress extraction. Normalize NUL before parsing, exclude control bytes from extracted targets, reject control-bearing redirects, and cover the behavior with raw-NUL, ESC, and --no-open runtime scenarios. Validation: bash -n packages/captive-portal/captive-portal.sh; shellcheck packages/captive-portal/captive-portal.sh; nix build path:.#checks.x86_64-linux."packages/captive-portal-runtime".
Review feedback resolvedImplemented:
Rejected items: None. Both current unresolved findings were validated as actionable and implemented. Decisions requested: None. No preference, identity, or consequential policy value was missing from the repository contract. Consequential follow-on fixes:
Validation:
Reliability: The new paths fail closed for malformed network-selected URLs, keep stdout suitable for wrappers, keep |
| tr '\n\000' ' ' <"$body_file" | | ||
| sed 's/<link[^>]*>//gI' | | ||
| grep -oiE '(^|[;[:space:]])('"$1"')=["'"'"']?https?://[^"'"'"'<>[:space:][:cntrl:]]+' | | ||
| grep -oiE 'https?://[^"'"'"'<>[:space:][:cntrl:]]+' | | ||
| grep -viE '^https?://([^/]*\.)?w3\.org([/:?]|$)' | | ||
| grep -viE '\.(css|js|mjs|png|jpe?g|gif|svg|ico|woff2?|ttf|eot)([?#]|$)' | | ||
| head -1 | | ||
| sed 's/&/\&/g' || true |
There was a problem hiding this comment.
[ISSUE] & ends in ;, and ; is in the anchor class, so the ?url=/&url= hole b3413e8 closed is reopened by the encoding HTML requires for that very separator.
A literal & inside an attribute value must be written &, so a portal page that parks the originally-requested address in a query parameter emits ...&url=http://.... Tier 2 anchors on (^|[;[:space:]]), the character immediately before url= is the ; of &, and the match fires. Decoding happens at line 291, after the anchoring grep has already run, so it cannot help.
Concretely, with a relative form action (so tier 1 finds nothing):
<a href="/login?res=x&url=http://detectportal.firefox.com/success.txt">continue</a>
<form action="/auth"></form>
tier 2 extracts http://detectportal.firefox.com/success.txt, and the run prints the canary's own URL as the portal and hands it to xdg-open — on a proxy portal that is a public host the browser will never reach from inside the garden. The scenario at modules/packages/captive-portal-check.nix:467 pins the raw ?url= form and passes for two independent reasons (the anchor excludes it and that page's <form action> is absolute), so it does not cover this variant.
Decoding before the anchored grep fixes it without touching the anchor: &url= becomes &url=, and & is not in the class, while content="0; url=..." and content="0;url=..." still match. The trailing decode then becomes redundant and must go, or &amp; would be decoded twice.
| tr '\n\000' ' ' <"$body_file" | | |
| sed 's/<link[^>]*>//gI' | | |
| grep -oiE '(^|[;[:space:]])('"$1"')=["'"'"']?https?://[^"'"'"'<>[:space:][:cntrl:]]+' | | |
| grep -oiE 'https?://[^"'"'"'<>[:space:][:cntrl:]]+' | | |
| grep -viE '^https?://([^/]*\.)?w3\.org([/:?]|$)' | | |
| grep -viE '\.(css|js|mjs|png|jpe?g|gif|svg|ico|woff2?|ttf|eot)([?#]|$)' | | |
| head -1 | | |
| sed 's/&/\&/g' || true | |
| tr '\n\000' ' ' <"$body_file" | | |
| sed -e 's/<link[^>]*>//gI' -e 's/&/\&/g' | | |
| grep -oiE '(^|[;[:space:]])('"$1"')=["'"'"']?https?://[^"'"'"'<>[:space:][:cntrl:]]+' | | |
| grep -oiE 'https?://[^"'"'"'<>[:space:][:cntrl:]]+' | | |
| grep -viE '^https?://([^/]*\.)?w3\.org([/:?]|$)' | | |
| grep -viE '\.(css|js|mjs|png|jpe?g|gif|svg|ico|woff2?|ttf|eot)([?#]|$)' | | |
| head -1 || true |
1 participant
Summary
Connecting to a captive-portal SSID (observed on
AC-WIFI) leaves tpnix with no usable DNS and no reachable sign-inpage. tailscaled registers a resolvconf entry that supersedes NetworkManager's, so
/etc/resolv.confcarries only100.100.100.100and the dnsmasq at127.0.0.1frommodules/hosts/common/private-dns-hosts.nixis shadowed.resolvconf -llists NetworkManager'snameserver 127.0.0.1while/etc/resolv.confdoes not.With MagicDNS and split DNS both disabled tailnet-wide, the coordination server pushes global resolvers
194.242.2.2and
1.1.1.1, so100.100.100.100forwards every query to the public internet. A portal drops those upstreams beforeauthentication, so lookups time out rather than being hijacked. The portal's DNS hijack is what produces the sign-in
redirect, so nothing surfaces the portal and NetworkManager's connectivity check settles on
limitedinstead ofportal.This adds a
captive-portalhelper that releases DNS back to dnsmasq, locates the portal by querying the access point'sresolver directly with
dig, opens it, and restores the saved Tailscale state afterwards.packages/captive-portal/with the script and itswriteShellApplicationwrapper.modules/apps/captive-portal.nixandmodules/custom-overlays/captive-portal.nixfollowing thednsleakpattern.modules/hosts/common/apps-enable.nix.programs.tailscale.extended.operatorinmodules/apps/tailscale.nix, so tailscaled takes a state change from theowner rather than root alone. It reaches the daemon through
services.tailscale.extraSetFlags, which nixpkgs replaysfrom a root oneshot on every boot, and an assertion rejects a name no host account carries.
modules/packages/captive-portal-check.nix, a runtime check that drives the packaged script against stubbeddig,curl,nmcli,ip,tailscaleandxdg-open. Nothing else executes this script: a host build lints it withshellcheck and stops there.
docs/networking/README.mdcovering the resolver chain, the helper, amanual fallback, and two host-specific traps.
Details worth review:
511, or acanary resolving to an address on this LAN. The first three also catch a proxy that intercepts HTTP and leaves DNS
alone. The sign-in URL comes from what a link, form or meta refresh points at, falling back to the address the canary
resolved to, because the first absolute URL in a page is as often a DOCTYPE's w3.org DTD or a CDN script tag. Link
metadata is stripped before extraction, wrapped attributes are flattened, and control bytes are excluded from
extracted targets, while a
Locationtarget containing one is rejected before it reaches diagnostics orxdg-open.--restorestarts the node only when the snapshot recorded one that was running. With no snapshot there is noevidence the node was ever up, and starting one the user stopped is not a restore. The call is a flagless
tailscale up, whichcheckForAccidentalSettingRevertsincmd/tailscale/cli/up.goshort-circuits to aWantRunning-only edit when no flag is set, so it cannot clear this node's
RunSSH.tailscale set --accept-dns=trueis the operator wall and names it; a refused
tailscale uphappens after DNS is already back, so it reports thestopped node instead and still runs the NetworkManager reload. Both keep the snapshot, so
--restorestaysretryable.
sudothe snapshot is keyed offSUDO_UID, becausesudo-rsenforcesenv_resetwith no opt-out andmodules/hosts/common/sudo.nixkeeps onlySSH_AUTH_SOCK, soXDG_RUNTIME_DIRis gone and root's own/run/user/0would hide the snapshot from a later plain--restore. Root still inherits noDISPLAY, Waylandsocket or session bus, so the browser half is documented rather than fixed: pass
--no-openand open the URL, whichprints either way.
save_prefsandrestore_dnsboth refuse to use it if it is a symlink:mkdir -pis a silent no-op against onethat already resolves to a directory,
hand_state_to_invokeruseschown -hfor the snapshot entries, and a plainreadandrm -fwould follow a symlink on the restore side. The guards prevent root from handing a root-ownedtarget's ownership to the invoking user, or reading and unlinking a file the invoking user named. No privilege
boundary crosses on hosts where sudo is wheel-wide, so this is hardening rather than a fix for an active escalation
here.
network.captive-portal-service.enabledfalse, so it never raises asign-in bar; the helper opens the portal URL directly.
Test plan
nix fmtreports 0 changed.nix build --impure --expr '(import <nixpkgs> {}).callPackage ./packages/captive-portal { }'succeeds, which alsoruns shellcheck through
writeShellApplication.nix build path:.#checks.x86_64-linux."packages/captive-portal-runtime"exits 0. Scenarios cover classification(DNS hijack, inline portal page, 30x with and without a
Location, RFC 6585's 511, a sign-in target read from alink rather than from a DOCTYPE or a CDN tag, one address per arm of
is_private_v4plus the public address thatmust not match any), every path that released DNS giving it back, the refused-release and refused-restore
mirrors, the snapshot's retry contract,
--down, and the failures that used to pass for success: a transfer thatdied after the response line, a scratch file that cannot be created and one that outlived the run, a snapshot
that cannot be written or unlinked or is reached through a symlinked state directory, an unreadable
tailscale debug prefs, nmcli failing to list devices, a--devicename nmcli does not know, wrapped linkattributes, loopback aliases, raw NUL and ESC-bearing portal targets, control-bearing redirects, and
--no-openbrowser suppression. Existing scenarios were confirmed non-vacuous by mutating the fix back out and watching the
named assertion fail. The new cases directly assert the raw-NUL extraction, control-free output and browser
argument, redirect fallback, and
--no-opensuppression contracts.nix eval path:.#nixosConfigurations.tpnix.config.environment.systemPackagesfindscaptive-portalexactly once,proving module discovery, overlay, and host closure wiring.
nix flake check --accept-flake-config --no-build path:.exits 0.pre-commithooks pass, including shellcheck, statix, deadnix, treefmt, and typos./etc/resolv.confmoved100.100.100.100to127.0.0.1and back, the networkwas classified as having no portal, and
CorpDNS,WantRunning, andRunSSHwere unchanged afterwards.sudohalves of the state-directory choice are not covered by the runtime check: the builder is uid 1000 andcannot create
/run/user, so neither theSUDO_UIDfallback nor the chown that hands the snapshot back can runthere. The check asserts the precedence an ordinary run does cross, and its header records what it cannot reach.
Note:
nix flake check --no-buildfirst fails with adid not exist in the store during evaluationerror, onbrowsers/firefoxpwa-module-evalandhm-apps/rclone-protondrive-otp. That is--no-buildrefusing those checks' IFDfixtures, unrelated to this change; building them directly succeeds and the full check then exits 0.