From 3550956cbbd1b193be3703dd88e0c727c1cd4682 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Fri, 13 Nov 2026 21:27:07 +0100 Subject: [PATCH] rproxy passes the app the client's AS number and country by default (closes #379) SWW_WAF_ADD_LOOKUP_HEADERS now defaults to true, so the app gets X-Client-ASN and X-Client-Country with no setting. While SWW_WAF_LOOKUP_SOURCE is off it defaults to false, so turning lookups off alone still starts; true set with the lookup off still stops the start. With GeoJS, a new client's first request now waits for its answer, up to SWW_WAF_ASN_AND_COUNTRY_LOOKUP_TIMEOUT_MS. Model: opus-5-5 --- README.md | 2 +- SPEC.md | 26 +++++++++++++------------- docs/settings.md | 8 +++++--- internal/config/waf.go | 7 +++++-- internal/config/waf_test.go | 11 ++++++++--- internal/waf/anomaly_test.go | 1 + internal/waf/biased_test.go | 8 +++++--- internal/waf/countries_test.go | 10 ++++++++-- internal/waf/lookup.go | 2 +- internal/waf/lookup_test.go | 5 ++++- internal/waf/proxy_test.go | 4 ++++ internal/waf/waf.go | 2 +- internal/waf/waf_test.go | 2 +- 13 files changed, 57 insertions(+), 31 deletions(-) diff --git a/README.md b/README.md index 24544b2..3417479 100644 --- a/README.md +++ b/README.md @@ -260,7 +260,7 @@ Every setting the code reads, with its default: | `SWW_WAF_LOOKUP_SOURCE` | `geojs` | Where each client's AS number and country come from: `geojs` (a web service), `file` (a local database) or `off`. | | `SWW_WAF_LOOKUP_DB_PATH` | empty | The IPinfo Lite database file, in its `.mmdb` form, read while the source is `file`. | | `SWW_WAF_ASN_AND_COUNTRY_LOOKUP_TIMEOUT_MS` | `800` | How long a request waits for its client's first GeoJS answer when a setting needs it, and how long a request to GeoJS may take, in milliseconds. | -| `SWW_WAF_ADD_LOOKUP_HEADERS` | `false` | `true` passes the app the client's AS number and country in `X-Client-ASN` and `X-Client-Country`. | +| `SWW_WAF_ADD_LOOKUP_HEADERS` | `true`, or `false` while the lookup source is `off` | `true` passes the app the client's AS number and country in `X-Client-ASN` and `X-Client-Country`. | | `SWW_WAF_EXCLUSIVELY_ALLOWED_COUNTRIES` | empty | When set, the only countries whose clients get through; a client the lookup cannot place is refused too. | | `SWW_WAF_TAG_LISTS` | empty | Address lists that each add a tag to their clients' requests, as `name=URL` pairs, such as `torexit=https://example.com/tor.txt`. | | `SWW_WAF_TAG_DENY` | empty | Tags whose requests are refused, such as `torexit,country=CN`. | diff --git a/SPEC.md b/SPEC.md index f25cb5e..2e6f880 100644 --- a/SPEC.md +++ b/SPEC.md @@ -161,16 +161,16 @@ field gives that name. `refusal_limit` and `byte_limits` give theirs in (`deny_nets`); a client in both lists is let through. 5. Bans. A client under a ban is refused (`active_ban`; see "Bans"). 6. The lookup, unless `SWW_WAF_LOOKUP_SOURCE` is `off` (see "Lookup of AS number - and country"). The request goes on at once. It waits for the first GeoJS - answer, up to `SWW_WAF_ASN_AND_COUNTRY_LOOKUP_TIMEOUT_MS`, only when one of - these needs it: + and country"). The request waits for the first GeoJS answer, up to + `SWW_WAF_ASN_AND_COUNTRY_LOOKUP_TIMEOUT_MS`, when one of these needs it: + - `SWW_WAF_ADD_LOOKUP_HEADERS`, which is `true` by default; - the country list; - a biased threshold; - `SWW_WAF_TAG_DENY` or `SWW_WAF_TAG_LIMIT_PERCENT` naming an `asn=` or - `country=` tag; - - `SWW_WAF_ADD_LOOKUP_HEADERS`. + `country=` tag. - A request that ends before the answer comes is logged without it. + Otherwise the request goes on at once. A request that ends before the answer + comes is logged without it. 7. The country list (`countries`; see "Country list"). 8. The reputation sources (see "Reputation"), in this order: @@ -197,10 +197,10 @@ field gives that name. `refusal_limit` and `byte_limits` give theirs in 16. A path under `/_sww/` is answered `404`, so that the app never sees that prefix. 17. `rproxy` passes the request to `SWW_RPROXY_UPSTREAM_URL`, streaming (see - "Passing a request to the app"). With `SWW_WAF_ADD_LOOKUP_HEADERS`, - `X-Client-ASN` and `X-Client-Country` are added for the app's own logs; a - header whose value is unknown is left out. Whatever that setting says, any - such header the client sent is removed. + "Passing a request to the app"). While `SWW_WAF_ADD_LOOKUP_HEADERS` is + `true`, as by default, `X-Client-ASN` and `X-Client-Country` are added for + the app's own logs; a header whose value is unknown is left out. Whatever + that setting says, any such header the client sent is removed. 18. After the visitor's answer has ended, `rproxy` sends the report (see "The report"). `waf` then counts: - For a request it allowed, the body bytes of the response, of the request @@ -2373,9 +2373,9 @@ runs no check, writes no request log line and counts nothing in the metrics. answer but the last below carries `X-Sww-Disposition`. - `X-Sww-Disposition: allow`, status `200`, no body. `rproxy` passes the request - to the app. With `SWW_WAF_ADD_LOOKUP_HEADERS` the answer also carries - `X-Client-ASN` and `X-Client-Country`, which `rproxy` adds to the request it - passes on. A header whose value is unknown is left out. + to the app. While `SWW_WAF_ADD_LOOKUP_HEADERS` is `true`, the answer also + carries `X-Client-ASN` and `X-Client-Country`, which `rproxy` adds to the + request it passes on. A header whose value is unknown is left out. - `X-Sww-Disposition: deny`, with the status, headers and body the visitor is to get. `rproxy` gives them to the visitor, less this header. The status is one of: diff --git a/docs/settings.md b/docs/settings.md index 765c1bf..f11b68d 100644 --- a/docs/settings.md +++ b/docs/settings.md @@ -150,7 +150,7 @@ cut off; the client is banned. - `SWW_WAF_LOOKUP_SOURCE` (default `geojs`): `geojs`, `file` or `off`. `off` stops the start while a setting that needs a lookup is set: - `SWW_WAF_EXCLUSIVELY_ALLOWED_COUNTRIES`; - - `SWW_WAF_ADD_LOOKUP_HEADERS`; + - `SWW_WAF_ADD_LOOKUP_HEADERS` set to `true`; - `SWW_WAF_TAG_DENY` or `SWW_WAF_TAG_LIMIT_PERCENT` naming a `country=` tag, or an `asn=` tag while `SWW_WAF_ROUTEWATCH_URL` is unset; - the two byte percentage lists and `SWW_WAF_ASN_LIMIT_PERCENT_URL`; @@ -161,9 +161,11 @@ cut off; the client is banned. - `SWW_WAF_ASN_AND_COUNTRY_LOOKUP_TIMEOUT_MS` (default `800`): how long a request waits for its client's first answer from GeoJS, and how long a request to GeoJS may take. A time in milliseconds; never `off`. -- `SWW_WAF_ADD_LOOKUP_HEADERS` (default `false`): `true` has `rproxy` pass the +- `SWW_WAF_ADD_LOOKUP_HEADERS` (default `true`): `true` has `rproxy` pass the app the client's AS number in `X-Client-ASN` and its country in - `X-Client-Country`. + `X-Client-Country`. The default is `false` while `SWW_WAF_LOOKUP_SOURCE` is + `off`. With GeoJS, a new client's first request waits for its answer, up to + `SWW_WAF_ASN_AND_COUNTRY_LOOKUP_TIMEOUT_MS`. ## Country list diff --git a/internal/config/waf.go b/internal/config/waf.go index 29e522c..cc166e3 100644 --- a/internal/config/waf.go +++ b/internal/config/waf.go @@ -84,7 +84,8 @@ type WAF struct { // client's first answer from GeoJS while a setting needs it // (SWW_WAF_ASN_AND_COUNTRY_LOOKUP_TIMEOUT_MS), which cannot be off. // AddLookupHeaders is true when the app is passed the client's AS number - // and country in headers (SWW_WAF_ADD_LOOKUP_HEADERS). + // and country in headers (SWW_WAF_ADD_LOOKUP_HEADERS), by default unless + // LookupSource is off. LookupSource string LookupDBPath string ASNAndCountryLookupTimeout time.Duration @@ -549,7 +550,6 @@ func WAFFromEnvironment(v *viper.Viper) (*WAF, error) { "SWW_WAF_ASN_AND_COUNTRY_LOOKUP_TIMEOUT_MS", "800", ), - AddLookupHeaders: env.boolean("SWW_WAF_ADD_LOOKUP_HEADERS", "false"), ExclusivelyAllowedCountries: env.countries( "SWW_WAF_EXCLUSIVELY_ALLOWED_COUNTRIES", ""), TagLists: env.tagLists("SWW_WAF_TAG_LISTS"), @@ -724,6 +724,9 @@ func WAFFromEnvironment(v *viper.Viper) (*WAF, error) { cfg.LogRemoteAppName = env.appName("SWW_WAF_LOG_REMOTE_APP_NAME", cfg.InstanceName, cfg.LogRemoteURL != nil) + // The app gets the headers by default, unless no client is looked up. + cfg.AddLookupHeaders = env.boolean("SWW_WAF_ADD_LOOKUP_HEADERS", + strconv.FormatBool(cfg.LookupSource != off)) cfg.BlocklistAction, cfg.BlocklistLimitPercent = env.action( "SWW_WAF_BLOCKLIST_ACTION", "deny") cfg.DNSBLAbuseIPDBAction, cfg.DNSBLAbuseIPDBLimitPercent = env.action( diff --git a/internal/config/waf_test.go b/internal/config/waf_test.go index 31dda14..c90c0f2 100644 --- a/internal/config/waf_test.go +++ b/internal/config/waf_test.go @@ -287,6 +287,7 @@ func TestDefaults(t *testing.T) { }) wantLookupSettings(t, cfg, config.WAF{ LookupSource: defaultLookupSource, ASNAndCountryLookupTimeout: 800 * time.Millisecond, + AddLookupHeaders: true, }) wantByteLimitSettings(t, cfg, config.WAF{ BytesLimitPerMinute: 0, BytesLimitPerHour: 50 << 30, @@ -1395,13 +1396,13 @@ func TestLookupSettingsAsSet(t *testing.T) { t.Parallel() cfg := wafFromEnvironment(t, environment{ - asnAndCountryLookupTimeout: "500", addLookupHeaders: enabled, + asnAndCountryLookupTimeout: "500", addLookupHeaders: disabled, }) wantLookupSettings(t, cfg, config.WAF{ LookupSource: defaultLookupSource, ASNAndCountryLookupTimeout: 500 * time.Millisecond, - AddLookupHeaders: true, }) + // With no client looked up, the headers are off unless set. cfg = wafFromEnvironment(t, environment{lookupSource: off}) wantLookupSettings(t, cfg, config.WAF{ LookupSource: off, ASNAndCountryLookupTimeout: 800 * time.Millisecond, @@ -1422,6 +1423,10 @@ func TestLookupDBPathGoesWithTheFileSourceAlone(t *testing.T) { cfg.LookupSource, cfg.LookupDBPath, path) } + if !cfg.AddLookupHeaders { + t.Error("headers off by default with lookups from file, want on") + } + for _, tc := range []struct { env environment want string @@ -2927,7 +2932,7 @@ func TestLogsEachSettingWithItsValue(t *testing.T) { lookupSource: defaultLookupSource, lookupDBPath: "", asnAndCountryLookupTimeout: "800", - addLookupHeaders: disabled, + addLookupHeaders: enabled, allowedCountries: "", tagLists: "", tagDeny: "", diff --git a/internal/waf/anomaly_test.go b/internal/waf/anomaly_test.go index 6579f30..a3e3356 100644 --- a/internal/waf/anomaly_test.go +++ b/internal/waf/anomaly_test.go @@ -349,6 +349,7 @@ func TestRequestCountsForTheASNumberGeoJSGivesBeforeItEnds(t *testing.T) { map[string]string{ trustedProxies: trustLocalhost, asnAndCountryLookupTimeout: "3600000", + addLookupHeaders: falseValue, anomalyASN + requestsPerMinute: aLot, }) server.Store(started) diff --git a/internal/waf/biased_test.go b/internal/waf/biased_test.go index 8bc2e00..17dadf7 100644 --- a/internal/waf/biased_test.go +++ b/internal/waf/biased_test.go @@ -387,14 +387,16 @@ func TestRequestWaitsForItsLookupWhileABiasedThresholdIsSet(t *testing.T) { // In a synctest bubble, as // TestRequestWaitsAsLongAsTheASNAndCountryLookupTimeoutSays says, - // with a GeoJS that never answers. + // with a GeoJS that never answers. The headers, which need the + // answer too, are off. synctest.Test(t, func(t *testing.T) { server, out, _ := newWAF(t, silentGeoJSURL, time.Now, map[string]string{ asnAndCountryLookupTimeout: strconv.FormatInt( timeout.Milliseconds(), 10), - tagLists: "vpn=" + vpnList, - tc.setting: tc.value, + addLookupHeaders: falseValue, + tagLists: "vpn=" + vpnList, + tc.setting: tc.value, }) began := time.Now() diff --git a/internal/waf/countries_test.go b/internal/waf/countries_test.go index 4212f59..ababca8 100644 --- a/internal/waf/countries_test.go +++ b/internal/waf/countries_test.go @@ -108,8 +108,11 @@ func TestCountryRefusalNamesItsHeuristicCountryAndSetting(t *testing.T) { t.Parallel() geojsURL, _ := startGeoJS(t) + // The headers are off, so that only the country list makes the request + // wait for its answer. s, _, _ := startWithClock(t, geojsURL, map[string]string{ - asnAndCountryLookupTimeout: "3600000", allowedCountries: "de", + asnAndCountryLookupTimeout: "3600000", addLookupHeaders: falseValue, + allowedCountries: "de", }) line := s.get( @@ -239,7 +242,10 @@ func TestPrivateAddressIsNeverLookedUp(t *testing.T) { name string env map[string]string }{ - {"no setting needs the lookup", nil}, + { + "no setting needs the lookup", + map[string]string{addLookupHeaders: falseValue}, + }, {"a tag setting needs it", map[string]string{tagDeny: denyKP}}, } { t.Run(tc.name, func(t *testing.T) { diff --git a/internal/waf/lookup.go b/internal/waf/lookup.go index d8fbeef..8627cd2 100644 --- a/internal/waf/lookup.go +++ b/internal/waf/lookup.go @@ -12,7 +12,7 @@ import ( ) // The headers in which an answer of allow gives rproxy the client's AS number -// and country, for the app, while SWW_WAF_ADD_LOOKUP_HEADERS is set. Go writes +// and country, for the app, while SWW_WAF_ADD_LOOKUP_HEADERS is true. Go writes // every header name in this form, as it sends it and as it receives it, so // X-Client-ASN arrives as X-Client-Asn; header names are not case-sensitive. const ( diff --git a/internal/waf/lookup_test.go b/internal/waf/lookup_test.go index 92d9212..407c87c 100644 --- a/internal/waf/lookup_test.go +++ b/internal/waf/lookup_test.go @@ -36,6 +36,7 @@ func TestEveryClientIsLookedUpWithoutWaitingWhileNoSettingNeedsIt( geojsURL, asked, release := startHeldGeoJS(t) s, _, server := startWithClock(t, geojsURL, map[string]string{ asnAndCountryLookupTimeout: "3600000", + addLookupHeaders: falseValue, rateLimitPerMinute: "1", }) @@ -418,7 +419,7 @@ func TestClientsOwnLookupHeadersAreRemovedWhileTheSettingIsOff( geojsURL, time.Now, map[string]string{ - trustedProxies: trustLocalhost, + trustedProxies: trustLocalhost, addLookupHeaders: falseValue, }, ) s := &sender{t: t, addr: addr, out: out} @@ -457,6 +458,8 @@ func TestRequestWaitsAsLongAsTheASNAndCountryLookupTimeoutSays( map[string]string{ asnAndCountryLookupTimeout: strconv.FormatInt( timeout.Milliseconds(), 10), + // Off, so that only the country list makes the request wait. + addLookupHeaders: falseValue, allowedCountries: "DE", }) diff --git a/internal/waf/proxy_test.go b/internal/waf/proxy_test.go index 0c91a91..e32b864 100644 --- a/internal/waf/proxy_test.go +++ b/internal/waf/proxy_test.go @@ -93,6 +93,10 @@ const off = "off" // on is the value that switches a setting that takes true or false on. const on = "true" +// falseValue is the value that switches a setting that takes true or false +// off. +const falseValue = "false" + // output collects what sww writes on stdout. type output struct { mu sync.Mutex diff --git a/internal/waf/waf.go b/internal/waf/waf.go index ad12c76..ec8d9ea 100644 --- a/internal/waf/waf.go +++ b/internal/waf/waf.go @@ -320,7 +320,7 @@ func asTheVisitorSentIt(r *http.Request, body []byte) *http.Request { } // allow answers a question with DispositionAllow and, while -// SWW_WAF_ADD_LOOKUP_HEADERS is set, the client's AS number and country. +// SWW_WAF_ADD_LOOKUP_HEADERS is true, the client's AS number and country. func (rq *request) allow() { rq.allowed = true diff --git a/internal/waf/waf_test.go b/internal/waf/waf_test.go index 8d9c441..2ab50a9 100644 --- a/internal/waf/waf_test.go +++ b/internal/waf/waf_test.go @@ -869,7 +869,7 @@ func TestAllowCarriesTheLookupHeadersWhileTheSettingIsSet(t *testing.T) { wantASN, wantCountry string }{ {on, "AS64496", "DE"}, - {"false", "", ""}, + {falseValue, "", ""}, } { t.Run(tc.setting, func(t *testing.T) { t.Parallel()