b16dd69 · skrepka · git.iko.soy

git.iko.soy/skrepka

Fix bug #13: key /poll and /messages rate limits on client IP, not bearer

byAuth keyed buckets on the raw Authorization value, so varying an invalid bearer minted a fresh bucket per request — bypassing the limit and growing the _knot_rate_limits table without bound. The limiter runs before the handler and can't distinguish valid sessions from invented tokens, so key on the client IP (same pickClientIp logic as the federation endpoints).

commitb16dd6948110f32c8ad2ae5ebe6668b4fe669071 authoriko <mail@iko.soy> dateThu 11 Jun 2026, 13:57 (4 months ago) parent23efe28

1 file changed+18−13

compare with master

unifiedsplit

server.knot+18−13

server.knot+18−13

@@ -120,12 +120,16 @@ rateLimitWindow = 60000

120120-- The normal handshake is one challenge + one verify, so a healthy flow uses 2.

121121authRateLimit : Int

122122authRateLimit = 10

123−-- Per-session (bearer token) budget for /poll per `rateLimitWindow`. Clients

124−-- reconnect after `pollTimeoutMs`, so steady state is ~3/min.

123+-- Per-client-IP budget for /poll per `rateLimitWindow`. Clients reconnect

124+-- after `pollTimeoutMs`, so steady state is ~3/min per client; the budget

125+-- leaves room for several clients behind one NAT. (Keyed on IP, not bearer

126+-- token — token-keyed buckets let an attacker mint a fresh bucket per

127+-- invented bearer value.)

125128pollRateLimit : Int

126129pollRateLimit = 60

127−-- Per-session budget for /messages per `rateLimitWindow`. Each call may carry

128−-- up to `maxBatchSize` ciphertexts, so the effective message ceiling is higher.

130+-- Per-client-IP budget for /messages per `rateLimitWindow`. Each call may

131+-- carry up to `maxBatchSize` ciphertexts, so the effective message ceiling

132+-- is higher.

129133sendRateLimit : Int

130134sendRateLimit = 60

131135-- Per-peer-IP budget for /federation/gossip per `rateLimitWindow`.

@@ -182,7 +186,7 @@ impl FromJSON GossipEventType where

182186-- /messages is all-or-nothing per call: either every message in the batch is

183187-- enqueued (delivered, federated, or buffered for federation), or none are

184188-- and the call returns a non-2xx HTTP status. The per-message rejection code

185−-- "self_send" surfaces in the HTTP error body. Per-session rate limiting is

189+-- "self_send" surfaces in the HTTP error body. Per-client-IP rate limiting is

186190-- enforced by the route's `rateLimit` clause, which short-circuits with 429

187191-- before the handler runs.

188192-- The sender's UI treats every success the same and learns "they got it"

@@ -266,10 +270,12 @@ type ForwardFailure = {server: Text, failedAt: Timestamp, failures: Int}

266270-- Rate limiting

267271-- ============================================

268272-- Per-endpoint token-bucket configuration. Auth endpoints key on the submitted

269−-- pubkey (so one keypair can't drain global capacity); authenticated client

270−-- endpoints key on the bearer token (per-session); federation endpoints key on

271−-- the client IP (last X-Forwarded-For hop behind a trusted proxy, else the

272−-- connection address). Each route has an independent bucket store, so endpoints

273+-- pubkey (so one keypair can't drain global capacity); client and federation

274+-- endpoints key on the client IP (last X-Forwarded-For hop behind a trusted

275+-- proxy, else the connection address). The bearer token is deliberately not a

276+-- bucket key: the limiter runs before the handler, so it can't tell valid

277+-- sessions from invented ones, and an attacker varying the token would mint a

278+-- fresh bucket per value. Each route has an independent bucket store, so endpoints

273279-- using the same key strategy don't share budget across routes. Buckets persist

274280-- in the runtime's `_knot_rate_limits` SQLite table; rejection responds 429

275281-- before the handler runs.

@@ -293,12 +299,11 @@ pickClientIp = \h connIp -> case h of

293299 Nothing {} -> connIp

294300

295301byPubkey = \{pubkey} _ -> Just {value: pubkey}

296−byAuth = \{authorization} _ -> Just {value: authorization}

297302byClientIp = \_ ctx -> Just {value: pickClientIp (ctx.header "X-Forwarded-For") ctx.clientIp}

298303

299304authLimit = {key: byPubkey, limit: {requests: authRateLimit, window: rateLimitWindow}}

300−pollLimit = {key: byAuth, limit: {requests: pollRateLimit, window: rateLimitWindow}}

301−sendLimit = {key: byAuth, limit: {requests: sendRateLimit, window: rateLimitWindow}}

305+pollLimit = {key: byClientIp, limit: {requests: pollRateLimit, window: rateLimitWindow}}

306+sendLimit = {key: byClientIp, limit: {requests: sendRateLimit, window: rateLimitWindow}}

302307gossipLimit = {key: byClientIp, limit: {requests: gossipRateLimit, window: rateLimitWindow}}

303308forwardLimit = {key: byClientIp, limit: {requests: forwardRateLimit, window: rateLimitWindow}}

304309

@@ -783,7 +788,7 @@ ackPolledMessages = \pubkey cursor -> do

783788-- either succeeds (every message enqueued) or fails with a non-2xx HTTP

784789-- status carrying the rejection code ("self_send" | ...).

785790-- Duplicates are silently treated as success so retries are idempotent.

786−-- Per-session send rate is enforced by the route's `rateLimit` clause.

791+-- Per-client-IP send rate is enforced by the route's `rateLimit` clause.

787792-- Federated forwards are kicked off in background forks after the atomic.

788793

789794-- POST /messages: send a list of ciphertexts atomically. Self-send is