[lxc-devel] [lxd/master] Network fixes

stgraber on Github lxc-bot at linuxcontainers.org
Tue Jan 28 19:34:04 UTC 2020


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 301 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200128/d00f8e8c/attachment-0001.bin>
-------------- next part --------------
From 6a09a3679fea2f15a97b5cdfe59821abf4045771 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Tue, 28 Jan 2020 16:54:32 +0100
Subject: [PATCH 1/2] lxd/devices: Remove dead xtables code
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
 lxd/device/nic_bridged.go | 95 ---------------------------------------
 1 file changed, 95 deletions(-)

diff --git a/lxd/device/nic_bridged.go b/lxd/device/nic_bridged.go
index 7633dac620..699caddf5a 100644
--- a/lxd/device/nic_bridged.go
+++ b/lxd/device/nic_bridged.go
@@ -462,68 +462,6 @@ func (d *nicBridged) getDHCPStaticIPs(network string, instanceName string) (dhcp
 	return IPv4, IPv6, nil
 }
 
-// generateFilterEbtablesRules returns a customised set of ebtables filter rules based on the device.
-func (d *nicBridged) generateFilterEbtablesRules(m deviceConfig.Device, IPv4 net.IP, IPv6 net.IP) [][]string {
-	// MAC source filtering rules. Blocks any packet coming from instance with an incorrect Ethernet source MAC.
-	// This is required for IP filtering too.
-	rules := [][]string{
-		{"ebtables", "-t", "filter", "-A", "INPUT", "-s", "!", m["hwaddr"], "-i", m["host_name"], "-j", "DROP"},
-		{"ebtables", "-t", "filter", "-A", "FORWARD", "-s", "!", m["hwaddr"], "-i", m["host_name"], "-j", "DROP"},
-	}
-
-	if shared.IsTrue(m["security.ipv4_filtering"]) && IPv4 != nil {
-		rules = append(rules,
-			// Prevent ARP MAC spoofing (prevents the instance poisoning the ARP cache of its neighbours with a MAC address that isn't its own).
-			[]string{"ebtables", "-t", "filter", "-A", "INPUT", "-p", "ARP", "-i", m["host_name"], "--arp-mac-src", "!", m["hwaddr"], "-j", "DROP"},
-			[]string{"ebtables", "-t", "filter", "-A", "FORWARD", "-p", "ARP", "-i", m["host_name"], "--arp-mac-src", "!", m["hwaddr"], "-j", "DROP"},
-			// Prevent ARP IP spoofing (prevents the instance redirecting traffic for IPs that are not its own).
-			[]string{"ebtables", "-t", "filter", "-A", "INPUT", "-p", "ARP", "-i", m["host_name"], "--arp-ip-src", "!", IPv4.String(), "-j", "DROP"},
-			[]string{"ebtables", "-t", "filter", "-A", "FORWARD", "-p", "ARP", "-i", m["host_name"], "--arp-ip-src", "!", IPv4.String(), "-j", "DROP"},
-			// Allow DHCPv4 to the host only. This must come before the IP source filtering rules below.
-			[]string{"ebtables", "-t", "filter", "-A", "INPUT", "-p", "IPv4", "-s", m["hwaddr"], "-i", m["host_name"], "--ip-src", "0.0.0.0", "--ip-dst", "255.255.255.255", "--ip-proto", "udp", "--ip-dport", "67", "-j", "ACCEPT"},
-			// IP source filtering rules. Blocks any packet coming from instance with an incorrect IP source address.
-			[]string{"ebtables", "-t", "filter", "-A", "INPUT", "-p", "IPv4", "-i", m["host_name"], "--ip-src", "!", IPv4.String(), "-j", "DROP"},
-			[]string{"ebtables", "-t", "filter", "-A", "FORWARD", "-p", "IPv4", "-i", m["host_name"], "--ip-src", "!", IPv4.String(), "-j", "DROP"},
-		)
-	}
-
-	if shared.IsTrue(m["security.ipv6_filtering"]) && IPv6 != nil {
-		rules = append(rules,
-			// Allow DHCPv6 and Router Solicitation to the host only. This must come before the IP source filtering rules below.
-			[]string{"ebtables", "-t", "filter", "-A", "INPUT", "-p", "IPv6", "-s", m["hwaddr"], "-i", m["host_name"], "--ip6-src", "fe80::/ffc0::", "--ip6-dst", "ff02::1:2/ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff", "--ip6-proto", "udp", "--ip6-dport", "547", "-j", "ACCEPT"},
-			[]string{"ebtables", "-t", "filter", "-A", "INPUT", "-p", "IPv6", "-s", m["hwaddr"], "-i", m["host_name"], "--ip6-src", "fe80::/ffc0::", "--ip6-dst", "ff02::2/ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff", "--ip6-proto", "ipv6-icmp", "--ip6-icmp-type", "router-solicitation", "-j", "ACCEPT"},
-			// IP source filtering rules. Blocks any packet coming from instance with an incorrect IP source address.
-			[]string{"ebtables", "-t", "filter", "-A", "INPUT", "-p", "IPv6", "-i", m["host_name"], "--ip6-src", "!", fmt.Sprintf("%s/ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff", IPv6.String()), "-j", "DROP"},
-			[]string{"ebtables", "-t", "filter", "-A", "FORWARD", "-p", "IPv6", "-i", m["host_name"], "--ip6-src", "!", fmt.Sprintf("%s/ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff", IPv6.String()), "-j", "DROP"},
-		)
-	}
-
-	return rules
-}
-
-// matchEbtablesRule compares an active rule to a supplied match rule to see if they match.
-// If deleteMode is true then the "-A" flag in the active rule will be modified to "-D" and will
-// not be part of the equality match. This allows delete commands to be generated from dumped add commands.
-func (d *nicBridged) matchEbtablesRule(activeRule []string, matchRule []string, deleteMode bool) bool {
-	for i := range matchRule {
-		// Active rules will be dumped in "add" format, we need to detect
-		// this and switch it to "delete" mode if requested. If this has already been
-		// done then move on, as we don't want to break the comparison below.
-		if deleteMode && (activeRule[i] == "-A" || activeRule[i] == "-D") {
-			activeRule[i] = "-D"
-			continue
-		}
-
-		// Check the match rule field matches the active rule field.
-		// If they don't match, then this isn't one of our rules.
-		if strings.Replace(activeRule[i], "/ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff", "", -1) != strings.Replace(matchRule[i], "/ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff", "", -1) {
-			return false
-		}
-	}
-
-	return true
-}
-
 // setFilters sets up any network level filters defined for the instance.
 // These are controlled by the security.mac_filtering, security.ipv4_Filtering and security.ipv6_filtering config keys.
 func (d *nicBridged) setFilters() (err error) {
@@ -712,39 +650,6 @@ func (d *nicBridged) allocateFilterIPs(netConfig map[string]string) (net.IP, net
 	return IPv4, IPv6, nil
 }
 
-// generateFilterIptablesRules returns a customised set of iptables filter rules based on the device.
-func (d *nicBridged) generateFilterIptablesRules(m deviceConfig.Device, IPv6 net.IP) (rules [][]string, err error) {
-	mac, err := net.ParseMAC(m["hwaddr"])
-	if err != nil {
-		return
-	}
-
-	macHex := hex.EncodeToString(mac)
-
-	// These rules below are implemented using ip6tables because the functionality to inspect
-	// the contents of an ICMPv6 packet does not exist in ebtables (unlike for IPv4 ARP).
-	// Additionally, ip6tables doesn't really provide a nice way to do what we need here, so we
-	// have resorted to doing a raw hex comparison of the packet contents at fixed positions.
-	// If these rules are not added then it is possible to hijack traffic for another IP that is
-	// not assigned to the instance by sending a specially crafted gratuitous NDP packet with
-	// correct source address and MAC at the IP & ethernet layers, but a fraudulent IP or MAC
-	// inside the ICMPv6 NDP packet.
-	if shared.IsTrue(m["security.ipv6_filtering"]) && IPv6 != nil {
-		ipv6Hex := hex.EncodeToString(IPv6)
-
-		rules = append(rules,
-			// Prevent Neighbor Advertisement IP spoofing (prevents the instance redirecting traffic for IPs that are not its own).
-			[]string{"ipv6", "INPUT", "-i", m["parent"], "-p", "ipv6-icmp", "-m", "physdev", "--physdev-in", m["host_name"], "-m", "icmp6", "--icmpv6-type", "136", "-m", "string", "!", "--hex-string", fmt.Sprintf("|%s|", ipv6Hex), "--algo", "bm", "--from", "48", "--to", "64", "-j", "DROP"},
-			[]string{"ipv6", "FORWARD", "-i", m["parent"], "-p", "ipv6-icmp", "-m", "physdev", "--physdev-in", m["host_name"], "-m", "icmp6", "--icmpv6-type", "136", "-m", "string", "!", "--hex-string", fmt.Sprintf("|%s|", ipv6Hex), "--algo", "bm", "--from", "48", "--to", "64", "-j", "DROP"},
-			// Prevent Neighbor Advertisement MAC spoofing (prevents the instance poisoning the NDP cache of its neighbours with a MAC address that isn't its own).
-			[]string{"ipv6", "INPUT", "-i", m["parent"], "-p", "ipv6-icmp", "-m", "physdev", "--physdev-in", m["host_name"], "-m", "icmp6", "--icmpv6-type", "136", "-m", "string", "!", "--hex-string", fmt.Sprintf("|%s|", macHex), "--algo", "bm", "--from", "66", "--to", "72", "-j", "DROP"},
-			[]string{"ipv6", "FORWARD", "-i", m["parent"], "-p", "ipv6-icmp", "-m", "physdev", "--physdev-in", m["host_name"], "-m", "icmp6", "--icmpv6-type", "136", "-m", "string", "!", "--hex-string", fmt.Sprintf("|%s|", macHex), "--algo", "bm", "--from", "66", "--to", "72", "-j", "DROP"},
-		)
-	}
-
-	return
-}
-
 // networkDHCPv4Ranges returns a parsed set of DHCPv4 ranges for a particular network.
 func (d *nicBridged) networkDHCPv4Ranges(netConfig map[string]string) []dhcpRange {
 	dhcpRanges := make([]dhcpRange, 0)

From fb5073eaa435fa266978aa48601fd82b7ffd0a62 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Tue, 28 Jan 2020 17:01:05 +0100
Subject: [PATCH 2/2] lxd/iptables: Fix matching of IPv6 link-local
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
 lxd/iptables/xtables.go | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/lxd/iptables/xtables.go b/lxd/iptables/xtables.go
index 9c06a926bd..5b7c86a332 100644
--- a/lxd/iptables/xtables.go
+++ b/lxd/iptables/xtables.go
@@ -330,9 +330,15 @@ func matchEbtablesRule(activeRule []string, matchRule []string, deleteMode bool)
 			continue
 		}
 
+		// Mangle to line up with different versions of ebtables.
+		active := strings.Replace(activeRule[i], "/ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff", "", -1)
+		match := strings.Replace(matchRule[i], "/ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff", "", -1)
+		active = strings.Replace(active, "fe80::/ffc0::", "fe80::/10", -1)
+		match = strings.Replace(match, "fe80::/ffc0::", "fe80::/10", -1)
+
 		// Check the match rule field matches the active rule field.
 		// If they don't match, then this isn't one of our rules.
-		if strings.Replace(activeRule[i], "/ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff", "", -1) != strings.Replace(matchRule[i], "/ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff", "", -1) {
+		if active != match {
 			return false
 		}
 	}


More information about the lxc-devel mailing list