[lxc-devel] [lxd/master] Firewall: Updates detection to warn when falling back to non-compatible xtables driver
tomponline on Github
lxc-bot at linuxcontainers.org
Wed May 20 16:03:31 UTC 2020
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 452 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200520/a9dcd3fd/attachment.bin>
-------------- next part --------------
From 33b8ecfeb4c0c2369b6866be39366aadd72a3622 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 20 May 2020 16:58:33 +0100
Subject: [PATCH 1/4] lxd/firewall/firewall/interface: Change definition of
Compat() to return compat issue error
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/firewall/firewall_interface.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lxd/firewall/firewall_interface.go b/lxd/firewall/firewall_interface.go
index 932c38ddb9..103b5fa11e 100644
--- a/lxd/firewall/firewall_interface.go
+++ b/lxd/firewall/firewall_interface.go
@@ -9,7 +9,7 @@ import (
// Firewall represents an LXD firewall.
type Firewall interface {
String() string
- Compat() (bool, bool)
+ Compat() (bool, error)
NetworkSetupForwardingPolicy(networkName string, ipVersion uint, allow bool) error
NetworkSetupOutboundNAT(networkName string, subnet *net.IPNet, srcIP net.IP, append bool) error
From b3f3013db74bbf2fba135874bf14d66c2fd8bf0b Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 20 May 2020 16:59:12 +0100
Subject: [PATCH 2/4] lxd/firewall/drivers/driver/nftables: Updates Compat() to
return compat issues as error
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/firewall/drivers/drivers_nftables.go | 26 ++++++++++--------------
1 file changed, 11 insertions(+), 15 deletions(-)
diff --git a/lxd/firewall/drivers/drivers_nftables.go b/lxd/firewall/drivers/drivers_nftables.go
index f4d295304e..5b99d3b292 100644
--- a/lxd/firewall/drivers/drivers_nftables.go
+++ b/lxd/firewall/drivers/drivers_nftables.go
@@ -15,7 +15,6 @@ import (
deviceConfig "github.com/lxc/lxd/lxd/device/config"
"github.com/lxc/lxd/lxd/project"
"github.com/lxc/lxd/shared"
- "github.com/lxc/lxd/shared/logger"
"github.com/lxc/lxd/shared/version"
)
@@ -38,60 +37,57 @@ func (d Nftables) String() string {
return "nftables"
}
-// Compat returns whether the host is compatible with this driver and whether the driver backend is in use.
-func (d Nftables) Compat() (bool, bool) {
+// Compat returns whether the driver backend is in use, and any host compatibility errors.
+func (d Nftables) Compat() (bool, error) {
// Get the kernel version.
uname, err := shared.Uname()
if err != nil {
- return false, false
+ return false, err
}
// We require a 5.x kernel to avoid weird conflicts with xtables.
if len(uname.Release) > 1 {
verInt, err := strconv.Atoi(uname.Release[0:1])
if err != nil {
- return false, false
+ return false, errors.Wrapf(err, "Failed parsing kernel version")
}
if verInt < 5 {
- return false, false
+ return false, fmt.Errorf("Kernel version does not meet minimum requirement of 5")
}
}
// Check if nftables nft command exists, if not use xtables.
_, err = exec.LookPath("nft")
if err != nil {
- return false, false
+ return false, fmt.Errorf("Backend command %q missing", "nft")
}
// Get nftables version.
nftVersion, err := d.hostVersion()
if err != nil {
- logger.Debugf("Firewall nftables failed detecting nft version: %v", err)
- return false, false
+ return false, errors.Wrapf(err, "Failed detecting nft version")
}
// Check nft version meets minimum required.
minVer, _ := version.NewDottedVersion(nftablesMinVersion)
if nftVersion.Compare(minVer) < 0 {
- logger.Debugf("Firewall nftables detected nft version %q is too low, need %q or above", nftVersion, nftablesMinVersion)
- return false, false
+ return false, fmt.Errorf("nft version %q is too low, need %q or above", nftVersion, nftablesMinVersion)
}
// Check whether in use by parsing ruleset and looking for existing rules.
ruleset, err := d.nftParseRuleset()
if err != nil {
- logger.Errorf("Firewall nftables unable to parse existing ruleset: %v", err)
- return false, false
+ return false, errors.Wrapf(err, "Failed parsing nftables existing ruleset")
}
for _, item := range ruleset {
if item.Type == "rule" {
- return true, true // At least one rule found indicates in use.
+ return true, nil // At least one rule found indicates in use.
}
}
- return true, false
+ return false, nil
}
// nftGenericItem represents some common fields amongst the different nftables types.
From 964f6522faf25b8c35efdea4998f5bc2b18e99ec Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 20 May 2020 16:59:35 +0100
Subject: [PATCH 3/4] lxd/firewall/drivers/drivers/xtables: Updates Compat() to
return compat issues as error
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/firewall/drivers/drivers_xtables.go | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/lxd/firewall/drivers/drivers_xtables.go b/lxd/firewall/drivers/drivers_xtables.go
index 836dfce32e..728161786e 100644
--- a/lxd/firewall/drivers/drivers_xtables.go
+++ b/lxd/firewall/drivers/drivers_xtables.go
@@ -25,8 +25,8 @@ func (d Xtables) String() string {
return "xtables"
}
-// Compat returns whether the host is compatible with this driver and whether the driver backend is in use.
-func (d Xtables) Compat() (bool, bool) {
+// Compat returns whether the driver backend is in use, and any host compatibility errors.
+func (d Xtables) Compat() (bool, error) {
// xtables commands can be powered by nftables, so check we are using non-nft version first, otherwise
// we should be using the nftables driver instead.
cmds := []string{"iptables", "ip6tables", "ebtables"}
@@ -34,34 +34,32 @@ func (d Xtables) Compat() (bool, bool) {
// Check command exists.
_, err := exec.LookPath(cmd)
if err != nil {
- logger.Debugf("Firewall xtables backend command %q missing", cmd)
- return false, false
+ return false, fmt.Errorf("Backend command %q missing", cmd)
}
// Check whether it is an nftables shim.
if d.xtablesIsNftables(cmd) {
- logger.Debugf("Firewall xtables backend command %q is an nftables shim", cmd)
- return false, false
+ return false, fmt.Errorf("Backend command %q is an nftables shim", cmd)
}
}
// Check whether any of the backends are in use already.
if d.iptablesInUse("iptables") {
logger.Debug("Firewall xtables detected iptables is in use")
- return true, true
+ return true, nil
}
if d.iptablesInUse("ip6tables") {
logger.Debug("Firewall xtables detected ip6tables is in use")
- return true, true
+ return true, nil
}
if d.ebtablesInUse() {
logger.Debug("Firewall xtables detected ebtables is in use")
- return true, true
+ return true, nil
}
- return true, false
+ return false, nil
}
// xtablesIsNftables checks whether the specified xtables backend command is actually an nftables shim.
From f6db49212b3bcd6f44a25c66d339d2453336529d Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 20 May 2020 17:01:07 +0100
Subject: [PATCH 4/4] lxd/firewall/firewall/load: Updates driver detection to
warn when falling back to non-compatible xtables
And why it is not compatible.
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/firewall/firewall_load.go | 42 ++++++++++++++++++++++-------------
1 file changed, 26 insertions(+), 16 deletions(-)
diff --git a/lxd/firewall/firewall_load.go b/lxd/firewall/firewall_load.go
index 367a4ec0d2..b85a9eb273 100644
--- a/lxd/firewall/firewall_load.go
+++ b/lxd/firewall/firewall_load.go
@@ -2,6 +2,7 @@ package firewall
import (
"github.com/lxc/lxd/lxd/firewall/drivers"
+ "github.com/lxc/lxd/shared/logger"
)
// New returns an appropriate firewall implementation.
@@ -10,26 +11,35 @@ func New() Firewall {
nftables := drivers.Nftables{}
xtables := drivers.Xtables{}
- // If nftables is compatible and already in use, then we prefer to use the nftables driver irrespective of
- // whether xtables is in use or not.
- nftablesCompat, nftablesInUse := nftables.Compat()
- if nftablesCompat && nftablesInUse {
+ nftablesInUse, nftablesCompatErr := nftables.Compat()
+ if nftablesCompatErr != nil {
+ logger.Debugf(`Firewall detected "nftables" incompatibility: %v`, nftablesCompatErr)
+ } else if nftablesInUse {
+ // If nftables is compatible and already in use, then we prefer to use the nftables driver
+ // irrespective of whether xtables is in use or not.
return nftables
- } else if !nftablesCompat {
- // Note: If nftables isn't compatible, we fallback to xtables without considering whether xtables
- // is itself compatible. This continues the existing behaviour of allowing LXD to start with
- // potentially an incomplete firewall backend, so that only networks and instances using those
- // features will fail to start later.
- return xtables
}
- // If xtables is compatible and already in use, then we prefer to stick with the xtables driver rather than
- // mix the use of firewall drivers on the system.
- xtablesCompat, xtablesInUse := xtables.Compat()
- if xtablesCompat && xtablesInUse {
+ xtablesInUse, xtablesCompatErr := xtables.Compat()
+ if xtablesCompatErr != nil {
+ logger.Debugf(`Firewall detected "xtables" incompatibility: %v`, xtablesCompatErr)
+ } else if xtablesInUse {
+ // If xtables is compatible and already in use, then we prefer to stick with the xtables driver
+ // rather than mix the use of firewall drivers on the system.
return xtables
}
- // Otherwise prefer nftables as default.
- return nftables
+ // If nftables is compatible, but not in use, and xtables is not compatible or not in use, use nftables.
+ if nftablesCompatErr == nil {
+ return nftables
+ }
+
+ // If neither nftables nor xtables are compatible, we fallback to xtables.
+ // This continues the existing behaviour of allowing LXD to start with potentially an incomplete firewall
+ // backend, so that only networks and instances using those features may fail to function properly.
+ // The most common scenario for this is when xtables is using nft shim commands but the nft command itself
+ // is not installed. In this case LXD will use the xtables shim commands but with the potential of problems
+ // due to differences between the original xtables commands and the shim commands provided by nft.
+ logger.Warnf(`Firewall failed to detect any compatible driver, falling back to "xtables" (but some features may not work as expected due to: %v)`, xtablesCompatErr)
+ return xtables
}
More information about the lxc-devel
mailing list