Skip to content

feat(gkapi): meter Tor exits through one shared invite ceiling - #82

Merged
sanity merged 3 commits into
mainfrom
tor-aggregate-bucket
Jul 25, 2026
Merged

feat(gkapi): meter Tor exits through one shared invite ceiling#82
sanity merged 3 commits into
mainfrom
tor-aggregate-bucket

Conversation

@sanity

@sanity sanity commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Status: reviewed, fixed, NOT deployed — needs a decision from @sanity.
See the review synthesis.
Review found a bug that would have crash-looped gkapi on deploy, and disproved
the sizing this PR was originally built on.

Problem

gkapi's per-IP invite limit (4/24h) is structurally defeated by Tor circuit
rotation: every exit node is a fresh IP and therefore a fresh bucket.

Measured from invite_rate_limits.json (1728 invites,
2026-07-24T06:23Z–2026-07-25T05:54Z), classified against the official bulk exit
list: the 02:35–03:05Z burst was 246 invites from 202 distinct IPs, 200 of
them via 159 Tor exits
— with no single IP near the per-IP limit. Tightening
the per-IP number cannot fix this; rotation routes around it at any value.

Approach

The Tor exit set is enumerable, so meter the whole set through one shared
hourly bucket
instead of treating exits as independent identities.

Ceiling: 60/hour, overridable at runtime (TOR_INVITES_PER_HOUR, 0
disables).

Tor invites/hour
organic (n=7, burst window excluded by time) mean 11.9, max 33
burst hour 02:00Z 208

An earlier revision used 25, derived by excluding high hours as "burst" by
magnitude
and taking the max of the rest — circular, and it understated the
peak 3x. 25 would have refused real users. 60 clears the measured organic
max with ~1.8x headroom and still cuts the burst hour ~71%.

This is a judgement call on ambiguous data, not a derived constant: hours 04Z
(28 invites / 25 exits) and 05Z (33/33) run ~1.0 invites per exit, which looks
like many real users — but a one-request-per-exit attacker is indistinguishable
from that by volume alone.

What this costs, stated plainly

Not a Tor block: Tor is ~7% of non-burst invite traffic (106 requests, 90 exits),
nearly all 1–2 requests each. A block denies those users in all states.

But the bucket is not free, and an earlier version of this PR was wrong to
say ordinary Tor users are unaffected. Sharing one budget across an anonymity set
means the loudest member can take all of it. Sustaining the ceiling costs an
attacker ~150 exits/day out of ~1400 available, refused requests cost them
nothing, and an automated poller beats a human to every freed slot. Before this
change, denying Tor users invites was not possible; after it, it is cheap.

That is inherent to any aggregate cap over an unlinkable pool. Closing it needs a
per-request cost signal — proof-of-work — which is #81 and remains the real fix.
Treat this as rate-shaping that buys time.

Failure policy: fails OPEN

No list and no cache ⇒ nothing is treated as Tor ⇒ pre-PR behaviour. A garbage or
truncated 200 never replaces a good list (plausible-minimum + shrinkage checks); a
fetch error never clears one; the cache is written write-then-rename; poisoned
locks resolve to "not Tor" / "has capacity". 0 disables the ceiling outright.

Handler ordering is load-bearing

The cheap capacity pre-check runs before the per-IP check, so a ceiling
refusal doesn't burn the requester's per-IP allowance. The atomic try_acquire()
— the actual admission authority — runs only after the per-IP check passes,
so a per-IP rejection never consumes shared Tor capacity. Both directions are now
pinned by mutation-verified tests.

Testing

35 unit tests + 1 HTTPS startup integration test.

  • 6 handler tests covering the wiring (previously untested: dropping
    try_acquire or reordering it left the suite green and the limiter defeated).
    Mutation-verified — each regression is caught by 4 tests.
  • 16-thread contention test pinning try_acquire atomicity.
  • tests/https_startup.rs spawns the real binary with real TLS material;
    verified to fail without the crypto-provider fix.
  • Sizing pin tied to the measured organic peak.
  • #[ignore]d live Tor-list fetch (1386 exits, cache round-trip):
    cargo test --bins -- --ignored

New CI job (rust-api-tests.yml): CI previously never built rust/api at
all, so none of these tests ran and a compile error would have merged green.

Operational surface (new)

  • hourly outbound HTTPS to check.torproject.org
  • writes /var/lib/gkapi/tor_exit_list.txt
  • new flags: --tor-exit-cache / TOR_EXIT_CACHE,
    --tor-invites-per-hour / TOR_INVITES_PER_HOUR
  • new deps: reqwest, rustls (the latter required to fix the provider clash)
  • unrelated cargo fmt reflow in errors.rs / invite.rs

Refs #81

[AI-assisted - Claude]

sanity added 2 commits July 25, 2026 00:45
## Problem

The per-IP invite limit (4/24h) is structurally defeated by Tor circuit
rotation: every exit node is a fresh IP and therefore a fresh bucket.
Measured 2026-07-25 from the live rate-limit store, one actor pulled 246
invites in ~20 minutes across 202 distinct IPs -- 200 of them via Tor
exits -- without any single IP approaching the limit. Tightening the
per-IP number cannot fix this; rotation routes around it at any value.

Downstream effect: every invite mints a fresh keypair, so the Official
River room got a stream of new identities that moderation could not
converge on (banning is per-identity).

## Approach

The Tor exit set is enumerable, so stop treating exits as independent
identities and meter the whole set through ONE shared hourly bucket.
Rotation then buys the attacker nothing.

Sizing from the same data: organic Tor traffic peaked at 11 invites/hour
(mean 6.8) across non-burst hours; the burst hit 158/hour. A ceiling of
25/hour leaves 2.3x headroom over the worst organic hour while capping a
rotation burst hard.

Deliberately NOT a Tor block. Tor is ~4.8% of non-burst invite traffic
(69 requests, 56 distinct exits, nearly all 1-2 requests each -- ordinary
usage). Freenet is a privacy project and quickstart is the main
onboarding path; blocking that traffic costs real users for no extra
protection a bucket does not already give.

Validated against the live exit list: 159/202 burst IPs are listed,
covering 200/246 burst invites. The ceiling would have admitted 25 and
refused 175 -- a ~71% cut to that burst. The rest came from non-Tor IPs
and is unaffected, which the ceiling does not claim to address.

## Failure policy

Fails OPEN throughout. If the list cannot be fetched and no cache exists,
nothing is treated as Tor and gkapi behaves exactly as before. A garbage
200 never wipes a good list; a fetch error never clears one; the cache is
written via write-then-rename so a crash cannot truncate it.

Ordering in the handler is deliberate: the Tor capacity check runs BEFORE
the per-IP check so a refused burst does not burn the requester's per-IP
allowance, and the matching record() runs only AFTER the per-IP check
passes so a per-IP rejection never consumes shared Tor capacity.

## Testing

9 new tests: parsing (v4/v6/junk/whitespace), fail-open on empty,
corrupt, and missing cache, atomic cache write, bucket limit/expiry,
shared-not-per-identity, and a sizing regression pinning the ceiling
above the observed organic peak and below the observed burst.

Plus an #[ignore]d live test against the real Tor Project list (run
before deploying changes to the fetch path); it fetched 1386 exits and
round-tripped the cache.

Refs #81

[AI-assisted - Claude]
@sanity

sanity commented Jul 25, 2026

Copy link
Copy Markdown
Contributor Author

External review (Codex, codex-cli 0.144.1)

Ran as the independent non-Claude pass required for a Full-tier change. Verbatim
findings, 8 issues (2 High). Verdict: "I would not deploy this as a hard 25/hour
control in its current form."

High

  1. has_capacity() / record() are not atomic (rate_limit.rs:236,
    routes.rs:398, routes.rs:457). Multiple Tor requests can all observe 24/25,
    pass the per-IP check, and all record. Today's overshoot is incidentally
    bounded by the synchronous per-IP mutex, but the ceiling is not actually
    enforced, and an async refactor would widen it. retry_after_seconds() also
    becomes wrong after overshoot. Fix: atomic reserve, with refund when the
    per-IP check or invite generation fails.

  2. The shared bucket is a cheap, sustained DoS against legitimate Tor users
    (routes.rs:398). An attacker needs only 25 accepted requests — 7 exits at the
    existing 4/IP limit — to make every Tor user receive 429s for the rest of the
    window, and can keep winning newly-freed slots indefinitely. The PR's
    "ordinary Tor users are unaffected" claim holds only when there is no attacker.

    Quantified against live data: sustaining the denial for 24h needs ~150 exits;
    the current list has 1386. So it is cheap and indefinitely sustainable.

    Narrowing that matters: organic Tor peaks at 11/hour, well under the 25
    ceiling, so in steady state the bucket has zero effect. It bites only above
    25/h, i.e. during a burst. The accurate risk statement is "an attacker can
    deliberately hold Tor onboarding degraded", not "Tor users suffer routinely".

    Codex is right that a shared anonymous bucket cannot fix this alone —
    preserving Tor availability under attack needs a per-request cost signal
    (proof-of-work / CAPTCHA / queued issuance). That is exactly gkapi: per-IP invite rate limit is defeated by Tor circuit rotation #81, and it is the
    real fix; this PR bounds the abuse but does not remove the tradeoff.

Medium

  1. Unbounded buffering (tor.rs:202): response.text().await buffers fully
    before the size check, so a chunked or mis-declared response can consume
    arbitrary memory within the 30s timeout. Stream and abort at the cap; compare
    declared length as u64, not a usize cast.

  2. "Non-empty" is insufficient validation (tor.rs:208): a truncated 200 or an
    HTML error page containing one IP silently replaces a good 1386-entry list with
    one entry — quietly disabling nearly all Tor aggregation. Needs a plausible
    minimum and rejection of implausible shrinkage; hitting MAX_EXITS should be an
    error, not a silent truncation.

  3. Stale snapshots enforce forever (tor.rs:153, tor.rs:177): staleness only
    changes logging. Reassigned ex-exit addresses keep getting metered as Tor,
    which contradicts the stated fail-open policy — stale data here causes denials,
    not just a weaker hint.

  4. No retry backoff (tor.rs:303): any failure sleeps a full hour, so a
    momentary outage at startup disables aggregation for an hour.

  5. Bucket is not persisted (rate_limit.rs:199): "25/hour" is really "25 per
    process lifetime per hour"; every restart grants a fresh burst.

Low

  1. Cache read is unbounded and disk IO is on the async path (tor.rs:261,
    tor.rs:273): apply the same byte cap on read, move disk work to
    spawn_blocking, and update the in-memory snapshot before best-effort persist.

Also: the refresh-failure log claims traffic is "metered per-IP only" even when a
cached set is still actively enforcing the bucket — misleading to an operator.

Codex explicitly did not flag the feed URL for lacking IP/port params, noting the
Tor Project's current bulk endpoint intentionally returns a simplified,
exit-policy-independent list.

Claude reviewers (code-first / testing / skeptical / big-picture) are running;
findings and the consolidated fix round follow.

[AI-assisted - Claude]

…ceiling

Five reviewers (4 Claude perspectives + Codex) found 20+ issues. The critical
one would have taken gkapi down on deploy.

## CRITICAL: HTTPS startup panic (would have caused an outage)

`reqwest`'s `rustls-tls` enables `rustls/ring`; axum-server's `tls-rustls`
enables `rustls/aws-lc-rs`. With BOTH provider features on, rustls 0.23's
`CryptoProvider::from_crate_features()` returns None by design and TLS
construction panics:

  no process-level CryptoProvider available

Confirmed empirically: the binary exits 101 before binding :443, taking down
donations and cert-signing too, not just invites. No unit test could catch it
(none construct a RustlsConfig) and `cargo run` without --tls-cert takes the
plain-HTTP branch.

Fix: install the aws-lc-rs provider explicitly in main(), preserving the
provider axum-server used before reqwest existed. Pinned by a new
tests/https_startup.rs that spawns the real binary with real TLS material and
makes a real HTTPS request -- verified to FAIL without the fix and pass with it.

## Sizing was wrong: 25/hour would have refused real users

The original ceiling came from a smaller sample filtered by MAGNITUDE (hours
above a threshold excluded as "burst", then the remaining max called the organic
peak) -- circular, and it understated the peak 3x. Re-derived from one dataset
excluding the burst window BY TIME: organic Tor peaks at 33/hour (mean 11.9),
not 11. Default raised to 60 and now overridable at runtime.

## Honest reframing

The claim "ordinary Tor users are unaffected" was false and is removed. Sharing
a budget across an anonymity set means the loudest member takes it: an attacker
sustaining the ceiling denies invites to every Tor user, and since refused
requests cost nothing, a poller beats a human to each freed slot. Inherent to
any aggregate cap over an unlinkable pool; closing it needs proof-of-work (#81).
This is rate-shaping that buys time, not a fix.

## Other fixes

- AggregateBucket: `try_acquire()` is now the atomic admission authority
  (check+record under one lock), so the ceiling holds under any concurrency and
  stays correct if an `.await` is later introduced. Pinned by a 16-thread
  contention test.
- Monotonic `Instant` instead of wall clock: an NTP step backwards would have
  stalled pruning and pinned the bucket full, denying all Tor users.
- Runtime off-switch: `--tor-invites-per-hour` / `TOR_INVITES_PER_HOUR`, `0`
  disables. Previously required a rebuild to disable or resize.
- Tor slot released when invite generation fails, so a generation-failure burst
  no longer locks out Tor users on top of the outage.
- IPv4-mapped IPv6 canonicalised: if the bind ever changes to `::`, every exit
  would otherwise silently escape metering.
- Fetch streams with a running cap; `response.text()` buffered the whole body
  BEFORE the size check, so a chunked response could OOM a small VM.
- List validation: "non-empty" was insufficient -- a truncated 200 or an HTML
  error page containing one IP could replace a good ~1400-entry list with a
  handful and silently disable metering. Now requires a plausible minimum and
  rejects implausible shrinkage.
- Refresh backoff (30s/1m/5m/15m) while the list is empty; previously one
  transient failure at startup disabled the ceiling for a full hour.
- Refresher panic is now logged at error!; previously it would freeze the exit
  list forever, silently.
- Exhaustion logged at warn! not info! -- it means real users are being refused.
- 429 copy no longer advises privacy users to stop using Tor, and no longer
  states the exact ceiling (which told an attacker what budget to drain).
- Per-IP 429 message interpolates the constant instead of hardcoding a stale 4.
- river-invite-button.html rendered every sub-hour retry as "approximately 1
  hour(s)". The Tor window is 60 min, so this PR made that routinely wrong; now
  renders seconds/minutes/hours, and treats 0 as a value rather than absent.
- Cargo.toml comment corrected: it claimed to avoid openssl, which was false
  (integration_test pulls native-tls in via feature unification).

## Testing

35 unit tests + 1 HTTPS startup integration test.

New handler tests close the gap every reviewer flagged: nothing previously
exercised the wiring, so dropping try_acquire or reordering it past the per-IP
check left the suite green and the limiter defeated. Mutation-verified -- both
regressions are caught by 4 tests each.

New CI job (rust-api-tests.yml): CI never built rust/api at all, so none of
these tests ran and a compile error would merge green.

Refs #81

[AI-assisted - Claude]
@sanity

sanity commented Jul 25, 2026

Copy link
Copy Markdown
Contributor Author

Consolidated review: 5 reviewers, 20+ findings, all addressed

Full-tier review per the multi-model rule: four Claude perspectives (code-first,
testing, skeptical, big-picture) plus an external Codex pass. Codex findings are
in the previous comment; this is the synthesis and what changed.

The one that mattered: HTTPS startup panic

Found independently by code-first and skeptical, from feature analysis
alone. I then confirmed it empirically:

$ ghostkey-api --tls-cert c.pem --tls-key k.pem --port 18443
[INFO ] TLS certificate and key provided. Starting in HTTPS mode.
thread 'main' panicked at rustls-0.23.12/src/crypto/mod.rs:259:14:
no process-level CryptoProvider available -- call CryptoProvider::install_default()
$ echo $?   # 101

reqwest's rustls-tls pulls rustls/ring; axum-server's tls-rustls pulls
rustls/aws-lc-rs. With both on, rustls 0.23 deliberately refuses to guess.
Deploying this would have crash-looped gkapi, taking down donations and
cert-signing as well as invites.

Nothing would have caught it: no unit test constructs a RustlsConfig, and
cargo run without --tls-cert takes the plain-HTTP branch. Every test I had
run was in the branch that doesn't panic.

Fixed by installing the aws-lc-rs provider explicitly in main(), pinned by
tests/https_startup.rs, which spawns the real binary with real TLS material.
Verified it fails without the fix (reproducing the exact panic) and passes
with it.

The sizing constant was wrong, and would have blocked real users

big-picture and code-first both flagged that my burst numbers disagreed
across the issue, PR body, and code comments. Reconciling them exposed a worse
problem: my "organic peak = 11/hour" was derived by excluding high-traffic hours
as "burst" by magnitude, then taking the max of what remained. That is
circular, and it understated the peak threefold.

Re-derived from one dataset, excluding the burst window by time:

Tor invites/hour
organic (n=7, burst excluded by time) mean 11.9, max 33
burst hour 02:00Z 208

A 25/hour ceiling would have refused real users. Default is now 60 (above
the measured organic max, still a ~71% cut to the burst hour) and overridable at
runtime.

Honest caveat now in the code: hours 04Z (28 invites / 25 exits) and 05Z (33/33)
run ~1.0 invites per exit, which looks like many distinct real users — but a
one-request-per-exit attacker is indistinguishable from that by volume alone.
This is a judgement call on ambiguous data, not a derived constant.

The central claim was false and has been removed

Every reviewer independently reached the same conclusion, and they are right:
"ordinary Tor users are unaffected" only holds when nobody is attacking.

Sustaining the ceiling costs an attacker ~150 exits/day out of ~1400 available,
and refused requests cost them nothing, so an automated poller wins every
freed slot against a human. Before this change, denying Tor users invites was
not possible at all; after it, it is cheap. skeptical added that
CorsLayer::permissive() makes it cheaper still — any webpage can make Tor
Browser visitors drain the shared budget without the attacker owning a circuit.

That is inherent to any aggregate cap over an unlinkable pool, not an
implementation defect. It is now stated plainly in the module docs and this PR
rather than claimed away.

Also fixed

Finding Fix
has_capacity/record not atomic try_acquire() is now the atomic admission authority; 16-thread contention test
Wall clock could pin the bucket full on NTP step-back monotonic Instant
No runtime off-switch --tor-invites-per-hour / TOR_INVITES_PER_HOUR, 0 disables
Tor slot burned on invite-generation failure released on the error path
IPv4-mapped IPv6 would bypass metering if bind became :: canonicalised
response.text() buffered before the size check streams with a running cap
A truncated 200 could silently disable metering plausible-minimum + shrinkage rejection
One transient failure disabled the ceiling for an hour 30s/1m/5m/15m backoff while empty
Refresher panic froze the list silently logged at error!
Exhaustion logged at info! warn! — it means real users are refused
429 told privacy users to stop using Tor, and named the exact budget reworded
Per-IP 429 hardcoded a stale "4" interpolates the constant
Client rendered every sub-hour retry as "approximately 1 hour(s)" renders s/min/h; treats 0 as a value
Cargo.toml comment claimed to avoid openssl (false) corrected

Testing gap closed

testing and skeptical both found that the wiring this PR calls
load-bearing had zero coverage — dropping try_acquire or reordering it past
the per-IP check left the whole suite green and the limiter fully defeated.

Six handler tests added, and mutation-verified: both regressions are caught
by 4 tests each.

testing also found that CI never built rust/api at alldeploy.yml
runs wasm+hugo only and integration-test.yml is workflow_dispatch:. The green
check on the first push of this PR was meaningless for this code; a syntax error
would have merged green. Added rust-api-tests.yml (fmt + build + test).

Now: 35 unit tests + 1 HTTPS startup integration test.

Not fixed, deliberately

  • CORS drive-by amplification — needs an Origin allowlist on
    /create-invite; a distinct change with its own risk, and it should not ride
    along with this one.
  • Bucket not persisted across restart — a restart forgives one window.
    gkapi restarts only on deploy; documented rather than adding IO to the hot path.
  • The DoS lever itself — unfixable with a shared bucket; needs proof-of-work
    (gkapi: per-IP invite rate limit is defeated by Tor circuit rotation #81).

Status: NOT deployed, and I don't think it should be without your call

Ian, you authorised deploy — but that was on the strength of two claims I have
since disproven: that organic Tor peaks at 11/hour (it's 33) and that ordinary
Tor users would be unaffected (they aren't, under attack). Issue #81 explicitly
routed the values question to you and it's still unanswered, and the
big-picture reviewer named that the one blocker it would not merge without.

The immediate spam is already handled out of band (the room's ban FIFO was
capped at 10 and is now 200, plus an auto-ban sweep), so there is no urgency
forcing this tonight.

[AI-assisted - Claude]

@sanity
sanity merged commit 6cd0c97 into main Jul 25, 2026
4 checks passed
@sanity
sanity deleted the tor-aggregate-bucket branch July 25, 2026 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant