Rate limit login attempts per client address (closes #66) #143

Merged
clawbot merged 4 commits from issue-66-login-rate-limit into next 2026-09-29 01:03:38 +02:00
Collaborator

Closes #66.

POST / compared a submitted key against the signing key with no limit, so the key could be guessed at no cost. It is now limited to 5 attempts per minute per client address; an attempt over the limit gets 429 with Retry-After. GET / and POST /generate are unchanged.

How the client address is wired: the limit is a new RateLimit(requestLimit, window) middleware in internal/middleware, on github.com/go-chi/httprate. Its key is the address the ClientIP middleware already resolves on every route and stores in the request context (X-Forwarded-For believed only from a peer in trusted_proxies, #94), never RemoteAddr or a raw header. The image routes can use the same middleware later.

Worth knowing:

  • The limit runs after the body-size and CSRF checks: requests those refuse are not counted; every attempt that reaches the key comparison is.
  • httprate counts over a sliding minute, keeps counts for the current and previous minute only, and sets X-RateLimit-Limit, -Remaining and -Reset on POST / responses.
  • With the default trusted_proxies (the RFC 1918 ranges), a client with a private address chooses its counted address through X-Forwarded-For, directly or through the proxy; README.md says so and that setting trusted_proxies to the proxy's own address closes it.
  • The tests build the server's real routes with the constructors cmd/pixad uses, database included.

Disclosures:

  • Judgement call: an IPv6 client is counted by its /64, as the library recommends; an IPv4-mapped address (::ffff:a.b.c.d) is counted as the IPv4 address it carries.
  • Judgement call: no config key; the limit is the constant LoginAttemptsPerMinute.
  • Commits: the failing tests first, then the change.

Model: opus-5-5

Closes https://git.eeqj.de/sneak/pixa/issues/66. `POST /` compared a submitted key against the signing key with no limit, so the key could be guessed at no cost. It is now limited to 5 attempts per minute per client address; an attempt over the limit gets 429 with `Retry-After`. `GET /` and `POST /generate` are unchanged. How the client address is wired: the limit is a new `RateLimit(requestLimit, window)` middleware in `internal/middleware`, on `github.com/go-chi/httprate`. Its key is the address the `ClientIP` middleware already resolves on every route and stores in the request context (`X-Forwarded-For` believed only from a peer in `trusted_proxies`, https://git.eeqj.de/sneak/pixa/issues/94), never `RemoteAddr` or a raw header. The image routes can use the same middleware later. Worth knowing: - The limit runs after the body-size and CSRF checks: requests those refuse are not counted; every attempt that reaches the key comparison is. - httprate counts over a sliding minute, keeps counts for the current and previous minute only, and sets `X-RateLimit-Limit`, `-Remaining` and `-Reset` on `POST /` responses. - With the default `trusted_proxies` (the RFC 1918 ranges), a client with a private address chooses its counted address through `X-Forwarded-For`, directly or through the proxy; `README.md` says so and that setting `trusted_proxies` to the proxy's own address closes it. - The tests build the server's real routes with the constructors `cmd/pixad` uses, database included. Disclosures: - Judgement call: an IPv6 client is counted by its /64, as the library recommends; an IPv4-mapped address (`::ffff:a.b.c.d`) is counted as the IPv4 address it carries. - Judgement call: no config key; the limit is the constant `LoginAttemptsPerMinute`. - Commits: the failing tests first, then the change. Model: opus-5-5
clawbot added the needs-review label 2026-09-28 23:25:53 +02:00
clawbot self-assigned this 2026-09-28 23:25:53 +02:00
clawbot added 2 commits 2026-09-28 23:25:53 +02:00
The tests build the server's real routes and log in as a browser does.
The attempt after LoginAttemptsPerMinute failed logins from one client
must get 429 with Retry-After; another client must still get the login
form and log in with the signing key; two clients behind a trusted proxy
must be counted separately; X-Forwarded-For from an untrusted peer must
not get around the limit; an IPv6 client must be counted by its /64.

They do not compile yet: LoginAttemptsPerMinute comes with the change.

Model: opus-5-5
POST / had no limit, so the signing key could be guessed at no cost. It
is now limited to LoginAttemptsPerMinute (5) attempts per minute per
client by a new RateLimit middleware on github.com/go-chi/httprate. It
counts by the address the ClientIP middleware resolved through
trusted_proxies, an IPv6 client by its /64, and answers an attempt over
the limit with 429 and Retry-After. It runs after the body-size and CSRF
checks, so every attempt that reaches the key comparison is counted. The
image routes can reuse it. README states the limit; TODO narrows the
per-IP item to the image routes.

Model: opus-5-5
Author
Collaborator

FAIL

  1. internal/middleware/middleware.go, the RateLimit key: an IPv4 client that the trusted proxy forwards in IPv4-mapped form (::ffff:203.0.113.1, as a proxy on a dual-stack listener writes it) is counted by the /64 of that form, which is :: for every IPv4 address. All such clients share one count, so one client's failed attempts refuse everyone's logins, the operator's included. Acceptable: an IPv4-mapped address is counted as the IPv4 address it carries, with a test through the routes that two such clients behind the proxy are counted separately.

  2. README.md, the new login-limit paragraph: it does not say that with the default trusted_proxies (all RFC 1918 ranges) the limit does not hold for a client with a private address. One that connects directly picks its counted address with X-Forwarded-For, and so does one that comes through the proxy, because the private address the proxy appends is itself trusted and skipped. The PR body discloses only the direct case, and only in the PR. Acceptable: the paragraph states this and says that setting trusted_proxies to the proxy's own address closes it, and the trusted_proxies entry's "a client connecting directly cannot spoof its address" is qualified so it no longer contradicts it.

Model: opus-5-5

FAIL 1. `internal/middleware/middleware.go`, the `RateLimit` key: an IPv4 client that the trusted proxy forwards in IPv4-mapped form (`::ffff:203.0.113.1`, as a proxy on a dual-stack listener writes it) is counted by the /64 of that form, which is `::` for every IPv4 address. All such clients share one count, so one client's failed attempts refuse everyone's logins, the operator's included. Acceptable: an IPv4-mapped address is counted as the IPv4 address it carries, with a test through the routes that two such clients behind the proxy are counted separately. 2. `README.md`, the new login-limit paragraph: it does not say that with the default `trusted_proxies` (all RFC 1918 ranges) the limit does not hold for a client with a private address. One that connects directly picks its counted address with `X-Forwarded-For`, and so does one that comes through the proxy, because the private address the proxy appends is itself trusted and skipped. The PR body discloses only the direct case, and only in the PR. Acceptable: the paragraph states this and says that setting `trusted_proxies` to the proxy's own address closes it, and the `trusted_proxies` entry's "a client connecting directly cannot spoof its address" is qualified so it no longer contradicts it. Model: opus-5-5
clawbot added needs-rework and removed needs-review labels 2026-09-29 00:44:56 +02:00
clawbot added 1 commit 2026-09-29 00:48:18 +02:00
Two IPv4 clients that the trusted proxy forwards in IPv4-mapped form must
not share one login count. Fails: both fall in the same /64.

Model: opus-5-5
clawbot added 1 commit 2026-09-29 00:52:35 +02:00
A proxy on a dual-stack listener forwards an IPv4 client as ::ffff:a.b.c.d,
whose /64 is the same for every IPv4 client, so one client's failed logins
refused everyone's. The rate limit key now unmaps the address first.

README.md now says that with the default trusted_proxies a client with a
private address can choose its counted address through X-Forwarded-For, and
that setting trusted_proxies to the proxy's own address closes this.

Model: opus-5-5
Author
Collaborator

Rework for #143 (comment):

  1. Fixed: the rate limit key unmaps the address before counting; a new test through the routes covers two IPv4-mapped clients behind the proxy.
  2. Fixed: the login-limit paragraph and the trusted_proxies entry in README.md; the PR body bullet now matches.

Model: opus-5-5

Rework for https://git.eeqj.de/sneak/pixa/pulls/143#issuecomment-104453: 1. Fixed: the rate limit key unmaps the address before counting; a new test through the routes covers two IPv4-mapped clients behind the proxy. 2. Fixed: the login-limit paragraph and the `trusted_proxies` entry in `README.md`; the PR body bullet now matches. Model: opus-5-5
clawbot added needs-review and removed needs-rework labels 2026-09-29 00:53:01 +02:00
Author
Collaborator

PASS: the login limit counts each client by its resolved address (IPv4-mapped as IPv4, IPv6 by /64), and README.md now states the default trusted_proxies caveat and how to close it.

Model: opus-5-5

PASS: the login limit counts each client by its resolved address (IPv4-mapped as IPv4, IPv6 by /64), and `README.md` now states the default `trusted_proxies` caveat and how to close it. Model: opus-5-5
clawbot merged commit e410146fb6 into next 2026-09-29 01:03:38 +02:00
clawbot deleted branch issue-66-login-rate-limit 2026-09-29 01:03:38 +02:00
clawbot removed the needs-review label 2026-09-29 01:03:38 +02:00
Sign in to join this conversation.
No Reviewers
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/pixa#143