Bound the access log line against client-chosen text (closes #146)
All checks were successful
check / check (push) Successful in 2m45s
All checks were successful
check / check (push) Successful in 2m45s
The access log wrote one INFO line per request carrying the full attacker-controlled URL, on the unauthenticated public receiver, so a client inventing paths wrote unbounded arbitrary text into the operator's logs. Rejected requests now log the chi route pattern instead of the concrete URL — extended to 3xx as well as 4xx, because RequireAuth answers 303 and so /user/<anything> was an unauthenticated path-varying vector. The query is redacted on the branches that keep a concrete path, and every client-supplied field is capped: url, useragent and referer at 512 bytes, request_id at 128, method at 32. The caps are spent in ENCODED bytes, so escaping cannot multiply them. One INFO line per request, at most 2,560 bytes — a figure derived arithmetically rather than observed, with the fixed portion measured at 336 (JSON) and 286 (text). Independently reviewed four times, and broken three of those times on the same class of defect: a stated bound the code did not have. Round 1 left the 2xx query and the headers unbounded; round 2 counted raw bytes against an encoded ceiling and broke at 2,611; round 3 charged 6 bytes for every non-printable when strconv.Quote spells astral ones as \UXXXXXXXX, and broke at 2,676. Two independent exhaustive audits over all 1,112,064 code points, built by different methods, now both report zero undercharged runes on either handler. Measured worst case over a real TCP socket is 1,972 bytes, 77% of the ceiling. Follow-up filed to assert that charge against every code point in the suite, so the ceiling defends itself rather than resting on one hand-picked rune.
This commit was merged in pull request #155.
This commit is contained in:
66
README.md
66
README.md
@@ -996,8 +996,70 @@ 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]`, which
|
||||
is charged on top of the budget rather than inside it.
|
||||
|
||||
Each budget is spent in _encoded_ bytes, not in the bytes the client
|
||||
sent. Every rune is charged what the wider of the two log handlers
|
||||
emits for it: two bytes for a quotation mark, a backslash or a tab; six
|
||||
for a non-printable rune below U+10000; ten for one at or above it,
|
||||
which the text handler spells `\UXXXXXXXX`. Go's header parser accepts
|
||||
all of them in a header value, so a budget counted raw would buy a
|
||||
field several times its nominal size — and the line, not the header, is
|
||||
what an operator has to store. Plain ASCII encodes one byte for one, so
|
||||
a real browser's `User-Agent` still fits whole; a value built out of
|
||||
escapes keeps a proportionally shorter prefix, which is the right
|
||||
trade.
|
||||
|
||||
Net: **one `INFO` line per request, of at most 2,560 bytes.** That
|
||||
ceiling is arithmetic, not an observation: 3 × (512 + 11) for `url`,
|
||||
`useragent` and `referer`, plus 128 + 11 for `request_id`, plus 32 + 11
|
||||
for `method`, plus a 336-byte fixed portion (the field names, the
|
||||
punctuation, both timestamps at their longest, an IPv6 `remoteIP` with
|
||||
a zone, the status and the latency) — 2,087 bytes, stated at 2,560 so
|
||||
the figure has headroom. `internal/middleware/accesslog_test.go`
|
||||
asserts it against 8 KB of client-chosen text in the path, in the
|
||||
query, and in each of `User-Agent`, `Referer` and `X-Request-Id`,
|
||||
including cases built from the characters the handlers escape, and
|
||||
against the widest line the service can be made to write: a 5xx that
|
||||
keeps its concrete path while all three header fields are also at their
|
||||
budget. Every case runs through both handlers `internal/logger` can
|
||||
select — the JSON one and the text one it installs on a tty — since the
|
||||
two do not escape alike and the ceiling is quoted unqualified. Measured
|
||||
over a real connection, the widest line is 1,972 bytes.
|
||||
|
||||
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