Re: [pve-devel] [PATCH proxmox-firewall 1/2] fix: firewall: apply `nt_conntrack_allow_invalid` to all chains

[Stefan Hanreich <s.hanreich@proxmox.com>]

On 3/12/25 11:18, Hannes Laimer wrote:
> On 3/4/25 13:24, Stefan Hanreich wrote:
>> default-in is also checking for conntrack status, so we should put this
> 
> I think `default-in` is currently noop'ing[1] ct state invalid, am I
> missing something? I though maybe there's a reason for that, so I
> left it as is, as with the change we'd drop there with invalid ct
> state.

Yes, it is - I think I also remember the reason now for not including
invalid initially. CT has issues with multicast / broadcast traffic,
since it is impossible to know the return IP. We had some issues with
DHCP on bridges with firewall-enabled guests for instance.

There are workarounds / solutions for this (ICMPv6 has explicit
exceptions in the conntrack module for instance, some protocols have
conntrack helpers), but they're not a silver bullet that work for every
conceivable protocol. There were some additional fixes in the kernel for
this in the meanwhile [1].

The old firewall does drop invalid CT traffic going in / out the host
unless the nt_conntrack_allow_invalid setting is set:

  -A PVEFW-HOST-IN -m conntrack --ctstate INVALID -j DROP
  -A PVEFW-HOST-IN -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT

  [...]

  -A PVEFW-HOST-OUT -m conntrack --ctstate INVALID -j DROP
  -A PVEFW-HOST-OUT -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT

Same for guest traffic:

  -A PVEFW-FORWARD -m conntrack --ctstate INVALID -j DROP
  -A PVEFW-FORWARD -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT


IMO we should explicitly check for CT state invalid everywhere we are
checking CT state, since dropping it at some place but not the other
seems wrong/inconsistent. I'd also definitely default to dropping
invalid traffic everywhere, unless explicitly disabled by the user. I
think it's best for now to stick to what the old firewall is doing? It
should have the same effect in the new firewall, and we don't really
have lots of reports of this behavior causing issues.

For users that *are* encountering issues with this behavior, we have an
escape hatch with nt_conntrack_allow_invalid, which is okay IMO since
that doesn't bypass the firewall (all invalid packets still go through
the ruleset and get either accepted / dropped according to the ruleset).
It only hurts performance for that connection and consumes some CPU cycles.

We need to rework how we are utilizing CT in the near future though. It
would be nice to implement support for notrack [2] in the firewall,
which would help users to handle this more surgically by explicitly
exempting certain connections from conntrack. For EVPN we also need to
revisit CT handling of the firewall. So this is something that's on the
roadmap anyway.


[1]
https://lore.kernel.org/lkml/ZfyeC8mjLnGkqnVT@calendula/t/#m32cb12199c69a77f14786ec17b2df8566f34fe95
[2] https://bugzilla.proxmox.com/show_bug.cgi?id=2441


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel