diff --git a/CHANGELOG.md b/CHANGELOG.md index d14c431..d3923bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,47 @@ 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, re-applying the standing posture the + instant they are not. 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 removed from + OUTSIDE the daemon (another firewall tool, `pfctl -F all`, + `nft flush ruleset`, an OS ruleset reload). Reported in `state.verify` + (`status --json`) only while something is wrong; disablable (`"0"`), 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 — 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`), 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. + ## [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_unix.go b/cmd/dezhban/lock_unix.go new file mode 100644 index 0000000..326a1ce --- /dev/null +++ b/cmd/dezhban/lock_unix.go @@ -0,0 +1,60 @@ +//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) { + fd, err := syscall.Open(path, syscall.O_CREAT|syscall.O_RDWR, 0644) + if err != nil { + return -1, fmt.Errorf("open 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("another dezhban is already running (holds %s) — see `dezhban status`", 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..b7c0a60 --- /dev/null +++ b/cmd/dezhban/lock_unix_test.go @@ -0,0 +1,60 @@ +//go:build !windows + +package main + +import ( + "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") + } +} diff --git a/cmd/dezhban/lock_windows.go b/cmd/dezhban/lock_windows.go new file mode 100644 index 0000000..1728759 --- /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("another dezhban is already running against this state directory — see `dezhban status`") + } + return nil +} diff --git a/cmd/dezhban/lock_windows_test.go b/cmd/dezhban/lock_windows_test.go new file mode 100644 index 0000000..4358051 --- /dev/null +++ b/cmd/dezhban/lock_windows_test.go @@ -0,0 +1,38 @@ +//go:build windows + +package main + +import "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) + } + if err := acquireRunLock(dir); err == nil { + t.Fatal("second acquire on the same directory succeeded; want refusal") + } +} + +// 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..27892d1 100644 --- a/cmd/dezhban/main.go +++ b/cmd/dezhban/main.go @@ -313,6 +313,21 @@ 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. + if err := state.EnsureDir(stateDir()); err != nil { + log.Warn("state directory not reachable; the single-instance lock will still be attempted", "err", err) + } + if err := acquireRunLock(stateDir()); err != nil { + fmt.Fprintln(os.Stderr, err) + return 1 + } + // 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 @@ -381,8 +396,9 @@ func assembleOptions(cfg *config.Config, cfgPath string, log *slog.Logger, ov ru // 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 := state.EnsureDir(stateDir()) + 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 +614,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 +660,8 @@ func assembleOptions(cfg *config.Config, cfgPath string, log *slog.Logger, ov ru }, EndpointRefresh: cfg.VPN.EndpointRefresh, EndpointGrace: cfg.VPN.EndpointGrace, + VerifyInterval: adv.VerifyInterval, + LivenessRedial: adv.LivenessRedial, AutoArm: cfg.VPN.AutoArm, ArmAtBoot: armAtBoot, TunnelEverUp: armedRec.TunnelEverUp, @@ -699,6 +737,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, @@ -1875,6 +1915,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 +2111,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 +2198,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 +2208,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 +2279,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/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..9915253 100644 --- a/docs/concepts/glossary.md +++ b/docs/concepts/glossary.md @@ -196,6 +196,27 @@ 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, re-applying them the instant they are not. Every other rule change is +triggered by something dezhban 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 — 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 +248,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..78f186e 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 the standing posture the instant they are not. Every other rule change dezhban makes is triggered by something the daemon itself did — this is the only one that notices a ruleset removed from OUTSIDE it (another firewall tool, `pfctl -F all`, `nft flush ruleset`, an OS ruleset reload). `"0"` disables the check, trusting the rules to stay put once applied. On Windows each check is a PowerShell invocation, 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..5bc4b21 100644 --- a/docs/usage/troubleshooting.md +++ b/docs/usage/troubleshooting.md @@ -312,6 +312,98 @@ 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. + +## 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. 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. + ## Preview rules before applying them Never find out what a block does by getting locked out — render the exact 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/render/render.go b/internal/render/render.go index 235faf5..3d3cb86 100644 --- a/internal/render/render.go +++ b/internal/render/render.go @@ -89,6 +89,7 @@ func Text(s state.Snapshot) Display { } d := postureDisplay(s) d.Detail = joinSentences(d.Detail, lookupNote(s)) + d.Detail = joinSentences(d.Detail, zombieNote(s)) d.Detail = joinSentences(d.Detail, pendingNote(s.Pending)) return d } @@ -435,6 +436,18 @@ 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." +} + // 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/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..04078fd --- /dev/null +++ b/internal/runner/liveness_test.go @@ -0,0 +1,126 @@ +package runner + +import ( + "context" + "net/netip" + "testing" + "time" + + "github.com/behnam-rk/dezhban/internal/decision" + "github.com/behnam-rk/dezhban/internal/state" +) + +// 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. +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 == 0 { + t.Fatalf("no redial window opened with livenessRedial on; calls = %v", 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..1256497 100644 --- a/internal/runner/reload_test.go +++ b/internal/runner/reload_test.go @@ -161,6 +161,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, diff --git a/internal/runner/runner.go b/internal/runner/runner.go index 203871b..011028b 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,22 @@ 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 + // 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 +391,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 +437,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 +953,49 @@ 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 + // 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 + // 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 := func() { + if zombieChecks == 0 && dg.zombie == nil { + return + } + zombieChecks = 0 + zombieSince = time.Time{} + dg.zombie = nil + } + // 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 +1025,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 +1042,36 @@ 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 — the caller decides whether + // its reason changed what the rules should say. Verification's reason did + // not: the rules are correct, they are simply absent. + 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 +1085,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() @@ -1641,6 +1762,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() + resetVerify() o.Log.Info("STANDBY (manual unblock, vpn.autoArm) — guard released; re-arms when a VPN connects") snapshot() return reply(true, "") @@ -1802,6 +1928,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 +2040,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 +2189,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() + } if next, changed := reconcileTunnels(tunnels, st.Names, pinned); changed { tunnels = next reapplyStanding("tunnel set changed") @@ -2201,6 +2376,41 @@ 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 + } + 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. + 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++ + o.Log.Error("dezhban's firewall rules are MISSING — something removed them; re-applying now", + "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: + 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,29 +2466,81 @@ 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() // nothing enforcing, nothing to diagnose + continue // not enforcing — nothing to decide, nothing to protect a probe with } if windowActive { - continue // window suppresses the geo state machine + resetZombie() // a window is already the response to a suspected problem + continue // window suppresses the geo state machine } if manualBlock { // An operator asked for this block. Recovery must not lift it behind // 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() continue } if len(tunnels) == 0 { + resetZombie() 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() // 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} + if o.LivenessRedial { + maybeAutoWindow(time.Now(), "tunnel reports up but appears to be hung (liveness redial)") + } + } + } else { + resetZombie() } // 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..642ebfc --- /dev/null +++ b/internal/runner/verify_test.go @@ -0,0 +1,152 @@ +package runner + +import ( + "context" + "errors" + "net/netip" + "testing" + "time" + + "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") + } +} + +// 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..c47027d 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 @@ -199,6 +224,54 @@ type DropRecord struct { // grants nothing, so the three sanctioned relaxation triggers are unchanged and // there is no fourth. Being strictly more restrictive is also why it needs no // config gate of its own: there is no setting to protect. +// 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"` +} + type HoldState struct { // Armed is true from the moment it is armed until the drop it covers, an // explicit cancel, or a tunnel coming back up.