feat(api): optional house password on the LAN - #912
Conversation
Stop the dashboard being framed by another origin. Apply the same headers on the boot-phase listener and the setup wizard.
Treat those GETs like backups and CalDAV credentials: same-origin on the LAN still works; a public hostname needs FTW_API_TOKEN. Status and energy reads stay compatible.
Off by default. When on, protected LAN routes need a Bearer secret; loopback and an app session do not. Live status stays readable as a viewer.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e093b9684
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| secret, ok := parseBearer(r.Header.Get("Authorization")) | ||
| if !ok { | ||
| return false, r | ||
| } | ||
| houseOK := admitLANSecret(policy.VerifyLANSecret, secret) |
There was a problem hiding this comment.
Keep public API tokens out of the LAN lockout
When a client with a private source address accesses FTW through a public/FQDN hostname—such as the documented setup where a separate LAN reverse proxy injects FTW_API_TOKEN—this verifies the public bearer as the house password before remoteRequired is calculated. Every request carrying the valid public token therefore calls admitLANSecret with the wrong secret; even unprotected dashboard polls do this, so five polls repeatedly lock out the actual house password for 30 seconds. Determine the public-host token path before calling the LAN verifier, or otherwise exclude that bearer from the LAN guess counter.
Useful? React with 👍 / 👎.
miravoss26
left a comment
There was a problem hiding this comment.
Solid threat-model work. Adds an opt-in house password for the LAN (api.lan_auth), plus clickjacking/MIME-sniffing headers on every response and the same local-or-token gate extended to config/logs/dumps/driver-source/system-info reads.
Security read:
- Password stored as argon2id (m=64MB, t=3, p=4) via
golang.org/x/crypto/argon2, constant-time compare on verify. No plaintext or hash ever appears in/api/configorconfig.yaml— explicitly tested (TestGetConfigDoesNotContainLANPasswordHash). - Guess limiter is global (5 fails → 30s lock), and the lock actually short-circuits further
verify()calls rather than just counting after the fact — so it also bounds how often the argon2id hash runs under load, not just how often a wrong guess is accepted. Documented as "counter on the secret, not on IP" in the PR body, which is a reasonable call for a single house-password model. - Loopback recovery path can't be locked out by a bad remote token, matches existing recovery story for
FTW_API_TOKEN. - The double-
Authenticate-wrap issue (bootstrap listener +Server.Handler()both call it) is handled with a context flag so the secret isn't hashed/counted twice per request — good catch, and there's a test for the LAN-auth-off passthrough case too. - App session (
KindApp, Noise-authenticated) is never touched by any of this, confirmed byTestLANAuthDoesNotReplaceAppCaller. /api/auth/loginand/api/auth/logoutare pre-emptively exempted from the LAN-auth gate but aren't registered as routes in this diff (per the PR body, that's the next PR's cookie/session work) — not a bug, just flagging that those paths 404 until then.
Correctness on remoteRequired vs the new LAN-auth block looks right: TestLANAuthDoesNotOverrideRemoteToken confirms the two checks stay independent (public/FQDN paths keep using Token, LAN paths use the house password).
No secrets in the diff, no new network destinations, one new dependency (x/crypto/argon2, well-known and appropriate for this).
GitHub shows mergeable: CONFLICTING — expected since this stacks on #911 → #909, which aren't merged yet; not a content issue I can see in the diff itself.
Safe to merge from my read, once the stack underneath lands.
|
Stänger som ersatt av #914. |
Summary
The LAN is still open by default. This adds an opt-in lock for houses where guest Wi-Fi or another device on the network should not be able to write config, move energy, or pull dumps.
api.lan_authis off unless you turn it on. When it is on:Authorization: Bearer <house-password>127.0.0.1/::1) never does — that is how you recover/api/status, energy, prices and plan stay readable; those callers become a LAN viewer, not an ownerstate.db, not inconfig.yamlFTW_API_TOKENis still only for a public hostname. Two jobs, two secrets.There is no browser login in this PR.
curlworks. The next PR adds the cookie and Settings → System.Stack
Sits on #911 (
agent/protect-sensitive-reads), which sits on #909. Merge those first.How to try it
Recovery:
curlto127.0.0.1, or setapi.lan_auth: falseinconfig.yamland restart.Test plan
go test ./go/internal/api/ ./go/internal/config/ ./go/cmd/ftw/Out of scope