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

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

[Ignat Korchagin]

Commit 67ee37360d41 ("netfilter: nf_tables: validate NFPROTO_* family") added
some validation of NFPROTO_* families in nftables, but it broke our use case for
xt_bpf module:

  * assuming we have a simple bpf program:

    #include <linux/bpf.h>
    #include <bpf/bpf_helpers.h>

    char _license[] SEC("license") = "GPL";

    SEC("socket")
    int prog(struct __sk_buff *skb) { return BPF_OK; }

  * we can compile it and pin into bpf FS:
    bpftool prog load bpf.o /sys/fs/bpf/test

  * now we want to create a following table

    table inet firewall {
        chain input {
                type filter hook prerouting priority filter; policy accept;
                bpf pinned "/sys/fs/bpf/test" drop
        }
    }

All above used to work, but now we get EOPNOTSUPP, when creating the table.

Fix this by allowing NFPROTO_INET for nft_(match/target)_validate()

Fixes: 67ee37360d41 ("netfilter: nf_tables: validate NFPROTO_* family")
Reported-by: Jordan Griege <jgriege@cloudflare.com>
Signed-off-by: Ignat Korchagin <ignat@cloudflare.com>
---
 net/netfilter/nft_compat.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/netfilter/nft_compat.c b/net/netfilter/nft_compat.c
index 1f9474fefe84..beea8c447e7a 100644
--- a/net/netfilter/nft_compat.c
+++ b/net/netfilter/nft_compat.c
@@ -359,6 +359,7 @@ 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;
@@ -610,6 +611,7 @@ 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;
-- 
2.39.2

Re: [PATCH] netfilter: nf_tables: allow NFPROTO_INET in nft_(match/target)_validate()

[Pablo Neira Ayuso]

Hi,

On Fri, Feb 09, 2024 at 12:19:54PM +0000, Ignat Korchagin wrote:
> Commit 67ee37360d41 ("netfilter: nf_tables: validate NFPROTO_* family") added
> some validation of NFPROTO_* families in nftables, but it broke our use case for
> xt_bpf module:
> 
>   * assuming we have a simple bpf program:
> 
>     #include <linux/bpf.h>
>     #include <bpf/bpf_helpers.h>
> 
>     char _license[] SEC("license") = "GPL";
> 
>     SEC("socket")
>     int prog(struct __sk_buff *skb) { return BPF_OK; }
> 
>   * we can compile it and pin into bpf FS:
>     bpftool prog load bpf.o /sys/fs/bpf/test
> 
>   * now we want to create a following table
> 
>     table inet firewall {
>         chain input {
>                 type filter hook prerouting priority filter; policy accept;
>                 bpf pinned "/sys/fs/bpf/test" drop

This feature does not exist in the tree.

>         }
>     }
> 
> All above used to work, but now we get EOPNOTSUPP, when creating the table.
> 
> Fix this by allowing NFPROTO_INET for nft_(match/target)_validate()

We don't support inet family for iptables.

Re: [PATCH] netfilter: nf_tables: allow NFPROTO_INET in nft_(match/target)_validate()

[Ignat Korchagin]

On Fri, Feb 9, 2024 at 12:38 PM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>
> Hi,

Thanks for the prompt reply

> On Fri, Feb 09, 2024 at 12:19:54PM +0000, Ignat Korchagin wrote:
> > Commit 67ee37360d41 ("netfilter: nf_tables: validate NFPROTO_* family") added
> > some validation of NFPROTO_* families in nftables, but it broke our use case for
> > xt_bpf module:
> >
> >   * assuming we have a simple bpf program:
> >
> >     #include <linux/bpf.h>
> >     #include <bpf/bpf_helpers.h>
> >
> >     char _license[] SEC("license") = "GPL";
> >
> >     SEC("socket")
> >     int prog(struct __sk_buff *skb) { return BPF_OK; }
> >
> >   * we can compile it and pin into bpf FS:
> >     bpftool prog load bpf.o /sys/fs/bpf/test
> >
> >   * now we want to create a following table
> >
> >     table inet firewall {
> >         chain input {
> >                 type filter hook prerouting priority filter; policy accept;
> >                 bpf pinned "/sys/fs/bpf/test" drop
>
> This feature does not exist in the tree.

Sorry - should have clarified this. We did indeed patch some userspace
tools to support easy creation of the above table (so it is presented
here for clarity) however we don't have any kernel-specific patches
with respect to this and it is technically possible to craft such a
table via raw netlink interface.

In fact - I just retested it on a freshly compiled 6.6.16 vanilla
kernel from a stable branch (with and without commit 67ee37360d41)
with a small program that does raw netlink messages.

> >         }
> >     }
> >
> > All above used to work, but now we get EOPNOTSUPP, when creating the table.
> >
> > Fix this by allowing NFPROTO_INET for nft_(match/target)_validate()
>
> We don't support inet family for iptables.

I've added Jordan Griege (Jordan, please comment) as he is likely more
competent here than me, but appears that we used it somehow.
For more context - we encountered this problem on 6.1 and 6.6 stable
kernels (when commit 67ee37360d41 was backported).

Ignat

Re: [PATCH] netfilter: nf_tables: allow NFPROTO_INET in nft_(match/target)_validate()

[Jordan Griege]

On Fri, Feb 9, 2024 at 9:03 AM Ignat Korchagin <ignat@cloudflare.com> wrote:
>
> On Fri, Feb 9, 2024 at 12:38 PM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> >
> > Hi,
>
> Thanks for the prompt reply
>
> > On Fri, Feb 09, 2024 at 12:19:54PM +0000, Ignat Korchagin wrote:
> > > Commit 67ee37360d41 ("netfilter: nf_tables: validate NFPROTO_* family") added
> > > some validation of NFPROTO_* families in nftables, but it broke our use case for
> > > xt_bpf module:
> > >
> > >   * assuming we have a simple bpf program:
> > >
> > >     #include <linux/bpf.h>
> > >     #include <bpf/bpf_helpers.h>
> > >
> > >     char _license[] SEC("license") = "GPL";
> > >
> > >     SEC("socket")
> > >     int prog(struct __sk_buff *skb) { return BPF_OK; }
> > >
> > >   * we can compile it and pin into bpf FS:
> > >     bpftool prog load bpf.o /sys/fs/bpf/test
> > >
> > >   * now we want to create a following table
> > >
> > >     table inet firewall {
> > >         chain input {
> > >                 type filter hook prerouting priority filter; policy accept;
> > >                 bpf pinned "/sys/fs/bpf/test" drop
> >
> > This feature does not exist in the tree.
>
> Sorry - should have clarified this. We did indeed patch some userspace
> tools to support easy creation of the above table (so it is presented
> here for clarity) however we don't have any kernel-specific patches
> with respect to this and it is technically possible to craft such a
> table via raw netlink interface.
>
> In fact - I just retested it on a freshly compiled 6.6.16 vanilla
> kernel from a stable branch (with and without commit 67ee37360d41)
> with a small program that does raw netlink messages.
>
> > >         }
> > >     }
> > >
> > > All above used to work, but now we get EOPNOTSUPP, when creating the table.
> > >
> > > Fix this by allowing NFPROTO_INET for nft_(match/target)_validate()
> >
> > We don't support inet family for iptables.
>
> I've added Jordan Griege (Jordan, please comment) as he is likely more
> competent here than me, but appears that we used it somehow.
> For more context - we encountered this problem on 6.1 and 6.6 stable
> kernels (when commit 67ee37360d41 was backported).

I'm only so familiar with the kernel side of nftables, but I believe we are
indeed using the nftables API.  The netlink messages we send to create an xt_bpf
rule in an inet table use the NFNL_SUBSYS_NFTABLES type in the netlink message
header.  It's my understanding that the nftables API is capable of creating
xtables matches, thought it doesn't seem to be a goal of the nft program to
support all of them.

As Ignat said, we aren't asking for the nft program to support this case, but
it seems like a breaking change to other users of the nftables API.  Am I
missing some detail of the way that the iptables and nftables systems are
separated in the kernel?

Thanks,
Jordan

Re: [PATCH] netfilter: nf_tables: allow NFPROTO_INET in nft_(match/target)_validate()

[Pablo Neira Ayuso]

Hi Ignat,

On Fri, Feb 09, 2024 at 12:19:54PM +0000, Ignat Korchagin wrote:
> diff --git a/net/netfilter/nft_compat.c b/net/netfilter/nft_compat.c
> index 1f9474fefe84..beea8c447e7a 100644
> --- a/net/netfilter/nft_compat.c
> +++ b/net/netfilter/nft_compat.c
> @@ -359,6 +359,7 @@ static int nft_target_validate(const struct nft_ctx *ctx,
>  
>  	if (ctx->family != NFPROTO_IPV4 &&
>  	    ctx->family != NFPROTO_IPV6 &&
> +	    ctx->family != NFPROTO_INET &&

Please send a v2 restricting this to hooks prerouting, input, forward,
output and postrouting which are the classic hooks, so ingress is not
allowed, both for matches and targets.

Thanks