[PATCH 1/1] bridge: Fix NAT66ed IPv6 packets not being bridged correctly

[PATCH 1/1] bridge: Fix NAT66ed IPv6 packets not being bridged correctly

[Bernhard Thaler]

Ethernet frames are not bridged to correct interface when packets have
been NAT66ed; compared to IPv4 logic in code, IPv6 code does not store
original address (before NAT) on transmit to determine if packet was
NAT66ed on receive to swap addresses back.

Changes added in br_netfilter.c to store original address, compare
against it and determine correct output interface. Changes needed in
netfilter_bridge.h to store IPv6 address in pre-existing union.
Export of ip6_route_input needed to use it in br_netfilter.c.

Problem may only affect systems doing NAT66 and ethernet bridging at
the same time. Tested in NAT66 setup on base of an ethernet bridge.

Signed-off-by: Bernhard Thaler <bernhard.thaler@wvnet.at>
---
 include/linux/netfilter_bridge.h |    2 +
 net/bridge/br_netfilter.c        |  136 ++++++++++++++++++++++++++++----------
 net/ipv6/route.c                 |    1 +
 3 files changed, 105 insertions(+), 34 deletions(-)

diff --git a/include/linux/netfilter_bridge.h b/include/linux/netfilter_bridge.h
index 8ab1c27..3a9cdcd 100644
--- a/include/linux/netfilter_bridge.h
+++ b/include/linux/netfilter_bridge.h
@@ -2,6 +2,7 @@
 #define __LINUX_BRIDGE_NETFILTER_H
 
 #include <uapi/linux/netfilter_bridge.h>
+#include <uapi/linux/in6.h>
 
 
 enum nf_br_hook_priorities {
@@ -79,6 +80,7 @@ static inline unsigned int nf_bridge_pad(const struct sk_buff *skb)
 struct bridge_skb_cb {
 	union {
 		__be32 ipv4;
+		struct in6_addr ipv6;
 	} daddr;
 };
 
diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c
index a615264..2ae3888 100644
--- a/net/bridge/br_netfilter.c
+++ b/net/bridge/br_netfilter.c
@@ -35,6 +35,9 @@
 #include <net/ip.h>
 #include <net/ipv6.h>
 #include <net/route.h>
+#include <net/ip6_route.h>
+#include <net/flow.h>
+#include <net/dst.h>
 
 #include <asm/uaccess.h>
 #include "br_private.h"
@@ -42,10 +45,18 @@
 #include <linux/sysctl.h>
 #endif
 
-#define skb_origaddr(skb)	 (((struct bridge_skb_cb *) \
-				 (skb->nf_bridge->data))->daddr.ipv4)
-#define store_orig_dstaddr(skb)	 (skb_origaddr(skb) = ip_hdr(skb)->daddr)
-#define dnat_took_place(skb)	 (skb_origaddr(skb) != ip_hdr(skb)->daddr)
+#define skb_origaddr(skb)		(((struct bridge_skb_cb *)\
+					(skb->nf_bridge->data))->daddr.ipv4)
+#define skb_origaddr_ipv6(skb)		(((struct bridge_skb_cb *)\
+					(skb->nf_bridge->data))->daddr.ipv6)
+#define store_orig_dstaddr(skb)		(skb_origaddr(skb) = ip_hdr(skb)->daddr)
+#define store_orig_dstaddr_ipv6(skb)	(skb_origaddr_ipv6(skb) = \
+					ipv6_hdr(skb)->daddr)
+#define dnat_took_place(skb)		(skb_origaddr(skb) != \
+					ip_hdr(skb)->daddr)
+#define dnat_took_place_ipv6(skb)	(memcmp(&skb_origaddr_ipv6(skb), \
+					&(ipv6_hdr(skb)->daddr), \
+					sizeof(struct in6_addr)) != 0)
 
 #ifdef CONFIG_SYSCTL
 static struct ctl_table_header *brnf_sysctl_header;
@@ -340,36 +351,6 @@ int nf_bridge_copy_header(struct sk_buff *skb)
 	return 0;
 }
 
-/* PF_BRIDGE/PRE_ROUTING *********************************************/
-/* Undo the changes made for ip6tables PREROUTING and continue the
- * bridge PRE_ROUTING hook. */
-static int br_nf_pre_routing_finish_ipv6(struct sk_buff *skb)
-{
-	struct nf_bridge_info *nf_bridge = skb->nf_bridge;
-	struct rtable *rt;
-
-	if (nf_bridge->mask & BRNF_PKT_TYPE) {
-		skb->pkt_type = PACKET_OTHERHOST;
-		nf_bridge->mask ^= BRNF_PKT_TYPE;
-	}
-	nf_bridge->mask ^= BRNF_NF_BRIDGE_PREROUTING;
-
-	rt = bridge_parent_rtable(nf_bridge->physindev);
-	if (!rt) {
-		kfree_skb(skb);
-		return 0;
-	}
-	skb_dst_set_noref(skb, &rt->dst);
-
-	skb->dev = nf_bridge->physindev;
-	nf_bridge_update_protocol(skb);
-	nf_bridge_push_encap_header(skb);
-	NF_HOOK_THRESH(NFPROTO_BRIDGE, NF_BR_PRE_ROUTING, skb, skb->dev, NULL,
-		       br_handle_frame_finish, 1);
-
-	return 0;
-}
-
 /* Obtain the correct destination MAC address, while preserving the original
  * source MAC address. If we already know this address, we just copy it. If we
  * don't, we use the neighbour framework to find out. In both cases, we make
@@ -527,6 +508,92 @@ bridged_dnat:
 	return 0;
 }
 
+/* PF_BRIDGE/PRE_ROUTING *********************************************
+ * Undo the changes made for ip6tables PREROUTING and continue the
+ * bridge PRE_ROUTING hook.
+ */
+static int br_nf_pre_routing_finish_ipv6(struct sk_buff *skb)
+{
+	struct net_device *dev = skb->dev;
+	struct ipv6hdr *iph = ipv6_hdr(skb);
+	struct nf_bridge_info *nf_bridge = skb->nf_bridge;
+	struct rtable *rt;
+	struct dst_entry *dst;
+	struct flowi6 fl6 = {
+		.flowi6_iif = skb->dev->ifindex,
+		.daddr = iph->daddr,
+		.saddr = iph->saddr,
+		.flowlabel = ip6_flowinfo(iph),
+		.flowi6_mark = skb->mark,
+		.flowi6_proto = iph->nexthdr,
+	};
+
+	if (nf_bridge->mask & BRNF_PKT_TYPE) {
+		skb->pkt_type = PACKET_OTHERHOST;
+		nf_bridge->mask ^= BRNF_PKT_TYPE;
+	}
+	nf_bridge->mask ^= BRNF_NF_BRIDGE_PREROUTING;
+
+	if (dnat_took_place_ipv6(skb)) {
+		ip6_route_input(skb);
+		/* ip6_route_input is void function,
+		 * no int returned as in ip4_route_input
+		 * changes value of skb->_skb_refdst) on success
+		 */
+		if (skb->_skb_refdst == 0) {
+			struct in_device *in_dev = __in_dev_get_rcu(dev);
+
+			if (!in_dev || IN_DEV_FORWARD(in_dev))
+				goto free_skb;
+
+			dst = ip6_route_output(dev_net(dev), skb->sk, &fl6);
+			if (!IS_ERR(dst)) {
+				/* - Bridged-and-DNAT'ed traffic doesn't
+				 *   require ip_forwarding.
+				 */
+				if (dst->dev == dev) {
+					skb_dst_set(skb, dst);
+					goto bridged_dnat;
+				}
+				dst_release(dst);
+			}
+free_skb:
+			kfree_skb(skb);
+			return 0;
+		} else {
+			if (skb_dst(skb)->dev == dev) {
+bridged_dnat:
+				skb->dev = nf_bridge->physindev;
+				nf_bridge_update_protocol(skb);
+				nf_bridge_push_encap_header(skb);
+				NF_HOOK_THRESH(NFPROTO_BRIDGE,
+					       NF_BR_PRE_ROUTING,
+					       skb, skb->dev, NULL,
+					       br_nf_pre_routing_finish_bridge,
+					       1);
+				return 0;
+			}
+			memcpy(eth_hdr(skb)->h_dest, dev->dev_addr, ETH_ALEN);
+			skb->pkt_type = PACKET_HOST;
+		}
+	} else {
+		rt = bridge_parent_rtable(nf_bridge->physindev);
+		if (!rt) {
+			kfree_skb(skb);
+			return 0;
+		}
+		skb_dst_set_noref(skb, &rt->dst);
+	}
+
+	skb->dev = nf_bridge->physindev;
+	nf_bridge_update_protocol(skb);
+	nf_bridge_push_encap_header(skb);
+	NF_HOOK_THRESH(NFPROTO_BRIDGE, NF_BR_PRE_ROUTING, skb, skb->dev, NULL,
+		       br_handle_frame_finish, 1);
+
+	return 0;
+}
+
 static struct net_device *brnf_get_logical_dev(struct sk_buff *skb, const struct net_device *dev)
 {
 	struct net_device *vlan, *br;
@@ -658,6 +725,7 @@ static unsigned int br_nf_pre_routing_ipv6(const struct nf_hook_ops *ops,
 	if (!setup_pre_routing(skb))
 		return NF_DROP;
 
+	store_orig_dstaddr_ipv6(skb);
 	skb->protocol = htons(ETH_P_IPV6);
 	NF_HOOK(NFPROTO_IPV6, NF_INET_PRE_ROUTING, skb, skb->dev, NULL,
 		br_nf_pre_routing_finish_ipv6);
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index f23fbd2..e328905 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1017,6 +1017,7 @@ void ip6_route_input(struct sk_buff *skb)
 
 	skb_dst_set(skb, ip6_route_input_lookup(net, skb->dev, &fl6, flags));
 }
+EXPORT_SYMBOL(ip6_route_input);
 
 static struct rt6_info *ip6_pol_route_output(struct net *net, struct fib6_table *table,
 					     struct flowi6 *fl6, int flags)
-- 
1.7.10.4

Re: [PATCH 1/1] bridge: Fix NAT66ed IPv6 packets not being bridged correctly

[David Miller]

From: Bernhard Thaler <bernhard.thaler@wvnet.at>
Date: Mon, 15 Sep 2014 23:27:28 +0200

CC:'ing netfilter-devel and the netfilter maintainer, which is
probably the primary place this patch should have been submitted.

> Ethernet frames are not bridged to correct interface when packets have
> been NAT66ed; compared to IPv4 logic in code, IPv6 code does not store
> original address (before NAT) on transmit to determine if packet was
> NAT66ed on receive to swap addresses back.
> 
> Changes added in br_netfilter.c to store original address, compare
> against it and determine correct output interface. Changes needed in
> netfilter_bridge.h to store IPv6 address in pre-existing union.
> Export of ip6_route_input needed to use it in br_netfilter.c.
> 
> Problem may only affect systems doing NAT66 and ethernet bridging at
> the same time. Tested in NAT66 setup on base of an ethernet bridge.
> 
> Signed-off-by: Bernhard Thaler <bernhard.thaler@wvnet.at>
> ---
>  include/linux/netfilter_bridge.h |    2 +
>  net/bridge/br_netfilter.c        |  136 ++++++++++++++++++++++++++++----------
>  net/ipv6/route.c                 |    1 +
>  3 files changed, 105 insertions(+), 34 deletions(-)
> 
> diff --git a/include/linux/netfilter_bridge.h b/include/linux/netfilter_bridge.h
> index 8ab1c27..3a9cdcd 100644
> --- a/include/linux/netfilter_bridge.h
> +++ b/include/linux/netfilter_bridge.h
> @@ -2,6 +2,7 @@
>  #define __LINUX_BRIDGE_NETFILTER_H
>  
>  #include <uapi/linux/netfilter_bridge.h>
> +#include <uapi/linux/in6.h>
>  
>  
>  enum nf_br_hook_priorities {
> @@ -79,6 +80,7 @@ static inline unsigned int nf_bridge_pad(const struct sk_buff *skb)
>  struct bridge_skb_cb {
>  	union {
>  		__be32 ipv4;
> +		struct in6_addr ipv6;
>  	} daddr;
>  };
>  
> diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c
> index a615264..2ae3888 100644
> --- a/net/bridge/br_netfilter.c
> +++ b/net/bridge/br_netfilter.c
> @@ -35,6 +35,9 @@
>  #include <net/ip.h>
>  #include <net/ipv6.h>
>  #include <net/route.h>
> +#include <net/ip6_route.h>
> +#include <net/flow.h>
> +#include <net/dst.h>
>  
>  #include <asm/uaccess.h>
>  #include "br_private.h"
> @@ -42,10 +45,18 @@
>  #include <linux/sysctl.h>
>  #endif
>  
> -#define skb_origaddr(skb)	 (((struct bridge_skb_cb *) \
> -				 (skb->nf_bridge->data))->daddr.ipv4)
> -#define store_orig_dstaddr(skb)	 (skb_origaddr(skb) = ip_hdr(skb)->daddr)
> -#define dnat_took_place(skb)	 (skb_origaddr(skb) != ip_hdr(skb)->daddr)
> +#define skb_origaddr(skb)		(((struct bridge_skb_cb *)\
> +					(skb->nf_bridge->data))->daddr.ipv4)
> +#define skb_origaddr_ipv6(skb)		(((struct bridge_skb_cb *)\
> +					(skb->nf_bridge->data))->daddr.ipv6)
> +#define store_orig_dstaddr(skb)		(skb_origaddr(skb) = ip_hdr(skb)->daddr)
> +#define store_orig_dstaddr_ipv6(skb)	(skb_origaddr_ipv6(skb) = \
> +					ipv6_hdr(skb)->daddr)
> +#define dnat_took_place(skb)		(skb_origaddr(skb) != \
> +					ip_hdr(skb)->daddr)
> +#define dnat_took_place_ipv6(skb)	(memcmp(&skb_origaddr_ipv6(skb), \
> +					&(ipv6_hdr(skb)->daddr), \
> +					sizeof(struct in6_addr)) != 0)
>  
>  #ifdef CONFIG_SYSCTL
>  static struct ctl_table_header *brnf_sysctl_header;
> @@ -340,36 +351,6 @@ int nf_bridge_copy_header(struct sk_buff *skb)
>  	return 0;
>  }
>  
> -/* PF_BRIDGE/PRE_ROUTING *********************************************/
> -/* Undo the changes made for ip6tables PREROUTING and continue the
> - * bridge PRE_ROUTING hook. */
> -static int br_nf_pre_routing_finish_ipv6(struct sk_buff *skb)
> -{
> -	struct nf_bridge_info *nf_bridge = skb->nf_bridge;
> -	struct rtable *rt;
> -
> -	if (nf_bridge->mask & BRNF_PKT_TYPE) {
> -		skb->pkt_type = PACKET_OTHERHOST;
> -		nf_bridge->mask ^= BRNF_PKT_TYPE;
> -	}
> -	nf_bridge->mask ^= BRNF_NF_BRIDGE_PREROUTING;
> -
> -	rt = bridge_parent_rtable(nf_bridge->physindev);
> -	if (!rt) {
> -		kfree_skb(skb);
> -		return 0;
> -	}
> -	skb_dst_set_noref(skb, &rt->dst);
> -
> -	skb->dev = nf_bridge->physindev;
> -	nf_bridge_update_protocol(skb);
> -	nf_bridge_push_encap_header(skb);
> -	NF_HOOK_THRESH(NFPROTO_BRIDGE, NF_BR_PRE_ROUTING, skb, skb->dev, NULL,
> -		       br_handle_frame_finish, 1);
> -
> -	return 0;
> -}
> -
>  /* Obtain the correct destination MAC address, while preserving the original
>   * source MAC address. If we already know this address, we just copy it. If we
>   * don't, we use the neighbour framework to find out. In both cases, we make
> @@ -527,6 +508,92 @@ bridged_dnat:
>  	return 0;
>  }
>  
> +/* PF_BRIDGE/PRE_ROUTING *********************************************
> + * Undo the changes made for ip6tables PREROUTING and continue the
> + * bridge PRE_ROUTING hook.
> + */
> +static int br_nf_pre_routing_finish_ipv6(struct sk_buff *skb)
> +{
> +	struct net_device *dev = skb->dev;
> +	struct ipv6hdr *iph = ipv6_hdr(skb);
> +	struct nf_bridge_info *nf_bridge = skb->nf_bridge;
> +	struct rtable *rt;
> +	struct dst_entry *dst;
> +	struct flowi6 fl6 = {
> +		.flowi6_iif = skb->dev->ifindex,
> +		.daddr = iph->daddr,
> +		.saddr = iph->saddr,
> +		.flowlabel = ip6_flowinfo(iph),
> +		.flowi6_mark = skb->mark,
> +		.flowi6_proto = iph->nexthdr,
> +	};
> +
> +	if (nf_bridge->mask & BRNF_PKT_TYPE) {
> +		skb->pkt_type = PACKET_OTHERHOST;
> +		nf_bridge->mask ^= BRNF_PKT_TYPE;
> +	}
> +	nf_bridge->mask ^= BRNF_NF_BRIDGE_PREROUTING;
> +
> +	if (dnat_took_place_ipv6(skb)) {
> +		ip6_route_input(skb);
> +		/* ip6_route_input is void function,
> +		 * no int returned as in ip4_route_input
> +		 * changes value of skb->_skb_refdst) on success
> +		 */
> +		if (skb->_skb_refdst == 0) {
> +			struct in_device *in_dev = __in_dev_get_rcu(dev);
> +
> +			if (!in_dev || IN_DEV_FORWARD(in_dev))
> +				goto free_skb;
> +
> +			dst = ip6_route_output(dev_net(dev), skb->sk, &fl6);
> +			if (!IS_ERR(dst)) {
> +				/* - Bridged-and-DNAT'ed traffic doesn't
> +				 *   require ip_forwarding.
> +				 */
> +				if (dst->dev == dev) {
> +					skb_dst_set(skb, dst);
> +					goto bridged_dnat;
> +				}
> +				dst_release(dst);
> +			}
> +free_skb:
> +			kfree_skb(skb);
> +			return 0;
> +		} else {
> +			if (skb_dst(skb)->dev == dev) {
> +bridged_dnat:
> +				skb->dev = nf_bridge->physindev;
> +				nf_bridge_update_protocol(skb);
> +				nf_bridge_push_encap_header(skb);
> +				NF_HOOK_THRESH(NFPROTO_BRIDGE,
> +					       NF_BR_PRE_ROUTING,
> +					       skb, skb->dev, NULL,
> +					       br_nf_pre_routing_finish_bridge,
> +					       1);
> +				return 0;
> +			}
> +			memcpy(eth_hdr(skb)->h_dest, dev->dev_addr, ETH_ALEN);
> +			skb->pkt_type = PACKET_HOST;
> +		}
> +	} else {
> +		rt = bridge_parent_rtable(nf_bridge->physindev);
> +		if (!rt) {
> +			kfree_skb(skb);
> +			return 0;
> +		}
> +		skb_dst_set_noref(skb, &rt->dst);
> +	}
> +
> +	skb->dev = nf_bridge->physindev;
> +	nf_bridge_update_protocol(skb);
> +	nf_bridge_push_encap_header(skb);
> +	NF_HOOK_THRESH(NFPROTO_BRIDGE, NF_BR_PRE_ROUTING, skb, skb->dev, NULL,
> +		       br_handle_frame_finish, 1);
> +
> +	return 0;
> +}
> +
>  static struct net_device *brnf_get_logical_dev(struct sk_buff *skb, const struct net_device *dev)
>  {
>  	struct net_device *vlan, *br;
> @@ -658,6 +725,7 @@ static unsigned int br_nf_pre_routing_ipv6(const struct nf_hook_ops *ops,
>  	if (!setup_pre_routing(skb))
>  		return NF_DROP;
>  
> +	store_orig_dstaddr_ipv6(skb);
>  	skb->protocol = htons(ETH_P_IPV6);
>  	NF_HOOK(NFPROTO_IPV6, NF_INET_PRE_ROUTING, skb, skb->dev, NULL,
>  		br_nf_pre_routing_finish_ipv6);
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index f23fbd2..e328905 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -1017,6 +1017,7 @@ void ip6_route_input(struct sk_buff *skb)
>  
>  	skb_dst_set(skb, ip6_route_input_lookup(net, skb->dev, &fl6, flags));
>  }
> +EXPORT_SYMBOL(ip6_route_input);
>  
>  static struct rt6_info *ip6_pol_route_output(struct net *net, struct fib6_table *table,
>  					     struct flowi6 *fl6, int flags)
> -- 
> 1.7.10.4
> 

Re: [PATCH 1/1] bridge: Fix NAT66ed IPv6 packets not being bridged correctly

[Pablo Neira Ayuso]

Hi Bernhard,

Sorry for taking a bit to get back to you with feedback. We've been
discussing recently some changes in br_netfilter. Basically, to
modularize it [1] and this has taken a while.

Regarding your change. Sven Eckelmann (CC'ed in this email) sent a RFC
out of the merge window that have remain tagged in my patchwork, you
can find it here.

http://patchwork.ozlabs.org/patch/381027/

I would like to see a submission that covers all NAT66 scenarios that
we currenty support.

Thanks.

[1] http://comments.gmane.org/gmane.comp.security.firewalls.netfilter.devel/54234

On Mon, Sep 15, 2014 at 05:40:09PM -0400, David Miller wrote:
> From: Bernhard Thaler <bernhard.thaler@wvnet.at>
> Date: Mon, 15 Sep 2014 23:27:28 +0200
> 
> CC:'ing netfilter-devel and the netfilter maintainer, which is
> probably the primary place this patch should have been submitted.
> 
> > Ethernet frames are not bridged to correct interface when packets have
> > been NAT66ed; compared to IPv4 logic in code, IPv6 code does not store
> > original address (before NAT) on transmit to determine if packet was
> > NAT66ed on receive to swap addresses back.
> > 
> > Changes added in br_netfilter.c to store original address, compare
> > against it and determine correct output interface. Changes needed in
> > netfilter_bridge.h to store IPv6 address in pre-existing union.
> > Export of ip6_route_input needed to use it in br_netfilter.c.
> > 
> > Problem may only affect systems doing NAT66 and ethernet bridging at
> > the same time. Tested in NAT66 setup on base of an ethernet bridge.
> > 
> > Signed-off-by: Bernhard Thaler <bernhard.thaler@wvnet.at>
> > ---
> >  include/linux/netfilter_bridge.h |    2 +
> >  net/bridge/br_netfilter.c        |  136 ++++++++++++++++++++++++++++----------
> >  net/ipv6/route.c                 |    1 +
> >  3 files changed, 105 insertions(+), 34 deletions(-)
> > 
> > diff --git a/include/linux/netfilter_bridge.h b/include/linux/netfilter_bridge.h
> > index 8ab1c27..3a9cdcd 100644
> > --- a/include/linux/netfilter_bridge.h
> > +++ b/include/linux/netfilter_bridge.h
> > @@ -2,6 +2,7 @@
> >  #define __LINUX_BRIDGE_NETFILTER_H
> >  
> >  #include <uapi/linux/netfilter_bridge.h>
> > +#include <uapi/linux/in6.h>
> >  
> >  
> >  enum nf_br_hook_priorities {
> > @@ -79,6 +80,7 @@ static inline unsigned int nf_bridge_pad(const struct sk_buff *skb)
> >  struct bridge_skb_cb {
> >  	union {
> >  		__be32 ipv4;
> > +		struct in6_addr ipv6;
> >  	} daddr;
> >  };
> >  
> > diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c
> > index a615264..2ae3888 100644
> > --- a/net/bridge/br_netfilter.c
> > +++ b/net/bridge/br_netfilter.c
> > @@ -35,6 +35,9 @@
> >  #include <net/ip.h>
> >  #include <net/ipv6.h>
> >  #include <net/route.h>
> > +#include <net/ip6_route.h>
> > +#include <net/flow.h>
> > +#include <net/dst.h>
> >  
> >  #include <asm/uaccess.h>
> >  #include "br_private.h"
> > @@ -42,10 +45,18 @@
> >  #include <linux/sysctl.h>
> >  #endif
> >  
> > -#define skb_origaddr(skb)	 (((struct bridge_skb_cb *) \
> > -				 (skb->nf_bridge->data))->daddr.ipv4)
> > -#define store_orig_dstaddr(skb)	 (skb_origaddr(skb) = ip_hdr(skb)->daddr)
> > -#define dnat_took_place(skb)	 (skb_origaddr(skb) != ip_hdr(skb)->daddr)
> > +#define skb_origaddr(skb)		(((struct bridge_skb_cb *)\
> > +					(skb->nf_bridge->data))->daddr.ipv4)
> > +#define skb_origaddr_ipv6(skb)		(((struct bridge_skb_cb *)\
> > +					(skb->nf_bridge->data))->daddr.ipv6)
> > +#define store_orig_dstaddr(skb)		(skb_origaddr(skb) = ip_hdr(skb)->daddr)
> > +#define store_orig_dstaddr_ipv6(skb)	(skb_origaddr_ipv6(skb) = \
> > +					ipv6_hdr(skb)->daddr)
> > +#define dnat_took_place(skb)		(skb_origaddr(skb) != \
> > +					ip_hdr(skb)->daddr)
> > +#define dnat_took_place_ipv6(skb)	(memcmp(&skb_origaddr_ipv6(skb), \
> > +					&(ipv6_hdr(skb)->daddr), \
> > +					sizeof(struct in6_addr)) != 0)
> >  
> >  #ifdef CONFIG_SYSCTL
> >  static struct ctl_table_header *brnf_sysctl_header;
> > @@ -340,36 +351,6 @@ int nf_bridge_copy_header(struct sk_buff *skb)
> >  	return 0;
> >  }
> >  
> > -/* PF_BRIDGE/PRE_ROUTING *********************************************/
> > -/* Undo the changes made for ip6tables PREROUTING and continue the
> > - * bridge PRE_ROUTING hook. */
> > -static int br_nf_pre_routing_finish_ipv6(struct sk_buff *skb)
> > -{
> > -	struct nf_bridge_info *nf_bridge = skb->nf_bridge;
> > -	struct rtable *rt;
> > -
> > -	if (nf_bridge->mask & BRNF_PKT_TYPE) {
> > -		skb->pkt_type = PACKET_OTHERHOST;
> > -		nf_bridge->mask ^= BRNF_PKT_TYPE;
> > -	}
> > -	nf_bridge->mask ^= BRNF_NF_BRIDGE_PREROUTING;
> > -
> > -	rt = bridge_parent_rtable(nf_bridge->physindev);
> > -	if (!rt) {
> > -		kfree_skb(skb);
> > -		return 0;
> > -	}
> > -	skb_dst_set_noref(skb, &rt->dst);
> > -
> > -	skb->dev = nf_bridge->physindev;
> > -	nf_bridge_update_protocol(skb);
> > -	nf_bridge_push_encap_header(skb);
> > -	NF_HOOK_THRESH(NFPROTO_BRIDGE, NF_BR_PRE_ROUTING, skb, skb->dev, NULL,
> > -		       br_handle_frame_finish, 1);
> > -
> > -	return 0;
> > -}
> > -
> >  /* Obtain the correct destination MAC address, while preserving the original
> >   * source MAC address. If we already know this address, we just copy it. If we
> >   * don't, we use the neighbour framework to find out. In both cases, we make
> > @@ -527,6 +508,92 @@ bridged_dnat:
> >  	return 0;
> >  }
> >  
> > +/* PF_BRIDGE/PRE_ROUTING *********************************************
> > + * Undo the changes made for ip6tables PREROUTING and continue the
> > + * bridge PRE_ROUTING hook.
> > + */
> > +static int br_nf_pre_routing_finish_ipv6(struct sk_buff *skb)
> > +{
> > +	struct net_device *dev = skb->dev;
> > +	struct ipv6hdr *iph = ipv6_hdr(skb);
> > +	struct nf_bridge_info *nf_bridge = skb->nf_bridge;
> > +	struct rtable *rt;
> > +	struct dst_entry *dst;
> > +	struct flowi6 fl6 = {
> > +		.flowi6_iif = skb->dev->ifindex,
> > +		.daddr = iph->daddr,
> > +		.saddr = iph->saddr,
> > +		.flowlabel = ip6_flowinfo(iph),
> > +		.flowi6_mark = skb->mark,
> > +		.flowi6_proto = iph->nexthdr,
> > +	};
> > +
> > +	if (nf_bridge->mask & BRNF_PKT_TYPE) {
> > +		skb->pkt_type = PACKET_OTHERHOST;
> > +		nf_bridge->mask ^= BRNF_PKT_TYPE;
> > +	}
> > +	nf_bridge->mask ^= BRNF_NF_BRIDGE_PREROUTING;
> > +
> > +	if (dnat_took_place_ipv6(skb)) {
> > +		ip6_route_input(skb);
> > +		/* ip6_route_input is void function,
> > +		 * no int returned as in ip4_route_input
> > +		 * changes value of skb->_skb_refdst) on success
> > +		 */
> > +		if (skb->_skb_refdst == 0) {
> > +			struct in_device *in_dev = __in_dev_get_rcu(dev);
> > +
> > +			if (!in_dev || IN_DEV_FORWARD(in_dev))
> > +				goto free_skb;
> > +
> > +			dst = ip6_route_output(dev_net(dev), skb->sk, &fl6);
> > +			if (!IS_ERR(dst)) {
> > +				/* - Bridged-and-DNAT'ed traffic doesn't
> > +				 *   require ip_forwarding.
> > +				 */
> > +				if (dst->dev == dev) {
> > +					skb_dst_set(skb, dst);
> > +					goto bridged_dnat;
> > +				}
> > +				dst_release(dst);
> > +			}
> > +free_skb:
> > +			kfree_skb(skb);
> > +			return 0;
> > +		} else {
> > +			if (skb_dst(skb)->dev == dev) {
> > +bridged_dnat:
> > +				skb->dev = nf_bridge->physindev;
> > +				nf_bridge_update_protocol(skb);
> > +				nf_bridge_push_encap_header(skb);
> > +				NF_HOOK_THRESH(NFPROTO_BRIDGE,
> > +					       NF_BR_PRE_ROUTING,
> > +					       skb, skb->dev, NULL,
> > +					       br_nf_pre_routing_finish_bridge,
> > +					       1);
> > +				return 0;
> > +			}
> > +			memcpy(eth_hdr(skb)->h_dest, dev->dev_addr, ETH_ALEN);
> > +			skb->pkt_type = PACKET_HOST;
> > +		}
> > +	} else {
> > +		rt = bridge_parent_rtable(nf_bridge->physindev);
> > +		if (!rt) {
> > +			kfree_skb(skb);
> > +			return 0;
> > +		}
> > +		skb_dst_set_noref(skb, &rt->dst);
> > +	}
> > +
> > +	skb->dev = nf_bridge->physindev;
> > +	nf_bridge_update_protocol(skb);
> > +	nf_bridge_push_encap_header(skb);
> > +	NF_HOOK_THRESH(NFPROTO_BRIDGE, NF_BR_PRE_ROUTING, skb, skb->dev, NULL,
> > +		       br_handle_frame_finish, 1);
> > +
> > +	return 0;
> > +}
> > +
> >  static struct net_device *brnf_get_logical_dev(struct sk_buff *skb, const struct net_device *dev)
> >  {
> >  	struct net_device *vlan, *br;
> > @@ -658,6 +725,7 @@ static unsigned int br_nf_pre_routing_ipv6(const struct nf_hook_ops *ops,
> >  	if (!setup_pre_routing(skb))
> >  		return NF_DROP;
> >  
> > +	store_orig_dstaddr_ipv6(skb);
> >  	skb->protocol = htons(ETH_P_IPV6);
> >  	NF_HOOK(NFPROTO_IPV6, NF_INET_PRE_ROUTING, skb, skb->dev, NULL,
> >  		br_nf_pre_routing_finish_ipv6);
> > diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> > index f23fbd2..e328905 100644
> > --- a/net/ipv6/route.c
> > +++ b/net/ipv6/route.c
> > @@ -1017,6 +1017,7 @@ void ip6_route_input(struct sk_buff *skb)
> >  
> >  	skb_dst_set(skb, ip6_route_input_lookup(net, skb->dev, &fl6, flags));
> >  }
> > +EXPORT_SYMBOL(ip6_route_input);
> >  
> >  static struct rt6_info *ip6_pol_route_output(struct net *net, struct fib6_table *table,
> >  					     struct flowi6 *fl6, int flags)
> > -- 
> > 1.7.10.4
> > 

Re: [PATCH 1/1] bridge: Fix NAT66ed IPv6 packets not being bridged correctly

[Bernhard Thaler]

Hi,

I tested Sven's patch in my setup and I think it should be safe to use
it. It is shorter and cleaner written and he submitted it earlier.

I will be happy to assist you or Sven if any further work is needed.

Regards,
Bernhard

On 03.10.2014 13:21, Pablo Neira Ayuso wrote:
> Hi Bernhard,
> 
> Sorry for taking a bit to get back to you with feedback. We've been
> discussing recently some changes in br_netfilter. Basically, to
> modularize it [1] and this has taken a while.
> 
> Regarding your change. Sven Eckelmann (CC'ed in this email) sent a RFC
> out of the merge window that have remain tagged in my patchwork, you
> can find it here.
> 
> http://patchwork.ozlabs.org/patch/381027/
> 
> I would like to see a submission that covers all NAT66 scenarios that
> we currenty support.
> 
> Thanks.
> 
> [1] http://comments.gmane.org/gmane.comp.security.firewalls.netfilter.devel/54234
> 
> On Mon, Sep 15, 2014 at 05:40:09PM -0400, David Miller wrote:
>> From: Bernhard Thaler <bernhard.thaler@wvnet.at>
>> Date: Mon, 15 Sep 2014 23:27:28 +0200
>>
>> CC:'ing netfilter-devel and the netfilter maintainer, which is
>> probably the primary place this patch should have been submitted.
>>
>>> Ethernet frames are not bridged to correct interface when packets have
>>> been NAT66ed; compared to IPv4 logic in code, IPv6 code does not store
>>> original address (before NAT) on transmit to determine if packet was
>>> NAT66ed on receive to swap addresses back.
>>>
>>> Changes added in br_netfilter.c to store original address, compare
>>> against it and determine correct output interface. Changes needed in
>>> netfilter_bridge.h to store IPv6 address in pre-existing union.
>>> Export of ip6_route_input needed to use it in br_netfilter.c.
>>>
>>> Problem may only affect systems doing NAT66 and ethernet bridging at
>>> the same time. Tested in NAT66 setup on base of an ethernet bridge.
>>>
>>> Signed-off-by: Bernhard Thaler <bernhard.thaler@wvnet.at>
>>> ---
>>>  include/linux/netfilter_bridge.h |    2 +
>>>  net/bridge/br_netfilter.c        |  136 ++++++++++++++++++++++++++++----------
>>>  net/ipv6/route.c                 |    1 +
>>>  3 files changed, 105 insertions(+), 34 deletions(-)
>>>
>>> diff --git a/include/linux/netfilter_bridge.h b/include/linux/netfilter_bridge.h
>>> index 8ab1c27..3a9cdcd 100644
>>> --- a/include/linux/netfilter_bridge.h
>>> +++ b/include/linux/netfilter_bridge.h
>>> @@ -2,6 +2,7 @@
>>>  #define __LINUX_BRIDGE_NETFILTER_H
>>>  
>>>  #include <uapi/linux/netfilter_bridge.h>
>>> +#include <uapi/linux/in6.h>
>>>  
>>>  
>>>  enum nf_br_hook_priorities {
>>> @@ -79,6 +80,7 @@ static inline unsigned int nf_bridge_pad(const struct sk_buff *skb)
>>>  struct bridge_skb_cb {
>>>  	union {
>>>  		__be32 ipv4;
>>> +		struct in6_addr ipv6;
>>>  	} daddr;
>>>  };
>>>  
>>> diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c
>>> index a615264..2ae3888 100644
>>> --- a/net/bridge/br_netfilter.c
>>> +++ b/net/bridge/br_netfilter.c
>>> @@ -35,6 +35,9 @@
>>>  #include <net/ip.h>
>>>  #include <net/ipv6.h>
>>>  #include <net/route.h>
>>> +#include <net/ip6_route.h>
>>> +#include <net/flow.h>
>>> +#include <net/dst.h>
>>>  
>>>  #include <asm/uaccess.h>
>>>  #include "br_private.h"
>>> @@ -42,10 +45,18 @@
>>>  #include <linux/sysctl.h>
>>>  #endif
>>>  
>>> -#define skb_origaddr(skb)	 (((struct bridge_skb_cb *) \
>>> -				 (skb->nf_bridge->data))->daddr.ipv4)
>>> -#define store_orig_dstaddr(skb)	 (skb_origaddr(skb) = ip_hdr(skb)->daddr)
>>> -#define dnat_took_place(skb)	 (skb_origaddr(skb) != ip_hdr(skb)->daddr)
>>> +#define skb_origaddr(skb)		(((struct bridge_skb_cb *)\
>>> +					(skb->nf_bridge->data))->daddr.ipv4)
>>> +#define skb_origaddr_ipv6(skb)		(((struct bridge_skb_cb *)\
>>> +					(skb->nf_bridge->data))->daddr.ipv6)
>>> +#define store_orig_dstaddr(skb)		(skb_origaddr(skb) = ip_hdr(skb)->daddr)
>>> +#define store_orig_dstaddr_ipv6(skb)	(skb_origaddr_ipv6(skb) = \
>>> +					ipv6_hdr(skb)->daddr)
>>> +#define dnat_took_place(skb)		(skb_origaddr(skb) != \
>>> +					ip_hdr(skb)->daddr)
>>> +#define dnat_took_place_ipv6(skb)	(memcmp(&skb_origaddr_ipv6(skb), \
>>> +					&(ipv6_hdr(skb)->daddr), \
>>> +					sizeof(struct in6_addr)) != 0)
>>>  
>>>  #ifdef CONFIG_SYSCTL
>>>  static struct ctl_table_header *brnf_sysctl_header;
>>> @@ -340,36 +351,6 @@ int nf_bridge_copy_header(struct sk_buff *skb)
>>>  	return 0;
>>>  }
>>>  
>>> -/* PF_BRIDGE/PRE_ROUTING *********************************************/
>>> -/* Undo the changes made for ip6tables PREROUTING and continue the
>>> - * bridge PRE_ROUTING hook. */
>>> -static int br_nf_pre_routing_finish_ipv6(struct sk_buff *skb)
>>> -{
>>> -	struct nf_bridge_info *nf_bridge = skb->nf_bridge;
>>> -	struct rtable *rt;
>>> -
>>> -	if (nf_bridge->mask & BRNF_PKT_TYPE) {
>>> -		skb->pkt_type = PACKET_OTHERHOST;
>>> -		nf_bridge->mask ^= BRNF_PKT_TYPE;
>>> -	}
>>> -	nf_bridge->mask ^= BRNF_NF_BRIDGE_PREROUTING;
>>> -
>>> -	rt = bridge_parent_rtable(nf_bridge->physindev);
>>> -	if (!rt) {
>>> -		kfree_skb(skb);
>>> -		return 0;
>>> -	}
>>> -	skb_dst_set_noref(skb, &rt->dst);
>>> -
>>> -	skb->dev = nf_bridge->physindev;
>>> -	nf_bridge_update_protocol(skb);
>>> -	nf_bridge_push_encap_header(skb);
>>> -	NF_HOOK_THRESH(NFPROTO_BRIDGE, NF_BR_PRE_ROUTING, skb, skb->dev, NULL,
>>> -		       br_handle_frame_finish, 1);
>>> -
>>> -	return 0;
>>> -}
>>> -
>>>  /* Obtain the correct destination MAC address, while preserving the original
>>>   * source MAC address. If we already know this address, we just copy it. If we
>>>   * don't, we use the neighbour framework to find out. In both cases, we make
>>> @@ -527,6 +508,92 @@ bridged_dnat:
>>>  	return 0;
>>>  }
>>>  
>>> +/* PF_BRIDGE/PRE_ROUTING *********************************************
>>> + * Undo the changes made for ip6tables PREROUTING and continue the
>>> + * bridge PRE_ROUTING hook.
>>> + */
>>> +static int br_nf_pre_routing_finish_ipv6(struct sk_buff *skb)
>>> +{
>>> +	struct net_device *dev = skb->dev;
>>> +	struct ipv6hdr *iph = ipv6_hdr(skb);
>>> +	struct nf_bridge_info *nf_bridge = skb->nf_bridge;
>>> +	struct rtable *rt;
>>> +	struct dst_entry *dst;
>>> +	struct flowi6 fl6 = {
>>> +		.flowi6_iif = skb->dev->ifindex,
>>> +		.daddr = iph->daddr,
>>> +		.saddr = iph->saddr,
>>> +		.flowlabel = ip6_flowinfo(iph),
>>> +		.flowi6_mark = skb->mark,
>>> +		.flowi6_proto = iph->nexthdr,
>>> +	};
>>> +
>>> +	if (nf_bridge->mask & BRNF_PKT_TYPE) {
>>> +		skb->pkt_type = PACKET_OTHERHOST;
>>> +		nf_bridge->mask ^= BRNF_PKT_TYPE;
>>> +	}
>>> +	nf_bridge->mask ^= BRNF_NF_BRIDGE_PREROUTING;
>>> +
>>> +	if (dnat_took_place_ipv6(skb)) {
>>> +		ip6_route_input(skb);
>>> +		/* ip6_route_input is void function,
>>> +		 * no int returned as in ip4_route_input
>>> +		 * changes value of skb->_skb_refdst) on success
>>> +		 */
>>> +		if (skb->_skb_refdst == 0) {
>>> +			struct in_device *in_dev = __in_dev_get_rcu(dev);
>>> +
>>> +			if (!in_dev || IN_DEV_FORWARD(in_dev))
>>> +				goto free_skb;
>>> +
>>> +			dst = ip6_route_output(dev_net(dev), skb->sk, &fl6);
>>> +			if (!IS_ERR(dst)) {
>>> +				/* - Bridged-and-DNAT'ed traffic doesn't
>>> +				 *   require ip_forwarding.
>>> +				 */
>>> +				if (dst->dev == dev) {
>>> +					skb_dst_set(skb, dst);
>>> +					goto bridged_dnat;
>>> +				}
>>> +				dst_release(dst);
>>> +			}
>>> +free_skb:
>>> +			kfree_skb(skb);
>>> +			return 0;
>>> +		} else {
>>> +			if (skb_dst(skb)->dev == dev) {
>>> +bridged_dnat:
>>> +				skb->dev = nf_bridge->physindev;
>>> +				nf_bridge_update_protocol(skb);
>>> +				nf_bridge_push_encap_header(skb);
>>> +				NF_HOOK_THRESH(NFPROTO_BRIDGE,
>>> +					       NF_BR_PRE_ROUTING,
>>> +					       skb, skb->dev, NULL,
>>> +					       br_nf_pre_routing_finish_bridge,
>>> +					       1);
>>> +				return 0;
>>> +			}
>>> +			memcpy(eth_hdr(skb)->h_dest, dev->dev_addr, ETH_ALEN);
>>> +			skb->pkt_type = PACKET_HOST;
>>> +		}
>>> +	} else {
>>> +		rt = bridge_parent_rtable(nf_bridge->physindev);
>>> +		if (!rt) {
>>> +			kfree_skb(skb);
>>> +			return 0;
>>> +		}
>>> +		skb_dst_set_noref(skb, &rt->dst);
>>> +	}
>>> +
>>> +	skb->dev = nf_bridge->physindev;
>>> +	nf_bridge_update_protocol(skb);
>>> +	nf_bridge_push_encap_header(skb);
>>> +	NF_HOOK_THRESH(NFPROTO_BRIDGE, NF_BR_PRE_ROUTING, skb, skb->dev, NULL,
>>> +		       br_handle_frame_finish, 1);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>>  static struct net_device *brnf_get_logical_dev(struct sk_buff *skb, const struct net_device *dev)
>>>  {
>>>  	struct net_device *vlan, *br;
>>> @@ -658,6 +725,7 @@ static unsigned int br_nf_pre_routing_ipv6(const struct nf_hook_ops *ops,
>>>  	if (!setup_pre_routing(skb))
>>>  		return NF_DROP;
>>>  
>>> +	store_orig_dstaddr_ipv6(skb);
>>>  	skb->protocol = htons(ETH_P_IPV6);
>>>  	NF_HOOK(NFPROTO_IPV6, NF_INET_PRE_ROUTING, skb, skb->dev, NULL,
>>>  		br_nf_pre_routing_finish_ipv6);
>>> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
>>> index f23fbd2..e328905 100644
>>> --- a/net/ipv6/route.c
>>> +++ b/net/ipv6/route.c
>>> @@ -1017,6 +1017,7 @@ void ip6_route_input(struct sk_buff *skb)
>>>  
>>>  	skb_dst_set(skb, ip6_route_input_lookup(net, skb->dev, &fl6, flags));
>>>  }
>>> +EXPORT_SYMBOL(ip6_route_input);
>>>  
>>>  static struct rt6_info *ip6_pol_route_output(struct net *net, struct fib6_table *table,
>>>  					     struct flowi6 *fl6, int flags)
>>> -- 
>>> 1.7.10.4
>>>

Re: [PATCH 1/1] bridge: Fix NAT66ed IPv6 packets not being bridged correctly

[Pablo Neira Ayuso]

On Thu, Nov 27, 2014 at 08:26:30PM +0100, Bernhard Thaler wrote:
> Hi,
> 
> I tested Sven's patch in my setup and I think it should be safe to use
> it. It is shorter and cleaner written and he submitted it earlier.

Thanks for testing.

> I will be happy to assist you or Sven if any further work is needed.

Please, resubmit this patch to netfilter-devel@vger.kernel.org.
Thanks.