unlock (#110 item 1) #149

New issue

Closed

opened 2026-09-15 18:22:58 +02:00 by lz · 1 comment

lz commented

2026-09-15 18:22:58 +02:00

Owner

This is a live exposure right now, independent of MCP. Filed here because MCP support makes /unlock the thing that authorizes OAuth token issuance, which raises what a successful guess is worth — but it should ship regardless of whether MCP ever does.

Splits item 1 out of #110, which says explicitly: "Filed as a checklist, not a plan. Each item is independently shippable."

Verified

routes/api/auth/unlock/+server.ts calls unlockMasterPassphrase directly with no attempt counter, no backoff, no lockout and no failure logging:

$ grep -rn "rateLimit|attempts|backoff|lockout" src/lib/server/auth/ src/routes/api/auth/
(empty)

The endpoint is in PUBLIC_API by necessity, and the instance is internet-facing at a.lck.sh today — confirmed: GET https://a.lck.sh/api/state returns 200 unauthenticated.

Argon2id bounds the cost per guess, not the number of guesses — and the server pays that cost, so parallel attempts are simultaneously a guessing channel and a CPU denial-of-service. #110 makes the same point: "the same property that slows an attacker is also a DoS vector."

Work

Attempt counter keyed on source IP and globally. The global key is the load-bearing one — per-IP alone is trivially defeated, and #110 says so.
Backoff, plus a lockout window.
Log failures with enough context to tell a typo from an attack.
Decide and document what happens to a locked-out legitimate operator. A permanent lockout on a single-operator instance with no second channel is its own outage.

Watch for

The counter must not itself become the DoS: an unbounded map keyed on attacker-controlled source IPs is a memory exhaustion primitive. Bound it.

Don't let the rate limiter run before cheap input validation — a 400 for a malformed body should not consume an attempt, or an attacker can lock the operator out for free.

Done when

Repeated wrong passphrases from one source back off and then lock out; the global counter trips independently of source; a correct passphrase during lockout is still refused; and each of those is a test that has been proven able to fail.

**This is a live exposure right now, independent of MCP.** Filed here because MCP support makes `/unlock` the thing that authorizes OAuth token issuance, which raises what a successful guess is worth — but it should ship regardless of whether MCP ever does. Splits item 1 out of #110, which says explicitly: *"Filed as a checklist, not a plan. Each item is independently shippable."* ## Verified `routes/api/auth/unlock/+server.ts` calls `unlockMasterPassphrase` directly with no attempt counter, no backoff, no lockout and no failure logging: ``` $ grep -rn "rateLimit|attempts|backoff|lockout" src/lib/server/auth/ src/routes/api/auth/ (empty) ``` The endpoint is in `PUBLIC_API` by necessity, and the instance is internet-facing at `a.lck.sh` today — confirmed: `GET https://a.lck.sh/api/state` returns `200` unauthenticated. Argon2id bounds the cost **per guess**, not the **number** of guesses — and the server pays that cost, so parallel attempts are simultaneously a guessing channel and a CPU denial-of-service. #110 makes the same point: *"the same property that slows an attacker is also a DoS vector."* ## Work - Attempt counter keyed on **source IP and globally**. The global key is the load-bearing one — per-IP alone is trivially defeated, and #110 says so. - Backoff, plus a lockout window. - Log failures with enough context to tell a typo from an attack. - Decide and document what happens to a locked-out legitimate operator. A permanent lockout on a single-operator instance with no second channel is its own outage. ## Watch for The counter must not itself become the DoS: an unbounded map keyed on attacker-controlled source IPs is a memory exhaustion primitive. Bound it. Don't let the rate limiter run *before* cheap input validation — a 400 for a malformed body should not consume an attempt, or an attacker can lock the operator out for free. ## Done when Repeated wrong passphrases from one source back off and then lock out; the global counter trips independently of source; a correct passphrase during lockout is still refused; and each of those is a test that has been proven able to fail.

lz added this to the MCP support (#140) milestone

2026-09-15 18:22:58 +02:00

lz commented

2026-09-15 18:40:03 +02:00

Author

Owner

The per-IP key does not work as written — resolve this first

Nexus sits behind nginx at a.lck.sh, and nothing in the repo configures client-IP resolution. Measured:

$ grep -rn "getClientAddress|X-Forwarded-For|ADDRESS_HEADER|XFF_DEPTH" src/ docker-compose.yml

…returns only the four /api/agent/* callbacks. Those are fine as they stand: workers reach host-mode Nexus directly over the nexus-workers bridge, no proxy in the path, so getClientAddress() is genuinely the worker's bridge IP — which is exactly what ip-guard.ts depends on.

/api/auth/unlock is different. Reached through nginx, getClientAddress() returns nginx's own address, identical for every client on the internet. Two consequences, and they are both bad:

A per-IP limiter silently becomes one global bucket. Every external request shares a key, so the counter is per-IP in name only.
That turns the feature into a self-inflicted DoS. One attacker trips the shared bucket and the legitimate operator is locked out of their own instance — with no second channel, since the passphrase is the only way in.

So the per-IP half of this issue is not implementable until client-IP resolution is correct.

The fix has a trap of its own

adapter-node derives getClientAddress() from the ADDRESS_HEADER and XFF_DEPTH environment variables. Setting ADDRESS_HEADER=x-forwarded-for with a correct XFF_DEPTH for this deployment makes it read the right hop.

XFF_DEPTH is the load-bearing part. X-Forwarded-For is client-appendable: anyone can send X-Forwarded-For: 1.2.3.4 and, with a naive left-most read, mint a fresh rate-limit key per request — defeating the limiter completely while making it look like it works. The depth must count back from the right by the number of proxies actually in front of Nexus, and it must match the real deployment rather than a guess.

Also confirm nginx is setting the header at all, and that it overwrites rather than appends a client-supplied one.

Consequences for this issue

Treat client-IP resolution as in scope. A rate limiter on a wrong key is worse than none, because it reports safety it does not provide.
The global counter still works regardless and should be built first — it is the one #110 calls load-bearing anyway ("per-IP alone is trivially defeated").
Add a test that a forged X-Forwarded-For cannot mint a new key. Prove it fails before trusting it.
If the correct XFF_DEPTH for this deployment cannot be established with confidence, ship the global limiter alone and say so, rather than shipping a per-IP key that is silently the proxy's address.

## The per-IP key does not work as written — resolve this first Nexus sits behind nginx at `a.lck.sh`, and **nothing in the repo configures client-IP resolution**. Measured: ``` $ grep -rn "getClientAddress|X-Forwarded-For|ADDRESS_HEADER|XFF_DEPTH" src/ docker-compose.yml ``` …returns only the four `/api/agent/*` callbacks. Those are fine as they stand: workers reach **host-mode** Nexus directly over the `nexus-workers` bridge, no proxy in the path, so `getClientAddress()` is genuinely the worker's bridge IP — which is exactly what `ip-guard.ts` depends on. `/api/auth/unlock` is different. Reached through nginx, `getClientAddress()` returns **nginx's own address**, identical for every client on the internet. Two consequences, and they are both bad: 1. **A per-IP limiter silently becomes one global bucket.** Every external request shares a key, so the counter is per-IP in name only. 2. **That turns the feature into a self-inflicted DoS.** One attacker trips the shared bucket and the legitimate operator is locked out of their own instance — with no second channel, since the passphrase is the only way in. So the per-IP half of this issue is not implementable until client-IP resolution is correct. ### The fix has a trap of its own adapter-node derives `getClientAddress()` from the `ADDRESS_HEADER` and `XFF_DEPTH` environment variables. Setting `ADDRESS_HEADER=x-forwarded-for` with a correct `XFF_DEPTH` for this deployment makes it read the right hop. **`XFF_DEPTH` is the load-bearing part.** `X-Forwarded-For` is client-appendable: anyone can send `X-Forwarded-For: 1.2.3.4` and, with a naive left-most read, mint a fresh rate-limit key per request — defeating the limiter completely while making it *look* like it works. The depth must count back from the right by the number of proxies actually in front of Nexus, and it must match the real deployment rather than a guess. Also confirm nginx is setting the header at all, and that it *overwrites* rather than appends a client-supplied one. ### Consequences for this issue - Treat client-IP resolution as in scope. A rate limiter on a wrong key is worse than none, because it reports safety it does not provide. - The **global** counter still works regardless and should be built first — it is the one #110 calls load-bearing anyway (*"per-IP alone is trivially defeated"*). - Add a test that a forged `X-Forwarded-For` cannot mint a new key. Prove it fails before trusting it. - If the correct `XFF_DEPTH` for this deployment cannot be established with confidence, **ship the global limiter alone and say so**, rather than shipping a per-IP key that is silently the proxy's address.

lz referenced this issue from a commit

2026-09-15 20:14:07 +02:00

refactor(auth): derive the lockout deadline, and correct a false claim

lz referenced this issue from a pull request that will close it,

2026-09-15 20:14:53 +02:00

Rate-limit /api/auth/unlock with one global counter #160

lz referenced this issue

2026-09-15 21:17:25 +02:00

Per-IP unlock rate limiting via X-Real-IP #161

lz referenced this issue

2026-09-15 21:17:37 +02:00

/api/auth/setup reaches the KDF unguarded before first-run completes #162