Country allow and deny lists, looked up through GeoJS (closes #44)

SWWAF_DENIED_COUNTRIES and SWWAF_EXCLUSIVELY_ALLOWED_COUNTRIES refuse a
request with 403 before its body is read or rate-limited, logged as
country_denied. internal/lookup asks GeoJS only while a list is set, 200
clients per request, one at a time, keeping answers 7 days. Failures, a
redirect or an answer leaving an address out included, are logged without
addresses; GeoJS is then left alone a second, doubling to five minutes.
Private, loopback and link-local clients are never sent.

Deviation, per the issue: no SWWAF_LOOKUP_SOURCE or SWWAF_LOOKUP_TIMEOUT; 403, not SWWAF_BAN_RESPONSE.
Deviation: GeoJS's country endpoint, not geo.json.
Judgement call: an IPv6 /64 is asked about by its first address; at most 10,000 clients wait.
Judgement call: config.go lists the ISO 3166-1 codes; no widely used library holds them.

Model: opus-5-5

README.md

@@ -417,19 +417,21 @@ is not answering, new visitors count as coming from an unknown country, which
 To keep your visitors' addresses on your own host, set
 `SWWAF_LOOKUP_SOURCE=off`, or use the database file instead of GeoJS:
 `SWWAF_LOOKUP_SOURCE=file` reads the free IPinfo Lite database
-(`ipinfo_lite.mmdb`). You download it with your own IPinfo account, mount the
-directory that holds it into the container, point `SWWAF_LOOKUP_DB_PATH` at the
-file and refresh it when you choose; `smallwebwaf` never downloads it itself,
-and reads it again when you replace it. It has to be the directory rather than
-the file itself: docker does not show a single mounted file being replaced, so a
-refresh would go unseen. IPinfo releases it under the Creative Commons
-Attribution-ShareAlike 4.0 International License and asks for attribution, in
-its own words on https://ipinfo.io/lite: "The attribution requirements can be
-met by giving our service credit as your data source. Simply place a link to
-IPinfo on the website, application, or social media account that uses our data."
-Its example of such a credit is a link mentioning "IP address data is powered by
-IPinfo". A service that uses the database through `smallwebwaf` should carry
-that link.
+(`ipinfo_lite.mmdb`). `SWWAF_LOOKUP_SOURCE` comes in milestone 3 or later (see
+the build order in [`SPEC.md`](SPEC.md)); until then GeoJS is asked only while a
+country list is set. You download the database with your own IPinfo account,
+mount the directory that holds it into the container, point
+`SWWAF_LOOKUP_DB_PATH` at the file and refresh it when you choose; `smallwebwaf`
+never downloads it itself, and reads it again when you replace it. It has to be
+the directory rather than the file itself: docker does not show a single mounted
+file being replaced, so a refresh would go unseen. IPinfo releases it under the
+Creative Commons Attribution-ShareAlike 4.0 International License and asks for
+attribution, in its own words on https://ipinfo.io/lite: "The attribution
+requirements can be met by giving our service credit as your data source. Simply
+place a link to IPinfo on the website, application, or social media account that
+uses our data." Its example of such a credit is a link mentioning "IP address
+data is powered by IPinfo". A service that uses the database through
+`smallwebwaf` should carry that link.
 
 Neither source can place a private address, so a client on one, such as a
 visitor on your local network, another container or your monitoring, has no
@@ -459,8 +461,7 @@ refusal comes with `SWWAF_ALLOW_NETS` in milestone 3 or later.
 
 Besides the Go standard library, `github.com/hashicorp/golang-lru/v2` keeps the
 table of clients to 20,000 and the GeoJS answers to 100,000, dropping the least
-recently seen, and `golang.org/x/text/language` knows which two-letter codes are
-countries.
+recently seen. The country codes are the list in `internal/config/config.go`.
 
 ## Entrypoints
 

internal/lookup/lookup.go

@@ -340,15 +340,18 @@ func (g *GeoJS) request(
 	ctx, cancel := context.WithTimeout(ctx, timeout)
 	defer cancel()
 
-	req, err := http.NewRequestWithContext(ctx, http.MethodGet,
-		g.url+"?ip="+strings.Join(addrs, ","), http.NoBody)
+	req, err := http.NewRequestWithContext(ctx, http.MethodGet, g.url, http.NoBody)
 	if err != nil {
 		return nil, fmt.Errorf("make the request to GeoJS: %w", err)
 	}
 
+	req.URL.RawQuery = "ip=" + strings.Join(addrs, ",")
+
 	res, err := g.httpClient.Do(req)
 	if err != nil {
-		return nil, fmt.Errorf("ask GeoJS: %w", err)
+		// Do's error names the URL, and so the visitors' addresses, which
+		// are not to be logged: only what went wrong is kept.
+		return nil, fmt.Errorf("ask GeoJS: %w", errors.Unwrap(err))
 	}
 
 	defer func() {

internal/lookup/lookup_test.go

@@ -168,6 +168,27 @@ func TestCountryIsKeptInCapitals(t *testing.T) {
 	wantCountry(t, g, netip.MustParsePrefix("203.0.113.9/32"), germany)
 }
 
+func TestFailureIsLoggedWithoutTheAddressesAskedAbout(t *testing.T) {
+	t.Parallel()
+
+	var log strings.Builder
+
+	// Nothing listens on port 1, so asking GeoJS fails.
+	g := lookup.New(lookup.Params{
+		URL:        "http://127.0.0.1:1",
+		Now:        time.Now,
+		ProcessLog: slog.New(slog.NewTextHandler(&log, nil)),
+	})
+
+	wantCountry(t, g, netip.MustParsePrefix("203.0.113.9/32"), "")
+
+	logged := log.String()
+	if !strings.Contains(logged, "asking GeoJS failed") ||
+		strings.Contains(logged, "203.0.113.9") {
+		t.Errorf("logged %q, want the failure without the address asked about", logged)
+	}
+}
+
 func TestWaitingClientsAreAskedAboutInOneRequest(t *testing.T) {
 	t.Parallel()
 
@@ -203,7 +224,9 @@ func TestKeptAnswersUnaffectedWhileGeoJSFailsAndAskedAgainWithBackoff(t *testing
 	wantRequests(t, geojs, 1)
 
 	// Each failure leaves GeoJS alone twice as long as the one before, up
-	// to five minutes. New clients meanwhile count as not found.
+	// to five minutes. New clients meanwhile count as not found, and the
+	// client with a kept answer still gets its country, without GeoJS being
+	// asked.
 	requests := 1
 
 	for _, delay := range []time.Duration{
@@ -218,6 +241,7 @@ func TestKeptAnswersUnaffectedWhileGeoJSFailsAndAskedAgainWithBackoff(t *testing
 
 		clock.advance(delay - time.Millisecond)
 		wantCountry(t, g, clients(), "")
+		wantCountry(t, g, kept, germany)
 		wantRequests(t, geojs, requests)
 
 		clock.advance(time.Millisecond)

internal/proxy/countries_test.go

@@ -10,6 +10,7 @@ import (
 	"sync"
 	"sync/atomic"
 	"testing"
+	"time"
 
 	"sneak.berlin/go/smallwebwaf/internal/requestlog"
 )
@@ -84,7 +85,7 @@ func TestCountryLists(t *testing.T) {
 	}
 }
 
-func TestCountryRefusalComesBeforeTheBodyAndTheRateLimits(t *testing.T) {
+func TestCountryRefusalComesBeforeTheBody(t *testing.T) {
 	t.Parallel()
 
 	var calls atomic.Int32
@@ -94,25 +95,19 @@ func TestCountryRefusalComesBeforeTheBodyAndTheRateLimits(t *testing.T) {
 	})
 	geojsURL, _ := startGeoJS(t)
 	addr, out := startProxyWithGeoJS(t, app.URL, geojsURL, map[string]string{
-		trustedProxies:     trustLocalhost,
-		deniedCountries:    "kp",
-		rateLimitPerMinute: "1",
+		trustedProxies:  trustLocalhost,
+		deniedCountries: "kp",
 	})
 
-	// With a limit of one request a minute, the second request would be
-	// over it, were refused requests counted.
-	for i := range 2 {
-		req := newRequest(t, http.MethodPost, addr, "/", strings.NewReader("a body"))
-		req.Header.Set(forwardedFor, fromKP)
-		wantStatus(t, do(t, req), http.StatusForbidden)
+	req := newRequest(t, http.MethodPost, addr, "/", strings.NewReader("a body"))
+	req.Header.Set(forwardedFor, fromKP)
+	wantStatus(t, do(t, req), http.StatusForbidden)
 
-		line := out.requestLines(t, i+1)[i]
-		wantLine(t, line, http.StatusForbidden, requestlog.ActionCountryDenied)
+	line := out.requestLine(t)
+	wantLine(t, line, http.StatusForbidden, requestlog.ActionCountryDenied)
 
-		if line.RequestBytes != 0 || line.LimitHit != "" {
-			t.Errorf("log line has request_bytes %d and limit_hit %q, want 0 and none",
-				line.RequestBytes, line.LimitHit)
-		}
+	if line.RequestBytes != 0 {
+		t.Errorf("log line has request_bytes %d, want 0", line.RequestBytes)
 	}
 
 	if calls.Load() != 0 {
@@ -120,6 +115,65 @@ func TestCountryRefusalComesBeforeTheBodyAndTheRateLimits(t *testing.T) {
 	}
 }
 
+func TestRequestRefusedByCountryIsNotCounted(t *testing.T) {
+	t.Parallel()
+
+	// The stand-in for GeoJS fails until placing is set, and then places
+	// every address in Germany.
+	var placing atomic.Bool
+
+	geojs := httptest.NewServer(http.HandlerFunc(
+		func(w http.ResponseWriter, r *http.Request) {
+			if !placing.Load() {
+				w.WriteHeader(http.StatusServiceUnavailable)
+
+				return
+			}
+
+			answer := []map[string]string{{"ip": r.URL.Query().Get("ip"), "country": "DE"}}
+
+			err := json.NewEncoder(w).Encode(answer)
+			if err != nil {
+				http.Error(w, err.Error(), http.StatusInternalServerError)
+			}
+		}))
+	t.Cleanup(geojs.Close)
+
+	app := startApp(t, func(http.ResponseWriter, *http.Request) {})
+	addr, _ := startProxyWithGeoJS(t, app.URL, geojs.URL, map[string]string{
+		trustedProxies:     trustLocalhost,
+		allowedCountries:   "de",
+		rateLimitPerMinute: "1",
+	})
+
+	request := func() answer {
+		req := newRequest(t, http.MethodGet, addr, "/", http.NoBody)
+		req.Header.Set(forwardedFor, fromDE)
+
+		return do(t, req)
+	}
+
+	// While GeoJS fails, the client's country cannot be found, and its
+	// request is refused.
+	wantStatus(t, request(), http.StatusForbidden)
+
+	// Once GeoJS places it, a second after the failure, its requests are let
+	// through. No refused one was counted, so the first let through is
+	// within the limit of one a minute.
+	placing.Store(true)
+
+	deadline := time.Now().Add(waitLimit)
+	got := request()
+
+	for got.status == http.StatusForbidden && time.Now().Before(deadline) {
+		time.Sleep(pollInterval)
+
+		got = request()
+	}
+
+	wantStatus(t, got, http.StatusOK)
+}
+
 func TestCountryNotLookedUpWithoutAListOrForAPrivateAddress(t *testing.T) {
 	t.Parallel()
 

internal/proxy/passthrough_test.go

@@ -246,8 +246,9 @@ func TestUpgradedConnectionOutlastsTheTimeouts(t *testing.T) {
 		t.Fatalf("status %d, want %d", res.StatusCode, http.StatusSwitchingProtocols)
 	}
 
-	// Wait past every timeout, then use the connection.
-	time.Sleep(3 * shortTimeout)
+	// Every timeout started before the upgrade was answered: wait past
+	// them all, then use the connection.
+	time.Sleep(3 * shortTimeout / 2)
 	send(t, conn, "still here\n")
 
 	echoed, err := reader.ReadString('\n')

internal/proxy/proxy_test.go

@@ -21,11 +21,15 @@ import (
 
 const (
 	// shortTimeout is what a test sets a timeout to, to see it run out.
-	shortTimeout = 300 * time.Millisecond
+	// It starts before the test has set up its case, such as an upgrade
+	// or the app's buffers filling, so it is as long as the hold-up of the
+	// test process that wantTimedOut allows, waitLimit/2: a shorter one
+	// can run out first on a busy host.
+	shortTimeout = 5 * time.Second
 	// shortTimeoutSetting is shortTimeout as a setting's value.
-	shortTimeoutSetting = "300ms"
+	shortTimeoutSetting = "5s"
 	// longTimeoutSetting is a timeout that does not run out in a test.
-	longTimeoutSetting = "10s"
+	longTimeoutSetting = "1m"
 	// waitLimit bounds how long a test waits for what should happen.
 	waitLimit = 10 * time.Second
 	// pollInterval is how often a test looks for a log line.