Skip to contentSkip to conversation

netfilter: nf_tables: allow NFPROTO_INET in nft_(match/target)_validate()

PATCH In review Ignat Korchagin 2 participants 4 messages Feb 22, 2024

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH v3 1/1 netfilter: nf_tables: allow NFPROTO_INET in nft_(match/target)_validate()

Commit d0009effa886 ("netfilter: nf_tables: validate NFPROTO_* family") added some validation of NFPROTO_* families in the nft_compat module, but it broke the ability to use legacy iptables modules in dual-stack nftables.

While with legacy iptables one had to independently manage IPv4 and IPv6 tables, with nftables it is possible to have dual-stack tables sharing the rules. Moreover, it was possible to use rules based on legacy iptables match/target modules in dual-stack nftables.

As an example, the program from [2] creates an INET dual-stack family table using an xt_bpf based rule, which looks like the following (the actual output was generated with a patched nft tool as the current nft tool does not parse dual stack tables with legacy match rules, so consider it for illustrative purposes only):

table inet testfw {
chain input {
type filter hook prerouting priority filter; policy accept; bytecode counter packets 0 bytes 0 accept
}
}

After d0009effa886 ("netfilter: nf_tables: validate NFPROTO_* family") we get EOPNOTSUPP for the above program.

Fix this by allowing NFPROTO_INET for nft_(match/target)_validate(), but also restrict the functions to classic iptables hooks.

Changes in v3:
* clarify that upstream nft will not display such configuration properly and that the output was generated with a patched nft tool
* remove example program from commit description and link to it instead
* no code changes otherwise

Changes in v2:
* restrict nft_(match/target)_validate() to classic iptables hooks
* rewrite example program to use unmodified libnftnl

---
 net/netfilter/nft_compat.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
net/netfilter/nft_compat.c +20 -0
index 1f9474fefe84..d3d11dede545 100644
@@ -359,10 +359,20 @@ static int nft_target_validate(const struct nft_ctx *ctx,
 
 	if (ctx->family != NFPROTO_IPV4 &&
 	    ctx->family != NFPROTO_IPV6 &&
+	    ctx->family != NFPROTO_INET &&
 	    ctx->family != NFPROTO_BRIDGE &&
 	    ctx->family != NFPROTO_ARP)
 		return -EOPNOTSUPP;
 
+	ret = nft_chain_validate_hooks(ctx->chain,
+				       (1 << NF_INET_PRE_ROUTING) |
+				       (1 << NF_INET_LOCAL_IN) |
+				       (1 << NF_INET_FORWARD) |
+				       (1 << NF_INET_LOCAL_OUT) |
+				       (1 << NF_INET_POST_ROUTING));
+	if (ret)
+		return ret;
+
 	if (nft_is_base_chain(ctx->chain)) {
 		const struct nft_base_chain *basechain =
 						nft_base_chain(ctx->chain);
@@ -610,10 +620,20 @@ static int nft_match_validate(const struct nft_ctx *ctx,
 
 	if (ctx->family != NFPROTO_IPV4 &&
 	    ctx->family != NFPROTO_IPV6 &&
+	    ctx->family != NFPROTO_INET &&
 	    ctx->family != NFPROTO_BRIDGE &&
 	    ctx->family != NFPROTO_ARP)
 		return -EOPNOTSUPP;
 
+	ret = nft_chain_validate_hooks(ctx->chain,
+				       (1 << NF_INET_PRE_ROUTING) |
+				       (1 << NF_INET_LOCAL_IN) |
+				       (1 << NF_INET_FORWARD) |
+				       (1 << NF_INET_LOCAL_OUT) |
+				       (1 << NF_INET_POST_ROUTING));
+	if (ret)
+		return ret;
+
 	if (nft_is_base_chain(ctx->chain)) {
 		const struct nft_base_chain *basechain =
 						nft_base_chain(ctx->chain);
Signature · 1 line
2.39.2
#1 lore raw

Hi Ignat,

Quoted 15 lines · Ignat Korchagin

Commit d0009effa886 ("netfilter: nf_tables: validate NFPROTO_* family") added some validation of NFPROTO_* families in the nft_compat module, but it broke the ability to use legacy iptables modules in dual-stack nftables.

While with legacy iptables one had to independently manage IPv4 and IPv6 tables, with nftables it is possible to have dual-stack tables sharing the rules. Moreover, it was possible to use rules based on legacy iptables match/target modules in dual-stack nftables.

As an example, the program from [2] creates an INET dual-stack family table using an xt_bpf based rule, which looks like the following (the actual output was generated with a patched nft tool as the current nft tool does not parse dual stack tables with legacy match rules, so consider it for illustrative purposes only):

Quoted 6 lines

table inet testfw {
chain input {
type filter hook prerouting priority filter; policy accept; bytecode counter packets 0 bytes 0 accept
}
}

This nft command does not exist in tree, this does not restores fine with nft -f. It provides a misleading hint to the reader.

I am fine with restoring this because you use it, but you have to find a better interface than using nft_compat to achieve this IMO.

The upstream consensus this far is not to expose nft_compat features through userspace nft. But as said, I understand and I am fine with restoring kernel behaviour so you can keep going with your out-of-tree patch.

Thanks !

Quoted 74 lines

After d0009effa886 ("netfilter: nf_tables: validate NFPROTO_* family") we get EOPNOTSUPP for the above program.

Fix this by allowing NFPROTO_INET for nft_(match/target)_validate(), but also restrict the functions to classic iptables hooks.

Changes in v3:
* clarify that upstream nft will not display such configuration properly and that the output was generated with a patched nft tool
* remove example program from commit description and link to it instead
* no code changes otherwise

Changes in v2:
* restrict nft_(match/target)_validate() to classic iptables hooks
* rewrite example program to use unmodified libnftnl

---
 net/netfilter/nft_compat.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
net/netfilter/nft_compat.c +20 -0
index 1f9474fefe84..d3d11dede545 100644
@@ -359,10 +359,20 @@ static int nft_target_validate(const struct nft_ctx *ctx,
 
 	if (ctx->family != NFPROTO_IPV4 &&
 	    ctx->family != NFPROTO_IPV6 &&
+	    ctx->family != NFPROTO_INET &&
 	    ctx->family != NFPROTO_BRIDGE &&
 	    ctx->family != NFPROTO_ARP)
 		return -EOPNOTSUPP;
 
+	ret = nft_chain_validate_hooks(ctx->chain,
+				       (1 << NF_INET_PRE_ROUTING) |
+				       (1 << NF_INET_LOCAL_IN) |
+				       (1 << NF_INET_FORWARD) |
+				       (1 << NF_INET_LOCAL_OUT) |
+				       (1 << NF_INET_POST_ROUTING));
+	if (ret)
+		return ret;
+
 	if (nft_is_base_chain(ctx->chain)) {
 		const struct nft_base_chain *basechain =
 						nft_base_chain(ctx->chain);
@@ -610,10 +620,20 @@ static int nft_match_validate(const struct nft_ctx *ctx,
 
 	if (ctx->family != NFPROTO_IPV4 &&
 	    ctx->family != NFPROTO_IPV6 &&
+	    ctx->family != NFPROTO_INET &&
 	    ctx->family != NFPROTO_BRIDGE &&
 	    ctx->family != NFPROTO_ARP)
 		return -EOPNOTSUPP;
 
+	ret = nft_chain_validate_hooks(ctx->chain,
+				       (1 << NF_INET_PRE_ROUTING) |
+				       (1 << NF_INET_LOCAL_IN) |
+				       (1 << NF_INET_FORWARD) |
+				       (1 << NF_INET_LOCAL_OUT) |
+				       (1 << NF_INET_POST_ROUTING));
+	if (ret)
+		return ret;
+
 	if (nft_is_base_chain(ctx->chain)) {
 		const struct nft_base_chain *basechain =
 						nft_base_chain(ctx->chain);
Signature · 1 line
2.39.2
#2 lore raw

Hi Pablo,

Quoted 26 lines · Pablo Neira Ayuso

Hi Ignat,

Quoted 15 lines · Ignat Korchagin

Commit d0009effa886 ("netfilter: nf_tables: validate NFPROTO_* family") added some validation of NFPROTO_* families in the nft_compat module, but it broke the ability to use legacy iptables modules in dual-stack nftables.

While with legacy iptables one had to independently manage IPv4 and IPv6 tables, with nftables it is possible to have dual-stack tables sharing the rules. Moreover, it was possible to use rules based on legacy iptables match/target modules in dual-stack nftables.

As an example, the program from [2] creates an INET dual-stack family table using an xt_bpf based rule, which looks like the following (the actual output was generated with a patched nft tool as the current nft tool does not parse dual stack tables with legacy match rules, so consider it for illustrative purposes only):

Quoted 6 lines

table inet testfw {
chain input {
type filter hook prerouting priority filter; policy accept; bytecode counter packets 0 bytes 0 accept
}
}

Quoted 2 lines

This nft command does not exist in tree, this does not restores fine with nft -f. It provides a misleading hint to the reader.

I tried to clarify above that this is for illustrative purposes only - just to give context about what we are trying to do, but do let me know if you prefer a v4 with this completely removed.

Quoted 2 lines

I am fine with restoring this because you use it, but you have to find a better interface than using nft_compat to achieve this IMO.

We're actually looking to restore the effort in [1] so some support would be appreciated.

Quoted 4 lines

The upstream consensus this far is not to expose nft_compat features through userspace nft. But as said, I understand and I am fine with restoring kernel behaviour so you can keep going with your out-of-tree patch.

Understood. There is no expectation from us that upstream userspace nft should natively support this (as it didn't before d0009effa886), but we can send the patch if consensus changes.

Quoted 61 lines

Thanks !

Quoted 59 lines

After d0009effa886 ("netfilter: nf_tables: validate NFPROTO_* family") we get EOPNOTSUPP for the above program.

Fix this by allowing NFPROTO_INET for nft_(match/target)_validate(), but also restrict the functions to classic iptables hooks.

Changes in v3:
* clarify that upstream nft will not display such configuration properly and that the output was generated with a patched nft tool
* remove example program from commit description and link to it instead
* no code changes otherwise

Changes in v2:
* restrict nft_(match/target)_validate() to classic iptables hooks
* rewrite example program to use unmodified libnftnl

---
 net/netfilter/nft_compat.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
net/netfilter/nft_compat.c +11 -0
index 1f9474fefe84..d3d11dede545 100644
@@ -359,10 +359,20 @@ static int nft_target_validate(const struct nft_ctx *ctx,
 
      if (ctx->family != NFPROTO_IPV4 &&
          ctx->family != NFPROTO_IPV6 &&
+         ctx->family != NFPROTO_INET &&
          ctx->family != NFPROTO_BRIDGE &&
          ctx->family != NFPROTO_ARP)
              return -EOPNOTSUPP;
 
+     ret = nft_chain_validate_hooks(ctx->chain,
+                                    (1 << NF_INET_PRE_ROUTING) |
+                                    (1 << NF_INET_LOCAL_IN) |
+                                    (1 << NF_INET_FORWARD) |
+                                    (1 << NF_INET_LOCAL_OUT) |
+                                    (1 << NF_INET_POST_ROUTING));
+     if (ret)
+             return ret;
+
      if (nft_is_base_chain(ctx->chain)) {
              const struct nft_base_chain *basechain =
                                              nft_base_chain(ctx->chain);
@@ -610,10 +620,20 @@ static int nft_match_validate(const struct nft_ctx *ctx,
 
      if (ctx->family != NFPROTO_IPV4 &&
          ctx->family != NFPROTO_IPV6 &&
+         ctx->family != NFPROTO_INET &&
          ctx->family != NFPROTO_BRIDGE &&
          ctx->family != NFPROTO_ARP)
              return -EOPNOTSUPP;
 
Quoted 15 lines
Quoted 13 lines
net/netfilter/nft_compat.c +9 -1
+     ret = nft_chain_validate_hooks(ctx->chain,
+                                    (1 << NF_INET_PRE_ROUTING) |
+                                    (1 << NF_INET_LOCAL_IN) |
+                                    (1 << NF_INET_FORWARD) |
+                                    (1 << NF_INET_LOCAL_OUT) |
+                                    (1 << NF_INET_POST_ROUTING));
+     if (ret)
+             return ret;
+
      if (nft_is_base_chain(ctx->chain)) {
              const struct nft_base_chain *basechain =
                                              nft_base_chain(ctx->chain);
--
Quoted 2 lines

2.39.2

[1]: https://lore.kernel.org/netfilter-devel/[email protected]/

Ignat

#3 lore raw
Quoted 29 lines · Ignat Korchagin

Hi Pablo,

Quoted 26 lines · Pablo Neira Ayuso

Hi Ignat,

Quoted 21 lines · Ignat Korchagin

Commit d0009effa886 ("netfilter: nf_tables: validate NFPROTO_* family") added some validation of NFPROTO_* families in the nft_compat module, but it broke the ability to use legacy iptables modules in dual-stack nftables.

While with legacy iptables one had to independently manage IPv4 and IPv6 tables, with nftables it is possible to have dual-stack tables sharing the rules. Moreover, it was possible to use rules based on legacy iptables match/target modules in dual-stack nftables.

As an example, the program from [2] creates an INET dual-stack family table using an xt_bpf based rule, which looks like the following (the actual output was generated with a patched nft tool as the current nft tool does not parse dual stack tables with legacy match rules, so consider it for illustrative purposes only):

table inet testfw {
chain input {
type filter hook prerouting priority filter; policy accept; bytecode counter packets 0 bytes 0 accept
}
}

Quoted 6 lines
Quoted 2 lines

This nft command does not exist in tree, this does not restores fine with nft -f. It provides a misleading hint to the reader.

I tried to clarify above that this is for illustrative purposes only - just to give context about what we are trying to do, but do let me know if you prefer a v4 with this completely removed.

Thanks for clarifying.

Quoted 6 lines
Quoted 2 lines

I am fine with restoring this because you use it, but you have to find a better interface than using nft_compat to achieve this IMO.

We're actually looking to restore the effort in [1] so some support would be appreciated.

Quoted 8 lines
Quoted 4 lines

The upstream consensus this far is not to expose nft_compat features through userspace nft. But as said, I understand and I am fine with restoring kernel behaviour so you can keep going with your out-of-tree patch.

Understood. There is no expectation from us that upstream userspace nft should natively support this (as it didn't before d0009effa886), but we can send the patch if consensus changes.

Thanks for explaining.