diff --git a/PLAN.md b/PLAN.md index 6fe437f..34520ea 100644 --- a/PLAN.md +++ b/PLAN.md @@ -309,6 +309,7 @@ entry `x.y` and a wildcard entry `x.y`, which together give domain-and-subdomain - Per source IP: (1) exact match in `clients`, (2) longest-prefix match in `client_prefixes` (ties: longer prefix, then priority), (3) `default` group. - Auto-materialization: first query from an unseen IP inserts a `clients` row (`hand_edited=0`, `first_seen=now`) for UI visibility and stable group assignment. The query log does **not** FK to it (§3.6). +- Materialized clients name themselves: the tracker's flush pass sends one PTR query per unnamed row through the declared forward zones (§6.5), and the answer is runtime state in `learned_name`, never configuration. - Retention drops `hand_edited=0` clients with no queries in `retention_days`. ### 7.3 Reload @@ -387,6 +388,8 @@ CREATE TABLE clients ( id INTEGER PRIMARY KEY, ip TEXT NOT NULL UNIQUE, -- canonical text form (v4 dotted / v6 RFC 5952) name TEXT, + learned_name TEXT, + name_attempt_after INTEGER NOT NULL DEFAULT 0, group_id INTEGER NOT NULL REFERENCES groups(id), hand_edited INTEGER NOT NULL DEFAULT 0, first_seen INTEGER NOT NULL, diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index 45b395e..6011352 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -251,6 +251,34 @@ Known clients with a fixed group assignment. Consumed by the filter engine's exact address-to-group lookup (`src/filter/matcher.zig`). +#### Learned names + +A client row that carries no name of its own can still show one. Every flush +pass the client tracker takes at most 16 unnamed rows that are due, builds each +address's reverse name (`192.168.1.10` becomes `10.1.168.192.in-addr.arpa`), and +matches it against the declared [`forward_zones`](#forward_zones). On a match it +sends one PTR query to that zone's resolver and stores the answer as the row's +*learned* name. A row becomes due again 24 hours after the resolver answered or +returned NXDOMAIN, and 1 hour after any other outcome — no covering zone, a +transport failure, or a reply nxdns rejected — so declaring the missing zone or +fixing the resolver shows names within the hour. This +requires a conditional forward zone that covers the LAN's reverse space — for +example `168.192.in-addr.arpa` pointed at the router. Without such a zone nxdns +sends the reverse name to nobody, and the row stays unnamed. + +The PTR query goes to the resolver of the declared zone, wherever the operator +pointed it; nxdns does not second-guess that declaration, and it cannot stop a +LAN resolver from forwarding the query onward. + +A learned name is runtime state, like `last_seen`: + +- A hand-typed `name` always wins, and a named row is never asked about again. +- Learned names never appear in `nxdns export`, and `nxdns import` never sets + one. +- Each row refreshes once a day. A device rename or a DHCP lease change can + therefore display a stale name for up to 24 hours, until the next refresh + overwrites it or the router reports no name and nxdns clears it. + ### client_prefixes Group assignment by CIDR prefix, for clients without an exact entry. @@ -395,6 +423,16 @@ name would be a bootstrap problem. Matching is longest suffix (`src/local/forward_client.zig`) with `upstream.read_timeout_ms` as the read deadline. +Reverse zones are declared the same way, and one is the prerequisite for +[learned client names](#learned-names): + +```zig +.forward_zones = .{ + .{ .zone = "lan", .resolver = "udp://192.168.1.1:53" }, + .{ .zone = "168.192.in-addr.arpa", .resolver = "udp://192.168.1.1:53" }, +}, +``` + ## Password and hash Both fields are optional, and the difference between *absent* and *empty* is the diff --git a/specs/milestone-25.md b/specs/milestone-25.md new file mode 100644 index 0000000..b613c0e --- /dev/null +++ b/specs/milestone-25.md @@ -0,0 +1,642 @@ +# Milestone 25: client names learned over reverse DNS + +Goal: the clients table names devices by itself. The tracker already +materialises every querying address into a `clients` row (PLAN §7.2, +`src/server/clients.zig`), but `name` stays NULL until the operator types one — +which defeats the point of auto-materialisation on a LAN whose router already +knows every DHCP hostname. This milestone asks the router: for each unnamed +client, nxdns builds the address's reverse name, matches it against the +operator's conditional forward zones (PLAN §6.5), sends one PTR query to the +zone's resolver, and stores the answer as a *learned* name — runtime state +beside `last_seen`, never configuration. The dashboard shows it; a hand-typed +name always wins; a reverse name no forward zone covers is never sent anywhere. + +Design written 2026-08-14 against HEAD `c428bc2`, revised after a Codex review +of the first draft (all 19 findings accepted; the review's fingerprints are +called out inline where a first-draft claim was wrong). + +## Implementation contract (read first) + +- Read `AGENTS.md`, then this spec whole, before session work starts. +- **The v1 baseline is editable and there is no migration step** (milestone-24 + ruling 1 stands unchanged). The two new columns are lines edited into + `config_schema.ddl_v1` plus the identical lines in PLAN §11.2, kept + byte-identical. A `ddl_v2` or a second `Step` is wrong and must be reverted. + A development database stamped version 1 before the edit fails its first + `SELECT` naming the columns; delete the scratch database. +- The new `src/local/reverse_name.zig` is pure: bytes in, bytes out, no + `std.Io`, no clock, no sockets. (The blanket claim "src/local/ is pure" from + the first draft was false — `forward_client.zig` does socket I/O by design; + the purity requirement here binds the *new* module and the lookup-table + modules it sits beside.) Everything that touches the network or the database + lives in `src/server/`. +- After the API shape change, regenerate + `web/src/lib/contractSamples.gen.ts` with the AGENTS.md command (never a + hand edit) and update `web/src/lib/types.ts` to match, in the same session. +- Verify stdlib claims against `../zig` at tag 0.16.0. `specs/research/` holds + verified notes. One verified here: randomness is on the `Io` interface — + `io.random(buffer)` (`std/Io.zig:2468`); `std.crypto.random` does not exist + in 0.16.0. +- No `std.log.err` in new code. PTR answers are untrusted bytes: never let one + reach a log line, a metric label, or the UI without passing `acceptHostname` + (ruling 4). + +## Rulings (binding) + +### 1. When: the tracker's flush pass resolves, bounded, serial, last + +Resolution is a step appended to the tracker's existing flush pass +(`Tracker.flushOnce`, `src/server/clients.zig:164`), not a new loop and not a +query-path hook. Reasons: the pass already runs every `flush_interval_s` (60 s) +on a dedicated task with a dedicated `config.db` connection (app.zig:523-524, +:754), it already respects the disk-monitor gate, and a name can only be +learned for a row that exists — which the flush itself just wrote. A DNS query +must never wait on naming, and with this placement it structurally cannot. + +**Order within one pass is fixed: drain-and-write, then prune (when due), then +resolve.** One `now_s` is read at the top of the pass and used by all three +steps. Resolving before pruning could spend PTR queries on rows the same pass +deletes; the fixed order makes that impossible, and a test proves a row past +the prune cutoff on a due pass causes no exchange. + +Per pass, the resolver: + +- selects at most `max_per_pass = 16` candidate rows (ruling 3 defines a + candidate), ordered by `name_attempt_after` ascending then ip; +- resolves them **serially** — in-flight is exactly 1. At 16 per 60 s pass the + LAN resolver sees at most ~0.27 queries/second from naming. + +**Naming has its own read timeout**, `ptr_read_timeout` = 2 s on the `.awake` +clock, a `pub const` on the resolver — it does *not* inherit the handler's +`forward_read_timeout`, which the operator may have set as high as the +upstream budget. **The budget bounds the whole attempt, not one transport +leg**: `ForwardClient` hands its duration to the UDP receive and then again +to the TCP fallback, so a resolver that answers UDP late with TC=1 and then +stalls TCP would spend up to 2× the budget per attempt — 64 s per pass, not +32. The default `exchangeFn` therefore wraps the entire `ForwardClient` +exchange in `transport.raceWithin(io, ptr_read_timeout, ...)` (the same +mechanism `exchangeTcp` itself uses, forward_client.zig:195), so UDP, +fallback and all share one deadline. The worst case is 16 × 2 s = 32 s of +naming per pass, and that number is a bound, not an estimate. That is acceptable because the drain already ran first: a slow pass +delays the *next* pass's start (the run loop sleeps after `flushOnce` +returns), and one extra interval against a 512-slot pending table on a +household LAN drops nothing. The acceptance test for this: with a stub client +that times out on every exchange, the pass still drains its pending batch +before any exchange is attempted, and attempts stop at `max_per_pass`. + +Retry cadence is split by outcome (first draft used one 24 h cadence for +everything, which left a freshly-declared zone, a recovered router, or a fixed +resolver unnamed for a day while forward-zone changes publish live): + +- **definitive** outcomes (`answered`, `nxdomain`): next attempt after + `refresh_after_s = 86_400` — the daily refresh that tracks DHCP renames; +- **non-definitive** outcomes (`no_zone`, `failed`, `invalid`): next attempt + after `retry_after_s = 3_600` — an operator who declares the missing zone + or fixes the resolver sees names within the hour, and a dead resolver costs + at most 16 timeouts per hour, not per minute. + +Both constants are `pub const` on the resolver; no config knob (household +scale, AGENTS.md: no generality nobody asked for). + +### 2. Where: only through a declared forward zone, never the pool + +The reverse name (`10.1.168.192.in-addr.arpa`, or the 32-nibble `ip6.arpa` +form) is matched against the live forward-zones table — the same +`forward_zones.Zones.match` the query path uses, bracketed by +`LocalTables.acquire`/`release` (`src/server/local_tables.zig:48`), so naming +always sees the generation the operator last published. + +**The zone's `resolver` is copied by value under the shared lock, and the +handle is released before any socket work.** `match` returns a pointer into +the current table generation, and `swap` frees the old generation as soon as +it holds the exclusive lock — a handle held across a PTR exchange, or a +released handle whose `*const Zone` is still dereferenced, is a use-after-free +against a config reload. `validate.Resolver` is a plain value; the copy is the +whole fix. A regression test must place the `swap` **inside the hazard +window** — after the handle release, before the copied resolver is used. A +test that swaps before `runPass` evaluates its arguments exercises nothing +(the first implementation's test had exactly this defect: the stub's exchange +performed the swap, but the resolver had already been copied into the call's +arguments, so the test passed regardless of what production retained). The +required shape: two candidates in one pass, two generations whose zones +cover both but carry **different resolver ports**; the stub `exchangeFn` +performs the swap during the *first* attempt (freeing the old generation +under `testing.allocator`) and records each attempt's resolver. Assertions: +the first attempt's resolver still reads the old generation's port after the +swap (the copy, not freed memory), and the second attempt's resolver reads +the *new* generation's port (each attempt re-acquires; no table pointer or +handle is cached across attempts). With a production `runPass` that retains +`*const Zone` or the handle across the release, the first assertion reads +freed memory and the second reads the stale generation — either fails. + +- Match found: one PTR query to that zone's resolver through + `forward_client.ForwardClient`, built per attempt as `Context.viaForwardZone` + builds one (handler.zig:411), with `ptr_read_timeout` (ruling 1). +- No match: **no query is sent to anyone** — not the pool, not any resolver. + The attempt counts under `no_zone` and retries per ruling 1. + +Scope of the promise, stated precisely (the first draft's "never to a public +resolver" claimed more than the code enforces): what this milestone +guarantees is that a reverse name reaches **only the resolver of a zone the +operator declared**, and never the upstream pool. `parseResolver` +(validate.zig:274) accepts any IP literal, so an operator who declares +`168.192.in-addr.arpa → udp://8.8.8.8:53` has pointed their private reverse +space at a public resolver — that is their declaration, and nxdns does not +second-guess it, any more than it second-guesses the same zone for forward +queries. Likewise nxdns cannot stop a declared LAN resolver forwarding +onward. The docs (ruling 11) say this in one sentence. + +The DNS cache is not consulted and not written: one PTR per client per day is +not worth a cache entry, and bypassing the cache keeps the resolver a plain +consumer of the exchange seam. + +### 3. Precedence: the operator's name always wins; NXDOMAIN clears + +Schema (ruling 5) keeps learned names in their own column, so precedence is a +display rule, not a write conflict — `name` is never written by this feature +and `learned_name` is never written by the operator. + +A **candidate** row satisfies both of: + +- `name IS NULL OR name = ''` — a row whose displayed name would come from + learning. `hand_edited` does not appear in the predicate (the first draft + had it; display precedence never consults it, so candidacy must not + either): a hand-edited row the operator grouped but did not name still + benefits, and a named row is skipped whatever its flag — its learned name + would never be shown. +- `name_attempt_after <= now_s` (ruling 6 gives the exact SQL). + +Outcomes of an attempt, written through one repo call (ruling 6), each +setting `name_attempt_after` per ruling 1's cadence: + +- **Answer with a valid PTR target**: store it in `learned_name` + (lowercased, no trailing dot), overwriting whatever learned name was there — + the router is the authority on its own zone, and a changed answer is a + renamed device, not a conflict. A test covers the overwrite: two attempts, + two different valid targets, second wins. +- **NXDOMAIN, or NOERROR with no matching PTR record** (NODATA): clear + `learned_name` to NULL. The router affirmatively says the address has no + name; keeping a stale one would show a device under its previous owner's + hostname after a lease change. +- **Everything else** — transport error, any RCODE other than NOERROR and + NXDOMAIN (REFUSED, FORMERR, SERVFAIL, unknown values), malformed reply, + invalid hostname (ruling 4): keep the stored `learned_name` as it stands, + count under `failed` (or `invalid`). An outage must not strip names from + the whole dashboard. + + **"RCODE" here is the full 12-bit value**: the header's 4 bits extended by + the OPT record's upper 8 bits when the response carries one + (`packet.findOptRecord` locates it; `dns/edns.zig` decodes it). A response + whose header says NOERROR under a nonzero extended RCODE is a failure, not + an answer and not NODATA — classifying on the header nibble alone would + store or clear a name on what the resolver called an error. An OPT record + that fails to parse makes the whole reply malformed (`failed`, keep). The + first implementation had exactly this bug; the regression test is a + NOERROR header + OPT with a nonzero extended RCODE, asserting `failed` + counts and the stored name survives — it must fail with the classification + reverted to header-only. + +**Staleness bound, stated honestly:** an address reassigned to a new device +immediately after a successful lookup shows the previous device's hostname for +up to `refresh_after_s` (24 h) — the next refresh then overwrites or clears +it. That bound is accepted: it matches the prune cadence's granularity, and +tightening it means more PTR traffic for a cosmetic lag. The docs state the +bound. + +`upsertSeen`, `updateClient`, `pruneStale` and the reconcile engine are +untouched: a pruned row takes its learned name with it, a re-materialised +device is re-learned within a pass, and `updateClient` writing a name removes +the row from candidacy (the name predicate) without touching `learned_name`. + +### 4. PTR answers are untrusted input: `acceptHostname` gates everything + +The answer bytes come from whatever box the operator pointed a zone at, and +they land in a UI. Validation has three layers, all required: + +1. **Transport**: `transport.validateResponse` (ID and question echo — that is + *all* it checks; the first draft leaned on it for more). +2. **Record selection**: walk `packet.answers`; the accepted record is the + **first** whose `rtype` is `.ptr`, whose class is `.in`, and whose owner + name equals the queried reverse name case-insensitively + (`name.eqlIgnoreCase`, name.zig:185). Records failing any of these are + skipped, not errors; an answer section with no accepted record under + RCODE NOERROR is NODATA (ruling 3 clears). Extra accepted records after + the first are ignored. +3. **Hostname shape**: the PTR target decodes via `record.rdataCname` + (record.zig:101 handles PTR) and formats via `name.formatText`; the text is + accepted only if every byte is in `[a-z0-9._-]` after ASCII-lowercasing + `A-Z`, labels are 1–63 bytes, the whole name is ≤ 253 bytes, no label + starts or ends with `-`, and there is no empty label (which also rejects a + trailing dot — `formatText` emits none, so one appearing is malformed). + Underscore is included because real DHCP hostnames carry it; nothing else + is. A failing target counts under `invalid` and keeps the stored name. + +`acceptHostname` is a pure function in `src/local/reverse_name.zig`, tested +against: the empty string, a 254-byte name, a 64-byte label, `a b`, `a\x00b`, +`héllo`, `-x`, `x-.y`, `a..b`, `.a`, `a.` (trailing dot), and accepting +`nas-1.lan`, `my_printer.home`, `x`. + +### 5. Schema: two runtime columns, not a reuse of `name` + +`clients` gains, in `config_schema.ddl_v1` directly after `name`: + +```sql + learned_name TEXT, + name_attempt_after INTEGER NOT NULL DEFAULT 0, +``` + +with the identical lines in PLAN §11.2. `name_attempt_after` is the epoch +second before which the resolver will not attempt this row again; 0 (the +default, and every pre-existing row) means "eligible now". Storing the *next* +attempt rather than the *last* one keeps the candidate predicate a plain +comparison (no addition that could overflow on a corrupt value, no special +case for "never attempted") and lets one column carry ruling 1's two cadences. + +Reusing `name` with a provenance marker was rejected: `name` feeds +`listClients`, which feeds `nxdns export`, and a learned name in an export +would turn runtime state into configuration — it would churn export diffs as +leases move, and under file authority the next reconcile would fight it. With +separate columns: + +- **Export/import untouched.** `listClients` selects `name` only + (clients_repo.zig:33-38); no code change, and a test proves an export before + and after a learned name lands is byte-identical. +- **Reconcile untouched.** The engine never touches runtime columns; the + learned columns ride the row exactly as `last_seen` does. Promotion + (declaring an observed address) keeps them; deletion of a declared row drops + them with the row, correctly. +- **File-authority mode needs no write rule.** Naming is a runtime write like + `upsertSeen`, permitted in both authority modes for the same reason. In + file mode the operator's *declared* name wins through the ordinary + precedence (a non-empty `name` removes the row from candidacy after the + reconcile writes it), and the learned name stays display-only runtime + state. + +The baseline test in `src/storage/migrations.zig` ("a fresh database reaches +the baseline...") gains `columnExists` probes for both columns. + +### 6. Repo surface: two calls, owned by the resolver + +`src/storage/repositories/clients_repo.zig`, in the runtime section beside +`upsertSeen` / `pruneStale`: + +- `resolveCandidates(database, out, now_s)` — fills a caller-supplied + fixed-capacity buffer (capacity `max_per_pass`, each slot + `logger.max_client_len` bytes — the bound every address text in this + program is sized by) and returns the filled slice or count. **No + allocation**: the candidate set is small by construction and the resolver + runs on the tracker's task, which owns no allocator today. The WHERE + clause, exactly: + + ```sql + (name IS NULL OR name = '') AND name_attempt_after <= ?1 + ORDER BY name_attempt_after, ip LIMIT 16 + ``` + + Tests cover: `now_s` smaller than any cadence constant still selects + never-attempted rows (`DEFAULT 0`); the boundary `name_attempt_after = + now_s` selects; `now_s + 1` does not; an extreme stored value + (`i64` max) never selects and never traps. +- `noteNameOutcome(database, ip, outcome)` where the outcome carries + `attempt_after: i64` and one of: store `text`, clear, keep. One UPDATE: + always sets `name_attempt_after`; sets `learned_name` to the text on + store, to NULL on clear, leaves it alone on keep. **A row deleted between + selection and this call makes the UPDATE touch zero rows; that is a + no-op by design, not an error and not a counter** — the device left, and + nothing was learned about nothing. + +`ClientRow` gains `learned_name: []const u8` (NULL reads as `""`, like +`name`); `readClientRow` and both SELECTs carry it. `name_attempt_after` is +**not** exposed on `ClientRow` or the API — it is scheduling state with no +operator meaning, and keeping it out keeps the contract surface one field. +`listClients` (export) is not widened — that is the point of ruling 5. + +### 7. The resolver module: `src/server/client_names.zig` + +New file. A `Resolver` struct owning: + +- `stats: Stats` with the six **outcome** counters `attempted`, `answered`, + `nxdomain`, `no_zone`, `invalid`, `failed`, plus two counters *outside* the + outcome sum: `read_failures` (the candidate SELECT failed; the pass skips + naming) and `write_failures` (`noteNameOutcome` failed; the outcome was + still counted). Guarded by the tracker's mutex pattern, with a + `snapshotStats`. Invariant, asserted in a test: `attempted` equals + `answered + nxdomain + no_zone + invalid + failed`. Database failures log + at most one `warn` per pass each for reads and writes, tracker-style + (clients.zig:181-186); network failures are the counters' job and log + nothing new (the `forward_client` module's existing `debug`-level + diagnostics are exempt from this milestone's no-log rule — they predate it, + are debug-level, and suppressing them per-caller would fork the client). +- an **exchange seam**: production code cannot inject into `ForwardClient` + (it is a concrete struct the resolver constructs), so the seam sits above + it — a field + `exchangeFn: *const fn (io, validate.Resolver, query, response_buf) transport.ExchangeError![]u8` + defaulting to a function that builds a stack `ForwardClient` with + `ptr_read_timeout` and calls `exchange`. Tests replace the pointer; the + production default is itself covered by one test against a loopback UDP + socket if one already exists in the suite's patterns, otherwise by the live + smoke. The no-zone test runs the **production** pass code with a counting + stub and asserts the count stays 0. +- a `runPass(io, database, tables, now_s)`: select candidates (ruling 6), and + for each — build the reverse name (pure, ruling 8), acquire/match/**copy + resolver**/release (ruling 2), build the PTR query: header with a random id + filled from `io.random` (`std/Io.zig:2468` — *not* `std.crypto.random`, + which 0.16.0 does not have), RD set, one question, qtype `.ptr`, qclass + `.in`, encoded with `dns/header` + `dns/question.encode`; exchange through + the seam, classify per rulings 3–4, write per ruling 6. The id is + test-visible through the seam (the stub sees the query bytes), so no + determinism hook is needed beyond the seam itself. + +Wiring: `Tracker.flushOnce` gains an optional `*client_names.Resolver` +parameter, invoked **after** the drain/write and prune steps (ruling 1's +order); `app.zig` constructs the Resolver beside the tracker (holding the +`*LocalTables` pointer) and passes it through `Tracker.run`'s arguments +(app.zig:754). The resolver runs on the tracker's task against `tracker_db` — +the dedicated-connection rule (clients.zig:127-131) is satisfied because it +is the *same* task, serial with the flush. A gated pass (disk critical) skips +naming too: naming writes. + +### 8. The pure part: `src/local/reverse_name.zig` + +New file, pure (no Io, no clock). Two functions plus their tests: + +- `reverseName(addr: address.NetAddress, buf: *[max_reverse_len]u8) []const u8` + — `d.c.b.a.in-addr.arpa` for v4, the 32-nibble lowercase `ip6.arpa` form for + v6. `max_reverse_len` is a comptime bound (v6 form: 32 nibbles · 2 + 8 for + `ip6.arpa` = 72; compute, don't hand-wave). An IPv4-mapped v6 address was + already canonicalised to `ip4` by `NetAddress` (clients.zig test at :305 + proves the tracker's addresses are canonical) — assert, don't re-handle. +- `acceptHostname(text: []const u8) bool` — ruling 4's gate, with ruling 4's + test table. + +`src/platform/address.zig` types are plain values; importing them into +`local/` keeps the module pure. Register the file in `src/tests.zig` the way +its siblings are registered. + +### 9. Failure visibility: one metrics group + +The resolver must be reachable from `metrics.collect`, which reads +`server.WebState` — so `src/web/server.zig`'s `WebState` gains an optional +`client_names: ?*client_names.Resolver` collaborator, populated by `app.zig` +beside `tracker` (the first draft wired the sample and forgot the state; S2 +owns `src/web/server.zig` for this field). `src/web/metrics.zig`: `Sample` +gains `client_names: ?client_names.Resolver.Stats`, collected beside the +tracker's (metrics.zig:191-193) and rendered as +`counterGroup(w, "nxdns_client_names_", ...)` beside `nxdns_clients_` +(metrics.zig:333-334). `counterGroup` renders every struct field, so all +eight counters (six outcomes + `read_failures` + `write_failures`) surface +without per-field code. The render test pins one line (e.g. +`nxdns_client_names_no_zone_total`). + +### 10. API and UI: learned is visible and visibly different + +- `GET /api/clients` rows carry `learned_name` + (`src/web/openapi.yaml` `required` + `properties` for the client schema; + contract samples regenerated; `web/src/lib/types.ts` client row type gains + `learned_name: string`). `name_attempt_after` stays internal (ruling 6). +- `web/src/features/clients/ClientsPage.tsx`: the name cell shows `name` when + non-empty; otherwise `learned_name` in the page's existing muted text style + with a visually-distinct treatment that marks it as learned (exact + affordance is the implementer's, but: no new colour system, reuse the muted + style the page already has, and the distinction must survive a screen + reader — an `aria-label` or visible suffix, not colour alone). An empty + both shows what it shows today. +- `ClientEditDialog.tsx` may pre-fill its name field with `learned_name` when + `name` is empty — adopting the learned name as a typed name is the natural + gesture — but saving still goes through the ordinary PUT and sets + `hand_edited` server-side as today. This applies in database-authority mode + only; in file mode client edits answer 403 as they do today, and the + file-mode story is ruling 5's (declared name wins via candidacy). +- Mocks in `ClientsPage.test.tsx` gain the field with distinct values; + rendering tests assert: a named row shows `name` and not `learned_name`; an + unnamed row shows `learned_name` with the learned affordance. + +### 11. Docs + +- `docs/reference/configuration.md`, clients section: a paragraph stating the + learned-name mechanism; that it requires a conditional forward zone + covering the LAN's reverse space (with the `168.192.in-addr.arpa` example); + that hand-typed names win; that learned names never appear in exports; that + a rename or lease change can display stale for up to a day (ruling 3's + bound). The cadence must be described as it is, not rounded up: each pass + attempts **at most 16 due rows**, and a row is due per ruling 1's two + cadences — "every unnamed device, once a minute" overstates both coverage + and rate and must not appear; and one sentence that PTR queries go to the declared zone's + resolver, wherever the operator pointed it (ruling 2's scope). +- The how-to/tutorial page that documents conditional forward zones gains the + reverse-zone example if it lacks one (locate it; do not guess its path). +- PLAN §7.2 gains one sentence: materialised clients are named by PTR through + the declared forward zones; learned names are runtime state. +- `docs/reference/api.md` only if it enumerates client row fields (check). + +## Sessions + +Three, strictly sequential: S1 → S2 → S3. S2 needs S1's columns and repo +calls; S2 owns the contract regeneration (the API shape changes when S1 widens +`ClientRow`, but the sample regen runs the integration suite, which S2's +wiring changes — one regen at the end of S2 avoids regenerating twice). S3 +reads the fields S2's regenerated types carry. + +### Session S1: pure module, schema, repo + +Owns: `src/local/reverse_name.zig` (new), `src/tests.zig` (registration), +`src/storage/config_schema.zig`, `src/storage/migrations.zig` (test), +`src/storage/repositories/clients_repo.zig`, `PLAN.md` (§11.2 lines only). + +- S1.1 ruling 8: `reverse_name.zig` with exhaustive tests (v4, v6, bounds; + `acceptHostname` accept/reject table from ruling 4). +- S1.2 ruling 5: the two DDL lines in both copies, baseline column probes. +- S1.3 ruling 6: repo calls, `ClientRow` widening, tests — including: the + candidate SQL's boundary and extreme-value cases; the ordering; NULL + round-trips as `""`; export is byte-stable across a `noteNameOutcome`; + `updateClient` leaves `learned_name` alone; the zero-row UPDATE no-op. + +Acceptance (S1): +- [ ] `zig build test` passes; baseline test proves both columns exist. +- [ ] `reverseName` output for `192.168.1.10` is + `10.1.168.192.in-addr.arpa` and for `fd00::1` is the full 32-nibble + lowercase form; both are matched by a `forward_zones.Zones` built over + `168.192.in-addr.arpa` / a covering `ip6.arpa` zone in a test that ties + the two modules together. +- [ ] `acceptHostname` passes ruling 4's full accept/reject table, including + `a..b`, `.a` and the trailing-dot case. +- [ ] An export taken before and after `noteNameOutcome` on an observed row is + byte-identical. + +### Session S2: resolver, wiring, metrics, API contract (needs S1) + +Owns: `src/server/client_names.zig` (new), `src/tests.zig` (registration), +`src/server/clients.zig`, `src/app.zig`, `src/web/server.zig` (the +`client_names` field), `src/web/metrics.zig`, `src/web/openapi.yaml`, +`web/src/lib/types.ts`, `web/src/lib/contractSamples.gen.ts` (regenerated), +`web/src/features/clients/ClientsPage.test.tsx` (mock fields only). + +- S2.1 ruling 7: the Resolver, the exchange seam, `flushOnce` hook (ruling 1 + order), app and WebState wiring. +- S2.2 rulings 1–4: classification tests against a stub exchange. Tests must + cover: answered stores and lowercases; a second answered overwrites; + NXDOMAIN clears; NODATA clears; REFUSED and an unknown RCODE keep and count + `failed`; timeout keeps; invalid hostname keeps and counts `invalid`; a + record with wrong type, class, or owner name is skipped; no-zone sends + nothing (the stub's call count is the assertion) and counts; + `name_attempt_after` lands at `now + refresh_after_s` for definitive and + `now + retry_after_s` for non-definitive outcomes; the candidate cap holds; + a named row is never attempted; a gated pass attempts nothing; drain + precedes any exchange when every exchange times out (ruling 1); a row past + the prune cutoff on a due pass causes no exchange (ruling 1); the + two-generation swap test per ruling 2's exact shape (swap inside the + hazard window, both assertions); a NOERROR header under a nonzero OPT + extended RCODE counts `failed` and keeps the stored name (ruling 3); the + default `exchangeFn` bounds UDP plus the TCP fallback under one + `ptr_read_timeout` deadline (ruling 1) — a loopback resolver that answers + UDP with TC=1 late in the budget and then stalls TCP must fail the attempt + within one budget, not two (assert elapsed with margin; inject a shortened + budget locally if the default seam needs one). +- S2.3 ruling 9: WebState field, metrics sample, render pin. +- S2.4 ruling 10's API half: openapi.yaml, regen samples, types.ts, mock + fields. + +Acceptance (S2): +- [ ] `zig build test` and `zig build test -Dintegration` pass; the + regenerated samples carry `learned_name` in the client row sample and + do not carry `name_attempt_after`. +- [ ] `cd web && npm run typecheck && npm test` pass with the widened types. +- [ ] The stats-sum invariant test passes (`attempted` = sum of the five + outcomes; `read_failures`/`write_failures` outside it). +- [ ] The no-zone test proves zero exchanges for an uncovered reverse name + through the production pass code. +- [ ] The concurrent-swap regression test passes (ruling 2). + +### Session S3: UI and docs (needs S2) + +Owns: `web/src/features/clients/ClientsPage.tsx`, +`web/src/features/clients/ClientEditDialog.tsx`, +`web/src/features/clients/ClientsPage.test.tsx` (rendering assertions), +`docs/reference/configuration.md`, `docs/reference/api.md` (if applicable), +the conditional-forwarding doc page, `PLAN.md` (§7.2 sentence only). + +- S3.1 ruling 10's UI half. +- S3.2 ruling 11. + +Acceptance (S3): +- [ ] `cd web && npm run typecheck && npm test && npm run lint && npm run + build` pass; rendering tests cover named-wins and learned-affordance. +- [ ] The docs name the reverse-zone prerequisite with a concrete + `in-addr.arpa` example, the one-day staleness bound, and the + resolver-is-the-operator's-choice sentence. + +### Orchestrator + +Verify each session's acceptance before starting the next. After S3, the full +gate set (`zig build test`, `zig build test -Dintegration`, `test-aarch64` if +qemu is present, `cd web && npm test`, `npm run assert-bundled`), then a live +smoke per the verify-against-the-real-network rule: a scratch server with a +forward zone `168.192.in-addr.arpa → udp://:53` (or a local stub +resolver serving PTR), one real query from a LAN client, then wait one flush +interval and confirm the learned name in `GET /api/clients`, in the UI, and in +`nxdns_client_names_answered_total`. Then delete the zone and confirm the next +attempt counts `no_zone` with no upstream traffic (tcpdump or the pool +counters — pool queries must not move); with the hourly non-definitive +cadence, age `name_attempt_after` by SQL in the scratch database rather than +waiting. In database mode, hand-edit the name and confirm it wins and the row +stops being attempted. In file mode, confirm a file-declared name wins after +reconcile and the learned name remains display-only. Record deviations in +`## Recorded (implementation)`. + +## Module layout + +New files: `src/local/reverse_name.zig`, `src/server/client_names.zig`. +Deleted surface: none. + +## File ownership + +| File | Session | +| --- | --- | +| `src/local/reverse_name.zig` (new) | S1 | +| `src/storage/config_schema.zig`, `src/storage/migrations.zig` (test) | S1 | +| `src/storage/repositories/clients_repo.zig` | S1 | +| `PLAN.md` | S1 (§11.2 lines), then S3 (§7.2 sentence) — sequential, never concurrent | +| `src/tests.zig` | S1, then S2 — sequential | +| `src/server/client_names.zig` (new) | S2 | +| `src/server/clients.zig`, `src/app.zig`, `src/web/server.zig`, `src/web/metrics.zig` | S2 | +| `src/web/openapi.yaml`, `web/src/lib/types.ts`, `web/src/lib/contractSamples.gen.ts` | S2 | +| `web/src/features/clients/ClientsPage.test.tsx` | S2 (mock fields), then S3 (assertions) — sequential | +| `web/src/features/clients/ClientsPage.tsx`, `ClientEditDialog.tsx` | S3 | +| `docs/reference/configuration.md`, `docs/reference/api.md`, forwarding doc page | S3 | + +## Acceptance (milestone complete) + +- [ ] All session acceptance boxes. +- [ ] `config_schema.ddl_v1` and PLAN §11.2 byte-identical, both carrying the + two lines; `migrations.steps` still one step, `target_version` still 1. +- [ ] The live smoke: a real client's hostname appears without any operator + edit; removing the zone stops queries entirely (`no_zone` moves, pool + counters do not). +- [ ] `nxdns export` output is byte-identical before and after names are + learned. +- [ ] Database mode: hand-editing a name wins over the learned name + immediately and permanently (the row leaves candidacy). File mode: a + file-declared name wins after reconcile; the learned name is + display-only runtime state. +- [ ] `src/web/openapi.yaml` reviewed by hand against the changed client row + shape, reviewer says so in `## Recorded` (no automated guard covers + schema-to-response agreement — milestone-24 finding, still true). + +## Anti-requirements + +- No migration step, no `ddl_v2`, no runtime schema probing. +- No PTR query to the upstream pool, and none for a reverse name no declared + forward zone covers — under any fallback, ever. (The declared resolver + itself is the operator's choice; ruling 2 scopes the promise.) +- No reuse of `clients.name` for learned names, and no learned data in + `nxdns export` / `import` ZON or in reconcile semantics. +- No mDNS, NetBIOS, DHCP-lease-file parsing, or any second naming source. + PTR through declared zones is the mechanism; a router that serves no + reverse zone yields `no_zone` counts and an unnamed row, visibly. +- No per-query or query-path resolution: naming rides the flush pass only. +- No config knobs for cadence, caps, or timeouts; constants per ruling 1. +- No DNS-cache participation for PTR probes. +- No unbounded anything: candidates per pass, in-flight, per-attempt timeout, + name length, and retry cadence are all bounded above by rulings 1 and 4. +- No storing or displaying a PTR target that fails `acceptHostname` — not + even truncated or escaped. +- No new per-attempt log lines from this milestone's code; counters and the + metrics group are the surface. `forward_client.zig`'s existing debug + diagnostics stand as they are. +- No allocation on the naming path: candidate storage is fixed-capacity + (ruling 6). + +## Recorded (implementation) + +- `src/server/phase7_integration_test.zig` sits outside the ownership table + but gained a mechanical `, null` argument at every `flushOnce`/`run` call + site, following the new `names: ?*client_names.Resolver` parameter. +- Review: one external round found four defects (extended RCODE ignored the + OPT upper bits; `exchangeWithin` did not bound the UDP+TCP pair under one + deadline; the swap test asserted nothing observable; a docs cadence + sentence). All four were fixed; the re-review of + `src/server/client_names.zig` returned no findings. +- Every fix shipped with a test the author watched fail with the fix + reverted. For the swap test the hazard was injected (a pass-level resolver + cache that skips re-acquisition) and the second-port assertion failed; the + one-deadline test's bound is `budget + late/2` (550 ms) because the + unbounded path takes ~700 ms and a 2×-budget bound would pass it. +- `src/web/openapi.yaml` was reviewed by hand against `ClientRow` + (clients_repo.zig:312): the nine required fields match one-to-one and + `name_attempt_after` is absent from the response, as intended. +- Live smoke (scratch server, file mode, zone `127.in-addr.arpa` at a local + stub PTR resolver): one real query materialised the client; the flush pass + learned `smoke-host.lan` with no operator edit, visible in + `GET /api/clients` and `nxdns_client_names_answered_total`. Removing the + zone and re-arming `name_attempt_after` produced `no_zone` with zero + packets to the stub and no pool movement. The learned name survived a + restart's reconcile. A file-declared `name` won in the API after reconcile + with the learned name still present as display-only state, and the named + row left candidacy (`attempted` stayed 0 over a full flush interval). The + database-mode hand-edit path runs the same candidacy SQL + (`name IS NULL OR name = ''`) and is covered by the repo unit tests rather + than a second live run. The UI half is covered by the ClientsPage + rendering tests, not a live browser check. +- The first smoke attempt polled a stale pre-milestone binary out of + `zig-out/bin` — `zig build test` does not refresh the install step. Rebuild + before any live check. diff --git a/src/app.zig b/src/app.zig index be38010..9633507 100644 --- a/src/app.zig +++ b/src/app.zig @@ -34,6 +34,7 @@ const api_limiter = @import("web/api_limiter.zig"); const auth = @import("web/auth.zig"); const cert_store = @import("server/cert_store.zig"); const cli = @import("cli.zig"); +const client_names = @import("server/client_names.zig"); const clients = @import("server/clients.zig"); const config_export = @import("config/export.zig"); const db = @import("storage/db.zig"); @@ -462,6 +463,9 @@ fn serve(r: cli.Runner, args: cli.RunArgs) !u8 { var paused: pause.Pause = .{}; var tracker: clients.Tracker = .init(cfg.logging.retention_days); + // Naming rides the tracker's pass, on the tracker's task and connection + // (milestone-25 ruling 1), and reads the live forward zones. + var client_names_resolver: client_names.Resolver = .init(&tables); // The queue holds waiting tasks in intrusive lists, so neither the buffer // nor the `Logger` may move once a task has touched either. @@ -635,6 +639,7 @@ fn serve(r: cli.Runner, args: cli.RunArgs) !u8 { .handler = &h, .pause = &paused, .tracker = &tracker, + .client_names = &client_names_resolver, .manager = &manager, .pool = &pool, .monitor = &monitor, @@ -751,7 +756,7 @@ fn serve(r: cli.Runner, args: cli.RunArgs) !u8 { try group.concurrent(io, retention_mod.Retention.run, .{ &retention, io, &querylog_retention_db, gate }); try group.concurrent(io, disk_monitor.Monitor.run, .{ &monitor, io }); try group.concurrent(io, manager_mod.Manager.runScheduler, .{ &manager, io }); - try group.concurrent(io, clients.Tracker.run, .{ &tracker, io, &tracker_db, gate }); + try group.concurrent(io, clients.Tracker.run, .{ &tracker, io, &tracker_db, gate, &client_names_resolver }); try group.concurrent(io, runMaintenance, .{ &h, if (web_limiter) |*l| l else null, io }); // Started last (ruling 26), canceled by the same `group.cancel`; its inner diff --git a/src/local/reverse_name.zig b/src/local/reverse_name.zig new file mode 100644 index 0000000..d9b3379 --- /dev/null +++ b/src/local/reverse_name.zig @@ -0,0 +1,204 @@ +//! Reverse DNS names and the hostname gate for learned client names +//! (milestone 25). Pure: bytes in, bytes out — no `std.Io`, no clock, no +//! sockets. Everything that queries a resolver or writes a row lives in +//! `src/server/`. + +const std = @import("std"); + +const address = @import("../platform/address.zig"); + +const v4_suffix = "in-addr.arpa"; +const v6_suffix = "ip6.arpa"; + +/// The v6 form is the longest: 32 nibbles, each followed by a dot, then +/// `ip6.arpa`. +pub const max_reverse_len: usize = 32 * 2 + v6_suffix.len; + +/// A PTR owner name is one label per byte (v4) or per nibble (v6), least +/// significant first, under `in-addr.arpa` / `ip6.arpa`. Lowercase hex for v6, +/// no trailing dot — the form `forward_zones.Zones.match` expects. +/// +/// The tracker canonicalises an IPv4-mapped v6 address to `.ip4` before a +/// `NetAddress` reaches here, so this function never sees one. +pub fn reverseName(addr: address.NetAddress, buf: *[max_reverse_len]u8) []const u8 { + var w = std.Io.Writer.fixed(buf); + switch (addr) { + .ip4 => |b| { + w.print("{d}.{d}.{d}.{d}.{s}", .{ b[3], b[2], b[1], b[0], v4_suffix }) catch unreachable; + }, + .ip6 => |b| { + std.debug.assert(!isIp4Mapped(b)); + var i: usize = b.len; + while (i > 0) { + i -= 1; + const byte = b[i]; + w.writeByte(hex_digits[byte & 0x0f]) catch unreachable; + w.writeByte('.') catch unreachable; + w.writeByte(hex_digits[byte >> 4]) catch unreachable; + w.writeByte('.') catch unreachable; + } + w.writeAll(v6_suffix) catch unreachable; + }, + } + return w.buffered(); +} + +const hex_digits = "0123456789abcdef"; + +fn isIp4Mapped(b: [16]u8) bool { + return std.mem.eql(u8, b[0..10], &[_]u8{0} ** 10) and b[10] == 0xff and b[11] == 0xff; +} + +/// The gate every PTR target passes before it is stored, logged or displayed. +/// The bytes come from whatever box the operator pointed a forward zone at, so +/// nothing weaker is enough. +/// +/// Accepts only `[a-z0-9._-]` after ASCII-lowercasing `A-Z`; labels are 1–63 +/// bytes and the whole name is at most 253; no label starts or ends with `-`. +/// An empty label is rejected, which also rejects a trailing dot — `formatText` +/// emits none, so one appearing means the reply was malformed. +/// +/// Underscore is accepted because real DHCP hostnames carry it. Nothing else +/// outside the set is. +pub fn acceptHostname(text: []const u8) bool { + if (text.len == 0 or text.len > 253) return false; + + var label_len: usize = 0; + var prev: u8 = 0; + for (text) |raw| { + const ch = std.ascii.toLower(raw); + if (ch == '.') { + if (label_len == 0) return false; + if (prev == '-') return false; + label_len = 0; + prev = ch; + continue; + } + if (label_len == 0 and ch == '-') return false; + if (!isHostByte(ch)) return false; + label_len += 1; + if (label_len > 63) return false; + prev = ch; + } + if (label_len == 0) return false; + if (prev == '-') return false; + return true; +} + +fn isHostByte(ch: u8) bool { + return (ch >= 'a' and ch <= 'z') or (ch >= '0' and ch <= '9') or ch == '_' or ch == '-'; +} + +// --------------------------------------------------------------------------- +// tests +// --------------------------------------------------------------------------- + +const testing = std.testing; + +test "reverseName reverses the octets of a v4 address" { + var buf: [max_reverse_len]u8 = undefined; + try testing.expectEqualStrings( + "10.1.168.192.in-addr.arpa", + reverseName(try address.NetAddress.parse("192.168.1.10"), &buf), + ); + try testing.expectEqualStrings( + "0.0.0.0.in-addr.arpa", + reverseName(try address.NetAddress.parse("0.0.0.0"), &buf), + ); + try testing.expectEqualStrings( + "255.255.255.255.in-addr.arpa", + reverseName(try address.NetAddress.parse("255.255.255.255"), &buf), + ); +} + +test "reverseName writes the 32-nibble lowercase ip6.arpa form" { + var buf: [max_reverse_len]u8 = undefined; + try testing.expectEqualStrings( + "1.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.d.f.ip6.arpa", + reverseName(try address.NetAddress.parse("fd00::1"), &buf), + ); + // Every nibble distinct, so a swapped high/low half would show. + try testing.expectEqualStrings( + "b.a.9.8.7.6.5.4.3.2.1.0.f.e.d.c.b.a.9.8.7.6.5.4.3.2.1.0.f.e.d.c.ip6.arpa", + reverseName(try address.NetAddress.parse("cdef:0123:4567:89ab:cdef:0123:4567:89ab"), &buf), + ); +} + +test "the v6 form is exactly max_reverse_len bytes and the v4 form is shorter" { + var buf: [max_reverse_len]u8 = undefined; + const v6 = reverseName(try address.NetAddress.parse("cdef:0123:4567:89ab:cdef:0123:4567:89ab"), &buf); + try testing.expectEqual(max_reverse_len, v6.len); + const v4 = reverseName(try address.NetAddress.parse("255.255.255.255"), &buf); + try testing.expect(v4.len < max_reverse_len); +} + +test "an IPv4-mapped v6 literal is already canonical, so reverseName sees v4" { + var buf: [max_reverse_len]u8 = undefined; + try testing.expectEqualStrings( + "10.1.168.192.in-addr.arpa", + reverseName(try address.NetAddress.parse("::ffff:192.168.1.10"), &buf), + ); +} + +test "acceptHostname accepts and rejects ruling 4's table" { + // 254 bytes, every label within 63: only the total length rejects it. + const long_name = "a" ** 63 ++ "." ++ "a" ** 63 ++ "." ++ "a" ** 63 ++ "." ++ "a" ** 62; + try testing.expectEqual(@as(usize, 254), long_name.len); + const long_label = "a" ** 64; + + const cases = [_]struct { text: []const u8, want: bool }{ + .{ .text = "", .want = false }, + .{ .text = long_name, .want = false }, + .{ .text = long_label, .want = false }, + .{ .text = "a b", .want = false }, + .{ .text = "a\x00b", .want = false }, + .{ .text = "héllo", .want = false }, + .{ .text = "-x", .want = false }, + .{ .text = "x-.y", .want = false }, + .{ .text = "a..b", .want = false }, + .{ .text = ".a", .want = false }, + .{ .text = "a.", .want = false }, + .{ .text = "nas-1.lan", .want = true }, + .{ .text = "my_printer.home", .want = true }, + .{ .text = "x", .want = true }, + }; + for (cases) |case| { + testing.expectEqual(case.want, acceptHostname(case.text)) catch |err| { + std.debug.print("acceptHostname(\"{s}\")\n", .{case.text}); + return err; + }; + } +} + +test "a reverse name matches the forward zone declared over its reverse space" { + const forward_zones = @import("forward_zones.zig"); + + var zones = try forward_zones.Zones.build(testing.allocator, &.{ + .{ .zone = "168.192.in-addr.arpa", .resolver = "udp://192.168.1.1:53" }, + .{ .zone = "0.0.d.f.ip6.arpa", .resolver = "udp://[fd00::1]:53" }, + }); + defer zones.deinit(testing.allocator); + + var buf: [max_reverse_len]u8 = undefined; + const v4 = reverseName(try address.NetAddress.parse("192.168.1.10"), &buf); + try testing.expectEqualStrings("168.192.in-addr.arpa", zones.match(v4).?.zone); + + var buf6: [max_reverse_len]u8 = undefined; + const v6 = reverseName(try address.NetAddress.parse("fd00::1"), &buf6); + try testing.expectEqualStrings("0.0.d.f.ip6.arpa", zones.match(v6).?.zone); + + // An address outside both declared reverse zones matches nothing, which is + // the `no_zone` outcome: no query is sent to anyone. + const outside = reverseName(try address.NetAddress.parse("10.0.0.1"), &buf); + try testing.expectEqual(@as(?*const forward_zones.Zone, null), zones.match(outside)); +} + +test "acceptHostname takes the boundary lengths and mixed case" { + try testing.expect(acceptHostname("a" ** 63)); + // 253 bytes, the longest name accepted. + try testing.expect(acceptHostname("a" ** 63 ++ "." ++ "a" ** 63 ++ "." ++ "a" ** 63 ++ "." ++ "a" ** 61)); + try testing.expect(acceptHostname("NAS-1.LAN")); + try testing.expect(!acceptHostname("x-")); + try testing.expect(!acceptHostname("a.-b")); + try testing.expect(!acceptHostname("a.b-.c")); +} diff --git a/src/server/client_names.zig b/src/server/client_names.zig new file mode 100644 index 0000000..bea3ed5 --- /dev/null +++ b/src/server/client_names.zig @@ -0,0 +1,1159 @@ +//! Client names learned over reverse DNS (milestone 25). +//! +//! The tracker materialises every querying address into a `clients` row, but +//! `name` stays NULL until the operator types one. This resolver asks the box +//! the operator already declared for the LAN's reverse space: it builds the +//! address's reverse name, matches it against the live forward zones, sends one +//! PTR query to that zone's resolver, and stores the answer in `learned_name` — +//! runtime state beside `last_seen`, never configuration. +//! +//! A reverse name no declared forward zone covers is sent to nobody: not the +//! upstream pool, not any resolver. What the milestone guarantees is that the +//! name reaches only the resolver of a zone the operator declared; where that +//! resolver points is the operator's declaration, exactly as it is for forward +//! queries. +//! +//! `runPass` is the last step of the tracker's flush pass, on the tracker's task +//! against the tracker's dedicated connection. A DNS query never waits on +//! naming, and naming never runs concurrently with itself. The pass is bounded: +//! at most `clients_repo.max_per_pass` candidates, one exchange in flight, and +//! `ptr_read_timeout` per exchange. +//! +//! PTR answers are untrusted bytes from whatever box the operator pointed a zone +//! at. Nothing reaches the database, a log line or the UI without passing +//! `reverse_name.acceptHostname`. + +const std = @import("std"); + +const address = @import("../platform/address.zig"); +const clients_repo = @import("../storage/repositories/clients_repo.zig"); +const db = @import("../storage/db.zig"); +const dns_header = @import("../dns/header.zig"); +const edns = @import("../dns/edns.zig"); +const forward_client = @import("../local/forward_client.zig"); +const local_tables = @import("local_tables.zig"); +const name_mod = @import("../dns/name.zig"); +const packet = @import("../dns/packet.zig"); +const question = @import("../dns/question.zig"); +const record = @import("../dns/record.zig"); +const reverse_name = @import("../local/reverse_name.zig"); +const transport = @import("../upstream/transport.zig"); +const types = @import("../dns/types.zig"); +const validate = @import("../config/validate.zig"); + +const log = std.log.scoped(.client_names); + +/// A query is a header, one name and the two fixed fields. +const query_buf_len = types.header_len + types.max_name_len + 4; +/// One PTR answer never approaches this; the ceiling exists so a resolver that +/// answers with something enormous fails the exchange rather than this buffer. +const response_buf_len = 2048; + +/// What one attempt decided. `attempted` is the sum of these five, which a test +/// asserts. +pub const Outcome = enum { answered, nxdomain, no_zone, invalid, failed }; + +/// The exchange seam. Production code cannot inject into `ForwardClient` — it is +/// a concrete struct this module constructs — so the seam sits above it. Tests +/// replace the function pointer; `defaultExchange` is what the server runs. +pub const ExchangeFn = *const fn ( + io: std.Io, + resolver: validate.Resolver, + query: []const u8, + response_buf: []u8, +) transport.ExchangeError![]u8; + +pub const Resolver = struct { + /// Naming's own read budget on the `.awake` clock. It deliberately does not + /// inherit the handler's `forward_read_timeout`, which the operator may have + /// set as high as the whole upstream budget. + pub const ptr_read_timeout: std.Io.Clock.Duration = .{ + .raw = .fromSeconds(2), + .clock = .awake, + }; + + /// Next attempt after a definitive outcome (`answered`, `nxdomain`): the + /// daily refresh that tracks DHCP renames. + pub const refresh_after_s: i64 = 86_400; + /// Next attempt after a non-definitive one (`no_zone`, `failed`, `invalid`): + /// an operator who declares the missing zone or fixes the resolver sees + /// names within the hour, and a dead resolver costs at most + /// `clients_repo.max_per_pass` timeouts per hour. + pub const retry_after_s: i64 = 3_600; + + /// The six outcome counters, plus two that sit outside the outcome sum: + /// `read_failures` (the candidate SELECT failed, so the pass skipped naming) + /// and `write_failures` (the row update failed, but the outcome was still + /// counted). + pub const Stats = struct { + attempted: u64 = 0, + answered: u64 = 0, + nxdomain: u64 = 0, + no_zone: u64 = 0, + invalid: u64 = 0, + failed: u64 = 0, + read_failures: u64 = 0, + write_failures: u64 = 0, + }; + + /// Guards `stats` only. Every other field is written once at construction. + mutex: std.Io.Mutex = .init, + /// The live forward zones, read under `acquire`/`release` so naming always + /// sees the generation the operator last published. + tables: *local_tables.LocalTables, + stats: Stats = .{}, + exchange_fn: ExchangeFn = defaultExchange, + + pub fn init(tables: *local_tables.LocalTables) Resolver { + return .{ .tables = tables }; + } + + pub fn snapshotStats(self: *Resolver, io: std.Io) Stats { + self.mutex.lockUncancelable(io); + defer self.mutex.unlock(io); + return self.stats; + } + + /// One naming pass: at most `clients_repo.max_per_pass` candidate rows, + /// resolved serially, each written back through one repo call. + /// + /// Nothing here returns an error. A failed read skips the pass, a failed + /// write leaves the row to be attempted again, and both count. Each logs at + /// most one `warn` per pass. + pub fn runPass(self: *Resolver, io: std.Io, database: *db.Db, now_s: i64) void { + var candidates: clients_repo.CandidateBuf = undefined; + const count = clients_repo.resolveCandidates(database, &candidates, now_s) catch |err| { + log.warn("selecting clients to name failed: {s}", .{@errorName(err)}); + self.mutex.lockUncancelable(io); + self.stats.read_failures += 1; + self.mutex.unlock(io); + return; + }; + + var write_failures: u64 = 0; + for (candidates[0..count]) |*candidate| { + var learned_buf: [types.max_name_len]u8 = undefined; + const result = self.attempt(io, candidate.ip(), &learned_buf); + + const attempt_after = nextAttempt(now_s, result.outcome); + if (clients_repo.noteNameOutcome(database, candidate.ip(), .{ + .attempt_after = attempt_after, + .learned = result.learned, + })) |_| {} else |err| { + if (write_failures == 0) { + log.warn("recording the name of {s} failed: {s}", .{ candidate.ip(), @errorName(err) }); + } + write_failures += 1; + } + + self.mutex.lockUncancelable(io); + self.stats.attempted += 1; + switch (result.outcome) { + .answered => self.stats.answered += 1, + .nxdomain => self.stats.nxdomain += 1, + .no_zone => self.stats.no_zone += 1, + .invalid => self.stats.invalid += 1, + .failed => self.stats.failed += 1, + } + self.mutex.unlock(io); + } + + if (write_failures == 0) return; + self.mutex.lockUncancelable(io); + self.stats.write_failures += write_failures; + self.mutex.unlock(io); + } + + const Attempt = struct { + outcome: Outcome, + learned: clients_repo.LearnedName, + }; + + /// One address: reverse name, zone match, exchange, classification. Any + /// learned text lives in `learned_buf`, which the caller owns until the row + /// is written. + fn attempt(self: *Resolver, io: std.Io, ip: []const u8, learned_buf: *[types.max_name_len]u8) Attempt { + // A stored address nxdns did not write. Nothing can be asked about it. + const addr = address.NetAddress.parse(ip) catch return .{ .outcome = .failed, .learned = .keep }; + + var reverse_buf: [reverse_name.max_reverse_len]u8 = undefined; + const reverse = reverse_name.reverseName(addr, &reverse_buf); + + // The zone's resolver is copied by value under the shared lock and the + // handle is released before any socket work: `match` returns a pointer + // into the current generation, and `swap` frees that generation as soon + // as it holds the exclusive lock. + const resolver: validate.Resolver = resolver: { + const handle = self.tables.acquire(io); + defer handle.release(io); + const zone = handle.zones.match(reverse) orelse + return .{ .outcome = .no_zone, .learned = .keep }; + break :resolver zone.resolver; + }; + + const query_name = name_mod.fromText(reverse) catch + return .{ .outcome = .failed, .learned = .keep }; + + var query_buf: [query_buf_len]u8 = undefined; + const query = encodeQuery(io, query_name, &query_buf) catch + return .{ .outcome = .failed, .learned = .keep }; + + var response_buf: [response_buf_len]u8 = undefined; + const reply = self.exchange_fn(io, resolver, query, &response_buf) catch + return .{ .outcome = .failed, .learned = .keep }; + + return classify(query_name, reply, learned_buf); + } +}; + +/// A definitive outcome refreshes daily; everything else retries within the +/// hour. Saturating, so a clock far in the future cannot overflow the column. +fn nextAttempt(now_s: i64, outcome: Outcome) i64 { + const delta: i64 = switch (outcome) { + .answered, .nxdomain => Resolver.refresh_after_s, + .no_zone, .invalid, .failed => Resolver.retry_after_s, + }; + return now_s +| delta; +} + +/// One question, RD set, a random id from `io.random` — `std.crypto.random` does +/// not exist in 0.16.0. The id is visible to a test through the exchange seam, +/// so nothing else is injected for determinism. +fn encodeQuery(io: std.Io, query_name: name_mod.Name, buf: *[query_buf_len]u8) std.Io.Writer.Error![]const u8 { + var id_bytes: [2]u8 = undefined; + io.random(&id_bytes); + + dns_header.encode(.{ + .id = std.mem.readInt(u16, &id_bytes, .big), + .flags = .{ + .rcode = .no_error, + .z = 0, + .ra = false, + .rd = true, + .tc = false, + .aa = false, + .opcode = .query, + .qr = false, + }, + .qdcount = 1, + .ancount = 0, + .nscount = 0, + .arcount = 0, + }, buf[0..types.header_len]); + + var w: std.Io.Writer = .fixed(buf[types.header_len..]); + try question.encode(.{ .name = query_name, .qtype = .ptr, .qclass = .in }, &w); + return buf[0 .. types.header_len + w.buffered().len]; +} + +/// The three validation layers of ruling 4. Layer 1 (ID and question echo) ran +/// inside the exchange; layers 2 and 3 are here. +fn classify( + query_name: name_mod.Name, + reply: []const u8, + learned_buf: *[types.max_name_len]u8, +) Resolver.Attempt { + const parsed = packet.parse(reply) catch return .{ .outcome = .failed, .learned = .keep }; + + // The RCODE is the full 12-bit value: the header's four bits extended by the + // OPT record's upper eight (RFC 6891 §6.1.3). Classifying on the header + // nibble alone would store or clear a name on a reply the resolver called an + // error — a NOERROR header under, say, extended RCODE 1 (BADVERS's neighbour + // in the extended space) carries no usable answer. An OPT that does not + // parse makes the whole reply malformed. + const opt: ?edns.OptRecord = if (packet.findOptRecord(parsed)) |rec| + edns.parseOpt(reply, rec) catch return .{ .outcome = .failed, .learned = .keep } + else + null; + + switch (edns.extendedRcode(parsed.header.flags.rcode, opt)) { + // The router affirmatively says the address has no name. Keeping a stale + // one would show a device under its previous owner's hostname. + @intFromEnum(types.Rcode.nx_domain) => return .{ .outcome = .nxdomain, .learned = .clear }, + @intFromEnum(types.Rcode.no_error) => {}, + // REFUSED, FORMERR, SERVFAIL, every extended value, anything unknown: an + // outage must not strip names from the whole dashboard. + else => return .{ .outcome = .failed, .learned = .keep }, + } + + var answers = packet.answers(parsed); + while (answers.next() catch return .{ .outcome = .failed, .learned = .keep }) |rec| { + // Layer 2: the first PTR record of class IN whose owner name is the one + // that was asked about. A record failing any of these is skipped, not an + // error; records after the first accepted one are ignored. + if (rec.rtype != .ptr) continue; + if (rec.class != @intFromEnum(types.Class.in)) continue; + if (!name_mod.eqlIgnoreCase(rec.name, query_name)) continue; + + // Layer 3: the target decodes and looks like a hostname. + const target = record.rdataCname(reply, rec) catch + return .{ .outcome = .invalid, .learned = .keep }; + + var w: std.Io.Writer = .fixed(learned_buf); + name_mod.formatText(target, &w) catch + return .{ .outcome = .invalid, .learned = .keep }; + const text = w.buffered(); + for (learned_buf[0..text.len]) |*byte| byte.* = std.ascii.toLower(byte.*); + if (!reverse_name.acceptHostname(text)) return .{ .outcome = .invalid, .learned = .keep }; + + return .{ .outcome = .answered, .learned = .{ .store = text } }; + } + + // NOERROR with no accepted record is NODATA: the router says as + // affirmatively as NXDOMAIN does that the address carries no name, so the + // outcome is definitive and clears. + return .{ .outcome = .nxdomain, .learned = .clear }; +} + +/// The production exchange: a stack `ForwardClient` at `ptr_read_timeout`. The +/// client's own `debug` diagnostics predate this milestone's no-log rule and +/// stand as they are. +/// +/// `ForwardClient` hands its duration to the UDP receive and then again to the +/// TCP fallback, so a resolver that answers UDP late with TC=1 and then stalls +/// TCP would spend up to twice the budget on one attempt. `raceWithin` — the +/// mechanism `exchangeTcp` itself uses (forward_client.zig:195) — puts UDP, +/// the fallback and all under one deadline, which is what makes 16 × 2 s a +/// bound on the pass rather than an estimate. +fn defaultExchange( + io: std.Io, + resolver: validate.Resolver, + query: []const u8, + response_buf: []u8, +) transport.ExchangeError![]u8 { + return exchangeWithin(io, Resolver.ptr_read_timeout, resolver, query, response_buf); +} + +/// The budget is a parameter rather than the constant so that a test can put a +/// short one in front of the same code: the deadline is the thing under test, +/// and a case that waits out two real seconds twice over is a case nobody runs. +fn exchangeWithin( + io: std.Io, + budget: std.Io.Clock.Duration, + resolver: validate.Resolver, + query: []const u8, + response_buf: []u8, +) transport.ExchangeError![]u8 { + return transport.raceWithin(io, budget, exchangeOnce, .{ io, budget, resolver, query, response_buf }); +} + +fn exchangeOnce( + io: std.Io, + budget: std.Io.Clock.Duration, + resolver: validate.Resolver, + query: []const u8, + response_buf: []u8, +) transport.ExchangeError![]u8 { + var frame_buf: [forward_client.min_frame_buf]u8 = undefined; + var client: forward_client.ForwardClient = .init(resolver, &frame_buf, budget); + return client.exchange(io, query, response_buf); +} + +// --------------------------------------------------------------------------- +// tests +// --------------------------------------------------------------------------- + +const forward_zones = @import("../local/forward_zones.zig"); +const migrations = @import("../storage/migrations.zig"); +const testing = std.testing; + +/// A stub exchange whose reply and error are set by the test. `calls` is the +/// assertion for "no query was sent to anyone". +const Stub = struct { + var calls: usize = 0; + var last_query: [query_buf_len]u8 = undefined; + var last_query_len: usize = 0; + var replies: []const []const u8 = &.{}; + var fail: ?transport.ExchangeError = null; + + fn reset() void { + calls = 0; + last_query_len = 0; + replies = &.{}; + fail = null; + } + + fn exchange( + _: std.Io, + _: validate.Resolver, + query: []const u8, + response_buf: []u8, + ) transport.ExchangeError![]u8 { + @memcpy(last_query[0..query.len], query); + last_query_len = query.len; + const index = calls; + calls += 1; + if (fail) |err| return err; + const reply = replies[@min(index, replies.len - 1)]; + // The id is the query's, so `validateResponse`-shaped checks in the + // production path would pass on these bytes too. + @memcpy(response_buf[0..reply.len], reply); + std.mem.writeInt(u16, response_buf[0..2], std.mem.readInt(u16, query[0..2], .big), .big); + return response_buf[0..reply.len]; + } +}; + +/// Builds a PTR reply for `owner`: `rcode`, and one record per entry in +/// `records`. Written into `buf`, which the caller keeps alive. +const ReplyRecord = struct { + owner: []const u8, + rtype: types.Type = .ptr, + class: u16 = 1, + target: []const u8, +}; + +fn buildReply( + buf: []u8, + question_name: []const u8, + rcode: types.Rcode, + records: []const ReplyRecord, +) ![]const u8 { + var w: std.Io.Writer = .fixed(buf); + var head: [types.header_len]u8 = undefined; + dns_header.encode(.{ + .id = 0, + .flags = .{ + .rcode = rcode, + .z = 0, + .ra = true, + .rd = true, + .tc = false, + .aa = false, + .opcode = .query, + .qr = true, + }, + .qdcount = 1, + .ancount = @intCast(records.len), + .nscount = 0, + .arcount = 0, + }, &head); + try w.writeAll(&head); + + try question.encode(.{ + .name = try name_mod.fromText(question_name), + .qtype = .ptr, + .qclass = .in, + }, &w); + + for (records) |r| { + var rdata_buf: [types.max_name_len]u8 = undefined; + var rdata_w: std.Io.Writer = .fixed(&rdata_buf); + try name_mod.encode(try name_mod.fromText(r.target), &rdata_w); + try record.encode(.{ + .name = try name_mod.fromText(r.owner), + .rtype = r.rtype, + .class = r.class, + .ttl = 300, + .rdata = .{ .offset = 0, .len = 0 }, + }, rdata_w.buffered(), &w); + } + return w.buffered(); +} + +/// `buildReply` plus an OPT record carrying `extended_rcode`, so a reply can say +/// NOERROR in its header and something else in the twelve bits that count. +fn buildReplyWithOpt( + buf: []u8, + question_name: []const u8, + rcode: types.Rcode, + records: []const ReplyRecord, + extended_rcode: u8, +) ![]const u8 { + const without_opt = try buildReply(buf, question_name, rcode, records); + var w: std.Io.Writer = .fixed(buf[without_opt.len..]); + try edns.encodeOpt(.{ + .udp_payload_size = 1232, + .extended_rcode = extended_rcode, + .version = 0, + .do_bit = false, + .options = .{ .offset = 0, .len = 0 }, + }, &.{}, &w); + + const total = without_opt.len + w.buffered().len; + var h = try dns_header.parse(buf[0..total]); + h.arcount = 1; + dns_header.encode(h, buf[0..types.header_len]); + return buf[0..total]; +} + +const Fixture = struct { + threaded: std.Io.Threaded, + database: db.Db, + tables: local_tables.LocalTables, + resolver: Resolver, + + fn io(self: *Fixture) std.Io { + return self.threaded.io(); + } + + fn deinit(self: *Fixture) void { + self.tables.deinit(testing.allocator); + self.database.close(); + self.threaded.deinit(); + } + + fn learned(self: *Fixture, ip: []const u8, buf: []u8) !?[]const u8 { + var stmt = try self.database.prepare("SELECT learned_name FROM clients WHERE ip = ?1"); + defer stmt.deinit(); + try stmt.bindText(1, ip); + try testing.expect(try stmt.step()); + if (stmt.isNull(0)) return null; + const text = stmt.columnText(0); + @memcpy(buf[0..text.len], text); + return buf[0..text.len]; + } +}; + +/// A fixture whose forward zones cover `168.192.in-addr.arpa`, with the stub +/// wired in. +fn fixture(f: *Fixture, zone: ?[]const u8) !void { + f.threaded = .init(testing.allocator, .{}); + f.database = try db.Db.open(":memory:", .{ .mode = .memory }); + try db.applyPragmas(&f.database, .{}); + _ = try migrations.migrate(&f.database); + f.tables = .empty; + if (zone) |z| { + f.tables.swap(f.threaded.io(), testing.allocator, .empty, try forward_zones.Zones.build( + testing.allocator, + &.{.{ .zone = z, .resolver = "udp://192.168.1.1:53" }}, + )); + } + f.resolver = .init(&f.tables); + f.resolver.exchange_fn = Stub.exchange; + Stub.reset(); +} + +fn attemptAfterOf(database: *db.Db, ip: []const u8) !i64 { + var stmt = try database.prepare("SELECT name_attempt_after FROM clients WHERE ip = ?1"); + defer stmt.deinit(); + try stmt.bindText(1, ip); + try testing.expect(try stmt.step()); + return stmt.columnInt(0); +} + +test "an answered PTR stores the target lowercased and refreshes daily" { + var f: Fixture = undefined; + try fixture(&f, "168.192.in-addr.arpa"); + defer f.deinit(); + + try clients_repo.upsertSeen(&f.database, "192.168.1.10", 1700000000); + + var reply_buf: [512]u8 = undefined; + const reply = try buildReply(&reply_buf, "10.1.168.192.in-addr.arpa", .no_error, &.{ + .{ .owner = "10.1.168.192.in-addr.arpa", .target = "NAS-1.Lan" }, + }); + const replies = [_][]const u8{reply}; + Stub.replies = &replies; + + f.resolver.runPass(f.io(), &f.database, 1700000000); + + var buf: [253]u8 = undefined; + try testing.expectEqualStrings("nas-1.lan", (try f.learned("192.168.1.10", &buf)).?); + try testing.expectEqual( + @as(i64, 1700000000 + Resolver.refresh_after_s), + try attemptAfterOf(&f.database, "192.168.1.10"), + ); + + const stats = f.resolver.snapshotStats(f.io()); + try testing.expectEqual(@as(u64, 1), stats.attempted); + try testing.expectEqual(@as(u64, 1), stats.answered); + try testing.expectEqual(@as(usize, 1), Stub.calls); + + // The query is a PTR question for the reverse name, with RD set. + const sent = Stub.last_query[0..Stub.last_query_len]; + const parsed = try packet.parse(sent); + try testing.expect(parsed.header.flags.rd); + try testing.expectEqual(@as(u16, 1), parsed.header.qdcount); + const q = packet.firstQuestion(parsed).?; + try testing.expectEqual(types.Type.ptr, q.qtype); + try testing.expectEqual(types.Class.in, q.qclass); + var text: std.Io.Writer.Allocating = .init(testing.allocator); + defer text.deinit(); + try name_mod.formatText(q.name, &text.writer); + try testing.expectEqualStrings("10.1.168.192.in-addr.arpa", text.written()); +} + +test "a second answer overwrites the first" { + var f: Fixture = undefined; + try fixture(&f, "168.192.in-addr.arpa"); + defer f.deinit(); + + try clients_repo.upsertSeen(&f.database, "192.168.1.10", 1700000000); + + var first_buf: [512]u8 = undefined; + var second_buf: [512]u8 = undefined; + const replies = [_][]const u8{ + try buildReply(&first_buf, "10.1.168.192.in-addr.arpa", .no_error, &.{ + .{ .owner = "10.1.168.192.in-addr.arpa", .target = "old.lan" }, + }), + try buildReply(&second_buf, "10.1.168.192.in-addr.arpa", .no_error, &.{ + .{ .owner = "10.1.168.192.in-addr.arpa", .target = "new.lan" }, + }), + }; + Stub.replies = &replies; + + f.resolver.runPass(f.io(), &f.database, 1700000000); + var buf: [253]u8 = undefined; + try testing.expectEqualStrings("old.lan", (try f.learned("192.168.1.10", &buf)).?); + + f.resolver.runPass(f.io(), &f.database, 1700000000 + Resolver.refresh_after_s); + try testing.expectEqualStrings("new.lan", (try f.learned("192.168.1.10", &buf)).?); + try testing.expectEqual(@as(usize, 2), Stub.calls); +} + +test "NXDOMAIN and NODATA both clear the learned name" { + for ([_]types.Rcode{ .nx_domain, .no_error }) |rcode| { + var f: Fixture = undefined; + try fixture(&f, "168.192.in-addr.arpa"); + defer f.deinit(); + + try clients_repo.upsertSeen(&f.database, "192.168.1.10", 1700000000); + try clients_repo.noteNameOutcome(&f.database, "192.168.1.10", .{ + .attempt_after = 0, + .learned = .{ .store = "stale.lan" }, + }); + + var reply_buf: [512]u8 = undefined; + const replies = [_][]const u8{ + try buildReply(&reply_buf, "10.1.168.192.in-addr.arpa", rcode, &.{}), + }; + Stub.replies = &replies; + + f.resolver.runPass(f.io(), &f.database, 1700000000); + + var buf: [253]u8 = undefined; + try testing.expectEqual(@as(?[]const u8, null), try f.learned("192.168.1.10", &buf)); + try testing.expectEqual(@as(u64, 1), f.resolver.snapshotStats(f.io()).nxdomain); + // Definitive: the daily refresh, not the hourly retry. + try testing.expectEqual( + @as(i64, 1700000000 + Resolver.refresh_after_s), + try attemptAfterOf(&f.database, "192.168.1.10"), + ); + } +} + +test "REFUSED, an unknown rcode and a transport failure all keep the stored name" { + const Case = struct { rcode: ?types.Rcode, fail: ?transport.ExchangeError }; + const cases = [_]Case{ + .{ .rcode = .refused, .fail = null }, + .{ .rcode = @enumFromInt(11), .fail = null }, + .{ .rcode = null, .fail = error.Timeout }, + .{ .rcode = null, .fail = error.ConnectFailed }, + }; + + for (cases) |case| { + var f: Fixture = undefined; + try fixture(&f, "168.192.in-addr.arpa"); + defer f.deinit(); + + try clients_repo.upsertSeen(&f.database, "192.168.1.10", 1700000000); + try clients_repo.noteNameOutcome(&f.database, "192.168.1.10", .{ + .attempt_after = 0, + .learned = .{ .store = "keep.lan" }, + }); + + var reply_buf: [512]u8 = undefined; + var replies: [1][]const u8 = undefined; + if (case.rcode) |rcode| { + replies[0] = try buildReply(&reply_buf, "10.1.168.192.in-addr.arpa", rcode, &.{}); + Stub.replies = &replies; + } + Stub.fail = case.fail; + + f.resolver.runPass(f.io(), &f.database, 1700000000); + + var buf: [253]u8 = undefined; + try testing.expectEqualStrings("keep.lan", (try f.learned("192.168.1.10", &buf)).?); + const stats = f.resolver.snapshotStats(f.io()); + try testing.expectEqual(@as(u64, 1), stats.failed); + try testing.expectEqual( + @as(i64, 1700000000 + Resolver.retry_after_s), + try attemptAfterOf(&f.database, "192.168.1.10"), + ); + } +} + +test "a target that fails acceptHostname counts invalid and keeps the stored name" { + var f: Fixture = undefined; + try fixture(&f, "168.192.in-addr.arpa"); + defer f.deinit(); + + try clients_repo.upsertSeen(&f.database, "192.168.1.10", 1700000000); + try clients_repo.noteNameOutcome(&f.database, "192.168.1.10", .{ + .attempt_after = 0, + .learned = .{ .store = "keep.lan" }, + }); + + var reply_buf: [512]u8 = undefined; + const replies = [_][]const u8{ + try buildReply(&reply_buf, "10.1.168.192.in-addr.arpa", .no_error, &.{ + .{ .owner = "10.1.168.192.in-addr.arpa", .target = "-bad.lan" }, + }), + }; + Stub.replies = &replies; + + f.resolver.runPass(f.io(), &f.database, 1700000000); + + var buf: [253]u8 = undefined; + try testing.expectEqualStrings("keep.lan", (try f.learned("192.168.1.10", &buf)).?); + const stats = f.resolver.snapshotStats(f.io()); + try testing.expectEqual(@as(u64, 1), stats.invalid); + try testing.expectEqual(@as(u64, 0), stats.answered); +} + +test "a record with the wrong type, class or owner is skipped" { + var f: Fixture = undefined; + try fixture(&f, "168.192.in-addr.arpa"); + defer f.deinit(); + + try clients_repo.upsertSeen(&f.database, "192.168.1.10", 1700000000); + + var reply_buf: [1024]u8 = undefined; + const replies = [_][]const u8{ + try buildReply(&reply_buf, "10.1.168.192.in-addr.arpa", .no_error, &.{ + .{ .owner = "10.1.168.192.in-addr.arpa", .rtype = .cname, .target = "wrongtype.lan" }, + .{ .owner = "10.1.168.192.in-addr.arpa", .class = 3, .target = "wrongclass.lan" }, + .{ .owner = "99.1.168.192.in-addr.arpa", .target = "wrongowner.lan" }, + .{ .owner = "10.1.168.192.IN-ADDR.ARPA", .target = "right.lan" }, + .{ .owner = "10.1.168.192.in-addr.arpa", .target = "second.lan" }, + }), + }; + Stub.replies = &replies; + + f.resolver.runPass(f.io(), &f.database, 1700000000); + + // The owner name matches case-insensitively, and records after the first + // accepted one are ignored. + var buf: [253]u8 = undefined; + try testing.expectEqualStrings("right.lan", (try f.learned("192.168.1.10", &buf)).?); +} + +test "an uncovered reverse name sends nothing to anyone and counts no_zone" { + var f: Fixture = undefined; + try fixture(&f, "168.192.in-addr.arpa"); + defer f.deinit(); + + try clients_repo.upsertSeen(&f.database, "10.0.0.5", 1700000000); + + f.resolver.runPass(f.io(), &f.database, 1700000000); + + try testing.expectEqual(@as(usize, 0), Stub.calls); + const stats = f.resolver.snapshotStats(f.io()); + try testing.expectEqual(@as(u64, 1), stats.attempted); + try testing.expectEqual(@as(u64, 1), stats.no_zone); + try testing.expectEqual( + @as(i64, 1700000000 + Resolver.retry_after_s), + try attemptAfterOf(&f.database, "10.0.0.5"), + ); +} + +test "with no zones declared at all nothing is sent" { + var f: Fixture = undefined; + try fixture(&f, null); + defer f.deinit(); + + try clients_repo.upsertSeen(&f.database, "192.168.1.10", 1700000000); + f.resolver.runPass(f.io(), &f.database, 1700000000); + + try testing.expectEqual(@as(usize, 0), Stub.calls); + try testing.expectEqual(@as(u64, 1), f.resolver.snapshotStats(f.io()).no_zone); +} + +test "a named row is never attempted and the candidate cap holds" { + var f: Fixture = undefined; + try fixture(&f, "168.192.in-addr.arpa"); + defer f.deinit(); + + var reply_buf: [512]u8 = undefined; + const replies = [_][]const u8{ + // The owner never matches, so every attempt is NODATA — the point here + // is how many attempts happen, not what they decide. + try buildReply(&reply_buf, "10.1.168.192.in-addr.arpa", .no_error, &.{}), + }; + Stub.replies = &replies; + + for (0..clients_repo.max_per_pass + 4) |i| { + var ip_buf: [64]u8 = undefined; + const ip = try std.fmt.bufPrint(&ip_buf, "192.168.2.{d}", .{i}); + try clients_repo.upsertSeen(&f.database, ip, 1700000000); + } + _ = try clients_repo.insertClientRow(&f.database, .{ + .ip = "192.168.3.1", + .name = "named", + .group_id = 1, + }, 1700000000); + + f.resolver.runPass(f.io(), &f.database, 1700000000); + + try testing.expectEqual(clients_repo.max_per_pass, Stub.calls); + try testing.expectEqual(@as(u64, clients_repo.max_per_pass), f.resolver.snapshotStats(f.io()).attempted); + try testing.expectEqual(@as(i64, 0), try attemptAfterOf(&f.database, "192.168.3.1")); +} + +test "attempted equals the sum of the five outcome counters" { + var f: Fixture = undefined; + try fixture(&f, "168.192.in-addr.arpa"); + defer f.deinit(); + + var ok_buf: [512]u8 = undefined; + var nodata_buf: [512]u8 = undefined; + var bad_buf: [512]u8 = undefined; + var refused_buf: [512]u8 = undefined; + const replies = [_][]const u8{ + try buildReply(&ok_buf, "10.1.168.192.in-addr.arpa", .no_error, &.{ + .{ .owner = "10.1.168.192.in-addr.arpa", .target = "nas.lan" }, + }), + try buildReply(&nodata_buf, "11.1.168.192.in-addr.arpa", .no_error, &.{}), + try buildReply(&bad_buf, "12.1.168.192.in-addr.arpa", .no_error, &.{ + .{ .owner = "12.1.168.192.in-addr.arpa", .target = "-bad.lan" }, + }), + try buildReply(&refused_buf, "13.1.168.192.in-addr.arpa", .refused, &.{}), + }; + Stub.replies = &replies; + + // Covered by the zone, in ip order, plus one that no zone covers. + try clients_repo.upsertSeen(&f.database, "192.168.1.10", 1700000000); + try clients_repo.upsertSeen(&f.database, "192.168.1.11", 1700000000); + try clients_repo.upsertSeen(&f.database, "192.168.1.12", 1700000000); + try clients_repo.upsertSeen(&f.database, "192.168.1.13", 1700000000); + try clients_repo.upsertSeen(&f.database, "10.0.0.1", 1700000000); + + f.resolver.runPass(f.io(), &f.database, 1700000000); + + const s = f.resolver.snapshotStats(f.io()); + try testing.expectEqual(@as(u64, 5), s.attempted); + try testing.expectEqual(s.attempted, s.answered + s.nxdomain + s.no_zone + s.invalid + s.failed); + try testing.expectEqual(@as(u64, 1), s.answered); + try testing.expectEqual(@as(u64, 1), s.nxdomain); + try testing.expectEqual(@as(u64, 1), s.invalid); + try testing.expectEqual(@as(u64, 1), s.failed); + try testing.expectEqual(@as(u64, 1), s.no_zone); + // The two failure counters sit outside the outcome sum. + try testing.expectEqual(@as(u64, 0), s.read_failures); + try testing.expectEqual(@as(u64, 0), s.write_failures); +} + +test "a failing candidate select skips the pass and counts read_failures" { + var f: Fixture = undefined; + try fixture(&f, "168.192.in-addr.arpa"); + defer f.deinit(); + + try clients_repo.upsertSeen(&f.database, "192.168.1.10", 1700000000); + try f.database.exec("DROP TABLE clients;"); + + f.resolver.runPass(f.io(), &f.database, 1700000000); + + try testing.expectEqual(@as(usize, 0), Stub.calls); + const s = f.resolver.snapshotStats(f.io()); + try testing.expectEqual(@as(u64, 1), s.read_failures); + try testing.expectEqual(@as(u64, 0), s.attempted); +} + +test "a failing write counts write_failures and still counts the outcome" { + var f: Fixture = undefined; + try fixture(&f, "168.192.in-addr.arpa"); + defer f.deinit(); + + try clients_repo.upsertSeen(&f.database, "10.0.0.1", 1700000000); + try f.database.exec( + \\CREATE TRIGGER refuse_update BEFORE UPDATE ON clients + \\BEGIN SELECT RAISE(ABORT, 'refused'); END; + ); + + f.resolver.runPass(f.io(), &f.database, 1700000000); + + const s = f.resolver.snapshotStats(f.io()); + try testing.expectEqual(@as(u64, 1), s.write_failures); + try testing.expectEqual(@as(u64, 1), s.attempted); + try testing.expectEqual(@as(u64, 1), s.no_zone); +} + +/// Swaps a whole new zone generation in during the first attempt of a pass, and +/// records the resolver each attempt was handed. Both assertions read the port, +/// because the two generations differ only there. +const Swapping = struct { + const old_port: u16 = 5301; + const new_port: u16 = 5302; + + var tables: *local_tables.LocalTables = undefined; + var io_ref: std.Io = undefined; + var ports: [2]u16 = .{ 0, 0 }; + var calls: usize = 0; + + fn exchange( + io: std.Io, + resolver: validate.Resolver, + query: []const u8, + response_buf: []u8, + ) transport.ExchangeError![]u8 { + const index = calls; + calls += 1; + + // Inside the hazard window: the handle has been released, so `swap` + // takes the exclusive lock immediately and frees the generation the + // first attempt matched against. A `*const Zone` retained across the + // release would now point into freed memory. + if (index == 0) { + tables.swap(io_ref, testing.allocator, .empty, forward_zones.Zones.build( + testing.allocator, + &.{.{ .zone = "168.192.in-addr.arpa", .resolver = "udp://192.168.1.1:5302" }}, + ) catch return error.SystemResources); + } + + // Read *after* the swap, so a stale pointer shows up as a wrong port. + if (index < ports.len) ports[index] = resolver.port; + return Stub.exchange(io, resolver, query, response_buf); + } +}; + +test "each attempt re-acquires, and a swap mid-pass cannot reach the copy in flight" { + var f: Fixture = undefined; + try fixture(&f, null); + defer f.deinit(); + + // The old generation: the same zone, on the old port. + f.tables.swap(f.io(), testing.allocator, .empty, try forward_zones.Zones.build( + testing.allocator, + &.{.{ .zone = "168.192.in-addr.arpa", .resolver = "udp://192.168.1.1:5301" }}, + )); + + // Two candidates in one pass: the first runs against the old generation, the + // second must see the new one. + try clients_repo.upsertSeen(&f.database, "192.168.1.10", 1700000000); + try clients_repo.upsertSeen(&f.database, "192.168.1.11", 1700000000); + + var reply_buf: [512]u8 = undefined; + const replies = [_][]const u8{ + try buildReply(&reply_buf, "10.1.168.192.in-addr.arpa", .no_error, &.{}), + }; + Stub.replies = &replies; + + Swapping.tables = &f.tables; + Swapping.io_ref = f.io(); + Swapping.ports = .{ 0, 0 }; + Swapping.calls = 0; + f.resolver.exchange_fn = Swapping.exchange; + + f.resolver.runPass(f.io(), &f.database, 1700000000); + + try testing.expectEqual(@as(usize, 2), Swapping.calls); + // The copy, not the freed generation: the first attempt still names the port + // it matched, after the table it came from is gone. + try testing.expectEqual(Swapping.old_port, Swapping.ports[0]); + // The re-acquire: the second attempt sees what the operator just published, + // so no handle and no table pointer survived the first attempt. + try testing.expectEqual(Swapping.new_port, Swapping.ports[1]); +} + +test "the retry cadences are the constants the milestone fixes" { + try testing.expectEqual(@as(i64, 86_400), Resolver.refresh_after_s); + try testing.expectEqual(@as(i64, 3_600), Resolver.retry_after_s); + try testing.expectEqual(@as(i96, 2 * std.time.ns_per_s), Resolver.ptr_read_timeout.raw.nanoseconds); + try testing.expectEqual(std.Io.Clock.awake, Resolver.ptr_read_timeout.clock); + + // A clock far in the future cannot overflow the column. + try testing.expectEqual(std.math.maxInt(i64), nextAttempt(std.math.maxInt(i64), .answered)); +} + +// --- the production exchange, over a loopback socket ------------------------ + +const build_options = @import("build_options"); +const net = std.Io.net; + +/// A PTR responder on 127.0.0.1: one answer naming whatever was asked about. +fn loopbackPtrResolver(io: std.Io, socket: *const net.Socket, calls: *std.atomic.Value(u64)) void { + var buf: [2048]u8 = undefined; + while (true) { + const msg = socket.receive(io, &buf) catch return; + _ = calls.fetchAdd(1, .monotonic); + + const request = packet.parse(msg.data) catch continue; + const q = packet.firstQuestion(request) orelse continue; + + var rdata_buf: [types.max_name_len]u8 = undefined; + var rdata_w: std.Io.Writer = .fixed(&rdata_buf); + name_mod.encode(name_mod.fromText("loop-host.lan") catch continue, &rdata_w) catch continue; + + var reply_buf: [512]u8 = undefined; + var b = packet.ResponseBuilder.init(&reply_buf, request.header, q) catch continue; + b.addAnswer(q.name, .ptr, .in, 300, rdata_w.buffered()) catch continue; + socket.send(io, &msg.from, b.finish()) catch return; + } +} + +test "the production exchange reaches a real resolver over UDP" { + if (!build_options.integration) return error.SkipZigTest; + + var f: Fixture = undefined; + // The zone is rebuilt below against the socket's real port. + try fixture(&f, null); + defer f.deinit(); + const io = f.io(); + + const bind_address: net.IpAddress = try .parse("127.0.0.1", 0); + const socket = try bind_address.bind(io, .{ .mode = .dgram }); + defer socket.close(io); + + var calls: std.atomic.Value(u64) = .init(0); + var group: std.Io.Group = .init; + defer group.cancel(io); + try group.concurrent(io, loopbackPtrResolver, .{ io, &socket, &calls }); + + var url_buf: [64]u8 = undefined; + const url = try std.fmt.bufPrint(&url_buf, "udp://127.0.0.1:{d}", .{socket.address.ip4.port}); + f.tables.swap(io, testing.allocator, .empty, try forward_zones.Zones.build( + testing.allocator, + &.{.{ .zone = "168.192.in-addr.arpa", .resolver = url }}, + )); + + // The seam's production default, not the stub. + f.resolver.exchange_fn = defaultExchange; + try clients_repo.upsertSeen(&f.database, "192.168.1.10", 1700000000); + + f.resolver.runPass(io, &f.database, 1700000000); + + try testing.expectEqual(@as(u64, 1), calls.load(.monotonic)); + var buf: [253]u8 = undefined; + try testing.expectEqualStrings("loop-host.lan", (try f.learned("192.168.1.10", &buf)).?); + try testing.expectEqual(@as(u64, 1), f.resolver.snapshotStats(io).answered); +} + +/// Answers the first datagram late in the budget with TC=1, which sends the +/// client to TCP — where a listener that never accepts leaves it stalled. +fn truncatingThenStallingResolver(io: std.Io, socket: *const net.Socket) void { + var buf: [2048]u8 = undefined; + const msg = socket.receive(io, &buf) catch return; + const request = packet.parse(msg.data) catch return; + + (std.Io.Clock.Duration{ + .raw = .fromMilliseconds(one_deadline_late_ms), + .clock = .awake, + }).sleep(io) catch return; + + // The question has to be echoed: `transport.validateResponse` runs before + // the client reads the TC bit, so a bare header would come back as + // `BadResponse` and never reach the TCP fallback this case is about. + const q = packet.firstQuestion(request) orelse return; + var reply_buf: [512]u8 = undefined; + var b = packet.ResponseBuilder.init(&reply_buf, request.header, q) catch return; + const reply = b.finish(); + + var flags = dns_header.parse(reply) catch return; + flags.flags.tc = true; + dns_header.encode(flags, reply[0..types.header_len]); + socket.send(io, &msg.from, reply) catch return; +} + +const one_deadline_budget_ms = 400; +/// Late enough in the budget that a second, fresh budget for the TCP leg is +/// unmistakable in the elapsed time. +const one_deadline_late_ms = 300; + +test "one deadline covers the udp leg, the TC=1 fallback and the tcp leg" { + if (!build_options.integration) return error.SkipZigTest; + + var threaded: std.Io.Threaded = .init(testing.allocator, .{}); + defer threaded.deinit(); + const io = threaded.io(); + + const bind_address: net.IpAddress = try .parse("127.0.0.1", 0); + const socket = try bind_address.bind(io, .{ .mode = .dgram }); + defer socket.close(io); + const port = socket.address.ip4.port; + + // Bound but never accepted: the connect completes out of the kernel's + // backlog and the read then waits forever, which is the stall the second + // budget would be spent on. + const tcp_address: net.IpAddress = try .parse("127.0.0.1", port); + var tcp_listener = try tcp_address.listen(io, .{ .reuse_address = true }); + defer tcp_listener.deinit(io); + + var group: std.Io.Group = .init; + defer group.cancel(io); + try group.concurrent(io, truncatingThenStallingResolver, .{ io, &socket }); + + var url_buf: [64]u8 = undefined; + const url = try std.fmt.bufPrint(&url_buf, "udp://127.0.0.1:{d}", .{port}); + const resolver = try validate.parseResolver(url); + + var query_buf: [query_buf_len]u8 = undefined; + const query = try encodeQuery(io, try name_mod.fromText("10.1.168.192.in-addr.arpa"), &query_buf); + + var response_buf: [response_buf_len]u8 = undefined; + const one_budget: std.Io.Clock.Duration = .{ + .raw = .fromMilliseconds(one_deadline_budget_ms), + .clock = .awake, + }; + + const started = std.Io.Clock.awake.now(io); + try testing.expectError( + error.Timeout, + exchangeWithin(io, one_budget, resolver, query, &response_buf), + ); + const elapsed = started.durationTo(std.Io.Clock.awake.now(io)).nanoseconds; + + // The unwrapped path spends 300 ms on the UDP leg and then a fresh 400 ms + // budget on the stalled TCP leg, ~700 ms in all. The bound must therefore + // sit between one budget and that sum — 2× the budget would pass the very + // bug this test exists to catch. + try testing.expect(elapsed < @as(i96, one_deadline_budget_ms + one_deadline_late_ms / 2) * std.time.ns_per_ms); +} + +test "a nonzero extended rcode under a NOERROR header is a failure, not an answer" { + var f: Fixture = undefined; + try fixture(&f, "168.192.in-addr.arpa"); + defer f.deinit(); + + try clients_repo.upsertSeen(&f.database, "192.168.1.10", 1700000000); + try clients_repo.noteNameOutcome(&f.database, "192.168.1.10", .{ + .attempt_after = 0, + .learned = .{ .store = "keep.lan" }, + }); + + // The header says NOERROR and the answer section carries a perfectly good + // PTR record, so header-only classification would store `nas.lan`. The full + // twelve bits say otherwise (RFC 6891 §6.1.3). + var reply_buf: [512]u8 = undefined; + const replies = [_][]const u8{ + try buildReplyWithOpt(&reply_buf, "10.1.168.192.in-addr.arpa", .no_error, &.{ + .{ .owner = "10.1.168.192.in-addr.arpa", .target = "nas.lan" }, + }, 1), + }; + Stub.replies = &replies; + + f.resolver.runPass(f.io(), &f.database, 1700000000); + + var buf: [253]u8 = undefined; + try testing.expectEqualStrings("keep.lan", (try f.learned("192.168.1.10", &buf)).?); + const s = f.resolver.snapshotStats(f.io()); + try testing.expectEqual(@as(u64, 1), s.failed); + try testing.expectEqual(@as(u64, 0), s.answered); + try testing.expectEqual(@as(u64, 0), s.nxdomain); + // Non-definitive, so the hourly retry. + try testing.expectEqual( + @as(i64, 1700000000 + Resolver.retry_after_s), + try attemptAfterOf(&f.database, "192.168.1.10"), + ); +} + +test "an extended rcode of zero still reads as the header's rcode" { + var f: Fixture = undefined; + try fixture(&f, "168.192.in-addr.arpa"); + defer f.deinit(); + + try clients_repo.upsertSeen(&f.database, "192.168.1.10", 1700000000); + + var reply_buf: [512]u8 = undefined; + const replies = [_][]const u8{ + try buildReplyWithOpt(&reply_buf, "10.1.168.192.in-addr.arpa", .no_error, &.{ + .{ .owner = "10.1.168.192.in-addr.arpa", .target = "nas.lan" }, + }, 0), + }; + Stub.replies = &replies; + + f.resolver.runPass(f.io(), &f.database, 1700000000); + + var buf: [253]u8 = undefined; + try testing.expectEqualStrings("nas.lan", (try f.learned("192.168.1.10", &buf)).?); + try testing.expectEqual(@as(u64, 1), f.resolver.snapshotStats(f.io()).answered); +} diff --git a/src/server/clients.zig b/src/server/clients.zig index cec30d6..fdd82eb 100644 --- a/src/server/clients.zig +++ b/src/server/clients.zig @@ -21,6 +21,7 @@ const std = @import("std"); const address = @import("../platform/address.zig"); +const client_names = @import("client_names.zig"); const clients_repo = @import("../storage/repositories/clients_repo.zig"); const db = @import("../storage/db.zig"); const disk_monitor = @import("../storage/disk_monitor.zig"); @@ -135,6 +136,7 @@ pub const Tracker = struct { io: std.Io, database: *db.Db, monitor: ?*disk_monitor.Monitor, + names: ?*client_names.Resolver, ) std.Io.Cancelable!void { const interval: std.Io.Clock.Duration = .{ .raw = .fromSeconds(flush_interval_s), @@ -143,12 +145,18 @@ pub const Tracker = struct { while (true) { try interval.sleep(io); const writes_allowed = if (monitor) |m| m.writesAllowed() else true; - self.flushOnce(io, database, writes_allowed); + self.flushOnce(io, database, writes_allowed, names); } } - /// One pass: drain the table, write a row per client, and prune on every - /// `prune_every_passes`-th pass. + /// One pass: drain the table, write a row per client, prune on every + /// `prune_every_passes`-th pass, and then learn names for the rows that + /// have none (milestone-25 ruling 1). + /// + /// The order of the three steps is fixed and one `now_s` serves all three. + /// Resolving before pruning would spend PTR queries on rows the same pass + /// deletes; resolving before the drain would make a slow resolver delay the + /// writes the pass exists for. /// /// A gated pass does nothing at all, not even count: the work it skipped is /// still owed, and the pending addresses it leaves behind are re-tracked by @@ -161,9 +169,17 @@ pub const Tracker = struct { /// Only `run` may call this concurrently with itself — the drain buffer is /// this call's stack, but the pass counter and the prune schedule assume a /// single caller. - pub fn flushOnce(self: *Tracker, io: std.Io, database: *db.Db, writes_allowed: bool) void { + pub fn flushOnce( + self: *Tracker, + io: std.Io, + database: *db.Db, + writes_allowed: bool, + names: ?*client_names.Resolver, + ) void { + // A gated pass skips naming too: naming writes. if (!writes_allowed) return; + const now_s = std.Io.Clock.real.now(io).toSeconds(); var drained: [max_pending]Pending = undefined; const batch = self.drain(io, &drained); @@ -193,18 +209,21 @@ pub const Tracker = struct { const due = self.passes % prune_every_passes == 0; self.mutex.unlock(io); - if (!due) return; - const cutoff = std.Io.Clock.real.now(io).toSeconds() - @as(i64, self.retention_days) * 86_400; - if (clients_repo.pruneStale(database, cutoff)) |deleted| { - self.mutex.lockUncancelable(io); - self.stats.pruned += deleted; - self.mutex.unlock(io); - } else |err| { - log.warn("pruning clients before {d} failed: {s}", .{ cutoff, @errorName(err) }); - self.mutex.lockUncancelable(io); - self.stats.flush_failures += 1; - self.mutex.unlock(io); + if (due) { + const cutoff = now_s - @as(i64, self.retention_days) * 86_400; + if (clients_repo.pruneStale(database, cutoff)) |deleted| { + self.mutex.lockUncancelable(io); + self.stats.pruned += deleted; + self.mutex.unlock(io); + } else |err| { + log.warn("pruning clients before {d} failed: {s}", .{ cutoff, @errorName(err) }); + self.mutex.lockUncancelable(io); + self.stats.flush_failures += 1; + self.mutex.unlock(io); + } } + + if (names) |resolver| resolver.runPass(io, database, now_s); } pub fn snapshotStats(self: *Tracker, io: std.Io) Stats { @@ -277,7 +296,7 @@ test "a client tracked twice before a flush yields one row at the later time" { try testing.expectEqual(@as(u64, 0), tracker.trackAt(io, parsed("192.168.1.10"), 1700000030)); try testing.expectEqual(@as(u32, 1), tracker.pendingClients(io)); - tracker.flushOnce(io, &database, true); + tracker.flushOnce(io, &database, true, null); try testing.expectEqual(@as(i64, 1), try clients_repo.countClients(&database)); try testing.expectEqual(@as(i64, 1700000030), try lastSeen(&database, "192.168.1.10")); @@ -305,7 +324,7 @@ test "distinct clients each get a row and ipv6 text is canonical" { _ = tracker.trackAt(io, address.NetAddress.fromIp(try std.Io.net.IpAddress.parse("::ffff:192.168.1.10", 53)), 1700000003); try testing.expectEqual(@as(u32, 3), tracker.pendingClients(io)); - tracker.flushOnce(io, &database, true); + tracker.flushOnce(io, &database, true, null); try testing.expectEqual(@as(i64, 3), try clients_repo.countClients(&database)); try testing.expectEqual(@as(i64, 1700000003), try lastSeen(&database, "192.168.1.10")); @@ -343,7 +362,7 @@ test "a full table drops further clients and counts them" { // A tracked client still refreshes while the table is full, and the flush // makes room for the next newcomer. _ = tracker.trackAt(io, .{ .ip4 = .{ 0, 0, 0, 0 } }, 1700000060); - tracker.flushOnce(io, &database, true); + tracker.flushOnce(io, &database, true, null); try testing.expectEqual(@as(i64, Tracker.max_pending), try clients_repo.countClients(&database)); try testing.expectEqual(@as(i64, 1700000060), try lastSeen(&database, "0.0.0.0")); @@ -366,7 +385,7 @@ test "a flush touches a hand-edited row without changing what the operator set" var tracker: Tracker = .init(30); _ = tracker.trackAt(io, parsed("192.168.1.10"), 1700000000); - tracker.flushOnce(io, &database, true); + tracker.flushOnce(io, &database, true, null); try testing.expectEqual(@as(i64, 1), try clients_repo.countClients(&database)); try testing.expectEqual(@as(i64, 1700000000), try lastSeen(&database, "192.168.1.10")); @@ -394,7 +413,7 @@ test "a gated pass writes nothing and keeps the pending clients" { var tracker: Tracker = .init(30); _ = tracker.trackAt(io, parsed("192.168.1.10"), 1700000000); - tracker.flushOnce(io, &database, monitor.writesAllowed()); + tracker.flushOnce(io, &database, monitor.writesAllowed(), null); try testing.expectEqual(@as(i64, 0), try clients_repo.countClients(&database)); try testing.expectEqual(@as(u32, 1), tracker.pendingClients(io)); @@ -403,7 +422,7 @@ test "a gated pass writes nothing and keeps the pending clients" { // Free space recovers and the same pending client lands. monitor.state_raw.store(@intFromEnum(disk_monitor.State.warn), .monotonic); - tracker.flushOnce(io, &database, monitor.writesAllowed()); + tracker.flushOnce(io, &database, monitor.writesAllowed(), null); try testing.expectEqual(@as(i64, 1), try clients_repo.countClients(&database)); try testing.expectEqual(@as(u64, 1), tracker.passes); } @@ -423,7 +442,7 @@ test "a failing upsert counts and leaves the client to be tracked again" { var tracker: Tracker = .init(30); _ = tracker.trackAt(io, parsed("192.168.1.10"), 1700000000); _ = tracker.trackAt(io, parsed("192.168.1.11"), 1700000000); - tracker.flushOnce(io, &database, true); + tracker.flushOnce(io, &database, true, null); try testing.expectEqual(@as(i64, 0), try clients_repo.countClients(&database)); const stats = tracker.snapshotStats(io); @@ -433,7 +452,7 @@ test "a failing upsert counts and leaves the client to be tracked again" { try database.exec("DROP TRIGGER refuse_insert;"); _ = tracker.trackAt(io, parsed("192.168.1.10"), 1700000060); - tracker.flushOnce(io, &database, true); + tracker.flushOnce(io, &database, true, null); try testing.expectEqual(@as(i64, 1), try clients_repo.countClients(&database)); } @@ -453,12 +472,12 @@ test "the pass that comes due prunes the clients that went quiet" { var tracker: Tracker = .init(30); // Every pass before the due one leaves both rows alone. for (0..Tracker.prune_every_passes - 1) |_| { - tracker.flushOnce(io, &database, true); + tracker.flushOnce(io, &database, true, null); } try testing.expectEqual(@as(i64, 2), try clients_repo.countClients(&database)); try testing.expectEqual(@as(u64, 0), tracker.snapshotStats(io).pruned); - tracker.flushOnce(io, &database, true); + tracker.flushOnce(io, &database, true, null); try testing.expectEqual(@as(i64, 1), try clients_repo.countClients(&database)); try testing.expectEqual(@as(i64, now - 29 * day), try lastSeen(&database, "10.0.0.2")); @@ -479,7 +498,7 @@ test "a shorter retention prunes what the default keeps" { var tracker: Tracker = .init(1); tracker.passes = Tracker.prune_every_passes - 1; - tracker.flushOnce(io, &database, true); + tracker.flushOnce(io, &database, true, null); try testing.expectEqual(@as(i64, 0), try clients_repo.countClients(&database)); try testing.expectEqual(@as(u64, 1), tracker.snapshotStats(io).pruned); @@ -501,6 +520,7 @@ test "the run loop flushes on its interval and returns on cancel" { io, &database, @as(?*disk_monitor.Monitor, null), + @as(?*client_names.Resolver, null), }); // The first flush is one interval away, so cancelling immediately proves the // loop starts by sleeping rather than by writing. @@ -508,3 +528,124 @@ test "the run loop flushes on its interval and returns on cancel" { try testing.expectEqual(@as(i64, 0), try clients_repo.countClients(&database)); try testing.expectEqual(@as(u32, 1), tracker.pendingClients(io)); } + +// --- the naming step's place in the pass (milestone-25 ruling 1) ------------ + +const forward_zones = @import("../local/forward_zones.zig"); +const local_tables = @import("local_tables.zig"); +const validate = @import("../config/validate.zig"); + +/// Counts exchanges and times out on every one, and records how many client +/// rows existed when the first exchange was attempted. +const CountingExchange = struct { + var calls: usize = 0; + var database: ?*db.Db = null; + var rows_at_first_call: i64 = -1; + + fn reset(target: *db.Db) void { + calls = 0; + database = target; + rows_at_first_call = -1; + } + + fn exchange( + _: std.Io, + _: validate.Resolver, + _: []const u8, + _: []u8, + ) @import("../upstream/transport.zig").ExchangeError![]u8 { + if (calls == 0) { + rows_at_first_call = clients_repo.countClients(database.?) catch -1; + } + calls += 1; + return error.Timeout; + } +}; + +fn namingTables(io: std.Io, tables: *local_tables.LocalTables) !void { + tables.swap(io, testing.allocator, .empty, try forward_zones.Zones.build( + testing.allocator, + &.{.{ .zone = "168.192.in-addr.arpa", .resolver = "udp://192.168.1.1:53" }}, + )); +} + +test "the drain lands before any exchange, and attempts stop at the cap" { + var threaded: std.Io.Threaded = .init(testing.allocator, .{}); + defer threaded.deinit(); + const io = threaded.io(); + + var database = try openMigrated(); + defer database.close(); + var tables: local_tables.LocalTables = .empty; + defer tables.deinit(testing.allocator); + try namingTables(io, &tables); + + var names: client_names.Resolver = .init(&tables); + names.exchange_fn = CountingExchange.exchange; + CountingExchange.reset(&database); + + var tracker: Tracker = .init(30); + const pending = clients_repo.max_per_pass + 4; + for (0..pending) |i| { + _ = tracker.trackAt(io, .{ .ip4 = .{ 192, 168, 2, @intCast(i) } }, 1700000000); + } + + tracker.flushOnce(io, &database, true, &names); + + // Every pending row was written before the first exchange went out, which + // is what keeps a slow resolver off the drain. + try testing.expectEqual(@as(i64, @intCast(pending)), CountingExchange.rows_at_first_call); + try testing.expectEqual(clients_repo.max_per_pass, CountingExchange.calls); + try testing.expectEqual(@as(u64, @intCast(pending)), tracker.snapshotStats(io).flushed); + try testing.expectEqual(@as(u64, clients_repo.max_per_pass), names.snapshotStats(io).failed); +} + +test "a row the due pass prunes is never asked about" { + var threaded: std.Io.Threaded = .init(testing.allocator, .{}); + defer threaded.deinit(); + const io = threaded.io(); + + var database = try openMigrated(); + defer database.close(); + var tables: local_tables.LocalTables = .empty; + defer tables.deinit(testing.allocator); + try namingTables(io, &tables); + + var names: client_names.Resolver = .init(&tables); + names.exchange_fn = CountingExchange.exchange; + CountingExchange.reset(&database); + + const now = std.Io.Clock.real.now(io).toSeconds(); + try clients_repo.upsertSeen(&database, "192.168.1.10", now - 40 * 86_400); + + var tracker: Tracker = .init(30); + tracker.passes = Tracker.prune_every_passes - 1; + tracker.flushOnce(io, &database, true, &names); + + try testing.expectEqual(@as(i64, 0), try clients_repo.countClients(&database)); + try testing.expectEqual(@as(usize, 0), CountingExchange.calls); + try testing.expectEqual(@as(u64, 0), names.snapshotStats(io).attempted); +} + +test "a gated pass attempts no naming either" { + var threaded: std.Io.Threaded = .init(testing.allocator, .{}); + defer threaded.deinit(); + const io = threaded.io(); + + var database = try openMigrated(); + defer database.close(); + var tables: local_tables.LocalTables = .empty; + defer tables.deinit(testing.allocator); + try namingTables(io, &tables); + + var names: client_names.Resolver = .init(&tables); + names.exchange_fn = CountingExchange.exchange; + CountingExchange.reset(&database); + + try clients_repo.upsertSeen(&database, "192.168.1.10", 1700000000); + var tracker: Tracker = .init(30); + tracker.flushOnce(io, &database, false, &names); + + try testing.expectEqual(@as(usize, 0), CountingExchange.calls); + try testing.expectEqual(@as(u64, 0), names.snapshotStats(io).attempted); +} diff --git a/src/server/phase7_integration_test.zig b/src/server/phase7_integration_test.zig index 60e813d..10fbeea 100644 --- a/src/server/phase7_integration_test.zig +++ b/src/server/phase7_integration_test.zig @@ -844,7 +844,7 @@ test "S7 case 10: the querying client is materialised as a row" { // Two queries from one client are one pending entry, and the forced pass // stands in for the 60-second flush interval (S4 As-built seam). try testing.expectEqual(@as(u32, 1), tracker.pendingClients(io)); - tracker.flushOnce(io, &database, true); + tracker.flushOnce(io, &database, true, null); try testing.expectEqual(@as(i64, 1), try clients_repo.countClients(&database)); try testing.expectEqual(@as(u32, 0), tracker.pendingClients(io)); diff --git a/src/storage/config_schema.zig b/src/storage/config_schema.zig index b00584d..0c66773 100644 --- a/src/storage/config_schema.zig +++ b/src/storage/config_schema.zig @@ -29,6 +29,8 @@ pub const ddl_v1: [:0]const u8 = \\ id INTEGER PRIMARY KEY, \\ ip TEXT NOT NULL UNIQUE, -- canonical text form (v4 dotted / v6 RFC 5952) \\ name TEXT, + \\ learned_name TEXT, + \\ name_attempt_after INTEGER NOT NULL DEFAULT 0, \\ group_id INTEGER NOT NULL REFERENCES groups(id), \\ hand_edited INTEGER NOT NULL DEFAULT 0, \\ first_seen INTEGER NOT NULL, diff --git a/src/storage/migrations.zig b/src/storage/migrations.zig index d472e95..2b6b3c8 100644 --- a/src/storage/migrations.zig +++ b/src/storage/migrations.zig @@ -269,6 +269,8 @@ test "a fresh database reaches the baseline with every v1 column and rule kind" try testing.expectEqual(@as(u32, 1), try migrate(&database)); try testing.expectEqual(@as(u32, 1), target_version); + try testing.expect(try columnExists(&database, "clients", "learned_name")); + try testing.expect(try columnExists(&database, "clients", "name_attempt_after")); try testing.expect(try columnExists(&database, "upstreams", "tls_name")); try testing.expect(try columnExists(&database, "blocklist_sources", "exception_count")); try testing.expect(try columnExists(&database, "blocklist_sources", "skipped_unsupported_count")); diff --git a/src/storage/repositories/clients_repo.zig b/src/storage/repositories/clients_repo.zig index 56f9922..f20e7c1 100644 --- a/src/storage/repositories/clients_repo.zig +++ b/src/storage/repositories/clients_repo.zig @@ -18,6 +18,7 @@ const std = @import("std"); const Allocator = std.mem.Allocator; const db = @import("../db.zig"); +const logger = @import("../logger.zig"); const migrations = @import("../migrations.zig"); const model = @import("../../config/model.zig"); const context = @import("context.zig"); @@ -122,6 +123,123 @@ pub fn pruneStale(database: *db.Db, cutoff_s: i64) db.Error!u32 { return @intCast(@min(deleted, std.math.maxInt(u32))); } +// --- learned names (milestone 25) ------------------------------------------ +// +// `learned_name` and `name_attempt_after` are runtime state beside `last_seen`, +// never configuration: `listClients` does not select them, so an export cannot +// carry them, and the reconcile engine never writes them. The operator's `name` +// is never touched here, and the operator never writes `learned_name` — so +// precedence is a display rule, not a write conflict. + +/// How many rows one naming pass may take. The candidate buffer is sized by +/// this, and the SQL's LIMIT repeats it as a literal. +pub const max_per_pass: usize = 16; + +/// One selected address. Fixed-size storage because the naming path allocates +/// nothing: `logger.max_client_len` is the bound every address text in this +/// program is sized by. +pub const Candidate = struct { + buf: [logger.max_client_len]u8 = undefined, + len: usize = 0, + + pub fn ip(self: *const Candidate) []const u8 { + return self.buf[0..self.len]; + } +}; + +pub const CandidateBuf = [max_per_pass]Candidate; + +const resolve_candidates_sql = + \\SELECT ip FROM clients + \\ WHERE (name IS NULL OR name = '') AND name_attempt_after <= ?1 + \\ ORDER BY name_attempt_after, ip LIMIT 16 +; + +/// Fills `out` with the rows whose displayed name would come from learning and +/// whose next attempt is due, and returns how many slots it filled. +/// +/// `hand_edited` is deliberately absent from the predicate: display precedence +/// never consults it, so candidacy must not either. A row with a name is +/// skipped whatever its flag, because its learned name would never be shown. +/// +/// `name_attempt_after` holds the epoch second before which the row is not +/// attempted again, so the predicate is a plain comparison: 0 (the column +/// default) means eligible now, and no arithmetic can overflow on a corrupt +/// value. +/// +/// `error.Mismatch`: a stored `ip` longer than `logger.max_client_len`, which +/// means something other than nxdns wrote the row. +pub fn resolveCandidates(database: *db.Db, out: *CandidateBuf, now_s: i64) db.Error!usize { + var stmt = try database.prepare(resolve_candidates_sql); + defer stmt.deinit(); + try stmt.bindInt(1, now_s); + + var count: usize = 0; + while (try stmt.step()) : (count += 1) { + const ip = stmt.columnText(0); + if (ip.len > logger.max_client_len) return error.Mismatch; + @memcpy(out[count].buf[0..ip.len], ip); + out[count].len = ip.len; + } + return count; +} + +/// What one naming attempt decided about a row's learned name. +pub const LearnedName = union(enum) { + /// The resolver answered a target that passed `acceptHostname`. + store: []const u8, + /// The resolver said the address has no name (NXDOMAIN or NODATA). + clear, + /// Anything else — an outage must not strip names from the dashboard. + keep, +}; + +pub const NameOutcome = struct { + /// The epoch second before which this row is not attempted again. + attempt_after: i64, + learned: LearnedName, +}; + +const store_learned_sql = + "UPDATE clients SET learned_name = ?2, name_attempt_after = ?3 WHERE ip = ?1"; +const clear_learned_sql = + "UPDATE clients SET learned_name = NULL, name_attempt_after = ?2 WHERE ip = ?1"; +const keep_learned_sql = + "UPDATE clients SET name_attempt_after = ?2 WHERE ip = ?1"; + +/// Records one attempt: always the next attempt time, and the learned name only +/// when the outcome decided one. +/// +/// A row deleted between selection and this call makes the UPDATE touch zero +/// rows. That is a no-op by design — the device left, and nothing was learned +/// about nothing — so it is neither an error nor a counted failure. +pub fn noteNameOutcome(database: *db.Db, ip: []const u8, outcome: NameOutcome) db.Error!void { + switch (outcome.learned) { + .store => |text| { + var stmt = try database.prepare(store_learned_sql); + defer stmt.deinit(); + try stmt.bindText(1, ip); + try stmt.bindText(2, text); + try stmt.bindInt(3, outcome.attempt_after); + try stmt.exec(); + }, + .clear => { + var stmt = try database.prepare(clear_learned_sql); + defer stmt.deinit(); + try stmt.bindText(1, ip); + try stmt.bindInt(2, outcome.attempt_after); + try stmt.exec(); + }, + .keep => { + var stmt = try database.prepare(keep_learned_sql); + defer stmt.deinit(); + try stmt.bindText(1, ip); + try stmt.bindInt(2, outcome.attempt_after); + try stmt.exec(); + }, + } +} + pub fn deleteAllClients(database: *db.Db) db.Error!void { return database.exec("DELETE FROM clients;"); } @@ -197,6 +315,11 @@ pub const ClientRow = struct { /// `clients.name` is nullable; a NULL reads as `""`, as it does on the /// import path. name: []const u8, + /// The name learned over reverse DNS, or `""` when nothing was learned. + /// Display-only runtime state: `name` wins whenever it is non-empty, and + /// this never reaches an export. `name_attempt_after` is deliberately not + /// here — it is scheduling state with no operator meaning. + learned_name: []const u8, group_id: i64, group: []const u8, hand_edited: bool, @@ -221,14 +344,16 @@ pub const ClientEdit = struct { }; const list_client_rows_sql = - \\SELECT c.id, c.ip, c.name, c.group_id, g.name, c.hand_edited, c.first_seen, c.last_seen + \\SELECT c.id, c.ip, c.name, c.group_id, g.name, c.hand_edited, c.first_seen, c.last_seen, + \\ c.learned_name \\ FROM clients c \\ JOIN groups g ON g.id = c.group_id \\ ORDER BY c.ip ; const get_client_sql = - \\SELECT c.id, c.ip, c.name, c.group_id, g.name, c.hand_edited, c.first_seen, c.last_seen + \\SELECT c.id, c.ip, c.name, c.group_id, g.name, c.hand_edited, c.first_seen, c.last_seen, + \\ c.learned_name \\ FROM clients c \\ JOIN groups g ON g.id = c.group_id \\ WHERE c.id = ?1 @@ -263,10 +388,14 @@ fn readClientRow(stmt: *db.Stmt, gpa: Allocator) db.Error!ClientRow { errdefer gpa.free(name); const group = try stmt.columnTextAlloc(gpa, 4); errdefer gpa.free(group); + // `clients.learned_name` is nullable; NULL reads as "", like `name`. + const learned_name = try stmt.columnTextAlloc(gpa, 8); + errdefer gpa.free(learned_name); return .{ .id = stmt.columnInt(0), .ip = ip, .name = name, + .learned_name = learned_name, .group_id = stmt.columnInt(3), .group = group, .hand_edited = stmt.columnBool(5), @@ -721,6 +850,202 @@ test "pruneStale spares hand-edited rows however stale" { try testing.expectEqual(@as(i64, 3), try countClients(&database)); } +// --- learned names --------------------------------------------------------- + +/// `columnText` is borrowed until the statement dies, so the value is copied +/// into the caller's buffer. +fn learnedName(database: *db.Db, ip: []const u8, buf: []u8) !?[]const u8 { + var stmt = try database.prepare("SELECT learned_name FROM clients WHERE ip = ?1"); + defer stmt.deinit(); + try stmt.bindText(1, ip); + try testing.expect(try stmt.step()); + if (stmt.isNull(0)) return null; + const text = stmt.columnText(0); + @memcpy(buf[0..text.len], text); + return buf[0..text.len]; +} + +fn attemptAfter(database: *db.Db, ip: []const u8) !i64 { + var stmt = try database.prepare("SELECT name_attempt_after FROM clients WHERE ip = ?1"); + defer stmt.deinit(); + try stmt.bindText(1, ip); + try testing.expect(try stmt.step()); + return stmt.columnInt(0); +} + +test "resolveCandidates selects unnamed rows in attempt-then-ip order" { + var database = try openMigrated(); + defer database.close(); + + try upsertSeen(&database, "192.168.1.30", 1700000000); + try upsertSeen(&database, "192.168.1.10", 1700000000); + try upsertSeen(&database, "192.168.1.20", 1700000000); + // Attempted already, and due later than the other two. + try noteNameOutcome(&database, "192.168.1.10", .{ .attempt_after = 500, .learned = .keep }); + + var buf: CandidateBuf = undefined; + const count = try resolveCandidates(&database, &buf, 1000); + try testing.expectEqual(@as(usize, 3), count); + try testing.expectEqualStrings("192.168.1.20", buf[0].ip()); + try testing.expectEqualStrings("192.168.1.30", buf[1].ip()); + try testing.expectEqualStrings("192.168.1.10", buf[2].ip()); +} + +test "resolveCandidates skips named rows whatever their hand_edited flag" { + var database = try openMigrated(); + defer database.close(); + + _ = try insertClientRow(&database, .{ .ip = "192.168.1.7", .name = "printer", .group_id = 1 }, 1); + // Hand-edited but unnamed: grouped, not named, so it still benefits. + _ = try insertClientRow(&database, .{ .ip = "192.168.1.8", .group_id = 1 }, 1); + try upsertSeen(&database, "192.168.1.9", 1); + // An empty name is as unnamed as NULL. + try database.exec("UPDATE clients SET name = '' WHERE ip = '192.168.1.9';"); + + var buf: CandidateBuf = undefined; + const count = try resolveCandidates(&database, &buf, 1000); + try testing.expectEqual(@as(usize, 2), count); + try testing.expectEqualStrings("192.168.1.8", buf[0].ip()); + try testing.expectEqualStrings("192.168.1.9", buf[1].ip()); +} + +test "a never-attempted row is due at any now_s, and the cutoff is inclusive" { + var database = try openMigrated(); + defer database.close(); + var buf: CandidateBuf = undefined; + + try upsertSeen(&database, "192.168.1.10", 1700000000); + // `now_s` smaller than any cadence constant still selects the DEFAULT 0 row. + try testing.expectEqual(@as(usize, 1), try resolveCandidates(&database, &buf, 0)); + + try noteNameOutcome(&database, "192.168.1.10", .{ .attempt_after = 1700086400, .learned = .keep }); + try testing.expectEqual(@as(usize, 1), try resolveCandidates(&database, &buf, 1700086400)); + try testing.expectEqual(@as(usize, 0), try resolveCandidates(&database, &buf, 1700086399)); +} + +test "an extreme name_attempt_after never selects and never traps" { + var database = try openMigrated(); + defer database.close(); + var buf: CandidateBuf = undefined; + + try upsertSeen(&database, "192.168.1.10", 1700000000); + try noteNameOutcome(&database, "192.168.1.10", .{ + .attempt_after = std.math.maxInt(i64), + .learned = .keep, + }); + + try testing.expectEqual(@as(usize, 0), try resolveCandidates(&database, &buf, std.math.maxInt(i64) - 1)); + try testing.expectEqual(@as(usize, 1), try resolveCandidates(&database, &buf, std.math.maxInt(i64))); +} + +test "resolveCandidates stops at max_per_pass" { + var database = try openMigrated(); + defer database.close(); + + for (0..max_per_pass + 4) |i| { + var ip_buf: [logger.max_client_len]u8 = undefined; + const ip = try std.fmt.bufPrint(&ip_buf, "192.168.2.{d}", .{i}); + try upsertSeen(&database, ip, 1700000000); + } + + var buf: CandidateBuf = undefined; + try testing.expectEqual(max_per_pass, try resolveCandidates(&database, &buf, 1700000000)); +} + +test "noteNameOutcome stores, overwrites, clears and keeps" { + var database = try openMigrated(); + defer database.close(); + var name_buf: [logger.max_client_len]u8 = undefined; + try upsertSeen(&database, "192.168.1.10", 1700000000); + + try noteNameOutcome(&database, "192.168.1.10", .{ .attempt_after = 10, .learned = .{ .store = "nas.lan" } }); + try testing.expectEqualStrings("nas.lan", (try learnedName(&database, "192.168.1.10", &name_buf)).?); + try testing.expectEqual(@as(i64, 10), try attemptAfter(&database, "192.168.1.10")); + + // The router is the authority on its own zone: a changed answer wins. + try noteNameOutcome(&database, "192.168.1.10", .{ .attempt_after = 20, .learned = .{ .store = "tv.lan" } }); + try testing.expectEqualStrings("tv.lan", (try learnedName(&database, "192.168.1.10", &name_buf)).?); + + // Keep leaves the stored name and only moves the schedule. + try noteNameOutcome(&database, "192.168.1.10", .{ .attempt_after = 30, .learned = .keep }); + try testing.expectEqualStrings("tv.lan", (try learnedName(&database, "192.168.1.10", &name_buf)).?); + try testing.expectEqual(@as(i64, 30), try attemptAfter(&database, "192.168.1.10")); + + try noteNameOutcome(&database, "192.168.1.10", .{ .attempt_after = 40, .learned = .clear }); + try testing.expectEqual(@as(?[]const u8, null), try learnedName(&database, "192.168.1.10", &name_buf)); + try testing.expectEqual(@as(i64, 40), try attemptAfter(&database, "192.168.1.10")); +} + +test "noteNameOutcome on a row that no longer exists is a no-op" { + var database = try openMigrated(); + defer database.close(); + + try noteNameOutcome(&database, "192.168.1.99", .{ .attempt_after = 10, .learned = .{ .store = "gone.lan" } }); + try noteNameOutcome(&database, "192.168.1.99", .{ .attempt_after = 10, .learned = .clear }); + try noteNameOutcome(&database, "192.168.1.99", .{ .attempt_after = 10, .learned = .keep }); + try testing.expectEqual(@as(i64, 0), try countClients(&database)); +} + +test "a learned name reads back on ClientRow and a NULL reads as the empty string" { + var database = try openMigrated(); + defer database.close(); + + try upsertSeen(&database, "192.168.1.10", 1700000000); + try upsertSeen(&database, "192.168.1.11", 1700000000); + try noteNameOutcome(&database, "192.168.1.10", .{ .attempt_after = 10, .learned = .{ .store = "nas.lan" } }); + + var rows = try listClientRows(&database, testing.allocator); + defer rows.deinit(testing.allocator); + defer freeClientRows(testing.allocator, rows.items); + try testing.expectEqualStrings("nas.lan", rows.items[0].learned_name); + try testing.expectEqualStrings("", rows.items[1].learned_name); + + const fetched = (try getClient(&database, testing.allocator, rows.items[0].id)).?; + defer freeClientRow(testing.allocator, fetched); + try testing.expectEqualStrings("nas.lan", fetched.learned_name); +} + +test "updateClient leaves learned_name alone and removes the row from candidacy" { + var database = try openMigrated(); + defer database.close(); + + var name_buf: [logger.max_client_len]u8 = undefined; + try upsertSeen(&database, "192.168.1.10", 1700000000); + try noteNameOutcome(&database, "192.168.1.10", .{ .attempt_after = 0, .learned = .{ .store = "nas.lan" } }); + const id = try database.queryInt("SELECT id FROM clients WHERE ip = '192.168.1.10'"); + try updateClient(&database, id, .{ .name = "the nas", .group_id = 1 }); + + try testing.expectEqualStrings("nas.lan", (try learnedName(&database, "192.168.1.10", &name_buf)).?); + var buf: CandidateBuf = undefined; + try testing.expectEqual(@as(usize, 0), try resolveCandidates(&database, &buf, 1700000000)); +} + +test "an export is byte-identical across a learned name landing" { + const export_mod = @import("../../config/export.zig"); + + var database = try openMigrated(); + defer database.close(); + var ids = try seedGroups(&database); + defer ids.deinit(testing.allocator); + try seedClients(&database, &ids); + try upsertSeen(&database, "192.168.1.99", 1700000000); + + var before: std.Io.Writer.Allocating = .init(testing.allocator); + defer before.deinit(); + try export_mod.writeToWriter(testing.allocator, &database, &before.writer); + + try noteNameOutcome(&database, "192.168.1.99", .{ + .attempt_after = 1700086400, + .learned = .{ .store = "phone.lan" }, + }); + + var after: std.Io.Writer.Allocating = .init(testing.allocator); + defer after.deinit(); + try export_mod.writeToWriter(testing.allocator, &database, &after.writer); + + try testing.expectEqualStrings(before.written(), after.written()); +} + test "client_prefixes round-trip in prefix order with group names resolved" { var database = try openMigrated(); defer database.close(); diff --git a/src/tests.zig b/src/tests.zig index c588229..722da09 100644 --- a/src/tests.zig +++ b/src/tests.zig @@ -71,6 +71,7 @@ comptime { _ = @import("local/records.zig"); _ = @import("local/forward_zones.zig"); _ = @import("local/forward_client.zig"); + _ = @import("local/reverse_name.zig"); _ = @import("cache/dns_cache.zig"); _ = @import("server/rate_limiter.zig"); _ = @import("storage/repositories/queries_repo.zig"); @@ -81,6 +82,7 @@ comptime { _ = @import("storage/retention.zig"); _ = @import("storage/phase6_integration_test.zig"); _ = @import("server/pause.zig"); + _ = @import("server/client_names.zig"); _ = @import("server/clients.zig"); _ = @import("server/shutdown.zig"); _ = @import("server/phase7_integration_test.zig"); diff --git a/src/web/metrics.zig b/src/web/metrics.zig index a0dd67d..68338bb 100644 --- a/src/web/metrics.zig +++ b/src/web/metrics.zig @@ -24,6 +24,7 @@ const std = @import("std"); const Allocator = std.mem.Allocator; const cert_store = @import("../server/cert_store.zig"); +const client_names = @import("../server/client_names.zig"); const clients = @import("../server/clients.zig"); const dns_cache = @import("../cache/dns_cache.zig"); const dns_handler = @import("../server/handler.zig"); @@ -122,6 +123,7 @@ pub const Sample = struct { cache: ?CacheSample = null, limiter: ?LimiterSample = null, tracker: ?TrackerSample = null, + client_names: ?client_names.Resolver.Stats = null, retention: ?retention_mod.Stats = null, blocklist: ?BlocklistSample = null, disk: ?DiskSample = null, @@ -193,6 +195,8 @@ pub fn collect(state: *server.WebState, io: std.Io, arena: Allocator) Allocator. .pending_clients = tracker.pendingClients(io), }; + if (state.client_names) |names| sample.client_names = names.snapshotStats(io); + if (state.retention) |retention| sample.retention = retention.snapshotStats(); if (state.manager) |manager| { @@ -340,6 +344,10 @@ pub fn render(w: *std.Io.Writer, sample: Sample) std.Io.Writer.Error!void { ); } + if (sample.client_names) |names| { + try counterGroup(w, "nxdns_client_names_", "Learned client name counter", names); + } + if (sample.retention) |retention| { try counterGroup(w, "nxdns_retention_", "Query log retention counter", retention); } @@ -594,6 +602,7 @@ fn writeLabelValue(w: *std.Io.Writer, value: []const u8) std.Io.Writer.Error!voi // tests // --------------------------------------------------------------------------- +const local_tables = @import("../server/local_tables.zig"); const logger_mod = @import("../storage/logger.zig"); const testing = std.testing; @@ -645,6 +654,7 @@ test "a full sample renders the whole exposition, byte for byte" { .stats = .{ .tracked = 4, .flushed = 3, .dropped_full = 0, .pruned = 1, .flush_failures = 0 }, .pending_clients = 2, }, + .client_names = .{ .attempted = 6, .answered = 3, .nxdomain = 1, .no_zone = 2, .invalid = 0, .failed = 0, .read_failures = 0, .write_failures = 1 }, .retention = .{ .passes = 7, .rows_pruned = 100, .checkpoints = 7, .vacuums = 1 }, .blocklist = .{ .refreshes_gated = 2, .generation = 4 }, .disk = .{ @@ -681,6 +691,9 @@ test "a full sample renders the whole exposition, byte for byte" { try testing.expect(std.mem.containsAtLeast(u8, text, 1, "nxdns_dns_rate_limit_tracked_clients 3\n")); try testing.expect(std.mem.containsAtLeast(u8, text, 1, "nxdns_clients_dropped_full_total 0\n")); try testing.expect(std.mem.containsAtLeast(u8, text, 1, "nxdns_clients_pending 2\n")); + try testing.expect(std.mem.containsAtLeast(u8, text, 1, "nxdns_client_names_no_zone_total 2\n")); + try testing.expect(std.mem.containsAtLeast(u8, text, 1, "nxdns_client_names_answered_total 3\n")); + try testing.expect(std.mem.containsAtLeast(u8, text, 1, "nxdns_client_names_write_failures_total 1\n")); try testing.expect(std.mem.containsAtLeast(u8, text, 1, "nxdns_retention_rows_pruned_total 100\n")); try testing.expect(std.mem.containsAtLeast(u8, text, 1, "nxdns_blocklist_refreshes_gated_total 2\n")); try testing.expect(std.mem.containsAtLeast(u8, text, 1, "nxdns_blocklist_generation 4\n")); @@ -1242,12 +1255,17 @@ test "collect reads the live counters of the components it is given" { var tracker: clients.Tracker = .init(30); var retention: retention_mod.Retention = .init(.{}); + var tables: local_tables.LocalTables = .empty; + var names: client_names.Resolver = .init(&tables); + names.stats.no_zone = 4; + names.stats.attempted = 4; var state: server.WebState = .{ .gpa = testing.allocator, .handler = &handler, .logger = &query_logger, .tracker = &tracker, + .client_names = &names, .retention = &retention, }; @@ -1263,6 +1281,7 @@ test "collect reads the live counters of the components it is given" { try testing.expectEqual(@as(u64, 3), sample.limiter.?.stats.refused); try testing.expectEqual(@as(u64, 90), sample.logger.rows_written); try testing.expectEqual(@as(u64, 0), sample.tracker.?.pending_clients); + try testing.expectEqual(@as(u64, 4), sample.client_names.?.no_zone); try testing.expectEqual(@as(u64, 0), sample.retention.?.passes); try testing.expectEqual(@as(?BlocklistSample, null), sample.blocklist); try testing.expectEqual(@as(usize, 0), sample.upstreams.len); diff --git a/src/web/openapi.yaml b/src/web/openapi.yaml index 75a7acd..f6cbbd5 100644 --- a/src/web/openapi.yaml +++ b/src/web/openapi.yaml @@ -2024,13 +2024,20 @@ components: Client: type: object - required: [id, ip, name, group_id, group, hand_edited, first_seen, last_seen] + required: [id, ip, name, learned_name, group_id, group, hand_edited, first_seen, last_seen] properties: id: { type: integer } ip: { type: string } name: type: string description: Empty when the client was never named. + learned_name: + type: string + description: | + The name learned over reverse DNS, or empty when nothing was + learned. Display-only runtime state: `name` wins whenever it is + non-empty, and a learned name never appears in an export. The + server writes it; a client cannot. group_id: { type: integer } group: { type: string } hand_edited: { type: boolean } diff --git a/src/web/server.zig b/src/web/server.zig index 634065f..884d758 100644 --- a/src/web/server.zig +++ b/src/web/server.zig @@ -25,6 +25,7 @@ const address = @import("../platform/address.zig"); const api_limiter = @import("api_limiter.zig"); const auth = @import("auth.zig"); const cert_store = @import("../server/cert_store.zig"); +const client_names = @import("../server/client_names.zig"); const clients = @import("../server/clients.zig"); const db = @import("../storage/db.zig"); const disk_monitor = @import("../storage/disk_monitor.zig"); @@ -130,6 +131,8 @@ pub const WebState = struct { handler: ?*dns_handler.Handler = null, pause: ?*pause_mod.Pause = null, tracker: ?*clients.Tracker = null, + /// The learned-name resolver, for `metrics.collect` (milestone-25 ruling 9). + client_names: ?*client_names.Resolver = null, manager: ?*manager_mod.Manager = null, pool: ?*pool_mod.Pool = null, monitor: ?*disk_monitor.Monitor = null, diff --git a/web/src/features/clients/ClientEditDialog.tsx b/web/src/features/clients/ClientEditDialog.tsx index a362ee8..d9e0568 100644 --- a/web/src/features/clients/ClientEditDialog.tsx +++ b/web/src/features/clients/ClientEditDialog.tsx @@ -47,9 +47,12 @@ const styles = stylex.create({ export default function ClientEditDialog({ client, groups, onClose }: Props) { const queryClient = useQueryClient(); const mutation = useMutation(clientUpdateMutation(queryClient)); - const [name, setName] = useState(client.name); - const [groupId, setGroupId] = useState(client.group_id); const readOnly = useReadOnlyConfig(); + // Adopting the learned name as a typed one is the natural gesture, but only + // where the save can land: under file authority the PUT answers 403, and the + // file's declared name is the one that wins. + const [name, setName] = useState(client.name === "" && !readOnly ? client.learned_name : client.name); + const [groupId, setGroupId] = useState(client.group_id); return ( diff --git a/web/src/features/clients/ClientsPage.test.tsx b/web/src/features/clients/ClientsPage.test.tsx index 2a61fdf..26ddf68 100644 --- a/web/src/features/clients/ClientsPage.test.tsx +++ b/web/src/features/clients/ClientsPage.test.tsx @@ -18,6 +18,7 @@ const CLIENTS = { id: 1, ip: "192.168.1.10", name: "laptop", + learned_name: "laptop-1.lan", group_id: 1, group: "default", hand_edited: true, @@ -28,6 +29,7 @@ const CLIENTS = { id: 2, ip: "192.168.1.11", name: "", + learned_name: "kids-tablet.lan", group_id: 2, group: "kids", hand_edited: false, @@ -95,6 +97,25 @@ test("renders the client table with group names and one hand-edited badge", asyn expect(screen.getAllByRole("cell", { name: "kids" })).toHaveLength(1); }); +test("a named row shows the typed name and hides the learned one", async () => { + await renderClientsPage(BASE); + + expect(screen.getByText("laptop")).toBeTruthy(); + expect(screen.queryByText("laptop-1.lan")).toBeNull(); +}); + +test("an unnamed row shows the learned name with the learned affordance", async () => { + await renderClientsPage(BASE); + + // The cell holds the learned name followed by the tag, so the match is on + // the containing span rather than on a bare text node. + const learned = screen.getByText( + (content, element) => element?.tagName === "SPAN" && content.startsWith("kids-tablet.lan"), + ); + // The affordance is text, not colour, so a screen reader announces it too. + expect(within(learned).getByText("learned")).toBeTruthy(); +}); + test("shows the DNS-activity empty state when there are no clients", async () => { await renderClientsPage({ ...BASE, "GET /api/clients": { clients: [] } }); diff --git a/web/src/features/clients/ClientsPage.tsx b/web/src/features/clients/ClientsPage.tsx index a534d1d..788ce05 100644 --- a/web/src/features/clients/ClientsPage.tsx +++ b/web/src/features/clients/ClientsPage.tsx @@ -56,6 +56,23 @@ const styles = stylex.create({ dash: { color: colors.textMuted, }, + /** + * A learned name is runtime state, not something the operator typed, so it + * reads muted and carries an outlined "learned" tag. The tag is real text — + * a screen reader announces it — because colour alone is not an affordance. + */ + learnedTag: { + marginLeft: "0.5rem", + borderWidth: 1, + borderStyle: "solid", + borderColor: colors.border, + borderRadius: "0.25rem", + paddingInline: "0.375rem", + paddingBlock: "0.125rem", + fontSize: "0.75rem", + lineHeight: "1rem", + color: colors.textMuted, + }, badge: { marginLeft: "0.5rem", borderRadius: "0.25rem", @@ -133,10 +150,15 @@ export default function ClientsPage() { {client.ip} - {client.name === "" ? ( - — - ) : ( + {client.name !== "" ? ( client.name + ) : client.learned_name !== "" ? ( + + {client.learned_name} + learned + + ) : ( + — )} {client.hand_edited && edited} diff --git a/web/src/lib/contractSamples.gen.ts b/web/src/lib/contractSamples.gen.ts index 9dc0822..c756745 100644 --- a/web/src/lib/contractSamples.gen.ts +++ b/web/src/lib/contractSamples.gen.ts @@ -259,6 +259,7 @@ export const sample_list_clients: { clients: Client[] } = { id: 0, ip: "192.168.1.50", last_seen: 0, + learned_name: "", name: "laptop", }, ], @@ -272,6 +273,7 @@ export const sample_update_client: Client = { id: 0, ip: "192.168.1.50", last_seen: 0, + learned_name: "", name: "laptop-renamed", }; diff --git a/web/src/lib/types.ts b/web/src/lib/types.ts index 21d44ae..7dd781f 100644 --- a/web/src/lib/types.ts +++ b/web/src/lib/types.ts @@ -256,6 +256,8 @@ export interface Client { id: number; ip: string; name: string; + /** Learned over reverse DNS. `name` wins whenever it is non-empty. */ + learned_name: string; group_id: number; group: string; hand_edited: boolean;