netdev - Re: [PATCH] netfilter: nft_paylaod: add base type NFT_PAYLOAD_LL_HEADER_NO

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 10 Jun 2019 11:44:33 +0200
From:   Florian Westphal <fw@...len.de>
To:     wenxu@...oud.cn
Cc:     pablo@...filter.org, netfilter-devel@...r.kernel.org,
        netdev@...r.kernel.org
Subject: Re: [PATCH] netfilter: nft_paylaod: add base type
 NFT_PAYLOAD_LL_HEADER_NO_TAG

wenxu@...oud.cn <wenxu@...oud.cn> wrote:
> From: wenxu <wenxu@...oud.cn>
> 
> nft add rule bridge firewall rule-100-ingress ip protocol icmp drop

nft --debug=netlink add rule bridge firewall rule-100-ingress ip protocol icmp drop
bridge firewall rule-100-ingress
  [ payload load 2b @ link header + 12 => reg 1 ]
  [ cmp eq reg 1 0x00000008 ]
  [ payload load 1b @ network header + 9 => reg 1 ]
  [ cmp eq reg 1 0x00000001 ]
  [ immediate reg 0 drop ]

so problem is that nft inserts a dependency on the ethernet protocol
type (0x800).

But when vlan is involved, that will fail to compare.

It would also fail for qinq etc.

Because of vlan tag offload, the rule about will probably already work
just fine when nft userspace is patched to insert the dependency based
on 'meta protocol'.  Can you see if this patch works?

Subject: Change bridge l3 dependency to meta protocol

This examines skb->protocol instead of ethernet header type, which
might be different when vlan is involved.

nft payload expression will re-insert the vlan tag so ether type
will not be ETH_P_IP.

---
 src/meta.c    |  6 +++++-
 src/payload.c | 20 ++++++++++++++++++++
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/src/meta.c b/src/meta.c
index 583e790ff47d..1e8964eb48c4 100644
--- a/src/meta.c
+++ b/src/meta.c
@@ -539,7 +539,11 @@ static void meta_expr_pctx_update(struct proto_ctx *ctx,
 		proto_ctx_update(ctx, PROTO_BASE_TRANSPORT_HDR, &expr->location, desc);
 		break;
 	case NFT_META_PROTOCOL:
-		if (h->base < PROTO_BASE_NETWORK_HDR && ctx->family != NFPROTO_NETDEV)
+		if (h->base != PROTO_BASE_LL_HDR)
+			return;
+
+		if (ctx->family != NFPROTO_NETDEV &&
+		    ctx->family != NFPROTO_BRIDGE)
 			return;
 
 		desc = proto_find_upper(h->desc, ntohs(mpz_get_uint16(right->value)));
diff --git a/src/payload.c b/src/payload.c
index 6a8118ece890..c99bb2f69977 100644
--- a/src/payload.c
+++ b/src/payload.c
@@ -18,6 +18,7 @@
 #include <net/if_arp.h>
 #include <arpa/inet.h>
 #include <linux/netfilter.h>
+#include <linux/if_ether.h>
 
 #include <rule.h>
 #include <expression.h>
@@ -307,6 +308,19 @@ payload_gen_special_dependency(struct eval_ctx *ctx, const struct expr *expr)
 	return NULL;
 }
 
+static const struct proto_desc proto_metaeth = {
+	.name		= "ethmeta",
+	.base		= PROTO_BASE_LL_HDR,
+	.protocols	= {
+		PROTO_LINK(__constant_htons(ETH_P_IP),	 &proto_ip),
+		PROTO_LINK(__constant_htons(ETH_P_ARP),	 &proto_arp),
+		PROTO_LINK(__constant_htons(ETH_P_IPV6), &proto_ip6),
+	},
+	.templates	= {
+		[0]	= PROTO_META_TEMPLATE("protocol", &ethertype_type, NFT_META_PROTOCOL, 16),
+	},
+};
+
 /**
  * payload_gen_dependency - generate match expression on payload dependency
  *
@@ -369,6 +383,12 @@ int payload_gen_dependency(struct eval_ctx *ctx, const struct expr *expr,
 				  "no %s protocol specified",
 				  proto_base_names[expr->payload.base - 1]);
 
+	if (ctx->pctx.family == NFPROTO_BRIDGE && desc == &proto_eth) {
+		if (expr->payload.desc == &proto_ip ||
+		    expr->payload.desc == &proto_ip6)
+			desc = &proto_metaeth;
+	}
+
 	return payload_add_dependency(ctx, desc, expr->payload.desc, expr, res);
 }
 
-- 
2.21.0