diff --git a/CHANGELOG.md b/CHANGELOG.md index d14c431..2428e9f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,66 @@ 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. +- **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. + +### 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. + ## [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/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..6fa853a --- /dev/null +++ b/cmd/dezhban/lock_windows.go @@ -0,0 +1,51 @@ +//go:build windows + +package main + +import ( + "fmt" + "hash/fnv" + "syscall" + "unsafe" +) + +var ( + modkernel32 = syscall.NewLazyDLL("kernel32.dll") + procCreateMutex = modkernel32.NewProc("CreateMutexW") +) + +// 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 (the state directory), not fixed, so two +// dezhban instances pointed at two different state directories — via +// $DEZHBAN_CONFIG or --config — don't contend with each other, matching the +// Unix implementation's per-directory scoping. Global\, not a session-local +// name: `run` already requires an elevated/admin context (requireRoot), which +// can create Global objects without SeCreateGlobalPrivilege, and 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)) + name, err := syscall.UTF16PtrFromString(fmt.Sprintf(`Global\dezhban-run-%x`, h.Sum64())) + 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..c592511 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`. + if err := clearPanicMarker(stateDir()); err != nil { + 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) @@ -598,6 +643,26 @@ 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 — it never blocks or delays startup, and every enforcement + // decision downstream is fail-closed regardless of what this reports. + // Deliberately checks nothing an eager resolve would cost real work for + // (endpoint hostnames are left to the run loop's own first resolve moments + // later): "endpoints known" here means configured or auto-discoverable, + // not resolved, so this line adds one cheap backend read and nothing else. + backendReachable := true + if _, err := fw.IsBlocked(); err != nil { + backendReachable = false + } + log.Info("startup self-test", + "firewallBackendReachable", backendReachable, + "stateDirWritable", stateDirErr == nil, + "tunnelsConfiguredOrDetected", len(tunnels) > 0, + "endpointsKnown", len(cfg.VPN.Endpoints) > 0 || cfg.VPN.AutoDiscoverEndpoints, + "tunnelEverUpOnThisHost", armedRec.TunnelEverUp, + ) + return runner.Options{ Monitor: mon, Decider: decision.New(cfg.BlockedCountries, cfg.Hysteresis), @@ -624,6 +689,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 +768,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, @@ -1051,6 +1122,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. + if err := clearPanicMarker(stateDir()); err != nil { + fmt.Fprintln(os.Stderr, "unblock: warning — could not clear the panic-disarm marker:", err) + } fmt.Println("dezhban: network unblocked") return 0 } @@ -1078,6 +1156,17 @@ func cmdPanic(args []string) int { fmt.Fprintln(os.Stderr, "panic: teardown reported an error (rules may persist):", 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. Best-effort: a failure + // to write the marker must never fail `panic` itself, since the teardown + // above 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) + } fmt.Println("dezhban: panic teardown complete — all dezhban rules removed, connectivity restored") return 0 } @@ -1875,6 +1964,53 @@ func buildArmAtBootCheck(armAtBoot bool, haveTunnel bool, rec *armed.Record, loa return c } +// buildLivenessCheck reports the two enforcement-diagnostic conditions the run +// loop tracks between polls but doctor cannot recompute on its own — a missing +// ruleset (Verify) and a tunnel that reports up but is not passing traffic +// (Zombie) — from the running daemon's own last-published snapshot. Pure: takes +// the snapshot and liveness already resolved by the caller, same shape as +// buildServiceCheck. +// +// Both are read-only diagnoses, not lockout risks — neither 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 + } + if snap.Verify == nil && snap.Zombie == nil { + return c + } + c.Status = checkWarn + var lines []string + if snap.Verify != nil { + // Missing and Err are mutually exclusive by construction (state.VerifyState's + // own doc comment) — the run loop sets exactly one per failed check — so + // checking Missing first and falling through to Err is exhaustive, not a + // default-case guess. + switch { + 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)) + } + } + 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)) + } + c.Summary = "enforcement is holding, but something needs attention." + 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 +2160,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 +2247,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 +2257,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 +2328,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..3884764 --- /dev/null +++ b/cmd/dezhban/panicmark.go @@ -0,0 +1,60 @@ +package main + +import ( + "os" + "path/filepath" + "time" +) + +// 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. +func setPanicMarker(dir string) error { + body := []byte("panic ran at " + time.Now().UTC().Format(time.RFC3339) + "\n") + if err := os.WriteFile(panicMarkerPath(dir), body, 0o600); err != nil { + return err + } + // WriteFile's mode only applies to a NEWLY-created file; enforce 0600 + // unconditionally in case the marker somehow already existed looser. + return os.Chmod(panicMarkerPath(dir), 0o600) +} + +// 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 +} + +// panicMarkerPresent reports whether panic's marker is currently set. +func panicMarkerPresent(dir string) bool { + _, err := os.Stat(panicMarkerPath(dir)) + return err == nil +} 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..26ba464 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 one or two PowerShell invocations (a second one only while the rule group is present, to cross-check the profile default hasn't drifted), 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..f7becdd 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 +itself 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..cbbcb4c 100644 --- a/gui/macos/Sources/DezhbanCore/Snapshot.swift +++ b/gui/macos/Sources/DezhbanCore/Snapshot.swift @@ -116,6 +116,37 @@ 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 two different problems: `missing` means the +/// backend answered and the rules were actually gone (already re-applied by +/// the time this is read); `err` 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 { @@ -144,6 +175,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/config/config.go b/internal/config/config.go index 2a97e91..67806ce 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -223,6 +223,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 +438,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 +786,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 +963,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 +977,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 } @@ -1088,6 +1156,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 +1206,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..3a99cd7 100644 --- a/internal/firewall/nft_linux.go +++ b/internal/firewall/nft_linux.go @@ -79,10 +79,22 @@ 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. +// 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) { - if _, err := nft("", "list", "table", "inet", tableName); err != nil { + 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") || @@ -91,7 +103,7 @@ func (b *nftBackend) IsBlocked() (bool, error) { } return false, err } - return true, nil + return strings.Contains(out, "policy drop"), nil } // Cleanup is best-effort teardown for shutdown/panic. It is just Unblock; any diff --git a/internal/firewall/pf_darwin.go b/internal/firewall/pf_darwin.go index 2366b47..536e98b 100644 --- a/internal/firewall/pf_darwin.go +++ b/internal/firewall/pf_darwin.go @@ -178,7 +178,24 @@ func (b *pfBackend) IsBlocked() (bool, error) { 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 := pfctl("", "-s", "rules") + if err != nil { + return false, err + } + return strings.Contains(main, anchorRef), nil } // Cleanup is best-effort teardown for shutdown/panic. It is just Unblock; any diff --git a/internal/firewall/wfp_windows.go b/internal/firewall/wfp_windows.go index 68f5eed..17f9b6b 100644 --- a/internal/firewall/wfp_windows.go +++ b/internal/firewall/wfp_windows.go @@ -43,6 +43,44 @@ 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 { + dir := stateDir() + tmp, err := os.CreateTemp(dir, ".fw.applied-*.tmp") + if err != nil { + return err + } + tmpName := tmp.Name() + defer os.Remove(tmpName) // no-op once the rename below succeeds + if _, err := tmp.WriteString(action); 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, 0o600); err != nil { + return err + } + return os.Rename(tmpName, appliedActionPath()) +} + // 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 +132,15 @@ 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): a plain WriteFile truncates in place, so a + // crash/power-loss mid-write would leave this file present-but-empty, and + // IsBlocked's drift check (comparing against "") would then read every + // correctly-applied profile as drifted, forever re-triggering repairs + // until the next successful Apply happens to overwrite it. + _ = writeAppliedAction(expectedOutboundAction(p)) return nil } @@ -129,10 +176,28 @@ 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 + @@ -140,7 +205,29 @@ func (b *wfpBackend) IsBlocked() (bool, error) { if err != nil { return false, err } - return strings.Contains(out, "blocked"), nil + if !strings.Contains(out, "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)) + + got, err := queryOutboundDefaults() + if err != nil { + return false, err + } + for _, prof := range fwProfiles { + if got[prof] != want { + return false, nil + } + } + return true, nil } // Cleanup is best-effort teardown for shutdown/panic. It is just Unblock; any @@ -179,19 +266,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 +329,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..34c2597 100644 --- a/internal/firewall/wfp_windows_test.go +++ b/internal/firewall/wfp_windows_test.go @@ -179,6 +179,33 @@ 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 TestRenderBlockScriptZeroTunnelStandingPosture(t *testing.T) { s := renderBlockScript(Policy{ Mode: ModeFullBlock, diff --git a/internal/render/render.go b/internal/render/render.go index 235faf5..030ab95 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 ) @@ -88,8 +88,24 @@ func Text(s state.Snapshot) Display { 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 +451,41 @@ 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 { + 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..997e1a5 100644 --- a/internal/render/render_test.go +++ b/internal/render/render_test.go @@ -306,6 +306,73 @@ 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).", + }, + { + 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..8758152 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,46 @@ 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) + o.AutoArm = 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..8490fcc --- /dev/null +++ b/internal/runner/exitip_test.go @@ -0,0 +1,93 @@ +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 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..c124c8b --- /dev/null +++ b/internal/runner/liveness_test.go @@ -0,0 +1,286 @@ +package runner + +import ( + "context" + "errors" + "net/netip" + "sync" + "testing" + "time" + + "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) + } +} + +// 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) + } + } +} 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..15a7b4d 100644 --- a/internal/runner/reload_test.go +++ b/internal/runner/reload_test.go @@ -5,6 +5,7 @@ import ( "net/netip" "reflect" "slices" + "sync" "sync/atomic" "testing" "time" @@ -14,6 +15,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 +163,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 +513,195 @@ 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) + } +} + +// 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..e03ab69 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,33 @@ 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 +} + +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 +453,9 @@ 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, } if r.IP.IsValid() { snap.IP = r.IP.String() @@ -902,8 +969,79 @@ 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 + // 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 + // 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. + resetZombie := func(full bool) { + if zombieChecks == 0 && dg.zombie == nil && (!full || !zombieRedialTried) { + return + } + zombieChecks = 0 + zombieSince = time.Time{} + dg.zombie = nil + if full { + zombieRedialTried = 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 + } snapshot := func() { - o.publish(blocked, standby, lastRes.Reading, lastRes.Err, enfErr, lastTun, endpoints, switchState(), activeProfile, lastDrop, holdState(), redialState()) + 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) } @@ -933,10 +1071,14 @@ 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() { - return - } + // 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 err := o.Backend.Apply(o.windowPolicy(tunnels, endpoints)); err != nil { enfErr = err o.Log.Error("re-apply switch window failed", "reason", reason, "err", err) @@ -946,25 +1088,39 @@ 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). + reapplyCurrent := func(reason string, force bool) { 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 err := o.Backend.Apply(fullBlock); err != nil { enfErr = err @@ -978,6 +1134,20 @@ 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. + // + // 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() { if windowTimer != nil { windowTimer.Stop() @@ -1171,7 +1341,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 +1401,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 +1415,19 @@ 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 + } + o.Log.Warn("redial window suppressed (hold the line was armed); "+ "guard holds, traffic stays cut", "detail", detail) return } @@ -1281,9 +1466,15 @@ 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). + if redialRefused == nil || (tunnelUp && dg.zombie == nil) { return } // Hold the line, armed AFTER the drop by an operator watching a cut they @@ -1625,6 +1816,18 @@ 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. + if o.ClearPanicDisarm != nil { + if err := o.ClearPanicDisarm(); err != nil { + o.Log.Debug("clear panic-disarm marker failed", "err", err) + } + } 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 +1844,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, "") @@ -1802,6 +2010,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 +2122,30 @@ 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() + 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, @@ -2022,6 +2271,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 +2295,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 @@ -2201,6 +2458,78 @@ 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 != nil && 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. + if dg.verify == nil || 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: + verifyRepairs++ + // 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) + } + dg.verify = &state.VerifyState{At: time.Now(), Missing: true, 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. + reapplyCurrent("enforcement verification: rules missing", true) + 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 @@ -2256,9 +2585,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 +2601,99 @@ 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 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()) + // 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). A + // failover between two servers in the same allowed country changes + // nothing CountryCode reports, but changes this — it is the signal + // that best explains "my exit flapped". + ip := lastRes.Reading.IP + if ip.IsValid() { + if lastGoodIP.IsValid() && ip != lastGoodIP { + o.Log.Info("exit IP changed", "from", lastGoodIP, "to", ip) + dg.exitIPChangedAt = time.Now() + } + lastGoodIP = ip + } + } + // 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. + if dg.zombie != nil && 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 diff --git a/internal/runner/runner_test.go b/internal/runner/runner_test.go index a7e5fc1..7e51cf4 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 --- @@ -1595,7 +1607,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 +1629,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 +1710,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..45452c4 --- /dev/null +++ b/internal/runner/verify_test.go @@ -0,0 +1,279 @@ +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") + } +} + +// 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..3867064 100644 --- a/internal/state/state.go +++ b/internal/state/state.go @@ -99,6 +99,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 +212,54 @@ 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 is set when the backend could not be READ at all. Not the same as + // Missing: an unreadable backend is not evidence of absence, so the daemon + // changes nothing and only reports — the same discipline as an + // undeterminable exit country holding the current posture. + 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. //