diff --git a/README.md b/README.md index d1cbdf4..4d188b5 100644 --- a/README.md +++ b/README.md @@ -1449,11 +1449,12 @@ whole service, its `asn`, for an AS number, its `name`, for a named netblock, or the `start` of a window in which it has requests or bytes; a list's `url`, `fetched` or `lines`, which is `[]` for an empty list; a verdict's `zone`, `client`, `listed`, which is `false` for a client the zone does not list, or -`fetched`. So does a ban whose `cause` is not `limit`, `attack`, `admin` or -`crowdsec`, alerts waiting for a destination that is not `webhook`, `slack` or -`ntfy`, an anomaly counter whose `scope` is not `client`, `net`, `asn`, `total` -or `watch`, and a copy of a list with a line that would make its fetch fail. An -answer's `asn` or `as_name` left out reads as empty. +`fetched`. So does a ban whose `expires` is not after its `start` or whose +`cause` is not `limit`, `attack`, `admin` or `crowdsec`, alerts waiting for a +destination that is not `webhook`, `slack` or `ntfy`, an anomaly counter whose +`scope` is not `client`, `net`, `asn`, `total` or `watch`, and a copy of a list +with a line that would make its fetch fail. An answer's `asn` or `as_name` left +out reads as empty. While it runs, `smallwebwaf` watches `SWWAF_STATE_DIR` and takes in your edit of a state file as soon as you save it: what the file then holds replaces what @@ -1463,14 +1464,15 @@ writes a file it takes in any edit made since, so your edit is not overwritten; a change `smallwebwaf` made after you opened the file, such as a new ban, is lost when you save over it. An edit that would stop the start, because it does not parse, has another `version`, leaves out a field an entry needs, gives a ban -another `cause`, names another destination, gives an anomaly counter another -`scope` or gives a list's copy a line that would make its fetch fail, does not -stop the running `smallwebwaf`: it keeps what it holds, and at the file's next -write renames your file to `.bad`, such as `bans.json.bad`, writes the -file again from memory, logs the file and where the error is, and raises a -`file_error` alert for it. It waits for that write because an editor's file can -be read before the editor has finished writing it. Mend the `.bad` file and move -it back. A file you remove is written again at its next write. +an `expires` not after its `start` or another `cause`, names another +destination, gives an anomaly counter another `scope` or gives a list's copy a +line that would make its fetch fail, does not stop the running `smallwebwaf`: it +keeps what it holds, and at the file's next write renames your file to +`.bad`, such as `bans.json.bad`, writes the file again from memory, logs +the file and where the error is, and raises a `file_error` alert for it. It +waits for that write because an editor's file can be read before the editor has +finished writing it. Mend the `.bad` file and move it back. A file you remove is +written again at its next write. To ban a netblock, add an entry to `bans.json` with its `netblock`, its `start` and its `expires`, `null` for a ban that never ends; its `reason` and its diff --git a/internal/state/state.go b/internal/state/state.go index b662075..a6321c4 100644 --- a/internal/state/state.go +++ b/internal/state/state.go @@ -61,6 +61,7 @@ var ( // errMissing is for an entry without a field it needs. errMissing = errors.New("has no") errCause = errors.New("is not limit, attack, admin or crowdsec") + errExpires = errors.New("is not after its start") errDestination = errors.New("is not webhook, slack or ntfy") errScope = errors.New("is not client, net, asn, total or watch") errWaitingList = errors.New(`waiting is a list, but now lists the alerts by ` + @@ -646,8 +647,10 @@ func (e BanEntry) ban() bans.Ban { // client, a start, from which the length of the netblock's next ban is // worked out, or an expires, which would make it permanent. A permanent // ban's expires is null, which Bans cannot tell from a missing one, so -// each expires is read again as written. A cause other than limit, -// attack, admin or crowdsec, most likely misspelt, is refused too. +// each expires is read again as written. An expires not after the start +// is refused too, since the netblock's next ban for a broken limit, three +// times as long as this one, would then refuse nothing; and so is a cause +// other than limit, attack, admin or crowdsec, most likely misspelt. func (f *bansFile) check(data []byte) error { var written struct { Bans []struct { @@ -668,6 +671,8 @@ func (f *bansFile) check(data []byte) error { return missing(i, "start") case written.Bans[i].Expires == nil: return missing(i, "expires") + case entry.Expires != nil && !entry.Expires.After(entry.Start): + return fmt.Errorf("entry %d's expires %w", i+1, errExpires) case entry.Cause != "" && entry.Cause != bans.CauseLimit && entry.Cause != bans.CauseAttack && entry.Cause != bans.CauseAdmin && entry.Cause != bans.CauseCrowdSec: diff --git a/internal/state/state_test.go b/internal/state/state_test.go index 0a95f2e..0b98d94 100644 --- a/internal/state/state_test.go +++ b/internal/state/state_test.go @@ -865,6 +865,21 @@ func TestBanWithAnotherCauseStopsTheStart(t *testing.T) { `: entry 4's cause "atack" is not limit, attack, admin or crowdsec`) } +func TestBanThatDoesNotEndAfterItsStartStopsTheStart(t *testing.T) { + t.Parallel() + + // The first ban ends a second after its start; the second ends as it + // starts, or an hour before. + for _, expires := range []string{`"2026-10-06T00:00:00Z"`, `"2026-10-05T23:00:00Z"`} { + wantRefused(t, bansJSON, `{"version": 1, "bans": [`+ + `{"netblock": "203.0.113.9/32", "start": "2026-10-06T00:00:00Z", `+ + `"expires": "2026-10-06T00:00:01Z"}, `+ + `{"netblock": "203.0.113.10/32", "start": "2026-10-06T00:00:00Z", `+ + `"expires": `+expires+`}]}`, + `: entry 2's expires is not after its start`) + } +} + func TestUnknownVersionStopsTheStart(t *testing.T) { t.Parallel()