AS number and country looked up for every client (closes #95)

GeoJS's geo.json is asked about every new visitor unless
SWWAF_LOOKUP_SOURCE is off. A request waits for its client's first
answer only while a country list or SWWAF_ADD_LOOKUP_HEADERS needs it;
otherwise the answer reaches the client's history and ban notes when it
comes. The AS number and name go beside the country in the request log,
history, ban notes, alerts and lookups.json, with metrics by AS number;
64512 counts as unknown. A client's own X-Client-ASN and
X-Client-Country never reach the app, whatever the setting says, and
make example-app sends no address to GeoJS.

Judgement call: AS numbers are written AS64496, as SPEC's settings write them.
Judgement call: SWWAF_LOOKUP_TIMEOUT is added, default 1s, and cannot be off.

Model: opus-5-5

README.md

@@ -279,7 +279,7 @@ effective settings are logged at start.
 - `SWWAF_LOOKUP_SOURCE` (default `geojs`): where each client's AS number and
   country are looked up: `geojs`, the GeoJS web service, which is then told the
   address of every new visitor, or `off`, which looks up no client and sends no
-  address anywhere. `file`, for the IPinfo Lite database, comes with
+  address to GeoJS. `file`, for the IPinfo Lite database, comes with
   https://git.eeqj.de/sneak/smallwebwaf/issues/22. With `off`, a country list
   that is not empty, or `SWWAF_ADD_LOOKUP_HEADERS` set to `true`, stops the
   start, with a message naming it and `SWWAF_LOOKUP_SOURCE`.
@@ -288,9 +288,10 @@ effective settings are logged at start.
   GeoJS may take before it is abandoned.
 - `SWWAF_ADD_LOOKUP_HEADERS` (default `false`): `true` passes the app the
   client's AS number, such as `AS64496`, in `X-Client-ASN`, and its country in
-  `X-Client-Country`, leaving out one that is unknown, and removes any such
-  headers the client sent. A request then waits for its client's first answer,
-  as it does while a country list is set.
+  `X-Client-Country`, leaving out one that is unknown. A request then waits for
+  its client's first answer, as it does while a country list is set. Whatever
+  this setting says, any `X-Client-ASN` or `X-Client-Country` the client sent,
+  in any case, is removed, so that the app never receives a client's own.
 - `SWWAF_DENIED_COUNTRIES` (default empty): countries whose clients are refused,
   for example `cn,ru,kp`.
 - `SWWAF_EXCLUSIVELY_ALLOWED_COUNTRIES` (default empty): when set, the only

internal/lookup/lookup_test.go

@@ -119,6 +119,58 @@ func TestNewClientWaitsAtMostOneSecondThenCountsAsNotFound(t *testing.T) {
 	})
 }
 
+func TestRequestWaitsAsLongAsTheTimeoutSaysAndGeoJSIsAbandonedAfterIt(t *testing.T) {
+	t.Parallel()
+
+	synctest.Test(t, func(t *testing.T) {
+		// A timeout longer than the default second, and a GeoJS that does
+		// not answer.
+		const longerTimeout = 3 * time.Second
+
+		m := metrics.New(1, "app")
+		g := lookup.New(lookup.Params{
+			URL:        lookup.URL,
+			Timeout:    longerTimeout,
+			Wait:       true,
+			Now:        time.Now,
+			ProcessLog: slog.New(slog.DiscardHandler),
+			Metrics:    m,
+			Alerts:     alerts.New(alerts.Params{}),
+		})
+		g.SetTransport(&standIn{answers: hanging})
+
+		var (
+			request sync.WaitGroup
+			waited  time.Duration
+		)
+
+		request.Go(func() {
+			began := time.Now()
+
+			wantCountry(t, g, netip.MustParsePrefix("203.0.113.9/32"), "")
+
+			waited = time.Since(began)
+		})
+
+		// A moment before the timeout runs out, GeoJS is still being asked:
+		// the request to it has not failed.
+		time.Sleep(longerTimeout - time.Millisecond)
+		synctest.Wait()
+		wantFailures(t, m, 0)
+
+		// As it runs out, the client's request goes on, and the request to
+		// GeoJS is abandoned, which counts as a failure.
+		request.Wait()
+		synctest.Wait()
+
+		if waited != longerTimeout {
+			t.Errorf("waited %s for the answer, want %s", waited, longerTimeout)
+		}
+
+		wantFailures(t, m, 1)
+	})
+}
+
 func TestAddressLeftOutOfAnAnswerIsAskedAboutAgain(t *testing.T) {
 	t.Parallel()
 
@@ -767,6 +819,16 @@ func wantUnanswered(t *testing.T, m *metrics.Metrics, want float64) {
 	}
 }
 
+// wantFailures checks how many requests to GeoJS m counts as failed.
+func wantFailures(t *testing.T, m *metrics.Metrics, want float64) {
+	t.Helper()
+
+	got := testutil.ToFloat64(m.GeoJSFailures)
+	if got != want {
+		t.Errorf("%v requests to GeoJS failed, want %v", got, want)
+	}
+}
+
 // waitForRequests waits until g has done all it can before time passes,
 // checks that GeoJS has had count requests, and returns the addresses each
 // asked about.

internal/proxy/lookup.go

@@ -9,9 +9,10 @@ import (
 )
 
 // The headers in which the app is passed the client's AS number and
-// country while SWWAF_ADD_LOOKUP_HEADERS is set. Go sends a header name in
-// this form, so X-Client-ASN arrives as X-Client-Asn; header names are not
-// case-sensitive.
+// country while SWWAF_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, and Del removes a client's own whatever their
+// case; header names are not case-sensitive.
 const (
 	asnHeader     = "X-Client-Asn"
 	countryHeader = "X-Client-Country"
@@ -45,12 +46,8 @@ func (h *handler) addLookup(answer lookup.Answer) {
 }
 
 // setLookupHeaders sets the headers in which the app is passed the
-// client's AS number and country, leaving out one that is unknown. Any
-// the client sent are removed, so that the app can believe them.
+// client's AS number and country, leaving out one that is unknown.
 func setLookupHeaders(header http.Header, asn, country string) {
-	header.Del(asnHeader)
-	header.Del(countryHeader)
-
 	if asn != "" {
 		header.Set(asnHeader, asn)
 	}

internal/proxy/lookup_test.go

@@ -189,9 +189,9 @@ func TestLookupHeadersArePassedToTheAppAndTheClientsOwnRemoved(t *testing.T) {
 	// Each client sends headers of its own. fromDE's first request waits
 	// for its answer, which the app is passed; unplaced has none to pass,
 	// and a client on a private address is not looked up.
-	own := "X-Client-ASN: AS1\r\nX-Client-Country: KP"
 	for _, from := range []string{fromDE, unplaced, "10.0.0.8"} {
-		s.requestWithHeader(from, "/", own, http.StatusOK, requestlog.ActionForward)
+		s.requestWithHeader(from, "/", clientsOwnLookupHeaders,
+			http.StatusOK, requestlog.ActionForward)
 	}
 
 	mu.Lock()
@@ -205,6 +205,43 @@ func TestLookupHeadersArePassedToTheAppAndTheClientsOwnRemoved(t *testing.T) {
 	}
 }
 
+func TestClientsOwnLookupHeadersAreRemovedWhileTheSettingIsOff(t *testing.T) {
+	t.Parallel()
+
+	var (
+		mu           sync.Mutex
+		asn, country []string
+	)
+
+	app := startApp(t, func(_ http.ResponseWriter, r *http.Request) {
+		mu.Lock()
+		defer mu.Unlock()
+
+		asn, country = r.Header.Values("X-Client-Asn"), r.Header.Values("X-Client-Country")
+	})
+	geojsURL, _ := startGeoJS(t)
+	addr, out, _ := startProxyWithClock(t, app.URL, geojsURL, time.Now, map[string]string{
+		trustedProxies: trustLocalhost,
+	})
+	s := &sender{t: t, addr: addr, out: out}
+
+	s.requestWithHeader(fromDE, "/", clientsOwnLookupHeaders,
+		http.StatusOK, requestlog.ActionForward)
+
+	mu.Lock()
+	defer mu.Unlock()
+
+	if asn != nil || country != nil {
+		t.Errorf("the app was passed X-Client-ASN %v and X-Client-Country %v, want neither",
+			asn, country)
+	}
+}
+
+// clientsOwnLookupHeaders are the X-Client-ASN and X-Client-Country a
+// client sends of its own, each twice, in two cases.
+const clientsOwnLookupHeaders = "X-Client-ASN: AS1\r\nx-client-asn: AS2\r\n" +
+	"X-CLIENT-COUNTRY: KP\r\nx-client-country: CN"
+
 // waitUntil waits until done reports true, for at most waitLimit.
 func waitUntil(done func() bool) {
 	deadline := time.Now().Add(waitLimit)

internal/proxy/request.go

@@ -293,8 +293,9 @@ func (rq *request) forward(ctx context.Context) {
 
 // rewrite makes the request the app receives: the client's request,
 // unchanged, sent to SWWAF_UPSTREAM_URL, with the forwarded headers and
-// the request's id set, and, while SWWAF_ADD_LOOKUP_HEADERS is set, the
-// client's AS number and country.
+// the request's id set, without any X-Client-ASN or X-Client-Country the
+// client sent, whatever SWWAF_ADD_LOOKUP_HEADERS says, and, while it is
+// set, with the client's AS number and country in them.
 func (rq *request) rewrite(pr *httputil.ProxyRequest) {
 	upstream := rq.h.config.UpstreamURL
 	pr.Out.URL.Scheme = upstream.Scheme
@@ -304,6 +305,8 @@ func (rq *request) rewrite(pr *httputil.ProxyRequest) {
 	pr.Out.URL.RawQuery = pr.In.URL.RawQuery
 	setForwardedHeaders(pr.In, pr.Out, rq.peer, rq.peerTrusted)
 	pr.Out.Header.Set(requestIDHeader, rq.line.RequestID)
+	pr.Out.Header.Del(asnHeader)
+	pr.Out.Header.Del(countryHeader)
 
 	if rq.h.config.AddLookupHeaders {
 		setLookupHeaders(pr.Out.Header, rq.line.ASN, rq.line.Country)

script/example-app

@@ -7,9 +7,10 @@
 # request bans for good, that `sv stop` stops smallwebwaf in order, that
 # `docker stop` stops the container without having to kill it, and that
 # a new container on the same volume still refuses the banned client. The
-# containers, the volume and both images are removed however the script
-# ends. Building the app needs network access, for nixpkgs' binary cache.
-# script/check does not run this.
+# containers run with SWWAF_LOOKUP_SOURCE=off, so that no address is sent
+# to GeoJS. The containers, the volume and both images are removed however
+# the script ends. Building the app needs network access, for nixpkgs'
+# binary cache. script/check does not run this.
 set -eu
 
 SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)"
@@ -63,12 +64,13 @@ logged() {
 }
 
 # start_container: run the app's container, with the state files on the
-# volume and a rate limit of one request a minute, and wait until it is
-# healthy.
+# volume, a rate limit of one request a minute and no client looked up,
+# and wait until it is healthy.
 start_container() {
     docker run --detach --name "$CONTAINER" --publish 127.0.0.1::8080 \
         --volume "$VOLUME:/var/lib/smallwebwaf" \
         --env SWWAF_RATE_LIMIT_PER_MINUTE=1 \
+        --env SWWAF_LOOKUP_SOURCE=off \
         "$APP_IMAGE" >/dev/null
     wait_for "the health check did not pass" healthy
     address="$(docker port "$CONTAINER" 8080/tcp)"