README.md

@@ -13,15 +13,17 @@ 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 the static lists,
+https://git.eeqj.de/sneak/smallwebwaf/issues/14), and so are two parts of
-which come next in the build order. `smallwebwaf` passes each request to the app
+milestone 3: the static lists, which come next in the build order, and the
-and the app's answer back, unchanged, within its timeouts and size limits, works
+header size and the idle time as settings, which come last in it. `smallwebwaf`
-out each client's address, refuses a client that sends too many requests, comes
+passes each request to the app and the app's answer back, unchanged, within its
-from a country you refuse or from a network you refuse, lets the networks you
+timeouts and size limits, works out each client's address, refuses a client that
-choose through, and writes a JSON log line for every request. It comes as the
+sends too many requests, comes from a country you refuse or from a network you
-image the app's own image is built on. The rest of the design comes after that,
+refuse, lets the networks you choose through, and writes a JSON log line for
-in the order of the build order in [`SPEC.md`](SPEC.md). The survey of existing
+every request. It comes as the image the app's own image is built on. The rest
-tools that led to the design is in [`EVALUATION.md`](EVALUATION.md).
+of the design comes after that, in the order of the build order in
+[`SPEC.md`](SPEC.md). The survey of existing tools that led to the design is in
+[`EVALUATION.md`](EVALUATION.md).
 
 ## Getting started
 
@@ -58,16 +60,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 four timeouts and the two size limits below. A limit passed
+- Enforces the timeouts and the size limits below. A limit passed before the
-  before the response has started gets `smallwebwaf`'s own answer: `408` for a
+  response has started gets `smallwebwaf`'s own answer: `408` for a client too
-  client too slow to send its request, `413` for a request body that is too
+  slow to send its request, `413` for a request body that is too large, `504`
-  large, `504` for an app too slow to answer, and `502` for a response that is
+  for an app too slow to answer, and `502` for a response that is too large or
-  too large or an app that cannot be reached. A request that announces a body
+  an app that cannot be reached. A request that announces a body over the limit
-  over the limit is refused before anything reaches the app. While a request
+  is refused before anything reaches the app. While a request body is still on
-  body is still on its way, a request timeout that runs out answers `408` if
+  its way, a request timeout that runs out answers `408` if `smallwebwaf` was
-  `smallwebwaf` was waiting for the client to send more, and `504` if it was
+  waiting for the client to send more, and `504` if it was waiting for the app
-  waiting for the app to take what it had. Once the response has started, a
+  to take what it had. Once the response has started, a limit can only cut the
-  limit can only cut the connection.
+  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 `429`
   before anything reaches the app, and so is each request after it until the
@@ -113,6 +115,16 @@ 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
@@ -145,16 +157,13 @@ 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; only
+`SWWAF_CLIENT_REQUEST_HEADER_MAX_BYTES` cannot be off.
 
-Several limits are fixed rather than settings. The request line and headers may
+Several limits are fixed rather than settings. At most 20,000 clients are kept
-take up to 32 KiB, above which the answer is `431` and nothing reaches the app.
+for the rate limits, and an IPv6 client is counted by its /64. A new client
-A kept-open connection that sends nothing for 120 seconds is closed. That is
+waits at most a second for its country, and at most 100,000 answers from GeoJS
-longer than the 90 seconds after which traefik closes a connection it is not
+are kept, for 7 days each.
-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
 
@@ -193,10 +202,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 32 KiB, which it answers `431`, headers
+without a line in the log: headers over `SWWAF_CLIENT_REQUEST_HEADER_MAX_BYTES`,
-slower than `SWWAF_CLIENT_REQUEST_TIMEOUT`, whose connection it closes without
+which it answers `431`, headers slower than `SWWAF_CLIENT_REQUEST_TIMEOUT`,
-an answer, and requests it cannot read at all, which it answers itself, mostly
+whose connection it closes without an answer, and requests it cannot read at
-with `400`.
+all, which it answers itself, mostly with `400`.
 
 ## Why
 
@@ -535,8 +544,9 @@ so that they run in minimal containers.
 
 ## TODO
 
-- The rest of milestone 3, after the static lists, and the rest of the design,
+- The rest of milestone 3, from the bans that broken request limits lead to
-  in the order of the build order in [`SPEC.md`](SPEC.md).
+  through the metrics endpoint, and the rest of the design, in the order of the
+  build order in [`SPEC.md`](SPEC.md).
 
 ## Documents
 

SPEC.md

@@ -293,7 +293,8 @@ 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`.
+- Any limit or threshold can be switched off with the value `off`, except
+  `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
@@ -413,7 +414,9 @@ 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.
+      `431` and closes the connection, 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.
       It is longer than the 90 seconds after which traefik, by default, closes a

internal/config/config.go

@@ -30,6 +30,13 @@ 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
@@ -103,6 +110,7 @@ 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")
 )
 
 // FromEnvironment reads the settings with lookupEnv, normally
@@ -111,10 +119,13 @@ 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"),
@@ -225,6 +236,15 @@ 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))
@@ -296,6 +316,19 @@ 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,6 +20,8 @@ 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"
@@ -66,16 +68,18 @@ func TestDefaults(t *testing.T) {
 	cfg := fromEnvironment(t, environment{})
 
 	wantSettings(t, cfg, config.Config{
-		ListenAddr:              ":8080",
+		ListenAddr:                  ":8080",
-		ClientRequestTimeout:    time.Minute,
+		ClientRequestTimeout:        time.Minute,
-		ClientResponseTimeout:   30 * time.Minute,
+		ClientRequestHeaderMaxBytes: 32 << 10,
-		UpstreamRequestTimeout:  time.Minute,
+		ClientIdleTimeout:           2 * time.Minute,
-		UpstreamResponseTimeout: 30 * time.Minute,
+		ClientResponseTimeout:       30 * time.Minute,
-		RequestMaxBytes:         100 << 20,
+		UpstreamRequestTimeout:      time.Minute,
-		ResponseMaxBytes:        5 << 30,
+		UpstreamResponseTimeout:     30 * time.Minute,
-		RateLimitPerMinute:      1000,
+		RequestMaxBytes:             100 << 20,
-		RateLimitPerHour:        10000,
+		ResponseMaxBytes:            5 << 30,
-		RateLimitPerDay:         50000,
+		RateLimitPerMinute:          1000,
+		RateLimitPerHour:            10000,
+		RateLimitPerDay:             50000,
 	})
 
 	if cfg.UpstreamURL.String() != "http://127.0.0.1:8081" {
@@ -99,6 +103,8 @@ 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,
@@ -115,16 +121,18 @@ 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,
-		ClientResponseTimeout:   7 * 24 * time.Hour,
+		ClientRequestHeaderMaxBytes: 8 << 10,
-		UpstreamRequestTimeout:  90 * time.Minute,
+		ClientIdleTimeout:           5 * time.Minute,
-		UpstreamResponseTimeout: 0,
+		ClientResponseTimeout:       7 * 24 * time.Hour,
-		RequestMaxBytes:         512 << 10,
+		UpstreamRequestTimeout:      90 * time.Minute,
-		ResponseMaxBytes:        1234,
+		UpstreamResponseTimeout:     0,
-		RateLimitPerMinute:      60,
+		RequestMaxBytes:             512 << 10,
-		RateLimitPerHour:        600,
+		ResponseMaxBytes:            1234,
-		RateLimitPerDay:         6000,
+		RateLimitPerMinute:          60,
+		RateLimitPerHour:            600,
+		RateLimitPerDay:             6000,
 	})
 
 	if cfg.UpstreamURL.String() != "https://app.internal:8443/" {
@@ -163,12 +171,42 @@ 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.ClientRequestTimeout != 0 || cfg.ClientIdleTimeout != 0 {
-		t.Errorf("3G, off and off read as %d, %d and %s",
+		t.Errorf("3G, off, off and off read as %d, %d, %s 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)
+			}
+		})
 	}
 }
 
@@ -222,6 +260,8 @@ 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"},
@@ -289,6 +329,8 @@ 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",
@@ -314,6 +356,8 @@ 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 TestServerHasTheFixedLimits(t *testing.T) {
+func TestServerHasTheDefaultLimits(t *testing.T) {
 	t.Parallel()
 
 	cfg, err := config.FromEnvironment(func(string) (string, bool) { return "", false })
@@ -319,37 +319,48 @@ func TestServerHasTheFixedLimits(t *testing.T) {
 	}
 }
 
-func TestRefusesHeadersOver32KiB(t *testing.T) {
+func TestRefusesHeadersOverTheLimit(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 {
-		size int
+		name  string
-		want int
+		env   map[string]string
+		limit int
 	}{
-		{size: 32 << 10, want: http.StatusOK},
+		{"by default", nil, 32 << 10},
-		{size: 32<<10 + 1, want: http.StatusRequestHeaderFieldsTooLarge},
+		{"as set", map[string]string{clientHeaderMaxBytes: "8K"}, 8 << 10},
 	} {
-		conn := dial(t, addr)
+		t.Run(tc.name, func(t *testing.T) {
-		send(t, conn, start+strings.Repeat("a", tc.size-len(start)-len(end))+end)
+			t.Parallel()
-		wantStatus(t, readResponse(t, conn), tc.want)
-	}
 
-	if calls.Load() != 1 {
+			var calls atomic.Int32
-		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

@@ -16,19 +16,6 @@ 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
@@ -52,8 +39,9 @@ type Params struct {
 }
 
 // New returns the server smallwebwaf runs: each request it reads passes
-// through the proxy. Go's server itself refuses headers over 32 KiB, with
+// through the proxy. Go's server itself refuses a request line and
-// 431, closes a connection idle for 120 seconds, and applies
+// headers over SWWAF_CLIENT_REQUEST_HEADER_MAX_BYTES, with 431, closes a
+// 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 {
@@ -79,9 +67,13 @@ func New(params Params) *http.Server {
 			}),
 		},
 		ReadHeaderTimeout: params.Config.ClientRequestTimeout,
-		IdleTimeout:       clientIdleTimeout,
+		// Off is an IdleTimeout of 0, which Go's server replaces with
-		MaxHeaderBytes:    requestHeaderMaxBytes,
+		// ReadTimeout: no limit, as long as ReadTimeout stays unset.
-		ErrorLog:          errorLog,
+		IdleTimeout: params.Config.ClientIdleTimeout,
+		// 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,6 +45,8 @@ 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,3 +273,26 @@ 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,23 +177,25 @@ 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_RESPONSE_TIMEOUT":       "30m",
+		"SWWAF_CLIENT_REQUEST_HEADER_MAX_BYTES": "32K",
-		"SWWAF_UPSTREAM_REQUEST_TIMEOUT":      "60s",
+		"SWWAF_CLIENT_IDLE_TIMEOUT":             "120s",
-		"SWWAF_UPSTREAM_RESPONSE_TIMEOUT":     "30m",
+		"SWWAF_CLIENT_RESPONSE_TIMEOUT":         "30m",
-		"SWWAF_REQUEST_MAX_BYTES":             "100M",
+		"SWWAF_UPSTREAM_REQUEST_TIMEOUT":        "60s",
-		"SWWAF_RESPONSE_MAX_BYTES":            "5G",
+		"SWWAF_UPSTREAM_RESPONSE_TIMEOUT":       "30m",
-		"SWWAF_ALLOW_NETS":                    "",
+		"SWWAF_REQUEST_MAX_BYTES":               "100M",
-		"SWWAF_RATE_LIMIT_EXEMPT_NETS":        "",
+		"SWWAF_RESPONSE_MAX_BYTES":              "5G",
-		"SWWAF_DENY_NETS":                     "",
+		"SWWAF_ALLOW_NETS":                      "",
-		"SWWAF_RATE_LIMIT_PER_MINUTE":         "1000",
+		"SWWAF_RATE_LIMIT_EXEMPT_NETS":          "",
-		"SWWAF_RATE_LIMIT_PER_HOUR":           "10000",
+		"SWWAF_DENY_NETS":                       "",
-		"SWWAF_RATE_LIMIT_PER_DAY":            "50000",
+		"SWWAF_RATE_LIMIT_PER_MINUTE":           "1000",
-		"SWWAF_DENIED_COUNTRIES":              "",
+		"SWWAF_RATE_LIMIT_PER_HOUR":             "10000",
-		"SWWAF_EXCLUSIVELY_ALLOWED_COUNTRIES": "",
+		"SWWAF_RATE_LIMIT_PER_DAY":              "50000",
+		"SWWAF_DENIED_COUNTRIES":                "",
+		"SWWAF_EXCLUSIVELY_ALLOWED_COUNTRIES":   "",
 	}
 
 	for name, value := range want {