The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/6875

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) ===
This is a WIP that adds initial nftables support to the `firewall` package.

- [ ] Network rules
- [ ] Instance filtering rules

One way that nftables is quite different to iptables is that there are no built in tables or chains.
Additionally `tables` in nftables are just namespaces (or containers) for chains and their rules, they have no inherent meaning to nftables itself.

Chains are added to tables, and so-called "base chains" hook themselves into the netfilter subsystem at the required point. Only then will these chains actually handle packets. Chains hook themselves into the netfilter system using a priority. I've used the same priorities as iptables does when hooking into netfilter as documented https://wiki.nftables.org/wiki-nftables/index.php/Configuring_chains#Base_chain_priority.

From from my research, if multiple chains are added to the same hook point with the same priority, netfilter processes them in the order they were added.

This means the user must come up with a suitable set of tables and chains for their purposes.

I have gone with the approach of using a single top-level table called `lxd` for each IP family (`ip` and `ip6`, and there will likely be another for bridge filtering). 

Inside the `lxd` tables several chains are created for each LXD managed network (prefixed with `in`, `fo` and `pr` for each netfilter hook type of `input`, `forwarding` and `postrouting` respectively, followed by the network name). A chain's name is a maximum length of 31 chars, so this should comfortably accommodate the network device name.

Each logical function (such as forward policy, outbound NAT and DHCP/DNS access) is implemented as a separate nftables config template in the same format as is generated from `nft list ruleset`, this means that it includes the table, chain and multiple rules in a single config definition, which is then applied atomically using a single command.

I've checked and if the table/chain exists already, the config is merged into it, rather than replacing it.

Because each managed network has its own chains, cleaning up the network is simply a case of removing those chains if they exist. I've not been able to figure out a way of listing chains in a table. In later versions of `nft`it is possible to dump the ruleset for an entire table in JSON, which would allow one to get a list of chains. However the version of `nft` in ubuntu 18.04 doesn't have this option, and the native dump format is non-trivial to parse. So I have resorted to attempting to list the rules in a specific chain and if that command fails then using that as an indication the chain doesn't exist and so doesn't need removing.

The `NetworkSetupOutboundNAT` function definition has an `append` boolean which indicates whether to add the rule at the start or the end of the chain in iptables. However with nftables, using our own chains, this doesn't make a lot of sense, and so I have ignored it.

However I do intend to do further testing to see how the default policies of other non-LXD chains may impact this (from looking at the nftables documentation, if a packet is accepted by one chain, it is still offered to other chains for the possibility of dropping it later).

The `NetworkSetupDHCPv4Checksum` always returns `nil` as this is not supported by nftables, and was only ever needed for older DHCP clients, so hopefully this isn't an issue anymore.

Example network rules config:

```
table ip lxd {
	chain in_lxdbr0 {
		type filter hook input priority 0; policy accept;
		iifname "lxdbr0" tcp dport domain accept
		iifname "lxdbr0" udp dport domain accept
		iifname "lxdbr0" udp dport bootps accept
	}

	chain fwd_lxdbr0 {
		type filter hook forward priority 0; policy accept;
		oifname "lxdbr0" accept
		iifname "lxdbr0" accept
	}

	chain nat_lxdbr0 {
		type nat hook postrouting priority 100; policy accept;
		ip saddr 10.16.75.0/24 ip daddr != 10.16.75.0/24 masquerade
	}
}
table ip6 lxd {
	chain in_lxdbr0 {
		type filter hook input priority 0; policy accept;
		iifname "lxdbr0" tcp dport domain accept
		iifname "lxdbr0" udp dport domain accept
		iifname "lxdbr0" udp dport bootps accept
	}

	chain fwd_lxdbr0 {
		type filter hook forward priority 0; policy accept;
		oifname "lxdbr0" accept
		iifname "lxdbr0" accept
	}

	chain nat_lxdbr0 {
		type nat hook postrouting priority 100; policy accept;
		ip6 saddr fd42:6792:c25a:f818::/64 ip6 daddr != fd42:6792:c25a:f818::/64 masquerade
	}
}
```

From 912fb4ea74231759d16b15cb8a3546035f4d00c4 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Wed, 12 Feb 2020 08:51:34 +0000
Subject: [PATCH 1/6] lxd/firewall/drivers/drivers/nftables: Initial structure

Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
 lxd/firewall/drivers/drivers_nftables.go | 55 ++++++++++++++++++++++++
 1 file changed, 55 insertions(+)
 create mode 100644 lxd/firewall/drivers/drivers_nftables.go

diff --git a/lxd/firewall/drivers/drivers_nftables.go 
b/lxd/firewall/drivers/drivers_nftables.go
new file mode 100644
index 0000000000..d1435a8c76
--- /dev/null
+++ b/lxd/firewall/drivers/drivers_nftables.go
@@ -0,0 +1,55 @@
+package drivers
+
+import (
+       deviceConfig "github.com/lxc/lxd/lxd/device/config"
+       "net"
+)
+
+// Nftables is an implmentation of LXD firewall using nftables.
+type Nftables struct{}
+
+// NetworkSetupForwardingPolicy allows forwarding dependent on boolean argument
+func (d Nftables) NetworkSetupForwardingPolicy(networkName string, ipVersion 
uint, allow bool) error {
+       return nil
+}
+
+// NetworkSetupOutboundNAT configures outbound NAT.
+// If srcIP is non-nil then SNAT is used with the specified address, otherwise 
MASQUERADE mode is used.
+func (d Nftables) NetworkSetupOutboundNAT(networkName string, subnet 
*net.IPNet, srcIP net.IP, append bool) error {
+       return nil
+}
+
+// NetworkSetupDHCPDNSAccess sets up basic iptables overrides for DHCP/DNS.
+func (d Nftables) NetworkSetupDHCPDNSAccess(networkName string, ipVersion 
uint) error {
+       return nil
+}
+
+// NetworkSetupDHCPv4Checksum attempts a workaround for broken DHCP clients.
+func (d Nftables) NetworkSetupDHCPv4Checksum(networkName string) error {
+       return nil
+}
+
+// NetworkClear removes network rules from filter, mangle and nat tables.
+func (d Nftables) NetworkClear(networkName string, ipVersion uint) error {
+       return nil
+}
+
+// InstanceSetupBridgeFilter sets up the filter rules to apply bridged device 
IP filtering.
+func (d Nftables) InstanceSetupBridgeFilter(projectName, instanceName, 
deviceName, parentName, hostName, hwAddr string, IPv4, IPv6 net.IP) error {
+       return nil
+}
+
+// InstanceClearBridgeFilter removes any filter rules that were added to apply 
bridged device IP filtering.
+func (d Nftables) InstanceClearBridgeFilter(projectName, instanceName, 
deviceName, parentName, hostName, hwAddr string, IPv4, IPv6 net.IP) error {
+       return nil
+}
+
+// InstanceSetupProxyNAT creates DNAT rules for proxy devices.
+func (d Nftables) InstanceSetupProxyNAT(projectName, instanceName, deviceName 
string, listen, connect *deviceConfig.ProxyAddress) error {
+       return nil
+}
+
+// InstanceClearProxyNAT remove DNAT rules for proxy devices.
+func (d Nftables) InstanceClearProxyNAT(projectName, instanceName, deviceName 
string) error {
+       return nil
+}

From 0179c5a3f8c9d3594ca51e047ae2666cb4a207f5 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Wed, 12 Feb 2020 17:05:51 +0000
Subject: [PATCH 2/6] lxd/daemon: Checks firewall is loaded OK

Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
 lxd/daemon.go | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lxd/daemon.go b/lxd/daemon.go
index 1ac9175d18..44ee367cbb 100644
--- a/lxd/daemon.go
+++ b/lxd/daemon.go
@@ -783,7 +783,10 @@ func (d *Daemon) init() error {
                return errors.Wrap(err, "failed to open cluster database")
        }
 
-       d.firewall = firewall.New()
+       d.firewall, err = firewall.New()
+       if err != nil {
+               return errors.Wrap(err, "Failed to load firewall")
+       }
 
        err = cluster.NotifyUpgradeCompleted(d.State(), certInfo)
        if err != nil {

From 8471c0405e0cba1174b8555fb4212a2881d76dfd Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Wed, 12 Feb 2020 17:06:09 +0000
Subject: [PATCH 3/6] lxd/state/testing: firewall.New usage

Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
 lxd/state/testing.go | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lxd/state/testing.go b/lxd/state/testing.go
index b0bc97fed5..d6e69f6f51 100644
--- a/lxd/state/testing.go
+++ b/lxd/state/testing.go
@@ -26,7 +26,8 @@ func NewTestState(t *testing.T) (*State, func()) {
                osCleanup()
        }
 
-       state := NewState(node, cluster, nil, os, nil, nil, nil, 
firewall.New(), nil)
+       fw, _ := firewall.New()
+       state := NewState(node, cluster, nil, os, nil, nil, nil, fw, nil)
 
        return state, cleanup
 }

From 86531f9badf2b1f9c214690ebba041257ea3c48f Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Wed, 12 Feb 2020 17:06:24 +0000
Subject: [PATCH 4/6] lxd/network/network: Handle errors during firewall setup

Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
 lxd/network/network.go | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/lxd/network/network.go b/lxd/network/network.go
index 40612fdc19..3510bec05a 100644
--- a/lxd/network/network.go
+++ b/lxd/network/network.go
@@ -350,12 +350,18 @@ func (n *Network) setup(oldConfig map[string]string) 
error {
        if n.config["bridge.mode"] == "fan" || 
!shared.StringInSlice(n.config["ipv4.address"], []string{"", "none"}) {
                if n.HasDHCPv4() && n.HasIPv4Firewall() {
                        // Setup basic iptables overrides for DHCP/DNS
-                       n.state.Firewall.NetworkSetupDHCPDNSAccess(n.name, 4)
+                       err = 
n.state.Firewall.NetworkSetupDHCPDNSAccess(n.name, 4)
+                       if err != nil {
+                               return err
+                       }
                }
 
                // Attempt a workaround for broken DHCP clients
                if n.HasIPv4Firewall() {
-                       n.state.Firewall.NetworkSetupDHCPv4Checksum(n.name)
+                       err = 
n.state.Firewall.NetworkSetupDHCPv4Checksum(n.name)
+                       if err != nil {
+                               return err
+                       }
                }
 
                // Allow forwarding
@@ -532,7 +538,10 @@ func (n *Network) setup(oldConfig map[string]string) error 
{
                if n.HasDHCPv6() {
                        if n.config["ipv6.firewall"] == "" || 
shared.IsTrue(n.config["ipv6.firewall"]) {
                                // Setup basic iptables overrides for DHCP/DNS
-                               
n.state.Firewall.NetworkSetupDHCPDNSAccess(n.name, 6)
+                               err = 
n.state.Firewall.NetworkSetupDHCPDNSAccess(n.name, 6)
+                               if err != nil {
+                                       return err
+                               }
                        }
 
                        // Build DHCP configuration

From e7df8d9e009af708b560a288c0c4588dddfc2984 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Wed, 12 Feb 2020 17:06:41 +0000
Subject: [PATCH 5/6] lxd/firewall/firewall/load: Add nftable support

Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
 lxd/firewall/firewall_load.go | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lxd/firewall/firewall_load.go b/lxd/firewall/firewall_load.go
index 3eb3543466..f30d28e522 100644
--- a/lxd/firewall/firewall_load.go
+++ b/lxd/firewall/firewall_load.go
@@ -5,7 +5,8 @@ import (
 )
 
 // New returns an appropriate firewall implementation.
-func New() Firewall {
+func New() (Firewall, error) {
        // TODO: Issue #6223: add startup logic to choose xtables or nftables
-       return drivers.XTables{}
+       d := drivers.Nftables{}
+       return d, nil
 }

From dd6daef0b6cf20cea26a278b0bb4e7ea953bf3c0 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Wed, 12 Feb 2020 17:07:09 +0000
Subject: [PATCH 6/6] lxd/firewall/drivers/drivers/nftables: Adds nftables
 driver

Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
 lxd/firewall/drivers/drivers_nftables.go      | 134 +++++++++++++++++-
 .../drivers/drivers_nftables_templates.go     |  39 +++++
 2 files changed, 168 insertions(+), 5 deletions(-)
 create mode 100644 lxd/firewall/drivers/drivers_nftables_templates.go

diff --git a/lxd/firewall/drivers/drivers_nftables.go 
b/lxd/firewall/drivers/drivers_nftables.go
index d1435a8c76..d426666747 100644
--- a/lxd/firewall/drivers/drivers_nftables.go
+++ b/lxd/firewall/drivers/drivers_nftables.go
@@ -1,36 +1,160 @@
 package drivers
 
 import (
-       deviceConfig "github.com/lxc/lxd/lxd/device/config"
+       "fmt"
        "net"
+       "strings"
+
+       "github.com/pkg/errors"
+
+       deviceConfig "github.com/lxc/lxd/lxd/device/config"
+       "github.com/lxc/lxd/shared"
 )
 
+const nftablesNamespace = "lxd"
+const nftableContentTemplate = "nftablesContent"
+
 // Nftables is an implmentation of LXD firewall using nftables.
 type Nftables struct{}
 
+// getIPFamily converts IP version number into family name used by nftables.
+func (d Nftables) getIPFamily(ipVersion uint) (string, error) {
+       if ipVersion == 4 {
+               return "ip", nil
+       } else if ipVersion == 6 {
+               return "ip6", nil
+       }
+
+       return "", fmt.Errorf("Invalid IP version")
+}
+
 // NetworkSetupForwardingPolicy allows forwarding dependent on boolean argument
 func (d Nftables) NetworkSetupForwardingPolicy(networkName string, ipVersion 
uint, allow bool) error {
+       action := "reject"
+       if allow {
+               action = "accept"
+       }
+
+       family, err := d.getIPFamily(ipVersion)
+       if err != nil {
+               return err
+       }
+
+       config := &strings.Builder{}
+       tplFields := map[string]interface{}{
+               "namespace":   nftablesNamespace,
+               "networkName": networkName,
+               "family":      family,
+               "action":      action,
+       }
+
+       nftablesCommonTable.AddParseTree(nftableContentTemplate, 
nftablesNetFwdPolicy.Tree)
+       err = nftablesCommonTable.Execute(config, tplFields)
+       if err != nil {
+               return errors.Wrapf(err, "Failed loading Forwarding Policy 
template for network %q (%s)", networkName, family)
+       }
+
+       _, err = shared.RunCommand("nft", config.String())
+       if err != nil {
+               return errors.Wrapf(err, "Failed adding nftables Forwarding 
Policy rules for network %q (%s)", networkName, family)
+       }
+
        return nil
 }
 
 // NetworkSetupOutboundNAT configures outbound NAT.
 // If srcIP is non-nil then SNAT is used with the specified address, otherwise 
MASQUERADE mode is used.
-func (d Nftables) NetworkSetupOutboundNAT(networkName string, subnet 
*net.IPNet, srcIP net.IP, append bool) error {
+// Append mode is always on and so the append argument is ignored.
+func (d Nftables) NetworkSetupOutboundNAT(networkName string, subnet 
*net.IPNet, srcIP net.IP, _ bool) error {
+       family := "ip"
+       if subnet.IP.To4() == nil {
+               family = "ip6"
+       }
+
+       // If SNAT IP not supplied then use the IP of the outbound interface 
(MASQUERADE).
+       srcIPStr := ""
+       if srcIP != nil {
+               srcIPStr = srcIP.String()
+       }
+
+       config := &strings.Builder{}
+       tplFields := map[string]interface{}{
+               "namespace":   nftablesNamespace,
+               "networkName": networkName,
+               "family":      family,
+               "subnet":      subnet.String(),
+               "srcIP":       srcIPStr,
+       }
+
+       nftablesCommonTable.AddParseTree(nftableContentTemplate, 
nftablesNetOutboundNAT.Tree)
+       err := nftablesCommonTable.Execute(config, tplFields)
+       if err != nil {
+               return errors.Wrapf(err, "Failed loading Outbound NAT template 
for network %q (%s)", networkName, family)
+       }
+
+       _, err = shared.RunCommand("nft", config.String())
+       if err != nil {
+               return errors.Wrapf(err, "Failed adding nftables Outbound NAT 
rules for network %q (%s)", networkName, family)
+       }
+
        return nil
 }
 
-// NetworkSetupDHCPDNSAccess sets up basic iptables overrides for DHCP/DNS.
+// NetworkSetupDHCPDNSAccess sets up basic nftables overrides for DHCP/DNS.
 func (d Nftables) NetworkSetupDHCPDNSAccess(networkName string, ipVersion 
uint) error {
+       family, err := d.getIPFamily(ipVersion)
+       if err != nil {
+               return err
+       }
+
+       config := &strings.Builder{}
+       tplFields := map[string]interface{}{
+               "namespace":   nftablesNamespace,
+               "networkName": networkName,
+               "family":      family,
+       }
+
+       nftablesCommonTable.AddParseTree(nftableContentTemplate, 
nftablesNetDHCPDNS.Tree)
+       err = nftablesCommonTable.Execute(config, tplFields)
+       if err != nil {
+               return errors.Wrapf(err, "Failed loading DHCP/DNS Access 
template for network %q (%s)", networkName, family)
+       }
+
+       _, err = shared.RunCommand("nft", config.String())
+       if err != nil {
+               return errors.Wrapf(err, "Failed adding nftables DHCP/DNS 
Access rules for network %q (%s)", networkName, family)
+       }
+
        return nil
 }
 
-// NetworkSetupDHCPv4Checksum attempts a workaround for broken DHCP clients.
+// NetworkSetupDHCPv4Checksum attempts a workaround for broken DHCP clients. 
No-op as not supported by nftables.
+// See 
https://wiki.nftables.org/wiki-nftables/index.php/Supported_features_compared_to_xtables#CHECKSUM.
 func (d Nftables) NetworkSetupDHCPv4Checksum(networkName string) error {
        return nil
 }
 
-// NetworkClear removes network rules from filter, mangle and nat tables.
+// NetworkClear removes the LXD network related chains.
 func (d Nftables) NetworkClear(networkName string, ipVersion uint) error {
+       family, err := d.getIPFamily(ipVersion)
+       if err != nil {
+               return err
+       }
+
+       chains := []string{"fwd", "nat", "in"}
+       for _, chain := range chains {
+               chainName := fmt.Sprintf("%s_%s", chain, networkName)
+
+               // Check if chain exists by attempting to list its rules, and 
if that succeeds, delete it.
+               _, err := shared.RunCommand("nft", "list", "chain", family, 
nftablesNamespace, chainName)
+               if err == nil {
+                       _, err = shared.RunCommand("nft", "delete", "chain", 
family, nftablesNamespace, chainName)
+                       if err != nil {
+                               return errors.Wrapf(err, "Failed clearing 
nftables rules for network %q, chain %q (%d)", networkName, chainName, 
ipVersion)
+                       }
+               }
+       }
+
        return nil
 }
 
diff --git a/lxd/firewall/drivers/drivers_nftables_templates.go 
b/lxd/firewall/drivers/drivers_nftables_templates.go
new file mode 100644
index 0000000000..85157be9b6
--- /dev/null
+++ b/lxd/firewall/drivers/drivers_nftables_templates.go
@@ -0,0 +1,39 @@
+package drivers
+
+import (
+       "text/template"
+)
+
+var nftablesCommonTable = 
template.Must(template.New("nftablesCommonTable").Parse(`
+table {{.family}} {{.namespace}} {
+       {{- template "nftablesContent" . -}}
+}
+`))
+
+var nftablesNetFwdPolicy = 
template.Must(template.New("nftablesNetFwdPolicy").Parse(`
+chain fwd_{{.networkName}} {
+       type filter hook forward priority 0; policy accept;
+       oifname "{{.networkName}}" {{.action}}
+       iifname "{{.networkName}}" {{.action}}
+}
+`))
+
+var nftablesNetOutboundNAT = 
template.Must(template.New("nftablesNetOutboundNAT").Parse(`
+chain nat_{{.networkName}} {
+       type nat hook postrouting priority 100; policy accept;
+       {{- if .srcIP -}}
+       {{.family}} saddr {{.subnet}} {{.family}} daddr != {{.subnet}} snat 
{{.srcIP}}
+       {{else}}
+       {{.family}} saddr {{.subnet}} {{.family}} daddr != {{.subnet}} 
masquerade
+       {{- end }}
+}
+`))
+
+var nftablesNetDHCPDNS = 
template.Must(template.New("nftablesNetDHCPDNS").Parse(`
+chain in_{{.networkName}} {
+       type filter hook input priority 0; policy accept;
+       iifname "{{.networkName}}" tcp dport 53 accept
+       iifname "{{.networkName}}" udp dport 53 accept
+       iifname "{{.networkName}}" udp dport 67 accept
+}
+`))
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel

Reply via email to