Compare commits
1 Commits
b5b3e1a926
...
issue-115-
| Author | SHA1 | Date | |
|---|---|---|---|
| 5c0ea2b44f |
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 (10x that per IP across the route) | `120` |
|
||||
| `RECEIVER_RATE_LIMIT` | Receiver requests/minute per IP per entrypoint | `120` |
|
||||
| `TRUSTED_PROXIES` | CIDRs whose forwarded headers are trusted | `""` (none) |
|
||||
|
||||
#### Trusted proxies
|
||||
@@ -851,20 +851,6 @@ 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
|
||||
|
||||
59
TODO.md
59
TODO.md
@@ -1,62 +1,31 @@
|
||||
# Workflow
|
||||
|
||||
One issue per unit of work, one branch and one PR per issue:
|
||||
|
||||
* ensure a tracked issue exists with a definition of done
|
||||
* branch from `next` (never from `main`)
|
||||
* do the work; open a PR based on `next` (never on `main`)
|
||||
* pass an independent review, then the manager squash-merges into `next`
|
||||
* push; nothing stays local-only
|
||||
|
||||
`next` is the branch for the next milestone and must stay green and
|
||||
mergeable to `main` without notice. One `next` -> `main` PR accumulates
|
||||
the milestone; releases are cut from `main` separately.
|
||||
|
||||
Issue branches do NOT touch this file — the manager maintains it on
|
||||
`next`. Every branch editing `TODO.md` conflicts with every other
|
||||
(#112).
|
||||
* branch (from `main`)
|
||||
* do the work in Next Step
|
||||
* move Next Step to the top of Completed Steps
|
||||
* move the top item of Future Steps into Next Step
|
||||
* commit (`TODO.md` changes in the same commit as the work)
|
||||
* merge to `main` if the branch is not protected, otherwise open a PR
|
||||
* push
|
||||
|
||||
# Status
|
||||
|
||||
pre-1.0. No git tags exist. `main` (4f5ecb1) is a working webhook proxy
|
||||
pre-1.0. No git tags exist. main (4f5ecb1) is a working webhook proxy
|
||||
with auth, CSRF/SSRF protections, login rate limiting, Slack target,
|
||||
event retention (#63), the database archiving target (#43), the admin
|
||||
password change flow (#65), policy compliance (#6), pinned lint tooling
|
||||
(#55), and fail-loud configuration parsing (#80).
|
||||
|
||||
`next` (9bfd033) holds the completed 1.0.0 milestone: every issue in it
|
||||
is closed, and it is verified green by cache-defeated container runs
|
||||
rather than by the CI badge, which can pass without executing anything
|
||||
(#119). Note: TODO.md was deliberately deleted from this repo in f9a9569
|
||||
(2026-03-01, #6); its content was folded into the README TODO section,
|
||||
which this draft reconstructs as of 2026-07-06.
|
||||
(#55), and fail-loud configuration parsing (#80). Note: TODO.md was
|
||||
deliberately deleted from this repo in f9a9569 (2026-03-01, #6); its
|
||||
content was folded into the README TODO section, which this draft
|
||||
reconstructs as of 2026-07-06.
|
||||
|
||||
# Next Step
|
||||
|
||||
Tag 1.0.0 from `main` once the milestone PR merges, then repair the CI
|
||||
gate (#119) before the next cycle's work lands — a gate that can report
|
||||
success without running is the one thing every other guarantee here
|
||||
rests on.
|
||||
Manual event redelivery from the web UI (replay is a core promised
|
||||
capability in the README rationale).
|
||||
|
||||
# Completed Steps
|
||||
|
||||
- 2026-08-12 Bound the `X-Forwarded-For` scan's allocation to the hop
|
||||
cap: the reverse walk cuts entries with `strings.LastIndexByte`
|
||||
instead of joining and splitting, so a 1 MB header allocates 16 bytes
|
||||
rather than 1.6 MB per request on the unauthenticated receiver.
|
||||
Semantics proven unchanged by differential testing against the
|
||||
previous implementation (#133)
|
||||
- 2026-08-12 Cap the `X-Forwarded-For` hop walk at 64 entries, so an
|
||||
attacker-supplied chain cannot burn unbounded CPU in the rate-limit
|
||||
key function; running off the end falls back to the peer address
|
||||
(#124)
|
||||
- 2026-08-12 Gate forwarded-header trust behind a `TRUSTED_PROXIES` CIDR
|
||||
list: all three rate limiters key on the connection's own address
|
||||
unless the direct peer is a configured proxy, in which case
|
||||
`X-Forwarded-For` is walked right to left for the first non-proxy hop.
|
||||
Default trusts nothing, and a set-but-unparseable value aborts
|
||||
startup. Before this, any client could mint a fresh bucket or drain
|
||||
another's by rotating a spoofed header (#88)
|
||||
- 2026-08-11 Web UI cleanup: nav terminology unified on Webhooks, the
|
||||
Profile settings placeholder removed, a progressive-enhancement copy
|
||||
button for the entrypoint URL, and retention form copy that states the
|
||||
|
||||
@@ -106,6 +106,12 @@ func slackConfigFields(configJSON string) []ConfigField {
|
||||
// and its retry settings. Header values are not shown — they
|
||||
// routinely carry authorization tokens — only how many are
|
||||
// configured.
|
||||
//
|
||||
// The destination is masked to scheme and host by the same
|
||||
// rule the Slack target uses. An HTTP target's destination is
|
||||
// commonly a Slack, Discord or Teams incoming-webhook endpoint
|
||||
// whose path segments are the credential, and the field takes
|
||||
// an arbitrary URL, so no segment can be assumed non-secret.
|
||||
func httpConfigFields(t *database.Target) []ConfigField {
|
||||
cfg, err := parseHTTPConfig(t.Config)
|
||||
if err != nil {
|
||||
@@ -114,7 +120,7 @@ func httpConfigFields(t *database.Target) []ConfigField {
|
||||
|
||||
fields := []ConfigField{{
|
||||
Label: "Destination URL",
|
||||
Value: cfg.URL,
|
||||
Value: MaskURL(cfg.URL),
|
||||
}}
|
||||
|
||||
if cfg.Timeout > 0 {
|
||||
|
||||
@@ -19,6 +19,7 @@ const (
|
||||
|
||||
viewExampleOrigin = "https://example.com"
|
||||
viewExampleHook = viewExampleOrigin + "/hook"
|
||||
viewMaskedOrigin = viewExampleOrigin + "/..."
|
||||
viewUnavailable = "(unavailable)"
|
||||
viewExpiryNever = "never"
|
||||
)
|
||||
@@ -162,7 +163,7 @@ func TestNewTargetViews_HTTP(t *testing.T) {
|
||||
assert.Equal(
|
||||
t,
|
||||
map[string]string{
|
||||
"Destination URL": viewExampleHook,
|
||||
"Destination URL": viewMaskedOrigin,
|
||||
"Timeout": "30s",
|
||||
"Headers": "1 configured",
|
||||
"Max Retries": "5",
|
||||
@@ -188,13 +189,41 @@ func TestNewTargetViews_HTTPFireAndForget(t *testing.T) {
|
||||
assert.Equal(
|
||||
t,
|
||||
map[string]string{
|
||||
"Destination URL": viewExampleHook,
|
||||
"Destination URL": viewMaskedOrigin,
|
||||
"Max Retries": "0 (fire-and-forget)",
|
||||
},
|
||||
fieldMap(view.Config),
|
||||
)
|
||||
}
|
||||
|
||||
// TestNewTargetViews_HTTPMasksDestinationURL proves the rule
|
||||
// holds for the http target too: an http destination is
|
||||
// routinely an incoming-webhook endpoint whose path segments
|
||||
// are the credential, so none of them is shown.
|
||||
func TestNewTargetViews_HTTPMasksDestinationURL(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
view := viewFor(t, database.Target{
|
||||
Type: database.TargetTypeHTTP,
|
||||
Config: `{"url":"` + slackWebhookURL + `"}`,
|
||||
})
|
||||
|
||||
fields := fieldMap(view.Config)
|
||||
|
||||
assert.Equal(
|
||||
t,
|
||||
"https://hooks.slack.com/...",
|
||||
fields["Destination URL"],
|
||||
)
|
||||
|
||||
for _, v := range fields {
|
||||
assert.NotContains(t, v, slackSecretPath)
|
||||
assert.NotContains(t, v, "T00000000")
|
||||
assert.NotContains(t, v, "B00000000")
|
||||
assert.NotContains(t, v, "XXXXXXXXXXXXXXXXXXXXXXXX")
|
||||
}
|
||||
}
|
||||
|
||||
func TestNewTargetViews_Database(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
|
||||
@@ -131,6 +131,47 @@ func TestHandleSourceDetail_MasksSlackWebhookURL(t *testing.T) {
|
||||
assert.Contains(t, body, "https://hooks.slack.com/...")
|
||||
}
|
||||
|
||||
// TestHandleSourceDetail_MasksHTTPDestinationURL is the
|
||||
// regression test for the same leak reached through the http
|
||||
// target: its destination is routinely an incoming-webhook
|
||||
// endpoint whose path segments are the credential, so the
|
||||
// rendered page must not contain them.
|
||||
func TestHandleSourceDetail_MasksHTTPDestinationURL(
|
||||
t *testing.T,
|
||||
) {
|
||||
t.Parallel()
|
||||
|
||||
var (
|
||||
h *handlers.Handlers
|
||||
sess *session.Session
|
||||
db *database.Database
|
||||
)
|
||||
|
||||
app := newTestApp(t, &h, &sess, &db)
|
||||
app.RequireStart()
|
||||
|
||||
t.Cleanup(app.RequireStop)
|
||||
|
||||
wh := seedWebhook(t, db)
|
||||
seedConfiguredTarget(
|
||||
t, db, wh.ID,
|
||||
database.TargetTypeHTTP,
|
||||
`{"url":"`+slackWebhookURL+`"}`,
|
||||
)
|
||||
|
||||
body := renderSourceDetailPage(t, h, sess, wh.ID)
|
||||
|
||||
assert.NotContains(t, body, slackSecretPath)
|
||||
assert.NotContains(t, body, "T00000000")
|
||||
assert.NotContains(t, body, "B00000000")
|
||||
assert.NotContains(
|
||||
t, body, "XXXXXXXXXXXXXXXXXXXXXXXX",
|
||||
)
|
||||
|
||||
assert.Contains(t, body, "Destination URL")
|
||||
assert.Contains(t, body, "https://hooks.slack.com/...")
|
||||
}
|
||||
|
||||
// TestHandleSourceDetail_RendersNamedTargetFields proves the
|
||||
// other target types render labelled fields rather than the
|
||||
// stored blob.
|
||||
@@ -172,7 +213,7 @@ func TestHandleSourceDetail_RendersNamedTargetFields(
|
||||
body := renderSourceDetailPage(t, h, sess, wh.ID)
|
||||
|
||||
assert.Contains(t, body, "Destination URL")
|
||||
assert.Contains(t, body, "https://example.com/hook")
|
||||
assert.Contains(t, body, "https://example.com/...")
|
||||
assert.Contains(t, body, "Timeout")
|
||||
assert.Contains(t, body, "1 configured")
|
||||
assert.NotContains(t, body, "sekrit")
|
||||
|
||||
@@ -39,6 +39,12 @@ func (h *Handlers) HandleWebhook() http.HandlerFunc {
|
||||
return
|
||||
}
|
||||
|
||||
h.log.Info("webhook request received",
|
||||
"entrypoint_uuid", entrypointUUID,
|
||||
"method", r.Method,
|
||||
"remote_addr", r.RemoteAddr,
|
||||
)
|
||||
|
||||
entrypoint, ok := h.lookupEntrypoint(
|
||||
w, r, entrypointUUID,
|
||||
)
|
||||
@@ -46,18 +52,6 @@ func (h *Handlers) HandleWebhook() http.HandlerFunc {
|
||||
return
|
||||
}
|
||||
|
||||
// Logged only once the UUID is known to name a real
|
||||
// entrypoint. The UUID comes straight out of the path on
|
||||
// the one unauthenticated endpoint, so logging it before
|
||||
// the lookup let a client write an INFO line per invented
|
||||
// path; the request itself is already in the access log
|
||||
// and a miss is already logged at DEBUG.
|
||||
h.log.Info("webhook request received",
|
||||
"entrypoint_uuid", entrypointUUID,
|
||||
"method", r.Method,
|
||||
"remote_addr", r.RemoteAddr,
|
||||
)
|
||||
|
||||
if !entrypoint.Active {
|
||||
http.Error(w, "Gone", http.StatusGone)
|
||||
|
||||
|
||||
@@ -25,11 +25,6 @@ func IPFromHostPort(hp string) string {
|
||||
return ipFromHostPort(hp)
|
||||
}
|
||||
|
||||
// ClientKeyForTest exposes clientKey for testing.
|
||||
func ClientKeyForTest(m *Middleware, r *http.Request) string {
|
||||
return m.clientKey(r)
|
||||
}
|
||||
|
||||
// IsClientTLS exposes isClientTLS for testing.
|
||||
func IsClientTLS(r *http.Request) bool {
|
||||
return isClientTLS(r)
|
||||
@@ -41,13 +36,3 @@ const LoginRateLimitConst = loginRateLimit
|
||||
// PasswordChangeRateLimitConst exposes the
|
||||
// passwordChangeRateLimit constant.
|
||||
const PasswordChangeRateLimitConst = passwordChangeRateLimit
|
||||
|
||||
// ReceiverAggregateMultiplierConst exposes the
|
||||
// receiverAggregateMultiplier constant.
|
||||
const ReceiverAggregateMultiplierConst = receiverAggregateMultiplier
|
||||
|
||||
// ReceiverAggregateLimitForTest exposes receiverAggregateLimit for
|
||||
// testing.
|
||||
func ReceiverAggregateLimitForTest(perEntrypoint int) int {
|
||||
return receiverAggregateLimit(perEntrypoint)
|
||||
}
|
||||
|
||||
@@ -1,7 +1,6 @@
|
||||
package middleware
|
||||
|
||||
import (
|
||||
"math"
|
||||
"net/http"
|
||||
"net/netip"
|
||||
"slices"
|
||||
@@ -33,21 +32,6 @@ const (
|
||||
// receiver rate limit. The configured limit is expressed in
|
||||
// requests per minute.
|
||||
receiverRateInterval = 1 * time.Minute
|
||||
|
||||
// receiverAggregateMultiplier scales the configured
|
||||
// per-entrypoint receiver limit into the aggregate limit one
|
||||
// client IP may spend across the whole /webhook/* route. Ten
|
||||
// entrypoints' worth lets a single sender address drive several
|
||||
// entrypoints at their full rate, while still capping what one
|
||||
// address costs the unauthenticated receiver.
|
||||
receiverAggregateMultiplier = 10
|
||||
|
||||
// maxForwardedHops bounds how many X-Forwarded-For entries the
|
||||
// chain walk examines. Real chains are one to three hops, but a
|
||||
// client can pad the header up to MaxHeaderBytes, so without a
|
||||
// bound every request pays a walk proportional to whatever the
|
||||
// client sent.
|
||||
maxForwardedHops = 64
|
||||
)
|
||||
|
||||
// normalizeAddr strips the IPv4-in-IPv6 wrapper and any zone from
|
||||
@@ -86,49 +70,26 @@ func (m *Middleware) isTrustedProxy(addr netip.Addr) bool {
|
||||
// a trusted proxy is the client. A hop that cannot be read as a bare
|
||||
// address ends the walk: past it the chain is not the shape assumed
|
||||
// here, so the caller falls back to the peer address.
|
||||
//
|
||||
// Only the last maxForwardedHops entries are examined. A longer chain
|
||||
// is padding, and running out of hops falls back to the peer address
|
||||
// the same way an unreadable hop does.
|
||||
//
|
||||
// The entries are cut off the right end of each header value in place
|
||||
// rather than split out of it: the receiver is unauthenticated and a
|
||||
// client can pad the header up to MaxHeaderBytes, so splitting would
|
||||
// allocate in proportion to the padding (about 8 MB for a 1 MB
|
||||
// header) before the cap could discard any of it. Multiple header
|
||||
// values are walked in reverse for the same reason, since joining
|
||||
// them copies the whole chain.
|
||||
func (m *Middleware) forwardedClientAddr(
|
||||
r *http.Request,
|
||||
) (netip.Addr, bool) {
|
||||
seen := 0
|
||||
hops := strings.Split(
|
||||
strings.Join(r.Header.Values("X-Forwarded-For"), ","), ",",
|
||||
)
|
||||
|
||||
for _, value := range slices.Backward(
|
||||
r.Header.Values("X-Forwarded-For"),
|
||||
) {
|
||||
for last := false; !last && seen < maxForwardedHops; seen++ {
|
||||
hop := value
|
||||
for _, hop := range slices.Backward(hops) {
|
||||
hop = strings.TrimSpace(hop)
|
||||
if hop == "" {
|
||||
continue
|
||||
}
|
||||
|
||||
comma := strings.LastIndexByte(value, ',')
|
||||
if comma < 0 {
|
||||
last = true
|
||||
} else {
|
||||
hop, value = value[comma+1:], value[:comma]
|
||||
}
|
||||
addr, err := netip.ParseAddr(hop)
|
||||
if err != nil {
|
||||
return netip.Addr{}, false
|
||||
}
|
||||
|
||||
hop = strings.TrimSpace(hop)
|
||||
if hop == "" {
|
||||
continue
|
||||
}
|
||||
|
||||
addr, err := netip.ParseAddr(hop)
|
||||
if err != nil {
|
||||
return netip.Addr{}, false
|
||||
}
|
||||
|
||||
if addr = normalizeAddr(addr); !m.isTrustedProxy(addr) {
|
||||
return addr, true
|
||||
}
|
||||
if addr = normalizeAddr(addr); !m.isTrustedProxy(addr) {
|
||||
return addr, true
|
||||
}
|
||||
}
|
||||
|
||||
@@ -152,10 +113,8 @@ func (m *Middleware) clientKey(r *http.Request) string {
|
||||
peer, err := netip.ParseAddr(ipFromHostPort(r.RemoteAddr))
|
||||
if err != nil {
|
||||
// Not an address we can reason about; key on the raw
|
||||
// value, the most specific identity left. On a
|
||||
// Unix-socket listener every peer carries the same
|
||||
// RemoteAddr and so shares one bucket, which is the
|
||||
// fail-closed direction.
|
||||
// value rather than collapsing such peers into one
|
||||
// shared bucket.
|
||||
return r.RemoteAddr
|
||||
}
|
||||
|
||||
@@ -251,26 +210,15 @@ func (m *Middleware) postRateLimit(
|
||||
}
|
||||
}
|
||||
|
||||
// ReceiverRateLimit returns middleware that rate-limits the public
|
||||
// webhook receiver endpoint with two limits in series.
|
||||
//
|
||||
// The inner limit is per client IP per request path: the path
|
||||
// contains the entrypoint UUID, so each sender is limited per
|
||||
// entrypoint without affecting other senders or other entrypoints.
|
||||
// It is Config.ReceiverRateLimit requests per minute.
|
||||
//
|
||||
// That limit alone bounds nothing in aggregate. The route pattern
|
||||
// /webhook/{uuid} matches any single segment, so a client that
|
||||
// invents a fresh path per request mints a fresh bucket per request
|
||||
// and never refills one — and every such request still reaches the
|
||||
// handler's entrypoint lookup before it 404s. The outer limit is
|
||||
// therefore keyed on the client IP alone, capping what one address
|
||||
// can spend across the whole route however it varies the path.
|
||||
//
|
||||
// Requests over either limit receive a 429. Clients are identified
|
||||
// by rateLimitKey.
|
||||
// ReceiverRateLimit returns middleware that rate-limits the
|
||||
// public webhook receiver endpoint per client IP per request
|
||||
// path (the path contains the entrypoint UUID, so each sender
|
||||
// is limited per entrypoint without affecting other senders or
|
||||
// other entrypoints). The limit is Config.ReceiverRateLimit
|
||||
// requests per minute. Requests over the limit receive a 429.
|
||||
// Clients are identified by rateLimitKey.
|
||||
func (m *Middleware) ReceiverRateLimit() func(http.Handler) http.Handler {
|
||||
perEntrypoint := httprate.Limit(
|
||||
return httprate.Limit(
|
||||
m.params.Config.ReceiverRateLimit,
|
||||
receiverRateInterval,
|
||||
httprate.WithKeyFuncs(
|
||||
@@ -282,31 +230,4 @@ func (m *Middleware) ReceiverRateLimit() func(http.Handler) http.Handler {
|
||||
"Too many requests. Please slow down.",
|
||||
)),
|
||||
)
|
||||
|
||||
aggregate := httprate.Limit(
|
||||
receiverAggregateLimit(m.params.Config.ReceiverRateLimit),
|
||||
receiverRateInterval,
|
||||
httprate.WithKeyFuncs(m.rateLimitKey),
|
||||
httprate.WithLimitHandler(m.tooManyRequests(
|
||||
"webhook receiver aggregate rate limit exceeded",
|
||||
"Too many requests. Please slow down.",
|
||||
)),
|
||||
)
|
||||
|
||||
return func(next http.Handler) http.Handler {
|
||||
return aggregate(perEntrypoint(next))
|
||||
}
|
||||
}
|
||||
|
||||
// receiverAggregateLimit is the per-IP aggregate limit derived from
|
||||
// the configured per-entrypoint limit. The operator sets the latter
|
||||
// and nothing bounds it from above, so the multiplication is
|
||||
// saturated rather than allowed to wrap into a negative limit that
|
||||
// would reject every request.
|
||||
func receiverAggregateLimit(perEntrypoint int) int {
|
||||
if perEntrypoint > math.MaxInt/receiverAggregateMultiplier {
|
||||
return math.MaxInt
|
||||
}
|
||||
|
||||
return perEntrypoint * receiverAggregateMultiplier
|
||||
}
|
||||
|
||||
@@ -4,15 +4,11 @@ import (
|
||||
"context"
|
||||
"fmt"
|
||||
"log/slog"
|
||||
"math"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"net/netip"
|
||||
"os"
|
||||
"runtime"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"sneak.berlin/go/webhooker/internal/config"
|
||||
@@ -572,176 +568,6 @@ func TestRateLimitKey_ChainWalkSkipsClientPrepended(t *testing.T) {
|
||||
)
|
||||
}
|
||||
|
||||
// TestRateLimitKey_LongChainCapsWalkAndFallsBackToPeer covers the
|
||||
// hop-walk cap. A client behind the trusted proxy can pad
|
||||
// X-Forwarded-For with tens of thousands of trusted-looking hops,
|
||||
// which costs a walk proportional to the padding and, once the walk
|
||||
// runs off the left end of the chain, reaches the entry the client
|
||||
// put there. Capping the walk stops both: the key falls back to the
|
||||
// peer address, so rotating the head of the chain mints no bucket,
|
||||
// and the run does not scale with the chain length.
|
||||
func TestRateLimitKey_LongChainCapsWalkAndFallsBackToPeer(
|
||||
t *testing.T,
|
||||
) {
|
||||
t.Parallel()
|
||||
|
||||
// 50k hops is roughly 0.9 MB, within the default
|
||||
// MaxHeaderBytes.
|
||||
const hops = 50000
|
||||
|
||||
padding := strings.Repeat(", 10.0.0.2", hops-1)
|
||||
|
||||
start := time.Now()
|
||||
|
||||
assertSharedBucket(
|
||||
t, trustedProxies("10.0.0.0/8"), "10.0.0.1:44444",
|
||||
func(i int) map[string]string {
|
||||
return map[string]string{
|
||||
headerXFF: fmt.Sprintf("9.9.9.%d%s", i+1, padding),
|
||||
}
|
||||
},
|
||||
"a padded X-Forwarded-For chain must fall back to the "+
|
||||
"peer address, not reach the client-controlled entry "+
|
||||
"at the head of the chain",
|
||||
)
|
||||
|
||||
assert.Less(
|
||||
t, time.Since(start), 2*time.Second,
|
||||
"the capped walk must not scale with the chain length",
|
||||
)
|
||||
}
|
||||
|
||||
// TestRateLimitKey_LongChainAllocationIsBounded is the allocation
|
||||
// half of the hop cap. Capping the walk still left every request
|
||||
// paying for the whole header the client sent, because the chain was
|
||||
// split before it was capped: about 8 MB of []string for the 1 MB a
|
||||
// default MaxHeaderBytes allows, on the unauthenticated receiver.
|
||||
//
|
||||
// Bytes are the measurement, not allocation count: strings.Split of a
|
||||
// 1 MB chain is a single allocation, so testing.AllocsPerRun scores
|
||||
// it as cheap. The test is deliberately sequential — it reads
|
||||
// process-wide counters, and Go runs this package's parallel tests
|
||||
// only after the sequential ones finish.
|
||||
//
|
||||
//nolint:paralleltest // reads process-wide allocation counters
|
||||
func TestRateLimitKey_LongChainAllocationIsBounded(t *testing.T) {
|
||||
// 100k hops of ", 10.0.0.2" is roughly 1 MB.
|
||||
const (
|
||||
hops = 100000
|
||||
iterations = 50
|
||||
maxBytesPerCall = 4096
|
||||
)
|
||||
|
||||
m := rateLimitMiddleware(t, &config.Config{
|
||||
TrustedProxies: trustedProxies("10.0.0.0/8"),
|
||||
})
|
||||
|
||||
req := httptest.NewRequestWithContext(
|
||||
context.Background(), http.MethodPost, loginPath, nil,
|
||||
)
|
||||
req.RemoteAddr = "10.0.0.1:44444"
|
||||
req.Header.Set(
|
||||
headerXFF, "9.9.9.9"+strings.Repeat(", 10.0.0.2", hops),
|
||||
)
|
||||
|
||||
var before, after runtime.MemStats
|
||||
|
||||
var key string
|
||||
|
||||
runtime.ReadMemStats(&before)
|
||||
|
||||
for range iterations {
|
||||
key = middleware.ClientKeyForTest(m, req)
|
||||
}
|
||||
|
||||
runtime.ReadMemStats(&after)
|
||||
|
||||
perCall := (after.TotalAlloc - before.TotalAlloc) / iterations
|
||||
|
||||
assert.Less(
|
||||
t, perCall, uint64(maxBytesPerCall),
|
||||
"a %d-byte X-Forwarded-For must not allocate in proportion "+
|
||||
"to its length, but cost %d bytes per call",
|
||||
len(req.Header.Get(headerXFF)), perCall,
|
||||
)
|
||||
|
||||
assert.Equal(
|
||||
t, "10.0.0.1", key,
|
||||
"the padded chain must still fall back to the peer address",
|
||||
)
|
||||
}
|
||||
|
||||
// TestReceiverRateLimit_LimitsAggregateAcrossInventedPaths is the
|
||||
// regression test for the per-path bucket key. The route pattern
|
||||
// matches any single segment, so a client that never reuses a path
|
||||
// never reuses a per-entrypoint bucket either, and its aggregate
|
||||
// rate against the receiver is whatever it likes — with every
|
||||
// request reaching an entrypoint lookup before it 404s. The IP-only
|
||||
// aggregate limiter is what bounds that, so this must fail if the
|
||||
// aggregate limiter is removed.
|
||||
func TestReceiverRateLimit_LimitsAggregateAcrossInventedPaths(
|
||||
t *testing.T,
|
||||
) {
|
||||
t.Parallel()
|
||||
|
||||
const (
|
||||
limit = 3
|
||||
ip = "6.6.6.6:1234"
|
||||
)
|
||||
|
||||
aggregate := limit * middleware.ReceiverAggregateMultiplierConst
|
||||
|
||||
handler := receiverLimitedHandler(t, limit)
|
||||
|
||||
// Every request goes to a path this client has never used, so
|
||||
// none of them shares a per-entrypoint bucket with another.
|
||||
for i := range aggregate {
|
||||
w := receiverPost(
|
||||
handler, ip, fmt.Sprintf("/webhook/invented-%d", i),
|
||||
)
|
||||
assert.Equal(
|
||||
t, http.StatusOK, w.Code,
|
||||
"request %d to a distinct path should pass", i,
|
||||
)
|
||||
}
|
||||
|
||||
w := receiverPost(
|
||||
handler, ip, fmt.Sprintf("/webhook/invented-%d", aggregate),
|
||||
)
|
||||
assert.Equal(
|
||||
t, http.StatusTooManyRequests, w.Code,
|
||||
"a client must not be able to raise its aggregate rate "+
|
||||
"against /webhook/* by varying the path",
|
||||
)
|
||||
|
||||
// The aggregate limit is still per client IP: exhausting one
|
||||
// address must not throttle another.
|
||||
w = receiverPost(handler, "6.6.6.7:1234", "/webhook/invented-0")
|
||||
assert.Equal(
|
||||
t, http.StatusOK, w.Code,
|
||||
"a different client IP must not be affected",
|
||||
)
|
||||
}
|
||||
|
||||
// TestReceiverAggregateLimit_SaturatesOnOverflow covers the derived
|
||||
// aggregate limit for a configured per-entrypoint limit large enough
|
||||
// that multiplying it would wrap negative, which httprate would read
|
||||
// as a limit that rejects every request.
|
||||
func TestReceiverAggregateLimit_SaturatesOnOverflow(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
assert.Equal(
|
||||
t, 1200,
|
||||
middleware.ReceiverAggregateLimitForTest(120),
|
||||
"the default limit scales by the multiplier",
|
||||
)
|
||||
assert.Equal(
|
||||
t, math.MaxInt,
|
||||
middleware.ReceiverAggregateLimitForTest(math.MaxInt),
|
||||
"an overflowing limit saturates instead of wrapping",
|
||||
)
|
||||
}
|
||||
|
||||
// TestReceiverRateLimit_IgnoresForwardedFromUntrustedPeer proves
|
||||
// the receiver limiter uses the same gated key function as the
|
||||
// POST limiters.
|
||||
|
||||
Reference in New Issue
Block a user