diff --git a/internal/bans/admin_test.go b/internal/bans/admin_test.go index d919772..4393f6c 100644 --- a/internal/bans/admin_test.go +++ b/internal/bans/admin_test.go @@ -61,6 +61,59 @@ func TestAdminsBansAreNeverDroppedAndDoNotCountTowardMaxBans(t *testing.T) { wantBans(t, ledger, map[netip.Prefix]int{adminsOnly: 2, both: 1, second: 0, third: 1}) } +func TestEarlierBansCountABanDroppedAfterAnAdminsBan(t *testing.T) { + t.Parallel() + + rules := defaultRules() + rules.MaxBans = 2 + ledger := bans.New(rules) + netblock := netip.MustParsePrefix("203.0.113.1/32") + other := netip.MustParsePrefix("203.0.113.2/32") + + // An admin bans the netblock for a minute, and it then breaks a limit + // twice, each time after its last ban has ended. + ledger.BanForAdmin(netblock, midnight(), midnight().Add(time.Minute), + "probes for logins") + first, _ := ledger.BanForLimit(netblock, midnight().Add(time.Minute), bans.Notes{}) + second, _ := ledger.BanForLimit(netblock, first.Expires, bans.Notes{}) + + // A ban on another netblock fills the ledger, and the netblock's first + // ban for a limit, its earliest that is not an admin's, goes. + ledger.BanForLimit(other, second.Start, bans.Notes{}) + wantBans(t, ledger, map[netip.Prefix]int{netblock: 2, other: 1}) + + // The netblock's next ban still counts it. + third, _ := ledger.BanForLimit(netblock, second.Expires, bans.Notes{}) + if third.Notes.EarlierBans != (bans.EarlierBans{Limit: 2, Admin: 1}) { + t.Errorf("the third ban for a limit has earlier bans %+v, "+ + "want 2 for a limit and 1 an admin's", third.Notes.EarlierBans) + } +} + +func TestEarlierBansCountABanAnAdminKeepsOnce(t *testing.T) { + t.Parallel() + + ledger := bans.New(defaultRules()) + netblock := netip.MustParsePrefix("203.0.113.1/32") + + // The netblock breaks a limit twice, the second time after its first + // ban has ended, and the second ban's notes count the first. An admin + // then keeps the first ban. + first, _ := ledger.BanForLimit(netblock, midnight(), bans.Notes{}) + second, _ := ledger.BanForLimit(netblock, first.Expires, bans.Notes{}) + + kept := first + kept.Cause = bans.CauseAdmin + ledger.LoadEdit([]bans.Ban{kept, second}) + + // The netblock's next ban counts the kept ban once, as an admin's. + third, _ := ledger.BanForLimit(netblock, second.Expires, bans.Notes{}) + if third.Notes.EarlierBans != (bans.EarlierBans{Limit: 1, Admin: 1}) { + t.Errorf("the third ban for a limit has earlier bans %+v, "+ + "want 1 for a limit and 1 an admin's", third.Notes.EarlierBans) + } +} + func TestReasonOfTheBansSmallwebwafMakes(t *testing.T) { t.Parallel() diff --git a/internal/bans/bans.go b/internal/bans/bans.go index c07b52f..e3848d8 100644 --- a/internal/bans/bans.go +++ b/internal/bans/bans.go @@ -687,26 +687,51 @@ func (l *Ledger) ban( } // earlierBans returns how many bans a netblock with the bans held, oldest -// first, has had, by cause: the first ban held counts the bans the -// netblock had before that one, since dropped to make room, and each ban -// held adds one. +// first, has had, by cause. Each ban held gives a count: the bans its +// notes count before it, and one for it and each ban held after it. A +// ban's count misses a ban dropped after it to make room, and that of an +// admin's ban added to bans.json without notes misses the bans before it, +// so the count that names the most bans is taken, whole: a ban an admin +// keeps by giving it CauseAdmin is still counted under its old cause in +// later bans' notes, and taking each cause from another count would count +// it twice. func earlierBans(held []Ban) EarlierBans { - earlier := held[0].Notes.EarlierBans + // fromHere counts ban and the bans held after it. + var most, fromHere EarlierBans - for _, ban := range held { + for _, ban := range slices.Backward(held) { switch ban.Cause { case CauseLimit: - earlier.Limit++ + fromHere.Limit++ case CauseAttack: - earlier.Attack++ + fromHere.Attack++ case CauseAdmin: - earlier.Admin++ + fromHere.Admin++ case CauseCrowdSec: - earlier.CrowdSec++ + fromHere.CrowdSec++ + } + + notes := ban.Notes.EarlierBans + count := EarlierBans{ + Limit: notes.Limit + fromHere.Limit, + Attack: notes.Attack + fromHere.Attack, + Admin: notes.Admin + fromHere.Admin, + CrowdSec: notes.CrowdSec + fromHere.CrowdSec, + } + + // Of counts that name as many bans, the earlier ban's is taken: it + // counts more of the bans held by the cause they have now. + if count.total() >= most.total() { + most = count } } - return earlier + return most +} + +// total returns how many bans e counts, of all causes. +func (e EarlierBans) total() int { + return e.Limit + e.Attack + e.Admin + e.CrowdSec } // markChanged has Changed receive a value, unless one is waiting already.