From 841d255cbbff228999117a3d09d29c6788c69d66 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 6 Oct 2026 10:30:15 +0000 Subject: [PATCH] Observe mode: log what would be refused, refuse nothing (closes #78) SWWAF_MODE (default enforce) takes enforce or observe. In observe mode a request that SWWAF_DENY_NETS, a ban, the country lists or a rate limit would refuse is passed to the app, and its log line names that refusal in would_action. The size and time limits and the 401 still apply. A broken limit makes no ban; bans read from bans.json are kept but refuse nothing, and Ledger.Find reads them without counting a refusal in their notes. Judgement call: in observe mode a broken limit does not reset the client's counters, since the reset comes with the ban. Judgement call: a request a ban would refuse keeps ban_expires. Model: opus-5-5 --- README.md | 85 ++++++---- internal/bans/bans.go | 69 +++++--- internal/bans/bans_test.go | 22 +++ internal/config/config.go | 18 ++ internal/config/config_test.go | 7 + internal/proxy/bans.go | 26 ++- internal/proxy/observe_test.go | 202 +++++++++++++++++++++++ internal/proxy/proxy_test.go | 1 + internal/proxy/request.go | 78 +++++---- internal/requestlog/requestlog.go | 4 + internal/smallwebwaf/smallwebwaf_test.go | 1 + 11 files changed, 425 insertions(+), 88 deletions(-) create mode 100644 internal/proxy/observe_test.go diff --git a/README.md b/README.md index 0c020a0..2a44452 100644 --- a/README.md +++ b/README.md @@ -13,17 +13,19 @@ 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 five parts of +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, 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, and -serves Prometheus metrics to a scraper that holds the metrics token. It comes as +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). @@ -118,6 +120,18 @@ in `bin/state` unless `SWWAF_STATE_DIR` is set. `SWWAF_ALLOW_NETS` too is let through. A client in `SWWAF_RATE_LIMIT_EXEMPT_NETS` is neither counted nor refused by the rate limits; the country lists and bans still apply to it. +- In `observe` mode, with `SWWAF_MODE=observe`, refuses none of the requests + that `SWWAF_DENY_NETS`, a ban, the country lists or a rate limit would refuse: + it passes them to the app, and their log lines name what `enforce` mode would + have done (see `would_action` in "Request log" below). The checks run, and + requests are counted, as in `enforce` mode, but a broken rate limit makes no + ban and does not set the client's counters back to zero, so each request over + the limit is logged as one that would be refused. The bans in `bans.json` are + kept, and refuse requests again when `smallwebwaf` next runs in `enforce` + mode, as long as they last. The timeouts and size limits still apply, since + they protect `smallwebwaf` and the app themselves, and a request for the + metrics without the token is still answered `401`. It is for trying a + configuration before enforcing it. - Answers `GET /_smallwebwaf/healthz` itself with `200` and `ok`, before any check and without asking the app, for the image's health check. - Answers `GET /_smallwebwaf/metrics` with its metrics (see "Metrics" below) for @@ -139,6 +153,9 @@ it, and the effective settings are logged at start. - `SWWAF_LISTEN_ADDR` (default `:8080`): where `smallwebwaf` listens. - `SWWAF_UPSTREAM_URL` (default `http://127.0.0.1:8081`): the app, as `http` or `https`, a host and an optional port, and nothing more. +- `SWWAF_MODE` (default `enforce`): `enforce`, or `observe` to pass on the + requests `smallwebwaf` would refuse and log what it would have done (see "What + it does so far" above). - `SWWAF_TRUSTED_PROXIES` (default `10.0.0.0/8,172.16.0.0/12,192.168.0.0/16`, the private address ranges): the netblocks whose `X-Forwarded-For` is believed. A list given replaces the default; set but empty, it trusts nothing. @@ -240,8 +257,8 @@ refused ones included: - `country` is the client's country as GeoJS places it. It is empty with neither country list set, for a client in `SWWAF_ALLOW_NETS` or `SWWAF_DENY_NETS`, for a client on a private, loopback or link-local address, when GeoJS cannot place - the client or has not answered in time, and for a request refused because a - ban covers its client, even when the client's country is known. + the client or has not answered in time, and for a request whose client a ban + covers, even when the client's country is known. - `status` is what the client was sent, `0` if nothing was; `upstream_status` is what the app answered, and is left out when the app did not answer. - `request_bytes` and `response_bytes` count body bytes. @@ -253,11 +270,18 @@ refused ones included: `timed_out` for one that ran out of time, `upstream_error` when the app could not be reached or its answer broke off, and `admin` for one `smallwebwaf` answered at its own endpoint. +- `would_action` is there in `observe` mode for a request that + `SWWAF_DENY_NETS`, a ban, the country lists or a rate limit would have refused + in `enforce` mode, and names the action that refusal would have had: `denied`, + `banned`, `country_denied` or `rate_limited`. `action` then names what was + done: `forward` for a request passed to the app, and another action, such as + `too_large`, for one a size or time limit refused. - `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`. - `ban_expires` is there for a request that made a ban or was refused under one, - and gives when the ban ends, in the same form as `time`, or `permanent`. + or in `observe` mode would have been refused under one, and gives when the ban + ends, in the same form as `time`, or `permanent`. - `aborted` is there, and true, when the client went away early. - `duration_total` and `duration_upstream_total` are in milliseconds. @@ -581,17 +605,16 @@ the metrics, failure behaviour and the build order. So far `smallwebwaf` looks up only the country, only through GeoJS, and only while `SWWAF_DENIED_COUNTRIES` or `SWWAF_EXCLUSIVELY_ALLOWED_COUNTRIES` is set: then the address of every new visitor is sent to GeoJS, except a visitor in -`SWWAF_ALLOW_NETS` or `SWWAF_DENY_NETS` and one refused because a ban covers its -netblock, and with neither set, none is. An IPv6 visitor is asked about by the -first address of its /64. A new visitor waits at most a second for its answer, -and without one counts as coming from an unknown country until the answer -arrives. The addresses waiting are asked about together, up to 200 in one -request, one request at a time; at most 10,000 visitors wait, and one more -counts as coming from an unknown country until there is room. While GeoJS fails, -visitors with a kept answer are unaffected and new ones count as coming from an -unknown country. GeoJS is then left alone for a second, twice as long after each -further failure up to five minutes, and asked again by the next request that -needs it. +`SWWAF_ALLOW_NETS` or `SWWAF_DENY_NETS` and one whose netblock a ban covers, and +with neither set, none is. An IPv6 visitor is asked about by the first address +of its /64. A new visitor waits at most a second for its answer, and without one +counts as coming from an unknown country until the answer arrives. The addresses +waiting are asked about together, up to 200 in one request, one request at a +time; at most 10,000 visitors wait, and one more counts as coming from an +unknown country until there is room. While GeoJS fails, visitors with a kept +answer are unaffected and new ones count as coming from an unknown country. +GeoJS is then left alone for a second, twice as long after each further failure +up to five minutes, and asked again by the next request that needs it. In the full design, `smallwebwaf` looks up the AS number and country of every client, for the request log, the metrics and the ban notes, and for the country @@ -643,8 +666,10 @@ addresses are never sent to GeoJS. limits, and writes the request's log line. Its `check` method is where a request is refused before anything reaches the app: for `SWWAF_DENY_NETS`, for a ban, for the country lists, for a rate limit, which bans the client, and for - an announced body over the size limit. A request under `/_smallwebwaf/` that - `check` lets through is answered by `answerAdmin` instead of reaching the app. + an announced body over the size limit; in `observe` mode, only for the size + limit, with what it would have refused for noted in the log line. A request + under `/_smallwebwaf/` that `check` lets through is answered by `answerAdmin` + instead of reaching the app. - `internal/metrics`: the metrics, counted as the other parts tell it what happened, and served in the Prometheus text format. - `internal/bans`: the ban ledger: each netblock's bans with their notes, how @@ -710,10 +735,10 @@ so that they run in minimal containers. ## TODO -- The rest of milestone 3, from taking in an admin's edits to the state files - (https://git.eeqj.de/sneak/smallwebwaf/issues/68) up to the rest of the - request log's fields, and the rest of the design, in the order of the build - order in [`SPEC.md`](SPEC.md). +- 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). ## Documents diff --git a/internal/bans/bans.go b/internal/bans/bans.go index 817131f..68ee022 100644 --- a/internal/bans/bans.go +++ b/internal/bans/bans.go @@ -107,8 +107,8 @@ type Ledger struct { changed chan struct{} mu sync.Mutex - // netblocks holds each banned netblock's bans, oldest first. Check - // makes each netblock it finds the most recently seen. + // netblocks holds each banned netblock's bans, oldest first. Check and + // Find make each netblock they find the most recently seen. netblocks *simplelru.LRU[netip.Prefix, *[]Ban] // held is how many bans netblocks holds, at most rules.MaxBans. held int @@ -144,36 +144,36 @@ func (l *Ledger) Changed() <-chan struct{} { return l.changed } -// Check is called for each request from client, at now. It reports -// whether a ban on a netblock client is in is active, and returns that -// ban, with the request counted among those it refused. +// Check is called for a request from client, at now. It reports whether +// a ban on a netblock client is in is active, and returns that ban, with +// the request counted among those it refused. func (l *Ledger) Check(client netip.Addr, now time.Time) (Ban, bool) { l.mu.Lock() defer l.mu.Unlock() - lengths := l.v6Lengths - if client.Is4() { - lengths = l.v4Lengths + ban := l.active(client, now) + if ban == nil { + return Ban{}, false } - for _, length := range lengths { - bans, found := l.netblocks.Get(netip.PrefixFrom(client, length).Masked()) - if !found { - continue - } + ban.Notes.Requests++ + ban.Notes.Refused++ - // A ban is made only once the one before has ended, so only the - // last can be active. - last := &(*bans)[len(*bans)-1] - if last.ActiveAt(now) { - last.Notes.Requests++ - last.Notes.Refused++ + return *ban, true +} - return *last, true - } +// Find is Check without counting the request among those the ban +// refused: in observe mode a ban refuses nothing. +func (l *Ledger) Find(client netip.Addr, now time.Time) (Ban, bool) { + l.mu.Lock() + defer l.mu.Unlock() + + ban := l.active(client, now) + if ban == nil { + return Ban{}, false } - return Ban{}, false + return *ban, true } // BanForLimit bans netblock at now for a broken limit, with notes, and @@ -304,6 +304,31 @@ func (l *Ledger) Load(bans []Ban) { } } +// active returns the ban active at now on a netblock client is in, or +// nil. +func (l *Ledger) active(client netip.Addr, now time.Time) *Ban { + lengths := l.v6Lengths + if client.Is4() { + lengths = l.v4Lengths + } + + for _, length := range lengths { + bans, found := l.netblocks.Get(netip.PrefixFrom(client, length).Masked()) + if !found { + continue + } + + // A ban is made only once the one before has ended, so only the + // last can be active. + last := &(*bans)[len(*bans)-1] + if last.ActiveAt(now) { + return last + } + } + + return nil +} + // add adds ban to its netblock's bans, after the last, and makes its // netblock the most recently seen. With MaxBans held, it drops one first. func (l *Ledger) add(ban Ban) { diff --git a/internal/bans/bans_test.go b/internal/bans/bans_test.go index 897bc3b..0bf38f1 100644 --- a/internal/bans/bans_test.go +++ b/internal/bans/bans_test.go @@ -162,6 +162,28 @@ func TestCheckRefusesWhileTheBanLastsAndCountsTheRefusals(t *testing.T) { } } +func TestFindCountsNothing(t *testing.T) { + t.Parallel() + + ledger := bans.New(defaultRules()) + netblock := netip.MustParsePrefix("203.0.113.9/32") + ban := ledger.BanForLimit(netblock, midnight(), bans.Notes{Requests: 5}) + + got, banned := ledger.Find(netblock.Addr(), ban.Expires.Add(-time.Nanosecond)) + if !banned || got != ban { + t.Errorf("find during the ban gives %+v and %t, want %+v", got, banned, ban) + } + + _, banned = ledger.Find(netblock.Addr(), ban.Expires) + if banned { + t.Error("the ban did not end") + } + + if notes := ledger.Bans(netblock)[0].Notes; notes != ban.Notes { + t.Errorf("the notes are %+v, want them unchanged, %+v", notes, ban.Notes) + } +} + func TestMaxBansDropsTheEarliestBanOfTheNetblockSeenLongestAgo(t *testing.T) { t.Parallel() diff --git a/internal/config/config.go b/internal/config/config.go index 65281c1..9a63fa5 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -27,6 +27,11 @@ type Config struct { ListenAddr string // UpstreamURL is the app (SWWAF_UPSTREAM_URL). UpstreamURL *url.URL + // Observe is true in observe mode, when SWWAF_MODE is observe rather + // than enforce: a request that SWWAF_DENY_NETS, a ban, the country + // lists or a rate limit would refuse is passed to the app instead, and + // no ban is made. + Observe bool // TrustedProxies are the netblocks whose X-Forwarded-For is // believed (SWWAF_TRUSTED_PROXIES). TrustedProxies []netip.Prefix @@ -162,6 +167,7 @@ var ( 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") ) // FromEnvironment reads the settings with lookupEnv, normally @@ -172,6 +178,7 @@ func FromEnvironment(lookupEnv func(string) (string, bool)) (*Config, error) { cfg := &Config{ ListenAddr: env.address("SWWAF_LISTEN_ADDR", ":8080"), UpstreamURL: env.appURL("SWWAF_UPSTREAM_URL", "http://127.0.0.1:8081"), + Observe: env.observe("SWWAF_MODE", "enforce"), TrustedProxies: env.netblocks("SWWAF_TRUSTED_PROXIES", privateRanges), ClientRequestTimeout: env.duration("SWWAF_CLIENT_REQUEST_TIMEOUT", "60s"), ClientRequestHeaderMaxBytes: env.headerSize( @@ -274,6 +281,17 @@ func (e *environment) appURL(name, defaultValue string) *url.URL { return upstream } +// observe reads the setting that is the mode, enforce or observe, and +// reports whether it is observe. +func (e *environment) observe(name, defaultValue string) bool { + mode := e.value(name, defaultValue) + if mode != "enforce" && mode != "observe" { + e.check(name, fmt.Errorf("%q %w", mode, errNotMode)) + } + + return mode == "observe" +} + // netblocks reads a setting that is a list of netblocks. func (e *environment) netblocks(name, defaultValue string) []netip.Prefix { netblocks, err := parseNetblocks(e.value(name, defaultValue)) diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 955ff70..100229c 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -18,6 +18,7 @@ import ( const ( listenAddr = "SWWAF_LISTEN_ADDR" upstreamURL = "SWWAF_UPSTREAM_URL" + mode = "SWWAF_MODE" trustedProxies = "SWWAF_TRUSTED_PROXIES" clientRequestTimeout = "SWWAF_CLIENT_REQUEST_TIMEOUT" clientHeaderMaxBytes = "SWWAF_CLIENT_REQUEST_HEADER_MAX_BYTES" @@ -83,6 +84,7 @@ func TestDefaults(t *testing.T) { wantSettings(t, cfg, config.Config{ ListenAddr: ":8080", + Observe: false, ClientRequestTimeout: time.Minute, ClientRequestHeaderMaxBytes: 32 << 10, ClientIdleTimeout: 2 * time.Minute, @@ -126,6 +128,7 @@ func TestValuesAsSet(t *testing.T) { cfg := fromEnvironment(t, environment{ listenAddr: "127.0.0.1:9000", upstreamURL: "https://app.internal:8443/", + mode: "observe", trustedProxies: " 192.0.2.1, 10.1.2.3/8 ,2001:db8::/32", clientRequestTimeout: "90s", clientHeaderMaxBytes: "8K", @@ -158,6 +161,7 @@ func TestValuesAsSet(t *testing.T) { wantSettings(t, cfg, config.Config{ ListenAddr: "127.0.0.1:9000", + Observe: true, ClientRequestTimeout: 90 * time.Second, ClientRequestHeaderMaxBytes: 8 << 10, ClientIdleTimeout: 5 * time.Minute, @@ -307,6 +311,7 @@ func TestInvalidValueStopsTheStart(t *testing.T) { {upstreamURL, "http://127.0.0.1:8081/app"}, {upstreamURL, "http://127.0.0.1:8081/?a=1"}, {upstreamURL, "http://user:secret@127.0.0.1:8081"}, + {mode, "Observe"}, {mode, "block"}, {mode, ""}, {trustedProxies, "10.0.0.0/33"}, {trustedProxies, "traefik"}, {trustedProxies, "10.0.0.0/8,,192.168.0.0/16"}, @@ -426,6 +431,7 @@ func TestLogsEachSettingWithItsValue(t *testing.T) { want := map[string]string{ listenAddr: ":8080", upstreamURL: "http://127.0.0.1:8081", + mode: "enforce", trustedProxies: "10.0.0.0/8,172.16.0.0/12,192.168.0.0/16", clientRequestTimeout: "45s", clientHeaderMaxBytes: "32K", @@ -465,6 +471,7 @@ func wantSettings(t *testing.T, got *config.Config, want config.Config) { t.Helper() if got.ListenAddr != want.ListenAddr || + got.Observe != want.Observe || got.ClientRequestTimeout != want.ClientRequestTimeout || got.ClientRequestHeaderMaxBytes != want.ClientRequestHeaderMaxBytes || got.ClientIdleTimeout != want.ClientIdleTimeout || diff --git a/internal/proxy/bans.go b/internal/proxy/bans.go index 596f6dc..df8de16 100644 --- a/internal/proxy/bans.go +++ b/internal/proxy/bans.go @@ -14,10 +14,15 @@ func (rq *request) banResponse(action string) *refusal { return &refusal{status: rq.h.config.BanResponse, action: action} } -// banned reports whether a ban on a netblock the client is in refuses -// the request at now, and notes for the log line when that ban ends. +// banned reports whether a ban on a netblock the client is in covers the +// request at now, and notes for the log line when that ban ends. func (rq *request) banned(now time.Time) bool { - ban, banned := rq.h.ledger.Check(rq.client, now) + check := rq.h.ledger.Check + if rq.h.config.Observe { + check = rq.h.ledger.Find // in observe mode the ban refuses nothing + } + + ban, banned := check(rq.client, now) if banned { rq.line.BanExpires = banExpires(ban) } @@ -26,8 +31,9 @@ func (rq *request) banned(now time.Time) bool { } // limitBroken counts the request for the rate limits at now, and reports -// whether it takes the client over one. Such a request bans the client's -// netblock, and sets the client's counters back to zero. +// whether it takes the client over one. In enforce mode such a request +// bans the client's netblock, and sets the client's counters back to +// zero; in observe mode it does neither. func (rq *request) limitBroken(now time.Time) bool { group := clientGroup(rq.client) @@ -36,6 +42,13 @@ func (rq *request) limitBroken(now time.Time) bool { return false } + rq.line.LimitHit = hit.Window + rq.line.Offence = requestlog.OffenceLimit + + if rq.h.config.Observe { + return true + } + netblock := rq.netblock() ban := rq.h.ledger.BanForLimit(netblock, now, bans.Notes{ Country: rq.line.Country, @@ -54,9 +67,6 @@ func (rq *request) limitBroken(now time.Time) bool { Requests: rq.h.limiter.Requests(netblock) + 1, }) rq.h.limiter.Reset(group) - - rq.line.LimitHit = hit.Window - rq.line.Offence = requestlog.OffenceLimit rq.line.BanExpires = banExpires(ban) return true diff --git a/internal/proxy/observe_test.go b/internal/proxy/observe_test.go new file mode 100644 index 0000000..7a87b7b --- /dev/null +++ b/internal/proxy/observe_test.go @@ -0,0 +1,202 @@ +package proxy_test + +import ( + "bytes" + "io" + "net/http" + "net/netip" + "sync/atomic" + "testing" + "time" + + "sneak.berlin/go/smallwebwaf/internal/bans" + "sneak.berlin/go/smallwebwaf/internal/proxy" + "sneak.berlin/go/smallwebwaf/internal/requestlog" +) + +// observe is the value of SWWAF_MODE for observe mode. +const observe = "observe" + +func TestObserveModeForwardsWhatEnforceModeRefuses(t *testing.T) { + t.Parallel() + + const ( + denied = "192.0.2.50" // in SWWAF_DENY_NETS + banned = otherClient // under a ban read from bans.json + ) + + for _, tc := range []struct { + setting string // "" leaves SWWAF_MODE at its default + observe bool + }{ + {"", false}, + {"enforce", false}, + {observe, true}, + } { + t.Run(mode+"="+tc.setting, func(t *testing.T) { + t.Parallel() + + geojsURL, _ := startGeoJS(t) + env := map[string]string{ + rateLimitPerMinute: "1", + denyNets: denied, + deniedCountries: "kp", + } + + if tc.setting != "" { + env[mode] = tc.setting + } + + s, clk, server := startWithClock(t, geojsURL, env) + server.Ledger.Load([]bans.Ban{{ + Netblock: netip.MustParsePrefix(banned + "/32"), + Start: clk.Now(), + Expires: clk.Now().Add(time.Hour), + }}) + + // fromDE's first request is within the limit of one a minute, + // and its second breaks it. + s.get(fromDE, http.StatusOK, requestlog.ActionForward) + + for _, sent := range []struct{ from, refusal string }{ + {denied, requestlog.ActionDenied}, + {banned, requestlog.ActionBanned}, + {fromKP, requestlog.ActionCountryDenied}, + {fromDE, requestlog.ActionRateLimited}, + } { + if !tc.observe { + line := s.get(sent.from, http.StatusForbidden, sent.refusal) + wantWouldAction(t, line, "") + + continue + } + + // Passed to the app, which answered it. + line := s.get(sent.from, http.StatusOK, requestlog.ActionForward) + wantWouldAction(t, line, sent.refusal) + + if line.UpstreamStatus != http.StatusOK { + t.Errorf("log line has upstream_status %d, want 200", + line.UpstreamStatus) + } + } + }) + } +} + +func TestObserveModeMakesNoBanAndKeepsTheBansItHas(t *testing.T) { + t.Parallel() + + s, clk, server := startWithClock(t, "", map[string]string{ + mode: observe, + rateLimitPerMinute: "1", + }) + kept := bans.Ban{ + Netblock: netip.MustParsePrefix(otherClient + "/32"), + Start: clk.Now(), + Expires: clk.Now().Add(time.Hour), + } + server.Ledger.Load([]bans.Ban{kept}) + + // No ban sets client's counters back to zero, so each request after + // the first breaks the limit of one a minute. + s.get(client, http.StatusOK, requestlog.ActionForward) + + for range 2 { + line := s.get(client, http.StatusOK, requestlog.ActionForward) + wantWouldAction(t, line, requestlog.ActionRateLimited) + + if line.LimitHit != minute || line.Offence != requestlog.OffenceLimit || + line.BanExpires != "" { + t.Errorf("log line has limit_hit %q, offence %q and ban_expires %q, "+ + "want minute, limit and none", line.LimitHit, line.Offence, + line.BanExpires) + } + } + + // The ban read from bans.json refuses nothing, and so counts no + // refusal in its notes, but is kept. + line := s.get(otherClient, http.StatusOK, requestlog.ActionForward) + wantWouldAction(t, line, requestlog.ActionBanned) + + if line.BanExpires != requestlog.FormatTime(kept.Expires) { + t.Errorf("log line has ban_expires %q, want %s", line.BanExpires, + requestlog.FormatTime(kept.Expires)) + } + + got := server.Ledger.Snapshot() + if len(got) != 1 || got[0] != kept { + t.Errorf("bans\n%+v\nwant only\n%+v", got, kept) + } +} + +func TestObserveModeKeepsTheSizeLimitsAndTheToken(t *testing.T) { + t.Parallel() + + const denied = "192.0.2.50" // in SWWAF_DENY_NETS + + var calls atomic.Int32 + + app := startApp(t, func(w http.ResponseWriter, _ *http.Request) { + calls.Add(1) + answerWithSize(w, 2*sizeLimit, true) + }) + addr, out := startProxy(t, app.URL, map[string]string{ + mode: observe, + trustedProxies: trustLocalhost, + denyNets: denied, + requestMaxBytes: sizeLimitSetting, + responseMaxBytes: sizeLimitSetting, + metricsToken: token, + }) + + // SWWAF_DENY_NETS would refuse each request; instead a size limit or + // the missing token does. + for i, tc := range []struct { + method, path string + body io.Reader + status int + action string + }{ + { + http.MethodPost, "/upload", bytes.NewReader(make([]byte, 2*sizeLimit)), + http.StatusRequestEntityTooLarge, requestlog.ActionTooLarge, + }, + { + http.MethodGet, "/download", http.NoBody, + http.StatusBadGateway, requestlog.ActionTooLarge, + }, + { + http.MethodGet, proxy.MetricsPath, http.NoBody, + http.StatusUnauthorized, requestlog.ActionAdmin, + }, + } { + req := newRequest(t, tc.method, addr, tc.path, tc.body) + req.Header.Set(forwardedFor, denied) + wantStatus(t, do(t, req), tc.status) + + line := out.requestLines(t, i+1)[i] + wantLine(t, line, tc.status, tc.action) + wantWouldAction(t, line, requestlog.ActionDenied) + } + + // The upload was refused before it reached the app. + if calls.Load() != 1 { + t.Errorf("the app was called %d times, want once", calls.Load()) + } +} + +// wantWouldAction checks the request log line's would_action, and that a +// line that should have none has no such field. +func wantWouldAction(t *testing.T, line logLine, want string) { + t.Helper() + + got, present := line.fields["would_action"] + + switch { + case want == "" && present: + t.Errorf("log line has would_action %v, want none", got) + case want != "" && got != want: + t.Errorf("log line has would_action %v, want %s", got, want) + } +} diff --git a/internal/proxy/proxy_test.go b/internal/proxy/proxy_test.go index 31334d0..bcff4cc 100644 --- a/internal/proxy/proxy_test.go +++ b/internal/proxy/proxy_test.go @@ -50,6 +50,7 @@ const ( clientResponseTimeout = "SWWAF_CLIENT_RESPONSE_TIMEOUT" upstreamRequestTimeout = "SWWAF_UPSTREAM_REQUEST_TIMEOUT" upstreamResponseTimeout = "SWWAF_UPSTREAM_RESPONSE_TIMEOUT" + mode = "SWWAF_MODE" requestMaxBytes = "SWWAF_REQUEST_MAX_BYTES" responseMaxBytes = "SWWAF_RESPONSE_MAX_BYTES" trustedProxies = "SWWAF_TRUSTED_PROXIES" diff --git a/internal/proxy/request.go b/internal/proxy/request.go index 31fdb5c..71d571e 100644 --- a/internal/proxy/request.go +++ b/internal/proxy/request.go @@ -109,38 +109,24 @@ func (h *handler) newRequest(w http.ResponseWriter, r *http.Request) *request { // check is the one place where a request can be refused once its client // is known, before its body is read or anything reaches the app. It -// returns nil to let the request through. A client in SWWAF_ALLOW_NETS -// skips every check but the size limit. For any other client, -// SWWAF_DENY_NETS comes first, then a ban on its netblock, so that a -// 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, -// one refused for its size too. Every refusal but the size limit's is -// answered with SWWAF_BAN_RESPONSE. ctx is the request's own context. +// returns nil to let the request through. The checks of checkClient come +// first, answered with SWWAF_BAN_RESPONSE, and then the size limit, so +// that a request the rate limits count is counted even when it is +// refused for its size. In observe mode a request checkClient refuses +// goes on to the size limit like any other. ctx is the request's own +// context. func (rq *request) check(ctx context.Context) *refusal { - cfg := rq.h.config - allowed := isInside(rq.client, cfg.AllowNets) - exempt := isInside(rq.client, cfg.RateLimitExemptNets) - now := rq.h.now() + action := rq.checkClient(ctx) + if action != "" { + if !rq.h.config.Observe { + return rq.banResponse(action) + } - if !allowed && isInside(rq.client, cfg.DenyNets) { - return rq.banResponse(requestlog.ActionDenied) + // The log line names what enforce mode would have done. + rq.line.WouldAction = action } - if !allowed && rq.banned(now) { - return rq.banResponse(requestlog.ActionBanned) - } - - if !allowed && rq.countryDenied(ctx) { - return rq.banResponse(requestlog.ActionCountryDenied) - } - - if !allowed && !exempt && rq.limitBroken(now) { - return rq.banResponse(requestlog.ActionRateLimited) - } - - maxBytes := cfg.RequestMaxBytes + maxBytes := rq.h.config.RequestMaxBytes if maxBytes > 0 && rq.in.ContentLength > maxBytes { return &refusal{ status: http.StatusRequestEntityTooLarge, @@ -152,6 +138,42 @@ func (rq *request) check(ctx context.Context) *refusal { return nil } +// checkClient runs the checks on the request's client, and returns the +// action of the first that refuses the request, or "" when none does. A +// client in SWWAF_ALLOW_NETS skips them. For any other client, +// SWWAF_DENY_NETS comes first, then a ban on its netblock, so that a +// 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. +// ctx is the request's own context. +func (rq *request) checkClient(ctx context.Context) string { + cfg := rq.h.config + if isInside(rq.client, cfg.AllowNets) { + return "" + } + + now := rq.h.now() + + if isInside(rq.client, cfg.DenyNets) { + return requestlog.ActionDenied + } + + if rq.banned(now) { + return requestlog.ActionBanned + } + + if rq.countryDenied(ctx) { + return requestlog.ActionCountryDenied + } + + if !isInside(rq.client, cfg.RateLimitExemptNets) && rq.limitBroken(now) { + return requestlog.ActionRateLimited + } + + return "" +} + // 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/requestlog/requestlog.go b/internal/requestlog/requestlog.go index 638088d..f552c20 100644 --- a/internal/requestlog/requestlog.go +++ b/internal/requestlog/requestlog.go @@ -67,6 +67,10 @@ type Line struct { Referer string `json:"referer"` UserAgent string `json:"user_agent"` Action string `json:"action"` + // WouldAction is, in observe mode, the action enforce mode would have + // taken with a request it would have refused: ActionDenied, + // ActionBanned, ActionCountryDenied or ActionRateLimited. + WouldAction string `json:"would_action,omitempty"` // LimitHit is the window whose rate limit the request went over: // minute, hour or day. LimitHit string `json:"limit_hit,omitempty"` diff --git a/internal/smallwebwaf/smallwebwaf_test.go b/internal/smallwebwaf/smallwebwaf_test.go index f0ddb88..76f6a22 100644 --- a/internal/smallwebwaf/smallwebwaf_test.go +++ b/internal/smallwebwaf/smallwebwaf_test.go @@ -383,6 +383,7 @@ func wantStartingLine(t *testing.T, line map[string]any, appURL, dir string) { listenAddr: localhost + ":0", upstreamURL: appURL, stateDir: dir, + "SWWAF_MODE": "enforce", "SWWAF_STATE_WRITE_DELAY": "10s", "SWWAF_STATE_COUNTER_INTERVAL": "15m", "SWWAF_TRUSTED_PROXIES": "10.0.0.0/8,172.16.0.0/12,192.168.0.0/16", -- 2.54.0