From 7581e3b6d5f46deafaf6d8b0b903c6f6b901a031 Mon Sep 17 00:00:00 2001 From: Laine Stump Date: Tue, 29 Oct 2024 23:21:27 -0400 Subject: [PATCH] Revert "network: add rule to nftables backend that zeroes checksum of DHCP responses" This reverts commit 42ab0148dd11727f7e3fd31dce4485469af290d5. This patch was supposed to fix the checksum of dhcp response packets by setting it to 0 (because having a non-0 but incorrect checksum was causing the packets to be droppe on FreeBSD guests). Early testing was positive, but after the patch was pushed upstream and more people could test it, it turned out that while it fixed the dhcp checksum problem for virtio-net interfaces on FreeBSD and OpenBSD, it also *broke* dhcp checksums for the e1000 emulated NIC on *all* guests (but not e1000e). So we're reverting this fix and looking for something more universal to be included in the next release. Signed-off-by: Laine Stump Reviewed-by: Andrea Bolognani --- src/network/network_nftables.c | 69 ------------------- tests/networkxml2firewalldata/base.nftables | 14 ---- .../forward-dev-linux.nftables | 16 ----- .../isolated-linux.nftables | 16 ----- .../nat-default-linux.nftables | 16 ----- .../nat-ipv6-linux.nftables | 16 ----- .../nat-ipv6-masquerade-linux.nftables | 16 ----- .../nat-many-ips-linux.nftables | 16 ----- .../nat-port-range-ipv6-linux.nftables | 16 ----- .../nat-port-range-linux.nftables | 16 ----- .../nat-tftp-linux.nftables | 16 ----- .../route-default-linux.nftables | 16 ----- 12 files changed, 243 deletions(-) diff --git a/src/network/network_nftables.c b/src/network/network_nftables.c index 5523207269..f8b5ab665d 100644 --- a/src/network/network_nftables.c +++ b/src/network/network_nftables.c @@ -51,7 +51,6 @@ VIR_LOG_INIT("network.nftables"); #define VIR_NFTABLES_FWD_OUT_CHAIN "guest_output" #define VIR_NFTABLES_FWD_X_CHAIN "guest_cross" #define VIR_NFTABLES_NAT_POSTROUTE_CHAIN "guest_nat" -#define VIR_NFTABLES_MANGLE_POSTROUTE_CHAIN "postroute_mangle" /* we must avoid using the standard "filter" table as used by * iptables, as any subsequent attempts to use iptables commands will @@ -107,10 +106,6 @@ nftablesGlobalChain nftablesChains[] = { /* chains for NAT rules */ {NULL, VIR_NFTABLES_NAT_POSTROUTE_CHAIN, "{ type nat hook postrouting priority 100; policy accept; }"}, - - /* chain for "mangle" rules that modify packets (e.g. 0 out UDP checksums) */ - {NULL, VIR_NFTABLES_MANGLE_POSTROUTE_CHAIN, "{ type filter hook postrouting priority 0; policy accept; }"}, - }; @@ -649,44 +644,6 @@ nftablesAddDontMasquerade(virFirewall *fw, } -/** - * nftablesAddOutputFixUdpChecksum: - * - * Add a rule to @fw that will 0 out the checksum of udp packets - * output from @iface with destination port @port. - - * Zeroing the checksum of a UDP packet tells the receiving end "you - * don't need to validate the checksum", which is useful in cases - * where the host (sender) thinks that packet checksums will be - * computed elsewhere (and so leaves a partially computed checksum in - * the packet header) while the guest (receiver) thinks that the - * checksum has already been fully computed; in the meantime none of - * the code in between has actually finished computing the - * checksum. - * - * An example of this is DHCP response packets from host to - * guest. If the checksum of each of these packets isn't zeroed, then - * many guests (e.g. FreeBSD) will drop them with reason BAD CHECKSUM; - * if the packets arrive at those guests with a checksum of 0, they - * will happily accept the packet. - */ -static void -nftablesAddOutputFixUdpChecksum(virFirewall *fw, - const char *iface, - int port) -{ - g_autofree char *portstr = g_strdup_printf("%d", port); - - virFirewallAddCmd(fw, VIR_FIREWALL_LAYER_IPV4, - "insert", "rule", "ip", - VIR_NFTABLES_PRIVATE_TABLE, - VIR_NFTABLES_MANGLE_POSTROUTE_CHAIN, - "oif", iface, "udp", "dport", portstr, - "counter", "udp", "checksum", "set", "0", - NULL); -} - - static const char networkLocalMulticastIPv4[] = "224.0.0.0/24"; static const char networkLocalMulticastIPv6[] = "ff02::/16"; static const char networkLocalBroadcast[] = "255.255.255.255/32"; @@ -944,30 +901,6 @@ nftablesAddGeneralFirewallRules(virFirewall *fw, } -static void -nftablesAddChecksumFirewallRules(virFirewall *fw, - virNetworkDef *def) -{ - size_t i; - virNetworkIPDef *ipv4def; - - /* Look for the first IPv4 address that has dhcp or tftpboot - * defined. We support dhcp config on 1 IPv4 interface only. - */ - for (i = 0; (ipv4def = virNetworkDefGetIPByIndex(def, AF_INET, i)); i++) { - if (ipv4def->nranges || ipv4def->nhosts) - break; - } - - /* If we are doing local DHCP service on this network, add a rule - * that will fixup the checksum of DHCP response packets back to - * the guests. - */ - if (ipv4def) - nftablesAddOutputFixUdpChecksum(fw, def->bridge, 68); -} - - static int nftablesAddIPSpecificFirewallRules(virFirewall *fw, virNetworkDef *def, @@ -1019,8 +952,6 @@ nftablesAddFirewallRules(virNetworkDef *def, virFirewall **fwRemoval) return -1; } - nftablesAddChecksumFirewallRules(fw, def); - if (virFirewallApply(fw) < 0) return -1; diff --git a/tests/networkxml2firewalldata/base.nftables b/tests/networkxml2firewalldata/base.nftables index 6371fc12dd..a064318739 100644 --- a/tests/networkxml2firewalldata/base.nftables +++ b/tests/networkxml2firewalldata/base.nftables @@ -68,13 +68,6 @@ libvirt_network \ guest_nat \ '{ type nat hook postrouting priority 100; policy accept; }' nft \ -add \ -chain \ -ip \ -libvirt_network \ -postroute_mangle \ -'{ type filter hook postrouting priority 0; policy accept; }' -nft \ list \ table \ ip6 \ @@ -143,10 +136,3 @@ ip6 \ libvirt_network \ guest_nat \ '{ type nat hook postrouting priority 100; policy accept; }' -nft \ -add \ -chain \ -ip6 \ -libvirt_network \ -postroute_mangle \ -'{ type filter hook postrouting priority 0; policy accept; }' diff --git a/tests/networkxml2firewalldata/forward-dev-linux.nftables b/tests/networkxml2firewalldata/forward-dev-linux.nftables index 9dea1a88a4..8badb74beb 100644 --- a/tests/networkxml2firewalldata/forward-dev-linux.nftables +++ b/tests/networkxml2firewalldata/forward-dev-linux.nftables @@ -156,19 +156,3 @@ daddr \ 224.0.0.0/24 \ counter \ return -nft \ --ae insert \ -rule \ -ip \ -libvirt_network \ -postroute_mangle \ -oif \ -virbr0 \ -udp \ -dport \ -68 \ -counter \ -udp \ -checksum \ -set \ -0 diff --git a/tests/networkxml2firewalldata/isolated-linux.nftables b/tests/networkxml2firewalldata/isolated-linux.nftables index 67ee0a2bf5..d1b4dac178 100644 --- a/tests/networkxml2firewalldata/isolated-linux.nftables +++ b/tests/networkxml2firewalldata/isolated-linux.nftables @@ -62,19 +62,3 @@ oif \ virbr0 \ counter \ accept -nft \ --ae insert \ -rule \ -ip \ -libvirt_network \ -postroute_mangle \ -oif \ -virbr0 \ -udp \ -dport \ -68 \ -counter \ -udp \ -checksum \ -set \ -0 diff --git a/tests/networkxml2firewalldata/nat-default-linux.nftables b/tests/networkxml2firewalldata/nat-default-linux.nftables index 951a5a6d60..28508292f9 100644 --- a/tests/networkxml2firewalldata/nat-default-linux.nftables +++ b/tests/networkxml2firewalldata/nat-default-linux.nftables @@ -142,19 +142,3 @@ daddr \ 224.0.0.0/24 \ counter \ return -nft \ --ae insert \ -rule \ -ip \ -libvirt_network \ -postroute_mangle \ -oif \ -virbr0 \ -udp \ -dport \ -68 \ -counter \ -udp \ -checksum \ -set \ -0 diff --git a/tests/networkxml2firewalldata/nat-ipv6-linux.nftables b/tests/networkxml2firewalldata/nat-ipv6-linux.nftables index 617ed8b753..d8a9ba706d 100644 --- a/tests/networkxml2firewalldata/nat-ipv6-linux.nftables +++ b/tests/networkxml2firewalldata/nat-ipv6-linux.nftables @@ -200,19 +200,3 @@ oif \ virbr0 \ counter \ accept -nft \ --ae insert \ -rule \ -ip \ -libvirt_network \ -postroute_mangle \ -oif \ -virbr0 \ -udp \ -dport \ -68 \ -counter \ -udp \ -checksum \ -set \ -0 diff --git a/tests/networkxml2firewalldata/nat-ipv6-masquerade-linux.nftables b/tests/networkxml2firewalldata/nat-ipv6-masquerade-linux.nftables index a710d0e296..a7f09cda59 100644 --- a/tests/networkxml2firewalldata/nat-ipv6-masquerade-linux.nftables +++ b/tests/networkxml2firewalldata/nat-ipv6-masquerade-linux.nftables @@ -272,19 +272,3 @@ daddr \ ff02::/16 \ counter \ return -nft \ --ae insert \ -rule \ -ip \ -libvirt_network \ -postroute_mangle \ -oif \ -virbr0 \ -udp \ -dport \ -68 \ -counter \ -udp \ -checksum \ -set \ -0 diff --git a/tests/networkxml2firewalldata/nat-many-ips-linux.nftables b/tests/networkxml2firewalldata/nat-many-ips-linux.nftables index 0be5fb7e65..b826fe6134 100644 --- a/tests/networkxml2firewalldata/nat-many-ips-linux.nftables +++ b/tests/networkxml2firewalldata/nat-many-ips-linux.nftables @@ -366,19 +366,3 @@ daddr \ 224.0.0.0/24 \ counter \ return -nft \ --ae insert \ -rule \ -ip \ -libvirt_network \ -postroute_mangle \ -oif \ -virbr0 \ -udp \ -dport \ -68 \ -counter \ -udp \ -checksum \ -set \ -0 diff --git a/tests/networkxml2firewalldata/nat-port-range-ipv6-linux.nftables b/tests/networkxml2firewalldata/nat-port-range-ipv6-linux.nftables index 7574356855..ceaed6fa40 100644 --- a/tests/networkxml2firewalldata/nat-port-range-ipv6-linux.nftables +++ b/tests/networkxml2firewalldata/nat-port-range-ipv6-linux.nftables @@ -384,19 +384,3 @@ daddr \ ff02::/16 \ counter \ return -nft \ --ae insert \ -rule \ -ip \ -libvirt_network \ -postroute_mangle \ -oif \ -virbr0 \ -udp \ -dport \ -68 \ -counter \ -udp \ -checksum \ -set \ -0 diff --git a/tests/networkxml2firewalldata/nat-port-range-linux.nftables b/tests/networkxml2firewalldata/nat-port-range-linux.nftables index 127536e4db..1dc37a26ec 100644 --- a/tests/networkxml2firewalldata/nat-port-range-linux.nftables +++ b/tests/networkxml2firewalldata/nat-port-range-linux.nftables @@ -312,19 +312,3 @@ oif \ virbr0 \ counter \ accept -nft \ --ae insert \ -rule \ -ip \ -libvirt_network \ -postroute_mangle \ -oif \ -virbr0 \ -udp \ -dport \ -68 \ -counter \ -udp \ -checksum \ -set \ -0 diff --git a/tests/networkxml2firewalldata/nat-tftp-linux.nftables b/tests/networkxml2firewalldata/nat-tftp-linux.nftables index 951a5a6d60..28508292f9 100644 --- a/tests/networkxml2firewalldata/nat-tftp-linux.nftables +++ b/tests/networkxml2firewalldata/nat-tftp-linux.nftables @@ -142,19 +142,3 @@ daddr \ 224.0.0.0/24 \ counter \ return -nft \ --ae insert \ -rule \ -ip \ -libvirt_network \ -postroute_mangle \ -oif \ -virbr0 \ -udp \ -dport \ -68 \ -counter \ -udp \ -checksum \ -set \ -0 diff --git a/tests/networkxml2firewalldata/route-default-linux.nftables b/tests/networkxml2firewalldata/route-default-linux.nftables index be9c4f5439..282c9542a5 100644 --- a/tests/networkxml2firewalldata/route-default-linux.nftables +++ b/tests/networkxml2firewalldata/route-default-linux.nftables @@ -56,19 +56,3 @@ oif \ virbr0 \ counter \ accept -nft \ --ae insert \ -rule \ -ip \ -libvirt_network \ -postroute_mangle \ -oif \ -virbr0 \ -udp \ -dport \ -68 \ -counter \ -udp \ -checksum \ -set \ -0