[lxc-devel] [lxd/master] Bridged IP Filtering DHCP checks

tomponline on Github lxc-bot at linuxcontainers.org
Thu Aug 1 08:59:30 UTC 2019


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 426 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20190801/1f188fd2/attachment.bin>
-------------- next part --------------
From 8829e5e464faac6174c21911a3512199902686d8 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 1 Aug 2019 09:56:03 +0100
Subject: [PATCH 1/2] device/nic/bridged: Adds checks for DHCP being enabled if
 no static IP set

Because IP filtering depends on dnsmasq's DHCP for dynamic IP allocation, it must be enabled in order to use IP filtering when no static IP is set in the device's config.

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

diff --git a/lxd/device/nic_bridged.go b/lxd/device/nic_bridged.go
index 043801e989..d2f08a3dc7 100644
--- a/lxd/device/nic_bridged.go
+++ b/lxd/device/nic_bridged.go
@@ -559,6 +559,9 @@ func (d *nicBridged) setFilters() (err error) {
 
 	// Retrieve existing IPs, or allocate new ones if needed.
 	IPv4, IPv6, err := d.allocateFilterIPs()
+	if err != nil {
+		return err
+	}
 
 	// If anything goes wrong, clean up so we don't leave orphaned rules.
 	defer func() {
@@ -598,7 +601,7 @@ func (d *nicBridged) allocateFilterIPs() (net.IP, net.IP, error) {
 	if d.config["ipv4.address"] != "" {
 		IPv4 = net.ParseIP(d.config["ipv4.address"])
 		if IPv4 == nil {
-			return IPv4, IPv6, fmt.Errorf("Invalid static IPv4 address %s", d.config["ipv4.address"])
+			return nil, nil, fmt.Errorf("Invalid static IPv4 address %s", d.config["ipv4.address"])
 		}
 	}
 
@@ -606,31 +609,41 @@ func (d *nicBridged) allocateFilterIPs() (net.IP, net.IP, error) {
 	if d.config["ipv6.address"] != "" {
 		IPv6 = net.ParseIP(d.config["ipv6.address"])
 		if IPv6 == nil {
-			return IPv4, IPv6, fmt.Errorf("Invalid static IPv6 address %s", d.config["ipv6.address"])
+			return nil, nil, fmt.Errorf("Invalid static IPv6 address %s", d.config["ipv6.address"])
 		}
 	}
 
+	_, dbInfo, err := d.state.Cluster.NetworkGet(d.config["parent"])
+	if err != nil {
+		return nil, nil, err
+	}
+
+	netConfig := dbInfo.Config
+
+	// Check DHCPv4 is enabled on parent if dynamic IPv4 allocation is needed.
+	if shared.IsTrue(d.config["security.ipv4_filtering"]) && IPv4 == nil && netConfig["ipv4.dhcp"] != "" && !shared.IsTrue(netConfig["ipv4.dhcp"]) {
+		return nil, nil, fmt.Errorf("Cannot use security.ipv4_filtering as DHCPv4 is disabled on parent %s and no static IPv4 address set", d.config["parent"])
+	}
+
+	// Check DHCPv6 is enabled on parent if dynamic IPv6 allocation is needed.
+	if shared.IsTrue(d.config["security.ipv6_filtering"]) && IPv6 == nil && netConfig["ipv6.dhcp"] != "" && !shared.IsTrue(netConfig["ipv6.dhcp"]) {
+		return nil, nil, fmt.Errorf("Cannot use security.ipv6_filtering as DHCPv6 is disabled on parent %s and no static IPv6 address set", d.config["parent"])
+	}
+
 	dnsmasq.ConfigMutex.Lock()
 	defer dnsmasq.ConfigMutex.Unlock()
 
 	// Read current static IP allocation configured from dnsmasq host config (if exists).
 	curIPv4, curIPv6, err := d.getDHCPStaticIPs(d.config["parent"], d.instance.Name())
 	if err != nil && !os.IsNotExist(err) {
-		return IPv4, IPv6, err
-	}
-
-	_, dbInfo, err := d.state.Cluster.NetworkGet(d.config["parent"])
-	if err != nil {
-		return IPv4, IPv6, err
+		return nil, nil, err
 	}
 
-	netConfig := dbInfo.Config
-
 	// If no static IPv4, then check if there is a valid static DHCP IPv4 address defined.
 	if IPv4 == nil && curIPv4.IP != nil {
 		_, subnet, err := net.ParseCIDR(netConfig["ipv4.address"])
 		if err != nil {
-			return IPv4, IPv6, err
+			return nil, nil, err
 		}
 
 		// Check the existing static DHCP IP is still valid in the subnet & ranges, if not
@@ -661,14 +674,14 @@ func (d *nicBridged) allocateFilterIPs() (net.IP, net.IP, error) {
 		// Get existing allocations in network.
 		IPv4Allocs, IPv6Allocs, err := d.getDHCPAllocatedIPs(d.config["parent"])
 		if err != nil {
-			return IPv4, IPv6, err
+			return nil, nil, err
 		}
 
 		// Allocate a new IPv4 address is IPv4 filtering enabled.
 		if IPv4 == nil && shared.IsTrue(d.config["security.ipv4_filtering"]) {
 			IPv4, err = d.getDHCPFreeIPv4(IPv4Allocs, netConfig, d.instance.Name(), d.config["hwaddr"])
 			if err != nil {
-				return IPv4, IPv6, err
+				return nil, nil, err
 			}
 		}
 
@@ -676,7 +689,7 @@ func (d *nicBridged) allocateFilterIPs() (net.IP, net.IP, error) {
 		if IPv6 == nil && shared.IsTrue(d.config["security.ipv6_filtering"]) {
 			IPv6, err = d.getDHCPFreeIPv6(IPv6Allocs, netConfig, d.instance.Name(), d.config["hwaddr"])
 			if err != nil {
-				return IPv4, IPv6, err
+				return nil, nil, err
 			}
 		}
 	}
@@ -695,12 +708,12 @@ func (d *nicBridged) allocateFilterIPs() (net.IP, net.IP, error) {
 
 		err = dnsmasq.UpdateStaticEntry(d.config["parent"], d.instance.Project(), d.instance.Name(), netConfig, d.config["hwaddr"], IPv4Str, IPv6Str)
 		if err != nil {
-			return IPv4, IPv6, err
+			return nil, nil, err
 		}
 
 		err = dnsmasq.Kill(d.config["parent"], true)
 		if err != nil {
-			return IPv4, IPv6, err
+			return nil, nil, err
 		}
 	}
 

From c43f4185074b7530d9c825997ac4095ea7698d01 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 1 Aug 2019 09:57:44 +0100
Subject: [PATCH 2/2] test: Adds nic bridged filtering tests for when DHCP is
 disabled

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

diff --git a/test/suites/container_devices_nic_bridged_filtering.sh b/test/suites/container_devices_nic_bridged_filtering.sh
index ac10a28b82..a94c43b009 100644
--- a/test/suites/container_devices_nic_bridged_filtering.sh
+++ b/test/suites/container_devices_nic_bridged_filtering.sh
@@ -221,13 +221,13 @@ test_container_devices_nic_bridged_filtering() {
 
   # Check IPv6 filter is present in ebtables.
   if ! ebtables -L --Lmac2 --Lx | grep -e "2001:db8::2" ; then
-       echo "IPv6 filter not applied as part of ipv6_filtering in ebtables"
+       echo "IPv6 ebtables filter not applied as part of ipv6_filtering in ebtables"
       false
   fi
 
   # Check IPv6 filter is present in ip6tables.
   if ! ip6tables -S -w -t filter | grep -e "20010db8000000000000000000000002" ; then
-      echo "IPv6 filter not applied as part of ipv6_filtering in ip6tables"
+      echo "IPv6 ip6tables filter not applied as part of ipv6_filtering in ip6tables"
       false
   fi
 
@@ -314,6 +314,59 @@ test_container_devices_nic_bridged_filtering() {
 
   lxc delete -f "${ctPrefix}A"
   lxc delete -f "${ctPrefix}B"
+
+  # Check filtering works with non-DHCP statically defined IPs.
+  lxc network set "${brName}" ipv4.dhcp false
+  lxc network set "${brName}" ipv6.dhcp false
+  lxc network set "${brName}" ipv6.dhcp.stateful false
+  lxc init testimage "${ctPrefix}A" -p "${ctPrefix}"
+  lxc config device add "${ctPrefix}A" eth0 nic nictype=nic name=eth0 nictype=bridged parent="${brName}" ipv4.address=192.0.2.2 ipv6.address=2001:db8::2 security.ipv4_filtering=true security.ipv6_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 -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"
+      false
+  fi
+
+  # Check MAC filter is present in ip6tables.
+  macHex=$(echo "${ctAMAC}" |sed "s/://g")
+  if ! ip6tables -S -w -t filter | grep -e "${macHex}" ; then
+      echo "MAC ip6tables filter not applied as part of ipv6_filtering in ip6tables"
+      false
+  fi
+
+  # Check IPv4 filter is present in ebtables.
+  if ! ebtables -L --Lmac2 --Lx | grep -e "192.0.2.2" ; then
+      echo "IPv4 filter not applied as part of ipv4_filtering in ebtables"
+      false
+  fi
+
+  # Check IPv6 filter is present in ebtables.
+  if ! ebtables -L --Lmac2 --Lx | grep -e "2001:db8::2" ; then
+       echo "IPv6 filter not applied as part of ipv6_filtering in ebtables"
+      false
+  fi
+
+  # Check IPv6 filter is present in ip6tables.
+  if ! ip6tables -S -w -t filter | grep -e "20010db8000000000000000000000002" ; then
+      echo "IPv6 filter not applied as part of ipv6_filtering in ip6tables"
+      false
+  fi
+
+  # Check that you cannot remove static IPs with filtering enabled and DHCP disabled.
+  if lxc config device unset "${ctPrefix}A" eth0 ipv4.address ; then
+    echo "Shouldn't be able to unset IPv4 address with ipv4_filtering enabled and DHCPv4 disabled"
+  fi
+
+  if lxc config device unset "${ctPrefix}A" eth0 ipv6.address ; then
+    echo "Shouldn't be able to unset IPv6 address with ipv4_filtering enabled and DHCPv6 disabled"
+  fi
+
+  # Cleanup.
+  lxc delete -f "${ctPrefix}A"
   lxc network delete "${brName}"
   lxc profile delete "${ctPrefix}"
 }


More information about the lxc-devel mailing list