[RFC manager/network v2 0/3] fix #5066: make generated snat rules flushable

[RFC manager/network v2 0/3] fix #5066: make generated snat rules flushable

[Lukas Sichert]

When creating a subnet with SNAT enabled and applying the changes, then
afterwards disabling SNAT and applying the changes again, the iptables
POSTROUTING rule still persists. This is because ifreload -a only
executes (post/pre-)down hooks when an interface is removed from
/etc/network/interfaces, while the (post/pre-)up hooks are always
executed [1]. As a result, the SNAT rule is not removed by 'ifreload -a' and
only a restart or 'ifdown' will remove it.

This series moves generated SDN SNAT rules into a dedicated
'PROXMOX-SDN' chain in the iptables nat table and adds a jump from
POSTROUTING to that chain. This keeps the generated rules separate from
custom rules added by users or other components.

The dedicated chain can then be flushed during network reload, removing
stale SDN SNAT rules without touching unrelated POSTROUTING rules.

As this changes the generated /etc/network/interfaces.d/sdn output, the
expected test output is adjusted accordingly.

[1] manpages.debian.org/testing/ifupdown2/ifreload.8.en.html
Link: https://bugzilla.proxmox.com/show_bug.cgi?id=5066

changes from v1 to v2 (thanks @Stefan):
- rebase on top of master
- create chain only when '$is_evpn_gateway' is true
- add Links to commits


network:

Lukas Sichert (2):
  fix #5066: snat: push evpn snat rules into separate iptables chain
  fix #5066: snat: push simplezone snat rules into separate iptables
    chain

 src/PVE/Network/SDN/Zones/EvpnPlugin.pm          | 11 +++++++++--
 src/PVE/Network/SDN/Zones/SimplePlugin.pm        | 15 ++++++++++++---
 .../evpn/exitnode_snat/expected_sdn_interfaces   | 16 ++++++++++++----
 .../simple/ipv4snat/expected_sdn_interfaces      |  8 ++++++--
 .../simple/ipv6snat/expected_sdn_interfaces      |  8 ++++++--
 5 files changed, 45 insertions(+), 13 deletions(-)


manager:

Lukas Sichert (1):
  fix #5066: reload networking: flush PROXMOX-SDN iptables chain at
    reload

 PVE/API2/Network.pm | 3 +++
 1 file changed, 3 insertions(+)


Summary over all repositories:
  6 files changed, 48 insertions(+), 13 deletions(-)

-- 
Generated by murpp 0.12.0

[PATCH network v2 1/3] fix #5066: snat: push evpn snat rules into separate iptables chain

[Lukas Sichert]

When creating a Subnet with SNAT enabled and applying the changes, then
afterwards disabling SNAT and applying the changes, the rule still
persists in iptables. This is because ifreload -a only executes
(post/pre-)down hooks when an interface is removed from
/etc/network/interfaces, but the (post/pre-)up hooks are always
executed. As a result, the SNAT rule is not removed by 'ifreload -a' and
only a restart or 'ifdown' will remove it.

To be able to flush only the rules created by the Proxmox stack, add a
separate 'PROXMOX-SDN' chain to the iptables nat table, if a plugin
needs SNAT rules. Then add a jump from POSTROUTING to the new chain and
append all SNAT rules to the new chain. The new chain can then be
flushed separately.

Signed-off-by: Lukas Sichert <l.sichert@proxmox.com>
Link: https://bugzilla.proxmox.com/show_bug.cgi?id=5066
---
 src/PVE/Network/SDN/Zones/EvpnPlugin.pm          | 11 +++++++++--
 .../evpn/exitnode_snat/expected_sdn_interfaces   | 16 ++++++++++++----
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/src/PVE/Network/SDN/Zones/EvpnPlugin.pm b/src/PVE/Network/SDN/Zones/EvpnPlugin.pm
index dfbd7e9..b1bfd56 100644
--- a/src/PVE/Network/SDN/Zones/EvpnPlugin.pm
+++ b/src/PVE/Network/SDN/Zones/EvpnPlugin.pm
@@ -273,10 +273,17 @@ sub generate_sdn_config {
                 PVE::Network::SDN::Zones::Plugin::get_local_route_ip($checkrouteip);
             if ($outip && $outiface && $is_evpn_gateway) {
                 #use snat, faster than masquerade
+                push @iface_config, "post-up $iptables -t nat -N PROXMOX-SDN 2>/dev/null || true";
                 push @iface_config,
-                    "post-up $iptables -t nat -A POSTROUTING -s '$cidr' -o $outiface -j SNAT --to-source $outip";
+                    "post-up $iptables -t nat -C POSTROUTING -j PROXMOX-SDN 2>/dev/null || $iptables -t nat -A POSTROUTING -j PROXMOX-SDN";
+                push @iface_config, "post-down $iptables -t nat -N PROXMOX-SDN 2>/dev/null || true";
                 push @iface_config,
-                    "post-down $iptables -t nat -D POSTROUTING -s '$cidr' -o $outiface -j SNAT --to-source $outip";
+                    "post-down $iptables -t nat -D POSTROUTING -j PROXMOX-SDN 2>/dev/null || true";
+
+                push @iface_config,
+                    "post-up $iptables -t nat -A PROXMOX-SDN -s '$cidr' -o $outiface -j SNAT --to-source $outip";
+                push @iface_config,
+                    "post-down $iptables -t nat -D PROXMOX-SDN -s '$cidr' -o $outiface -j SNAT --to-source $outip";
                 #add conntrack zone once on outgoing interface
                 push @iface_config,
                     "post-up $iptables -t raw -I PREROUTING -i fwbr+ -j CT --zone 1";
diff --git a/src/test/zones/evpn/exitnode_snat/expected_sdn_interfaces b/src/test/zones/evpn/exitnode_snat/expected_sdn_interfaces
index 0d7d174..2905f20 100644
--- a/src/test/zones/evpn/exitnode_snat/expected_sdn_interfaces
+++ b/src/test/zones/evpn/exitnode_snat/expected_sdn_interfaces
@@ -3,8 +3,12 @@
 auto myvnet
 iface myvnet
 	address 10.0.0.1/24
-	post-up iptables -t nat -A POSTROUTING -s '10.0.0.0/24' -o vmbr0 -j SNAT --to-source 192.168.0.1
-	post-down iptables -t nat -D POSTROUTING -s '10.0.0.0/24' -o vmbr0 -j SNAT --to-source 192.168.0.1
+	post-up iptables -t nat -N PROXMOX-SDN 2>/dev/null || true
+	post-up iptables -t nat -C POSTROUTING -j PROXMOX-SDN 2>/dev/null || iptables -t nat -A POSTROUTING -j PROXMOX-SDN
+	post-down iptables -t nat -N PROXMOX-SDN 2>/dev/null || true
+	post-down iptables -t nat -D POSTROUTING -j PROXMOX-SDN 2>/dev/null || true
+	post-up iptables -t nat -A PROXMOX-SDN -s '10.0.0.0/24' -o vmbr0 -j SNAT --to-source 192.168.0.1
+	post-down iptables -t nat -D PROXMOX-SDN -s '10.0.0.0/24' -o vmbr0 -j SNAT --to-source 192.168.0.1
 	post-up iptables -t raw -I PREROUTING -i fwbr+ -j CT --zone 1
 	post-down iptables -t raw -D PREROUTING -i fwbr+ -j CT --zone 1
 	bridge_ports vxlan_myvnet
@@ -18,8 +22,12 @@ iface myvnet
 auto myvnet2
 iface myvnet2
 	address 2a08:2142:302:3::1/64
-	post-up ip6tables -t nat -A POSTROUTING -s '2a08:2142:302:3::/64' -o vmbr0 -j SNAT --to-source 2001:db8::2
-	post-down ip6tables -t nat -D POSTROUTING -s '2a08:2142:302:3::/64' -o vmbr0 -j SNAT --to-source 2001:db8::2
+	post-up ip6tables -t nat -N PROXMOX-SDN 2>/dev/null || true
+	post-up ip6tables -t nat -C POSTROUTING -j PROXMOX-SDN 2>/dev/null || ip6tables -t nat -A POSTROUTING -j PROXMOX-SDN
+	post-down ip6tables -t nat -N PROXMOX-SDN 2>/dev/null || true
+	post-down ip6tables -t nat -D POSTROUTING -j PROXMOX-SDN 2>/dev/null || true
+	post-up ip6tables -t nat -A PROXMOX-SDN -s '2a08:2142:302:3::/64' -o vmbr0 -j SNAT --to-source 2001:db8::2
+	post-down ip6tables -t nat -D PROXMOX-SDN -s '2a08:2142:302:3::/64' -o vmbr0 -j SNAT --to-source 2001:db8::2
 	post-up ip6tables -t raw -I PREROUTING -i fwbr+ -j CT --zone 1
 	post-down ip6tables -t raw -D PREROUTING -i fwbr+ -j CT --zone 1
 	bridge_ports vxlan_myvnet2
-- 
2.47.3

[PATCH network v2 2/3] fix #5066: snat: push simplezone snat rules into separate iptables chain

[Lukas Sichert]

When creating a Subnet with SNAT enabled and applying the changes, then
afterwards disabling SNAT and applying the changes, the rule still
persists in iptables. This is because ifreload -a only executes
(post/pre-)down hooks when an interface is removed from
/etc/network/interfaces, but the (post/pre-)up hooks are always
executed. As a result, the SNAT rule is not removed by 'ifreload -a' and
only a restart or 'ifdown' will remove it.

To be able to flush only the rules created by the Proxmox stack, add a
separate 'PROXMOX-SDN' chain to the iptables nat table, if a plugin
needs SNAT rules. Then add a jump from POSTROUTING to the new chain and
append all SNAT rules to the new chain. The new chain can then be
flushed separately.

Signed-off-by: Lukas Sichert <l.sichert@proxmox.com>
Link: https://bugzilla.proxmox.com/show_bug.cgi?id=5066
---
 src/PVE/Network/SDN/Zones/SimplePlugin.pm         | 15 ++++++++++++---
 .../zones/simple/ipv4snat/expected_sdn_interfaces |  8 ++++++--
 .../zones/simple/ipv6snat/expected_sdn_interfaces |  8 ++++++--
 3 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/src/PVE/Network/SDN/Zones/SimplePlugin.pm b/src/PVE/Network/SDN/Zones/SimplePlugin.pm
index f5cd18e..29943e8 100644
--- a/src/PVE/Network/SDN/Zones/SimplePlugin.pm
+++ b/src/PVE/Network/SDN/Zones/SimplePlugin.pm
@@ -114,12 +114,22 @@ sub generate_sdn_config {
             #find outgoing interface
             my ($outip, $outiface) =
                 PVE::Network::SDN::Zones::Plugin::get_local_route_ip($checkrouteip);
+
+            push @iface_config, "post-up $iptables -t nat -N PROXMOX-SDN 2>/dev/null || true";
+            push @iface_config,
+                "post-up $iptables -t nat -C POSTROUTING -j PROXMOX-SDN 2>/dev/null || $iptables -t nat -A POSTROUTING -j PROXMOX-SDN";
+
+            push @iface_config, "post-down $iptables -t nat -N PROXMOX-SDN 2>/dev/null || true";
+            push @iface_config,
+                "post-down $iptables -t nat -D POSTROUTING -j PROXMOX-SDN 2>/dev/null || true";
             if ($outip && $outiface) {
                 #use snat, faster than masquerade
                 push @iface_config,
-                    "post-up $iptables -t nat -A POSTROUTING -s '$cidr' -o $outiface -j SNAT --to-source $outip";
+                    "post-up $iptables -t nat -A PROXMOX-SDN -s '$cidr' -o $outiface -j SNAT --to-source $outip";
+
                 push @iface_config,
-                    "post-down $iptables -t nat -D POSTROUTING -s '$cidr' -o $outiface -j SNAT --to-source $outip";
+                    "post-down $iptables -t nat -D PROXMOX-SDN -s '$cidr' -o $outiface -j SNAT --to-source $outip";
+
                 #add conntrack zone once on outgoing interface
                 push @iface_config,
                     "post-up $iptables -t raw -I PREROUTING -i fwbr+ -j CT --zone 1";
@@ -168,4 +178,3 @@ sub get_mtu {
 }
 
 1;
-
diff --git a/src/test/zones/simple/ipv4snat/expected_sdn_interfaces b/src/test/zones/simple/ipv4snat/expected_sdn_interfaces
index 69d7986..34488eb 100644
--- a/src/test/zones/simple/ipv4snat/expected_sdn_interfaces
+++ b/src/test/zones/simple/ipv4snat/expected_sdn_interfaces
@@ -3,8 +3,12 @@
 auto myvnet
 iface myvnet
 	address 10.0.0.1/24
-	post-up iptables -t nat -A POSTROUTING -s '10.0.0.0/24' -o vmbr0 -j SNAT --to-source 192.168.0.1
-	post-down iptables -t nat -D POSTROUTING -s '10.0.0.0/24' -o vmbr0 -j SNAT --to-source 192.168.0.1
+	post-up iptables -t nat -N PROXMOX-SDN 2>/dev/null || true
+	post-up iptables -t nat -C POSTROUTING -j PROXMOX-SDN 2>/dev/null || iptables -t nat -A POSTROUTING -j PROXMOX-SDN
+	post-down iptables -t nat -N PROXMOX-SDN 2>/dev/null || true
+	post-down iptables -t nat -D POSTROUTING -j PROXMOX-SDN 2>/dev/null || true
+	post-up iptables -t nat -A PROXMOX-SDN -s '10.0.0.0/24' -o vmbr0 -j SNAT --to-source 192.168.0.1
+	post-down iptables -t nat -D PROXMOX-SDN -s '10.0.0.0/24' -o vmbr0 -j SNAT --to-source 192.168.0.1
 	post-up iptables -t raw -I PREROUTING -i fwbr+ -j CT --zone 1
 	post-down iptables -t raw -D PREROUTING -i fwbr+ -j CT --zone 1
 	bridge_ports none
diff --git a/src/test/zones/simple/ipv6snat/expected_sdn_interfaces b/src/test/zones/simple/ipv6snat/expected_sdn_interfaces
index 5f6d40b..c5cdfd1 100644
--- a/src/test/zones/simple/ipv6snat/expected_sdn_interfaces
+++ b/src/test/zones/simple/ipv6snat/expected_sdn_interfaces
@@ -3,8 +3,12 @@
 auto myvnet
 iface myvnet
 	address 2a08:2142:302:3::1/64
-	post-up ip6tables -t nat -A POSTROUTING -s '2a08:2142:302:3::/64' -o vmbr0 -j SNAT --to-source 2001:db8::2
-	post-down ip6tables -t nat -D POSTROUTING -s '2a08:2142:302:3::/64' -o vmbr0 -j SNAT --to-source 2001:db8::2
+	post-up ip6tables -t nat -N PROXMOX-SDN 2>/dev/null || true
+	post-up ip6tables -t nat -C POSTROUTING -j PROXMOX-SDN 2>/dev/null || ip6tables -t nat -A POSTROUTING -j PROXMOX-SDN
+	post-down ip6tables -t nat -N PROXMOX-SDN 2>/dev/null || true
+	post-down ip6tables -t nat -D POSTROUTING -j PROXMOX-SDN 2>/dev/null || true
+	post-up ip6tables -t nat -A PROXMOX-SDN -s '2a08:2142:302:3::/64' -o vmbr0 -j SNAT --to-source 2001:db8::2
+	post-down ip6tables -t nat -D PROXMOX-SDN -s '2a08:2142:302:3::/64' -o vmbr0 -j SNAT --to-source 2001:db8::2
 	post-up ip6tables -t raw -I PREROUTING -i fwbr+ -j CT --zone 1
 	post-down ip6tables -t raw -D PREROUTING -i fwbr+ -j CT --zone 1
 	bridge_ports none
-- 
2.47.3

[PATCH manager v2 3/3] fix #5066: reload networking: flush PROXMOX-SDN iptables chain at reload

[Lukas Sichert]

With the addition of a separate PROXMOX-SDN iptables chain it is now
possible to flush this distinct chain, without affecting unrelated
POSTROUTING rules set by the user.

Flush old rules by running 'iptables -t nat -F PROXMOX-SDN' at the end
of the reload_network_config API call, before executing 'ifreload -a'.
The reload then adds the currently valid rules again.

Signed-off-by: Lukas Sichert <l.sichert@proxmox.com>
Link: https://bugzilla.proxmox.com/show_bug.cgi?id=5066
---
 PVE/API2/Network.pm | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/PVE/API2/Network.pm b/PVE/API2/Network.pm
index c5863ca7..a8ec88a4 100644
--- a/PVE/API2/Network.pm
+++ b/PVE/API2/Network.pm
@@ -932,6 +932,9 @@ __PACKAGE__->register_method({
                     print "$2 : $line \n";
                 }
             };
+            PVE::Tools::run_command(['iptables', '-t', 'nat', '-F', 'PROXMOX-SDN'], noerr => 1);
+            PVE::Tools::run_command(['ip6tables', '-t', 'nat', '-F', 'PROXMOX-SDN'],
+                noerr => 1);
             PVE::Tools::run_command(['ifreload', '-a'], errfunc => $err);
 
             if (defined($regenerate_frr)) {
-- 
2.47.3