The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/6358
This e-mail was sent by the LXC bot, direct replies will not reach the author
unless they happen to be subscribed to this list.
=== Description (from pull-request) ===
Fixes https://discuss.linuxcontainers.org/t/lxd-3-18-security-mac-filtering-on-bridged-nic-fails/6022/9
From d993f4a7a2399caeeab727415bcb8429f1ed67e7 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@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.parr...@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.parr...@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.parr...@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}"
}
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
