The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/6849
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) ===
If using a `bridged` NIC connected to an unmanaged bridge (or one that didn't use DHCP) with static IPs assigned (just for filtering purposes not DHCP allocation) then when the device is stopped the static IPs defined in config were not considered when removing filters.
This could lead to ebtables filter rules leaks.
Adds a test for this scenario too.
From 586544246055075ac066ee348712db0222af2b5a Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Fri, 7 Feb 2020 09:05:15 +0000
Subject: [PATCH 1/2] test/suites/container/devices/nic/bridged: Adds test to
detect leaked filters
When deleting/stopping an instance using security filtering on an unmanaged
network, the filters should be removed.
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
test/suites/container_devices_nic_bridged_filtering.sh | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/test/suites/container_devices_nic_bridged_filtering.sh
b/test/suites/container_devices_nic_bridged_filtering.sh
index fe19c1259d..7b2728f9b5 100644
--- a/test/suites/container_devices_nic_bridged_filtering.sh
+++ b/test/suites/container_devices_nic_bridged_filtering.sh
@@ -376,7 +376,12 @@ test_container_devices_nic_bridged_filtering() {
echo "Shouldn't be able to unset IPv6 address with ipv4_filtering enabled
and DHCPv6 disabled"
fi
+ # Delete container and check filters are cleaned up.
lxc delete -f "${ctPrefix}A"
+ if ebtables --concurrent -L --Lmac2 --Lx | grep -e "${ctAHost}" ; then
+ echo "ebtables filter still applied after delete"
+ false
+ fi
# Test MAC filtering on unmanaged bridge.
ip link add "${brName}2" type bridge
From 20df689d34cf948f1e97942fa78a907c21d2b683 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Fri, 7 Feb 2020 09:25:01 +0000
Subject: [PATCH 2/2] lxd/device/nic/bridged: Fixes bug that leaks ebtables
filters
When using static IPs with an unmanaged bridge parent.
Because there is no DHCP allocation in dnsmasq, previously the attempt to
remove the rules based on the device's static IPs was not attempted.
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/device/nic_bridged.go | 54 +++++++++++++++++++++++++++------------
1 file changed, 38 insertions(+), 16 deletions(-)
diff --git a/lxd/device/nic_bridged.go b/lxd/device/nic_bridged.go
index ad089ae747..2c60fb7b8f 100644
--- a/lxd/device/nic_bridged.go
+++ b/lxd/device/nic_bridged.go
@@ -360,10 +360,7 @@ func (d *nicBridged) postStop() error {
}
networkRemoveVethRoutes(d.config)
- err := d.removeFilters(d.config)
- if err != nil {
- logger.Errorf("Failed to remove nic filters: %v", err)
- }
+ d.removeFilters(d.config)
return nil
}
@@ -467,31 +464,56 @@ func (d *nicBridged) setupHostFilters(oldConfig
deviceConfig.Device) error {
}
// removeFilters removes any network level filters defined for the instance.
-func (d *nicBridged) removeFilters(m deviceConfig.Device) error {
+func (d *nicBridged) removeFilters(m deviceConfig.Device) {
if m["hwaddr"] == "" {
- return fmt.Errorf("Failed to remove network filters for %s:
hwaddr not defined", m["name"])
+ logger.Errorf("Failed to remove network filters for %s: hwaddr
not defined", m["name"])
+ return
}
if m["host_name"] == "" {
- return fmt.Errorf("Failed to remove network filters for %s:
host_name not defined", m["name"])
+ logger.Errorf("Failed to remove network filters for %s:
host_name not defined", m["name"])
+ return
}
// Remove any IPv6 filters used for this instance.
err := d.state.Firewall.InstanceClear(firewallConsts.FamilyIPv6,
firewallConsts.TableFilter, fmt.Sprintf("%s - ipv6_filtering", d.inst.Name()))
if err != nil {
- return fmt.Errorf("Failed to clear ip6tables ipv6_filter rules
for %s: %v", m["name"], err)
+ logger.Errorf("Failed to clear ip6tables ipv6_filter rules for
%s: %v", m["name"], err)
}
- // Read current static IP allocation configured from dnsmasq host
config (if exists).
- var IPv4, IPv6 dhcpAllocation
- if shared.PathExists(shared.VarPath("networks", m["parent"],
"dnsmasq.hosts") + "/" + d.inst.Name()) {
- IPv4, IPv6, err = d.getDHCPStaticIPs(m["parent"], d.inst.Name())
- if err != nil {
- return fmt.Errorf("Failed to retrieve static IPs for
filter removal from %s: %v", m["name"], err)
+ var IPv4, IPv6 net.IP
+
+ if m["ipv4.address"] != "" {
+ IPv4 = net.ParseIP(m["ipv4.address"])
+ }
+
+ if m["ipv6.address"] != "" {
+ IPv6 = net.ParseIP(m["ipv6.address"])
+ }
+
+ // Remove filters for static MAC and IPs (if specified above).
+ // This covers the case when filtering is used with an unmanaged bridge.
+ err = d.state.Firewall.InstanceNicBridgedRemoveFilters(m, IPv4, IPv6)
+ if err != nil {
+ logger.Errorf("Failed to remove nic static filters: %v", err)
+ }
+
+ // Read current static DHCP IP allocation configured from dnsmasq host
config (if exists).
+ // This covers the case when IPs are not defined in config, but have
been assigned in managed DHCP.
+ IPv4Alloc, IPv6Alloc, err := d.getDHCPStaticIPs(m["parent"],
d.inst.Name())
+ if err != nil {
+ if os.IsNotExist(err) {
+ return
}
+
+ logger.Errorf("Failed to retrieve static IPs for filter removal
from %s: %v", m["name"], err)
+ return
}
- return d.state.Firewall.InstanceNicBridgedRemoveFilters(m, IPv4.IP,
IPv6.IP)
+ err = d.state.Firewall.InstanceNicBridgedRemoveFilters(m, IPv4Alloc.IP,
IPv6Alloc.IP)
+ if err != nil {
+ logger.Errorf("Failed to remove nic DHCP assigned filters: %v",
err)
+ }
}
// getDHCPStaticIPs retrieves the dnsmasq statically allocated IPs for a
instance.
@@ -499,7 +521,7 @@ func (d *nicBridged) removeFilters(m deviceConfig.Device)
error {
func (d *nicBridged) getDHCPStaticIPs(network string, instanceName string)
(dhcpAllocation, dhcpAllocation, error) {
var IPv4, IPv6 dhcpAllocation
- file, err := os.Open(shared.VarPath("networks", network,
"dnsmasq.hosts") + "/" + instanceName)
+ file, err := os.Open(shared.VarPath("networks", network,
"dnsmasq.hosts", instanceName))
if err != nil {
return IPv4, IPv6, err
}
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
