diff mbox series

[nf-next,3/3] nftables: reject nat hook registration if prio is before conntrack

Message ID 20171208160155.968-4-fw@strlen.de
State Accepted
Delegated to: Pablo Neira
Headers show
Series netfilter: disable parallel use of xtables and nftables nat | expand

Commit Message

Florian Westphal Dec. 8, 2017, 4:01 p.m. UTC
No problem for iptables as priorities are fixed values defined in the
nat modules, but in nftables the priority its coming from userspace.

Reject in case we see that such a hook would not work.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/netfilter/nf_tables_api.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Pablo Neira Ayuso Dec. 8, 2017, 9:22 p.m. UTC | #1
On Fri, Dec 08, 2017 at 05:01:55PM +0100, Florian Westphal wrote:
> No problem for iptables as priorities are fixed values defined in the
> nat modules, but in nftables the priority its coming from userspace.
> 
> Reject in case we see that such a hook would not work.
> 
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>  net/netfilter/nf_tables_api.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index f000d4399c7a..4ed66f1b40b5 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -1303,6 +1303,11 @@ static int nft_chain_parse_hook(struct net *net,
>  	}
>  	if (!(type->hook_mask & (1 << hook->num)))
>  		return -EOPNOTSUPP;
> +
> +	if (type->type == NFT_CHAIN_T_NAT &&
> +	    hook->priority <= NF_IP_PRI_CONNTRACK)
> +		return -EINVAL;

EINVAL is usually for missing netlink attributes, so I'd go for
EOPNOTSUPP instead. No need to resend I can mangle this here if you
prefer.

> +
>  	if (!try_module_get(type->owner))
>  		return -ENOENT;
>  
> -- 
> 2.13.6
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pablo Neira Ayuso Dec. 13, 2017, 4:28 p.m. UTC | #2
On Fri, Dec 08, 2017 at 10:22:57PM +0100, Pablo Neira Ayuso wrote:
> On Fri, Dec 08, 2017 at 05:01:55PM +0100, Florian Westphal wrote:
> > No problem for iptables as priorities are fixed values defined in the
> > nat modules, but in nftables the priority its coming from userspace.
> > 
> > Reject in case we see that such a hook would not work.
> > 
> > Signed-off-by: Florian Westphal <fw@strlen.de>
> > ---
> >  net/netfilter/nf_tables_api.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> > index f000d4399c7a..4ed66f1b40b5 100644
> > --- a/net/netfilter/nf_tables_api.c
> > +++ b/net/netfilter/nf_tables_api.c
> > @@ -1303,6 +1303,11 @@ static int nft_chain_parse_hook(struct net *net,
> >  	}
> >  	if (!(type->hook_mask & (1 << hook->num)))
> >  		return -EOPNOTSUPP;
> > +
> > +	if (type->type == NFT_CHAIN_T_NAT &&
> > +	    hook->priority <= NF_IP_PRI_CONNTRACK)
> > +		return -EINVAL;
> 
> EINVAL is usually for missing netlink attributes, so I'd go for
> EOPNOTSUPP instead. No need to resend I can mangle this here if you
> prefer.

This patch also needs this, otherwise
tests/shell/chains/0006masquerade_0 breaks.

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 1cc1faefed69..2a29a5a58913 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1259,7 +1259,7 @@ static void nf_tables_chain_destroy(struct
nft_chain *chain)
 
 struct nft_chain_hook {
        u32                             num;
-       u32                             priority;
+       s32                             priority;
        const struct nf_chain_type      *type;
        struct net_device               *dev;
 };

I didn't push out this yet, so I'm going to collapse this to your
original patch.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox series

Patch

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index f000d4399c7a..4ed66f1b40b5 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1303,6 +1303,11 @@  static int nft_chain_parse_hook(struct net *net,
 	}
 	if (!(type->hook_mask & (1 << hook->num)))
 		return -EOPNOTSUPP;
+
+	if (type->type == NFT_CHAIN_T_NAT &&
+	    hook->priority <= NF_IP_PRI_CONNTRACK)
+		return -EINVAL;
+
 	if (!try_module_get(type->owner))
 		return -ENOENT;