diff --git a/CHANGELOG.md b/CHANGELOG.md index b03c88d..f228366 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,40 @@ Conventional Commits by release-please. now warns however the address is written. An OpenRouter address in capitals is also recognised as OpenRouter. +- Security: a program that took your server's place while it was stopped could keep a + request Kotiko sent it and play it to your server once it was back, and get your word + list, when the clock of the computer Kotiko runs on was ahead of the server's. Your + server now makes a new random id each time it starts, gives it with its proof, and + accepts only requests signed with the current one; after a restart Kotiko asks for a + new proof and sends the request again by itself. Update the server and the extension + together: each turns the other's older version away and says so. + +- Release process: the check that holds store uploads until the security review is closed + still missed "Gate: open" written as a list item, a table row, code, HTML, an entity or + math, a fake "Appendix" heading inside a comment or code block, a closing line hidden in + a comment, and "the review is still open" written as prose. It now reads all of these as + open, counts the closing line and the heading only where a reader sees them, and says + which line keeps the gate open. + +- Self-hosted server: the two addresses that answer without your access key, `/health` and + `/api/v1/proof`, also answered when written another way, such as `/health/` or + `//api/v1/proof`. Now only the exact addresses do; any other spelling needs the key. + +- Self-hosted server: a key written in the query of `LLM_URL` or `TRANSCRIBE_URL` (some + providers take `?key=...`) was written to the log at start. The log now shows the address + without its query (`https://host/v1?…`), and the query's values are taken out of every + log line. A query such as `?api-version=...` now also reaches the provider on every + request, after the path. + +- Self-hosted server behind a reverse proxy: everyone the proxy passes on was counted as + one, so a stranger sending ten wrong keys a minute could keep your other devices out + for as long as they kept at it. Set the new `TRUSTED_PROXY_HEADER` to the header your + proxy puts each visitor's address in (`x-forwarded-for`, `x-real-ip`, + `cf-connecting-ip` or `forwarded`), and each visitor is counted on their own. The server + also recognises more of the headers proxies add (`Via`, `X-Client-IP`, + `Fastly-Client-IP` and others), so visitors through such a proxy are no longer taken for + your own computer, which is never limited. + - Security: a Kotiko server set to a model of its own (`LLM_MODEL`), for example on a paid provider, sent no limit on how long an answer may be, so a model that kept writing could cost you up to the provider's own limit for one word. Every lookup now asks for at most diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index 1f83f17..5cb1dbb 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -95,6 +95,30 @@ behind a reverse proxy). Used only in the pairing string from - Wrong: anything else stops the server ("isn't a web address"), and so does a user name or password in the address (`https://user:password@...`). +### `TRUSTED_PROXY_HEADER` + +Behind a reverse proxy on this computer (Caddy, nginx, `tailscale serve`, a Cloudflare +tunnel): the header that proxy sets to the address of the client it forwards. The +wrong-token lockout and the proof limit then count each client on its own, so a stranger +who sends wrong tokens locks out only themselves, not your other devices. + +- Default: none. Every request a proxy on this computer forwards is then counted together + under the proxy's address (see [`API_TOKEN`](#api_token)): ten wrong tokens a minute + from anyone keep everyone who comes through the proxy out, your other devices included. +- Allowed: `x-forwarded-for` (the rightmost address in the header, the one the proxy + added), `x-real-ip`, `cf-connecting-ip` (its one address), or `forwarded` (the last + element's `for=`), in any case. Wrong: anything else stops the server. +- It is read only for requests from this computer that carry a forwarding header. The + address is counted like any other: an IPv6 address by its /64, an IPv4-mapped one as + IPv4. A request without a usable address in that header (none, two `X-Real-IP` lines, + `unknown`, an obfuscated name) is counted with the proxy's shared count. +- Set it only when the proxy sets or overwrites that header on every request. Otherwise a + client can write any address there and gets a new count with each one. Caddy sets + `X-Forwarded-For`, and nginx appends to it with + `proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;`: either way its rightmost + address is the client the proxy saw. Behind Cloudflare, use `cf-connecting-ip`, since + the rightmost `X-Forwarded-For` address is then Cloudflare's. + ### `API_TOKEN` The token that opens the server. A secret. The extension never sends it: it signs each @@ -118,9 +142,14 @@ send it as `Authorization: Bearer ` (see the [HTTP API](http-api.md#reque page shares, so none of them can lock the extension out; [`POST /api/v1/proof`](http-api.md#post-apiv1proof) isn't limited for them either. Requests a reverse proxy on this computer forwards (they carry a header such as - `X-Forwarded-For` or `Forwarded`) are limited, all together: a stranger's lockout - refuses everyone coming through the proxy for the rest of that minute, never the - extension. A token the server made (256 random bits) can't be guessed, so this costs + `X-Forwarded-For`, `Forwarded` or `Via`; the [HTTP API](http-api.md#requests) lists them) + are limited, all together: a stranger's lockout refuses everyone coming through the + proxy for the rest of that minute, including the extension on your other devices that + reach the server through it. Only the extension on this computer, which sends no + forwarding header, is never locked out. Set + [`TRUSTED_PROXY_HEADER`](#trusted_proxy_header) to count each client behind the proxy + on its own. A proxy that adds none of the listed headers makes its clients look like + this computer's own: they are never limited. A token the server made (256 random bits) can't be guessed, so this costs nothing; a weak token you chose can be guessed by any program on this computer without limit, and the proof lets anyone who can reach the server test guesses offline. Keep the generated token. @@ -152,6 +181,11 @@ The model API: any OpenAI-compatible API's base URL. - A user name or password in the address (`https://user:password@host/v1`) stops the server too, without showing it: the address is written to the log at start. Put the key in [`LLM_API_KEY`](#llm_api_key) (or [`LLM_API_KEY_FILE`](#llm_api_key_file)) instead. +- A query (`?api-version=2024-10-21`, or `?key=...` for a provider that takes its key + there) is kept on every request, after the path (`/chat/completions?api-version=...`). + It is never logged: the start-up summary shows the address as `https://host/v1?…`, and + the query's values are taken out of every log line like the other secrets. A key that + the provider accepts in a header still belongs in `LLM_API_KEY`. - Your typed text goes to this address (README, "A different model"). ### `LLM_MODEL` @@ -196,6 +230,7 @@ upload. - Allowed: an `http://` or `https://` address. Wrong: anything else stops the server, and so does a user name or password in the address: put the key in [`TRANSCRIBE_API_KEY`](#transcribe_api_key) instead. +- A query is kept on the request and never logged, as for [`LLM_URL`](#llm_url). ### `TRANSCRIBE_MODEL` diff --git a/docs/reference/http-api.md b/docs/reference/http-api.md index 7da9e64..d7ce7c8 100644 --- a/docs/reference/http-api.md +++ b/docs/reference/http-api.md @@ -22,7 +22,7 @@ a route is added or removed without updating this page. [`POST /api/v1/proof`](#post-apiv1proof) needs the API token, in one of two ways: - `Authorization: Bearer `, for `curl` and other tools you run yourself; -- a [signed request](#signed-requests), `Authorization: Kotiko-HMAC v1 ...`, which proves +- a [signed request](#signed-requests), `Authorization: Kotiko-HMAC v2 ...`, which proves the sender holds the token without sending it. The browser extension uses only this, so the token never leaves the browser. @@ -50,15 +50,24 @@ logs the lockout once an hour per address. A request from this computer (127.0.0.0/8, `::1` or IPv4-mapped 127.x) with no forwarding header is never locked out: every program on the computer, and any web page open in a browser there, reaches the server from those addresses, so any of them could otherwise -lock the extension out. A request from this computer that carries a forwarding header -(`Forwarded`, `X-Forwarded-For`, `X-Real-IP`, `X-Forwarded-Host`, `CF-Connecting-IP` or `True-Client-IP`, any value) is what a reverse proxy on the same machine sends -for each remote client: those are counted, all together under one key per proxy address, -whatever client address the header claims (it can be forged). A stranger's lockout there -refuses everyone who comes through the proxy for the rest of that minute, but never the -extension, which sends no forwarding header; a local program that adds one locks out only -the proxied requests. A token the server made (256 random bits) can't be guessed in any -case; a weak token you chose can, and the server warns about it at start -([`API_TOKEN`](configuration.md#api_token)). +lock the extension out. A request from this computer that carries a forwarding header is +what a reverse proxy on the same machine sends for each remote client. These headers +count, any value, in any case: `Forwarded`, `Forwarded-For`, `X-Forwarded`, +`X-Forwarded-For`, `X-Forwarded-Host` (and any other `X-Forwarded-*`), +`X-Original-Forwarded-For`, `X-Real-IP`, `X-Client-IP`, `X-Cluster-Client-IP`, +`CF-Connecting-IP`, `True-Client-IP`, `Fastly-Client-IP`, `Via`, and Tailscale's +`Tailscale-User-*`. Those requests are counted all together, under one key per proxy +address, whatever client address the header claims (it can be forged): a stranger's +lockout there refuses everyone who comes through the proxy for the rest of that minute, +the extension on your other devices included. It never refuses the extension on this +computer, which sends no forwarding header; a local program that adds one locks out only +the proxied requests. With [`TRUSTED_PROXY_HEADER`](configuration.md#trusted_proxy_header) +set to the header your proxy sets, each proxied client is counted on its own, by the +address in that header. A proxy that adds none of these headers makes its clients look +like this computer's own, which are never locked out; check that yours adds one +(Caddy and nginx add `X-Forwarded-For` when set up as their guides show). A token the +server made (256 random bits) can't be guessed in any case; a weak token you chose can, +and the server warns about it at start ([`API_TOKEN`](configuration.md#api_token)). **Host names.** Before anything else, the server checks the `Host` header, to stop web pages reaching it through DNS rebinding. It answers to `localhost`, IP addresses, this machine's @@ -93,27 +102,30 @@ client can tell its answers aren't the server's. The extension signs every reque may keep using `Bearer`. ``` -Authorization: Kotiko-HMAC v1 ts=, nonce=, body=, mac= +Authorization: Kotiko-HMAC v2 ts=, nonce=, boot=, body=, mac= ``` -Exactly this form: one space after `Kotiko-HMAC`, `v1`, and after each comma; the four -fields in this order. +Exactly this form: one space after `Kotiko-HMAC`, `v2`, and after each comma; the five +fields in this order. (`v1`, without `boot`, from 1.0.0-rc.1 to rc.3, is refused as +`malformed`.) | Field | Value | |---|---| | `ts` | The client's time, whole seconds since 1970-01-01 UTC (1 to 12 digits) | | `nonce` | 22 to 128 base64url characters (`A-Z a-z 0-9 - _`), new for every request; the extension sends 16 random bytes (22 characters) | +| `boot` | The server's boot id, 22 to 64 base64url characters, as its [proof](#post-apiv1proof) answer gave it | | `body` | SHA-256 of the request body's bytes, 64 lowercase hex digits; of the empty string when there is no body | | `mac` | base64url, without padding (43 characters), of HMAC-SHA256 with the token's UTF-8 bytes as the key, over the canonical request below | -The canonical request is six lines joined by `\n` (LF), with no newline at the end: +The canonical request is seven lines joined by `\n` (LF), with no newline at the end: ``` -kotiko-req-v1 +kotiko-req-v2 + ``` @@ -122,17 +134,29 @@ kotiko-req-v1 percent-decoded), then `?` and the query string if there is one (`/api/v1/words?status=active,paused`). An address with a path prefix (a reverse proxy that mounts the server under `/kotiko` and strips it) signs the path the server sees, without the prefix. -- `ts`, `nonce` and `body` exactly as in the header. - -The server accepts the request when the MAC matches (compared in constant time), `ts` is -within 120 seconds of its own clock and not earlier than the second it started, it hasn't -seen the nonce before, and the body it reads has the signed hash. It remembers each nonce -for the 120 seconds, once the MAC has matched (so only a token holder can fill that -memory), for at most 50,000 requests at once; beyond that it refuses signed requests with -`429 rate_limited` (`details.reason`: `too_many_requests`, with `Retry-After`) rather -than forget a nonce that could then be replayed. Refusing a `ts` from before the server -started covers the nonces a restart forgot: a request someone else received while the -server was stopped can't be played to it once it is back. +- `ts`, `nonce`, `boot` and `body` exactly as in the header. + +The server accepts the request when the MAC matches (compared in constant time), `boot` is +its current boot id, `ts` is no more than 120 seconds before or after its own clock, it +hasn't seen the nonce before, and the body it reads has the signed hash. It remembers each +nonce for those 120 seconds, once the MAC has matched (so only a token holder can fill +that memory), for at most 50,000 requests at once; beyond that it refuses signed requests +with `429 rate_limited` (`details.reason`: `too_many_requests`, with `Retry-After`) +rather than forget a nonce that could then be replayed. + +**The boot id.** The server keeps those nonces in memory, so a restart forgets them. It +makes a new random boot id (16 bytes, 22 base64url characters) each time it starts, and +refuses a request signed with any other (`stale_boot`). So a request someone else received +while the server was stopped (a program that took its port) can't be played to it once it +is back, whatever its `ts`: it carries the old boot id. A client gets the current id from +the proof and signs it into every request; when a request is refused as `stale_boot` (the +server restarted since), the client asks for a new proof and sends the request again. The +server refused it before doing anything, so sending it again is safe. The extension does +this once per request. + +**Clocks.** `ts` may differ from the server's clock by up to 120 seconds either way, so the +computer the extension runs on and the server's need roughly the right time (any system +that sets its clock over the network does). A larger difference gets `stale`. Every answer to an accepted request, errors included, carries the server's signature: @@ -148,30 +172,34 @@ its server (`not_kotiko_server`): it uses nothing from it, forgets the server's A signed request that is refused gets `401 server_key_rejected` without `X-Kotiko-Server`; `details.reason` and the challenge's `error` say why: `malformed` (not the form above; -doesn't count towards the [lockout](#requests)), `bad_mac`, `stale` (outside the 120 -seconds: check both clocks), `replayed` or `body_mismatch`. A server from before signed +doesn't count towards the [lockout](#requests)), `bad_mac`, `stale_boot` (signed for +another boot of the server: ask for a new proof and send it again; doesn't count towards +the lockout, since its MAC matched), `stale` (outside the 120 seconds: check both +clocks), `replayed` or `body_mismatch`. A server from before signed requests answers them `401` with `www-authenticate: Bearer` alone; the extension then asks you to update the server. Before a request that carries your words or settings (anything but `GET`), the extension also asks for a [proof](#post-apiv1proof) when its last one is more than 30 seconds old. -**Test vectors**, with the token `example-token-0123456789abcdef`: +**Test vectors**, with the token `example-token-0123456789abcdef` and the boot id +`AAECAwQFBgcICQoLDA0ODw` (the 16 bytes 00 to 0f): `PUT /api/v1/profile` with the body `{"base_langs":["es","en"],"ui_lang":null}` (those exact bytes), `ts` 1791331200, nonce `q1aP3n0ZKcB1x5mW0u7S9b`: ``` -kotiko-req-v1 +kotiko-req-v2 PUT /api/v1/profile 1791331200 q1aP3n0ZKcB1x5mW0u7S9b +AAECAwQFBgcICQoLDA0ODw 40921055f301e073c3aa377b5ac7d8827ce72c5443e994014c2087b2895f51ff ``` ``` -Authorization: Kotiko-HMAC v1 ts=1791331200, nonce=q1aP3n0ZKcB1x5mW0u7S9b, body=40921055f301e073c3aa377b5ac7d8827ce72c5443e994014c2087b2895f51ff, mac=osa_O2pBaygJz8c64WTOrTLSiY_6vV70NHtPyZ2CZpA +Authorization: Kotiko-HMAC v2 ts=1791331200, nonce=q1aP3n0ZKcB1x5mW0u7S9b, boot=AAECAwQFBgcICQoLDA0ODw, body=40921055f301e073c3aa377b5ac7d8827ce72c5443e994014c2087b2895f51ff, mac=KYWsYYqoHJ_1CJtht-ji67yNLQd-XNEXFaKUCZeTm1s ``` and the server's `200` answer to it: @@ -184,11 +212,13 @@ X-Kotiko-Server: v1 mac=CAmhh5A5suXJTIJNSarztt97Q-RO5eA9K0baiuiw1u0 `dE8gH0iL2nO4pQ6rS8tU0v`: ``` -Authorization: Kotiko-HMAC v1 ts=1791331200, nonce=dE8gH0iL2nO4pQ6rS8tU0v, body=e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855, mac=uHVRpP6SeDO3nxpRjs_zp8V9NYSCV2XP15w9BtwCdPg +Authorization: Kotiko-HMAC v2 ts=1791331200, nonce=dE8gH0iL2nO4pQ6rS8tU0v, boot=AAECAwQFBgcICQoLDA0ODw, body=e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855, mac=_SuhCp1q3PZ4gjDHP-zY0KQKrBe1zK5lxuFmBQuRPdk ``` -(`ts` this old is refused by a live server; the vectors are for checking an -implementation. Both the server's and the extension's tests check them.) +The [proof](#post-apiv1proof) vector gives the `boot_mac` for this boot id. + +(`ts` this old is refused by a live server, and its boot id is random; the vectors are for +checking an implementation. Both the server's and the extension's tests check them.) ## Errors @@ -252,8 +282,9 @@ The [0.2 routes](#get-apiwords) answer errors as `{"error": ""}` instea ### GET /health -Which server this is and whether its database works. No token needed. Answers nothing else -(no word counts, no settings). +Which server this is and whether its database works. No token needed, for this exact path +only: any other spelling (`/health/`, `//health`, `/%68ealth`) needs the token. Answers +nothing else (no word counts, no settings). ```bash curl -s http://127.0.0.1:4747/health @@ -297,11 +328,11 @@ curl -s -H 'Content-Type: application/json' -d '{"nonce": "q1aP3n0ZKcB1x5mW0u7S9 ``` ```json -{"proof": "E5CHaF60VqTTPPduGD1yAlfWMF5xf9_hmV2qCufV-ek"} +{"proof": "E5CHaF60VqTTPPduGD1yAlfWMF5xf9_hmV2qCufV-ek", "boot": "AAECAwQFBgcICQoLDA0ODw", "boot_mac": "U8MycDq8d4O9G5n_qj1tMxNoTFoXHv5cex9_OKZifPY"} ``` -That is the answer of a server whose token is `example-token-0123456789abcdef`; use it to -check your own implementation. +That is the answer of a server whose token is `example-token-0123456789abcdef` and whose +boot id is `AAECAwQFBgcICQoLDA0ODw`; use it to check your own implementation. `proof` is `base64url(HMAC-SHA256(key = the API token, message = "kotiko-proof-v1:" + nonce))`, without padding (43 characters). The key is the token's UTF-8 bytes as you'd send them @@ -309,6 +340,13 @@ in `Authorization`; the message is the ASCII prefix followed by the nonce exactl Compute the same value with the token you have and compare in constant time; send requests only if they match. Use a new nonce each time. +`boot` is the server's [boot id](#signed-requests), new at each start, which every signed +request carries. `boot_mac` is +`base64url(HMAC-SHA256(key = the API token, message = "kotiko-boot-v1\n" + nonce + "\n" + boot))`, +so the boot id is known to be this server's, for this proof. Check it like `proof`; sign +requests with `boot` only if it matches. An answer without `boot` is from a server from +before boot ids (1.0.0-rc.3 and earlier), which needs an update. + - `415 invalid_request` (`details.reason`: `content_type`): the request has no `Content-Type: application/json`. A web page can't send that type to another site without the server's consent (CORS), so it can't use up the limit below. @@ -319,11 +357,12 @@ requests only if they match. Use a new nonce each time. again. Refused requests (`415`, `400`) don't count, and neither do requests from this computer (127.0.0.0/8, `::1`, IPv4-mapped 127.x) without a forwarding header, which every local program and web page shares. Requests from this computer with a forwarding header - (a reverse proxy's) are limited all together, as for wrong tokens ([Requests](#requests)). + (a reverse proxy's) are limited all together, or each by its address with + `TRUSTED_PROXY_HEADER`, as for wrong tokens ([Requests](#requests)). - The answer never contains the token and is sent with `cache-control: no-store`. Like `/health`, it is checked against the `Host` names the server answers to. - Only this exact path and `POST` are open: any other method or spelling - (`/%61pi/v1/proof`) needs the token. + (`/%61pi/v1/proof`, `//api/v1/proof`, `/api/v1/proof/`) needs the token. - Someone who gets a proof can test guesses at the token offline. A token the server made (256 random bits) can't be guessed; a weak one you chose can, and the server warns about those at start ([`API_TOKEN`](configuration.md#api_token)). diff --git a/docs/security/assurance-case.md b/docs/security/assurance-case.md index be07564..2ee6378 100644 --- a/docs/security/assurance-case.md +++ b/docs/security/assurance-case.md @@ -32,13 +32,13 @@ its argument and evidence. | Strangers can't make it do work | Body parsing is a plug after `authorize`; 64 KB cap, 1 MB for the batch route | [`router.ex`](../../server/lib/kotiko/router.ex) `parse_body/2`; [`router_auth_test.exs`](../../server/test/kotiko/router_auth_test.exs) "are parsed only after auth", "over 64 KB are refused with 413"; [`router_v1_test.exs`](../../server/test/kotiko/router_v1_test.exs) "takes at most 500 words, and bodies up to 1 MB" | | Words go only where the user sent them | The only outbound hosts are `LLM_URL`, `TRANSCRIBE_URL` (both set by the owner), `api.telegram.org` when a bot token is set, and the Wiktionary endpoint with the word percent-encoded in the path; tests can't reach the network at all | [`llm/client.ex`](../../server/lib/kotiko/llm/client.ex); [`transcriber.ex`](../../server/lib/kotiko/transcriber.ex); [`telegram.ex`](../../server/lib/kotiko/telegram.ex); [`pronounce.ex`](../../server/lib/kotiko/pronounce.ex) `fetch_page`; [`spec/wiktionary.json`](../../spec/wiktionary.json) `endpoint`; [`no_network_test.exs`](../../server/test/kotiko/no_network_test.exs) | | The bot obeys only listed IDs | Every update's sender ID is checked against `ALLOWED_TELEGRAM_IDS` before dispatch; with no list, the bot only replies with the sender's ID | [`bot.ex`](../../server/lib/kotiko/bot.ex) `handle/1`; [`bot_test.exs`](../../server/test/kotiko/bot_test.exs) "messages from people not in ALLOWED_TELEGRAM_IDS are ignored" | -| Keys and tokens stay out of logs | A primary `:logger` filter rewrites every event that contains a configured secret or a key-shaped string; an event it can't read is dropped | [`log/redact.ex`](../../server/lib/kotiko/log/redact.ex); [`redact_test.exs`](../../server/test/kotiko/log/redact_test.exs) | +| Keys and tokens stay out of logs | A primary `:logger` filter rewrites every event that contains a configured secret (the four secret settings and the values in `LLM_URL`'s and `TRANSCRIBE_URL`'s queries, E-03) or a key-shaped string; an event it can't read is dropped. Service addresses are logged without user name, password or query (`?…`) | [`log/redact.ex`](../../server/lib/kotiko/log/redact.ex); [`config.ex`](../../server/lib/kotiko/config.ex) `loggable_url/1`, `url_secrets/1`; [`redact_test.exs`](../../server/test/kotiko/log/redact_test.exs); [`startup_summary_test.exs`](../../server/test/kotiko/startup_summary_test.exs) "a key in LLM_URL's or TRANSCRIBE_URL's query (E-03)" | | Words stay out of logs at the default level | Lookup input and model answers are logged only with `LOG_LOOKUPS=true`, at debug; save failures log the field names, not the values | [`llm.ex`](../../server/lib/kotiko/llm.ex) `log_lookups?/0`; [`router_v1.ex`](../../server/lib/kotiko/router_v1.ex) `result_json/1`; [`llm_test.exs`](../../server/test/kotiko/llm_test.exs) "no path logs the input or a returned word at the default level" | | Bad settings stop the server; secrets aren't echoed | Every variable is parsed by one function that returns a problem; any problem halts with status 78 and one message; secret problems are labelled "(value hidden)" | [`config.ex`](../../server/lib/kotiko/config.ex) `parse/2`, `secret_error/2`; [`config_test.exs`](../../server/test/kotiko/config_test.exs) "a short API_TOKEN is refused without echoing it" | | Text never becomes HTML in the extension | ESLint bans `innerHTML`, `outerHTML` and `insertAdjacentHTML` in `extension/` and runs `eslint-plugin-no-unsanitized`; CI fails on any finding | [`eslint.config.js`](../../eslint.config.js); [`ci.yml`](../../.github/workflows/ci.yml) `npm run lint` | | Pages can't use the extension to change words | One message router checks each sender's kind (extension page or content script) against the handler's allowed list; content scripts may only `sync`, read `sensitiveSites` and mark one word never to swap (`neverSwap`); `oauth.code` is accepted only from the docs site's callback page (`https://kotiko.org/connect/` with a code; the site's own test checks that page has no script), and only with the random `state` of the sign-in waiting, so a page that opens the callback with a code of its own neither reaches OpenRouter nor uses up the sign-in | [`lib/messages.js`](../../extension/lib/messages.js) `senderKinds`, `createMessageRouter`; [`background.js`](../../extension/background.js) handler table; [`sync-libs.test.mjs`](../../test/unit/sync-libs.test.mjs) "disallowed senders get forbidden"; [`local-mode.test.mjs`](../../test/bg/local-mode.test.mjs) "content scripts can't set, describe or remove secrets, change modes, write words or move them" | | Pages can't change your settings, word list or add queue | Content scripts can write `storage.local` and `storage.sync` (Firefox can't stop them). The background keeps the real copy of every setting, of the pages' word list and of the add jobs in its IndexedDB, reads only that, and mirrors it to `storage.local`; a change it didn't make there is put back and an unknown key removed. Pages change settings with `settings.set` (pages only), checked against an allowlist of keys and shapes; content scripts may only add or remove one never-swap word. Chrome 140+ closes `storage.sync` to content scripts (`setAccessLevel`), and only then are changes there taken | [`lib/settings.js`](../../extension/lib/settings.js) `createArea`, `edit`; [`background.js`](../../extension/background.js) `area`, `adoptOnce`, `adoptSync`, `saveSettings`; [`settings.test.mjs`](../../test/bg/settings.test.mjs) (one test per attack); [`settings.test.mjs`](../../test/unit/settings.test.mjs); [`privacy.spec.mjs`](../../test/e2e/privacy.spec.mjs) "settings, the add queue and the bases written from a content script are put back and never used" | -| The token never leaves the browser, and a server that doesn't hold it gets nothing it can use | The extension never sends the token (slice 54, D-01). It signs each request with it: HMAC-SHA256 over the method, the path and query, a timestamp, a random nonce and the body's SHA-256 (`Authorization: Kotiko-HMAC v1 ...`). The server checks the MAC in constant time, refuses a timestamp more than 120 s off or from before it started and a nonce it has seen, and checks the body it reads against the signed hash; a failed signature counts like a wrong token. It signs every answer to an accepted request (`X-Kotiko-Server`, HMAC over the nonce and status), and the extension uses no answer without that signature: it forgets the server's proof and sends nothing more until a new one. Before the first request to an address, and before any request that carries the learner's words or settings (anything but `GET`) when the last proof is over 30 s old, the server proves it holds the token (`POST /api/v1/proof`, HMAC of a fresh nonce, checked with WebCrypto); a missing or wrong proof means nothing is sent. The proof is also forgotten on `server.connect`, an address or token change and any connection error. The extension uses `127.0.0.1` for a typed `localhost` (B-01) | [`lib/server-auth.js`](../../extension/lib/server-auth.js); [`background.js`](../../extension/background.js) `request`, `proveServer`, `askProof`; [`request_auth.ex`](../../server/lib/kotiko/request_auth.ex); [`router.ex`](../../server/lib/kotiko/router.ex) `check_signed/3`, `parse_body/2`; [`router_signed_test.exs`](../../server/test/kotiko/router_signed_test.exs) (the vectors, wrong MAC, stale time, replay, tampered body, every route); [`server-auth.test.mjs`](../../test/unit/server-auth.test.mjs) (the same vectors); [`background.test.mjs`](../../test/bg/background.test.mjs) "signed requests: the token never leaves the browser (D-01)", "the server proves it holds the token before it gets it"; [`loopback.spec.mjs`](../../test/e2e/loopback.spec.mjs) "after the server stops and a listener takes its port, the listener gets no token and no word" | +| The token never leaves the browser, and a server that doesn't hold it gets nothing it can use | The extension never sends the token (slice 54, D-01). It signs each request with it: HMAC-SHA256 over the method, the path and query, a timestamp, a random nonce, the server's boot id and the body's SHA-256 (`Authorization: Kotiko-HMAC v2 ...`). The server checks the MAC in constant time, refuses a request signed for another boot id (a new random one at each start, given with the proof and MAC'd for its nonce; security review E-01), a timestamp more than 120 s off either way and a nonce it has seen, and checks the body it reads against the signed hash; a failed signature counts like a wrong token (a `stale_boot` one, whose MAC matched, doesn't; the extension proves again and resends it once). It signs every answer to an accepted request (`X-Kotiko-Server`, HMAC over the nonce and status), and the extension uses no answer without that signature: it forgets the server's proof and sends nothing more until a new one. Before the first request to an address, and before any request that carries the learner's words or settings (anything but `GET`) when the last proof is over 30 s old, the server proves it holds the token (`POST /api/v1/proof`, HMAC of a fresh nonce, checked with WebCrypto); a missing or wrong proof means nothing is sent. The proof is also forgotten on `server.connect`, an address or token change and any connection error. The extension uses `127.0.0.1` for a typed `localhost` (B-01) | [`lib/server-auth.js`](../../extension/lib/server-auth.js); [`background.js`](../../extension/background.js) `request`, `proveServer`, `askProof`; [`request_auth.ex`](../../server/lib/kotiko/request_auth.ex); [`router.ex`](../../server/lib/kotiko/router.ex) `check_signed/3`, `parse_body/2`; [`router_signed_test.exs`](../../server/test/kotiko/router_signed_test.exs) (the vectors, wrong MAC, stale time, replay, a request signed before a restart, tampered body, every route); [`server-auth.test.mjs`](../../test/unit/server-auth.test.mjs) (the same vectors); [`background.test.mjs`](../../test/bg/background.test.mjs) "signed requests: the token never leaves the browser (D-01)" (with the `stale_boot` retry), "the server proves it holds the token before it gets it"; [`fullstack.spec.mjs`](../../test/e2e/fullstack.spec.mjs) "a request caught while the server was stopped" (reviewer E's replay after a restart, on the real server); [`loopback.spec.mjs`](../../test/e2e/loopback.spec.mjs) "after the server stops and a listener takes its port, the listener gets no token and no word" | | Key and token out of content scripts' reach | Secrets live in the extension origin's IndexedDB, which content scripts can't open; the upgrade moves an old token out of `storage.local` | [`lib/store.js`](../../extension/lib/store.js) `secrets`; [`lib/local-mode.js`](../../extension/lib/local-mode.js) migration step 6; [`local-mode.test.mjs`](../../test/bg/local-mode.test.mjs) "the key and the server token are in no storage area a content script can read" | | Answers are checked before use | Server word lists pass `validateWordsResponse` (shape, types, lengths, counts); model answers pass the shared word spec in both runtimes, against the same fixtures | [`lib/validate-words.js`](../../extension/lib/validate-words.js); [`lib/wordspec.js`](../../extension/lib/wordspec.js); [`word_spec.ex`](../../server/lib/kotiko/word_spec.ex) `process/2`; [`sync-libs.test.mjs`](../../test/unit/sync-libs.test.mjs) "validateWordsResponse"; [`wordspec.test.mjs`](../../test/unit/wordspec.test.mjs), [`word_spec_test.exs`](../../server/test/kotiko/word_spec_test.exs) | | No telemetry | The only network calls are to the user's server, the model provider, OpenRouter's key exchange (when the user signs in there), Telegram and Wiktionary, as listed above and in [`background.js`](../../extension/background.js) and [`lib/llm/client.js`](../../extension/lib/llm/client.js) | Code reading; slice 28 writes the full data inventory (planned) | @@ -133,7 +133,7 @@ a small JSON API. | CWE-78 OS command injection | The server runs no external commands (no `System.cmd`, `Port.open` or `:os.cmd` in `server/lib`). The shell scripts read only the owner's `.env` | In place | `server/lib/`; ShellCheck in [`ci.yml`](../../.github/workflows/ci.yml) | | CWE-22 Path traversal | No file path comes from a request. The data folder comes from the owner's settings; backup names are built from the version and the time. Sobelow fails CI on any new file call with a path it can't rule out; each accepted one says why | In place | [`config.ex`](../../server/lib/kotiko/config.ex) `data_dir/4`; [`migrations.ex`](../../server/lib/kotiko/migrations.ex); [`.sobelow-conf`](../../server/.sobelow-conf) | | CWE-306, CWE-862 Missing authentication or authorization | Deny by default; one token for everything (no roles yet, so no authorization levels to get wrong). The only open routes are `/health` and `POST /api/v1/proof`, which proves the server holds the token (HMAC of a client nonce) without revealing it | In place; per-user authorization planned (slice 48) | `authorize/2`; [`token.ex`](../../server/lib/kotiko/token.ex) `proof/2`; [`router_auth_test.exs`](../../server/test/kotiko/router_auth_test.exs); [`router_proof_test.exs`](../../server/test/kotiko/router_proof_test.exs) | -| CWE-307 Excessive authentication attempts | After 10 wrong tokens or failed signatures from one address (an IPv6 /64) within a minute, that address gets 429 on every authenticated route until the minute ends; other addresses are unaffected; the lockout is logged once an hour per address. Proofs with a well-formed nonce and `Content-Type: application/json` are limited to 30 a minute per address; malformed ones are refused before they are counted, so a web page (which can't send JSON cross-site without CORS) can't use them up. Requests from this computer's own addresses (127.0.0.0/8, `::1`, IPv4-mapped 127.x) without a forwarding header are exempt from both (slice 54, D-02): every local program and web page shares them, so a limit there would let any of them lock the extension out. Loopback requests with a forwarding header (`Forwarded`, `X-Forwarded-For`, `X-Real-IP`, `X-Forwarded-Host`, `CF-Connecting-IP`, `True-Client-IP`), which is how a reverse proxy on the same machine forwards remote clients, keep both limits, counted together per proxy address whatever client the header claims (it can be forged); the extension sends no such header, so a local program that adds one locks out only the proxied requests. A generated token (256 random bits) can't be guessed; a proof lets someone test guesses offline, so a weak chosen token gets a warning at start | In place | [`auth_throttle.ex`](../../server/lib/kotiko/auth_throttle.ex); [`rate_limit.ex`](../../server/lib/kotiko/rate_limit.ex) `peer/1`; [`router_throttle_test.exs`](../../server/test/kotiko/router_throttle_test.exs) "this computer's addresses are never locked out", "loopback requests with a forwarding header are limited together"; [`router_proof_test.exs`](../../server/test/kotiko/router_proof_test.exs) "31 malformed proofs", "this computer's addresses aren't limited", "proofs through a reverse proxy on this computer are limited together" | +| CWE-307 Excessive authentication attempts | After 10 wrong tokens or failed signatures from one address (an IPv6 /64) within a minute, that address gets 429 on every authenticated route until the minute ends; other addresses are unaffected; the lockout is logged once an hour per address. Proofs with a well-formed nonce and `Content-Type: application/json` are limited to 30 a minute per address; malformed ones are refused before they are counted, so a web page (which can't send JSON cross-site without CORS) can't use them up. Requests from this computer's own addresses (127.0.0.0/8, `::1`, IPv4-mapped 127.x) without a forwarding header are exempt from both (slice 54, D-02): every local program and web page shares them, so a limit there would let any of them lock the extension out. Loopback requests with a forwarding header (`Forwarded`, `Forwarded-For`, `X-Forwarded`, any `X-Forwarded-*`, `X-Original-Forwarded-For`, `X-Real-IP`, `X-Client-IP`, `X-Cluster-Client-IP`, `CF-Connecting-IP`, `True-Client-IP`, `Fastly-Client-IP`, `Via`, `Tailscale-User-*`; security review E-02), which is how a reverse proxy on the same machine forwards remote clients, keep both limits, counted together per proxy address whatever client the header claims (it can be forged); the extension on this computer sends no such header, so a local program that adds one locks out only the proxied requests (the owner's other devices behind the proxy among them). With `TRUSTED_PROXY_HEADER` (validated at start) naming the header the proxy sets, each proxied client is counted by that header's address (rightmost `X-Forwarded-For`, last `Forwarded` `for=`, or the one `X-Real-IP`/`CF-Connecting-IP`), normalised like a peer, so a stranger locks out only themselves. A proxy that adds none of the listed headers makes its clients look local (unlimited); the reference says so. A generated token (256 random bits) can't be guessed; a proof lets someone test guesses offline, so a weak chosen token gets a warning at start | In place | [`auth_throttle.ex`](../../server/lib/kotiko/auth_throttle.ex); [`rate_limit.ex`](../../server/lib/kotiko/rate_limit.ex) `peer/1`; [`router_throttle_test.exs`](../../server/test/kotiko/router_throttle_test.exs) "this computer's addresses are never locked out", "loopback requests with a forwarding header are limited together", "every forwarding header a proxy may add keeps the limit", "with TRUSTED_PROXY_HEADER, each proxied client is counted on its own"; [`rate_limit_test.exs`](../../server/test/kotiko/rate_limit_test.exs) "TRUSTED_PROXY_HEADER (E-02)"; [`router_proof_test.exs`](../../server/test/kotiko/router_proof_test.exs) "31 malformed proofs", "this computer's addresses aren't limited", "proofs through a reverse proxy on this computer are limited together" | | CWE-352 Cross-site request forgery | The credential (a `Bearer` token or a request signature) travels in a header, never a cookie; the server sends no CORS headers, so a page can't attach the header (that needs a preflight the server doesn't answer) | In place | [`router_auth_test.exs`](../../server/test/kotiko/router_auth_test.exs) "a CORS preflight gets no CORS headers" | | CWE-918 Server-side request forgery | The server fetches only URLs the owner set (`LLM_URL`, `TRANSCRIBE_URL`) and fixed hosts (Telegram, Wiktionary with the word percent-encoded in the path). No request can choose a URL | In place | [`config.ex`](../../server/lib/kotiko/config.ex); [`pronounce.ex`](../../server/lib/kotiko/pronounce.ex) | | CWE-502 Unsafe deserialization | Nothing from the network becomes an Erlang term: requests and API answers are parsed as JSON, Wiktionary pages are read as text. One call reads back the lookup cache the server itself wrote to its own database, with `Plug.Crypto.non_executable_binary_to_term/2` and `:safe`, so a changed row can neither create atoms nor carry a function; Sobelow checks for any other | In place | [`llm/cache.ex`](../../server/lib/kotiko/llm/cache.ex) `decode/1`; `llm_test.exs` ("a stored entry holding a function is a miss") | @@ -148,7 +148,7 @@ a small JSON API. | Input | Where it enters | Rule | Location | |---|---|---|---| -| HTTP method, path, headers | Every request | Host allowlist; exactly one `Authorization` header: `Bearer` compared in constant time, or `Kotiko-HMAC v1` in exactly one form (time 1 to 12 digits, nonce 22 to 128 base64url, body hash 64 hex, MAC 43 base64url), its MAC compared in constant time, its time within 120 s and not before the server started, its nonce unseen; unknown routes 404 | [`host_check.ex`](../../server/lib/kotiko/plug/host_check.ex); [`router.ex`](../../server/lib/kotiko/router.ex) | +| HTTP method, path, headers | Every request | Host allowlist; exactly one `Authorization` header: `Bearer` compared in constant time, or `Kotiko-HMAC v2` in exactly one form (time 1 to 12 digits, nonce 22 to 128 base64url, boot id 22 to 64 base64url, body hash 64 hex, MAC 43 base64url), its MAC compared in constant time, its boot id this start's, its time within 120 s either way, its nonce unseen; unknown routes 404 | [`host_check.ex`](../../server/lib/kotiko/plug/host_check.ex); [`router.ex`](../../server/lib/kotiko/router.ex) | | HTTP body | `POST` and `PATCH` routes | JSON only, size cap, parsed after auth (a signed request's body read first, within the cap, and checked against its signed hash); then per-field checks: text 1 to 200 characters after normalisation, base languages (at most 4, canonical tags), `client_request_id` a UUID, word fields against the word spec and its caps, `limit` 1 to 20,000, `status` from a fixed list, `if_updated_at` an ISO 8601 time | [`router_v1.ex`](../../server/lib/kotiko/router_v1.ex); [`word_spec.ex`](../../server/lib/kotiko/word_spec.ex) `prepare_input/1`, `validate_word/2`, `patch/1`; [`spec/rules.json`](../../spec/rules.json) | | Legacy `POST /api/words` body | 0.2 extensions | Text must be a non-empty string; not run through the 200-character input check (the 64 KB body cap applies) | [`router.ex`](../../server/lib/kotiko/router.ex) | | Environment (`.env`) | Server start | One parser per variable; any problem stops the server | [`config.ex`](../../server/lib/kotiko/config.ex) `parse/2` | @@ -193,7 +193,12 @@ These match "What you can't expect" in the [requirements](requirements.md). it took the port within 30 s of the last proof and before any other request found it out; then it reads that request's body (and each other one already in flight at that moment), never the token. It can't play any request to the server later: a nonce is - accepted once, a request's time must be within 120 s and not before the server started. + accepted once while the server runs, a request's time must be within 120 s of the + server's clock, and every request signs the boot id the server gave with its last proof, + which each start replaces with a new random one, so a request caught while the server + was stopped is refused (`stale_boot`) once it is back, whatever its time (security + review E-01; before, a request stamped up to 120 s ahead of the server's clock was + accepted after a restart, since the nonces it had seen were gone). - **What a signature doesn't do.** It authenticates; it doesn't encrypt. The server's signature covers the answer's status, not its body, so a program that sits between the extension and a running server (on a network without HTTPS, or a local listener that diff --git a/docs/security/requirements.md b/docs/security/requirements.md index 9822229..1882859 100644 --- a/docs/security/requirements.md +++ b/docs/security/requirements.md @@ -17,7 +17,9 @@ Kotiko has two parts: the browser extension, and an optional server you run your - **Nothing but `/health` answers without your token.** Every other request, whatever its method or however its path is spelled (`/%61pi`, `/API`, `//api`), gets `401` unless it carries `Authorization: Bearer ` or is signed with it (the extension signs; - it never sends the token, and uses an answer only if the server signed it too). `/health` says only the server's name, + it never sends the token, and uses an answer only if the server signed it too). A signed + request can't be sent to the server again, while it runs or after it restarts, and the + two computers' clocks may differ by up to two minutes. `/health` says only the server's name, version and whether its database works. - **A strong token, kept private.** On first start the server makes a random 256-bit token and saves it in `api-token` in the data folder, readable only by you (mode 0600). If you @@ -26,7 +28,8 @@ Kotiko has two parts: the browser extension, and an optional server you run your tokens from one address within a minute, that address is refused until the minute is over; other addresses keep working. Requests from this computer itself are never refused, so no local program or web page can lock the extension out; those a reverse - proxy on this computer forwards are limited together. + proxy on this computer forwards are limited together (your other devices behind the + proxy included), or each by its own address when you set `TRUSTED_PROXY_HEADER`. - **Other accounts on the computer can't read your words.** The database, its `-wal` and `-shm` files, the backups, the token and the model list cache are readable only by you (0600), and a data folder the server makes only by you (0700). At every start it makes @@ -53,7 +56,8 @@ Kotiko has two parts: the browser extension, and an optional server you run your token, your model and transcription keys, your bot token, and anything shaped like a bearer token or an OpenRouter or OpenAI key, before a log line is written. A user name or password inside `LLM_URL`, `TRANSCRIBE_URL` or `PUBLIC_URL` stops the server instead - of reaching the log. + of reaching the log. A query in `LLM_URL` or `TRANSCRIBE_URL` (where some providers take + a key) is never logged, and its values are removed from every log line. - **Your words stay out of the logs** at the default level. What you look up and what the model answered go to the log only if you set `LOG_LOOKUPS=true`, and then only at debug level. diff --git a/docs/stores.md b/docs/stores.md index 6244d7a..3ac6699 100644 --- a/docs/stores.md +++ b/docs/stores.md @@ -264,7 +264,13 @@ The candidate it names is one of the reviewed version's. A second `Gate:` line ( `Gate: open`) above the appendix keeps the gate open, and lines under the appendix count for nothing. Lines are read as a reader sees them: a gate line written with a lookalike letter from another alphabet, an invisible character or a fullwidth colon still counts as a second -gate line, and the closing line itself must be plain ASCII (security review D-05). When the +gate line, and the closing line itself must be plain ASCII (security review D-05). So does +an open line dressed up as a list item, a table row, inline code, HTML, an entity or math +(`1. Gate: open`, `| Gate | open |`, `Gate: open`, `Gate: open`), a line in an +HTML comment or a code block, and prose that says "open" beside "gate", "review" or +"status". The closing line and the `## Appendix` heading count only where a reader sees +them: not in an HTML comment, a code block or an HTML block, and the heading in plain ASCII. +Keep the closing line in a paragraph of its own (security review E-06). When the gate stays open, the job says why for each report. That blocks every store upload until the first review is done. When a review is opened for the version being released (a `review-vX.Y.Z-rc.N.md` for that version), that review must be the closed one. Whether a later release needs a review at all (it touches diff --git a/extension/background.js b/extension/background.js index bfdb938..f6fd3b8 100644 --- a/extension/background.js +++ b/extension/background.js @@ -499,16 +499,23 @@ async function connection() { // found it out. The proof is forgotten on `server.connect`, when the address or token // changes, when the server can't be reached, and after any answer the server didn't sign; // nothing more is sent until a new proof succeeds. -// - 200 with a proof that doesn't match: that server has another token (the learner's is -// wrong, or it isn't theirs): server_key_rejected, reason "wrong_proof". +// The proof answer also carries the server's boot id, signed for this nonce (`boot_mac`), +// which every request signs: the server makes a new one at each start and refuses a request +// signed for another (`stale_boot`), so a request another program caught while the server +// was stopped can't be played to it once it is back (security review E-01). When a request +// is refused as `stale_boot` (the server restarted since the last proof), Kotiko proves +// again and sends it once more. +// - 200 with a proof, or a boot id, that doesn't match: that server has another token +// (the learner's is wrong, or it isn't theirs): server_key_rejected, reason "wrong_proof". // - no route (401, 404 or 405: a server from before this check), or an answer with no -// proof: not_kotiko_server, reason "no_proof". An older server has to be updated: -// anything could answer 404, so a missing route can't be trusted either. +// proof or no boot id (a server from before E-01): not_kotiko_server, reason "no_proof". +// An older server has to be updated: anything could answer 404, so a missing route +// can't be trusted either. const PROOF_CONTEXT = "kotiko-proof-v1:"; const PROOF_TIMEOUT_MS = 10_000; // How old a proof may be before a request that carries the learner's data. const PROOF_FRESH_MS = 30_000; -let proven = null; // {base, token, at} proven in this worker's life +let proven = null; // {base, token, at, boot} proven in this worker's life let proving = null; // {base, token, promise} under way const utf8 = (text) => new TextEncoder().encode(text); const b64url = (bytes) => btoa(String.fromCharCode(...bytes)).replace(/\+/g, "-").replace(/\//g, "_").replace(/=+$/, ""); @@ -518,13 +525,15 @@ function forgetProof() { proving = null; } -// Resolves once `base` has proved it holds `token`, within `maxAge` ms when given. +// Resolves to the server's boot id once `base` has proved it holds `token`, within +// `maxAge` ms when given. function proveServer(base, token, maxAge = Infinity) { - if (proven?.base === base && proven.token === token && now() - proven.at < maxAge) return Promise.resolve(); + if (proven?.base === base && proven.token === token && now() - proven.at < maxAge) return Promise.resolve(proven.boot); if (proving?.base === base && proving.token === token) return proving.promise; const promise = askProof(base, token) - .then(() => { - if (proving?.promise === promise) proven = { base, token, at: now() }; + .then((boot) => { + if (proving?.promise === promise) proven = { base, token, at: now(), boot }; + return boot; }) .finally(() => { if (proving?.promise === promise) proving = null; @@ -561,17 +570,22 @@ async function askProof(base, token) { } const proof = typeof body?.proof === "string" && /^[A-Za-z0-9_-]{43}$/.test(body.proof) ? body.proof : null; if (!proof) throw noProof(res.status); + const wrong = () => codedError("server_key_rejected", `The server at ${base} has another token.`, { reason: "wrong_proof", status: res.status }); const key = await crypto.subtle.importKey("raw", utf8(token), { name: "HMAC", hash: "SHA-256" }, false, ["verify"]); - if (!(await crypto.subtle.verify("HMAC", key, ServerAuth.fromB64url(proof), utf8(PROOF_CONTEXT + nonce)))) { - throw codedError("server_key_rejected", `The server at ${base} has another token.`, { reason: "wrong_proof", status: res.status }); - } + if (!(await crypto.subtle.verify("HMAC", key, ServerAuth.fromB64url(proof), utf8(PROOF_CONTEXT + nonce)))) throw wrong(); + // A server from before boot ids (E-01) can't check today's requests: it needs an update. + if (body.boot === undefined) throw noProof(res.status); + if (!(await ServerAuth.verifyBoot(token, nonce, body.boot, body.boot_mac))) throw wrong(); + return body.boot; } // An answer the server didn't sign: nothing in it is used. A 401 from a server from before // signed requests (its challenge is `Bearer` alone) means it needs an update ("no_proof"); // today's server names `Kotiko-HMAC` and why it refused (`stale`: the clocks differ by -// over two minutes). Neither is authenticated, so they only choose the message. Anything -// else (a captive portal, another program) isn't a Kotiko server ("unsigned"). +// over two minutes; `stale_boot`: the server restarted since the proof, which request() +// handles with one more proof). Neither is authenticated, so they only choose the message +// or that one retry. Anything else (a captive portal, another program) isn't a Kotiko +// server ("unsigned"). function unsignedAnswer(base, res) { const c = ServerAuth.challenge(res.headers.get("www-authenticate")); if (res.status === 401 && c.signs) return codedError("server_key_rejected", `${base} refused the request's signature.`, { reason: c.reason ?? "signature", status: 401 }); @@ -581,14 +595,16 @@ function unsignedAnswer(base, res) { } // Calls the server, signed, and returns its answer once the server's signature on it is -// checked. Network failures and unsigned answers become coded errors. -async function request(conn, path, init = {}) { +// checked. Network failures and unsigned answers become coded errors. A request refused as +// signed for the server's previous boot (it restarted since the proof) is sent once more +// after a new proof; the server refused it before doing anything with it. +async function request(conn, path, init = {}, retried = false) { const method = String(init.method ?? "GET").toUpperCase(); // The learner's words and settings go only to a server that proved itself just now. - if (method !== "GET") await proveServer(conn.base, conn.token, PROOF_FRESH_MS); + const boot = await proveServer(conn.base, conn.token, method === "GET" ? Infinity : PROOF_FRESH_MS); const url = `${conn.base}${path}`; const body = typeof init.body === "string" ? init.body : ""; - const { header, nonce } = await ServerAuth.sign(conn.token, { method, path: ServerAuth.target(conn.base, url), body, ts: Math.floor(now() / 1000) }); + const { header, nonce } = await ServerAuth.sign(conn.token, { method, path: ServerAuth.target(conn.base, url), body, ts: Math.floor(now() / 1000), boot }); let res; try { res = await fetch(url, { @@ -612,7 +628,9 @@ async function request(conn, path, init = {}) { if (!(await ServerAuth.verifyResponse(conn.token, nonce, res.status, res.headers.get("x-kotiko-server")))) { forgetProof(); res.body?.cancel?.().catch?.(() => {}); - throw unsignedAnswer(conn.base, res); + const refused = unsignedAnswer(conn.base, res); + if (!retried && refused.details?.reason === "stale_boot") return request(conn, path, init, true); + throw refused; } return res; } diff --git a/extension/lib/server-auth.js b/extension/lib/server-auth.js index 18a9b6a..02c2e4f 100644 --- a/extension/lib/server-auth.js +++ b/extension/lib/server-auth.js @@ -8,17 +8,25 @@ // answer. The server's side is server/lib/kotiko/request_auth.ex; the scheme and its test // vectors are in docs/reference/http-api.md ("Signed requests"). // -// Authorization: Kotiko-HMAC v1 ts=, nonce=, body=, mac= -// mac = base64url(HMAC-SHA256(token, "kotiko-req-v1\n" + METHOD + "\n" + path + "\n" + ts -// + "\n" + nonce + "\n" + body)) +// Authorization: Kotiko-HMAC v2 ts=, nonce=, boot=, body=, mac= +// mac = base64url(HMAC-SHA256(token, "kotiko-req-v2\n" + METHOD + "\n" + path + "\n" + ts +// + "\n" + nonce + "\n" + boot + "\n" + body)) // X-Kotiko-Server: v1 mac= // // `path` is the request target from the server's root: the path and query as sent, without // the prefix of an address that has one (a reverse proxy that mounts the server under // /kotiko hands it /api/words). `body` is the hex SHA-256 of the body's UTF-8 bytes. +// `boot` is the server's boot id, from its proof answer, where it comes with +// boot_mac = base64url(HMAC-SHA256(token, "kotiko-boot-v1\n" + proof nonce + "\n" + boot)). +// The server makes a new one at each start and refuses a request signed with another +// (`stale_boot`), so a request caught while it was stopped can't be played to it after a +// restart (security review E-01). (function () { - const REQUEST = "kotiko-req-v1"; + const REQUEST = "kotiko-req-v2"; const RESPONSE = "kotiko-resp-v1"; + const BOOT = "kotiko-boot-v1"; + const BOOT_ID = /^[A-Za-z0-9_-]{22,64}$/; + const MAC = /^[A-Za-z0-9_-]{43}$/; const NONCE_BYTES = 16; const RESPONSE_HEADER = /^v1 mac=([A-Za-z0-9_-]{43})$/; const utf8 = (text) => new TextEncoder().encode(text); @@ -50,19 +58,29 @@ return path + u.search; } - const canonical = ({ method, path, ts, nonce, bodyHash }) => [REQUEST, method, path, String(ts), nonce, bodyHash].join("\n"); + const canonical = ({ method, path, ts, nonce, boot, bodyHash }) => [REQUEST, method, path, String(ts), nonce, boot, bodyHash].join("\n"); const hmacKey = (subtle, token, usage) => subtle.importKey("raw", utf8(token), { name: "HMAC", hash: "SHA-256" }, false, [usage]); - // The Authorization value for one request, and its nonce (to check the answer with). - // `ts` and `nonce` are for tests; a request gets the time and 16 random bytes. - async function sign(token, { method, path, body = "", ts, nonce }, crypto = globalThis.crypto) { + // The Authorization value for one request to the server whose boot id is `boot`, and its + // nonce (to check the answer with). `ts` and `nonce` are for tests; a request gets the + // time and 16 random bytes. + async function sign(token, { method, path, body = "", ts, nonce, boot }, crypto = globalThis.crypto) { + if (!BOOT_ID.test(String(boot ?? ""))) throw new TypeError("A signed request needs the server's boot id."); const n = nonce ?? b64url(crypto.getRandomValues(new Uint8Array(NONCE_BYTES))); const t = String(ts ?? Math.floor(Date.now() / 1000)); const bodyHash = hex(await crypto.subtle.digest("SHA-256", utf8(body))); const key = await hmacKey(crypto.subtle, token, "sign"); - const mac = b64url(await crypto.subtle.sign("HMAC", key, utf8(canonical({ method: method.toUpperCase(), path, ts: t, nonce: n, bodyHash })))); - return { header: `Kotiko-HMAC v1 ts=${t}, nonce=${n}, body=${bodyHash}, mac=${mac}`, nonce: n }; + const mac = b64url(await crypto.subtle.sign("HMAC", key, utf8(canonical({ method: method.toUpperCase(), path, ts: t, nonce: n, boot, bodyHash })))); + return { header: `Kotiko-HMAC v2 ts=${t}, nonce=${n}, boot=${boot}, body=${bodyHash}, mac=${mac}`, nonce: n }; + } + + // Whether a proof answer's `boot` and `boot_mac` are the server's, for the proof's + // `nonce`: the boot id to sign requests with. subtle.verify compares in constant time. + async function verifyBoot(token, nonce, boot, bootMac, crypto = globalThis.crypto) { + if (!BOOT_ID.test(String(boot ?? "")) || !MAC.test(String(bootMac ?? ""))) return false; + const key = await hmacKey(crypto.subtle, token, "verify"); + return crypto.subtle.verify("HMAC", key, fromB64url(bootMac), utf8([BOOT, nonce, boot].join("\n"))); } // Whether `header` (X-Kotiko-Server) is the server's signature of an answer with `status` @@ -76,14 +94,15 @@ // What a signed request's 401 says, when its answer isn't signed: a server from before // signed requests challenges with `Bearer` alone; today's names `Kotiko-HMAC` and why - // (`stale`, `bad_mac`, ...). Neither is authenticated: it only picks the message. + // (`stale_boot`, `stale`, `bad_mac`, ...). Neither is authenticated: it only picks the + // message, or, for `stale_boot`, a new proof before one more try. function challenge(header) { const value = String(header ?? ""); if (!/(^|[\s,])Kotiko-HMAC(\s|,|$)/.test(value)) return { signs: false, reason: null }; return { signs: true, reason: /Kotiko-HMAC error="([a-z_]{1,32})"/.exec(value)?.[1] ?? null }; } - const api = { canonical, target, sign, verifyResponse, challenge, fromB64url }; + const api = { canonical, target, sign, verifyResponse, verifyBoot, challenge, fromB64url }; globalThis.KotikoServerAuth = api; if (typeof module === "object" && module.exports) module.exports = api; })(); diff --git a/scripts/check-security-gate.mjs b/scripts/check-security-gate.mjs index b58dd49..3ac9216 100755 --- a/scripts/check-security-gate.mjs +++ b/scripts/check-security-gate.mjs @@ -16,15 +16,25 @@ // // "One gate line" is judged as a reader sees the text, not byte by byte (security review // D-05: an open line in a Cyrillic letter, behind a zero-width space or with a fullwidth -// colon sat unseen beside the closed one). Each line is NFKC-normalised, stripped of -// invisible characters (format and default-ignorable code points) and of combining marks; -// then a line whose first four characters, after any leading markup, are g, a, t, e (any -// case) or a character outside ASCII, not followed by a letter or digit, reads as a gate -// line. Any non-ASCII character stands in for any letter, so lookalikes from every -// script count without a confusables table, and a colon-like character after the word -// counts too. That is stricter than a reader (a stray "Gâté:" line keeps the gate open, -// with a message saying which line), which is the safe side. The closed line itself must -// still be the exact ASCII form. +// colon sat unseen beside the closed one; E-06: in a list, a table, inline code, HTML, an +// entity or prose). Lines above the appendix are read several ways, and a line counts as a +// gate line if any reading of it, or of it joined with the next line (a paragraph's soft +// break), has the word "gate" followed, within a few non-letters, by a colon-like or +// table-cell separator. A reading: the line as written, and with HTML comments taken out +// (also across lines); then HTML tags removed, entities decoded (`a`, `:`), +// NFKC, invisible characters and combining marks removed. In the word, any non-ASCII +// character stands in for a letter, and any non-ASCII symbol for the colon, so lookalikes +// from every script count without a confusables table. Text inside HTML comments and code +// still counts: a stray `` keeps the gate open, which is the safe side. +// A paragraph (or line) that says "open" next to "gate", "review" or "status" keeps it +// open too ("Status: the review is still OPEN"). That is stricter than a reader, and the +// message says which line. +// +// The closed line itself must be the exact ASCII form, visible: not inside an HTML +// comment, a code fence or an HTML block, and in a paragraph without strikethrough, HTML, +// math, code or entity markup (`~ < $ \` &`). The "## Appendix" heading is the first one +// that is visible, in plain ASCII (security review E-06: one inside a comment or code +// fence, or followed by a zero-width character, doesn't count). // // This check passes when the tagged tree has a closed report. With --version X.Y.Z (the // release workflow passes the tag's), a review opened for that version must itself be @@ -43,11 +53,18 @@ import { fileURLToPath } from "node:url"; const ROOT = resolve(dirname(fileURLToPath(import.meta.url)), ".."); export const REPORT_RE = /^review-v(\d+\.\d+\.\d+)(?:-rc\.\d+)?\.md$/; export const CLOSED_RE = /^Gate: closed (\d{4}-\d{2}-\d{2}) by (.+), fixes confirmed on (v\d+\.\d+\.\d+-rc\.\d+)$/; -export const APPENDIX_RE = /^## Appendix\b/; -// Any line that reads as a gate line, however it is dressed up, so that only one may exist: -// after `visible`, leading markup or punctuation, then four characters that are each the -// letter of "gate" or not ASCII, and no letter or digit right after (D-05). -const ANY_GATE_RE = /^[\s\p{P}>+]*[g\P{ASCII}][a\P{ASCII}][t\P{ASCII}][e\P{ASCII}](?![\p{L}\p{N}])/iu; +export const APPENDIX_RE = /^## Appendix(?:[:\s].*)?$/; +const PLAIN = /^[\x20-\x7E]*$/; +// Each letter of `word`, or any non-ASCII character in its place. +const look = (word) => [...word].map((c) => `[${c}\\P{ASCII}]`).join(""); +const WORD = (w) => `(? @@ -57,27 +74,168 @@ const visible = (line) => .normalize("NFD") .replace(/\p{M}/gu, "") .normalize("NFC"); -const isGateLine = (line) => ANY_GATE_RE.test(visible(line)); + +const ENTITIES = { amp: "&", lt: "<", gt: ">", quot: '"', apos: "'", nbsp: " ", colon: ":", vert: "|", verbar: "|", VerticalLine: "|", num: "#", period: ".", comma: ",", semi: ";", lowbar: "_", ast: "*", grave: "`", tilde: "~", dollar: "$", equals: "=", hyphen: "-", dash: "‐", ndash: "–", mdash: "—", lpar: "(", rpar: ")" }; +// An entity the table doesn't know becomes a non-ASCII character, which stands in for a +// letter or the colon, as a lookalike does. +const decode = (text) => + text.replace(/&(?:#(\d{1,7})|#[xX]([0-9A-Fa-f]{1,6})|([A-Za-z][A-Za-z0-9]{0,31}));?/g, (_whole, dec, hex, name) => { + if (name) return ENTITIES[name] ?? "�"; + const code = dec ? Number(dec) : parseInt(hex, 16); + return code > 0 && code <= 0x10ffff ? String.fromCodePoint(code) : "�"; + }); +// Tags come out until none is left, so one hidden inside another ("<b>") goes too. +const stripTags = (line) => { + let prev; + let out = line; + do { + prev = out; + out = out.replace(/<\/?[A-Za-z][^>]*>/g, ""); + } while (out !== prev); + return out; +}; +const render = (line) => visible(decode(stripTags(line))); + +// The text with every HTML comment taken out (also over several lines; an unclosed one +// runs to the end), as lines, each with the number of the line it starts on. +function withoutComments(lines) { + const out = [{ text: "", n: 1 }]; + let n = 1; + const text = lines.join("\n"); + let i = 0; + while (i < text.length) { + const open = text.indexOf("", open + 4); + const end = close < 0 ? text.length : close + 3; + n += (text.slice(open, end).match(/\n/g) ?? []).length; + i = end; + } + return out; +} + +// Which lines a reader can't see, or can't take as plain text: inside a code fence (its +// fences too), an HTML comment, or an HTML block (a line starting with a tag, to the next +// blank line). Generous on purpose: it decides where the appendix starts and whether the +// closed line is visible, where hiding too much only keeps the gate open. +function hiddenLines(lines) { + const hidden = []; + let fence = null; + let comment = false; + let html = false; + for (const line of lines) { + let hide = false; + if (fence) { + hide = true; + const m = /^ {0,3}(`{3,}|~{3,})\s*$/.exec(line); + if (m && m[1][0] === fence[0] && m[1].length >= fence.length) fence = null; + } else if (comment) { + hide = true; + const close = line.indexOf("-->"); + if (close >= 0) comment = line.indexOf(""); + } else if (/^ {0,3}<[A-Za-z/!?]/.test(line)) { + hide = true; + html = true; + } + hidden.push(hide); + } + return hidden; +} + +// Every reading of the lines (see above): [{ text, n }], n the line it starts on. +function readings(lines) { + return [lines.map((text, i) => ({ text, n: i + 1 })), withoutComments(lines)].map((read) => read.map((l) => ({ ...l, text: render(l.text) }))); +} + +// The numbers of the lines that read as a gate line. +function gateLines(lines) { + const found = new Set(); + for (const read of readings(lines)) { + read.forEach((line, i) => { + const next = read[i + 1]?.text ?? ""; + for (const m of `${line.text} ${next}`.matchAll(GATE_RE)) if (m.index < line.text.length) found.add(line.n); + }); + } + return [...found].sort((a, b) => a - b); +} + +// The line that says "open" in a paragraph that also says "gate", "review" or "status", or 0. +// Each list item, table row and heading is a paragraph of its own. +function openLine(lines) { + for (const read of readings(lines)) { + let para = []; + const check = () => { + const text = para.map((l) => l.text).join(" "); + return OPEN_RE.test(text) && STATUS_RE.test(text) ? (para.find((l) => OPEN_RE.test(l.text)) ?? para[0]).n : 0; + }; + for (const line of [...read, { text: "", n: 0 }]) { + const own = /^\s*(?:[-*+>|#]|\d+[.)])/.test(line.text); + if ((line.text.trim() === "" || own) && para.length) { + const n = check(); + if (n) return n; + para = []; + } + if (line.text.trim() !== "") para.push(line); + if (own && para.length) { + const n = check(); + if (n) return n; + para = []; + } + } + } + return 0; +} + +// The lines of the paragraph line n (1-based) is in. +function paragraphOf(lines, n) { + let a = n - 1; + let b = n - 1; + while (a > 0 && lines[a - 1].trim() !== "") a--; + while (b < lines.length - 1 && lines[b + 1].trim() !== "") b++; + return lines.slice(a, b + 1); +} const realDate = (d) => !Number.isNaN(Date.parse(`${d}T00:00:00Z`)) && new Date(`${d}T00:00:00Z`).toISOString().startsWith(d); /** `{ gate }` (date, lead, candidate) when the report's gate is closed, else `{ problem }`. */ function readGate(text) { const lines = text.split(/\r?\n/); - const end = lines.findIndex((l) => APPENDIX_RE.test(l)); - if (end < 0) return { problem: 'no "## Appendix" heading' }; - const gates = lines.slice(0, end).flatMap((l, i) => (isGateLine(l) ? [i + 1] : [])); + const hidden = hiddenLines(lines); + const end = lines.findIndex((l, i) => !hidden[i] && PLAIN.test(l) && APPENDIX_RE.test(l)); + if (end < 0) return { problem: 'no visible "## Appendix" heading in plain ASCII' }; + const above = lines.slice(0, end); + const gates = gateLines(above); if (gates.length === 0) return { problem: 'no "Gate:" line above the appendix' }; if (gates.length > 1) { return { problem: `${gates.length} lines above the appendix read as a gate line (lines ${gates.join(", ")}); keep only the one` }; } const [n] = gates; const line = lines[n - 1]; - if (/[^\x20-\x7E]/.test(line)) { + if (!PLAIN.test(line)) { return { problem: `line ${n} reads as a gate line but has characters outside plain ASCII (a lookalike letter, an invisible or fullwidth character); write it in plain ASCII` }; } const m = line.match(CLOSED_RE); if (!m) return { problem: `line ${n} is not the exact closed line` }; + if (hidden[n - 1]) return { problem: `line ${n}, the closed line, is inside an HTML comment, a code block or an HTML block, where a reader doesn't see it as it is` }; + if (paragraphOf(lines, n).some((l) => MARKUP_RE.test(l))) { + return { problem: `line ${n}, the closed line, shares its paragraph with markup that could change how it reads (~ < $ \` &); put it on its own, with a blank line before and after` }; + } + const said = openLine(above); + if (said) return { problem: `line ${said} says "open" beside "gate", "review" or "status" above the appendix; once the gate is closed, only the closed line says how it stands` }; if (!realDate(m[1])) return { problem: `line ${n}: ${m[1]} is not a date` }; return { gate: { date: m[1], by: m[2], candidate: m[3] } }; } diff --git a/server/.env.example b/server/.env.example index 175e37c..35b9e1c 100644 --- a/server/.env.example +++ b/server/.env.example @@ -54,6 +54,12 @@ ALLOWED_HOSTS= # reverse proxy). Only used for the pairing string from mix kotiko.token. PUBLIC_URL= +# Behind a reverse proxy on this computer: the header it sets to each client's address +# (x-forwarded-for, x-real-ip, cf-connecting-ip or forwarded), so a stranger's wrong +# tokens lock out only that stranger. Empty: everyone through the proxy is counted +# together. Set it only if the proxy always sets (or overwrites) that header. +TRUSTED_PROXY_HEADER= + # Where the SQLite file lives (default $XDG_DATA_HOME/kotiko, or ~/.local/share/kotiko # without XDG_DATA_HOME; words already in ~/.local/share/kotiko stay there). The name # before the rename, SLOVO_DATA_DIR, still works for now, with a warning. legacy-name-ok diff --git a/server/config/runtime.exs b/server/config/runtime.exs index 9f53b79..a9c00f5 100644 --- a/server/config/runtime.exs +++ b/server/config/runtime.exs @@ -10,8 +10,8 @@ if config_env() != :test do # A secret's NAME_FILE (the path of a file that holds it) comes along with it; the # LLM_ and TRANSCRIBE_ ones come with their prefix. names = - ~w(PORT BIND API_TOKEN API_TOKEN_FILE ALLOWED_HOSTS PUBLIC_URL ALLOWED_TELEGRAM_IDS - TELEGRAM_BOT_TOKEN TELEGRAM_BOT_TOKEN_FILE LOG_LEVEL LOG_LOOKUPS) + ~w(PORT BIND API_TOKEN API_TOKEN_FILE ALLOWED_HOSTS PUBLIC_URL TRUSTED_PROXY_HEADER + ALLOWED_TELEGRAM_IDS TELEGRAM_BOT_TOKEN TELEGRAM_BOT_TOKEN_FILE LOG_LEVEL LOG_LOOKUPS) # SLOVO_ is the old prefix: its variables still work, with a warning. legacy-name-ok prefixes = ~w(KOTIKO_ SLOVO_ LLM_ TRANSCRIBE_) diff --git a/server/lib/kotiko/application.ex b/server/lib/kotiko/application.ex index e60e8bb..f3dd4cb 100644 --- a/server/lib/kotiko/application.ex +++ b/server/lib/kotiko/application.ex @@ -138,8 +138,9 @@ defmodule Kotiko.Application do defp model_line do url = Application.fetch_env!(:kotiko, :llm_url) models = Application.fetch_env!(:kotiko, :llm_models) - # Config refuses a user name or password in LLM_URL; never log one anyway (B-05). - provider = if Config.openrouter?(url), do: "OpenRouter", else: without_userinfo(url) + # Config refuses a user name or password in LLM_URL; never log one anyway (B-05), nor + # the query, which may hold a key (`?key=...`, E-03). + provider = if Config.openrouter?(url), do: "OpenRouter", else: Config.loggable_url(url) case Application.get_env(:kotiko, :llm_model_source, :env) do :default -> "#{provider}, its current free models (checked 10 s after start)" @@ -147,8 +148,6 @@ defmodule Kotiko.Application do end end - defp without_userinfo(url), do: URI.to_string(%{URI.parse(url) | userinfo: nil}) - defp telegram_line do cond do is_nil(Application.get_env(:kotiko, :telegram_token)) -> diff --git a/server/lib/kotiko/auth_throttle.ex b/server/lib/kotiko/auth_throttle.ex index 64db565..765b606 100644 --- a/server/lib/kotiko/auth_throttle.ex +++ b/server/lib/kotiko/auth_throttle.ex @@ -19,7 +19,11 @@ defmodule Kotiko.AuthThrottle do otherwise lock the extension out. Loopback requests with a forwarding header, as a reverse proxy on this computer sends every remote client, are counted together under one key: a stranger's lockout there locks out everyone behind the proxy for the rest - of the minute, but never the extension. Addresses are counted as `Kotiko.RateLimit.client/1` says (an IPv6 /64 is one). + of the minute, the extension on the owner's other devices included (never the + extension on this computer, which sends no such header). With `TRUSTED_PROXY_HEADER` + set, each of them is counted by the client address the proxy puts in that header + instead (security review E-02). Addresses are counted as `Kotiko.RateLimit.client/1` + says (an IPv6 /64 is one). Each lockout is logged once an hour per address, for at most 100 addresses an hour (`Kotiko.Log.Limiter`). `auth_failures_per_minute: nil` turns the throttle off (tests). @@ -78,6 +82,7 @@ defmodule Kotiko.AuthThrottle do # headers from it would count. defp key(:local), do: :local defp key({:proxied, _} = key), do: key + defp key({:forwarded, _} = key), do: key defp key({_, _, _, _, :"/64"} = key), do: key defp key(ip) when is_tuple(ip), @@ -113,6 +118,7 @@ defmodule Kotiko.AuthThrottle do end defp format({:proxied, peer}), do: "requests forwarded by " <> format(peer) + defp format({:forwarded, client}), do: format(client) <> " (through the reverse proxy)" defp format({a, b, c, d, :"/64"}), do: Enum.map_join([a, b, c, d], ":", &String.downcase(Integer.to_string(&1, 16))) <> "::/64" diff --git a/server/lib/kotiko/config.ex b/server/lib/kotiko/config.ex index 7f39567..0d1e9b4 100644 --- a/server/lib/kotiko/config.ex +++ b/server/lib/kotiko/config.ex @@ -37,8 +37,10 @@ defmodule Kotiko.Config do @max_secret_file_bytes 64 * 1024 # The other settings, without the KOTIKO_ prefix. config/runtime.exs copies them into # the app env by name or by their LLM_ and TRANSCRIBE_ prefixes. - @plain_vars ~w(PORT BIND ALLOWED_HOSTS PUBLIC_URL ALLOWED_TELEGRAM_IDS LLM_URL LLM_MODEL - TRANSCRIBE_URL TRANSCRIBE_MODEL LOG_LEVEL LOG_LOOKUPS) + @plain_vars ~w(PORT BIND ALLOWED_HOSTS PUBLIC_URL TRUSTED_PROXY_HEADER ALLOWED_TELEGRAM_IDS + LLM_URL LLM_MODEL TRANSCRIBE_URL TRANSCRIBE_MODEL LOG_LEVEL LOG_LOOKUPS) + # The headers TRUSTED_PROXY_HEADER may name (Kotiko.RateLimit.peer/1). + @proxy_headers ~w(x-forwarded-for x-real-ip cf-connecting-ip forwarded) @wrap_at 84 @typedoc "A label (`BIND=kotiko.local`) and the lines that explain it." @@ -151,6 +153,7 @@ defmodule Kotiko.Config do api_token: api_token(vars["API_TOKEN"]), allowed_hosts: allowed_hosts(vars["ALLOWED_HOSTS"]), public_url: optional_url("PUBLIC_URL", vars["PUBLIC_URL"]), + trusted_proxy_header: trusted_proxy_header(vars["TRUSTED_PROXY_HEADER"]), telegram_token: telegram_token(vars["TELEGRAM_BOT_TOKEN"]), allowed_ids: telegram_ids(vars["ALLOWED_TELEGRAM_IDS"]), llm_url: llm_url, @@ -185,6 +188,7 @@ defmodule Kotiko.Config do api_token: parsed.api_token, allowed_hosts: parsed.allowed_hosts, public_url: parsed.public_url, + trusted_proxy_header: parsed.trusted_proxy_header, telegram_token: parsed.telegram_token, allowed_ids: parsed.allowed_ids, llm_url: parsed.llm_url, @@ -443,6 +447,23 @@ defmodule Kotiko.Config do match?({:ok, _}, :inet.parse_strict_address(to_charlist(name))) end + # Security review E-02: which header a reverse proxy on this computer sets to the client + # it forwards, so the throttles count each client on its own. + defp trusted_proxy_header(nil), do: {:ok, nil} + + defp trusted_proxy_header(raw) do + header = String.downcase(raw) + + if header in @proxy_headers do + {:ok, header} + else + error("TRUSTED_PROXY_HEADER", raw, [ + "Use one of: #{Enum.join(@proxy_headers, ", ")}. Set it only when a reverse proxy " <> + "on this computer sets that header for every request; leave it empty otherwise." + ]) + end + end + defp optional_url(_name, nil), do: {:ok, nil} # A user name or password in the address (slice 54, B-05) would reach the log with the @@ -548,6 +569,41 @@ defmodule Kotiko.Config do end end + @doc """ + A service address (LLM_URL, TRANSCRIBE_URL) as it may be logged: without a user name or + password, query or fragment. A query may hold a key (`?key=...`; security review E-03), + so it shows as `?…`. Requests still go to the whole address. + """ + def loggable_url(url) when is_binary(url) do + uri = URI.parse(url) + shown = URI.to_string(%{uri | userinfo: nil, query: nil, fragment: nil}) + if is_nil(uri.query), do: shown, else: shown <> "?…" + end + + @doc """ + The values in the queries of LLM_URL and TRANSCRIBE_URL, as written and decoded, for the + log redactor (`Kotiko.Log.Redact`): a key a provider takes in the query never reaches + the log, wherever it turns up (security review E-03). + """ + def url_secrets(urls) do + for url <- urls, + is_binary(url), + query = URI.parse(url).query, + is_binary(query) and query != "", + part <- [query | String.split(query, ["&", ";"])], + value = part |> String.split("=", parts: 2) |> List.last(), + secret <- [value, decode_query(value)], + uniq: true, + do: secret + end + + defp decode_query(value) do + URI.decode_www_form(value) + rescue + # A malformed percent escape: the value as written is redacted. + ArgumentError -> value + end + @doc "Whether the model API is OpenRouter (it has the built-in free models)." def openrouter?(url) do # URI.parse/1 lowercases the scheme but not the host, which is case-insensitive too. diff --git a/server/lib/kotiko/llm/client.ex b/server/lib/kotiko/llm/client.ex index e726fff..8534da4 100644 --- a/server/lib/kotiko/llm/client.ex +++ b/server/lib/kotiko/llm/client.ex @@ -152,7 +152,12 @@ defmodule Kotiko.LLM.Client do if(key, do: [{"authorization", "Bearer " <> key}], else: []) end - defp url(path), do: Application.fetch_env!(:kotiko, :llm_url) <> path + # The path goes before LLM_URL's query, if it has one (`?api-version=...`): the query + # stays on every request. + defp url(path) do + uri = URI.parse(Application.fetch_env!(:kotiko, :llm_url)) + URI.to_string(%{uri | path: String.trim_trailing(uri.path || "", "/") <> path}) + end defp put_if(map, true, key, value), do: Map.put(map, key, value) defp put_if(map, _false, _key, _value), do: map diff --git a/server/lib/kotiko/log/redact.ex b/server/lib/kotiko/log/redact.ex index 2c0cb33..41b5833 100644 --- a/server/lib/kotiko/log/redact.ex +++ b/server/lib/kotiko/log/redact.ex @@ -11,7 +11,8 @@ defmodule Kotiko.Log.Redact do * the Telegram bot token in request URLs (`/bot123:ABC.../getUpdates`) becomes `bot[redacted]` * the configured values of API_TOKEN, LLM_API_KEY, TELEGRAM_BOT_TOKEN and - TRANSCRIBE_API_KEY become `[redacted]` + TRANSCRIBE_API_KEY become `[redacted]`, and so do the values in the queries of + LLM_URL and TRANSCRIBE_URL, where some providers take a key (`?key=...`, E-03) Events that contain none of these pass through untouched; one that does is rewritten to plain text. @@ -49,12 +50,16 @@ defmodule Kotiko.Log.Redact do :persistent_term.put(@secrets_key, secrets) end - @doc "The values of the secret settings in the app env." + @doc "The values of the secret settings in the app env, and of the service URLs' queries." def configured_secrets do Enum.map( [:api_token, :llm_api_key, :telegram_token, :transcribe_api_key], &Application.get_env(:kotiko, &1) - ) + ) ++ + Kotiko.Config.url_secrets([ + Application.get_env(:kotiko, :llm_url), + Application.get_env(:kotiko, :transcribe_url) + ]) end @doc "Redacts one string." diff --git a/server/lib/kotiko/rate_limit.ex b/server/lib/kotiko/rate_limit.ex index a301519..3841597 100644 --- a/server/lib/kotiko/rate_limit.ex +++ b/server/lib/kotiko/rate_limit.ex @@ -39,20 +39,32 @@ defmodule Kotiko.RateLimit do :ok end - # Headers a reverse proxy adds for the client it forwards (any value, any case). - @forwarding ~w(forwarded x-forwarded-for x-real-ip x-forwarded-host cf-connecting-ip true-client-ip) + # Headers a reverse proxy adds for the client it forwards (any value, any case), and any + # header whose name starts with one of the prefixes (security review E-02: a proxy that + # adds only a header outside the list made its clients look like this computer's own). + @forwarding ~w(forwarded forwarded-for x-forwarded x-forwarded-for x-forwarded-host + x-original-forwarded-for x-real-ip x-client-ip x-cluster-client-ip + cf-connecting-ip true-client-ip fastly-client-ip via) + @forwarding_prefixes ["x-forwarded-", "tailscale-user-"] @doc """ - Who a request counts as for the proof limit and the wrong-token lockout (slice 54, D-02): + Who a request counts as for the proof limit and the wrong-token lockout (slice 54, D-02; + security review E-02): * `:local`: a loopback peer with no forwarding header. Every program on this computer, and any web page in a browser here, shares it, so a limit there would let any of them lock the extension out: it is never limited. - * `{:proxied, peer}`: a loopback peer with a forwarding header, as a reverse proxy on - this computer (Caddy, nginx) sends every remote client. Limited, all of them - together: the header's claimed client address can be forged, so it isn't used. - A local program that adds such a header can only lock out this bucket, never the - extension, which sends none. + * `{:forwarded, client}`: a loopback peer with a forwarding header, when + `TRUSTED_PROXY_HEADER` names a header the reverse proxy on this computer sets, and + that header holds a client address. Each client is counted on its own, by that + address as `client/1` counts it, so a stranger's lockout doesn't keep the owner's + other devices out. + * `{:proxied, peer}`: a loopback peer with a forwarding header otherwise, as a reverse + proxy on this computer (Caddy, nginx) sends every remote client. Limited, all of + them together: a header's claimed client address can be forged, so it isn't used + unless the owner says which one the proxy sets. A local program that adds such a + header locks out only this bucket, never the extension on this computer, which + sends none; the extension on another device that comes through the proxy is in it. * otherwise the peer's address as `client/1` counts it. """ def peer(%Plug.Conn{remote_ip: ip, req_headers: headers}) do @@ -60,11 +72,72 @@ defmodule Kotiko.RateLimit do not loopback?(ip) -> client(ip) - Enum.any?(headers, fn {name, _} -> String.downcase(name) in @forwarding end) -> - {:proxied, client(ip)} + not Enum.any?(headers, fn {name, _} -> forwarding?(String.downcase(name)) end) -> + :local + + address = trusted_client(headers) -> + {:forwarded, client(address)} true -> - :local + {:proxied, client(ip)} + end + end + + defp forwarding?(name), + do: name in @forwarding or String.starts_with?(name, @forwarding_prefixes) + + # The client address in the header TRUSTED_PROXY_HEADER names, or nil: the rightmost + # X-Forwarded-For entry (the one the proxy added), the last Forwarded element's for=, or + # the one X-Real-IP or CF-Connecting-IP value. + defp trusted_client(headers) do + case Application.get_env(:kotiko, :trusted_proxy_header) do + nil -> nil + header -> header |> values(headers) |> client_address(header) |> address() + end + end + + defp values(name, headers), + do: for({key, value} <- headers, String.downcase(key) == name, do: value) + + defp client_address([], _header), do: nil + + defp client_address(values, "x-forwarded-for"), + do: values |> Enum.join(",") |> String.split(",") |> List.last() + + defp client_address(values, "forwarded") do + values + |> Enum.join(",") + |> String.split(",") + |> List.last() + |> String.split(";") + |> Enum.find_value(fn pair -> + case String.split(pair, "=", parts: 2) do + [key, value] -> if String.downcase(String.trim(key)) == "for", do: value + _ -> nil + end + end) + end + + defp client_address([value], _x_real_ip_or_cf), do: value + defp client_address(_several, _header), do: nil + + # An IP address, with or without a port (`203.0.113.9:4711`, `[2001:db8::1]:80`) or + # quotes; nil for anything else (`unknown`, an obfuscated `_name`). + defp address(nil), do: nil + + defp address(text) do + text = text |> String.trim() |> String.trim("\"") + + host = + case Regex.run(~r/\A\[([^\]]+)\](?::\d+)?\z|\A([0-9.]+):\d+\z/, text) do + [_, v6] -> v6 + [_, "", v4] -> v4 + nil -> text + end + + case :inet.parse_strict_address(String.to_charlist(host)) do + {:ok, ip} -> ip + {:error, _} -> nil end end diff --git a/server/lib/kotiko/request_auth.ex b/server/lib/kotiko/request_auth.ex index d89467b..03bfd7c 100644 --- a/server/lib/kotiko/request_auth.ex +++ b/server/lib/kotiko/request_auth.ex @@ -9,23 +9,25 @@ defmodule Kotiko.RequestAuth do A signed request carries one header: - Authorization: Kotiko-HMAC v1 ts=, nonce=, body=, mac= + Authorization: Kotiko-HMAC v2 ts=, nonce=, boot=, body=, mac= * `ts`: the client's time, in whole seconds since 1970 (1 to 12 digits). * `nonce`: 22 to 128 base64url characters (16 or more random bytes), new for every request. + * `boot`: this server's boot id (`boot/0`), 22 to 64 base64url characters, as the + proof (`POST /api/v1/proof`) gave it. * `body`: the SHA-256 of the request body's bytes, 64 lowercase hex digits (of the empty string when there is no body). * `mac`: base64url, without padding, of HMAC-SHA256 with the token's UTF-8 bytes as the key over the canonical request: - "kotiko-req-v1\\n" <> METHOD <> "\\n" <> path <> "\\n" <> ts <> "\\n" <> nonce <> "\\n" <> body + "kotiko-req-v2\\n" <> METHOD <> "\\n" <> path <> "\\n" <> ts <> "\\n" <> nonce <> "\\n" <> boot <> "\\n" <> body METHOD is the method in capitals; `path` is the request target as the server receives it, raw (not percent-decoded), with `?` and the query string when there is one; `ts`, - `nonce` and `body` are the header's values exactly as sent. + `nonce`, `boot` and `body` are the header's values exactly as sent. - It is accepted when the MAC matches (compared in constant time), `ts` is within - 120 seconds of the server's clock and not before the server started, the nonce wasn't + It is accepted when the MAC matches (compared in constant time), `boot` is this server's + boot id, `ts` is within 120 seconds of the server's clock either way, the nonce wasn't seen before, and the body's hash is the one signed. Every answer to an accepted request carries @@ -39,9 +41,15 @@ defmodule Kotiko.RequestAuth do nonce, which would let that request be replayed. Only a token holder can fill it, and only by sending that many requests within four minutes. - A server restarted between a request and its replay has forgotten the nonce; refusing a - `ts` from before the server started closes that gap: a request a squatter caught while - the server was stopped can't be played to the server once it is back. + **The boot id** (security review E-01). The nonces live in memory, so a restarted server + has forgotten them. Each start makes a new random boot id (128 bits), which the proof + answer carries, authenticated with the token (`boot_mac/3`), and every signed request + signs. A request signed before a restart carries the old id and is refused + (`:stale_boot`, 401 `error="stale_boot"`); the extension then asks for a new proof and + sends the request again, once. So a request a squatter caught while the server was + stopped can't be played to the server once it is back, however far ahead the client's + clock was. (Before, only a `ts` from before the start was refused, which let a request + stamped up to 120 s ahead through after a restart.) """ import Plug.Conn @@ -49,32 +57,45 @@ defmodule Kotiko.RequestAuth do @max_nonces 50_000 @purge_every 1_000 @table Kotiko.RequestAuth.Nonces - @started {__MODULE__, :started} - @request_label "kotiko-req-v1" + @boot {__MODULE__, :boot} + @request_label "kotiko-req-v2" @response_label "kotiko-resp-v1" + @boot_label "kotiko-boot-v1" @scheme "Kotiko-HMAC " - @params ~r/\Av1 ts=(\d{1,12}), nonce=([A-Za-z0-9_-]{22,128}), body=([0-9a-f]{64}), mac=([A-Za-z0-9_-]{43})\z/ + @params ~r/\Av2 ts=(\d{1,12}), nonce=([A-Za-z0-9_-]{22,128}), boot=([A-Za-z0-9_-]{22,64}), body=([0-9a-f]{64}), mac=([A-Za-z0-9_-]{43})\z/ @doc "The window, in seconds, either side of the server's clock." def window, do: @window - @doc "Creates the nonce table and notes when the server started (at boot)." + @doc "Creates the nonce table and this boot's id (at boot)." def init do if :ets.whereis(@table) == :undefined do :ets.new(@table, [:named_table, :public, :set, write_concurrency: true]) end - :persistent_term.put(@started, System.system_time(:second)) + :persistent_term.put(@boot, new_boot()) :ok end - @doc "Forgets every nonce (for tests); `started` sets the start time." - def reset(started \\ 0) do + @doc "Forgets every nonce (for tests); `boot` sets the boot id (a new one by default)." + def reset(boot \\ nil) do if :ets.whereis(@table) != :undefined, do: :ets.delete_all_objects(@table) - :persistent_term.put(@started, started) + :persistent_term.put(@boot, boot || new_boot()) :ok end + @doc "This boot's id: 16 random bytes made at start, as 22 base64url characters." + def boot, do: :persistent_term.get(@boot) + + defp new_boot, do: :crypto.strong_rand_bytes(16) |> Base.url_encode64(padding: false) + + @doc """ + The proof answer's `boot_mac`: base64url(HMAC-SHA256(token, "kotiko-boot-v1\\n" <> nonce <> + "\\n" <> boot)), for the proof's `nonce`, so the client knows the boot id is this + server's, for its own request. + """ + def boot_mac(token, nonce, boot), do: mac(token, Enum.join([@boot_label, nonce, boot], "\n")) + @doc "Whether an `Authorization` value uses this scheme." def signed?(value), do: String.starts_with?(value, @scheme) @@ -84,8 +105,9 @@ defmodule Kotiko.RequestAuth do """ def parse(@scheme <> params) do case Regex.run(@params, params) do - [_, ts, nonce, body, mac] -> - {:ok, %{ts: String.to_integer(ts), ts_text: ts, nonce: nonce, body: body, mac: mac}} + [_, ts, nonce, boot, body, mac] -> + {:ok, + %{ts: String.to_integer(ts), ts_text: ts, nonce: nonce, boot: boot, body: body, mac: mac}} nil -> :error @@ -95,8 +117,8 @@ defmodule Kotiko.RequestAuth do def parse(_), do: :error @doc "The canonical request the MAC is made over." - def canonical(method, path, ts_text, nonce, body_hash), - do: Enum.join([@request_label, method, path, ts_text, nonce, body_hash], "\n") + def canonical(method, path, ts_text, nonce, boot, body_hash), + do: Enum.join([@request_label, method, path, ts_text, nonce, boot, body_hash], "\n") @doc "The request target as the server got it: the raw path, then `?query` if any." def target(%Plug.Conn{request_path: path, query_string: ""}), do: path @@ -115,22 +137,24 @@ defmodule Kotiko.RequestAuth do @doc """ Checks a parsed header against `conn` and `token`. `:ok`, or `{:error, reason}`: - `:bad_mac`, `:stale` (outside the window, or from before the server started), - `:replayed`, or `:full` (the nonce table is full; the request was genuine). + `:bad_mac`, `:stale_boot` (signed for another boot of the server: before a restart), + `:stale` (more than 120 s before or after the server's clock), `:replayed`, or `:full` + (the nonce table is full; the request was genuine). """ def verify(conn, token, %{} = h) do - expected = mac(token, canonical(conn.method, target(conn), h.ts_text, h.nonce, h.body)) + expected = + mac(token, canonical(conn.method, target(conn), h.ts_text, h.nonce, h.boot, h.body)) + now = System.system_time(:second) cond do not Plug.Crypto.secure_compare(expected, h.mac) -> {:error, :bad_mac} - abs(now - h.ts) > @window or h.ts < started() -> {:error, :stale} + not Plug.Crypto.secure_compare(h.boot, boot()) -> {:error, :stale_boot} + h.ts > now + @window or h.ts < now - @window -> {:error, :stale} true -> remember(h.nonce, h.ts + @window + 1, now) end end - defp started, do: :persistent_term.get(@started, 0) - # insert_new is atomic: of two requests with one nonce, exactly one gets in. defp remember(nonce, expires, now) do cond do diff --git a/server/lib/kotiko/router.ex b/server/lib/kotiko/router.ex index 2de1095..bb2c676 100644 --- a/server/lib/kotiko/router.ex +++ b/server/lib/kotiko/router.ex @@ -91,8 +91,17 @@ defmodule Kotiko.Router do true -> token = Application.fetch_env!(:kotiko, :api_token) - proof = Token.proof(token, nonce) - conn |> put_resp_header("cache-control", "no-store") |> json(200, %{proof: proof}) + boot = RequestAuth.boot() + + # The boot id goes into every signed request, so one signed before a restart is + # refused after it (security review E-01); boot_mac ties it to this proof. + body = %{ + proof: Token.proof(token, nonce), + boot: boot, + boot_mac: RequestAuth.boot_mac(token, nonce, boot) + } + + conn |> put_resp_header("cache-control", "no-store") |> json(200, body) end end @@ -106,7 +115,8 @@ defmodule Kotiko.Router do # Seconds to wait, or nil. This computer's own requests (`:local`, no forwarding header) # aren't limited (slice 54, D-02): every local program shares 127.0.0.1, so any of them # could use up the extension's proofs. Strangers on the network, and those a reverse - # proxy here forwards (all together), keep the limit (Kotiko.RateLimit.peer/1). + # proxy here forwards (all together, or each by TRUSTED_PROXY_HEADER's address), keep the + # limit (Kotiko.RateLimit.peer/1). defp too_many_proofs(:local), do: nil defp too_many_proofs(peer) do @@ -280,13 +290,15 @@ defmodule Kotiko.Router do else: json(conn, status, %{error: message}) end - # Deny by default: only the exact GET or HEAD /health and POST /api/v1/proof are open. - # path_info is not yet percent-decoded here, but routing decodes it, so matching on - # ["api" | _] let "/%61pi/words" through without a token. - defp authorize(%{path_info: ["health"], method: m} = conn, _opts) when m in ~w(GET HEAD), + # Deny by default: only the exact GET or HEAD /health and POST /api/v1/proof are open, + # matched on the raw request path as sent. path_info is not yet percent-decoded here, but + # routing decodes it, so matching on ["api" | _] let "/%61pi/words" through without a + # token; and path_info drops empty segments, so matching on it let "//api/v1/proof", + # "/api/v1/proof/" and "/health/" through too (security review E-07). + defp authorize(%{request_path: "/health", method: m} = conn, _opts) when m in ~w(GET HEAD), do: conn - defp authorize(%{path_info: ["api", "v1", "proof"], method: "POST"} = conn, _opts), do: conn + defp authorize(%{request_path: "/api/v1/proof", method: "POST"} = conn, _opts), do: conn # An address that sent too many wrong tokens gets 429 whatever it sends now # (Kotiko.AuthThrottle). @@ -333,6 +345,11 @@ defmodule Kotiko.Router do |> RequestAuth.accept(token, header) |> rate_limited("too_many_requests", RequestAuth.retry_after()) + # Genuine (the MAC matched), but signed for this server's previous boot: not a + # guess, so not counted. The client proves again and resends (E-01). + {:error, :stale_boot} -> + deny(conn, "stale_boot") + {:error, reason} -> signature_failed(conn, reason) end @@ -355,7 +372,7 @@ defmodule Kotiko.Router do end # The challenge names both schemes; for a signed request, why it failed (`malformed`, - # `bad_mac`, `stale`, `replayed`, `body_mismatch`). The extension tells a server from + # `bad_mac`, `stale_boot`, `stale`, `replayed`, `body_mismatch`). The extension tells a server from # before signed requests by its plain `Bearer` challenge. defp deny(conn, reason \\ nil) do error = diff --git a/server/test/kotiko/config_test.exs b/server/test/kotiko/config_test.exs index cb84ae2..f0c9ce0 100644 --- a/server/test/kotiko/config_test.exs +++ b/server/test/kotiko/config_test.exs @@ -152,6 +152,21 @@ defmodule Kotiko.ConfigTest do assert {:ok, _, []} = parse(%{"LLM_API_KEY" => "sk-or-v1-abc"}) end + # Security review E-02: behind a reverse proxy, the throttles can count each client by + # the address the proxy puts in this header. + test "TRUSTED_PROXY_HEADER: unset by default; one of four headers, in any case" do + assert ok!(%{})[:trusted_proxy_header] == nil + + for {raw, value} <- [ + {"x-forwarded-for", "x-forwarded-for"}, + {"X-Real-IP", "x-real-ip"}, + {" CF-Connecting-IP ", "cf-connecting-ip"}, + {"Forwarded", "forwarded"} + ] do + assert ok!(%{"TRUSTED_PROXY_HEADER" => raw})[:trusted_proxy_header] == value, raw + end + end + test "LOG_LEVEL accepts warn as warning" do assert ok!(%{"LOG_LEVEL" => "warn"})[:log_level] == :warning assert ok!(%{"LOG_LEVEL" => "Warning"})[:log_level] == :warning @@ -209,7 +224,14 @@ defmodule Kotiko.ConfigTest do ["Use one of: debug, info, warning, error."]}, {%{"LOG_LOOKUPS" => "maybe"}, "LOG_LOOKUPS=maybe", ["Use true or false."]}, {%{"KOTIKO_LOG_SQL" => "2"}, "KOTIKO_LOG_SQL=2", ["Use true or false."]}, - {%{"KOTIKO_WIKTIONARY" => "sometimes"}, "KOTIKO_WIKTIONARY=sometimes", ["Use true or false."]} + {%{"KOTIKO_WIKTIONARY" => "sometimes"}, "KOTIKO_WIKTIONARY=sometimes", + ["Use true or false."]}, + {%{"TRUSTED_PROXY_HEADER" => "x-client-ip"}, "TRUSTED_PROXY_HEADER=x-client-ip", + [ + "Use one of: x-forwarded-for, x-real-ip, cf-connecting-ip, forwarded. Set it only " <> + "when a reverse proxy on this computer sets that header for every request; " <> + "leave it empty otherwise." + ]} ] for {{vars, label, lines}, i} <- Enum.with_index(@mistakes) do diff --git a/server/test/kotiko/rate_limit_test.exs b/server/test/kotiko/rate_limit_test.exs index 79aeee7..233d8cd 100644 --- a/server/test/kotiko/rate_limit_test.exs +++ b/server/test/kotiko/rate_limit_test.exs @@ -31,6 +31,93 @@ defmodule Kotiko.RateLimitTest do assert RateLimit.client({0x2001, 0xDB8, 0, 1, 2, 3, 4, 5}) == {0x2001, 0xDB8, 0, 1, :"/64"} end + defp conn(ip, headers) do + conn = Plug.Test.conn("GET", "/api/v1/words") + %{conn | remote_ip: ip, req_headers: conn.req_headers ++ headers} + end + + # Security review E-02: a proxy that adds a header outside the list made its clients look + # like this computer's own (never limited). + test "every header a reverse proxy may add marks a loopback request as forwarded" do + for name <- ~w(forwarded forwarded-for x-forwarded x-forwarded-for x-forwarded-host + x-forwarded-proto x-forwarded-port x-original-forwarded-for x-real-ip + x-client-ip x-cluster-client-ip cf-connecting-ip true-client-ip + fastly-client-ip via tailscale-user-login), + spelled <- [name, String.upcase(name)] do + assert RateLimit.peer(conn({127, 0, 0, 1}, [{spelled, "203.0.113.9"}])) == + {:proxied, {127, 0, 0, 1}}, + spelled + end + + assert RateLimit.peer(conn({127, 0, 0, 1}, [{"user-agent", "x"}])) == :local + assert RateLimit.peer(conn({203, 0, 113, 5}, [{"via", "1.1 x"}])) == {203, 0, 113, 5} + end + + describe "TRUSTED_PROXY_HEADER (E-02)" do + setup do + previous = Application.get_env(:kotiko, :trusted_proxy_header) + on_exit(fn -> Application.put_env(:kotiko, :trusted_proxy_header, previous) end) + end + + defp trusting(header, headers, ip \\ {127, 0, 0, 1}) do + Application.put_env(:kotiko, :trusted_proxy_header, header) + RateLimit.peer(conn(ip, headers)) + end + + test "x-forwarded-for: the rightmost address, the one the proxy added" do + assert trusting("x-forwarded-for", [{"x-forwarded-for", "198.51.100.1, 203.0.113.9"}]) == + {:forwarded, {203, 0, 113, 9}} + + # Two header lines read as one list. + assert trusting("x-forwarded-for", [ + {"x-forwarded-for", "198.51.100.1"}, + {"x-forwarded-for", "203.0.113.9:4711"} + ]) == {:forwarded, {203, 0, 113, 9}} + + # Normalised like a peer: an IPv6 /64, an IPv4-mapped address as IPv4. + assert trusting("x-forwarded-for", [{"x-forwarded-for", "2001:db8:0:1:2:3:4:5"}]) == + {:forwarded, {0x2001, 0xDB8, 0, 1, :"/64"}} + + assert trusting("x-forwarded-for", [{"x-forwarded-for", "[::ffff:192.0.2.1]:80"}]) == + {:forwarded, {192, 0, 2, 1}} + end + + test "forwarded: the last element's for=" do + header = ~s(for=198.51.100.1;proto=https, For="[2001:db8::17]:4711";by=_proxy) + + assert trusting("forwarded", [{"forwarded", header}]) == + {:forwarded, {0x2001, 0xDB8, 0, 0, :"/64"}} + + assert trusting("forwarded", [{"forwarded", "for=203.0.113.9"}]) == + {:forwarded, {203, 0, 113, 9}} + end + + test "x-real-ip and cf-connecting-ip: their one address" do + assert trusting("x-real-ip", [{"x-real-ip", " 203.0.113.9 "}]) == + {:forwarded, {203, 0, 113, 9}} + + assert trusting("cf-connecting-ip", [{"cf-connecting-ip", "2001:db8::1"}]) == + {:forwarded, {0x2001, 0xDB8, 0, 0, :"/64"}} + end + + test "without a usable address in that header, the proxy's shared count" do + shared = {:proxied, {127, 0, 0, 1}} + assert trusting("x-real-ip", [{"x-forwarded-for", "203.0.113.9"}]) == shared + assert trusting("x-real-ip", [{"x-real-ip", "unknown"}]) == shared + assert trusting("x-real-ip", [{"x-real-ip", "1.2.3.4"}, {"x-real-ip", "5.6.7.8"}]) == shared + assert trusting("forwarded", [{"forwarded", "for=_hidden"}]) == shared + assert trusting("x-forwarded-for", [{"x-forwarded-for", "203.0.113.9, "}]) == shared + assert trusting("x-forwarded-for", [{"x-forwarded-for", "999.1.1.1"}]) == shared + end + + test "only for a loopback peer, and only with a forwarding header" do + assert trusting("x-forwarded-for", []) == :local + + assert trusting("x-forwarded-for", [{"x-forwarded-for", "203.0.113.9"}], {198, 51, 100, 7}) == + {198, 51, 100, 7} + end + end + test "at most 10,000 counters: when full, expired ones go, else new clients aren't counted" do for i <- 1..9_999, do: RateLimit.hit(:fill, i, 60_000) RateLimit.hit(:fill, :expiring, 20) diff --git a/server/test/kotiko/router_auth_test.exs b/server/test/kotiko/router_auth_test.exs index be59871..24da08c 100644 --- a/server/test/kotiko/router_auth_test.exs +++ b/server/test/kotiko/router_auth_test.exs @@ -72,6 +72,25 @@ defmodule Kotiko.RouterAuthTest do end end + # Security review E-07: the open routes were matched on path_info, so `//api/v1/proof`, + # `/api/v1/proof/` and `GET /health/` were open too. Only the exact spelling is. + test "only the exact spelling of an open route is open (E-07)" do + nonce = %{nonce: "q1aP3n0ZKcB1x5mW0u7S9bJ2rV4yT6dE8gH0iL2nO4p"} + + for path <- ~w(//api/v1/proof /api/v1/proof/ /api/v1//proof /api//v1/proof //api/v1/proof/) do + conn = request("POST", path, [], body: nonce) + assert conn.status == 401, "POST #{path} answered #{conn.status} without a token" + assert get_resp_header(conn, "www-authenticate") == ["Bearer, Kotiko-HMAC"] + end + + for method <- ~w(GET HEAD), path <- ~w(/health/ //health /health// //health/) do + assert request(method, path).status == 401, "#{method} #{path} answered without a token" + end + + assert request("POST", "/api/v1/proof", [], body: nonce).status == 200 + assert request("GET", "/health").status == 200 + end + test "a CORS preflight gets no CORS headers" do conn = request("OPTIONS", "/api/words", [ diff --git a/server/test/kotiko/router_proof_test.exs b/server/test/kotiko/router_proof_test.exs index 2765bd3..d8d0bca 100644 --- a/server/test/kotiko/router_proof_test.exs +++ b/server/test/kotiko/router_proof_test.exs @@ -9,6 +9,11 @@ defmodule Kotiko.RouterProofTest do @nonce "n0nce_from-the-extension_0123456789abcdef" + setup do + Kotiko.RateLimit.reset() + on_exit(&Kotiko.RateLimit.reset/0) + end + defp expected(token, nonce) do :crypto.mac(:hmac, :sha256, token, "kotiko-proof-v1:" <> nonce) |> Base.url_encode64(padding: false) @@ -31,11 +36,28 @@ defmodule Kotiko.RouterProofTest do conn = prove(%{nonce: @nonce}) assert conn.status == 200 - assert json_body(conn) == %{"proof" => expected(token(), @nonce)} + assert %{"proof" => proof} = json_body(conn) + assert proof == expected(token(), @nonce) refute conn.resp_body =~ token() assert get_resp_header(conn, "cache-control") == ["no-store"] end + # Security review E-01: the boot id every signed request carries, which a restart changes, + # comes with the proof, signed with the token for this nonce. + test "carries this boot's id, with its MAC for the nonce" do + boot = Kotiko.RequestAuth.boot() + body = json_body(prove(%{nonce: @nonce})) + assert Map.keys(body) |> Enum.sort() == ~w(boot boot_mac proof) + assert body["boot"] == boot + + assert body["boot_mac"] == + :crypto.mac(:hmac, :sha256, token(), "kotiko-boot-v1\n#{@nonce}\n#{boot}") + |> Base.url_encode64(padding: false) + + Kotiko.RequestAuth.reset() + refute json_body(prove(%{nonce: @nonce}))["boot"] == boot + end + test "matches the example in docs/reference/http-api.md" do assert Kotiko.Token.proof( "example-token-0123456789abcdef", @@ -161,6 +183,21 @@ defmodule Kotiko.RouterProofTest do for _ <- 1..10, do: assert(prove(%{nonce: @nonce}, ip: {127, 0, 0, 1}).status == 200) end + test "with TRUSTED_PROXY_HEADER, proofs through the proxy are limited per client (E-02)" do + put_app_env(:proof_requests_per_minute, 3) + put_app_env(:trusted_proxy_header, "x-real-ip") + + via_proxy = fn client -> + build_conn(%{nonce: @nonce}, ip: {127, 0, 0, 1}) + |> Plug.Conn.put_req_header("x-real-ip", client) + |> Kotiko.Router.call(Kotiko.Router.init([])) + end + + for _ <- 1..3, do: assert(via_proxy.("203.0.113.9").status == 200) + assert via_proxy.("203.0.113.9").status == 429 + assert via_proxy.("198.51.100.7").status == 200 + end + test "an unknown Host is refused before the proof" do conn = request("POST", "/api/v1/proof", [], body: %{nonce: @nonce}, host: "evil.example") assert conn.status == 421 diff --git a/server/test/kotiko/router_signed_test.exs b/server/test/kotiko/router_signed_test.exs index a4ce6f8..e58de55 100644 --- a/server/test/kotiko/router_signed_test.exs +++ b/server/test/kotiko/router_signed_test.exs @@ -27,11 +27,14 @@ defmodule Kotiko.RouterSignedTest do nonce = Keyword.get(opts, :nonce, Base.url_encode64(:crypto.strong_rand_bytes(16), padding: false)) + boot = Keyword.get(opts, :boot, RequestAuth.boot()) hash = RequestAuth.body_hash(Keyword.get(opts, :signed_body, body)) - mac = RequestAuth.mac(token, RequestAuth.canonical(method, target, ts, nonce, hash)) + mac = RequestAuth.mac(token, RequestAuth.canonical(method, target, ts, nonce, boot, hash)) - {[{"authorization", "Kotiko-HMAC v1 ts=#{ts}, nonce=#{nonce}, body=#{hash}, mac=#{mac}"}], - nonce} + {[ + {"authorization", + "Kotiko-HMAC v2 ts=#{ts}, nonce=#{nonce}, boot=#{boot}, body=#{hash}, mac=#{mac}"} + ], nonce} end # Sends a signed request; returns the conn and its nonce. @@ -76,6 +79,8 @@ defmodule Kotiko.RouterSignedTest do # (test/unit/server-auth.test.mjs). test "matches the examples in docs/reference/http-api.md" do token = "example-token-0123456789abcdef" + boot = "AAECAwQFBgcICQoLDA0ODw" + assert boot == Base.url_encode64(:binary.list_to_bin(Enum.to_list(0..15)), padding: false) body = ~s({"base_langs":["es","en"],"ui_lang":null}) hash = RequestAuth.body_hash(body) assert hash == "40921055f301e073c3aa377b5ac7d8827ce72c5443e994014c2087b2895f51ff" @@ -86,13 +91,21 @@ defmodule Kotiko.RouterSignedTest do "/api/v1/profile", "1791331200", "q1aP3n0ZKcB1x5mW0u7S9b", + boot, hash ) assert canonical == - "kotiko-req-v1\nPUT\n/api/v1/profile\n1791331200\nq1aP3n0ZKcB1x5mW0u7S9b\n" <> hash + "kotiko-req-v2\nPUT\n/api/v1/profile\n1791331200\nq1aP3n0ZKcB1x5mW0u7S9b\n" <> + boot <> "\n" <> hash - assert RequestAuth.mac(token, canonical) == "osa_O2pBaygJz8c64WTOrTLSiY_6vV70NHtPyZ2CZpA" + assert RequestAuth.mac(token, canonical) == "KYWsYYqoHJ_1CJtht-ji67yNLQd-XNEXFaKUCZeTm1s" + + header = + "Kotiko-HMAC v2 ts=1791331200, nonce=q1aP3n0ZKcB1x5mW0u7S9b, boot=#{boot}, " <> + "body=#{hash}, mac=KYWsYYqoHJ_1CJtht-ji67yNLQd-XNEXFaKUCZeTm1s" + + assert {:ok, %{boot: ^boot, nonce: "q1aP3n0ZKcB1x5mW0u7S9b"}} = RequestAuth.parse(header) assert RequestAuth.response_header(token, "q1aP3n0ZKcB1x5mW0u7S9b", 200) == "v1 mac=CAmhh5A5suXJTIJNSarztt97Q-RO5eA9K0baiuiw1u0" @@ -105,18 +118,25 @@ defmodule Kotiko.RouterSignedTest do "/api/v1/words?status=active,paused", "1791331200", "dE8gH0iL2nO4pQ6rS8tU0v", + boot, empty ) assert empty == "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" - assert RequestAuth.mac(token, get) == "uHVRpP6SeDO3nxpRjs_zp8V9NYSCV2XP15w9BtwCdPg" + assert RequestAuth.mac(token, get) == "_SuhCp1q3PZ4gjDHP-zY0KQKrBe1zK5lxuFmBQuRPdk" + + # The proof answer's boot_mac, for the proof vector's nonce. + assert RequestAuth.boot_mac(token, "q1aP3n0ZKcB1x5mW0u7S9bJ2rV4yT6dE8gH0iL2nO4p", boot) == + "U8MycDq8d4O9G5n_qj1tMxNoTFoXHv5cex9_OKZifPY" doc = File.read!(Path.join(@root, "docs/reference/http-api.md")) for value <- [ - "osa_O2pBaygJz8c64WTOrTLSiY_6vV70NHtPyZ2CZpA", + header, + "KYWsYYqoHJ_1CJtht-ji67yNLQd-XNEXFaKUCZeTm1s", "CAmhh5A5suXJTIJNSarztt97Q-RO5eA9K0baiuiw1u0", - "uHVRpP6SeDO3nxpRjs_zp8V9NYSCV2XP15w9BtwCdPg", + "_SuhCp1q3PZ4gjDHP-zY0KQKrBe1zK5lxuFmBQuRPdk", + "U8MycDq8d4O9G5n_qj1tMxNoTFoXHv5cex9_OKZifPY", hash ], do: assert(doc =~ value, value) @@ -202,10 +222,10 @@ defmodule Kotiko.RouterSignedTest do end end - test "a stale or future ts, or one from before the server started, is refused" do + test "a ts more than 120 s before or after the server's clock is refused" do now = System.system_time(:second) - for ts <- [now - 121, now + 121, 0] do + for ts <- [now - 121, now + 121, now + 3600, 0] do {conn, _} = signed("GET", "/api/v1/words", nil, ts: ts) assert conn.status == 401, "ts #{ts}" @@ -217,12 +237,66 @@ defmodule Kotiko.RouterSignedTest do for ts <- [now - 119, now + 119] do assert {%{status: 200}, _} = signed("GET", "/api/v1/words", nil, ts: ts) end + end + + # Security review E-01: the nonces are in memory, so a restarted server had forgotten + # them, and refusing only a ts from before the start let a request stamped up to 119 s + # ahead (a client clock ahead of the server's) be played to it after a restart. Every + # request now signs the boot id the proof gave, and a restart makes a new one. + test "a request signed for another boot is refused (stale_boot), and not counted" do + put_app_env(:auth_failures_per_minute, 2) + word_fixture() + old_boot = RequestAuth.boot() + now = System.system_time(:second) + + # Caught while the server was stopped, stamped ahead of its clock as a client's may be. + caught = + for ahead <- [0, 30, 90, 119] do + {[{_, value}], _} = sign("GET", "/api/v1/words", "", ts: now + ahead) + value + end + + # The server restarts: a new boot id, no nonces remembered. + RequestAuth.reset() + refute RequestAuth.boot() == old_boot + + for value <- caught do + conn = + Plug.Test.conn("GET", "http://localhost/api/v1/words") + |> Map.put(:remote_ip, {203, 0, 113, 70}) + |> put_req_header("authorization", value) + |> Kotiko.Router.call(Kotiko.Router.init([])) + + assert conn.status == 401, value + assert get_resp_header(conn, "x-kotiko-server") == [] + + assert get_resp_header(conn, "www-authenticate") == [ + ~s(Bearer, Kotiko-HMAC error="stale_boot") + ] + + assert %{"code" => "server_key_rejected", "details" => %{"reason" => "stale_boot"}} = + json_body(conn)["error"] + + refute conn.resp_body =~ "да" + end + + # Genuine requests (the MAC matched), so no lockout, even from a remote address. + assert {%{status: 200}, _} = signed("GET", "/api/v1/words", nil, ip: {203, 0, 113, 70}) + + # A boot id the server never had is refused the same way; the current one is accepted. + assert {%{status: 401}, _} = signed("GET", "/api/v1/words", nil, boot: old_boot) + assert {%{status: 200}, _} = signed("GET", "/api/v1/words", nil, boot: RequestAuth.boot()) + end + + test "the boot id is new at every start: 16 random bytes, 22 base64url characters" do + boots = + for _ <- 1..20 do + RequestAuth.init() + RequestAuth.boot() + end - # A request made before this server started (one a squatter kept while the server was - # stopped) is refused even inside the window: the restarted server forgot its nonces. - RequestAuth.reset(now) - assert {%{status: 401}, _} = signed("GET", "/api/v1/words", nil, ts: now - 1) - assert {%{status: 200}, _} = signed("GET", "/api/v1/words", nil, ts: now) + assert Enum.uniq(boots) == boots + assert Enum.all?(boots, &(&1 =~ ~r/\A[A-Za-z0-9_-]{22}\z/)) end test "a nonce is accepted once" do @@ -274,7 +348,9 @@ defmodule Kotiko.RouterSignedTest do for bad <- [ "Kotiko-HMAC", - "Kotiko-HMAC v2 " <> String.replace_prefix(good, "Kotiko-HMAC v1 ", ""), + "Kotiko-HMAC v1 " <> String.replace_prefix(good, "Kotiko-HMAC v2 ", ""), + String.replace(good, ~r/boot=[^,]+, /, ""), + String.replace(good, ~r/boot=[^,]+/, "boot=short"), String.replace(good, ", ", ","), String.replace(good, "body=", "hash="), good <> ", extra=1", diff --git a/server/test/kotiko/router_throttle_test.exs b/server/test/kotiko/router_throttle_test.exs index cc65086..858b51e 100644 --- a/server/test/kotiko/router_throttle_test.exs +++ b/server/test/kotiko/router_throttle_test.exs @@ -100,6 +100,42 @@ defmodule Kotiko.RouterThrottleTest do assert log =~ "wrong API tokens from requests forwarded by 127.0.0.1" end + # Security review E-02: a proxy that adds a header outside the list made strangers look + # like this computer (never limited). + test "every forwarding header a proxy may add keeps the limit" do + for header <- ~w(via x-client-ip x-cluster-client-ip fastly-client-ip + x-original-forwarded-for x-forwarded forwarded-for) do + Kotiko.AuthThrottle.reset() + proxied = [{header, "203.0.113.9"}] + + capture_log(fn -> + for _ <- 1..10, do: assert(from({127, 0, 0, 1}, @wrong ++ proxied).status == 401) + end) + + assert from({127, 0, 0, 1}, auth() ++ proxied).status == 429, header + assert from({127, 0, 0, 1}, auth()).status == 200 + end + end + + # Security review E-02: with every proxied client in one count, a stranger's ten wrong + # tokens a minute kept the owner's other devices out. TRUSTED_PROXY_HEADER counts each by + # the address the proxy puts in that header. + test "with TRUSTED_PROXY_HEADER, each proxied client is counted on its own" do + put_app_env(:trusted_proxy_header, "x-forwarded-for") + stranger = [{"x-forwarded-for", "198.51.100.1, 203.0.113.9"}] + owner = [{"x-forwarded-for", "203.0.113.9, 198.51.100.7"}] + + log = + capture_log(fn -> + for _ <- 1..10, do: assert(from({127, 0, 0, 1}, @wrong ++ stranger).status == 401) + end) + + assert from({127, 0, 0, 1}, auth() ++ stranger).status == 429 + assert from({127, 0, 0, 1}, auth() ++ owner).status == 200 + assert from({127, 0, 0, 1}, auth()).status == 200 + assert log =~ "wrong API tokens from 203.0.113.9 (through the reverse proxy)" + end + test "requests without a token don't count; the lockout line is logged once" do ip = {203, 0, 113, 10} for _ <- 1..20, do: assert(from(ip, []).status == 401) diff --git a/server/test/kotiko/startup_summary_test.exs b/server/test/kotiko/startup_summary_test.exs index 77426b5..18bab63 100644 --- a/server/test/kotiko/startup_summary_test.exs +++ b/server/test/kotiko/startup_summary_test.exs @@ -6,8 +6,12 @@ defmodule Kotiko.StartupSummaryTest do # owner reads to see the server is set up as they meant. The boot itself runs in # boot_test.exs, in a separate VM. use ExUnit.Case, async: false + import ExUnit.CaptureLog import Kotiko.DataCase, only: [put_app_env: 2] + require Logger alias Kotiko.Application, as: App + alias Kotiko.LLM.Client + alias Kotiko.Log.Redact defp line(lines, prefix), do: Enum.find(lines, &String.starts_with?(&1, prefix)) @@ -86,4 +90,68 @@ defmodule Kotiko.StartupSummaryTest do model = line(App.summary_lines(:env, 0, ""), "Model:") assert model == "Model: http://127.0.0.1:42100/llm/v1, 1 model from LLM_MODEL" end + + # Security review E-03: a provider that takes its key in the query (`?key=...`) had it + # logged at info, in the model line. LLM_URL keeps its query (some providers need + # `?api-version=`), but the query is never logged. + describe "a key in LLM_URL's or TRANSCRIBE_URL's query (E-03)" do + @key "AIzaQuerySecret-0123456789abcdef" + @stt_key "stt-query-secret-0123456789" + + setup do + put_app_env( + :llm_url, + "https://llm.example/v1beta/openai?key=#{@key}&api-version=2026-01-01" + ) + + put_app_env( + :transcribe_url, + "https://stt.example/v1/audio/transcriptions?token=#{@stt_key}" + ) + + put_app_env(:llm_model_source, :env) + put_app_env(:llm_models, ["m"]) + on_exit(fn -> Redact.put_secrets(Redact.configured_secrets()) end) + end + + test "the startup summary shows the address without its query" do + lines = App.summary_lines(:env, 0, "") + + assert line(lines, "Model:") == + "Model: https://llm.example/v1beta/openai?…, 1 model from LLM_MODEL" + + refute Enum.join(lines, "\n") =~ @key + refute Enum.join(lines, "\n") =~ "api-version" + end + + test "the query's values are redacted from every log line, as written and decoded" do + put_app_env(:llm_url, "https://llm.example/v1?key=#{@key}&sig=a%2Fb%2Bc%3Dlong") + Redact.put_secrets(Redact.configured_secrets()) + + log = + capture_log(fn -> + Logger.info("Model: https://llm.example/v1?key=#{@key}") + Logger.error("failed: key #{@key}, voice #{@stt_key}, sig a/b+c=long a%2Fb%2Bc%3Dlong") + end) + + refute log =~ @key + refute log =~ @stt_key + refute log =~ "a/b+c=long" + refute log =~ "a%2Fb%2Bc%3Dlong" + assert log =~ "[redacted]" + end + + test "requests keep the query, after the path" do + test = self() + + Req.Test.stub(Kotiko.LLM, fn conn -> + send(test, {:asked, conn.request_path, conn.query_string}) + Req.Test.json(conn, %{data: []}) + end) + + assert {:ok, _} = Client.get("/models", 1_000) + assert_received {:asked, "/v1beta/openai/models", "key=" <> _ = query} + assert query =~ "api-version=2026-01-01" + end + end end diff --git a/test/bg/background.test.mjs b/test/bg/background.test.mjs index 4488f84..f3ae841 100644 --- a/test/bg/background.test.mjs +++ b/test/bg/background.test.mjs @@ -7,7 +7,7 @@ import { after, before, describe, test } from "node:test"; import assert from "node:assert/strict"; import { createFakeChrome } from "../helpers/fake-chrome.mjs"; import { proofFor, startFixtureServer } from "../helpers/fixture-server.mjs"; -import { serverSignature, verifySigned } from "../helpers/kotiko-auth.mjs"; +import { proofAnswer, serverSignature, TEST_BOOT, verifySigned } from "../helpers/kotiko-auth.mjs"; import { webcrypto } from "node:crypto"; import { manifest, readExt, runInVm, sleep } from "../helpers/load-script.mjs"; @@ -27,10 +27,11 @@ const targetOf = (url) => { }; const signedWith = (r, token) => verifySigned(token, { method: r.method ?? "GET", target: targetOf(r.url), body: r.body ?? "", authorization: r.headers?.Authorization }).ok && !JSON.stringify(r).includes(token); -// A server's answer to a request signed with `token`, signed back as the real server does. -// A request that isn't signed with it gets the server's 401. -async function signedAnswer(token, url, init, answer) { - const v = verifySigned(token, { method: init.method ?? "GET", target: targetOf(url), body: init.body ?? "", authorization: init.headers?.Authorization }); +// A server's answer to a request signed with `token` (for its boot id `boot`, when given), +// signed back as the real server does. A request that isn't signed with it gets the +// server's 401. +async function signedAnswer(token, url, init, answer, boot = null) { + const v = verifySigned(token, { method: init.method ?? "GET", target: targetOf(url), body: init.body ?? "", authorization: init.headers?.Authorization, boot }); if (!v.ok) return new Response(JSON.stringify({ error: { code: "server_key_rejected" } }), { status: 401, headers: { "www-authenticate": `Bearer, Kotiko-HMAC error="${v.reason}"` } }); const res = await answer(); res.headers.set("x-kotiko-server", serverSignature(token, v.nonce, res.status)); @@ -41,25 +42,28 @@ async function signedAnswer(token, url, init, answer) { // the background sends after a sync (slice 41 §9) is recorded apart, in `profile`, and // answered by `onProfile` (an echo by default), so the sync tests count only their own. // The proof a Kotiko server gives before it gets requests (slice 54, B-01) is answered here -// too, for `token` (the harness's, or a function for a test that changes it), and recorded in -// `proofs`; every answer is signed with that token, as the real server signs (D-01). -function stubFetch(handler, { onProfile = (init) => json(200, JSON.parse(init.body)), token = "good-token" } = {}) { +// too, for `token` (the harness's, or a function for a test that changes it), with the boot id +// `boot` (a function for a test that restarts the server: requests signed for another get +// `stale_boot`), and recorded in `proofs`; every answer is signed with that token, as the +// real server signs (D-01). +function stubFetch(handler, { onProfile = (init) => json(200, JSON.parse(init.body)), token = "good-token", boot = TEST_BOOT } = {}) { const requests = []; const profile = []; const proofs = []; const held = () => (typeof token === "function" ? token() : token); + const booted = () => (typeof boot === "function" ? boot() : boot); const fetch = async (url, init = {}) => { const r = { url: String(url), method: init.method ?? "GET", headers: { ...init.headers }, body: init.body, cache: init.cache, credentials: init.credentials }; if (r.url.endsWith("/api/v1/proof")) { proofs.push(r); - return json(200, { proof: proofFor(held(), JSON.parse(init.body).nonce) }); + return json(200, proofAnswer(held(), JSON.parse(init.body).nonce, booted())); } if (r.url.endsWith("/api/v1/profile")) { profile.push(r); - return signedAnswer(held(), url, init, () => onProfile(init)); + return signedAnswer(held(), url, init, () => onProfile(init), booted()); } requests.push(r); - return signedAnswer(held(), url, init, () => handler(String(url), init)); + return signedAnswer(held(), url, init, () => handler(String(url), init), booted()); }; return { fetch, requests, profile, proofs }; } @@ -726,7 +730,7 @@ describe("sync correctness (slice 26)", () => { const fetch = (url, init) => { // The profile sent after connecting (slice 41 §9) isn't the sync this test watches. if (String(url).endsWith("/api/v1/profile")) return Promise.resolve(json(200, {})); - if (String(url).endsWith("/api/v1/proof")) return Promise.resolve(json(200, { proof: proofFor(serverToken, JSON.parse(init.body).nonce) })); + if (String(url).endsWith("/api/v1/proof")) return Promise.resolve(json(200, proofAnswer(serverToken, JSON.parse(init.body).nonce))); // Which token signed it (slice 54, D-01: the token itself is never sent). const token = ["bad", "good"].find((t) => signedWith({ url: String(url), method: init.method, headers: init.headers, body: init.body }, t)); const entry = { token, aborted: false }; @@ -1119,7 +1123,7 @@ describe("the server proves it holds the token before it gets it", () => { if (String(url).endsWith("/api/v1/proof")) { const body = JSON.parse(init.body); proofs.push({ headers: { ...init.headers }, body, credentials: init.credentials }); - return json(200, { proof: proofFor(TOKEN, body.nonce) }); + return json(200, proofAnswer(TOKEN, body.nonce)); } if (String(url).endsWith("/api/v1/profile")) return signedAnswer(TOKEN, url, init, () => json(200, {})); requests.push({ url: String(url), method: init.method, headers: { ...init.headers } }); @@ -1142,8 +1146,18 @@ describe("the server proves it holds the token before it gets it", () => { ["a route it doesn't have (404)", () => new Response("not found", { status: 404 }), "not_kotiko_server", "no_proof"], ["a 401, as a server from before the proof answers", () => new Response("unauthorized", { status: 401 }), "not_kotiko_server", "no_proof"], ["a 200 with no proof", () => json(200, { words: [] }), "not_kotiko_server", "no_proof"], - ["a 200 with a proof made with another token", (init) => json(200, { proof: proofFor("another-token", JSON.parse(init.body).nonce) }), "server_key_rejected", "wrong_proof"], - ["a 200 with a proof for another nonce", () => json(200, { proof: proofFor("good-token", "q1aP3n0ZKcB1x5mW0u7S9bJ2rV4yT6dE8gH0iL2nO4p") }), "server_key_rejected", "wrong_proof"], + ["a 200 with a proof made with another token", (init) => json(200, proofAnswer("another-token", JSON.parse(init.body).nonce)), "server_key_rejected", "wrong_proof"], + ["a 200 with a proof for another nonce", () => json(200, proofAnswer("good-token", "q1aP3n0ZKcB1x5mW0u7S9bJ2rV4yT6dE8gH0iL2nO4p")), "server_key_rejected", "wrong_proof"], + // Security review E-01: a server from before boot ids can't check today's requests. + ["a 200 with a proof but no boot id (a server from before E-01)", (init) => json(200, { proof: proofFor("good-token", JSON.parse(init.body).nonce) }), "not_kotiko_server", "no_proof"], + ["a 200 with a boot id signed with another token", (init) => { + const { nonce } = JSON.parse(init.body); + return json(200, { ...proofAnswer("good-token", nonce), boot_mac: proofAnswer("another-token", nonce).boot_mac }); + }, "server_key_rejected", "wrong_proof"], + ["a 200 with a boot id signed for another nonce", (init) => { + const { nonce } = JSON.parse(init.body); + return json(200, { ...proofAnswer("good-token", nonce), boot_mac: proofAnswer("good-token", `${nonce}x`).boot_mac }); + }, "server_key_rejected", "wrong_proof"], ]; for (const [name, answer, code, reason] of impostors) { test(`a listener that answers ${name} never gets the token`, async () => { @@ -1271,6 +1285,50 @@ describe("signed requests: the token never leaves the browser (D-01)", () => { assert.ok(proofs.length >= 2, "connecting again proves again"); }); + // Security review E-01: a request caught while the server was stopped could be played to it + // after a restart. Every request now signs the server's boot id, which a restart changes; a + // request the restarted server refuses as signed for its previous boot is sent once more, + // after a new proof. + test("after the server restarts, Kotiko proves again and sends the refused request once more (E-01)", async () => { + const RESTARTED = "AAECAwQFBgcICQoLDA0OEA"; + let boot = TEST_BOOT; + const server = stubFetch(() => json(200, { words: WORDS }), { boot: () => boot }); + const { send, store, fake } = loadBackground({ fetch: server.fetch }); + assert.deepEqual(await send({ type: "sync", force: true }, POPUP), { ok: true }); + await requested(fake, server.profile, 1); + await fake.idle(); + assert.equal(server.proofs.length, 1); + const bootOf = (r) => /boot=([\w-]+)/.exec(r.headers.Authorization)[1]; + assert.ok(server.requests.every((r) => bootOf(r) === TEST_BOOT), "signed for the boot the proof gave"); + + boot = RESTARTED; + const sent = server.requests.length; + assert.deepEqual(await send({ type: "sync", force: true }, POPUP), { ok: true }); + assert.equal(store.syncError, null); + assert.deepEqual(store.words, WORDS); + assert.equal(server.proofs.length, 2, "proved again once the server said stale_boot"); + const tries = server.requests.slice(sent).filter((r) => new URL(r.url).pathname === "/api/words"); + assert.deepEqual(tries.map(bootOf), [TEST_BOOT, RESTARTED], "the refused request, then the same one for the new boot"); + }); + + test("a stale_boot refusal is retried only once", async () => { + const requests = []; + const proofs = []; + const fetch = async (url, init = {}) => { + if (String(url).endsWith("/api/v1/proof")) { + proofs.push(url); + return json(200, proofAnswer("good-token", JSON.parse(init.body).nonce)); + } + requests.push(url); + return new Response("{}", { status: 401, headers: { "www-authenticate": 'Bearer, Kotiko-HMAC error="stale_boot"' } }); + }; + const { send, store } = loadBackground({ fetch, local: { words: WORDS } }); + await send({ type: "sync", force: true }, POPUP); + assert.deepEqual([store.syncError?.code, store.syncError?.details?.reason], ["server_key_rejected", "stale_boot"]); + assert.deepEqual([proofs.length, requests.length], [2, 2]); + assert.deepEqual(store.words, WORDS); + }); + const unsigned = [ ["a server from before signed requests (401, Bearer challenge)", () => new Response("unauthorized", { status: 401, headers: { "www-authenticate": "Bearer" } }), "not_kotiko_server", "no_proof"], ["today's server refusing a stale signature", () => new Response("{}", { status: 401, headers: { "www-authenticate": 'Bearer, Kotiko-HMAC error="stale"' } }), "server_key_rejected", "stale"], @@ -1288,7 +1346,7 @@ describe("signed requests: the token never leaves the browser (D-01)", () => { const fetch = async (url, init = {}) => { if (String(url).endsWith("/api/v1/proof")) { proofs.push(url); - return json(200, { proof: proofFor("good-token", JSON.parse(init.body).nonce) }); + return json(200, proofAnswer("good-token", JSON.parse(init.body).nonce)); } return answer(url, init); }; diff --git a/test/e2e/fullstack.spec.mjs b/test/e2e/fullstack.spec.mjs index f709de6..54b5980 100644 --- a/test/e2e/fullstack.spec.mjs +++ b/test/e2e/fullstack.spec.mjs @@ -9,12 +9,14 @@ // (no HTTP listener, stubbed HTTP clients, environment ignored), so it can't serve a // browser. Set FULLSTACK_MIX_ENV to override. Skipped when `mix` isn't installed. import { spawn, spawnSync } from "node:child_process"; +import { randomBytes } from "node:crypto"; import fs from "node:fs/promises"; import net from "node:net"; import os from "node:os"; import path from "node:path"; import { fileURLToPath } from "node:url"; import { test, expect } from "./fixtures.mjs"; +import { requireExt } from "../helpers/load-script.mjs"; const SERVER_DIR = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../../server"); const TOKEN = "fullstack-token-0123456789abcdefghijklmnopq"; // 43 characters @@ -50,6 +52,55 @@ async function waitForHealth(url, child, log) { throw new Error(`The server didn't answer /health within ${BOOT_TIMEOUT_MS / 1000} s:\n${log.join("")}`); } +// Starts the real server on `port` with `dataDir`, and waits until it answers /health. +async function startServer(port, dataDir, fixtureUrl, log) { + const child = spawn("mix", ["run", "--no-halt"], { + cwd: SERVER_DIR, + detached: true, // own process group, so teardown stops the BEAM as well as mix + stdio: ["ignore", "pipe", "pipe"], + env: { + ...process.env, + MIX_ENV, + PORT: String(port), + BIND: "127.0.0.1", + API_TOKEN: TOKEN, + KOTIKO_DATA_DIR: dataDir, + LLM_URL: `${fixtureUrl}/llm/v1`, + LLM_MODEL: "fake/model-a:free,fake/model-b:free", + LLM_API_KEY: "fake-llm-key", + TELEGRAM_BOT_TOKEN: "", + ALLOWED_TELEGRAM_IDS: "", + TRANSCRIBE_URL: "", + // Self-contained: the real server never asks Wiktionary (slice 49 §4b) in tests. + KOTIKO_WIKTIONARY: "false", + }, + }); + child.stdout.on("data", (d) => log.push(String(d))); + child.stderr.on("data", (d) => log.push(String(d))); + await waitForHealth(`http://127.0.0.1:${port}`, child, log); + return child; +} + +// Stops a server startServer started (its whole process group), and waits until it has. +async function stopServer(child) { + if (!child || child.exitCode !== null) return; + const exited = new Promise((r) => child.once("exit", r)); + try { + process.kill(-child.pid, "SIGTERM"); + } catch { + // already gone + } + const timer = setTimeout(() => { + try { + process.kill(-child.pid, "SIGKILL"); + } catch { + // already gone + } + }, 5000); + await exited; + clearTimeout(timer); +} + test.describe("full stack", () => { test.skip(!hasMix, "`mix` isn't on PATH: install Elixir to run the full-stack smoke (CI runs it)."); test.describe.configure({ timeout: BOOT_TIMEOUT_MS + 60_000 }); @@ -66,50 +117,11 @@ test.describe("full stack", () => { const port = await freePort(); serverUrl = `http://127.0.0.1:${port}`; - child = spawn("mix", ["run", "--no-halt"], { - cwd: SERVER_DIR, - detached: true, // own process group, so teardown stops the BEAM as well as mix - stdio: ["ignore", "pipe", "pipe"], - env: { - ...process.env, - MIX_ENV, - PORT: String(port), - BIND: "127.0.0.1", - API_TOKEN: TOKEN, - KOTIKO_DATA_DIR: dataDir, - LLM_URL: `${fixtureUrl}/llm/v1`, - LLM_MODEL: "fake/model-a:free,fake/model-b:free", - LLM_API_KEY: "fake-llm-key", - TELEGRAM_BOT_TOKEN: "", - ALLOWED_TELEGRAM_IDS: "", - TRANSCRIBE_URL: "", - // Self-contained: the real server never asks Wiktionary (slice 49 §4b) in tests. - KOTIKO_WIKTIONARY: "false", - }, - }); - child.stdout.on("data", (d) => log.push(String(d))); - child.stderr.on("data", (d) => log.push(String(d))); - await waitForHealth(serverUrl, child, log); + child = await startServer(port, dataDir, fixtureUrl, log); }); test.afterAll(async () => { - if (child && child.exitCode === null) { - const exited = new Promise((r) => child.once("exit", r)); - try { - process.kill(-child.pid, "SIGTERM"); - } catch { - // already gone - } - const timer = setTimeout(() => { - try { - process.kill(-child.pid, "SIGKILL"); - } catch { - // already gone - } - }, 5000); - await exited; - clearTimeout(timer); - } + await stopServer(child); if (dataDir) await fs.rm(dataDir, { recursive: true, force: true }); }); @@ -228,3 +240,84 @@ test.describe("full stack", () => { await server.control({ llm: null }); }); }); + +// Security review E-01, reviewer E's e-replay-skew scenario on the real server. A program +// that took the port while the server was stopped catches the extension's signed requests; +// the server keeps its nonces only in memory, so after a restart it had forgotten them, and +// a request stamped ahead of the server's clock (a client clock up to 120 s fast) was +// answered with the learner's words. Every request now signs the boot id the server's proof +// gave, and a restart makes a new one, so those requests are refused (`stale_boot`). +test.describe("full stack: a request caught while the server was stopped", () => { + test.skip(!hasMix, "`mix` isn't on PATH: install Elixir to run the full-stack smoke (CI runs it)."); + test.describe.configure({ timeout: 2 * BOOT_TIMEOUT_MS + 60_000 }); + + const Auth = requireExt("lib/server-auth.js"); + const PATH = "/api/v1/words?status=active,paused"; + let child; + let dataDir; + const log = []; + + test.afterAll(async () => { + await stopServer(child); + if (dataDir) await fs.rm(dataDir, { recursive: true, force: true }); + }); + + test.afterEach(async ({}, testInfo) => { + if (testInfo.status !== testInfo.expectedStatus) { + await testInfo.attach("server log", { body: log.join(""), contentType: "text/plain" }); + } + }); + + const proof = async (url) => { + const nonce = randomBytes(32).toString("base64url"); + const res = await fetch(`${url}/api/v1/proof`, { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ nonce }) }); + expect(res.status).toBe(200); + const body = await res.json(); + expect(await Auth.verifyBoot(TOKEN, nonce, body.boot, body.boot_mac)).toBe(true); + return body.boot; + }; + const play = (url, header) => fetch(`${url}${PATH}`, { headers: { Authorization: header } }); + + test("is refused after the restart, however far ahead its clock was", async () => { + const fixtureUrl = process.env.FIXTURE_URL; + if (!fixtureUrl) throw new Error("FIXTURE_URL isn't set; run the tests through `npm run e2e`."); + dataDir = await fs.mkdtemp(path.join(os.tmpdir(), "kotiko-replay-")); + const port = await freePort(); + const url = `http://127.0.0.1:${port}`; + + child = await startServer(port, dataDir, fixtureUrl, log); + const before = await proof(url); + const added = await fetch(`${url}/api/v1/words`, { + method: "POST", + headers: { Authorization: `Bearer ${TOKEN}`, "Content-Type": "application/json" }, + body: JSON.stringify({ word: { lang: "ru", native: "кот", base_lang: "en", gloss: "cat" } }), + }); + expect(added.status).toBe(200); + await stopServer(child); + + // What the squatter receives while the server is stopped: the extension's requests, + // signed for the boot it last proved, stamped with a clock up to 119 s ahead. + const now = Math.floor(Date.now() / 1000); + const caught = []; + for (const ahead of [0, 30, 90, 119]) caught.push((await Auth.sign(TOKEN, { method: "GET", path: PATH, ts: now + ahead, boot: before })).header); + + child = await startServer(port, dataDir, fixtureUrl, log); + for (const header of caught) { + const res = await play(url, header); + const body = await res.text(); + expect(res.status, header).toBe(401); + expect(res.headers.get("www-authenticate")).toBe('Bearer, Kotiko-HMAC error="stale_boot"'); + expect(res.headers.get("x-kotiko-server")).toBeNull(); + expect(body).not.toContain("кот"); + } + + // The extension proves again and gets the new boot id; its requests are answered. + const after = await proof(url); + expect(after).not.toBe(before); + const { header, nonce } = await Auth.sign(TOKEN, { method: "GET", path: PATH, boot: after }); + const res = await play(url, header); + expect(res.status).toBe(200); + expect(await Auth.verifyResponse(TOKEN, nonce, 200, res.headers.get("x-kotiko-server"))).toBe(true); + expect((await res.json()).words.map((w) => w.native)).toEqual(["кот"]); + }); +}); diff --git a/test/helpers/fixture-server.mjs b/test/helpers/fixture-server.mjs index b4938b6..e234ea9 100644 --- a/test/helpers/fixture-server.mjs +++ b/test/helpers/fixture-server.mjs @@ -14,10 +14,12 @@ // jobs/pronunciation-refresh (GET, POST pause/resume), profile (GET, PUT; // slice 41 §9, kept as sent in `state.profile`) and llm/status // (slice 10: the quota is `llmRemaining` of 50; each add uses one), and -// POST /api/v1/proof (slice 54 B-01, no token: the HMAC of the nonce). -// Requests are signed (slice 54 D-01, helpers/kotiko-auth.mjs) and every -// answer to one is signed back; `Bearer ` works too, unsigned, as for -// curl. The log notes `signed: true` for a request whose signature held +// POST /api/v1/proof (slice 54 B-01, no token: the HMAC of the nonce, and +// the server's boot id with its MAC, E-01). +// Requests are signed (slice 54 D-01, helpers/kotiko-auth.mjs) with the +// boot id, and every answer to one is signed back; `Bearer ` works +// too, unsigned, as for curl. The log notes `signed: true` for a request +// whose signature held // /llm/v1/* a fake OpenAI-compatible model: /models, /key, /chat/completions // /__control POST to switch behaviours, GET to read state and the request log // @@ -35,11 +37,13 @@ // "job": {state, done, total, retry_at} the pronunciation-refresh job // "failNext": {method, path, status, code, details} one v1 request (or a legacy POST // /api/words, with a 0.2 string error) answers this error -// "token": "..." } the token /kotiko/api expects (signed or bearer) +// "token": "...", the token /kotiko/api expects (signed or bearer) +// "restart": true } a new boot id and no remembered nonces, as after +// a restart: requests signed before are stale_boot // // Run it by hand with `node test/helpers/fixture-server.mjs [port]`. -import { createHmac } from "node:crypto"; -import { serverSignature, verifySigned } from "./kotiko-auth.mjs"; +import { randomBytes } from "node:crypto"; +import { proofAnswer, proofFor, serverSignature, verifySigned } from "./kotiko-auth.mjs"; import fs from "node:fs"; import http from "node:http"; import path from "node:path"; @@ -51,7 +55,7 @@ const NPM = { "react.js": "react/umd/react.production.min.js", "react-dom.js": " const readJson = (rel) => JSON.parse(fs.readFileSync(path.join(FIXTURES, rel), "utf8")); // What a Kotiko server answers to `POST /api/v1/proof` (slice 54, B-01). -export const proofFor = (token, nonce) => createHmac("sha256", token).update(`kotiko-proof-v1:${nonce}`).digest("base64url"); +export { proofAnswer, proofFor }; // 43 characters, like a token from `openssl rand -base64 32`. export const TEST_TOKEN = "test-token-0123456789abcdefghijklmnopqrstuv"; @@ -358,18 +362,22 @@ export async function startFixtureServer({ port = 0, host = "127.0.0.1", token = return v1Error(res, 404, "not_found", "No such route."); } - // A signed request whose signature holds (its answer is then signed back), or the right - // `Bearer` token. Nonces are remembered for the server's life. - const seen = new Set(); + // A signed request whose signature holds, for this boot (its answer is then signed back), + // or the right `Bearer` token. Nonces are remembered for the boot's life. + let seen = new Set(); + let boot = randomBytes(16).toString("base64url"); async function authorized(req, res) { const auth = req.headers.authorization ?? ""; if (auth === `Bearer ${state.token}`) return true; if (!auth.startsWith("Kotiko-HMAC ")) return false; req.rawBody = await rawBody(req); - const v = verifySigned(state.token, { method: req.method, target: req.url.slice("/kotiko".length), body: req.rawBody, authorization: auth, seen }); + const v = verifySigned(state.token, { method: req.method, target: req.url.slice("/kotiko".length), body: req.rawBody, authorization: auth, seen, boot }); const entry = state.log.findLast((r) => r.req === req); if (entry) entry.signed = v.ok; - if (!v.ok) return false; + if (!v.ok) { + req.refusal = v.reason; + return false; + } const writeHead = res.writeHead.bind(res); res.writeHead = (status, ...rest) => { res.setHeader("x-kotiko-server", serverSignature(state.token, v.nonce, status)); @@ -391,7 +399,7 @@ export async function startFixtureServer({ port = 0, host = "127.0.0.1", token = const { nonce } = await readBody(req); if (typeof nonce !== "string" || !/^[A-Za-z0-9_-]{32,128}$/.test(nonce)) return send(res, 400, { error: { code: "invalid_request", message: "nonce", details: { field: "nonce" } } }); const held = state.kotiko === "401" ? `another-${state.token}` : state.token; - return send(res, 200, { proof: proofFor(held, nonce) }, { "cache-control": "no-store" }); + return send(res, 200, proofAnswer(held, nonce, boot), { "cache-control": "no-store" }); } // As today's server answers a request signed with another token (slice 54, D-01). if (state.kotiko === "401") return send(res, 401, "unauthorized", { "www-authenticate": 'Bearer, Kotiko-HMAC error="bad_mac"' }); @@ -401,7 +409,10 @@ export async function startFixtureServer({ port = 0, host = "127.0.0.1", token = "content-type": "text/html; charset=utf-8", }); } - if (!(await authorized(req, res))) return send(res, 401, "unauthorized", { "www-authenticate": "Bearer, Kotiko-HMAC" }); + if (!(await authorized(req, res))) { + const challenge = req.refusal === "stale_boot" ? 'Bearer, Kotiko-HMAC error="stale_boot"' : "Bearer, Kotiko-HMAC"; + return send(res, 401, "unauthorized", { "www-authenticate": challenge }); + } if (route.startsWith("/api/v1/")) return v1(req, res, route.slice("/api/v1".length)); if (route === "/api/words" && req.method === "GET") return send(res, 200, { words: state.words }); @@ -491,6 +502,10 @@ export async function startFixtureServer({ port = 0, host = "127.0.0.1", token = if (req.method === "GET") return send(res, 200, state); const body = await readBody(req); if (body.reset) reset(); + if (body.restart) { + boot = randomBytes(16).toString("base64url"); + seen = new Set(); + } for (const k of ["kotiko", "llm", "delayMs", "token", "job", "failNext", "llmRemaining", "llmDelayMs"]) if (k in body) state[k] = body[k]; if (Array.isArray(body.words)) { state.words = body.words; diff --git a/test/helpers/kotiko-auth.mjs b/test/helpers/kotiko-auth.mjs index 2258520..6c4c389 100644 --- a/test/helpers/kotiko-auth.mjs +++ b/test/helpers/kotiko-auth.mjs @@ -6,35 +6,46 @@ // are checked against each other and against the vectors in docs/reference/http-api.md. import { createHash, createHmac, timingSafeEqual } from "node:crypto"; -export const SIGNED = /^Kotiko-HMAC v1 ts=(\d{1,12}), nonce=([A-Za-z0-9_-]{22,128}), body=([0-9a-f]{64}), mac=([A-Za-z0-9_-]{43})$/; +export const SIGNED = /^Kotiko-HMAC v2 ts=(\d{1,12}), nonce=([A-Za-z0-9_-]{22,128}), boot=([A-Za-z0-9_-]{22,64}), body=([0-9a-f]{64}), mac=([A-Za-z0-9_-]{43})$/; + +// The boot id the fake servers give unless told otherwise (16 bytes 00..0f, as in the +// vectors in docs/reference/http-api.md). +export const TEST_BOOT = "AAECAwQFBgcICQoLDA0ODw"; export const bodyHash = (body = "") => createHash("sha256").update(body).digest("hex"); const mac = (token, text) => createHmac("sha256", token).update(text).digest("base64url"); -export const canonical = ({ method, target, ts, nonce, hash }) => ["kotiko-req-v1", method, target, ts, nonce, hash].join("\n"); +export const canonical = ({ method, target, ts, nonce, boot, hash }) => ["kotiko-req-v2", method, target, ts, nonce, boot, hash].join("\n"); + +// What a Kotiko server answers to `POST /api/v1/proof` (slice 54, B-01; E-01's boot id). +export const proofFor = (token, nonce) => mac(token, `kotiko-proof-v1:${nonce}`); +export const bootMac = (token, nonce, boot) => mac(token, ["kotiko-boot-v1", nonce, boot].join("\n")); +export const proofAnswer = (token, nonce, boot = TEST_BOOT) => ({ proof: proofFor(token, nonce), boot, boot_mac: bootMac(token, nonce, boot) }); // The X-Kotiko-Server value for an answer with `status` to the request `nonce`. export const serverSignature = (token, nonce, status) => `v1 mac=${mac(token, ["kotiko-resp-v1", nonce, String(status)].join("\n"))}`; // An Authorization value, as a client that holds `token` makes it. -export function signRequest(token, { method = "GET", target, body = "", ts = Math.floor(Date.now() / 1000), nonce }) { +export function signRequest(token, { method = "GET", target, body = "", ts = Math.floor(Date.now() / 1000), nonce, boot = TEST_BOOT }) { const hash = bodyHash(body); - return `Kotiko-HMAC v1 ts=${ts}, nonce=${nonce}, body=${hash}, mac=${mac(token, canonical({ method, target, ts: String(ts), nonce, hash }))}`; + return `Kotiko-HMAC v2 ts=${ts}, nonce=${nonce}, boot=${boot}, body=${hash}, mac=${mac(token, canonical({ method, target, ts: String(ts), nonce, boot, hash }))}`; } // Checks a request's Authorization against `token`: {ok, nonce} or {ok: false, reason}. // The time isn't checked (tests run the extension on a fake clock); `seen`, a Set, refuses -// a nonce used before. -export function verifySigned(token, { method, target, body = "", authorization, seen = null }) { +// a nonce used before; `boot`, when given, refuses a request signed for another boot id +// (`stale_boot`), as a restarted server does. +export function verifySigned(token, { method, target, body = "", authorization, seen = null, boot = null }) { const m = SIGNED.exec(String(authorization ?? "")); if (!m) return { ok: false, reason: "malformed" }; - const [, ts, nonce, hash, got] = m; - const want = mac(token, canonical({ method, target, ts, nonce, hash })); + const [, ts, nonce, signedBoot, hash, got] = m; + const want = mac(token, canonical({ method, target, ts, nonce, boot: signedBoot, hash })); if (!timingSafeEqual(Buffer.from(want), Buffer.from(got))) return { ok: false, reason: "bad_mac" }; + if (boot !== null && signedBoot !== boot) return { ok: false, reason: "stale_boot" }; if (hash !== bodyHash(body)) return { ok: false, reason: "body_mismatch" }; if (seen) { if (seen.has(nonce)) return { ok: false, reason: "replayed" }; seen.add(nonce); } - return { ok: true, nonce, ts: Number(ts) }; + return { ok: true, nonce, ts: Number(ts), boot: signedBoot }; } diff --git a/test/unit/security-gate.test.mjs b/test/unit/security-gate.test.mjs index 872f0cd..d7e85b7 100644 --- a/test/unit/security-gate.test.mjs +++ b/test/unit/security-gate.test.mjs @@ -129,3 +129,70 @@ test("the command line: --version, and the exit code", (t) => { assert.match(r.stdout, /Security review closed: review-v1\.0\.0-rc\.1\.md/); assert.equal(gate("--version", "v1.0.0").status, 2, "a version, not a tag"); }); + +// Security review E-06: reviewer E's disguises (test/security/poc/e-gate-variants.mjs at +// v1.0.0-rc.3). Each put an "open" line a reader sees beside the exact closed line, or hid +// the closed line or the appendix heading from the reader, and the gate read closed. +test("E-06: open lines in list, table, code, markup, entity or prose form keep the gate open", (t) => { + const W = ["G", "a", "t", "e"].join(""); + const C = `${W}: closed 2026-10-08 by Lead, fixes confirmed on v1.0.0-rc.3`; + const A = "## Appendix"; + const read = (body) => closedReviews(gateDir(t, { "review-v1.0.0-rc.3.md": body }), { version: "1.0.0" }); + assert.deepEqual(read(`# Review\n${C}\n\n${A}\n`), ["review-v1.0.0-rc.3.md"], "the exact closed line alone closes it"); + const open = { + "ordered list '1. : open'": `1. ${W}: open\n${C}\n${A}\n`, + "table row '| | open |'": `| ${W} | open |\n|---|---|\n${C}\n${A}\n`, + "inline code '`: open`'": `\`${W}: open\`\n${C}\n${A}\n`, + "strikethrough-tilde '~: open~'": `~${W}: open~\n${C}\n${A}\n`, + "emoji prefix '🚧 : open'": `🚧 ${W}: open\n${C}\n${A}\n`, + "arrow prefix '→ : open'": `→ ${W}: open\n${C}\n${A}\n`, + "HTML tag ': open'": `${W}: open\n${C}\n${A}\n`, + "entity 'Gate: open'": `Gate: open\n${C}\n${A}\n`, + "dollar math '$$: open'": `$${W}$: open\n${C}\n${A}\n`, + "fake appendix inside an HTML comment, open below it": `${C}\n\n${W}: open\n${A}\n`, + "fake appendix inside a code fence, open below it": `${C}\n\`\`\`\n${A}\n\`\`\`\n${W}: open\n${A}\n`, + "'## Appendix' + zero-width as the only heading": `${C}\n${A}\u200B\n${W}: open\n`, + "closed line inside an HTML comment only": `\n${A}\n`, + "closed line, then 'Status: still OPEN' prose": `${C}\nStatus: the review is still OPEN; two findings unfixed.\n${A}\n`, + "open as setext heading text": `${W}\n: open\n${C}\n${A}\n`, + // More of the same kind. + "a table with the gate as a column": `| ${W} | Date |\n|---|---|\n| open | today |\n\n${C}\n\n${A}\n`, + "a named entity for the colon": `${W}: open\n\n${C}\n\n${A}\n`, + "a word split by an inline comment": `Gate: open\n\n${C}\n\n${A}\n`, + "a word split by a comment over two lines": `Gate: open\n\n${C}\n\n${A}\n`, + "the closed line inside a code fence": `\`\`\`\n${C}\n\`\`\`\n\n${A}\n`, + "the closed line inside an HTML block": `\n\n${A}\n`, + "the closed line struck through by the lines around it": `~~\n${C}\n~~\n\n${A}\n`, + "'Gate open' prose beside the review": `${C}\n\nThe review stays open until E-01 is fixed.\n\n${A}\n`, + "a blockquote": `> ${W}: open\n\n${C}\n\n${A}\n`, + "a heading": `### ${W}: open\n\n${C}\n\n${A}\n`, + }; + for (const [label, body] of Object.entries(open)) assert.deepEqual(read(body), [], label); +}); + +test("E-06: ordinary prose about the gate, and the real report's shape, don't count", () => { + const text = [ + "# Security review: v1.0.0-rc.3", + "", + "The gate closes when a fifth reviewer confirms every fix; the store gate reads lines above the appendix.", + "", + "| ID | Severity | Status | Fix |", + "|---|---|---|---|", + "| E-07 | Info | Open routes answer other spellings | fixed in #40 |", + "| E-06 | Info | The store gate still reads disguised lines as closed | fixed in #40 |", + "", + "", + "", + "```", + "an example: ## Appendix", + "```", + "", + CLOSED, + "", + "## Appendix: the reviewers' reports, unchanged", + "", + "Gate: open, says reviewer A's report, quoted.", + ].join("\n"); + assert.equal(gateProblem(text), null); + assert.deepEqual(gateLine(text), { date: "2026-11-02", by: "Ayla Croft", candidate: "v1.0.0-rc.2" }); +}); diff --git a/test/unit/server-auth.test.mjs b/test/unit/server-auth.test.mjs index 0748694..fde13f6 100644 --- a/test/unit/server-auth.test.mjs +++ b/test/unit/server-auth.test.mjs @@ -1,43 +1,80 @@ // SPDX-FileCopyrightText: 2026 ScriptKittyOS and the Kotiko contributors // SPDX-License-Identifier: Apache-2.0 -// Signed requests (slice 54, D-01): the extension's lib/server-auth.js against the vectors -// in docs/reference/http-api.md (the server checks the same ones in router_signed_test.exs) -// and against the tests' own node:crypto server (helpers/kotiko-auth.mjs). +// Signed requests (slice 54, D-01; the boot id, security review E-01): the extension's +// lib/server-auth.js against the vectors in docs/reference/http-api.md (the server checks +// the same ones in router_signed_test.exs) and against the tests' own node:crypto server +// (helpers/kotiko-auth.mjs). import { describe, test } from "node:test"; import assert from "node:assert/strict"; import fs from "node:fs"; import path from "node:path"; import { requireExt, ROOT } from "../helpers/load-script.mjs"; -import { serverSignature, signRequest, verifySigned } from "../helpers/kotiko-auth.mjs"; +import { bootMac, proofFor, serverSignature, signRequest, verifySigned } from "../helpers/kotiko-auth.mjs"; const Auth = requireExt("lib/server-auth.js"); const TOKEN = "example-token-0123456789abcdef"; const DOC = fs.readFileSync(path.join(ROOT, "docs/reference/http-api.md"), "utf8"); const BODY = '{"base_langs":["es","en"],"ui_lang":null}'; +const BOOT = "AAECAwQFBgcICQoLDA0ODw"; const PUT = { - header: "Kotiko-HMAC v1 ts=1791331200, nonce=q1aP3n0ZKcB1x5mW0u7S9b, body=40921055f301e073c3aa377b5ac7d8827ce72c5443e994014c2087b2895f51ff, mac=osa_O2pBaygJz8c64WTOrTLSiY_6vV70NHtPyZ2CZpA", + header: "Kotiko-HMAC v2 ts=1791331200, nonce=q1aP3n0ZKcB1x5mW0u7S9b, boot=AAECAwQFBgcICQoLDA0ODw, body=40921055f301e073c3aa377b5ac7d8827ce72c5443e994014c2087b2895f51ff, mac=KYWsYYqoHJ_1CJtht-ji67yNLQd-XNEXFaKUCZeTm1s", response: "v1 mac=CAmhh5A5suXJTIJNSarztt97Q-RO5eA9K0baiuiw1u0", }; -const GET = "Kotiko-HMAC v1 ts=1791331200, nonce=dE8gH0iL2nO4pQ6rS8tU0v, body=e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855, mac=uHVRpP6SeDO3nxpRjs_zp8V9NYSCV2XP15w9BtwCdPg"; +const GET = "Kotiko-HMAC v2 ts=1791331200, nonce=dE8gH0iL2nO4pQ6rS8tU0v, boot=AAECAwQFBgcICQoLDA0ODw, body=e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855, mac=_SuhCp1q3PZ4gjDHP-zY0KQKrBe1zK5lxuFmBQuRPdk"; +const PROOF_NONCE = "q1aP3n0ZKcB1x5mW0u7S9bJ2rV4yT6dE8gH0iL2nO4p"; +const PROOF = { proof: "E5CHaF60VqTTPPduGD1yAlfWMF5xf9_hmV2qCufV-ek", boot: BOOT, boot_mac: "U8MycDq8d4O9G5n_qj1tMxNoTFoXHv5cex9_OKZifPY" }; describe("signed requests: lib/server-auth.js", () => { test("the documented vectors, which the server checks too", async () => { - const put = await Auth.sign(TOKEN, { method: "put", path: "/api/v1/profile", body: BODY, ts: 1791331200, nonce: "q1aP3n0ZKcB1x5mW0u7S9b" }); + const put = await Auth.sign(TOKEN, { method: "put", path: "/api/v1/profile", body: BODY, ts: 1791331200, nonce: "q1aP3n0ZKcB1x5mW0u7S9b", boot: BOOT }); assert.deepEqual(put, { header: PUT.header, nonce: "q1aP3n0ZKcB1x5mW0u7S9b" }); - const get = await Auth.sign(TOKEN, { method: "GET", path: "/api/v1/words?status=active,paused", ts: 1791331200, nonce: "dE8gH0iL2nO4pQ6rS8tU0v" }); + const get = await Auth.sign(TOKEN, { method: "GET", path: "/api/v1/words?status=active,paused", ts: 1791331200, nonce: "dE8gH0iL2nO4pQ6rS8tU0v", boot: BOOT }); assert.equal(get.header, GET); - for (const value of [PUT.header, PUT.response, GET]) assert.ok(DOC.includes(value), value); + for (const value of [PUT.header, PUT.response, GET, PROOF.proof, PROOF.boot_mac, BOOT]) assert.ok(DOC.includes(value), value); assert.equal(await Auth.verifyResponse(TOKEN, "q1aP3n0ZKcB1x5mW0u7S9b", 200, PUT.response), true); assert.equal(serverSignature(TOKEN, "q1aP3n0ZKcB1x5mW0u7S9b", 200), PUT.response); - assert.equal(signRequest(TOKEN, { method: "PUT", target: "/api/v1/profile", body: BODY, ts: 1791331200, nonce: "q1aP3n0ZKcB1x5mW0u7S9b" }), PUT.header); + assert.equal(signRequest(TOKEN, { method: "PUT", target: "/api/v1/profile", body: BODY, ts: 1791331200, nonce: "q1aP3n0ZKcB1x5mW0u7S9b", boot: BOOT }), PUT.header); + // The proof answer's boot id, signed for the proof's nonce. + assert.equal(proofFor(TOKEN, PROOF_NONCE), PROOF.proof); + assert.equal(bootMac(TOKEN, PROOF_NONCE, BOOT), PROOF.boot_mac); + assert.equal(await Auth.verifyBoot(TOKEN, PROOF_NONCE, BOOT, PROOF.boot_mac), true); + }); + + // Security review E-01: a request caught while the server was stopped could be played to + // it after a restart, when its ts was ahead of the server's clock. Every request now signs + // the server's boot id, which a restart changes. + test("the boot id is signed: a request for another boot doesn't verify as this one", async () => { + await assert.rejects(Auth.sign(TOKEN, { method: "GET", path: "/" }), /boot id/, "no request without a boot id"); + await assert.rejects(Auth.sign(TOKEN, { method: "GET", path: "/", boot: "short" }), /boot id/); + const { header } = await Auth.sign(TOKEN, { method: "GET", path: "/api/v1/words", boot: BOOT }); + const req = { method: "GET", target: "/api/v1/words", authorization: header }; + assert.equal(verifySigned(TOKEN, { ...req, boot: BOOT }).ok, true); + assert.deepEqual(verifySigned(TOKEN, { ...req, boot: "AAECAwQFBgcICQoLDA0OEA" }), { ok: false, reason: "stale_boot" }); + // Changing the boot id in the header breaks the MAC. + assert.equal(verifySigned(TOKEN, { ...req, authorization: header.replace(BOOT, "AAECAwQFBgcICQoLDA0OEA") }).reason, "bad_mac"); + }); + + test("a proof answer's boot id must be the server's, for this nonce", async () => { + const mac = bootMac(TOKEN, PROOF_NONCE, BOOT); + for (const [label, args] of [ + ["another token", ["another-token-0123456789", PROOF_NONCE, BOOT, mac]], + ["another nonce", [TOKEN, `${PROOF_NONCE}x`, BOOT, mac]], + ["another boot", [TOKEN, PROOF_NONCE, "AAECAwQFBgcICQoLDA0OEA", mac]], + ["no boot", [TOKEN, PROOF_NONCE, undefined, mac]], + ["a boot that isn't base64url", [TOKEN, PROOF_NONCE, "AAECAwQFBgcICQoLDA0O+w", mac]], + ["no mac", [TOKEN, PROOF_NONCE, BOOT, undefined]], + ["a mac that isn't 43 characters", [TOKEN, PROOF_NONCE, BOOT, `${mac}A`]], + ]) { + assert.equal(await Auth.verifyBoot(...args), false, label); + } }); test("a request it signs verifies on the server's side; a changed part doesn't", async () => { - const { header, nonce } = await Auth.sign(TOKEN, { method: "POST", path: "/api/v1/words", body: '{"text":"кот"}' }); + const { header, nonce } = await Auth.sign(TOKEN, { method: "POST", path: "/api/v1/words", body: '{"text":"кот"}', boot: BOOT }); assert.match(nonce, /^[A-Za-z0-9_-]{22}$/, "16 random bytes"); const req = { method: "POST", target: "/api/v1/words", body: '{"text":"кот"}', authorization: header }; - assert.deepEqual(verifySigned(TOKEN, req), { ok: true, nonce, ts: Number(/ts=(\d+)/.exec(header)[1]) }); + assert.deepEqual(verifySigned(TOKEN, req), { ok: true, nonce, ts: Number(/ts=(\d+)/.exec(header)[1]), boot: BOOT }); assert.equal(Math.abs(Number(/ts=(\d+)/.exec(header)[1]) - Date.now() / 1000) < 5, true); for (const [part, change] of [ ["method", { method: "PUT" }], @@ -49,7 +86,7 @@ describe("signed requests: lib/server-auth.js", () => { assert.equal(out.ok, false, part); } // A new nonce every time. - assert.notEqual((await Auth.sign(TOKEN, { method: "GET", path: "/" })).nonce, (await Auth.sign(TOKEN, { method: "GET", path: "/" })).nonce); + assert.notEqual((await Auth.sign(TOKEN, { method: "GET", path: "/", boot: BOOT })).nonce, (await Auth.sign(TOKEN, { method: "GET", path: "/", boot: BOOT })).nonce); }); test("an answer's signature must be the server's, for this nonce and status", async () => { @@ -79,6 +116,7 @@ describe("signed requests: lib/server-auth.js", () => { assert.deepEqual(Auth.challenge(null), { signs: false, reason: null }); assert.deepEqual(Auth.challenge("Bearer, Kotiko-HMAC"), { signs: true, reason: null }); assert.deepEqual(Auth.challenge('Bearer, Kotiko-HMAC error="stale"'), { signs: true, reason: "stale" }); + assert.deepEqual(Auth.challenge('Bearer, Kotiko-HMAC error="stale_boot"'), { signs: true, reason: "stale_boot" }); assert.deepEqual(Auth.challenge("Bearer, Kotiko-HMACX"), { signs: false, reason: null }); }); });