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

[Tim FitzGeorge <ipfr@tfitzgeorge.me.uk>]

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

Hello Peter and Michael,

My responses to both of your emails are below:

On 28/11/2019 21:39, Peter Müller 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?

I think there are a number of points here.

Firstly, from the point of a third party using IPFire, is this really
solving the privacy disclosure problem?  There's no way round disclosing
your public IP address to someone you're downloading from; all this does
is change who that information is being disclosed to.  For the  user
there's no way of knowing whether the source is more or less protective
of the user's privacy than the blacklist provider.  Indeed it won't be
possible to know who the lists are being downloaded from until the
download starts.

Secondly, latency; some of the lists are updated every 5 minutes.  While
I've limited the maximum check rate to hourly, will the updates
propagate quickly enough.  For reference on my main system the 24
updates on the CIARMY list made 143 498 changes (additions or
deletions).  I've seem it do over 200 000.

Third, bandwidth; while the downloads are fairly small (ALIENVAULT is
the largest at a few MB), there are going to be a lot of them.  How will
this affect the willingness of people to mirror IPFire?

> 
> 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.
> 
You're only going to get one result for a packet whichever way round the
IP blacklist and IPS are since whichever comes first will drop the
packet before it reaches the second (well it would be possible to put
the IP blacklist first and get it to log and mark packets which are then
dropped after the IPS, but I think that's getting a little complicated.
 In addition I've seen the messages about the trouble marking was
causing in the QoS).

I think it's a 50/50 choice as to which is more valuable first; it's
probably going to differ from packet to packet.  For me the possibility
of reducing the IPS load means I prefer putting the IP blacklist first

It should be fairly easy to add the choice of where to put the IP
blacklist.  I think it'll have to be in the main firewall script, so
it'll require a firewall restart, but it's not something that'll be
changed often.

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

I agree.  I added the possibility to turn off logging because of user
requests for the earlier version I made available on GitHub, but logging
is enabled by default.
> 
> 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.

I can add a field to the options file that controls whether a list is
enabled by default.

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

It's intended to provide a quick local response to portscans based on
packets which are dropped due to the red input policy.

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

Indeed.  I've seen the messages from people who have turned on all the
IPS rules.  In addition I know that at least one person who used the
version on GitHub turned on all the lists, even the ones where one is a
subset of another list (I added code to prevent this).  I don't believe
that it caused major problems since, unlike the IPS, it doesn't have
rules that can block what for some people is normal activity.  It's
still a bad idea.  I think it makes sense to deliver it with a default
configuration, which might hint that it's not meant to have all the
lists enabled.

I'll obviously cover this in the Wiki page(s), but that doesn't mean
people will read it...
>>
>> 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.

This is one of the areas I was unsure about.  I've looked at the
existing code, but I obviously don't have the detailed knowledge of how
the different parts of the system interact when setting up the firewall.

The rule added to POLICYIN is the one that adds addresses to the
autoblacklist; it's not all that critical where it goes in POLICYIN, but
the ideal place is just before the LOG.  Normally the code in
firewall-policy will put it in the right place, but I think I know how
to place it better in ipblacklist.

(Aside - I write onboard software for spacecraft and we always try to
bear in mind that it could be use that has to modify it in a hurry after
working on something else for ten years.  It does tend to encourage
being tidy).

>>
>> Well done!
>>
>> -Michael

If you let me know what you think, I can make some modifications and
then send a V2.

Tim

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