Re: [PATCH 0/5] ipblacklist: IP Address Blacklists

["Peter Müller" <peter.mueller@ipfire.org>]

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

Hello Tim, hello Michael, hello *,

first, I'd like to apologise for the late response. I used to
be more active on this mailing list than I am today, but hopefully
this will be a temporary situation only.

Michael is right, I did not have time to mention my concerns about
fetching the blacklists from external sources. In my opinion, this
causes privacy issues as we disclose the public IP addresses of
all IPFire installations using a blacklist to its provider/vendor.

In my humble opinion, we can rely on a diverse and robust mirror
infrastructure for our Core Updates and packages, so why not use them
for other data such as blacklists or upcoming libloc as well?

We could periodically update the blacklists on our main mirror (and
wait for the network to sync it), make sure it is signed and write
a small downloader that fetches, validates and installs them.

@All: Thoughts on this?

Talking about the preference of packet filter and IPS, I prefer to
use the latter as well as it gains more insight in what kind of malicious
traffic tried to pass a firewall machine. On systems with low resources,
this might be problematic and removing load from the IPS can be preferred
(make this configurable?!), on others, people might want to have both
results.

Regarding the GeoIP block feature, this has always been my strongest
point against it: Neither were dropped packets logged (hope this is
still true, as I have not tried for quite a while), nor is more fine
grained filtering possible - e.g. by limiting this to certain services -,
nor are IPS results available for packets dropped.

We should _always_ log dropped packets (no matter which feature drops them),
and leave it to the user to decide whether traffic from/to blacklisted
IP addresses should traverse the IPS or not.

Personally, I consider Spamhaus DROP/EDROP can be safely enabled by default.
I would love to see the bogon ruleset here, too (think about 8chan successor
hosted at unallocated RIPE space in Saint Petersburg), but that will likely cause
interference if RED does not have a public IP address assigned.

Regarding the code quality, I agree with Michael. Thanks again for providing
this. :-) As soon as we agree on answers to the questions raised meanwhile, I
will add my "Reviewed-by"-tag.

Thanks, and best regards,
Peter Müller


> Hello Tim,
> 
> Thank you for sending in this patchset.
> 
> I think as well that this functionality would be a great addition to the IPS that we have right now and we have talked about this on numerous occasions - including implementation details.
> 
> Now, you have done the whole thing. Well done.
> 
> Before we dive into the code, can I ask a couple of high-level questions?
> 
> Peter is always bringing up that downloading blacklists isn’t a good idea. It has actually become one of the biggest obstacles in our conversations and I am surprised he didn’t bring it up again :)
> 
> The automatic blacklist feature. What is the objective here? We saw value in having traffic even from malicious sources passed to the IPS so that it will examine it and log it. The idea was to have a better picture of the threats instead of just silencing them. Not sure what is best in the end.
> 
> I am unsure how users will deal with this and turn on “all the lists”(TM) and suddenly things do not work any more. How are they meant to figure out a good threshold? Should we not make that decision for them instead?
> 
> About the implementation: Your code is very very clean as always. There are a couple of things that I would like to see changed around how those iptables rules are being inserted into the existing chains. You are adding something into the POLICYIN chain with surgical precision which might break when the chain is being modified. This is potentially all minor stuff and can be fixed in minutes.
> 
> Well done!
> 
> -Michael
> 
>> On 25 Nov 2019, at 20:13, Tim FitzGeorge <ipfr(a)tfitzgeorge.me.uk> wrote:
>>
>> Implements downloading of IP address blacklists and implementing
>> them as IPSets.  A separate IPSet is used for each blacklist; this
>> simplifies handling of overlaps between different lists.  Traffic
>> to or from the red0/ppp0 interface is checked against the IPSets.
>> The check is placed before the IPS check as the IPSet check is
>> much lighter on CPU use which means that overall CPU use is
>> reduced.
>>
>> The available lists are defined in a separate file.  A WUI page
>> allows the desired lists to be enabled and the interval between
>> checks for updates to be defined.  A minimum update check interval
>> is defined for each blacklist in the definition file.
>>
>> Optionally, an automatically updating blacklist can be enabled.
>> This adds addresses to an IPSet if the rate of packets dropped by
>> the default red0/ppp0 input policy exceeds a user defined threshold.
>> The addresses are kept in the IPSet until a user defined period
>> without packets from the blocked address has passed.
>>
>> Tim FitzGeorge (5):
>>  ipblacklist: Main script
>>  ipblacklist: WUI and language file
>>  ipblacklist: Ancillary files
>>  ipblacklist: Modifications to system
>>  ipblacklist: Build infrastructure
>>
>> config/backup/backup.pl                     |    1 +
>> config/backup/include                       |    2 +
>> config/firewall/firewall-policy             |    5 +
>> config/ipblacklist/sources                  |  151 +++
>> config/logwatch/ipblacklist                 |  103 ++
>> config/logwatch/ipblacklist.conf            |   34 +
>> config/menu/50-firewall.menu                |    5 +
>> config/rootfiles/common/aarch64/stage2      |    1 +
>> config/rootfiles/common/configroot          |    2 +
>> config/rootfiles/common/ipblacklist-sources |    1 +
>> config/rootfiles/common/logwatch            |    2 +
>> config/rootfiles/common/misc-progs          |    2 +
>> config/rootfiles/common/stage2              |    1 +
>> config/rootfiles/common/web-user-interface  |    1 +
>> config/rootfiles/common/x86_64/stage2       |    1 +
>> html/cgi-bin/ipblacklist.cgi                |  725 +++++++++++++
>> html/cgi-bin/logs.cgi/log.dat               |    2 +
>> langs/en/cgi-bin/en.pl                      |   31 +
>> lfs/configroot                              |    4 +-
>> lfs/ipblacklist-sources                     |   53 +
>> lfs/logwatch                                |    2 +
>> make.sh                                     |   11 +-
>> src/initscripts/system/firewall             |   20 +
>> src/misc-progs/Makefile                     |    2 +-
>> src/misc-progs/getipsetstat.c               |   28 +
>> src/misc-progs/ipblacklistctrl.c            |   52 +
>> src/scripts/ipblacklist                     | 1558 +++++++++++++++++++++++++++
>> 27 files changed, 2792 insertions(+), 8 deletions(-)
>> create mode 100644 config/ipblacklist/sources
>> create mode 100644 config/logwatch/ipblacklist
>> create mode 100644 config/logwatch/ipblacklist.conf
>> create mode 100644 config/rootfiles/common/ipblacklist-sources
>> create mode 100644 html/cgi-bin/ipblacklist.cgi
>> create mode 100644 lfs/ipblacklist-sources
>> create mode 100644 src/misc-progs/getipsetstat.c
>> create mode 100644 src/misc-progs/ipblacklistctrl.c
>> create mode 100755 src/scripts/ipblacklist
>>
>> -- 
>> 2.16.4
>>
>