diff --git a/CHANGELOG.md b/CHANGELOG.md index d14c431..77321c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,219 @@ current as you land changes. ## [Unreleased] +### Added + +- **Enforcement verification** (`vpn.advanced.verifyInterval`, default `1m`). + The run loop now periodically confirms the firewall rules it believes are + installed are actually still there AND still enforcing — not just present + but disconnected from what makes them bite (the pf main ruleset no longer + referencing our anchor, an nft chain's policy rewritten off `drop` in + place, a Windows profile's outbound default flipped back to Allow while + our rules sit untouched) — re-applying whatever posture is currently in + force (the standing guard, a full block, or an open switch/redial window + or pause) the instant either check fails. Every other rule change was + already triggered by something dezhban itself did — a tunnel change, an + endpoint refresh, a posture flip; this is the only one that notices a + ruleset (or the switch that makes it matter) disturbed from OUTSIDE the + daemon (another firewall tool, `pfctl -F all`, `nft flush ruleset`, an OS + ruleset reload). Reported in `state.verify` + (`status --json`), the plain-text `dezhban status`/menubar posture sentence, + and turns the menubar icon amber, only while something is wrong; + disablable (`"0"`) from the CLI or the macOS app's Settings pane, and an + unreadable backend is never treated as evidence the rules are gone. See + [docs/usage/config.md](docs/usage/config.md#advanced-tunables-vpnadvanced). +- **Zombie-tunnel detection.** A tunnel interface that reports up while a run + of exit-country lookups through it has failed is now diagnosed as such — + reported in `state.zombie`, `dezhban doctor`'s new "enforcement liveness" + check, and the rendered posture sentence (which now also turns the menubar + icon amber for the duration of the streak) — instead of sitting correctly + cut with no signal to anyone. Detection is always on; letting a confirmed + streak open an automatic redial window is a separate, off-by-default key + (`vpn.advanced.livenessRedial`, settable from the CLI or the macOS app's + Settings pane), because an exit that censors the geo providers produces the + identical symptom on a tunnel that was never actually down. See + [ADR-0010](docs/adr/0010-tunnel-liveness.md). +- **A single-instance guard on `run`.** A second `dezhban run` — with or + without `--no-daemon` — started alongside an already-running daemon now + refuses immediately instead of racing it to apply firewall rules. The lock + is released by the OS the moment the holding process ends, by any means, so + a killed daemon never wedges the next start. `panic`, `unblock`, and the + service-lifecycle commands deliberately take no such lock — they remain the + escape hatch, usable with no daemon running at all. +- **Exit-IP change observation.** The daemon now logs and publishes + (`state.exitIpChangedAt`) when the observed exit IP differs from the + previous successful reading — purely informational, like the exit-country + check it sits beside: it never affects `blocked`, `countryCode`, or the + hysteresis streak. A failover between two servers in the same allowed + country changes nothing those fields report, but changes this. Now also + reported in `dezhban doctor`'s "enforcement liveness" check (text and + `--json`) and the macOS app's Diagnostics pane — previously the field was + published but rendered nowhere. +- **A startup self-test log line.** `dezhban run` now logs one summary at + startup — firewall backend reachable, state directory writable, tunnels + configured/detected, endpoints known, whether this host has ever observed a + tunnel up — diagnostic only, never blocking startup. +- **Panic-disarm status visibility.** `dezhban panic` tearing down the + firewall while a daemon keeps running (and every automatic Apply path + standing down because of it) is now reported to the operator instead of + being silent — `state.panicDisarmed` (`status --json`) and a "Panic + disarmed" headline that wins over the plain-text `dezhban status`/menubar + posture sentence, so `status`/the menubar can no longer say "Guarding" or + "Full block" while enforcement is actually torn down and waiting on + `dezhban unblock` (or a daemon restart). + +### Changed + +- The automatic-redial-refusal log lines no longer hardcode "vpn tunnel + down", since a refusal can now also come from the zombie-tunnel liveness + trigger, whose interface never goes down: `"vpn tunnel down — no redial + window (...)"` is now `"no automatic redial window (...)"`, and + `"vpn tunnel down — redial window suppressed"` is now `"redial window + suppressed"`. Anyone grepping or alerting on the old strings should update + the pattern. + +### Fixed + +- **`dezhban panic`'s teardown now stays effective across every automatic + enforcement path, not just periodic verification.** Previously the + panic-disarm marker (which tells enforcement verification to stand down + after a deliberate `panic` teardown) was consulted only by the + `verifyInterval` tick — the automatic redial window, the geo-provider + GUARD/FULL BLOCK state machine, tunnel/endpoint-change re-applies, and + auto-arm from standby could all still silently reinstall rules within + moments, turning the documented lockout escape hatch into a brief flicker. + Every automatic path now stands down while the marker is set; every + explicit operator command (`block`, `unblock`, `switch`, `pause`/`resume`) + clears it unconditionally instead, exactly as `unblock` already did — an + explicit command is never blocked by the marker. +- **A hung tunnel `vpn.advanced.livenessRedial` refused to redial could stay + refused forever**, even once it had genuinely been up long enough to pass + `redialMinUptime`'s anti-flap check. The re-decision that fires once that + bound lifts was reusing the uptime captured at the very first widening + attempt — correct for an ordinary drop (the tunnel is down, so uptime is a + frozen historical fact) but wrong for a zombie streak, where the tunnel + never goes down and its uptime keeps growing for real. It now re-derives + the uptime for a standing zombie streak instead of reusing the stale value. +- **A manual `block` no longer leaves a stale "tunnel reports up but exit + checks failing" warning on screen.** If a zombie-tunnel streak was showing + when the operator ran `dezhban block` (or the control-socket equivalent), + the warning previously lingered until the next periodic geo check cleared + it; it is now cleared in the same publish as the block itself. +- **Exit-IP change observation now also covers the window between daemon + startup and the first periodic geo check.** A VPN failover landing in that + window was previously invisible to `state.exitIpChangedAt` because only the + periodic check recorded the last-seen exit IP; the startup reading now + records it too. +- **Windows enforcement verification (`vpn.advanced.verifyInterval`) no + longer needs two sequential PowerShell calls per tick.** The group-existence + check and the per-profile default-action query are now one invocation, so a + slow PowerShell/WMI response can no longer nearly double the time a verify + tick can hold up the run loop. +- **macOS enforcement verification's three `pfctl` reads now share one + deadline instead of each getting its own.** Under `pf` lock contention a + verify tick could previously stall the run loop's single goroutine — window + timers, geo ticks, control-socket replies — for up to 3x `pfctl`'s 10s + timeout; it is now bounded to that timeout once, the same fix already + applied to the Windows backend above. +- **A live daemon could briefly reinstate rules `dezhban panic` had just torn + down.** `panic` now records the panic-disarm marker BEFORE tearing down the + firewall rather than after — every automatic Apply path a running daemon + might take checks the marker first and only then reads the firewall, so the + old ordering left a window where such a check could land between the two + calls, see the rules already gone, find no marker yet, and silently + reinstate them. +- **Windows enforcement verification no longer reports false rule drift when + a PowerShell/WMI hiccup drops one firewall profile's line from the query + output.** A profile missing from the parsed result used to read as the Go + zero value, which never matched the wanted action and triggered an + unwarranted repair; it is now treated as an unreadable check, the same + discipline already applied to a fully failed query. +- **A local, unprivileged process could block the Windows kill switch from + ever starting.** The single-instance lock's mutex lived under a predictable + `Global\` name, so anyone could pre-create it first and either get treated + as the legitimate "already running" holder or deny the daemon's own + `CreateMutexW` with a hostile DACL. It now lives inside a boundary-restricted + private namespace that only `LocalSystem` or `BUILTIN\Administrators` can + create or open objects in, falling back to the old name only if that setup + fails. +- **`dezhban panic`'s teardown could be silently undone by a transient read + error.** The running daemon checked whether its panic marker was still + present with a bare stat that treated ANY error — not just a missing file — + as "gone", so a momentary I/O or permission hiccup could make enforcement + verification re-apply the very posture the operator just tore down. Only a + definite "not found" now counts as absent. +- **`dezhban doctor`'s liveness check now reports a suspended `dezhban panic` + teardown**, and no longer claims "enforcement is holding" when it genuinely + can't confirm that (an unreadable firewall, or a repair attempt that itself + failed) — both previously fell through to the same reassuring summary a + routine, already-repaired finding gets. +- **Enforcement verification's repair counter and log line no longer claim + success before the re-apply actually lands.** A repair whose + `Backend.Apply` call itself failed was previously counted and reported as + completed; it is now counted only once the re-apply confirms, and a failed + attempt surfaces through the daemon's normal enforcement-error reporting + instead. +- **Live-disabling `vpn.advanced.verifyInterval` while `dezhban panic` had + suspended verification could silently swallow the next "verification + suspended" warning** if the interval was later re-enabled. The suspended + flag is now reset when the interval is turned off, so the warning fires + again on the next suspend. +- **Live-disabling `vpn.advanced.livenessRedial` no longer leaves a standing + zombie-tunnel redial refusal free to open a window anyway** once its + budget/cooldown lifts — the retry now re-checks the live setting, not just + whether the streak is still standing. +- **Exit-IP change observation now also covers readings that land FULL + BLOCK**, not only allowed ones — a failover between two servers in the same + forbidden country previously went unrecorded, as did the very first + reading whenever startup itself observed a blocked country. +- **`dezhban run`'s startup self-test no longer delays the boot-time firewall + apply.** Its firewall-reachability check (and, with auto-discovery on, its + endpoint resolve) now run off the startup path instead of ahead of the very + `Apply(guard)` call ADR-0008's arm-at-boot promise depends on landing + immediately. The self-test's "state directory writable" and "endpoints + known" fields also now probe a real write and a real resolve instead of + inferring success from configuration alone. +- **Windows enforcement verification no longer misreports drift after its own + bookkeeping write fails.** The write that records what `Apply` last set is + atomic, so a failure left the PREVIOUS `Apply`'s value on disk; `IsBlocked`'s + drift check then compared a live, correctly re-applied profile against that + stale value and could report a false repair. The stale record is now + cleared on a failed write instead of left in place. +- Config/state writes made through the shared `atomicfile` helper now also + fsync the containing directory after the rename, hardening durability + across a crash on filesystems that require it (covers the panic marker and + the arm-at-boot record, among others). +- **A failed enforcement-verification repair's error could be silently + overwritten by the very next ordinary geo-poll tick**, replacing it with a + falsely reassuring "rules re-applied" message even though the repair + actually failed and the firewall was still unenforced. The geo-poll step + now only overwrites the enforcement-error surface on a tick that actually + touched the backend, never on an uneventful no-op reading. +- **A manual `block`/`unblock` over the control socket no longer leaves a + stale "rules missing, re-applied N times" message on screen** after + installing fresh rules — enforcement-verification state is now reset in + the same command, the same fix already applied to the zombie-tunnel + warning. +- **`dezhban hold` armed during a hung-tunnel (zombie) streak no longer + permanently forfeits that streak's one-shot `vpn.advanced.livenessRedial` + attempt.** Because that attempt is suppressed without being "spent" by + hold (a later real disconnect still needs its own hold), cancelling hold + previously had nothing to act on and the attempt was lost for the rest of + the streak. Cancelling hold now restores it, the same way it already + restores a refused-and-waiting ordinary drop's retry. +- **Windows enforcement verification could misreport rules as missing when + PowerShell wrote incidental output ahead of the group-existence check's + own marker line.** The check required that marker on the exact first line + of output; it now scans for it instead, still requiring an exact line + match (never a substring) so unrelated text can't be mistaken for it + either. +- **Linux (`nft`) enforcement verification's drift check is now scoped to + the `output` chain specifically**, instead of searching the whole `nft + list table` output for the text "policy drop" anywhere. A table with more + than one chain could previously have its `output` chain's policy drift to + `accept` while another chain's unrelated "policy drop" text kept the check + reporting enforcement as healthy. + ## [0.9.0] - 2026-07-29 ### Added diff --git a/cmd/dezhban/config_cmd.go b/cmd/dezhban/config_cmd.go index c0977d8..2cfa5d7 100644 --- a/cmd/dezhban/config_cmd.go +++ b/cmd/dezhban/config_cmd.go @@ -369,6 +369,30 @@ var configFields = map[string]configField{ return nil }, }, + "vpn.advanced.verifyInterval": { + get: func(c *config.Config) string { + if c.VPN.Advanced.VerifyInterval < 0 { + return "0s" // explicitly disabled + } + return c.VPN.Advanced.VerifyInterval.String() + }, + set: func(c *config.Config, v string) error { + if err := setDuration(&c.VPN.Advanced.VerifyInterval, v); err != nil { + return err + } + if c.VPN.Advanced.VerifyInterval == 0 { + // "0" means enforcement verification is off, not "reset to + // default" — same explicit-opt-out sentinel as the three windows + // and RedialMinUptime. + c.VPN.Advanced.VerifyInterval = config.Disabled + } + return nil + }, + }, + "vpn.advanced.livenessRedial": { + get: func(c *config.Config) string { return strconv.FormatBool(c.VPN.Advanced.LivenessRedial) }, + set: func(c *config.Config, v string) error { return setBool(&c.VPN.Advanced.LivenessRedial, v) }, + }, "vpn.advanced.redialBudget": { get: func(c *config.Config) string { return c.VPN.Advanced.RedialBudget.String() }, set: func(c *config.Config, v string) error { diff --git a/cmd/dezhban/config_roundtrip_test.go b/cmd/dezhban/config_roundtrip_test.go index 47f827c..778400e 100644 --- a/cmd/dezhban/config_roundtrip_test.go +++ b/cmd/dezhban/config_roundtrip_test.go @@ -54,6 +54,8 @@ var roundTripCases = map[string]roundTripCase{ "vpn.advanced.switchWindowMax": {set: "4m", want: "4m0s"}, "vpn.advanced.redialWindowMax": {set: "11m", want: "11m0s"}, "vpn.advanced.redialMinUptime": {set: "20s", want: "20s"}, + "vpn.advanced.verifyInterval": {set: "90s", want: "1m30s"}, + "vpn.advanced.livenessRedial": {set: "true", want: "true"}, "vpn.advanced.redialBudget": {set: "3m", want: "3m0s"}, "vpn.advanced.redialBudgetWindow": {set: "20m", want: "20m0s"}, "vpn.advanced.commandFreshness": {set: "45s", want: "45s"}, diff --git a/cmd/dezhban/doctor_test.go b/cmd/dezhban/doctor_test.go index 182564c..123e5fc 100644 --- a/cmd/dezhban/doctor_test.go +++ b/cmd/dezhban/doctor_test.go @@ -7,10 +7,12 @@ import ( "net/netip" "strings" "testing" + "time" "github.com/behnam-rk/dezhban/internal/config" "github.com/behnam-rk/dezhban/internal/control" "github.com/behnam-rk/dezhban/internal/netdetect" + "github.com/behnam-rk/dezhban/internal/state" ) // The pure formatters (buildTunnelsCheck/buildEndpointsCheck/buildLockoutCheck) @@ -66,6 +68,115 @@ func TestBuildTunnelsCheck(t *testing.T) { }) } +func TestBuildLivenessCheck(t *testing.T) { + t.Run("daemon not running", func(t *testing.T) { + c := buildLivenessCheck(state.Snapshot{}, false) + if c.Status != checkOK { + t.Errorf("status = %q, want %q", c.Status, checkOK) + } + if c.Summary != "not checked — dezhban isn't running." { + t.Errorf("summary = %q", c.Summary) + } + }) + + t.Run("clean snapshot reports OK with no details", func(t *testing.T) { + c := buildLivenessCheck(state.Snapshot{}, true) + if c.Status != checkOK { + t.Errorf("status = %q, want %q", c.Status, checkOK) + } + if len(c.Details) != 0 { + t.Errorf("details = %v, want none", c.Details) + } + }) + + t.Run("verify missing escalates to warn", func(t *testing.T) { + snap := state.Snapshot{Verify: &state.VerifyState{Missing: true, Repairs: 2}} + c := buildLivenessCheck(snap, true) + if c.Status != checkWarn { + t.Errorf("status = %q, want %q", c.Status, checkWarn) + } + if len(c.Details) != 1 || !strings.Contains(c.Details[0], "re-applied 2 time(s)") { + t.Errorf("details = %v", c.Details) + } + }) + + // ExitIPChangedAt is purely observational (CLAUDE.md: "it never flips + // posture and never touches the hysteresis streak") and must not escalate + // Status on its own — a failover between VPN servers in the same allowed + // country is not a lockout risk, just a fact worth surfacing. + t.Run("exit IP change alone stays OK but is reported", func(t *testing.T) { + changedAt := time.Date(2026, 8, 9, 15, 4, 0, 0, time.UTC) + snap := state.Snapshot{ExitIPChangedAt: changedAt} + c := buildLivenessCheck(snap, true) + if c.Status != checkOK { + t.Errorf("status = %q, want %q", c.Status, checkOK) + } + if len(c.Details) != 1 || !strings.Contains(c.Details[0], "Exit IP last changed at") { + t.Errorf("details = %v", c.Details) + } + }) + + t.Run("verify and exit IP change combine under warn", func(t *testing.T) { + snap := state.Snapshot{ + Verify: &state.VerifyState{Err: "permission denied"}, + ExitIPChangedAt: time.Date(2026, 8, 9, 15, 4, 0, 0, time.UTC), + } + c := buildLivenessCheck(snap, true) + if c.Status != checkWarn { + t.Errorf("status = %q, want %q", c.Status, checkWarn) + } + if len(c.Details) != 2 { + t.Errorf("details = %v, want 2 lines", c.Details) + } + }) + + // A verify read failure means doctor has no basis to claim enforcement is + // holding at all — it must say so distinctly, not fold into the generic + // "holding, but something needs attention" summary written for confirmed + // conditions like a completed repair or a zombie diagnosis. + t.Run("verify read error reports could-not-confirm, not holding", func(t *testing.T) { + snap := state.Snapshot{Verify: &state.VerifyState{Err: "permission denied"}} + c := buildLivenessCheck(snap, true) + if c.Status != checkWarn { + t.Errorf("status = %q, want %q", c.Status, checkWarn) + } + if !strings.Contains(c.Summary, "could not confirm") { + t.Errorf("summary = %q, want it to say enforcement could not be confirmed", c.Summary) + } + }) + + // A failed enforcement action (a repair or apply that didn't land) must + // surface even with no Verify state at all — EnforcementErr is its own + // signal, set by the run loop's general apply path, not just verification. + t.Run("enforcement error reports could-not-confirm", func(t *testing.T) { + snap := state.Snapshot{EnforcementErr: "apply: exit status 1"} + c := buildLivenessCheck(snap, true) + if c.Status != checkWarn { + t.Errorf("status = %q, want %q", c.Status, checkWarn) + } + if len(c.Details) != 1 || !strings.Contains(c.Details[0], "apply: exit status 1") { + t.Errorf("details = %v", c.Details) + } + if !strings.Contains(c.Summary, "could not confirm") { + t.Errorf("summary = %q, want it to say enforcement could not be confirmed", c.Summary) + } + }) + + // PanicDisarmed means verification is deliberately suspended — doctor + // must say so by name rather than silently reporting OK while every + // automatic repair path stands down (see runner.Options.PanicDisarmed). + t.Run("panic disarmed reports suspended verification", func(t *testing.T) { + snap := state.Snapshot{PanicDisarmed: true} + c := buildLivenessCheck(snap, true) + if c.Status != checkWarn { + t.Errorf("status = %q, want %q", c.Status, checkWarn) + } + if !strings.Contains(c.Summary, "panic") { + t.Errorf("summary = %q, want it to mention panic", c.Summary) + } + }) +} + func TestBuildControlCheck(t *testing.T) { base := func() config.Config { cfg := config.Default() diff --git a/cmd/dezhban/lock.go b/cmd/dezhban/lock.go new file mode 100644 index 0000000..308a290 --- /dev/null +++ b/cmd/dezhban/lock.go @@ -0,0 +1,14 @@ +package main + +import "errors" + +// ErrRunLockHeld distinguishes genuine single-instance contention — another +// `dezhban run` already holds the lock — from every other reason +// acquireRunLock can fail (an unwritable state directory, a missing parent, +// a permission error). Only the former should refuse to start: the lock is a +// safety NET around Backend.Apply, and its own failure to establish must +// never become a reason the kill switch does not enforce — the same +// principle state.EnsureDir's own tolerated failure already follows for the +// directory underneath it. See acquireRunLock's doc comment in +// lock_unix.go/lock_windows.go for what the lock protects. +var ErrRunLockHeld = errors.New("another dezhban is already running") diff --git a/cmd/dezhban/lock_unix.go b/cmd/dezhban/lock_unix.go new file mode 100644 index 0000000..9689e5f --- /dev/null +++ b/cmd/dezhban/lock_unix.go @@ -0,0 +1,73 @@ +//go:build !windows + +package main + +import ( + "errors" + "fmt" + "path/filepath" + "syscall" +) + +// runLockName is the lock file's name under the state directory. Not tagged +// "dezhban" like the firewall rules (nothing else in this file needs the +// backend's surgical-teardown discipline — it is deleted with the rest of the +// state directory, never parsed, never shared). +const runLockName = "dezhban.lock" + +// acquireRunLock takes an exclusive, non-blocking lock on /dezhban.lock, +// held for the daemon's entire lifetime. It is the guard `panic`, `unblock`, +// and the service-lifecycle commands deliberately do NOT take (they must stay +// usable with no daemon running at all) — only `run` calls this, once, before +// the run loop starts. +// +// Without it, `sudo dezhban run --no-daemon` started beside an already-running +// service gives two processes both calling Backend.Apply — one process each, +// so the "single run-loop goroutine owns every Apply" invariant +// (docs/contribute/architecture.md) holds inside a process but nothing enforced +// it across two. +// +// A raw file descriptor, not *os.File: os.File attaches a GC finalizer that +// closes the fd — and so releases the flock — the moment the wrapper becomes +// unreachable, which can happen before the daemon actually exits since nothing +// here reads the descriptor again. The fd below is intentionally never closed; +// the kernel releases the lock when the process ends, by any means (a clean +// stop, a crash, a SIGKILL), so a killed daemon never leaves the next start +// wedged behind a stale lock. +func acquireRunLock(dir string) error { + _, err := tryRunLock(filepath.Join(dir, runLockName)) + return err +} + +// tryRunLock does the actual open+flock and returns the raw fd on success, so +// tests can acquire and explicitly release a lock to exercise contention — +// acquireRunLock itself never exposes or closes it, by design (see its doc +// comment). Not used by acquireRunLock's own error message, which reports the +// path rather than the fd. +func tryRunLock(path string) (int, error) { + // 0600, not 0644: flock only requires a readable descriptor, so a + // world/group-readable lock file would let any local user hold it open + // (e.g. `flock -x -c 'sleep inf'`) and starve the guard from ever + // starting, including at boot. O_CLOEXEC keeps the descriptor from leaking + // into a backend child (pfctl/nft) that outlives the daemon, which would + // otherwise hold the flock past this process's own exit. + fd, err := syscall.Open(path, syscall.O_CREAT|syscall.O_RDWR|syscall.O_CLOEXEC, 0600) + if err != nil { + return -1, fmt.Errorf("open lock file %s: %w", path, err) + } + // O_CREAT's mode only applies to a newly-created file, so an upgrade from + // a build that created this file 0644 would otherwise keep the looser + // mode forever. Enforce 0600 unconditionally. + if err := syscall.Fchmod(fd, 0600); err != nil { + _ = syscall.Close(fd) + return -1, fmt.Errorf("chmod lock file %s: %w", path, err) + } + if err := syscall.Flock(fd, syscall.LOCK_EX|syscall.LOCK_NB); err != nil { + _ = syscall.Close(fd) + if errors.Is(err, syscall.EWOULDBLOCK) { + return -1, fmt.Errorf("%w (holds %s) — see `dezhban status`", ErrRunLockHeld, path) + } + return -1, fmt.Errorf("lock %s: %w", path, err) + } + return fd, nil +} diff --git a/cmd/dezhban/lock_unix_test.go b/cmd/dezhban/lock_unix_test.go new file mode 100644 index 0000000..8d730c4 --- /dev/null +++ b/cmd/dezhban/lock_unix_test.go @@ -0,0 +1,123 @@ +//go:build !windows + +package main + +import ( + "errors" + "os" + "path/filepath" + "syscall" + "testing" +) + +// The single-instance guard has one job: a second `dezhban run` against the +// same state directory must refuse, and a released lock must let the next one +// through. Both are exercised directly against the fd, not through +// acquireRunLock — which deliberately never exposes or closes what it holds +// (see its doc comment) — via the tryRunLock test seam. + +func TestRunLockRefusesASecondHolder(t *testing.T) { + path := filepath.Join(t.TempDir(), runLockName) + + fd1, err := tryRunLock(path) + if err != nil { + t.Fatalf("first lock: %v", err) + } + defer syscall.Close(fd1) + + if _, err := tryRunLock(path); err == nil { + t.Fatal("second lock on the same path succeeded; want refusal") + } +} + +func TestRunLockAvailableAfterRelease(t *testing.T) { + path := filepath.Join(t.TempDir(), runLockName) + + fd1, err := tryRunLock(path) + if err != nil { + t.Fatalf("first lock: %v", err) + } + if err := syscall.Close(fd1); err != nil { + t.Fatalf("release: %v", err) + } + + fd2, err := tryRunLock(path) + if err != nil { + t.Fatalf("lock after release: %v", err) + } + defer syscall.Close(fd2) +} + +// acquireRunLock is the production entry point: same guarantee, exercised end +// to end (directory → path → open → flock) rather than against a raw path. +func TestAcquireRunLockRefusesASecondHolder(t *testing.T) { + dir := t.TempDir() + + if err := acquireRunLock(dir); err != nil { + t.Fatalf("first acquire: %v", err) + } + if err := acquireRunLock(dir); err == nil { + t.Fatal("second acquire on the same directory succeeded; want refusal") + } +} + +// A second holder's error must be identifiable as genuine contention via +// errors.Is(err, ErrRunLockHeld) — cmdRun uses exactly that check to decide +// between refusing to start and logging a warning and continuing anyway. +func TestAcquireRunLockSecondHolderIsErrRunLockHeld(t *testing.T) { + dir := t.TempDir() + + if err := acquireRunLock(dir); err != nil { + t.Fatalf("first acquire: %v", err) + } + err := acquireRunLock(dir) + if err == nil { + t.Fatal("second acquire on the same directory succeeded; want refusal") + } + if !errors.Is(err, ErrRunLockHeld) { + t.Fatalf("second acquire error = %v, want errors.Is(err, ErrRunLockHeld)", err) + } +} + +// Any local user must not be able to flock the lock file open and starve the +// guard from starting: 0644 (readable by everyone) allowed exactly that. +func TestRunLockFileIsOwnerOnly(t *testing.T) { + path := filepath.Join(t.TempDir(), runLockName) + + fd, err := tryRunLock(path) + if err != nil { + t.Fatalf("lock: %v", err) + } + defer syscall.Close(fd) + + fi, err := os.Stat(path) + if err != nil { + t.Fatalf("stat: %v", err) + } + if got := fi.Mode().Perm(); got != 0600 { + t.Fatalf("lock file mode = %o, want 0600", got) + } +} + +// An upgrade from a build that created this file 0644 must not leave it that +// way forever — O_CREAT's mode argument only applies to a brand-new file. +func TestRunLockFileModeFixedOnPreExistingFile(t *testing.T) { + path := filepath.Join(t.TempDir(), runLockName) + if err := os.WriteFile(path, nil, 0644); err != nil { + t.Fatalf("seed pre-existing 0644 file: %v", err) + } + + fd, err := tryRunLock(path) + if err != nil { + t.Fatalf("lock: %v", err) + } + defer syscall.Close(fd) + + fi, err := os.Stat(path) + if err != nil { + t.Fatalf("stat: %v", err) + } + if got := fi.Mode().Perm(); got != 0600 { + t.Fatalf("lock file mode = %o, want 0600 (fixed from pre-existing 0644)", got) + } +} diff --git a/cmd/dezhban/lock_windows.go b/cmd/dezhban/lock_windows.go new file mode 100644 index 0000000..60de8af --- /dev/null +++ b/cmd/dezhban/lock_windows.go @@ -0,0 +1,165 @@ +//go:build windows + +package main + +import ( + "fmt" + "hash/fnv" + "syscall" + "unsafe" +) + +var ( + modkernel32 = syscall.NewLazyDLL("kernel32.dll") + modadvapi32 = syscall.NewLazyDLL("advapi32.dll") + + procCreateMutex = modkernel32.NewProc("CreateMutexW") + procCreateBoundaryDescriptor = modkernel32.NewProc("CreateBoundaryDescriptorW") + procAddSIDToBoundary = modkernel32.NewProc("AddSIDToBoundaryDescriptor") + procCreatePrivateNamespace = modkernel32.NewProc("CreatePrivateNamespaceW") + procOpenPrivateNamespace = modkernel32.NewProc("OpenPrivateNamespaceW") + procCreateWellKnownSid = modadvapi32.NewProc("CreateWellKnownSid") +) + +// Well-known SID types (winnt.h WELL_KNOWN_SID_TYPE) admitted into the +// boundary descriptor below. Both are needed: `run` requires an +// elevated/admin context (requireRoot) for an interactive shell, but +// kardianos/service commonly runs the installed service as LocalSystem — a +// distinct SID from BUILTIN\Administrators — and the guard is meant to hold +// across both (a service-manager session and an interactive admin shell), +// not just one. +const ( + winLocalSystemSidType = 22 + winBuiltinAdministratorsSidType = 26 + // securityMaxSidSize is SECURITY_MAX_SID_SIZE from the Windows SDK — the + // buffer CreateWellKnownSid requires to guarantee success. + securityMaxSidSize = 68 +) + +const ( + runLockBoundaryName = `dezhban-run-boundary` + runLockNamespaceAlias = `dezhban-run-ns` +) + +// wellKnownSid returns the binary SID for a WELL_KNOWN_SID_TYPE. +func wellKnownSid(sidType uintptr) ([]byte, error) { + buf := make([]byte, securityMaxSidSize) + cb := uint32(len(buf)) + ret, _, callErr := procCreateWellKnownSid.Call( + sidType, + 0, // DomainSid: NULL — not needed for either SID used here + uintptr(unsafe.Pointer(&buf[0])), + uintptr(unsafe.Pointer(&cb)), + ) + if ret == 0 { + return nil, fmt.Errorf("create well-known sid: %w", callErr) + } + return buf[:cb], nil +} + +// runLockNamespacePrefix builds (or, on a race with another dezhban process, +// opens) a boundary-restricted private namespace that only a process +// carrying the LocalSystem or BUILTIN\Administrators SID can create or open +// objects in, and returns the "alias\" prefix objects must use to live +// inside it. This is the documented Windows mitigation for named-object +// squatting ("Preventing Squatting Attacks" in Microsoft's kernel object +// namespaces docs): a predictable Global\ name can be pre-created by an +// unprivileged local process, which CreateMutexW then either silently opens +// (letting that process masquerade as the lock holder) or fails to open at +// all if the squatter's DACL denies us — either way blocking the privileged +// daemon from ever starting, across service restarts, until an admin +// intervenes. +// +// Returns "" if any step fails, so the caller can fall back to the +// unhardened Global\ name — degrading to the pre-hardening behavior rather +// than blocking the daemon from starting over an environment (SID lookup +// failure, a namespace API removed by some future Windows edition) this +// code didn't anticipate. +func runLockNamespacePrefix() string { + boundaryUTF16, err := syscall.UTF16PtrFromString(runLockBoundaryName) + if err != nil { + return "" + } + bd, _, _ := procCreateBoundaryDescriptor.Call(uintptr(unsafe.Pointer(boundaryUTF16)), 0) + if bd == 0 { + return "" + } + + for _, sidType := range []uintptr{winLocalSystemSidType, winBuiltinAdministratorsSidType} { + sid, err := wellKnownSid(sidType) + if err != nil { + return "" + } + ret, _, _ := procAddSIDToBoundary.Call(uintptr(unsafe.Pointer(&bd)), uintptr(unsafe.Pointer(&sid[0]))) + if ret == 0 { + return "" + } + } + + aliasUTF16, err := syscall.UTF16PtrFromString(runLockNamespaceAlias) + if err != nil { + return "" + } + ns, _, callErr := procCreatePrivateNamespace.Call(0, bd, uintptr(unsafe.Pointer(aliasUTF16))) + if ns == 0 { + // Another dezhban process (or a prior run) already created this + // namespace — open it instead of creating a second one. + if errno, ok := callErr.(syscall.Errno); ok && errno == syscall.ERROR_ALREADY_EXISTS { + ns, _, _ = procOpenPrivateNamespace.Call(bd, uintptr(unsafe.Pointer(aliasUTF16))) + } + if ns == 0 { + return "" + } + } + return runLockNamespaceAlias + `\` +} + +// acquireRunLock takes a named Windows mutex for the daemon's entire lifetime +// — the Windows twin of the Unix flock in lock_unix.go; see that file's doc +// comment for why this exists and what it guards. +// +// The name is derived from dir so that two callers pointed at different +// directories don't contend with each other, matching the Unix +// implementation's per-directory scoping — useful for tests +// (lock_windows_test.go exercises this directly). In production the only +// caller always passes stateDir(), a fixed OS path unaffected by +// $DEZHBAN_CONFIG or --config (those select the config *file*, not the +// state *directory*), so all `dezhban run` invocations on a host contend on +// the same lock regardless of which config each used — correct, since they +// would otherwise fight over the same firewall backend. +// +// The object lives inside runLockNamespacePrefix's hardened private +// namespace when available, falling back to a plain Global\ name otherwise. +// Global\, not a session-local name either way: the guard is meant to hold +// across sessions (a service-manager session and an interactive admin +// shell), not just within one. +// +// The handle returned by CreateMutexW is intentionally never closed. Windows +// releases a mutex, and the OS reclaims its handle, when the owning process +// exits by any means — a crashed or killed daemon never leaves this locked. +func acquireRunLock(dir string) error { + h := fnv.New64a() + _, _ = h.Write([]byte(dir)) + mutexName := fmt.Sprintf("dezhban-run-%x", h.Sum64()) + + prefix := runLockNamespacePrefix() + if prefix == "" { + prefix = `Global\` + } + + name, err := syscall.UTF16PtrFromString(prefix + mutexName) + if err != nil { + return fmt.Errorf("lock name: %w", err) + } + ret, _, callErr := procCreateMutex.Call(0, 0, uintptr(unsafe.Pointer(name))) + if ret == 0 { + return fmt.Errorf("create run-lock mutex: %w", callErr) + } + // CreateMutexW always sets last-error even on success (ERROR_SUCCESS); + // ERROR_ALREADY_EXISTS specifically means another process already owns + // this name, which for a still-live process means it is still running. + if errno, ok := callErr.(syscall.Errno); ok && errno == syscall.ERROR_ALREADY_EXISTS { + return fmt.Errorf("%w against this state directory — see `dezhban status`", ErrRunLockHeld) + } + return nil +} diff --git a/cmd/dezhban/lock_windows_test.go b/cmd/dezhban/lock_windows_test.go new file mode 100644 index 0000000..85694c2 --- /dev/null +++ b/cmd/dezhban/lock_windows_test.go @@ -0,0 +1,48 @@ +//go:build windows + +package main + +import ( + "errors" + "testing" +) + +// Unlike lock_unix_test.go, there is no exposed seam here comparable to +// tryRunLock: acquireRunLock's doc comment explains why the handle returned by +// CreateMutexW is deliberately never closed (the OS reclaims it on process +// exit), so there is nothing to release and re-acquire within a single test +// process. Only the production entry point's refusal is testable in-process — +// a second CreateMutexW call against the same name, even from the same +// process, sets ERROR_ALREADY_EXISTS, which is exactly the ownership question +// this guard cares about. +func TestAcquireRunLockRefusesASecondHolder(t *testing.T) { + dir := t.TempDir() + + if err := acquireRunLock(dir); err != nil { + t.Fatalf("first acquire: %v", err) + } + err := acquireRunLock(dir) + if err == nil { + t.Fatal("second acquire on the same directory succeeded; want refusal") + } + // cmdRun uses errors.Is(err, ErrRunLockHeld) to distinguish genuine + // contention (refuse to start) from every other lock failure (warn and + // continue) — see lock.go's doc comment. + if !errors.Is(err, ErrRunLockHeld) { + t.Fatalf("second acquire error = %v, want errors.Is(err, ErrRunLockHeld)", err) + } +} + +// Two different state directories must never contend with each other — the +// mutex name is derived from the directory, matching the Unix flock's +// per-path scoping (see acquireRunLock's doc comment). +func TestAcquireRunLockDoesNotContendAcrossDirectories(t *testing.T) { + dir1, dir2 := t.TempDir(), t.TempDir() + + if err := acquireRunLock(dir1); err != nil { + t.Fatalf("acquire dir1: %v", err) + } + if err := acquireRunLock(dir2); err != nil { + t.Fatalf("acquire dir2 should not contend with dir1's lock: %v", err) + } +} diff --git a/cmd/dezhban/main.go b/cmd/dezhban/main.go index a631710..7c3ff9f 100644 --- a/cmd/dezhban/main.go +++ b/cmd/dezhban/main.go @@ -313,6 +313,47 @@ func cmdRun(args []string) int { return 1 } + // Single-instance guard: nothing about `--no-daemon` or a bare `run` stops + // two copies of this process calling Backend.Apply at once, and the + // "single run-loop goroutine owns every Apply" invariant + // (docs/contribute/architecture.md) is a per-process guarantee that + // enforces nothing across a second process. `panic`, `unblock`, and the + // service-lifecycle commands deliberately do NOT take this lock — they are + // the escape hatch and must stay usable with no daemon running. + // + // This EnsureDir is the one call for the whole `run` invocation — the + // Builder below is documented (internal/svc.Builder) to run exactly once, + // so assembleOptions takes the result rather than re-establishing the + // directory itself. + stateDirErr := state.EnsureDir(stateDir()) + if stateDirErr != nil { + log.Warn("state directory not reachable; the single-instance lock will still be attempted", "err", stateDirErr) + } + if err := acquireRunLock(stateDir()); err != nil { + // Only genuine contention — another dezhban already holds the lock — + // is a reason to refuse to start: two daemons both calling + // Backend.Apply is the exact race this lock exists to prevent. Any + // other failure (the state directory above being unwritable, a full + // disk, a permission error) must degrade to "no single-instance + // protection this run", never to "the kill switch does not start" — + // the same principle EnsureDir's own tolerated failure follows for + // the directory underneath it. + if errors.Is(err, ErrRunLockHeld) { + fmt.Fprintln(os.Stderr, err) + return 1 + } + log.Warn("single-instance lock unavailable; continuing without it — enforcement is not gated on this lock", "err", err) + } + + // A fresh daemon start already re-applies the initial posture + // unconditionally below (runGuard's startup Apply), so any panic-disarm + // marker left over from a PRIOR run has done its job — clear it now, or + // it would silently suppress this run's own enforcement verification + // forever, until someone thought to run `dezhban unblock`. + clearPanicMarkerBestEffort(stateDir(), func(err error) { + log.Debug("clear panic-disarm marker failed", "err", err) + }) + // Persistent log capture, always on: every daemon run appends to // /logs/dezhban.log (size-rotated), whether launched from a shell // or by the service manager — stderr is lost when the shell closes and the @@ -335,7 +376,7 @@ func cmdRun(args []string) int { // platform logger. The build closure assembles the run loop lazily so it can // use whichever logger the service selects. build := func(l *slog.Logger) (runner.Options, error) { - return assembleOptions(cfg, resolveConfigPath(*cfgPath), l, ov) + return assembleOptions(cfg, resolveConfigPath(*cfgPath), l, ov, stateDirErr) } if err := svc.Run(build, log, effectiveLevel(cfg), *cfgPath, persist); err != nil { log.Error("run loop failed", "err", err) @@ -374,15 +415,19 @@ func parseOverrides(simCountry, simTunDown string) (runOverrides, error) { // live reload re-reads exactly the same file, rather than re-running resolution // and possibly landing on a different one mid-run. Empty means built-in // defaults, which is a config that cannot change, so reloading is not offered. -func assembleOptions(cfg *config.Config, cfgPath string, log *slog.Logger, ov runOverrides) (runner.Options, error) { - // Everything the daemon publishes to the outside world lives under this one - // directory — state.json for the menubar app, control.sock for passwordless - // routine ops. It must be traversable by the unprivileged user or both silently - // stop working, so establish (and repair) its mode once, here, before anything - // writes into it. Non-fatal: a stale mode degrades observability, it must never - // stop the kill switch from enforcing. - if err := state.EnsureDir(stateDir()); err != nil { - log.Warn("state directory not reachable by unprivileged readers; the menubar app and control socket may not work", "err", err) +// stateDirErr is the result of establishing (and repairing) the state +// directory's mode, already done once by the caller before this runs — +// internal/svc.Builder documents that the Builder this feeds is called exactly +// once per process, so a second state.EnsureDir here would just repeat the same +// mkdir/chmod for no new information. Everything the daemon publishes to the +// outside world lives under that one directory — state.json for the menubar +// app, control.sock for passwordless routine ops — and it must be traversable +// by the unprivileged user or both silently stop working; a stale mode is +// non-fatal and must never stop the kill switch from enforcing, hence a +// warning here rather than an aborted startup. +func assembleOptions(cfg *config.Config, cfgPath string, log *slog.Logger, ov runOverrides, stateDirErr error) (runner.Options, error) { + if stateDirErr != nil { + log.Warn("state directory not reachable by unprivileged readers; the menubar app and control socket may not work", "err", stateDirErr) } providers := monitor.ProvidersFromURLs(cfg.Providers, log) @@ -416,7 +461,10 @@ func assembleOptions(cfg *config.Config, cfgPath string, log *slog.Logger, ov ru if _, lerr := learned.Load(learnedPath); lerr != nil { log.Warn("learned endpoints store unreadable; starting empty", "err", lerr) } - epSrc.Learned = func() []netip.Addr { + // A plain func value, not a method — reused below to give the startup + // self-test's own endpoint source (never epSrc itself) the same + // fresh-from-disk behavior without a second copy of the parsing logic. + loadLearnedEndpoints := func() []netip.Addr { store, lerr := learned.Load(learnedPath) if lerr != nil { log.Debug("learned endpoints reload failed; skipping this cycle", "err", lerr) @@ -435,6 +483,7 @@ func assembleOptions(cfg *config.Config, cfgPath string, log *slog.Logger, ov ru } return out } + epSrc.Learned = loadLearnedEndpoints learnHook := func(profile, iface string, addrs []netip.Addr) { // Reload before mutating so a concurrent forget/edit is merged with, not // overwritten by, the new entry (Load returns a usable empty store on error). @@ -598,6 +647,89 @@ func assembleOptions(cfg *config.Config, cfgPath string, log *slog.Logger, ov ru } } + // Startup self-test: one Info line summarizing whether the pieces this + // daemon depends on are actually reachable. Diagnostic only, like CVG's + // equivalent — every enforcement decision downstream is fail-closed + // regardless of what this reports. + // + // Backgrounded, not inline: fw.IsBlocked() shells out (pfctl/nft/WFP — + // up to psTimeout on Windows) and an endpoint resolve can hit the network, + // and assembleOptions runs before runner.Run's first Apply(guard), so + // doing either synchronously here would directly delay that boot-time + // apply — undermining ADR-0008's promise to close the post-reboot leak + // window immediately. Logged from its own goroutine instead, arriving a + // beat after "run loop started" rather than gating it. + // + // Uses its OWN backend and endpoint-source instances — never the `fw`/ + // `epSrc` handed to runner.Options below — so this goroutine cannot touch + // the objects the run loop's single goroutine owns, per that invariant + // (see epSrc.Learned's doc comment above). Both constructors are cheap: + // firewall.New() and buildEndpointSource() build stateless value types, + // so a second instance costs nothing and keeps the invariant true by + // construction rather than by the backends happening to have no fields + // today. + selftestFW, selftestFWErr := firewall.New() + // withDiscovery is gated on darwin rather than passed as a bare `true`: + // buildEndpointSource only wires a Discover on darwin, and on every other + // OS a `true` here does nothing except re-emit its "live discovery is only + // supported on macOS" warning — a second copy of a line the epSrc above + // already logged, at every daemon start, for a condition that has not + // changed between the two calls. + selftestEpSrc := buildEndpointSource(cfg, log, tunnels, runtime.GOOS == "darwin") + selftestEpSrc.Learned = loadLearnedEndpoints + go func() { + backendReachable := selftestFWErr == nil + if backendReachable { + if _, err := selftestFW.IsBlocked(); err != nil { + backendReachable = false + } + } + + // EnsureDir (called once by the caller before this) only creates, + // stats, and repairs the directory's mode — it never attempts a + // write. A directory that already existed with the right bits keeps + // reporting writable even after going read-only underneath it (a + // remount, a quota, an ACL change), so probe an actual write rather + // than trust stateDirErr alone. + // Reuses elevate.go's pathWritable rather than a second inline probe: + // a nonexistent name under stateDir() hits its IsNotExist branch, + // which walks up to the nearest existing ancestor and probes with + // os.CreateTemp's random suffix — unlike a fixed probe filename, that + // can't collide if something else probes the same directory + // concurrently. + stateDirWritable := stateDirErr == nil + if stateDirWritable { + stateDirWritable = pathWritable(filepath.Join(stateDir(), ".selftest-write")) + } + + // cfg.VPN.AutoDiscoverEndpoints==true only means discovery is + // configured to run — it says nothing about whether it actually + // found anything on this host. Resolve for real (safe here, off the + // critical path) so this reports the endpoint set that exists, not + // just the intent to look for one. + // + // EffectiveEndpoints, not cfg.VPN.Endpoints: the union of the flat list + // and every profile's own endpoints is what buildEndpointSource (and so + // the guard itself) actually resolves. Reading the flat list alone + // reported endpointsKnown=false for a profile-only config — a host that + // is fully configured — which is exactly the wrong answer from a check + // whose whole job is telling an operator whether the pieces are there. + endpointCount := len(config.EffectiveEndpoints(cfg, nil)) + if cfg.VPN.AutoDiscoverEndpoints { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + endpointCount = len(selftestEpSrc.Resolve(ctx).Addrs) + cancel() + } + + log.Info("startup self-test", + "firewallBackendReachable", backendReachable, + "stateDirWritable", stateDirWritable, + "tunnelsConfiguredOrDetected", len(tunnels) > 0, + "endpointsKnown", endpointCount > 0, + "tunnelEverUpOnThisHost", armedRec.TunnelEverUp, + ) + }() + return runner.Options{ Monitor: mon, Decider: decision.New(cfg.BlockedCountries, cfg.Hysteresis), @@ -624,6 +756,10 @@ func assembleOptions(cfg *config.Config, cfgPath string, log *slog.Logger, ov ru }, EndpointRefresh: cfg.VPN.EndpointRefresh, EndpointGrace: cfg.VPN.EndpointGrace, + VerifyInterval: adv.VerifyInterval, + PanicDisarmed: func() bool { return panicMarkerPresent(stateDir()) }, + ClearPanicDisarm: func() error { return clearPanicMarker(stateDir()) }, + LivenessRedial: adv.LivenessRedial, AutoArm: cfg.VPN.AutoArm, ArmAtBoot: armAtBoot, TunnelEverUp: armedRec.TunnelEverUp, @@ -699,6 +835,8 @@ func liveSettingsFrom(cfg *config.Config) runner.LiveSettings { WindowDiscoveryInterval: adv.WindowDiscoveryInterval, EndpointRefresh: cfg.VPN.EndpointRefresh, EndpointGrace: cfg.VPN.EndpointGrace, + VerifyInterval: adv.VerifyInterval, + LivenessRedial: adv.LivenessRedial, AllowSwitchOps: cfg.Control.AllowSwitchOps, AllowPauseOps: cfg.Control.AllowPauseOps, AllowConfigOps: cfg.Control.AllowConfigOps, @@ -891,6 +1029,15 @@ func cmdBlock(args []string) int { log.Info("network full-blocked (vpn)", "tunnels", d.Tunnels) } } + // This path runs as root with no daemon involved (or bypassing one via + // --force/--guard), so it clears the panic-disarm marker itself, mirroring + // cmdUnblock — a direct block is just as much an explicit operator engaging + // with enforcement as an explicit unblock is, and leaving a stale marker + // here would keep any co-running daemon's enforcement verification + // suspended even after real rules were just installed by hand. + clearPanicMarkerBestEffort(stateDir(), func(err error) { + fmt.Fprintln(os.Stderr, "block: warning — could not clear the panic-disarm marker:", err) + }) return 0 } @@ -1051,6 +1198,13 @@ func cmdUnblock(args []string) int { fmt.Fprintln(os.Stderr, "unblock failed:", err) return 1 } + // This path runs as root with no daemon involved (or bypassing one via + // --force), so it clears the panic-disarm marker itself — the + // control-socket path instead asks the running daemon to clear it (see + // runner.Options.ClearPanicDisarm), since that path may run unprivileged. + clearPanicMarkerBestEffort(stateDir(), func(err error) { + fmt.Fprintln(os.Stderr, "unblock: warning — could not clear the panic-disarm marker:", err) + }) fmt.Println("dezhban: network unblocked") return 0 } @@ -1072,6 +1226,23 @@ func cmdPanic(args []string) int { fmt.Fprintln(os.Stderr, "firewall backend unavailable:", err) return 1 } + // Tell a daemon that might still be running (this command is deliberately + // daemon-independent, so there is no other way to reach it) to stand its + // enforcement verification down — otherwise it would notice the rules + // missing on its next VerifyInterval tick and silently put them back, + // turning this escape hatch into a brief flicker. Set BEFORE Cleanup, not + // after: every automatic path a live daemon might take (verifyC's tick, + // reapplyStanding, applyWindowPolicy, reapplyCurrent) checks the marker + // first and only THEN reads the firewall — marker-after-Cleanup left a + // window where such a tick could land between the two calls, see the + // rules already gone, find no marker yet, and silently reinstate them. + // Best-effort: a failure to write the marker must never abort the + // teardown below, which is the half of this command that actually + // matters. + if err := setPanicMarker(stateDir()); err != nil { + fmt.Fprintln(os.Stderr, "panic: warning — could not record the teardown; if dezhban is still "+ + "running, its enforcement verification may re-apply the rules within a minute:", err) + } // Cleanup is best-effort and idempotent: it restores any saved prior state // (e.g. pf) and removes dezhban's rules whether or not a daemon owns them. if err := fw.Cleanup(); err != nil { @@ -1875,6 +2046,89 @@ func buildArmAtBootCheck(armAtBoot bool, haveTunnel bool, rec *armed.Record, loa return c } +// buildLivenessCheck reports the enforcement-diagnostic conditions the run +// loop tracks between polls but doctor cannot recompute on its own — a missing +// ruleset (Verify), a tunnel that reports up but is not passing traffic +// (Zombie), and the exit IP's last observed change (ExitIPChangedAt) — from +// the running daemon's own last-published snapshot. Pure: takes the snapshot +// and liveness already resolved by the caller, same shape as buildServiceCheck. +// +// All three are read-only diagnoses, not lockout risks — none moves the exit +// code — so this stays informational like the service and arm-at-boot checks. +// A stale or absent snapshot says nothing (the daemon isn't running or hasn't +// published yet, which buildServiceCheck already reports); it does not read as +// "everything is fine". +func buildLivenessCheck(snap state.Snapshot, daemonLive bool) doctorCheck { + c := doctorCheck{Name: "liveness", Status: checkOK, Summary: "OK"} + if !daemonLive { + c.Summary = "not checked — dezhban isn't running." + return c + } + var lines []string + // cannotConfirm distinguishes "enforcement is holding, but something + // needs attention" (a repair happened, or a diagnosis needs a look) from + // genuinely not knowing: an unreadable backend or a failed enforcement + // action means doctor has no basis to claim the firewall is holding at + // all, so it must not say so. + cannotConfirm := false + if snap.Verify != nil { + // The three cases below are exhaustive over what the run loop publishes + // (state.VerifyState's own doc comment): rules missing and repaired, + // rules missing and the repair FAILED (Missing with Err — the host is + // unenforced), or the backend unreadable (Err alone, nothing changed). + switch { + case snap.Verify.Missing && snap.Verify.Err != "": + lines = append(lines, fmt.Sprintf( + "Firewall rules were found missing and could NOT be re-applied: %s — "+ + "this host is not being guarded right now.", snap.Verify.Err)) + cannotConfirm = true + case snap.Verify.Missing: + lines = append(lines, fmt.Sprintf( + "Firewall rules were found missing and re-applied %d time(s) since startup — "+ + "something on this host keeps removing them.", snap.Verify.Repairs)) + case snap.Verify.Err != "": + lines = append(lines, fmt.Sprintf("Could not read the firewall to verify enforcement: %s", snap.Verify.Err)) + cannotConfirm = true + } + } + if snap.EnforcementErr != "" { + lines = append(lines, fmt.Sprintf( + "The last enforcement action failed: %s — dezhban could not confirm the firewall is enforcing.", + snap.EnforcementErr)) + cannotConfirm = true + } + if snap.Zombie != nil { + lines = append(lines, fmt.Sprintf( + "Tunnel interface reports up, but %d consecutive exit checks through it have failed — "+ + "it may need reconnecting. Guard holds either way; this is diagnosis, not a leak.", snap.Zombie.Checks)) + } + switch { + case snap.PanicDisarmed: + c.Status = checkWarn + c.Summary = "`dezhban panic` tore the rules down and dezhban has not re-armed — enforcement verification is suspended." + lines = append(lines, "Run `dezhban unblock` (or restart dezhban) to resume automatic enforcement repair.") + case cannotConfirm: + c.Status = checkWarn + c.Summary = "could not confirm enforcement is holding — see details." + case len(lines) > 0: + c.Status = checkWarn + c.Summary = "enforcement is holding, but something needs attention." + } + // Purely observational — never flips Status — since a legitimate failover + // between two VPN servers in the same allowed country changes nothing + // CountryCode reports, but changes this. It is the one signal that + // explains "my exit flapped" when the blocked-country check alone would + // say nothing happened. + if !snap.ExitIPChangedAt.IsZero() { + lines = append(lines, fmt.Sprintf( + "Exit IP last changed at %s (a failover between VPN servers in the same "+ + "country would show here even though the blocked-country check saw no change).", + snap.ExitIPChangedAt.Local().Format(time.RFC1123))) + } + c.Details = lines + return c +} + // buildEndpointRetentionCheck reports on the learned-endpoint store, which is // what lets a dropped tunnel redial with no window at all: the guard passes // known server addresses on the physical link, so a drop whose endpoint is still @@ -2024,6 +2278,7 @@ func runDoctor(cfg *config.Config, log *slog.Logger, discover bool) doctorReport snap, snapErr := state.Read(defaultStatePath()) daemonLive := snapErr == nil && !render.IsStale(snap, now) checks = append(checks, buildServiceCheck(svc.Boot(), daemonLive)) + checks = append(checks, buildLivenessCheck(snap, daemonLive)) armedPath := defaultArmedPath() armedRec, armedErr := armed.Load(armedPath) @@ -2110,6 +2365,7 @@ var unattendedSections = []struct{ name, heading string }{ {"service", "boot service"}, {"armAtBoot", "arm at boot"}, {"endpointRetention", "learned endpoints"}, + {"liveness", "enforcement liveness"}, } // sectionedChecks names every check printDoctor has a hand-written section for. @@ -2119,7 +2375,7 @@ var unattendedSections = []struct{ name, heading string }{ // instead of being appended, unformatted, after `discover`. var sectionedChecks = []string{ "config", "tunnels", "endpoints", "lockout", - "service", "armAtBoot", "endpointRetention", + "service", "armAtBoot", "endpointRetention", "liveness", "control", "touchID", "discover", } @@ -2190,9 +2446,10 @@ func printDoctor(r doctorReport) { } fmt.Println() - // The three "will this need me again" checks share one shape — heading, - // summary, details, fixes — so they share one printer rather than three - // copies that would drift apart the first time one of them grew a line. + // The "will this need me again" / "is enforcement actually holding" checks + // share one shape — heading, summary, details, fixes — so they share one + // printer rather than one copy per check that would drift apart the first + // time one of them grew a line. for _, s := range unattendedSections { c, ok := get(s.name) if !ok { diff --git a/cmd/dezhban/panicmark.go b/cmd/dezhban/panicmark.go new file mode 100644 index 0000000..2811f74 --- /dev/null +++ b/cmd/dezhban/panicmark.go @@ -0,0 +1,92 @@ +package main + +import ( + "fmt" + "os" + "path/filepath" + "time" + + "github.com/behnam-rk/dezhban/internal/atomicfile" +) + +// panicMarkerName is the file whose presence tells a RUNNING daemon that +// `dezhban panic` tore down the firewall rules deliberately, and enforcement +// verification (vpn.advanced.verifyInterval) must stand down instead of +// silently re-applying the posture the operator just removed on purpose — +// see runner.Options.PanicDisarmed's doc comment for the full rationale and +// docs/usage/troubleshooting.md for the operator-facing workflow. +// +// Not tagged "dezhban" like the firewall rules (cmd/dezhban/lock_unix.go's +// runLockName follows the same convention for the same reason) — it lives in +// the state directory, deleted with the rest of it, never parsed by the +// firewall backend. +const panicMarkerName = "panic.marker" + +func panicMarkerPath(dir string) string { + return filepath.Join(dir, panicMarkerName) +} + +// setPanicMarker records that panic ran. Root-owned and 0600: a marker any +// local user could create would let them suppress verification's self-healing +// after something else — accidentally or maliciously — removed the rules, +// which is exactly the silent-failure mode verification exists to close. +// Best-effort — a failure here must never fail `panic` itself, since the +// teardown it protects is the half of the command that actually matters. +// +// Atomic (temp + fsync + rename), same convention as internal/armed's Save +// and learned.json for this class of daemon-owned state-dir file: a crash +// mid-write must never leave the marker at a looser mode than 0600, or +// missing content a future caller starts relying on. +func setPanicMarker(dir string) error { + body := []byte("panic ran at " + time.Now().UTC().Format(time.RFC3339) + "\n") + if err := atomicfile.Write(panicMarkerPath(dir), body, 0o600); err != nil { + return fmt.Errorf("panic marker: write: %w", err) + } + return nil +} + +// clearPanicMarker removes the marker — called when an operator explicitly +// asks to resume enforcement (`unblock`) or a fresh daemon start re-arms on +// its own (a restart already re-applies the initial posture unconditionally, +// so a marker surviving past it would silently suppress verification forever +// until someone thought to run `unblock`). Clearing a marker that was never +// set is not an error. +func clearPanicMarker(dir string) error { + err := os.Remove(panicMarkerPath(dir)) + if err != nil && !os.IsNotExist(err) { + return err + } + return nil +} + +// clearPanicMarkerBestEffort clears the marker and reports a failure through +// warn, never through a returned error — every caller treats this clear as +// best-effort (see clearPanicMarker's doc comment) but each logs the failure +// through its own surface (structured daemon log vs. CLI stderr), so the +// logging call is left to warn rather than fixed here. +// +// internal/runner/runner.go has its own clearPanicDisarmBestEffort with the +// same shape — not merged with this one. That side never sees dir or +// clearPanicMarker directly (it only holds the injected +// runner.Options.ClearPanicDisarm), and this is a main package, which +// nothing else can import — see that function's doc comment for the full +// reasoning. +func clearPanicMarkerBestEffort(dir string, warn func(err error)) { + if err := clearPanicMarker(dir); err != nil { + warn(err) + } +} + +// panicMarkerPresent reports whether panic's marker is currently set. Only a +// definite os.IsNotExist counts as absent — any other stat error (a +// transient I/O or permission blip) is indistinguishable from "still there" +// and must not be read as absent, or a running daemon's enforcement +// verification would silently re-apply the very posture panic just tore +// down. +func panicMarkerPresent(dir string) bool { + _, err := os.Stat(panicMarkerPath(dir)) + if err == nil { + return true + } + return !os.IsNotExist(err) +} diff --git a/cmd/dezhban/reload_test.go b/cmd/dezhban/reload_test.go index 6443a03..d36274e 100644 --- a/cmd/dezhban/reload_test.go +++ b/cmd/dezhban/reload_test.go @@ -37,6 +37,8 @@ func TestLiveSettingsFromMapsEveryField(t *testing.T) { cfg.VPN.Advanced.RedialBudget = 2 * time.Minute cfg.VPN.Advanced.RedialBudgetWindow = 15 * time.Minute cfg.VPN.Advanced.WindowDiscoveryInterval = time.Second + cfg.VPN.Advanced.VerifyInterval = time.Minute + cfg.VPN.Advanced.LivenessRedial = true got := reflect.ValueOf(liveSettingsFrom(&cfg)) typ := got.Type() diff --git a/docs/adr/0010-tunnel-liveness.md b/docs/adr/0010-tunnel-liveness.md new file mode 100644 index 0000000..1a00409 --- /dev/null +++ b/docs/adr/0010-tunnel-liveness.md @@ -0,0 +1,181 @@ +# ADR-0010: Zombie-tunnel detection is unconditional; acting on it is opt-in + +**Date**: 2026-08-02 +**Status**: accepted, implemented +**Deciders**: Behnam RK + +## Context + +`isTunnelIface` (`internal/netdetect/netdetect.go`) asks the OS one question: +is this interface up, named like a tunnel, and carrying a global-unicast +address? None of that says packets are flowing. A tunnel can hang — the +interface object stays exactly as it looked when healthy, and no bytes make it +through — and dezhban has no event for that. `internal/netdetect/watch.go`'s +own comment names the shape of the problem for adapter-level watchers in +general: some failures never produce an interface event at all. + +The consequence is not a leak. `decision.Evaluate` short-circuits on a failed +exit-country reading without touching the hysteresis streak — an unknown +country **HOLDS** the current posture, it never escalates — so a hung tunnel +that keeps failing its exit-country lookup keeps the guard exactly where it +was: traffic cut, physical egress blocked, endpoints open for redial. That part +of the design is correct and this ADR does not touch it. + +What is missing is everything downstream of "correctly cut". Nothing tells the +operator the tunnel looks hung rather than merely dropped. Nothing tries to +recover automatically the way an ordinary tunnel-down edge does — trigger 2 +(`vpn.redialWindow`) never fires, because the watcher never reports a down +edge for an interface that still looks up. A host can sit correctly blocked, +silently, for as long as the VPN client takes to notice its own tunnel died — +which, unlike a socket close, an OS interface object may never signal. + +This has a real failure mode distinct from "hung": an exit that **censors the +geo providers** produces the identical symptom — the interface reports up, the +lookup keeps failing — on a tunnel that is working perfectly. `state.Snapshot`'s +`LookupErr` doc already names this by example ("an Iranian exit blocking them +looks exactly like this"). Any mechanism that reacts to a failing-lookup streak +by relaxing the guard has to reckon with the fact that it cannot tell a hung +tunnel from a working one behind a hostile exit. + +## Decision + +Split the feature into two halves with different defaults. + +**Diagnosis is unconditional, on by default, and never changes.** The run loop +counts consecutive failed exit-country lookups while the tunnel interface +reports up, not standby, not in a window, and not already in FULL BLOCK. The +streak length reuses the Decider's own configured hysteresis (`o.Decider.Pending()`'s +`need`) rather than a new tunable, so it tracks the same "how many agreeing +readings before we act" tuning the rest of the state machine already uses. Once +the streak reaches that count, dezhban: + +- publishes `state.Snapshot.Zombie` (`{Since, Checks}`) — an additive field, + present only while the streak stands, cleared the instant a lookup succeeds, + the tunnel reports down, or anything suspends the geo state machine (standby, + a window, a manual block); +- logs one `Warn` line at the moment the streak crosses the threshold, not on + every tick after (matching the existing "log the edge, not the level" style + used for an ordinary tunnel-down transition); +- surfaces in `dezhban doctor` (the `liveness` check) and in the rendered + posture sentence (`internal/render`'s `zombieNote`), alongside — never in + place of — the existing `LookupErr` note. + +This half carries no censoring-exit hazard: it changes nothing the guard +enforces. The guard is already holding; this only says so out loud. + +**Acting on it is opt-in and off by default.** `vpn.advanced.livenessRedial` +(bool, default `false`) lets a confirmed streak call the **existing** +`maybeAutoWindow`, the same closure an ordinary tunnel-down edge calls. This is +trigger 2 (the automatic redial window) widening its own definition of "down" +to include "reports up but is not passing traffic" — not a fourth trigger. +Every rail that already governs trigger 2 applies completely unchanged: +`vpn.advanced.redialBudget` and `redialBudgetWindow`, the `redialMinUptime` +backoff, `dezhban hold`, `vpn.advanced.redialWindowMax`, and the one-window- +per-drop rule. `vpn.redialWindow: "0"` still removes trigger 2 outright, +`livenessRedial` or not — the streak calls the same gated closure, and +`autoWindowPossible()` still checks `RedialWindow > 0` first. + +Deliberately **not** implemented: mutating the runner's `tunnelUp` variable to +pretend the tunnel went down. `internal/netdetect/watch.go`'s `Watcher` keeps +its own `emitted` state independent of the runner, so if the runner faked a +down edge the real interface coming back up later would never look like a +change to the watcher — no up edge would ever be emitted, and the daemon would +wedge. The zombie streak is tracked entirely in the run loop, `tunnelUp` is +never touched, and the streak clears itself on the loop's own next successful +lookup. + +## Alternatives considered + +### Alternative 1: A dedicated liveness probe instead of reusing the geo lookup + +- **Pros**: distinguishes "the geo providers are unreachable" from "the tunnel + is dead" — two different root causes currently produce one symptom. +- **Cons**: a new probe target through the tunnel is a new destination-scoped + firewall pass, alongside the geo-provider pass ADR-0006 already scoped + narrowly on purpose. A second such hole needs the same tunnel+destination + double-scoping and the same scrutiny, for a diagnostic feature. +- **Why not**: the existing geo lookup already proves liveness on success — + `runGuard`'s own startup-observation comment states this outright ("a + confirmed allowed exit proves the tunnel is carrying traffic"). Reusing it + costs no new I/O, no new pass, and no new attack surface. The + cannot-distinguish-censorship-from-death limitation is real, but it is the + same limitation `LookupErr` already lives with; a second signal would not + remove it unless the new probe target were *also* uncensorable, which is not + a property a probe target can promise. + +### Alternative 2: Escalate to FULL BLOCK on a confirmed streak + +- **Pros**: makes the "something is wrong" signal impossible to miss. +- **Cons**: FULL BLOCK is reserved for a *confirmed blocked country* — the one + thing this tool exists to prevent physically. A hung tunnel is not that; the + guard is already the correct response. Escalating would also cut the + tunnel's own egress on a genuinely censoring exit, livelocking the very + recovery a redial window is meant to offer — precisely the failure mode + `decision.Evaluate`'s "undeterminable HOLDS" rule already exists to prevent + for an ordinary unknown reading. +- **Why not**: it repeats a mistake this codebase has already reasoned its way + out of once, for the same failure shape. + +### Alternative 3: `livenessRedial` on by default + +- **Pros**: better automatic recovery out of the box, matching CVG's own + watchdog (which has no equivalent opt-out). +- **Cons**: a censoring exit is not a hypothetical for this project's stated + threat model — the docs name Iran by example more than once. Defaulting to + "trust a failing lookup enough to relax the guard" hands a censoring exit a + way to trigger a relaxation window on a tunnel that was never actually down. +- **Why not**: the cost of getting this wrong (a brief real-IP exposure handed + to an adversary who controls the exit) is categorically worse than the cost + of getting it right by hand (`dezhban switch`). Default off, opt-in for + operators who have judged their own exit trustworthy enough for the + trade-off. + +## Consequences + +### Positive + +- A hung tunnel finally explains itself — in the log, in `doctor`, and in the + rendered posture — instead of sitting correctly cut with no signal to anyone. +- The diagnosis costs nothing: no new I/O, no new firewall pass, no new + destination-scoped hole. +- Recovery is available for operators who want it, through the exact same + budget/backoff/hold rails as an ordinary drop, with no new machinery to + audit. + +### Negative + +- One more advanced tunable (`vpn.advanced.livenessRedial`), declared in + `internal/config/schema.go` like every other, so every surface still derives + its hint and default from the same table. +- `internal/runner/verify_test.go`/`liveness_test.go`-style coverage aside, + this is a heuristic: a streak length tuned to the Decider's hysteresis can + still misfire on a link that is merely slow, not dead. Mitigated by reusing + the same hysteresis the rest of the state machine already trusts, rather than + inventing a separate, unvalidated threshold. + +### Risks + +- **A user enables `livenessRedial` behind a censoring exit.** This is the + hazard the whole split exists around. Mitigated by defaulting off, and by + every relaxation rail (budget, backoff, `redialWindowMax`, hold) still + applying — a censoring exit can trigger at most a budget's worth of exposure + before the ledger holds, same as any other flapping link. +- **The diagnosis itself is noisy on a merely slow link.** Mitigated by gating + the report on the Decider's own hysteresis count rather than a single failed + reading, and by clearing it the moment a lookup succeeds. + +## What this does not change + +- **The switch window still has exactly THREE sanctioned triggers** + (`docs/contribute/architecture.md`). A confirmed liveness streak reaches + `maybeAutoWindow` — trigger 2's own entry point, alongside the ordinary + tunnel-down edge and the bound-lifted re-decision (`retryAutoWindow`) — so + this widens what trigger 2 recognises as "down"; it adds no fourth trigger. +- **`vpn.redialWindow: "0"` still removes trigger 2 entirely**, regardless of + `livenessRedial`. +- **The undeterminable-country-HOLDS rule is untouched.** This ADR adds a + second thing that HOLDS (a hung tunnel) rather than changing what holding + means. +- **`internal/netdetect/watch.go` is untouched.** The watcher's own up/down + edge detection, debounce, and `emitted` state carry no knowledge of the + zombie streak. diff --git a/docs/adr/README.md b/docs/adr/README.md index be84e32..0159a0c 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -21,6 +21,7 @@ New records use [template.md](template.md) and take the next free number. | [0007](0007-upgrade-disclosed-window-not-holding-block.md) | `dezhban upgrade` discloses the activation window instead of holding a block through it | accepted, implemented | | [0008](0008-arm-at-boot.md) | Arm at boot from a persisted observation, plus a bounded pause | accepted, implemented | | [0009](0009-redial-budget.md) | The automatic redial window spends from a bounded budget | accepted, implemented | +| [0010](0010-tunnel-liveness.md) | Zombie-tunnel detection is unconditional; acting on it is opt-in | accepted, implemented | > **0006 is the one to read first if you are touching the geo lookup.** It records why > the obvious implementation silently defeats the exit-country check, and it exists @@ -43,3 +44,9 @@ New records use [template.md](template.md) and take the next free number. > still literally true) — it records why pause was added as a *third*, and > why arming at boot needed the `TunnelEverUp` persistence rather than a > plain unconditional fail-closed start. +> +> **0010 is the one to read before defaulting `vpn.advanced.livenessRedial` +> to on**, or before treating a failed exit-country lookup as evidence a +> tunnel is dead. It records why that exact symptom is indistinguishable from +> a censoring exit, and why the diagnosis (always on) is kept separate from +> the relaxation it may trigger (opt-in). diff --git a/docs/concepts/glossary.md b/docs/concepts/glossary.md index aa5e2d0..f562922 100644 --- a/docs/concepts/glossary.md +++ b/docs/concepts/glossary.md @@ -196,6 +196,33 @@ streak resolves or a bounded budget runs out. It changes **cadence only** — hy still gates the change, and it is skipped entirely when checking would require lifting the guard. +**Enforcement verification** — a periodic check (`vpn.advanced.verifyInterval`, +default `1m`) that the firewall rules dezhban believes it installed are still +installed AND still actually enforcing — not just present but disconnected +from what makes them bite (the pf main ruleset no longer referencing our +anchor, an nft chain's policy rewritten off `drop` in place, a Windows profile's +outbound default flipped back to Allow while our rules sit untouched) — +re-applying whatever posture is currently in force — the standing guard, a +full block, or an open switch/redial window or pause — the instant it is not. +Every other rule change is triggered by something dezhban itself did; this is +the only one that notices a ruleset (or the switch that makes it matter) +disturbed from OUTSIDE it — another firewall tool, `pfctl -F all`, +`nft flush ruleset`, an OS ruleset reload — the one failure mode that used to be +completely silent. Reported in `state.verify` (`status --json`) only while +something is wrong. Disablable (`"0"`); an unreadable backend is never treated +as evidence the rules are gone, the same discipline **fail closed** already +applies to an undeterminable country. + +**Zombie tunnel** — a tunnel interface that reports up while a run of +exit-country lookups through it has failed. Diagnosis, not a leak: the guard is +already holding exactly as it would for any other unknown reading (see **Fail +closed**). Detection reuses **Hysteresis**'s streak length and is always on, +reported in `state.zombie`. Acting on it — letting a confirmed streak open an +automatic redial window — is a separate, off-by-default key +(`vpn.advanced.livenessRedial`): an exit that censors the geo providers produces +the identical symptom on a tunnel that was never actually down, so relaxing the +guard on this signal is opt-in. See [ADR-0010](../adr/0010-tunnel-liveness.md). + **Preset** — a named bundle of values for the keys that answer "how strict am I" (the three relaxation windows, poll cadence and hysteresis, the two firewall-pass toggles, arm-at-boot): **Strict**, **Balanced** (the shipped defaults), **Relaxed**. @@ -227,6 +254,14 @@ available, root-only, and independent of the socket. root, **with no daemon running**. Deliberately not a socket operation, because the escape hatch must never depend on the thing it is escaping from. +**Single-instance lock** — an exclusive lock `run` holds over the state directory +for its entire lifetime, so a second `run` — with or without `--no-daemon` — +refuses outright instead of racing the first to call `Backend.Apply`. Released +by the OS the moment the process ends, by any means, so a killed daemon never +wedges the next start. `panic`, `unblock`, and the service-lifecycle commands +take no such lock — they are the escape hatch and must stay usable with no +daemon running at all. + ## Words we do not use **This table is machine-read.** `internal/vocab` parses it and fails the build, diff --git a/docs/contribute/testing.md b/docs/contribute/testing.md index 7653f8d..8e701a5 100644 --- a/docs/contribute/testing.md +++ b/docs/contribute/testing.md @@ -90,6 +90,18 @@ root and no real firewall. `task test:cover` enforces the coverage floors in error. - [ ] **`--force` bypasses detection.** `block --force` / `unblock --force` act without consulting the geo state. +- [ ] **Enforcement verification notices a ruleset removed from outside it, and + repairs it.** With the daemon running and enforcing (guard or a manual + block), flush the ruleset by hand — `sudo pfctl -a dezhban -F all` (macOS), + `sudo nft flush ruleset` (Linux; a full flush, not just the `dezhban` + table, since this is testing that dezhban notices ANY removal), or delete + the WFP rule group (Windows). Within one `vpn.advanced.verifyInterval` + (default `1m`; set it lower, e.g. `5s`, to speed up the check) the daemon + logs `dezhban's firewall rules are MISSING — ... re-applying now`, + `dezhban status --json` shows `state.verify.missing: true` with + `repairs` incremented, and the rule dump shows the ruleset back. Set + `vpn.advanced.verifyInterval: "0"` and repeat — the daemon must NOT + notice or repair (the check should stay off, not merely run slower). Per-OS rule inspection: @@ -206,6 +218,12 @@ Only a live host can prove these — CI cannot reach a printer. **stays** `guard` however many error-ticks pass, and the log says the exit country is unknown. It must never reach `full-block` on errors alone: that would cut the tunnel's own egress and livelock the redial. +- [ ] **An exit-IP change is observed and reported, without touching posture.** + Switch the VPN to a different server that still exits through an allowed + country (so `countryCode`/`blocked` are unaffected) → the daemon logs + `exit IP changed` and `dezhban status --json` shows a fresh + `exitIpChangedAt`. Confirm it does NOT reset on an unchanged reading, and + that `pending`/hysteresis progress is untouched by the comparison itself. - [ ] **An unknown country does not lift a block either.** Repeat while in `full-block` → it stays blocked. - [ ] **An error mid-streak does not cancel a pending flip.** With `hysteresis: 3`, @@ -235,6 +253,22 @@ The guard is where a misconfiguration locks the host out. Run escalating — escalating on an unknown would cut the tunnel's own egress and livelock the redial. - [ ] **Unblock restores everything.** +- [ ] **A hung tunnel (interface up, no traffic) is diagnosed, not silently + left cut with no signal.** With the VPN connected and the guard armed, + block the tunnel's traffic at the OS level without bringing the + interface down — e.g. a host-level firewall rule dropping packets on the + tunnel interface, or disconnect the VPN server side while the client's + interface stays configured. After `hysteresis` consecutive failed exit + checks, the daemon logs `tunnel interface reports up, but exit lookups + through it keep failing`, `dezhban status --json` shows + `state.zombie.checks`, and `dezhban doctor`'s "enforcement liveness" + section reports it. Confirm the guard itself is untouched throughout — + still cutting egress exactly as it would for any other tunnel-up state — + and that with `vpn.advanced.livenessRedial` at its default (`false`) NO + switch-window rule ever appears in the rule dump. Set it to `true` and + repeat: a switch-window pass should appear once the streak is confirmed, + through the same `redialBudget`/`redialMinUptime` machinery an ordinary + drop uses. ### macOS worked example (pf) @@ -429,6 +463,13 @@ Per OS, privileged: restart-on-failure brings it back and it re-enforces. - [ ] **`restart` applies the restart-required keys** (most keys apply live — see the section below), and `start` and `stop` are idempotent. +- [ ] **A second `run` refuses.** With the service running, `sudo dezhban run` + (with or without `--no-daemon`) in a second terminal refuses immediately + with "another dezhban is already running", and the first daemon's + enforcement is undisturbed — no duplicate rules, no double-Apply. `kill -9` + the first daemon, then start a second `run`: it succeeds (the OS released + the lock with the process), confirming a killed daemon never wedges the + next start. ## Unattended recovery (`doctor`'s boot and retention checks) diff --git a/docs/usage/cli.md b/docs/usage/cli.md index 2a31a57..c716307 100644 --- a/docs/usage/cli.md +++ b/docs/usage/cli.md @@ -97,6 +97,13 @@ Pass `--no-sudo` (or `DEZHBAN_NO_SUDO=1`) to opt out and get the plain "must run root" error; on Windows, and when there's no terminal (CI/pipes), it never auto-elevates. Pass `--no-daemon` (or `DEZHBAN_NO_DAEMON=1`) to skip the control socket and act on the firewall directly — the escape hatch for a wedged daemon. +That escape hatch is exactly the case `run` guards against contending with a +still-running service: it takes an exclusive lock over the state directory for +its whole lifetime, so a second `run` — with or without `--no-daemon` — refuses +immediately ("another dezhban is already running") instead of racing the first +to apply firewall rules. `panic`, `unblock`, and the service-lifecycle commands +take no such lock; they are the recovery path and must stay usable with no +daemon running at all. A manual `block` **holds**: the daemon suspends its geo state machine until you `unblock`, so an allowed country won't quietly undo what you asked for. @@ -152,6 +159,38 @@ does not do is *cause* anything — the budget is still only consulted on a tunnel-down edge, so watching it reach a full window tells you a window would be granted, not that one is coming. +`state.verify` is present only when enforcement verification (`vpn.advanced.verifyInterval`) +last found something wrong — the firewall rules dezhban believes it installed +were missing, or the backend could not be read at all. `missing: true` means +they were found gone and have already been re-applied (`repairs` is the +cumulative count since startup — a number that keeps climbing means something on +this host is repeatedly removing dezhban's rules). An `err` string instead means +the backend itself could not be read; that is **not** treated as evidence the +rules are gone, so `missing` stays false and nothing is re-applied — the same +discipline as an undeterminable exit country holding the current posture. Absent +means the last check was clean, or verification is disabled (`"0"`) — the two +look the same here; `pollIntervalSeconds`-scale staleness rules apply the same +way they do to the rest of the snapshot. + +`state.zombie` is present only while a run of exit-country lookups has failed +through a tunnel that still reports up — `checks` is the streak length, `since` +when it started. This is **diagnosis, not a leak**: the guard is holding exactly +as it would for any other unknown reading. Absent means either nothing is wrong +or the tunnel is plainly down instead (a different, already-explained state — +see `state.drop`). Whether this can also open an automatic redial window is +controlled by `vpn.advanced.livenessRedial` (default off); see +[ADR-0010](../adr/0010-tunnel-liveness.md) for why that default matters — an exit +that censors the geo lookup produces the identical symptom on a tunnel that was +never actually down. + +`state.exitIpChangedAt` is set the first time the observed exit IP differs from +the previous successful reading, and stays set (it is not cleared by a later +unchanged reading). Purely observational: it never affects `blocked`, +`countryCode`, or `pending` — a failover between two servers in the same allowed +country changes nothing those fields report, but changes this. Absent means no +change has been observed since the daemon started, not that the exit has never +had an IP. + ```sh dezhban status # config + service + block state dezhban status --json # machine-readable (merges the state file) diff --git a/docs/usage/config.md b/docs/usage/config.md index 3b03809..da34f59 100644 --- a/docs/usage/config.md +++ b/docs/usage/config.md @@ -305,13 +305,14 @@ entirely to keep the defaults; set only the knobs you need. Every field below is reachable with `dezhban config set vpn.advanced.=` — the same validated write-and-reload path as any other key — not just by hand-editing the file. `switchWindowMax`, `redialWindowMax`, `redialMinUptime`, `redialBudget`, -`redialBudgetWindow`, and +`redialBudgetWindow`, `verifyInterval`, `livenessRedial`, and `windowDiscoveryInterval` apply live; the rest (built into something the run loop constructs once at startup, or — for `windowProtocols`/`windowPorts` — only re-read when a switch window opens) need `dezhban restart` to take effect, which `config set` says so at the time. -**`0` is not "off" here.** Only the three windows and `redialMinUptime` treat a +**`0` is not "off" here.** Only the three windows, `redialMinUptime`, and +`verifyInterval` treat a `0` as an explicit opt-out; every other field in this table has no disabled state, so a non-positive value is replaced with the default shown below. That replacement is not silent — `config set` echoes the value actually stored and @@ -348,6 +349,8 @@ were on. Turning it off is fine; turning it off by accident is not. | `redialBudget` | `2m` | Total time automatic redial windows may leave the guard relaxed within `redialBudgetWindow`. Debited when a window opens and **credited back when it closes early**, so a redial that succeeded in three seconds costs three seconds — the budget measures the exposure actually taken, not the exposure offered. When it can no longer afford a window the guard simply holds and traffic stays cut. Not disablable (see below). | | `redialBudgetWindow` | `15m` | The rolling period `redialBudget` is measured over. Each window's cost is returned as it falls out of the period, so a busy link recovers its allowance progressively rather than needing a full quiet stretch. Not disablable. | | `endpointWarnThreshold` | `256` | Union size at which `doctor` warns about rule-list bloat. | +| `verifyInterval` | `1m` | How often the daemon re-reads the firewall to confirm the rules it believes are installed are still there, re-applying whatever is currently in force — the standing guard, a full block, or an open switch/redial window or pause — the instant they are not. Every other rule change dezhban makes is triggered by something the daemon itself did — this is the only one that notices a ruleset removed from OUTSIDE it (another firewall tool, `pfctl -F all`, `nft flush ruleset`, an OS ruleset reload). `"0"` disables the check, trusting the rules to stay put once applied. On Windows each check is a whole PowerShell invocation (one, not two: the group-existence test and the profile-default cross-check share a single script), so a very short interval has a real cost — the default is deliberately conservative. | +| `livenessRedial` | `false` | Lets a tunnel that reports up but has stopped passing traffic open an automatic redial window — see [ADR-0010](../adr/0010-tunnel-liveness.md). Off by default: an exit that censors the geo lookup produces the identical failure pattern as a genuinely hung tunnel, and turning this on lets that exit trigger a window on a tunnel that was never actually down. The diagnosis itself (`dezhban doctor`, the state file) is always on regardless of this key — only ACTING on it is gated. | | `windowProtocols` | `[]` | Restrict a switch window to these protocols (e.g. `["udp"]`) instead of allowing all outbound. Empty allows all — only worth setting when every VPN you switch to uses a fixed protocol. | | `windowPorts` | `[]` | Restrict a switch window to these ports (e.g. `[51820]`) instead of allowing all outbound. Empty allows all — only worth setting when every VPN you switch to uses a fixed port set (e.g. WireGuard on 51820). | diff --git a/docs/usage/troubleshooting.md b/docs/usage/troubleshooting.md index ec7ee9c..436b8c5 100644 --- a/docs/usage/troubleshooting.md +++ b/docs/usage/troubleshooting.md @@ -312,6 +312,117 @@ There is no override — this is the same rule `dezhban upgrade apply` enforces for its own restart, and `sudo dezhban restart` is already the deliberate, by-name escape hatch for an operator who wants to force it anyway. +## dezhban says its firewall rules went missing and were re-applied + +Symptom (from the daemon log): + +``` +msg="dezhban's firewall rules are MISSING — something removed them; re-applying now" posture=guard repairs=1 +``` + +**Cause.** Every other rule change dezhban makes is triggered by something it +did — a tunnel change, an endpoint refresh, a posture flip. This message +means something else removed the rules: another firewall tool, `pfctl -F all` +/ `nft flush ruleset` run by hand, an OS-level firewall reset, or a +misbehaving script. Enforcement verification (`vpn.advanced.verifyInterval`, +default `1m`) noticed the gap on its next check and closed it immediately. + +**This is not a leak that happened** — verification found the gap and repaired +it before you saw this message, not after. What it tells you is that *something +on this host keeps removing dezhban's rules*, which is worth tracking down +regardless: a `repairs` count that keeps climbing across restarts means it is +recurring, not a one-off. + +```sh +dezhban status --json # state.verify — At, Missing/Err, Repairs +dezhban doctor # the "enforcement liveness" section +``` + +**Fix.** Find and stop whatever else is touching the firewall — a competing +security tool, a system firewall reset on network change, a cron job. If you +need to intentionally flush rules for testing, expect dezhban to notice and +repair within one `verifyInterval` — that is the feature working, not a bug to +route around. Setting `vpn.advanced.verifyInterval: "0"` disables the check +entirely; do this only if you understand you are giving up the one signal that +would otherwise catch a rules-removed-from-outside gap. + +**`sudo dezhban panic` is the one deliberate exception.** Tearing down the +rules on purpose is still "the rules going missing" from a running daemon's +point of view, so `panic` leaves behind a marker telling that daemon's +enforcement verification to stand down instead of re-applying the posture you +just asked it to remove — otherwise the two features would fight, and +verification would win within a minute of every `panic`. The marker clears +automatically the next time you `dezhban unblock` (or restart the daemon, +which re-applies the initial posture on its own anyway), so verification +never stays suspended longer than it takes to explicitly resume enforcement. + +## dezhban says my VPN might be hung (zombie tunnel) + +Symptom (from the daemon log, or in `dezhban doctor`): + +``` +msg="tunnel interface reports up, but exit lookups through it keep failing — it may need reconnecting; guard holds either way" checks=2 +``` + +**Cause.** The tunnel interface still looks up to the OS, but a run of +exit-country lookups through it have failed — the same signal count as +`hysteresis`. dezhban's posture never escalates on a lookup failure alone (an +unknown country **holds**, it never flips — see +[glossary § Fail closed](../concepts/glossary.md#mechanism)), so a hung tunnel +stays correctly cut, but until this check existed it explained itself to no +one and recovered only if a person noticed and ran `dezhban switch` by hand. + +This has one important false-positive case: an exit that **censors the geo +providers** produces the identical symptom — interface up, lookups failing — +on a tunnel that is working perfectly. That is why this diagnosis alone never +opens a redial window; see below. + +```sh +dezhban status --json # state.zombie — Since, Checks +dezhban doctor # the "enforcement liveness" section +``` + +**Fix.** Reconnect your VPN client. If it happens repeatedly with the same VPN, +consider `vpn.advanced.livenessRedial: true` to let a confirmed streak open an +automatic redial window through the same budget/backoff machinery an ordinary +drop uses (off by default — see +[ADR-0010](../adr/0010-tunnel-liveness.md) for the censoring-exit trade-off +before turning it on). + +## A second `dezhban run` refuses to start + +``` +another dezhban is already running (holds /var/db/dezhban/dezhban.lock) — see `dezhban status` +``` + +**Cause.** `run` takes an exclusive lock over the state directory for its +entire lifetime, so a second daemon process — started by hand, via +`--no-daemon`, or by accident alongside the service — cannot start and race the +first to apply firewall rules. This is expected and correct: only one process +may own `Backend.Apply` at a time. + +```sh +dezhban status # confirm the already-running daemon's posture +sudo dezhban restart # restart the ONE daemon, rather than starting a second +``` + +The lock is released by the OS the moment the holding process ends, by any +means — a crash, `SIGKILL`, a clean stop — so it never survives past the +process it belonged to; there is no stale-lock case to clean up by hand. The +lock file itself is root-owned and `0600`, so an unprivileged local user +cannot hold it open to keep `run` from starting. If this refuses and +`dezhban status`/your process manager show nothing running, look for the +daemon in a genuinely stuck (not dead) state rather than assuming a leftover +lock file — `sudo dezhban panic` removes firewall rules without needing this +lock at all, regardless of what is holding it. + +This lock is a safety net around a race, not part of the kill switch itself: +if acquiring it fails for any reason OTHER than the contention above — an +unwritable or missing state directory, for example — `run` logs a warning and +starts anyway, without the single-instance guard for that run, rather than +refusing to enforce. A lock that cannot be established must never become a +reason the guard does not arm. + ## Preview rules before applying them Never find out what a block does by getting locked out — render the exact diff --git a/gui/macos/Sources/DezhbanCore/SettingsFields.swift b/gui/macos/Sources/DezhbanCore/SettingsFields.swift index 02923ef..97061c6 100644 --- a/gui/macos/Sources/DezhbanCore/SettingsFields.swift +++ b/gui/macos/Sources/DezhbanCore/SettingsFields.swift @@ -31,6 +31,7 @@ public struct SettingsFields { "vpn.advanced.commandFreshness", "vpn.advanced.windowDiscoveryInterval", "vpn.advanced.tunnelPruneAfter", "vpn.advanced.learnedEndpointTTL", "vpn.advanced.learnedMaxPerProfile", "vpn.advanced.promoteAfterRefreshes", "vpn.advanced.endpointWarnThreshold", "vpn.advanced.windowProtocols", "vpn.advanced.windowPorts", + "vpn.advanced.verifyInterval", "vpn.advanced.livenessRedial", ] /// Raw staged values, exactly as `config get` returned them and exactly as @@ -60,6 +61,7 @@ public struct SettingsFields { /// representation. static let boolKeys: Set = [ "vpn.autoDetect", "vpn.autoDiscoverEndpoints", "vpn.autoArm", "vpn.allowLocalNetwork", + "vpn.advanced.livenessRedial", ] /// Reads one staged value by key. Returns "" for a key this pane does not @@ -203,4 +205,10 @@ public struct SettingsFields { public var advWindowPorts: String { get { string("vpn.advanced.windowPorts") } set { setString("vpn.advanced.windowPorts", newValue) } } + public var advVerifyInterval: String { + get { string("vpn.advanced.verifyInterval") } set { setString("vpn.advanced.verifyInterval", newValue) } + } + public var advLivenessRedial: Bool { + get { bool("vpn.advanced.livenessRedial") } set { setBool("vpn.advanced.livenessRedial", newValue) } + } } diff --git a/gui/macos/Sources/DezhbanCore/Snapshot.swift b/gui/macos/Sources/DezhbanCore/Snapshot.swift index edc5100..62fc4f7 100644 --- a/gui/macos/Sources/DezhbanCore/Snapshot.swift +++ b/gui/macos/Sources/DezhbanCore/Snapshot.swift @@ -116,6 +116,39 @@ public struct RedialState: Codable { public let fastDrops: Int? } +/// Enforcement verification's last unhappy answer — mirrors Go's +/// `state.VerifyState`. Present only while something is wrong; a clean check +/// clears it. Distinguishes three different problems: `missing` alone means +/// the backend answered, the rules were actually gone, and they were already +/// re-applied by the time this is read; `missing` WITH `err` means the +/// re-apply itself failed and the host is unenforced right now; `err` alone +/// means the backend could not be read at all, which is NOT evidence the +/// rules are gone and changes nothing — the same discipline as an +/// undeterminable exit country holding the current posture. +/// See ADR-0010 and docs/usage/config.md's `verifyInterval` row. +public struct VerifyState: Codable { + /// Optional to match Go's `omitzero` — see `SwitchState.until`. + public let at: Date? + /// Omitted (nil here) rather than `false` when the last check was the + /// `err` case instead — Go's `omitempty` never writes a literal `false`. + public let missing: Bool? + public let err: String? + /// How many times verification has re-applied the posture since startup. + /// Omitted (nil here, read as 0) when it hasn't repaired anything yet — + /// e.g. the very first check already failed to read the backend. + public let repairs: Int? +} + +/// A tunnel interface that reports up while a run of exit-country lookups +/// through it has failed — mirrors Go's `state.ZombieState`. Present only +/// while such a streak stands. This is diagnosis, not a leak: the guard is +/// holding exactly as designed either way. See ADR-0010. +public struct ZombieState: Codable { + /// Optional to match Go's `omitzero` — see `SwitchState.until`. + public let since: Date? + public let checks: Int +} + /// The daemon's posture at a point in time — mirrors Go's `state.Snapshot`. /// JSON keys match the lowerCamelCase struct tags in internal/state/state.go. public struct Snapshot: Codable { @@ -132,6 +165,10 @@ public struct Snapshot: Codable { public let lookupErr: String? // a GENUINE failure: a tunnel was up and measuring it failed public let exitUnknown: String? // EXPECTED: no tunnel up, so there is no exit to measure public let enforcementErr: String? // last firewall-action failure, nil when clear + /// `dezhban panic` tore the rules down and the daemon is standing down + /// rather than reinstating them — already folded into `display` by the + /// Go renderer, carried here only for parity with the rest of Snapshot. + public let panicDisarmed: Bool? public let tunnels: [Tunnel]? public let endpoints: [String]? public let pollIntervalSeconds: Int? // daemon poll cadence, for sizing staleness @@ -144,6 +181,12 @@ public struct Snapshot: Codable { public let drop: DropRecord? // present from a tunnel drop until a tunnel is up again public let hold: HoldState? // present only while "hold the line" is armed public let redial: RedialState? // present only while a redial window stands refused + public let verify: VerifyState? // present only while enforcement verification found something wrong + public let zombie: ZombieState? // present only while a hung-tunnel streak stands + /// When the observed exit IP last differed from the previous successful + /// reading. Optional to match Go's `omitzero` — see `SwitchState.until`. + /// Purely observational: never affects `blocked`/`countryCode`/`pending`. + public let exitIpChangedAt: Date? /// Wall-clock age of this snapshot. public var age: TimeInterval { Date().timeIntervalSince(time) } diff --git a/gui/macos/Sources/DezhbanMenu/SettingsView.swift b/gui/macos/Sources/DezhbanMenu/SettingsView.swift index 915b70c..b25dd1b 100644 --- a/gui/macos/Sources/DezhbanMenu/SettingsView.swift +++ b/gui/macos/Sources/DezhbanMenu/SettingsView.swift @@ -419,6 +419,10 @@ struct SettingsView: View { text: $fields.advWindowProtocols) schemaField("vpn.advanced.windowPorts", "Window ports (comma-sep)", text: $fields.advWindowPorts) + durationField("vpn.advanced.verifyInterval", "Enforcement verification interval", + text: $fields.advVerifyInterval) + schemaToggle("vpn.advanced.livenessRedial", "Redial on a hung tunnel", + isOn: $fields.advLivenessRedial) } } diff --git a/gui/macos/Tests/DezhbanCoreTests/SettingsFieldsTests.swift b/gui/macos/Tests/DezhbanCoreTests/SettingsFieldsTests.swift index 4f57ce0..260942c 100644 --- a/gui/macos/Tests/DezhbanCoreTests/SettingsFieldsTests.swift +++ b/gui/macos/Tests/DezhbanCoreTests/SettingsFieldsTests.swift @@ -54,6 +54,9 @@ private func testSchema() -> ConfigSchema { defaultValue: "256"), tunable("vpn.advanced.windowProtocols", "Window protocols", "list"), tunable("vpn.advanced.windowPorts", "Window ports", "list"), + tunable("vpn.advanced.verifyInterval", "Enforcement verification interval", "duration", + defaultValue: "1m0s", disablable: true), + tunable("vpn.advanced.livenessRedial", "Redial on a hung tunnel", "bool", defaultValue: "false"), ]) } @@ -96,6 +99,8 @@ struct SettingsFieldsTests { f.advEndpointWarnThreshold = "512" f.advWindowProtocols = "udp,tcp" f.advWindowPorts = "51820,443" + f.advVerifyInterval = "90s" + f.advLivenessRedial = true // Named accessor and keyed lookup are the same storage, not two copies. #expect(f.value(for: "vpn.tunnelInterfaces") == "utun9") @@ -104,6 +109,8 @@ struct SettingsFieldsTests { #expect(f.value(for: "vpn.advanced.windowPorts") == "51820,443") #expect(f.value(for: "vpn.autoDetect") == "true") #expect(f.value(for: "vpn.allowLocalNetwork") == "false") + #expect(f.value(for: "vpn.advanced.verifyInterval") == "90s") + #expect(f.value(for: "vpn.advanced.livenessRedial") == "true") let pairs = f.pairs() #expect(pairs.contains("vpn.switchWindow=10s")) @@ -156,7 +163,7 @@ struct SettingsFieldsTests { "VPN server address grace", "VPN server address refresh", "Tunnel check interval", "Switch window cap", "Redial window cap", "Redial anti-flap uptime", "Command freshness", "Window discovery interval", "Tunnel prune delay", - "Learned address lifetime", + "Learned address lifetime", "Enforcement verification interval", ]) } diff --git a/gui/macos/Tests/DezhbanCoreTests/SnapshotTests.swift b/gui/macos/Tests/DezhbanCoreTests/SnapshotTests.swift index 13e7252..bcd0b2b 100644 --- a/gui/macos/Tests/DezhbanCoreTests/SnapshotTests.swift +++ b/gui/macos/Tests/DezhbanCoreTests/SnapshotTests.swift @@ -208,4 +208,70 @@ struct SnapshotTests { #expect(s.redial == nil) #expect(s.drop?.at != nil) } + + /// Enforcement verification's "rules missing, already repaired" answer must + /// decode, `repairs` included — this is the count an operator uses to tell a + /// one-off from something recurring. + @Test func decodesAMissingRulesVerifyFinding() { + let json = """ + { "time": "2026-07-25T10:00:00Z", "posture": "guard", "blocked": false, + "verify": { "at": "2026-07-25T09:59:50Z", "missing": true, "repairs": 3 } } + """.data(using: .utf8)! + let s = try! #require(StateReader.decode(json)) + #expect(s.verify?.missing == true) + #expect(s.verify?.err == nil) + #expect(s.verify?.repairs == 3) + } + + /// The unreadable-backend case: `err` set, `missing` and `repairs` both + /// absent (Go's `omitempty` never writes a literal `false`/`0`) — must not + /// be misread as "rules confirmed missing" or fail to decode. + @Test func decodesAVerifyReadError() { + let json = """ + { "time": "2026-07-25T10:00:00Z", "posture": "guard", "blocked": false, + "verify": { "at": "2026-07-25T09:59:50Z", "err": "pfctl: no such process" } } + """.data(using: .utf8)! + let s = try! #require(StateReader.decode(json)) + #expect(s.verify?.err == "pfctl: no such process") + #expect(s.verify?.missing == nil) + #expect(s.verify?.repairs == nil) + } + + /// A zombie streak's `checks` count is never omitted by Go (no `omitempty` + /// on that field) — must decode even at its lowest meaningful value. + @Test func decodesAZombieStreak() { + let json = """ + { "time": "2026-07-25T10:00:00Z", "posture": "guard", "blocked": false, + "zombie": { "since": "2026-07-25T09:59:00Z", "checks": 2 } } + """.data(using: .utf8)! + let s = try! #require(StateReader.decode(json)) + #expect(s.zombie?.checks == 2) + #expect(s.zombie?.since != nil) + } + + /// A failover between two servers in the same allowed country changes + /// `exitIpChangedAt` and nothing else this decodes — must survive on its + /// own with no other diagnostic field present. + @Test func decodesExitIPChangedAt() { + let json = """ + { "time": "2026-07-25T10:00:00Z", "posture": "guard", "blocked": false, + "exitIpChangedAt": "2026-07-25T09:58:30Z" } + """.data(using: .utf8)! + let s = try! #require(StateReader.decode(json)) + #expect(s.exitIpChangedAt != nil) + } + + /// Same additive rule as `redial`/`drop`/`hold`: every snapshot an older + /// daemon ever wrote lacks all three PR #39 diagnostic fields, and absent + /// must read as "nothing to report", never a decode failure that blanks + /// the menubar. + @Test func absentVerifyZombieAndExitIPAreNotAFailure() { + let json = """ + { "time": "2026-07-25T10:00:00Z", "posture": "guard", "blocked": false } + """.data(using: .utf8)! + let s = try! #require(StateReader.decode(json)) + #expect(s.verify == nil) + #expect(s.zombie == nil) + #expect(s.exitIpChangedAt == nil) + } } diff --git a/internal/armed/armed.go b/internal/armed/armed.go index 279a99a..d4f5942 100644 --- a/internal/armed/armed.go +++ b/internal/armed/armed.go @@ -22,6 +22,8 @@ import ( "os" "path/filepath" "time" + + "github.com/behnam-rk/dezhban/internal/atomicfile" ) // version is the on-disk schema version. Bump on an incompatible change. @@ -100,28 +102,8 @@ func (r *Record) Save(path string) error { if err != nil { return fmt.Errorf("armed: marshal: %w", err) } - tmp, err := os.CreateTemp(dir, ".armed-*.json.tmp") - if err != nil { - return fmt.Errorf("armed: create temp: %w", err) - } - tmpName := tmp.Name() - defer func() { _ = os.Remove(tmpName) }() - if _, err := tmp.Write(data); err != nil { - _ = tmp.Close() - return fmt.Errorf("armed: write temp: %w", err) - } - if err := tmp.Sync(); err != nil { - _ = tmp.Close() - return fmt.Errorf("armed: sync temp: %w", err) - } - if err := tmp.Close(); err != nil { - return fmt.Errorf("armed: close temp: %w", err) - } - if err := os.Chmod(tmpName, 0o644); err != nil { - return fmt.Errorf("armed: chmod temp: %w", err) - } - if err := os.Rename(tmpName, path); err != nil { - return fmt.Errorf("armed: rename into place: %w", err) + if err := atomicfile.Write(path, data, 0o644); err != nil { + return fmt.Errorf("armed: write %q: %w", path, err) } return nil } diff --git a/internal/atomicfile/atomicfile.go b/internal/atomicfile/atomicfile.go new file mode 100644 index 0000000..8e52527 --- /dev/null +++ b/internal/atomicfile/atomicfile.go @@ -0,0 +1,65 @@ +// Package atomicfile writes a file's full contents atomically, so a crash or a +// concurrent reader can never observe a partially written result — only the +// old contents or the new ones, never a truncated mix of both. +// +// It replaces what used to be the same ~15 lines (CreateTemp in the target's +// own directory → Write → Sync → Close → Chmod → Rename) copied independently +// into internal/armed, internal/learned, internal/token, internal/config, +// internal/state, internal/command, internal/firewall (pf_darwin's pf.conf/ +// state writes and wfp_windows' applied-action marker), and +// cmd/dezhban/panicmark.go — each free to drift from the others on the next +// edit. One implementation means a future correction (fsync ordering, a +// platform-specific quirk) is made once, not rediscovered per package. +package atomicfile + +import ( + "os" + "path/filepath" +) + +// Write atomically replaces path's contents with data. It creates a temp file +// in the SAME directory as path — required for the final os.Rename to be a +// same-filesystem, atomic replace rather than a cross-filesystem copy — writes +// data, fsyncs it before closing (so a rename can never publish a name whose +// contents are still sitting in a buffer the kernel hasn't flushed), sets mode, +// then renames it over path. The temp file is removed on any failure path +// before the rename commits; once the rename succeeds the removal is a no-op. +func Write(path string, data []byte, mode os.FileMode) error { + tmp, err := os.CreateTemp(filepath.Dir(path), ".atomicfile-*.tmp") + if err != nil { + return err + } + tmpName := tmp.Name() + defer os.Remove(tmpName) // no-op once the rename below succeeds + + if _, err := tmp.Write(data); err != nil { + tmp.Close() + return err + } + if err := tmp.Sync(); err != nil { + tmp.Close() + return err + } + if err := tmp.Close(); err != nil { + return err + } + if err := os.Chmod(tmpName, mode); err != nil { + return err + } + if err := os.Rename(tmpName, path); err != nil { + return err + } + + // The rename above is atomic, but without fsyncing the containing + // directory the rename itself is not guaranteed durable across a crash + // or power loss on filesystems that require it — the directory entry can + // still be sitting in write-back cache. Best-effort: some platforms and + // filesystems (Windows, some virtual/overlay FS) don't support syncing a + // directory handle at all, and this hardens durability rather than the + // atomicity the rename already guarantees on its own. + if dir, err := os.Open(filepath.Dir(path)); err == nil { + _ = dir.Sync() + dir.Close() + } + return nil +} diff --git a/internal/atomicfile/atomicfile_test.go b/internal/atomicfile/atomicfile_test.go new file mode 100644 index 0000000..65ed033 --- /dev/null +++ b/internal/atomicfile/atomicfile_test.go @@ -0,0 +1,65 @@ +package atomicfile + +import ( + "os" + "path/filepath" + "testing" +) + +func TestWriteCreatesFileWithModeAndContents(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "out.txt") + + if err := Write(path, []byte("hello"), 0o640); err != nil { + t.Fatalf("Write: %v", err) + } + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("ReadFile: %v", err) + } + if string(data) != "hello" { + t.Errorf("contents = %q, want %q", data, "hello") + } + info, err := os.Stat(path) + if err != nil { + t.Fatalf("Stat: %v", err) + } + if info.Mode().Perm() != 0o640 { + t.Errorf("mode = %v, want %v", info.Mode().Perm(), os.FileMode(0o640)) + } +} + +func TestWriteReplacesExistingFileAtomically(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "out.txt") + if err := os.WriteFile(path, []byte("old"), 0o644); err != nil { + t.Fatalf("seed: %v", err) + } + + if err := Write(path, []byte("new"), 0o644); err != nil { + t.Fatalf("Write: %v", err) + } + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("ReadFile: %v", err) + } + if string(data) != "new" { + t.Errorf("contents = %q, want %q", data, "new") + } + + // No leftover temp file: the rename must have consumed it. + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatalf("ReadDir: %v", err) + } + if len(entries) != 1 || entries[0].Name() != "out.txt" { + t.Errorf("dir entries = %v, want only out.txt", entries) + } +} + +func TestWriteFailsOnMissingParentDir(t *testing.T) { + path := filepath.Join(t.TempDir(), "missing-subdir", "out.txt") + if err := Write(path, []byte("x"), 0o644); err == nil { + t.Fatal("Write: expected an error for a missing parent directory") + } +} diff --git a/internal/command/command.go b/internal/command/command.go index 1419e79..40248d2 100644 --- a/internal/command/command.go +++ b/internal/command/command.go @@ -22,6 +22,8 @@ import ( "os" "path/filepath" "time" + + "github.com/behnam-rk/dezhban/internal/atomicfile" ) // Op identifies a control operation. New ops are additive. @@ -71,25 +73,8 @@ func Write(path string, c Command) error { if err != nil { return fmt.Errorf("command: marshal: %w", err) } - tmp, err := os.CreateTemp(dir, ".command-*.json.tmp") - if err != nil { - return fmt.Errorf("command: create temp: %w", err) - } - tmpName := tmp.Name() - defer func() { _ = os.Remove(tmpName) }() - if _, err := tmp.Write(data); err != nil { - _ = tmp.Close() - return fmt.Errorf("command: write temp: %w", err) - } - if err := tmp.Sync(); err != nil { - _ = tmp.Close() - return fmt.Errorf("command: sync temp: %w", err) - } - if err := tmp.Close(); err != nil { - return fmt.Errorf("command: close temp: %w", err) - } - if err := os.Rename(tmpName, path); err != nil { - return fmt.Errorf("command: rename into place: %w", err) + if err := atomicfile.Write(path, data, 0o600); err != nil { + return fmt.Errorf("command: write %q: %w", path, err) } return nil } diff --git a/internal/config/config.go b/internal/config/config.go index 2a97e91..599e2d0 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -10,6 +10,8 @@ import ( "path/filepath" "strings" "time" + + "github.com/behnam-rk/dezhban/internal/atomicfile" ) // Allowlist names the destinations that must stay reachable even while blocking, @@ -223,6 +225,44 @@ type Advanced struct { // EndpointWarnThreshold is the union-size at which doctor warns about // rule-list bloat. Default 256. EndpointWarnThreshold int + // VerifyInterval is how often the daemon re-reads the firewall to confirm + // its rules are still installed, re-applying the posture in force when they + // are not. Default 1m; an explicit "0" disables the check entirely (negative + // sentinel internally, same convention as RedialMinUptime). + // + // It exists because every other Apply is triggered by something dezhban + // itself did — a tunnel change, an endpoint refresh, a posture flip. Nothing + // noticed a ruleset removed from OUTSIDE (another firewall tool, `pfctl -F + // all`, `nft flush ruleset`, an OS ruleset reload), so the daemon went on + // reporting GUARD while the host was open. A guard that can fail silently is + // the worst failure this tool has. + // + // The cadence is deliberately slow. Backend.IsBlocked costs two pfctl calls + // on macOS and one nft on Linux, but a whole PowerShell invocation on + // Windows, and it runs in the single run-loop goroutine that also owns window + // expiry and geo ticks — see docs/usage/config.md. + VerifyInterval time.Duration + // LivenessRedial lets a hung tunnel — the interface reports up, but a run of + // exit lookups through it has failed — open an automatic redial window, the + // same as an ordinary tunnel-down edge (trigger 2; see the package doc + // comment's "THREE sanctioned triggers"). Default false. + // + // This is the one knob in this file that WIDENS a relaxation trigger rather + // than narrowing or bounding one, which is why it defaults off and ships + // with its own ADR (docs/adr/0010-tunnel-liveness.md) rather than living + // here as a plain tunable. The hazard: an exit that CENSORS the geo + // providers produces the exact same failure streak as a genuinely dead + // tunnel — state.Snapshot's LookupErr doc names this case by name ("an + // Iranian exit blocking them looks exactly like this"). With this on, that + // censoring exit can trigger a relaxation window on a tunnel that was never + // actually down. The streak, its diagnosis, and the state field that + // reports it (state.ZombieState) are unconditional and on by default — + // only ACTING on the streak is gated by this key. + // + // Every existing rail on the automatic trigger still applies unchanged: + // vpn.advanced.redialBudget, redialMinUptime backoff, `dezhban hold`, + // one window per drop, redialWindowMax. + LivenessRedial bool // RedialMinUptime seeds the backoff on the automatic redial window: a tunnel // that was up for less than this, with no confirmed exit during that uptime, // still gets a window but a shortened one, halved again for each consecutive @@ -400,20 +440,24 @@ type fileProfile struct { } type fileAdvanced struct { - SwitchWindowMax string `json:"switchWindowMax,omitempty"` - RedialWindowMax string `json:"redialWindowMax,omitempty"` - CommandFreshness string `json:"commandFreshness,omitempty"` - WindowDiscoveryInterval string `json:"windowDiscoveryInterval,omitempty"` - TunnelPruneAfter string `json:"tunnelPruneAfter,omitempty"` - LearnedEndpointTTL string `json:"learnedEndpointTTL,omitempty"` - LearnedMaxPerProfile int `json:"learnedMaxPerProfile,omitempty"` - PromoteAfterRefreshes int `json:"promoteAfterRefreshes,omitempty"` - EndpointWarnThreshold int `json:"endpointWarnThreshold,omitempty"` - WindowProtocols []string `json:"windowProtocols,omitempty"` - WindowPorts []int `json:"windowPorts,omitempty"` - RedialMinUptime string `json:"redialMinUptime,omitempty"` - RedialBudget string `json:"redialBudget,omitempty"` - RedialBudgetWindow string `json:"redialBudgetWindow,omitempty"` + SwitchWindowMax string `json:"switchWindowMax,omitempty"` + RedialWindowMax string `json:"redialWindowMax,omitempty"` + CommandFreshness string `json:"commandFreshness,omitempty"` + WindowDiscoveryInterval string `json:"windowDiscoveryInterval,omitempty"` + TunnelPruneAfter string `json:"tunnelPruneAfter,omitempty"` + LearnedEndpointTTL string `json:"learnedEndpointTTL,omitempty"` + LearnedMaxPerProfile int `json:"learnedMaxPerProfile,omitempty"` + PromoteAfterRefreshes int `json:"promoteAfterRefreshes,omitempty"` + EndpointWarnThreshold int `json:"endpointWarnThreshold,omitempty"` + VerifyInterval string `json:"verifyInterval,omitempty"` + // Pointer, like every other bool in this file: an absent key must keep the + // default rather than being indistinguishable from an explicit "off". + LivenessRedial *bool `json:"livenessRedial,omitempty"` + WindowProtocols []string `json:"windowProtocols,omitempty"` + WindowPorts []int `json:"windowPorts,omitempty"` + RedialMinUptime string `json:"redialMinUptime,omitempty"` + RedialBudget string `json:"redialBudget,omitempty"` + RedialBudgetWindow string `json:"redialBudgetWindow,omitempty"` } // Default returns a Config with safe, security-first defaults. @@ -744,6 +788,23 @@ func applyAdvanced(fa *fileAdvanced) (Advanced, error) { a.RedialMinUptime = d } } + if fa.VerifyInterval != "" { + d, err := time.ParseDuration(fa.VerifyInterval) + if err != nil { + return a, fmt.Errorf("vpn.advanced.verifyInterval: %w", err) + } + if d < 0 { + return a, fmt.Errorf("vpn.advanced.verifyInterval: must not be negative (got %s); use \"0\" to disable", d) + } + if d == 0 { + a.VerifyInterval = Disabled // explicit opt-out of enforcement verification + } else { + a.VerifyInterval = d + } + } + if fa.LivenessRedial != nil { + a.LivenessRedial = *fa.LivenessRedial + } // The two budget keys take no Disabled sentinel (see Advanced.RedialBudget): // they are limits, so "0" would have to mean "no limit", which is the opposite // of what "0" means everywhere else in this config. Both a written "0" and a @@ -904,6 +965,10 @@ func toFileAdvanced(a Advanced) *fileAdvanced { fa.RedialMinUptime = optDurString(a.RedialMinUptime) nonDefault = true } + if a.VerifyInterval != defaultVerifyInterval { + fa.VerifyInterval = optDurString(a.VerifyInterval) + nonDefault = true + } // durString, not optDurString: these two carry no Disabled sentinel, so there // is no "0" to render. if a.RedialBudget != defaultRedialBudget { @@ -914,6 +979,11 @@ func toFileAdvanced(a Advanced) *fileAdvanced { fa.RedialBudgetWindow = durString(a.RedialBudgetWindow) nonDefault = true } + if a.LivenessRedial { + v := true + fa.LivenessRedial = &v + nonDefault = true + } if !nonDefault { return nil } @@ -958,38 +1028,15 @@ func Save(path string, c *Config) error { return fmt.Errorf("create config dir %q: %w", dir, err) } } - tmp, err := os.CreateTemp(dir, ".dezhban-config-*") - if err != nil { - return fmt.Errorf("stage config %q: %w", path, err) - } - tmpName := tmp.Name() - defer func() { _ = os.Remove(tmpName) }() // no-op once the rename succeeds - - // CreateTemp makes 0600; the published file must stay readable by the - // unprivileged tools that inspect it. - if err := tmp.Chmod(0o644); err != nil { - _ = tmp.Close() - return fmt.Errorf("write config %q: %w", path, err) - } - if _, err := tmp.Write(data); err != nil { - _ = tmp.Close() - return fmt.Errorf("write config %q: %w", path, err) - } // Flushed before the rename, or the rename could publish a name whose contents // are not on disk yet: a power loss then leaves a zero-length config, which is // the very "unparseable at boot, host unprotected" outcome staging exists to // prevent. The same reason internal/learned, internal/armed and internal/state - // all sync here. - if err := tmp.Sync(); err != nil { - _ = tmp.Close() - return fmt.Errorf("flush config %q: %w", path, err) - } - if err := tmp.Close(); err != nil { + // all sync here. 0644, not the temp file's default 0600: the published config + // must stay readable by the unprivileged tools that inspect it. + if err := atomicfile.Write(path, data, 0o644); err != nil { return fmt.Errorf("write config %q: %w", path, err) } - if err := os.Rename(tmpName, path); err != nil { - return fmt.Errorf("install config %q: %w", path, err) - } return nil } @@ -1088,6 +1135,12 @@ func normalizeAdvanced(a *Advanced) { if a.RedialMinUptime == 0 { a.RedialMinUptime = defaultRedialMinUptime } + // `== 0`, not `<= 0`: the negative Disabled sentinel is an explicit opt-out + // and must survive Normalize, exactly like the three windows above. Coercing + // it back to the default would silently re-enable a check the user turned off. + if a.VerifyInterval == 0 { + a.VerifyInterval = defaultVerifyInterval + } // Reached only for an ABSENT key: unlike the three windows and RedialMinUptime // above, these two take no Disabled sentinel, and applyAdvanced rejects any // written "0" or negative by name rather than letting it arrive here. So this @@ -1132,6 +1185,7 @@ const ( defaultLearnedMaxPerProfile = 16 defaultPromoteAfterRefreshes = 3 defaultEndpointWarnThreshold = 256 + defaultVerifyInterval = 1 * time.Minute defaultEndpointRefresh = 1 * time.Minute defaultTunnelWatch = 1 * time.Second // how fast a tunnel drop is noticed diff --git a/internal/config/reload.go b/internal/config/reload.go index 6817bfd..25610f2 100644 --- a/internal/config/reload.go +++ b/internal/config/reload.go @@ -77,6 +77,8 @@ func KeyValues(c *Config) map[string]string { "vpn.advanced.learnedMaxPerProfile": strconv.Itoa(adv.LearnedMaxPerProfile), "vpn.advanced.promoteAfterRefreshes": strconv.Itoa(adv.PromoteAfterRefreshes), "vpn.advanced.endpointWarnThreshold": strconv.Itoa(adv.EndpointWarnThreshold), + "vpn.advanced.verifyInterval": dur(adv.VerifyInterval), + "vpn.advanced.livenessRedial": strconv.FormatBool(adv.LivenessRedial), "vpn.advanced.windowProtocols": strings.Join(adv.WindowProtocols, ","), "vpn.advanced.windowPorts": joinInts(adv.WindowPorts), } @@ -163,6 +165,8 @@ var liveKeys = map[string]bool{ "vpn.advanced.redialBudget": true, "vpn.advanced.redialBudgetWindow": true, "vpn.advanced.windowDiscoveryInterval": true, + "vpn.advanced.verifyInterval": true, + "vpn.advanced.livenessRedial": true, } // restartReasonFor returns why a key cannot be applied live, or "" when it can. @@ -248,6 +252,8 @@ func MergeLive(base, cur *Config) *Config { out.VPN.Advanced.RedialBudget = cur.VPN.Advanced.RedialBudget out.VPN.Advanced.RedialBudgetWindow = cur.VPN.Advanced.RedialBudgetWindow out.VPN.Advanced.WindowDiscoveryInterval = cur.VPN.Advanced.WindowDiscoveryInterval + out.VPN.Advanced.VerifyInterval = cur.VPN.Advanced.VerifyInterval + out.VPN.Advanced.LivenessRedial = cur.VPN.Advanced.LivenessRedial return &out } diff --git a/internal/config/reload_test.go b/internal/config/reload_test.go index 4443ba3..89f4276 100644 --- a/internal/config/reload_test.go +++ b/internal/config/reload_test.go @@ -207,6 +207,8 @@ func TestMergeLiveCoversExactlyTheLiveKeys(t *testing.T) { cur.VPN.Advanced.RedialBudget = base.VPN.Advanced.RedialBudget + time.Second cur.VPN.Advanced.RedialBudgetWindow = base.VPN.Advanced.RedialBudgetWindow + time.Second cur.VPN.Advanced.WindowDiscoveryInterval = base.VPN.Advanced.WindowDiscoveryInterval + time.Second + cur.VPN.Advanced.VerifyInterval = base.VPN.Advanced.VerifyInterval + time.Second + cur.VPN.Advanced.LivenessRedial = !base.VPN.Advanced.LivenessRedial moved := map[string]bool{} for _, ch := range Changes(&base, MergeLive(&base, &cur)) { diff --git a/internal/config/schema.go b/internal/config/schema.go index e1ac8f0..fbb3a3f 100644 --- a/internal/config/schema.go +++ b/internal/config/schema.go @@ -327,6 +327,23 @@ var tunables = []Tunable{ Help: "A tunnel that was up for less than this still gets a window, but a shorter one for each consecutive fast drop, with a growing wait between them. Off gives every drop a full window until the budget runs out.", DocAnchor: anchorAdvanced, }, + { + Key: "vpn.advanced.verifyInterval", + Label: "Enforcement verification interval", + Kind: KindDuration, + Advanced: true, + Disablable: true, + Help: "How often dezhban confirms its firewall rules are still installed, re-applying them if something removed them from outside. Off trusts the rules to stay put once applied.", + DocAnchor: anchorAdvanced, + }, + { + Key: "vpn.advanced.livenessRedial", + Label: "Redial on a hung tunnel", + Kind: KindBool, + Advanced: true, + Help: "Lets a tunnel that reports up but has stopped passing traffic open an automatic redial window, the same as an ordinary drop. Off by default: an exit that censors the geo lookup looks identical to a hung tunnel, and this would let it trigger a window on a tunnel that was never actually down.", + DocAnchor: anchorAdvanced, + }, // Not Disablable, unlike almost every other duration here. These two are // limits, so an Off switch would have to mean "no limit" — the opposite of // what Off means on every other row, and the wrong direction to offer on a diff --git a/internal/config/schema_test.go b/internal/config/schema_test.go index cb3778b..45022a0 100644 --- a/internal/config/schema_test.go +++ b/internal/config/schema_test.go @@ -112,6 +112,7 @@ func TestDisablableKeysSurviveNormalize(t *testing.T) { "vpn.redialWindow": func(c *Config) *time.Duration { return &c.VPN.RedialWindow }, "vpn.pauseMax": func(c *Config) *time.Duration { return &c.VPN.PauseMax }, "vpn.advanced.redialMinUptime": func(c *Config) *time.Duration { return &c.VPN.Advanced.RedialMinUptime }, + "vpn.advanced.verifyInterval": func(c *Config) *time.Duration { return &c.VPN.Advanced.VerifyInterval }, } var disablable []string diff --git a/internal/firewall/nft_linux.go b/internal/firewall/nft_linux.go index c22505d..4e69883 100644 --- a/internal/firewall/nft_linux.go +++ b/internal/firewall/nft_linux.go @@ -67,10 +67,19 @@ func (b *nftBackend) Apply(p Policy) error { // Unblock removes ONLY dezhban's table (surgical — touches nothing else). A // missing table is not an error: unblock must be safe to run when nothing is // blocked. +// +// Gated on the table EXISTING, deliberately not on IsBlocked: IsBlocked is the +// stricter "is this actually enforcing" question and answers false for a table +// whose output-chain policy drifted to accept. Teardown must not inherit that +// strictness — a drifted table is still dezhban's table, and skipping the +// delete would have `unblock`/`panic`/`Cleanup` report success while leaving +// our ruleset installed, breaking the surgical-teardown invariant. func (b *nftBackend) Unblock() error { - if blocked, err := b.IsBlocked(); err != nil { + _, exists, err := b.listTable() + if err != nil { return err - } else if !blocked { + } + if !exists { return nil } if _, err := nft("", "delete", "table", "inet", tableName); err != nil { @@ -79,19 +88,84 @@ func (b *nftBackend) Unblock() error { return nil } -// IsBlocked reports whether the `inet dezhban` table exists. Unlike pf there is -// no separate enabled/disabled state: a present table is always enforcing. -func (b *nftBackend) IsBlocked() (bool, error) { - if _, err := nft("", "list", "table", "inet", tableName); err != nil { +// listTable renders `nft list table inet dezhban`, reporting whether the table +// exists at all separately from a real failure to ask. Shared by Unblock (which +// only cares about existence) and IsBlocked (which also inspects the rendered +// policy), so the two can never drift on what "the table is not there" looks +// like. +func (b *nftBackend) listTable() (out string, exists bool, err error) { + out, err = nft("", "list", "table", "inet", tableName) + if err != nil { // nft exits non-zero when the table does not exist. Distinguish that // (not blocked) from a real failure by matching the kernel's message. if strings.Contains(err.Error(), "No such file or directory") || strings.Contains(err.Error(), "does not exist") { - return false, nil + return "", false, nil } + return "", false, err + } + return out, true, nil +} + +// IsBlocked reports whether the `inet dezhban` table exists AND its output +// chain's policy is still drop. +// +// The table existing is not sufficient on its own: nft lets a chain's hook +// policy be rewritten in place (`nft add chain inet dezhban output { policy +// accept; }`) without deleting or recreating the table, which leaves every +// accept rule we installed intact while unmatched egress — the actual +// default-deny this whole ruleset exists to provide — sails straight through. +// A bare table-existence check would report "blocked" through that gap the +// whole time. `policy drop` is the literal text nft echoes back for the +// chain's hook policy in `list table`, so checking for it here catches that +// drift the same way pf's anchor-reference check (pf_darwin.go) and +// Windows' DefaultOutboundAction check (wfp_windows.go) catch theirs. +func (b *nftBackend) IsBlocked() (bool, error) { + out, exists, err := b.listTable() + if err != nil || !exists { return false, err } - return true, nil + return outputChainPolicyIsDrop(out), nil +} + +// outputChainPolicyIsDrop reports whether nft's rendered `list table` output +// still shows the output chain's hook policy as drop. Split out from +// IsBlocked so it can be exercised in tests against captured `nft list table` +// output without shelling out — nft requires root/CAP_NET_ADMIN and this +// package has no test seam for it, the same rationale as pf_darwin's +// mainRulesetReferencesAnchor and wfp_windows' parseProfileQuery. +func outputChainPolicyIsDrop(out string) bool { + // Scoped to the "output" chain block specifically (matching + // renderNftRuleset's `add chain inet %s output {...}`), not a table-wide + // substring search: a table-wide search would report "blocked" as long as + // the text "policy drop" appears ANYWHERE in `list table`'s output — which + // would go on being true even after the output chain's own policy drifted + // to accept, as long as some other chain in the table still says "policy + // drop". Only the "output" chain's own policy line may answer this. + inOutputChain := false + depth := 0 + for _, line := range strings.Split(out, "\n") { + trimmed := strings.TrimSpace(line) + if !inOutputChain { + if trimmed == "chain output {" || strings.HasPrefix(trimmed, "chain output ") { + inOutputChain = true + depth = strings.Count(line, "{") - strings.Count(line, "}") + } + continue + } + if strings.Contains(line, "policy drop") { + return true + } + depth += strings.Count(line, "{") - strings.Count(line, "}") + if depth <= 0 { + // The output chain's block closed without ever showing "policy + // drop" — its policy is something else (accept), or nft's output + // shape changed in a way this no longer recognises. Either way, + // this is not the confirmed-drop state IsBlocked promises. + return false + } + } + return false } // Cleanup is best-effort teardown for shutdown/panic. It is just Unblock; any diff --git a/internal/firewall/nft_linux_test.go b/internal/firewall/nft_linux_test.go index d767964..79fb1ae 100644 --- a/internal/firewall/nft_linux_test.go +++ b/internal/firewall/nft_linux_test.go @@ -47,6 +47,57 @@ func assertDefaultDrop(t *testing.T, rs string) { } } +// TestOutputChainPolicyIsDrop exercises IsBlocked's drift-detection substring +// match against the shape `nft list table inet dezhban` actually renders. +// Captured by hand rather than run against a live kernel — nft requires +// root/CAP_NET_ADMIN and this package has no test seam to fake it, same as +// pf_darwin's TestMainRulesetReferencesAnchor and wfp_windows' +// TestParseProfileQuery. +func TestOutputChainPolicyIsDrop(t *testing.T) { + const dropPolicy = `table inet dezhban { + chain output { + type filter hook output priority 0; policy drop; + oifname "lo" accept + ip daddr 10.0.0.1 accept + } +}` + if !outputChainPolicyIsDrop(dropPolicy) { + t.Errorf("expected policy drop to be found in:\n%s", dropPolicy) + } + + // A chain's hook policy can be rewritten in place (`nft add chain inet + // dezhban output { policy accept; }`) without touching a single accept + // rule or deleting the table — the exact drift this check exists to catch. + const accept = `table inet dezhban { + chain output { + type filter hook output priority 0; policy accept; + oifname "lo" accept + ip daddr 10.0.0.1 accept + } +}` + if outputChainPolicyIsDrop(accept) { + t.Errorf("expected no policy drop to be found in:\n%s", accept) + } + + // The check must be scoped to the "output" chain specifically, not a + // table-wide substring search: some OTHER chain still saying "policy + // drop" must never paper over the output chain itself having drifted to + // accept — that is the exact drift this function exists to catch. + const otherChainStillDrops = `table inet dezhban { + chain output { + type filter hook output priority 0; policy accept; + oifname "lo" accept + } + chain unrelated { + type filter hook input priority 0; policy drop; + } +}` + if outputChainPolicyIsDrop(otherChainStillDrops) { + t.Errorf("expected no policy drop — the output chain itself is accept, "+ + "even though another chain still says drop:\n%s", otherChainStillDrops) + } +} + func TestRenderNftLegacyFullBlock(t *testing.T) { p := Policy{ Mode: ModeFullBlock, diff --git a/internal/firewall/pf_darwin.go b/internal/firewall/pf_darwin.go index 2366b47..4e348ac 100644 --- a/internal/firewall/pf_darwin.go +++ b/internal/firewall/pf_darwin.go @@ -3,6 +3,7 @@ package firewall import ( + "context" "encoding/json" "fmt" "net/netip" @@ -10,6 +11,7 @@ import ( "path/filepath" "strings" + "github.com/behnam-rk/dezhban/internal/atomicfile" "github.com/behnam-rk/dezhban/internal/state" ) @@ -139,7 +141,7 @@ func (b *pfBackend) Unblock() error { if err != nil { return fmt.Errorf("read pf.conf backup: %w", err) } - if err := atomicWrite(pfConfPath, data, 0o644); err != nil { + if err := atomicfile.Write(pfConfPath, data, 0o644); err != nil { return fmt.Errorf("restore pf.conf: %w", err) } _ = os.Remove(backupPath) @@ -164,8 +166,17 @@ func (b *pfBackend) Unblock() error { return nil } +// IsBlocked issues three pfctl reads (anchor rules, pf status, main ruleset). +// They share ONE pfctlTimeout deadline via pfctlCtx rather than each getting +// its own fresh budget: the daemon's run loop calls IsBlocked from its single +// goroutine on every verifyC tick, and window timers / geo ticks / control- +// socket replies are select cases on that same loop — a bare 3x pfctl("", ...) +// here could stall them for up to 3*pfctlTimeout under pf lock contention. func (b *pfBackend) IsBlocked() (bool, error) { - out, err := pfctl("", "-a", anchorName, "-s", "rules") + ctx, cancel := context.WithTimeout(context.Background(), pfctlTimeout) + defer cancel() + + out, err := pfctlCtx(ctx, "", "-a", anchorName, "-s", "rules") if err != nil { return false, err } @@ -174,11 +185,36 @@ func (b *pfBackend) IsBlocked() (bool, error) { } // Anchor rules are loaded but only enforced while pf itself is enabled, so a // stale anchor under a disabled pf must not report as blocked. - info, err := pfctl("", "-s", "info") + info, err := pfctlCtx(ctx, "", "-s", "info") if err != nil { return false, err } - return strings.Contains(info, "Status: Enabled"), nil + if !strings.Contains(info, "Status: Enabled") { + return false, nil + } + // The anchor's own rules can be loaded and non-empty while pf never actually + // evaluates them: pf only descends into a sub-anchor if the MAIN ruleset + // references it, and that reference lives in /etc/pf.conf — a file something + // else (a config-management tool, a manual `pfctl -f`) can overwrite without + // touching our anchor at all. Loaded-but-unreferenced would report blocked + // while every packet sails past the anchor unevaluated, exactly the silent + // gap enforcement verification exists to catch. `pfctl -s rules` lists the + // main ruleset as loaded right now, independent of what /etc/pf.conf says on + // disk, so this catches both a missing anchor line AND a main ruleset that + // was reloaded from some other file entirely. + main, err := pfctlCtx(ctx, "", "-s", "rules") + if err != nil { + return false, err + } + return mainRulesetReferencesAnchor(main), nil +} + +// mainRulesetReferencesAnchor reports whether pfctl's rendered main ruleset +// still contains our anchor reference. Split out from IsBlocked so it can be +// exercised in tests against captured `pfctl -s rules` output without +// shelling out — pfctl requires root and this repo has no test seam for it. +func mainRulesetReferencesAnchor(main string) bool { + return strings.Contains(main, anchorRef) } // Cleanup is best-effort teardown for shutdown/panic. It is just Unblock; any @@ -397,7 +433,7 @@ func ensureAnchorRef() error { body += "\n" } body += "# dezhban anchor (Phase 2) — removed on unblock by restoring the backup\n" + anchorRef + "\n" - if err := atomicWrite(pfConfPath, []byte(body), 0o644); err != nil { + if err := atomicfile.Write(pfConfPath, []byte(body), 0o644); err != nil { return fmt.Errorf("append anchor to %s: %w", pfConfPath, err) } return nil @@ -414,7 +450,7 @@ func backupPfConf() error { if err != nil { return fmt.Errorf("read %s for backup: %w", pfConfPath, err) } - if err := atomicWrite(backupPath, data, 0o600); err != nil { + if err := atomicfile.Write(backupPath, data, 0o600); err != nil { return fmt.Errorf("write pf.conf backup: %w", err) } return nil @@ -433,39 +469,12 @@ func saveState(s savedState) error { if err != nil { return err } - if err := atomicWrite(statePath, data, 0o600); err != nil { + if err := atomicfile.Write(statePath, data, 0o600); err != nil { return fmt.Errorf("write state: %w", err) } return nil } -// atomicWrite writes data to a temp file in the same directory, fsyncs it, then -// renames it over path — so a crash mid-write can never leave a partially -// written /etc/pf.conf or state file that a later restore would trust. -func atomicWrite(path string, data []byte, mode os.FileMode) error { - tmp, err := os.CreateTemp(filepath.Dir(path), ".dezhban-*.tmp") - if err != nil { - return err - } - tmpName := tmp.Name() - defer os.Remove(tmpName) // no-op once the rename succeeds - if _, err := tmp.Write(data); err != nil { - tmp.Close() - return err - } - if err := tmp.Sync(); err != nil { - tmp.Close() - return err - } - if err := tmp.Close(); err != nil { - return err - } - if err := os.Chmod(tmpName, mode); err != nil { - return err - } - return os.Rename(tmpName, path) -} - func loadState() (savedState, bool) { data, err := os.ReadFile(statePath) if err != nil { diff --git a/internal/firewall/pf_darwin_test.go b/internal/firewall/pf_darwin_test.go index 9b1f9a0..ad8e8e0 100644 --- a/internal/firewall/pf_darwin_test.go +++ b/internal/firewall/pf_darwin_test.go @@ -389,3 +389,32 @@ func TestRenderTunnelScopedProviders(t *testing.T) { t.Errorf("GUARD should not emit a provider pass — it already passes all tunnel egress:\n%s", g) } } + +// TestMainRulesetReferencesAnchor exercises IsBlocked's drift-detection +// substring match against the shape `pfctl -s rules` actually renders: an +// anchor declared without filter criteria in pf.conf comes back with pfctl's +// own "all" appended, sitting among Apple's own scrub-anchor/anchor lines and +// dezhban's loopback-always rule. Captured by hand from a `pfctl -s rules` +// run rather than the live daemon — pfctl requires root and this package has +// no test seam to fake it, so this fixture is the best available substitute +// for the on-host check the PR's own test plan still flags as unverified. +func TestMainRulesetReferencesAnchor(t *testing.T) { + const rendered = `scrub-anchor "com.apple/*" all fragment reassemble +anchor "com.apple/*" all +pass on lo0 all flags S/SA no state +anchor "dezhban" all +block drop out all` + + if !mainRulesetReferencesAnchor(rendered) { + t.Errorf("expected anchor reference to be found in:\n%s", rendered) + } + + const missing = `scrub-anchor "com.apple/*" all fragment reassemble +anchor "com.apple/*" all +pass on lo0 all flags S/SA no state +block drop out all` + + if mainRulesetReferencesAnchor(missing) { + t.Errorf("expected no anchor reference to be found in:\n%s", missing) + } +} diff --git a/internal/firewall/pfctl.go b/internal/firewall/pfctl.go index c9f6f00..327c509 100644 --- a/internal/firewall/pfctl.go +++ b/internal/firewall/pfctl.go @@ -21,7 +21,13 @@ const pfctlTimeout = 10 * time.Second func pfctl(stdin string, args ...string) (string, error) { ctx, cancel := context.WithTimeout(context.Background(), pfctlTimeout) defer cancel() + return pfctlCtx(ctx, stdin, args...) +} +// pfctlCtx is pfctl with a caller-supplied context, so a run-loop caller that +// issues several pfctl invocations back to back (e.g. IsBlocked) can share one +// deadline instead of each call getting its own fresh pfctlTimeout budget. +func pfctlCtx(ctx context.Context, stdin string, args ...string) (string, error) { cmd := exec.CommandContext(ctx, "pfctl", args...) if stdin != "" { cmd.Stdin = strings.NewReader(stdin) diff --git a/internal/firewall/wfp_windows.go b/internal/firewall/wfp_windows.go index 68f5eed..3023f42 100644 --- a/internal/firewall/wfp_windows.go +++ b/internal/firewall/wfp_windows.go @@ -9,6 +9,8 @@ import ( "os" "path/filepath" "strings" + + "github.com/behnam-rk/dezhban/internal/atomicfile" ) // Windows enforcement via the Windows Firewall (NetSecurity cmdlets). @@ -43,6 +45,23 @@ func stateDir() string { func statePath() string { return filepath.Join(stateDir(), "fw.state") } +// appliedActionPath records the DefaultOutboundAction the last successful Apply +// set every profile to — IsBlocked's cross-check that the boundary Apply +// actually installed hasn't drifted out from under the still-present allow +// rules (see IsBlocked's doc comment). +func appliedActionPath() string { return filepath.Join(stateDir(), "fw.applied") } + +// writeAppliedAction records action as the DefaultOutboundAction Apply just +// applied, via temp-file-then-rename rather than a plain WriteFile — the +// rename is atomic, so a crash between the two never leaves this file +// half-written. IsBlocked's drift check reads this file's exact bytes back +// and compares them against the live profiles, so a truncated/empty file +// from a non-atomic write would read as "every profile drifted" and force a +// repair loop that only a later, fully-written Apply would clear. +func writeAppliedAction(action string) error { + return atomicfile.Write(appliedActionPath(), []byte(action), 0o600) +} + // wfpBackend is the Windows FirewallBackend. It holds no in-memory state: the // authoritative state is the dezhban rule group plus the saved DefaultOutbound // snapshot on disk, so it survives across separate invocations. @@ -94,6 +113,21 @@ func (b *wfpBackend) Apply(p Policy) error { if _, err := powershell(renderBlockScript(p)); err != nil { return fmt.Errorf("apply dezhban firewall rules: %w", err) } + // Best-effort: IsBlocked degrades to its old, weaker check if this is + // missing (e.g. leftover state from before this file existed), so a + // failure here must not fail the Apply that just succeeded. Written + // atomically (temp + rename), which means a failed write leaves the + // PREVIOUS Apply's value untouched on disk rather than truncating it — + // so on failure we remove it outright instead of leaving it in place. + // A stale-but-present file would make IsBlocked's drift check compare + // the live (correctly re-applied) profiles against what an EARLIER + // Apply set, not this one, misreporting real drift and triggering an + // unwanted repair. An absent file is already the designed fallback — + // "no record of what was last applied" degrades to the weaker + // group-existence-only check, not evidence of tampering. + if err := writeAppliedAction(expectedOutboundAction(p)); err != nil { + _ = os.Remove(appliedActionPath()) + } return nil } @@ -129,18 +163,127 @@ func (b *wfpBackend) Unblock() error { if ok { _ = os.Remove(statePath()) } + _ = os.Remove(appliedActionPath()) return nil } -// IsBlocked reports whether the dezhban rule group is currently installed. +// IsBlocked reports whether the dezhban rule group is currently installed AND +// the profile outbound default Apply set still matches what it applied. +// +// The rule group existing is not sufficient on its own: our rules are all +// Allow exceptions layered on the profile DefaultOutboundAction doing the +// actual blocking (see the Model note above renderBlockScript). Something +// else — Group Policy refresh, another security tool, an admin running +// `Set-NetFirewallProfile` by hand — can flip that default back to Allow +// without touching a single dezhban rule, leaving the group intact while +// every packet the Allow rules didn't already cover sails through unfiltered. +// A bare group-existence check would report "blocked" through that entire +// gap. Cross-checking against the action Apply actually persisted +// (appliedActionPath) catches it, while still tolerating the ONE posture +// where Allow is the deliberately-correct default: an unrestricted switch +// window (see expectedOutboundAction) — comparing against what THIS Apply +// call set, not a hardcoded "Block", is what makes that distinction safe +// instead of reporting a false "MISSING" (and triggering an unwanted repair) +// every time a window opens. func (b *wfpBackend) IsBlocked() (bool, error) { - out, err := powershell( - "if (Get-NetFirewallRule -Group " + groupName + - " -ErrorAction SilentlyContinue) { 'blocked' } else { 'clear' }") + blocked, got, err := queryBlockedAndDefaults() if err != nil { return false, err } - return strings.Contains(out, "blocked"), nil + if !blocked { + return false, nil + } + + wantRaw, err := os.ReadFile(appliedActionPath()) + if err != nil { + // No record of what we last applied (state predates this file, or was + // cleared) — degrade to the group-existence check above rather than + // treat an unrelated read failure as evidence of tampering. + return true, nil + } + want := strings.TrimSpace(string(wantRaw)) + + for _, prof := range fwProfiles { + if got[prof] != want { + return false, nil + } + } + return true, nil +} + +// queryBlockedAndDefaults combines the group-existence check and the +// per-profile DefaultOutboundAction query into a single PowerShell +// invocation. IsBlocked is called synchronously from the run loop's verifyC +// tick, which CLAUDE.md requires stay bounded — two sequential psTimeout- +// bounded subprocess calls would nearly double that tick's worst case. +func queryBlockedAndDefaults() (bool, map[string]string, error) { + out, err := powershell( + "$g = Get-NetFirewallRule -Group " + groupName + " -ErrorAction SilentlyContinue\n" + + "if ($g) {\n" + + " 'blocked'\n" + + " Get-NetFirewallProfile -All | ForEach-Object { \"$($_.Name)=$($_.DefaultOutboundAction)\" }\n" + + "} else {\n" + + " 'clear'\n" + + "}") + if err != nil { + return false, nil, err + } + return parseProfileQuery(out) +} + +// parseProfileQuery parses queryBlockedAndDefaults' captured PowerShell output. +// Split out so it can be exercised in tests against captured tool output +// without shelling out to PowerShell — same rationale as pf_darwin's +// mainRulesetReferencesAnchor. +func parseProfileQuery(out string) (bool, map[string]string, error) { + lines := strings.Split(out, "\n") + // Scan for the exact "blocked"/"clear" marker rather than requiring it on + // lines[0]: -ErrorAction SilentlyContinue on Get-NetFirewallRule only + // suppresses the error stream, not a warning written to the success + // stream, so incidental leading text ahead of the script's own output is + // possible even under -NonInteractive. Matching a whole trimmed line — + // never a substring — still refuses to treat the marker text as found + // merely because it appears inside unrelated output. + markerAt, blocked := -1, false + for i, line := range lines { + switch strings.TrimSpace(line) { + case "blocked": + markerAt, blocked = i, true + case "clear": + markerAt, blocked = i, false + default: + continue + } + break + } + if markerAt < 0 || !blocked { + return false, nil, nil + } + res := make(map[string]string) + for _, line := range lines[markerAt+1:] { + line = strings.TrimSpace(line) + if name, action, ok := strings.Cut(line, "="); ok { + res[strings.TrimSpace(name)] = strings.TrimSpace(action) + } + } + // Every profile in fwProfiles must have a line, or a caller comparing + // got[prof] against a wanted action would silently read a missing entry as + // "" — the Go zero value — which never matches, so a transient + // PowerShell/WMI hiccup that drops one profile's line (while the script + // still exits 0) would report real drift and trigger an unwanted repair. + // That is not evidence of tampering, same discipline as the unreadable + // appliedActionPath case in IsBlocked: an incomplete read is an error, not + // a "missing" verdict. + var missing []string + for _, prof := range fwProfiles { + if _, ok := res[prof]; !ok { + missing = append(missing, prof) + } + } + if len(missing) > 0 { + return false, nil, fmt.Errorf("firewall profile query missing output for: %s", strings.Join(missing, ", ")) + } + return true, res, nil } // Cleanup is best-effort teardown for shutdown/panic. It is just Unblock; any @@ -179,19 +322,15 @@ func renderBlockScript(p Policy) string { // Loopback always passes. rule("loopback", "-RemoteAddress 127.0.0.1,::1") - // defaultAction is the profile's outbound default installed at the end. It is - // Block for every posture EXCEPT an unrestricted switch window, which must - // allow all outbound so a brand-new VPN's handshake can complete. (Windows - // ignores TunnelGroups — it matches interfaces by exact alias only.) - defaultAction := "Block" + defaultAction := expectedOutboundAction(p) switch p.Mode { case ModeSwitchWindow: if len(p.WindowProtos) == 0 && len(p.WindowPorts) == 0 { // Unrestricted: keep only the marker (loopback) rule so the group stays - // non-empty for surgical teardown, and flip the default to Allow. The - // daemon reverts to guard (default Block) when the window closes. - defaultAction = "Allow" + // non-empty for surgical teardown. defaultAction is already Allow (see + // expectedOutboundAction). The daemon reverts to guard (default Block) + // when the window closes. } else { if len(p.TunnelIfaces) > 0 { rule("tunnel", "-InterfaceAlias "+psStringList(p.TunnelIfaces)) @@ -246,6 +385,19 @@ func renderBlockScript(p Policy) string { return b.String() } +// expectedOutboundAction is the profile DefaultOutboundAction renderBlockScript +// installs for p, and what IsBlocked cross-checks the live profiles against +// (see IsBlocked's doc comment). Block for every posture EXCEPT an unrestricted +// switch window, which must allow all outbound so a brand-new VPN's handshake +// can complete. Factored out of renderBlockScript so Apply can persist the +// value it actually applied without the two ever drifting apart. +func expectedOutboundAction(p Policy) string { + if p.Mode == ModeSwitchWindow && len(p.WindowProtos) == 0 && len(p.WindowPorts) == 0 { + return "Allow" + } + return "Block" +} + // emitWindowPortRules renders the proto/port allows for a restricted switch // window (WFP). Protocols default to udp+tcp when unspecified. func emitWindowPortRules(rule func(name, args string), p Policy) { diff --git a/internal/firewall/wfp_windows_test.go b/internal/firewall/wfp_windows_test.go index f07a538..041745b 100644 --- a/internal/firewall/wfp_windows_test.go +++ b/internal/firewall/wfp_windows_test.go @@ -179,6 +179,110 @@ func TestRenderBlockScriptSwitchWindowRestricted(t *testing.T) { } } +// expectedOutboundAction is what Apply persists for IsBlocked's drift check +// (see wfp_windows.go), so it must agree with what renderBlockScript actually +// installs — pinned directly rather than only indirectly via the script string. +func TestExpectedOutboundAction(t *testing.T) { + cases := []struct { + name string + p Policy + want string + }{ + {"guard", Policy{Mode: ModeGuard, TunnelIfaces: []string{"utun4"}}, "Block"}, + {"full block", Policy{Mode: ModeFullBlock}, "Block"}, + {"unrestricted switch window", Policy{Mode: ModeSwitchWindow}, "Allow"}, + {"restricted switch window", Policy{ + Mode: ModeSwitchWindow, + WindowProtos: []string{"udp"}, + WindowPorts: []int{51820}, + }, "Block"}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + if got := expectedOutboundAction(c.p); got != c.want { + t.Errorf("expectedOutboundAction(%+v) = %q, want %q", c.p, got, c.want) + } + }) + } +} + +func TestParseProfileQuery(t *testing.T) { + t.Run("clear", func(t *testing.T) { + blocked, got, err := parseProfileQuery("clear\n") + if err != nil || blocked || got != nil { + t.Fatalf("parseProfileQuery(clear) = (%v, %v, %v), want (false, nil, nil)", blocked, got, err) + } + }) + + t.Run("blocked with all profiles present", func(t *testing.T) { + out := "blocked\nDomain=Block\nPrivate=Block\nPublic=Allow\n" + blocked, got, err := parseProfileQuery(out) + if err != nil { + t.Fatalf("parseProfileQuery: unexpected error: %v", err) + } + if !blocked { + t.Fatalf("parseProfileQuery: got blocked=false, want true") + } + want := map[string]string{"Domain": "Block", "Private": "Block", "Public": "Allow"} + if len(got) != len(want) { + t.Fatalf("parseProfileQuery: got %v, want %v", got, want) + } + for k, v := range want { + if got[k] != v { + t.Errorf("parseProfileQuery: got[%q] = %q, want %q", k, got[k], v) + } + } + }) + + // A transient PowerShell/WMI hiccup that drops one profile's line — while + // the script still exits 0 — must read as an unreadable query, not as + // evidence that the Public profile's default actually changed. A caller + // comparing the zero-value "" against a wanted action would otherwise + // report false drift and trigger an unwanted repair. + t.Run("blocked but missing a profile line", func(t *testing.T) { + out := "blocked\nDomain=Block\nPrivate=Block\n" + blocked, got, err := parseProfileQuery(out) + if err == nil { + t.Fatalf("parseProfileQuery: got no error for a missing profile line, want an error") + } + if blocked || got != nil { + t.Fatalf("parseProfileQuery: got (%v, %v) alongside the error, want (false, nil)", blocked, got) + } + }) + + // -ErrorAction SilentlyContinue only suppresses the error stream, not a + // warning PowerShell writes to the success stream ahead of the script's + // own output — this must not be misread as "not blocked" just because it + // is not lines[0]. + t.Run("blocked with an incidental leading line", func(t *testing.T) { + out := "WARNING: some incidental PowerShell notice\nblocked\nDomain=Block\nPrivate=Block\nPublic=Block\n" + blocked, got, err := parseProfileQuery(out) + if err != nil { + t.Fatalf("parseProfileQuery: unexpected error: %v", err) + } + if !blocked { + t.Fatalf("parseProfileQuery: got blocked=false, want true — a leading noise line must not hide the marker") + } + want := map[string]string{"Domain": "Block", "Private": "Block", "Public": "Block"} + for k, v := range want { + if got[k] != v { + t.Errorf("parseProfileQuery: got[%q] = %q, want %q", k, got[k], v) + } + } + }) + + // The marker text must still only match on its own line — never as a + // substring inside unrelated output, which would be a worse regression + // than the strict lines[0] check this replaced. + t.Run("blocked text embedded in noise is not a marker", func(t *testing.T) { + out := "this line mentions blocked in passing\nclear\n" + blocked, got, err := parseProfileQuery(out) + if err != nil || blocked || got != nil { + t.Fatalf("parseProfileQuery(embedded) = (%v, %v, %v), want (false, nil, nil)", blocked, got, err) + } + }) +} + func TestRenderBlockScriptZeroTunnelStandingPosture(t *testing.T) { s := renderBlockScript(Policy{ Mode: ModeFullBlock, diff --git a/internal/learned/learned.go b/internal/learned/learned.go index bf5a19e..44c93cb 100644 --- a/internal/learned/learned.go +++ b/internal/learned/learned.go @@ -28,6 +28,8 @@ import ( "sort" "strings" "time" + + "github.com/behnam-rk/dezhban/internal/atomicfile" ) // version is the on-disk schema version. Bump on an incompatible change. @@ -94,28 +96,8 @@ func (s *Store) Save(path string) error { if err != nil { return fmt.Errorf("learned: marshal: %w", err) } - tmp, err := os.CreateTemp(dir, ".learned-*.json.tmp") - if err != nil { - return fmt.Errorf("learned: create temp: %w", err) - } - tmpName := tmp.Name() - defer func() { _ = os.Remove(tmpName) }() - if _, err := tmp.Write(data); err != nil { - _ = tmp.Close() - return fmt.Errorf("learned: write temp: %w", err) - } - if err := tmp.Sync(); err != nil { - _ = tmp.Close() - return fmt.Errorf("learned: sync temp: %w", err) - } - if err := tmp.Close(); err != nil { - return fmt.Errorf("learned: close temp: %w", err) - } - if err := os.Chmod(tmpName, 0o644); err != nil { - return fmt.Errorf("learned: chmod temp: %w", err) - } - if err := os.Rename(tmpName, path); err != nil { - return fmt.Errorf("learned: rename into place: %w", err) + if err := atomicfile.Write(path, data, 0o644); err != nil { + return fmt.Errorf("learned: write %q: %w", path, err) } return nil } diff --git a/internal/render/render.go b/internal/render/render.go index 235faf5..ad7ec19 100644 --- a/internal/render/render.go +++ b/internal/render/render.go @@ -41,7 +41,7 @@ const ( KeyOn = "on" // guarding, traffic flows through the tunnel KeyOff = "off" // standby or stopped — nothing is enforced KeyBlocked = "blocked" // full block, or a guard holding a downed tunnel - KeyWarning = "warning" // a switch/redial window is open, or enforcement failed + KeyWarning = "warning" // a switch/redial window is open, enforcement failed, or verification/liveness flagged something KeyPaused = "paused" // an operator-requested pause is open ) @@ -79,17 +79,57 @@ func Posture(s state.Snapshot) Display { // not recognise (an older or newer daemon), so a caller never needs its own // fallback. func Text(s state.Snapshot) Display { + if s.PanicDisarmed { + // `dezhban panic` tore the rules down deliberately and this daemon is + // standing down rather than silently reinstating them (see + // runner.Options.PanicDisarmed). Wins over posture AND EnforcementErr: + // every automatic Apply is being skipped, so nothing below this would + // describe what the firewall is actually doing — Blocked/Posture may + // still say "full-block" or "guard" while egress is wide open. + return Display{ + Key: KeyWarning, + Headline: "Panic disarmed", + Detail: "`dezhban panic` tore down the firewall rules and dezhban is not re-applying them. " + + "Run `dezhban unblock` (or restart the background service) to resume enforcement.", + } + } if s.EnforcementErr != "" { // Wins over posture: the daemon tried to enforce and the backend // refused, so posture/blocked describe the data plane truthfully but // the intended posture was not achieved. That is more urgent than // whatever the intended posture was, so there is no point deriving // its display just to discard it. + // + // A verification repair that failed sets BOTH EnforcementErr and + // Verify.Missing+Err (the run loop publishes the same error through + // both), so without this the more specific sentence below could never + // be reached from a real snapshot — the reader would see the raw + // backend error with no hint that the rules are GONE. Same headline, + // the detail that actually says what happened. + if note := verifyNote(s); note != "" && s.Verify.Missing && s.Verify.Err != "" { + return Display{Key: KeyWarning, Headline: "Enforcement failed", Detail: note} + } return Display{Key: KeyWarning, Headline: "Enforcement failed", Detail: s.EnforcementErr} } d := postureDisplay(s) + vNote := verifyNote(s) d.Detail = joinSentences(d.Detail, lookupNote(s)) + d.Detail = joinSentences(d.Detail, zombieNote(s)) + d.Detail = joinSentences(d.Detail, vNote) d.Detail = joinSentences(d.Detail, pendingNote(s.Pending)) + // Zombie/Verify only ever UPGRADE a healthy-looking Key to KeyWarning, never + // downgrade one that is already worse. Both conditions are diagnosis, not a + // leak — the guard is enforcing correctly either way, or (for Verify) was + // already repaired before this is read — so KeyBlocked (an actual exposure + // risk: FULL BLOCK, or the guard holding a downed tunnel) must stay + // KeyBlocked, and an open window's KeyWarning/KeyPaused is already the + // right tier. Without this, a user glancing at the menubar during "rules + // were found missing and re-applied" or a hung tunnel saw a plain green + // "Guarding" icon — the Detail sentence existed, but nothing drew the eye + // to it. + if d.Key == KeyOn && (s.Zombie != nil || vNote != "") { + d.Key = KeyWarning + } return d } @@ -435,6 +475,49 @@ func lookupNote(s state.Snapshot) string { return fmt.Sprintf("Last exit-country check failed: %s.", s.LookupErr) } +// zombieNote reports a tunnel that reports up but has stopped passing traffic +// — diagnosis, not a leak: the guard is holding exactly as designed, same as +// any other tunnel-down state. Appended alongside lookupNote rather than +// replacing the posture headline, so "Guarding" stays accurate (it is) while +// the detail explains why the checks keep failing. +func zombieNote(s state.Snapshot) string { + if s.Zombie == nil { + return "" + } + return "Your VPN's interface looks up, but exit checks through it keep failing — it may need reconnecting." +} + +// verifyNote reports enforcement verification's last unhappy answer. +// +// Missing means the rules were found gone and have ALREADY been re-applied by +// the time this is read — the guard's current state is correct, this only +// explains why "something removed them" is worth knowing about. Err means the +// backend could not be read at all, which is not evidence the rules are gone +// (the same discipline as an undeterminable exit country holding the current +// posture): nothing was re-applied, and the note says so rather than +// implying a repair that did not happen. +func verifyNote(s state.Snapshot) string { + if s.Verify == nil { + return "" + } + if s.Verify.Missing { + if s.Verify.Err != "" { + // Missing AND Err: the rules are gone and putting them back FAILED, + // so the host is unenforced right now. Saying "have been re-applied" + // here would be the single most dangerous sentence this renderer + // can produce — see state.VerifyState.Err. + return fmt.Sprintf("Your firewall rules were found missing and could NOT be re-applied: %s. "+ + "Your traffic is not being guarded.", s.Verify.Err) + } + return fmt.Sprintf("Your firewall rules were found missing and have been re-applied (%d time(s) since startup).", + s.Verify.Repairs) + } + if s.Verify.Err != "" { + return fmt.Sprintf("Could not verify your firewall rules are still installed: %s.", s.Verify.Err) + } + return "" +} + // pendingNote reports a hysteresis streak in progress, in the one spelling // ("confirming checks") that replaces the CLI's "agreeing readings" and the // macOS app's "confirming checks"/decision.Pending's own doc-comment "good diff --git a/internal/render/render_test.go b/internal/render/render_test.go index 572fe83..95ef099 100644 --- a/internal/render/render_test.go +++ b/internal/render/render_test.go @@ -306,6 +306,123 @@ func TestText(t *testing.T) { wantHeadline: "Guarding", wantDetail: "Traffic leaves only through your VPN tunnel. Last exit-country check failed: malformed response.", }, + { + name: "zombie note appended to guard detail, Key upgraded to warning", + snap: state.Snapshot{ + Posture: PostureGuard, + Tunnels: []state.Tunnel{{Name: "utun4", Up: true}}, + Zombie: &state.ZombieState{Checks: 2}, + }, + wantKey: KeyWarning, + wantHeadline: "Guarding", + wantDetail: "Traffic leaves only through your VPN tunnel. Your VPN's interface looks up, " + + "but exit checks through it keep failing — it may need reconnecting.", + }, + { + name: "verify-missing note appended to guard detail, Key upgraded to warning", + snap: state.Snapshot{ + Posture: PostureGuard, + Tunnels: []state.Tunnel{{Name: "utun4", Up: true}}, + Verify: &state.VerifyState{Missing: true, Repairs: 2}, + }, + wantKey: KeyWarning, + wantHeadline: "Guarding", + wantDetail: "Traffic leaves only through your VPN tunnel. Your firewall rules were found " + + "missing and have been re-applied (2 time(s) since startup).", + }, + { + // Missing WITH Err: the rules were gone and the re-apply FAILED, so + // the host is unenforced. The note must not claim a repair that did + // not happen — that sentence is the one this renderer must never + // produce for an unguarded host. + name: "verify-missing with a failed repair says so, never 'has been re-applied'", + snap: state.Snapshot{ + Posture: PostureGuard, + Tunnels: []state.Tunnel{{Name: "utun4", Up: true}}, + Verify: &state.VerifyState{Missing: true, Err: "pfctl: /dev/pf: Device busy", Repairs: 0}, + }, + wantKey: KeyWarning, + wantHeadline: "Guarding", + wantDetail: "Traffic leaves only through your VPN tunnel. Your firewall rules were found " + + "missing and could NOT be re-applied: pfctl: /dev/pf: Device busy. " + + "Your traffic is not being guarded.", + }, + { + // What the run loop ACTUALLY publishes for a failed repair: it sets + // enfErr and Verify.Missing+Err from the same error, so the generic + // EnforcementErr short-circuit would otherwise swallow the sentence + // above and leave the reader with a bare backend error that never + // says the rules are gone. + name: "failed repair keeps the specific sentence even with EnforcementErr set", + snap: state.Snapshot{ + Posture: PostureGuard, + Tunnels: []state.Tunnel{{Name: "utun4", Up: true}}, + EnforcementErr: "pfctl: /dev/pf: Device busy", + Verify: &state.VerifyState{Missing: true, Err: "pfctl: /dev/pf: Device busy"}, + }, + wantKey: KeyWarning, + wantHeadline: "Enforcement failed", + wantDetail: "Your firewall rules were found missing and could NOT be re-applied: " + + "pfctl: /dev/pf: Device busy. Your traffic is not being guarded.", + }, + { + // An EnforcementErr with no verification finding behind it still + // reports the raw backend error — the specific sentence above must + // not swallow the general case. + name: "enforcement error with a read-only verify finding keeps the raw error", + snap: state.Snapshot{ + Posture: PostureGuard, + Tunnels: []state.Tunnel{{Name: "utun4", Up: true}}, + EnforcementErr: "nft: permission denied", + Verify: &state.VerifyState{Err: "nft: permission denied"}, + }, + wantKey: KeyWarning, + wantHeadline: "Enforcement failed", + wantDetail: "nft: permission denied", + }, + { + name: "verify-read-error note appended to guard detail, Key upgraded to warning", + snap: state.Snapshot{ + Posture: PostureGuard, + Tunnels: []state.Tunnel{{Name: "utun4", Up: true}}, + Verify: &state.VerifyState{Err: "pfctl: no such process"}, + }, + wantKey: KeyWarning, + wantHeadline: "Guarding", + wantDetail: "Traffic leaves only through your VPN tunnel. Could not verify your firewall " + + "rules are still installed: pfctl: no such process.", + }, + { + // A clean verification check (Verify present but neither Missing nor + // Err set) must never happen in practice — the run loop always clears + // Verify to nil on success — but render must not crash or append an + // empty sentence if it somehow did. + name: "verify present but clean is not surfaced and does not upgrade Key", + snap: state.Snapshot{ + Posture: PostureGuard, + Tunnels: []state.Tunnel{{Name: "utun4", Up: true}}, + Verify: &state.VerifyState{}, + }, + wantKey: KeyOn, + wantHeadline: "Guarding", + wantDetail: "Traffic leaves only through your VPN tunnel.", + }, + { + // Zombie/Verify only ever UPGRADE Key — a posture that is already + // KeyBlocked (a real exposure risk: FULL BLOCK) must stay KeyBlocked, + // never get quietly downgraded to the less alarming amber warning. + name: "verify-missing during full block never downgrades Key from blocked", + snap: state.Snapshot{ + Posture: PostureFullBlock, + CountryCode: "IR", + Verify: &state.VerifyState{Missing: true, Repairs: 1}, + }, + wantKey: KeyBlocked, + wantHeadline: "Full block (IR)", + wantDetail: "Your VPN is exiting through a country you've blocked (IR). Everything is cut " + + "until it moves. Your firewall rules were found missing and have been re-applied " + + "(1 time(s) since startup).", + }, { name: "exit-unknown never surfaced", snap: state.Snapshot{ diff --git a/internal/runner/control_test.go b/internal/runner/control_test.go index 91b4394..225373d 100644 --- a/internal/runner/control_test.go +++ b/internal/runner/control_test.go @@ -16,6 +16,7 @@ import ( "github.com/behnam-rk/dezhban/internal/control" "github.com/behnam-rk/dezhban/internal/decision" "github.com/behnam-rk/dezhban/internal/firewall" + "github.com/behnam-rk/dezhban/internal/state" ) // pollUntil polls cond every 5ms until it returns true or timeout elapses, at @@ -373,6 +374,57 @@ func TestControlSocketRemovedOnShutdown(t *testing.T) { } } +// A control-driven unblock into standby (vpn.autoArm, tunnel down) must clear +// any stale enforcement-verification finding left over from the armed state +// that just ended. dg.verify is otherwise only ever touched by the verifyC +// tick, which is (correctly) skipped in standby — so without an explicit +// reset at the transition, a "rules missing" finding from before the drop +// would keep being republished forever, even though nothing is installed in +// standby by design. +func TestVerifyFindingClearedOnStandbyEntry(t *testing.T) { + be := &fakeBackend{isBlockedFn: func() (bool, error) { return false, nil }} // rules always "missing" + o := vpnOpts(be) + // AutoArm is what makes the unblock below land in STANDBY rather than an + // open guard — but on its own it also decides the STARTING posture from a + // live probe of the host's own interfaces (netdetect.TunnelInterfaces), + // which is not something a unit test may depend on: a developer machine + // with a VPN up starts armed, a CI runner with no tunnel interface starts + // in standby, and the transition under test only exists on the first. + // ArmAtBoot + TunnelEverUp is the supported override for exactly that + // startup race (see shouldArmAtBoot), so pin the armed start through it and + // let the host probe say whatever it likes. + o.AutoArm = true + o.ArmAtBoot = true + o.TunnelEverUp = true + o.Watcher = downWatcher() + o.VerifyInterval = 5 * time.Millisecond + var downEdges atomic.Int64 + o.Log = slog.New(countingHandler{substr: "vpn tunnel down — guard holds the line", count: &downEdges}) + var last atomic.Pointer[state.Snapshot] + o.Publish = func(s state.Snapshot) { last.Store(&s) } + path := startControlled(t, o) + + // Wait for the watcher's down edge to actually reach the run loop (tunnelUp + // = false), not just for the watcher to start — the log fires on the same + // goroutine right after the assignment, so seeing it guarantees the + // unblock below observes tunnelUp already false. + pollUntil(t, 2*time.Second, func() bool { return downEdges.Load() >= 1 }, + "tunnel-down edge was never observed by the run loop") + pollUntil(t, 2*time.Second, func() bool { + s := last.Load() + return s != nil && s.Verify != nil && s.Verify.Missing + }, "enforcement verification never reported the rules missing") + + resp := do(t, path, control.Request{Op: control.OpUnblock}) + if !resp.OK || resp.Posture != "standby" { + t.Fatalf("unblock response = %+v, want an OK standby", resp) + } + + if s := last.Load(); s.Verify != nil { + t.Fatalf("a stale verify finding survived the standby transition: %+v", s.Verify) + } +} + func contains(calls []string, want string) bool { for _, c := range calls { if c == want { diff --git a/internal/runner/exitip_test.go b/internal/runner/exitip_test.go new file mode 100644 index 0000000..b3a3f2b --- /dev/null +++ b/internal/runner/exitip_test.go @@ -0,0 +1,141 @@ +package runner + +import ( + "context" + "net/netip" + "testing" + "time" + + "github.com/behnam-rk/dezhban/internal/decision" + "github.com/behnam-rk/dezhban/internal/monitor" + "github.com/behnam-rk/dezhban/internal/state" +) + +// Purely observational, like CVG's equivalent check: a change in the observed +// exit IP is published, but never flips posture and never touches the +// hysteresis streak (CountryCode/Pending already own that job). It exists +// because a failover between two servers in the same allowed country changes +// nothing CountryCode reports. +func TestExitIPChangeIsObservedAndPublished(t *testing.T) { + be := &fakeBackend{} + ip1 := netip.MustParseAddr("203.0.113.10") + ip2 := netip.MustParseAddr("203.0.113.20") + ctx, cancel := context.WithCancel(context.Background()) + mon := &fakeMonitor{cancel: cancel, results: []monitor.Result{ + {Reading: monitor.Reading{IP: ip1, CountryCode: "US"}}, // first reading: nothing to compare against + {Reading: monitor.Reading{IP: ip1, CountryCode: "US"}}, // same IP: no change + {Reading: monitor.Reading{IP: ip2, CountryCode: "US"}}, // different IP: a change + }} + var snaps []state.Snapshot + o := Options{ + Monitor: mon, + Decider: decision.New([]string{"IR"}, 1), + Backend: be, + Log: discardLog(), + Interval: time.Millisecond, + Tunnels: []string{"utun4"}, + Endpoints: []netip.Addr{netip.MustParseAddr("198.51.100.7")}, + Publish: func(s state.Snapshot) { snaps = append(snaps, s) }, + } + if err := Run(ctx, o); err != nil { + t.Fatal(err) + } + if len(snaps) == 0 { + t.Fatal("no snapshots published") + } + if !snaps[0].ExitIPChangedAt.IsZero() { + t.Error("the very first reading was reported as a change; there was nothing yet to compare it against") + } + // Not snaps[len(snaps)-1]: the run's final publish is the terminal "stopped" + // snapshot (publishStopped), a fresh minimal Snapshot that carries none of + // the run loop's diagnostic state — so the change has to be found among the + // snapshots the geo ticks themselves published, not assumed to be the last. + var sawChange bool + for _, s := range snaps { + if !s.ExitIPChangedAt.IsZero() { + sawChange = true + } + } + if !sawChange { + t.Error("ExitIPChangedAt was never set after the exit IP genuinely changed") + } +} + +// A failover between two servers in the same FORBIDDEN country still changes +// the exit IP — and the readings that observe it are exactly the ones FULL +// BLOCK is watching over, so excluding them here would leave "my exit +// flapped" unexplained for the one posture where an operator most wants to +// know. observeExitIP must run on every successful reading, not just an +// ALLOWED one. +func TestExitIPChangeIsObservedWhileBlocked(t *testing.T) { + be := &fakeBackend{} + ip1 := netip.MustParseAddr("203.0.113.10") + ip2 := netip.MustParseAddr("203.0.113.20") + ctx, cancel := context.WithCancel(context.Background()) + mon := &fakeMonitor{cancel: cancel, results: []monitor.Result{ + {Reading: monitor.Reading{IP: ip1, CountryCode: "IR"}}, // blocked country: nothing to compare against yet + {Reading: monitor.Reading{IP: ip1, CountryCode: "IR"}}, // same IP, still blocked: no change + {Reading: monitor.Reading{IP: ip2, CountryCode: "IR"}}, // different IP, still blocked: a change + }} + var snaps []state.Snapshot + o := Options{ + Monitor: mon, + Decider: decision.New([]string{"IR"}, 1), + Backend: be, + Log: discardLog(), + Interval: time.Millisecond, + Tunnels: []string{"utun4"}, + Endpoints: []netip.Addr{netip.MustParseAddr("198.51.100.7")}, + Publish: func(s state.Snapshot) { snaps = append(snaps, s) }, + } + if err := Run(ctx, o); err != nil { + t.Fatal(err) + } + + var sawBlocked, sawChange bool + for _, s := range snaps { + if s.Blocked { + sawBlocked = true + } + if !s.ExitIPChangedAt.IsZero() { + sawChange = true + } + } + if !sawBlocked { + t.Fatal("posture never escalated to FULL BLOCK; this fixture tests nothing") + } + if !sawChange { + t.Error("ExitIPChangedAt was never set for an exit-IP change observed while blocked") + } +} + +// A steady exit IP across every reading must never be reported as a change. +func TestSteadyExitIPNeverReportsAChange(t *testing.T) { + be := &fakeBackend{} + ip := netip.MustParseAddr("203.0.113.10") + ctx, cancel := context.WithCancel(context.Background()) + mon := &fakeMonitor{cancel: cancel, results: []monitor.Result{ + {Reading: monitor.Reading{IP: ip, CountryCode: "US"}}, + {Reading: monitor.Reading{IP: ip, CountryCode: "US"}}, + {Reading: monitor.Reading{IP: ip, CountryCode: "US"}}, + }} + var snaps []state.Snapshot + o := Options{ + Monitor: mon, + Decider: decision.New([]string{"IR"}, 1), + Backend: be, + Log: discardLog(), + Interval: time.Millisecond, + Tunnels: []string{"utun4"}, + Endpoints: []netip.Addr{netip.MustParseAddr("198.51.100.7")}, + Publish: func(s state.Snapshot) { snaps = append(snaps, s) }, + } + if err := Run(ctx, o); err != nil { + t.Fatal(err) + } + for _, s := range snaps { + if !s.ExitIPChangedAt.IsZero() { + t.Fatalf("a steady exit IP was reported as changed: %+v", s) + } + } +} diff --git a/internal/runner/liveness_test.go b/internal/runner/liveness_test.go new file mode 100644 index 0000000..67dead4 --- /dev/null +++ b/internal/runner/liveness_test.go @@ -0,0 +1,470 @@ +package runner + +import ( + "context" + "errors" + "net/netip" + "sync" + "testing" + "time" + + "github.com/behnam-rk/dezhban/internal/command" + "github.com/behnam-rk/dezhban/internal/decision" + "github.com/behnam-rk/dezhban/internal/monitor" + "github.com/behnam-rk/dezhban/internal/state" +) + +// scriptedZombieMonitor fails every lookup except the call at successAt +// (0-indexed), which succeeds — just enough to genuinely resolve a zombie +// streak (the same way a real recovered exit would) so a second, distinct +// streak can start immediately after, all without the tunnel interface +// itself ever reporting down. +type scriptedZombieMonitor struct { + mu sync.Mutex + calls int + successAt int +} + +func (m *scriptedZombieMonitor) Poll(ctx context.Context) <-chan monitor.Result { + ch := make(chan monitor.Result) + go func() { <-ctx.Done(); close(ch) }() + return ch +} + +func (m *scriptedZombieMonitor) Once(context.Context) (monitor.Reading, error) { + m.mu.Lock() + n := m.calls + m.calls++ + m.mu.Unlock() + if n == m.successAt { + return monitor.Reading{CountryCode: "US"}, nil + } + return monitor.Reading{}, errors.New("lookup failed") +} + +// dezhban's posture never escalates on a lookup failure alone — an unknown +// exit country HOLDS the current posture rather than flipping it (see +// decision.Evaluate). So a tunnel that reports up but has stopped passing +// traffic stayed correctly cut, forever, with no signal to anyone. These tests +// pin the diagnosis (always on) separately from the relaxation it MAY trigger +// (opt-in, off by default) — the two halves of docs/adr/0010-tunnel-liveness.md. + +// A run of failed exit checks through an up tunnel must be reported once it +// reaches the Decider's own hysteresis count, and — with the default config — +// must never open a redial window on its own. Detecting is not the same as +// acting. +func TestZombieStreakReportedButRedialStaysOffByDefault(t *testing.T) { + be := &fakeBackend{} + var snaps []state.Snapshot + ctx, cancel := context.WithTimeout(context.Background(), 150*time.Millisecond) + defer cancel() + o := Options{ + Monitor: steadyFailMonitor{}, // every exit check fails, like a censoring exit or a hung tunnel + Decider: decision.New([]string{"IR"}, 2), + Backend: be, + Log: discardLog(), + Interval: 15 * time.Millisecond, + Tunnels: []string{"utun4"}, + Endpoints: []netip.Addr{netip.MustParseAddr("203.0.113.7")}, + Watcher: edgeWatcher(100000), // interface reports up for the whole run + Publish: func(s state.Snapshot) { snaps = append(snaps, s) }, + // LivenessRedial left at its zero value: off. + } + if err := Run(ctx, o); err != nil { + t.Fatal(err) + } + + var sawZombie bool + for _, s := range snaps { + if s.Zombie != nil && s.Zombie.Checks >= 2 { + sawZombie = true + } + } + if !sawZombie { + t.Fatal("no published snapshot reported the zombie streak reaching the hysteresis count") + } + + for _, c := range be.calls { + if c == "apply-switch" { + t.Fatalf("a redial window opened with livenessRedial off; calls = %v", be.calls) + } + } +} + +// The same streak, with vpn.advanced.livenessRedial on, must open an automatic +// redial window through the EXISTING trigger-2 machinery — this is that +// trigger widening what counts as "down", not a fourth trigger, so it has to +// land on the same apply-switch path an ordinary tunnel drop uses. +// +// Exactly ONE window, never more: the run's 400ms comfortably outlasts a +// 30ms window plus the ~30ms (Hysteresis=2 × 15ms interval) it takes the +// streak to re-cross the threshold once the window closes, so a version that +// reopens on every expiry (the bug resetZombie's full/partial split fixed — +// zombieRedialTried was being cleared just because a window was open, not +// because the hang had actually resolved) would open several here, not one. +func TestZombieStreakOpensRedialWindowWhenEnabled(t *testing.T) { + be := &fakeBackend{} + ctx, cancel := context.WithTimeout(context.Background(), 400*time.Millisecond) + defer cancel() + o := Options{ + Monitor: steadyFailMonitor{}, + Decider: decision.New([]string{"IR"}, 2), + Backend: be, + Log: discardLog(), + Interval: 15 * time.Millisecond, + Tunnels: []string{"utun4"}, + Endpoints: []netip.Addr{netip.MustParseAddr("203.0.113.7")}, + Watcher: edgeWatcher(100000), + LivenessRedial: true, + RedialWindow: 30 * time.Millisecond, + RedialBudget: testRedialBudget, + RedialBudgetWindow: testRedialBudgetWindow, + } + if err := Run(ctx, o); err != nil { + t.Fatal(err) + } + + var switches int + for _, c := range be.calls { + if c == "apply-switch" { + switches++ + } + } + if switches != 1 { + t.Fatalf("apply-switch called %d time(s) for one continuous, never-resolving zombie streak; "+ + "want exactly 1 — a streak's window expiring must never reopen a new one on its own. calls = %v", + switches, be.calls) + } +} + +// A liveness-redial attempt refused by the budget must still be retried once +// it refills — WITHOUT the tunnel ever reporting down. Unlike an ordinary +// drop, a zombie streak's tunnel stays up for the whole episode, so +// retryAutoWindow's guard must recognise a standing zombie streak as "the +// drop is still open", not just tunnelUp == false, or a refused +// liveness-redial attempt would never get a second chance. +// +// A single continuous streak, though, gets at most ONE automatic attempt — +// its window expiring must never reopen a new one on its own (see +// resetZombie's full/partial split in runner.go, which fixed exactly that: +// an earlier version reset zombieRedialTried whenever a window was open, +// letting a still-hung tunnel reopen a window every expiry). So the refusal +// this test needs has to come from a SECOND, genuinely distinct streak +// spending a budget the FIRST streak's own grant already mostly used up — +// not from the same streak reattempting after its window closes. +func TestAZombieRefusedRedialRetriesWithoutTheTunnelGoingDown(t *testing.T) { + be := &fakeBackend{} + ctx, cancel := context.WithTimeout(context.Background(), 500*time.Millisecond) + defer cancel() + + var ( + mu sync.Mutex + snaps []state.Snapshot + ) + o := Options{ + // Fails every lookup except call index 3, which succeeds — just + // enough to genuinely resolve the FIRST zombie streak right after its + // window closes, so a SECOND streak starts immediately and is the one + // that gets refused. Index 3, not 2: index 0 is runGuard's own + // pre-loop startup observation (len(tunnels)>0 && len(endpoints)>0), + // which never touches zombieChecks; indices 1-2 are the two real + // geoTick failures that cross the Hysteresis(2) threshold and open + // the first window. No confirmed exit ever closes a window EARLY + // (both streaks' windows suppress lookups entirely while open), so + // each granted window costs its full duration. + Monitor: &scriptedZombieMonitor{successAt: 3}, + Decider: decision.New([]string{"IR"}, 2), + Backend: be, + Log: discardLog(), + Interval: time.Millisecond, + Tunnels: []string{"utun4"}, + Endpoints: []netip.Addr{netip.MustParseAddr("203.0.113.7")}, + Watcher: edgeWatcher(100000), // interface reports up for the WHOLE run — no down edge, ever + LivenessRedial: true, + RedialWindow: 20 * time.Millisecond, + // Room for one full window and no more, refilling 120ms after the + // first streak's window cost is recorded — same shape as + // TestARefusedRedialRetriesWhenTheBudgetRefills, but two zombie + // streaks stand in for the two ordinary drops that fixture uses. + RedialBudget: 25 * time.Millisecond, + RedialBudgetWindow: 120 * time.Millisecond, + Publish: func(s state.Snapshot) { + mu.Lock() + defer mu.Unlock() + snaps = append(snaps, s) + }, + } + if err := Run(ctx, o); err != nil { + t.Fatal(err) + } + mu.Lock() + defer mu.Unlock() + + // A refusal must have been published, or the test proved nothing: the + // second streak's attempt has to have been refused because the first + // streak's window already spent the budget. + refusedAt := -1 + for i, s := range snaps { + if s.Redial != nil { + refusedAt = i + break + } + } + if refusedAt < 0 { + t.Fatal("no redial refusal was ever published; the budget never ran out and this fixture tests nothing") + } + + reopened := false + for _, s := range snaps[refusedAt:] { + if s.Switch == nil || !s.Switch.Open { + continue + } + if s.Switch.Trigger != state.TriggerAuto { + t.Errorf("window after the refusal has trigger %q, want %q — the retry must stay trigger 2", + s.Switch.Trigger, state.TriggerAuto) + } + reopened = true + if s.Redial != nil { + t.Errorf("a window is open but state.redial still reports %q — "+ + "exactly one of the two may be present", s.Redial.Reason) + } + break + } + if !reopened { + t.Error("the refused zombie-redial attempt never got a window once the budget refilled — " + + "retryAutoWindow's tunnelUp guard is refusing a retry the streak has earned") + } + + // The whole point: no tunnel-down edge ever happened. Confirms the retry + // above was earned by the zombie streak, not by an ordinary drop/recovery + // this fixture never produced. + for _, s := range snaps { + if s.Drop != nil { + t.Fatalf("a tunnel drop was recorded; this fixture's tunnel must never go down: %+v", *s.Drop) + } + } + + // Exactly two automatic windows total — one grant per streak, never a + // third from either streak reopening on its own once its window expires. + var switches int + for _, c := range be.calls { + if c == "apply-switch" { + switches++ + } + } + if switches != 2 { + t.Errorf("apply-switch called %d time(s), want exactly 2 (one grant per streak); calls = %v", switches, be.calls) + } +} + +// Disabling vpn.advanced.livenessRedial live, after a zombie streak's +// automatic attempt has already been refused by the budget, must cancel that +// standing refusal's retry — not just future streaks. retryAutoWindow's +// tunnelUp/dg.zombie guard alone would still let the retry fire once the +// budget refills, silently bypassing the operator's just-disabled opt-in. +// Same fixture as TestAZombieRefusedRedialRetriesWithoutTheTunnelGoingDown, +// but the refusal triggers a reload turning livenessRedial off instead of +// letting it stand. +func TestDisablingLivenessRedialDropsAStandingZombieRefusal(t *testing.T) { + be := &fakeBackend{} + ctx, cancel := context.WithTimeout(context.Background(), 500*time.Millisecond) + defer cancel() + + reloadC := make(chan LiveSettings, 1) + var ( + mu sync.Mutex + snaps []state.Snapshot + disabled bool + ) + o := Options{ + Monitor: &scriptedZombieMonitor{successAt: 3}, + Decider: decision.New([]string{"IR"}, 2), + Backend: be, + Log: discardLog(), + Interval: time.Millisecond, + Tunnels: []string{"utun4"}, + Endpoints: []netip.Addr{netip.MustParseAddr("203.0.113.7")}, + Watcher: edgeWatcher(100000), // interface reports up for the WHOLE run — no down edge, ever + LivenessRedial: true, + RedialWindow: 20 * time.Millisecond, + RedialBudget: 25 * time.Millisecond, + RedialBudgetWindow: 120 * time.Millisecond, + ReloadC: reloadC, + } + o.Publish = func(s state.Snapshot) { + mu.Lock() + defer mu.Unlock() + snaps = append(snaps, s) + // The moment the second streak's attempt is refused, turn + // livenessRedial off — event-driven so the test cannot race the + // refusal or the budget refilling. + if s.Redial != nil && !disabled { + disabled = true + ls := o.Live() + ls.LivenessRedial = false + select { + case reloadC <- ls: + default: + } + } + } + if err := Run(ctx, o); err != nil { + t.Fatal(err) + } + mu.Lock() + defer mu.Unlock() + + if !disabled { + t.Fatal("no redial refusal was ever published; the budget never ran out and this fixture tests nothing") + } + + refusedAt := -1 + for i, s := range snaps { + if s.Redial != nil { + refusedAt = i + break + } + } + for _, s := range snaps[refusedAt+1:] { + if s.Switch != nil && s.Switch.Open && s.Switch.Trigger == state.TriggerAuto { + t.Fatalf("an automatic window opened after livenessRedial was disabled following the refusal: %+v", *s.Switch) + } + } + + // Exactly ONE automatic window total — the first streak's grant — never + // a second from the disabled retry firing once the budget refilled. + var switches int + for _, c := range be.calls { + if c == "apply-switch" { + switches++ + } + } + if switches != 1 { + t.Errorf("apply-switch called %d time(s), want exactly 1 (the first streak's grant only); calls = %v", switches, be.calls) + } +} + +// A tunnel that plainly reports down must never be reported as a zombie — that +// is a different, already-explained state (the guard holding a downed tunnel), +// and conflating the two would blur two distinct diagnoses into one. +func TestPlainlyDownTunnelIsNeverReportedAsZombie(t *testing.T) { + be := &fakeBackend{} + var snaps []state.Snapshot + ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) + defer cancel() + o := Options{ + Monitor: steadyFailMonitor{}, + Decider: decision.New([]string{"IR"}, 1), + Backend: be, + Log: discardLog(), + Interval: 15 * time.Millisecond, + Tunnels: []string{"utun4"}, + Endpoints: []netip.Addr{netip.MustParseAddr("203.0.113.7")}, + Watcher: downWatcher(), // interface reports down for the whole run + Publish: func(s state.Snapshot) { snaps = append(snaps, s) }, + } + if err := Run(ctx, o); err != nil { + t.Fatal(err) + } + for _, s := range snaps { + if s.Zombie != nil { + t.Fatalf("a plainly-down tunnel was reported as a zombie: %+v", *s.Zombie) + } + } +} + +// If `dezhban hold` is armed before a zombie streak's one-shot +// liveness-redial attempt fires, maybeAutoWindow's holdArmed branch +// (consumeHold:false for this caller) suppresses it without ever reaching +// grantAutoWindow — so unlike an ordinary drop's refusal, there is no +// standing redialRefused for retryAutoWindow to act on once hold is +// cancelled. Without zombieHoldSuppressed restoring the attempt directly, +// the streak's one shot (already spent via zombieRedialTried) was forfeited +// for good: nothing else would ever re-arm it while the same streak stands. +// This pins that cancelling hold gives the streak's attempt back — a window +// must open afterward, all without the tunnel ever reporting down. +func TestHoldCancelRestoresASuppressedZombieRedialAttempt(t *testing.T) { + be := &fakeBackend{} + ctx, cancel := context.WithTimeout(context.Background(), 400*time.Millisecond) + defer cancel() + + var ( + mu sync.Mutex + snaps []state.Snapshot + armed bool + canceled bool + start = time.Now() + ) + o := Options{ + Monitor: steadyFailMonitor{}, // never resolves — one continuous streak + Decider: decision.New([]string{"IR"}, 2), + Backend: be, + Log: discardLog(), + Interval: 15 * time.Millisecond, + Tunnels: []string{"utun4"}, + Endpoints: []netip.Addr{netip.MustParseAddr("203.0.113.7")}, + Watcher: edgeWatcher(100000), // interface reports up for the whole run + LivenessRedial: true, + RedialWindow: 30 * time.Millisecond, + RedialBudget: testRedialBudget, + RedialBudgetWindow: testRedialBudgetWindow, + CommandPoll: time.Millisecond, + Publish: func(s state.Snapshot) { + mu.Lock() + defer mu.Unlock() + snaps = append(snaps, s) + }, + } + o.PollCommand = func() (command.Command, bool) { + mu.Lock() + defer mu.Unlock() + // Armed on the very first poll (~1ms in), well before the streak can + // cross the Hysteresis(2) threshold (~30ms in, at Interval=15ms), so + // the streak's one attempt is suppressed by hold rather than actually + // run. + if !armed { + armed = true + return command.Command{Op: command.OpHoldArm, IssuedAt: time.Now(), Nonce: "arm"}, true + } + if armed && !canceled && time.Since(start) > 150*time.Millisecond { + canceled = true + return command.Command{Op: command.OpHoldCancel, IssuedAt: time.Now(), Nonce: "cancel"}, true + } + return command.Command{}, false + } + if err := Run(ctx, o); err != nil { + t.Fatal(err) + } + mu.Lock() + defer mu.Unlock() + if !armed || !canceled { + t.Fatalf("fixture never reached the state under test (armed=%v canceled=%v)", armed, canceled) + } + + cancelIdx := -1 + for i := 1; i < len(snaps); i++ { + prev, cur := snaps[i-1], snaps[i] + if prev.Hold != nil && prev.Hold.Armed && (cur.Hold == nil || !cur.Hold.Armed) { + cancelIdx = i + } + } + if cancelIdx < 0 { + t.Fatal("never observed hold going from armed to cancelled; fixture proved nothing") + } + + for _, s := range snaps[cancelIdx:] { + if s.Switch == nil || !s.Switch.Open || s.Switch.Trigger != state.TriggerAuto { + continue + } + // Confirm no tunnel-down edge ever happened — the window must be + // attributable to the restored zombie attempt, not an ordinary drop. + for _, d := range snaps { + if d.Drop != nil { + t.Fatalf("a tunnel drop was recorded; this fixture's tunnel must never go down: %+v", *d.Drop) + } + } + return + } + t.Error("after hold the line was cancelled, the suppressed zombie liveness-redial attempt never got " + + "a window: it was silently forfeited instead of being restored") +} diff --git a/internal/runner/panic_test.go b/internal/runner/panic_test.go new file mode 100644 index 0000000..9da8bcb --- /dev/null +++ b/internal/runner/panic_test.go @@ -0,0 +1,273 @@ +package runner + +import ( + "context" + "net/netip" + "sync/atomic" + "testing" + "time" + + "github.com/behnam-rk/dezhban/internal/control" + "github.com/behnam-rk/dezhban/internal/decision" + "github.com/behnam-rk/dezhban/internal/state" +) + +// `dezhban panic` tears down every rule directly, as root, independent of the +// daemon. These tests pin the two rules that keep the daemon from silently +// undoing that: every AUTOMATIC re-apply path must stand down while +// PanicDisarmed reports true, and every EXPLICIT operator command must clear +// the marker (never be blocked by it) — exactly like OpUnblock already did +// before this change. + +// fakePanicMarker is a minimal PanicDisarmed/ClearPanicDisarm pair for tests: +// an atomic flag plus a call counter for the clear, so assertions can check +// both "did it stand down" and "did it clear" without a real state-dir file. +type fakePanicMarker struct { + disarmed atomic.Bool + clears atomic.Int64 +} + +func (m *fakePanicMarker) Disarmed() bool { return m.disarmed.Load() } +func (m *fakePanicMarker) Clear() error { + m.clears.Add(1) + m.disarmed.Store(false) + return nil +} + +// A tunnel drop from healthy GUARD must NOT open the automatic redial window +// while panic-disarmed — the marker must fully suppress trigger 2, both the +// drop edge (maybeAutoWindow) and the bound-lifted retry (retryAutoWindow), +// via autoWindowPossible. +func TestVPNAutoRedialWindowSuppressedWhilePanicDisarmed(t *testing.T) { + be := &fakeBackend{} + m := &fakePanicMarker{} + m.disarmed.Store(true) + ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond) + defer cancel() + o := Options{ + Monitor: steadyMonitor{cc: "US"}, + Decider: decision.New([]string{"IR"}, 1), + Backend: be, + Log: discardLog(), + Interval: time.Millisecond, + Tunnels: []string{"utun4"}, + Endpoints: []netip.Addr{netip.MustParseAddr("203.0.113.7")}, + Watcher: edgeWatcher(5), // clean up->down edge ~5ms in + RedialWindow: 50 * time.Millisecond, + RedialBudget: testRedialBudget, + RedialBudgetWindow: testRedialBudgetWindow, + PanicDisarmed: m.Disarmed, + ClearPanicDisarm: m.Clear, + } + if err := Run(ctx, o); err != nil { + t.Fatal(err) + } + if containsCall(be.calls, "apply-switch") { + t.Fatalf("automatic redial window opened while panic-disarmed; calls=%v", be.calls) + } + if m.clears.Load() != 0 { + t.Errorf("ClearPanicDisarm called %d times; an automatic trigger must never clear the marker itself", m.clears.Load()) + } +} + +// A blocked-country reading must not drive the geo state machine's Apply +// calls while panic-disarmed — vpnGeoStep (and its lift-and-probe fallback) +// must never run at all. +func TestVPNGeoStateMachineSuppressedWhilePanicDisarmed(t *testing.T) { + be := &fakeBackend{} + m := &fakePanicMarker{} + m.disarmed.Store(true) + ctx, cancel := context.WithTimeout(context.Background(), 60*time.Millisecond) + defer cancel() + o := Options{ + Monitor: steadyMonitor{cc: "IR"}, // blocked country every reading + Decider: decision.New([]string{"IR"}, 1), + Backend: be, + Log: discardLog(), + Interval: time.Millisecond, + Tunnels: []string{"utun4"}, + Endpoints: []netip.Addr{netip.MustParseAddr("203.0.113.7")}, + PanicDisarmed: m.Disarmed, + ClearPanicDisarm: m.Clear, + } + if err := Run(ctx, o); err != nil { + t.Fatal(err) + } + if containsCall(be.calls, "apply-fullblock") { + t.Fatalf("geo state machine applied FULL BLOCK while panic-disarmed; calls=%v", be.calls) + } +} + +// A tunnel-set change (autodetect growth) must not re-apply the standing +// guard while panic-disarmed — reapplyStanding must stand down, same as +// enforcement verification already does. +func TestVPNTunnelChangeReapplySuppressedWhilePanicDisarmed(t *testing.T) { + be := &fakeBackend{} + m := &fakePanicMarker{} + m.disarmed.Store(true) + ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond) + defer cancel() + o := Options{ + Monitor: steadyMonitor{cc: "US"}, + Decider: decision.New([]string{"IR"}, 1), + Backend: be, + Log: discardLog(), + Interval: time.Hour, // suppress geoTick; isolate the tunnel-change path + AutoDetect: true, + Tunnels: []string{"utun4"}, + Endpoints: []netip.Addr{netip.MustParseAddr("203.0.113.7")}, + Watcher: growWatcher(), // {utun4} -> {utun4,utun6} + PanicDisarmed: m.Disarmed, + ClearPanicDisarm: m.Clear, + } + if err := Run(ctx, o); err != nil { + t.Fatal(err) + } + for _, p := range be.policies { + for _, ifc := range p.TunnelIfaces { + if ifc == "utun6" { + t.Fatalf("standing guard re-applied with the grown tunnel set while panic-disarmed; policies=%v", be.policies) + } + } + } +} + +// An explicit `block` (control socket) is an operator engaging with +// enforcement, so it must clear a standing panic-disarm marker unconditionally +// and never be refused because of it. +func TestControlOpBlockClearsPanicMarker(t *testing.T) { + be := &fakeBackend{} + m := &fakePanicMarker{} + m.disarmed.Store(true) + o := vpnOpts(be) + o.PanicDisarmed = m.Disarmed + o.ClearPanicDisarm = m.Clear + path := startControlled(t, o) + + resp := do(t, path, control.Request{Op: control.OpBlock}) + if !resp.OK { + t.Fatalf("block over the control socket failed while panic-disarmed: %+v", resp) + } + if m.clears.Load() == 0 { + t.Error("ClearPanicDisarm was never called by an explicit block") + } + if !containsCall(be.calls, "apply-fullblock") { + t.Errorf("expected apply-fullblock; calls=%v", be.calls) + } +} + +// An explicit `switch` open and an explicit `pause` (control socket) are both +// operator commands — each must clear a standing panic-disarm marker via +// openWindow's shared choke point, and must never be refused because of it. +func TestControlSwitchAndPauseClearPanicMarker(t *testing.T) { + for _, tc := range []struct { + name string + op control.Op + }{ + {"switch", control.OpOpenSwitch}, + {"pause", control.OpPause}, + } { + t.Run(tc.name, func(t *testing.T) { + be := &fakeBackend{} + m := &fakePanicMarker{} + m.disarmed.Store(true) + o := vpnOpts(be) + o.PanicDisarmed = m.Disarmed + o.ClearPanicDisarm = m.Clear + path := startControlled(t, o) + + resp := do(t, path, control.Request{Op: tc.op}) + if !resp.OK { + t.Fatalf("%s over the control socket failed while panic-disarmed: %+v", tc.name, resp) + } + if m.clears.Load() == 0 { + t.Errorf("ClearPanicDisarm was never called by an explicit %s", tc.name) + } + if !containsCall(be.calls, "apply-switch") { + t.Errorf("expected apply-switch; calls=%v", be.calls) + } + }) + } +} + +// An automatic redial window's own expiry timer firing while panic-disarmed +// must NOT reinstate rules — but the window's bookkeeping still closes (its +// clock is genuinely up), so a later snapshot must not keep reporting it open. +func TestVPNWindowExpiresWithoutReapplyWhilePanicDisarmed(t *testing.T) { + be := &fakeBackend{} + start := time.Now() + const disarmAfter = 15 * time.Millisecond // after the window opens (~5ms), before it expires (~25ms) + var lastSnap state.Snapshot + ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond) + defer cancel() + o := Options{ + Monitor: steadyMonitor{cc: "US"}, + Decider: decision.New([]string{"IR"}, 1), + Backend: be, + Log: discardLog(), + Interval: time.Millisecond, + Tunnels: []string{"utun4"}, + Endpoints: []netip.Addr{netip.MustParseAddr("203.0.113.7")}, + Watcher: edgeWatcher(5), + RedialWindow: 20 * time.Millisecond, + RedialBudget: testRedialBudget, + RedialBudgetWindow: testRedialBudgetWindow, + PanicDisarmed: func() bool { return time.Since(start) > disarmAfter }, + Publish: func(s state.Snapshot) { lastSnap = s }, + } + if err := Run(ctx, o); err != nil { + t.Fatal(err) + } + if !containsCall(be.calls, "apply-switch") { + t.Fatalf("expected the window to open before the marker was set; calls=%v", be.calls) + } + seenSwitch := false + for _, c := range be.calls { + if c == "apply-switch" { + seenSwitch = true + continue + } + if seenSwitch && c == "apply-guard" { + t.Fatalf("window revert re-applied rules after expiry while panic-disarmed; calls=%v", be.calls) + } + } + if lastSnap.Switch != nil && lastSnap.Switch.Open { + t.Error("final snapshot still reports the switch window open past its expired deadline") + } +} + +// An explicit cancel (control socket) of an open window must clear a standing +// panic-disarm marker and still perform the revert — an operator interacting +// with the window is never blocked by the marker, even mid-window. +func TestControlOpCancelSwitchClearsMarkerAndReverts(t *testing.T) { + be := &fakeBackend{} + m := &fakePanicMarker{} + o := vpnOpts(be) + o.PanicDisarmed = m.Disarmed + o.ClearPanicDisarm = m.Clear + // Long enough that the test's own cancel — not the window's own deadline — + // is what closes it. + o.SwitchWindow = time.Minute + path := startControlled(t, o) + + resp := do(t, path, control.Request{Op: control.OpOpenSwitch}) + if !resp.OK { + t.Fatalf("open switch failed: %+v", resp) + } + // Simulate an operator running `dezhban panic` mid-window: the daemon's own + // windowActive bookkeeping does not know the rules were just torn down out + // from under it. + m.disarmed.Store(true) + m.clears.Store(0) + + resp = do(t, path, control.Request{Op: control.OpCancelSwitch}) + if !resp.OK { + t.Fatalf("cancel switch failed while panic-disarmed: %+v", resp) + } + if m.clears.Load() == 0 { + t.Error("ClearPanicDisarm was never called by an explicit cancel") + } + if !applyGuardAfterSwitch(be.calls) { + t.Errorf("expected a guard-apply revert after the explicit cancel; calls=%v", be.calls) + } +} diff --git a/internal/runner/recovery_test.go b/internal/runner/recovery_test.go index 0698255..3522e21 100644 --- a/internal/runner/recovery_test.go +++ b/internal/runner/recovery_test.go @@ -31,7 +31,7 @@ func TestSnapshotCarriesTheHysteresisStreak(t *testing.T) { Interval: time.Minute, Publish: func(s state.Snapshot) { got = s }, } - o.publish(false, false, monitor.Reading{CountryCode: "IR"}, nil, nil, nil, nil, nil, "", nil, nil, nil) + o.publish(false, false, monitor.Reading{CountryCode: "IR"}, nil, nil, nil, nil, nil, "", nil, nil, nil, diag{}) if got.Pending == nil { t.Fatal("no pending flip published while a hysteresis streak was running") @@ -49,7 +49,7 @@ func TestPublishingProgressDoesNotDisturbTheStreak(t *testing.T) { o := Options{Decider: d, Interval: time.Minute, Publish: func(state.Snapshot) {}} for range 5 { - o.publish(false, false, monitor.Reading{}, nil, nil, nil, nil, nil, "", nil, nil, nil) + o.publish(false, false, monitor.Reading{}, nil, nil, nil, nil, nil, "", nil, nil, nil, diag{}) } _, have, _ := d.Pending() if have != 1 { diff --git a/internal/runner/reload.go b/internal/runner/reload.go index 2cd1008..4e4d263 100644 --- a/internal/runner/reload.go +++ b/internal/runner/reload.go @@ -56,6 +56,8 @@ type LiveSettings struct { EndpointRefresh time.Duration EndpointGrace time.Duration + VerifyInterval time.Duration + LivenessRedial bool AllowSwitchOps bool AllowPauseOps bool @@ -97,6 +99,8 @@ func (o Options) Live() LiveSettings { WindowDiscoveryInterval: o.WindowDiscoveryInterval, EndpointRefresh: o.EndpointRefresh, EndpointGrace: o.EndpointGrace, + VerifyInterval: o.VerifyInterval, + LivenessRedial: o.LivenessRedial, AllowSwitchOps: o.AllowSwitchOps, AllowPauseOps: o.AllowPauseOps, AllowConfigOps: o.AllowConfigOps, diff --git a/internal/runner/reload_test.go b/internal/runner/reload_test.go index bc7bde3..a9a3d4d 100644 --- a/internal/runner/reload_test.go +++ b/internal/runner/reload_test.go @@ -2,9 +2,11 @@ package runner import ( "context" + "log/slog" "net/netip" "reflect" "slices" + "sync" "sync/atomic" "testing" "time" @@ -14,6 +16,7 @@ import ( "github.com/behnam-rk/dezhban/internal/firewall" "github.com/behnam-rk/dezhban/internal/monitor" "github.com/behnam-rk/dezhban/internal/netdetect" + "github.com/behnam-rk/dezhban/internal/state" ) func hasCall(calls []string, want string) bool { @@ -161,6 +164,8 @@ func TestLiveCapturesEveryLiveSetting(t *testing.T) { WindowDiscoveryInterval: time.Second, EndpointRefresh: time.Minute, EndpointGrace: 15 * time.Minute, + VerifyInterval: time.Minute, + LivenessRedial: true, AllowSwitchOps: true, AllowPauseOps: true, AllowConfigOps: true, @@ -509,3 +514,280 @@ func TestReloadedRedialBudgetDecidesTheNextDrop(t *testing.T) { } }) } + +// vpn.advanced.verifyInterval is declared live-appliable, which is the same +// promise as the redial-budget tests above: the run loop's verifyTick is +// created/stopped/reset by applyLive, not merely a field getting copied. +// Booting with verification OFF and never calling IsBlocked proves the +// ticker did not already exist; a reload that turns it on has to actually +// start calling IsBlocked, and a rules-missing finding it discovers has to +// reach a repair — the same two-part promise TestReloadedRedialBudgetDecidesTheNextDrop +// pins for the redial ledger. +func TestReloadedVerifyIntervalStartsCheckingLive(t *testing.T) { + var calls atomic.Int32 + be := &fakeBackend{isBlockedFn: func() (bool, error) { + calls.Add(1) + return false, nil // missing, every time — a repair should follow + }} + ctx, cancel := context.WithTimeout(context.Background(), 250*time.Millisecond) + defer cancel() + + reloadC := make(chan LiveSettings, 1) + reloadC <- LiveSettings{ + Interval: time.Hour, + VerifyInterval: 10 * time.Millisecond, // the change under test: off → on + } + + o := Options{ + Monitor: steadyMonitor{cc: "US"}, + Decider: decision.New([]string{"IR"}, 1), + Backend: be, + Log: discardLog(), + Interval: time.Hour, // the reload and the verify ticker are the only events + Tunnels: []string{"utun4"}, + Endpoints: []netip.Addr{netip.MustParseAddr("203.0.113.7")}, + VerifyInterval: -1, // the config.Disabled sentinel: no ticker exists at boot + ReloadC: reloadC, + } + if err := Run(ctx, o); err != nil { + t.Fatal(err) + } + + if calls.Load() == 0 { + t.Fatal("IsBlocked was never called; reloading vpn.advanced.verifyInterval on did not start the ticker") + } + + guards := 0 + for _, c := range be.calls { + if c == "apply-guard" { + guards++ + } + } + if guards < 2 { + t.Errorf("apply-guard count = %d, want at least 2 (startup + a repair from the newly-live "+ + "verify tick); calls = %v", guards, be.calls) + } +} + +// The other half of the same promise: disabling verification live must stop +// the ticker AND clear whatever finding it last published — resetVerify()'s +// whole reason to exist (see its doc comment in runner.go). Without that +// call, a "rules missing" finding from before the reload would keep being +// republished forever off a tick that no longer runs, misreporting an +// enforcement problem while the daemon is correctly idle. +func TestReloadedVerifyIntervalDisableClearsStaleFinding(t *testing.T) { + be := &fakeBackend{isBlockedFn: func() (bool, error) { return false, nil }} // missing, forever + ctx, cancel := context.WithTimeout(context.Background(), 300*time.Millisecond) + defer cancel() + + reloadC := make(chan LiveSettings, 1) + + var ( + mu sync.Mutex + snaps []state.Snapshot + reloaded bool + ) + o := Options{ + Monitor: steadyMonitor{cc: "US"}, + Decider: decision.New([]string{"IR"}, 1), + Backend: be, + Log: discardLog(), + Interval: time.Hour, + Tunnels: []string{"utun4"}, + Endpoints: []netip.Addr{netip.MustParseAddr("203.0.113.7")}, + VerifyInterval: 10 * time.Millisecond, // on at boot, so a finding can accumulate first + ReloadC: reloadC, + } + o.Publish = func(s state.Snapshot) { + mu.Lock() + defer mu.Unlock() + snaps = append(snaps, s) + // The moment a Missing finding is actually published, turn + // verification off — event-driven so the test needs no sleep and + // cannot race the first verify tick. + if s.Verify != nil && s.Verify.Missing && !reloaded { + reloaded = true + ls := o.Live() + ls.VerifyInterval = -1 // the config.Disabled sentinel + select { + case reloadC <- ls: + default: + } + } + } + if err := Run(ctx, o); err != nil { + t.Fatal(err) + } + mu.Lock() + defer mu.Unlock() + + if !reloaded { + t.Fatal("no Missing finding was ever published; this fixture tests nothing") + } + + // The LAST live snapshot (shutdown publishes a terminal posture:"stopped" + // record that carries no Verify either way, which would pass regardless of + // whether resetVerify actually ran). + var last *state.Snapshot + for i := len(snaps) - 1; i >= 0; i-- { + if snaps[i].Posture != "stopped" { + last = &snaps[i] + break + } + } + if last == nil { + t.Fatal("no live snapshot found after the reload") + } + if last.Verify != nil { + t.Errorf("Verify = %+v after disabling verifyInterval; want nil — resetVerify() did not run", last.Verify) + } +} + +// recordingHandler is a minimal slog.Handler that keeps every record's +// message, so a test can count how many times a specific log line fired +// without depending on discardLog's silence. +type recordingHandler struct { + mu sync.Mutex + msgs []string +} + +func (h *recordingHandler) Enabled(context.Context, slog.Level) bool { return true } +func (h *recordingHandler) Handle(_ context.Context, r slog.Record) error { + h.mu.Lock() + defer h.mu.Unlock() + h.msgs = append(h.msgs, r.Message) + return nil +} +func (h *recordingHandler) WithAttrs([]slog.Attr) slog.Handler { return h } +func (h *recordingHandler) WithGroup(string) slog.Handler { return h } + +func (h *recordingHandler) count(msg string) int { + h.mu.Lock() + defer h.mu.Unlock() + n := 0 + for _, m := range h.msgs { + if m == msg { + n++ + } + } + return n +} + +// verifySuspended (the edge-trigger flag guarding the "verification +// suspended" log line) is only ever cleared by the verifyC tick's own +// resume branch — so a reload that disables verifyInterval WHILE +// panic-disarmed, stopping that tick forever, must clear it too. Otherwise a +// later reload that re-enables verification finds verifySuspended already +// true and the edge never re-fires, silently dropping the operator-visible +// warning the second time around. +func TestReloadedVerifyIntervalDisableResetsSuspendedFlag(t *testing.T) { + const suspendMsg = "enforcement verification suspended — `dezhban panic` tore down the rules " + + "deliberately; run `dezhban unblock` (or restart the daemon) to resume" + + m := &fakePanicMarker{} + m.disarmed.Store(true) // panic-disarmed for the whole test + be := &fakeBackend{} + h := &recordingHandler{} + + ctx, cancel := context.WithTimeout(context.Background(), 400*time.Millisecond) + defer cancel() + + reloadC := make(chan LiveSettings, 2) + o := Options{ + Monitor: steadyMonitor{cc: "US"}, + Decider: decision.New([]string{"IR"}, 1), + Backend: be, + Log: slog.New(h), + Interval: time.Hour, + Tunnels: []string{"utun4"}, + Endpoints: []netip.Addr{netip.MustParseAddr("203.0.113.7")}, + VerifyInterval: 10 * time.Millisecond, // on at boot, so the first suspend edge fires early + PanicDisarmed: m.Disarmed, + ReloadC: reloadC, + } + + go func() { + time.Sleep(60 * time.Millisecond) // let the first verify tick log the suspend edge + ls := o.Live() + ls.VerifyInterval = -1 // config.Disabled sentinel + reloadC <- ls + + time.Sleep(60 * time.Millisecond) // stay disabled a while, still disarmed + ls2 := o.Live() + ls2.VerifyInterval = 10 * time.Millisecond // re-enable, panic still armed + reloadC <- ls2 + }() + + if err := Run(ctx, o); err != nil { + t.Fatal(err) + } + + if got := h.count(suspendMsg); got < 2 { + t.Errorf("suspend-edge warning logged %d time(s), want at least 2 "+ + "(once before the disable, once after re-enabling while still panic-disarmed)", got) + } +} + +// vpn.advanced.livenessRedial is declared live-appliable too, but unlike the +// windows above it needs no ticker of its own — maybeAutoWindow's zombie-widen +// branch just reads o.LivenessRedial directly on every geoTick (see runner.go). +// So the live-reload promise here is narrower but just as real: a zombie streak +// that has ALREADY crossed the hysteresis threshold while the key was off must +// still earn its one automatic attempt the moment a reload turns it on, rather +// than waiting for an entirely new streak to form. +func TestReloadedLivenessRedialLetsAStandingStreakOpenAWindow(t *testing.T) { + be := &fakeBackend{} + ctx, cancel := context.WithTimeout(context.Background(), 400*time.Millisecond) + defer cancel() + + reloadC := make(chan LiveSettings, 1) + + var ( + mu sync.Mutex + reloaded bool + ) + o := Options{ + Monitor: steadyFailMonitor{}, // every exit check fails — a standing zombie streak + Decider: decision.New([]string{"IR"}, 2), + Backend: be, + Log: discardLog(), + Interval: 10 * time.Millisecond, + Tunnels: []string{"utun4"}, + Endpoints: []netip.Addr{netip.MustParseAddr("203.0.113.7")}, + Watcher: edgeWatcher(100000), // interface reports up for the whole run + // LivenessRedial left false: off at boot, same as the key's real default. + RedialWindow: 30 * time.Millisecond, + RedialBudget: testRedialBudget, + RedialBudgetWindow: testRedialBudgetWindow, + ReloadC: reloadC, + } + o.Publish = func(s state.Snapshot) { + mu.Lock() + defer mu.Unlock() + // The moment the streak is actually reported, turn the key on — + // event-driven, so the test cannot race the streak crossing + // hysteresis and needs no sleep to line the two up. + if s.Zombie != nil && s.Zombie.Checks >= 2 && !reloaded { + reloaded = true + ls := o.Live() + ls.LivenessRedial = true // the change under test: off → on, mid-streak + select { + case reloadC <- ls: + default: + } + } + } + if err := Run(ctx, o); err != nil { + t.Fatal(err) + } + mu.Lock() + defer mu.Unlock() + + if !reloaded { + t.Fatal("the zombie streak never reached hysteresis; this fixture tests nothing") + } + if !hasCall(be.calls, "apply-switch") { + t.Errorf("no automatic window opened after vpn.advanced.livenessRedial was reloaded on for an "+ + "already-standing streak; calls = %v", be.calls) + } +} diff --git a/internal/runner/runner.go b/internal/runner/runner.go index 203871b..e66f603 100644 --- a/internal/runner/runner.go +++ b/internal/runner/runner.go @@ -107,6 +107,12 @@ type Backend interface { Apply(p firewall.Policy) error Unblock() error Cleanup() error + // IsBlocked reports whether dezhban's rules are currently installed. The + // run loop uses it for enforcement verification, and it is part of this + // narrow interface rather than an optional capability discovered by type + // assertion on purpose: a backend that silently could not be verified would + // reintroduce the very silent-failure mode verification exists to close. + IsBlocked() (bool, error) } // Options bundles everything the run loop needs. main assembles it from config @@ -213,6 +219,38 @@ type Options struct { // last sighting once a refresh no longer reports it (VPN mode) — the window // in which a dropped VPN can redial the same server. <=0 → 15m. EndpointGrace time.Duration + // VerifyInterval is how often Backend.IsBlocked is consulted to confirm the + // rules dezhban believes it installed are still there, re-applying the + // posture in force when they are not. <=0 → disabled (the negative + // config.Disabled sentinel arrives here as an explicit opt-out). + // + // Every other Apply in this loop is triggered by something dezhban itself + // did. This is the only one that notices a ruleset removed from OUTSIDE the + // daemon, which until it existed left the guard able to fail silently — the + // daemon reporting GUARD, `status` reporting blocked, and the host open. + VerifyInterval time.Duration + // PanicDisarmed reports whether `dezhban panic` has torn down the rules + // deliberately while this daemon keeps running. Consulted on every + // verifyC tick: without it, verification cannot tell "something else + // removed my rules" from "the operator removed them on purpose", and + // would re-apply the standing posture within one VerifyInterval of a + // panic teardown — turning the documented lockout escape hatch into a + // brief flicker. nil → verification never stands down (tests / legacy + // callers, or panic never wired up a marker). + PanicDisarmed func() bool + // ClearPanicDisarm removes the marker PanicDisarmed reads, so an operator + // explicitly asking THIS running daemon to resume enforcement (the + // control socket's unblock op) clears it — the CLI process handling a + // direct/--force unblock has root and clears the marker itself instead. + // nil → nothing to clear (tests / legacy callers, or panic never wired up + // a marker). + ClearPanicDisarm func() error + // LivenessRedial (vpn.advanced.livenessRedial): let a hung tunnel — up + // interface, failing exit lookups — open an automatic redial window via the + // existing trigger 2 machinery. Default false; see the config doc comment + // for the censoring-exit hazard this guards against, and + // docs/adr/0010-tunnel-liveness.md for the full rationale. + LivenessRedial bool // AutoArm (vpn.autoArm): start PASSIVE (standby, no enforcement) when no // tunnel interface is present, and arm the guard automatically the moment // one appears. Arming is one-way on tunnel loss — a drop is @@ -369,7 +407,53 @@ func anyTunnelUp(tunnels []state.Tunnel) bool { // only a nil check when observability is off. Each call emits a complete snapshot // (the file is replaced atomically), so callers pass the last-known reading even // on tunnel/endpoint events to avoid blanking IP/country between polls. -func (o Options) publish(blocked bool, standby bool, r monitor.Reading, lookupErr error, enfErr error, tunnels []state.Tunnel, endpoints []netip.Addr, win *state.SwitchState, profile string, drop *state.DropRecord, hold *state.HoldState, redialRefused *state.RedialState) { + +// diag carries diagnostic and observational run-loop state that isn't central +// enough to the posture decision to earn its own publish parameter. Grouping it +// keeps publish's parameter list from growing by one every time the run loop +// learns something new worth surfacing — it was already at twelve positional +// parameters, a length where a swap of two same-typed arguments compiles clean +// and says nothing. +// +// Most fields are CONDITIONS, not measurements: each is set while something is +// wrong and cleared when it is not, so the zero value is the healthy state and +// the whole struct is safe to pass by value. exitIPChangedAt is the one sticky +// exception — a fact that is never cleared once observed. +type diag struct { + // verify is the last unhappy enforcement-verification result, nil when the + // rules were confirmed present (or verification is disabled). + verify *state.VerifyState + // zombie is set while the tunnel interface reports up but a run of geo + // lookups through it has failed — nil once a lookup succeeds, the tunnel + // goes down, or anything else ends the streak's eligibility. + zombie *state.ZombieState + // exitIPChangedAt is when the observed exit IP last differed from the + // previous successful reading. Zero means no change has been observed + // yet. Sticky — never reset by a later clean tick, unlike verify/zombie. + exitIPChangedAt time.Time + // panicDisarmed mirrors o.PanicDisarmed() as of this snapshot — a live read + // of the marker file, not state accumulated over ticks like verify/zombie, + // but it belongs here for the same reason: it is the one piece of context + // that explains why blocked/posture may not describe what the firewall is + // actually doing. Set by the snapshot closure in runGuard, the one place + // that publishes for every code path that can skip an Apply while + // panic-disarmed (closeWindowRevert included), rather than by each such + // call site individually. + panicDisarmed bool +} + +// panicDisarmed reports whether `dezhban panic` has torn the rules down and +// this daemon should stand down its automatic Apply paths — o.PanicDisarmed +// with the nil check folded in, since it is optional (tests / legacy callers, +// or panic never wired up a marker) and every one of the run loop's dozen +// call sites otherwise had to re-prove that nil safety itself, which is +// exactly the kind of copy-paste a future edit (a new call site, a flipped +// polarity) can get wrong silently. +func (o Options) panicDisarmed() bool { + return o.PanicDisarmed != nil && o.PanicDisarmed() +} + +func (o Options) publish(blocked bool, standby bool, r monitor.Reading, lookupErr error, enfErr error, tunnels []state.Tunnel, endpoints []netip.Addr, win *state.SwitchState, profile string, drop *state.DropRecord, hold *state.HoldState, redialRefused *state.RedialState, d diag) { if o.Publish == nil { return } @@ -389,6 +473,10 @@ func (o Options) publish(blocked bool, standby bool, r monitor.Reading, lookupEr Drop: drop, Hold: hold, Redial: redialRefused, + Verify: d.verify, + Zombie: d.zombie, + ExitIPChangedAt: d.exitIPChangedAt, + PanicDisarmed: d.panicDisarmed, } if r.IP.IsValid() { snap.IP = r.IP.String() @@ -902,8 +990,142 @@ func (o Options) runGuard(ctx context.Context) error { redialRetryTimer = time.NewTimer(d) redialRetryC = redialRetryTimer.C } + // dg is the run loop's diagnostic conditions. Owned by this goroutine like + // everything else here, and republished on every snapshot so a condition + // raised by one tick stays visible until the tick that clears it. + var dg diag + // verifyRepairs counts re-applies since startup. Deliberately cumulative and + // never reset by a clean check: a host where this keeps climbing has + // something repeatedly removing dezhban's rules, and that pattern is the + // finding — a counter that reset on every good tick would hide it. + var verifyRepairs int + // verifySuspended tracks whether the last verifyC tick found + // PanicDisarmed true, purely so the log line below fires at the edge + // (suspend / resume) rather than once per tick for as long as the marker + // stands — same reasoning as the zombie/verify log-at-edge pattern. + var verifySuspended bool + // zombieChecks / zombieSince track a run of failed exit lookups through a + // tunnel that reports up. Reset to zero whenever the streak stops meaning + // what it meant — a successful lookup, the tunnel going down, or anything + // that suspends the geo state machine entirely (standby, a window, a + // manual block). See resetZombie below. + var zombieChecks int + var zombieSince time.Time + // zombieRedialTried gates the ONE liveness-redial attempt a given zombie + // streak gets, mirroring how an ordinary drop calls maybeAutoWindow exactly + // once, at its own edge. Without this, the zombie tunnel never producing a + // down edge means the per-tick zombie check would otherwise re-invoke + // maybeAutoWindow on every geoTick for as long as the streak stands — + // spamming the ledger and, on a refusal, re-arming a retry timer every tick + // instead of once. + // + // It survives a window opening and closing: an auto-granted window + // suspends the geo state machine (see the windowActive branch of geoTick), + // but suspending observation does not mean the hang resolved, so a window + // that expires with the tunnel still hung must not look like a fresh + // streak. It is cleared only by resetZombie(full: true) — a genuine + // resolution (a lookup succeeds, the tunnel actually goes down, or the + // posture otherwise changes out from under it) — never by + // resetZombie(full: false), which the windowActive branch uses. + var zombieRedialTried bool + // zombieHoldSuppressed records that THIS streak's one-shot liveness-redial + // attempt was suppressed by an armed hold (maybeAutoWindow's holdArmed + // branch, called with consumeHold:false) rather than actually run. That + // call returns before grantAutoWindow, so no redialRefused ever gets set — + // unlike the ordinary drop trigger, there is nothing standing for + // retryAutoWindow to act on later. Without this flag, cancelling hold + // afterward (resumeRedialRetry) would find redialRefused == nil and do + // nothing, permanently forfeiting the streak's attempt even though hold + // never consumed it. Cleared alongside zombieRedialTried, both on a + // genuine resolution (resetZombie(full: true)) and once resumeRedialRetry + // has acted on it — see both for how. + var zombieHoldSuppressed bool + // lastGoodIP is the exit IP from the last SUCCESSFUL reading, kept + // separately from lastRes.Reading (which a failed lookup overwrites with a + // zero Reading) so a failure streak can never be misread as a change. + // Purely observational: comparing against it never touches blocked, + // CountryCode, or the hysteresis streak. + var lastGoodIP netip.Addr + // observeExitIP records a confirmed exit IP and logs a change against the + // last one seen. Shared by the startup probe and the geoTick branch so a + // failover landing between the two can't be missed by only wiring this + // into one of them. Both callers run it on every successful reading, + // including one that lands FULL BLOCK — a blocked-country exit still has + // an IP, and excluding it would both miss a failover between two + // forbidden-country servers and leave the very first tick's baseline + // unseeded whenever startup itself reads blocked, misreading the NEXT + // good reading's IP as a "change" from nothing. + observeExitIP := func(ip netip.Addr) { + if !ip.IsValid() { + return + } + if lastGoodIP.IsValid() && ip != lastGoodIP { + o.Log.Info("exit IP changed", "from", lastGoodIP, "to", ip) + dg.exitIPChangedAt = time.Now() + } + lastGoodIP = ip + } + // resetZombie clears the zombie streak's observation (zombieChecks, + // zombieSince, the published dg.zombie). full additionally clears + // zombieRedialTried, i.e. declares the underlying hang itself resolved + // rather than merely un-observed for a while — see zombieRedialTried's doc + // comment for why those are different questions. + // zombieEpisodeOpen reports whether a zombie EPISODE is still unresolved, as + // opposed to whether it is being observed right now. The two differ while a + // window is open: resetZombie(false) drops the observation (dg.zombie) but + // deliberately keeps zombieRedialTried, because suspending observation is + // not evidence the hang healed. Everything that has to outlive that + // suspension — identifying which trigger a standing refusal belongs to, and + // deciding whether that refusal is still live — must ask this, never + // dg.zombie, or it silently answers "no episode" for the whole window and + // strands the refusal with nothing left to reconsider it. + zombieEpisodeOpen := func() bool { return dg.zombie != nil || zombieRedialTried } + resetZombie := func(full bool) { + if zombieChecks == 0 && dg.zombie == nil && (!full || (!zombieRedialTried && !zombieHoldSuppressed)) { + return + } + zombieChecks = 0 + zombieSince = time.Time{} + dg.zombie = nil + if full { + zombieRedialTried = false + zombieHoldSuppressed = false + } + } + // resetVerify clears a stale enforcement-verification finding. dg.verify is + // otherwise only ever set or cleared by the verifyC tick handler itself, so + // anything that stops that tick from running — standby (skipped there by + // design; see the verifyC case) or a live reload that disables + // verifyInterval — must clear it explicitly, or a "rules missing, N + // repairs" finding from before the transition would keep being republished + // forever, misreporting an enforcement problem while the daemon is + // correctly idle. + resetVerify := func() { + dg.verify = nil + } + // clearPanicDisarmBestEffort clears a standing panic-disarm marker, + // never failing its caller over it — see each call site for why that + // particular one must clear it. Factored out because the same nil-check + // + best-effort debug log was copy-pasted at every site. + // + // Not merged with cmd/dezhban/panicmark.go's clearPanicMarkerBestEffort, + // its twin on the other side of the marker file: this package never + // touches the marker path directly (o.ClearPanicDisarm is main's + // injected func() error, kept that way so runner stays testable without + // a real state directory), and a "main" package cannot be imported by + // anything else in Go — there is no third package the two could share + // without a generic wrapper for a 3-line pattern, which is not worth it. + clearPanicDisarmBestEffort := func() { + if o.ClearPanicDisarm == nil { + return + } + if err := o.ClearPanicDisarm(); err != nil { + o.Log.Debug("clear panic-disarm marker failed", "err", err) + } + } snapshot := func() { - o.publish(blocked, standby, lastRes.Reading, lastRes.Err, enfErr, lastTun, endpoints, switchState(), activeProfile, lastDrop, holdState(), redialState()) + dg.panicDisarmed = o.panicDisarmed() + o.publish(blocked, standby, lastRes.Reading, lastRes.Err, enfErr, lastTun, endpoints, switchState(), activeProfile, lastDrop, holdState(), redialState(), dg) } rebuild := func() { guard, fullBlock = o.vpnPolicies(tunnels, endpoints, providers) } @@ -915,6 +1137,13 @@ func (o Options) runGuard(ctx context.Context) error { if windowActive || blocked || standby { return } + if o.panicDisarmed() { + // `dezhban panic` tore the rules down deliberately; an automatic + // re-apply here would silently undo that. See applyWindowPolicy's + // twin of this guard for the full rationale. + o.Log.Debug("panic-disarm marker set — not re-applying standing posture", "reason", reason) + return + } // The standing posture is usually ModeGuard but is ModeFullBlock in the // zero-tunnel standing case — log the actual mode so autodetect/zero-tunnel // runs aren't misreported as "guard". @@ -933,8 +1162,23 @@ func (o Options) runGuard(ctx context.Context) error { // needs no rule update. A restricted window filters by proto/port and must // learn the new tunnel/endpoint, or that traffic stays blocked and the // verified early-close can never succeed. - reapplyWindow := func(reason string) { - if !windowActive || !o.windowRestricted() { + // applyWindowPolicy installs the open window's policy unconditionally. Split + // out of reapplyWindow because the two callers disagree about the + // unrestricted case: a tunnel/endpoint change genuinely does not affect a + // window that already passes everything, but enforcement verification finding + // the rules GONE does — an unrestricted window's pass vanished with them, and + // skipping it there would leave the host open while the daemon logged a + // repair. + applyWindowPolicy := func(reason string) { + if o.panicDisarmed() { + // `dezhban panic` tore the rules down deliberately, and this daemon + // is still running. An automatic re-apply here — reached from a + // tunnel/endpoint change or enforcement verification finding the + // window's pass gone — would turn the documented lockout escape + // hatch into a brief flicker, same as the verifyC check this + // mirrors. Stand down until `dezhban unblock` or a fresh daemon + // start clears the marker. + o.Log.Debug("panic-disarm marker set — not re-applying switch window", "reason", reason) return } if err := o.Backend.Apply(o.windowPolicy(tunnels, endpoints)); err != nil { @@ -946,26 +1190,52 @@ func (o Options) runGuard(ctx context.Context) error { } } - // reapplyPolicyFlags re-installs whatever posture is currently in force after - // vpn.allowPhysicalDNS / vpn.allowLocalNetwork changed under a live reload. + reapplyWindow := func(reason string) { + if !windowActive || !o.windowRestricted() { + return + } + applyWindowPolicy(reason) + } + + // reapplyCurrent re-installs whatever posture is currently in force, whatever + // that is. It is the one place that knows how to answer "put back what should + // be there", and has two callers with quite different reasons for asking: + // a live reload of the two policy flags (below), and enforcement verification + // finding the rules gone from under the daemon. // - // It exists because reapplyStanding deliberately skips FULL BLOCK — correct for - // a tunnel/endpoint change, which lands on the next guard restore — but wrong - // for these two flags: FullBlock CARRIES both passes (see - // firewall.PolicyInput.FullBlock), so turning one off while cut would leave the - // old pass installed while the reload reported the key as applied. A tightening - // reported as applied has to actually be in force. - reapplyPolicyFlags := func(reason string) { - rebuild() + // It deliberately does not rebuild the policies ITSELF — the caller decides + // whether its reason changed what the rules should say; verification's did + // not: the rules are correct, they are simply absent. The guard-posture + // case below still ends up rebuilding, but via reapplyStanding, which + // always does — a no-op recompute here, since verification changes none of + // reapplyStanding's own inputs (tunnels, endpoints, providers). + // + // Returns the resulting enfErr so a caller that needs to know whether THIS + // attempt actually landed — verification's repair path, below — can tell + // a successful re-apply from a failed one instead of assuming success the + // moment this returns. Every branch below sets enfErr itself before + // returning (nil on success, the Backend.Apply error on failure); the + // panic-disarm skip is deliberately not a failure, so it returns without + // touching enfErr and this reports whatever it already was. + reapplyCurrent := func(reason string, force bool) error { switch { case standby: // Nothing is installed in standby; the rebuilt sets arm with the guard. case windowActive: - // An unrestricted window already passes everything, so only the - // restricted form carries AllowLocalNetwork — which is exactly what - // reapplyWindow re-applies. - reapplyWindow(reason) + // An unrestricted window already passes everything, so a policy-flag + // change only reaches a restricted one — the check reapplyWindow + // makes. `force` is verification's path: the rules are absent, so + // even an unrestricted window has to be re-installed. + if force { + applyWindowPolicy(reason) + } else { + reapplyWindow(reason) + } case blocked: + if o.panicDisarmed() { + o.Log.Debug("panic-disarm marker set — not re-applying full block", "reason", reason) + return enfErr + } if err := o.Backend.Apply(fullBlock); err != nil { enfErr = err o.Log.Error("re-apply full block failed", "reason", reason, "err", err) @@ -976,6 +1246,21 @@ func (o Options) runGuard(ctx context.Context) error { default: reapplyStanding(reason) } + return enfErr + } + + // reapplyPolicyFlags re-installs whatever posture is currently in force after + // vpn.allowPhysicalDNS / vpn.allowLocalNetwork changed under a live reload. + // + // It exists because reapplyStanding deliberately skips FULL BLOCK — correct for + // a tunnel/endpoint change, which lands on the next guard restore — but wrong + // for these two flags: FullBlock CARRIES both passes (see + // firewall.PolicyInput.FullBlock), so turning one off while cut would leave the + // old pass installed while the reload reported the key as applied. A tightening + // reported as applied has to actually be in force. + reapplyPolicyFlags := func(reason string) { + rebuild() + reapplyCurrent(reason, false) } stopWindowTimers := func() { @@ -1026,6 +1311,16 @@ func (o Options) runGuard(ctx context.Context) error { } openWindow := func(now time.Time, dur time.Duration, profile, trigger string) { + // A manual switch or a pause is an explicit operator command — it clears + // a standing panic-disarm marker unconditionally, same as OpBlock/ + // OpUnblock, covering both the fresh-open branch below and the + // takeover/extend branch right after (neither Applies while the auto + // trigger is excluded here: clearing on its behalf would defeat + // autoWindowPossible's own PanicDisarmed check, which must be the only + // thing standing between trigger 2 and the marker). + if trigger != state.TriggerAuto { + clearPanicDisarmBestEffort() + } if windowActive { // A manual command takes over an auto window's attribution (the // operator is now driving); an auto trigger never fires while a window @@ -1136,7 +1431,16 @@ func (o Options) runGuard(ctx context.Context) error { // A second copy is how the retry would come to relax a guard the drop edge // would have refused. autoWindowPossible := func() bool { - return o.RedialWindow > 0 && !windowActive && !standby && !blocked && sawTunnelUp + // The panic-disarm marker being set stands trigger 2 down exactly like + // vpn.redialWindow being "0" does: `dezhban panic` tore the rules down + // deliberately, and an automatic window here would silently undo that. + // Both maybeAutoWindow (the drop edge) and retryAutoWindow (the + // bound-lifted re-decision) call through this one check, so this single + // line covers trigger 2 in full — including dropping a stale refusal and + // disarming its retry timer via the existing "no longer available" + // handling in grantAutoWindow. + return o.RedialWindow > 0 && !windowActive && !standby && !blocked && sawTunnelUp && + (!o.panicDisarmed()) } // grantAutoWindow asks the ledger and acts on the answer. Shared by the drop @@ -1171,7 +1475,12 @@ func (o Options) runGuard(ctx context.Context) error { // declines to help is the failure this project treats as worst, so the // refusal carries the numbers behind it and `status`/the app turn the // same facts into a sentence (see the redial object in the snapshot). - o.Log.Warn("vpn tunnel down — no redial window ("+redialRefusal(g.Reason)+ + // + // Deliberately does not say "vpn tunnel down": this closure is also + // trigger 2's zombie-tunnel widening (LivenessRedial), where the + // interface reports up the whole time — detail carries the specific + // reason either way. + o.Log.Warn("no automatic redial window ("+redialRefusal(g.Reason)+ "); guard holds, traffic stays cut", "reason", string(g.Reason), "uptime", uptime.Round(time.Second), @@ -1226,7 +1535,7 @@ func (o Options) runGuard(ctx context.Context) error { } } - maybeAutoWindow := func(now time.Time, detail string) { + maybeAutoWindow := func(now time.Time, detail string, consumeHold bool) { if !autoWindowPossible() { return } @@ -1240,9 +1549,28 @@ func (o Options) runGuard(ctx context.Context) error { // never seen up), so the flag survives those. That is safe because a // drop cannot follow a drop without an intervening tunnel-up edge, and // that edge disarms it — see the st.Up branch in the watcher. + // + // consumeHold distinguishes the ordinary tunnel-down trigger (true) from + // the liveness-redial trigger (false): hold's promise is "my NEXT + // DISCONNECT is deliberate", and a zombie streak — the interface never + // goes down — is not the event it was armed for. The flag still + // suppresses a liveness attempt (hold only ever subtracts a relaxation, + // and this is one), it just isn't spent by an event it didn't name, so a + // later real disconnect still gets the hold the operator asked for. if holdArmed { - holdArmed = false - o.Log.Warn("vpn tunnel down — redial window suppressed (hold the line was armed); "+ + if consumeHold { + holdArmed = false + } else { + // The zombie/liveness caller passes consumeHold:false and + // returns here without ever reaching grantAutoWindow below, so + // no redialRefused gets set for retryAutoWindow to act on + // later. Record the suppression directly so a subsequent + // hold-cancel (resumeRedialRetry) can still give the streak's + // one-shot attempt back — see zombieHoldSuppressed's doc + // comment. + zombieHoldSuppressed = true + } + o.Log.Warn("redial window suppressed (hold the line was armed); "+ "guard holds, traffic stays cut", "detail", detail) return } @@ -1281,9 +1609,40 @@ func (o Options) runGuard(ctx context.Context) error { // refusal stands, and a grant clears the refusal and disarms the timer. // Nothing re-arms it, so an expired window never re-opens. retryAutoWindow := func(now time.Time) { - // A refusal must still stand and the tunnel must still be down. Either - // being false means the drop this retry belongs to is over. - if redialRefused == nil || tunnelUp { + // A refusal must still stand, and the condition it was refused for must + // still be open. An ordinary drop needs the tunnel still down + // (tunnelUp == false); a zombie-tunnel drop (LivenessRedial widening + // trigger 2) needs its streak still standing instead, since its tunnel + // reports up for the whole episode — tunnelUp alone would never let + // this retry fire for that trigger, leaving a refused liveness-redial + // attempt stuck forever once the every-tick reattempt below was + // tightened to fire only once per streak (see zombieRedialTried). + // + // The episode test is zombieEpisodeOpen(), NOT `dg.zombie != nil`: + // dg.zombie is the *observation*, and the windowActive branch of geoTick + // clears it (resetZombie(false)) for as long as any window is open. A + // refusal earned by a zombie streak would then hit this guard with + // tunnelUp true and dg.zombie nil, return without dropping it, and be + // stranded — the retry channel is disarmed by its own case before this + // runs, and nothing else re-arms it — leaving `status` and the app + // promising a nextEligible that has already passed for the rest of the + // cut. zombieRedialTried survives exactly that suspension, which is what + // makes it the episode's identity. + if redialRefused == nil || (tunnelUp && !zombieEpisodeOpen()) { + return + } + // A zombie-triggered refusal only re-asks while the opt-in that armed + // it is still live — vpn.advanced.livenessRedial is what widened + // trigger 2 to a hung-but-up tunnel in the first place, and checking + // only dg.zombie above would let an operator's live "off" be silently + // bypassed the moment the budget/cooldown that refused it lifts. + // Ordinary (tunnel-down) drops are untouched: that trigger has no such + // opt-in to disable. Clears the stale refusal rather than leaving it + // standing with nothing left to reconsider it. + if tunnelUp && zombieEpisodeOpen() && !o.LivenessRedial { + o.Log.Info("zombie-tunnel redial retry skipped — vpn.advanced.livenessRedial was disabled after the refusal") + redialRefused = nil + disarmRedialRetry() return } // Hold the line, armed AFTER the drop by an operator watching a cut they @@ -1295,7 +1654,18 @@ func (o Options) runGuard(ctx context.Context) error { o.Log.Info("redial retry skipped — hold the line is armed; guard holds, traffic stays cut") return } - grantAutoWindow(now, dropUptime, dropGoodExit, dropDetail) + // dropUptime is frozen by design for an ordinary drop (see its doc + // comment above) because the tunnel is down and re-deriving it would + // grow for no real reason. A zombie streak is the opposite: the + // interface never went down, so tunnelUpSince is a live connection + // start time and now.Sub(tunnelUpSince) growing is exactly correct — + // reusing the frozen value would let redialMinUptime's backoff refuse + // a tunnel forever even once it has genuinely been up long enough. + uptime := dropUptime + if tunnelUp && zombieEpisodeOpen() && !tunnelUpSince.IsZero() { + uptime = now.Sub(tunnelUpSince) + } + grantAutoWindow(now, uptime, dropGoodExit, dropDetail) } // resumeRedialRetry restores the pending re-decision that an armed hold @@ -1320,6 +1690,25 @@ func (o Options) runGuard(ctx context.Context) error { // original timer running and correct; re-deciding then would ask against a // bound that has not lifted yet. resumeRedialRetry := func() { + // A zombie streak's suppressed attempt never went through + // grantAutoWindow, so it left no redialRefused for the retryAutoWindow + // call below to find — handled separately, and not gated on + // redialRetryC below since that suppression never armed a retry timer + // either. If the streak is still standing, clearing zombieRedialTried + // lets the very next geoTick's zombie check retry maybeAutoWindow + // exactly as it would have on a fresh streak; if it already resolved, + // there is nothing left to give back. + // + // Not gated on dg.zombie either: the two flags are set together and + // cleared together (resetZombie(full: true)), so zombieHoldSuppressed + // still being set IS the proof the episode never resolved — whereas + // dg.zombie is nil for the whole of any open window (resetZombie(false)), + // which would silently forfeit the attempt for a hold cancelled there. + if zombieHoldSuppressed { + zombieHoldSuppressed = false + zombieRedialTried = false + o.Log.Info("liveness redial attempt restored for the standing zombie streak — hold the line was cancelled") + } if redialRetryC != nil { return } @@ -1337,12 +1726,33 @@ func (o Options) runGuard(ctx context.Context) error { // the ledger at the same instant openWindow opened against. The package is // clock-injected precisely so window accounting never depends on where in a // function the clock happened to be read. - closeWindowRevert := func(now time.Time, reason string) { + // explicit distinguishes an operator's cancel/resume (control socket or the + // root command file) from the window's own expiry timer firing. An explicit + // close clears a standing panic-disarm marker unconditionally — same as + // OpBlock/OpUnblock/openWindow — and always Applies. An automatic expiry + // while disarmed must not silently undo a `dezhban panic` teardown, so it + // skips the Apply but still closes the window's own bookkeeping: the + // window's clock is genuinely up, and reporting state.switch as still open + // past its deadline would be its own bug. + closeWindowRevert := func(now time.Time, reason string, explicit bool) { rebuild() target := guard if windowPrevBlocked { target = fullBlock } + if explicit { + clearPanicDisarmBestEffort() + } else if o.panicDisarmed() { + stopWindowTimers() + windowActive = false + redialLedger.Close(now) + blocked = windowPrevBlocked + enfErr = nil + o.Log.Warn(windowNoun()+" expired while panic-disarmed — not reinstating rules; "+ + "run `dezhban unblock` (or restart the daemon) to resume enforcement", "reason", reason) + snapshot() + return + } if err := o.Backend.Apply(target); err != nil { // The revert failed, so the firewall may still be in switch-window // posture. Do NOT report the window closed or unsuppress the geo state @@ -1413,6 +1823,11 @@ func (o Options) runGuard(ctx context.Context) error { if probeInFlight || !windowActive || len(tunnels) == 0 { return } + if o.panicDisarmed() { + // No point spending a network probe on a close that finishCloseProbe + // would refuse to Apply anyway while disarmed. + return + } if len(discoveredAddrs(lastSet)) == 0 { return // no live VPN socket yet — a static endpoint alone can't confirm a connect } @@ -1436,6 +1851,12 @@ func (o Options) runGuard(ctx context.Context) error { if !windowActive || len(tunnels) == 0 { return } + if o.panicDisarmed() { + // The marker may have been set while this probe was already in + // flight (started before `dezhban panic` ran) — must not Apply(guard) + // on completion, or a deliberate teardown gets silently undone. + return + } disc := discoveredAddrs(lastSet) if len(disc) == 0 { return // the socket vanished mid-probe — evidence gone, hold the window @@ -1491,6 +1912,13 @@ func (o Options) runGuard(ctx context.Context) error { if !standby { return } + if o.panicDisarmed() { + // A tunnel appearing is not an explicit operator command; arming + // out of standby here would silently undo a deliberate `dezhban + // panic` teardown, same as every other automatic Apply path. + o.Log.Debug("panic-disarm marker set — not auto-arming", "detail", detail) + return + } fresh := o.resolveEndpointsWith(ctx, tunnels) lastSet = fresh if next, changed := reconcileWithGrace(endpoints, fresh, false, epLastSeen, time.Now(), epGrace()); changed { @@ -1602,6 +2030,14 @@ func (o Options) runGuard(ctx context.Context) error { // contradict the operator's other explicit request; make them choose. return reply(false, "switch window is open — cancel it first") } + // An explicit block is an operator asking THIS running daemon to + // enforce, so — mirroring OpUnblock below — it clears a standing + // panic-disarm marker unconditionally, even along the branch right + // below that finds nothing new to Apply. Any real drift left behind + // by `dezhban panic` is caught by the next enforcement-verification + // tick once the marker is gone. Best-effort: a failure here must not + // fail the block itself. + clearPanicDisarmBestEffort() if blocked { manualBlock = true // already blocked by geo: adopt and hold it return reply(true, "") @@ -1617,6 +2053,17 @@ func (o Options) runGuard(ctx context.Context) error { manualBlock = true standby = false // a manual block arms enforcement out of standby enfErr = nil + // A manual block is one of the states enumerated above resetZombie's + // own doc comment as ending what a standing zombie streak meant — + // without this, a stale "tunnel reports up but exit checks failing" + // warning from before the block would survive in this same publish, + // self-healing only at the next geoTick's own manualBlock branch. + resetZombie(true) + // The Apply above just froze fresh rules onto the backend, so a stale + // "rules missing, N repairs" finding from before this command no + // longer describes reality — see resetVerify's doc comment. Otherwise + // it would keep being republished until the next verifyC tick. + resetVerify() o.Log.Warn("FULL BLOCK (manual, via control socket) — held until unblock") snapshot() return reply(true, "") @@ -1625,6 +2072,14 @@ func (o Options) runGuard(ctx context.Context) error { if windowActive { return reply(false, "switch window is open — cancel it first") } + // An explicit unblock is an operator asking THIS running daemon to + // resume enforcement, so it clears a standing panic-disarm marker + // unconditionally — even along the branches below that find + // nothing to actually re-apply (e.g. the daemon's own `blocked` + // is already false, unaware that `panic` removed the rules out + // from under it). Best-effort: a failure here must not fail the + // unblock itself. + clearPanicDisarmBestEffort() manualBlock = false // vpn.autoArm: with the tunnel DOWN, an explicit unblock is the // operator saying "the VPN is off on purpose — release the line". @@ -1641,6 +2096,11 @@ func (o Options) runGuard(ctx context.Context) error { standby = true blocked = false enfErr = nil + // Nothing is installed in standby by design, so any diagnostic + // findings from the armed state that just ended no longer apply — + // see resetVerify's doc comment. + resetZombie(true) + resetVerify() o.Log.Info("STANDBY (manual unblock, vpn.autoArm) — guard released; re-arms when a VPN connects") snapshot() return reply(true, "") @@ -1657,6 +2117,10 @@ func (o Options) runGuard(ctx context.Context) error { } blocked = false enfErr = nil + // Same reasoning as OpBlock's resetVerify above: the Apply just + // installed fresh rules, so a stale "rules missing, N repairs" + // finding from before this command no longer applies. + resetVerify() o.Log.Info("GUARD (manual unblock, via control socket) — geo state machine resumed") snapshot() return reply(true, "") @@ -1701,7 +2165,7 @@ func (o Options) runGuard(ctx context.Context) error { if windowTrigger == state.TriggerPause { return reply(false, "a pause is open, not a switch window — use resume instead") } - closeWindowRevert(time.Now(), "cancelled (control socket)") + closeWindowRevert(time.Now(), "cancelled (control socket)", true) if windowActive { return reply(false, "cancel failed — window held open, revert is being retried") } @@ -1744,7 +2208,7 @@ func (o Options) runGuard(ctx context.Context) error { if !windowActive || windowTrigger != state.TriggerPause { return reply(true, "") // already closed — the caller's intent already holds } - closeWindowRevert(time.Now(), "resumed (control socket)") + closeWindowRevert(time.Now(), "resumed (control socket)", true) if windowActive { return reply(false, "resume failed — pause held open, revert is being retried") } @@ -1802,6 +2266,23 @@ func (o Options) runGuard(ctx context.Context) error { geoTick := time.NewTicker(o.Interval) defer geoTick.Stop() + // Enforcement verification runs on its own slow ticker, nil when disabled — + // a nil channel in a select blocks forever, which is exactly "this case does + // not exist". Created lazily so a reload can switch it on, and stopped via a + // closure rather than a plain `defer verifyTick.Stop()` because the ticker + // the deferred call must stop may be one applyLive created later. + var verifyTick *time.Ticker + var verifyC <-chan time.Time + if o.VerifyInterval > 0 { + verifyTick = time.NewTicker(o.VerifyInterval) + verifyC = verifyTick.C + } + defer func() { + if verifyTick != nil { + verifyTick.Stop() + } + }() + // applyLive adopts replacement settings on the run-loop goroutine. It updates // `o` (a per-call copy, so nothing is shared with another run) plus the // locals derived from it at startup, and reinstalls the standing rules when @@ -1897,6 +2378,40 @@ func (o Options) runGuard(ctx context.Context) error { o.EndpointRefresh = ls.EndpointRefresh } + // Unlike epTick, the verify ticker may not exist at all — it honors the + // Disabled sentinel, so a reload can turn it on, off, or just retime it. + if ls.VerifyInterval != o.VerifyInterval { + switch { + case ls.VerifyInterval <= 0: + if verifyTick != nil { + verifyTick.Stop() + verifyTick = nil + verifyC = nil + } + // The tick that would otherwise clear a stale finding no longer + // runs, so clear it here — turning verification off must not leave + // its last answer stuck. + resetVerify() + // verifySuspended is also only ever cleared by that same tick + // (the panic-disarm resume branch), so it must be reset here + // too. Otherwise a panic that suspended verification, followed + // by a reload that disables verifyInterval, leaves verifySuspended + // stuck true — and a LATER reload that re-enables verification + // finds it already true, so the edge-triggered "verification + // suspended" warning silently never re-fires even though + // verification just went from not-running to + // running-but-suspended with no operator-visible notice. + verifySuspended = false + case verifyTick == nil: + verifyTick = time.NewTicker(ls.VerifyInterval) + verifyC = verifyTick.C + default: + verifyTick.Reset(ls.VerifyInterval) + } + o.VerifyInterval = ls.VerifyInterval + } + o.LivenessRedial = ls.LivenessRedial + o.Log.Info("configuration reloaded", "interval", o.Interval, "blocked_countries", o.BlockedCountries, @@ -1968,19 +2483,40 @@ func (o Options) runGuard(ctx context.Context) error { // Startup observation: only meaningful with a tunnel up and an endpoint known. // With zero tunnels (standing posture) a lookup egresses nowhere useful. - if len(tunnels) > 0 && len(endpoints) > 0 { - lastRes, enfErr = o.vpnGeoStep(ctx, guard, fullBlock, &blocked, tunnelUp) - if lastRes.Err == nil && !blocked { - // A confirmed allowed exit proves the tunnel is carrying traffic. - goodExitThisUp = true - // But with a watcher, up/down is the watcher's to report: this - // startup reading only presumes up, and had the tunnel actually been - // down it could have egressed the allowlisted physical path. Let the - // watcher's own up sample set sawTunnelUp, so an auto redial window - // never opens for a tunnel it never observed up. - if o.Watcher == nil { - sawTunnelUp = true - markTunnelEverUp(time.Now()) + // + // Also skipped while panic-disarmed: unlike the startup Apply(guard) above + // (guaranteed marker-free in the real `dezhban run` entrypoint, since + // cmd/dezhban clears it before calling Run), this vpnGeoStep call can + // escalate straight to FULL BLOCK, and Run's own contract — never silently + // undo a deliberate `dezhban panic` teardown — must not depend on every + // caller having cleared the marker first. + if len(tunnels) > 0 && len(endpoints) > 0 && (!o.panicDisarmed()) { + res, err, attempted := o.vpnGeoStep(ctx, guard, fullBlock, &blocked, tunnelUp) + lastRes = res + if attempted { + enfErr = err + } + if lastRes.Err == nil { + // Seed lastGoodIP here too, on EVERY successful reading including + // one that lands FULL BLOCK — otherwise a failover between this + // startup reading and the first geoTick reading is silently + // missed (only the geoTick branch used to ever assign it), and a + // blocked-country startup reading would leave the baseline + // unseeded, misreading the next good reading's IP as a "change" + // from nothing. See observeExitIP's doc comment. + observeExitIP(lastRes.Reading.IP) + if !blocked { + // A confirmed allowed exit proves the tunnel is carrying traffic. + goodExitThisUp = true + // But with a watcher, up/down is the watcher's to report: this + // startup reading only presumes up, and had the tunnel actually been + // down it could have egressed the allowlisted physical path. Let the + // watcher's own up sample set sawTunnelUp, so an auto redial window + // never opens for a tunnel it never observed up. + if o.Watcher == nil { + sawTunnelUp = true + markTunnelEverUp(time.Now()) + } } } } @@ -2022,6 +2558,14 @@ func (o Options) runGuard(ctx context.Context) error { o.Log.Warn("vpn tunnel down — guard holds the line (physical egress stays blocked, "+ "endpoints open for redial)", "detail", st.Detail) } + if !st.Up { + // A plainly-down tunnel is a different, already-explained state — + // don't leave a stale "hung" diagnosis attached to it. The next + // geoTick would clear this anyway (its own down-tunnel skip does + // the same reset); doing it here means the down edge itself is + // never shown carrying a leftover zombie streak. + resetZombie(true) + } if next, changed := reconcileTunnels(tunnels, st.Names, pinned); changed { tunnels = next reapplyStanding("tunnel set changed") @@ -2038,7 +2582,7 @@ func (o Options) runGuard(ctx context.Context) error { // state unreachable on the common path. lastDrop = &state.DropRecord{At: time.Now()} snapshot() - maybeAutoWindow(time.Now(), st.Detail) + maybeAutoWindow(time.Now(), st.Detail, true) } if st.Up { // The drop is over the moment a tunnel is back, whether or not @@ -2116,7 +2660,7 @@ func (o Options) runGuard(ctx context.Context) error { continue } if windowActive { - closeWindowRevert(now, "cancelled") + closeWindowRevert(now, "cancelled", true) } case command.OpPause: if standby { @@ -2146,7 +2690,7 @@ func (o Options) runGuard(ctx context.Context) error { manualBlock = false case command.OpResume: if windowActive && windowTrigger == state.TriggerPause { - closeWindowRevert(now, "resumed") + closeWindowRevert(now, "resumed", true) } case command.OpHoldArm: // Same account the socket path gives, for the same reason as the @@ -2180,7 +2724,7 @@ func (o Options) runGuard(ctx context.Context) error { cr.Reply <- handleControl(cr.Req) case <-windowTimerC: if windowActive { - closeWindowRevert(time.Now(), "expired") + closeWindowRevert(time.Now(), "expired", false) } case <-redialRetryC: // The bound that refused this drop has lifted, so re-ask. Disarm @@ -2201,6 +2745,106 @@ func (o Options) runGuard(ctx context.Context) error { reapplyWindow("in-window endpoint discovery") } maybeStartCloseProbe() + case <-verifyC: + // Enforcement verification: confirm the rules dezhban believes it + // installed are still installed, and put them back when they are not. + // + // Skipped in standby, where nothing is installed BY DESIGN — a false + // answer is the correct one there, and "repairing" it would arm a host + // that has never seen a tunnel, which is exactly the lockout ADR-0002 + // exists to prevent. + if standby { + break + } + if o.panicDisarmed() { + // `dezhban panic` tore this down deliberately, and this daemon + // is still running. Verification must not silently undo a + // deliberate teardown — that would turn the documented + // lockout escape hatch into a ~1-VerifyInterval flicker. + // Stand down until `dezhban unblock` or a fresh daemon start + // clears the marker (see docs/usage/troubleshooting.md). + if !verifySuspended { + verifySuspended = true + o.Log.Warn("enforcement verification suspended — `dezhban panic` tore down the rules " + + "deliberately; run `dezhban unblock` (or restart the daemon) to resume") + } + break + } + if verifySuspended { + verifySuspended = false + o.Log.Info("enforcement verification resumed") + } + installed, err := o.Backend.IsBlocked() + switch { + case err != nil: + // An unreadable backend is NOT evidence the rules are gone, so + // this reports and changes nothing — the same discipline as an + // undeterminable exit country holding the current posture. + // Re-applying on a failed read would let a transient backend + // hiccup churn the ruleset on every tick. + // + // Logged at the edge only, mirroring the zombie streak below: a + // persistently unreadable backend would otherwise emit one Warn + // per tick forever into the size-rotated log, rotating away the + // evidence of the original problem before anyone reads it. + // `dg.verify.Missing` counts as a different condition, not the + // same edge: a failed repair also carries an Err, and coming from + // there to "cannot read the firewall at all" is a transition an + // operator needs to see rather than have deduplicated away. + if dg.verify == nil || dg.verify.Missing || dg.verify.Err == "" { + o.Log.Warn("enforcement verification could not read the firewall — posture held", + "err", err) + } + dg.verify = &state.VerifyState{At: time.Now(), Err: err.Error(), Repairs: verifyRepairs} + case !installed: + // Edge-triggered Error; a persisting problem (something keeps + // removing the rules) still repairs every tick — that part must + // not be edge-triggered — but logs at Info after the first tick, + // for the same log-rotation reason as the Err case above. + if dg.verify == nil || !dg.verify.Missing { + o.Log.Error("dezhban's firewall rules are MISSING — something removed them; re-applying now", + "posture", postureName(blocked, windowActive, standby), "repairs", verifyRepairs) + } else { + o.Log.Info("dezhban's firewall rules are still missing — re-applying again", + "posture", postureName(blocked, windowActive, standby), "repairs", verifyRepairs) + } + // force: the rules are absent, so even an unrestricted window — + // which no tunnel/endpoint change would ever need to re-apply — + // has lost its pass and must be reinstalled. + // + // Only count/report a completed repair once this actually + // succeeds — reapplyCurrent's return is the real Backend.Apply + // result, not an assumption. A failed attempt leaves the host + // still unenforced; enfErr (set by reapplyCurrent itself) carries + // that through the normal EnforcementErr surface on snapshot() + // below, instead of Repairs climbing for a repair that never + // landed. + // + // A failed repair is carried in Err ALONGSIDE Missing, not just + // in EnforcementErr: every reader of Missing (`status`, the + // menubar app, `doctor`) otherwise words it as "found missing + // and re-applied", which is exactly backwards for a host that is + // missing its rules AND could not get them back. Missing+Err is + // therefore a real, distinct state — the rules are gone and the + // repair did not land — not a contradiction. + repairErr := reapplyCurrent("enforcement verification: rules missing", true) + if repairErr != nil { + o.Log.Error("enforcement verification: repair attempt failed — still unenforced", "err", repairErr) + } else { + verifyRepairs++ + } + v := &state.VerifyState{At: time.Now(), Missing: true, Repairs: verifyRepairs} + if repairErr != nil { + v.Err = repairErr.Error() + } + dg.verify = v + default: + if dg.verify != nil { + o.Log.Info("enforcement verification: rules confirmed present again", "repairs", verifyRepairs) + } + dg.verify = nil + } + snapshot() case <-epTick.C: // Refresh the provider IPs on the same cadence. CDN-fronted providers // rotate addresses, and a stale set means the tunnel-scoped pass no @@ -2218,11 +2862,12 @@ func (o Options) runGuard(ctx context.Context) error { if fresh := o.ResolveProviders(ctx); len(fresh) > 0 && !sameAddrs(fresh, providers) { providers = fresh reapplyStanding("provider refresh") - if blocked { + if blocked && (!o.panicDisarmed()) { // FULL BLOCK is the posture that carries these rules, and // reapplyStanding deliberately skips it. Re-apply directly so a // rotated provider IP becomes reachable without waiting for the - // exit to change. + // exit to change. Skipped while panic-disarmed for the same + // reason every other automatic Apply path is. if err := o.Backend.Apply(fullBlock); err != nil { o.Log.Error("provider refresh: re-applying full block failed", "err", err) } @@ -2256,9 +2901,15 @@ func (o Options) runGuard(ctx context.Context) error { stopFastProbe("geo state machine suspended") } if standby { - continue // not enforcing — nothing to decide, nothing to protect a probe with + resetZombie(true) // nothing enforcing, nothing to diagnose + continue // not enforcing — nothing to decide, nothing to protect a probe with } if windowActive { + // Partial: a window is already the response to a suspected + // problem, so suspend observation — but a window (including one + // LivenessRedial itself opened) is not evidence the hang + // resolved, so zombieRedialTried survives. See its doc comment. + resetZombie(false) continue // window suppresses the geo state machine } if manualBlock { @@ -2266,19 +2917,118 @@ func (o Options) runGuard(ctx context.Context) error { // their back — including the probe, which would briefly open egress to // observe a country nobody is going to act on. Held until `unblock`. o.Log.Debug("manual block held — skipping geo lookup (run `dezhban unblock` to resume)") + resetZombie(true) + continue + } + if o.panicDisarmed() { + // `dezhban panic` tore the rules down deliberately. The geo state + // machine's Block/Allow transitions (vpnGeoStep) and its + // lift-and-probe recovery fallback (probe) both Apply — must not + // run at all while disarmed, same reasoning as manualBlock above. + o.Log.Debug("panic-disarm marker set — skipping geo lookup (run `dezhban unblock` to resume enforcement)") + resetZombie(true) continue } if len(tunnels) == 0 { + resetZombie(true) continue // standing posture: nothing to observe until a tunnel exists } if o.Watcher != nil && !tunnelUp && !blocked { o.Log.Debug("vpn tunnel down — skipping geo lookup (guard holds, endpoints open for redial)") + resetZombie(true) // plainly down is a different, already-explained state continue } - lastRes, enfErr = o.vpnGeoStep(ctx, guard, fullBlock, &blocked, tunnelUp) - if lastRes.Err == nil && !blocked { - goodExitThisUp, sawTunnelUp = true, true // confirmed exit through the tunnel - markTunnelEverUp(time.Now()) + res, err, attempted := o.vpnGeoStep(ctx, guard, fullBlock, &blocked, tunnelUp) + lastRes = res + if attempted { + enfErr = err + } + if lastRes.Err == nil { + // Exit-IP change observation: purely informational, like CVG's + // equivalent check — it never flips posture and never touches the + // hysteresis streak (CountryCode/Pending already own that). Run on + // EVERY successful reading, not just an allowed one: a failover + // between two servers in the same FORBIDDEN country still changes + // the exit IP, and skipping it here would leave "my exit flapped" + // unexplained for exactly the readings FULL BLOCK itself is + // watching over. See observeExitIP's own doc comment for the + // startup-reading half of the same seeding rule. + observeExitIP(lastRes.Reading.IP) + if !blocked { + goodExitThisUp, sawTunnelUp = true, true // confirmed ALLOWED exit through the tunnel + markTunnelEverUp(time.Now()) + } + } + // Zombie-tunnel detection: the interface reports up, but a run of exit + // lookups through it have failed. dezhban's posture never escalates on + // a lookup error alone (an unknown country HOLDS — see decision logic), + // so without this a hung tunnel stayed correctly cut but explained + // itself to no one and recovered only if a person noticed. Reusing the + // Decider's own hysteresis count as the streak length keeps this + // aligned with the same "how many agreeing readings before we act" + // tuning the rest of the state machine already uses. + // + // The hazard this is built around: an exit that CENSORS the geo + // providers produces this exact same failure streak on a perfectly + // live tunnel (see state.Snapshot's LookupErr doc). That is why + // reporting is unconditional but ACTING on it (LivenessRedial) is not. + if tunnelUp && !blocked && lastRes.Err != nil { + zombieChecks++ + if zombieChecks == 1 { + zombieSince = time.Now() + } + _, _, need := o.Decider.Pending() + if zombieChecks >= need { + if dg.zombie == nil { + o.Log.Warn("tunnel interface reports up, but exit lookups through it keep failing — "+ + "it may need reconnecting; guard holds either way", + "checks", zombieChecks, "since", zombieSince) + } + dg.zombie = &state.ZombieState{Since: zombieSince, Checks: zombieChecks} + // One attempt per streak, matching the ordinary drop trigger's + // own edge-only call to maybeAutoWindow: a refusal is left to + // retryAutoWindow's bound-lifted re-decision (its guard now + // recognises a standing zombie streak, not just tunnelUp), not + // to this tick trying again immediately. Without + // zombieRedialTried, a persisting streak would re-invoke + // maybeAutoWindow on every geoTick — hammering the ledger and, + // on a refusal, re-arming (and instantly re-expiring) a retry + // timer every tick instead of once. + if o.LivenessRedial && !zombieRedialTried { + zombieRedialTried = true + // consumeHold=false: this streak is not the disconnect hold + // was armed for (the interface never went down), so a + // standing hold suppresses this attempt without being spent + // by it — see maybeAutoWindow's doc comment. + maybeAutoWindow(time.Now(), "tunnel reports up but appears to be hung (liveness redial)", false) + } + } + } else { + // A stale refusal earned by a zombie streak that just resolved + // (a lookup succeeded, or posture moved to FULL BLOCK) must not + // survive it. Gated on dg.zombie != nil directly, not on tunnelUp: + // tunnelUp is NOT guaranteed true here (this branch is also + // reached with the tunnel genuinely down, when blocked == true + // skipped the down-tunnel `continue` above, or when o.Watcher is + // nil and tunnelUp never updates) — but dg.zombie is only ever + // set inside the zombie-streak branch above, so checking it + // directly still limits this to a liveness-redial refusal, + // never an ordinary drop's. Mirrors the cleanup the real + // tunnel-up edge already does for that case. + // zombieEpisodeOpen(), not dg.zombie: a window that opened and + // closed mid-streak cleared the observation but not the episode, + // and the refusal has to be dropped by whichever tick actually + // ends the episode — this one. Still limited to a liveness + // refusal, never an ordinary drop's: the only way to reach this + // with an episode open is tunnelUp, and a tunnel going down + // clears both flags at the watcher's own down edge before any + // ordinary-drop refusal can be recorded. + if zombieEpisodeOpen() && redialRefused != nil { + redialRefused = nil + disarmRedialRetry() + o.Log.Info("standing redial refusal dropped — the zombie streak it was refused for is over") + } + resetZombie(true) } // End the accelerated episode once it has done its job, or once its // budget is spent. Recovery is the success case; the budget is what @@ -2415,14 +3165,24 @@ func sameStrings(a, b []string) bool { // firewall-action failure (a failed FULL BLOCK / guard restore, or a probe re-cut // that left egress open), or nil when the intended posture was achieved. // +// The third return, attempted, is true only when this call actually touched the +// backend (a probe while blocked, or a Block/Allow transition's Apply) — i.e. +// only when the second return is a meaningful, fresh answer to "did enforcement +// just succeed". A steady no-op reading (already in the right posture, nothing +// to Apply) leaves it false so the caller does not overwrite a still-relevant +// enforcement error from an unrelated source (e.g. a failed verification +// repair) with this tick's uninformative nil. +// // tunnelUp only classifies how a FAILED lookup is reported — it never changes // enforcement. With no tunnel there is no exit to measure, so a failure is // expected rather than a fault. -func (o Options) vpnGeoStep(ctx context.Context, guard, fullBlock firewall.Policy, blocked *bool, tunnelUp bool) (monitor.Result, error) { +func (o Options) vpnGeoStep(ctx context.Context, guard, fullBlock firewall.Policy, blocked *bool, tunnelUp bool) (monitor.Result, error, bool) { var res monitor.Result var enfErr error + attempted := false if *blocked { res, enfErr = o.probe(ctx, guard, fullBlock) + attempted = true } else { r, err := o.Monitor.Once(ctx) res = monitor.Result{Reading: r, Err: err} @@ -2442,13 +3202,14 @@ func (o Options) vpnGeoStep(ctx context.Context, guard, fullBlock firewall.Polic // physical leaks, so an unknown must not escalate GUARD→FULL BLOCK (which // cuts tunnel egress and livelocks the redial) nor lift an active FULL // BLOCK on a blip. Only a *successful* reading moves the state machine. - return res, enfErr + return res, enfErr, attempted } cc := res.Reading.CountryCode switch o.Decider.Evaluate(res) { case decision.Block: if !*blocked { + attempted = true if err := o.Backend.Apply(fullBlock); err != nil { o.Log.Error("full block failed", "err", err, "country", cc) enfErr = err @@ -2462,6 +3223,7 @@ func (o Options) vpnGeoStep(ctx context.Context, guard, fullBlock firewall.Polic // re-cut failure it reported. case decision.Allow: if *blocked { + attempted = true if err := o.Backend.Apply(guard); err != nil { o.Log.Error("guard restore failed", "err", err, "country", cc) enfErr = err @@ -2472,7 +3234,7 @@ func (o Options) vpnGeoStep(ctx context.Context, guard, fullBlock firewall.Polic } } } - return res, enfErr + return res, enfErr, attempted } // probe is the VPN recovery probe: observe the exit country while in FULL BLOCK, diff --git a/internal/runner/runner_test.go b/internal/runner/runner_test.go index a7e5fc1..2f9ef17 100644 --- a/internal/runner/runner_test.go +++ b/internal/runner/runner_test.go @@ -68,6 +68,10 @@ type fakeBackend struct { policies []firewall.Policy blockErr error applyErr error + // isBlockedFn drives enforcement verification. nil answers "the rules are + // present" — the healthy reply — so every test that does not care about + // verification is unaffected by its existence. + isBlockedFn func() (bool, error) } func (b *fakeBackend) Apply(p firewall.Policy) error { @@ -94,6 +98,13 @@ func (b *fakeBackend) Cleanup() error { b.calls = append(b.calls, "cleanup") return nil } +func (b *fakeBackend) IsBlocked() (bool, error) { + b.calls = append(b.calls, "is-blocked") + if b.isBlockedFn == nil { + return true, nil + } + return b.isBlockedFn() +} func reading(cc string) monitor.Result { return monitor.Result{Reading: monitor.Reading{CountryCode: cc}} @@ -368,6 +379,7 @@ type failingGuardBackend struct { func (b *failingGuardBackend) Apply(p firewall.Policy) error { return errors.New("guard apply failed") } func (b *failingGuardBackend) Block(a firewall.Allowlist) error { return nil } func (b *failingGuardBackend) Unblock() error { return nil } +func (b *failingGuardBackend) IsBlocked() (bool, error) { return true, nil } func (b *failingGuardBackend) Cleanup() error { b.cleanups++; return nil } // --- tunnel watcher --- @@ -401,8 +413,9 @@ func (b *signalBackend) Block(a firewall.Allowlist) error { } return nil } -func (b *signalBackend) Unblock() error { b.record("unblock"); return nil } -func (b *signalBackend) Cleanup() error { b.record("cleanup"); return nil } +func (b *signalBackend) Unblock() error { b.record("unblock"); return nil } +func (b *signalBackend) Cleanup() error { b.record("cleanup"); return nil } +func (b *signalBackend) IsBlocked() (bool, error) { return true, nil } func (b *signalBackend) has(call string) bool { b.mu.Lock() defer b.mu.Unlock() @@ -1595,7 +1608,7 @@ func TestLookupFailureClassification(t *testing.T) { t.Run(c.name, func(t *testing.T) { var got state.Snapshot o := Options{Publish: func(s state.Snapshot) { got = s }} - o.publish(false, false, monitor.Reading{}, errors.New("all providers failed"), nil, c.tunnels, nil, nil, "", nil, nil, nil) + o.publish(false, false, monitor.Reading{}, errors.New("all providers failed"), nil, c.tunnels, nil, nil, "", nil, nil, nil, diag{}) if hasErr := got.LookupErr != ""; hasErr != c.wantLookupErr { t.Errorf("LookupErr set = %v, want %v (got %q)", hasErr, c.wantLookupErr, got.LookupErr) @@ -1617,7 +1630,7 @@ func TestSuccessfulLookupSetsNoErrorFields(t *testing.T) { var got state.Snapshot o := Options{Publish: func(s state.Snapshot) { got = s }} o.publish(false, false, monitor.Reading{CountryCode: "NL"}, nil, nil, - []state.Tunnel{{Name: "utun4", Up: true}}, nil, nil, "", nil, nil, nil) + []state.Tunnel{{Name: "utun4", Up: true}}, nil, nil, "", nil, nil, nil, diag{}) if got.LookupErr != "" || got.ExitUnknown != "" { t.Errorf("a successful lookup set LookupErr=%q ExitUnknown=%q, want both empty", got.LookupErr, got.ExitUnknown) } @@ -1698,6 +1711,7 @@ func (b *firstWindowFailsBackend) Apply(p firewall.Policy) error { } func (b *firstWindowFailsBackend) Block(a firewall.Allowlist) error { return nil } func (b *firstWindowFailsBackend) Unblock() error { return nil } +func (b *firstWindowFailsBackend) IsBlocked() (bool, error) { return true, nil } func (b *firstWindowFailsBackend) Cleanup() error { return nil } func (b *firstWindowFailsBackend) seen() []string { b.mu.Lock() diff --git a/internal/runner/verify_test.go b/internal/runner/verify_test.go new file mode 100644 index 0000000..bc37ee1 --- /dev/null +++ b/internal/runner/verify_test.go @@ -0,0 +1,345 @@ +package runner + +import ( + "context" + "errors" + "net/netip" + "testing" + "time" + + "github.com/behnam-rk/dezhban/internal/command" + "github.com/behnam-rk/dezhban/internal/decision" + "github.com/behnam-rk/dezhban/internal/state" +) + +// Every other Apply in the run loop is triggered by something dezhban itself +// did. Enforcement verification is the one path that notices a ruleset removed +// from OUTSIDE the daemon and puts it back — these tests pin that behaviour +// directly, plus the two ways it must NOT act: an unreadable backend, and the +// key turned off. + +// A missing ruleset must be re-applied, and the repair must show up in the +// published snapshot so an observer can see it happened. +func TestVerifyTickRepairsMissingRules(t *testing.T) { + var calls int + be := &fakeBackend{isBlockedFn: func() (bool, error) { + calls++ + return calls > 1, nil // first check: missing; every check after: present + }} + + var snaps []state.Snapshot + ctx, cancel := context.WithTimeout(context.Background(), 150*time.Millisecond) + defer cancel() + o := Options{ + Monitor: steadyMonitor{cc: "US"}, // allowed exit: guard holds steady throughout + Decider: decision.New([]string{"IR"}, 1), + Backend: be, + Log: discardLog(), + Interval: 50 * time.Millisecond, + Tunnels: []string{"utun4"}, + Endpoints: []netip.Addr{netip.MustParseAddr("203.0.113.7")}, + VerifyInterval: 10 * time.Millisecond, + Publish: func(s state.Snapshot) { snaps = append(snaps, s) }, + } + if err := Run(ctx, o); err != nil { + t.Fatal(err) + } + + if calls < 2 { + t.Fatalf("IsBlocked called %d times, want at least 2 (one missing, one clean)", calls) + } + + guards := 0 + for _, c := range be.calls { + if c == "apply-guard" { + guards++ + } + } + if guards < 2 { + t.Errorf("apply-guard count = %d, want at least 2 (startup + repair); calls = %v", guards, be.calls) + } + + var sawMissing, sawClearedAfter bool + for _, s := range snaps { + if s.Verify != nil && s.Verify.Missing { + sawMissing = true + continue + } + if sawMissing && s.Verify == nil { + sawClearedAfter = true + } + } + if !sawMissing { + t.Error("no published snapshot reported the missing ruleset") + } + if !sawClearedAfter { + t.Error("Verify was never cleared by a later clean check") + } +} + +// A repair attempt whose Backend.Apply itself fails must not be counted or +// reported as a completed repair — Repairs must stay put and the failure +// must surface through EnforcementErr instead, or an observer reading +// "repairs=N" would believe the host is enforcing again when it is not. +func TestVerifyTickDoesNotCountAFailedRepair(t *testing.T) { + var calls int + be := &fakeBackend{} + be.isBlockedFn = func() (bool, error) { + calls++ + if calls == 1 { + return true, nil // first check: present, matching the successful startup apply + } + // From here on the rules are missing, and the repair Apply that + // verification is about to trigger fails too. + be.applyErr = errors.New("apply boom") + return false, nil + } + + var snaps []state.Snapshot + ctx, cancel := context.WithTimeout(context.Background(), 150*time.Millisecond) + defer cancel() + o := Options{ + Monitor: steadyMonitor{cc: "US"}, + Decider: decision.New([]string{"IR"}, 1), + Backend: be, + Log: discardLog(), + Interval: 50 * time.Millisecond, + Tunnels: []string{"utun4"}, + Endpoints: []netip.Addr{netip.MustParseAddr("203.0.113.7")}, + VerifyInterval: 10 * time.Millisecond, + Publish: func(s state.Snapshot) { snaps = append(snaps, s) }, + } + if err := Run(ctx, o); err != nil { + t.Fatal(err) + } + + if calls < 2 { + t.Fatalf("IsBlocked called %d times, want at least 2 (one present, one missing)", calls) + } + + var sawFailedRepair, sawEnforcementErr bool + for _, s := range snaps { + if s.Verify == nil || !s.Verify.Missing { + continue + } + sawFailedRepair = true + if s.Verify.Repairs != 0 { + t.Errorf("Repairs = %d on a failed repair attempt, want 0", s.Verify.Repairs) + } + // enfErr is a single last-attempt variable shared with the regular + // poll tick's own vpnGeoStep call, which resets it to nil on a + // no-op steady reading — a pre-existing quirk unrelated to what + // this test pins, so only require it show up on SOME snapshot + // rather than every one. + if s.EnforcementErr != "" { + sawEnforcementErr = true + } + } + if !sawEnforcementErr { + t.Error("no snapshot with a failed repair ever surfaced EnforcementErr") + } + if !sawFailedRepair { + t.Fatal("no published snapshot reported the missing ruleset") + } +} + +// Enforcement verification finding the rules gone must re-apply even an +// UNRESTRICTED switch window's policy — the one case reapplyWindow's own +// ordinary reason (a tunnel/endpoint change) would skip, since an unrestricted +// window already passes everything and no such change ever needs to touch it. +// Verification's reason is different: the pass itself vanished along with the +// rest of the ruleset, so reapplyCurrent's force path has to reach it anyway, +// or the host would sit open behind a window while the daemon logged a repair. +func TestVerifyTickRepairsAnOpenUnrestrictedWindow(t *testing.T) { + var calls int + be := &fakeBackend{isBlockedFn: func() (bool, error) { + calls++ + return calls > 1, nil // first check: missing; every check after: present + }} + + ctx, cancel := context.WithTimeout(context.Background(), 300*time.Millisecond) + defer cancel() + o := Options{ + Monitor: steadyMonitor{cc: "US"}, + Decider: decision.New([]string{"IR"}, 1), + Backend: be, + Log: discardLog(), + Interval: time.Hour, // no geo ticks needed; the window stays open throughout + Tunnels: []string{"utun4"}, + Endpoints: []netip.Addr{netip.MustParseAddr("203.0.113.7")}, + SwitchWindow: 5 * time.Second, // outlives the test; no WindowProtocols/Ports set → unrestricted + SwitchWindowMax: time.Minute, + CommandPoll: 5 * time.Millisecond, + PollCommand: scriptedCommands(command.Command{Op: command.OpOpenSwitchWindow}), + VerifyInterval: 10 * time.Millisecond, + } + if err := Run(ctx, o); err != nil { + t.Fatal(err) + } + + if calls < 2 { + t.Fatalf("IsBlocked called %d times, want at least 2 (one missing, one clean)", calls) + } + + var switches int + for _, c := range be.calls { + if c == "apply-switch" { + switches++ + } + } + if switches < 2 { + t.Fatalf("apply-switch count = %d, want at least 2 (the initial open, plus verification's repair "+ + "of the unrestricted window's vanished pass); calls = %v", switches, be.calls) + } +} + +// Enforcement verification finding the rules gone while FULL BLOCK is the +// standing posture must re-apply the full block, not fall through to guard — +// reapplyCurrent's `case blocked:` branch, the one shape TestVerifyTickRepairsMissingRules +// (guard) and TestVerifyTickRepairsAnOpenUnrestrictedWindow (an open window) +// don't exercise. Missing this branch would mean a rules-removed-from-outside +// gap silently downgrades a forbidden-country block to an ordinary guard on +// its very next repair — the one posture where that matters most. +func TestVerifyTickRepairsFullBlock(t *testing.T) { + var calls int + be := &fakeBackend{isBlockedFn: func() (bool, error) { + calls++ + return calls > 1, nil // first check: missing; every check after: present + }} + + var snaps []state.Snapshot + ctx, cancel := context.WithTimeout(context.Background(), 150*time.Millisecond) + defer cancel() + o := Options{ + Monitor: steadyMonitor{cc: "IR"}, // forbidden exit → FULL BLOCK at startup + Decider: decision.New([]string{"IR"}, 1), + Backend: be, + Log: discardLog(), + Interval: time.Hour, // no further geo ticks needed; FULL BLOCK holds on its own + Tunnels: []string{"utun4"}, + Endpoints: []netip.Addr{netip.MustParseAddr("203.0.113.7")}, + VerifyInterval: 10 * time.Millisecond, + Publish: func(s state.Snapshot) { snaps = append(snaps, s) }, + } + if err := Run(ctx, o); err != nil { + t.Fatal(err) + } + + if calls < 2 { + t.Fatalf("IsBlocked called %d times, want at least 2 (one missing, one clean)", calls) + } + + // The very first call is the startup posture arming with guard before the + // first geo reading lands — expected, and not a repair. Only a *later* + // apply-guard, once verification (is-blocked) has started ticking, would + // mean a repair mistakenly downgraded FULL BLOCK to guard. + var verifying bool + fullBlocks := 0 + for _, c := range be.calls { + switch c { + case "is-blocked": + verifying = true + case "apply-fullblock": + fullBlocks++ + case "apply-guard": + if verifying { + t.Fatalf("verification repaired FULL BLOCK by installing guard instead; calls = %v", be.calls) + } + } + } + if fullBlocks < 2 { + t.Errorf("apply-fullblock count = %d, want at least 2 (startup + repair); calls = %v", fullBlocks, be.calls) + } + + var sawMissing, sawClearedAfter bool + for _, s := range snaps { + if s.Verify != nil && s.Verify.Missing { + sawMissing = true + continue + } + if sawMissing && s.Verify == nil { + sawClearedAfter = true + } + } + if !sawMissing { + t.Error("no published snapshot reported the missing ruleset") + } + if !sawClearedAfter { + t.Error("Verify was never cleared by a later clean check") + } +} + +// An unreadable backend is not evidence the rules are gone — the daemon must +// report it and change nothing, the same discipline as an undeterminable exit +// country holding the current posture. +func TestVerifyTickHoldsOnReadError(t *testing.T) { + readErr := errors.New("pfctl: no such process") + be := &fakeBackend{isBlockedFn: func() (bool, error) { return false, readErr }} + + var snaps []state.Snapshot + ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) + defer cancel() + o := Options{ + Monitor: steadyMonitor{cc: "US"}, + Decider: decision.New([]string{"IR"}, 1), + Backend: be, + Log: discardLog(), + Interval: 50 * time.Millisecond, + Tunnels: []string{"utun4"}, + Endpoints: []netip.Addr{netip.MustParseAddr("203.0.113.7")}, + VerifyInterval: 10 * time.Millisecond, + Publish: func(s state.Snapshot) { snaps = append(snaps, s) }, + } + if err := Run(ctx, o); err != nil { + t.Fatal(err) + } + + guards := 0 + for _, c := range be.calls { + if c == "apply-guard" { + guards++ + } + } + if guards != 1 { + t.Errorf("apply-guard count = %d, want exactly 1 (startup only) — a read error must never trigger a repair; calls = %v", guards, be.calls) + } + + var sawErr bool + for _, s := range snaps { + if s.Verify != nil && s.Verify.Err != "" { + sawErr = true + if s.Verify.Missing { + t.Error("a read error must not also be reported as Missing") + } + } + } + if !sawErr { + t.Error("no published snapshot reported the read error") + } +} + +// vpn.advanced.verifyInterval: "0" must actually turn verification off, not +// merely slow it down — the same "0 is an explicit opt-out" discipline as the +// three relaxation windows. +func TestVerifyIntervalDisabledNeverChecks(t *testing.T) { + be := &fakeBackend{isBlockedFn: func() (bool, error) { + t.Fatal("IsBlocked called with verification disabled") + return true, nil + }} + + ctx, cancel := context.WithTimeout(context.Background(), 60*time.Millisecond) + defer cancel() + o := Options{ + Monitor: steadyMonitor{cc: "US"}, + Decider: decision.New([]string{"IR"}, 1), + Backend: be, + Log: discardLog(), + Interval: 50 * time.Millisecond, + Tunnels: []string{"utun4"}, + Endpoints: []netip.Addr{netip.MustParseAddr("203.0.113.7")}, + VerifyInterval: -1, // the Disabled sentinel, however the caller spells it + } + if err := Run(ctx, o); err != nil { + t.Fatal(err) + } +} diff --git a/internal/state/state.go b/internal/state/state.go index 1fc2d62..beb4468 100644 --- a/internal/state/state.go +++ b/internal/state/state.go @@ -15,6 +15,8 @@ import ( "os" "path/filepath" "time" + + "github.com/behnam-rk/dezhban/internal/atomicfile" ) // Tunnel is one VPN tunnel interface's observed state (VPN mode only). @@ -67,9 +69,17 @@ type Snapshot struct { // failure (see runner.publishStopped and docs/contribute/architecture.md); a clean, operator-requested // stop leaves it empty. Either way the contract holds: the intended posture (enforcing) // was not achieved. - EnforcementErr string `json:"enforcementErr,omitempty"` - Tunnels []Tunnel `json:"tunnels,omitempty"` // VPN mode - Endpoints []string `json:"endpoints,omitempty"` // resolved VPN endpoints (VPN mode) + EnforcementErr string `json:"enforcementErr,omitempty"` + // PanicDisarmed reports that `dezhban panic` tore the firewall rules down + // and this daemon is standing down rather than silently reinstating them + // (see runner.Options.PanicDisarmed). Distinct from EnforcementErr: this is + // not a failed Apply, it is every automatic Apply being deliberately + // skipped, so Posture/Blocked can describe an intended posture (e.g. + // "full-block") the data plane is not actually enforcing at all. Cleared + // once the marker is gone (`dezhban unblock` or a fresh daemon start). + PanicDisarmed bool `json:"panicDisarmed,omitempty"` + Tunnels []Tunnel `json:"tunnels,omitempty"` // VPN mode + Endpoints []string `json:"endpoints,omitempty"` // resolved VPN endpoints (VPN mode) // PollIntervalSeconds is the daemon's poll cadence, so a reader can size its own // staleness threshold off the actual interval instead of hardcoding one. 0 when unknown. PollIntervalSeconds int `json:"pollIntervalSeconds,omitempty"` @@ -99,6 +109,31 @@ type Snapshot struct { // while such a refusal stands. Additive field: absent from older snapshots, // so nil means "nothing refused", never "no budget exists". Redial *RedialState `json:"redial,omitempty"` + // Verify reports that enforcement verification found something wrong: the + // rules dezhban believes it installed are missing, or the backend could not + // be read at all. Present only while such a condition stands, and cleared by + // the next clean check. Additive field: absent from older snapshots, so nil + // means "nothing wrong is being reported", never "no verification happens". + // + // Distinct from EnforcementErr, which means the daemon TRIED to enforce and + // the backend rejected it. This one means enforcement previously SUCCEEDED + // and the rules are gone now — the silent-failure case that had no signal + // at all before. + Verify *VerifyState `json:"verify,omitempty"` + // Zombie reports a hung tunnel: interface up, exit lookups through it + // failing. Present only while such a streak stands. Additive field, like + // Verify: absent from older snapshots, so nil means "nothing wrong is being + // reported", never "no tunnel is being watched". + Zombie *ZombieState `json:"zombie,omitempty"` + // ExitIPChangedAt is when the observed exit IP last differed from the + // previous successful reading — purely observational, like CVG's + // equivalent check: it never flips posture and never touches the + // hysteresis streak (CountryCode/Pending already own that). It is the + // signal that best explains "my exit country flapped" — a failover between + // two VPN servers in the same allowed country changes nothing CountryCode + // reports, but changes this. omitzero: zero means no change has been + // observed yet, not "the exit has never had an IP". + ExitIPChangedAt time.Time `json:"exitIpChangedAt,omitzero"` // Display is the rendered posture sentence — see internal/render, the // package that composes it from this same Snapshot. Carried here for the // one consumer that cannot call Go directly: the macOS menubar app reads @@ -187,6 +222,62 @@ type DropRecord struct { At time.Time `json:"at,omitzero"` } +// VerifyState is what enforcement verification found the last time it did not +// like the answer. It exists because every other Apply the daemon makes is +// triggered by something the daemon itself did, so a ruleset removed from +// OUTSIDE — another firewall tool, `pfctl -F all`, `nft flush ruleset`, an OS +// ruleset reload — used to go entirely unnoticed. The daemon kept reporting its +// posture, `status` kept reporting blocked, and the host was open. +// +// Present only while something is wrong; a clean check clears it. Publishing it +// only on failure is deliberate: a field that says "verified OK" on every +// snapshot is noise, and its absence must not be readable as "never checked" — +// that is what the configured interval is for. +type VerifyState struct { + // At is when the failing check ran. + At time.Time `json:"at,omitzero"` + // Missing is true when the backend answered and said the rules are gone. + // This is the actionable case: the daemon re-applies immediately. + Missing bool `json:"missing,omitempty"` + // Err carries the reason this check is unhappy, and its meaning depends on + // Missing: + // + // - Err WITHOUT Missing: the backend could not be READ at all. Not the + // same as absence — an unreadable backend is not evidence the rules are + // gone — so the daemon changes nothing and only reports, the same + // discipline as an undeterminable exit country holding the current + // posture. + // - Err WITH Missing: the rules were confirmed gone AND the re-apply that + // was supposed to put them back failed. The host is unenforced right + // now. Readers must not word this as "found missing and re-applied"; + // Repairs has deliberately NOT been incremented for it. + Err string `json:"err,omitempty"` + // Repairs counts how many times verification has re-applied the posture + // since the daemon started. A number that keeps climbing means something on + // this host is repeatedly removing dezhban's rules, which is worth seeing. + Repairs int `json:"repairs,omitempty"` +} + +// ZombieState reports a tunnel interface that reports up while a run of exit +// lookups through it has failed — the interface object still looks fine, but +// nothing is getting through it. dezhban's posture never escalates on a lookup +// failure alone (an unknown country holds, never flips — see decision.Evaluate), +// so without this a hung tunnel stayed correctly cut but explained itself to +// no one and recovered only if a person noticed and intervened. +// +// This is diagnosis, not a leak: the guard is holding exactly as designed. +// Present only while a streak stands; cleared the moment a lookup succeeds, the +// tunnel reports down, or anything else ends the streak's eligibility (standby, +// a switch window, a manual block). Additive field, like Verify: absent from +// older snapshots, so nil means "nothing wrong is being reported". +type ZombieState struct { + // Since is when the failing streak started. + Since time.Time `json:"since,omitzero"` + // Checks is how many consecutive geo lookups have failed through this + // otherwise-up tunnel. + Checks int `json:"checks"` +} + // HoldState reports that "hold the line" is armed: the next tunnel drop will // NOT open an automatic redial window, so a deliberate disconnect stays cut. // @@ -323,36 +414,9 @@ func Write(path string, s Snapshot) error { if err != nil { return fmt.Errorf("state: marshal: %w", err) } - - tmp, err := os.CreateTemp(dir, ".state-*.json.tmp") - if err != nil { - return fmt.Errorf("state: create temp: %w", err) - } - tmpName := tmp.Name() - // Best-effort cleanup if we bail before the rename. - defer func() { _ = os.Remove(tmpName) }() - - if _, err := tmp.Write(data); err != nil { - _ = tmp.Close() - return fmt.Errorf("state: write temp: %w", err) - } - // fsync before rename so a crash/power-loss right after the rename can't leave a - // truncated snapshot behind (same guarantee internal/firewall/pf_darwin.go's - // atomicWrite provides for its state files; kept as a separate impl because that - // helper is darwin build-tagged). - if err := tmp.Sync(); err != nil { - _ = tmp.Close() - return fmt.Errorf("state: sync temp: %w", err) - } - if err := tmp.Close(); err != nil { - return fmt.Errorf("state: close temp: %w", err) - } - // CreateTemp makes the file 0600; the reader is the unprivileged user. - if err := os.Chmod(tmpName, 0o644); err != nil { - return fmt.Errorf("state: chmod temp: %w", err) - } - if err := os.Rename(tmpName, path); err != nil { - return fmt.Errorf("state: rename into place: %w", err) + // 0644, not the temp file's default 0600: the reader is the unprivileged user. + if err := atomicfile.Write(path, data, 0o644); err != nil { + return fmt.Errorf("state: write %q: %w", path, err) } return nil } diff --git a/internal/token/token.go b/internal/token/token.go index 2e1b428..ba77a64 100644 --- a/internal/token/token.go +++ b/internal/token/token.go @@ -27,8 +27,9 @@ import ( "fmt" "io/fs" "os" - "path/filepath" "strings" + + "github.com/behnam-rk/dezhban/internal/atomicfile" ) // FileMode is the hash file's permission: readable and writable by root only. @@ -68,36 +69,13 @@ func Save(path, tok string) error { if strings.TrimSpace(tok) == "" { return errors.New("refusing to enroll an empty control token") } - dir := filepath.Dir(path) - tmp, err := os.CreateTemp(dir, ".control-token-*") - if err != nil { - return fmt.Errorf("stage control token: %w", err) - } - tmpName := tmp.Name() - defer func() { _ = os.Remove(tmpName) }() // no-op once the rename succeeds - - if err := tmp.Chmod(FileMode); err != nil { - _ = tmp.Close() - return fmt.Errorf("secure control token: %w", err) - } - if _, err := tmp.WriteString(hashOf(tok) + "\n"); err != nil { - _ = tmp.Close() - return fmt.Errorf("write control token: %w", err) - } // Flushed before the rename: an unsynced rename can publish a zero-length hash // after a power loss, which Verify reads as "not enrolled" — locking out a // token the user still holds. Same convention as internal/learned and // internal/armed. - if err := tmp.Sync(); err != nil { - _ = tmp.Close() - return fmt.Errorf("flush control token: %w", err) - } - if err := tmp.Close(); err != nil { + if err := atomicfile.Write(path, []byte(hashOf(tok)+"\n"), FileMode); err != nil { return fmt.Errorf("write control token: %w", err) } - if err := os.Rename(tmpName, path); err != nil { - return fmt.Errorf("install control token: %w", err) - } return nil }