Iptables support removal #1353
Iptables support removal #1353
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: l0rd The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Luap99
left a comment
Luap99
left a comment
There was a problem hiding this comment.
Thanks, overall this looks right but I didn't look super carefully. But note the testing comment, we need to make sure we do not remove important test cases that do not directly relate to iptables.
|
Ephemeral COPR build failed. @containers/packit-build please check. |
Code and documentation changes to remove the support of iptables as a firewall driver. Signed-off-by: Mario Loriedo <[email protected]>
Signed-off-by: Mario Loriedo <[email protected]>
Signed-off-by: Mario Loriedo <[email protected]>
Luap99
left a comment
Luap99
left a comment
There was a problem hiding this comment.
LGTM, though reading this accept dns by default rules I really think our ruleset isn't exactly ideal for nftables usage but this will be for another time.
| run_in_host_netns nft add chain inet netavark INPUT \{ type filter hook input priority 0 \; policy drop \; \} | ||
| run_in_host_netns nft add rule inet netavark INPUT ip saddr 10.89.3.0/24 meta l4proto \{ tcp, udp \} th dport $dns_port accept | ||
| run_in_host_netns nft add rule inet netavark INPUT ip6 saddr fd10:88:a::/64 meta l4proto \{ tcp, udp \} th dport $dns_port accept | ||
| run_in_host_netns nft add rule inet netavark INPUT ct state related,established accept | ||
| run_in_host_netns nft add rule inet netavark INPUT meta l4proto ipv6-icmp accept # allow ICMPv6, required for DNS resolution |
There was a problem hiding this comment.
I am not sure what value this adds practically, you are basically adding the accept rule for dns manually which is not what we want to test but also...
In general there is a big difference between how iptables and nftables work. In iptables all shared the same rule namespace so the default drop there caused us to not be able to get dns packages, so we just add to inject one accept rule before the the final drop happens.
With nftables it is different however because generally each tool uses its own unique table like we do with netavark. So no user should ever touch our INPUT rule and set to to drop as we "own" it.
What is more likely to happen is that a user has another table with an INPUT chain that drops our packages and in the that case it doesn't matter that we accept it in our table. As long as one chain in another table drops the package it gets dropped here so really I don't think us adding these rules even provides much purpose atm.
So now that I thought about all that I guess the answer is our current ruleset doesn't really makes sense and it needs to be rework for 2.0 so I guess we can keep this in for now. It doesn't do any harm either.
cc @mheon
|
@mheon PTAL |
3 participants
Removal of the source code and documentation that supported iptables as a firewall driver.