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, every file every
SWWAF_STATE_COUNTER_INTERVAL and at the stop, each through a synced
temporary file renamed over it. A file that does not parse, an unknown
version or an unwritable directory stops the start. The image gets
/var/lib/smallwebwaf, which the run script gives to the smallwebwaf user.

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

Model: opus-5-5

README.md

@@ -283,17 +283,14 @@ 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 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.
+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.
 
 ## Why
 

internal/bans/bans.go

@@ -107,17 +107,11 @@ type Ledger struct {
 	changed chan struct{}
 
 	mu sync.Mutex
-	// netblocks holds each banned netblock's bans, oldest first. Check
-	// makes each netblock it finds the most recently seen.
+	// netblocks holds each banned netblock's bans, oldest first. Each
+	// request from a netblock makes it 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.
@@ -142,36 +136,29 @@ func (l *Ledger) Changed() <-chan struct{} {
 	return l.changed
 }
 
-// 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) {
+// 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) {
 	l.mu.Lock()
 	defer l.mu.Unlock()
 
-	lengths := l.v6Lengths
-	if client.Is4() {
-		lengths = l.v4Lengths
+	bans, found := l.netblocks.Get(netblock)
+	if !found {
+		return Ban{}, false
 	}
 
-	for _, length := range lengths {
-		bans, found := l.netblocks.Get(netip.PrefixFrom(client, length).Masked())
-		if !found {
-			continue
-		}
-
-		// 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) {
-			last.Notes.Requests++
-			last.Notes.Refused++
-
-			return *last, true
-		}
+	// 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
 	}
 
-	return Ban{}, false
+	last.Notes.Requests++
+	last.Notes.Refused++
+
+	return *last, true
 }
 
 // BanForLimit bans netblock at now for a broken limit, with notes, and
@@ -249,10 +236,9 @@ 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 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.
+// 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.
 func (l *Ledger) Load(bans []Ban) {
 	l.mu.Lock()
 	defer l.mu.Unlock()
@@ -263,7 +249,6 @@ 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)
 	}
@@ -286,15 +271,6 @@ 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.Addr(), now.Add(100*365*day))
+	_, banned := ledger.Check(netblock, 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.Addr(), ban.Expires.Add(-time.Nanosecond))
+		got, banned := ledger.Check(netblock, 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.MustParseAddr("203.0.113.10"), midnight())
+	_, banned := ledger.Check(netip.MustParsePrefix("203.0.113.10/32"), midnight())
 	if banned {
 		t.Error("another netblock is banned")
 	}
 
-	_, banned = ledger.Check(netblock.Addr(), ban.Expires)
+	_, banned = ledger.Check(netblock, 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.Addr(), now)
+	ledger.Check(a, 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.Addr(), first.Expires)
+	ledger.Check(d, 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.Addr(), midnight().Add(time.Minute))
+	ledger.Check(netblock, 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.Addr(), ban.Expires.Add(-time.Second))
+	_, banned := after.Check(netblock, ban.Expires.Add(-time.Second))
 	if !banned {
 		t.Error("the loaded ban does not refuse")
 	}
@@ -88,48 +88,6 @@ 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 a netblock the client is in refuses
-// the request at now, and notes for the log line when that ban ends.
+// 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.
 func (rq *request) banned(now time.Time) bool {
-	ban, banned := rq.h.ledger.Check(rq.client, now)
+	ban, banned := rq.h.ledger.Check(rq.netblock(), now)
 	if banned {
 		rq.line.BanExpires = banExpires(ban)
 	}

internal/smallwebwaf/smallwebwaf.go

@@ -164,12 +164,8 @@ func serve(
 		return 1
 	}
 
-	// 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.
+	// Every request has ended, and so has Run's last write: the files
+	// written now hold everything, and nothing else writes them.
 	<-written
 
 	err = files.WriteAll()

internal/smallwebwaf/smallwebwaf_test.go

@@ -26,11 +26,10 @@ const (
 	// testVersion is the version the tests give smallwebwaf.
 	testVersion = "test"
 	// localhost is where the tests listen.
-	localhost       = "127.0.0.1"
-	listenAddr      = "SWWAF_LISTEN_ADDR"
-	upstreamURL     = "SWWAF_UPSTREAM_URL"
-	stateDir        = "SWWAF_STATE_DIR"
-	rateLimitPerDay = "SWWAF_RATE_LIMIT_PER_DAY"
+	localhost   = "127.0.0.1"
+	listenAddr  = "SWWAF_LISTEN_ADDR"
+	upstreamURL = "SWWAF_UPSTREAM_URL"
+	stateDir    = "SWWAF_STATE_DIR"
 	// greeting is what the tests' app answers.
 	greeting = "hello from the app"
 )
@@ -189,10 +188,10 @@ func TestStateKeptAcrossRestarts(t *testing.T) {
 	t.Parallel()
 
 	env := map[string]string{
-		listenAddr:      localhost + ":0",
-		upstreamURL:     startApp(t),
-		stateDir:        t.TempDir(),
-		rateLimitPerDay: "2",
+		listenAddr:                 localhost + ":0",
+		upstreamURL:                startApp(t),
+		stateDir:                   t.TempDir(),
+		"SWWAF_RATE_LIMIT_PER_DAY": "2",
 		// Neither comes due in the test: the files are written as
 		// smallwebwaf stops.
 		"SWWAF_STATE_WRITE_DELAY":      "1h",
@@ -219,46 +218,6 @@ 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()
 
@@ -377,7 +336,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",
-		rateLimitPerDay:                         "50000",
+		"SWWAF_RATE_LIMIT_PER_DAY":              "50000",
 		"SWWAF_DENIED_COUNTRIES":                "",
 		"SWWAF_EXCLUSIVELY_ALLOWED_COUNTRIES":   "",
 		"SWWAF_BAN_RESPONSE":                    "403",
@@ -450,31 +409,3 @@ 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,7 +10,6 @@ import (
 	"slices"
 	"strings"
 	"testing"
-	"testing/synctest"
 	"time"
 
 	"sneak.berlin/go/smallwebwaf/internal/bans"
@@ -20,6 +19,10 @@ 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"
@@ -228,78 +231,41 @@ func TestUnwritableDirectoryStopsTheStart(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) {
+func TestBanWrittenAfterTheWriteDelay(t *testing.T) {
 	t.Parallel()
 
-	synctest.Test(t, func(t *testing.T) {
-		dir := t.TempDir()
-		params := newParams(dir)
-		params.WriteDelay = 10 * time.Second
-		run(t, load(t, params))
+	dir := t.TempDir()
+	params := newParams(dir)
+	params.WriteDelay = 10 * time.Millisecond
+	run(t, load(t, params))
 
-		// 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{})
+	ban := params.Ledger.BanForLimit(netip.MustParsePrefix("203.0.113.9/32"),
+		midnight(), bans.Notes{Limit: 1})
 
-		time.Sleep(5 * time.Second)
+	waitForFile(t, filepath.Join(dir, bansJSON))
 
-		second := params.Ledger.BanForLimit(netip.MustParsePrefix("203.0.113.10/32"),
-			midnight(), bans.Notes{})
+	read := newParams(dir)
+	load(t, read)
 
-		time.Sleep(5*time.Second - time.Nanosecond)
-		synctest.Wait()
-		wantFiles(t, dir)
+	if got := read.Ledger.Snapshot(); !slices.Equal(got, []bans.Ban{ban}) {
+		t.Errorf("bans.json holds %+v, want %+v", got, ban)
+	}
 
-		time.Sleep(time.Nanosecond)
-		synctest.Wait()
-		wantFiles(t, dir, bansJSON)
-
-		read := newParams(dir)
-		load(t, read)
-
-		want := []bans.Ban{first, second}
-		if got := read.Ledger.Snapshot(); !slices.Equal(got, want) {
-			t.Errorf("bans.json holds %+v, want %+v", got, want)
-		}
-
-		// 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)
-	})
+	// The other files wait for the interval, an hour away.
+	wantFiles(t, dir, bansJSON)
 }
 
 func TestEveryFileWrittenEveryCounterInterval(t *testing.T) {
 	t.Parallel()
 
-	synctest.Test(t, func(t *testing.T) {
-		dir := t.TempDir()
-		params := newParams(dir)
-		params.CounterInterval = time.Minute
-		run(t, load(t, params))
+	dir := t.TempDir()
+	params := newParams(dir)
+	params.CounterInterval = 10 * time.Millisecond
+	run(t, load(t, params))
 
-		// 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)
-		}
-	})
+	for _, file := range []string{bansJSON, clientsJSON, lookupsJSON} {
+		waitForFile(t, filepath.Join(dir, file))
+	}
 }
 
 func TestFailedWriteLeavesTheFileAsItWas(t *testing.T) {
@@ -474,16 +440,21 @@ func readFile(t *testing.T, path string) string {
 	return string(data)
 }
 
-// removeFiles removes the named files from dir.
-func removeFiles(t *testing.T, dir string, names ...string) {
+// waitForFile waits for the file at path to exist.
+func waitForFile(t *testing.T, path string) {
 	t.Helper()
 
-	for _, name := range names {
-		err := os.Remove(filepath.Join(dir, name))
-		if err != nil {
-			t.Fatalf("remove: %v", err)
+	deadline := time.Now().Add(waitLimit)
+	for time.Now().Before(deadline) {
+		_, err := os.Stat(path)
+		if err == nil {
+			return
 		}
+
+		time.Sleep(pollInterval)
 	}
+
+	t.Fatalf("no %s after %s", path, waitLimit)
 }
 
 // wantFiles checks the names of the files in dir.