From 74bdc6a4491d95ffc9b6394e68dcfce0cad69ddb Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 6 Oct 2026 18:47:13 +0200 Subject: [PATCH] Leave SWWAF_RATE_LIMIT_EXEMPT_PATHS out of the request rate limits (closes #77) A request is neither counted nor refused by the request rate limits when its path as sent, the path the app receives, not percent-decoded, starts with one of the comma-separated prefixes in SWWAF_RATE_LIMIT_EXEMPT_PATHS, so /%61ssets/x is not under /assets/. A request whose decoded path contains .. or a backslash, or whose path as sent holds an encoded slash, is never exempt, since an app may act on it as a path outside every prefix, such as /assets/..%2Flogin as /login. The static lists, bans and the country lists still apply, and its log line has no counts. The setting is empty by default, and a prefix that does not start with / stops the start. README.md documents it. Model: opus-5-5 --- README.md | 73 ++++++++++++-------- internal/config/config.go | 36 +++++++++- internal/config/config_test.go | 29 +++++++- internal/proxy/proxy_test.go | 1 + internal/proxy/ratelimits_test.go | 88 ++++++++++++++++++++++++ internal/proxy/request.go | 33 ++++++++- internal/smallwebwaf/smallwebwaf_test.go | 1 + 7 files changed, 228 insertions(+), 33 deletions(-) diff --git a/README.md b/README.md index d09e8b6..67ed797 100644 --- a/README.md +++ b/README.md @@ -13,22 +13,23 @@ JSON log line for every request. Status: the first two milestones are built (https://git.eeqj.de/sneak/smallwebwaf/issues/13 and -https://git.eeqj.de/sneak/smallwebwaf/issues/14), and so are seven parts of -milestone 3: the static lists, the bans that broken rate limits lead to and the -JSON state files with your edits taken in while it runs, which come next in the -build order, `observe` mode and the rest of the request log's fields, which come -a little later, and the metrics endpoint and the header size and the idle time -as settings, which come last in it. `smallwebwaf` passes each request to the app -and the app's answer back, unchanged, within its timeouts and size limits, works -out each client's address, bans a client that sends too many requests, refuses a -client that comes from a country you refuse or from a network you refuse, lets -the networks you choose through, keeps its bans, each client's counters and -history, and GeoJS's answers in JSON files across restarts, takes in your edits -of those files while it runs, writes a JSON log line for every request, serves -Prometheus metrics to a scraper that holds the metrics token, and in `observe` -mode passes on the requests it would refuse, logging what it would have done -with them. It comes as the image the app's own image is built on. The rest of -the design comes after that, in the order of the build order in +https://git.eeqj.de/sneak/smallwebwaf/issues/14), and so are eight parts of +milestone 3: the static lists, the bans that broken rate limits lead to, the +JSON state files with your edits taken in while it runs and the paths the rate +limits do not count, which come next in the build order, `observe` mode and the +rest of the request log's fields, which come a little later, and the metrics +endpoint and the header size and the idle time as settings, which come last in +it. `smallwebwaf` passes each request to the app and the app's answer back, +unchanged, within its timeouts and size limits, works out each client's address, +bans a client that sends too many requests, not counting those for the paths you +choose, refuses a client that comes from a country you refuse or from a network +you refuse, lets the networks you choose through, keeps its bans, each client's +counters and history, and GeoJS's answers in JSON files across restarts, takes +in your edits of those files while it runs, writes a JSON log line for every +request, serves Prometheus metrics to a scraper that holds the metrics token, +and in `observe` mode passes on the requests it would refuse, logging what it +would have done with them. It comes as the image the app's own image is built +on. The rest of the design comes after that, in the order of the build order in [`SPEC.md`](SPEC.md). The survey of existing tools that led to the design is in [`EVALUATION.md`](EVALUATION.md). @@ -83,12 +84,15 @@ in `bin/state` unless `SWWAF_STATE_DIR` is set. - Counts each client's requests over a minute, an hour and a day. A request that takes the client over one of the rate limits below is refused with `SWWAF_BAN_RESPONSE`, `403` by default, before anything reaches the app, and - bans the client. A client is one IPv4 address, or one IPv6 /64, since one - abuser usually holds a whole /64. Each window is counted in two fixed buckets, - the earlier one weighted by how much of it the window still covers. At most - 20,000 clients are kept, the least recently seen dropped first, with their - history, and a restart gives no client a fresh allowance (see "State files" - below). + bans the client. A request whose path starts with one of + `SWWAF_RATE_LIMIT_EXEMPT_PATHS`, as that setting below describes, is neither + counted nor refused by the rate limits; the static lists, bans and the country + lists still apply to it. A client is one IPv4 address, or one IPv6 /64, since + one abuser usually holds a whole /64. Each window is counted in two fixed + buckets, the earlier one weighted by how much of it the window still covers. + At most 20,000 clients are kept, the least recently seen dropped first, with + their history, and a restart gives no client a fresh allowance (see "State + files" below). - Bans a client that breaks a rate limit, as "Bans" in [`SPEC.md`](SPEC.md) describes: the first ban lasts an hour, and a limit broken again within a day of a ban ending bans for three times as long as that ban, so 1, 3, 9, 27 and @@ -200,6 +204,20 @@ it, and the effective settings are logged at start. requests a client may make in a minute, an hour and a day. The defaults are several times what one busy person produces, since a browser loading a heavy page makes a few hundred requests and several people often share one address. +- `SWWAF_RATE_LIMIT_EXEMPT_PATHS` (default empty): path prefixes whose requests + the rate limits neither count nor refuse, such as `/assets/` for static + assets; each starts with `/`. A request whose path, percent-decoded, contains + `..` anywhere or a backslash, or whose path as sent holds an encoded slash + (`%2F` or `%2f`), is never exempt, since the app may act on it as a path + outside every prefix: `/assets/..%2Flogin` as `/login`. Any other request is + exempt when its path as sent, the path the app receives, before any query + string and not percent-decoded, starts with a prefix, character for character. + `/assets/` matches `/assets/app.js` and `/assets/`, but not `/assets`, + `/Assets/app.js`, `/%61ssets/app.js`, `/static/assets/app.js`, + `/static/../assets/app.js` or `/assets%2Fapp.js`. A character the client sends + percent-encoded, such as a space, is written percent-encoded in a prefix, as + in `/my%20files/`, and there are no wildcards: `*` is a character like any + other. - `SWWAF_DENIED_COUNTRIES` (default empty): countries whose clients are refused, for example `cn,ru,kp`. - `SWWAF_EXCLUSIVELY_ALLOWED_COUNTRIES` (default empty): when set, the only @@ -322,9 +340,10 @@ A field that does not apply to a request is left out of its line, apart from have a fraction. For a request that broke a limit, they are the counts that broke it. It is left out for a request the rate limits do not count: the health check, one from a client in `SWWAF_ALLOW_NETS` or - `SWWAF_RATE_LIMIT_EXEMPT_NETS`, and one that `SWWAF_DENY_NETS`, a ban or the - country lists refuse, or would refuse in `observe` mode. The byte totals come - with the byte limits. + `SWWAF_RATE_LIMIT_EXEMPT_NETS`, one for a path that + `SWWAF_RATE_LIMIT_EXEMPT_PATHS` exempts, and one that `SWWAF_DENY_NETS`, a ban + or the country lists refuse, or would refuse in `observe` mode. The byte + totals come with the byte limits. - `limit_hit` is there for a request that broke a rate limit, and names the window whose limit it went over: `minute`, `hour` or `day`, the shortest if it went over several. `offence` is then `limit`. @@ -830,8 +849,8 @@ so that they run in minimal containers. ## TODO -- The rest of milestone 3: exemptions; then the rest of the design, in the order - of the build order in [`SPEC.md`](SPEC.md). +- The rest of the design, in the order of the build order in + [`SPEC.md`](SPEC.md). ## Documents diff --git a/internal/config/config.go b/internal/config/config.go index 1c62687..0b94680 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -80,6 +80,10 @@ type Config struct { RateLimitPerMinute int64 RateLimitPerHour int64 RateLimitPerDay int64 + // RateLimitExemptPaths are the path prefixes whose requests the rate + // limits neither count nor refuse (SWWAF_RATE_LIMIT_EXEMPT_PATHS). + // Each starts with /. + RateLimitExemptPaths []string // DeniedCountries are the countries whose clients are refused // (SWWAF_DENIED_COUNTRIES). ExclusivelyAllowedCountries, when not // empty, are the only countries whose clients are let through @@ -179,8 +183,10 @@ var ( "is not the length of an IPv4 netblock, from 0 to 32, such as 24") errNotAbsolutePath = errors.New( "is not an absolute path, such as /var/lib/smallwebwaf") - errShortToken = errors.New("is shorter than 32 characters") - errNotMode = errors.New("is not enforce or observe") + errShortToken = errors.New("is shorter than 32 characters") + errNotMode = errors.New("is not enforce or observe") + errNotPathPrefix = errors.New( + "is not a path prefix starting with /, such as /assets/") ) // FromEnvironment reads the settings with lookupEnv, normally @@ -210,6 +216,7 @@ func FromEnvironment(lookupEnv func(string) (string, bool)) (*Config, error) { RateLimitPerMinute: env.count("SWWAF_RATE_LIMIT_PER_MINUTE", "1000"), RateLimitPerHour: env.count("SWWAF_RATE_LIMIT_PER_HOUR", "10000"), RateLimitPerDay: env.count("SWWAF_RATE_LIMIT_PER_DAY", "50000"), + RateLimitExemptPaths: env.pathPrefixes("SWWAF_RATE_LIMIT_EXEMPT_PATHS", ""), DeniedCountries: env.countries("SWWAF_DENIED_COUNTRIES", ""), ExclusivelyAllowedCountries: env.countries( "SWWAF_EXCLUSIVELY_ALLOWED_COUNTRIES", ""), @@ -350,6 +357,14 @@ func (e *environment) count(name, defaultValue string) int64 { return count } +// pathPrefixes reads a setting that is a list of path prefixes. +func (e *environment) pathPrefixes(name, defaultValue string) []string { + prefixes, err := parsePathPrefixes(e.value(name, defaultValue)) + e.check(name, err) + + return prefixes +} + // countries reads a setting that is a list of countries. func (e *environment) countries(name, defaultValue string) []string { countries, err := parseCountries(e.value(name, defaultValue)) @@ -637,6 +652,23 @@ func parseNetblock(value string) (netip.Prefix, error) { return netip.PrefixFrom(addr, addr.BitLen()), nil } +// parsePathPrefixes reads a comma-separated list of path prefixes, each +// starting with /. +func parsePathPrefixes(value string) ([]string, error) { + prefixes, err := parseList(value) + if err != nil { + return nil, err + } + + for _, prefix := range prefixes { + if !strings.HasPrefix(prefix, "/") { + return nil, fmt.Errorf("%q %w", prefix, errNotPathPrefix) + } + } + + return prefixes, nil +} + // countryCodes are the two-letter codes ISO 3166-1 assigns today, and XK, // the code in common use for Kosovo. golang.org/x/text/language cannot // check them: it also takes withdrawn codes such as su, and reserved ones diff --git a/internal/config/config_test.go b/internal/config/config_test.go index ccfe4c7..cfb1a6e 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -35,6 +35,7 @@ const ( rateLimitPerMinute = "SWWAF_RATE_LIMIT_PER_MINUTE" rateLimitPerHour = "SWWAF_RATE_LIMIT_PER_HOUR" rateLimitPerDay = "SWWAF_RATE_LIMIT_PER_DAY" + rateLimitExemptPaths = "SWWAF_RATE_LIMIT_EXEMPT_PATHS" deniedCountries = "SWWAF_DENIED_COUNTRIES" allowedCountries = "SWWAF_EXCLUSIVELY_ALLOWED_COUNTRIES" banResponse = "SWWAF_BAN_RESPONSE" @@ -139,6 +140,10 @@ func TestDefaults(t *testing.T) { t.Errorf("%s gave %v, want %v", logRequestHeaders, cfg.LogRequestHeaders, wantHeaders) } + + if len(cfg.RateLimitExemptPaths) != 0 { + t.Errorf("%s gave %v, want none", rateLimitExemptPaths, cfg.RateLimitExemptPaths) + } } func TestValuesAsSet(t *testing.T) { @@ -163,6 +168,7 @@ func TestValuesAsSet(t *testing.T) { rateLimitPerMinute: "60", rateLimitPerHour: "600", rateLimitPerDay: "6000", + rateLimitExemptPaths: "/assets/, /favicon.ico", deniedCountries: "cn, RU,kp,Xk", allowedCountries: "de", banResponse: "429", @@ -215,6 +221,24 @@ func TestValuesAsSet(t *testing.T) { wantNetblocks(t, cfg.DenyNets, "198.51.100.0/24") wantCountries(t, deniedCountries, cfg.DeniedCountries, "CN", "RU", "KP", "XK") wantCountries(t, allowedCountries, cfg.ExclusivelyAllowedCountries, "DE") + + if !slices.Equal(cfg.RateLimitExemptPaths, []string{"/assets/", "/favicon.ico"}) { + t.Errorf("%s gave %v, want /assets/ and /favicon.ico", + rateLimitExemptPaths, cfg.RateLimitExemptPaths) + } +} + +func TestPathPrefixNotStartingWithSlashStopsTheStart(t *testing.T) { + t.Parallel() + + _, err := config.FromEnvironment( + environment{rateLimitExemptPaths: "/favicon.ico,assets/"}.lookupEnv) + + want := rateLimitExemptPaths + `: "assets/" is not a path prefix ` + + `starting with /, such as /assets/` + if err == nil || err.Error() != want { + t.Errorf("error %v, want %s", err, want) + } } func TestInstanceNameAndLoggedHeadersAsSet(t *testing.T) { @@ -368,8 +392,8 @@ func TestInvalidValueStopsTheStart(t *testing.T) { {rateLimitPerMinute, "1K"}, {rateLimitPerHour, "0"}, {rateLimitPerHour, "1.5"}, - {rateLimitPerDay, "-1"}, - {rateLimitPerDay, "lots"}, + {rateLimitPerDay, "-1"}, {rateLimitPerDay, "lots"}, + {rateLimitExemptPaths, "/assets/,,/static/"}, {deniedCountries, "nk"}, {deniedCountries, "kp,,ir"}, {deniedCountries, "prk"}, @@ -505,6 +529,7 @@ func TestLogsEachSettingWithItsValue(t *testing.T) { rateLimitPerMinute: "1000", rateLimitPerHour: "10000", rateLimitPerDay: "50000", + rateLimitExemptPaths: "", deniedCountries: "", allowedCountries: "", banResponse: "403", diff --git a/internal/proxy/proxy_test.go b/internal/proxy/proxy_test.go index ccb3bed..4a3980d 100644 --- a/internal/proxy/proxy_test.go +++ b/internal/proxy/proxy_test.go @@ -63,6 +63,7 @@ const ( denyNets = "SWWAF_DENY_NETS" rateLimitPerMinute = "SWWAF_RATE_LIMIT_PER_MINUTE" rateLimitPerDay = "SWWAF_RATE_LIMIT_PER_DAY" + rateLimitExemptPaths = "SWWAF_RATE_LIMIT_EXEMPT_PATHS" deniedCountries = "SWWAF_DENIED_COUNTRIES" allowedCountries = "SWWAF_EXCLUSIVELY_ALLOWED_COUNTRIES" banResponse = "SWWAF_BAN_RESPONSE" diff --git a/internal/proxy/ratelimits_test.go b/internal/proxy/ratelimits_test.go index 8d57142..1d8b504 100644 --- a/internal/proxy/ratelimits_test.go +++ b/internal/proxy/ratelimits_test.go @@ -5,6 +5,8 @@ import ( "sync/atomic" "testing" + "sneak.berlin/go/smallwebwaf/internal/lookup" + "sneak.berlin/go/smallwebwaf/internal/ratelimit" "sneak.berlin/go/smallwebwaf/internal/requestlog" ) @@ -69,3 +71,89 @@ func TestRateLimitRefusesBeforeTheApp(t *testing.T) { t.Errorf("the app was called %d times, want 4", calls.Load()) } } + +func TestRateLimitExemptPathsAreNeitherCountedNorRefused(t *testing.T) { + t.Parallel() + + const denied = "192.0.2.50" // in SWWAF_DENY_NETS + + s, _, server := startWithClock(t, "", map[string]string{ + rateLimitPerMinute: "1", + rateLimitExemptPaths: "/assets/,/favicon.ico", + denyNets: denied, + deniedCountries: "kp", + }) + + // The answers are kept before the requests, so that none waits for + // GeoJS. + server.GeoJS.Load([]lookup.Answer{ + keptAnswer(client, "DE"), keptAnswer(fromKP, "KP"), + }) + + // With a limit of one request a minute, the requests for paths under a + // prefix are not counted, so client's first request for / is within + // the limit; and once client has reached it, they are not refused. + s.request(client, "/assets/app.js", http.StatusOK, requestlog.ActionForward) + s.request(client, "/favicon.ico?v=2", http.StatusOK, requestlog.ActionForward) + s.get(client, http.StatusOK, requestlog.ActionForward) + + line := s.request(client, "/assets/app.js", http.StatusOK, requestlog.ActionForward) + if line.LimitHit != "" || line.Counts != (ratelimit.Counts{}) { + t.Errorf("log line has limit_hit %q and counts %+v, want neither", + line.LimitHit, line.Counts) + } + + // A path outside every prefix is counted: /assets is not under + // /assets/, and breaks the limit. + s.request(client, "/assets", http.StatusForbidden, requestlog.ActionRateLimited) + + // A ban, SWWAF_DENY_NETS and the country lists still refuse a path + // under a prefix. + s.request(client, "/assets/app.js", http.StatusForbidden, requestlog.ActionBanned) + s.request(denied, "/assets/app.js", http.StatusForbidden, requestlog.ActionDenied) + s.request(fromKP, "/assets/app.js", + http.StatusForbidden, requestlog.ActionCountryDenied) +} + +func TestRateLimitCountsPathsThatAreNotExempt(t *testing.T) { + t.Parallel() + + for _, sent := range []string{ + // A prefix matches only at the start of the path. + "/static/assets/app.js", + // A prefix matches the path as sent: a router that matches the + // path as received does not take /%61ssets/x for a path under + // /assets/. + "/%61ssets/x", + // .. once percent-decoded: an app may act on these as /login, the + // last as a path under /sneak/app/ or as /assets/x. + "/assets/../login", + "/assets/%2e%2e/login", + "/assets/..%2Flogin", + "/assets/..;/login", + "/sneak/app/src/branch/main/..%2F..%2F..%2F..%2F..%2F..%2Fassets/x", + // Not under /assets/ as sent: Go's router takes /assets%2Fx for one + // path segment, not a path under /assets/. + "/assets%2Fx", + "/assets%2fx", + // Under /assets/ as sent, but holding an encoded slash, in either + // case, or a backslash: never exempt, whatever the prefix. + "/assets/x%2Fy", + "/assets/x%2fy", + `/assets/x\y`, + } { + t.Run(sent, func(t *testing.T) { + t.Parallel() + + s, _, _ := startWithClock(t, "", map[string]string{ + rateLimitPerMinute: "1", + rateLimitExemptPaths: "/assets/", + }) + + // Counted, the second request breaks the limit of one request + // a minute. + s.request(client, sent, http.StatusOK, requestlog.ActionForward) + s.request(client, sent, http.StatusForbidden, requestlog.ActionRateLimited) + }) + } +} diff --git a/internal/proxy/request.go b/internal/proxy/request.go index 09dfef1..fded4b7 100644 --- a/internal/proxy/request.go +++ b/internal/proxy/request.go @@ -7,7 +7,9 @@ import ( "net/http/httptrace" "net/http/httputil" "net/netip" + "net/url" "os" + "slices" "strings" "sync" "sync/atomic" @@ -192,7 +194,8 @@ func (rq *request) check(ctx context.Context) *refusal { // client either refuses is not looked up, and then the country lists; a // request any of them refuses is not counted for the rate limits. Then // come the rate limits, unless the client is in -// SWWAF_RATE_LIMIT_EXEMPT_NETS, so that every other request is counted. +// SWWAF_RATE_LIMIT_EXEMPT_NETS or the request's path is exempt under +// SWWAF_RATE_LIMIT_EXEMPT_PATHS, so that every other request is counted. // ctx is the request's own context. func (rq *request) checkClient(ctx context.Context) string { cfg := rq.h.config @@ -214,13 +217,39 @@ func (rq *request) checkClient(ctx context.Context) string { return requestlog.ActionCountryDenied } - if !isInside(rq.client, cfg.RateLimitExemptNets) && rq.limitBroken(now) { + exempt := isInside(rq.client, cfg.RateLimitExemptNets) || + pathExempt(rq.in.URL, cfg.RateLimitExemptPaths) + if !exempt && rq.limitBroken(now) { return requestlog.ActionRateLimited } return "" } +// pathExempt reports whether the rate limits leave out a request for u +// because of SWWAF_RATE_LIMIT_EXEMPT_PATHS: whether its path as sent, the +// path the app receives, not percent-decoded, starts with one of +// prefixes, so that /%61ssets/x is not under /assets/ for an app whose +// router matches the path as received. A request whose decoded path +// contains .. anywhere or a backslash, or whose path as sent holds an +// encoded slash (%2F or %2f), never is, since an app may act on it as a +// path outside every prefix: /assets/..%2Flogin as /login, or /assets%2Fx +// as one path segment, as Go's router does. +func pathExempt(u *url.URL, prefixes []string) bool { + decoded := u.Path + // EscapedPath is the path as the app receives it, not decoded. + sent := u.EscapedPath() + + if strings.Contains(decoded, "..") || strings.Contains(decoded, `\`) || + strings.Contains(strings.ToLower(sent), "%2f") { + return false + } + + return slices.ContainsFunc(prefixes, func(prefix string) bool { + return strings.HasPrefix(sent, prefix) + }) +} + // forward passes the request to the app and the app's answer back. ctx // is the request's own context. func (rq *request) forward(ctx context.Context) { diff --git a/internal/smallwebwaf/smallwebwaf_test.go b/internal/smallwebwaf/smallwebwaf_test.go index befbab9..d70f809 100644 --- a/internal/smallwebwaf/smallwebwaf_test.go +++ b/internal/smallwebwaf/smallwebwaf_test.go @@ -436,6 +436,7 @@ func wantStartingLine(t *testing.T, line map[string]any, appURL, dir string) { "SWWAF_RATE_LIMIT_PER_MINUTE": "1000", "SWWAF_RATE_LIMIT_PER_HOUR": "10000", rateLimitPerDay: "50000", + "SWWAF_RATE_LIMIT_EXEMPT_PATHS": "", "SWWAF_DENIED_COUNTRIES": "", "SWWAF_EXCLUSIVELY_ALLOWED_COUNTRIES": "", "SWWAF_BAN_RESPONSE": "403",