The report relay has brakes, and they count nobody's address

PRIVACY.md

@@ -177,15 +177,24 @@ still worth checking before you tick the box, and is why the preview is the size
 ### Where it goes
 
 To `ppc-reports.jirpos.workers.dev`, a Cloudflare Worker operated by the maintainer, which forwards
-it to a private channel the maintainer reads and does nothing else with it. The Worker keeps no
-database, writes no log of requests, and stores nothing: the report is relayed and the request is
-over. Its source is in [`worker/`](worker/) in this repository, so what it does is readable rather
-than promised.
+it to a private channel the maintainer reads and does nothing else with it. The Worker writes no log
+of requests and keeps no part of a report: once it is relayed the request is over. The only thing it
+stores is two counters, described below. Its source is in [`worker/`](worker/) in this repository, so
+what it does is readable rather than promised.
 
 Cloudflare sits in front of it and sees your IP address, as any host you make a request to does;
 their [privacy policy](https://www.cloudflare.com/privacypolicy/) applies. The Worker uses that
 address for one thing - an hourly cap, so the endpoint cannot be flooded - and it is never part of
-what reaches the channel.
+what reaches the channel. Applying that cap is what the counters are: one is the number of reports
+sent in the last hour by whoever you are, and the other is the relay's total for the day, which is
+tied to nobody. Neither records what you sent, or when, or that a particular report was yours.
+
+The Worker does not store your address in order to count against it. It stores a keyed hash of it -
+`HMAC-SHA-256`, under a key that exists only in Cloudflare's secret store and is never written
+alongside the counter - which is enough to recognise a repeat and not enough to name anyone: without
+that key, the stored value cannot be worked back to an address, and it is not a value anything else
+in the world uses. It stops counting an hour after the first report it counted, is dropped the next
+time anyone sends one, and expires within a day whether or not anyone does.
 
 A report stays in that channel until it is dealt with. If you want one removed, quote its id: the
 dialog shows it after a successful send and it is the only handle either of us has on it.

worker/README.md

@@ -47,7 +47,7 @@ You already have the account (the GitHub SSO login is fine; API tokens work the
 - **API token** — <https://dash.cloudflare.com/profile/api-tokens> → **Create Token** → **Custom
   token**, with:
   - `Account` → `Workers Scripts` → **Edit**
-  - `Account` → `Workers KV Storage` → **Edit** *(only if you set up rate limiting in step 6)*
+  - `Account` → `Workers KV Storage` → **Edit** *(step 6 creates a namespace with it)*
   - Account Resources → Include → your account
 
   The **Edit Cloudflare Workers** template also works, but it asks for zone permissions this Worker
@@ -85,18 +85,50 @@ in the repo.
 Paste the Discord URL at the prompt. It goes straight to Cloudflare's secret store — not into
 `.env`, not into any file. The same script replaces it later.
 
-### 6. Rate limiting (optional, recommended)
+### 6. Rate limiting
 
-Without this the relay works but has no brakes.
+Do not skip this. The caps live in `[vars]` but they are read only when the KV binding exists — with
+no `RL` binding the limiter returns before it looks at them, so the relay is a public endpoint that
+posts to your Discord as fast as anyone cares to ask.
 
 ```sh
 npx --yes wrangler kv namespace create ppc-reports-rl
 ```
 
 Put the printed id into the `[[kv_namespaces]]` block in `wrangler.toml`, uncomment it, and
-`./publish.sh` again. Defaults are 5 reports per IP per hour and 300 per day across the whole relay
-— both in `[vars]`. The free KV tier allows 1000 writes a day and each report costs two, so the
-daily cap keeps the relay inside it.
+`./publish.sh` again. Then:
+
+```sh
+./rotate-rl-salt.sh
+```
+
+Defaults are 5 reports per IP per hour and 600 per day across the whole relay — both in `[vars]`.
+
+Both caps live in **one** KV entry, `rl:<date>`, so an accepted report costs one read and one write
+and a refusal costs a read and nothing else. That makes `MAX_PER_DAY_GLOBAL` the day's write budget
+directly: the free tier allows 1000 writes, and at one per report the cap is the ceiling on how many
+are spent. The two-key version cost two writes per report and one per daily refusal, so the same
+protection cost more than double and the ceiling was not a number you could read off a var.
+
+Do not spend the remaining 400 on a higher cap. Two things live there. A limiter that cannot write
+fails open, so exhausting the quota does not throttle the relay, it un-throttles it for the rest of
+the day. And the count is approximate in exactly the case it exists for: KV reads can be up to a
+minute stale, so a sustained flood is read against a counter that lags it and writes more than the
+cap says. The margin is what absorbs that. The quota is also **per account**, not per namespace — a
+second KV-using Worker on the same account spends the same 1000.
+
+Both the counter's day (`rl:<date>`, UTC) and the quota's reset are UTC, so they roll over together.
+If you want a cap that is exact rather than approximate, that is a Durable Object, not a bigger
+number here.
+
+Inside that entry the day's total is a number and each reporter is `HMAC-SHA-256(RL_SALT, address)`
+truncated to 12 bytes, holding a count and the start of its hour. Entries older than an hour are
+dropped on the next write, so the value is the last hour's reporters rather than the day's. The salt
+is a Cloudflare secret, is never written to KV, and nobody needs to know it — `rotate-rl-salt.sh`
+generates it and does not print it. **Without it the limiter still works**, but an IPv4 address is 32
+bits, so the stored digest would be a lookup table away from the address; `wrangler tail` says so on
+every report until it is set. This paragraph is what [PRIVACY.md](../PRIVACY.md) promises about the
+relay's storage — a change here is a change there.
 
 ### 7. Check it end to end
 
@@ -116,6 +148,7 @@ is untrue, stop and say so rather than shipping a release that points at this.
 | --- | --- |
 | Mark a report handled | close the forum post. Deleting it works too, but a wording that comes back in three months is then gone. |
 | Change the Discord channel | make a new webhook, `./rotate-webhook.sh`. Clients never notice. |
+| Replace the rate limiter's salt | `./rotate-rl-salt.sh`. Every open hourly counter is forgotten, which is all it costs. |
 | Move back to a text channel | `DISCORD_FORUM = "0"` in `wrangler.toml`, `./publish.sh`. It has to match the channel: a forum rejects a message with no thread name and a text channel rejects one that has it. |
 | Turn reporting off | `REPORTS_ENABLED = "0"` in `wrangler.toml`, `./publish.sh`. |
 | Turn it off *now* | `npx --yes wrangler delete` — the app treats a dead endpoint as a dropped report. |

worker/rotate-rl-salt.sh

@@ -0,0 +1,14 @@
+#!/usr/bin/env bash
+# Set or replace the HMAC key the rate limiter derives its per-IP counter keys from.
+#
+# Nobody needs to know this value, so unlike the webhook it is generated here and never shown: it
+# goes from /dev/urandom to Cloudflare's secret store and is not echoed, not written to disk, and not
+# recoverable. Running this again forgets the hour's counters, which is the only thing it costs.
+set -euo pipefail
+
+cd "$(dirname "$0")"
+# shellcheck source=_env.sh
+. ./_env.sh
+
+set +x # the salt is a credential; keep it out of a trace for the same reason .env is
+openssl rand -hex 32 | tr -d '\n' | npx --yes wrangler secret put RL_SALT

worker/src/index.js

@@ -197,33 +197,73 @@ function webhook_ok(u) {
 }
 
 /**
- * Per-IP and whole-relay caps, both in KV. Optional: with no KV binding the relay still works, it
- * just has no brakes. Failures here fail *open* — losing a real report to a KV blip is worse than
- * letting one extra through.
+ * Per-IP and whole-relay caps. Optional: with no KV binding the relay still works, it just has no
+ * brakes. Failures here fail *open* — losing a real report to a KV blip is worse than letting one
+ * extra through.
+ *
+ * Both caps live in **one** key, and it costs one read and one write. Two keys would be the obvious
+ * shape and is the wrong one: the free tier's thousand writes a day are the thing the daily cap
+ * exists to protect, and a cap that spends two of them per report is guarding a budget it is the
+ * largest consumer of. A refusal writes nothing at all.
+ *
+ * The read-then-write is not atomic and a KV read can be a minute stale, so reports landing together
+ * lose increments and a sustained flood is counted against a total that lags it. Both caps are
+ * therefore approximate, and approximate in the direction of letting too much through — which is why
+ * the daily cap is set well under the write quota rather than up against it.
  */
 async function rate_limit(env, ip) {
     if (!env.RL) return null;
     const per_ip = Number(env.MAX_PER_IP_PER_HOUR || 5);
     const per_day = Number(env.MAX_PER_DAY_GLOBAL || 300);
+    const now = Date.now();
     try {
-        const day = new Date().toISOString().slice(0, 10);
-        if (ip && !(await bump(env.RL, `ip:${ip}`, 3600, per_ip))) {
-            return 'too many reports from this address, try again later';
-        }
-        if (!(await bump(env.RL, `all:${day}`, 86400, per_day))) {
-            return 'the relay is over its daily limit, try again tomorrow';
+        const key = `rl:${new Date(now).toISOString().slice(0, 10)}`;
+        const state = JSON.parse((await env.RL.get(key)) || '{}');
+        const total = Number(state.total) || 0;
+        if (total >= per_day) return 'the relay is over its daily limit, try again tomorrow';
+
+        // The whole value is rewritten anyway, so an address whose hour has run out is dropped here
+        // rather than kept until the day rolls over. This is also what bounds the value's size: it
+        // holds the last hour's reporters, not the day's.
+        const ips = {};
+        for (const [k, v] of Object.entries(state.ips || {})) {
+            if (now - v.at < 3600_000) ips[k] = v;
         }
+
+        const who = ip ? await ip_key(env, ip) : null;
+        const mine = who ? ips[who] : null;
+        if (mine && mine.n >= per_ip) return 'too many reports from this address, try again later';
+        if (who) ips[who] = { n: (mine ? mine.n : 0) + 1, at: mine ? mine.at : now };
+
+        const next = JSON.stringify({ total: total + 1, ips });
+        await env.RL.put(key, next, { expirationTtl: 86400 });
     } catch (e) {
         console.error(`rate limiter unavailable: ${e}`);
     }
     return null;
 }
 
-async function bump(kv, key, ttl, max) {
-    const cur = Number((await kv.get(key)) || 0);
-    if (cur >= max) return false;
-    await kv.put(key, String(cur + 1), { expirationTtl: ttl });
-    return true;
+/**
+ * What stands in for an address in KV. HMAC and not a digest on purpose: an IPv4 address is 32 bits,
+ * so a plain hash of one is a lookup table away from being the address again. The key is a
+ * Cloudflare secret and is never itself written to KV, so the namespace holds counts against strings
+ * that cannot be turned back into anyone. Set it with `./rotate-rl-salt.sh`; unset, this degrades to
+ * exactly that reversible digest rather than to no cap at all.
+ */
+async function ip_key(env, ip) {
+    if (!env.RL_SALT) console.error('RL_SALT is unset — per-IP keys are reversible');
+    const bytes = new TextEncoder();
+    const key = await crypto.subtle.importKey(
+        'raw',
+        bytes.encode(env.RL_SALT || 'ppc-reports'),
+        { name: 'HMAC', hash: 'SHA-256' },
+        false,
+        ['sign'],
+    );
+    const mac = new Uint8Array(await crypto.subtle.sign('HMAC', key, bytes.encode(ip)));
+    // 12 of the 32 bytes: a counter key, not a signature. Collisions at this width would mean two
+    // reporters sharing an hourly allowance, and there are not 2^48 reporters.
+    return [...mac.slice(0, 12)].map((b) => b.toString(16).padStart(2, '0')).join('');
 }
 
 async function post_to_discord(env, report, id, at) {

worker/test.mjs

@@ -193,12 +193,24 @@ test('REPORTS_ENABLED=0 turns the relay off', async () => {
     assert.equal(res.status, 503);
 });
 
-test('the per-IP cap rejects once it is reached', async () => {
+/** A KV stand-in that counts its writes, because how many there are is part of the contract. */
+function kv() {
     const store = new Map();
-    const RL = {
+    let writes = 0;
+    return {
+        store,
+        writes: () => writes,
+        only: () => JSON.parse([...store.values()][0]),
         get: async (k) => store.get(k) ?? null,
-        put: async (k, v) => void store.set(k, v),
+        put: async (k, v) => {
+            writes++;
+            store.set(k, v);
+        },
     };
+}
+
+test('the per-IP cap rejects once it is reached', async () => {
+    const RL = kv();
     const env = { RL, MAX_PER_IP_PER_HOUR: '2' };
     for (let i = 0; i < 2; i++) {
         assert.equal((await send({ item: ITEM }, env)).res.status, 200);
@@ -208,6 +220,60 @@ test('the per-IP cap rejects once it is reached', async () => {
     assert.match(json.error, /too many reports/);
 });
 
+test('the whole-relay cap rejects once it is reached', async () => {
+    const RL = kv();
+    const env = { RL, MAX_PER_DAY_GLOBAL: '1' };
+    assert.equal((await send({ item: ITEM }, env)).res.status, 200);
+    const { res, json } = await send({ item: ITEM }, { ...env, ip: '198.51.100.9' });
+    assert.equal(res.status, 429);
+    assert.match(json.error, /daily limit/);
+});
+
+test('a report costs one KV write, and a refusal costs none', async () => {
+    const RL = kv();
+    const env = { RL, MAX_PER_IP_PER_HOUR: '2' };
+    await send({ item: ITEM }, env);
+    await send({ item: ITEM }, env);
+    assert.equal(RL.writes(), 2);
+    assert.equal(RL.store.size, 1); // both caps in one key, not one key each
+    assert.equal((await send({ item: ITEM }, env)).res.status, 429);
+    assert.equal(RL.writes(), 2);
+});
+
+test('the address is stored as an HMAC of it, never as itself', async () => {
+    const RL = kv();
+    const salt = 'a'.repeat(64);
+    const env = { RL, RL_SALT: salt, ip: '203.0.113.7' };
+    await send({ item: ITEM }, env);
+
+    const [key, value] = [...RL.store.entries()][0];
+    assert.ok(!key.includes('203.0.113.7') && !value.includes('203.0.113.7'), `${key} ${value}`);
+    const who = Object.keys(RL.only().ips);
+    assert.equal(who.length, 1);
+    assert.match(who[0], /^[0-9a-f]{24}$/);
+
+    // The same address has to land on the same entry or the cap counts to one forever — and a
+    // different salt on a different one, which is what makes the entry depend on the secret.
+    await send({ item: ITEM }, env);
+    assert.deepEqual(Object.keys(RL.only().ips), who);
+    assert.equal(RL.only().ips[who[0]].n, 2);
+    await send({ item: ITEM }, { ...env, RL_SALT: 'b'.repeat(64) });
+    assert.equal(Object.keys(RL.only().ips).length, 2);
+});
+
+test('an address that has gone quiet for its hour is dropped from the value', async () => {
+    const RL = kv();
+    const [key] = [`rl:${new Date().toISOString().slice(0, 10)}`];
+    const stale = 'f'.repeat(24);
+    RL.store.set(key, JSON.stringify({ total: 4, ips: { [stale]: { n: 5, at: Date.now() - 7200_000 } } }));
+    await send({ item: ITEM }, { RL });
+
+    const state = RL.only();
+    assert.equal(state.total, 5, 'the day total is not what expires');
+    assert.ok(!(stale in state.ips));
+    assert.equal(Object.keys(state.ips).length, 1);
+});
+
 test('a broken rate limiter fails open', async () => {
     const RL = {
         get: async () => { throw new Error('KV is down'); },

worker/wrangler.toml

@@ -13,15 +13,21 @@ enabled = false
 # refusal the same as any other failure: the report is dropped and nothing is said.
 REPORTS_ENABLED = "1"
 MAX_PER_IP_PER_HOUR = "5"
-MAX_PER_DAY_GLOBAL = "300"
+# An accepted report costs one KV write and a refused one costs none, so this number *is* the day's
+# write budget. The free tier allows 1000, and the rest of that is not spare change: a limiter that
+# cannot write fails open, and under a real flood the counter reads stale (see README) and lets more
+# through than it says. 600 is the cap; the other 400 are what keep the cap honest.
+MAX_PER_DAY_GLOBAL = "600"
 # The webhook points at a forum channel, so each report is posted as its own thread. Set to "0" if
 # it is ever moved back to a plain text channel — a forum rejects a message with no thread name,
 # and a text channel rejects one that has it, so this has to match the channel.
 DISCORD_FORUM = "1"
 
-# Rate limiting is optional — without this binding the relay still works, it just has no brakes.
-# Create the namespace once, paste the id, uncomment:
-#   npx --yes wrangler kv namespace create ppc-reports-rl
-# [[kv_namespaces]]
-# binding = "RL"
-# id = "paste-the-id-here"
+# The caps above are read only when this binding exists, and only under this name: `env.RL` is what
+# the limiter looks for, so a binding spelled anything else deploys cleanly and caps nothing. The
+# namespace it points at can be called whatever you like — `ppc-reports-rl` is the one that was made
+# with `wrangler kv namespace create`. After deploying, set the salt the per-IP counters are hashed
+# under, once: ./rotate-rl-salt.sh
+[[kv_namespaces]]
+binding = "RL"
+id = "7cc232be372340d19361fa9629a010dd"