Trust the RFC 1918 ranges as proxies when TRUSTED_PROXIES is unset (closes #333)
check / check (push) Successful in 4m43s
check / check (push) Successful in 4m43s
Unset or empty, TRUSTED_PROXIES now defaults to 10.0.0.0/8, 172.16.0.0/12 and 192.168.0.0/16, so a reverse proxy reaching webhooker from one of those ranges gets per-client rate-limit buckets with nothing set. A set value replaces the default entirely; an unparseable one still fails startup. The startup warning for an empty list is gone. The README gives the default, one rule (if any client can reach webhooker, or the proxy in front of it, from an RFC 1918 source address, set the list to the proxy's address alone), and where to find that address: the remoteIP field of the http request log line. Loopback is not in the default. Model: opus-5-5
This commit was merged in pull request #336.
This commit is contained in:
@@ -108,10 +108,10 @@ type failureWindow struct {
|
||||
//
|
||||
// A limiter that spends budget on arrival cannot protect a
|
||||
// single-admin product: behind the reverse proxy the deployment
|
||||
// requires, with TRUSTED_PROXIES unset, every client keys on the
|
||||
// proxy, so a stranger trickling five POSTs a minute keeps the one
|
||||
// bucket full and the operator's own correct password is answered 429
|
||||
// forever. There is no second administrative path.
|
||||
// requires, when TRUSTED_PROXIES does not cover it, every client
|
||||
// keys on the proxy, so a stranger trickling five POSTs a minute
|
||||
// keeps the one bucket full and the operator's own correct password
|
||||
// is answered 429 forever. There is no second administrative path.
|
||||
//
|
||||
// So budget is spent only by a FAILED verification. A correct
|
||||
// password is never throttled, whatever the counters say, which is
|
||||
|
||||
@@ -123,9 +123,8 @@ func bucketKey(addr netip.Addr) string {
|
||||
return prefix.String()
|
||||
}
|
||||
|
||||
// isTrustedProxy reports whether addr belongs to a network the
|
||||
// operator listed in TRUSTED_PROXIES. The list is empty by default,
|
||||
// so by default nothing is trusted.
|
||||
// isTrustedProxy reports whether addr belongs to a network in
|
||||
// TRUSTED_PROXIES, which by default is the RFC 1918 private ranges.
|
||||
func (m *Middleware) isTrustedProxy(addr netip.Addr) bool {
|
||||
for _, prefix := range m.params.Config.TrustedProxies {
|
||||
if prefix.Contains(addr) {
|
||||
|
||||
@@ -384,8 +384,8 @@ const (
|
||||
// trustedProxyCIDR is the proxy network the forwarded-path
|
||||
// tests configure, and trustedPeer an address inside it. A
|
||||
// production deployment is required to run behind a reverse
|
||||
// proxy with TRUSTED_PROXIES set, so this is the shape the
|
||||
// bucketing has to hold in.
|
||||
// proxy that TRUSTED_PROXIES covers, either by the default or by
|
||||
// a set value, so this is the shape the bucketing has to hold in.
|
||||
trustedProxyCIDR = "10.0.0.0/8"
|
||||
trustedPeer = "10.0.0.1:44444"
|
||||
)
|
||||
@@ -426,8 +426,8 @@ func assertSharedBucket(
|
||||
}
|
||||
|
||||
// TestRateLimitKey_SpoofedForwardedFromUntrustedPeer is the test
|
||||
// this gating exists for: with no trusted proxies configured (the
|
||||
// default), a client that rotates a forwarded header on every
|
||||
// this gating exists for: from a peer that is not a trusted
|
||||
// proxy, a client that rotates a forwarded header on every
|
||||
// request must stay in one bucket. If forwarded headers were
|
||||
// trusted unconditionally, each spoofed value would mint a fresh
|
||||
// bucket and the limit would stop no one.
|
||||
@@ -1097,8 +1097,9 @@ func TestPostRateLimit_IPv4IndependentPerAddress(t *testing.T) {
|
||||
// that arrives from trustedPeer — a configured trusted proxy — and
|
||||
// names forwarded as its client in X-Forwarded-For. That is the
|
||||
// production path: a deployment is required to run behind a reverse
|
||||
// proxy with TRUSTED_PROXIES set, so the forwarded address, not the
|
||||
// peer, is what the limiters bucket on there.
|
||||
// proxy that TRUSTED_PROXIES covers, either by the default or by a
|
||||
// set value, so the forwarded address, not the peer, is what the
|
||||
// limiters bucket on there.
|
||||
func forwardedKeyFor(
|
||||
t *testing.T, m *middleware.Middleware, forwarded string,
|
||||
) string {
|
||||
@@ -1178,9 +1179,9 @@ func TestRateLimitKey_ForwardedIPv6BucketsByPrefix(t *testing.T) {
|
||||
//
|
||||
// Every existing test of this fallback uses an IPv4 proxy, where
|
||||
// bucketKey is the identity function, so replacing the call with
|
||||
// peer.String() leaves the whole suite green. Only operator-listed
|
||||
// addresses reach this line and the fallback is fail-closed, so this
|
||||
// pins behaviour rather than fixing a defect.
|
||||
// peer.String() leaves the whole suite green. Only addresses inside
|
||||
// TRUSTED_PROXIES reach this line and the fallback is fail-closed, so
|
||||
// this pins behaviour rather than fixing a defect.
|
||||
func TestRateLimitKey_TrustedPeerUnusableForwardedMasksPeer(
|
||||
t *testing.T,
|
||||
) {
|
||||
|
||||
Reference in New Issue
Block a user