From ae5445f70f8a66b17fc1e9395039eef23b1ec224 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 6 Oct 2026 10:06:38 +0000 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, percent-decoded, starts with one of the comma-separated prefixes in SWWAF_RATE_LIMIT_EXEMPT_PATHS. 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. 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 | 66 ++++++++++++-------- internal/config/config.go | 36 ++++++++++- internal/config/config_test.go | 26 ++++++++ internal/proxy/proxy_test.go | 1 + internal/proxy/ratelimits_test.go | 78 ++++++++++++++++++++++++ internal/proxy/request.go | 32 +++++++++- internal/smallwebwaf/smallwebwaf_test.go | 1 + 7 files changed, 211 insertions(+), 29 deletions(-) diff --git a/README.md b/README.md index 2a44452..32241b2 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 six parts of -milestone 3: the static lists, the bans that broken rate limits lead to and the -JSON state files, which come next in the build order, `observe` mode, which -comes 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, 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). +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, the +JSON state files and the paths the rate limits do not count, which come next in +the build order, `observe` mode, which comes 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, 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). ## Getting started @@ -79,12 +80,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 @@ -191,6 +195,18 @@ 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, percent-decoded and before any query string, starts with + a prefix, character for character. `/assets/` matches `/assets/app.js` and + `/assets/`, but not `/assets`, `/Assets/app.js`, `/static/assets/app.js`, + `/static/../assets/app.js` or `/assets%2Fapp.js`. A prefix is written without + percent-encoding, 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 @@ -736,9 +752,9 @@ so that they run in minimal containers. ## TODO - The rest of milestone 3: taking in an admin's edits to the state files - (https://git.eeqj.de/sneak/smallwebwaf/issues/68), exemptions and the rest of - the request log's fields; then the rest of the design, in the order of the - build order in [`SPEC.md`](SPEC.md). + (https://git.eeqj.de/sneak/smallwebwaf/issues/68) and the rest of the request + log's fields; then 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 9a63fa5..a7265e8 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -75,6 +75,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 @@ -166,8 +170,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 @@ -195,6 +201,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", ""), @@ -333,6 +340,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)) @@ -611,6 +626,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 100229c..e3969fb 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -34,6 +34,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" @@ -120,6 +121,10 @@ func TestDefaults(t *testing.T) { wantNetblocks(t, cfg.DenyNets) wantCountries(t, deniedCountries, cfg.DeniedCountries) wantCountries(t, allowedCountries, cfg.ExclusivelyAllowedCountries) + + if len(cfg.RateLimitExemptPaths) != 0 { + t.Errorf("%s gave %v, want none", rateLimitExemptPaths, cfg.RateLimitExemptPaths) + } } func TestValuesAsSet(t *testing.T) { @@ -144,6 +149,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", @@ -196,6 +202,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 TestCodeOnBothCountryListsStopsTheStart(t *testing.T) { @@ -339,6 +363,7 @@ func TestInvalidValueStopsTheStart(t *testing.T) { {rateLimitPerHour, "1.5"}, {rateLimitPerDay, "-1"}, {rateLimitPerDay, "lots"}, + {rateLimitExemptPaths, "/assets/,,/static/"}, {deniedCountries, "nk"}, {deniedCountries, "kp,,ir"}, {deniedCountries, "prk"}, @@ -447,6 +472,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 bcff4cc..71eaaac 100644 --- a/internal/proxy/proxy_test.go +++ b/internal/proxy/proxy_test.go @@ -59,6 +59,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..6cd484a 100644 --- a/internal/proxy/ratelimits_test.go +++ b/internal/proxy/ratelimits_test.go @@ -5,6 +5,7 @@ import ( "sync/atomic" "testing" + "sneak.berlin/go/smallwebwaf/internal/lookup" "sneak.berlin/go/smallwebwaf/internal/requestlog" ) @@ -69,3 +70,80 @@ 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 != "" { + t.Errorf("log line has limit_hit %q, want none", line.LimitHit) + } + + // 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", + // .. 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", + // An encoded slash or a backslash: Go's router takes /assets%2Fx + // for one path segment, not a path under /assets/. + "/assets%2Fx", + "/assets%2fx", + `/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 71d571e..4c98906 100644 --- a/internal/proxy/request.go +++ b/internal/proxy/request.go @@ -7,7 +7,10 @@ import ( "net/http/httptrace" "net/http/httputil" "net/netip" + "net/url" "os" + "slices" + "strings" "sync" "sync/atomic" "time" @@ -145,7 +148,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 @@ -167,13 +171,37 @@ 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, +// percent-decoded, starts with one of prefixes. 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 := strings.ToLower(u.EscapedPath()) + + if strings.Contains(decoded, "..") || strings.Contains(decoded, `\`) || + strings.Contains(sent, "%2f") { + return false + } + + return slices.ContainsFunc(prefixes, func(prefix string) bool { + return strings.HasPrefix(decoded, 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 76f6a22..87eae27 100644 --- a/internal/smallwebwaf/smallwebwaf_test.go +++ b/internal/smallwebwaf/smallwebwaf_test.go @@ -401,6 +401,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",