A ban in bans.json that ends at or before its start is refused, as other malformed entries are (closes #150) #153

Merged
clawbot merged 1 commits from issue-150-ban-expires-before-start into next 2026-10-20 17:42:02 +02:00
3 changed files with 37 additions and 15 deletions
+15 -13
View File
@@ -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`, 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`, `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 `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 `fetched`. So does a ban whose `expires` is not after its `start` or whose
`crowdsec`, alerts waiting for a destination that is not `webhook`, `slack` or `cause` is not `limit`, `attack`, `admin` or `crowdsec`, alerts waiting for a
`ntfy`, an anomaly counter whose `scope` is not `client`, `net`, `asn`, `total` destination that is not `webhook`, `slack` or `ntfy`, an anomaly counter whose
or `watch`, and a copy of a list with a line that would make its fetch fail. An `scope` is not `client`, `net`, `asn`, `total` or `watch`, and a copy of a list
answer's `asn` or `as_name` left out reads as empty. 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 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 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 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 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 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 an `expires` not after its `start` or another `cause`, names another
`scope` or gives a list's copy a line that would make its fetch fail, does not destination, gives an anomaly counter another `scope` or gives a list's copy a
stop the running `smallwebwaf`: it keeps what it holds, and at the file's next line that would make its fetch fail, does not stop the running `smallwebwaf`: it
write renames your file to `<name>.bad`, such as `bans.json.bad`, writes the keeps what it holds, and at the file's next write renames your file to
file again from memory, logs the file and where the error is, and raises a `<name>.bad`, such as `bans.json.bad`, writes the file again from memory, logs
`file_error` alert for it. It waits for that write because an editor's file can the file and where the error is, and raises a `file_error` alert for it. It
be read before the editor has finished writing it. Mend the `.bad` file and move waits for that write because an editor's file can be read before the editor has
it back. A file you remove is written again at its next write. 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` 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 and its `expires`, `null` for a ban that never ends; its `reason` and its
+7 -2
View File
@@ -61,6 +61,7 @@ var (
// errMissing is for an entry without a field it needs. // errMissing is for an entry without a field it needs.
errMissing = errors.New("has no") errMissing = errors.New("has no")
errCause = errors.New("is not limit, attack, admin or crowdsec") 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") errDestination = errors.New("is not webhook, slack or ntfy")
errScope = errors.New("is not client, net, asn, total or watch") errScope = errors.New("is not client, net, asn, total or watch")
errWaitingList = errors.New(`waiting is a list, but now lists the alerts by ` + 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 // 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 // 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 // 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, // each expires is read again as written. An expires not after the start
// attack, admin or crowdsec, most likely misspelt, is refused too. // 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 { func (f *bansFile) check(data []byte) error {
var written struct { var written struct {
Bans []struct { Bans []struct {
@@ -668,6 +671,8 @@ func (f *bansFile) check(data []byte) error {
return missing(i, "start") return missing(i, "start")
case written.Bans[i].Expires == nil: case written.Bans[i].Expires == nil:
return missing(i, "expires") 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 && case entry.Cause != "" && entry.Cause != bans.CauseLimit &&
entry.Cause != bans.CauseAttack && entry.Cause != bans.CauseAdmin && entry.Cause != bans.CauseAttack && entry.Cause != bans.CauseAdmin &&
entry.Cause != bans.CauseCrowdSec: entry.Cause != bans.CauseCrowdSec:
+15
View File
@@ -865,6 +865,21 @@ func TestBanWithAnotherCauseStopsTheStart(t *testing.T) {
`: entry 4's cause "atack" is not limit, attack, admin or crowdsec`) `: 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) { func TestUnknownVersionStopsTheStart(t *testing.T) {
t.Parallel() t.Parallel()