Bouncer does not block when ban gets readded

[cuthulino]

I run an instance of Uptime-Kuma and added the IP of the Container to my crowdsec banlist.
Like this I can check the functionality of the firewall bouncer with hitting the secured host with a tcp request and get a timeout because the IP is blocked. If the blocking does not work anymore, I get an notification.
The secured host uses nftables as firewall.

I created a cron job, adding the descision every night with 25h duration. Like this I am sure, the ban does never end.

But now the strange things happen.

• the firewallbouncer blocks everything like expected
• after 24h I get notified the block is no longer working
• when the decision is readded at the second night, the ban starts working again

In crowdsec itself i can list the decision and my manually added one is there all the time.

It looks to me like the firewallbouncer adds a rule with the bantime at time x.
When the decision gets added again at time y the bouncer just checks that the ip is already banned but does not extend or rewrite the bantime of the nftables rule to "y + bantime". Therefore the rule expires after "x + bantime".
(just my impression)

[LaurenceJJones]

Attempt to replicate doesn't seem to be an issue with the actual LAPI. It may be an issue adding the IP to the ruleset if the IP already exists so most likely need to implement a check if it does then update it??

╭─loz repo: crowdsec on  master [!] via  v1.20.2 as 🧙 took 11ms
╰─λ sudo cscli decisions add --ip 1.2.3.4
INFO[16-03-2023 13:54:32] Decision successfully added

╭─loz repo: crowdsec on  master [!] via  v1.20.2 as 🧙 took 85ms
╰─λ curl -H "X-Api-Key: $C_API_KEY" "http://127.0.0.1:8080/v1/decisions/stream?startup=true"
{"deleted":null,"new":[{"duration":"3h59m58.43935709s","id":452794,"origin":"cscli","scenario":"manual 'ban' from '6df9e06234fa428d9ba8e78f9b557c4aGkQkN2JSuj3A6d7I'","scope":"Ip","type":"ban","uuid":"21040800-6d54-4d87-b4f4-db560636cea0","value":"1.2.3.4"}]}⏎                                                        

╭─loz repo: crowdsec on  master [!] via  v1.20.2 as 🧙 took 11ms
╰─λ curl -H "X-Api-Key: $C_API_KEY" "http://127.0.0.1:8080/v1/decisions/stream?startup=false"
{"deleted":null,"new":null}⏎           
                                                                  
╭─loz repo: crowdsec on  master [!] via  v1.20.2 as 🧙 took 11ms
╰─λ sudo cscli decisions add --ip 1.2.3.4
INFO[16-03-2023 13:54:47] Decision successfully added

╭─loz repo: crowdsec on  master [!] via  v1.20.2 as 🧙 took 89ms
╰─λ curl -H "X-Api-Key: $C_API_KEY" "http://127.0.0.1:8080/v1/decisions/stream?startup=false"
{"deleted":null,"new":[{"duration":"3h59m57.396779288s","id":452795,"origin":"cscli","scenario":"manual 'ban' from '6df9e06234fa428d9ba8e78f9b557c4aGkQkN2JSuj3A6d7I'","scope":"Ip","type":"ban","uuid":"81ac7236-a96a-4b14-9f6c-aec27ab42fb5","value":"1.2.3.4"}]}⏎      
                                                 

EDIT: checking with the team it most likely an issue just with NFTables as it doesn't like overlapping IP's or ranges so we implemented a check but I guess the check didn't go as fast as checking if the decisions is new as you can run the bouncer in ipset mode only meaning nothing is lost between reboots.

[pdf]

In nftables it is not an error to add a duplicate element to a set (it is an error to add overlapping ranges, but that requires set flag interval, which crowdsec-created sets do not use, and can be mitigiated using the auto-merge attr should ranges be supported in future). However, when adding a duplicate element, the timer will not get updated as it does when doing the same via ipset -exist.

The only way to refresh timers currently appears to be to delete and re-add the element, which is likely to be very slow if performed one element at a time on busy systems, however deleting a non-existent element will produce an error, which exposes some difficulty when performing updates in batches. It may be possible to achieve the desired result while retaining batches by performing a sequence of:

1. add batch (this will always succeed since duplicates are ignored)
2. delete batch (this will succeed since we just added all elems in the previous step)
3. add batch (as we deleted the batch, adding again will set new timer values)

This seems rather inefficient, but is likely still more efficient than performing get/delete/add operations for each elem individually. The current delete code appears to fall back to deleting each elem individually in the case of a flush failure, I assume to handle the non-existent elem issue, however the whole update operation must be performed atomically to avoid exposing the system to attackers between delete and add, so the same logic cannot be applied during add/update, as the flush must happen after all operations are applied, hence the suggested add/delete/add sequence, rather than just delete/add.

---

There does appear to be some work toward supporting timer updates on the netfilter list, however even if merged on the netfilter side, this would require updates on the kernel side too, since timer updates are apparently ignored completely on modern kernels.