Ban the netblock of a client that breaks a rate limit, in memory (closes #18)

A request over a rate limit is refused with SWWAF_BAN_RESPONSE and bans
the client's netblock: an hour at first, three times the last ban when
broken again within a day of its end, permanent past seven days. The
ban ledger in internal/bans is checked after the static lists and
before the lookup, and the requests it refuses are not counted. A ban
resets the client's counters and carries notes holding the request
that broke the limit, as SPEC.md now says. At most SWWAF_MAX_BANS are
held. SWWAF_BAN_RESPONSE also answers SWWAF_DENY_NETS and the country
lists.

Judgement call: the six ban settings cannot be off.
Judgement call: a permanent ban's ban_expires is "permanent".

Model: opus-5-5

README.md

@@ -13,18 +13,16 @@ 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 three parts of
+https://git.eeqj.de/sneak/smallwebwaf/issues/14), and so are the static lists
-milestone 3: the static lists and the bans that broken rate limits lead to,
+and the bans that broken rate limits lead to, which come next in the build
-which come next in the build order, and the header size and the idle time as
+order. `smallwebwaf` passes each request to the app and the app's answer back,
-settings, which come last in it. `smallwebwaf` passes each request to the app
+unchanged, within its timeouts and size limits, works out each client's address,
-and the app's answer back, unchanged, within its timeouts and size limits, works
+bans a client that sends too many requests, refuses a client that comes from a
-out each client's address, bans a client that sends too many requests, refuses a
+country you refuse or from a network you refuse, lets the networks you choose
-client that comes from a country you refuse or from a network you refuse, lets
+through, and writes a JSON log line for every request. It comes as the image the
-the networks you choose through, and writes a JSON log line for every request.
+app's own image is built on. The rest of the design comes after that, in the
-It comes as the image the app's own image is built on. The rest of the design
+order of the build order in [`SPEC.md`](SPEC.md). The survey of existing tools
-comes after that, in the order of the build order in [`SPEC.md`](SPEC.md). The
+that led to the design is in [`EVALUATION.md`](EVALUATION.md).
-survey of existing tools that led to the design is in
-[`EVALUATION.md`](EVALUATION.md).
 
 ## Getting started
 
@@ -61,16 +59,16 @@ and `make run` builds and runs it, listening on port 8080 in front of an app at
   is inside, the leftmost is, and with no header the peer is. The app sees what
   it would see from traefik directly: the same `Host`, the same
   `X-Forwarded-Proto`, and `X-Forwarded-For` with the peer added at the end.
-- Enforces the timeouts and the size limits below. A limit passed before the
+- Enforces the four timeouts and the two size limits below. A limit passed
-  response has started gets `smallwebwaf`'s own answer: `408` for a client too
+  before the response has started gets `smallwebwaf`'s own answer: `408` for a
-  slow to send its request, `413` for a request body that is too large, `504`
+  client too slow to send its request, `413` for a request body that is too
-  for an app too slow to answer, and `502` for a response that is too large or
+  large, `504` for an app too slow to answer, and `502` for a response that is
-  an app that cannot be reached. A request that announces a body over the limit
+  too large or an app that cannot be reached. A request that announces a body
-  is refused before anything reaches the app. While a request body is still on
+  over the limit is refused before anything reaches the app. While a request
-  its way, a request timeout that runs out answers `408` if `smallwebwaf` was
+  body is still on its way, a request timeout that runs out answers `408` if
-  waiting for the client to send more, and `504` if it was waiting for the app
+  `smallwebwaf` was waiting for the client to send more, and `504` if it was
-  to take what it had. Once the response has started, a limit can only cut the
+  waiting for the app to take what it had. Once the response has started, a
-  connection.
+  limit can only cut the connection.
 - 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
@@ -133,16 +131,6 @@ it, and the effective settings are logged at start.
 - `SWWAF_CLIENT_REQUEST_TIMEOUT` (default `60s`): how long a client may take to
   send its request line and headers, and then, from the end of the headers, its
   body.
-- `SWWAF_CLIENT_REQUEST_HEADER_MAX_BYTES` (default `32K`): the largest request
-  line and headers a client may send. Over it, the answer is `431` and nothing
-  reaches the app. It must be more than `4K`, and cannot be `off`: Go's HTTP
-  server always has such a limit, and reads 4 KiB past the one it is given
-  before it refuses.
-- `SWWAF_CLIENT_IDLE_TIMEOUT` (default `120s`): how long a kept-open connection
-  may wait for its next request before `smallwebwaf` closes it. The default is
-  longer than the 90 seconds after which traefik closes a connection it is not
-  using, so traefik never sends a request on a connection `smallwebwaf` is
-  closing.
 - `SWWAF_CLIENT_RESPONSE_TIMEOUT` (default `30m`): how long the response may
   take to reach the client, from the end of the request to the last byte.
 - `SWWAF_UPSTREAM_REQUEST_TIMEOUT` (default `60s`): how long connecting to the
@@ -192,13 +180,16 @@ and a bare address stands for itself alone. Countries are the two-letter codes
 ISO 3166-1 assigns today, and `xk` for Kosovo, in either case (`de` and `DE` are
 the same); any other code, such as `nk` (North Korea is `kp`) or the withdrawn
 `su`, stops the start, and so does a code on both country lists. `off` switches
-a timeout, a size limit or a rate limit off;
+a timeout, a size limit or a rate limit off; the ban settings cannot be off.
-`SWWAF_CLIENT_REQUEST_HEADER_MAX_BYTES` and the ban settings cannot be off.
 
-Several limits are fixed rather than settings. At most 20,000 clients are kept
+Several limits are fixed rather than settings. The request line and headers may
-for the rate limits, and an IPv6 client is counted by its /64. A new client
+take up to 32 KiB, above which the answer is `431` and nothing reaches the app.
-waits at most a second for its country, and at most 100,000 answers from GeoJS
+A kept-open connection that sends nothing for 120 seconds is closed. That is
-are kept, for 7 days each.
+longer than the 90 seconds after which traefik closes a connection it is not
+using, so traefik never sends a request on a connection `smallwebwaf` is
+closing. At most 20,000 clients are kept for the rate limits, and an IPv6 client
+is counted by its /64. A new client waits at most a second for its country, and
+at most 100,000 answers from GeoJS are kept, for 7 days each.
 
 ## Request log
 
@@ -241,10 +232,10 @@ settings, stop, errors) share the stream as JSON lines marked
 
 Go's HTTP server, on which `smallwebwaf` is built, reads a request's line and
 headers before `smallwebwaf` sees the request, and some requests end there,
-without a line in the log: headers over `SWWAF_CLIENT_REQUEST_HEADER_MAX_BYTES`,
+without a line in the log: headers over 32 KiB, which it answers `431`, headers
-which it answers `431`, headers slower than `SWWAF_CLIENT_REQUEST_TIMEOUT`,
+slower than `SWWAF_CLIENT_REQUEST_TIMEOUT`, whose connection it closes without
-whose connection it closes without an answer, and requests it cannot read at
+an answer, and requests it cannot read at all, which it answers itself, mostly
-all, which it answers itself, mostly with `400`.
+with `400`.
 
 ## Why
 
@@ -587,9 +578,9 @@ so that they run in minimal containers.
 
 ## TODO
 
-- The rest of milestone 3, after the bans that broken rate limits lead to and up
+- The rest of milestone 3, after the bans that broken rate limits lead to, and
-  to the metrics endpoint, and the rest of the design, in the order of the build
+  the rest of the design, in the order of the build order in
-  order in [`SPEC.md`](SPEC.md).
+  [`SPEC.md`](SPEC.md).
 
 ## Documents
 

SPEC.md

@@ -293,8 +293,7 @@ it.
   needs: an alert destination, an account key, a token.
 - Every setting's name starts with `SWWAF_`, since `smallwebwaf` shares its
   container, and so its environment variables, with the app it protects.
-- Any limit or threshold can be switched off with the value `off`, except
+- Any limit or threshold can be switched off with the value `off`.
-  `SWWAF_CLIENT_REQUEST_HEADER_MAX_BYTES`.
 - A list set to an empty value is an empty list, and replaces the default.
 - Every setting may instead be given as a file holding the value, named by the
   setting's name with `_FILE` added, such as `SWWAF_ADMIN_TOKEN_FILE`, for
@@ -414,9 +413,7 @@ The settings, by group:
       headers, its body.
     - `SWWAF_CLIENT_REQUEST_HEADER_MAX_BYTES` (default `32K`): the largest
       request line and headers a client may send. Over it, `smallwebwaf` answers
-      `431` and closes the connection, and nothing reaches the app. It must be
+      `431` and closes the connection, and nothing reaches the app.
-      more than `4K`, and cannot be `off`: Go's HTTP server always has such a
-      limit, and reads 4 KiB past the one it is given before it refuses.
     - `SWWAF_CLIENT_IDLE_TIMEOUT` (default `120s`): how long a kept-open
       connection may wait for its next request before `smallwebwaf` closes it.
       It is longer than the 90 seconds after which traefik, by default, closes a

internal/bans/bans_test.go

@@ -197,26 +197,6 @@ func TestMaxBansDropsTheEarliestBanOfTheNetblockSeenLongestAgo(t *testing.T) {
 	}
 }
 
-func TestFullLedgerDropsTheEarlierBanOfTheNetblockBannedAgain(t *testing.T) {
-	t.Parallel()
-
-	// With room for one ban, the netblock's ended ban goes to make room for
-	// its new one, whose notes still count it.
-	rules := defaultRules()
-	rules.MaxBans = 1
-	ledger := bans.New(rules)
-	netblock := netip.MustParsePrefix("203.0.113.9/32")
-
-	first := ledger.BanForLimit(netblock, midnight(), bans.Notes{})
-	second := ledger.BanForLimit(netblock, first.Expires, bans.Notes{})
-
-	held := ledger.Bans(netblock)
-	if len(held) != 1 || held[0] != second || held[0].Notes.EarlierBans != 1 {
-		t.Errorf("the ledger holds %+v, want only the second ban, with 1 earlier ban",
-			held)
-	}
-}
-
 func TestRequestTextsAreCutTo256Bytes(t *testing.T) {
 	t.Parallel()
 

internal/config/config.go

@@ -31,13 +31,6 @@ type Config struct {
 	// ClientRequestTimeout bounds reading the whole request from the
 	// client (SWWAF_CLIENT_REQUEST_TIMEOUT).
 	ClientRequestTimeout time.Duration
-	// ClientRequestHeaderMaxBytes is the largest request line and headers
-	// a client may send (SWWAF_CLIENT_REQUEST_HEADER_MAX_BYTES). It is
-	// never off, and always more than 4K.
-	ClientRequestHeaderMaxBytes int64
-	// ClientIdleTimeout bounds how long a kept-open client connection
-	// may wait for its next request (SWWAF_CLIENT_IDLE_TIMEOUT).
-	ClientIdleTimeout time.Duration
 	// ClientResponseTimeout bounds writing the whole response to the
 	// client (SWWAF_CLIENT_RESPONSE_TIMEOUT).
 	ClientResponseTimeout time.Duration
@@ -131,7 +124,6 @@ var (
 	errNotCountry = errors.New(
 		"is not a two-letter country code such as de or kp")
 	errOnBothLists          = errors.New("is in SWWAF_DENIED_COUNTRIES too")
-	errNotOver4K            = errors.New("is not a size of more than 4K, such as 32K")
 	errNotDurationAboveZero = errors.New(
 		"is not a duration above zero, such as 1h or 7d")
 	errNotNumberAboveZero = errors.New(
@@ -147,13 +139,10 @@ var (
 func FromEnvironment(lookupEnv func(string) (string, bool)) (*Config, error) {
 	env := &environment{lookupEnv: lookupEnv}
 	cfg := &Config{
-		ListenAddr:           env.address("SWWAF_LISTEN_ADDR", ":8080"),
+		ListenAddr:              env.address("SWWAF_LISTEN_ADDR", ":8080"),
-		UpstreamURL:          env.appURL("SWWAF_UPSTREAM_URL", "http://127.0.0.1:8081"),
+		UpstreamURL:             env.appURL("SWWAF_UPSTREAM_URL", "http://127.0.0.1:8081"),
-		TrustedProxies:       env.netblocks("SWWAF_TRUSTED_PROXIES", privateRanges),
+		TrustedProxies:          env.netblocks("SWWAF_TRUSTED_PROXIES", privateRanges),
-		ClientRequestTimeout: env.duration("SWWAF_CLIENT_REQUEST_TIMEOUT", "60s"),
+		ClientRequestTimeout:    env.duration("SWWAF_CLIENT_REQUEST_TIMEOUT", "60s"),
-		ClientRequestHeaderMaxBytes: env.headerSize(
-			"SWWAF_CLIENT_REQUEST_HEADER_MAX_BYTES", "32K"),
-		ClientIdleTimeout:       env.duration("SWWAF_CLIENT_IDLE_TIMEOUT", "120s"),
 		ClientResponseTimeout:   env.duration("SWWAF_CLIENT_RESPONSE_TIMEOUT", "30m"),
 		UpstreamRequestTimeout:  env.duration("SWWAF_UPSTREAM_REQUEST_TIMEOUT", "60s"),
 		UpstreamResponseTimeout: env.duration("SWWAF_UPSTREAM_RESPONSE_TIMEOUT", "30m"),
@@ -270,15 +259,6 @@ func (e *environment) size(name, defaultValue string) int64 {
 	return size
 }
 
-// headerSize reads the setting that is the largest request line and
-// headers.
-func (e *environment) headerSize(name, defaultValue string) int64 {
-	size, err := parseHeaderSize(e.value(name, defaultValue))
-	e.check(name, err)
-
-	return size
-}
-
 // count reads a setting that is a number of requests.
 func (e *environment) count(name, defaultValue string) int64 {
 	count, err := parseCount(e.value(name, defaultValue))
@@ -384,19 +364,6 @@ func parseSize(value string) (int64, error) {
 	return n * unit, nil
 }
 
-// parseHeaderSize reads the largest request line and headers: a size as
-// parseSize reads it, but more than 4K and never off. Go's server reads 4K
-// past the limit it is given before it refuses, so proxy.New gives it this
-// size less 4K, which must leave a limit.
-func parseHeaderSize(value string) (int64, error) {
-	size, err := parseSize(value)
-	if err != nil || size <= 4*kibibyte {
-		return 0, fmt.Errorf("%q %w", value, errNotOver4K)
-	}
-
-	return size, nil
-}
-
 // splitUnit splits a size into its number and the bytes its suffix
 // stands for.
 func splitUnit(value string) (string, int64) {

internal/config/config_test.go

@@ -20,8 +20,6 @@ const (
 	upstreamURL             = "SWWAF_UPSTREAM_URL"
 	trustedProxies          = "SWWAF_TRUSTED_PROXIES"
 	clientRequestTimeout    = "SWWAF_CLIENT_REQUEST_TIMEOUT"
-	clientHeaderMaxBytes    = "SWWAF_CLIENT_REQUEST_HEADER_MAX_BYTES"
-	clientIdleTimeout       = "SWWAF_CLIENT_IDLE_TIMEOUT"
 	clientResponseTimeout   = "SWWAF_CLIENT_RESPONSE_TIMEOUT"
 	upstreamRequestTimeout  = "SWWAF_UPSTREAM_REQUEST_TIMEOUT"
 	upstreamResponseTimeout = "SWWAF_UPSTREAM_RESPONSE_TIMEOUT"
@@ -74,24 +72,22 @@ func TestDefaults(t *testing.T) {
 	cfg := fromEnvironment(t, environment{})
 
 	wantSettings(t, cfg, config.Config{
-		ListenAddr:                  ":8080",
+		ListenAddr:              ":8080",
-		ClientRequestTimeout:        time.Minute,
+		ClientRequestTimeout:    time.Minute,
-		ClientRequestHeaderMaxBytes: 32 << 10,
+		ClientResponseTimeout:   30 * time.Minute,
-		ClientIdleTimeout:           2 * time.Minute,
+		UpstreamRequestTimeout:  time.Minute,
-		ClientResponseTimeout:       30 * time.Minute,
+		UpstreamResponseTimeout: 30 * time.Minute,
-		UpstreamRequestTimeout:      time.Minute,
+		RequestMaxBytes:         100 << 20,
-		UpstreamResponseTimeout:     30 * time.Minute,
+		ResponseMaxBytes:        5 << 30,
-		RequestMaxBytes:             100 << 20,
+		RateLimitPerMinute:      1000,
-		ResponseMaxBytes:            5 << 30,
+		RateLimitPerHour:        10000,
-		RateLimitPerMinute:          1000,
+		RateLimitPerDay:         50000,
-		RateLimitPerHour:            10000,
+		BanResponse:             403,
-		RateLimitPerDay:             50000,
+		LimitBanDuration:        time.Hour,
-		BanResponse:                 403,
+		LimitBanRepeatWindow:    24 * time.Hour,
-		LimitBanDuration:            time.Hour,
+		MaxBanDuration:          7 * 24 * time.Hour,
-		LimitBanRepeatWindow:        24 * time.Hour,
+		MaxBans:                 5000,
-		MaxBanDuration:              7 * 24 * time.Hour,
+		BanScopeV4Prefix:        32,
-		MaxBans:                     5000,
-		BanScopeV4Prefix:            32,
 	})
 
 	if cfg.UpstreamURL.String() != "http://127.0.0.1:8081" {
@@ -115,8 +111,6 @@ func TestValuesAsSet(t *testing.T) {
 		upstreamURL:             "https://app.internal:8443/",
 		trustedProxies:          " 192.0.2.1, 10.1.2.3/8 ,2001:db8::/32",
 		clientRequestTimeout:    "90s",
-		clientHeaderMaxBytes:    "8K",
-		clientIdleTimeout:       "5m",
 		clientResponseTimeout:   "7d",
 		upstreamRequestTimeout:  "1h30m",
 		upstreamResponseTimeout: off,
@@ -139,24 +133,22 @@ func TestValuesAsSet(t *testing.T) {
 	})
 
 	wantSettings(t, cfg, config.Config{
-		ListenAddr:                  "127.0.0.1:9000",
+		ListenAddr:              "127.0.0.1:9000",
-		ClientRequestTimeout:        90 * time.Second,
+		ClientRequestTimeout:    90 * time.Second,
-		ClientRequestHeaderMaxBytes: 8 << 10,
+		ClientResponseTimeout:   7 * 24 * time.Hour,
-		ClientIdleTimeout:           5 * time.Minute,
+		UpstreamRequestTimeout:  90 * time.Minute,
-		ClientResponseTimeout:       7 * 24 * time.Hour,
+		UpstreamResponseTimeout: 0,
-		UpstreamRequestTimeout:      90 * time.Minute,
+		RequestMaxBytes:         512 << 10,
-		UpstreamResponseTimeout:     0,
+		ResponseMaxBytes:        1234,
-		RequestMaxBytes:             512 << 10,
+		RateLimitPerMinute:      60,
-		ResponseMaxBytes:            1234,
+		RateLimitPerHour:        600,
-		RateLimitPerMinute:          60,
+		RateLimitPerDay:         6000,
-		RateLimitPerHour:            600,
+		BanResponse:             429,
-		RateLimitPerDay:             6000,
+		LimitBanDuration:        15 * time.Minute,
-		BanResponse:                 429,
+		LimitBanRepeatWindow:    48 * time.Hour,
-		LimitBanDuration:            15 * time.Minute,
+		MaxBanDuration:          30 * 24 * time.Hour,
-		LimitBanRepeatWindow:        48 * time.Hour,
+		MaxBans:                 100,
-		MaxBanDuration:              30 * 24 * time.Hour,
+		BanScopeV4Prefix:        24,
-		MaxBans:                     100,
-		BanScopeV4Prefix:            24,
 	})
 
 	if cfg.UpstreamURL.String() != "https://app.internal:8443/" {
@@ -195,42 +187,12 @@ func TestSizesAndOff(t *testing.T) {
 		requestMaxBytes:      "3G",
 		responseMaxBytes:     off,
 		clientRequestTimeout: off,
-		clientIdleTimeout:    off,
 	})
 
 	if cfg.RequestMaxBytes != 3<<30 || cfg.ResponseMaxBytes != 0 ||
-		cfg.ClientRequestTimeout != 0 || cfg.ClientIdleTimeout != 0 {
+		cfg.ClientRequestTimeout != 0 {
-		t.Errorf("3G, off, off and off read as %d, %d, %s and %s",
+		t.Errorf("3G, off and off read as %d, %d and %s",
-			cfg.RequestMaxBytes, cfg.ResponseMaxBytes, cfg.ClientRequestTimeout,
+			cfg.RequestMaxBytes, cfg.ResponseMaxBytes, cfg.ClientRequestTimeout)
-			cfg.ClientIdleTimeout)
-	}
-}
-
-func TestRequestHeaderMaxBytesJustOver4K(t *testing.T) {
-	t.Parallel()
-
-	cfg := fromEnvironment(t, environment{clientHeaderMaxBytes: "4097"})
-	if cfg.ClientRequestHeaderMaxBytes != 4097 {
-		t.Errorf("4097 read as %d", cfg.ClientRequestHeaderMaxBytes)
-	}
-}
-
-func TestRequestHeaderMaxBytesRefusalNeverOffersOff(t *testing.T) {
-	t.Parallel()
-
-	for _, value := range []string{"32KB", "0", "4K", off} {
-		t.Run(value, func(t *testing.T) {
-			t.Parallel()
-
-			_, err := config.FromEnvironment(
-				environment{clientHeaderMaxBytes: value}.lookupEnv)
-
-			want := clientHeaderMaxBytes + `: "` + value +
-				`" is not a size of more than 4K, such as 32K`
-			if err == nil || err.Error() != want {
-				t.Errorf("error %v, want %s", err, want)
-			}
-		})
 	}
 }
 
@@ -293,8 +255,6 @@ func TestInvalidValueStopsTheStart(t *testing.T) {
 		{denyNets, "198.51.100.0/24,"},
 		{clientRequestTimeout, "60"},
 		{clientRequestTimeout, ""},
-		{clientIdleTimeout, "0s"},
-		{clientIdleTimeout, "2 minutes"},
 		{clientResponseTimeout, "1y"},
 		{upstreamRequestTimeout, "-1s"},
 		{upstreamResponseTimeout, "0s"},
@@ -368,8 +328,6 @@ func TestLogsEachSettingWithItsValue(t *testing.T) {
 		upstreamURL:             "http://127.0.0.1:8081",
 		trustedProxies:          "10.0.0.0/8,172.16.0.0/12,192.168.0.0/16",
 		clientRequestTimeout:    "45s",
-		clientHeaderMaxBytes:    "32K",
-		clientIdleTimeout:       "120s",
 		clientResponseTimeout:   "30m",
 		upstreamRequestTimeout:  "60s",
 		upstreamResponseTimeout: "30m",
@@ -401,8 +359,6 @@ func wantSettings(t *testing.T, got *config.Config, want config.Config) {
 
 	if got.ListenAddr != want.ListenAddr ||
 		got.ClientRequestTimeout != want.ClientRequestTimeout ||
-		got.ClientRequestHeaderMaxBytes != want.ClientRequestHeaderMaxBytes ||
-		got.ClientIdleTimeout != want.ClientIdleTimeout ||
 		got.ClientResponseTimeout != want.ClientResponseTimeout ||
 		got.UpstreamRequestTimeout != want.UpstreamRequestTimeout ||
 		got.UpstreamResponseTimeout != want.UpstreamResponseTimeout ||

internal/proxy/passthrough_test.go

@@ -297,7 +297,7 @@ func echoAfterUpgrade(w http.ResponseWriter, r *http.Request) {
 	}
 }
 
-func TestServerHasTheDefaultLimits(t *testing.T) {
+func TestServerHasTheFixedLimits(t *testing.T) {
 	t.Parallel()
 
 	cfg, err := config.FromEnvironment(func(string) (string, bool) { return "", false })
@@ -319,48 +319,37 @@ func TestServerHasTheDefaultLimits(t *testing.T) {
 	}
 }
 
-func TestRefusesHeadersOverTheLimit(t *testing.T) {
+func TestRefusesHeadersOver32KiB(t *testing.T) {
 	t.Parallel()
 
+	var calls atomic.Int32
+
+	app := startApp(t, func(http.ResponseWriter, *http.Request) {
+		calls.Add(1)
+	})
+	addr, _ := startProxy(t, app.URL, nil)
+
+	// size counts every byte of the request: the request line, the
+	// headers and the blank line that ends them.
+	const (
+		start = "GET / HTTP/1.1\r\nHost: app\r\nX-Large: "
+		end   = "\r\n\r\n"
+	)
+
 	for _, tc := range []struct {
-		name  string
+		size int
-		env   map[string]string
+		want int
-		limit int
 	}{
-		{"by default", nil, 32 << 10},
+		{size: 32 << 10, want: http.StatusOK},
-		{"as set", map[string]string{clientHeaderMaxBytes: "8K"}, 8 << 10},
+		{size: 32<<10 + 1, want: http.StatusRequestHeaderFieldsTooLarge},
 	} {
-		t.Run(tc.name, func(t *testing.T) {
+		conn := dial(t, addr)
-			t.Parallel()
+		send(t, conn, start+strings.Repeat("a", tc.size-len(start)-len(end))+end)
+		wantStatus(t, readResponse(t, conn), tc.want)
+	}
 
-			var calls atomic.Int32
+	if calls.Load() != 1 {
-
+		t.Errorf("the app was called %d times, want once", calls.Load())
-			app := startApp(t, func(http.ResponseWriter, *http.Request) {
-				calls.Add(1)
-			})
-			addr, _ := startProxy(t, app.URL, tc.env)
-
-			// size counts every byte of the request: the request line,
-			// the headers and the blank line that ends them.
-			const (
-				start = "GET / HTTP/1.1\r\nHost: app\r\nX-Large: "
-				end   = "\r\n\r\n"
-			)
-
-			for _, sent := range []struct{ size, want int }{
-				{tc.limit, http.StatusOK},
-				{tc.limit + 1, http.StatusRequestHeaderFieldsTooLarge},
-			} {
-				conn := dial(t, addr)
-				send(t, conn,
-					start+strings.Repeat("a", sent.size-len(start)-len(end))+end)
-				wantStatus(t, readResponse(t, conn), sent.want)
-			}
-
-			if calls.Load() != 1 {
-				t.Errorf("the app was called %d times, want once", calls.Load())
-			}
-		})
 	}
 }
 

internal/proxy/proxy.go

@@ -17,6 +17,19 @@ import (
 	"sneak.berlin/go/smallwebwaf/internal/requestlog"
 )
 
+// The request line and headers a client may send, and how long a
+// kept-open client connection may wait for its next request, are fixed
+// rather than settings. The limit on the request line and headers is
+// 32 KiB, but Go's server reads 4 KiB past its MaxHeaderBytes before it
+// refuses, so MaxHeaderBytes is set 4 KiB lower. The idle time is longer
+// than the 90 seconds after which traefik closes a connection it is not
+// using, so traefik never sends a request on a connection smallwebwaf is
+// closing.
+const (
+	requestHeaderMaxBytes = 32<<10 - 4<<10
+	clientIdleTimeout     = 120 * time.Second
+)
+
 // How smallwebwaf keeps connections to the app open between requests.
 const (
 	appIdleConns       = 100
@@ -43,9 +56,8 @@ type Params struct {
 }
 
 // New returns the server smallwebwaf runs: each request it reads passes
-// through the proxy. Go's server itself refuses a request line and
+// through the proxy. Go's server itself refuses headers over 32 KiB, with
-// headers over SWWAF_CLIENT_REQUEST_HEADER_MAX_BYTES, with 431, closes a
+// 431, closes a connection idle for 120 seconds, and applies
-// connection idle for SWWAF_CLIENT_IDLE_TIMEOUT, and applies
 // SWWAF_CLIENT_REQUEST_TIMEOUT while the headers arrive; the proxy
 // applies the timeouts and size limits from then on.
 func New(params Params) *http.Server {
@@ -78,13 +90,9 @@ func New(params Params) *http.Server {
 			}),
 		},
 		ReadHeaderTimeout: params.Config.ClientRequestTimeout,
-		// Off is an IdleTimeout of 0, which Go's server replaces with
+		IdleTimeout:       clientIdleTimeout,
-		// ReadTimeout: no limit, as long as ReadTimeout stays unset.
+		MaxHeaderBytes:    requestHeaderMaxBytes,
-		IdleTimeout: params.Config.ClientIdleTimeout,
+		ErrorLog:          errorLog,
-		// Go's server reads 4 KiB past MaxHeaderBytes before it refuses,
-		// so the limit a client meets is the setting.
-		MaxHeaderBytes: int(params.Config.ClientRequestHeaderMaxBytes - 4<<10),
-		ErrorLog:       errorLog,
 	}
 }
 

internal/proxy/proxy_test.go

@@ -45,8 +45,6 @@ var shortTimeoutSetting = shortTimeout.String()
 // The settings the tests set.
 const (
 	clientRequestTimeout    = "SWWAF_CLIENT_REQUEST_TIMEOUT"
-	clientHeaderMaxBytes    = "SWWAF_CLIENT_REQUEST_HEADER_MAX_BYTES"
-	clientIdleTimeout       = "SWWAF_CLIENT_IDLE_TIMEOUT"
 	clientResponseTimeout   = "SWWAF_CLIENT_RESPONSE_TIMEOUT"
 	upstreamRequestTimeout  = "SWWAF_UPSTREAM_REQUEST_TIMEOUT"
 	upstreamResponseTimeout = "SWWAF_UPSTREAM_RESPONSE_TIMEOUT"

internal/proxy/timeouts_test.go

@@ -273,26 +273,3 @@ func TestClientTooSlowToTakeTheAnswer(t *testing.T) {
 	wantTimedOut(t, start)
 	wantLine(t, line, http.StatusOK, requestlog.ActionTimedOut)
 }
-
-func TestClosesAnIdleConnection(t *testing.T) {
-	t.Parallel()
-
-	app := startApp(t, func(http.ResponseWriter, *http.Request) {})
-	addr, _ := startProxy(t, app.URL, map[string]string{
-		clientIdleTimeout: shortTimeoutSetting,
-	})
-
-	// The idle time starts once the answer is sent, so after start.
-	start := time.Now()
-	conn := dial(t, addr)
-	send(t, conn, "GET / HTTP/1.1\r\nHost: app\r\n\r\n")
-	wantStatus(t, readResponse(t, conn), http.StatusOK)
-
-	// The read deadline readResponse set still bounds this read.
-	_, err := conn.Read(make([]byte, 1))
-	if !errors.Is(err, io.EOF) {
-		t.Fatalf("read on the idle connection: %v, want it closed", err)
-	}
-
-	wantTimedOut(t, start)
-}

internal/smallwebwaf/smallwebwaf_test.go

@@ -177,31 +177,29 @@ func wantStartingLine(t *testing.T, line map[string]any, appURL string) {
 
 	settings, _ := line["settings"].(map[string]any)
 	want := map[string]any{
-		listenAddr:                              localhost + ":0",
+		listenAddr:                            localhost + ":0",
-		upstreamURL:                             appURL,
+		upstreamURL:                           appURL,
-		"SWWAF_TRUSTED_PROXIES":                 "10.0.0.0/8,172.16.0.0/12,192.168.0.0/16",
+		"SWWAF_TRUSTED_PROXIES":               "10.0.0.0/8,172.16.0.0/12,192.168.0.0/16",
-		"SWWAF_CLIENT_REQUEST_TIMEOUT":          "60s",
+		"SWWAF_CLIENT_REQUEST_TIMEOUT":        "60s",
-		"SWWAF_CLIENT_REQUEST_HEADER_MAX_BYTES": "32K",
+		"SWWAF_CLIENT_RESPONSE_TIMEOUT":       "30m",
-		"SWWAF_CLIENT_IDLE_TIMEOUT":             "120s",
+		"SWWAF_UPSTREAM_REQUEST_TIMEOUT":      "60s",
-		"SWWAF_CLIENT_RESPONSE_TIMEOUT":         "30m",
+		"SWWAF_UPSTREAM_RESPONSE_TIMEOUT":     "30m",
-		"SWWAF_UPSTREAM_REQUEST_TIMEOUT":        "60s",
+		"SWWAF_REQUEST_MAX_BYTES":             "100M",
-		"SWWAF_UPSTREAM_RESPONSE_TIMEOUT":       "30m",
+		"SWWAF_RESPONSE_MAX_BYTES":            "5G",
-		"SWWAF_REQUEST_MAX_BYTES":               "100M",
+		"SWWAF_ALLOW_NETS":                    "",
-		"SWWAF_RESPONSE_MAX_BYTES":              "5G",
+		"SWWAF_RATE_LIMIT_EXEMPT_NETS":        "",
-		"SWWAF_ALLOW_NETS":                      "",
+		"SWWAF_DENY_NETS":                     "",
-		"SWWAF_RATE_LIMIT_EXEMPT_NETS":          "",
+		"SWWAF_RATE_LIMIT_PER_MINUTE":         "1000",
-		"SWWAF_DENY_NETS":                       "",
+		"SWWAF_RATE_LIMIT_PER_HOUR":           "10000",
-		"SWWAF_RATE_LIMIT_PER_MINUTE":           "1000",
+		"SWWAF_RATE_LIMIT_PER_DAY":            "50000",
-		"SWWAF_RATE_LIMIT_PER_HOUR":             "10000",
+		"SWWAF_DENIED_COUNTRIES":              "",
-		"SWWAF_RATE_LIMIT_PER_DAY":              "50000",
+		"SWWAF_EXCLUSIVELY_ALLOWED_COUNTRIES": "",
-		"SWWAF_DENIED_COUNTRIES":                "",
+		"SWWAF_BAN_RESPONSE":                  "403",
-		"SWWAF_EXCLUSIVELY_ALLOWED_COUNTRIES":   "",
+		"SWWAF_LIMIT_BAN_DURATION":            "1h",
-		"SWWAF_BAN_RESPONSE":                    "403",
+		"SWWAF_LIMIT_BAN_REPEAT_WINDOW":       "24h",
-		"SWWAF_LIMIT_BAN_DURATION":              "1h",
+		"SWWAF_MAX_BAN_DURATION":              "7d",
-		"SWWAF_LIMIT_BAN_REPEAT_WINDOW":         "24h",
+		"SWWAF_MAX_BANS":                      "5000",
-		"SWWAF_MAX_BAN_DURATION":                "7d",
+		"SWWAF_BAN_SCOPE_V4_PREFIX":           "32",
-		"SWWAF_MAX_BANS":                        "5000",
-		"SWWAF_BAN_SCOPE_V4_PREFIX":             "32",
 	}
 
 	for name, value := range want {