[lxc-devel] [lxd/master] Allow MAC filtering on unmanaged parent bridge

tomponline on Github lxc-bot at linuxcontainers.org
Mon Oct 28 11:23:00 UTC 2019


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 404 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20191028/649a77c7/attachment.bin>
-------------- next part --------------
From d993f4a7a2399caeeab727415bcb8429f1ed67e7 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 28 Oct 2019 11:20:35 +0000
Subject: [PATCH 1/2] lxd/device/nic/bridged: Allow MAC filtering on unmanaged
 bridges

Allows to use the security.mac_filtering nic option when parent is an unmanaged bridge.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/device/nic_bridged.go | 42 +++++++++++++++++++++++++--------------
 1 file changed, 27 insertions(+), 15 deletions(-)

diff --git a/lxd/device/nic_bridged.go b/lxd/device/nic_bridged.go
index 8b2776ea58..b4a26ff265 100644
--- a/lxd/device/nic_bridged.go
+++ b/lxd/device/nic_bridged.go
@@ -19,6 +19,7 @@ import (
 	"github.com/google/gopacket/layers"
 	"github.com/mdlayher/eui64"
 
+	"github.com/lxc/lxd/lxd/db"
 	deviceConfig "github.com/lxc/lxd/lxd/device/config"
 	"github.com/lxc/lxd/lxd/dnsmasq"
 	"github.com/lxc/lxd/lxd/instance/instancetype"
@@ -265,7 +266,10 @@ func (d *nicBridged) postStop() error {
 	}
 
 	networkRemoveVethRoutes(d.config)
-	d.removeFilters(d.config)
+	err := d.removeFilters(d.config)
+	if err != nil {
+		logger.Errorf("Failed to remove nic filters: %v", err)
+	}
 
 	return nil
 }
@@ -385,9 +389,12 @@ func (d *nicBridged) removeFilters(m deviceConfig.Device) error {
 	}
 
 	// Read current static IP allocation configured from dnsmasq host config (if exists).
-	IPv4, IPv6, err := d.getDHCPStaticIPs(m["parent"], d.instance.Name())
-	if err != nil {
-		return fmt.Errorf("Failed to remove network filters for %s: %v", m["name"], err)
+	var IPv4, IPv6 dhcpAllocation
+	if shared.PathExists(shared.VarPath("networks", m["parent"], "dnsmasq.hosts") + "/" + d.instance.Name()) {
+		IPv4, IPv6, err = d.getDHCPStaticIPs(m["parent"], d.instance.Name())
+		if err != nil {
+			return fmt.Errorf("Failed to retrieve static IPs for filter removal from %s: %v", m["name"], err)
+		}
 	}
 
 	// Get a current list of rules active on the host.
@@ -564,12 +571,22 @@ func (d *nicBridged) setFilters() (err error) {
 		}
 	}
 
-	// Retrieve existing IPs, or allocate new ones if needed.
-	IPv4, IPv6, err := d.allocateFilterIPs()
-	if err != nil {
+	// Check if the parent is managed and load config. If parent is unmanaged continue anyway.
+	var IPv4, IPv6 net.IP
+	_, netInfo, err := d.state.Cluster.NetworkGet(d.config["parent"])
+	if err != nil && err != db.ErrNoSuchObject {
 		return err
 	}
 
+	// If network is unmanaged we cannot allocate static IPs.
+	if netInfo != nil {
+		// Retrieve existing IPs, or allocate new ones if needed.
+		IPv4, IPv6, err = d.allocateFilterIPs(netInfo.Config)
+		if err != nil {
+			return err
+		}
+	}
+
 	// If anything goes wrong, clean up so we don't leave orphaned rules.
 	defer func() {
 		if err != nil {
@@ -601,7 +618,9 @@ func (d *nicBridged) setFilters() (err error) {
 }
 
 // networkAllocateVethFilterIPs retrieves previously allocated IPs, or allocate new ones if needed.
-func (d *nicBridged) allocateFilterIPs() (net.IP, net.IP, error) {
+// This function only works with LXD managed networks, and as such, requires the managed network's
+// config to be supplied.
+func (d *nicBridged) allocateFilterIPs(netConfig map[string]string) (net.IP, net.IP, error) {
 	var IPv4, IPv6 net.IP
 
 	// Check if there is a valid static IPv4 address defined.
@@ -620,13 +639,6 @@ func (d *nicBridged) allocateFilterIPs() (net.IP, net.IP, error) {
 		}
 	}
 
-	_, dbInfo, err := d.state.Cluster.NetworkGet(d.config["parent"])
-	if err != nil {
-		return nil, nil, err
-	}
-
-	netConfig := dbInfo.Config
-
 	// Check the conditions required to dynamically allocated IPs.
 	canIPv4Allocate := netConfig["ipv4.address"] != "" && netConfig["ipv4.address"] != "none" && (netConfig["ipv4.dhcp"] == "" || shared.IsTrue(netConfig["ipv4.dhcp"]))
 	canIPv6Allocate := netConfig["ipv6.address"] != "" && netConfig["ipv6.address"] != "none" && (netConfig["ipv6.dhcp"] == "" || shared.IsTrue(netConfig["ipv6.dhcp"]))

From ab0236efb708366d412bdb87e614301ae5d1c49d Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 28 Oct 2019 11:21:15 +0000
Subject: [PATCH 2/2] test: Adds test for using security.mac_filtering with
 unmanaged parent

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 ...container_devices_nic_bridged_filtering.sh | 50 +++++++++++++++----
 1 file changed, 41 insertions(+), 9 deletions(-)

diff --git a/test/suites/container_devices_nic_bridged_filtering.sh b/test/suites/container_devices_nic_bridged_filtering.sh
index a7a3eeb76f..9376b2aaa8 100644
--- a/test/suites/container_devices_nic_bridged_filtering.sh
+++ b/test/suites/container_devices_nic_bridged_filtering.sh
@@ -311,13 +311,6 @@ test_container_devices_nic_bridged_filtering() {
   lxc stop -f "${ctPrefix}A"
   lxc stop -f "${ctPrefix}B"
 
-  # Check we haven't left any NICS lying around.
-  endNicCount=$(find /sys/class/net | wc -l)
-  if [ "$startNicCount" != "$endNicCount" ]; then
-    echo "leftover NICS detected"
-    false
-  fi
-
   lxc delete -f "${ctPrefix}A"
   lxc delete -f "${ctPrefix}B"
 
@@ -345,7 +338,7 @@ test_container_devices_nic_bridged_filtering() {
   ctAHost=$(lxc config get "${ctPrefix}A" volatile.eth0.host_name)
   ctAMAC=$(lxc config get "${ctPrefix}A" volatile.eth0.hwaddr)
   if ! ebtables --concurrent -L --Lmac2 --Lx | grep -e "-s ! ${ctAMAC} -i ${ctAHost} -j DROP" ; then
-      echo "MAC ebtables filter not applied as part of ipv6_filtering in ebtables"
+      echo "MAC ebtables filter not applied as part of ip_filtering in ebtables"
       false
   fi
 
@@ -383,8 +376,47 @@ test_container_devices_nic_bridged_filtering() {
     echo "Shouldn't be able to unset IPv6 address with ipv4_filtering enabled and DHCPv6 disabled"
   fi
 
-  # Cleanup.
   lxc delete -f "${ctPrefix}A"
+
+  # Test MAC filtering on unmanaged bridge.
+  ip link add "${brName}2" type bridge
+
+  lxc init testimage "${ctPrefix}A" -p "${ctPrefix}"
+  lxc config device add "${ctPrefix}A" eth0 nic \
+    nictype=nic \
+    name=eth0 \
+    nictype=bridged \
+    parent="${brName}2" \
+    security.mac_filtering=true
+  lxc start "${ctPrefix}A"
+
+  # Check MAC filter is present in ebtables.
+  ctAHost=$(lxc config get "${ctPrefix}A" volatile.eth0.host_name)
+  ctAMAC=$(lxc config get "${ctPrefix}A" volatile.eth0.hwaddr)
+  if ! ebtables --concurrent -L --Lmac2 --Lx | grep -e "-s ! ${ctAMAC} -i ${ctAHost} -j DROP" ; then
+      echo "MAC ebtables filter not applied as part of mac_filtering in ebtables"
+      false
+  fi
+
+  # Stop container and check filters are cleaned up.
+  lxc stop -f "${ctPrefix}A"
+  if ebtables --concurrent -L --Lmac2 --Lx | grep -e "${ctAHost}" ; then
+      echo "MAC filter still applied as part of mac_filtering in ebtables"
+      false
+  fi
+
+
+  lxc delete -f "${ctPrefix}A"
+  ip link delete "${brName}2"
+
+  # Check we haven't left any NICS lying around.
+  endNicCount=$(find /sys/class/net | wc -l)
+  if [ "$startNicCount" != "$endNicCount" ]; then
+    echo "leftover NICS detected"
+    false
+  fi
+
+  # Cleanup.
   lxc network delete "${brName}"
   lxc profile delete "${ctPrefix}"
 }


More information about the lxc-devel mailing list