search

voxpupuli / puppet-nftables

Puppet Module to manage nftables firewall rules.
Apache License 2.0
12 stars 33 forks source link

Do not drop invalid packets by default #227

Open bastelfreak opened 9 months ago

bastelfreak commented 9 months ago

This could be considered a breaking change. I don't mind keeping it open until the next major release.

duritong commented 8 months ago

Can you elaborate, why you want to change the default? I think it's pretty standard to drop invalid packets, not?

bastelfreak commented 8 months ago

I doesn't make sense to have that rule when the default policy is already drop. It's redundant. In addition to that there were bug reports in the past where ceph traffic was marked as invalid for whatever reasons and firewalls dropped it by accident. Same applies for Qemu hypervisors with bridges.

duritong commented 8 months ago

It is not redundant, since the invalid drop comes before the jump into the default chain.

Which essentially means, that invalid packets of (later) allowed connections are dropped (as you observed). If you are relaying on the default deny for that, then it won't work since the invalid packets matched the (now) previous allowed rule and thus the firewall allows invalid packets.

But that is the whole idea: A firewall should also filter out packets that are not part of an established or related connection (stateful filtering) and secondly it should also filter out any other malicious traffic like invalid packets.

I am fine with making this configurable, but changing the default would definitely weaken the default configuration of the module and thus making it less secure out of the box.

As I said I think it is pretty standard for every firewall to filter out invalid packets by default (https://wiki.nftables.org/wiki-nftables/index.php/Simple_ruleset_for_a_server) and therefor I think it should be in there by default.

What you must be observing is that applications do send invalid new packets, since any other kind of invalid packet would still be passed by the established&related rule (which is before dropping invalid packets), which is quite weird to be honest, but there might be reasons for it, but this does cannot be an argumentation for an overall weakening of the default configuration of the configuration generated by this module.

Correct me if I got any of my understanding wrong, but I think your assumption of the default policy is wrong and thus it works differently than you currently assume and thus argue for the change.

JohnHolmesII commented 5 months ago

I would also ask that the default not be changed. Configurable should be good enough.

amuckart commented 5 months ago

I don't think this is a good idea for a default setting, and I think @duritong's comments are spot on.

Dropping invalid packets is a pretty standard expected behaviour for most firewalls. Changing the default behaviour to fix a handful of misbehaving protocols seems like it will open more dangerous cases than some protocol not working.

The best case is the invalid packet hits the destination host and the destination host drops it. The worst case is that the invalid packet triggers an explot on the destination host and now you've got a compromised network.