diff --git a/Makefile b/Makefile index 9f89701..e8da879 100644 --- a/Makefile +++ b/Makefile @@ -4,10 +4,6 @@ # per the scripts-to-rule-them-all pattern (see the Entrypoints section # of README.md). build and run are for working on the code by hand. -# The version the binary reports. It comes from git, so it is computed -# here on the host; the Dockerfile is passed it as a build argument. -VERSION ?= $(shell git describe --tags --always --dirty 2>/dev/null || echo unknown) - bootstrap: @script/bootstrap @@ -36,8 +32,7 @@ hooks: @script/install-precommit build: - go build -trimpath -ldflags "-X main.Version=$(VERSION)" \ - -o bin/smallwebwaf ./cmd/smallwebwaf + @script/build -run: build - ./bin/smallwebwaf +run: + @script/run diff --git a/README.md b/README.md index efe3990..20b760c 100644 --- a/README.md +++ b/README.md @@ -70,7 +70,7 @@ 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 a port, and nothing more. + `https`, a host and an optional port, and nothing more. - `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. @@ -125,10 +125,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` (reading -up to 4 KiB past the limit first), headers slower than -`SWWAF_CLIENT_REQUEST_TIMEOUT`, whose connection it closes without an answer, -and requests it cannot read at all, which it answers itself, mostly with `400`. +without a line in the log: headers over 32 KiB, which it answers `431`, headers +slower than `SWWAF_CLIENT_REQUEST_TIMEOUT`, whose connection it closes without +an answer, and requests it cannot read at all, which it answers itself, mostly +with `400`. ## Why @@ -406,8 +406,10 @@ so that they run in minimal containers. image build. - `script/precommit`: run by the git pre-commit hook; runs `script/check`. - `script/install-precommit`: installs that hook; `make hooks` runs it. - -`make build` builds `bin/smallwebwaf`, and `make run` builds and runs it. +- `script/build`: builds `bin/smallwebwaf` on the host, with Go installed, for + working on the code by hand; `make build` runs it. +- `script/run`: builds `bin/smallwebwaf` with `script/build` and runs it; + `make run` runs it. ## TODO diff --git a/SPEC.md b/SPEC.md index 1b6dac1..918ed49 100644 --- a/SPEC.md +++ b/SPEC.md @@ -442,8 +442,7 @@ The settings, by group: longer than `SWWAF_CLIENT_REQUEST_TIMEOUT` to send them gets no answer: the server closes its connection. Headers over `SWWAF_CLIENT_REQUEST_HEADER_MAX_BYTES` are answered `431` by the server - itself, which reads up to 4 KiB past the limit before it refuses. Neither - request gets a line in the request log. + itself. Neither request gets a line in the request log. - A WebSocket connection leaves these limits behind once it is upgraded: it stays open until either side closes it. - Lookup of AS number and country (R7). On by default through GeoJS, which needs diff --git a/internal/config/config.go b/internal/config/config.go index b426319..a0236c2 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -71,7 +71,7 @@ var ( errNotListenAddr = errors.New( "is not an address to listen on, such as :8080") errNotUpstreamURL = errors.New( - "is not a URL with only a scheme, a host and a port, " + + "is not a URL with only a scheme, a host and an optional port, " + "such as http://127.0.0.1:8081") ) @@ -325,8 +325,8 @@ func parseListenAddr(value string) (string, error) { } // parseUpstreamURL reads the app's URL: http or https, a host and an -// optional port, and nothing else, since the request's own path and -// query go to the app unchanged. +// optional port from 1 to 65535, and nothing else, since the request's +// own path and query go to the app unchanged. func parseUpstreamURL(value string) (*url.URL, error) { upstream, err := url.Parse(value) if err != nil { @@ -334,12 +334,19 @@ func parseUpstreamURL(value string) (*url.URL, error) { } onlySchemeAndHost := (upstream.Scheme == "http" || upstream.Scheme == "https") && - upstream.Host != "" && upstream.User == nil && upstream.Opaque == "" && + upstream.Hostname() != "" && upstream.User == nil && upstream.Opaque == "" && (upstream.Path == "" || upstream.Path == "/") && upstream.RawQuery == "" && upstream.Fragment == "" if !onlySchemeAndHost { return nil, fmt.Errorf("%q %w", value, errNotUpstreamURL) } + if upstream.Port() != "" { + port, err := strconv.ParseUint(upstream.Port(), 10, 16) + if err != nil || port == 0 { + return nil, fmt.Errorf("%q %w", value, errNotUpstreamURL) + } + } + return upstream, nil } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index b97db18..964ffa1 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -142,6 +142,9 @@ func TestInvalidValueStopsTheStart(t *testing.T) { {upstreamURL, "127.0.0.1:8081"}, {upstreamURL, "ftp://127.0.0.1:8081"}, {upstreamURL, "http://"}, + {upstreamURL, "http://:8081"}, + {upstreamURL, "http://127.0.0.1:0"}, + {upstreamURL, "http://127.0.0.1:99999"}, {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"}, diff --git a/internal/proxy/passthrough_test.go b/internal/proxy/passthrough_test.go index 25dfa9f..66caee7 100644 --- a/internal/proxy/passthrough_test.go +++ b/internal/proxy/passthrough_test.go @@ -309,7 +309,7 @@ func TestServerHasTheFixedLimits(t *testing.T) { ProcessLog: requestlog.NewProcessLogger(io.Discard), }) - if server.Addr != ":8080" || server.MaxHeaderBytes != 32<<10 || + if server.Addr != ":8080" || server.MaxHeaderBytes != 28<<10 || server.IdleTimeout != 2*time.Minute || server.ReadHeaderTimeout != time.Minute { t.Errorf("server listens on %q with header limit %d, idle time %s and "+ "header timeout %s", server.Addr, server.MaxHeaderBytes, @@ -327,16 +327,23 @@ func TestRefusesHeadersOver32KiB(t *testing.T) { }) 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 { - headerSize int - want int + size int + want int }{ - {headerSize: 30 << 10, want: http.StatusOK}, - {headerSize: 40 << 10, want: http.StatusRequestHeaderFieldsTooLarge}, + {size: 32 << 10, want: http.StatusOK}, + {size: 32<<10 + 1, want: http.StatusRequestHeaderFieldsTooLarge}, } { - req := newRequest(t, http.MethodGet, addr, "/", http.NoBody) - req.Header.Set("X-Large", strings.Repeat("a", tc.headerSize)) - wantStatus(t, do(t, req), tc.want) + conn := dial(t, addr) + send(t, conn, start+strings.Repeat("a", tc.size-len(start)-len(end))+end) + wantStatus(t, readResponse(t, conn), tc.want) } if calls.Load() != 1 { diff --git a/internal/proxy/proxy.go b/internal/proxy/proxy.go index 717cf27..327ba81 100644 --- a/internal/proxy/proxy.go +++ b/internal/proxy/proxy.go @@ -15,11 +15,14 @@ import ( // 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 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. +// 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 + requestHeaderMaxBytes = 32<<10 - 4<<10 clientIdleTimeout = 120 * time.Second ) diff --git a/script/build b/script/build new file mode 100755 index 0000000..94bd69c --- /dev/null +++ b/script/build @@ -0,0 +1,18 @@ +#!/bin/sh +# script/build: build bin/smallwebwaf on the host, with Go installed, for +# working on the code by hand. The version it reports comes from git, as +# in script/docker. +set -eu + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)" +ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)" + +main() { + cd "$ROOT" + version="$(git describe --tags --always --dirty 2>/dev/null || true)" + [ -n "$version" ] || version="unknown" + go build -trimpath -ldflags "-X main.Version=$version" \ + -o bin/smallwebwaf ./cmd/smallwebwaf +} + +main "$@" diff --git a/script/run b/script/run new file mode 100755 index 0000000..a64ade2 --- /dev/null +++ b/script/run @@ -0,0 +1,14 @@ +#!/bin/sh +# script/run: build bin/smallwebwaf with script/build and run it, with +# the settings in the environment. +set -eu + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)" +ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)" + +main() { + "$SCRIPT_DIR/build" + exec "$ROOT/bin/smallwebwaf" +} + +main "$@"