Keep the bans, the clients and GeoJS's answers in state files (closes #17)

smallwebwaf now copies its state to bans.json, clients.json and
lookups.json in SWWAF_STATE_DIR, as "Persistent state" in SPEC.md
describes, and reads them back at start, so a restart lifts no ban and
gives no client a fresh allowance. Each client gains a history, and a
ban's notes count the netblock's requests. bans.json is written
SWWAF_STATE_WRITE_DELAY after a ban, and every file every
SWWAF_STATE_COUNTER_INTERVAL and at the stop. A ban read back is masked
to its netblock and refuses every client in it, whatever
SWWAF_BAN_SCOPE_V4_PREFIX is now. A file that does not parse, an unknown
version or an unwritable directory stops the start.

Deviation: no AS number or name, and no ban cause, reason or lifting yet.

Model: opus-5-5

README.md

@@ -283,14 +283,17 @@ new one, whole. A write that fails is logged, and tried again at the next write.
 A hard kill loses what changed since the last write.
 
 At start the files are read back: each client keeps its counts, so a restart
-gives it no fresh allowance, and each ban keeps refusing until it ends. Buckets
-and answers whose time has passed are dropped. A missing file is empty state, as
-on a first start. A file that does not parse, or has another `version`, stops
-the start with a message naming the file, and the line and column where Go's
-JSON decoder gives them; so does a state directory `smallwebwaf` cannot write.
-An edit made while `smallwebwaf` runs is overwritten by its next write: taking
-it in comes with https://git.eeqj.de/sneak/smallwebwaf/issues/68. The AS number
-and AS name come with their lookup.
+gives it no fresh allowance, and each ban keeps refusing every client in its
+netblock until it ends, even after `SWWAF_BAN_SCOPE_V4_PREFIX` has changed. A
+netblock whose address has bits past its length, such as `203.0.113.9/24`, is
+read as the netblock it is in, `203.0.113.0/24`. Buckets and answers whose time
+has passed are dropped. A missing file is empty state, as on a first start. A
+file that does not parse, or has another `version`, stops the start with a
+message naming the file, and the line and column where Go's JSON decoder gives
+them; so does a state directory `smallwebwaf` cannot write. An edit made while
+`smallwebwaf` runs is overwritten by its next write: taking it in comes with
+https://git.eeqj.de/sneak/smallwebwaf/issues/68. The AS number and AS name come
+with their lookup.
 
 ## Why
 

internal/bans/bans.go

@@ -107,11 +107,17 @@ type Ledger struct {
 	changed chan struct{}
 
 	mu sync.Mutex
-	// netblocks holds each banned netblock's bans, oldest first. Each
-	// request from a netblock makes it the most recently seen.
+	// netblocks holds each banned netblock's bans, oldest first. Check
+	// makes each netblock it finds the most recently seen.
 	netblocks *simplelru.LRU[netip.Prefix, *[]Ban]
 	// held is how many bans netblocks holds, at most rules.MaxBans.
 	held int
+	// v4Lengths and v6Lengths are the lengths of the IPv4 and IPv6
+	// netblocks that have been banned. Check looks for a ban at each of
+	// them, so that a ban read from bans.json refuses every client in its
+	// netblock even when it was made with another SWWAF_BAN_SCOPE_V4_PREFIX,
+	// or another length of an IPv6 client's netblock.
+	v4Lengths, v6Lengths []int
 }
 
 // New returns a Ledger with no ban yet.
@@ -136,30 +142,37 @@ func (l *Ledger) Changed() <-chan struct{} {
 	return l.changed
 }
 
-// Check is called for each request from netblock, at now. It reports
-// whether a ban on netblock is active, and returns that ban, with the
-// request counted among those it refused.
-func (l *Ledger) Check(netblock netip.Prefix, now time.Time) (Ban, bool) {
+// Check is called for each request from client, at now. It reports
+// whether a ban on a netblock client is in is active, and returns that
+// ban, with the request counted among those it refused.
+func (l *Ledger) Check(client netip.Addr, now time.Time) (Ban, bool) {
 	l.mu.Lock()
 	defer l.mu.Unlock()
 
-	bans, found := l.netblocks.Get(netblock)
+	lengths := l.v6Lengths
+	if client.Is4() {
+		lengths = l.v4Lengths
+	}
+
+	for _, length := range lengths {
+		bans, found := l.netblocks.Get(netip.PrefixFrom(client, length).Masked())
 		if !found {
-		return Ban{}, false
+			continue
 		}
 
-	// A ban is made only once the one before has ended, so only the last
-	// can be active.
+		// A ban is made only once the one before has ended, so only the
+		// last can be active.
 		last := &(*bans)[len(*bans)-1]
-	if !last.ActiveAt(now) {
-		return Ban{}, false
-	}
-
+		if last.ActiveAt(now) {
 			last.Notes.Requests++
 			last.Notes.Refused++
 
 			return *last, true
 		}
+	}
+
+	return Ban{}, false
+}
 
 // BanForLimit bans netblock at now for a broken limit, with notes, and
 // returns the ban. A first ban lasts LimitBanDuration. A ban made within
@@ -236,9 +249,10 @@ func (l *Ledger) Snapshot() []Ban {
 
 // Load puts bans read from bans.json into a ledger that holds none yet,
 // in the order they started, so that a netblock whose last ban started
-// latest counts as the most recently seen. Each text in their notes is
-// cut to 256 bytes, and past MaxBans the earliest bans are dropped, as
-// when they are made.
+// latest counts as the most recently seen. Each netblock is masked to its
+// length, so that 203.0.113.9/24 is 203.0.113.0/24, and each text in the
+// notes is cut to 256 bytes. Past MaxBans the earliest bans are dropped,
+// as when they are made.
 func (l *Ledger) Load(bans []Ban) {
 	l.mu.Lock()
 	defer l.mu.Unlock()
@@ -249,6 +263,7 @@ func (l *Ledger) Load(bans []Ban) {
 	})
 
 	for _, ban := range bans {
+		ban.Netblock = ban.Netblock.Masked()
 		ban.Notes.Request = ban.Notes.Request.cut()
 		l.add(ban)
 	}
@@ -271,6 +286,15 @@ func (l *Ledger) add(ban Ban) {
 
 	*bans = append(*bans, ban)
 	l.held++
+
+	lengths := &l.v6Lengths
+	if ban.Netblock.Addr().Is4() {
+		lengths = &l.v4Lengths
+	}
+
+	if !slices.Contains(*lengths, ban.Netblock.Bits()) {
+		*lengths = append(*lengths, ban.Netblock.Bits())
+	}
 }
 
 // expiry returns when a ban for a broken limit made at now ends, or zero

internal/bans/bans_test.go

@@ -39,7 +39,7 @@ func TestRepeatsTripleUntilPermanent(t *testing.T) {
 		t.Fatalf("sixth ban ends at %s, want a permanent one", ban.Expires)
 	}
 
-	_, banned := ledger.Check(netblock, now.Add(100*365*day))
+	_, banned := ledger.Check(netblock.Addr(), now.Add(100*365*day))
 	if !banned {
 		t.Error("a permanent ban ended")
 	}
@@ -138,18 +138,18 @@ func TestCheckRefusesWhileTheBanLastsAndCountsTheRefusals(t *testing.T) {
 	ban := ledger.BanForLimit(netblock, midnight(), bans.Notes{Requests: 5})
 
 	for range 3 {
-		got, banned := ledger.Check(netblock, ban.Expires.Add(-time.Nanosecond))
+		got, banned := ledger.Check(netblock.Addr(), ban.Expires.Add(-time.Nanosecond))
 		if !banned || got.Start != ban.Start {
 			t.Fatalf("check during the ban gives %+v and %t", got, banned)
 		}
 	}
 
-	_, banned := ledger.Check(netip.MustParsePrefix("203.0.113.10/32"), midnight())
+	_, banned := ledger.Check(netip.MustParseAddr("203.0.113.10"), midnight())
 	if banned {
 		t.Error("another netblock is banned")
 	}
 
-	_, banned = ledger.Check(netblock, ban.Expires)
+	_, banned = ledger.Check(netblock.Addr(), ban.Expires)
 	if banned {
 		t.Error("the ban did not end")
 	}
@@ -180,7 +180,7 @@ func TestMaxBansDropsTheEarliestBanOfTheNetblockSeenLongestAgo(t *testing.T) {
 
 	// A request from a makes b the netblock seen longest ago, and its ban
 	// goes to make room for d's.
-	ledger.Check(a, now)
+	ledger.Check(a.Addr(), now)
 	ledger.BanForLimit(d, now, bans.Notes{})
 	wantBans(t, ledger, map[netip.Prefix]int{a: 1, b: 0, c: 1, d: 1})
 
@@ -190,7 +190,7 @@ func TestMaxBansDropsTheEarliestBanOfTheNetblockSeenLongestAgo(t *testing.T) {
 
 	// With d seen since, a is seen longest ago, and its earlier ban goes
 	// first.
-	ledger.Check(d, first.Expires)
+	ledger.Check(d.Addr(), first.Expires)
 	ledger.BanForLimit(b, first.Expires, bans.Notes{})
 	wantBans(t, ledger, map[netip.Prefix]int{a: 1, b: 1, d: 1})
 

internal/bans/snapshot_test.go

@@ -24,7 +24,7 @@ func TestChangedAfterABanIsMade(t *testing.T) {
 	// A limit broken during the ban makes no other, and a refusal changes
 	// only the counts in the notes, which wait for the interval's write.
 	ledger.BanForLimit(netblock, midnight().Add(time.Minute), bans.Notes{})
-	ledger.Check(netblock, midnight().Add(time.Minute))
+	ledger.Check(netblock.Addr(), midnight().Add(time.Minute))
 	wantChanged(t, ledger, false)
 
 	// Two bans before the value is read leave one.
@@ -76,7 +76,7 @@ func TestLoadedBansCarryOn(t *testing.T) {
 	after := bans.New(defaultRules())
 	after.Load(before.Snapshot())
 
-	_, banned := after.Check(netblock, ban.Expires.Add(-time.Second))
+	_, banned := after.Check(netblock.Addr(), ban.Expires.Add(-time.Second))
 	if !banned {
 		t.Error("the loaded ban does not refuse")
 	}
@@ -88,6 +88,48 @@ func TestLoadedBansCarryOn(t *testing.T) {
 	}
 }
 
+func TestLoadedBanRefusesEveryClientInItsNetblock(t *testing.T) {
+	t.Parallel()
+
+	// Two entries as an admin might write them, with addresses not masked
+	// to their lengths, the IPv6 one shorter than the /64 an IPv6 client's
+	// ban covers, beside a ban the ledger makes on one IPv4 address.
+	ledger := bans.New(defaultRules())
+	ledger.Load([]bans.Ban{
+		{Netblock: netip.MustParsePrefix("203.0.113.9/24"), Start: midnight()},
+		{Netblock: netip.MustParsePrefix("2001:db8::1/48"), Start: midnight()},
+	})
+	ledger.BanForLimit(netip.MustParsePrefix("198.51.100.7/32"), midnight(), bans.Notes{})
+
+	for client, want := range map[string]bool{
+		"203.0.113.0":     true,
+		"203.0.113.200":   true,
+		"203.0.114.1":     false,
+		"2001:db8:0:5::1": true,
+		"2001:db8:1::1":   false,
+		"198.51.100.7":    true,
+		"198.51.100.8":    false,
+	} {
+		_, banned := ledger.Check(netip.MustParseAddr(client), midnight())
+		if banned != want {
+			t.Errorf("%s is refused: %t, want %t", client, banned, want)
+		}
+	}
+
+	// The loaded netblocks are written back masked.
+	snapshot := ledger.Snapshot()
+
+	got := make([]string, 0, len(snapshot))
+	for _, ban := range snapshot {
+		got = append(got, ban.Netblock.String())
+	}
+
+	want := []string{"198.51.100.7/32", "203.0.113.0/24", "2001:db8::/48"}
+	if !slices.Equal(got, want) {
+		t.Errorf("the ledger holds bans on %v, want %v", got, want)
+	}
+}
+
 func TestLoadKeepsAtMostMaxBansDroppingTheEarliest(t *testing.T) {
 	t.Parallel()
 

internal/proxy/bans.go

@@ -14,10 +14,10 @@ func (rq *request) banResponse(action string) *refusal {
 	return &refusal{status: rq.h.config.BanResponse, action: action}
 }
 
-// banned reports whether a ban on the client's netblock refuses the
-// request at now, and notes for the log line when that ban ends.
+// banned reports whether a ban on a netblock the client is in refuses
+// the request at now, and notes for the log line when that ban ends.
 func (rq *request) banned(now time.Time) bool {
-	ban, banned := rq.h.ledger.Check(rq.netblock(), now)
+	ban, banned := rq.h.ledger.Check(rq.client, now)
 	if banned {
 		rq.line.BanExpires = banExpires(ban)
 	}

internal/smallwebwaf/smallwebwaf.go

@@ -164,8 +164,12 @@ func serve(
 		return 1
 	}
 
-	// Every request has ended, and so has Run's last write: the files
-	// written now hold everything, and nothing else writes them.
+	// Run's last write has ended, so nothing else writes the files. Every
+	// request has ended too, but for two kinds that Go's server does not
+	// wait for: one cut off because Shutdown timed out, and one whose
+	// connection switched protocols, such as a WebSocket. Such a request
+	// adds to its client's history only as it ends, which can be after
+	// this write, and then that request is missing from clients.json.
 	<-written
 
 	err = files.WriteAll()

internal/smallwebwaf/smallwebwaf_test.go

@@ -30,6 +30,7 @@ const (
 	listenAddr      = "SWWAF_LISTEN_ADDR"
 	upstreamURL     = "SWWAF_UPSTREAM_URL"
 	stateDir        = "SWWAF_STATE_DIR"
+	rateLimitPerDay = "SWWAF_RATE_LIMIT_PER_DAY"
 	// greeting is what the tests' app answers.
 	greeting = "hello from the app"
 )
@@ -191,7 +192,7 @@ func TestStateKeptAcrossRestarts(t *testing.T) {
 		listenAddr:      localhost + ":0",
 		upstreamURL:     startApp(t),
 		stateDir:        t.TempDir(),
-		"SWWAF_RATE_LIMIT_PER_DAY": "2",
+		rateLimitPerDay: "2",
 		// Neither comes due in the test: the files are written as
 		// smallwebwaf stops.
 		"SWWAF_STATE_WRITE_DELAY":      "1h",
@@ -218,6 +219,46 @@ func TestStateKeptAcrossRestarts(t *testing.T) {
 	out.line(t, "action", "banned")
 }
 
+func TestBanRefusesItsNetblockAfterARestartWithAnotherScope(t *testing.T) {
+	t.Parallel()
+
+	const scope = "SWWAF_BAN_SCOPE_V4_PREFIX"
+
+	env := map[string]string{
+		listenAddr:              localhost + ":0",
+		upstreamURL:             startApp(t),
+		stateDir:                t.TempDir(),
+		"SWWAF_TRUSTED_PROXIES": localhost + "/32",
+		rateLimitPerDay:         "1",
+		scope:                   "24",
+	}
+
+	// 203.0.113.9's second request breaks the day limit, and bans
+	// 203.0.113.0/24.
+	runUntilStopped(t, env, func(url string) {
+		wantStatus(t, url, "203.0.113.9", http.StatusOK)
+		wantStatus(t, url, "203.0.113.9", http.StatusForbidden)
+	})
+
+	// With each address a netblock of its own after a restart, that ban
+	// still refuses all of 203.0.113.0/24. 198.51.100.7 is banned alone.
+	env[scope] = "32"
+	runUntilStopped(t, env, func(url string) {
+		wantStatus(t, url, "203.0.113.200", http.StatusForbidden)
+		wantStatus(t, url, "203.0.114.1", http.StatusOK)
+		wantStatus(t, url, "198.51.100.7", http.StatusOK)
+		wantStatus(t, url, "198.51.100.7", http.StatusForbidden)
+	})
+
+	// With /24 netblocks again, that ban still refuses 198.51.100.7, and
+	// no other address.
+	env[scope] = "24"
+	runUntilStopped(t, env, func(url string) {
+		wantStatus(t, url, "198.51.100.7", http.StatusForbidden)
+		wantStatus(t, url, "198.51.100.8", http.StatusOK)
+	})
+}
+
 func TestStateFileThatDoesNotParseStopsTheStart(t *testing.T) {
 	t.Parallel()
 
@@ -336,7 +377,7 @@ func wantStartingLine(t *testing.T, line map[string]any, appURL, dir string) {
 		"SWWAF_DENY_NETS":                       "",
 		"SWWAF_RATE_LIMIT_PER_MINUTE":           "1000",
 		"SWWAF_RATE_LIMIT_PER_HOUR":             "10000",
-		"SWWAF_RATE_LIMIT_PER_DAY":              "50000",
+		rateLimitPerDay:                         "50000",
 		"SWWAF_DENIED_COUNTRIES":                "",
 		"SWWAF_EXCLUSIVELY_ALLOWED_COUNTRIES":   "",
 		"SWWAF_BAN_RESPONSE":                    "403",
@@ -409,3 +450,31 @@ func wantRefused(t *testing.T, url string) {
 		t.Errorf("status %d, want %d", res.StatusCode, http.StatusForbidden)
 	}
 }
+
+// wantStatus checks that a request to url from the client at from, as
+// X-Forwarded-For names it, is answered with status.
+func wantStatus(t *testing.T, url, from string, status int) {
+	t.Helper()
+
+	req, err := http.NewRequestWithContext(t.Context(), http.MethodGet, url,
+		http.NoBody)
+	if err != nil {
+		t.Fatalf("new request: %v", err)
+	}
+
+	req.Header.Set("X-Forwarded-For", from)
+
+	transport := &http.Transport{}
+	defer transport.CloseIdleConnections()
+
+	res, err := (&http.Client{Transport: transport}).Do(req)
+	if err != nil {
+		t.Fatalf("request: %v", err)
+	}
+
+	_ = res.Body.Close()
+
+	if res.StatusCode != status {
+		t.Errorf("request from %s: status %d, want %d", from, res.StatusCode, status)
+	}
+}

internal/state/state_test.go

@@ -10,6 +10,7 @@ import (
 	"slices"
 	"strings"
 	"testing"
+	"testing/synctest"
 	"time"
 
 	"sneak.berlin/go/smallwebwaf/internal/bans"
@@ -19,10 +20,6 @@ import (
 )
 
 const (
-	// waitLimit bounds how long a test waits for what should happen.
-	waitLimit = 10 * time.Second
-	// pollInterval is how often a test looks again.
-	pollInterval = 10 * time.Millisecond
 	// The state files.
 	bansJSON    = "bans.json"
 	clientsJSON = "clients.json"
@@ -231,41 +228,78 @@ func TestUnwritableDirectoryStopsTheStart(t *testing.T) {
 	}
 }
 
-func TestBanWrittenAfterTheWriteDelay(t *testing.T) {
+// The two tests below run Run in a synctest bubble, where time is a clock
+// of the test's own: time.Sleep moves it on at once, and synctest.Wait
+// returns once Run waits for its next write, so that every write due by
+// then is on disk.
+
+func TestBansWrittenOnceWriteDelayAfterABan(t *testing.T) {
 	t.Parallel()
 
+	synctest.Test(t, func(t *testing.T) {
 		dir := t.TempDir()
 		params := newParams(dir)
-	params.WriteDelay = 10 * time.Millisecond
+		params.WriteDelay = 10 * time.Second
 		run(t, load(t, params))
 
-	ban := params.Ledger.BanForLimit(netip.MustParsePrefix("203.0.113.9/32"),
-		midnight(), bans.Notes{Limit: 1})
+		// A second ban, made while the first waits to be written, puts the
+		// write off no further, and is written with it.
+		first := params.Ledger.BanForLimit(netip.MustParsePrefix("203.0.113.9/32"),
+			midnight(), bans.Notes{})
 
-	waitForFile(t, filepath.Join(dir, bansJSON))
+		time.Sleep(5 * time.Second)
+
+		second := params.Ledger.BanForLimit(netip.MustParsePrefix("203.0.113.10/32"),
+			midnight(), bans.Notes{})
+
+		time.Sleep(5*time.Second - time.Nanosecond)
+		synctest.Wait()
+		wantFiles(t, dir)
+
+		time.Sleep(time.Nanosecond)
+		synctest.Wait()
+		wantFiles(t, dir, bansJSON)
 
 		read := newParams(dir)
 		load(t, read)
 
-	if got := read.Ledger.Snapshot(); !slices.Equal(got, []bans.Ban{ban}) {
-		t.Errorf("bans.json holds %+v, want %+v", got, ban)
+		want := []bans.Ban{first, second}
+		if got := read.Ledger.Snapshot(); !slices.Equal(got, want) {
+			t.Errorf("bans.json holds %+v, want %+v", got, want)
 		}
 
-	// The other files wait for the interval, an hour away.
-	wantFiles(t, dir, bansJSON)
+		// That write was the only one: bans.json is not written again for
+		// the second ban. The other files wait for the interval, an hour
+		// away.
+		removeFiles(t, dir, bansJSON)
+		time.Sleep(params.WriteDelay)
+		synctest.Wait()
+		wantFiles(t, dir)
+	})
 }
 
 func TestEveryFileWrittenEveryCounterInterval(t *testing.T) {
 	t.Parallel()
 
+	synctest.Test(t, func(t *testing.T) {
 		dir := t.TempDir()
 		params := newParams(dir)
-	params.CounterInterval = 10 * time.Millisecond
+		params.CounterInterval = time.Minute
 		run(t, load(t, params))
 
-	for _, file := range []string{bansJSON, clientsJSON, lookupsJSON} {
-		waitForFile(t, filepath.Join(dir, file))
+		// The files are removed once written, so that each interval shows
+		// them written again.
+		for range 3 {
+			time.Sleep(time.Minute - time.Nanosecond)
+			synctest.Wait()
+			wantFiles(t, dir)
+
+			time.Sleep(time.Nanosecond)
+			synctest.Wait()
+			wantFiles(t, dir, bansJSON, clientsJSON, lookupsJSON)
+			removeFiles(t, dir, bansJSON, clientsJSON, lookupsJSON)
 		}
+	})
 }
 
 func TestFailedWriteLeavesTheFileAsItWas(t *testing.T) {
@@ -440,21 +474,16 @@ func readFile(t *testing.T, path string) string {
 	return string(data)
 }
 
-// waitForFile waits for the file at path to exist.
-func waitForFile(t *testing.T, path string) {
+// removeFiles removes the named files from dir.
+func removeFiles(t *testing.T, dir string, names ...string) {
 	t.Helper()
 
-	deadline := time.Now().Add(waitLimit)
-	for time.Now().Before(deadline) {
-		_, err := os.Stat(path)
-		if err == nil {
-			return
+	for _, name := range names {
+		err := os.Remove(filepath.Join(dir, name))
+		if err != nil {
+			t.Fatalf("remove: %v", err)
 		}
-
-		time.Sleep(pollInterval)
 	}
-
-	t.Fatalf("no %s after %s", path, waitLimit)
 }
 
 // wantFiles checks the names of the files in dir.