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