stamparm / ipsum

Daily feed of bad IPs (with blacklist hit scores)
The Unlicense
1.58k stars 145 forks source link

Update `ipset` example #28

Closed neon-dev closed 2 years ago

neon-dev commented 2 years ago

The ipset script looked to be designed so that it could be executed repeatedly without issues, due to the flush statement and -q parameters. However, doing so would generate duplicate iptables entries and temporarily disable the blocklist while new IPs were being fetched. The new script solves these issues and uses apt-get to avoid a CLI warning when executed from cron.

If this new example script is too complex I would instead suggest removing the ipset -q flush ipsum line and make it more clear that it's not a ready-to-deploy script.

PS: Thanks for your work and the constant updates, greatly appreciated.

neon-dev commented 2 years ago

@stamparm I think something went wrong or did you not want to update the readme with the latest force-push?

stamparm commented 2 years ago

@neon-dev repository is force cleaned each 24h, to prevent the build up of data (because of diffs). please write here what you wanted to suggest

neon-dev commented 2 years ago

Yes I know, that's why I expected your latest force-push would include some changes in the readme, but it didn't. This is the change I proposed: https://github.com/neon-dev/ipsum/commit/66afab0bf698bc66e1a81b5013fb29c2402bee9a

stamparm commented 2 years ago

can you please elaborate: However, doing so would generate duplicate iptables entries?

also, i don't see any issue in temporarily disable the blocklist while new IPs were being fetched, so will just ignore that part TBH

stamparm commented 2 years ago

it is clear enough from the following that it's not a ready-to-deploy script.:

image

neon-dev commented 2 years ago

can you please elaborate

Of course: It will only happen if you run the commands more than once, since iptables -I will insert a new rule each time. And because of the reasons I explained in the PR description, to me the script implied it should be safe to execute it multiple times.

stamparm commented 2 years ago

with latest revision: image

neon-dev commented 2 years ago

Looks good, thanks 👍