Admin endpoints for bans and clients on the single listener (closes #27)

SWWAF_ADMIN_TOKEN, or its _FILE form, opens GET and POST
/_smallwebwaf/bans, DELETE /_smallwebwaf/bans/<client> and GET
/_smallwebwaf/clients/<ip>. Unset, they answer 404; a missing or wrong
token gets 401, in observe mode too. They go through every check, as
the metrics do. POST takes a netblock, not IPv4-mapped and without a
zone, or a client's address, a duration or permanent, and a reason, and
makes an admin ban even while another lasts. DELETE lifts every active
ban covering the address, kept and marked lifted. Bans come back as
bans.json entries; a client as clients.json holds it, with its bans.

Judgement call: answers leave out bans.json's version field.
Judgement call: DELETE takes an address, not a netblock.
Rule suppressed: gosec G304 on a test reading bans.json.

Model: opus-5-5

README.md

@@ -748,8 +748,10 @@ request that carries the token as `Authorization: Bearer <token>`:
   its IPv6 /64. `duration` is a duration such as `1h` or `7d`, or `permanent`.
   The ban starts at once, its `cause` is `admin`, and it is made even while
   another ban on the netblock lasts. A body that is not such an object, has
-  another field, or is longer than 4 KiB is answered `400`, saying what is
-  wrong.
+  another field, has anything but whitespace after the object, or is longer than
+  4 KiB is answered `400`, saying what is wrong, and so is an IPv4-mapped
+  netblock, such as `::ffff:203.0.113.0/120`, or a value with a zone, such as
+  `fe80::1%eth0`.
 - `DELETE /_smallwebwaf/bans/<client>`: lifts every active ban on a netblock
   that `<client>`, an address, is in, as adding `lifted` to its entry in
   `bans.json` does, and answers `404` when no ban on it is active.

internal/proxy/admin.go

@@ -1,6 +1,7 @@
 package proxy
 
 import (
+	"bytes"
 	"crypto/subtle"
 	"encoding/json"
 	"errors"
@@ -8,6 +9,7 @@ import (
 	"io"
 	"net/http"
 	"net/netip"
+	"os"
 	"strings"
 	"time"
 
@@ -27,8 +29,13 @@ const banBodyMaxBytes = 4 << 10
 const permanent = "permanent"
 
 var (
+	errNotBanToAdd = errors.New(
+		"the body is not a JSON object of netblock, duration and reason")
 	errNotNetblock = errors.New(
 		"is not an address or a netblock, such as 203.0.113.9 or 203.0.113.0/24")
+	errMappedNetblock = errors.New(
+		"is IPv4-mapped: give the IPv4 netblock, such as 203.0.113.0/24")
+	errZone        = errors.New("has a zone, which a netblock cannot have")
 	errNotDuration = errors.New(
 		"is not a duration above zero, such as 1h or 7d, or permanent")
 	errNotAddress = errors.New("is not an address, such as 203.0.113.9")
@@ -111,6 +118,10 @@ type banToAdd struct {
 // an admin, from now for the duration the body gives, with its reason,
 // and answers with that ban.
 func (rq *request) addBan() {
+	// The body must arrive within SWWAF_CLIENT_REQUEST_TIMEOUT, as any
+	// other request's must.
+	rq.stopReadingBody(rq.clientRequestDeadline())
+
 	toAdd, err := rq.readBanToAdd()
 	if refused := rq.refused.Load(); refused != nil {
 		rq.answer(*refused) // the body is over SWWAF_REQUEST_MAX_BYTES
@@ -118,6 +129,16 @@ func (rq *request) addBan() {
 		return
 	}
 
+	if errors.Is(err, os.ErrDeadlineExceeded) {
+		rq.answer(refusal{
+			status: http.StatusRequestTimeout,
+			action: requestlog.ActionTimedOut,
+			limit:  "SWWAF_CLIENT_REQUEST_TIMEOUT",
+		})
+
+		return
+	}
+
 	var (
 		netblock netip.Prefix
 		expires  time.Time
@@ -142,23 +163,33 @@ func (rq *request) addBan() {
 	rq.answerBans([]bans.Ban{ban})
 }
 
-// readBanToAdd reads the body of POST BansPath, at most banBodyMaxBytes
-// of it.
+// readBanToAdd reads the body of POST BansPath: a JSON object with
+// nothing but whitespace after it, in at most banBodyMaxBytes.
 func (rq *request) readBanToAdd() (banToAdd, error) {
 	var body io.ReadCloser = http.NoBody
 	if rq.body != nil {
 		body = rq.body
 	}
 
+	data, err := io.ReadAll(http.MaxBytesReader(nil, body, banBodyMaxBytes))
+	if err != nil {
+		return banToAdd{}, fmt.Errorf("%w: %w", errNotBanToAdd, err)
+	}
+
 	var toAdd banToAdd
 
-	decoder := json.NewDecoder(http.MaxBytesReader(nil, body, banBodyMaxBytes))
+	decoder := json.NewDecoder(bytes.NewReader(data))
 	decoder.DisallowUnknownFields()
 
-	err := decoder.Decode(&toAdd)
+	err = decoder.Decode(&toAdd)
 	if err != nil {
-		return banToAdd{}, fmt.Errorf(
-			"the body is not a JSON object of netblock, duration and reason: %w", err)
+		return banToAdd{}, fmt.Errorf("%w: %w", errNotBanToAdd, err)
+	}
+
+	// Token returns io.EOF only when nothing but whitespace is left.
+	_, err = decoder.Token()
+	if !errors.Is(err, io.EOF) {
+		return banToAdd{}, fmt.Errorf("%w: more follows the object", errNotBanToAdd)
 	}
 
 	return toAdd, nil
@@ -166,18 +197,30 @@ func (rq *request) readBanToAdd() (banToAdd, error) {
 
 // banNetblock reads value, a netblock such as 203.0.113.0/24, or a
 // client's address, which stands for the netblock a ban on that client
-// covers.
+// covers. An IPv4-mapped netblock, such as ::ffff:203.0.113.0/120, is
+// refused, since a client's address is looked up as IPv4 and a ban on it
+// would refuse nothing, and so is a value with a zone.
 func (h *handler) banNetblock(value string) (netip.Prefix, error) {
 	netblock, err := netip.ParsePrefix(value)
 	if err == nil {
+		if netblock.Addr().Is4In6() {
+			return netip.Prefix{}, fmt.Errorf("netblock %q %w", value, errMappedNetblock)
+		}
+
 		return netblock, nil
 	}
 
+	// ParsePrefix refuses a zone, but ParseAddr reads the /48 of
+	// 2001:db8::1%x/48 as part of the zone.
 	addr, err := netip.ParseAddr(value)
 	if err != nil {
 		return netip.Prefix{}, fmt.Errorf("netblock %q %w", value, errNotNetblock)
 	}
 
+	if addr.Zone() != "" {
+		return netip.Prefix{}, fmt.Errorf("netblock %q %w", value, errZone)
+	}
+
 	return h.netblock(addr), nil
 }
 

internal/proxy/admin_test.go

@@ -173,7 +173,9 @@ func TestBanToAddGivesItsNetblockAndDuration(t *testing.T) {
 		{"198.51.100.7/16", "1h", "198.51.0.0/16", time.Hour},
 		{"2001:db8:6::/48", "1h", "2001:db8:6::/48", time.Hour},
 	} {
-		body := `{"netblock": "` + tc.netblock + `", "duration": "` + tc.duration + `"}`
+		// Whitespace may follow the object.
+		body := `{"netblock": "` + tc.netblock + `", "duration": "` + tc.duration + `"}` +
+			"\r\n"
 		want := state.BanEntry{
 			Netblock: netip.MustParsePrefix(tc.want), Start: start, Cause: bans.CauseAdmin,
 		}
@@ -203,6 +205,29 @@ func TestBanToAddThatCannotBeReadIsRefused(t *testing.T) {
 			`{"netblock": "203.0.113", "duration": "1h"}`,
 			`netblock "203.0.113" is not an address or a netblock`,
 		},
+		// A client's address is looked up as IPv4, so a ban on an
+		// IPv4-mapped netblock would refuse nothing.
+		{
+			`{"netblock": "::ffff:203.0.113.0/120", "duration": "1h"}`,
+			`netblock "::ffff:203.0.113.0/120" is IPv4-mapped`,
+		},
+		// Read as an address, its zone would be "x/48", and its ban on the
+		// /64 around it.
+		{
+			`{"netblock": "2001:db8::1%x/48", "duration": "1h"}`,
+			`netblock "2001:db8::1%x/48" has a zone`,
+		},
+		{
+			`{"netblock": "fe80::1%eth0", "duration": "1h"}`,
+			`netblock "fe80::1%eth0" has a zone`,
+		},
+		// Anything but whitespace after the object.
+		{
+			`{"netblock": "203.0.113.9", "duration": "1h"}` +
+				`{"netblock": "198.51.100.0/24", "duration": "1h"}`,
+			"more follows the object",
+		},
+		{`{"netblock": "203.0.113.9", "duration": "1h"} x`, "more follows the object"},
 		{`{"duration": "1h"}`, `netblock "" is not an address or a netblock`},
 		{`{"netblock": "203.0.113.9"}`, `duration "" is not a duration above zero`},
 		{
@@ -218,12 +243,16 @@ func TestBanToAddThatCannotBeReadIsRefused(t *testing.T) {
 			`duration "forever" is not a duration above zero, such as 1h or 7d, ` +
 				`or permanent`,
 		},
-		// Over the 4 KiB read of a body.
+		// Over the 4 KiB read of a body, even when the object comes first.
 		{
 			`{"netblock": "203.0.113.9", "duration": "1h", "reason": "` +
 				strings.Repeat("x", 4<<10) + `"}`,
 			"request body too large",
 		},
+		{
+			`{"netblock": "203.0.113.9", "duration": "1h"}` + strings.Repeat(" ", 4<<10),
+			"request body too large",
+		},
 	} {
 		got := s.admin(http.MethodPost, proxy.BansPath, tc.body, http.StatusBadRequest)
 		if !strings.Contains(string(got.body), tc.want) {
@@ -257,6 +286,36 @@ func TestBanToAddOverTheRequestSizeLimitIsRefused(t *testing.T) {
 	}
 }
 
+func TestBanToAddSlowerThanTheClientRequestTimeoutIsRefused(t *testing.T) {
+	t.Parallel()
+
+	s, _, server := startWithClock(t, "", map[string]string{
+		adminToken:           adminSecret,
+		metricsToken:         token,
+		clientRequestTimeout: shortTimeoutSetting,
+	})
+
+	// The chunk announces 256 bytes and the rest of it never comes, so only
+	// the timeout ends the wait. A hold-up of the test process can only
+	// make the answer later, so the time is checked only for not being
+	// shorter than the timeout.
+	start := time.Now()
+
+	s.adminRequest(adminClient, adminBearer+"\r\nTransfer-Encoding: chunked",
+		http.MethodPost, proxy.BansPath, "100\r\n"+`{"netblock": "203.0.113.9", `,
+		http.StatusRequestTimeout, requestlog.ActionTimedOut)
+
+	if took := time.Since(start); took < shortTimeout {
+		t.Errorf("answered after %s, before the timeout of %s ran out", took, shortTimeout)
+	}
+
+	wantLimitHits(t, s.addr, clientRequestTimeout, 1)
+
+	if held := server.Ledger.Snapshot(); len(held) != 0 {
+		t.Errorf("the ledger holds %+v, want no ban", held)
+	}
+}
+
 func TestClientEndpointShowsTheClientAndItsBans(t *testing.T) {
 	t.Parallel()