Bound the access log line against client-chosen text (closes #146)
All checks were successful
check / check (push) Successful in 2m51s
All checks were successful
check / check (push) Successful in 2m51s
The access log wrote one INFO line per request carrying r.URL.String(). Registered with Use, it runs ahead of the route limiter, so a client flooding the unauthenticated receiver with invented paths wrote attacker-chosen text of attacker-chosen length into the operator's log, one line per request. 3xx and 4xx responses now log the chi route pattern in place of the concrete URL, and the fixed literal "(unmatched)" when routing matched nothing at all. One line per request is retained, so real traffic stays observable and rate accounting still works, but the line's content is now bounded by the service's own route table. The pattern is only populated after routing, so it is read in the deferred part of the handler rather than before next.ServeHTTP. The route pattern alone does not close the hole, because it leaves two other ways for a request to choose the size of the line it writes. The query string is one: /.well-known/healthcheck and /s/* answer 200 to anyone with no rate limiter in front of them, and /pages/login behind only the login limiter, so appending 8 KB after the '?' bought the same amplification as an invented 404 path. The branches that keep the concrete URL now log the path only, with the query replaced by the fixed marker "?(redacted)". Nothing debuggable is lost: `page`, on the authenticated pagination links, is the only query parameter this service reads. The headers are the other: useragent and referer are logged on every line, including the correctly redacted ones, so an 8 KB User-Agent plus an 8 KB Referer produced a 24 KB line whose url field read "(unmatched)". Each field a client supplies is now truncated rather than dropped -- a truncated User-Agent is still worth reading -- to 512 bytes for url, useragent and referer, 128 for request_id (chi passes an inbound X-Request-Id header straight through), and 32 for method, which Go accepts as any token up to the header size limit. Truncation also drops invalid UTF-8, which a JSON encoder would otherwise expand six-fold past the budget. A complete line is now at most 2,560 bytes, which the tests assert against a request carrying 8 KB in the query and 8 KB in each of three headers, and which the README states so an operator can size log storage against it.
This commit is contained in:
40
README.md
40
README.md
@@ -904,8 +904,44 @@ requests and has the rest of its aggregate budget rejected there, so
|
||||
the aggregate limit is what bounds those `WARN` lines — to under ten
|
||||
times `RECEIVER_RATE_LIMIT` per minute per client IP, 1080 at the
|
||||
defaults, where before it there was no bound at all. The access log is
|
||||
bounded by neither limit: every request is recorded once at `INFO` with
|
||||
its full URL, served or rejected alike.
|
||||
bounded by neither limit: every request is recorded once at `INFO`,
|
||||
served or rejected alike.
|
||||
|
||||
What the access log does bound is the _content_ of those lines. A 3xx
|
||||
or 4xx response logs the chi route pattern — `/webhook/{uuid}`,
|
||||
`/user/{username}//`, or the literal `(unmatched)` when the request hit
|
||||
no route at all — in place of the concrete URL. Those are the outcomes
|
||||
an unauthenticated client can drive for free: 404 and 429 on any
|
||||
invented receiver path, a login redirect on any invented profile path.
|
||||
Logging the URL there would let a flood write text of its own choosing,
|
||||
at a length of its own choosing, into the log. 2xx and 5xx responses
|
||||
keep the concrete path — a success resolved against a static route or
|
||||
against the operator's own data (on the receiver, against a stored
|
||||
entrypoint UUID), and a 5xx is a bug in this service, where the exact
|
||||
path is the evidence and no client can provoke one at will.
|
||||
|
||||
The query string is never logged; it is replaced by the fixed marker
|
||||
`?(redacted)`. It is client-chosen on every route, and
|
||||
`/.well-known/healthcheck` and `/s/*` answer 200 to anyone with no rate
|
||||
limiter in front of them, so a query on a fixed 200 URL would otherwise
|
||||
buy the same amplification as an invented path. Nothing debuggable is
|
||||
lost: `page`, on the authenticated pagination links, is the only query
|
||||
parameter this service reads.
|
||||
|
||||
The remaining client-supplied fields are truncated rather than dropped,
|
||||
each to a fixed budget: 512 bytes for `url`, `useragent` and `referer`,
|
||||
128 for `request_id` (chi passes an inbound `X-Request-Id` header
|
||||
through), and 32 for `method`. A truncated `User-Agent` is still worth
|
||||
reading; an absent one is not. A cut value ends in `[truncated]`.
|
||||
|
||||
Net: **one `INFO` line per request, of at most 2,560 bytes** —
|
||||
`internal/middleware/accesslog_test.go` asserts that ceiling against a
|
||||
request carrying an 8 KB query, an 8 KB `User-Agent`, an 8 KB `Referer`
|
||||
and an 8 KB `X-Request-Id`, which together produce a 1,460-byte line.
|
||||
Multiply that ceiling by the request rate to size log storage. Note
|
||||
that the rate is not bounded by the limits above on every route:
|
||||
`/.well-known/healthcheck` and `/s/*` sit behind no limiter, so there
|
||||
the multiplier is whatever the deployment will serve.
|
||||
|
||||
Every limiter here — receiver, login, and password change — identifies
|
||||
the client the same way, through one shared key function: the
|
||||
|
||||
Reference in New Issue
Block a user