Re: ipblacklist V2

[Michael Tremer <michael.tremer@ipfire.org>]

[-- Attachment #1: Type: text/plain, Size: 5603 bytes --]

Hello Stefan,

Thank you for diving into this.

As you will all have seen, Stefan and Peter have recently migrated things away from xt_geoip and used ipset which is also being used for the blocklists. It would be great to combine all the code so that we have to maintain as little as possible.

On that note, something that I didn’t bring up yet, but which should probably be raised is that “blacklist” is probably not a good name any more in 2022. We use that term in many other places and didn’t change it yet, however, I would like to lead with a good example and add new code only under the more current names. I don’t have the intention to change the existing places myself, but I would be happy to accept a patch just so that we are consistent. I also believe that “blocklist” and “allowlist” describe the whole functionality much better than black and white.

I am not sure whether it is a good idea to rename everything rather sooner or at the end of the development process. Frankly it doesn’t matter, but it would be cool if the patchset put forward for merging into next would have this change.

On top of that I have a number of random things we would need to consider:

https://git.ipfire.org/?p=ipfire-2.x.git;a=blob;f=config/kernel/kernel.config.x86_64-ipfire;hb=6e2c8f48182c169edb177526d7f639b0631d57cc#l1272

> CONFIG_IP_SET_MAX=256

This is the maximum number of ipsets the kernel is able to load at a time. Are we in any danger that we might hit this limit? If some (and yes, they shouldn’t, but the bloody do) enables all countries on the location filter that brings them already to 260. I would say that we should potentially raise this to 1024 or 2048 depending on how many people might make a lot of use of the AS block feature when that arrives.

I also stumbled over the hash size parameter. Stefan reported that the new DROP HOSTILE feature is slowing down his network, which is quite likely happening because of us hardcoding the hash size to 1024 (which is also the default for ipset).

Investigating any further, and testing a little bit around with memory overhead, I rewrite this in libloc:

  https://git.ipfire.org/?p=location/libloc.git;a=commitdiff;h=47de14b01a0fae69994131e4fb9239738570ab38

We are now counting the networks that we export (which is a little bit complicated, but doesn’t matter in this context) and then calculate the correct hash size with a factor. Choosing that factor is now the hard part and I have decided to start with 0.75. That means that we will definitely waste some memory, but we will hopefully not have to perform any linked list search whatsoever. Maybe this will need some adjustment, but it looks good to me. See the comment message for more details.

In IP blocklist, we have a factor of one which we potentially might want to look at as well, since those lists will be hit a lot. Has anyone conducted any benchmarks already?

-Michael

> On 28 Feb 2022, at 19:32, Stefan Schantl <stefan.schantl(a)ipfire.org> wrote:
> 
> Hello Rob, Hello Tim, Hello *,
> 
> as anounced on this list, I'm currently working on getting the
> ipblacklist feature as a core component into IPFire.
> 
> I already had a look on the code, which looks nice and very clean to
> me. As I'm currently also working on getting all ipset related set
> stuff and rule creation under one hood, this perfectly fits to this.
> 
> So my idea to put the ipblacklist feature over the line, was to split
> some parts of the ipblacklist "main script" (especially the ipset and
> iptables related stuff) into the perl-based script which is responsible
> for iptables rule creation.
> 
> In this case some other parts of the script (which where necessary in
> the past, because ipblacklist initial has been designed as an addon)
> also can be stipped.
> 
> Affected parts for example would be the "start", "stop", "enable" and
> "disable" code, which is not longer required and therefore safely can
> be dropped.
> 
> In the very end the main task for the script would be to download,
> update, convert and store the blacklists into an ipset compatible
> format.
> 
> Apart from this, I currently do not see any bigger changes for the WUI
> related stuff.
> 
> @Tim: I hope these changes are okay for you.
> 
> Getting started, I noticed, that there currently are two git
> repositories available, which contain the source for ipblacklist.
> 
> There is the origin one from Tim and a slightly modified (fixed) v3
> version from Rob. I' currently trying to determine, which one would be
> the best to start from - are there any deeper changes/differences
> between them?
> 
> Please feel free to ask any kind of questions or share you opinion. As
> usual, I'll share any progress here.
> 
> Best regards,
> 
> -Stefan 
>> Hi.
>> 
>> I have been looking at Tim FitzGeorge's code for ipblacklist v2 on 
>> https://patchwork.ipfire.org/project/ipfire/list/?series=1215 to see
>> if I 
>> can help progress its incorporation into IPFire.
>> After I extracted the programs from Patchwork I have been able to
>> build them 
>> into my firewall where they are running very successfully.
>> The code on the server seems to be in good shape and apart from a few
>> small 
>> patches and additions of a few missing scripts I think it could be 
>> successfully introduced into the IPFire code base.
>> I am more than happy to help in seeing this process carried out but
>> need to 
>> know if this is acceptable to yourselves.
>> 
>> Regards
>> Rob Brewer
> 
>