Commit Graph
982 Commits
Author SHA1 Message Date
Codescribe 0f310594b0 Redact provision token in logs 2026-08-24 15:14:59 +07:00
Cuong Manh Le 0db5ee44be cmd/cli: log the config error that rejected a custom config
The warning reported err, the resolver-config fetch error, which is nil on
every path that reaches it - so a rejected custom config was logged with no
reason attached. cfgErr holds the validation failure.
2026-08-14 15:30:19 +07:00
Cuong Manh Le 062dcbfb12 cmd/cli: report whether ctrld finished starting up, not just what SCM thinks
"ctrld status" reported the service manager's view and nothing else, so it
printed "Service is running" and exited 0 for the incident's process: alive,
registered as started, stuck in API preflight, with no control socket, no DNS
listener and no policy applied. The one command an operator reaches for first
confirmed the service was fine while the host had no working DNS.

Probe the control server's /started endpoint before reporting success. That
endpoint only answers once the onStarted hooks have completed, which is after the
listeners are up, so a successful probe means the process is serving rather than
merely alive. A service that is registered as running but cannot confirm startup
is now reported as such, with a pointer to the log, and exits 3 - distinct from
stopped (1) and unknown (2), because it needs a different response.

A probe blocked by permissions is not evidence of a broken service: an
unprivileged caller still gets "Service is running", with a note that startup
was not verified. The probe is bounded by a short timeout so status stays fast.

Document the exit codes in the command's help, and cover the probe (ready, not
finished starting, no socket, timed out) and the classification, including that
an unreadable socket is not reported as a failure.

The not-ready verdict is only reported when the probe could have found the
daemon's socket. socketDir() is caller-relative on unix - the system directory
when writable, the caller's home otherwise - so an unprivileged "ctrld status"
looks somewhere the root-owned daemon never listened and gets ENOENT, which is
"wrong path", not "not ready". Since only darwin has an elevation PreRun and the
root-level alias has none, that is the normal invocation; reporting exit 3 there
would have told a monitoring check to restart healthy daemons. Such a caller now
gets the service manager's view with startup reported as unverified. Windows and
mobile resolve the same directory for every caller, so the verdict stays fully
available on the platform the hung start was seen on. A successful probe is still
conclusive whoever ran it.
2026-08-14 15:30:05 +07:00
Cuong Manh Le 88e076abe7 cmd/cli: stop the replacement before rolling its binary back
Rollback ran os.Remove(bin) while the replacement service was still running from
that image. Windows locks a running executable, so the remove failed with
"Access is denied" - and it was fatal, so the os.Rename that restores the
previous binary never ran. The upgrade ended with the broken replacement still
installed and the working binary stranded at its _previous name, which is what
the Firewall Mode incident produced.

Readiness failing is not evidence the process exited: the service manager can
report a started service whose process never became operational. So rollback now
stops the service and waits until the manager reports it stopped before touching
the executable, then resets DNS the way the restart path's Cleanup task does.
That also leaves the host in a known state - a stopped ctrld holds no WFP or
NRPT enforcement, so a replacement that was blocking traffic stops blocking it
at rollback instead of at the next reboot.

Restoring is now conditional on the previous binary reporting a version. The
incident's _previous file existed but produced no version output; renaming that
over the current binary would have traded a service that starts and hangs for
one that cannot start at all. When it is unusable, rollback keeps it for
inspection, leaves the installed binary alone, and says so instead of pressing
on. The --version probe is bounded by a timeout so a binary that hangs cannot
hang the upgrade.

Remaining failures return errors rather than calling Fatal, so each one reports
what state the host was left in. os.Remove is retried while the path stays
locked, since Windows releases an image lock asynchronously after the process
exits.

Extract the rollback into rollbackToPreviousBinary() and cover it: the stop
happens while the executable is still present, an unusable previous binary is
kept without swapping or restarting, and a failed stop aborts before anything is
modified. Reversing the stop and the remove fails these tests.

The version probe is called through a variable so those tests do not have to
stage a runnable executable. Staging one is not portable: oldBin is
bin+"_previous", so a fixture named "ctrld" yields the extension-less
"ctrld_previous", which Windows refuses to execute, and a symlink to the test
binary needs a privilege Windows does not grant by default. The probe itself is
still covered against the real test binary. Production is unaffected: ctrld.exe
_previous does have an extension, and os/exec only appends PATHEXT entries when
a path has none at all - noted at binaryVersion so the suffix is not renamed
into something extension-less by accident.
2026-08-14 15:29:49 +07:00
Cuong Manh Le 052b057756 all: keep the first-attempt error when the direct-ip fallback also fails
Both API requests and binary downloads retry against a hard-coded IP when the
attempt via hostname fails. Both then overwrote the first error with the
fallback's, so only the last failure was reported.

That discarded the diagnosis. During the Firewall Mode incident the hostname
attempt was denied locally - WSAEACCES, "An attempt was made to access a socket
in a way forbidden by its access permissions", which means the host is blocking
ctrld - while the direct-ip fallback failed with an unreachable IPv6 route. What
surfaced to the operator was "dial tcp6: no route to host", pointing at a
routing problem that did not exist, while the WSAEACCES that named the real
cause was visible only in debug logs.

Report both failures instead, keeping the error chain intact so errors.Is still
matches either one. Also switch the final wrap in doWithRetry from %v to %w,
which had been flattening the chain even when a single error was reported.

Add coverage for both paths, including that the fallback is still attempted and
that a successful fallback returns no error.

This also changes retry classification, which is worth stating explicitly because
it is not obvious from "report both errors". processCDFlags decides whether to
keep backing off with errUrlNetworkError, which uses errors.As - and errors.As
returns the first match in the tree. Wrapping the hostname attempt first
therefore hands the predicate that attempt's failure, where previously only the
fallback's error survived to be classified.

The effect is intended. A locally denied socket (WSAEACCES) is not a transient
network error, so preflight now fails fast and reports instead of retrying
against a firewall that is not going to clear on its own - the incident logged
256 retry cycles doing exactly that. The case that justifies retrying forever, a
network unreachable on both attempts at boot, is unchanged. Both classifications
are pinned by tests, along with the wrap order they depend on at each composition
site, so reversing it fails loudly rather than silently restoring the old
behaviour.
2026-08-14 15:29:32 +07:00
Cuong Manh Le 891d5d9821 cmd/cli: drop stale intercept enforcement at startup, before API bootstrap
A host carrying orphaned WFP filters from a ctrld build that predates
session-scoped ownership cannot recover on its own. In Firewall Mode those
filters block all non-allowlisted outbound traffic machine-wide, which denies
the replacement ctrld's own API bootstrap. API-managed startup then sits in the
resolver-config retry loop forever and never reaches startWFPFilters, where the
only stale-sublayer cleanup lived. Cleanup needs startup, startup needs the
network, the network needs cleanup - the host stays locked out until a reboot.

Add cleanupStaleDNSInterceptState() and call it early in run(), before the
network-up wait and before the API preflight. On Windows it deletes ctrld's WFP
sublayer, which takes its child filters with it, so a previous process's
enforcement is gone before this process makes its first connection. Objects
owned by a live session cannot be deleted, so a running ctrld is unaffected and
"nothing to clean up" stays at debug level; an actual removal logs a warning,
since it means a previous ctrld left machine-wide enforcement installed.

macOS and the other platforms get no-op implementations: pf enforcement does not
outlive the process, and startDNSIntercept already flushes the anchor and
removes a stale anchor file before loading rules.

The cleanup inside startWFPFilters stays as a second line of defense.

The cleanup session is deliberately NOT dynamic. FwpmSubLayerDeleteByKey0 is
documented to fail with FWP_E_DYNAMIC_SESSION_IN_PROGRESS when called from a
dynamic session for an object that was not added in one, and the only orphans
that can exist are exactly those: a ctrld predating session-scoped ownership
added its sublayer statically. Anything a newer ctrld leaves behind is removed by
the OS when its session ends. Opening this session dynamically would have made
the cleanup a no-op in the one case it exists for, while still logging "no stale
WFP state".

A concurrently running ctrld is protected by documented ownership rather than by
the child-filter question below: a session-scoped ctrld's sublayer belongs to a
different dynamic session, so the delete fails with FWP_E_WRONG_SESSION. That
matters because this call is made unconditionally at startup, in every intercept
mode, so an interactive "ctrld run" alongside a healthy service reaches it.

A ctrld predating session scoping has no such protection: it holds a non-dynamic
sublayer, which is exactly what this targets and is indistinguishable from an
orphan. Two guards cover that instead. Elevation, because opening a WFP engine
and deleting ctrld's sublayer must not be reachable from an unprivileged local
process - FwpmEngineOpen0 is expected to fail without elevation, but that is a
property of the API rather than something this code checked, and it was the only
barrier. And interactive invocation, since a service start is not interactive: the
deadlock case still gets cleaned, while a hand-run "ctrld run" beside a live
service does not strip its enforcement. That second guard requires positive
evidence of absence - ctrldServiceLiveness answers unknown for an unreachable SCM
or a service mid-stop, and unknown skips the cleanup exactly as running does,
because "could not be observed" is not "not there".

Both are asserted against the deletion itself, not only against the predicate: a
caller-level test substitutes the WFP delete and the guard inputs, so a future
change that stops consulting the guard, or consults it and deletes anyway, fails
rather than staying green.

Delete failures are no longer collapsed into "nothing to clean up". Only
FWP_E_SUBLAYER_NOT_FOUND means that; any other code means state exists under our
GUID that we could not remove, which is the lockout condition itself, so it is
logged as a warning naming the code.

Whether deleting the sublayer is sufficient is left explicitly UNRESOLVED rather
than asserted. It is sufficient only if the delete also removes the filters
inside it. FwpmSubLayerDeleteByKey0's Remarks say nothing about child filters
either way, while object management states that an object cannot be deleted until
everything referencing it has been - and FWP_E_IN_USE exists for that. Whether a
filter's subLayerKey counts as such a reference is not documented, and this code
cannot be exercised off-Windows, so the earlier claim that the delete "takes its
child filters with it" is removed from the comment here and from
docs/wfp-dns-intercept.md, which carried it from before this branch.

The behaviour is safe under both readings: the delete is attempted, and
FWP_E_IN_USE is reported rather than counted as success, so a support log
distinguishes "cleared it" from "could not clear it". If a live Windows check
shows FWP_E_IN_USE against orphaned filters, the cleanup must enumerate and
delete those filters first. That is deliberately not written blind:
FWPM_FILTER_ENUM_TEMPLATE0 has no sublayer field, so selecting ctrld's own
filters means reading subLayerKey at a computed offset in FWPM_FILTER0, and
getting that offset wrong would delete other software's filters - a worse failure
than not cleaning up.

Refs: https://learn.microsoft.com/en-us/windows/win32/api/fwpmu/nf-fwpmu-fwpmsublayerdeletebykey0
Refs: https://learn.microsoft.com/en-us/windows/win32/fwp/object-management
2026-08-14 15:29:06 +07:00
Cuong Manh Le 7de6298fa4 cmd/cli: scope ctrld's WFP objects to a dynamic session on Windows
ctrld opened the WFP engine with a plain FWPM_SESSION0, so every filter and
sublayer it installed was persistent for the engine's boot lifetime: the kernel
kept enforcing them after the installing process was gone. Any exit that did
not run the shutdown path - kill, crash, or a service stop during upgrade -
left them behind.

In hard intercept mode that orphaned the DNS block filters. With Firewall Mode
enabled it orphaned machine-wide block-all filters that carry no process or SID
condition, so the entire host lost outbound traffic: browsers, other users, and
a replacement ctrld's own API bootstrap alike, with no way back short of a
reboot.

Set FWPM_SESSION_FLAG_DYNAMIC on both engine sessions (hard intercept and
loopback protect). Windows then deletes everything the session owns when the
handle closes, including on abnormal termination, so ctrld's enforcement can no
longer outlive the process that installed it.

This removes the cause. The next commit adds startup self-heal for hosts
already carrying orphaned filters from a build that predates this change.
2026-08-14 15:28:16 +07:00
Cuong Manh Le 52b7aaab87 cmd/cli, internal/controld: bound API preflight by service lifetime
processCDFlags retries the resolver-config fetch indefinitely by design: a
device that has no working network at boot must eventually come up. The loop
had no cancellation, so a stop request arriving while the API is unreachable
was ignored - the process kept retrying long after "Service stopped" was
logged, doing work on behalf of a service the OS considers stopped. The
Windows Firewall Mode incident showed this concretely: API retries continued
15 seconds after the stop completed, so stopping the service could not release
what the process was still holding.

Thread a context through processCDFlags and derive it from p.stopCh, in both
the startup preflight and the config-reload path. The loop now returns as soon
as the context is cancelled, checked both before a retry and after backoff
returns (backoff can wake up on cancellation).

A stop during preflight now exits the way a normal stop does, without Fatal,
so the service manager does not treat it as a failed start and apply its
restart policy to a service the operator just asked to stop.

Also bind the two API requests in internal/controld to the caller's context.
They were built with http.NewRequest, so an in-flight request ignored
cancellation and waited out its own timeout instead.

Covered by tests that assert what the incident needed: a stop request during
preflight ends the retry/backoff loop and reports cancellation rather than
continuing to retry after the service reports itself stopped, an
already-cancelled context makes at most one attempt, and contextFromStopCh
handles its three cases (cancelled by stopCh, released by cancel, usable with
no stop channel). Removing either cancellation check makes these tests hang
until the test timeout. The tests are also what exercise fetchResolverConfig,
the seam this commit introduces.

Sampling the stop state is the whole point of runAPIPreflight rather than doing
this inline. A stop and a failure need opposite handling - one exits quietly, the
other self-uninstalls a deleted device, surfaces the error to a mobile app, and
reports a failed start - so the two must not be confused. Reading it from the
context after cancelling would report stopped for every failure, since
CancelFunc sets ctx.Err() regardless of whether anyone asked to stop; the stop
channel is read directly instead, which also does not depend on the context
watcher goroutine having been scheduled.
2026-08-14 15:27:54 +07:00
Cuong Manh Le e5f2506199 cmd/cli: fix linter 2026-08-12 14:47:04 +07:00
Cuong Manh Le 0c8281c879 fix: ensure log and cache flags are processed during reload
During reload operations, log and cache flags were not being processed,
which prevented runtime internal logs from working correctly. To fix this,
processLogAndCacheFlags was refactored to accept explicit viper and config
parameters instead of relying on global state, enabling it to be called
during reload with the new configuration. This ensures that log and cache
settings are properly applied when the service reloads its configuration.
2026-08-12 14:46:48 +07:00
Dev Scribe 24fc9085fa fix: retry macOS OS resolver with route-selected source 2026-08-12 14:41:47 +07:00
Dev Scribe e4e1c0abb9 windows: adopt GP-managed NRPT catch-all 2026-08-12 14:32:13 +07:00
Dev Scribe 58a5ea56af fix: validate pf state before stabilization 2026-08-12 14:05:18 +07:00
Cuong Manh Le 171dd0a7e6 fix(firewall): support VM/container egress under macOS Firewall Mode
macOS Firewall Mode blocked forwarded/NATed VM/container egress: a guest
resolves DNS through a path host ctrld does not observe, so the guest-resolved
public IP never enters <ctrld_allowed> and the blanket outbound block drops the
guest's TCP/443.

Make VM/container guests first-class Firewall Mode clients by forcing their DNS
through ctrld. The trusted source subnets are the UNION of:

  - Auto-detected VM/NAT networks (default, no config): interfaces that are up,
    carry an RFC1918 IPv4 address, and whose VM ownership can be proven either
    by a vendor-specific name (vnic/vboxnet/vmnet) or by being a bridge* whose
    member list contains a vendor VM interface (typically vmenet*). Each keeps
    its ingress interface, and its pf rules are scoped "on <iface>" so an
    unrelated interface on the same private range is never affected.
  - service.firewall_forwarded_sources (opt-in): explicit IPv4 CIDRs, matched on
    the source CIDR alone, for stacks whose ownership cannot be proven. Config
    adds only; an invalid or non-IPv4 entry is dropped with a warning.

Checking bridge membership is what makes the common case work without config.
Every vmnet.framework stack - UTM and other Virtualization.framework guests,
Docker Desktop, Multipass, Fusion 12.1+ NAT - puts the RFC1918 gateway address
on a bridge10x interface and attaches the vendor-named vmenet* interface as an
address-less member, so matching on interface name alone never sees them.
Membership is the ownership proof a bridge name lacks: macOS shares that
namespace with Thunderbolt/aggregated links (ctrld's own tunnel-change code
treats bridge0 as physical), and such a bridge has en* members, so it stays
untrusted however private its address. Members are read with ifconfig, and only
for a bridge that already carries an RFC1918 IPv4 address, so a host with no VM
running executes no subprocess.

Per source subnet, plaintext DNS (53) is force-routed through ctrld (route-to
lo0 -> existing rdr-on-lo0) so guest resolutions are policy-enforced and
populate the allowlist; guest egress to allowed IPs is then permitted by the
existing <ctrld_allowed> rule. DoT (853) is blocked so guests cannot swap in an
alternate resolver. DoH/443 is a documented limitation.

Rules are emitted strictly per address family. Sources are IPv4 (interception
targets ctrld's IPv4 listener), so only inet rules are generated: pf rejects an
entire anchor over a single "inet6 ... from 192.168.x.0/24" mismatch, which
would take DNS interception down with it. Guest IPv6 DoT is covered by the
blanket IPv6 block instead, since such a resolver never enters <ctrld_allowed>.

firewall_forwarded_sources is deliberately not validated with `cidr`. Entries
are checked where they are parsed and a bad one is dropped while the rest of the
set still applies. A hard validator would make one typo in an MDM-pushed subnet
fatal at startup - validateConfig exits the process - taking DNS service down
for the whole host over a line that only ever widened a firewall allowance. The
warning fires when the set of unusable entries changes rather than on every
parse, since config is re-read on every anchor build and every watchdog tick.

Reconcile the trust set at runtime, since VM interfaces appear and disappear
while ctrld runs and no existing path rebuilds an intact anchor for that
(ensurePFAnchorActive returns early, checkTunnelInterfaceChanges tracks only
tunnels, pfInterceptMonitor rebuilds only after a failed host probe). On a
change, rebuild the anchor and drop the pf states of the affected subnets
(targeted pfctl -k, not a global state flush), because rules govern only new
states: a stopped guest would otherwise keep using states created while it was
trusted, and a newly trusted one would keep bypassing interception until its
states expired. Reconciliation runs on interface appear/disappear, on network
changes, on the delayed post-change re-checks (a VM network often gets its
address after its interface appears), and on the pf watchdog tick, which bounds
guest start/stop convergence to one interval even with no network event.

Convergence is not latched on failure: the applied set advances, and states are
killed, only after pf has accepted the new anchor. reloadForwardedSourceAnchor
reports write/pfctl failures to the caller, which then keeps the previous set
recorded and logs a warning, so the next reconcile retries the same transition
instead of going quiet with the old anchor still installed. The whole
compare-reload-record sequence is serialized so a watchdog tick and a network
change cannot both rebuild or interleave snapshots.

Every anchor rebuild records the forwarded-source set it installed, and takes
that set as a parameter rather than re-detecting internally. Otherwise a rebuild
triggered by something else (tunnel change, watchdog restore, VPN DNS
exemptions, forced reload, startup) leaves the snapshot at the older set and the
next reconcile "discovers" the same change again: another rebuild, another round
of killed guest states, and a transition logged for something already in effect.
Passing the set in also means what pf loaded is exactly what gets recorded, so
an interface appearing mid-reconcile cannot leave the snapshot describing a set
that was never installed.

The anchor file is replaced atomically (temp file plus rename). Seven paths
rebuild it from timers and network-change callbacks in their own goroutines with
no lock between them, and os.WriteFile truncates before writing, so a pfctl -f
racing that window could read a partial ruleset and reject the anchor - taking
DNS interception down until the next watchdog restore.

Report the effective trust set at startup and on every change, naming each
subnet's origin ("192.168.64.0/24 (configured)" vs "(auto-detected on
bridge100)"), and say so explicitly when the set is empty, including what
auto-detection requires. Configured entries previously produced no log output at
all, so an admin who set firewall_forwarded_sources could not confirm it had
taken effect without reading pf rules. Per-source detection logging is at debug,
since detection re-runs on every anchor build.

Explicit, per-subnet trust boundary (RFC1918-only auto-detect, proven VM
ownership, interface-scoped rules, config adds only), not an interface-wide
permit: direct public IPs the guest never resolved through ctrld stay blocked.

Tests cover rule generation and its interface scoping, the no-blanket-permit
boundary, single-family emission (no inet6 rule for an IPv4 source, IPv6 sources
skipped), a real pfctl -n -f parse of both the forwarded-source rules alone and
the full anchor (group-scoped rules stripped, since _ctrld exists only where the
service is installed), invalid/duplicate/non-IPv4 config entries, that a bad
entry does not fail config validation, the union set and its signature, a
deterministic guest start/stop lifecycle asserting both the rebuild points and
which subnets' states must be dropped, anchor-reload failure followed by a
successful retry, that reconcile is inert outside firewall mode, ifconfig member
parsing against real bridge output, and the trust decision per interface -
including everything that must NOT qualify: a Thunderbolt bridge, a
public-range VM bridge, an IPv6-only bridge, an address-less vendor interface, a
physical uplink and a VPN tunnel. docs/firewall-mode.md documents the boundary,
what auto-detection can and cannot prove, the address-family constraint, the
lifecycle/retry behavior, how to confirm the trust set from the log, and that a
resolver running inside the guest is not supported (its encrypted upstream
leaves the host nothing to allowlist).
2026-08-04 14:12:15 +07:00
Cuong Manh Le cbef8b911c fix: partition DNS cache by EDNS Client Subnet
With cache_enable = true, one cache entry was shared by every client
asking the same name against the same upstream: the cache key
({Qtype, Qclass, Name, Upstream}) and the osResolver hot-cache/singleflight
key ("name:qtype:") both ignored the EDNS Client Subnet (ECS). A response
tailored for subnet A was therefore served to subnet B.

A cached answer's records are scoped to the network that generated them
(RFC 7871 §7.3), so sharing them across subnets returns the wrong
CDN/policy answer. Rewriting only the ECS option on the shared answer is
worse: forwarders that validate the echoed ECS (e.g. dnsmasq with
add-subnet) then accept the wrong-subnet answer instead of rejecting it as
a mismatch.

Partition both cache paths by a canonical ECS tuple (family, source-prefix,
masked address) via the new dnscache.CanonicalECS: the LRU key gains an ECS
field and the singleflight/hot-cache key appends the canonical ECS. Same
subnet still shares an entry; different subnets (or address families) never
do. Only a request with no ECS option collapses to the shared empty
partition; a carried /0 keeps its own family-scoped token, since it is
forwarded with an ECS option and must stay distinguishable from a no-ECS
query (RFC 7871 §7.3.1). SetCacheReply no longer touches ECS and only
reconciles the EDNS Cookie.

Adds real cache-path regression tests (LRU and osResolver hot cache) that
serve a different A record per subnet and verify the second subnet never
receives the first's record.

Fixes https://github.com/Control-D-Inc/ctrld/issues/324
2026-07-23 19:17:41 +07:00
Cuong Manh Le 06238e5b12 Bump golang.org/x/text to v0.40.0
For fixing GO-2026-5970.
2026-07-23 19:16:41 +07:00
Cuong Manh Le 8ffe8d4aaf fix: harden DoH oversized-body tests against server write timing
TestDoHResolve_{OversizedBody_Rejected,NonOKStatus_BoundedErrorBody,
OversizedBody_DoH3} asserted how many bytes the test server managed to
write before the client tore down the connection. That count reflects
kernel socket send buffers and HTTP/2 flow-control windows, which vary
by OS and load, so the server could buffer the whole body before
teardown and fail the assertion. It flaked on the Windows CI runner, but
reproduces on Linux too.

Replace the server-side byte counter with a deterministic synchronization
point. The handler writes exactly the read cap (dohMaxResponseSize+1 for
the body, dohMaxErrorBodySize for the error path), flushes, then blocks
without ever returning, so the response stream never gets an EOF. The
test then requires Resolve to return the size/status error before the
handler is released: ctrld's bounded read (io.LimitReader) returns after
the capped prefix, while a read to EOF would block on the withheld stream
and trip the deadline.

This removes the socket-buffer timing dependence and, unlike asserting on
the returned error alone, still fails if the caps are removed -- verified
by reverting both reads in doh.go to io.ReadAll(resp.Body), which makes
all three tests time out.
2026-07-17 20:13:02 +07:00
Cuong Manh Le 19b222f6a8 fix: count DoQ/UDP test server call before writing reply
countHandler incremented its call counter after w.WriteMsg, but the DNS
client returns as soon as it receives the reply. A test reading the
counter right after Resolve returned could therefore observe a stale
zero, e.g. Test_Edns0_CacheReply intermittently failing on CI with
"cache not hit, server was called: 0" while passing on retry.

Increment the counter before writing the reply so it is guaranteed
visible once the client has the response. Verified by widening the
post-write window to reproduce the failure deterministically, then
confirming the reordered handler passes 500x and under -race.
2026-07-16 20:15:36 +07:00
Dev Scribe c43739e42d fix: restore v1 intercept parity on master
Restore the previously merged macOS VPN DNS post-settle refresh,
unchanged-exemption suppression, forced pf state flush, and self-upgrade
test after they were removed by the firewall-mode merge.

Also keep Windows NRPT pointed at loopback when the listener is configured
on a wildcard address.
2026-07-15 23:50:19 +07:00
Cuong Manh Le 7f3d332b64 cmd/cli: discard upstream answers whose question mismatches the request
Defense in depth against cache poisoning: a compromised or misbehaving
upstream can return an answer for a different name than was asked (e.g.
records for attacker.example in response to a query for victim.example).
Such an answer would be cached under the legitimate request key and
served to subsequent queries.

Validate that the upstream answer echoes the request's question
(case-insensitive name plus Qtype/Qclass, per RFC 1035 section 4.1.2)
before serving or caching it. A mismatch is logged at debug level and
the upstream is skipped, failing safe to the next upstream or SERVFAIL.

Refs github.com/Control-D-Inc/ctrld/issues/322
2026-07-14 23:20:51 +07:00
Cuong Manh Le 9cf8bc3b5b fix: wrong DoQ resolver rewriting upstream responses with SetReply
The DoQ resolver called SetReply on the already-unpacked upstream
response. SetReply is meant to build a reply from a request, so it
forces the RCODE to NOERROR and overwrites the Question with the
request's question. This masked upstream failures from the proxy's
failover logic (a SERVFAIL looked like a successful empty response) and
corrupted the Question section of the response served to clients.

Restore only the downstream transaction ID instead (RFC 9250 section
4.2.1 puts the DNS Message ID at 0 on the wire), preserving the upstream
RCODE, Question, and answer sections untouched. This matches how the DoH
and DoT resolvers return unpacked upstream responses.

Refs github.com/Control-D-Inc/ctrld/issues/322
2026-07-14 23:20:39 +07:00
Dev Scribe 265573c744 fix: restore DNS during invalid-device uninstall 2026-07-14 23:20:27 +07:00
Cuong Manh Le 5ef0a59081 cmd/cli: preserve fallback listener port across reload in DNS intercept
On macOS DNS-intercept mode, when mDNSResponder owns *:53 ctrld falls back
to listening on 127.0.0.1:5354, and the pf rdr rules correctly redirect DNS
to the bound port at startup. However, DNS resolution later breaks with an
endless watchdog "anchor intact but probe FAILED -> force reload" loop and
`dig @127.0.0.1` timeouts.

Root cause is config reload. In CD mode, apiConfigReload refetches the
generated config (which always declares port 53) every hour and the reload
merge in runWait only inherits the running port when the new port is 0. The
generated config explicitly says 53, so `*p.cfg = *newCfg` reverts p.cfg to
port 53. The DNS listener goroutines are started only when !reload, so they
are never re-bound and stay on 5354. Every subsequent pf rebuild reads p.cfg
and targets the dead port 53.

Fix: after applying the reloaded config in DNS-intercept mode on darwin,
restore the actual bound listener IP/Port into the in-memory config via the
new preserveBoundListeners helper, logging the configured-vs-actual
divergence. A reload cannot move the running listener anyway, so this keeps
p.cfg consistent with reality; all pf rdr rules and the watchdog probe then
target the live port. The on-disk generated config is intentionally left
unchanged (still 53), so no generated-config change is required.
2026-07-13 21:50:02 +07:00
Dev Scribe 3226c2d0e2 Port NRPT recovery limits to master
Also adapts the logging fields to the master branch logger API.
2026-07-13 20:18:26 +07:00
Cuong Manh Le eb8756bbe5 fix: treat 464XLAT CLAT source as local, not WAN
On networks using 464XLAT (common on IPv6-only cellular carriers and iPhone
hotspots), the local machine's DNS queries can reach ctrld's listener with a
source address in the RFC 7335 IPv4 Service Continuity Prefix (192.0.0.0/29,
e.g. 192.0.0.2 on the CLAT/host side). isWanClient classified 192.0.0.x as a
WAN client, so with allow_wan_clients unset (the default) the query was refused,
breaking DNS resolution entirely on the affected connection even though it
originated from the local host.

Recognize the IPv4 Service Continuity Prefix as a local range, mirroring the
existing CGNAT special case:

- Add ipv4ServiceContinuityPrefix (192.0.0.0/29) and an isServiceContinuityAddr
  helper (single definition, reused by both call sites).
- isWanClient excludes the range, so 464XLAT/CLAT queries are served normally.
- isPrivatePtrLookup treats the range as private so reverse lookups are handled
  consistently.

Scoped to 192.0.0.0/29 (the exact 464XLAT range); this does not weaken
allow_wan_clients since no globally routable remote client can appear from it.
2026-07-13 20:15:33 +07:00
Cuong Manh Le ff14fc8ac9 fix: back off unroutable IPv6 DoH upstream health-check spam
When IPv6 is available locally but the selected IPv6 DoH endpoint is
unroutable (e.g. dialing [2606:1a40::22]:443 returns "no route to host"
while IPv4 stays usable), ctrld re-bootstrapped and re-dialed the endpoint
every ~2s. A weekend soak produced ~46.7k "no route to host" lines, with
the dial/health-check loop dominating the log during bad windows.

Add bounded backoff/suppression for network-unreachable endpoints at two
levels:

- ParallelDialer (internal/net): track dial addresses that fail with
  ENETUNREACH/EHOSTUNREACH and skip them for an exponentially growing,
  bounded window (5s -> 60s). A successful dial clears the entry
  immediately, so recovery is preserved when the route returns. When every
  candidate is suppressed the dial fails fast and quietly instead of
  hammering known-unroutable addresses.

- Upstream recovery loop (cmd/cli): demote unreachable check failures to
  debug and back off the retry cadence (2s -> 60s) for an unreachable
  streak; any other failure resets to the base cadence.

The new IsUnreachable classifier lives in internal/net and is reused by
cmd/cli's errNetworkError, so the unreachable-errno matching has a single
definition. Note the explicit winsock constants (10051/10065) are required
on Windows: syscall.ENETUNREACH/EHOSTUNREACH are Go's portable "invented"
values and never equal the raw WSA codes a failing connect surfaces.

Suppression and backoff are always bounded, so IPv6 is never disabled until
restart and recovers on its own once the route is back. Split-stack
selection and the #549 macOS intercept recovery work are untouched.

Adds unit tests for the classifier, the dialer's suppression tracker, and
the recovery backoff schedule.
2026-07-09 16:41:55 +07:00
Dev Scribe 8948fa402b fix: back off macOS pf watchdog exec storms 2026-07-09 16:25:47 +07:00
Codescribe 8330049b66 feat: add firewall mode DNS-resolved IP allowlist 2026-07-08 15:31:54 +07:00
Cuong Manh Le 9eb7067fbe Upgrade quic-go to v0.59.1
For fixing CVE-2026-40898.
2026-06-22 17:04:19 +07:00
Cuong Manh Le c1d3686f9a fix: stop self-upgrade tests from fork-bombing the windows test runner
The test:windows CI job intermittently failed to clean up .testbin with
"Access to the path '...cmd_cli.test.exe' is denied". This was previously
attributed to Windows Defender scanning the large unsigned test binaries,
and mitigated with Defender exclusions and cleanup retries. That was
treating a symptom.

Root cause: performUpgrade() self-upgrades by running
exec.Command(os.Executable(), "upgrade", "prod", "-vv") as a detached,
windowless child. In the real ctrld binary this re-execs ctrld and is
correct. Under `go test`, os.Executable() is the test binary itself, and
`go test` stops flag parsing at the first positional arg ("upgrade") and
ignores the rest -- so the child silently re-runs the entire test suite.
That child hits the upgrade tests again and spawns more detached children,
recursively: a fork bomb of hidden processes that pins the runner's
CPU/memory and keeps the test binary's image file locked. Windows refuses
to delete the image of a running process, hence the "Access is denied"
during after_script. Whether any children are still alive when cleanup
runs is a timing race, which is why the failure was flaky.

Two tests reached this path: Test_performUpgrade (directly) and
Test_selfUpgradeCheck (via selfUpgradeCheck -> performUpgrade on the
"upgrade allowed" case).

Fix:
- prog.go: extract the command construction into a package-level
  newUpgradeCmd var. Production behavior is unchanged.
- main_test.go: stub newUpgradeCmd once in TestMain so the whole test
  binary self-execs with `-test.run=^$` (matches no tests, exits
  immediately) instead of re-running the suite. This covers every test
  that reaches performUpgrade, present and future, while still exercising
  the cmd.Start() success path.
2026-06-22 17:04:08 +07:00
Dev Scribe 9399f4590b fix: refresh macOS VPN DNS after pf stabilization 2026-06-22 17:04:06 +07:00
Codescribe a4bc23d17e fix: allow intercept fallback for default listener 2026-06-22 17:00:57 +07:00
Codescribe 7a8450cc40 fix: flush pf states after forced DNS intercept reload 2026-06-22 17:00:47 +07:00
Cuong Manh Le 5ccbf63e58 test: isolate VPN DNS settling tests from host adapters 2026-06-22 17:00:38 +07:00
Codescribe 6c5489873b fix(windows): include domainless VPN DNS adapters in discovery 2026-05-29 13:38:52 +07:00
Cuong Manh Le f281466118 fix(doh,doq): port oversized response caps to master 2026-05-29 13:38:39 +07:00
Codescribe 3c740b9693 fix: port Windows VPN DNS settling fix to master 2026-05-29 13:38:27 +07:00
Cuong Manh Le 0d183feddb Bump golang.org/x/net to v0.55.0
For GO-2026-5026 security fix.
2026-05-29 13:36:54 +07:00
Cuong Manh Le e15ca5c466 docker: update Dockerfile to use bookworm
Stick to go1.25 for now, since using go1.26 causing a runtime panic when
building arm platforms.
2026-05-29 13:35:40 +07:00
Cuong Manh Le 98ca63325f fix(doq): share QUIC transport, close send side before read (RFC 9250)
DoQ pools now keep a single quic.Transport and UDP socket for all dials,
so parallel dial and reconnect churn no longer allocate a new socket per
attempt or leak the winner's UDP conn when the caller owns the packet
conn.

quicParallelDialer accepts an optional transport: when set, dials use
Transport.DialEarly on that socket; when nil, behavior matches the old
per-dial ListenUDP path (losers close their sockets).

Per RFC 9250 §4.2, close the query stream's send side before reading the
response so strict upstreams see STREAM FIN before answering.

CloseIdleConnections closes the shared transport and underlying UDP
conn so checked-out connections and the OS socket are torn down.

Add a FIN-strict test server, coverage for bootstrap vs parallel-dial
paths, and a Linux-only FD churn regression test.
2026-05-16 04:13:38 +07:00
Cuong Manh Le 7b360288ed doq: validate DNS-over-QUIC response framing
DoQ responses are length-prefixed per RFC 9250. The resolver previously
assumed the stream always contained at least two bytes and unpacked from
buf[2:], which could panic on truncated or malicious replies.

Validate the prefix against the bytes read, return a clear error, and
retire the connection from the pool on framing failure. Unpack only the
slice declared by the prefix so a short read cannot be misinterpreted as
a full message.

Add regression coverage with a small test server that returns malformed
raw payloads (empty, one byte, prefix-only, prefix larger than payload).
2026-05-16 04:13:11 +07:00
Cuong Manh Le 65d3d468f7 Bump golang.org/x/net to v0.53.0 2026-05-12 12:42:41 +07:00
Cuong Manh Le 01490434a6 cmd/cli: rate-limit PIN brute-force on control socket
Currently there is no limit on PIN attempts, allowing unlimited
brute force if an attacker gains socket access. While the socket is
root-only by default, rate limiting is cheap defense-in-depth.
2026-05-12 12:42:07 +07:00
Cuong Manh Le a61677b6e4 cmd/cli: use os.CreateTemp for symlink-safe temp file creation
Current code writes to a predictable path, which on systems without
`fs.protected_symlinks` (e.g. embedded routers) could allow a local
attacker with API compromise to perform symlink attacks.
2026-05-12 12:41:56 +07:00
Cuong Manh Le 8e2ef7ca65 all: explicit TLS MinVersion in tls.Config
Go's default is already TLS 1.2+ (since Go 1.18), but making this
explicit satisfies RFC 7858/9250 recommendations and makes the security
intent clear for auditors.
2026-05-12 12:41:47 +07:00
Codescribe 1735d3d55b cmd/cli: skip upstream.os healthcheck when WFP loopback protect enabled
When WFP loopback protect is active, the upstream.os healthcheck will
always fail because an external WFP block filter is interfering with
plain DNS. This demotes those expected failures to debug level and
returns errOsHealthcheckSuppressed so the recovery loop treats them
as non-fatal, eliminating the log spam described in #526.
2026-05-07 19:37:42 +07:00
Codescribe 81aa6b237b dns_intercept: add WFP loopback protect for VPN block-outside-dns
When third-party VPN software (e.g., OpenVPN) installs WFP block filters via
block-outside-dns, all DNS traffic to non-tunnel interfaces is blocked —
including DNS to 127.0.0.1 (ctrld's NRPT target). This breaks DNS mode
interception because the NRPT catch-all rule routes queries to loopback,
but WFP blocks the connection before it reaches ctrld's listener.

Fix: after exhausting all NRPT recovery attempts, activate a minimal WFP
session with "hard permit" filters (FWPM_FILTER_FLAG_CLEAR_ACTION_RIGHT)
for DNS to localhost in a max-priority sublayer (weight 0xFFFF). This
overrides the VPN's block for loopback DNS only, while preserving the
VPN's DNS leak protection for all other (non-loopback) DNS traffic.

The loopback protect is:
- Only activated when NRPT probes fail (not preemptively)
- Harmless when no conflicting WFP blocks exist (permit-only, no blocks)
- Persistent until ctrld shutdown (survives VPN reconnect cycles)
- Cleaned up by the existing cleanupWFPFilters path on shutdown
2026-04-30 19:30:43 +07:00
Codescribe 8abeeea4c3 log: persist internal runtime logs to disk
Add file-backed persistence to the internal logWriter so runtime logs
survive service restarts. When internal logging is enabled (CD mode,
no explicit log_path), writes are teed to both the existing in-memory
ring buffer and a rotated file on disk (ctrld.log in the home directory).

File rotation: 5MB max with 1 backup (ctrld.log.1), so max ~10MB on disk.
Log view/send now reads from the persisted files (including backup) to
provide complete history across restarts. Live tail continues to use
the in-memory subscriber mechanism unchanged.

Activation: same conditions as existing internal logging — CD mode only,
no log_path configured. No new config options or dependencies.
2026-04-30 19:19:19 +07:00
Codescribe b3c670b17e dns: fix recovery race condition during rapid network transitions
When multiple network changes fire in quick succession (e.g., VPN
disconnect + interface swap), the second handleRecovery() call cancels
the first but inherits stale DoH transports, causing DNS blackouts
of up to 30 seconds.

Three changes to reduce worst-case recovery from ~30s to <3s:

1. ForceReBootstrap() on recovery entry — closes dead connections and
   creates fresh transports synchronously before probing, replacing the
   lazy ReBootstrap() flag that left stale connections for probes to hit.

2. Debounce handleRecovery() for network changes (500ms window) — only
   the recovery flow is debounced; all other state updates (IP, pf
   anchor, VPN DNS, tunnel checks) still run immediately on every event.
   This eliminates the cancel-and-restart race without missing state.

3. Combined effect: ForceReBootstrap closes old in-flight connections
   (closeTransports) and builds new ones (SetupTransport) atomically,
   so recovery probes never inherit dead connections from a prior
   recovery attempt.
2026-04-30 19:19:19 +07:00
Cuong Manh Le 70b45710e7 docs: improve common gotchas and non-obvious patterns
README.md: fix Go version requirement (1.23 -> 1.24), update OS
support architectures (add arm64/mipsle/mips64 for Linux, arm64 for
Windows/FreeBSD, remove windows/arm), fix broken PowerShell install
path, demote H1 section headings to H2.
2026-04-30 19:19:19 +07:00