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

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

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

Hi,

> On 28 Nov 2019, at 21:39, Peter Müller <peter.mueller(a)ipfire.org> wrote:
> 
> 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?

Well, this causes us new problems:

* Does the licence of those lists allow us to redistribute them?

* They are usually updated very often (within minutes). We ask our mirror servers to sync at least every 12 hours but not more often than every 6. In practice everyone seems to be doing it once an hour. This will be way too slow and we will distribute a list that might be a day old.

* Bandwidth

* If we sign them, how do we validate that we have seen the correct list? This has to be automated and I do not see a way how we can make this bullet-proof.

In my personal opinion: This all dies with the first reason. One list that has a strict licence will be enough.

> 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.

True. But that relies on the IPS having a ruleset that would catch this threat.

Traffic from Spamhaus DROP should be dropped regardless.

I think we can say that the blacklists will always drop when an IP address is on the list. The IPS won’t. So is there not a slight advantage to security if the blacklists come first?

> 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.

The GeoIP feature was designed to prevent noise in the firewall logs and drop those packets silently.

It works as designed. You want something different which could be added with a single checkbox.

> 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.

Yes, I was planning to have this enabled by default in IPFire 3.

However, there must be an obvious switch to allow RFC1918.

We should distinguish between bogons from public IP address space and RFC1918 and similar address space.

Best,
-Michael

> 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.

You can do that right now, but I suppose we might have a couple of iterations of this patchset and I would suggest doing it at the end.

But please review every time.

> 
> 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
>>> 
>>