From 213dedf5e01e9805961ba160242708a51907b778 Mon Sep 17 00:00:00 2001 From: Jens Ahrensfeld Date: Sun, 26 Jul 2026 09:53:26 +0200 Subject: [PATCH] Fix dyndns per-user auth binding with real per-host TSIG keys Closes the gap where any dyndns credential could update any of the zone's 5 hostnames. Confirmed live that a FritzBox was already exposed to this (authenticating as the shared 'jens' account while updating 'uschi'), so implemented a phased rollout: new per-host TSIG keys and .htpasswd accounts enforce the binding at the BIND layer itself, with a logged legacy fallback for the pre-existing shared account only, so unmigrated devices keep working until reconfigured. Also fixed a bug where nic/update.php reported "good" even on a rejected update. Co-Authored-By: Claude Sonnet 5 --- TODO.md | 69 +++++++++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 57 insertions(+), 12 deletions(-) diff --git a/TODO.md b/TODO.md index 1d47057..975befa 100644 --- a/TODO.md +++ b/TODO.md @@ -31,18 +31,63 @@ Nothing here is urgent; all are low-risk, no-downtime changes. is a manual follow-up to Barracuda citing the confirmation number above. -- [ ] **dyndns app has no real per-user auth binding** — found 2026-07-20, - while fixing the command injection below. - `index.php`'s `pass == '_NO_PASS_'` check is a hardcoded magic - string, not a real per-user credential — there's nothing tying the - single shared Apache Basic Auth identity to *which* hostname label - gets updated. Any caller with valid dyndns credentials can update - any of the zone's 5 hostnames (`jens`, `kack`, `test`, `uschi`, - `vpn`), not just their own. Low severity today since apparently only - one person (`jens`) has credentials, but worth fixing before adding - any second dyndns user. Fix: either move to per-host TSIG-style - tokens, or add a static host-allowlist per `.htpasswd` user in the - PHP. +- [x] **dyndns app has no real per-user auth binding** — found 2026-07-20, + fixed 2026-07-26. `index.php`'s `pass == '_NO_PASS_'` check is a + hardcoded magic string, not a real per-user credential — nothing tied + the single shared Apache Basic Auth identity (`jens`, the only + `.htpasswd` account) to *which* hostname label got updated. Confirmed + live in the access log: a FritzBox ("Fritz!Box DDNS/1.0.3") was + authenticating as `jens` while updating the `uschi` record — i.e. every + device was already relying on this exact gap, self-policing which + host it claimed via an unauthenticated `hostname=`/`user=` param. + Fixed with real per-host TSIG binding, not just an app-level check: + - Added a dedicated `.htpasswd` account for each of `kack`, `test`, + `uschi`, `vpn` (`jens`'s existing account/password untouched). + - Generated 5 per-host TSIG keys (`hmac-sha512`, `tsig-keygen`), one + per hostname, each in its own `/etc/bind/.dyndns.jayfield.org.key` + (`root:bind 640` + `setfacl` grant for `www-data`, same pattern as + the original key). + - Replaced the zone's flat `allow-update { key dyndns.jayfield.org; };` + in `named.conf.default-zones` with an `update-policy` granting each + new key permission to update *only* its own name (`grant "" + name . A AAAA TXT;`), plus one explicit legacy fallback grant + for the original shared key (`subdomain` rule, zone-wide) — kept + temporarily so unmigrated devices don't break (see below). + - `nsupdate.php`/`nsupdate_ipv4.php`/`nsupdate_ipv6.php` now sign with + the key belonging to `$_SERVER['PHP_AUTH_USER']` (the *authenticated* + identity), not whatever hostname was requested. If they match, the + new per-host key is used — BIND itself then refuses the update if + that key isn't granted that name, independent of any PHP-side + check. If the authenticated user is specifically the legacy `jens` + account and doesn't match, it falls back to the old shared key + (logged via `syslog(LOG_WARNING, ...)` for migration tracking) — + every other account gets rejected outright (`-1`) on a mismatch, + so newly-created per-host logins can never touch another host, even + via the fallback. + - Also fixed a pre-existing bug found in `nic/update.php` while + testing this: it printed `good ` unconditionally, without ever + checking `nsupdate`'s return code — so a rejected update silently + looked successful to the calling device. Now prints the real + dyndns2-protocol response (`good ` / `badauth`). + All files backed up to + `/root/removed-configs-backup/dyndns-per-host-auth-20260726094500/`. + Validated with `named-checkconf`/`apache2ctl configtest`, applied via + `rndc reload`/`systemctl reload apache2`. Tested both by driving + `nsupdate()` directly as `www-data` and end-to-end over HTTPS with + the new `vpn` credentials: own-host updates succeed, cross-host + attempts from new accounts are rejected (`-1`/`badauth`) with the + target record provably unchanged, and the legacy `jens`→`uschi` + pattern still succeeds unchanged. Test IPs used during verification + were restored to the real production values (`test`, `uschi`, `vpn`) + immediately after; live `dig` confirmed the final state matches the + pre-change records exactly. + **Follow-up**: the `jens` legacy fallback (and its `.htpasswd` + account) should stay only until every real device — including + whichever ones are still authenticating as `jens` to update `kack`/ + `test`/`uschi` — has been reconfigured with its own dedicated login. + Once `dyndns legacy shared-key update` stops appearing in `syslog`, + remove the fallback `grant` and the `jens` account can be scoped down + to just its own host too. - [x] **dyndns/default-vhost access logging is a no-op** — done 2026-07-25 found 2026-07-20 while investigating the command injection below.