Bound the receiver rate limit per client IP across /webhook/* (closes #139)
All checks were successful
check / check (push) Successful in 3m51s
All checks were successful
check / check (push) Successful in 3m51s
The receiver limiter keyed buckets on (client IP, request path). The
route pattern /webhook/{uuid} matches any single segment, so a client
that invented a fresh path per request minted a fresh bucket per
request and never refilled one: its aggregate rate against the only
unauthenticated, internet-exposed endpoint was unbounded, and every
one of those requests reached an entrypoint lookup before it 404ed.
Put a second limiter in front of it, keyed on the client IP alone and
covering the whole route at ten times the configured per-entrypoint
limit (1200/min by default). The per-entrypoint limit is unchanged and
still wanted; it just bounds nothing in aggregate on its own. Ten
entrypoints' worth of headroom lets one sender address drive several
entrypoints at full rate while still capping what one address costs
the receiver. The multiplication saturates rather than wrapping, since
nothing bounds RECEIVER_RATE_LIMIT from above and a negative limit
would reject every request.
Move the handler's INFO line for an incoming webhook below the
entrypoint lookup. The UUID is attacker-controlled path text, so
logging it first let a client write an INFO line per invented path; a
miss is already logged at DEBUG and the request is already in the
access log.
This commit is contained in:
16
README.md
16
README.md
@@ -94,7 +94,7 @@ TTY detection, and security headers are always applied.
|
||||
| `METRICS_PASSWORD` | Basic auth password for `/metrics` | `""` |
|
||||
| `SENTRY_DSN` | Sentry error reporting DSN | `""` |
|
||||
| `SESSION_IDLE_TIMEOUT` | Idle session timeout (Go duration) | `24h` |
|
||||
| `RECEIVER_RATE_LIMIT` | Receiver requests/minute per IP per entrypoint | `120` |
|
||||
| `RECEIVER_RATE_LIMIT` | Receiver requests/minute per IP per entrypoint (10x that per IP across the route) | `120` |
|
||||
| `TRUSTED_PROXIES` | CIDRs whose forwarded headers are trusted | `""` (none) |
|
||||
|
||||
#### Trusted proxies
|
||||
@@ -851,6 +851,20 @@ legitimate webhook senders). Requests over the limit receive HTTP 429
|
||||
with a `Retry-After` header. A set-but-invalid `RECEIVER_RATE_LIMIT`
|
||||
value aborts startup rather than silently falling back to the default.
|
||||
|
||||
A second limit sits in front of that one, keyed on the client IP alone
|
||||
and covering the whole route at ten times `RECEIVER_RATE_LIMIT` requests
|
||||
per minute (default 1200). The per-entrypoint limit needs it: the route
|
||||
pattern matches any single path segment, so a client that invents a
|
||||
fresh path per request gets a fresh per-entrypoint bucket every time and
|
||||
would otherwise have no aggregate limit at all — while each of those
|
||||
requests still costs an entrypoint lookup before it 404s. The aggregate
|
||||
limit leaves room for one address to drive several entrypoints at their
|
||||
full rate, and it is not configurable separately.
|
||||
|
||||
Requests to a `/webhook/` path that names no entrypoint are logged at
|
||||
`DEBUG` only, since the path is attacker-controlled; the request itself
|
||||
still appears in the access log.
|
||||
|
||||
Every limiter here — receiver, login, and password change — identifies
|
||||
the client the same way, through one shared key function: the
|
||||
connection's own address, unless the peer is listed in
|
||||
|
||||
Reference in New Issue
Block a user