[PATCH -next] netfilter: meta: add support for setting skb->pkttype

[PATCH -next] netfilter: meta: add support for setting skb->pkttype

[Florian Westphal]

This allows to redirect bridged packets to local machine.

ether type ip ether daddr set aa:53:08:12:34:56 meta pkttype set unicast

Without 'set unicast', ip stack discards PACKET_OTHERHOST skbs.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 Hi Patrick,

 as discussed this adds meta set support for pkttype.

 I'm not sure how restricted it should be; this only allows
 changing PACKET_OTHERHOST to something else, but I'm not sure
 if this should do more checks on value (e.g. only allow
 PACKET_HOST?) or even less (e.g. also allow mangling
 mcast and the like?).

 It only allows NETDEV and BRIDGE families but thats mainly
 because the other ones make no sense (PACKET_OTHERHOST
 cannot happen w. IPV4, 6, INET since stack already drops them earlier).

diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c
index 9dfaf4d..3d64742 100644
--- a/net/netfilter/nft_meta.c
+++ b/net/netfilter/nft_meta.c
@@ -24,6 +24,8 @@
 #include <net/netfilter/nf_tables.h>
 #include <net/netfilter/nft_meta.h>
 
+#include <uapi/linux/netfilter_bridge.h> /* NF_BR_PRE_ROUTING */
+
 void nft_meta_get_eval(const struct nft_expr *expr,
 		       struct nft_regs *regs,
 		       const struct nft_pktinfo *pkt)
@@ -203,6 +205,10 @@ void nft_meta_set_eval(const struct nft_expr *expr,
 	case NFT_META_PRIORITY:
 		skb->priority = value;
 		break;
+	case NFT_META_PKTTYPE:
+		if (skb->pkt_type == PACKET_OTHERHOST)
+			skb->pkt_type = value;
+		break;
 	case NFT_META_NFTRACE:
 		skb->nf_trace = 1;
 		break;
@@ -271,6 +277,24 @@ int nft_meta_get_init(const struct nft_ctx *ctx,
 }
 EXPORT_SYMBOL_GPL(nft_meta_get_init);
 
+static int nft_meta_set_init_pkttype(const struct nft_ctx *ctx)
+{
+	unsigned int hooks;
+
+	switch (ctx->afi->family) {
+	case NFPROTO_BRIDGE:
+		hooks = 1 << NF_BR_PRE_ROUTING;
+		break;
+	case NFPROTO_NETDEV:
+		hooks = 1 << NF_NETDEV_INGRESS;
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	return nft_chain_validate_hooks(ctx->chain, hooks);
+}
+
 int nft_meta_set_init(const struct nft_ctx *ctx,
 		      const struct nft_expr *expr,
 		      const struct nlattr * const tb[])
@@ -288,6 +312,12 @@ int nft_meta_set_init(const struct nft_ctx *ctx,
 	case NFT_META_NFTRACE:
 		len = sizeof(u8);
 		break;
+	case NFT_META_PKTTYPE:
+		err = nft_meta_set_init_pkttype(ctx);
+		if (err)
+			return err;
+		len = sizeof(u8);
+		break;
 	default:
 		return -EOPNOTSUPP;
 	}
-- 
2.4.10

Re: [PATCH -next] netfilter: meta: add support for setting skb->pkttype

[Pablo Neira Ayuso]

On Thu, Dec 03, 2015 at 04:56:41PM +0100, Florian Westphal wrote:
> This allows to redirect bridged packets to local machine.
> 
> ether type ip ether daddr set aa:53:08:12:34:56 meta pkttype set unicast
> 
> Without 'set unicast', ip stack discards PACKET_OTHERHOST skbs.
> 
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>  Hi Patrick,
> 
>  as discussed this adds meta set support for pkttype.
> 
>  I'm not sure how restricted it should be; this only allows
>  changing PACKET_OTHERHOST to something else, but I'm not sure
>  if this should do more checks on value (e.g. only allow
>  PACKET_HOST?) or even less (e.g. also allow mangling
>  mcast and the like?).
>
>  It only allows NETDEV and BRIDGE families but thats mainly
>  because the other ones make no sense (PACKET_OTHERHOST
>  cannot happen w. IPV4, 6, INET since stack already drops them earlier).

With a 'hash' expression and this mangling, we can emulate the cluster
match (hence its CLUSTERIP target predecesor too).

If mangling doesn't crash the kernel, ie. nonsense configuration just
results in a drop, then I would make no restrictions.

Re: [PATCH -next] netfilter: meta: add support for setting skb->pkttype

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Thu, Dec 03, 2015 at 04:56:41PM +0100, Florian Westphal wrote:
> > This allows to redirect bridged packets to local machine.
> > 
> > ether type ip ether daddr set aa:53:08:12:34:56 meta pkttype set unicast
> > 
> > Without 'set unicast', ip stack discards PACKET_OTHERHOST skbs.
> > 
> > Signed-off-by: Florian Westphal <fw@strlen.de>
> > ---
> >  Hi Patrick,
> > 
> >  as discussed this adds meta set support for pkttype.
> > 
> >  I'm not sure how restricted it should be; this only allows
> >  changing PACKET_OTHERHOST to something else, but I'm not sure
> >  if this should do more checks on value (e.g. only allow
> >  PACKET_HOST?) or even less (e.g. also allow mangling
> >  mcast and the like?).
> >
> >  It only allows NETDEV and BRIDGE families but thats mainly
> >  because the other ones make no sense (PACKET_OTHERHOST
> >  cannot happen w. IPV4, 6, INET since stack already drops them earlier).
> 
> With a 'hash' expression and this mangling, we can emulate the cluster
> match (hence its CLUSTERIP target predecesor too).

I forgot about -m cluster, so we definitely want to be able to change
MULTICAST to HOST.

So the only question is wheter we should allow setting it to arbitrary
value (e.g. PACKET_LOOPBACK).  I'm reasonably sure HOST/BROAD/MULTICAST
are okay.

I'll dig if that might cause any problems (esp. when considering
mangling happens in a netns).

> If mangling doesn't crash the kernel, ie. nonsense configuration just
> results in a drop, then I would make no restrictions.

Makes sense.

An updated patch to also permit MCAST will arrive in any case,
moved patchwork state to 'changes requested'.

Thanks!