diff --git a/internal/bans/admin_test.go b/internal/bans/admin_test.go index d919772..5afefb3 100644 --- a/internal/bans/admin_test.go +++ b/internal/bans/admin_test.go @@ -61,6 +61,35 @@ 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 TestReasonOfTheBansSmallwebwafMakes(t *testing.T) { t.Parallel() diff --git a/internal/bans/bans.go b/internal/bans/bans.go index c07b52f..d4e9e88 100644 --- a/internal/bans/bans.go +++ b/internal/bans/bans.go @@ -687,26 +687,35 @@ 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 each cause gets the largest of these counts. 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 + most.Limit = max(most.Limit, notes.Limit+fromHere.Limit) + most.Attack = max(most.Attack, notes.Attack+fromHere.Attack) + most.Admin = max(most.Admin, notes.Admin+fromHere.Admin) + most.CrowdSec = max(most.CrowdSec, notes.CrowdSec+fromHere.CrowdSec) } - return earlier + return most } // markChanged has Changed receive a value, unless one is waiting already.