Compare commits

...
Author SHA1 Message Date
Ginder Singh e6441e29a3 added missing func. 2026-07-28 16:57:12 -04:00
Ginder Singh f6377209be Add mobile sandbox optimizations for v1.5.3
- Skip systemd-resolved initialization on Android (ChromeOS crash fix)
- Skip system DNS discovery commands on iOS mobile (sandbox restrictions)
- Skip route-based DNS discovery on Android
- Skip systemd resolver on Android
- Add mobile platform checks to prevent sandbox access violations

These changes ensure ctrld works correctly in mobile sandboxed environments
where system commands and file access are restricted.
2026-07-28 16:57:12 -04:00
Cuong Manh Le 8ebe911b1a Bump golang.org/x/text to v0.40.0
For fixing GO-2026-5970.
2026-07-28 16:15:14 +07:00
Cuong Manh Le d38538f593 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-28 16:14:15 +07:00
Cuong Manh Le 737fc79b58 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-28 16:13:46 +07:00
Cuong Manh Le fa074f1f5e 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-28 16:13:36 +07:00
Cuong Manh Le c596ef586b fix: skip internal logging in silent mode
Running with --silent still created and grew log files in cd mode.
needInternalLogging() only checked cdUID and Service.LogPath, so a
--silent flag enabled internal logging, persisted it to disk, and
reset the global log level back to debug, overriding the NoLevel that
--silent had set.

Return false from needInternalLogging() when silent is set, so ctrld
neither creates the internal log file nor writes debug logs. Add
regression tests asserting needInternalLogging() is false in silent mode
and that initInternalLogging() creates no log file.

Refs https://github.com/Control-D-Inc/ctrld/issues/320
2026-07-28 16:13:19 +07:00
Cuong Manh Le a4cfd4e479 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-28 16:12:47 +07:00
Dev ScribeandCuong Manh Le b79098658a fix: restore DNS during invalid-device uninstall 2026-07-28 16:12:36 +07:00
Cuong Manh Le d29e7d131e 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-28 16:12:23 +07:00
Cuong Manh Le 836c9ccf12 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-28 16:11:51 +07:00
Cuong Manh Le 53d3d3d44a 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-28 16:11:37 +07:00
Cuong Manh LeandGitHub d7f43ea4bf Merge pull request #323 from Control-D-Inc/release-branch-v1.5.4
Release v1.5.4
2026-07-14 21:06:20 +07:00
Dev ScribeandCuong Manh Le 41ca69849a Add Windows NRPT recovery circuit breaker
Windows DNS intercept mode runs an NRPT health monitor that restores the
catch-all rule and re-signals DNS Client whenever Windows stops routing
queries to the local listener. When another agent (MDM, VPN, GPO) keeps
putting NRPT back into a broken state, that loop never converges: ctrld
repeatedly calls RefreshPolicyEx, Dnscache paramchange, and flushes the
DNS cache, producing continuous flash writes and SIEM noise while never
fixing anything.

Add a recovery limiter that trips after a configurable number of
consecutive recovery flows and enters a cooldown, during which recovery
is suppressed (logged at most once every 5 minutes). Clearing the
circuit requires two consecutive stable health successes rather than
one, because a probe can pass briefly right after delete/re-add even
when the underlying NRPT state is still broken.

New [service] options gate the behavior and default to the previous
unlimited behavior:
  - nrpt_recovery_max_attempts (default 0 = unlimited)
  - nrpt_recovery_cooldown     (default 30m)

Also collapse the repeated refresh + paramchange + flush sequence into a
single signalNRPTChange() helper, and make cleanGPPath /
cleanEmptyNRPTParent only mutate the registry and report whether cleanup
happened, so callers send exactly one DNS Client change signal instead
of several. When the GP DnsPolicyConfig parent exists but is empty,
nrptProbeAndHeal now cleans it and signals once before spending the
normal policy-refresh retry budget, since those retries cannot succeed
while DNS Client is stuck in GP mode.

Add unit tests for the limiter's cooldown, stable-reset, and unlimited
paths, and document the new options and the empty-GP repro.
2026-07-14 01:12:37 +07:00
Cuong Manh Le 0d8df38dc1 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-14 01:12:34 +07:00
Dev ScribeandCuong Manh Le 3ef17bc5b9 fix: back off macOS pf watchdog exec storms 2026-07-14 01:09:14 +07:00
Cuong Manh LeandGitHub 5bf26da585 Merge pull request #318 from Control-D-Inc/release-branch-v1.5.3
Release branch v1.5.3
2026-06-22 14:40:36 +07:00
Cuong Manh LeandCuong Manh Le a5d536ab79 Upgrade quic-go to v0.59.1
For fixing CVE-2026-40898.
2026-06-16 15:17:11 +07:00
CodescribeandCuong Manh Le 735590d244 fix: allow intercept fallback for default listener 2026-06-16 15:05:19 +07:00
CodescribeandCuong Manh Le 18f01baa01 fix: flush pf states after forced DNS intercept reload 2026-06-16 15:05:08 +07:00
Cuong Manh LeandCuong Manh Le 723c7827ba 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-16 14:52:45 +07:00
Dev ScribeandCuong Manh Le 1e1c998c89 Refresh macOS VPN DNS after pf stabilization 2026-06-16 14:52:28 +07:00
Cuong Manh LeandCuong Manh Le da454db8ef 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-06-16 14:49:33 +07:00
Cuong Manh LeandCuong Manh Le 3fe9b27fb4 fix(doh,doq): reject oversized upstream DNS responses
DoH, DoH3, and DoQ response paths previously used io.ReadAll on
attacker-controlled upstream responses before enforcing any protocol-level
size limit. A malicious or compromised upstream could return an oversized
body or stream and force ctrld to buffer unbounded data before eventually
failing DNS parsing.

Cap DoH/DoH3 response bodies at dns.MaxMsgSize and cap DoQ streams at the
2-byte length prefix plus dns.MaxMsgSize. Also limit non-200 DoH error
bodies so error formatting cannot consume large upstream responses.
2026-06-16 14:49:01 +07:00
Cuong Manh LeandCuong Manh Le 35455eb0b9 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-06-16 14:48:43 +07:00
Cuong Manh LeandCuong Manh Le f1309121ae 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-06-16 14:48:33 +07:00
Cuong Manh LeandCuong Manh Le 06668a2b6c 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-06-16 14:47:27 +07:00
Cuong Manh LeandCuong Manh Le 97e5e99b8d 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-06-16 14:47:19 +07:00
Cuong Manh LeandCuong Manh Le c54ff701bd internal/router/dnsmasq: use text/template instead of html/template
Since this is a plain-text config, not html.
2026-06-16 14:47:08 +07:00
Cuong Manh LeandCuong Manh Le 33682e2312 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-06-16 14:46:42 +07:00
51 changed files with 3108 additions and 218 deletions
+91 -37
View File
@@ -451,18 +451,7 @@ func run(appCallback *AppCallback, stopCh chan struct{}) {
p.onStopped = append(p.onStopped, func() {
// restore static DNS settings or DHCP
p.resetDNS(false, true)
// Iterate over all physical interfaces and restore static DNS if a saved static config exists.
withEachPhysicalInterfaces("", "restore static DNS", func(i *net.Interface) error {
file := savedStaticDnsSettingsFilePath(i)
if _, err := os.Stat(file); err == nil {
if err := restoreDNS(i); err != nil {
mainLog.Load().Error().Err(err).Msgf("Could not restore static DNS on interface %s", i.Name)
} else {
mainLog.Load().Debug().Msgf("Restored static DNS on interface %s successfully", i.Name)
}
}
return nil
})
restoreSavedStaticDNS("", false)
})
close(waitCh)
@@ -638,6 +627,19 @@ const defaultDeactivationPin = -1
// cdDeactivationPin is used in cd mode to decide whether stop and uninstall commands can be run.
var cdDeactivationPin atomic.Int64
// Brute-force protection for the deactivation PIN endpoint on the control socket.
// After deactivationMaxFailedAttempts consecutive wrong PINs, further attempts are
// rejected for deactivationLockoutSeconds. Counter resets on a correct PIN.
const (
deactivationMaxFailedAttempts = 5
deactivationLockoutSeconds = 60
)
var (
deactivationFailedAttempts atomic.Int64
deactivationLockedUntil atomic.Int64
)
func init() {
cdDeactivationPin.Store(defaultDeactivationPin)
}
@@ -813,7 +815,7 @@ func processLogAndCacheFlags(v *viper.Viper, cfg *ctrld.Config) {
}
func netInterface(ifaceName string) (*net.Interface, error) {
if ifaceName == "auto" {
if ifaceName == autoIface {
ifaceName = defaultIfaceName()
}
var iface *net.Interface
@@ -1099,23 +1101,7 @@ func uninstall(p *prog, s service.Service) {
}
// restore static DNS settings or DHCP
p.resetDNS(false, true)
// Iterate over all physical interfaces and restore DNS if a saved static config exists.
withEachPhysicalInterfaces(p.runningIface, "restore static DNS", func(i *net.Interface) error {
file := savedStaticDnsSettingsFilePath(i)
if _, err := os.Stat(file); err == nil {
if err := restoreDNS(i); err != nil {
mainLog.Load().Error().Err(err).Msgf("Could not restore static DNS on interface %s", i.Name)
} else {
mainLog.Load().Debug().Msgf("Restored static DNS on interface %s successfully", i.Name)
err = os.Remove(file)
if err != nil {
mainLog.Load().Debug().Err(err).Msgf("Could not remove saved static DNS file for interface %s", i.Name)
}
}
}
return nil
})
restoreSavedStaticDNS(p.runningIface, true)
if router.Name() != "" {
mainLog.Load().Debug().Msg("Router cleanup")
@@ -1128,6 +1114,26 @@ func uninstall(p *prog, s service.Service) {
}
}
// restoreSavedStaticDNS restores DNS from saved static config files on physical interfaces.
func restoreSavedStaticDNS(excludeIfaceName string, removeSaved bool) {
withEachPhysicalInterfaces(excludeIfaceName, "restore static DNS", func(i *net.Interface) error {
file := savedStaticDnsSettingsFilePath(i)
if _, err := os.Stat(file); err == nil {
if err := restoreDNS(i); err != nil {
mainLog.Load().Error().Err(err).Msgf("Could not restore static DNS on interface %s", i.Name)
} else {
mainLog.Load().Debug().Msgf("Restored static DNS on interface %s successfully", i.Name)
if removeSaved {
if err := os.Remove(file); err != nil {
mainLog.Load().Debug().Err(err).Msgf("Could not remove saved static DNS file for interface %s", i.Name)
}
}
}
}
return nil
})
}
func validateConfig(cfg *ctrld.Config) error {
if err := ctrld.ValidateConfig(validator.New(), cfg); err != nil {
var ve validator.ValidationErrors
@@ -1245,7 +1251,7 @@ func tryUpdateListenerConfigIntercept(cfg *ctrld.Config, notifyFunc func(), fata
return false, true
}
hasExplicitConfig := lc.IP != "" && lc.IP != "0.0.0.0" && lc.Port != 0
hasExplicitConfig := isExplicitInterceptListener(lc.IP, lc.Port)
if !hasExplicitConfig {
// Set defaults for intercept mode
if lc.IP == "" || lc.IP == "0.0.0.0" {
@@ -1303,6 +1309,16 @@ func tryUpdateListenerConfigIntercept(cfg *ctrld.Config, notifyFunc func(), fata
return updated, false
}
func isExplicitInterceptListener(ip string, port int) bool {
if ip == "" || ip == "0.0.0.0" || port == 0 {
return false
}
// 127.0.0.1:53 is the default macOS DNS-intercept listener. It can appear
// in generated/custom Control D configs, but it should still be allowed to
// fall back to 127.0.0.1:5354 when mDNSResponder already owns port 53.
return !(ip == "127.0.0.1" && port == 53)
}
// tryUpdateListenerConfig tries updating listener config with a working one.
// If fatal is true, and there's listen address conflicted, the function do
// fatal error.
@@ -1809,6 +1825,9 @@ var errInvalidDeactivationPin = errors.New("deactivation pin is invalid")
// errRequiredDeactivationPin indicates that the deactivation pin is required but not provided by users.
var errRequiredDeactivationPin = errors.New("deactivation pin is required to stop or uninstall the service")
// errTooManyDeactivationPin represents an error indicating excessive deactivation PIN request attempts.
var errTooManyDeactivationPin = errors.New("too many request attempts")
// checkDeactivationPin validates if the deactivation pin matches one in ControlD config.
func checkDeactivationPin(s service.Service, stopCh chan struct{}) error {
mainLog.Load().Debug().Msg("Checking deactivation pin")
@@ -1837,6 +1856,9 @@ func checkDeactivationPin(s service.Service, stopCh chan struct{}) error {
case http.StatusBadRequest:
mainLog.Load().Error().Msg(errRequiredDeactivationPin.Error())
return errRequiredDeactivationPin // pin is required
case http.StatusTooManyRequests:
mainLog.Load().Error().Msg(errTooManyDeactivationPin.Error())
return errTooManyDeactivationPin
case http.StatusOK:
return nil // valid pin
case http.StatusNotFound:
@@ -1849,7 +1871,9 @@ func checkDeactivationPin(s service.Service, stopCh chan struct{}) error {
// isCheckDeactivationPinErr reports whether there is an error during check deactivation pin process.
func isCheckDeactivationPinErr(err error) bool {
return errors.Is(err, errInvalidDeactivationPin) || errors.Is(err, errRequiredDeactivationPin)
return errors.Is(err, errInvalidDeactivationPin) ||
errors.Is(err, errRequiredDeactivationPin) ||
errors.Is(err, errTooManyDeactivationPin)
}
// ensureUninstall ensures that s.Uninstall will remove ctrld service from system completely.
@@ -2002,17 +2026,25 @@ func doValidateCdRemoteConfig(cdUID string, fatal bool) error {
} else {
if errors.As(cfgErr, &viper.ConfigParseError{}) {
if configStr, _ := base64.StdEncoding.DecodeString(rc.Ctrld.CustomConfig); len(configStr) > 0 {
tmpDir := os.TempDir()
tmpConfFile := filepath.Join(tmpDir, "ctrld.toml")
errorLogged := false
// Write remote config to a temporary file to get details error.
if we := os.WriteFile(tmpConfFile, configStr, 0600); we == nil {
// Write remote config to a uniquely named temporary file to get detailed error.
if tmpFile, tmpErr := os.CreateTemp("", "ctrld-*.toml"); tmpErr == nil {
tmpConfFile := tmpFile.Name()
if _, err := tmpFile.Write(configStr); err != nil {
mainLog.Load().Error().Err(err).Msg("failed to write temporary config file")
}
if err := tmpFile.Close(); err != nil {
mainLog.Load().Error().Err(err).Msg("failed to save temporary config file")
}
if de := decoderErrorFromTomlFile(tmpConfFile); de != nil {
row, col := de.Position()
mainLog.Load().Error().Msgf("failed to parse custom config at line: %d, column: %d, error: %s", row, col, de.Error())
errorLogged = true
}
_ = os.Remove(tmpConfFile)
if err := os.Remove(tmpConfFile); err != nil {
mainLog.Load().Error().Err(err).Msg("failed to remove temporary config file")
}
}
// If we could not log details error, emit what we have already got.
if !errorLogged {
@@ -2030,6 +2062,23 @@ func doValidateCdRemoteConfig(cdUID string, fatal bool) error {
return nil
}
// ensureRunningIfaceForInvalidUninstall populates p.runningIface before the
// invalid-device self-uninstall resets DNS. This path can run early during
// service startup (e.g. right after a reboot) before the running interface is
// otherwise known. resetDNS, via resetDNSForRunningIface, silently skips DNS
// restoration when p.runningIface is empty, which would leave the OS pointed at
// ctrld's local listener after the service is removed. See issue-556.
func ensureRunningIfaceForInvalidUninstall(p *prog, s service.Service) {
if iface == "" {
iface = autoIface
}
p.preRun()
if ir := runningIface(s); ir != nil {
p.runningIface = ir.Name
p.requiredMultiNICsConfig = ir.All
}
}
// uninstallInvalidCdUID performs self-uninstallation because the ControlD device does not exist.
func uninstallInvalidCdUID(p *prog, logger zerolog.Logger, doStop bool) bool {
s, err := newService(p, svcConfig)
@@ -2037,8 +2086,13 @@ func uninstallInvalidCdUID(p *prog, logger zerolog.Logger, doStop bool) bool {
logger.Warn().Err(err).Msg("failed to create new service")
return false
}
ensureRunningIfaceForInvalidUninstall(p, s)
// restore static DNS settings or DHCP
p.resetDNS(false, true)
// The invalid-device path may run early during service startup before runningIface
// is known. Restore every saved static DNS file so uninstalling does not leave the
// OS pointed at ctrld's local listener after the service is removed.
restoreSavedStaticDNS("", true)
tasks := []task{{s.Uninstall, true, "Uninstall"}}
if doTasks(tasks) {
+116
View File
@@ -0,0 +1,116 @@
package cli
import (
"testing"
"github.com/Control-D-Inc/ctrld"
)
func TestIsExplicitInterceptListener(t *testing.T) {
tests := []struct {
name string
ip string
port int
want bool
}{
{name: "empty", ip: "", port: 0, want: false},
{name: "wildcard", ip: "0.0.0.0", port: 53, want: false},
{name: "zero port", ip: "127.0.0.1", port: 0, want: false},
{name: "default intercept listener", ip: "127.0.0.1", port: 53, want: false},
{name: "fallback port explicit", ip: "127.0.0.1", port: 5354, want: true},
{name: "custom loopback explicit", ip: "127.0.0.2", port: 53, want: true},
{name: "custom address explicit", ip: "192.0.2.10", port: 53, want: true},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := isExplicitInterceptListener(tt.ip, tt.port); got != tt.want {
t.Fatalf("isExplicitInterceptListener(%q, %d) = %v, want %v", tt.ip, tt.port, got, tt.want)
}
})
}
}
// TestPreserveBoundListeners is a regression test for #551: on reload, the on-disk
// generated config still declares 127.0.0.1:53, but the running listener has fallen back
// to 127.0.0.1:5354. preserveBoundListeners must keep the in-memory config on the actual
// bound port so pf rdr rules and probes do not target the dead default port.
func TestPreserveBoundListeners(t *testing.T) {
// cur = actual running listener (fell back to 5354); newCfg = freshly read from disk (53).
cur := map[string]*ctrld.ListenerConfig{"0": {IP: "127.0.0.1", Port: 5354}}
newListeners := map[string]*ctrld.ListenerConfig{"0": {IP: "127.0.0.1", Port: 53}}
preserveBoundListeners(newListeners, cur)
if got := newListeners["0"].Port; got != 5354 {
t.Errorf("listener port after reload = %d, want 5354 (actual bound port)", got)
}
if got := newListeners["0"].IP; got != "127.0.0.1" {
t.Errorf("listener IP after reload = %q, want 127.0.0.1", got)
}
}
// TestPreserveBoundListeners_NoChange verifies that when the on-disk config matches the
// running listener, the config is left untouched (a legitimate reload with the same port).
func TestPreserveBoundListeners_NoChange(t *testing.T) {
cur := map[string]*ctrld.ListenerConfig{"0": {IP: "127.0.0.1", Port: 5354}}
newListeners := map[string]*ctrld.ListenerConfig{"0": {IP: "127.0.0.1", Port: 5354}}
preserveBoundListeners(newListeners, cur)
if got := newListeners["0"].Port; got != 5354 {
t.Errorf("listener port = %d, want 5354", got)
}
}
// TestPreserveBoundListeners_MissingCurrent verifies that a listener present on disk but not
// in the current running set (e.g. newly added) is left as configured.
func TestPreserveBoundListeners_MissingCurrent(t *testing.T) {
cur := map[string]*ctrld.ListenerConfig{"0": {IP: "127.0.0.1", Port: 5354}}
newListeners := map[string]*ctrld.ListenerConfig{
"0": {IP: "127.0.0.1", Port: 53},
"1": {IP: "127.0.0.1", Port: 5355},
}
preserveBoundListeners(newListeners, cur)
if got := newListeners["0"].Port; got != 5354 {
t.Errorf("listener 0 port = %d, want 5354 (preserved)", got)
}
if got := newListeners["1"].Port; got != 5355 {
t.Errorf("listener 1 port = %d, want 5355 (unchanged, no current binding)", got)
}
}
// TestPreserveBoundListeners_ExplicitChangeNotMasked verifies that an explicit, non-default
// listener in the reloaded config is applied rather than reverted to the old bound listener.
// Reverting an explicit change would make the control-server reload comparison return 200
// instead of 201, silently dropping the new listener. Regression guard for #551 review.
func TestPreserveBoundListeners_ExplicitChangeNotMasked(t *testing.T) {
// Running listener fell back to 5354; user reloads with an explicit new listener.
cur := map[string]*ctrld.ListenerConfig{"0": {IP: "127.0.0.1", Port: 5354}}
newListeners := map[string]*ctrld.ListenerConfig{"0": {IP: "127.0.0.2", Port: 5399}}
preserveBoundListeners(newListeners, cur)
if got := newListeners["0"].IP; got != "127.0.0.2" {
t.Errorf("explicit listener IP = %q, want 127.0.0.2 (not reverted)", got)
}
if got := newListeners["0"].Port; got != 5399 {
t.Errorf("explicit listener port = %d, want 5399 (not reverted)", got)
}
}
// TestPreserveBoundListeners_ExplicitDefaultPreserved verifies that the default
// 127.0.0.1:53 listener remains fallback-eligible: when it diverges from the running
// fallback port it is still preserved (isExplicitInterceptListener treats :53 as non-explicit).
func TestPreserveBoundListeners_ExplicitDefaultPreserved(t *testing.T) {
cur := map[string]*ctrld.ListenerConfig{"0": {IP: "127.0.0.1", Port: 5354}}
newListeners := map[string]*ctrld.ListenerConfig{"0": {IP: "127.0.0.1", Port: 53}}
preserveBoundListeners(newListeners, cur)
if got := newListeners["0"].Port; got != 5354 {
t.Errorf("default listener port = %d, want 5354 (preserved fallback)", got)
}
}
+8 -8
View File
@@ -399,7 +399,7 @@ NOTE: running "ctrld start" without any arguments will start already installed c
reportSetDnsOk := func(sockDir string) {
if cc := newSocketControlClient(ctx, s, sockDir); cc != nil {
if resp, _ := cc.post(ifacePath, nil); resp != nil && resp.StatusCode == http.StatusOK {
if iface == "auto" {
if iface == autoIface {
iface = defaultIfaceName()
}
res := &ifaceResponse{}
@@ -748,7 +748,7 @@ NOTE: running "ctrld start" without any arguments will start already installed c
startCmd.Run(cmd, args)
},
}
startCmdAlias.Flags().StringVarP(&ifaceStartStop, "iface", "", "auto", `Update DNS setting for iface, "auto" means the default interface gateway`)
startCmdAlias.Flags().StringVarP(&ifaceStartStop, "iface", "", autoIface, `Update DNS setting for iface, "auto" means the default interface gateway`)
startCmdAlias.Flags().AddFlagSet(startCmd.Flags())
rootCmd.AddCommand(startCmdAlias)
@@ -833,7 +833,7 @@ func initStopCmd() *cobra.Command {
stopCmd.Run(cmd, args)
},
}
stopCmdAlias.Flags().StringVarP(&ifaceStartStop, "iface", "", "auto", `Reset DNS setting for iface, "auto" means the default interface gateway`)
stopCmdAlias.Flags().StringVarP(&ifaceStartStop, "iface", "", autoIface, `Reset DNS setting for iface, "auto" means the default interface gateway`)
stopCmdAlias.Flags().AddFlagSet(stopCmd.Flags())
rootCmd.AddCommand(stopCmdAlias)
@@ -865,7 +865,7 @@ func initRestartCmd() *cobra.Command {
return
}
if iface == "" {
iface = "auto"
iface = autoIface
}
p.preRun()
if ir := runningIface(s); ir != nil {
@@ -1111,7 +1111,7 @@ NOTE: Uninstalling will set DNS to values provided by DHCP.`,
return
}
if iface == "" {
iface = "auto"
iface = autoIface
}
p.preRun()
if ir := runningIface(s); ir != nil {
@@ -1207,7 +1207,7 @@ NOTE: Uninstalling will set DNS to values provided by DHCP.`,
uninstallCmd.Run(cmd, args)
},
}
uninstallCmdAlias.Flags().StringVarP(&ifaceStartStop, "iface", "", "auto", `Reset DNS setting for iface, "auto" means the default interface gateway`)
uninstallCmdAlias.Flags().StringVarP(&ifaceStartStop, "iface", "", autoIface, `Reset DNS setting for iface, "auto" means the default interface gateway`)
uninstallCmdAlias.Flags().AddFlagSet(uninstallCmd.Flags())
rootCmd.AddCommand(uninstallCmdAlias)
@@ -1400,7 +1400,7 @@ func initUpgradeCmd() *cobra.Command {
return
}
if iface == "" {
iface = "auto"
iface = autoIface
}
p.preRun()
if ir := runningIface(s); ir != nil {
@@ -1595,7 +1595,7 @@ func onlyInterceptFlags(args []string) bool {
} else {
return false
}
case arg == "--iface=auto" || arg == "--iface" || arg == "auto":
case arg == "--iface="+autoIface || arg == "--iface" || arg == autoIface:
// Auto-added by startCmdAlias or its value; safe to ignore.
continue
default:
+21 -3
View File
@@ -59,12 +59,18 @@ func newControlServer(addr string) (*controlServer, error) {
func (s *controlServer) start() error {
_ = os.Remove(s.addr)
unixListener, err := net.Listen("unix", s.addr)
if l, ok := unixListener.(*net.UnixListener); ok {
l.SetUnlinkOnClose(true)
}
if err != nil {
return err
}
// Restrict socket permissions to owner-only (0600) so that only the
// process owner (typically root) can connect. Defense-in-depth since
// the control server endpoints carry no authentication of their own.
if err := os.Chmod(s.addr, 0600); err != nil {
return err
}
if l, ok := unixListener.(*net.UnixListener); ok {
l.SetUnlinkOnClose(true)
}
go s.server.Serve(unixListener)
return nil
}
@@ -219,6 +225,12 @@ func (p *prog) registerControlServerHandler() {
return
}
// Reject further attempts while locked out due to repeated wrong PINs.
if now := time.Now().Unix(); now < deactivationLockedUntil.Load() {
w.WriteHeader(http.StatusTooManyRequests)
return
}
// Re-fetch pin code from API.
rcReq := &controld.ResolverConfigRequest{
RawUID: cdUID,
@@ -252,6 +264,7 @@ func (p *prog) registerControlServerHandler() {
switch req.Pin {
case cdDeactivationPin.Load():
code = http.StatusOK
deactivationFailedAttempts.Store(0)
select {
case p.pinCodeValidCh <- struct{}{}:
default:
@@ -259,6 +272,11 @@ func (p *prog) registerControlServerHandler() {
case defaultDeactivationPin:
// If the pin code was set, but users do not provide --pin, return proper code to client.
code = http.StatusBadRequest
default:
if deactivationFailedAttempts.Add(1) >= deactivationMaxFailedAttempts {
deactivationLockedUntil.Store(time.Now().Unix() + deactivationLockoutSeconds)
deactivationFailedAttempts.Store(0)
}
}
w.WriteHeader(code)
}))
+105 -3
View File
@@ -41,6 +41,11 @@ const (
// 2s re-check misses.
pfAnchorRecheckDelayLong = 4 * time.Second
// pfExecFailureBackoff suppresses repeated pfctl/scutil checks briefly after
// macOS reports local resource exhaustion. Without this, network-change storms
// can turn a pf ruleset race into a fork/file-descriptor exhaustion loop.
pfExecFailureBackoff = 5 * time.Second
// pfVPNInterfacePrefixes lists interface name prefixes that indicate VPN/tunnel
// interfaces on macOS. Used to add interface-specific DNS intercept rules so that
// VPN software with "pass out quick on <iface>" rules cannot bypass our intercept.
@@ -1207,7 +1212,6 @@ func stringSlicesEqual(a, b []string) bool {
return true
}
// pfStartStabilization enters stabilization mode, suppressing all pf restores
// until the VPN's ruleset stops changing. This prevents a death spiral where
// ctrld and the VPN repeatedly overwrite each other's pf rules.
@@ -1284,6 +1288,10 @@ func (p *prog) pfStabilizationLoop(ctx context.Context, stableRequired time.Dura
p.pfStabilizing.Store(false)
mainLog.Load().Info().Msgf("DNS intercept: pf stable for %s — restoring anchor rules", stableRequired)
p.ensurePFAnchorActive()
routes, domainlessServers, exemptions := p.refreshDNSAfterVPNSettle("pf_stabilized")
if routes == 0 && domainlessServers == 0 && exemptions == 0 {
p.scheduleDNSAfterVPNSettleRefresh("pf_stabilized_followup", pfAnchorRecheckDelayLong)
}
p.pfLastRestoreTime.Store(time.Now().UnixMilli())
return
}
@@ -1300,6 +1308,15 @@ func (p *prog) ensurePFAnchorActive() bool {
if p.dnsInterceptState == nil {
return false
}
if !p.pfEnsureRunning.CompareAndSwap(false, true) {
mainLog.Load().Debug().Msg("DNS intercept watchdog: check already running, skipping duplicate")
return false
}
defer p.pfEnsureRunning.Store(false)
if p.pfExecBackoffActive() {
return false
}
// While stabilizing (VPN connecting), suppress all restores.
// The stabilization loop will restore once pf settles.
@@ -1333,6 +1350,9 @@ func (p *prog) ensurePFAnchorActive() bool {
// Check 1: anchor references in the main ruleset.
natOut, err := exec.Command("pfctl", "-sn").CombinedOutput()
if err != nil {
if p.pfBackoffResourceExhaustion(err, natOut, "dump NAT rules") {
return false
}
mainLog.Load().Warn().Err(err).Msg("DNS intercept watchdog: could not dump NAT rules")
return false
}
@@ -1345,6 +1365,9 @@ func (p *prog) ensurePFAnchorActive() bool {
if !needsRestore {
filterOut, err := exec.Command("pfctl", "-sr").CombinedOutput()
if err != nil {
if p.pfBackoffResourceExhaustion(err, filterOut, "dump filter rules") {
return false
}
mainLog.Load().Warn().Err(err).Msg("DNS intercept watchdog: could not dump filter rules")
return false
}
@@ -1362,6 +1385,9 @@ func (p *prog) ensurePFAnchorActive() bool {
if !needsRestore {
anchorFilter, err := exec.Command("pfctl", "-a", pfAnchorName, "-sr").CombinedOutput()
if err != nil || len(strings.TrimSpace(string(anchorFilter))) == 0 {
if p.pfBackoffResourceExhaustion(err, anchorFilter, "dump anchor filter rules") {
return false
}
mainLog.Load().Warn().Msg("DNS intercept watchdog: anchor has no filter rules — content was flushed")
needsRestore = true
}
@@ -1369,6 +1395,9 @@ func (p *prog) ensurePFAnchorActive() bool {
if !needsRestore {
anchorNat, err := exec.Command("pfctl", "-a", pfAnchorName, "-sn").CombinedOutput()
if err != nil || len(strings.TrimSpace(string(anchorNat))) == 0 {
if p.pfBackoffResourceExhaustion(err, anchorNat, "dump anchor NAT rules") {
return false
}
mainLog.Load().Warn().Msg("DNS intercept watchdog: anchor has no rdr rules — translation was flushed (will cause packet loop on lo0)")
needsRestore = true
}
@@ -1402,6 +1431,7 @@ func (p *prog) ensurePFAnchorActive() bool {
// Restore: re-inject anchor references into the main ruleset.
mainLog.Load().Info().Msg("DNS intercept watchdog: restoring pf anchor references")
if err := p.ensurePFAnchorReference(); err != nil {
p.pfBackoffResourceExhaustion(err, nil, "restore anchor references")
mainLog.Load().Error().Err(err).Msg("DNS intercept watchdog: failed to restore anchor references")
return true
}
@@ -1418,6 +1448,7 @@ func (p *prog) ensurePFAnchorActive() bool {
if err := os.WriteFile(pfAnchorFile, []byte(rulesStr), 0644); err != nil {
mainLog.Load().Error().Err(err).Msg("DNS intercept watchdog: failed to write anchor file")
} else if out, err := exec.Command("pfctl", "-a", pfAnchorName, "-f", pfAnchorFile).CombinedOutput(); err != nil {
p.pfBackoffResourceExhaustion(err, out, "load rebuilt anchor")
mainLog.Load().Error().Err(err).Msgf("DNS intercept watchdog: failed to load rebuilt anchor (output: %s)", strings.TrimSpace(string(out)))
} else {
flushPFStates()
@@ -1446,6 +1477,58 @@ func (p *prog) ensurePFAnchorActive() bool {
return true
}
func (p *prog) scheduleDNSAfterVPNSettleRefresh(reason string, delay time.Duration) {
time.AfterFunc(delay, func() {
if p.dnsInterceptState == nil {
return
}
p.refreshDNSAfterVPNSettle(reason)
})
}
func (p *prog) pfExecBackoffActive() bool {
until := p.pfExecBackoffUntil.Load()
if until == 0 {
return false
}
remaining := time.Until(time.UnixMilli(until))
if remaining <= 0 {
p.pfExecBackoffUntil.CompareAndSwap(until, 0)
return false
}
mainLog.Load().Debug().Dur("remaining", remaining).Msg("DNS intercept watchdog: suppressed during pf exec backoff")
return true
}
func (p *prog) pfBackoffResourceExhaustion(err error, output []byte, operation string) bool {
if !isResourceExhaustion(err, output) {
return false
}
until := time.Now().Add(pfExecFailureBackoff)
p.pfExecBackoffUntil.Store(until.UnixMilli())
mainLog.Load().Warn().Err(err).Dur("backoff", pfExecFailureBackoff).Str("operation", operation).
Msg("DNS intercept watchdog: backing off after local exec resource exhaustion")
return true
}
func isResourceExhaustion(err error, output []byte) bool {
if err == nil && len(output) == 0 {
return false
}
var msg string
if err != nil {
msg = err.Error()
}
if len(output) > 0 {
msg += "\n" + string(output)
}
msg = strings.ToLower(msg)
return strings.Contains(msg, "resource temporarily unavailable") ||
strings.Contains(msg, "too many open files") ||
strings.Contains(msg, "too many processes") ||
strings.Contains(msg, "cannot allocate memory")
}
// pfWatchdog periodically checks that our pf anchor is still active.
// Other programs (e.g., Windscribe desktop app, macOS configd) can replace
// scheduleDelayedRechecks schedules delayed re-checks after a network change event.
@@ -1458,8 +1541,19 @@ func (p *prog) ensurePFAnchorActive() bool {
//
// Two delays (2s and 4s) cover both fast and slow VPN teardowns.
func (p *prog) scheduleDelayedRechecks() {
p.pfDelayedRecheckMu.Lock()
defer p.pfDelayedRecheckMu.Unlock()
for _, timer := range p.pfDelayedRecheckTimers {
if timer != nil {
timer.Stop()
}
}
p.pfDelayedRecheckTimers = p.pfDelayedRecheckTimers[:0]
for _, delay := range []time.Duration{pfAnchorRecheckDelay, pfAnchorRecheckDelayLong} {
time.AfterFunc(delay, func() {
delay := delay
timer := time.AfterFunc(delay, func() {
if p.dnsInterceptState == nil || p.pfStabilizing.Load() {
return
}
@@ -1473,6 +1567,7 @@ func (p *prog) scheduleDelayedRechecks() {
p.vpnDNS.Refresh(true)
}
})
p.pfDelayedRecheckTimers = append(p.pfDelayedRecheckTimers, timer)
}
}
@@ -1811,7 +1906,14 @@ func (p *prog) forceReloadPFMainRuleset() {
mainLog.Load().Error().Err(err).Msgf("DNS intercept: force reload — failed to load anchor (output: %s)", strings.TrimSpace(string(out)))
}
// Reset upstream transports — pf reload flushes state table, killing DoH connections.
// Flush stale rdr/reply states after the forced ruleset + anchor reload.
// Without this, macOS can keep using pre-reload state and try to send
// redirected DNS replies directly from loopback to tunnel client addresses
// (for example, 127.0.0.1:<listener> -> 100.64.0.0/10), which fails with
// "sendmsg: can't assign requested address".
flushPFStates()
// Reset upstream transports — pf reload/state flush kills existing DoH connections.
p.resetUpstreamTransports()
mainLog.Load().Info().Msg("DNS intercept: force reload — pf ruleset and anchor reloaded successfully")
+74
View File
@@ -3,6 +3,7 @@
package cli
import (
"errors"
"strings"
"testing"
@@ -122,6 +123,35 @@ func TestPFBuildAnchorRules_Ordering(t *testing.T) {
}
}
// TestPFBuildAnchorRules_FallbackPort verifies that when the listener falls back
// to an alternate local port (e.g. 5354 because mDNSResponder owns *:53), the pf
// rdr rules redirect DNS to the ACTUAL bound port, not the configured default 53.
// Regression test for #551: pf redirected to a dead port after listener fallback.
func TestPFBuildAnchorRules_FallbackPort(t *testing.T) {
// Configured/generated listener is 127.0.0.1:53, but the runtime bound port is 5354.
p := &prog{cfg: &ctrld.Config{Listener: map[string]*ctrld.ListenerConfig{"0": {IP: "127.0.0.1", Port: 5354}}}}
rules := p.buildPFAnchorRules(nil)
// rdr must redirect to the actual bound port 5354.
if !strings.Contains(rules, "rdr on lo0 inet proto udp from any to ! 127.0.0.1 port 53 -> 127.0.0.1 port 5354") {
t.Errorf("UDP rdr must redirect to bound port 5354, got:\n%s", rules)
}
if !strings.Contains(rules, "rdr on lo0 inet proto tcp from any to ! 127.0.0.1 port 53 -> 127.0.0.1 port 5354") {
t.Errorf("TCP rdr must redirect to bound port 5354, got:\n%s", rules)
}
// The rdr redirect target must NOT point at the dead default port 53.
// Match the exact port at line end so "port 5354" is not a false positive.
if strings.Contains(rules, "-> 127.0.0.1 port 53\n") {
t.Errorf("rdr must not redirect to dead port 53 after fallback, got:\n%s", rules)
}
// The inbound accept rule must also target the actual bound port.
if !strings.Contains(rules, "127.0.0.1 port 5354") {
t.Errorf("pass in rule must reference bound port 5354, got:\n%s", rules)
}
}
// TestPFAddressFamily tests the pfAddressFamily helper.
func TestPFAddressFamily(t *testing.T) {
tests := []struct {
@@ -141,3 +171,47 @@ func TestPFAddressFamily(t *testing.T) {
}
}
}
func TestIsResourceExhaustion(t *testing.T) {
tests := []struct {
name string
err error
output []byte
want bool
}{
{
name: "exec start failure",
err: errors.New("fork/exec /sbin/pfctl: resource temporarily unavailable"),
want: true,
},
{
name: "fd exhaustion from stderr output",
err: errors.New("exit status 1"),
output: []byte("pfctl: Pipe: Too many open files"),
want: true,
},
{
name: "process exhaustion from wrapped restore error",
err: errors.New("failed to dump running filter rules: exit status 1 (output: too many processes)"),
want: true,
},
{
name: "ordinary pf syntax failure",
err: errors.New("exit status 1"),
output: []byte("pfctl: syntax error"),
want: false,
},
{
name: "nil error and empty output",
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := isResourceExhaustion(tt.err, tt.output); got != tt.want {
t.Fatalf("isResourceExhaustion() = %v, want %v", got, tt.want)
}
})
}
}
+50
View File
@@ -0,0 +1,50 @@
package cli
import "github.com/Control-D-Inc/ctrld"
var initializeOsResolver = ctrld.InitializeOsResolver
func (p *prog) refreshDNSAfterVPNSettle(reason string) (routes, domainlessServers, exemptions int) {
mainLog.Load().Info().Msgf("DNS intercept: refreshing OS/VPN DNS route state after VPN settle (%s)", reason)
ns := initializeOsResolver(true)
mainLog.Load().Debug().Msgf("DNS intercept: post-settle OS resolver nameservers: %v", ns)
if p.vpnDNS == nil {
mainLog.Load().Debug().Msg("DNS intercept: post-settle VPN DNS route refresh skipped — manager unavailable")
return 0, 0, 0
}
beforeExemptions := p.vpnDNS.CurrentExemptions()
routes, domainlessServers, exemptions = p.vpnDNS.RefreshRoutesOnly()
afterExemptions := p.vpnDNS.CurrentExemptions()
if vpnDNSExemptionsEqual(beforeExemptions, afterExemptions) {
mainLog.Load().Info().Msgf("DNS intercept: post-settle VPN DNS route refresh completed — %d routes, %d domainless servers, %d exemptions (pf unchanged)",
routes, domainlessServers, exemptions)
return routes, domainlessServers, exemptions
}
if err := p.exemptVPNDNSServers(afterExemptions); err != nil {
mainLog.Load().Warn().Err(err).Msg("DNS intercept: post-settle VPN DNS exemption update failed")
} else {
mainLog.Load().Info().Msgf("DNS intercept: post-settle VPN DNS exemptions changed — updated pf/WFP with %d exemptions", len(afterExemptions))
}
return routes, domainlessServers, exemptions
}
func vpnDNSExemptionsEqual(a, b []vpnDNSExemption) bool {
if len(a) != len(b) {
return false
}
seen := make(map[vpnDNSExemption]int, len(a))
for _, ex := range a {
seen[ex]++
}
for _, ex := range b {
if seen[ex] == 0 {
return false
}
seen[ex]--
}
return true
}
+49
View File
@@ -0,0 +1,49 @@
package cli
import (
"context"
"testing"
"github.com/Control-D-Inc/ctrld"
)
func TestRefreshDNSAfterVPNSettleRefreshesOSResolverAndVPNRoutes(t *testing.T) {
oldInitialize := initializeOsResolver
defer func() { initializeOsResolver = oldInitialize }()
var initialized []bool
initializeOsResolver = func(force bool) []string {
initialized = append(initialized, force)
return []string{"10.102.26.10:53"}
}
var exemptionUpdates [][]vpnDNSExemption
p := &prog{}
p.vpnDNS = newVPNDNSManager(func(exemptions []vpnDNSExemption) error {
exemptionUpdates = append(exemptionUpdates, append([]vpnDNSExemption{}, exemptions...))
return nil
})
p.vpnDNS.discoverVPNDNS = func(context.Context) []ctrld.VPNDNSConfig {
return []ctrld.VPNDNSConfig{{
InterfaceName: "utun4",
Servers: []string{"10.102.26.10"},
Domains: []string{"bmwgroup.net"},
}}
}
routes, domainlessServers, exemptions := p.refreshDNSAfterVPNSettle("test")
if routes != 1 || domainlessServers != 0 || exemptions != 1 {
t.Fatalf("expected 1 route, 0 domainless servers, 1 exemption, got routes=%d domainless=%d exemptions=%d",
routes, domainlessServers, exemptions)
}
if len(initialized) != 1 || !initialized[0] {
t.Fatalf("expected forced OS resolver refresh once, got %v", initialized)
}
if got := p.vpnDNS.UpstreamForDomain("jira.cc.bmwgroup.net."); len(got) != 1 || got[0] != "10.102.26.10" {
t.Fatalf("expected refreshed VPN DNS route, got %v", got)
}
if len(exemptionUpdates) != 0 {
t.Fatalf("expected route-only refresh to avoid pf exemption updates, got %+v", exemptionUpdates)
}
}
+121 -66
View File
@@ -278,6 +278,9 @@ type wfpState struct {
loopbackProtectActive bool
// loopbackPermitIDs stores the filter IDs for the loopback protect permits.
loopbackPermitIDs []uint64
// nrptRecoveryLimiter prevents repeated Windows policy/Dnscache signaling
// when another agent keeps putting NRPT back into a broken state.
nrptRecoveryLimiter nrptRecoveryLimiter
}
// Lazy-loaded WFP DLL procedures.
@@ -343,9 +346,10 @@ const (
// - GP path: SOFTWARE\Policies\...\DnsPolicyConfig (Group Policy)
// - Local path: SYSTEM\CurrentControlSet\...\DnsPolicyConfig (service store)
//
// If ANY rules exist in the GP path (from IT policy, VPN, MDM, etc.), DNS Client
// enters "GP mode" and ignores ALL local-path rules entirely. Conversely, if the
// GP path is empty/absent, DNS Client reads from the local path only.
// If the GP path contains real rules (from IT policy, VPN, MDM, etc.), DNS
// Client enters "GP mode" and ignores ALL local-path rules entirely. An empty GP
// parent key is worse: it still puts DNS Client in GP mode, but contributes no
// usable rule, so our local catch-all is hidden until that empty parent is gone.
//
// Strategy (matching Tailscale's approach):
// - Always write to the local path (baseline for non-domain machines).
@@ -395,18 +399,20 @@ func otherGPRulesExist() bool {
return false
}
// cleanGPPath removes our CtrldCatchAll rule from the GP path and deletes
// the GP DnsPolicyConfig parent key if no other rules remain. Removing the
// empty GP key is critical: its mere existence forces DNS Client into "GP mode"
// where local-path rules are ignored.
func cleanGPPath() {
// cleanGPPath removes only ctrld's GP-path rule and deletes the GP parent when
// no rules remain. The return value tells callers whether the parent key was
// actually deleted, which means DNS Client should be signaled once.
//
// Do not leave an empty GP parent behind: Windows treats the parent key itself
// as the policy store boundary, so an empty key can still hide local-path rules.
func cleanGPPath() bool {
// Delete our specific rule.
registry.DeleteKey(registry.LOCAL_MACHINE, nrptBaseKey+`\`+nrptRuleName)
// If the GP parent key is now empty, delete it entirely to exit "GP mode".
k, err := registry.OpenKey(registry.LOCAL_MACHINE, nrptBaseKey, registry.ENUMERATE_SUB_KEYS)
if err != nil {
return // Key doesn't exist — clean state.
return false // Key doesn't exist — clean state.
}
names, err := k.ReadSubKeyNames(-1)
k.Close()
@@ -414,12 +420,14 @@ func cleanGPPath() {
if len(names) > 0 {
mainLog.Load().Debug().Strs("remaining", names).Msg("DNS intercept: GP path has other rules, leaving parent key")
}
return
return false
}
// Empty — delete it to exit "GP mode".
if err := registry.DeleteKey(registry.LOCAL_MACHINE, nrptBaseKey); err == nil {
mainLog.Load().Info().Msg("DNS intercept: deleted empty GP DnsPolicyConfig key (exits GP mode)")
return true
}
return false
}
// writeNRPTRule writes a single NRPT catch-all rule at the given registry keyPath.
@@ -542,10 +550,11 @@ func refreshNRPTPolicy() {
// Group Policy refresh so NRPT changes take effect immediately.
// Uses DnsFlushResolverCache from dnsapi.dll + RefreshPolicyEx from userenv.dll.
func flushDNSCache() {
// Step 1: Refresh GP so DNS Client loads the new NRPT rules from registry.
refreshNRPTPolicy()
flushDNSCacheOnly()
}
// Step 2: Flush the DNS cache so stale entries from pre-NRPT resolution are cleared.
func flushDNSCacheOnly() {
if err := dnsapiDLL.Load(); err == nil {
if err := procDnsFlushResolverCache.Find(); err == nil {
ret, _, _ := procDnsFlushResolverCache.Call()
@@ -555,7 +564,6 @@ func flushDNSCache() {
}
}
}
// Fallback: use ipconfig /flushdns.
if out, err := exec.Command("ipconfig", "/flushdns").CombinedOutput(); err != nil {
mainLog.Load().Debug().Msgf("DNS intercept: ipconfig /flushdns failed: %v: %s", err, string(out))
} else {
@@ -563,6 +571,12 @@ func flushDNSCache() {
}
}
func signalNRPTChange() {
refreshNRPTPolicy()
sendParamChange()
flushDNSCacheOnly()
}
// startDNSIntercept activates WFP-based DNS interception on Windows.
// It creates a WFP sublayer and adds filters that block all outbound DNS (port 53)
// traffic except to localhost (127.0.0.1/::1), ensuring all DNS queries must go
@@ -598,21 +612,19 @@ func (p *prog) startDNSIntercept() error {
mainLog.Load().Info().Msgf("DNS intercept: initializing (mode: %s)", interceptMode)
logNRPTParentKeyState("pre-write")
// Two-phase empty parent key recovery: if the GP DnsPolicyConfig key exists
// but is empty, DNS Client has cached a "no rules" state and won't accept
// new rules even after they're written. Delete the empty key and signal DNS
// Client to reset before writing our rule.
// Two-phase recovery handles its own 2s signaling burst internally.
cleanEmptyNRPTParent()
// Empty parent key recovery: if the GP DnsPolicyConfig key exists but is
// empty, DNS Client enters GP mode and hides local rules. Delete empty
// parents first, then send one change signal so DNS Client drops stale state.
if cleanEmptyNRPTParent() {
signalNRPTChange()
}
if err := addNRPTCatchAllRule(listenerIP); err != nil {
return fmt.Errorf("dns intercept: failed to add NRPT catch-all rule: %w", err)
}
logNRPTParentKeyState("post-write")
state.nrptActive = true
refreshNRPTPolicy()
sendParamChange()
flushDNSCache()
signalNRPTChange()
mainLog.Load().Info().Msgf("DNS intercept: NRPT catch-all rule active — all DNS queries directed to %s", listenerIP)
// Step 2: In hard mode, also set up WFP filters to block non-local DNS.
@@ -1555,7 +1567,7 @@ func (p *prog) scheduleDelayedRechecks() {
mainLog.Load().Error().Err(err).Msg("DNS intercept: failed to re-add NRPT catch-all rule")
state.nrptActive = false
} else {
flushDNSCache()
signalNRPTChange()
mainLog.Load().Info().Msg("DNS intercept: NRPT catch-all rule restored")
}
}
@@ -1593,14 +1605,22 @@ func (p *prog) nrptHealthMonitor(state *wfpState) {
// Step 1: Check registry key exists.
if !nrptCatchAllRuleExists() {
now := time.Now()
if ok, wait := state.nrptRecoveryLimiter.allow(now, p.cfg); !ok {
if state.nrptRecoveryLimiter.shouldLogSkip(now) {
mainLog.Load().Warn().Dur("remaining", wait).
Msg("DNS intercept: NRPT rule restore suppressed after repeated recovery flows")
}
continue
}
mainLog.Load().Warn().Msg("DNS intercept: NRPT health check — catch-all rule missing, restoring")
if err := addNRPTCatchAllRule(state.listenerIP); err != nil {
mainLog.Load().Error().Err(err).Msg("DNS intercept: failed to restore NRPT catch-all rule")
state.nrptActive = false
continue
}
refreshNRPTPolicy()
flushDNSCache()
signalNRPTChange()
state.nrptRecoveryLimiter.recordRecoveryFlow(time.Now(), p.cfg)
mainLog.Load().Info().Msg("DNS intercept: NRPT catch-all rule restored by health monitor")
// After restoring, verify it's actually working.
go p.nrptProbeAndHeal()
@@ -1612,6 +1632,8 @@ func (p *prog) nrptHealthMonitor(state *wfpState) {
if !p.probeNRPT() {
mainLog.Load().Warn().Msg("DNS intercept: NRPT health check — rule present but probe failed, running heal cycle")
go p.nrptProbeAndHeal()
} else {
state.nrptRecoveryLimiter.recordStableSuccess()
}
// Step 3: In hard mode, also verify WFP sublayer.
@@ -1710,43 +1732,39 @@ func sendParamChange() {
}
// cleanEmptyNRPTParent removes empty NRPT parent keys that block activation.
// An empty DnsPolicyConfig key (exists but no subkeys) causes DNS Client to
// cache "no rules" and ignore subsequently-added rules.
// Empty GP and local parents have different failure shapes:
// - empty GP parent: DNS Client is in GP mode and ignores local-path rules;
// - empty local parent: DNS Client can cache an empty local policy store.
//
// Also cleans the GP path entirely if it has no non-ctrld rules, since the GP
// path's existence forces DNS Client into "GP mode" where it ignores the local
// service store path.
// This helper only changes registry state. The caller sends the single
// RefreshPolicyEx/paramchange/flush signal after it knows cleanup occurred.
//
// Returns true if cleanup was performed (caller should add a delay).
// Returns true if cleanup was performed (caller should signal DNS Client).
func cleanEmptyNRPTParent() bool {
cleaned := false
// Always clean the GP path — its existence blocks local path activation.
cleanGPPath()
cleaned := cleanGPPath()
// Clean empty local/direct path parent key.
k, err := registry.OpenKey(registry.LOCAL_MACHINE, nrptDirectKey, registry.ENUMERATE_SUB_KEYS)
if err != nil {
return false
}
names, err := k.ReadSubKeyNames(-1)
k.Close()
if err != nil || len(names) > 0 {
return false
if !nrptParentKeyEmpty(nrptDirectKey) {
return cleaned
}
mainLog.Load().Warn().Msg("DNS intercept: found empty NRPT local parent key (blocks activation) — removing")
if err := registry.DeleteKey(registry.LOCAL_MACHINE, nrptDirectKey); err != nil {
mainLog.Load().Warn().Err(err).Msg("DNS intercept: failed to delete empty NRPT local parent key")
return cleaned
}
return true
}
func nrptParentKeyEmpty(keyPath string) bool {
k, err := registry.OpenKey(registry.LOCAL_MACHINE, keyPath, registry.ENUMERATE_SUB_KEYS)
if err != nil {
return false
}
cleaned = true
// Signal DNS Client to process the deletion and reset its internal cache.
mainLog.Load().Info().Msg("DNS intercept: empty NRPT parent key removed — signaling DNS Client")
sendParamChange()
flushDNSCache()
return cleaned
names, err := k.ReadSubKeyNames(-1)
k.Close()
return err == nil && len(names) == 0
}
// logNRPTParentKeyState logs the state of both NRPT registry paths for diagnostics.
@@ -1783,18 +1801,39 @@ func logNRPTParentKeyState(context string) {
// nrptProbeAndHeal runs the NRPT probe with retries and escalating remediation.
// Called asynchronously after startup and from the health monitor.
//
// Retry sequence (each attempt: GP refresh + paramchange + flush → sleep → probe):
// 1. Immediate probe
// 2. GP refresh + paramchange + flush → 1s → probe
// 3. GP refresh + paramchange + flush → 2s → probe
// 4. GP refresh + paramchange + flush → 4s → probe
// Retry sequence:
// 1. Immediate probe.
// 2. If the GP parent is empty, clean it immediately, signal once, then probe.
// This is intentionally before the normal retry loop: policy refresh and
// Dnscache paramchange cannot make local rules visible while GP mode is
// selected by an empty GP parent.
// 3. Otherwise, signal DNS Client with increasing backoff between probes.
func (p *prog) nrptProbeAndHeal() {
state, _ := p.dnsInterceptState.(*wfpState)
if state != nil {
now := time.Now()
if ok, wait := state.nrptRecoveryLimiter.allow(now, p.cfg); !ok {
if state.nrptRecoveryLimiter.shouldLogSkip(now) {
mainLog.Load().Warn().Dur("remaining", wait).
Msg("DNS intercept: NRPT recovery suppressed after repeated failed recovery flows")
}
return
}
}
if !nrptProbeRunning.CompareAndSwap(false, true) {
mainLog.Load().Debug().Msg("DNS intercept: NRPT probe already running, skipping")
return
}
defer nrptProbeRunning.Store(false)
remediated := false
defer func() {
if remediated && state != nil {
state.nrptRecoveryLimiter.recordRecoveryFlow(time.Now(), p.cfg)
}
}()
mainLog.Load().Info().Msg("DNS intercept: starting NRPT verification probe sequence")
// Log parent key state for diagnostics.
@@ -1805,17 +1844,37 @@ func (p *prog) nrptProbeAndHeal() {
mainLog.Load().Info().Msg("DNS intercept: NRPT verified working")
return
}
remediated = true
// Attempts 2-4: GP refresh + paramchange + flush with increasing backoff
// If the GP parent exists but is empty, do not burn retries on Windows
// signaling. Those retries create SIEM noise but cannot succeed because DNS
// Client is still reading the empty GP store instead of the populated local
// store. Delete the blocker, send one notification, then re-probe.
if nrptParentKeyEmpty(nrptBaseKey) {
mainLog.Load().Warn().Msg("DNS intercept: NRPT probe failed with empty GP parent — cleaning before retry signaling")
if cleanEmptyNRPTParent() {
signalNRPTChange()
time.Sleep(1 * time.Second)
logNRPTParentKeyState("empty-gp-after-clean")
if p.probeNRPT() {
mainLog.Load().Info().Msg("DNS intercept: NRPT verified working after empty GP parent cleanup")
return
}
}
if nrptParentKeyEmpty(nrptBaseKey) {
mainLog.Load().Warn().Msg("DNS intercept: empty GP NRPT parent still present after cleanup; skipping redundant policy refresh retries")
return
}
}
// Attempts 2-4: signal DNS Client with increasing backoff between probes.
delays := []time.Duration{1 * time.Second, 2 * time.Second, 4 * time.Second}
for i, delay := range delays {
attempt := i + 2
mainLog.Load().Info().Int("attempt", attempt).Dur("delay", delay).
Msg("DNS intercept: NRPT probe failed, retrying with GP refresh + paramchange")
Msg("DNS intercept: NRPT probe failed, retrying with policy refresh + paramchange")
logNRPTParentKeyState(fmt.Sprintf("probe-attempt-%d", attempt))
refreshNRPTPolicy()
sendParamChange()
flushDNSCache()
signalNRPTChange()
time.Sleep(delay)
if p.probeNRPT() {
mainLog.Load().Info().Int("attempt", attempt).
@@ -1829,7 +1888,7 @@ func (p *prog) nrptProbeAndHeal() {
// signal DNS Client to forget it, wait, then re-add and signal again.
mainLog.Load().Warn().Msg("DNS intercept: all probes failed — attempting two-phase NRPT recovery (delete → signal → re-add)")
listenerIP := "127.0.0.1"
if state, ok := p.dnsInterceptState.(*wfpState); ok {
if state != nil {
listenerIP = state.listenerIP
}
@@ -1837,9 +1896,7 @@ func (p *prog) nrptProbeAndHeal() {
_ = removeNRPTCatchAllRule()
// If parent key is now empty after removing our rule, delete it too.
cleanEmptyNRPTParent()
refreshNRPTPolicy()
sendParamChange()
flushDNSCache()
signalNRPTChange()
logNRPTParentKeyState("nuclear-after-delete")
// Wait for DNS Client to process the deletion.
@@ -1850,9 +1907,7 @@ func (p *prog) nrptProbeAndHeal() {
mainLog.Load().Error().Err(err).Msg("DNS intercept: failed to re-add NRPT after nuclear recovery")
return
}
refreshNRPTPolicy()
sendParamChange()
flushDNSCache()
signalNRPTChange()
logNRPTParentKeyState("nuclear-after-readd")
// Final probe after recovery.
+100 -5
View File
@@ -736,6 +736,17 @@ func (p *prog) proxy(ctx context.Context, req *proxyRequest) *proxyResponse {
}
continue
}
// Reject an answer whose question does not match the request before it
// can be served or cached. A mismatched question means the upstream
// answered a different name/type than asked; caching it would poison
// the shared cache with wrong-domain records for the requested name.
// See github.com/Control-D-Inc/ctrld/issues/322.
if !sameQuestion(req.msg, answer) {
ctrld.Log(ctx, mainLog.Load().Debug(),
"discarding answer from %s: question mismatch (asked %q, got %q)",
upstreams[n], questionString(req.msg), questionString(answer))
continue
}
// We are doing LAN/PTR lookup using private resolver, so always process next one.
// Except for the last, we want to send response instead of saying all upstream failed.
if answer.Rcode != dns.RcodeSuccess && isLanOrPtrQuery && n != len(upstreamConfigs)-1 {
@@ -899,6 +910,33 @@ func containRcode(rcodes []int, rcode int) bool {
return false
}
// sameQuestion reports whether the upstream answer echoes the request's
// question. A well-behaved resolver always copies the question section from
// the query (RFC 1035 section 4.1.2); names are compared case-insensitively
// because DNS names are case-insensitive. A mismatch means the upstream
// answered a different name/type than asked - malformed or malicious - and the
// answer must not be served or cached, or it would poison the shared cache with
// wrong-domain records. See github.com/Control-D-Inc/ctrld/issues/322.
func sameQuestion(req, answer *dns.Msg) bool {
if req == nil || answer == nil {
return false
}
if len(req.Question) == 0 || len(answer.Question) == 0 {
return false
}
rq, aq := req.Question[0], answer.Question[0]
return rq.Qtype == aq.Qtype && rq.Qclass == aq.Qclass && strings.EqualFold(rq.Name, aq.Name)
}
// questionString renders a message's first question as "name/type" for logging.
func questionString(msg *dns.Msg) string {
if msg == nil || len(msg.Question) == 0 {
return "<none>"
}
q := msg.Question[0]
return q.Name + "/" + dns.TypeToString[q.Qtype]
}
func setCachedAnswerTTL(answer *dns.Msg, now, expiredTime time.Time) {
ttlSecs := expiredTime.Sub(now).Seconds()
if ttlSecs < 0 {
@@ -1299,7 +1337,8 @@ func isPrivatePtrLookup(m *dns.Msg) bool {
return addr.IsPrivate() ||
addr.IsLoopback() ||
addr.IsLinkLocalUnicast() ||
tsaddr.CGNATRange().Contains(addr)
tsaddr.CGNATRange().Contains(addr) ||
isServiceContinuityAddr(addr)
}
}
return false
@@ -1337,6 +1376,20 @@ func isLanHostname(name string) bool {
strings.HasSuffix(name, ".local")
}
// ipv4ServiceContinuityPrefix is the RFC 7335 IPv4 Service Continuity Prefix
// (192.0.0.0/29), used by the CLAT in 464XLAT/DS-Lite transition setups. On such
// networks (common on IPv6-only cellular carriers and iPhone hotspots) the local
// machine's DNS queries reach ctrld with a source in this range (e.g. 192.0.0.2),
// so they must be treated as local, not WAN. Go's netip.IsPrivate does not cover
// this range — the same reason the CGNAT range is special-cased below. See #552.
var ipv4ServiceContinuityPrefix = netip.MustParsePrefix("192.0.0.0/29")
// isServiceContinuityAddr reports whether ip is in the RFC 7335 IPv4 Service
// Continuity Prefix (464XLAT/DS-Lite CLAT).
func isServiceContinuityAddr(ip netip.Addr) bool {
return ipv4ServiceContinuityPrefix.Contains(ip)
}
// isWanClient reports whether the input is a WAN address.
func isWanClient(na net.Addr) bool {
var ip netip.Addr
@@ -1347,7 +1400,8 @@ func isWanClient(na net.Addr) bool {
!ip.IsPrivate() &&
!ip.IsLinkLocalUnicast() &&
!ip.IsLinkLocalMulticast() &&
!tsaddr.CGNATRange().Contains(ip)
!tsaddr.CGNATRange().Contains(ip) &&
!isServiceContinuityAddr(ip)
}
// isIPv6LoopbackListener reports whether the listener address is [::1].
@@ -1733,6 +1787,15 @@ func (p *prog) checkUpstreamOnce(upstream string, uc *ctrld.UpstreamConfig) erro
mainLog.Load().Debug().Err(err).Msgf("Upstream %s check failed after %v (WFP loopback protect active)", upstream, duration)
return errOsHealthcheckSuppressed
}
// A no-route/network-unreachable failure means the endpoint's address
// family is available locally but unroutable (e.g. an IPv6 DoH endpoint
// while IPv6 is up but has no route). These repeat until the route
// returns and are handled by bounded backoff in the recovery loop, so
// keep them at debug to avoid sustained error-log spam.
if ctrldnet.IsUnreachable(err) {
mainLog.Load().Debug().Err(err).Msgf("Upstream %s check failed after %v (network unreachable)", upstream, duration)
return err
}
mainLog.Load().Error().Err(err).Msgf("Upstream %s check failed after %v", upstream, duration)
return err
}
@@ -1937,6 +2000,9 @@ func (p *prog) handleRecovery(reason RecoveryReason) {
// waitForUpstreamRecovery checks the provided upstreams concurrently until one recovers.
// It returns the name of the recovered upstream or an error if the check times out.
func (p *prog) waitForUpstreamRecovery(ctx context.Context, upstreams map[string]*ctrld.UpstreamConfig) (string, error) {
recoveryCtx, cancel := context.WithCancel(ctx)
defer cancel()
recoveredCh := make(chan string, 1)
var wg sync.WaitGroup
@@ -1948,9 +2014,10 @@ func (p *prog) waitForUpstreamRecovery(ctx context.Context, upstreams map[string
defer wg.Done()
mainLog.Load().Debug().Msgf("Starting recovery check loop for upstream: %s", name)
attempts := 0
unreachableStreak := 0
for {
select {
case <-ctx.Done():
case <-recoveryCtx.Done():
mainLog.Load().Debug().Msgf("Context canceled for upstream %s", name)
return
default:
@@ -1962,13 +2029,30 @@ func (p *prog) waitForUpstreamRecovery(ctx context.Context, upstreams map[string
select {
case recoveredCh <- name:
mainLog.Load().Debug().Msgf("Sent recovery notification for upstream %s", name)
cancel()
default:
mainLog.Load().Debug().Msg("Recovery channel full, another upstream already recovered")
}
return
}
mainLog.Load().Debug().Msgf("Upstream %s check failed, sleeping before retry", name)
time.Sleep(checkUpstreamBackoffSleep)
// Back off the retry cadence for an unroutable endpoint so a
// host with IPv6 up but no route to the IPv6 DoH endpoint does
// not re-bootstrap/re-check every checkUpstreamBackoffSleep and
// spam the log. The backoff is bounded (checkUpstreamUnreachableBackoffMax)
// so the endpoint is still re-probed and recovers when the route
// returns; any other failure resets to the base cadence.
sleep := checkUpstreamBackoffSleep
if ctrldnet.IsUnreachable(err) {
unreachableStreak++
sleep = unreachableRecoveryBackoff(unreachableStreak)
mainLog.Load().Debug().Msgf("Upstream %s unreachable (streak %d), backing off %s before retry", name, unreachableStreak, sleep)
} else {
unreachableStreak = 0
mainLog.Load().Debug().Msgf("Upstream %s check failed, sleeping before retry", name)
}
if !sleepWithContext(recoveryCtx, sleep) {
return
}
// if this is the upstreamOS and it's the 3rd attempt (or multiple of 3),
// we should try to reinit the OS resolver to ensure we can recover
@@ -1996,6 +2080,17 @@ func (p *prog) waitForUpstreamRecovery(ctx context.Context, upstreams map[string
return recovered, nil
}
func sleepWithContext(ctx context.Context, d time.Duration) bool {
timer := time.NewTimer(d)
defer timer.Stop()
select {
case <-timer.C:
return true
case <-ctx.Done():
return false
}
}
// buildRecoveryUpstreams constructs the map of upstream configurations to test.
// For OS failures we supply the manual OS resolver upstream configuration.
// For network change or regular failure we use the upstreams defined in p.cfg (ignoring OS).
+37
View File
@@ -405,6 +405,8 @@ func Test_isPrivatePtrLookup(t *testing.T) {
{"CGNAT", newDnsMsgPtr("100.66.27.28", t), true},
{"Loopback", newDnsMsgPtr("127.0.0.1", t), true},
{"Link Local Unicast", newDnsMsgPtr("fe80::69f6:e16e:8bdb:433f", t), true},
// RFC 7335 IPv4 Service Continuity Prefix (464XLAT/DS-Lite CLAT), see #552.
{"464XLAT CLAT host", newDnsMsgPtr("192.0.0.2", t), true},
{"Public IP", newDnsMsgPtr("8.8.8.8", t), false},
}
for _, tc := range tests {
@@ -452,6 +454,11 @@ func Test_isWanClient(t *testing.T) {
{"CGNAT", &net.UDPAddr{IP: net.ParseIP("100.66.27.28")}, false},
{"Loopback", &net.UDPAddr{IP: net.ParseIP("127.0.0.1")}, false},
{"Link Local Unicast", &net.UDPAddr{IP: net.ParseIP("fe80::69f6:e16e:8bdb:433f")}, false},
// RFC 7335 IPv4 Service Continuity Prefix (464XLAT/DS-Lite CLAT), see #552.
{"464XLAT PLAT side", &net.UDPAddr{IP: net.ParseIP("192.0.0.1")}, false},
{"464XLAT CLAT host", &net.UDPAddr{IP: net.ParseIP("192.0.0.2")}, false},
// Outside the /29 but inside 192.0.0.0/24: still WAN (fix is scoped to /29).
{"192.0.0.0/24 outside /29", &net.UDPAddr{IP: net.ParseIP("192.0.0.100")}, true},
{"Public", &net.UDPAddr{IP: net.ParseIP("8.8.8.8")}, true},
}
for _, tc := range tests {
@@ -474,3 +481,33 @@ func Test_prog_queryFromSelf(t *testing.T) {
p.queryFromSelf("foo")
})
}
func Test_sameQuestion(t *testing.T) {
mk := func(name string, qtype uint16) *dns.Msg {
m := new(dns.Msg)
m.SetQuestion(name, qtype)
return m
}
tests := []struct {
name string
req *dns.Msg
answer *dns.Msg
want bool
}{
{"identical", mk("example.com.", dns.TypeA), mk("example.com.", dns.TypeA), true},
{"case insensitive", mk("Example.COM.", dns.TypeA), mk("example.com.", dns.TypeA), true},
{"different name", mk("victim.example.", dns.TypeA), mk("attacker.example.", dns.TypeA), false},
{"different type", mk("example.com.", dns.TypeA), mk("example.com.", dns.TypeAAAA), false},
{"nil req", nil, mk("example.com.", dns.TypeA), false},
{"nil answer", mk("example.com.", dns.TypeA), nil, false},
{"empty answer question", mk("example.com.", dns.TypeA), new(dns.Msg), false},
}
for _, tc := range tests {
tc := tc
t.Run(tc.name, func(t *testing.T) {
if got := sameQuestion(tc.req, tc.answer); got != tc.want {
t.Errorf("sameQuestion() = %v, want %v", got, tc.want)
}
})
}
}
+6
View File
@@ -318,6 +318,12 @@ func (p *prog) initInternalLogging(writers []io.Writer) {
// needInternalLogging reports whether prog needs to run internal logging.
func (p *prog) needInternalLogging() bool {
// Do not run in silent mode: the user explicitly asked for no logging, so
// ctrld must not create or write the persisted internal log file (nor reset
// the global level back to debug). See https://github.com/Control-D-Inc/ctrld/issues/320.
if silent {
return false
}
// Do not run in non-cd mode.
if cdUID == "" {
return false
+66
View File
@@ -0,0 +1,66 @@
package cli
import (
"os"
"path/filepath"
"testing"
"github.com/Control-D-Inc/ctrld"
)
// Test_needInternalLogging_silent is a regression test for
// https://github.com/Control-D-Inc/ctrld/issues/320: running with --silent must
// not enable internal logging, otherwise ctrld creates and writes
// <homedir>/ctrld.log (and, when verbose==0, resets the global level back to
// debug) despite the user asking for silence.
func Test_needInternalLogging_silent(t *testing.T) {
origSilent, origCdUID := silent, cdUID
t.Cleanup(func() { silent, cdUID = origSilent, origCdUID })
tests := []struct {
name string
silent bool
cdUID string
logPath string
want bool
}{
{"silent suppresses internal logging in cd mode", true, "test-uid", "", false},
{"cd mode enables internal logging", false, "test-uid", "", true},
{"non-cd mode disabled", false, "", "", false},
{"explicit log path disables internal logging", false, "test-uid", "/var/log/ctrld.log", false},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
silent = tt.silent
cdUID = tt.cdUID
p := &prog{cfg: &ctrld.Config{}}
p.cfg.Service.LogPath = tt.logPath
if got := p.needInternalLogging(); got != tt.want {
t.Fatalf("needInternalLogging() = %v, want %v", got, tt.want)
}
})
}
}
// Test_initInternalLogging_silentCreatesNoFile drives the real initInternalLogging
// path and asserts that a --silent --cd run does not create <homedir>/ctrld.log,
// which is the observable failure reported in
// https://github.com/Control-D-Inc/ctrld/issues/320.
func Test_initInternalLogging_silentCreatesNoFile(t *testing.T) {
origSilent, origCdUID, origHomedir := silent, cdUID, homedir
t.Cleanup(func() { silent, cdUID, homedir = origSilent, origCdUID, origHomedir })
dir := t.TempDir()
homedir = dir
cdUID = "test-uid" // cd mode, which would otherwise enable internal logging
silent = true
p := &prog{cfg: &ctrld.Config{}}
p.initInternalLogging(nil)
logPath := filepath.Join(dir, logFileName)
if _, err := os.Stat(logPath); !os.IsNotExist(err) {
t.Fatalf("silent mode must not create %s (stat err = %v)", logPath, err)
}
}
+3
View File
@@ -56,6 +56,9 @@ const (
cdOrgFlagName = "cd-org"
customHostnameFlagName = "custom-hostname"
nextdnsFlagName = "nextdns"
// autoIface is the sentinel --iface value meaning "use the default gateway interface".
autoIface = "auto"
)
func init() {
+16
View File
@@ -2,6 +2,7 @@ package cli
import (
"os"
"os/exec"
"strings"
"testing"
@@ -13,5 +14,20 @@ var logOutput strings.Builder
func TestMain(m *testing.M) {
l := zerolog.New(&logOutput)
mainLog.Store(&l)
// Stub the self-upgrade command builder for the whole test binary. The real
// builder execs os.Executable() — which under `go test` IS this test binary
// — with positional args ("upgrade", ...). `go test` stops flag parsing at
// the first positional arg and ignores the rest, so the child just re-runs
// the entire suite, hits the upgrade tests again, and spawns more children:
// a fork bomb of detached processes that stalls the host and (on Windows)
// holds the test binary's image locked, breaking CI artifact cleanup.
// Point it at the test binary with a no-match -test.run so any test that
// reaches performUpgrade still exercises the cmd.Start() success path while
// the child exits immediately without recursing.
newUpgradeCmd = func(exe string) *exec.Cmd {
return exec.Command(exe, "-test.run=^$")
}
os.Exit(m.Run())
}
+101
View File
@@ -0,0 +1,101 @@
//go:build windows
package cli
import (
"sync"
"time"
"github.com/Control-D-Inc/ctrld"
)
const (
// Default to current behavior: keep recovering indefinitely unless configured.
defaultNRPTRecoveryMaxAttempts = 0
defaultNRPTRecoveryCooldown = 30 * time.Minute
// Require more than one good health tick before clearing the circuit. A probe can
// pass briefly after delete/re-add even when another agent recreates broken NRPT state.
nrptRecoveryStableSuccessesToReset = 2
)
type nrptRecoveryLimiter struct {
mu sync.Mutex
attempts int
stableSuccesses int
cooldownUntil time.Time
lastSkipLog time.Time
}
func nrptRecoveryMaxAttempts(cfg *ctrld.Config) int {
if cfg != nil && cfg.Service.NRPTRecoveryMaxAttempts != nil {
return *cfg.Service.NRPTRecoveryMaxAttempts
}
return defaultNRPTRecoveryMaxAttempts
}
func nrptRecoveryCooldown(cfg *ctrld.Config) time.Duration {
if cfg != nil && cfg.Service.NRPTRecoveryCooldown != nil {
return *cfg.Service.NRPTRecoveryCooldown
}
return defaultNRPTRecoveryCooldown
}
func (l *nrptRecoveryLimiter) allow(now time.Time, cfg *ctrld.Config) (bool, time.Duration) {
maxAttempts := nrptRecoveryMaxAttempts(cfg)
if maxAttempts <= 0 {
return true, 0
}
l.mu.Lock()
defer l.mu.Unlock()
if now.Before(l.cooldownUntil) {
return false, l.cooldownUntil.Sub(now)
}
return true, 0
}
func (l *nrptRecoveryLimiter) recordRecoveryFlow(now time.Time, cfg *ctrld.Config) {
maxAttempts := nrptRecoveryMaxAttempts(cfg)
if maxAttempts <= 0 {
return
}
cooldown := nrptRecoveryCooldown(cfg)
if cooldown <= 0 {
cooldown = defaultNRPTRecoveryCooldown
}
l.mu.Lock()
defer l.mu.Unlock()
l.stableSuccesses = 0
l.attempts++
if l.attempts >= maxAttempts {
l.cooldownUntil = now.Add(cooldown)
}
}
func (l *nrptRecoveryLimiter) recordStableSuccess() {
l.mu.Lock()
defer l.mu.Unlock()
l.stableSuccesses++
if l.stableSuccesses >= nrptRecoveryStableSuccessesToReset {
l.attempts = 0
l.cooldownUntil = time.Time{}
l.lastSkipLog = time.Time{}
}
}
func (l *nrptRecoveryLimiter) shouldLogSkip(now time.Time) bool {
l.mu.Lock()
defer l.mu.Unlock()
if l.lastSkipLog.IsZero() || now.Sub(l.lastSkipLog) >= 5*time.Minute {
l.lastSkipLog = now
return true
}
return false
}
@@ -0,0 +1,74 @@
//go:build windows
package cli
import (
"testing"
"time"
"github.com/Control-D-Inc/ctrld"
)
func TestNRPTRecoveryLimiterCooldownAndStableReset(t *testing.T) {
maxAttempts := 2
cooldown := 10 * time.Minute
cfg := &ctrld.Config{}
cfg.Service.NRPTRecoveryMaxAttempts = &maxAttempts
cfg.Service.NRPTRecoveryCooldown = &cooldown
limiter := &nrptRecoveryLimiter{}
now := time.Unix(100, 0)
if ok, wait := limiter.allow(now, cfg); !ok || wait != 0 {
t.Fatalf("initial allow = %v, %v; want true, 0", ok, wait)
}
limiter.recordRecoveryFlow(now, cfg)
if ok, wait := limiter.allow(now.Add(time.Second), cfg); !ok || wait != 0 {
t.Fatalf("allow after first flow = %v, %v; want true, 0", ok, wait)
}
limiter.recordRecoveryFlow(now.Add(2*time.Second), cfg)
if ok, wait := limiter.allow(now.Add(3*time.Second), cfg); ok || wait <= 0 {
t.Fatalf("allow after max flows = %v, %v; want false, positive wait", ok, wait)
}
limiter.recordStableSuccess()
if ok, _ := limiter.allow(now.Add(4*time.Second), cfg); ok {
t.Fatal("one stable success cleared cooldown; want cooldown to remain")
}
limiter.recordStableSuccess()
if ok, wait := limiter.allow(now.Add(5*time.Second), cfg); !ok || wait != 0 {
t.Fatalf("allow after stable reset = %v, %v; want true, 0", ok, wait)
}
}
func TestNRPTRecoveryLimiterDefaultIsUnlimited(t *testing.T) {
cfg := &ctrld.Config{}
limiter := &nrptRecoveryLimiter{}
now := time.Unix(100, 0)
for i := 0; i < 10; i++ {
limiter.recordRecoveryFlow(now.Add(time.Duration(i)*time.Second), cfg)
}
if ok, wait := limiter.allow(now.Add(time.Hour), cfg); !ok || wait != 0 {
t.Fatalf("default allow after recovery flows = %v, %v; want true, 0", ok, wait)
}
}
func TestNRPTRecoveryLimiterUnlimited(t *testing.T) {
maxAttempts := 0
cfg := &ctrld.Config{}
cfg.Service.NRPTRecoveryMaxAttempts = &maxAttempts
limiter := &nrptRecoveryLimiter{}
now := time.Unix(100, 0)
for i := 0; i < 10; i++ {
limiter.recordRecoveryFlow(now.Add(time.Duration(i)*time.Second), cfg)
}
if ok, wait := limiter.allow(now.Add(time.Hour), cfg); !ok || wait != 0 {
t.Fatalf("unlimited allow = %v, %v; want true, 0", ok, wait)
}
}
+83 -9
View File
@@ -34,6 +34,7 @@ import (
"github.com/Control-D-Inc/ctrld/internal/clientinfo"
"github.com/Control-D-Inc/ctrld/internal/controld"
"github.com/Control-D-Inc/ctrld/internal/dnscache"
ctrldnet "github.com/Control-D-Inc/ctrld/internal/net"
"github.com/Control-D-Inc/ctrld/internal/router"
"github.com/Control-D-Inc/ctrld/internal/router/dnsmasq"
)
@@ -188,6 +189,21 @@ type prog struct {
// interception with exponential backoff and auto-heals if broken.
pfMonitorRunning atomic.Bool //lint:ignore U1000 used on darwin
// pfEnsureRunning ensures only one pf anchor validation/restoration runs at a time.
// Network-change callbacks, delayed rechecks, and the periodic watchdog can all
// converge during macOS interface churn; concurrent pfctl/scutil exec storms can
// exhaust process/file limits and make the outage worse.
pfEnsureRunning atomic.Bool //lint:ignore U1000 used on darwin
// pfExecBackoffUntil suppresses pf anchor validation after pfctl/scutil execs
// fail due host resource exhaustion (fork unavailable, too many open files).
pfExecBackoffUntil atomic.Int64 //lint:ignore U1000 used on darwin
// pfDelayedRecheckTimers coalesces delayed DNS-intercept rechecks after noisy
// network changes. Protected by pfDelayedRecheckMu.
pfDelayedRecheckMu sync.Mutex //lint:ignore U1000 used on darwin
pfDelayedRecheckTimers []*time.Timer //lint:ignore U1000 used on darwin
// pfProbeExpected holds the domain name of a pending pf interception probe.
// When non-empty, the DNS handler checks incoming queries against this value
// and signals pfProbeCh if matched. The probe verifies that pf's rdr rules
@@ -321,6 +337,18 @@ func (p *prog) runWait() {
p.mu.Lock()
*p.cfg = *newCfg
// In DNS-intercept mode on macOS, the DNS listener is bound once at startup and is
// NOT re-bound on reload (see prog.run: serveDNS is started only when !reload). When
// the configured/generated port (e.g. 127.0.0.1:53) is unavailable at startup because
// mDNSResponder owns *:53, ctrld falls back to an alternate local port (e.g. 5354).
// The on-disk config still declares 53, so adopting it here would revert p.cfg to a
// port nothing is listening on, and the pf rdr rules/probes rebuilt from p.cfg would
// target a dead port. Since a reload cannot move the running listener anyway, keep
// p.cfg pointing at the actual bound listener. The on-disk config (written above) is
// left unchanged. See #551.
if dnsIntercept && runtime.GOOS == "darwin" {
preserveBoundListeners(p.cfg.Listener, curListener)
}
p.mu.Unlock()
logger.Notice().Msg("reloading config successfully")
@@ -332,8 +360,41 @@ func (p *prog) runWait() {
}
}
// preserveBoundListeners overrides the IP/Port of each listener in newListeners with the
// actual bound address from curListeners when they differ, logging the divergence. It is used
// on config reload in DNS-intercept mode where the running listener is never re-bound, so a
// port change on disk (e.g. reverting a fallback 5354 back to the generated 53) must not be
// applied to the in-memory config that drives pf rdr rules and probes.
//
// Preservation is limited to fallback-eligible (default/unset, i.e. 127.0.0.1:53) listeners.
// An explicit, non-default listener in the reloaded config is an intentional change that must
// be applied: tryUpdateListenerConfigIntercept binds explicit listeners exactly (no fallback),
// and the control-server reload handler detects the IP/port diff to trigger a restart that
// re-binds. Reverting an explicit change here would make that comparison return 200 instead of
// 201, silently dropping the new listener. See #551.
func preserveBoundListeners(newListeners, curListeners map[string]*ctrld.ListenerConfig) {
for n, curLc := range curListeners {
newLc := newListeners[n]
if newLc == nil || curLc == nil {
continue
}
if newLc.IP == curLc.IP && newLc.Port == curLc.Port {
continue
}
if isExplicitInterceptListener(newLc.IP, newLc.Port) {
continue
}
mainLog.Load().Info().
Str("configured", net.JoinHostPort(newLc.IP, strconv.Itoa(newLc.Port))).
Str("actual", net.JoinHostPort(curLc.IP, strconv.Itoa(curLc.Port))).
Msg("DNS intercept: preserving actual bound listener across reload; on-disk config port not applied to running listener")
newLc.IP = curLc.IP
newLc.Port = curLc.Port
}
}
func (p *prog) preRun() {
if iface == "auto" {
if iface == autoIface {
iface = defaultIfaceName()
p.requiredMultiNICsConfig = requiredMultiNICsConfig()
}
@@ -1328,13 +1389,14 @@ func errAddrInUse(err error) bool {
var _ = errAddrInUse
// The unreachable winsock errnos (ENETUNREACH/EHOSTUNREACH) are matched via
// ctrldnet.IsUnreachable, which owns their definitions.
//
// https://learn.microsoft.com/en-us/windows/win32/winsock/windows-sockets-error-codes-2
var (
windowsECONNREFUSED = syscall.Errno(10061)
windowsENETUNREACH = syscall.Errno(10051)
windowsEINVAL = syscall.Errno(10022)
windowsEADDRINUSE = syscall.Errno(10048)
windowsEHOSTUNREACH = syscall.Errno(10065)
)
func errUrlNetworkError(err error) bool {
@@ -1351,14 +1413,14 @@ func errNetworkError(err error) bool {
if opErr.Temporary() {
return true
}
if ctrldnet.IsUnreachable(err) {
return true
}
switch {
case errors.Is(opErr.Err, syscall.ECONNREFUSED),
errors.Is(opErr.Err, syscall.EINVAL),
errors.Is(opErr.Err, syscall.ENETUNREACH),
errors.Is(opErr.Err, windowsENETUNREACH),
errors.Is(opErr.Err, windowsEINVAL),
errors.Is(opErr.Err, windowsECONNREFUSED),
errors.Is(opErr.Err, windowsEHOSTUNREACH):
errors.Is(opErr.Err, windowsECONNREFUSED):
return true
}
}
@@ -1651,6 +1713,19 @@ func shouldUpgrade(vt string, cv *semver.Version, logger *zerolog.Logger) bool {
return true
}
// newUpgradeCmd builds the detached command used to self-upgrade. It is a
// package-level variable so tests can stub it. With the real implementation a
// *test* binary would re-exec itself — os.Executable() is the test binary, and
// because `go test` stops flag parsing at the first positional arg ("upgrade")
// it ignores the args and re-runs the entire suite. That child hits the same
// upgrade test and spawns another child, recursively: a fork bomb of detached
// processes that pins the host and locks the test binary's image file.
var newUpgradeCmd = func(exe string) *exec.Cmd {
cmd := exec.Command(exe, "upgrade", "prod", "-vv")
cmd.SysProcAttr = sysProcAttrForDetachedChildProcess()
return cmd
}
// performUpgrade executes the self-upgrade command.
// Returns true if upgrade was initiated successfully, false otherwise.
func performUpgrade(vt string) bool {
@@ -1659,8 +1734,7 @@ func performUpgrade(vt string) bool {
mainLog.Load().Error().Err(err).Msg("failed to get executable path, skipped self-upgrade")
return false
}
cmd := exec.Command(exe, "upgrade", "prod", "-vv")
cmd.SysProcAttr = sysProcAttrForDetachedChildProcess()
cmd := newUpgradeCmd(exe)
if err := cmd.Start(); err != nil {
mainLog.Load().Error().Err(err).Msg("failed to start self-upgrade")
return false
+3
View File
@@ -14,6 +14,9 @@ import (
)
func init() {
if isAndroid() {
return
}
if r, err := newLoopbackOSConfigurator(); err == nil {
useSystemdResolved = r.Mode() == "systemd-resolved"
}
+32
View File
@@ -1,7 +1,11 @@
package cli
import (
"context"
"net"
"net/url"
"runtime"
"syscall"
"testing"
"time"
@@ -12,6 +16,32 @@ import (
"github.com/Control-D-Inc/ctrld"
)
func TestErrNetworkErrorTreatsNoRouteAsNetworkError(t *testing.T) {
err := &net.OpError{Op: "dial", Net: "tcp", Err: syscall.EHOSTUNREACH}
assert.True(t, errNetworkError(err))
assert.True(t, errUrlNetworkError(&url.Error{Op: "Get", URL: "https://dns.controld.com", Err: err}))
}
func TestSleepWithContext(t *testing.T) {
assert.True(t, sleepWithContext(context.Background(), time.Millisecond))
ctx, cancel := context.WithCancel(context.Background())
cancel()
start := time.Now()
assert.False(t, sleepWithContext(ctx, time.Minute))
assert.Less(t, time.Since(start), 100*time.Millisecond)
}
func TestUnreachableRecoveryBackoff(t *testing.T) {
// Streak starts at the base cadence and doubles each attempt, capped at the max.
assert.Equal(t, checkUpstreamBackoffSleep, unreachableRecoveryBackoff(0))
assert.Equal(t, checkUpstreamBackoffSleep, unreachableRecoveryBackoff(1))
assert.Equal(t, 2*checkUpstreamBackoffSleep, unreachableRecoveryBackoff(2))
assert.Equal(t, 4*checkUpstreamBackoffSleep, unreachableRecoveryBackoff(3))
assert.Equal(t, checkUpstreamUnreachableBackoffMax, unreachableRecoveryBackoff(100))
}
func Test_prog_dnsWatchdogEnabled(t *testing.T) {
p := &prog{cfg: &ctrld.Config{}}
@@ -262,6 +292,8 @@ func Test_performUpgrade(t *testing.T) {
},
}
// newUpgradeCmd is stubbed in TestMain so performUpgrade does not re-exec
// (and fork-bomb) the test binary; see the comment there.
for _, tc := range tests {
tc := tc
t.Run(tc.name, func(t *testing.T) {
+61
View File
@@ -0,0 +1,61 @@
package cli
import "testing"
// Test_ensureRunningIfaceForInvalidUninstall is a regression test for issue-556:
// after a reboot, the invalid-device self-uninstall path could run before the
// running interface was known. Because resetDNS (via resetDNSForRunningIface)
// silently skips DNS restoration when p.runningIface is empty, the OS was left
// pointed at ctrld's local listener with no internet after the service was
// removed. ensureRunningIfaceForInvalidUninstall must populate p.runningIface
// before resetDNS runs.
func Test_ensureRunningIfaceForInvalidUninstall(t *testing.T) {
// preRun mutates the package-level iface global; restore it after the test.
origIface := iface
t.Cleanup(func() { iface = origIface })
// newService needs the package service config; it is safe to build here
// because ensureRunningIfaceForInvalidUninstall only queries the (absent)
// control socket via runningIface, which returns nil when ctrld is not
// running, and performs no DNS or service mutation.
s, err := newService(&prog{}, svcConfig)
if err != nil {
t.Fatalf("newService: %v", err)
}
t.Run("iface flag already resolved but not yet copied", func(t *testing.T) {
iface = "eth-test"
p := &prog{}
// Precondition mirrors the buggy post-reboot state: an empty running
// interface would make resetDNS skip restoration entirely.
if p.runningIface != "" {
t.Fatalf("precondition: runningIface = %q, want empty", p.runningIface)
}
ensureRunningIfaceForInvalidUninstall(p, s)
if p.runningIface == "" {
t.Fatal("runningIface still empty after prepare: resetDNS would skip DNS " +
"restoration and leave the OS pointed at ctrld's local listener")
}
if p.runningIface != "eth-test" {
t.Fatalf("runningIface = %q, want the resolved iface %q", p.runningIface, "eth-test")
}
})
t.Run("iface unset falls back to auto-detected interface", func(t *testing.T) {
iface = ""
p := &prog{}
ensureRunningIfaceForInvalidUninstall(p, s)
// With iface unset the prep resolves "auto" to the default interface
// (defaultIfaceName never returns empty on the supported platforms), so
// resetDNS has a concrete interface to restore.
if p.runningIface == "" {
t.Fatal("runningIface still empty after prepare with iface unset: " +
"resetDNS would skip DNS restoration")
}
})
}
+19
View File
@@ -12,8 +12,27 @@ const (
maxFailureRequest = 50
// checkUpstreamBackoffSleep is the time interval between each upstream checks.
checkUpstreamBackoffSleep = 2 * time.Second
// checkUpstreamUnreachableBackoffMax caps the recovery retry interval for an
// endpoint that keeps failing with a network-unreachable error. It bounds
// the backoff so an unroutable endpoint is still re-probed periodically and
// recovers once the route returns.
checkUpstreamUnreachableBackoffMax = 60 * time.Second
)
// unreachableRecoveryBackoff returns the retry interval for the given streak of
// consecutive network-unreachable failures. It starts at checkUpstreamBackoffSleep
// and doubles each attempt, capped at checkUpstreamUnreachableBackoffMax.
func unreachableRecoveryBackoff(streak int) time.Duration {
d := checkUpstreamBackoffSleep
for i := 1; i < streak; i++ {
d *= 2
if d >= checkUpstreamUnreachableBackoffMax {
return checkUpstreamUnreachableBackoffMax
}
}
return d
}
// upstreamMonitor performs monitoring upstreams health.
type upstreamMonitor struct {
cfg *ctrld.Config
+89 -6
View File
@@ -6,7 +6,9 @@ import (
"runtime"
"strings"
"sync"
"sync/atomic"
"github.com/rs/zerolog"
"tailscale.com/net/netmon"
"github.com/Control-D-Inc/ctrld"
@@ -49,6 +51,9 @@ type vpnDNSManager struct {
// discoverVPNDNS is injected for tests so Refresh does not depend on the
// runner host's real VPN/virtual adapter state.
discoverVPNDNS func(context.Context) []ctrld.VPNDNSConfig
// refreshRunning keeps noisy network-change storms from running overlapping
// scutil/networksetup VPN DNS discovery work.
refreshRunning atomic.Bool
// Called when VPN DNS server list changes, to update intercept exemptions.
onServersChanged vpnDNSExemptFunc
}
@@ -68,6 +73,11 @@ func newVPNDNSManager(exemptFunc vpnDNSExemptFunc) *vpnDNSManager {
// Called on network change events.
func (m *vpnDNSManager) Refresh(guardAgainstNoNameservers bool) {
logger := mainLog.Load()
if !m.refreshRunning.CompareAndSwap(false, true) {
logger.Debug().Msg("VPN DNS refresh already running, skipping duplicate")
return
}
defer m.refreshRunning.Store(false)
logger.Debug().Msg("Refreshing VPN DNS configurations")
discoverVPNDNS := m.discoverVPNDNS
@@ -94,6 +104,8 @@ func (m *vpnDNSManager) Refresh(guardAgainstNoNameservers bool) {
m.mu.Lock()
defer m.mu.Unlock()
previousExemptions := m.currentExemptionsLocked()
if vpnDNSSettlingEnabled && len(configs) == 0 && guardAgainstNoNameservers && m.hasVPNDNSStateLocked() {
if !m.retainedAfterEmptyDiscovery {
exemptions := m.currentExemptionsLocked()
@@ -180,14 +192,85 @@ func (m *vpnDNSManager) Refresh(guardAgainstNoNameservers bool) {
logger.Debug().Msgf("VPN DNS refresh completed: %d configs, %d routes, %d domainless servers, %d unique exemptions",
len(m.configs), len(m.routes), len(m.domainlessServers), len(exemptions))
// Update intercept rules to permit VPN DNS traffic.
// Always call onServersChanged — including when exemptions is empty — so that
// stale exemptions from a previous VPN session get cleared on disconnect.
if m.onServersChanged != nil {
if err := m.onServersChanged(exemptions); err != nil {
logger.Error().Err(err).Msg("Failed to update intercept exemptions for VPN DNS servers")
// Update intercept rules to permit VPN DNS traffic only when the exemption set
// actually changes. Network-change events can fire repeatedly while macOS/VPN
// state is otherwise identical; rewriting pf for identical exemptions can feed
// a self-triggering network-change loop. Empty exemptions are still applied
// when they differ from the previous set, so stale VPN exemptions are cleared
// on disconnect.
m.updateInterceptExemptionsIfChanged(logger, previousExemptions, exemptions, "VPN DNS")
}
func (m *vpnDNSManager) updateInterceptExemptionsIfChanged(logger *zerolog.Logger, before, after []vpnDNSExemption, reason string) {
if m.onServersChanged == nil {
return
}
if vpnDNSExemptionsEqual(before, after) {
logger.Debug().Msgf("VPN DNS exemptions unchanged after %s refresh; skipping intercept rule update", reason)
return
}
if err := m.onServersChanged(after); err != nil {
logger.Error().Err(err).Msg("Failed to update intercept exemptions for VPN DNS servers")
}
}
// RefreshRoutesOnly re-discovers VPN DNS configs and updates only ctrld's
// in-memory split-DNS routes. It intentionally does not call onServersChanged,
// so it does not rewrite/reload pf/WFP rules. Use this for post-settle discovery
// checks where we only need to learn late-published VPN search domains.
func (m *vpnDNSManager) RefreshRoutesOnly() (routes, domainlessServers, exemptions int) {
logger := mainLog.Load()
logger.Debug().Msg("Refreshing VPN DNS route state only")
discoverVPNDNS := m.discoverVPNDNS
if discoverVPNDNS == nil {
discoverVPNDNS = ctrld.DiscoverVPNDNS
}
configs := discoverVPNDNS(context.Background())
if dri, err := netmon.DefaultRouteInterface(); err == nil && dri != "" {
for i := range configs {
if configs[i].InterfaceName == dri {
configs[i].IsExitMode = true
}
}
}
m.mu.Lock()
defer m.mu.Unlock()
m.retainedAfterEmptyDiscovery = false
m.configs = configs
m.routes = make(map[string][]string)
for _, config := range configs {
for _, domain := range config.Domains {
domain = strings.TrimPrefix(domain, "~")
domain = strings.TrimPrefix(domain, ".")
domain = strings.ToLower(domain)
if domain != "" {
m.routes[domain] = append([]string{}, config.Servers...)
}
}
}
var domainless []string
seenDomainless := make(map[string]bool)
for _, config := range configs {
if len(config.Domains) == 0 && len(config.Servers) > 0 {
for _, server := range config.Servers {
if !seenDomainless[server] {
seenDomainless[server] = true
domainless = append(domainless, server)
}
}
}
}
m.domainlessServers = domainless
logger.Debug().Msgf("VPN DNS route-only refresh completed: %d configs, %d routes, %d domainless servers, %d exemptions",
len(m.configs), len(m.routes), len(m.domainlessServers), len(m.currentExemptionsLocked()))
return len(m.routes), len(m.domainlessServers), len(m.currentExemptionsLocked())
}
func (m *vpnDNSManager) hasVPNDNSStateLocked() bool {
+57
View File
@@ -2,6 +2,8 @@ package cli
import (
"context"
"sync"
"sync/atomic"
"testing"
"github.com/Control-D-Inc/ctrld"
@@ -14,6 +16,36 @@ func withVPNDNSSettlingEnabled(t *testing.T) {
t.Cleanup(func() { vpnDNSSettlingEnabled = old })
}
func TestVPNDNSRefreshSkipsConcurrentDuplicate(t *testing.T) {
m := newVPNDNSManager(nil)
started := make(chan struct{})
release := make(chan struct{})
done := make(chan struct{})
var once sync.Once
var calls atomic.Int32
m.discoverVPNDNS = func(context.Context) []ctrld.VPNDNSConfig {
calls.Add(1)
once.Do(func() { close(started) })
<-release
return nil
}
go func() {
defer close(done)
m.Refresh(true)
}()
<-started
m.Refresh(true)
close(release)
<-done
if calls.Load() != 1 {
t.Fatalf("expected overlapping refresh to be skipped, got %d discovery calls", calls.Load())
}
}
func TestVPNDNSRefreshRetainsStateForOneGuardedEmptyDiscovery(t *testing.T) {
withVPNDNSSettlingEnabled(t)
var gotExemptions []vpnDNSExemption
@@ -69,6 +101,31 @@ func TestVPNDNSRefreshClearsOnSecondGuardedEmptyDiscovery(t *testing.T) {
}
}
func TestVPNDNSRefreshSkipsUnchangedInterceptExemptions(t *testing.T) {
var updates [][]vpnDNSExemption
m := newVPNDNSManager(func(exemptions []vpnDNSExemption) error {
updates = append(updates, append([]vpnDNSExemption{}, exemptions...))
return nil
})
m.discoverVPNDNS = func(context.Context) []ctrld.VPNDNSConfig {
return []ctrld.VPNDNSConfig{{
InterfaceName: "utun-test",
Servers: []string{"10.102.26.10"},
Domains: []string{"example.internal"},
}}
}
m.Refresh(true)
m.Refresh(true)
if len(updates) != 1 {
t.Fatalf("expected exactly one intercept exemption update for unchanged VPN DNS state, got %d", len(updates))
}
if len(updates[0]) != 1 || updates[0][0].Server != "10.102.26.10" || updates[0][0].Interface != "utun-test" {
t.Fatalf("unexpected exemption update: %+v", updates[0])
}
}
func TestVPNDNSTransportFailureSuppressesFallbackOnlyWhileRetainingState(t *testing.T) {
withVPNDNSSettlingEnabled(t)
m := newVPNDNSManager(nil)
+3
View File
@@ -241,6 +241,8 @@ type ServiceConfig struct {
ForceRefetchWaitTime *int `mapstructure:"force_refetch_wait_time" toml:"force_refetch_wait_time,omitempty"`
LeakOnUpstreamFailure *bool `mapstructure:"leak_on_upstream_failure" toml:"leak_on_upstream_failure,omitempty"`
InterceptMode string `mapstructure:"intercept_mode" toml:"intercept_mode,omitempty" validate:"omitempty,oneof=off dns hard"`
NRPTRecoveryMaxAttempts *int `mapstructure:"nrpt_recovery_max_attempts" toml:"nrpt_recovery_max_attempts,omitempty" validate:"omitempty,gte=0"`
NRPTRecoveryCooldown *time.Duration `mapstructure:"nrpt_recovery_cooldown" toml:"nrpt_recovery_cooldown,omitempty"`
Daemon bool `mapstructure:"-" toml:"-"`
AllocateIP bool `mapstructure:"-" toml:"-"`
}
@@ -640,6 +642,7 @@ func (uc *UpstreamConfig) newDOHTransport(addrs []string) *http.Transport {
transport.TLSClientConfig = &tls.Config{
RootCAs: uc.certPool,
ClientSessionCache: tls.NewLRUClientSessionCache(0),
MinVersion: tls.VersionTLS12,
}
// Prevent bad tcp connection hanging the requests for too long.
+29 -7
View File
@@ -18,7 +18,7 @@ func (uc *UpstreamConfig) newDOH3Transport(addrs []string) http.RoundTripper {
return nil
}
rt := &http3.Transport{}
rt.TLSClientConfig = &tls.Config{RootCAs: uc.certPool}
rt.TLSClientConfig = &tls.Config{RootCAs: uc.certPool, MinVersion: tls.VersionTLS12}
rt.Dial = func(ctx context.Context, addr string, tlsCfg *tls.Config, cfg *quic.Config) (*quic.Conn, error) {
_, port, _ := net.SplitHostPort(addr)
// if we have a bootstrap ip set, use it to avoid DNS lookup
@@ -77,7 +77,17 @@ type parallelDialerResult struct {
err error
}
type quicParallelDialer struct{}
// quicParallelDialer races DialEarly across a list of remote addresses and
// returns the first successful connection. When transport is non-nil, all
// dials share that transport's UDP socket, which removes both the per-dial
// socket allocation and the winner-path socket leak that an owner-of-the-conn
// receiver cannot clean up. When transport is nil, the dialer falls back to a
// fresh UDP socket per attempt (compat path used where no shared transport is
// available yet); the loser paths close their sockets, and the winner path's
// socket is owned by quic.DialEarly's internal transport.
type quicParallelDialer struct {
transport *quic.Transport
}
// Dial performs parallel dialing to the given address list.
func (d *quicParallelDialer) Dial(ctx context.Context, addrs []string, tlsCfg *tls.Config, cfg *quic.Config) (*quic.Conn, error) {
@@ -105,12 +115,24 @@ func (d *quicParallelDialer) Dial(ctx context.Context, addrs []string, tlsCfg *t
ch <- &parallelDialerResult{conn: nil, err: err}
return
}
udpConn, err := net.ListenUDP("udp", nil)
if err != nil {
ch <- &parallelDialerResult{conn: nil, err: err}
return
var (
conn *quic.Conn
udpConn *net.UDPConn
)
if d.transport != nil {
conn, err = d.transport.DialEarly(ctx, remoteAddr, tlsCfg, cfg)
} else {
udpConn, err = net.ListenUDP("udp", nil)
if err != nil {
ch <- &parallelDialerResult{conn: nil, err: err}
return
}
conn, err = quic.DialEarly(ctx, udpConn, remoteAddr, tlsCfg, cfg)
if err != nil {
udpConn.Close()
udpConn = nil
}
}
conn, err := quic.DialEarly(ctx, udpConn, remoteAddr, tlsCfg, cfg)
select {
case ch <- &parallelDialerResult{conn: conn, err: err}:
case <-done:
+6
View File
@@ -14,6 +14,12 @@ func SetCacheReply(answer, msg *dns.Msg, code int) {
// See https://datatracker.ietf.org/doc/html/rfc7873#section-4
sCookie.Cookie = cCookie.Cookie[:16] + sCookie.Cookie[16:]
}
// NOTE: the answer's EDNS Client Subnet (ECS) is intentionally left as the
// upstream returned it. Correctness across clients is guaranteed by
// partitioning the cache and singleflight keys by ECS (see
// dnscache.CanonicalECS), so a cache hit only ever serves an answer that was
// resolved for the requester's own subnet. Rewriting the ECS option here
// without re-scoping the Answer records would violate RFC 7871 §7.3.
}
// getEdns0Cookie returns Edns0 cookie from *dns.OPT if present.
+57
View File
@@ -0,0 +1,57 @@
package ctrld
import (
"net"
"testing"
"github.com/miekg/dns"
)
// Test_SetCacheReply_DoesNotRewriteECS documents the post-#564 contract: cross-client
// correctness is guaranteed by partitioning the cache/singleflight keys by ECS
// (dnscache.CanonicalECS), NOT by rewriting the cached answer's ECS option. Rewriting the
// ECS metadata while leaving the Answer records scoped to another subnet would violate
// RFC 7871 §7.3 and make forwarders accept a wrong-subnet answer. SetCacheReply must
// therefore leave the answer's ECS untouched.
func Test_SetCacheReply_DoesNotRewriteECS(t *testing.T) {
answer := new(dns.Msg)
answer.SetQuestion(dns.Fqdn("controld.com"), dns.TypeA)
answer.SetEdns0(4096, false)
cachedSubnet := &dns.EDNS0_SUBNET{
Code: dns.EDNS0SUBNET,
Family: 2,
SourceNetmask: 64,
SourceScope: 64,
Address: net.ParseIP("2001:db8:1::"),
}
answer.IsEdns0().Option = append(answer.IsEdns0().Option, cachedSubnet)
req := new(dns.Msg)
req.SetQuestion(dns.Fqdn("controld.com"), dns.TypeA)
req.SetEdns0(4096, true)
req.IsEdns0().Option = append(req.IsEdns0().Option, &dns.EDNS0_SUBNET{
Code: dns.EDNS0SUBNET,
Family: 2,
SourceNetmask: 64,
Address: net.ParseIP("2001:db8:2::"),
})
SetCacheReply(answer, req, dns.RcodeSuccess)
var got *dns.EDNS0_SUBNET
for _, o := range answer.IsEdns0().Option {
if e, ok := o.(*dns.EDNS0_SUBNET); ok {
got = e
break
}
}
if got == nil {
t.Fatal("SetCacheReply dropped the answer's ECS option")
}
if want := net.ParseIP("2001:db8:1::"); !got.Address.Equal(want) {
t.Fatalf("SetCacheReply rewrote the answer ECS to the requester's subnet: got %v, want %v (unchanged)", got.Address, want)
}
if got.SourceScope != 64 {
t.Fatalf("SetCacheReply altered the answer ECS scope: got %d, want 64 (unchanged)", got.SourceScope)
}
}
+4 -3
View File
@@ -1,4 +1,4 @@
# Using Debian bullseye for building regular image.
# Using Debian bookworm for building regular image.
# Using scratch image for minimal image size.
# The final image has:
#
@@ -8,11 +8,12 @@
# - Non-cgo ctrld binary.
#
# CI_COMMIT_TAG is used to set the version of ctrld binary.
FROM golang:1.20-bullseye as base
FROM golang:1.25-bookworm AS base
WORKDIR /app
RUN apt-get update && apt-get install -y upx-ucl
RUN echo "deb http://deb.debian.org/debian bookworm-backports main" | tee /etc/apt/sources.list.d/backports.list
RUN apt update && apt install -t bookworm-backports upx-ucl
COPY . .
+4 -3
View File
@@ -1,4 +1,4 @@
# Using Debian bullseye for building regular image.
# Using Debian bookworm for building regular image.
# Using scratch image for minimal image size.
# The final image has:
#
@@ -8,11 +8,12 @@
# - Non-cgo ctrld binary.
#
# CI_COMMIT_TAG is used to set the version of ctrld binary.
FROM golang:bullseye as base
FROM golang:1.25-bookworm AS base
WORKDIR /app
RUN apt-get update && apt-get install -y upx-ucl
RUN echo "deb http://deb.debian.org/debian bookworm-backports main" | tee /etc/apt/sources.list.d/backports.list
RUN apt update && apt install -t bookworm-backports upx-ucl
COPY . .
+16
View File
@@ -295,6 +295,22 @@ If a remote upstream fails to resolve a query or is unreachable, `ctrld` will fo
- Required: no
- Default: true on Windows, MacOS and non-router Linux.
### nrpt_recovery_max_attempts
Windows DNS intercept mode uses NRPT health probes and recovery when Windows stops routing queries to the local `ctrld` listener. This limits how many consecutive recovery flows can run before `ctrld` enters a cooldown and stops making policy/Dnscache changes.
Set to `0` to disable this circuit breaker and keep retrying indefinitely.
- Type: integer
- Required: no
- Default: 0 (unlimited, current behavior)
### nrpt_recovery_cooldown
Cooldown duration after `nrpt_recovery_max_attempts` consecutive Windows NRPT recovery flows. During cooldown, `ctrld` logs the suppressed recovery and avoids additional `RefreshPolicyEx`, Dnscache `paramchange`, and DNS cache flush calls.
- Type: time duration string
- Required: no
- Default: 30m
## Upstream
The `[upstream]` section specifies the DNS upstream servers that `ctrld` will forward DNS requests to.
+14
View File
@@ -91,6 +91,20 @@ ctrld uses an adaptive strategy (matching [Tailscale's approach](https://github.
the empty GP parent key. This ensures DNS Client stays in "local mode" where
the local-path rule activates immediately via `paramchange`.
### Reproducing the Empty GP Parent Case
This is a production code reference, so the temporary repro script is not kept in
the repository. For MR !942 review, the test script and exact before/after steps
are posted in the MR discussion. The scenario to compare is:
1. Run the same approved PowerShell repro script against a pre-fix build and this
branch with the same ctrld config.
2. Create an empty GP NRPT parent key while ctrld is running in DNS intercept mode.
3. Confirm pre-fix logs can spend policy refresh/paramchange retries while the GP
parent remains empty.
4. Confirm post-fix logs clean the empty GP parent, send one NRPT-change signal,
and re-probe before normal retries.
### VPN Coexistence
NRPT uses most-specific-match. VPN NRPT rules for specific domains (e.g.,
+18 -4
View File
@@ -25,6 +25,16 @@ const (
dohOsHeader = "x-cd-os"
dohClientIDPrefHeader = "x-cd-cpref"
headerApplicationDNS = "application/dns-message"
// dohMaxResponseSize caps the response body read from a DoH/DoH3
// upstream. A DNS message is bounded by the protocol's 16-bit length
// field; anything larger cannot be a valid response. The cap stops a
// malicious or compromised upstream from driving ctrld into unbounded
// memory growth via io.ReadAll on attacker-controlled bytes.
dohMaxResponseSize = dns.MaxMsgSize
// dohMaxErrorBodySize bounds how much of a non-200 response body is
// read for inclusion in the returned error.
dohMaxErrorBodySize = 1024
)
// EncodeOsNameMap provides mapping from OS name to a shorter string, used for encoding x-cd-os value.
@@ -130,13 +140,17 @@ func (r *dohResolver) Resolve(ctx context.Context, msg *dns.Msg) (*dns.Msg, erro
}
defer resp.Body.Close()
buf, err := io.ReadAll(resp.Body)
if resp.StatusCode != http.StatusOK {
body, _ := io.ReadAll(io.LimitReader(resp.Body, dohMaxErrorBodySize))
return nil, fmt.Errorf("wrong response from DOH server, got: %s, status: %d", string(body), resp.StatusCode)
}
buf, err := io.ReadAll(io.LimitReader(resp.Body, dohMaxResponseSize+1))
if err != nil {
return nil, fmt.Errorf("could not read message from response: %w", err)
}
if resp.StatusCode != http.StatusOK {
return nil, fmt.Errorf("wrong response from DOH server, got: %s, status: %d", string(buf), resp.StatusCode)
if len(buf) > dohMaxResponseSize {
return nil, fmt.Errorf("DoH response exceeds %d-byte maximum DNS message size", dohMaxResponseSize)
}
answer := new(dns.Msg)
+214
View File
@@ -196,6 +196,7 @@ func testTLSServer(t *testing.T, handler http.Handler) (*httptest.Server, *x509.
server := httptest.NewUnstartedServer(handler)
server.TLS = &tls.Config{
Certificates: []tls.Certificate{testCert.tlsCert},
MinVersion: tls.VersionTLS12,
}
server.StartTLS()
@@ -232,6 +233,7 @@ func newTestHTTP3Server(t *testing.T, handler http.Handler) *testHTTP3Server {
tlsConfig := &tls.Config{
Certificates: []tls.Certificate{testCert.tlsCert},
NextProtos: []string{"h3"}, // HTTP/3 protocol identifier
MinVersion: tls.VersionTLS12,
}
// Create HTTP/3 server
@@ -264,3 +266,215 @@ func newTestHTTP3Server(t *testing.T, handler http.Handler) *testHTTP3Server {
return h3Server
}
// blockingBodyHandler writes exactly nbytes of body with the given status,
// flushes them, then blocks until release is closed WITHOUT ever returning.
// Because the handler does not return, the response stream is never terminated
// (no EOF/FIN). A client that stops after a bounded prefix therefore completes,
// while a client that reads to EOF blocks. Tests set nbytes to the exact read
// cap so the client consumes the whole written body (no half-written frame is
// left blocking on flow control) yet still never sees EOF.
func blockingBodyHandler(status, nbytes int, release <-chan struct{}) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", headerApplicationDNS)
w.WriteHeader(status)
if _, err := w.Write(make([]byte, nbytes)); err != nil {
return
}
if f, ok := w.(http.Flusher); ok {
f.Flush()
}
<-release
}
}
// requireBoundedResolve asserts that r.Resolve returns the expected size/status
// error while the server is still withholding EOF (the handler is blocked in
// blockingBodyHandler). Returning under those conditions proves ctrld read only
// a bounded prefix of the body: a resolver that instead read to EOF would block
// on the withheld stream and trip the deadline. This is the deterministic
// regression guard for the issue-312 OOM protections, replacing the earlier
// flaky server-side byte counter (issue-561).
func requireBoundedResolve(t *testing.T, r Resolver, msg *dns.Msg, wantErrSubstr string) {
t.Helper()
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
defer cancel()
type result struct {
answer *dns.Msg
err error
}
done := make(chan result, 1)
go func() {
answer, err := r.Resolve(ctx, msg)
done <- result{answer, err}
}()
select {
case res := <-done:
if res.err == nil {
t.Fatalf("Resolve unexpectedly succeeded; answer=%v", res.answer)
}
if !strings.Contains(res.err.Error(), wantErrSubstr) {
t.Fatalf("error %q does not contain %q", res.err, wantErrSubstr)
}
if res.answer != nil {
t.Fatalf("Resolve returned non-nil answer alongside error: %v", res.answer)
}
case <-time.After(5 * time.Second):
t.Fatal("Resolve did not return while the server withheld EOF: the body is being read to EOF instead of a bounded prefix (issue-312 OOM protection missing)")
}
}
// dohUpstreamForTLSServer wires an UpstreamConfig at a local httptest TLS
// server, trusting its self-signed certificate. BootstrapIP is set so no
// real DNS lookup runs.
func dohUpstreamForTLSServer(t *testing.T, srv *httptest.Server) *UpstreamConfig {
t.Helper()
pool := x509.NewCertPool()
pool.AddCert(srv.Certificate())
u, err := url.Parse(srv.URL)
if err != nil {
t.Fatalf("parse server URL: %v", err)
}
uc := &UpstreamConfig{
Name: "doh-oversize",
Type: ResolverTypeDOH,
Endpoint: srv.URL + "/dns-query",
BootstrapIP: u.Hostname(),
Timeout: 2000,
}
uc.SetCertPool(pool)
uc.Init()
return uc
}
// doh3UpstreamForAddr wires an UpstreamConfig at a local HTTP/3 server,
// trusting its self-signed certificate.
func doh3UpstreamForAddr(t *testing.T, addr string, cert *x509.Certificate) *UpstreamConfig {
t.Helper()
pool := x509.NewCertPool()
pool.AddCert(cert)
host, _, err := net.SplitHostPort(addr)
if err != nil {
t.Fatalf("split host/port %q: %v", addr, err)
}
uc := &UpstreamConfig{
Name: "doh3-oversize",
Type: ResolverTypeDOH3,
Endpoint: "h3://" + addr + "/dns-query",
BootstrapIP: host,
Timeout: 5000,
}
uc.SetCertPool(pool)
uc.Init()
return uc
}
// TestDoHResolve_OversizedBody_Rejected locks in the fix for
// github.com/Control-D-Inc/ctrld/issues/312: a malicious DoH upstream
// returning a body larger than the DNS protocol allows must be rejected
// with an explicit size error rather than buffered into ctrld memory.
func TestDoHResolve_OversizedBody_Rejected(t *testing.T) {
// Write exactly the LimitReader cap, then withhold EOF. ctrld's bounded
// read (io.LimitReader of dohMaxResponseSize+1) returns after this prefix;
// an unbounded read would block on the missing EOF and trip the deadline.
release := make(chan struct{})
defer close(release)
srv := httptest.NewUnstartedServer(blockingBodyHandler(http.StatusOK, dohMaxResponseSize+1, release))
testCert := generateTestCertificate(t)
srv.TLS = &tls.Config{
Certificates: []tls.Certificate{testCert.tlsCert},
NextProtos: []string{"h2", "http/1.1"},
MinVersion: tls.VersionTLS12,
}
srv.StartTLS()
t.Cleanup(srv.Close)
uc := dohUpstreamForTLSServer(t, srv)
r, err := NewResolver(uc)
if err != nil {
t.Fatalf("NewResolver: %v", err)
}
msg := new(dns.Msg)
msg.SetQuestion("example.com.", dns.TypeA)
msg.RecursionDesired = true
requireBoundedResolve(t, r, msg, "maximum DNS message size")
}
// TestDoHResolve_NonOKStatus_BoundedErrorBody locks in that a non-200
// response with a huge body does not pull the body fully into ctrld
// memory just to format an error string.
func TestDoHResolve_NonOKStatus_BoundedErrorBody(t *testing.T) {
// Same synchronization as the oversized-body test, but at the error-body
// cap: the non-200 path reads through an io.LimitReader of
// dohMaxErrorBodySize, so it must return after this prefix without EOF.
release := make(chan struct{})
defer close(release)
srv := httptest.NewUnstartedServer(blockingBodyHandler(http.StatusBadGateway, dohMaxErrorBodySize, release))
testCert := generateTestCertificate(t)
srv.TLS = &tls.Config{
Certificates: []tls.Certificate{testCert.tlsCert},
NextProtos: []string{"h2", "http/1.1"},
MinVersion: tls.VersionTLS12,
}
srv.StartTLS()
t.Cleanup(srv.Close)
uc := dohUpstreamForTLSServer(t, srv)
r, err := NewResolver(uc)
if err != nil {
t.Fatalf("NewResolver: %v", err)
}
msg := new(dns.Msg)
msg.SetQuestion("example.com.", dns.TypeA)
msg.RecursionDesired = true
requireBoundedResolve(t, r, msg, "status: 502")
}
// TestDoHResolve_OversizedBody_DoH3 mirrors the DoH oversized-body check
// on the HTTP/3 transport, since github-312 specifically reproduced the
// OOM via DoH3.
func TestDoHResolve_OversizedBody_DoH3(t *testing.T) {
release := make(chan struct{})
defer close(release)
testCert := generateTestCertificate(t)
udpConn, err := net.ListenUDP("udp", &net.UDPAddr{IP: net.ParseIP("127.0.0.1"), Port: 0})
if err != nil {
t.Fatalf("udp listen: %v", err)
}
h3 := &http3.Server{
Handler: blockingBodyHandler(http.StatusOK, dohMaxResponseSize+1, release),
TLSConfig: &tls.Config{
Certificates: []tls.Certificate{testCert.tlsCert},
NextProtos: []string{"h3"},
MinVersion: tls.VersionTLS12,
},
}
go func() {
if err := h3.Serve(udpConn); err != nil && !errors.Is(err, http.ErrServerClosed) {
t.Logf("h3 server: %v", err)
}
}()
t.Cleanup(func() {
_ = h3.Close()
_ = udpConn.Close()
})
time.Sleep(100 * time.Millisecond)
uc := doh3UpstreamForAddr(t, udpConn.LocalAddr().String(), testCert.cert)
r, err := NewResolver(uc)
if err != nil {
t.Fatalf("NewResolver: %v", err)
}
msg := new(dns.Msg)
msg.SetQuestion("example.com.", dns.TypeA)
msg.RecursionDesired = true
requireBoundedResolve(t, r, msg, "maximum DNS message size")
}
+127 -24
View File
@@ -6,15 +6,23 @@ import (
"context"
"crypto/tls"
"errors"
"fmt"
"io"
"net"
"runtime"
"sync"
"time"
"github.com/miekg/dns"
"github.com/quic-go/quic-go"
)
// doqMaxResponseSize caps the bytes read from a DoQ stream: a 2-byte
// length prefix plus a DNS message bounded by dns.MaxMsgSize. Anything
// larger cannot be a valid response and is rejected before buffering more
// data from the upstream.
const doqMaxResponseSize = 2 + dns.MaxMsgSize
type doqResolver struct {
uc *UpstreamConfig
}
@@ -41,6 +49,10 @@ func (r *doqResolver) Resolve(ctx context.Context, msg *dns.Msg) (*dns.Msg, erro
const doqPoolSize = 16
// doqConnPool manages a pool of QUIC connections for DoQ queries using a buffered channel.
// A single quic.Transport (and its UDP socket) is shared by every connection in the pool,
// so the OS socket lifecycle is tied to the pool rather than to each dial. Without this
// ownership model, a strict DoQ upstream that triggers reconnect churn would leak one
// caller-owned UDP socket per dial — see github.com/Control-D-Inc/ctrld/issues/309.
type doqConnPool struct {
uc *UpstreamConfig
addrs []string
@@ -48,6 +60,13 @@ type doqConnPool struct {
tlsConfig *tls.Config
quicConfig *quic.Config
conns chan *doqConn
transportMu sync.Mutex
transport *quic.Transport
transportConn *net.UDPConn
transportErr error
transportInit bool
closed bool
}
type doqConn struct {
@@ -64,6 +83,7 @@ func newDOQConnPool(uc *UpstreamConfig, addrs []string) *doqConnPool {
NextProtos: []string{"doq"},
RootCAs: uc.certPool,
ServerName: uc.Domain,
MinVersion: tls.VersionTLS12,
}
quicConfig := &quic.Config{
@@ -167,29 +187,70 @@ func (p *doqConnPool) doResolve(ctx context.Context, msg *dns.Msg) (*dns.Msg, er
return nil, err
}
// Read response
buf, err := io.ReadAll(stream)
stream.Close()
// Return connection to pool (mark as potentially bad if error occurred)
isGood := err == nil && len(buf) > 0
p.putConn(conn, isGood)
if err != nil {
// RFC 9250 section 4.2 requires the client to indicate end-of-request by
// closing the send side of the stream (STREAM FIN). Servers may defer
// processing until FIN arrives, so the close must happen before reading.
// Stream.Close closes only the send direction; the receive direction
// remains open for the response.
if err := stream.Close(); err != nil {
p.putConn(conn, false)
return nil, err
}
// io.ReadAll hides io.EOF error, so check for empty buffer
// A DoQ response is a 2-byte length prefix followed by a DNS message.
// The DNS message is bounded by the protocol at dns.MaxMsgSize, so a
// well-formed response is at most doqMaxResponseSize bytes. Read one
// byte past that cap to distinguish "at limit" from "over limit" and
// reject oversized responses before they can drive memory growth from
// a malicious or compromised upstream.
buf, err := io.ReadAll(io.LimitReader(stream, doqMaxResponseSize+1))
if err != nil {
p.putConn(conn, false)
return nil, err
}
// io.ReadAll hides io.EOF error, so check for empty buffer.
if len(buf) == 0 {
p.putConn(conn, false)
return nil, io.EOF
}
// Unpack DNS response (skip 2-byte length prefix)
if len(buf) > doqMaxResponseSize {
p.putConn(conn, false)
return nil, fmt.Errorf("DoQ response exceeds %d-byte maximum", doqMaxResponseSize)
}
// RFC 9250: each DoQ DNS message is encoded as a 2-octet length field
// followed by the DNS message. Reject responses that are shorter than
// the prefix or whose prefix declares more bytes than were received,
// and retire the misbehaving connection. Without this guard, buf[2:]
// would panic when len(buf) < 2.
if len(buf) < 2 {
p.putConn(conn, false)
return nil, fmt.Errorf("malformed DoQ response: %d byte(s), need >= 2 for length prefix", len(buf))
}
respLen := int(buf[0])<<8 | int(buf[1])
if 2+respLen > len(buf) {
p.putConn(conn, false)
return nil, fmt.Errorf("malformed DoQ response: length prefix %d exceeds payload %d", respLen, len(buf)-2)
}
p.putConn(conn, true)
// Unpack DNS response (skip 2-byte length prefix).
answer := new(dns.Msg)
if err := answer.Unpack(buf[2:]); err != nil {
if err := answer.Unpack(buf[2 : 2+respLen]); err != nil {
return nil, err
}
answer.SetReply(msg)
// RFC 9250 section 4.2.1 requires the DNS Message ID to be 0 on the wire,
// so restore the downstream transaction ID for the client. Do NOT use
// SetReply here: it rewrites the RCODE to NOERROR and overwrites the
// Question with the request's, which would mask upstream failures from the
// failover logic (a SERVFAIL would look like success) and let a
// wrong-question answer pass validation and poison the cache. Preserve the
// upstream RCODE, Question, and answer sections untouched so the proxy can
// evaluate them. See github.com/Control-D-Inc/ctrld/issues/322.
answer.Id = msg.Id
return answer, nil
}
@@ -233,25 +294,26 @@ func (p *doqConnPool) putConn(conn *quic.Conn, isGood bool) {
}
// dialConn creates a new QUIC connection using parallel dialing like DoH3.
// All connections from the pool multiplex on a single pool-owned UDP socket,
// so reconnect churn cannot grow the host's FD count.
func (p *doqConnPool) dialConn(ctx context.Context) (string, *quic.Conn, error) {
logger := ProxyLogger.Load()
tr, err := p.getOrInitTransport()
if err != nil {
return "", nil, err
}
// If we have a bootstrap IP, use it directly
if p.uc.BootstrapIP != "" {
addr := net.JoinHostPort(p.uc.BootstrapIP, p.port)
Log(ctx, logger.Debug(), "Sending DoQ request to: %s", addr)
udpConn, err := net.ListenUDP("udp", nil)
if err != nil {
return "", nil, err
}
remoteAddr, err := net.ResolveUDPAddr("udp", addr)
if err != nil {
udpConn.Close()
return "", nil, err
}
conn, err := quic.DialEarly(ctx, udpConn, remoteAddr, p.tlsConfig, p.quicConfig)
conn, err := tr.DialEarly(ctx, remoteAddr, p.tlsConfig, p.quicConfig)
if err != nil {
udpConn.Close()
return "", nil, err
}
return addr, conn, nil
@@ -263,7 +325,7 @@ func (p *doqConnPool) dialConn(ctx context.Context) (string, *quic.Conn, error)
dialAddrs[i] = net.JoinHostPort(p.addrs[i], p.port)
}
pd := &quicParallelDialer{}
pd := &quicParallelDialer{transport: tr}
conn, err := pd.Dial(ctx, dialAddrs, p.tlsConfig, p.quicConfig)
if err != nil {
return "", nil, err
@@ -274,9 +336,35 @@ func (p *doqConnPool) dialConn(ctx context.Context) (string, *quic.Conn, error)
return addr, conn, nil
}
// CloseIdleConnections closes all connections in the pool.
// Connections currently checked out (in use) are not closed.
// getOrInitTransport returns the pool's shared quic.Transport, initialising it
// on first call. Once the pool has been closed it permanently returns an error
// so that callers cannot resurrect a dead pool.
func (p *doqConnPool) getOrInitTransport() (*quic.Transport, error) {
p.transportMu.Lock()
defer p.transportMu.Unlock()
if p.closed {
return nil, errors.New("doq pool closed")
}
if p.transportInit {
return p.transport, p.transportErr
}
p.transportInit = true
udpConn, err := net.ListenUDP("udp", nil)
if err != nil {
p.transportErr = err
return nil, err
}
p.transportConn = udpConn
p.transport = &quic.Transport{Conn: udpConn}
return p.transport, nil
}
// CloseIdleConnections closes all idle connections, the shared quic.Transport,
// and the pool's UDP socket. Connections currently checked out (in use) get
// terminated by the transport close as well — without that, the OS socket
// would remain bound to a goroutine that the caller cannot reach to clean up.
func (p *doqConnPool) CloseIdleConnections() {
drain:
for {
select {
case dc := <-p.conns:
@@ -284,7 +372,22 @@ func (p *doqConnPool) CloseIdleConnections() {
dc.conn.CloseWithError(quic.ApplicationErrorCode(quic.NoError), "")
}
default:
return
break drain
}
}
p.transportMu.Lock()
if p.closed {
p.transportMu.Unlock()
return
}
p.closed = true
tr := p.transport
udpConn := p.transportConn
p.transportMu.Unlock()
if tr != nil {
_ = tr.Close()
}
if udpConn != nil {
_ = udpConn.Close()
}
}
+596 -1
View File
@@ -1,4 +1,3 @@
// test_helpers.go
package ctrld
import (
@@ -8,8 +7,11 @@ import (
"crypto/tls"
"crypto/x509"
"crypto/x509/pkix"
"io"
"math/big"
"net"
"os"
"runtime"
"strings"
"testing"
"time"
@@ -99,6 +101,7 @@ func newTestQUICServer(t *testing.T) *testQUICServer {
tlsConfig := &tls.Config{
Certificates: []tls.Certificate{testCert.tlsCert},
NextProtos: []string{"doq"},
MinVersion: tls.VersionTLS12,
}
// Create QUIC listener
@@ -221,3 +224,595 @@ func (s *testQUICServer) handleStream(t *testing.T, stream *quic.Stream) {
return
}
}
// malformedDoQServer is a test QUIC server that drains the client's DoQ
// request and writes caller-supplied raw bytes back. The bytes are not
// required to be a well-framed DoQ response, which is what lets the
// regression tests exercise malformed-response handling.
type malformedDoQServer struct {
listener *quic.Listener
cert *x509.Certificate
addr string
response []byte
}
func newMalformedDoQServer(t *testing.T, response []byte) *malformedDoQServer {
t.Helper()
testCert := generateTestCertificate(t)
tlsConfig := &tls.Config{
Certificates: []tls.Certificate{testCert.tlsCert},
NextProtos: []string{"doq"},
}
listener, err := quic.ListenAddr("127.0.0.1:0", tlsConfig, nil)
if err != nil {
t.Fatalf("failed to create QUIC listener: %v", err)
}
s := &malformedDoQServer{
listener: listener,
cert: testCert.cert,
addr: listener.Addr().String(),
response: response,
}
go s.serve()
t.Cleanup(func() { _ = listener.Close() })
return s
}
func (s *malformedDoQServer) serve() {
for {
conn, err := s.listener.Accept(context.Background())
if err != nil {
return
}
go s.handleConn(conn)
}
}
func (s *malformedDoQServer) handleConn(conn *quic.Conn) {
for {
stream, err := conn.AcceptStream(context.Background())
if err != nil {
return
}
go s.handleStream(stream)
}
}
func (s *malformedDoQServer) handleStream(stream *quic.Stream) {
defer stream.Close()
// Drain the client's DoQ-framed request so the client's writes complete
// cleanly before we reply with our attacker-controlled bytes. Using
// io.ReadFull because a single Read on a QUIC stream may return short.
lenBuf := make([]byte, 2)
if _, err := io.ReadFull(stream, lenBuf); err != nil {
return
}
msgLen := uint16(lenBuf[0])<<8 | uint16(lenBuf[1])
if msgLen > 0 {
discard := make([]byte, msgLen)
if _, err := io.ReadFull(stream, discard); err != nil {
return
}
}
if len(s.response) > 0 {
_, _ = stream.Write(s.response)
}
}
// newMalformedDoQUpstream builds an UpstreamConfig wired to a local
// malformed test server with the test certificate trusted via a custom
// cert pool. We bypass SetupBootstrapIP by setting BootstrapIP directly,
// so the pool dials 127.0.0.1 without any DNS lookup.
func newMalformedDoQUpstream(t *testing.T, cert *x509.Certificate, addr string) *UpstreamConfig {
t.Helper()
pool := x509.NewCertPool()
pool.AddCert(cert)
host, _, err := net.SplitHostPort(addr)
if err != nil {
t.Fatalf("split host/port %q: %v", addr, err)
}
uc := &UpstreamConfig{
Name: "doq-malformed",
Type: ResolverTypeDOQ,
Endpoint: addr,
Domain: host,
BootstrapIP: host,
Timeout: 2000,
}
uc.SetCertPool(pool)
return uc
}
// TestDoQResolve_MalformedResponse verifies that DoQ upstream
// responses violating RFC 9250 framing — fewer than 2 bytes, or a
// length prefix declaring more payload than was received — return a
// handled error instead of panicking on the length-prefix slice.
func TestDoQResolve_MalformedResponse(t *testing.T) {
tests := []struct {
name string
response []byte
}{
// Empty stream is already handled via io.EOF; locked in so a
// future change that drops that branch is caught.
{"empty response", nil},
// One byte: too short to hold the 2-octet length prefix.
{"single byte response", []byte{0x00}},
// Length prefix declares 16 bytes; payload is absent.
{"length prefix only", []byte{0x00, 0x10}},
// Length prefix declares 65535 bytes; only 1 byte of payload
// arrived.
{"length prefix larger than payload", []byte{0xFF, 0xFF, 0x00}},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
server := newMalformedDoQServer(t, tt.response)
uc := newMalformedDoQUpstream(t, server.cert, server.addr)
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
defer cancel()
pool := newDOQConnPool(uc, []string{"127.0.0.1"})
t.Cleanup(pool.CloseIdleConnections)
msg := new(dns.Msg)
msg.SetQuestion("example.com.", dns.TypeA)
msg.RecursionDesired = true
answer, err := pool.Resolve(ctx, msg)
if err == nil {
t.Fatalf("Resolve unexpectedly succeeded for malformed response %v; answer=%v", tt.response, answer)
}
if answer != nil {
t.Fatalf("Resolve returned non-nil answer alongside error: answer=%v err=%v", answer, err)
}
})
}
}
// strictDoQServer accepts DoQ queries but defers the response until the
// client signals end-of-request with STREAM FIN, as required by RFC 9250
// section 4.2. It exists to lock in the fix for
// github.com/Control-D-Inc/ctrld/issues/309 where a client
// that never closes its send side caused the server to wait forever and the
// client to churn through reconnects.
type strictDoQServer struct {
listener *quic.Listener
cert *x509.Certificate
addr string
}
func newStrictDoQServer(t *testing.T) *strictDoQServer {
t.Helper()
testCert := generateTestCertificate(t)
tlsConfig := &tls.Config{
Certificates: []tls.Certificate{testCert.tlsCert},
NextProtos: []string{"doq"},
MinVersion: tls.VersionTLS12,
}
listener, err := quic.ListenAddr("127.0.0.1:0", tlsConfig, nil)
if err != nil {
t.Fatalf("failed to create QUIC listener: %v", err)
}
s := &strictDoQServer{
listener: listener,
cert: testCert.cert,
addr: listener.Addr().String(),
}
go s.serve()
t.Cleanup(func() { _ = listener.Close() })
return s
}
func (s *strictDoQServer) serve() {
for {
conn, err := s.listener.Accept(context.Background())
if err != nil {
return
}
go s.handleConn(conn)
}
}
func (s *strictDoQServer) handleConn(conn *quic.Conn) {
for {
stream, err := conn.AcceptStream(context.Background())
if err != nil {
return
}
go s.handleStream(stream)
}
}
func (s *strictDoQServer) handleStream(stream *quic.Stream) {
defer stream.Close()
// Drain until the client closes the send side. This is the behaviour
// that triggered the bug: if the client never sends STREAM FIN, this
// read blocks until the stream's deadline fires.
body, err := io.ReadAll(stream)
if err != nil {
return
}
if len(body) < 2 {
return
}
msgLen := uint16(body[0])<<8 | uint16(body[1])
if int(msgLen) != len(body)-2 {
return
}
msg := new(dns.Msg)
if err := msg.Unpack(body[2:]); err != nil {
return
}
response := new(dns.Msg)
response.SetReply(msg)
response.Authoritative = true
if len(msg.Question) > 0 && msg.Question[0].Qtype == dns.TypeA {
response.Answer = append(response.Answer, &dns.A{
Hdr: dns.RR_Header{
Name: msg.Question[0].Name,
Rrtype: dns.TypeA,
Class: dns.ClassINET,
Ttl: 300,
},
A: net.ParseIP("192.0.2.1"),
})
}
respBytes, err := response.Pack()
if err != nil {
return
}
respLen := uint16(len(respBytes))
if _, err := stream.Write([]byte{byte(respLen >> 8), byte(respLen & 0xFF)}); err != nil {
return
}
if _, err := stream.Write(respBytes); err != nil {
return
}
}
func newStrictDoQUpstream(t *testing.T, cert *x509.Certificate, addr string, useBootstrap bool) *UpstreamConfig {
t.Helper()
pool := x509.NewCertPool()
pool.AddCert(cert)
host, _, err := net.SplitHostPort(addr)
if err != nil {
t.Fatalf("split host/port %q: %v", addr, err)
}
uc := &UpstreamConfig{
Name: "doq-strict",
Type: ResolverTypeDOQ,
Endpoint: addr,
Domain: host,
Timeout: 3000,
}
if useBootstrap {
uc.BootstrapIP = host
}
uc.SetCertPool(pool)
return uc
}
// TestDoQResolve_StrictServerWaitsForFIN exercises the RFC 9250 client-FIN
// requirement. With the bug present, the server's io.ReadAll blocks until
// the stream deadline expires and the client sees a timeout, so a successful
// resolve here proves that the client now sends STREAM FIN before reading.
func TestDoQResolve_StrictServerWaitsForFIN(t *testing.T) {
t.Parallel()
server := newStrictDoQServer(t)
uc := newStrictDoQUpstream(t, server.cert, server.addr, true)
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
defer cancel()
host, _, _ := net.SplitHostPort(server.addr)
pool := newDOQConnPool(uc, []string{host})
t.Cleanup(pool.CloseIdleConnections)
msg := new(dns.Msg)
msg.SetQuestion("example.com.", dns.TypeA)
msg.RecursionDesired = true
answer, err := pool.Resolve(ctx, msg)
if err != nil {
t.Fatalf("Resolve failed against strict DoQ server: %v", err)
}
if answer == nil || len(answer.Answer) == 0 {
t.Fatalf("Resolve returned no answer records: %+v", answer)
}
a, ok := answer.Answer[0].(*dns.A)
if !ok || !a.A.Equal(net.ParseIP("192.0.2.1")) {
t.Fatalf("unexpected answer: %+v", answer.Answer[0])
}
}
// TestDoQResolve_ParallelDialPathStrictFIN exercises the parallel-dial path
// (no BootstrapIP) against the same FIN-strict server, so that both the
// single-dial branch and the parallel-dial branch are covered.
func TestDoQResolve_ParallelDialPathStrictFIN(t *testing.T) {
t.Parallel()
server := newStrictDoQServer(t)
uc := newStrictDoQUpstream(t, server.cert, server.addr, false)
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
defer cancel()
host, _, _ := net.SplitHostPort(server.addr)
pool := newDOQConnPool(uc, []string{host})
t.Cleanup(pool.CloseIdleConnections)
msg := new(dns.Msg)
msg.SetQuestion("example.com.", dns.TypeA)
msg.RecursionDesired = true
answer, err := pool.Resolve(ctx, msg)
if err != nil {
t.Fatalf("Resolve (parallel-dial path) failed against strict DoQ server: %v", err)
}
if answer == nil || len(answer.Answer) == 0 {
t.Fatalf("Resolve (parallel-dial path) returned no answer records: %+v", answer)
}
}
// TestDoQPool_ChurnDoesNotGrowFDs exercises the reconnect-churn scenario
// described in github.com/Control-D-Inc/ctrld/issues/309: repeated dials
// against a server that closes existing connections must not grow the process
// FD count, because the pool now shares one UDP socket via quic.Transport instead
// of allocating one per dial. Linux-only because /proc/self/fd is the cheapest
// portable proxy for "what's still open."
func TestDoQPool_ChurnDoesNotGrowFDs(t *testing.T) {
if runtime.GOOS != "linux" {
t.Skip("FD accounting via /proc/self/fd is linux-only")
}
t.Parallel()
server := newStrictDoQServer(t)
uc := newStrictDoQUpstream(t, server.cert, server.addr, true)
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()
host, _, _ := net.SplitHostPort(server.addr)
pool := newDOQConnPool(uc, []string{host})
t.Cleanup(pool.CloseIdleConnections)
makeQuery := func(i int) *dns.Msg {
msg := new(dns.Msg)
// Vary the question so any caching layer cannot short-circuit.
msg.SetQuestion(dns.Fqdn(strings.Repeat("a", 1+i%8)+".example.com"), dns.TypeA)
msg.RecursionDesired = true
return msg
}
// Warm the pool so the steady-state transport and at least one
// connection are open. Without this, the first resolve in the measured
// loop would inflate the baseline.
if _, err := pool.Resolve(ctx, makeQuery(0)); err != nil {
t.Fatalf("warm-up Resolve failed: %v", err)
}
baseline := countOpenFDs(t)
// Force reconnect churn by closing the connection between each query.
// Without the fix this would leak one UDP socket per round; with the
// fix the pool's shared transport keeps a single socket open.
const rounds = 20
for i := 1; i <= rounds; i++ {
// Drain any pooled connection so the next Resolve has to redial.
drainPooledConns(pool)
if _, err := pool.Resolve(ctx, makeQuery(i)); err != nil {
t.Fatalf("Resolve in churn loop iteration %d failed: %v", i, err)
}
}
// Give quic-go a moment to drop any background goroutines that hold
// references to closed sockets.
time.Sleep(200 * time.Millisecond)
after := countOpenFDs(t)
// Allow a small slack for transient FDs (goroutine wake-ups, qlog,
// etc.) but reject anything that scales with the number of rounds.
const slack = 5
if after > baseline+slack {
t.Fatalf("FD count grew under DoQ churn: baseline=%d after=%d rounds=%d (slack=%d)", baseline, after, rounds, slack)
}
}
// drainPooledConns removes any idle pooled connections so the next Resolve
// is forced to dial a fresh one. It does not close the pool's transport.
func drainPooledConns(p *doqConnPool) {
for {
select {
case dc := <-p.conns:
if dc.conn != nil {
dc.conn.CloseWithError(quic.ApplicationErrorCode(quic.NoError), "")
}
default:
return
}
}
}
func countOpenFDs(t *testing.T) int {
t.Helper()
entries, err := os.ReadDir("/proc/self/fd")
if err != nil {
t.Fatalf("read /proc/self/fd: %v", err)
}
return len(entries)
}
// TestDoQResolve_OversizedResponse_Rejected locks in the fix for
// github.com/Control-D-Inc/ctrld/issues/312 on the DoQ transport: a
// malicious upstream that writes a response larger than the DNS protocol
// allows must be rejected with an explicit size error, not buffered
// without bound into ctrld memory.
func TestDoQResolve_OversizedResponse_Rejected(t *testing.T) {
t.Parallel()
// doqMaxResponseSize is 2 + dns.MaxMsgSize. Send something well past
// that. 256 KiB is enough to exceed the cap while keeping the test
// fast on loopback.
response := make([]byte, 256*1024)
// A well-formed length prefix isn't required: the size cap should
// fire before any framing check runs. Use a non-zero prefix so the
// test also documents that the order of validation is "size first,
// framing later."
response[0] = 0xFF
response[1] = 0xFF
server := newMalformedDoQServer(t, response)
uc := newMalformedDoQUpstream(t, server.cert, server.addr)
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
defer cancel()
pool := newDOQConnPool(uc, []string{"127.0.0.1"})
t.Cleanup(pool.CloseIdleConnections)
msg := new(dns.Msg)
msg.SetQuestion("example.com.", dns.TypeA)
msg.RecursionDesired = true
answer, err := pool.Resolve(ctx, msg)
if err == nil {
t.Fatalf("Resolve unexpectedly succeeded for oversized response; answer=%v", answer)
}
if !strings.Contains(err.Error(), "exceeds") {
t.Fatalf("error %q does not surface the size cap", err)
}
if answer != nil {
t.Fatalf("Resolve returned non-nil answer alongside error: %v", answer)
}
}
// frameDoQResponse packs msg and prepends the RFC 9250 2-octet length prefix,
// producing the exact bytes a DoQ server writes on the wire.
func frameDoQResponse(t *testing.T, msg *dns.Msg) []byte {
t.Helper()
b, err := msg.Pack()
if err != nil {
t.Fatalf("pack response: %v", err)
}
n := uint16(len(b))
return append([]byte{byte(n >> 8), byte(n & 0xFF)}, b...)
}
// TestDoQResolve_PreservesRcode locks in the fix for
// github.com/Control-D-Inc/ctrld/issues/322: the DoQ resolver must not rewrite
// an upstream response with SetReply, which would clobber a SERVFAIL into
// NOERROR and hide the failure from the proxy's failover logic. The upstream
// RCODE must survive; only the transaction ID is restored for the client.
func TestDoQResolve_PreservesRcode(t *testing.T) {
t.Parallel()
// RFC 9250 puts the DNS Message ID at 0 on the wire.
resp := new(dns.Msg)
resp.SetQuestion("example.com.", dns.TypeA)
resp.Response = true
resp.Id = 0
resp.Rcode = dns.RcodeServerFailure
server := newMalformedDoQServer(t, frameDoQResponse(t, resp))
uc := newMalformedDoQUpstream(t, server.cert, server.addr)
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
defer cancel()
pool := newDOQConnPool(uc, []string{"127.0.0.1"})
t.Cleanup(pool.CloseIdleConnections)
msg := new(dns.Msg)
msg.SetQuestion("example.com.", dns.TypeA)
msg.RecursionDesired = true
answer, err := pool.Resolve(ctx, msg)
if err != nil {
t.Fatalf("Resolve failed: %v", err)
}
if answer.Rcode != dns.RcodeServerFailure {
t.Fatalf("upstream SERVFAIL was rewritten to %s; failover would be bypassed",
dns.RcodeToString[answer.Rcode])
}
if answer.Id != msg.Id {
t.Fatalf("transaction ID not restored: got %d, want %d", answer.Id, msg.Id)
}
}
// TestDoQResolve_PreservesWrongQuestion locks in the fix for
// github.com/Control-D-Inc/ctrld/issues/322: when an upstream answers a
// different name than asked, the resolver must preserve the upstream's
// question rather than rewriting it to the request's question (as SetReply
// did). Rewriting would hide the mismatch and let wrong-domain records poison
// the shared cache.
func TestDoQResolve_PreservesWrongQuestion(t *testing.T) {
t.Parallel()
resp := new(dns.Msg)
resp.SetQuestion("attacker.example.", dns.TypeA)
resp.Response = true
resp.Id = 0
resp.Answer = append(resp.Answer, &dns.A{
Hdr: dns.RR_Header{
Name: "attacker.example.",
Rrtype: dns.TypeA,
Class: dns.ClassINET,
Ttl: 300,
},
A: net.ParseIP("192.0.2.1"),
})
server := newMalformedDoQServer(t, frameDoQResponse(t, resp))
uc := newMalformedDoQUpstream(t, server.cert, server.addr)
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
defer cancel()
pool := newDOQConnPool(uc, []string{"127.0.0.1"})
t.Cleanup(pool.CloseIdleConnections)
msg := new(dns.Msg)
msg.SetQuestion("victim.example.", dns.TypeA)
msg.RecursionDesired = true
answer, err := pool.Resolve(ctx, msg)
if err != nil {
t.Fatalf("Resolve failed: %v", err)
}
if len(answer.Question) == 0 || !strings.EqualFold(answer.Question[0].Name, "attacker.example.") {
t.Fatalf("upstream question was rewritten; got %v, want the upstream's attacker.example.",
answer.Question)
}
if answer.Id != msg.Id {
t.Fatalf("transaction ID not restored: got %d, want %d", answer.Id, msg.Id)
}
}
+2 -1
View File
@@ -64,7 +64,8 @@ func newDOTClientPool(uc *UpstreamConfig, addrs []string) *dotConnPool {
dialer := newDialer(net.JoinHostPort(controldPublicDns, "53"))
tlsConfig := &tls.Config{
RootCAs: uc.certPool,
RootCAs: uc.certPool,
MinVersion: tls.VersionTLS12,
}
if uc.BootstrapIP != "" {
+8 -8
View File
@@ -29,16 +29,16 @@ require (
github.com/prometheus/client_golang v1.19.1
github.com/prometheus/client_model v0.5.0
github.com/prometheus/prom2json v1.3.3
github.com/quic-go/quic-go v0.57.1
github.com/quic-go/quic-go v0.59.1
github.com/rs/zerolog v1.28.0
github.com/spf13/cobra v1.9.1
github.com/spf13/pflag v1.0.6
github.com/spf13/viper v1.16.0
github.com/stretchr/testify v1.11.1
github.com/vishvananda/netlink v1.3.1
golang.org/x/net v0.55.0
golang.org/x/sync v0.20.0
golang.org/x/sys v0.45.0
golang.org/x/net v0.56.0
golang.org/x/sync v0.22.0
golang.org/x/sys v0.46.0
golang.zx2c4.com/wireguard/windows v0.5.3
tailscale.com v1.74.0
)
@@ -92,11 +92,11 @@ require (
github.com/yusufpapurcu/wmi v1.2.4 // indirect
go4.org/mem v0.0.0-20220726221520-4f986261bf13 // indirect
go4.org/netipx v0.0.0-20231129151722-fdeea329fbba // indirect
golang.org/x/crypto v0.51.0 // indirect
golang.org/x/crypto v0.53.0 // indirect
golang.org/x/exp v0.0.0-20240119083558-1b970713d09a // indirect
golang.org/x/mod v0.35.0 // indirect
golang.org/x/text v0.37.0 // indirect
golang.org/x/tools v0.44.0 // indirect
golang.org/x/mod v0.37.0 // indirect
golang.org/x/text v0.40.0 // indirect
golang.org/x/tools v0.47.0 // indirect
google.golang.org/protobuf v1.33.0 // indirect
gopkg.in/ini.v1 v1.67.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
+16 -18
View File
@@ -271,8 +271,8 @@ github.com/prometheus/prom2json v1.3.3 h1:IYfSMiZ7sSOfliBoo89PcufjWO4eAR0gznGcET
github.com/prometheus/prom2json v1.3.3/go.mod h1:Pv4yIPktEkK7btWsrUTWDDDrnpUrAELaOCj+oFwlgmc=
github.com/quic-go/qpack v0.6.0 h1:g7W+BMYynC1LbYLSqRt8PBg5Tgwxn214ZZR34VIOjz8=
github.com/quic-go/qpack v0.6.0/go.mod h1:lUpLKChi8njB4ty2bFLX2x4gzDqXwUpaO1DP9qMDZII=
github.com/quic-go/quic-go v0.57.1 h1:25KAAR9QR8KZrCZRThWMKVAwGoiHIrNbT72ULHTuI10=
github.com/quic-go/quic-go v0.57.1/go.mod h1:ly4QBAjHA2VhdnxhojRsCUOeJwKYg+taDlos92xb1+s=
github.com/quic-go/quic-go v0.59.1 h1:0Gmua0HW1Tv7ANR7hUYwRyD0MG5OJfgvYSZasGZzBic=
github.com/quic-go/quic-go v0.59.1/go.mod h1:upnsH4Ju1YkqpLXC305eW3yDZ4NfnNbmQRCMWS58IKU=
github.com/rivo/uniseg v0.2.0/go.mod h1:J6wj4VEh+S6ZtnVlnTBMWIodfgj8LQOQFoIToxlJtxc=
github.com/rivo/uniseg v0.4.4 h1:8TfxU8dW6PdqD27gjM8MVNuicgxIjxpm4K7x4jp8sis=
github.com/rivo/uniseg v0.4.4/go.mod h1:FN3SvrM+Zdj16jyLfmOkMNblXMcoc8DfTHruCPUcx88=
@@ -349,8 +349,8 @@ golang.org/x/crypto v0.0.0-20210421170649-83a5a9bb288b/go.mod h1:T9bdIzuCu7OtxOm
golang.org/x/crypto v0.0.0-20211209193657-4570a0811e8b/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4=
golang.org/x/crypto v0.0.0-20211215153901-e495a2d5b3d3/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4=
golang.org/x/crypto v0.0.0-20220722155217-630584e8d5aa/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4=
golang.org/x/crypto v0.51.0 h1:IBPXwPfKxY7cWQZ38ZCIRPI50YLeevDLlLnyC5wRGTI=
golang.org/x/crypto v0.51.0/go.mod h1:8AdwkbraGNABw2kOX6YFPs3WM22XqI4EXEd8g+x7Oc8=
golang.org/x/crypto v0.53.0 h1:QZ4Muo8THX6CizN2vPPd5fBGHyogrdK9fG4wLPFUsto=
golang.org/x/crypto v0.53.0/go.mod h1:DNLU434OwVakk9PzuwV8w62mAJpRJL3vsgcfp4Qnsio=
golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
golang.org/x/exp v0.0.0-20190306152737-a1d7652674e8/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
golang.org/x/exp v0.0.0-20190510132918-efd6b22b2522/go.mod h1:ZjyILWgesfNpC6sMxTJOJm9Kp84zZh5NQWvqDGG3Qr8=
@@ -386,8 +386,8 @@ golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
golang.org/x/mod v0.4.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
golang.org/x/mod v0.4.1/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
golang.org/x/mod v0.35.0 h1:Ww1D637e6Pg+Zb2KrWfHQUnH2dQRLBQyAtpr/haaJeM=
golang.org/x/mod v0.35.0/go.mod h1:+GwiRhIInF8wPm+4AoT6L0FA1QWAad3OMdTRx4tFYlU=
golang.org/x/mod v0.37.0 h1:vF1DjpVEshcIqoEaauuHebaLk1O1forxjxBaVn884JQ=
golang.org/x/mod v0.37.0/go.mod h1:m8S8VeM9r4dzDwjrKO0a1sZP3YjeMamRRlD+fmR2Q/0=
golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
golang.org/x/net v0.0.0-20190108225652-1e06a53dbb7e/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
@@ -420,8 +420,8 @@ golang.org/x/net v0.0.0-20201209123823-ac852fbbde11/go.mod h1:m0MpNAwzfU5UDzcl9v
golang.org/x/net v0.0.0-20201224014010-6772e930b67b/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg=
golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg=
golang.org/x/net v0.0.0-20211112202133-69e39bad7dc2/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
golang.org/x/net v0.55.0 h1:bcvxaJn3e1U6InsFWt1JUq1aSjnRxLzT2rtD2KfkDF8=
golang.org/x/net v0.55.0/go.mod h1:L5U2KuzuOe1lY7Z+aWVIKK6qEeJXnXV9yzGA+WCHJww=
golang.org/x/net v0.56.0 h1:Rw8j/hFzGvJUZwNBXnAtf5sVDVt+65SK2C7IxCxZt5o=
golang.org/x/net v0.56.0/go.mod h1:D3Ku6r+V6JROoZK144D2XfMHFcMq/0zSfLelVTCFKec=
golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
@@ -441,8 +441,8 @@ golang.org/x/sync v0.0.0-20200317015054-43a5402ce75a/go.mod h1:RxMgew5VJxzue5/jJ
golang.org/x/sync v0.0.0-20200625203802-6e8e738ad208/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20201207232520-09787c993a3a/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4=
golang.org/x/sync v0.20.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0=
golang.org/x/sync v0.22.0 h1:SZjpbeLmrCk4xhRSZFNZW5gFUeCeFgjekvI/+gfScek=
golang.org/x/sync v0.22.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0=
golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20190312061237-fead79001313/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
@@ -492,8 +492,8 @@ golang.org/x/sys v0.4.1-0.20230131160137-e7d7f63158de/go.mod h1:oPkhp1MJrh7nUepC
golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.10.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.12.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.45.0 h1:dO4czNzziLiiXplLQgBCEpCvXQ3dnkn0SdaZSYdQ+FY=
golang.org/x/sys v0.45.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw=
golang.org/x/sys v0.46.0 h1:noSf2Fq6F8DBgS+LysIkx7rIExoNHJsxOAtPp4rthXw=
golang.org/x/sys v0.46.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw=
golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw=
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
@@ -504,13 +504,11 @@ golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
golang.org/x/text v0.3.4/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ=
golang.org/x/text v0.37.0 h1:Cqjiwd9eSg8e0QAkyCaQTNHFIIzWtidPahFWR83rTrc=
golang.org/x/text v0.37.0/go.mod h1:a5sjxXGs9hsn/AJVwuElvCAo9v8QYLzvavO5z2PiM38=
golang.org/x/text v0.40.0 h1:Ub2Z6/xjgF1WrYQz2nuITOEegKFtiIy+rieRJ5lHZKs=
golang.org/x/text v0.40.0/go.mod h1:hpnzDAfGV753zIKo+wk3u1bVKCGPbrnF7+7LBF/UHVY=
golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
golang.org/x/time v0.0.0-20191024005414-555d28b269f0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
golang.org/x/time v0.12.0 h1:ScB/8o8olJvc+CQPWrK3fPZNfh7qgwCrY0zJmoEQLSE=
golang.org/x/time v0.12.0/go.mod h1:CDIdPxbZBQxdj6cxyCIdrNogrJKMJ7pr37NYpMcMDSg=
golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
golang.org/x/tools v0.0.0-20190114222345-bf090417da8b/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
golang.org/x/tools v0.0.0-20190226205152-f727befe758c/go.mod h1:9Yl7xja0Znq3iFh3HoIrodX9oNMXvdceNzlUR8zjMvY=
@@ -558,8 +556,8 @@ golang.org/x/tools v0.0.0-20201208233053-a543418bbed2/go.mod h1:emZCQorbCU4vsT4f
golang.org/x/tools v0.0.0-20210105154028-b0ab187a4818/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA=
golang.org/x/tools v0.0.0-20210108195828-e2f9c7f1fc8e/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA=
golang.org/x/tools v0.1.0/go.mod h1:xkSsbof2nBLbhDlRMhhhyNLN/zl3eTqcnHD5viDpcZ0=
golang.org/x/tools v0.44.0 h1:UP4ajHPIcuMjT1GqzDWRlalUEoY+uzoZKnhOjbIPD2c=
golang.org/x/tools v0.44.0/go.mod h1:KA0AfVErSdxRZIsOVipbv3rQhVXTnlU6UhKxHd1seDI=
golang.org/x/tools v0.47.0 h1:7Kn5x/d1svx/PzryTsqeoZN4TZwqeH5pGWjefhLi/1Q=
golang.org/x/tools v0.47.0/go.mod h1:dFHnyTvFWY212G+h7ZY4Vsp/K3U4/7W9TyVaAul8uCA=
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
+2 -1
View File
@@ -11,7 +11,8 @@ func TestCACertPool(t *testing.T) {
c := &http.Client{
Transport: &http.Transport{
TLSClientConfig: &tls.Config{
RootCAs: CACertPool(),
RootCAs: CACertPool(),
MinVersion: tls.VersionTLS12,
},
},
Timeout: 2 * time.Second,
+1 -1
View File
@@ -293,7 +293,7 @@ func apiTransport(cdDev bool) *http.Transport {
return dial(ctx, "tcp6", addrsFromPort(apiIpsV6, port))
}
if router.Name() == ddwrt.Name || runtime.GOOS == "android" {
transport.TLSClientConfig = &tls.Config{RootCAs: certs.CACertPool()}
transport.TLSClientConfig = &tls.Config{RootCAs: certs.CACertPool(), MinVersion: tls.VersionTLS12}
}
return transport
}
+60 -1
View File
@@ -1,6 +1,8 @@
package dnscache
import (
"fmt"
"net"
"strings"
"time"
@@ -16,11 +18,17 @@ type Cacher interface {
}
// Key is the caching key for DNS message.
//
// ECS partitions the cache by EDNS Client Subnet so an answer resolved for one
// subnet is never served to a client in a different subnet. Answer records are
// scoped to the network that generated them (RFC 7871 §7.3), so they must not
// be shared across subnets even when the question is otherwise identical.
type Key struct {
Qtype uint16
Qclass uint16
Name string
Upstream string
ECS string
}
type Value struct {
@@ -60,7 +68,58 @@ func NewLRUCache(size int) (*LRUCache, error) {
// NewKey creates a new cache key for given DNS message.
func NewKey(msg *dns.Msg, upstream string) Key {
q := msg.Question[0]
return Key{Qtype: q.Qtype, Qclass: q.Qclass, Name: normalizeQname(q.Name), Upstream: upstream}
return Key{Qtype: q.Qtype, Qclass: q.Qclass, Name: normalizeQname(q.Name), Upstream: upstream, ECS: CanonicalECS(msg)}
}
// CanonicalECS returns a canonical string form of the EDNS Client Subnet (ECS,
// EDNS option 8) carried by msg, suitable for partitioning cache and
// singleflight keys. A request with no ECS option returns "", so all ECS-less
// queries share one partition and behave as before.
//
// A request that DOES carry an ECS option is never mapped to "", even at SOURCE
// PREFIX-LENGTH 0: the /0 query is forwarded with an ECS option, may draw
// different upstream data than an ECS-less query, and its answer (with the
// echoed option) must not be served to a client that sent no ECS — RFC 7871
// §7.3.1 requires /0-cached data to remain distinguishable. The family is part
// of the token, so an IPv4 /0 and an IPv6 /0 stay distinct too.
//
// The address is masked to its source prefix length so only the significant
// subnet bits contribute to the key: two clients in the same subnet share a
// partition, while different subnets (or address families) never do. The
// response-only SCOPE PREFIX-LENGTH is deliberately excluded — it is not part of
// what the client asked for.
func CanonicalECS(msg *dns.Msg) string {
opt := msg.IsEdns0()
if opt == nil {
return ""
}
for _, o := range opt.Option {
e, ok := o.(*dns.EDNS0_SUBNET)
if !ok {
continue
}
bits := int(e.SourceNetmask)
total := 128
zero := net.IPv6zero
if e.Family == 1 {
total = 32
zero = net.IPv4zero
}
addr := e.Address
if len(addr) == 0 {
// A /0 request commonly carries an empty address; normalize it to
// the family zero so its token is stable regardless of encoding.
addr = zero
}
masked := addr
if bits <= total {
if m := addr.Mask(net.CIDRMask(bits, total)); m != nil {
masked = m
}
}
return fmt.Sprintf("%d/%d/%s", e.Family, e.SourceNetmask, masked.String())
}
return ""
}
// NewValue creates a new cache value for given DNS message.
+186
View File
@@ -0,0 +1,186 @@
package dnscache
import (
"net"
"testing"
"time"
"github.com/miekg/dns"
)
// msgWithECS builds an A query for name carrying an EDNS Client Subnet option, or none
// when family == 0.
func msgWithECS(name string, family uint16, prefix uint8, addr string) *dns.Msg {
m := new(dns.Msg)
m.SetQuestion(dns.Fqdn(name), dns.TypeA)
if family == 0 {
return m
}
m.SetEdns0(4096, true)
m.IsEdns0().Option = append(m.IsEdns0().Option, &dns.EDNS0_SUBNET{
Code: dns.EDNS0SUBNET,
Family: family,
SourceNetmask: prefix,
Address: net.ParseIP(addr),
})
return m
}
// answerWithA builds a cached answer holding a single A record with the given address.
func answerWithA(name, a string) *dns.Msg {
m := new(dns.Msg)
m.SetQuestion(dns.Fqdn(name), dns.TypeA)
rr, err := dns.NewRR(dns.Fqdn(name) + " 300 IN A " + a)
if err != nil {
panic(err)
}
m.Answer = []dns.RR{rr}
return m
}
func firstA(msg *dns.Msg) string {
for _, rr := range msg.Answer {
if a, ok := rr.(*dns.A); ok {
return a.A.String()
}
}
return ""
}
// TestCanonicalECS covers the key-partitioning helper: only a request with no ECS option
// collapses to the shared empty partition, while a carried /0 keeps its own family-scoped
// token (distinct from no-ECS, per RFC 7871 §7.3.1); host bits below the source prefix do
// not fragment the key, and different subnets / families produce different keys.
func TestCanonicalECS(t *testing.T) {
tests := []struct {
name string
msg *dns.Msg
want string
}{
{"no ecs", msgWithECS("controld.com", 0, 0, ""), ""},
// A carried ECS option is never "" (RFC 7871 §7.3.1), and IPv4 /0 vs
// IPv6 /0 stay distinct; the address encoding must not change the token.
{"ipv4 /0", msgWithECS("controld.com", 1, 0, "0.0.0.0"), "1/0/0.0.0.0"},
{"ipv4 /0 empty addr", msgWithECS("controld.com", 1, 0, ""), "1/0/0.0.0.0"},
{"ipv6 /0", msgWithECS("controld.com", 2, 0, "::"), "2/0/::"},
{"ipv6 /0 empty addr", msgWithECS("controld.com", 2, 0, ""), "2/0/::"},
{"ipv4 /24", msgWithECS("controld.com", 1, 24, "203.0.113.0"), "1/24/203.0.113.0"},
{"ipv4 host bits masked", msgWithECS("controld.com", 1, 24, "203.0.113.7"), "1/24/203.0.113.0"},
{"ipv6 /64", msgWithECS("controld.com", 2, 64, "2001:db8:1::"), "2/64/2001:db8:1::"},
{"ipv6 host bits masked", msgWithECS("controld.com", 2, 64, "2001:db8:1::dead:beef"), "2/64/2001:db8:1::"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := CanonicalECS(tt.msg); got != tt.want {
t.Fatalf("CanonicalECS = %q, want %q", got, tt.want)
}
})
}
}
// TestNewKey_ECSPartition verifies the cache key distinguishes subnets while collapsing
// same-subnet requests, so the LRU cache cannot serve one subnet's records to another.
func TestNewKey_ECSPartition(t *testing.T) {
const up = "https://dns.example/dns-query"
subnetA := NewKey(msgWithECS("controld.com", 2, 64, "2001:db8:1::"), up)
subnetB := NewKey(msgWithECS("controld.com", 2, 64, "2001:db8:2::"), up)
subnetAHost := NewKey(msgWithECS("controld.com", 2, 64, "2001:db8:1::5"), up)
ipv4 := NewKey(msgWithECS("controld.com", 1, 24, "203.0.113.0"), up)
noECS := NewKey(msgWithECS("controld.com", 0, 0, ""), up)
if subnetA == subnetB {
t.Fatal("different subnets must not share a cache key")
}
if subnetA != subnetAHost {
t.Fatal("same subnet (different host bits) must share a cache key")
}
if subnetA == ipv4 || subnetA == noECS || ipv4 == noECS {
t.Fatal("different families / no-ECS must not collide")
}
}
// TestLRUCache_ECSZeroPrefixDistinctFromNoECS is the regression test for the /0-vs-no-ECS
// collision (RFC 7871 §7.3.1). A query carrying an ECS /0 option is forwarded WITH ECS and
// may draw different upstream data, so its cached answer must never be served to a client
// that sent no ECS at all, nor may IPv4 /0 and IPv6 /0 cross-serve each other.
func TestLRUCache_ECSZeroPrefixDistinctFromNoECS(t *testing.T) {
c, err := NewLRUCache(16)
if err != nil {
t.Fatalf("NewLRUCache: %v", err)
}
const up = "https://dns.example/dns-query"
expire := time.Now().Add(time.Minute)
noECS := msgWithECS("controld.com", 0, 0, "")
zeroV4 := msgWithECS("controld.com", 1, 0, "0.0.0.0")
zeroV6 := msgWithECS("controld.com", 2, 0, "::")
// Only the IPv4 /0 query's answer is cached.
c.Add(NewKey(zeroV4, up), NewValue(answerWithA("controld.com", "192.0.2.1"), expire))
// A no-ECS client must NOT receive the ECS /0 cached answer.
if got := c.Get(NewKey(noECS, up)); got != nil {
t.Fatalf("no-ECS client received the ECS /0 cached record %q", firstA(got.Msg))
}
// An IPv6 /0 client must NOT receive the IPv4 /0 answer.
if got := c.Get(NewKey(zeroV6, up)); got != nil {
t.Fatalf("IPv6 /0 client received the IPv4 /0 cached record %q", firstA(got.Msg))
}
// The IPv4 /0 client still hits its own entry.
if got := c.Get(NewKey(zeroV4, up)); got == nil || firstA(got.Msg) != "192.0.2.1" {
t.Fatalf("IPv4 /0 lost its own cached record: %v", got)
}
// The no-ECS partition is independent and does not corrupt the /0 entry.
c.Add(NewKey(noECS, up), NewValue(answerWithA("controld.com", "198.51.100.1"), expire))
if got := c.Get(NewKey(noECS, up)); got == nil || firstA(got.Msg) != "198.51.100.1" {
t.Fatalf("no-ECS partition wrong record: %v", got)
}
if got := c.Get(NewKey(zeroV4, up)); got == nil || firstA(got.Msg) != "192.0.2.1" {
t.Fatalf("IPv4 /0 record corrupted by no-ECS insert: %v", got)
}
}
// TestLRUCache_ECSNoCrossSubnetServe is the real cache-path regression test for #564:
// an entry populated for subnet A must never be returned to a client in subnet B, even
// though the question (name/type/class/upstream) is identical. The two subnets carry
// different A records; the second client must get its own record or a miss, never A's.
func TestLRUCache_ECSNoCrossSubnetServe(t *testing.T) {
c, err := NewLRUCache(16)
if err != nil {
t.Fatalf("NewLRUCache: %v", err)
}
const up = "https://dns.example/dns-query"
expire := time.Now().Add(time.Minute)
reqA := msgWithECS("controld.com", 2, 64, "2001:db8:1::")
reqB := msgWithECS("controld.com", 2, 64, "2001:db8:2::")
// Only subnet A's answer (A record 192.0.2.1) is cached.
c.Add(NewKey(reqA, up), NewValue(answerWithA("controld.com", "192.0.2.1"), expire))
// A client in subnet B must NOT hit subnet A's entry.
if got := c.Get(NewKey(reqB, up)); got != nil {
t.Fatalf("subnet B received subnet A's cached record %q; cache is not ECS-partitioned", firstA(got.Msg))
}
// Subnet A still hits its own entry.
if got := c.Get(NewKey(reqA, up)); got == nil || firstA(got.Msg) != "192.0.2.1" {
t.Fatalf("subnet A lost its own cached record: %+v", got)
}
// Now cache subnet B's distinct answer and confirm the two never cross.
c.Add(NewKey(reqB, up), NewValue(answerWithA("controld.com", "198.51.100.1"), expire))
if got := c.Get(NewKey(reqB, up)); got == nil || firstA(got.Msg) != "198.51.100.1" {
t.Fatalf("subnet B got the wrong record: %v", got)
}
if got := c.Get(NewKey(reqA, up)); got == nil || firstA(got.Msg) != "192.0.2.1" {
t.Fatalf("subnet A record corrupted by subnet B insert: %v", got)
}
// A second host within subnet A shares the partition (cache hit with A's record).
reqAHost := msgWithECS("controld.com", 2, 64, "2001:db8:1::9")
if got := c.Get(NewKey(reqAHost, up)); got == nil || firstA(got.Msg) != "192.0.2.1" {
t.Fatalf("same-subnet host missed the shared cache entry: %v", got)
}
}
+135 -3
View File
@@ -157,6 +157,101 @@ type parallelDialerResult struct {
err error
}
const (
// unreachableBackoffBase is the initial suppression window applied to a
// dial address after it returns a network-unreachable error (e.g.
// "connect: no route to host"). The window grows exponentially up to
// unreachableBackoffMax on repeated failures, and is cleared as soon as
// the address dials successfully.
unreachableBackoffBase = 5 * time.Second
// unreachableBackoffMax caps the suppression window so an address is
// always re-probed within a bounded interval, preserving recovery when
// the route comes back.
unreachableBackoffMax = 60 * time.Second
)
// Windows winsock codes for the unreachable errnos. A failing connect on
// Windows surfaces these raw WSA codes (WSAENETUNREACH/WSAEHOSTUNREACH), whereas
// syscall.ENETUNREACH/EHOSTUNREACH are Go's portable "invented" values
// (APPLICATION_ERROR + iota) that never equal them. Matching these explicitly is
// therefore required for the classifier to detect unreachable errors on Windows;
// errors.Is against the syscall.* constants alone would not.
//
// https://learn.microsoft.com/en-us/windows/win32/winsock/windows-sockets-error-codes-2
var (
windowsENETUNREACH = syscall.Errno(10051)
windowsEHOSTUNREACH = syscall.Errno(10065)
)
// IsUnreachable reports whether err indicates the destination network or host
// has no route (ENETUNREACH/EHOSTUNREACH). These are the errors produced when
// an endpoint's address family is available locally but unroutable, e.g. an
// IPv6 DoH endpoint while the host has IPv6 but no route to it.
func IsUnreachable(err error) bool {
if err == nil {
return false
}
var opErr *net.OpError
if errors.As(err, &opErr) {
return errors.Is(opErr.Err, syscall.ENETUNREACH) ||
errors.Is(opErr.Err, syscall.EHOSTUNREACH) ||
errors.Is(opErr.Err, windowsENETUNREACH) ||
errors.Is(opErr.Err, windowsEHOSTUNREACH)
}
return false
}
type unreachableEntry struct {
until time.Time
backoff time.Duration
}
// unreachableTracker records dial addresses that recently failed with a
// network-unreachable error so ParallelDialer can temporarily stop hammering
// them. This prevents an unroutable endpoint from generating a sustained dial
// /health-check storm, while still re-probing each address once its bounded
// backoff window expires so genuine recovery is never permanently blocked.
type unreachableTracker struct {
mu sync.Mutex
entries map[string]unreachableEntry
}
var unreachable = &unreachableTracker{entries: make(map[string]unreachableEntry)}
// suppressed reports whether addr is currently within its unreachable backoff
// window and should be skipped.
func (t *unreachableTracker) suppressed(addr string, now time.Time) bool {
t.mu.Lock()
defer t.mu.Unlock()
e, ok := t.entries[addr]
return ok && now.Before(e.until)
}
// markUnreachable extends the suppression window for addr using a bounded
// exponential backoff.
func (t *unreachableTracker) markUnreachable(addr string, now time.Time) {
t.mu.Lock()
defer t.mu.Unlock()
e := t.entries[addr]
if e.backoff == 0 {
e.backoff = unreachableBackoffBase
} else {
e.backoff *= 2
if e.backoff > unreachableBackoffMax {
e.backoff = unreachableBackoffMax
}
}
e.until = now.Add(e.backoff)
t.entries[addr] = e
}
// markReachable clears any suppression for addr after a successful dial.
func (t *unreachableTracker) markReachable(addr string) {
t.mu.Lock()
defer t.mu.Unlock()
delete(t.entries, addr)
}
type ParallelDialer struct {
net.Dialer
}
@@ -165,26 +260,63 @@ func (d *ParallelDialer) DialContext(ctx context.Context, network string, addrs
if len(addrs) == 0 {
return nil, errors.New("empty addresses")
}
// Skip addresses that recently returned a network-unreachable error so an
// unroutable endpoint (e.g. an IPv6 DoH address while the host has IPv6
// but no route to it) does not generate a sustained dial storm. Suppression
// is bounded: once an address's backoff window expires it is re-probed, so
// genuine recovery is preserved.
now := time.Now()
live := make([]string, 0, len(addrs))
var suppressed int
for _, addr := range addrs {
if unreachable.suppressed(addr, now) {
suppressed++
continue
}
live = append(live, addr)
}
if len(live) == 0 {
// Every candidate is within its unreachable backoff window. Fail fast
// and quietly instead of re-dialing known-unroutable addresses; the
// windows expire and re-probe shortly, so recovery still happens.
logger.Debug().Msgf("skipping %d unreachable address(es), all in backoff", suppressed)
// TODO: the errno here is hardcoded to EHOSTUNREACH, but the actual
// failure that triggered suppression may have been ENETUNREACH. This is
// harmless today (IsUnreachable treats both the same and nothing else
// inspects the errno), but if these errors are ever recorded/reported we
// should retain the real error in unreachableEntry and surface it here.
return nil, &net.OpError{Op: "dial", Net: network, Err: syscall.EHOSTUNREACH}
}
if suppressed > 0 {
logger.Debug().Msgf("skipping %d unreachable address(es) in backoff", suppressed)
}
ctx, cancel := context.WithCancel(ctx)
defer cancel()
done := make(chan struct{})
defer close(done)
ch := make(chan *parallelDialerResult, len(addrs))
ch := make(chan *parallelDialerResult, len(live))
var wg sync.WaitGroup
wg.Add(len(addrs))
wg.Add(len(live))
go func() {
wg.Wait()
close(ch)
}()
for _, addr := range addrs {
for _, addr := range live {
go func(addr string) {
defer wg.Done()
logger.Debug().Msgf("dialing to %s", addr)
conn, err := d.Dialer.DialContext(ctx, network, addr)
if err != nil {
logger.Debug().Msgf("failed to dial %s: %v", addr, err)
if IsUnreachable(err) {
unreachable.markUnreachable(addr, time.Now())
}
} else {
unreachable.markReachable(addr)
}
select {
case ch <- &parallelDialerResult{conn: conn, err: err}:
+67
View File
@@ -2,10 +2,77 @@ package net
import (
"context"
"net"
"syscall"
"testing"
"time"
)
func TestIsUnreachable(t *testing.T) {
tests := []struct {
name string
err error
want bool
}{
{"nil", nil, false},
{"enetunreach", &net.OpError{Op: "dial", Net: "tcp", Err: syscall.ENETUNREACH}, true},
{"ehostunreach", &net.OpError{Op: "dial", Net: "tcp", Err: syscall.EHOSTUNREACH}, true},
{"windows enetunreach", &net.OpError{Op: "dial", Net: "tcp", Err: windowsENETUNREACH}, true},
{"windows ehostunreach", &net.OpError{Op: "dial", Net: "tcp", Err: windowsEHOSTUNREACH}, true},
{"connection refused", &net.OpError{Op: "dial", Net: "tcp", Err: syscall.ECONNREFUSED}, false},
{"not an opError", syscall.ENETUNREACH, false},
}
for _, tc := range tests {
tc := tc
t.Run(tc.name, func(t *testing.T) {
if got := IsUnreachable(tc.err); got != tc.want {
t.Errorf("IsUnreachable(%v) = %v, want %v", tc.err, got, tc.want)
}
})
}
}
func TestUnreachableTracker(t *testing.T) {
tr := &unreachableTracker{entries: make(map[string]unreachableEntry)}
const addr = "[2606:1a40::22]:443"
now := time.Unix(0, 0)
// Not suppressed before any failure.
if tr.suppressed(addr, now) {
t.Fatal("addr suppressed before any failure")
}
// First failure suppresses for the base window.
tr.markUnreachable(addr, now)
if !tr.suppressed(addr, now.Add(unreachableBackoffBase-time.Millisecond)) {
t.Fatal("addr not suppressed within base backoff window")
}
if tr.suppressed(addr, now.Add(unreachableBackoffBase)) {
t.Fatal("addr still suppressed at end of base backoff window")
}
// Backoff grows exponentially and is capped at the max.
tr.markUnreachable(addr, now)
if got := tr.entries[addr].backoff; got != 2*unreachableBackoffBase {
t.Fatalf("backoff after second failure = %v, want %v", got, 2*unreachableBackoffBase)
}
for i := 0; i < 10; i++ {
tr.markUnreachable(addr, now)
}
if got := tr.entries[addr].backoff; got != unreachableBackoffMax {
t.Fatalf("backoff not capped: got %v, want %v", got, unreachableBackoffMax)
}
// A successful dial clears suppression entirely.
tr.markReachable(addr)
if tr.suppressed(addr, now) {
t.Fatal("addr still suppressed after markReachable")
}
if _, ok := tr.entries[addr]; ok {
t.Fatal("entry not removed after markReachable")
}
}
func TestProbeStackTimeout(t *testing.T) {
done := make(chan struct{})
started := make(chan struct{})
+1 -1
View File
@@ -2,11 +2,11 @@ package dnsmasq
import (
"errors"
"html/template"
"net"
"os"
"path/filepath"
"strings"
"text/template"
"github.com/Control-D-Inc/ctrld"
)
+7
View File
@@ -1,7 +1,14 @@
package ctrld
import "runtime"
type dnsFn func() []string
// isMobile reports whether the current OS is a mobile platform.
func isMobile() bool {
return runtime.GOOS == "android" || runtime.GOOS == "ios"
}
// nameservers returns DNS nameservers from system settings.
func nameservers() []string {
var dns []string
+16
View File
@@ -25,6 +25,12 @@ func dnsFns() []dnsFn {
func getDNSFromScutil() []string {
logger := *ProxyLogger.Load()
// Skip scutil on mobile platforms - not available in sandbox
if isMobile() {
Log(context.Background(), logger.Debug(), "skipping scutil DNS discovery on mobile platform")
return nil
}
const (
maxRetries = 10
retryInterval = 100 * time.Millisecond
@@ -89,6 +95,11 @@ func getDNSFromScutil() []string {
}
func getDHCPNameservers(iface string) ([]string, error) {
// Skip ipconfig on mobile platforms - not available in sandbox
if isMobile() {
return nil, fmt.Errorf("ipconfig not available on mobile")
}
// Run the ipconfig command for the given interface.
cmd := exec.Command("ipconfig", "getpacket", iface)
output, err := cmd.Output()
@@ -201,6 +212,11 @@ func getAllDHCPNameservers() []string {
}
func patchNetIfaceName(iface *net.Interface) (bool, error) {
// Skip networksetup on mobile platforms - not available in sandbox
if isMobile() {
return false, nil
}
b, err := exec.Command("networksetup", "-listnetworkserviceorder").Output()
if err != nil {
return false, err
+16
View File
@@ -7,6 +7,7 @@ import (
"net"
"net/netip"
"os"
"runtime"
"strings"
"tailscale.com/net/netmon"
@@ -24,6 +25,11 @@ func dnsFns() []dnsFn {
}
func dns4() []string {
// Skip route-based DNS discovery on Android
if runtime.GOOS == "android" {
return nil
}
f, err := os.Open(v4RouteFile)
if err != nil {
return nil
@@ -64,6 +70,11 @@ func dns4() []string {
}
func dns6() []string {
// Skip route-based DNS discovery on Android
if runtime.GOOS == "android" {
return nil
}
f, err := os.Open(v6RouteFile)
if err != nil {
return nil
@@ -98,6 +109,11 @@ func dns6() []string {
}
func dnsFromSystemdResolver() []string {
// Skip systemd resolver on Android
if runtime.GOOS == "android" {
return nil
}
c, err := resolvconffile.ParseFile("/run/systemd/resolve/resolv.conf")
if err != nil {
return nil
+29 -4
View File
@@ -19,6 +19,8 @@ import (
"golang.org/x/sync/singleflight"
"tailscale.com/net/netmon"
"tailscale.com/net/tsaddr"
"github.com/Control-D-Inc/ctrld/internal/dnscache"
)
const (
@@ -45,8 +47,12 @@ const (
const controldPublicDns = "76.76.2.0"
const maxConcurrentOSResolverExchanges = 128
var controldPublicDnsWithPort = net.JoinHostPort(controldPublicDns, "53")
var osResolverExchangeSem = make(chan struct{}, maxConcurrentOSResolverExchanges)
var localResolver Resolver
func init() {
@@ -136,14 +142,15 @@ func availableNameservers() []string {
// It's the caller's responsibility to ensure the system DNS is in a clean state before
// calling this function.
func InitializeOsResolver(guardAgainstNoNameservers bool) []string {
resolverMutex.Lock()
defer resolverMutex.Unlock()
nameservers := availableNameservers()
// if no nameservers, return empty slice so we dont remove all nameservers
if len(nameservers) == 0 && guardAgainstNoNameservers {
return []string{}
}
ns := initializeOsResolver(nameservers)
resolverMutex.Lock()
defer resolverMutex.Unlock()
or = newResolverWithNameserver(ns)
return ns
}
@@ -373,8 +380,10 @@ func (o *osResolver) Resolve(ctx context.Context, msg *dns.Msg) (*dns.Msg, error
domain := strings.TrimSuffix(msg.Question[0].Name, ".")
qtype := msg.Question[0].Qtype
// Unique key for the singleflight group.
key := fmt.Sprintf("%s:%d:", domain, qtype)
// Unique key for the singleflight group. The EDNS Client Subnet is part of
// the key so subnet-specific answers are neither coalesced nor hot-cached
// across different subnets (RFC 7871 §7.3).
key := fmt.Sprintf("%s:%d:%s", domain, qtype, dnscache.CanonicalECS(msg))
// Checking the cache first.
if val, ok := o.cache.Load(key); ok {
@@ -466,6 +475,13 @@ func (o *osResolver) resolve(ctx context.Context, msg *dns.Msg) (*dns.Msg, error
for _, server := range servers {
go func(server string) {
defer wg.Done()
release, ok := acquireOSResolverExchangeSlot(ctx)
if !ok {
ch <- &osResolverResult{err: ctx.Err(), server: server, lan: isLan}
return
}
defer release()
var answer *dns.Msg
var err error
var localOSResolverIP net.IP
@@ -576,6 +592,15 @@ func (o *osResolver) resolve(ctx context.Context, msg *dns.Msg) (*dns.Msg, error
return nil, errors.Join(errs...)
}
func acquireOSResolverExchangeSlot(ctx context.Context) (func(), bool) {
select {
case osResolverExchangeSem <- struct{}{}:
return func() { <-osResolverExchangeSem }, true
case <-ctx.Done():
return nil, false
}
}
func (o *osResolver) removeCache(key string) {
o.cache.Delete(key)
}
+92 -1
View File
@@ -282,6 +282,93 @@ func Test_Edns0_CacheReply(t *testing.T) {
}
}
// ecsAnswerHandler returns a distinct A record per EDNS Client Subnet, so a test can
// prove one subnet never receives another subnet's cached record. It counts upstream
// calls to confirm the hot cache/singleflight is partitioned by ECS rather than shared.
func ecsAnswerHandler(call *atomic.Int64) dns.HandlerFunc {
return func(w dns.ResponseWriter, msg *dns.Msg) {
call.Add(1)
a := "203.0.113.1" // no/other subnet
if opt := msg.IsEdns0(); opt != nil {
for _, o := range opt.Option {
if e, ok := o.(*dns.EDNS0_SUBNET); ok {
switch {
case e.Address.Equal(net.ParseIP("2001:db8:1::")):
a = "192.0.2.1"
case e.Address.Equal(net.ParseIP("2001:db8:2::")):
a = "198.51.100.1"
}
}
}
}
m := new(dns.Msg)
m.SetReply(msg)
rr, _ := dns.NewRR(msg.Question[0].Name + " 300 IN A " + a)
m.Answer = []dns.RR{rr}
w.WriteMsg(m)
}
}
// Test_osResolver_HotCache_ECSPartition is the real cache-path regression test for #564 on
// the osResolver hot cache / singleflight path: the upstream returns a different A record
// per subnet, and a client in subnet B must never be served subnet A's hot-cached record.
func Test_osResolver_HotCache_ECSPartition(t *testing.T) {
lanPC, err := net.ListenPacket("udp", "127.0.0.1:0")
if err != nil {
t.Fatalf("failed to listen on LAN address: %v", err)
}
call := &atomic.Int64{}
lanServer, lanAddr, err := runLocalPacketConnTestServer(t, lanPC, ecsAnswerHandler(call))
if err != nil {
t.Fatalf("failed to run LAN test server: %v", err)
}
defer lanServer.Shutdown()
or := newResolverWithNameserver([]string{lanAddr})
query := func(subnet string) string {
m := new(dns.Msg)
m.SetQuestion(dns.Fqdn("controld.com"), dns.TypeA)
m.RecursionDesired = true
m.SetEdns0(4096, true)
m.IsEdns0().Option = append(m.IsEdns0().Option, &dns.EDNS0_SUBNET{
Code: dns.EDNS0SUBNET,
Family: 2,
SourceNetmask: 64,
Address: net.ParseIP(subnet),
})
answer, err := or.Resolve(context.Background(), m)
if err != nil {
t.Fatal(err)
}
for _, rr := range answer.Answer {
if a, ok := rr.(*dns.A); ok {
return a.A.String()
}
}
return ""
}
// Subnet A populates the hot cache; a repeat hits it (upstream called once).
if got := query("2001:db8:1::"); got != "192.0.2.1" {
t.Fatalf("subnet A: got %q, want 192.0.2.1", got)
}
if got := query("2001:db8:1::"); got != "192.0.2.1" {
t.Fatalf("subnet A repeat: got %q, want 192.0.2.1", got)
}
if call.Load() != 1 {
t.Fatalf("subnet A repeat did not hit the hot cache: %d upstream calls", call.Load())
}
// Subnet B must get ITS OWN record, not subnet A's hot-cached one, and this
// requires a fresh upstream call (the cache is partitioned, not shared).
if got := query("2001:db8:2::"); got != "198.51.100.1" {
t.Fatalf("subnet B was served the wrong record %q (want 198.51.100.1); hot cache is not ECS-partitioned", got)
}
if call.Load() != 2 {
t.Fatalf("subnet B unexpectedly served from subnet A's cache: %d upstream calls, want 2", call.Load())
}
}
// https://github.com/Control-D-Inc/ctrld/issues/255
func Test_legacyResolverWithBigExtraSection(t *testing.T) {
lanPC, err := net.ListenPacket("udp", "127.0.0.1:0") // 127.0.0.1 is considered LAN (loopback)
@@ -383,6 +470,11 @@ func nonSuccessHandlerWithRcode(rcode int) dns.HandlerFunc {
func countHandler(call *atomic.Int64) dns.HandlerFunc {
return func(w dns.ResponseWriter, msg *dns.Msg) {
// Count the call before writing the reply. The client returns as soon
// as it receives the response, so a caller that reads this counter right
// after Resolve returns would race an increment done after WriteMsg and
// could observe a stale zero.
call.Add(1)
m := new(dns.Msg)
m.SetRcode(msg, dns.RcodeSuccess)
if cookie := getEdns0Cookie(msg.IsEdns0()); cookie != nil {
@@ -395,7 +487,6 @@ func countHandler(call *atomic.Int64) dns.HandlerFunc {
m.IsEdns0().Option = append(m.IsEdns0().Option, cookieOption)
}
w.WriteMsg(m)
call.Add(1)
}
}