waf: refuse request lines and header lines over 8 KiB first (closes #339)

The header limits now also refuse a request line over 8 KiB with 414
(action request_line_too_long) and a header line over 8 KiB, Host
included, with 431. Like the other header limits they are fixed, run
first and make no ban. One long query string or cookie no longer keeps
the Core Rule Set busy for a third of a second.

Model: opus-5-5

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`                      | `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_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_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`.                                                                                                                                                                                                  |

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 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;
+   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:
     - the country list;
     - a biased threshold;
     - `SWW_WAF_TAG_DENY` or `SWW_WAF_TAG_LIMIT_PERCENT` naming an `asn=` or
-      `country=` tag.
+      `country=` tag;
+    - `SWW_WAF_ADD_LOOKUP_HEADERS`.
 
-    Otherwise the request goes on at once. A request that ends before the answer
-    comes is logged without it.
+    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"). 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.
+    "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.
 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. 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.
+  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.
 - `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:

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` set to `true`;
+    - `SWW_WAF_ADD_LOOKUP_HEADERS`;
     - `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,11 +161,9 @@ 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 `true`): `true` has `rproxy` pass the
+- `SWW_WAF_ADD_LOOKUP_HEADERS` (default `false`): `true` has `rproxy` pass the
   app the client's AS number in `X-Client-ASN` and its country in
-  `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`.
+  `X-Client-Country`.
 
 ## Country list
 

internal/config/waf.go

@@ -84,8 +84,7 @@ 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), by default unless
-	// LookupSource is off.
+	// and country in headers (SWW_WAF_ADD_LOOKUP_HEADERS).
 	LookupSource               string
 	LookupDBPath               string
 	ASNAndCountryLookupTimeout time.Duration
@@ -550,6 +549,7 @@ 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,9 +724,6 @@ 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(

internal/config/waf_test.go

@@ -287,7 +287,6 @@ 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,
@@ -1396,13 +1395,13 @@ func TestLookupSettingsAsSet(t *testing.T) {
 	t.Parallel()
 
 	cfg := wafFromEnvironment(t, environment{
-		asnAndCountryLookupTimeout: "500", addLookupHeaders: disabled,
+		asnAndCountryLookupTimeout: "500", addLookupHeaders: enabled,
 	})
 	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,
@@ -1423,10 +1422,6 @@ 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
@@ -2932,7 +2927,7 @@ func TestLogsEachSettingWithItsValue(t *testing.T) {
 		lookupSource:                   defaultLookupSource,
 		lookupDBPath:                   "",
 		asnAndCountryLookupTimeout:     "800",
-		addLookupHeaders:               enabled,
+		addLookupHeaders:               disabled,
 		allowedCountries:               "",
 		tagLists:                       "",
 		tagDeny:                        "",

internal/requestlog/requestlog.go

@@ -21,8 +21,8 @@ const (
 	// the app.
 	ActionForward = "forward"
 
-	// ActionRequestLineTooLong is a request whose request line is too long for
-	// the header limits.
+	// ActionRequestLineTooLong is a request whose request line is over the
+	// header limits' longest line.
 	ActionRequestLineTooLong = "request_line_too_long"
 
 	// ActionRequestHeaderTooLarge is a request whose headers are over the

internal/waf/anomaly_test.go

@@ -349,7 +349,6 @@ func TestRequestCountsForTheASNumberGeoJSGivesBeforeItEnds(t *testing.T) {
 		map[string]string{
 			trustedProxies:                 trustLocalhost,
 			asnAndCountryLookupTimeout:     "3600000",
-			addLookupHeaders:               falseValue,
 			anomalyASN + requestsPerMinute: aLot,
 		})
 	server.Store(started)

internal/waf/answertime_test.go

@@ -204,10 +204,6 @@ func TestAHeaderLineOverTheLimitIsRefusedWith431(t *testing.T) {
 		{"a Host line over the limit", func(req *http.Request) {
 			req.Host = strings.Repeat("a", lineLimit+1-len("Host: "))
 		}, true},
-		{"a header sent twice, each line within the limit", func(req *http.Request) {
-			req.Header.Add("X-Long", strings.Repeat("a", 8000))
-			req.Header.Add("X-Long", strings.Repeat("a", 8000))
-		}, false},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()

internal/waf/biased_test.go

@@ -387,16 +387,14 @@ func TestRequestWaitsForItsLookupWhileABiasedThresholdIsSet(t *testing.T) {
 
 			// In a synctest bubble, as
 			// TestRequestWaitsAsLongAsTheASNAndCountryLookupTimeoutSays says,
-			// with a GeoJS that never answers. The headers, which need the
-			// answer too, are off.
+			// with a GeoJS that never answers.
 			synctest.Test(t, func(t *testing.T) {
 				server, out, _ := newWAF(t, silentGeoJSURL, time.Now,
 					map[string]string{
 						asnAndCountryLookupTimeout: strconv.FormatInt(
 							timeout.Milliseconds(), 10),
-						addLookupHeaders: falseValue,
-						tagLists:         "vpn=" + vpnList,
-						tc.setting:       tc.value,
+						tagLists:   "vpn=" + vpnList,
+						tc.setting: tc.value,
 					})
 				began := time.Now()
 

internal/waf/countries_test.go

@@ -108,11 +108,8 @@ 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", addLookupHeaders: falseValue,
-		allowedCountries: "de",
+		asnAndCountryLookupTimeout: "3600000", allowedCountries: "de",
 	})
 
 	line := s.get(
@@ -242,10 +239,7 @@ func TestPrivateAddressIsNeverLookedUp(t *testing.T) {
 		name string
 		env  map[string]string
 	}{
-		{
-			"no setting needs the lookup",
-			map[string]string{addLookupHeaders: falseValue},
-		},
+		{"no setting needs the lookup", nil},
 		{"a tag setting needs it", map[string]string{tagDeny: denyKP}},
 	} {
 		t.Run(tc.name, func(t *testing.T) {

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 true. Go writes
+// and country, for the app, while SWW_WAF_ADD_LOOKUP_HEADERS is set. 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 (

internal/waf/lookup_test.go

@@ -36,7 +36,6 @@ func TestEveryClientIsLookedUpWithoutWaitingWhileNoSettingNeedsIt(
 	geojsURL, asked, release := startHeldGeoJS(t)
 	s, _, server := startWithClock(t, geojsURL, map[string]string{
 		asnAndCountryLookupTimeout: "3600000",
-		addLookupHeaders:           falseValue,
 		rateLimitPerMinute:         "1",
 	})
 
@@ -419,7 +418,7 @@ func TestClientsOwnLookupHeadersAreRemovedWhileTheSettingIsOff(
 		geojsURL,
 		time.Now,
 		map[string]string{
-			trustedProxies: trustLocalhost, addLookupHeaders: falseValue,
+			trustedProxies: trustLocalhost,
 		},
 	)
 	s := &sender{t: t, addr: addr, out: out}
@@ -458,8 +457,6 @@ 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",
 			})
 

internal/waf/proxy_test.go

@@ -93,10 +93,6 @@ 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

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 true, the client's AS number and country.
+// SWW_WAF_ADD_LOOKUP_HEADERS is set, the client's AS number and country.
 func (rq *request) allow() {
 	rq.allowed = true
 

internal/waf/waf_test.go

@@ -869,7 +869,7 @@ func TestAllowCarriesTheLookupHeadersWhileTheSettingIsSet(t *testing.T) {
 		wantASN, wantCountry string
 	}{
 		{on, "AS64496", "DE"},
-		{falseValue, "", ""},
+		{"false", "", ""},
 	} {
 		t.Run(tc.setting, func(t *testing.T) {
 			t.Parallel()