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