diff --git a/SECURITY.md b/SECURITY.md index 6ad8aaed..4950a0b2 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -211,30 +211,64 @@ That is a real reduction and it is not a fix for cross-site scripting. The CSP on the site and the escaping in the pages remain the thing that stops a script running in the first place; this only bounds what one gets if it does. -#### What is still open - -The `GET /api/user/account/key` reveal route still exists, and ParaSend was not -its only caller. Three pages still fetch the raw key into the browser and use it -as a relay credential: - -| Page | File | What it does with the key | -|------|------|---------------------------| -| `/account` | `frontend/js/account.inline1.js` | reveals it on the screen, deliberately | -| `/pricing` | `frontend/js/pricing-billing.js` | `X-Api-Key` on `POST /v2/billing/checkout` | -| `/dashboard` | `frontend/js/dashboard-history.js` | `X-Api-Key` on the usage and history reads | - -So the honest statement of the ceiling is this: on `/parashare` the key is gone, -and on a browser that has loaded any of those three pages it is not. The route -is reachable from any signed-in browser and answers with the raw key, so a -script with a session cookie can also simply ask for it. - -Closing that is the next change, and it is not one line: `/pricing` and -`/dashboard` need scoped credentials of their own (or server-side proxies, which -is what `/api/user/documents` already does), and `/account` has to keep a way to -show a key that a self-hoster genuinely needs. Recorded here rather than fixed, -because a partial fix that removed the route would break three pages, and one -that left it while claiming the key is out of the browser would be the same kind -of untruth this section exists to correct. +#### What was still open, and what closed it + +The `GET /api/user/account/key` reveal route existed and ParaSend was not its +only caller: `/account`, `/pricing` and `/dashboard` each fetched the raw key +into the browser and used it as a relay credential. The honest statement of the +ceiling at the time was that on `/parashare` the key was gone, and on a browser +that had loaded any of those three pages it was not. + +That is now closed, and closing it needed a second purpose rather than a wider +scope. + +**A token is minted FOR a purpose, and the purpose picks the allowlist.** +`relay/lib/session-token.js` holds two lists. `SCOPE` is the ParaSend one and is +unchanged: the five transfer routes above. `APP_SCOPE` is the new one and holds +three routes, each because one page needed exactly it: + +| Route | Page | Why | +|-------|------|-----| +| `POST /v2/billing/checkout` | `/pricing` | pressing a price button creates the Mollie payment | +| `GET /v2/user/history` | `/dashboard` | the account's own send/envelope history, read-only | +| `GET /v2/parasign/audit-export` | `/dashboard` | the account's own signing audit, read-only, Business+ | + +The two lists are **disjoint**. A token minted on `/parashare` is `403` on all +three app routes, and a token minted on `/pricing` is `403` on all five transfer +routes. Merging them into one flat allowlist would have widened the ParaSend +token by three routes to give three other pages a credential they needed, which +is how a narrow credential quietly becomes an api-key again. `/v2/user/history` +is named on its own path, never as a prefix, so the rest of `/v2/user/*` (the +signing-key and TOTP surface) stays shut under both purposes, as do `/v2/keys`, +`/v2/outbound`, `/v2/audit`, `/v2/admin/*` and a second mint. + +The purpose is chosen by the ADMIN ROUTE, never by the caller: +`POST /api/user/parasend/token` asks for `parasend` and +`POST /api/user/app/token` asks for `app`, both ignore their request body, and a +purpose the relay does not recognise is refused at the mint with `400 +unknown_purpose` rather than folded onto a default. A stored record with no +purpose field predates this change and is a ParaSend token; a record carrying a +purpose the running build does not know authenticates nobody at all. + +**What each page does now.** + +| Page | File | Credential | +|------|------|------------| +| `/account` | `frontend/js/account.inline1.js` | the reveal route, and only when the "Advanced account key" fold is opened. Nothing is fetched or rendered on load | +| `/pricing` | `frontend/js/pricing-billing.js` | `Authorization: Bearer pst_...`, purpose `app`, minted on the first click | +| `/dashboard` | `frontend/js/dashboard-history.js` | the same, minted on the click that needs it. `/dashboard` no longer prints a key in any form, masked included | + +`tests/app-pages-no-api-key.test.mjs` drives real Chromium over the three pages +and fails if any of them asks for the key on load, or renders anything shaped +like one. + +**What is left, stated plainly.** The reveal route still exists and still +answers any signed-in browser with the raw key, because `/account` is the page +whose job is to show it to you and a self-hoster genuinely needs it. So a script +that runs on paramant.app with a session cookie can still ask for the key +directly. What changed is that it no longer finds one lying in a variable on a +page nobody opened for that reason, and that the two pages which used to put it +there now run on a credential that expires and opens three routes. --- diff --git a/admin/server.js b/admin/server.js index 0a055e35..0822620b 100644 --- a/admin/server.js +++ b/admin/server.js @@ -2531,7 +2531,12 @@ function maskIp(ip) { // GET /api/user/account/key below stays as it is. It is the reveal a // self-hoster and the account page still need, and it is no longer what // /parashare uses. -api.post("/user/parasend/token", authUser, async (req, res) => { +// +// The purpose is chosen HERE, by the route, never by the caller. A browser that +// could name its own purpose could ask /parashare's endpoint for an `app` token +// and start a checkout with it. Two routes, two fixed words, and the body the +// browser sends is still ignored. +async function mintSessionToken(req, res, purpose, label) { const key = proxyApiKey(req.userSession); if (!key) return res.status(403).json({ error: "no_account_key" }); try { @@ -2542,7 +2547,7 @@ api.post("/user/parasend/token", authUser, async (req, res) => { "X-Internal-Auth": INTERNAL_TOKEN, "X-Api-Key": key, }, - body: "{}", + body: JSON.stringify({ purpose }), signal: AbortSignal.timeout(10000), }); const body = await rr.json().catch(() => ({ error: "bad_relay_response" })); @@ -2556,10 +2561,29 @@ api.post("/user/parasend/token", authUser, async (req, res) => { } return res.json({ token: body.token, expires_in_s: body.expires_in_s }); } catch (err) { - console.error("[user/parasend/token]", err.message); + console.error(label, err.message); return res.status(502).json({ error: "relay_unreachable" }); } -}); +} + +api.post("/user/parasend/token", authUser, (req, res) => + mintSessionToken(req, res, "parasend", "[user/parasend/token]")); + +// POST /api/user/app/token +// +// The same trade for the signed-in app pages. /pricing needs to start a +// checkout and /dashboard needs to read the account's own history and audit +// export; all three used to fetch GET /api/user/account/key and authenticate to +// the relay with the pgp_ key itself, which put a credential with no expiry and +// no scope into any tab that visited them. +// +// A DIFFERENT token from the ParaSend one, not a wider one. The relay judges an +// `app` token against APP_SCOPE (checkout, history, audit-export) and a +// `parasend` token against SCOPE (the five transfer routes); neither list +// contains the other, so this route gives /pricing and /dashboard what they +// need without giving /parashare anything it did not already have. +api.post("/user/app/token", authUser, (req, res) => + mintSessionToken(req, res, "app", "[user/app/token]")); // GET /api/user/account/key api.get("/user/account/key", authUser, async (req, res) => { @@ -2582,7 +2606,11 @@ api.get("/user/dashboard/overview", authUser, async (req, res) => { try { const u = await findUserByEmail(req.userSession.email); if (u && u.plan) plan = u.plan; } catch {} try { const snap = await buildSnapshot({ redis, getAuditEvents, plan }, req.userSession); - const data = { plan: snap.plan, key_masked: snap.key_masked, quota: snap.quota, audit: snap.audit }; + // key_masked is deliberately NOT passed on. /dashboard no longer prints a + // key in any form, and a payload that still carries one is a payload that + // will be printed again by the next person who reads it and assumes it is + // there to be used. The account key lives on /account, behind its fold. + const data = { plan: snap.plan, quota: snap.quota, audit: snap.audit }; _ovCache.set(uid, { at: Date.now(), data }); if (_ovCache.size > 500) _ovCache.clear(); res.json(data); diff --git a/admin/test/parasend-token.test.js b/admin/test/parasend-token.test.js index 5affd6e3..ce19fbc0 100644 --- a/admin/test/parasend-token.test.js +++ b/admin/test/parasend-token.test.js @@ -107,6 +107,7 @@ after(async () => { }); const mint = (headers = {}) => srv.post('/api/user/parasend/token', { headers }); +const mintApp = (headers = {}) => srv.post('/api/user/app/token', { headers }); const mintCalls = () => relay.state.calls.filter(c => c.path === '/v2/session-token'); test('no session is 401, and the relay is never asked', async (t) => { @@ -172,6 +173,62 @@ test('a body cannot name an account: the session decides, always', async (t) => did(); }); +// ── The second route: the signed-in app pages ──────────────────────────────── +// /pricing and /dashboard stopped fetching the account's pgp_ key and mint a +// token of their own. It is a SECOND route rather than a parameter on the first +// one, because a purpose the browser could name is a purpose the browser could +// change: a script on /parashare would ask for an `app` token and start a +// checkout with it. The word is fixed by the route, and the body is still +// ignored. + +test('the app route mints with purpose app, and the browser never chooses that word', async (t) => { + if (!redis) return t.skip('no redis'); + relay.state.calls.length = 0; + const r = await mintApp(await session()); + assert.strictEqual(r.status, 200, r.text); + assert.strictEqual(r.json.token, MINTED); + assert.deepStrictEqual(Object.keys(r.json).sort(), ['expires_in_s', 'token'], + 'the app route answers exactly what the ParaSend one does: a token and its life, nothing else'); + assert.ok(!r.text.includes('pgp_'), 'nothing shaped like an api-key may leave this route either'); + + const call = mintCalls().at(-1); + assert.ok(call, 'the admin must actually ask the relay'); + assert.strictEqual(call.headers['x-api-key'], ACCOUNT_KEY); + assert.strictEqual(call.headers['x-internal-auth'], INTERNAL); + assert.deepStrictEqual(call.body, { purpose: 'app' }, + 'the purpose the relay is asked for must be the route\'s own word'); + did(); +}); + +test('the ParaSend route still asks for the ParaSend purpose, and a body cannot change either', async (t) => { + if (!redis) return t.skip('no redis'); + relay.state.calls.length = 0; + await mint(await session()); + assert.deepStrictEqual(mintCalls().at(-1).body, { purpose: 'parasend' }); + + // A browser trying to name the other purpose on either route gets its own + // route's word, not the one it asked for. This is the whole reason the + // purpose is not a parameter. + for (const [path, expected] of [['/api/user/parasend/token', 'parasend'], ['/api/user/app/token', 'app']]) { + relay.state.calls.length = 0; + const r = await srv.post(path, { headers: await session(), body: { purpose: 'admin' } }); + assert.strictEqual(r.status, 200, r.text); + assert.deepStrictEqual(mintCalls().at(-1).body, { purpose: expected }, + `${path} passed on a purpose the caller supplied`); + } + did(); +}); + +test('the app route needs a session too: no cookie, no token, no mint', async (t) => { + if (!redis) return t.skip('no redis'); + const before = mintCalls().length; + const r = await mintApp(); + assert.strictEqual(r.status, 401); + assert.deepStrictEqual(r.json, { error: 'unauthenticated' }); + assert.strictEqual(mintCalls().length, before, 'an unauthenticated caller must not cause a mint'); + did(); +}); + test('a relay that refuses is passed through as a status, without its words', async (t) => { if (!redis) return t.skip('no redis'); for (const status of [401, 403]) { diff --git a/docs/api.md b/docs/api.md index 80425ba3..5d034532 100644 --- a/docs/api.md +++ b/docs/api.md @@ -36,19 +36,28 @@ Three credential types are in use across different API surfaces: > with the same care as API keys, and revoke the DID enrollment when a device > is retired or compromised. -> **ParaSend session tokens.** A `pst_` token is a narrow, short-lived stand-in +> **Browser session tokens.** A `pst_` token is a narrow, short-lived stand-in > for an account API key, so a browser never has to hold the key itself. It is -> minted by the admin panel on behalf of a logged-in user -> (`POST /api/user/parasend/token`, session cookie), which asks the relay for it -> over the internal channel; the browser is only ever handed the token, never -> the key. Properties, all enforced by the relay: +> minted by the admin panel on behalf of a logged-in user, which asks the relay +> for it over the internal channel; the browser is only ever handed the token, +> never the key. Properties, all enforced by the relay: > -> - **Scope.** An allowlist, checked above every route handler. A token opens +> - **Purpose, and two allowlists.** A token is minted FOR a purpose, and the +> purpose picks the list it is judged against. `parasend` +> (`POST /api/user/parasend/token`, used by `/parashare`) opens > `/v2/check-key`, `POST /v2/ws-ticket`, `POST /v2/pubkey`, -> `GET /v2/pubkey/:device` and `POST /v2/inbound`, and nothing else. Any other -> path answers `403 session_token_out_of_scope`, including `/v2/user/*`, -> `/v2/outbound/:hash`, `/v2/audit`, `/v2/admin/*`, the ParaSign envelope -> routes, and a second `POST /v2/session-token`: a token cannot mint another. +> `GET /v2/pubkey/:device` and `POST /v2/inbound`. `app` +> (`POST /api/user/app/token`, used by `/pricing` and `/dashboard`) opens +> `POST /v2/billing/checkout`, `GET /v2/user/history` and +> `GET /v2/parasign/audit-export`. The two lists are disjoint: neither purpose +> can do the other's work. The purpose is fixed by the admin route, not by the +> caller, and an unknown one is refused at the mint with `400 unknown_purpose`. +> - **Scope.** The allowlist is checked above every route handler. Any path not +> on the list for that purpose answers `403 session_token_out_of_scope`, +> including the rest of `/v2/user/*`, `/v2/keys`, `/v2/outbound/:hash`, +> `/v2/audit`, `/v2/admin/*`, the ParaSign envelope routes, and a second +> `POST /v2/session-token`: no token mints another, whatever it was minted +> for. > - **Identity.** Inside that scope the token authenticates **as the API key it > was minted for**. Monthly quotas (`transfers_month`), the audit chain, > device queues and per-tier limits all resolve against the owner's account, @@ -505,8 +514,15 @@ curl -X POST https://health.paramant.app/v2/session-token \ | 429 | The account already holds 20 live tokens. `Retry-After: 60`. | | 503 | The relay store is unreachable, so no checkable token can be issued. | -The browser-facing half of this is `POST /api/user/parasend/token` on the admin -panel (session cookie, no body, returns only `token` and `expires_in_s`). +The browser-facing half of this is two routes on the admin panel, both session +cookie, both ignoring their request body, both returning only `token` and +`expires_in_s`: `POST /api/user/parasend/token` mints purpose `parasend` and +`POST /api/user/app/token` mints purpose `app`. The purpose is a property of the +route, so a page cannot ask for the other one's authority. + +`POST /v2/session-token` itself takes an optional body `{"purpose": "parasend" | +"app"}`. An absent purpose means `parasend`, so a caller written before purposes +existed is unchanged; an unrecognised one is `400 unknown_purpose`. --- diff --git a/frontend/account.html b/frontend/account.html index 8a49b46b..2ed7ec29 100644 --- a/frontend/account.html +++ b/frontend/account.html @@ -202,7 +202,7 @@