[lxc-devel] [lxd/master] Firewall: Filter unrecognised Ethernet frame types when IP filtering is enabled

[tomponline on Github]

A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 361 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200623/133a4384/attachment.bin>
-------------- next part --------------
From 696c84f5c9c0c51451d7b0b2a93eaf39f70da74f Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 23 Jun 2020 08:59:53 +0100
Subject: [PATCH] lxd/firewall: Filter unrecognised ethernet frame types when
 IP filtering is enabled

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/firewall/drivers/drivers_nftables.go      |  4 ++--
 .../drivers/drivers_nftables_templates.go     | 14 +++++++-------
 lxd/firewall/drivers/drivers_xtables.go       | 19 +++++++++++++------
 3 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/lxd/firewall/drivers/drivers_nftables.go b/lxd/firewall/drivers/drivers_nftables.go
index 3a58db8f08..b1c1d07b67 100644
--- a/lxd/firewall/drivers/drivers_nftables.go
+++ b/lxd/firewall/drivers/drivers_nftables.go
@@ -299,9 +299,9 @@ func (d Nftables) InstanceSetupBridgeFilter(projectName string, instanceName str
 		"hwAddrHex":      fmt.Sprintf("0x%s", hex.EncodeToString(mac)),
 	}
 
-	// Filter VLAN tagged frames when using IP filtering.
+	// Filter unrecognised ethernet frames when using IP filtering.
 	if IPv4 != nil || IPv6 != nil {
-		tplFields["vlanFilter"] = true
+		tplFields["filterUnrecognised"] = true
 	}
 
 	if IPv4 != nil {
diff --git a/lxd/firewall/drivers/drivers_nftables_templates.go b/lxd/firewall/drivers/drivers_nftables_templates.go
index 5c9af7d209..bd10ed4bf8 100644
--- a/lxd/firewall/drivers/drivers_nftables_templates.go
+++ b/lxd/firewall/drivers/drivers_nftables_templates.go
@@ -83,16 +83,13 @@ chain pstrt{{.chainSeparator}}{{.deviceLabel}} {
 // Nftables doesn't support the equivalent of "arp saddr" and "arp saddr ether" at this time so in order to filter
 // NDP advertisements that come from the genuine Ethernet MAC address but have a spoofed NDP source MAC/IP adddress
 // we need to use manual header offset extraction. This also drops IPv6 router advertisements from instance.
-// If IP filtering is enabled, this also drops tagged VLAN (802.1Q) frames.
+// If IP filtering is enabled, this also drops unrecognised ethernet frames.
 var nftablesInstanceBridgeFilter = template.Must(template.New("nftablesInstanceBridgeFilter").Parse(`
 chain in{{.chainSeparator}}{{.deviceLabel}} {
 	type filter hook input priority -200; policy accept;
 	iifname "{{.hostName}}" ether saddr != {{.hwAddr}} drop
 	iifname "{{.hostName}}" ether type arp arp saddr ether != {{.hwAddr}} drop
 	iifname "{{.hostName}}" ether type ip6 icmpv6 type 136 @nh,528,48 != {{.hwAddrHex}} drop
-	{{if .vlanFilter -}}
-	iifname "{{.hostName}}" ether type vlan drop
-	{{- end}}
 	{{if .ipv4FilterAll -}}
 	iifname "{{.hostName}}" ether type arp drop
 	iifname "{{.hostName}}" ether type ip drop
@@ -112,6 +109,9 @@ chain in{{.chainSeparator}}{{.deviceLabel}} {
 	iifname "{{.hostName}}" ether type ip6 ip6 saddr != {{.ipv6Addr}} drop
 	iifname "{{.hostName}}" ether type ip6 icmpv6 type 134 drop
 	{{- end}}
+	{{if .filterUnrecognised -}}
+	iifname "{{.hostName}}" ether type != {arp, ip, ip6} drop
+	{{- end}}
 }
 
 chain fwd{{.chainSeparator}}{{.deviceLabel}} {
@@ -119,9 +119,6 @@ chain fwd{{.chainSeparator}}{{.deviceLabel}} {
 	iifname "{{.hostName}}" ether saddr != {{.hwAddr}} drop
 	iifname "{{.hostName}}" ether type arp arp saddr ether != {{.hwAddr}} drop
 	iifname "{{.hostName}}" ether type ip6 icmpv6 type 136 @nh,528,48 != {{.hwAddrHex}} drop
-	{{if .vlanFilter -}}
-	iifname "{{.hostName}}" ether type vlan drop
-	{{- end}}
 	{{if .ipv4FilterAll -}}
 	iifname "{{.hostName}}" ether type arp drop
 	iifname "{{.hostName}}" ether type ip drop
@@ -138,6 +135,9 @@ chain fwd{{.chainSeparator}}{{.deviceLabel}} {
 	iifname "{{.hostName}}" ether type ip6 icmpv6 type 136 @nh,384,128 != {{.ipv6AddrHex}} drop
 	iifname "{{.hostName}}" ether type ip6 icmpv6 type 134 drop
 	{{- end}}
+	{{if .filterUnrecognised -}}
+	iifname "{{.hostName}}" ether type != {arp, ip, ip6} drop
+	{{- end}}
 }
 `))
 
diff --git a/lxd/firewall/drivers/drivers_xtables.go b/lxd/firewall/drivers/drivers_xtables.go
index 6134b37ba7..aeb193616d 100644
--- a/lxd/firewall/drivers/drivers_xtables.go
+++ b/lxd/firewall/drivers/drivers_xtables.go
@@ -449,12 +449,6 @@ func (d Xtables) generateFilterEbtablesRules(hostName string, hwAddr string, IPv
 		{"ebtables", "-t", "filter", "-A", "FORWARD", "-s", "!", hwAddr, "-i", hostName, "-j", "DROP"},
 	}
 
-	if IPv4 != nil || IPv6 != nil {
-		// Filter VLAN tagged frames when using IP filtering.
-		rules = append(rules, []string{"ebtables", "-t", "filter", "-A", "INPUT", "-p", "802_1Q", "-i", hostName, "-j", "DROP"})
-		rules = append(rules, []string{"ebtables", "-t", "filter", "-A", "FORWARD", "-p", "802_1Q", "-i", hostName, "-j", "DROP"})
-	}
-
 	if IPv4 != nil {
 		if IPv4.String() == FilterIPv4All {
 			rules = append(rules,
@@ -501,6 +495,19 @@ func (d Xtables) generateFilterEbtablesRules(hostName string, hwAddr string, IPv
 		}
 	}
 
+	if IPv4 != nil || IPv6 != nil {
+		// Filter unrecognised ethernet frames when using IP filtering.
+		rules = append(rules, []string{"ebtables", "-t", "filter", "-A", "INPUT", "-p", "ARP", "-i", hostName, "-j", "ACCEPT"})
+		rules = append(rules, []string{"ebtables", "-t", "filter", "-A", "INPUT", "-p", "IPv4", "-i", hostName, "-j", "ACCEPT"})
+		rules = append(rules, []string{"ebtables", "-t", "filter", "-A", "INPUT", "-p", "IPv6", "-i", hostName, "-j", "ACCEPT"})
+		rules = append(rules, []string{"ebtables", "-t", "filter", "-A", "INPUT", "-i", hostName, "-j", "DROP"})
+
+		rules = append(rules, []string{"ebtables", "-t", "filter", "-A", "FORWARD", "-p", "ARP", "-i", hostName, "-j", "ACCEPT"})
+		rules = append(rules, []string{"ebtables", "-t", "filter", "-A", "FORWARD", "-p", "IPv4", "-i", hostName, "-j", "ACCEPT"})
+		rules = append(rules, []string{"ebtables", "-t", "filter", "-A", "FORWARD", "-p", "IPv6", "-i", hostName, "-j", "ACCEPT"})
+		rules = append(rules, []string{"ebtables", "-t", "filter", "-A", "FORWARD", "-i", hostName, "-j", "DROP"})
+	}
+
 	return rules
 }