[PATCH nf-next] netfilter: nft_compat: restrict raw table targets/matches

[PATCH nf-next] netfilter: nft_compat: restrict raw table targets/matches

[Florian Westphal]

xt targets and matches declaring .table = "raw" (CT, NOTRACK) rely on
only ever running in the legacy raw table's prerouting/output chains,
ahead of conntrack. nft_compat_chain_validate_dependency() already
enforces this kind of correspondence for "nat" by requiring the chain
type to actually be NAT, but did nothing for "raw": a rule wrapping
the CT target could be loaded into any hook and any priority via
nft_compat, including after the real conntrack hook has already run.

The CT target attaches a conntrack template to the skb when none is
set yet.

Require that a "raw" table dependency actually matches what xtables
enforces via the implict/builtin table dependency.

Fixes: 0ca743a55991 ("netfilter: nf_tables: add compatibility layer for x_tables")
Assisted-by: LLM
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/netfilter/nft_compat.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/net/netfilter/nft_compat.c b/net/netfilter/nft_compat.c
index 63864b928259..88ddaf47f587 100644
--- a/net/netfilter/nft_compat.c
+++ b/net/netfilter/nft_compat.c
@@ -14,6 +14,7 @@
 #include <linux/netfilter/nf_tables.h>
 #include <linux/netfilter/nf_tables_compat.h>
 #include <linux/netfilter/x_tables.h>
+#include <linux/netfilter_ipv4.h>
 #include <linux/netfilter_ipv4/ip_tables.h>
 #include <linux/netfilter_ipv6/ip6_tables.h>
 #include <linux/netfilter_bridge/ebtables.h>
@@ -45,6 +46,20 @@ static int nft_compat_chain_validate_dependency(const struct nft_ctx *ctx,
 			type = NFT_CHAIN_T_NAT;
 		if (basechain->type->type != type)
 			return -EINVAL;
+	} else if (strcmp(tablename, "raw") == 0) {
+		/* iptables-nft emulates the legacy raw table, which only
+		 * ever has prerouting/output chains at this exact priority.
+		 */
+		if (ctx->family != NFPROTO_IPV4 &&
+		    ctx->family != NFPROTO_IPV6)
+			return -EINVAL;
+
+		if (basechain->ops.hooknum != NF_INET_PRE_ROUTING &&
+		    basechain->ops.hooknum != NF_INET_LOCAL_OUT)
+			return -EINVAL;
+
+		if (basechain->ops.priority != NF_IP_PRI_RAW)
+			return -EINVAL;
 	}
 
 	return 0;
-- 
2.55.0

Re: [PATCH nf-next] netfilter: nft_compat: restrict raw table targets/matches

[Florian Westphal]

Florian Westphal <fw@strlen.de> wrote:
> xt targets and matches declaring .table = "raw" (CT, NOTRACK) rely on
> only ever running in the legacy raw table's prerouting/output chains,
> ahead of conntrack. nft_compat_chain_validate_dependency() already
> enforces this kind of correspondence for "nat" by requiring the chain
> type to actually be NAT, but did nothing for "raw": a rule wrapping
> the CT target could be loaded into any hook and any priority via
> nft_compat, including after the real conntrack hook has already run.
> 
> The CT target attaches a conntrack template to the skb when none is
> set yet.
>
> Require that a "raw" table dependency actually matches what xtables
> enforces via the implict/builtin table dependency.

https://sashiko.dev/#/patchset/20261006231549.5901-1-fw%40strlen.de

| Does this strict equality check reject legitimate pre-defragmentation
| priorities?

I don't think so.  While its technically allowed, iptables-nft doesn't
support NF_IP_PRI_RAW_BEFORE_DEFRAG / NF_IP6_PRI_RAW_BEFORE_DEFRAG.

| If users recreate this state in nftables by creating a base chain at priority
| -450 and attach the xt_CT target via nft_compat, it seems this check would
| unintentionally reject NF_IP_PRI_RAW_BEFORE_DEFRAG and return -EINVAL.

I don't think that makes sense.  Native nft users would not use
nft_compat.

But if you prefer, I can relax this check to use ">" instead.