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.

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.
Judgement call: a client's own X-Client-* headers are removed only while SWWAF_ADD_LOOKUP_HEADERS is set.

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 to GeoJS. `file`, for the IPinfo Lite database, comes with
+  address anywhere. `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,10 +288,9 @@ 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. A request then waits for
+  `X-Client-Country`, leaving out one that is unknown, and removes any such
-  its client's first answer, as it does while a country list is set. Whatever
+  headers the client sent. A request then waits for its client's first answer,
-  this setting says, any `X-Client-ASN` or `X-Client-Country` the client sent,
+  as it does while a country list is set.
-  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,58 +119,6 @@ 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()
 
@@ -819,16 +767,6 @@ 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,10 +9,9 @@ import (
 )
 
 // The headers in which the app is passed the client's AS number and
-// country while SWWAF_ADD_LOOKUP_HEADERS is set. Go writes every header
+// country while SWWAF_ADD_LOOKUP_HEADERS is set. Go sends a header name in
-// name in this form, as it sends it and as it receives it, so X-Client-ASN
+// this form, so X-Client-ASN arrives as X-Client-Asn; header names are not
-// arrives as X-Client-Asn, and Del removes a client's own whatever their
+// case-sensitive.
-// case; header names are not case-sensitive.
 const (
 	asnHeader     = "X-Client-Asn"
 	countryHeader = "X-Client-Country"
@@ -46,8 +45,12 @@ 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.
+// 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.
 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, "/", clientsOwnLookupHeaders,
+		s.requestWithHeader(from, "/", own, http.StatusOK, requestlog.ActionForward)
-			http.StatusOK, requestlog.ActionForward)
 	}
 
 	mu.Lock()
@@ -205,43 +205,6 @@ 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,9 +293,8 @@ 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, without any X-Client-ASN or X-Client-Country the
+// the request's id set, and, while SWWAF_ADD_LOOKUP_HEADERS is set, the
-// client sent, whatever SWWAF_ADD_LOOKUP_HEADERS says, and, while it is
+// client's AS number and country.
-// 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
@@ -305,8 +304,6 @@ 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,10 +7,9 @@
 # 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 run with SWWAF_LOOKUP_SOURCE=off, so that no address is sent
+# containers, the volume and both images are removed however the script
-# to GeoJS. The containers, the volume and both images are removed however
+# ends. Building the app needs network access, for nixpkgs' binary cache.
-# the script ends. Building the app needs network access, for nixpkgs'
+# script/check does not run this.
-# binary cache. script/check does not run this.
 set -eu
 
 SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)"
@@ -64,13 +63,12 @@ logged() {
 }
 
 # start_container: run the app's container, with the state files on the
-# volume, a rate limit of one request a minute and no client looked up,
+# volume and a rate limit of one request a minute, and wait until it is
-# and wait until it is healthy.
+# 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)"