ip_route_me_harder -> xfrm_lookup

ip_route_me_harder -> xfrm_lookup

[Herbert Xu]

[-- Attachment #1: Type: text/plain, Size: 627 bytes --]

Hi:

I've received a number of reports that the any packets that are modified
by the PREROUTING mangle table will not be protected by IPsec.

The reason is that ip_route_me_harder which is called upon the exit
of the mangle table does not set the proto field.  This means that
xfrm_lookup is never called.

The following patch sets the proto field so that the packet can be
protected by IPsec.

Cheers,
-- 
Debian GNU/Linux 3.0 is out! ( http://www.debian.org/ )
Email:  Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[-- Attachment #2: p --]
[-- Type: text/plain, Size: 580 bytes --]

Index: kernel-2.5/net/core/netfilter.c
===================================================================
RCS file: /home/gondolin/herbert/src/CVS/debian/kernel-source-2.5/net/core/netfilter.c,v
retrieving revision 1.1.1.10
diff -u -r1.1.1.10 netfilter.c
--- kernel-2.5/net/core/netfilter.c	8 Oct 2003 19:24:04 -0000	1.1.1.10
+++ kernel-2.5/net/core/netfilter.c	8 Mar 2004 10:52:39 -0000
@@ -639,6 +639,7 @@
 #ifdef CONFIG_IP_ROUTE_FWMARK
 		fl.nl_u.ip4_u.fwmark = (*pskb)->nfmark;
 #endif
+		fl.proto = iph->protocol;
 		if (ip_route_output_key(&rt, &fl) != 0)
 			return -1;
 

Re: ip_route_me_harder -> xfrm_lookup

[Patrick McHardy]

Hi Herbert,

Herbert Xu wrote:
> Hi:
> 
> I've received a number of reports that the any packets that are modified
> by the PREROUTING mangle table will not be protected by IPsec.
> 
> The reason is that ip_route_me_harder which is called upon the exit
> of the mangle table does not set the proto field.  This means that
> xfrm_lookup is never called.
> 
> The following patch sets the proto field so that the packet can be
> protected by IPsec.

I have been working on a set of patches for IPsec+Netfilter, the
latest set has been posted to netfilter-devel last week. They will
go in patch-o-matic for testing soon, but I will post them
to netdev later today, so we won't waste time testing patches
before Dave is fine with them.

Regards,
Patrick

> 
> Cheers,
> 
> 
> ------------------------------------------------------------------------
> 
> Index: kernel-2.5/net/core/netfilter.c
> ===================================================================
> RCS file: /home/gondolin/herbert/src/CVS/debian/kernel-source-2.5/net/core/netfilter.c,v
> retrieving revision 1.1.1.10
> diff -u -r1.1.1.10 netfilter.c
> --- kernel-2.5/net/core/netfilter.c	8 Oct 2003 19:24:04 -0000	1.1.1.10
> +++ kernel-2.5/net/core/netfilter.c	8 Mar 2004 10:52:39 -0000
> @@ -639,6 +639,7 @@
>  #ifdef CONFIG_IP_ROUTE_FWMARK
>  		fl.nl_u.ip4_u.fwmark = (*pskb)->nfmark;
>  #endif
> +		fl.proto = iph->protocol;
>  		if (ip_route_output_key(&rt, &fl) != 0)
>  			return -1;
>  

Re: ip_route_me_harder -> xfrm_lookup

[David S. Miller]

On Mon, 08 Mar 2004 15:46:37 +0100
Patrick McHardy <kaber@trash.net> wrote:

> Herbert Xu wrote:
> > The reason is that ip_route_me_harder which is called upon the exit
> > of the mangle table does not set the proto field.  This means that
> > xfrm_lookup is never called.
> > 
> > The following patch sets the proto field so that the packet can be
> > protected by IPsec.
> 
> I have been working on a set of patches for IPsec+Netfilter, the
> latest set has been posted to netfilter-devel last week. They will
> go in patch-o-matic for testing soon, but I will post them
> to netdev later today, so we won't waste time testing patches
> before Dave is fine with them.

Regardless, and I look forward to your work, Herbert's patch is
absolutely correct so I'm going to apply it for now.

In fact, your work is less likely to be 2.6.4 material I imagine :)
So best to get Herbert's simpler fix in for now.

Re: ip_route_me_harder -> xfrm_lookup

[Patrick McHardy]

David S. Miller wrote:
> On Mon, 08 Mar 2004 15:46:37 +0100
> Patrick McHardy <kaber@trash.net> wrote:
>>
>>I have been working on a set of patches for IPsec+Netfilter, the
>>latest set has been posted to netfilter-devel last week. They will
>>go in patch-o-matic for testing soon, but I will post them
>>to netdev later today, so we won't waste time testing patches
>>before Dave is fine with them.
> 
> 
> Regardless, and I look forward to your work, Herbert's patch is
> absolutely correct so I'm going to apply it for now.
> 
> In fact, your work is less likely to be 2.6.4 material I imagine :)
> So best to get Herbert's simpler fix in for now.
> 

Sorry for the late reply. You're right of course, Herbert's patch is
best for now. The following five mails contain what I've currently
got, it's still rough, but I hope you have some suggestions how to
avoid some ugly things.

Best regards
Patrick

[RFC, PATCH 1/5]: netfilter+ipsec - nf_reset

[Patrick McHardy]

[-- Attachment #1: Type: text/plain, Size: 173 bytes --]

This patch adds a new function nf_reset to reset netfilter
related skb-fields. It has no real relationship to
netfilter+ipsec, but it's required for the follow-up patches.


[-- Attachment #2: 01-nf_reset.diff --]
[-- Type: text/x-patch, Size: 5499 bytes --]

# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
#   2004/03/18 03:22:26+01:00 kaber@trash.net 
#   [NETFILTER]: Add new function 'nf_reset' to reset netfilter related skb-fields
# 
# net/ipv6/sit.c
#   2004/03/18 03:19:58+01:00 kaber@trash.net +2 -14
#   [NETFILTER]: Add new function 'nf_reset' to reset netfilter related skb-fields
# 
# net/ipv6/ip6_tunnel.c
#   2004/03/18 03:19:58+01:00 kaber@trash.net +1 -7
#   [NETFILTER]: Add new function 'nf_reset' to reset netfilter related skb-fields
# 
# net/ipv4/ipip.c
#   2004/03/18 03:19:58+01:00 kaber@trash.net +2 -14
#   [NETFILTER]: Add new function 'nf_reset' to reset netfilter related skb-fields
# 
# net/ipv4/ip_input.c
#   2004/03/18 03:19:58+01:00 kaber@trash.net +1 -5
#   [NETFILTER]: Add new function 'nf_reset' to reset netfilter related skb-fields
# 
# net/ipv4/ip_gre.c
#   2004/03/18 03:19:58+01:00 kaber@trash.net +2 -14
#   [NETFILTER]: Add new function 'nf_reset' to reset netfilter related skb-fields
# 
# include/linux/skbuff.h
#   2004/03/18 03:19:58+01:00 kaber@trash.net +12 -3
#   [NETFILTER]: Add new function 'nf_reset' to reset netfilter related skb-fields
# 
diff -Nru a/include/linux/skbuff.h b/include/linux/skbuff.h
--- a/include/linux/skbuff.h	Thu Mar 18 16:45:10 2004
+++ b/include/linux/skbuff.h	Thu Mar 18 16:45:10 2004
@@ -1201,6 +1201,14 @@
 	if (nfct)
 		atomic_inc(&nfct->master->use);
 }
+static inline void nf_reset(struct sk_buff *skb)
+{
+	nf_conntrack_put(skb->nfct);
+	skb->nfct = NULL;
+#ifdef CONFIG_NETFILTER_DEBUG
+	skb->nf_debug = 0;
+#endif
+}
 
 #ifdef CONFIG_BRIDGE_NETFILTER
 static inline void nf_bridge_put(struct nf_bridge_info *nf_bridge)
@@ -1213,9 +1221,10 @@
 	if (nf_bridge)
 		atomic_inc(&nf_bridge->use);
 }
-#endif
-
-#endif
+#endif /* CONFIG_BRIDGE_NETFILTER */
+#else /* CONFIG_NETFILTER */
+static inline void nf_reset(struct sk_buff *skb) {}
+#endif /* CONFIG_NETFILTER */
 
 #endif	/* __KERNEL__ */
 #endif	/* _LINUX_SKBUFF_H */
diff -Nru a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
--- a/net/ipv4/ip_gre.c	Thu Mar 18 16:45:10 2004
+++ b/net/ipv4/ip_gre.c	Thu Mar 18 16:45:10 2004
@@ -643,13 +643,7 @@
 		skb->dev = tunnel->dev;
 		dst_release(skb->dst);
 		skb->dst = NULL;
-#ifdef CONFIG_NETFILTER
-		nf_conntrack_put(skb->nfct);
-		skb->nfct = NULL;
-#ifdef CONFIG_NETFILTER_DEBUG
-		skb->nf_debug = 0;
-#endif
-#endif
+		nf_reset(skb);
 		ipgre_ecn_decapsulate(iph, skb);
 		netif_rx(skb);
 		read_unlock(&ipgre_lock);
@@ -877,13 +871,7 @@
 		}
 	}
 
-#ifdef CONFIG_NETFILTER
-	nf_conntrack_put(skb->nfct);
-	skb->nfct = NULL;
-#ifdef CONFIG_NETFILTER_DEBUG
-	skb->nf_debug = 0;
-#endif
-#endif
+	nf_reset(skb);
 
 	IPTUNNEL_XMIT();
 	tunnel->recursion--;
diff -Nru a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
--- a/net/ipv4/ip_input.c	Thu Mar 18 16:45:10 2004
+++ b/net/ipv4/ip_input.c	Thu Mar 18 16:45:10 2004
@@ -202,17 +202,13 @@
 
 #ifdef CONFIG_NETFILTER_DEBUG
 	nf_debug_ip_local_deliver(skb);
-	skb->nf_debug = 0;
 #endif /*CONFIG_NETFILTER_DEBUG*/
 
 	__skb_pull(skb, ihl);
 
-#ifdef CONFIG_NETFILTER
 	/* Free reference early: we don't need it any more, and it may
            hold ip_conntrack module loaded indefinitely. */
-	nf_conntrack_put(skb->nfct);
-	skb->nfct = NULL;
-#endif /*CONFIG_NETFILTER*/
+	nf_reset(skb);
 
         /* Point into the IP datagram, just past the header. */
         skb->h.raw = skb->data;
diff -Nru a/net/ipv4/ipip.c b/net/ipv4/ipip.c
--- a/net/ipv4/ipip.c	Thu Mar 18 16:45:10 2004
+++ b/net/ipv4/ipip.c	Thu Mar 18 16:45:10 2004
@@ -496,13 +496,7 @@
 		skb->dev = tunnel->dev;
 		dst_release(skb->dst);
 		skb->dst = NULL;
-#ifdef CONFIG_NETFILTER
-		nf_conntrack_put(skb->nfct);
-		skb->nfct = NULL;
-#ifdef CONFIG_NETFILTER_DEBUG
-		skb->nf_debug = 0;
-#endif
-#endif
+		nf_reset(skb);
 		ipip_ecn_decapsulate(iph, skb);
 		netif_rx(skb);
 		read_unlock(&ipip_lock);
@@ -647,13 +641,7 @@
 	if ((iph->ttl = tiph->ttl) == 0)
 		iph->ttl	=	old_iph->ttl;
 
-#ifdef CONFIG_NETFILTER
-	nf_conntrack_put(skb->nfct);
-	skb->nfct = NULL;
-#ifdef CONFIG_NETFILTER_DEBUG
-	skb->nf_debug = 0;
-#endif
-#endif
+	nf_reset(skb);
 
 	IPTUNNEL_XMIT();
 	tunnel->recursion--;
diff -Nru a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
--- a/net/ipv6/ip6_tunnel.c	Thu Mar 18 16:45:10 2004
+++ b/net/ipv6/ip6_tunnel.c	Thu Mar 18 16:45:10 2004
@@ -715,13 +715,7 @@
 	ipv6h->nexthdr = proto;
 	ipv6_addr_copy(&ipv6h->saddr, &fl.fl6_src);
 	ipv6_addr_copy(&ipv6h->daddr, &fl.fl6_dst);
-#ifdef CONFIG_NETFILTER
-	nf_conntrack_put(skb->nfct);
-	skb->nfct = NULL;
-#ifdef CONFIG_NETFILTER_DEBUG
-	skb->nf_debug = 0;
-#endif
-#endif
+	nf_reset(skb);
 	pkt_len = skb->len;
 	err = NF_HOOK(PF_INET6, NF_IP6_LOCAL_OUT, skb, NULL, 
 		      skb->dst->dev, dst_output);
diff -Nru a/net/ipv6/sit.c b/net/ipv6/sit.c
--- a/net/ipv6/sit.c	Thu Mar 18 16:45:10 2004
+++ b/net/ipv6/sit.c	Thu Mar 18 16:45:10 2004
@@ -388,13 +388,7 @@
 		skb->dev = tunnel->dev;
 		dst_release(skb->dst);
 		skb->dst = NULL;
-#ifdef CONFIG_NETFILTER
-		nf_conntrack_put(skb->nfct);
-		skb->nfct = NULL;
-#ifdef CONFIG_NETFILTER_DEBUG
-		skb->nf_debug = 0;
-#endif
-#endif
+		nf_reset(skb);
 		ipip6_ecn_decapsulate(iph, skb);
 		netif_rx(skb);
 		read_unlock(&ipip6_lock);
@@ -580,13 +574,7 @@
 	if ((iph->ttl = tiph->ttl) == 0)
 		iph->ttl	=	iph6->hop_limit;
 
-#ifdef CONFIG_NETFILTER
-	nf_conntrack_put(skb->nfct);
-	skb->nfct = NULL;
-#ifdef CONFIG_NETFILTER_DEBUG
-	skb->nf_debug = 0;
-#endif
-#endif
+	nf_reset(skb);
 
 	IPTUNNEL_XMIT();
 	tunnel->recursion--;

Re: [RFC, PATCH 1/5]: netfilter+ipsec - nf_reset

[David S. Miller]

On Thu, 18 Mar 2004 17:31:27 +0100
Patrick McHardy <kaber@trash.net> wrote:

> This patch adds a new function nf_reset to reset netfilter
> related skb-fields. It has no real relationship to
> netfilter+ipsec, but it's required for the follow-up patches.

I like and thus want this cleanup regardless of what happens
to the rest of your patches :)

[RFC, PATCH 2/5]: netfilter+ipsec - output hooks

[Patrick McHardy]

[-- Attachment #1: Type: text/plain, Size: 295 bytes --]

This patch adds new output-hooks. Packets with dst->xfrm != NULL
traverse the POST_ROUTING hook before dst_output is called. The
transformers mark the packets in the control buffer with a new flag
IPSKB_XFRM_TRANSFORMED, these packets then traverse the LOCAL_OUT
hook when they hit ip_output.



[-- Attachment #2: 02-output-hooks.diff --]
[-- Type: text/x-patch, Size: 7045 bytes --]

# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
#   2004/03/18 14:59:24+01:00 kaber@trash.net 
#   [NETFILTER}: Pass packets to POST_ROUTING hook before encryption and LOCAL_OUT afterwards
# 
# net/ipv4/xfrm4_tunnel.c
#   2004/03/18 14:59:14+01:00 kaber@trash.net +1 -0
#   [NETFILTER}: Pass packets to POST_ROUTING hook before encryption and LOCAL_OUT afterwards
# 
# net/ipv4/ipcomp.c
#   2004/03/18 14:59:14+01:00 kaber@trash.net +1 -0
#   [NETFILTER}: Pass packets to POST_ROUTING hook before encryption and LOCAL_OUT afterwards
# 
# net/ipv4/ip_output.c
#   2004/03/18 14:59:14+01:00 kaber@trash.net +20 -4
#   [NETFILTER}: Pass packets to POST_ROUTING hook before encryption and LOCAL_OUT afterwards
# 
# net/ipv4/ip_forward.c
#   2004/03/18 14:59:14+01:00 kaber@trash.net +2 -1
#   [NETFILTER}: Pass packets to POST_ROUTING hook before encryption and LOCAL_OUT afterwards
# 
# net/ipv4/esp4.c
#   2004/03/18 14:59:14+01:00 kaber@trash.net +1 -0
#   [NETFILTER}: Pass packets to POST_ROUTING hook before encryption and LOCAL_OUT afterwards
# 
# net/ipv4/ah4.c
#   2004/03/18 14:59:14+01:00 kaber@trash.net +1 -0
#   [NETFILTER}: Pass packets to POST_ROUTING hook before encryption and LOCAL_OUT afterwards
# 
# include/net/ip.h
#   2004/03/18 14:59:14+01:00 kaber@trash.net +1 -0
#   [NETFILTER}: Pass packets to POST_ROUTING hook before encryption and LOCAL_OUT afterwards
# 
# include/linux/netfilter.h
#   2004/03/18 14:59:14+01:00 kaber@trash.net +9 -4
#   [NETFILTER}: Pass packets to POST_ROUTING hook before encryption and LOCAL_OUT afterwards
# 
diff -Nru a/include/linux/netfilter.h b/include/linux/netfilter.h
--- a/include/linux/netfilter.h	Thu Mar 18 16:45:22 2004
+++ b/include/linux/netfilter.h	Thu Mar 18 16:45:22 2004
@@ -119,12 +119,14 @@
 /* This is gross, but inline doesn't cut it for avoiding the function
    call in fast path: gcc doesn't inline (needs value tracking?). --RR */
 #ifdef CONFIG_NETFILTER_DEBUG
-#define NF_HOOK(pf, hook, skb, indev, outdev, okfn)			\
- nf_hook_slow((pf), (hook), (skb), (indev), (outdev), (okfn), INT_MIN)
+#define NF_HOOK_COND(pf, hook, skb, indev, outdev, okfn, cond)		\
+(!(cond)									\
+ ? (okfn)(skb) 								\
+ : nf_hook_slow((pf), (hook), (skb), (indev), (outdev), (okfn), INT_MIN))
 #define NF_HOOK_THRESH nf_hook_slow
 #else
-#define NF_HOOK(pf, hook, skb, indev, outdev, okfn)			\
-(list_empty(&nf_hooks[(pf)][(hook)])					\
+#define NF_HOOK_COND(pf, hook, skb, indev, outdev, okfn, cond)		\
+(!(cond) || list_empty(&nf_hooks[(pf)][(hook)])				\
  ? (okfn)(skb)								\
  : nf_hook_slow((pf), (hook), (skb), (indev), (outdev), (okfn), INT_MIN))
 #define NF_HOOK_THRESH(pf, hook, skb, indev, outdev, okfn, thresh)	\
@@ -132,6 +134,8 @@
  ? (okfn)(skb)								\
  : nf_hook_slow((pf), (hook), (skb), (indev), (outdev), (okfn), (thresh)))
 #endif
+#define NF_HOOK(pf, hook, skb, indev, outdev, okfn)			\
+ NF_HOOK_COND((pf), (hook), (skb), (indev), (outdev), (okfn), 1)
 
 int nf_hook_slow(int pf, unsigned int hook, struct sk_buff *skb,
 		 struct net_device *indev, struct net_device *outdev,
@@ -164,6 +168,7 @@
 
 #else /* !CONFIG_NETFILTER */
 #define NF_HOOK(pf, hook, skb, indev, outdev, okfn) (okfn)(skb)
+#define NF_HOOK_COND NF_HOOK
 #endif /*CONFIG_NETFILTER*/
 
 #endif /*__KERNEL__*/
diff -Nru a/include/net/ip.h b/include/net/ip.h
--- a/include/net/ip.h	Thu Mar 18 16:45:22 2004
+++ b/include/net/ip.h	Thu Mar 18 16:45:22 2004
@@ -48,6 +48,7 @@
 #define IPSKB_TRANSLATED	2
 #define IPSKB_FORWARDED		4
 #define IPSKB_XFRM_TUNNEL_SIZE	8
+#define IPSKB_XFRM_TRANSFORMED	16
 };
 
 struct ipcm_cookie
diff -Nru a/net/ipv4/ah4.c b/net/ipv4/ah4.c
--- a/net/ipv4/ah4.c	Thu Mar 18 16:45:22 2004
+++ b/net/ipv4/ah4.c	Thu Mar 18 16:45:22 2004
@@ -145,6 +145,7 @@
 		err = -EHOSTUNREACH;
 		goto error_nolock;
 	}
+	IPCB(skb)->flags |= IPSKB_XFRM_TRANSFORMED;
 	return NET_XMIT_BYPASS;
 
 error:
diff -Nru a/net/ipv4/esp4.c b/net/ipv4/esp4.c
--- a/net/ipv4/esp4.c	Thu Mar 18 16:45:22 2004
+++ b/net/ipv4/esp4.c	Thu Mar 18 16:45:22 2004
@@ -199,6 +199,7 @@
 		err = -EHOSTUNREACH;
 		goto error_nolock;
 	}
+	IPCB(skb)->flags |= IPSKB_XFRM_TRANSFORMED;
 	return NET_XMIT_BYPASS;
 
 error:
diff -Nru a/net/ipv4/ip_forward.c b/net/ipv4/ip_forward.c
--- a/net/ipv4/ip_forward.c	Thu Mar 18 16:45:22 2004
+++ b/net/ipv4/ip_forward.c	Thu Mar 18 16:45:22 2004
@@ -51,7 +51,8 @@
 	if (unlikely(opt->optlen))
 		ip_forward_options(skb);
 
-	return dst_output(skb);
+	return NF_HOOK_COND(PF_INET, NF_IP_POST_ROUTING, skb, NULL,
+	                    skb->dst->dev, dst_output, skb->dst->xfrm != NULL);
 }
 
 int ip_forward(struct sk_buff *skb)
diff -Nru a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
--- a/net/ipv4/ip_output.c	Thu Mar 18 16:45:22 2004
+++ b/net/ipv4/ip_output.c	Thu Mar 18 16:45:22 2004
@@ -123,6 +123,12 @@
 	return ttl;
 }
 
+static inline int ip_dst_output(struct sk_buff *skb)
+{
+	return NF_HOOK_COND(PF_INET, NF_IP_POST_ROUTING, skb, NULL,
+	                    skb->dst->dev, dst_output, skb->dst->xfrm != NULL);
+}
+
 /* 
  *		Add an ip header to a skbuff and send it out.
  *
@@ -165,7 +171,7 @@
 
 	/* Send it out. */
 	return NF_HOOK(PF_INET, NF_IP_LOCAL_OUT, skb, NULL, rt->u.dst.dev,
-		       dst_output);
+		       ip_dst_output);
 }
 
 static inline int ip_finish_output2(struct sk_buff *skb)
@@ -283,7 +289,7 @@
 		return ip_finish_output(skb);
 }
 
-int ip_output(struct sk_buff *skb)
+static inline int ip_output2(struct sk_buff *skb)
 {
 	IP_INC_STATS(IpOutRequests);
 
@@ -294,6 +300,16 @@
 		return ip_finish_output(skb);
 }
 
+int ip_output(struct sk_buff *skb)
+{
+	int transformed = IPCB(skb)->flags & IPSKB_XFRM_TRANSFORMED;
+
+	if (transformed)
+		nf_reset(skb);
+	return NF_HOOK_COND(PF_INET, NF_IP_LOCAL_OUT, skb, NULL,
+	                    skb->dst->dev, ip_output2, transformed);
+}
+
 int ip_queue_xmit(struct sk_buff *skb, int ipfragok)
 {
 	struct sock *sk = skb->sk;
@@ -387,7 +403,7 @@
 	skb->priority = sk->sk_priority;
 
 	return NF_HOOK(PF_INET, NF_IP_LOCAL_OUT, skb, NULL, rt->u.dst.dev,
-		       dst_output);
+		       ip_dst_output);
 
 no_route:
 	IP_INC_STATS(IpOutNoRoutes);
@@ -1177,7 +1193,7 @@
 
 	/* Netfilter gets whole the not fragmented skb. */
 	err = NF_HOOK(PF_INET, NF_IP_LOCAL_OUT, skb, NULL, 
-		      skb->dst->dev, dst_output);
+		      skb->dst->dev, ip_dst_output);
 	if (err) {
 		if (err > 0)
 			err = inet->recverr ? net_xmit_errno(err) : 0;
diff -Nru a/net/ipv4/ipcomp.c b/net/ipv4/ipcomp.c
--- a/net/ipv4/ipcomp.c	Thu Mar 18 16:45:22 2004
+++ b/net/ipv4/ipcomp.c	Thu Mar 18 16:45:22 2004
@@ -231,6 +231,7 @@
 		err = -EHOSTUNREACH;
 		goto error_nolock;
 	}
+	IPCB(skb)->flags |= IPSKB_XFRM_TRANSFORMED;
 	err = NET_XMIT_BYPASS;
 
 out_exit:
diff -Nru a/net/ipv4/xfrm4_tunnel.c b/net/ipv4/xfrm4_tunnel.c
--- a/net/ipv4/xfrm4_tunnel.c	Thu Mar 18 16:45:22 2004
+++ b/net/ipv4/xfrm4_tunnel.c	Thu Mar 18 16:45:22 2004
@@ -76,6 +76,7 @@
 		err = -EHOSTUNREACH;
 		goto error_nolock;
 	}
+	IPCB(skb)->flags |= IPSKB_XFRM_TRANSFORMED;
 	return NET_XMIT_BYPASS;
 
 error_nolock:

Re: [RFC, PATCH 2/5]: netfilter+ipsec - output hooks

[David S. Miller]

This one isn't going to compile with netfilter disabled, I can tell
just by looking at it :-)

The reason is that NF_HOOK_COND is defined to NF_HOOK in that case,
and these two macros take a different number of args so the compiler
is going to barf.

Re: [RFC, PATCH 2/5]: netfilter+ipsec - output hooks

[Herbert Xu]

On Thu, Mar 18, 2004 at 05:31:40PM +0100, Patrick McHardy wrote:
> This patch adds new output-hooks. Packets with dst->xfrm != NULL
> traverse the POST_ROUTING hook before dst_output is called. The
> transformers mark the packets in the control buffer with a new flag
> IPSKB_XFRM_TRANSFORMED, these packets then traverse the LOCAL_OUT
> hook when they hit ip_output.

Thank you very much for your patches.  This is definitely the biggest
show stopper with the current IPsec stack.

I've just got a minor point about this one:

> @@ -119,12 +119,14 @@
>  /* This is gross, but inline doesn't cut it for avoiding the function
>     call in fast path: gcc doesn't inline (needs value tracking?). --RR */
>  #ifdef CONFIG_NETFILTER_DEBUG
> -#define NF_HOOK(pf, hook, skb, indev, outdev, okfn)			\
> - nf_hook_slow((pf), (hook), (skb), (indev), (outdev), (okfn), INT_MIN)
> +#define NF_HOOK_COND(pf, hook, skb, indev, outdev, okfn, cond)		\
> +(!(cond)									\
> + ? (okfn)(skb) 								\
> + : nf_hook_slow((pf), (hook), (skb), (indev), (outdev), (okfn), INT_MIN))

Any reason why this is written with a negated cond? I get confused by
double negations :)

>  #define NF_HOOK_THRESH nf_hook_slow
>  #else
> -#define NF_HOOK(pf, hook, skb, indev, outdev, okfn)			\
> -(list_empty(&nf_hooks[(pf)][(hook)])					\
> +#define NF_HOOK_COND(pf, hook, skb, indev, outdev, okfn, cond)		\
> +(!(cond) || list_empty(&nf_hooks[(pf)][(hook)])				\

Ditto, what about

((cond) && !list_empty(&nf_hooks[(pf)][(hook)) \
 ? nf_hook_slow((pf), (hook), (skb), (indev), (outdev), (okfn), INT_MIN) \
 : (okfn)(skb))
-- 
Debian GNU/Linux 3.0 is out! ( http://www.debian.org/ )
Email:  Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[RFC, PATCH 3/5]: netfilter+ipsec - input hooks

[Patrick McHardy]

[-- Attachment #1: Type: text/plain, Size: 1151 bytes --]

This patch adds new hooks for input, it is the worst of all patches.
Packets should traverse PRE_ROUTING+LOCAL_IN before decryption and
PRE_ROUTING+LOCAL_IN/FORWARD afterwards. This is tricky because I
didn't find an elegant way to determine when decapsulation is done.
Currently, packets reposted into the stack don't traverse the hooks
at the normal positions but at a later time when we know decapsulation
if done:

- if a reposted packet has a non-local destination after input routing
decapsulation must be done, it then traverses the PRE_ROUTING hook.

- otherwise, it continues until ip_local_deliver_finish but also
skips the LOCAL_IN hook. All xfrm protocol-handlers are marked with
xfrm_prot. If the protocol handler of a packet with a secpath
pointer is a non-xfrm-protocol the packet was handled by ipsec and
is done now, it traverses the PRE_ROUTING and LOCAL_IN hooks then.
This catches packets from both tunnel-mode and transport-mode SAs.

Because the hooks are traversed at a later time than usual several
not-so-nice things have to be done in the nf_postxfrm_* functions.

I don't see a better way currently, but I'm sure you do ;)


[-- Attachment #2: 03-input-hooks.diff --]
[-- Type: text/x-patch, Size: 7170 bytes --]

# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
#   2004/03/18 16:41:37+01:00 kaber@trash.net 
#   [NETFILTER]: post-ipsec input hooks
# 
# net/ipv4/xfrm4_tunnel.c
#   2004/03/18 16:41:31+01:00 kaber@trash.net +1 -0
#   [NETFILTER]: post-ipsec input hooks
# 
# net/ipv4/ipcomp.c
#   2004/03/18 16:41:31+01:00 kaber@trash.net +1 -0
#   [NETFILTER]: post-ipsec input hooks
# 
# net/ipv4/ip_input.c
#   2004/03/18 16:41:31+01:00 kaber@trash.net +15 -6
#   [NETFILTER]: post-ipsec input hooks
# 
# net/ipv4/esp4.c
#   2004/03/18 16:41:31+01:00 kaber@trash.net +1 -0
#   [NETFILTER]: post-ipsec input hooks
# 
# net/ipv4/ah4.c
#   2004/03/18 16:41:31+01:00 kaber@trash.net +1 -0
#   [NETFILTER]: post-ipsec input hooks
# 
# net/core/netfilter.c
#   2004/03/18 16:41:31+01:00 kaber@trash.net +50 -3
#   [NETFILTER]: post-ipsec input hooks
# 
# include/net/protocol.h
#   2004/03/18 16:41:31+01:00 kaber@trash.net +1 -0
#   [NETFILTER]: post-ipsec input hooks
# 
# include/linux/netfilter_ipv4.h
#   2004/03/18 16:41:31+01:00 kaber@trash.net +15 -0
#   [NETFILTER]: post-ipsec input hooks
# 
diff -Nru a/include/linux/netfilter_ipv4.h b/include/linux/netfilter_ipv4.h
--- a/include/linux/netfilter_ipv4.h	Thu Mar 18 16:45:35 2004
+++ b/include/linux/netfilter_ipv4.h	Thu Mar 18 16:45:35 2004
@@ -83,6 +83,21 @@
    Returns true or false. */
 extern int skb_ip_make_writable(struct sk_buff **pskb,
 				unsigned int writable_len);
+
+#ifdef CONFIG_XFRM
+extern int nf_postxfrm_input(struct sk_buff *skb);
+extern int nf_postxfrm_nonlocal(struct sk_buff *skb);
+#else /* CONFIG_XFRM */
+static inline int nf_postxfrm_input(struct sk_buff *skb)
+{
+	return 0;
+}
+
+static inline int nf_postxfrm_nonlocal(struct sk_buff *skb)
+{
+	return 0;
+}
+#endif /* CONFIG_XFRM */
 #endif /*__KERNEL__*/
 
 #endif /*__LINUX_IP_NETFILTER_H*/
diff -Nru a/include/net/protocol.h b/include/net/protocol.h
--- a/include/net/protocol.h	Thu Mar 18 16:45:35 2004
+++ b/include/net/protocol.h	Thu Mar 18 16:45:35 2004
@@ -39,6 +39,7 @@
 	int			(*handler)(struct sk_buff *skb);
 	void			(*err_handler)(struct sk_buff *skb, u32 info);
 	int			no_policy;
+	int			xfrm_prot;
 };
 
 #if defined(CONFIG_IPV6) || defined (CONFIG_IPV6_MODULE)
diff -Nru a/net/core/netfilter.c b/net/core/netfilter.c
--- a/net/core/netfilter.c	Thu Mar 18 16:45:35 2004
+++ b/net/core/netfilter.c	Thu Mar 18 16:45:35 2004
@@ -11,6 +11,7 @@
  */
 #include <linux/config.h>
 #include <linux/netfilter.h>
+#include <linux/netfilter_ipv4.h>
 #include <net/protocol.h>
 #include <linux/init.h>
 #include <linux/skbuff.h>
@@ -25,6 +26,7 @@
 #include <linux/icmp.h>
 #include <net/sock.h>
 #include <net/route.h>
+#include <net/xfrm.h>
 #include <linux/ip.h>
 
 /* In this code, we can be waiting indefinitely for userspace to
@@ -625,9 +627,6 @@
 	struct dst_entry *odst;
 	unsigned int hh_len;
 
-	/* some non-standard hacks like ipt_REJECT.c:send_reset() can cause
-	 * packets with foreign saddr to appear on the NF_IP_LOCAL_OUT hook.
-	 */
 	if (inet_addr_type(iph->saddr) == RTN_LOCAL) {
 		fl.nl_u.ip4_u.daddr = iph->daddr;
 		fl.nl_u.ip4_u.saddr = iph->saddr;
@@ -679,6 +678,54 @@
 
 	return 0;
 }
+
+#ifdef CONFIG_XFRM
+static inline int nf_postxfrm_done(struct sk_buff *skb)
+{
+	return 0;
+}
+
+static inline int nf_postxfrm_input2(struct sk_buff *skb)
+{
+	if (inet_addr_type(skb->nh.iph->daddr) != RTN_LOCAL) {
+		if (ip_route_me_harder(&skb) == 0)
+			dst_input(skb);
+		else
+			kfree_skb(skb);
+		return -1;
+	}
+
+	return NF_HOOK(PF_INET, NF_IP_LOCAL_IN, skb, skb->dev, NULL,
+	               nf_postxfrm_done);
+}
+
+int nf_postxfrm_input(struct sk_buff *skb)
+{
+	int off = skb->data - skb->nh.raw;
+
+	__skb_push(skb, off);
+	/* Fix header len and checksum if last xfrm was transport mode */
+	if (!skb->sp->x[skb->sp->len - 1].xvec->props.mode) {
+		skb->nh.iph->tot_len = htons(skb->len);
+		ip_send_check(skb->nh.iph);
+	}
+
+	nf_reset(skb);
+	if (NF_HOOK(PF_INET, NF_IP_PRE_ROUTING, skb, skb->dev, NULL,
+	            nf_postxfrm_input2) != 0)
+		return -1;
+
+	__skb_pull(skb, off);
+	return 0;
+}
+
+int nf_postxfrm_nonlocal(struct sk_buff *skb)
+{
+	nf_reset(skb);
+	return NF_HOOK(PF_INET, NF_IP_PRE_ROUTING, skb, skb->dev, NULL,
+	               nf_postxfrm_done);
+}
+#endif /* CONFIG_XFRM */
 
 int skb_ip_make_writable(struct sk_buff **pskb, unsigned int writable_len)
 {
diff -Nru a/net/ipv4/ah4.c b/net/ipv4/ah4.c
--- a/net/ipv4/ah4.c	Thu Mar 18 16:45:35 2004
+++ b/net/ipv4/ah4.c	Thu Mar 18 16:45:35 2004
@@ -344,6 +344,7 @@
 	.handler	=	xfrm4_rcv,
 	.err_handler	=	ah4_err,
 	.no_policy	=	1,
+	.xfrm_prot	=	1,
 };
 
 static int __init ah4_init(void)
diff -Nru a/net/ipv4/esp4.c b/net/ipv4/esp4.c
--- a/net/ipv4/esp4.c	Thu Mar 18 16:45:35 2004
+++ b/net/ipv4/esp4.c	Thu Mar 18 16:45:35 2004
@@ -577,6 +577,7 @@
 	.handler	=	xfrm4_rcv,
 	.err_handler	=	esp4_err,
 	.no_policy	=	1,
+	.xfrm_prot	=	1,
 };
 
 static int __init esp4_init(void)
diff -Nru a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
--- a/net/ipv4/ip_input.c	Thu Mar 18 16:45:35 2004
+++ b/net/ipv4/ip_input.c	Thu Mar 18 16:45:35 2004
@@ -224,6 +224,12 @@
 	resubmit:
 		hash = protocol & (MAX_INET_PROTOS - 1);
 		raw_sk = sk_head(&raw_v4_htable[hash]);
+		ipprot = inet_protos[hash];
+		smp_read_barrier_depends();
+
+		if (skb->sp && !ipprot->xfrm_prot)
+			if (nf_postxfrm_input(skb))
+				goto out;
 
 		/* If there maybe a raw socket we must check - if not we
 		 * don't care less
@@ -231,10 +237,9 @@
 		if (raw_sk)
 			raw_v4_input(skb, skb->nh.iph, hash);
 
-		if ((ipprot = inet_protos[hash]) != NULL) {
+		if (ipprot != NULL) {
 			int ret;
 
-			smp_read_barrier_depends();
 			if (!ipprot->no_policy &&
 			    !xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb)) {
 				kfree_skb(skb);
@@ -279,8 +284,8 @@
 			return 0;
 	}
 
-	return NF_HOOK(PF_INET, NF_IP_LOCAL_IN, skb, skb->dev, NULL,
-		       ip_local_deliver_finish);
+	return NF_HOOK_COND(PF_INET, NF_IP_LOCAL_IN, skb, skb->dev, NULL,
+	                    ip_local_deliver_finish, !skb->sp);
 }
 
 static inline int ip_rcv_finish(struct sk_buff *skb)
@@ -346,6 +351,10 @@
 		}
 	}
 
+	if (skb->sp && !(((struct rtable *)skb->dst)->rt_flags&RTCF_LOCAL))
+		if (nf_postxfrm_nonlocal(skb))
+			goto drop;
+
 	return dst_input(skb);
 
 inhdr_error:
@@ -418,8 +427,8 @@
 		}
 	}
 
-	return NF_HOOK(PF_INET, NF_IP_PRE_ROUTING, skb, dev, NULL,
-		       ip_rcv_finish);
+	return NF_HOOK_COND(PF_INET, NF_IP_PRE_ROUTING, skb, dev, NULL,
+	                    ip_rcv_finish, !skb->sp);
 
 inhdr_error:
 	IP_INC_STATS_BH(IpInHdrErrors);
diff -Nru a/net/ipv4/ipcomp.c b/net/ipv4/ipcomp.c
--- a/net/ipv4/ipcomp.c	Thu Mar 18 16:45:35 2004
+++ b/net/ipv4/ipcomp.c	Thu Mar 18 16:45:35 2004
@@ -408,6 +408,7 @@
 	.handler	=	xfrm4_rcv,
 	.err_handler	=	ipcomp4_err,
 	.no_policy	=	1,
+	.xfrm_prot	=	1,
 };
 
 static int __init ipcomp4_init(void)
diff -Nru a/net/ipv4/xfrm4_tunnel.c b/net/ipv4/xfrm4_tunnel.c
--- a/net/ipv4/xfrm4_tunnel.c	Thu Mar 18 16:45:35 2004
+++ b/net/ipv4/xfrm4_tunnel.c	Thu Mar 18 16:45:35 2004
@@ -171,6 +171,7 @@
 	.handler	=	ipip_rcv,
 	.err_handler	=	ipip_err,
 	.no_policy	=	1,
+	.xfrm_prot	=	1,
 };
 
 static int __init ipip_init(void)

Re: [RFC, PATCH 3/5]: netfilter+ipsec - input hooks

[David S. Miller]

On Thu, 18 Mar 2004 17:32:14 +0100
Patrick McHardy <kaber@trash.net> wrote:

> If the protocol handler of a packet with a secpath
> pointer is a non-xfrm-protocol the packet was handled by ipsec and
> is done now, it traverses the PRE_ROUTING and LOCAL_IN hooks then.
> This catches packets from both tunnel-mode and transport-mode SAs.

Be careful!  xfrm4_tunnel handles both uncompressed ipcomp packets
_and_ IPIP encapsulator device packets.  Yet you will intepret usage
of the ipprot as 'xfrm_prot==1' in all cases.

Yes this is ugly... if we added some kind of flag bit-mask to sk_buff,
would that allow an easier implementation?

Re: [RFC, PATCH 3/5]: netfilter+ipsec - input hooks

[Herbert Xu]

On Thu, Mar 18, 2004 at 10:15:23PM -0800, David S. Miller wrote:
> 
> Be careful!  xfrm4_tunnel handles both uncompressed ipcomp packets
> _and_ IPIP encapsulator device packets.  Yet you will intepret usage
> of the ipprot as 'xfrm_prot==1' in all cases.

Good point.

> Yes this is ugly... if we added some kind of flag bit-mask to sk_buff,
> would that allow an easier implementation?

I'm not sure if this'll help in the degenerate IPCOMP case.  Perhaps
we need a way to tell if it is a degenerate IPCOMP tunnel or an IPIP
tunnel without actually processing the packet.
-- 
Debian GNU/Linux 3.0 is out! ( http://www.debian.org/ )
Email:  Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [RFC, PATCH 3/5]: netfilter+ipsec - input hooks

[Patrick McHardy]

David S. Miller wrote:
> 
> Be careful!  xfrm4_tunnel handles both uncompressed ipcomp packets
> _and_ IPIP encapsulator device packets.  Yet you will intepret usage
> of the ipprot as 'xfrm_prot==1' in all cases.
> 
> Yes this is ugly... if we added some kind of flag bit-mask to sk_buff,
> would that allow an easier implementation?
> 

I can't imagine how. Best would be to avoid the xfrm_prot flag
completely. Maybe we can add a flag to xfrm_state which indicates
that this is the last xfrm specified in the policy ?

Re: [RFC, PATCH 3/5]: netfilter+ipsec - input hooks

[Herbert Xu]

On Fri, Mar 19, 2004 at 05:17:30PM +0100, Patrick McHardy wrote:
> 
> I can't imagine how. Best would be to avoid the xfrm_prot flag
> completely. Maybe we can add a flag to xfrm_state which indicates
> that this is the last xfrm specified in the policy ?

The same state can be terminal in one policy but not another.
-- 
Debian GNU/Linux 3.0 is out! ( http://www.debian.org/ )
Email:  Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [RFC, PATCH 3/5]: netfilter+ipsec - input hooks

[Herbert Xu]

On Thu, Mar 18, 2004 at 05:32:14PM +0100, Patrick McHardy wrote:
>
> diff -Nru a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
> --- a/net/ipv4/ip_input.c	Thu Mar 18 16:45:35 2004
> +++ b/net/ipv4/ip_input.c	Thu Mar 18 16:45:35 2004
> @@ -224,6 +224,12 @@
>  	resubmit:
>  		hash = protocol & (MAX_INET_PROTOS - 1);
>  		raw_sk = sk_head(&raw_v4_htable[hash]);
> +		ipprot = inet_protos[hash];
> +		smp_read_barrier_depends();
> +
> +		if (skb->sp && !ipprot->xfrm_prot)
> +			if (nf_postxfrm_input(skb))
> +				goto out;

Need to check ipprot != NULL here.
-- 
Debian GNU/Linux 3.0 is out! ( http://www.debian.org/ )
Email:  Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [RFC, PATCH 3/5]: netfilter+ipsec - input hooks

[Herbert Xu]

On Thu, Mar 18, 2004 at 05:32:14PM +0100, Patrick McHardy wrote:
>
> diff -Nru a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
> --- a/net/ipv4/ip_input.c	Thu Mar 18 16:45:35 2004
> +++ b/net/ipv4/ip_input.c	Thu Mar 18 16:45:35 2004
> @@ -224,6 +224,12 @@
>  	resubmit:
>  		hash = protocol & (MAX_INET_PROTOS - 1);
>  		raw_sk = sk_head(&raw_v4_htable[hash]);
> +		ipprot = inet_protos[hash];
> +		smp_read_barrier_depends();
> +
> +		if (skb->sp && !ipprot->xfrm_prot)
> +			if (nf_postxfrm_input(skb))
> +				goto out;

Just an idea: what if we reinject the packet just as we do in tunnel mode?

> @@ -346,6 +351,10 @@
>  		}
>  	}
>  
> +	if (skb->sp && !(((struct rtable *)skb->dst)->rt_flags&RTCF_LOCAL))
> +		if (nf_postxfrm_nonlocal(skb))
> +			goto drop;
> +

What if the PRE_ROUTING turns it into a local address again?
-- 
Debian GNU/Linux 3.0 is out! ( http://www.debian.org/ )
Email:  Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [RFC, PATCH 3/5]: netfilter+ipsec - input hooks

[Patrick McHardy]

Herbert Xu wrote:
> On Thu, Mar 18, 2004 at 05:32:14PM +0100, Patrick McHardy wrote:
> 
>>diff -Nru a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
>>--- a/net/ipv4/ip_input.c	Thu Mar 18 16:45:35 2004
>>+++ b/net/ipv4/ip_input.c	Thu Mar 18 16:45:35 2004
>>@@ -224,6 +224,12 @@
>> 	resubmit:
>> 		hash = protocol & (MAX_INET_PROTOS - 1);
>> 		raw_sk = sk_head(&raw_v4_htable[hash]);
>>+		ipprot = inet_protos[hash];
>>+		smp_read_barrier_depends();
>>+
>>+		if (skb->sp && !ipprot->xfrm_prot)
>>+			if (nf_postxfrm_input(skb))
>>+				goto out;
> 
> 
> Just an idea: what if we reinject the packet just as we do in tunnel mode?

If we also have a possibility to determine when decapsulation is done
we could avoid the nf_postxfrm_* hacks. Without that, I don't think it
helps.

> 
> 
>>@@ -346,6 +351,10 @@
>> 		}
>> 	}
>> 
>>+	if (skb->sp && !(((struct rtable *)skb->dst)->rt_flags&RTCF_LOCAL))
>>+		if (nf_postxfrm_nonlocal(skb))
>>+			goto drop;
>>+
> 
> 
> What if the PRE_ROUTING turns it into a local address again?

You have good eyes ;) The packet needs to be rerouted in
nf_postxfrm_nonlocal and needs to be special-cased in nf_postxfrm_input
so it doesn't traverse PRE_ROUTING twice.

Thanks for your other suggestions as well, I'm going to make these
changes.

Regards
Patrick

[RFC, PATCH 4/5]: netfilter+ipsec - policy lookup

[Patrick McHardy]

[-- Attachment #1: Type: text/plain, Size: 126 bytes --]

This patch adds policy lookups to ip_route_me_harder and makes NAT
reroute for any change that affects route/policy lookups.


[-- Attachment #2: 04-nat-xfrm_lookup.diff --]
[-- Type: text/x-patch, Size: 5617 bytes --]

# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
#   2004/03/18 16:43:22+01:00 kaber@trash.net 
#   [NETFILTER]: Add policy lookups to ip_route_me_harder, make NAT reroute for any change in route/policy key
# 
# net/ipv4/netfilter/ip_nat_standalone.c
#   2004/03/18 16:43:16+01:00 kaber@trash.net +72 -8
#   [NETFILTER]: Add policy lookups to ip_route_me_harder, make NAT reroute for any change in route/policy key
# 
# net/ipv4/netfilter/ip_conntrack_standalone.c
#   2004/03/18 16:43:16+01:00 kaber@trash.net +1 -0
#   [NETFILTER]: Add policy lookups to ip_route_me_harder, make NAT reroute for any change in route/policy key
# 
# net/core/netfilter.c
#   2004/03/18 16:43:16+01:00 kaber@trash.net +15 -1
#   [NETFILTER]: Add policy lookups to ip_route_me_harder, make NAT reroute for any change in route/policy key
# 
diff -Nru a/net/core/netfilter.c b/net/core/netfilter.c
--- a/net/core/netfilter.c	Thu Mar 18 16:46:02 2004
+++ b/net/core/netfilter.c	Thu Mar 18 16:46:02 2004
@@ -27,6 +27,7 @@
 #include <net/sock.h>
 #include <net/route.h>
 #include <net/xfrm.h>
+#include <net/ip.h>
 #include <linux/ip.h>
 
 /* In this code, we can be waiting indefinitely for userspace to
@@ -635,7 +636,6 @@
 #ifdef CONFIG_IP_ROUTE_FWMARK
 		fl.nl_u.ip4_u.fwmark = (*pskb)->nfmark;
 #endif
-		fl.proto = iph->protocol;
 		if (ip_route_output_key(&rt, &fl) != 0)
 			return -1;
 
@@ -661,6 +661,20 @@
 	
 	if ((*pskb)->dst->error)
 		return -1;
+
+#ifdef CONFIG_XFRM
+	if (!(IPCB(*pskb)->flags & IPSKB_XFRM_TRANSFORMED)) {
+		struct xfrm_policy_afinfo *afinfo;
+
+		afinfo = xfrm_policy_get_afinfo(AF_INET);
+		if (afinfo != NULL) {
+			afinfo->decode_session(*pskb, &fl);
+			xfrm_policy_put_afinfo(afinfo);
+			if (xfrm_lookup(&(*pskb)->dst, &fl, (*pskb)->sk, 0) != 0)
+				return -1;
+		}
+	}
+#endif
 
 	/* Change in oif may mean change in hh_len. */
 	hh_len = (*pskb)->dst->dev->hard_header_len;
diff -Nru a/net/ipv4/netfilter/ip_conntrack_standalone.c b/net/ipv4/netfilter/ip_conntrack_standalone.c
--- a/net/ipv4/netfilter/ip_conntrack_standalone.c	Thu Mar 18 16:46:02 2004
+++ b/net/ipv4/netfilter/ip_conntrack_standalone.c	Thu Mar 18 16:46:02 2004
@@ -583,6 +583,7 @@
 EXPORT_SYMBOL(ip_conntrack_alter_reply);
 EXPORT_SYMBOL(ip_conntrack_destroyed);
 EXPORT_SYMBOL(ip_conntrack_get);
+EXPORT_SYMBOL(__ip_conntrack_confirm);
 EXPORT_SYMBOL(need_ip_conntrack);
 EXPORT_SYMBOL(ip_conntrack_helper_register);
 EXPORT_SYMBOL(ip_conntrack_helper_unregister);
diff -Nru a/net/ipv4/netfilter/ip_nat_standalone.c b/net/ipv4/netfilter/ip_nat_standalone.c
--- a/net/ipv4/netfilter/ip_nat_standalone.c	Thu Mar 18 16:46:02 2004
+++ b/net/ipv4/netfilter/ip_nat_standalone.c	Thu Mar 18 16:46:02 2004
@@ -166,6 +166,45 @@
 	return do_bindings(ct, ctinfo, info, hooknum, pskb);
 }
 
+struct nat_route_key
+{
+	u_int32_t addr;
+#ifdef CONFIG_XFRM
+	u_int16_t port;
+#endif
+};
+
+static inline void
+nat_route_key_get(struct sk_buff *skb, struct nat_route_key *key, int which)
+{
+	struct iphdr *iph = skb->nh.iph;
+
+	key->addr = which ? iph->daddr : iph->saddr;
+#ifdef CONFIG_XFRM
+	key->port = 0;
+	if (iph->protocol == IPPROTO_TCP || iph->protocol == IPPROTO_UDP) {
+		u_int16_t *ports = (u_int16_t *)(skb->nh.raw + iph->ihl*4);
+		key->port = ports[which];
+	}
+#endif
+}
+
+static inline int
+nat_route_key_compare(struct sk_buff *skb, struct nat_route_key *key, int which)
+{
+	struct iphdr *iph = skb->nh.iph;
+
+	if (key->addr != (which ? iph->daddr : iph->saddr))
+		return 1;
+#ifdef CONFIG_XFRM
+	if (iph->protocol == IPPROTO_TCP || iph->protocol == IPPROTO_UDP) {
+		u_int16_t *ports = (u_int16_t *)(skb->nh.raw + iph->ihl*4);
+		if (key->port != ports[which])
+			return 1;
+	}
+#endif
+}
+
 static unsigned int
 ip_nat_out(unsigned int hooknum,
 	   struct sk_buff **pskb,
@@ -173,6 +212,9 @@
 	   const struct net_device *out,
 	   int (*okfn)(struct sk_buff *))
 {
+	struct nat_route_key key;
+	unsigned int ret;
+
 	/* root is playing with raw sockets. */
 	if ((*pskb)->len < sizeof(struct iphdr)
 	    || (*pskb)->nh.iph->ihl * 4 < sizeof(struct iphdr))
@@ -195,7 +237,29 @@
 			return NF_STOLEN;
 	}
 
-	return ip_nat_fn(hooknum, pskb, in, out, okfn);
+	nat_route_key_get(*pskb, &key, 0);
+	ret = ip_nat_fn(hooknum, pskb, in, out, okfn);
+
+	if (ret != NF_DROP && ret != NF_STOLEN
+	    && nat_route_key_compare(*pskb, &key, 0)) {
+		if (ip_route_me_harder(pskb) != 0)
+			ret = NF_DROP;
+#ifdef CONFIG_XFRM
+		/*
+		 * POST_ROUTING hook is called with fixed outfn, we need
+		 * to manually confirm the packet and direct it to the
+		 * transformers if a policy matches.
+		 */
+		else if ((*pskb)->dst->xfrm != NULL) {
+			ret = ip_conntrack_confirm(*pskb);
+			if (ret != NF_DROP) {
+				dst_output(*pskb);
+				ret = NF_STOLEN;
+			}
+		}
+#endif
+	}
+	return ret;
 }
 
 #ifdef CONFIG_IP_NF_NAT_LOCAL
@@ -206,7 +270,7 @@
 		const struct net_device *out,
 		int (*okfn)(struct sk_buff *))
 {
-	u_int32_t saddr, daddr;
+	struct nat_route_key key;
 	unsigned int ret;
 
 	/* root is playing with raw sockets. */
@@ -214,14 +278,14 @@
 	    || (*pskb)->nh.iph->ihl * 4 < sizeof(struct iphdr))
 		return NF_ACCEPT;
 
-	saddr = (*pskb)->nh.iph->saddr;
-	daddr = (*pskb)->nh.iph->daddr;
-
+	nat_route_key_get(*pskb, &key, 1);
 	ret = ip_nat_fn(hooknum, pskb, in, out, okfn);
+
 	if (ret != NF_DROP && ret != NF_STOLEN
-	    && ((*pskb)->nh.iph->saddr != saddr
-		|| (*pskb)->nh.iph->daddr != daddr))
-		return ip_route_me_harder(pskb) == 0 ? ret : NF_DROP;
+	    && nat_route_key_compare(*pskb, &key, 1)) {
+		if (ip_route_me_harder(pskb) != 0)
+			ret = NF_DROP;
+	}
 	return ret;
 }
 #endif

Re: [RFC, PATCH 4/5]: netfilter+ipsec - policy lookup

[David S. Miller]

On Thu, 18 Mar 2004 17:32:23 +0100
Patrick McHardy <kaber@trash.net> wrote:

> This patch adds policy lookups to ip_route_me_harder and makes NAT
> reroute for any change that affects route/policy lookups.

Why are you deleting that "fl.proto = iph->protocol;" line in
net/core/netfilter.c?  Is something else going to set it properly?

Re: [RFC, PATCH 4/5]: netfilter+ipsec - policy lookup

[Patrick McHardy]

David S. Miller wrote:
> On Thu, 18 Mar 2004 17:32:23 +0100
> Patrick McHardy <kaber@trash.net> wrote:
> 
> 
>>This patch adds policy lookups to ip_route_me_harder and makes NAT
>>reroute for any change that affects route/policy lookups.
> 
> 
> Why are you deleting that "fl.proto = iph->protocol;" line in
> net/core/netfilter.c?  Is something else going to set it properly?
> 

The patch adds a call to decode_session/xfrm_lookup below. This
handles packets with local and non-local source, setting fl.proto
only handles packets with local source. Also we must check if the
packet was already transformed to prevent loops.

Regards
Patrick

Re: [RFC, PATCH 4/5]: netfilter+ipsec - policy lookup

[Herbert Xu]

On Thu, Mar 18, 2004 at 05:32:23PM +0100, Patrick McHardy wrote:
>
> @@ -635,7 +636,6 @@
>  #ifdef CONFIG_IP_ROUTE_FWMARK
>  		fl.nl_u.ip4_u.fwmark = (*pskb)->nfmark;
>  #endif
> -		fl.proto = iph->protocol;

Better call __ip_route_output_key rather than not setting proto because
you'll need proto in xfrm_lookup.

>  		if (ip_route_output_key(&rt, &fl) != 0)
>  			return -1;
>  

> @@ -661,6 +661,20 @@
>  	
>  	if ((*pskb)->dst->error)
>  		return -1;
> +
> +#ifdef CONFIG_XFRM
> +	if (!(IPCB(*pskb)->flags & IPSKB_XFRM_TRANSFORMED)) {
> +		struct xfrm_policy_afinfo *afinfo;
> +
> +		afinfo = xfrm_policy_get_afinfo(AF_INET);
> +		if (afinfo != NULL) {
> +			afinfo->decode_session(*pskb, &fl);
> +			xfrm_policy_put_afinfo(afinfo);
> +			if (xfrm_lookup(&(*pskb)->dst, &fl, (*pskb)->sk, 0) != 0)
> +				return -1;
> +		}
> +	}
> +#endif

If we can reinject transport packets then we can move this back into
the if clause.
-- 
Debian GNU/Linux 3.0 is out! ( http://www.debian.org/ )
Email:  Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [RFC, PATCH 4/5]: netfilter+ipsec - policy lookup

[Patrick McHardy]

Herbert Xu wrote:
> On Thu, Mar 18, 2004 at 05:32:23PM +0100, Patrick McHardy wrote:
> 
>>@@ -635,7 +636,6 @@
>> #ifdef CONFIG_IP_ROUTE_FWMARK
>> 		fl.nl_u.ip4_u.fwmark = (*pskb)->nfmark;
>> #endif
>>-		fl.proto = iph->protocol;
> 
> 
> Better call __ip_route_output_key rather than not setting proto because
> you'll need proto in xfrm_lookup.
> 
> 
>> 		if (ip_route_output_key(&rt, &fl) != 0)
>> 			return -1;
>> 
> 
> 
>>@@ -661,6 +661,20 @@
>> 	
>> 	if ((*pskb)->dst->error)
>> 		return -1;
>>+
>>+#ifdef CONFIG_XFRM
>>+	if (!(IPCB(*pskb)->flags & IPSKB_XFRM_TRANSFORMED)) {
>>+		struct xfrm_policy_afinfo *afinfo;
>>+
>>+		afinfo = xfrm_policy_get_afinfo(AF_INET);
>>+		if (afinfo != NULL) {
>>+			afinfo->decode_session(*pskb, &fl);
>>+			xfrm_policy_put_afinfo(afinfo);
>>+			if (xfrm_lookup(&(*pskb)->dst, &fl, (*pskb)->sk, 0) != 0)
>>+				return -1;
>>+		}
>>+	}
>>+#endif
> 
> 
> If we can reinject transport packets then we can move this back into
> the if clause.

I don't understand the relationship to transport mode packets. I used an
explicit call to xfrm_lookup so packets with non-local source are also
handled. We also need to protect against loops, packets which are
already transformed should not be transformed again.

Regards
Patrick

Re: [RFC, PATCH 4/5]: netfilter+ipsec - policy lookup

[Herbert Xu]

On Fri, Mar 19, 2004 at 05:34:58PM +0100, Patrick McHardy wrote:
> Herbert Xu wrote:
> >On Thu, Mar 18, 2004 at 05:32:23PM +0100, Patrick McHardy wrote:
> >
> >>@@ -635,7 +636,6 @@
> >>#ifdef CONFIG_IP_ROUTE_FWMARK
> >>		fl.nl_u.ip4_u.fwmark = (*pskb)->nfmark;
> >>#endif
> >>-		fl.proto = iph->protocol;
> >
> >
> >Better call __ip_route_output_key rather than not setting proto because
> >you'll need proto in xfrm_lookup.

Right, you're calling decode_session below which is much better.

> >>@@ -661,6 +661,20 @@
> >>	
> >>	if ((*pskb)->dst->error)
> >>		return -1;
> >>+
> >>+#ifdef CONFIG_XFRM
> >>+	if (!(IPCB(*pskb)->flags & IPSKB_XFRM_TRANSFORMED)) {
> >>+		struct xfrm_policy_afinfo *afinfo;
> >>+
> >>+		afinfo = xfrm_policy_get_afinfo(AF_INET);
> >>+		if (afinfo != NULL) {
> >>+			afinfo->decode_session(*pskb, &fl);
> >>+			xfrm_policy_put_afinfo(afinfo);
> >>+			if (xfrm_lookup(&(*pskb)->dst, &fl, (*pskb)->sk, 0) 
> >>!= 0)
> >>+				return -1;
> >>+		}
> >>+	}
> >>+#endif
> >
> >
> >If we can reinject transport packets then we can move this back into
> >the if clause.
> 
> I don't understand the relationship to transport mode packets. I used an

Actually it was me who was confused.  ip_route_me_harder can be called
on both incoming/outgoing packets.  That's what the if clause is trying
to determine.  You should only call xfrm_lookup on the outgoing path.

So this should be moved back to the if clause above:

		fl.proto = iph->protocol;
		lookup = __ip_route_output_key;
#ifdef CONFIG_XFRM
		if (!(IPCB(*pskb)->flags & IPSKB_XFRM_TRANSFORMED)) {
			lookup = ip_route_output_key;
			do_decode
		}
#endif
		if (lookup(&rt, &fl) != 0)
			return -1;
-- 
Debian GNU/Linux 3.0 is out! ( http://www.debian.org/ )
Email:  Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [RFC, PATCH 4/5]: netfilter+ipsec - policy lookup

[Patrick McHardy]

Herbert Xu wrote:
> 
> Actually it was me who was confused.  ip_route_me_harder can be called
> on both incoming/outgoing packets.  That's what the if clause is trying
> to determine.  You should only call xfrm_lookup on the outgoing path.

No, ip_route_me_harder is currently (without the patches) only called
for outgoing packets. The if-clause is there because ip_route_output
doesn't handle packets with non-local source, and we don't want to set
the source to 0 (as was done before) because it prevents policy routing
from working properly. That's why we need the xfrm_lookup for both
cases.

Regards
Patrick

> 
> So this should be moved back to the if clause above:
> 
> 		fl.proto = iph->protocol;
> 		lookup = __ip_route_output_key;
> #ifdef CONFIG_XFRM
> 		if (!(IPCB(*pskb)->flags & IPSKB_XFRM_TRANSFORMED)) {
> 			lookup = ip_route_output_key;
> 			do_decode
> 		}
> #endif
> 		if (lookup(&rt, &fl) != 0)
> 			return -1;

Re: [RFC, PATCH 4/5]: netfilter+ipsec - policy lookup

[Herbert Xu]

On Sat, Mar 20, 2004 at 03:01:55PM +0100, Patrick McHardy wrote:
> Herbert Xu wrote:
> >
> >Actually it was me who was confused.  ip_route_me_harder can be called
> >on both incoming/outgoing packets.  That's what the if clause is trying
> >to determine.  You should only call xfrm_lookup on the outgoing path.
> 
> No, ip_route_me_harder is currently (without the patches) only called
> for outgoing packets. The if-clause is there because ip_route_output
> doesn't handle packets with non-local source, and we don't want to set
> the source to 0 (as was done before) because it prevents policy routing
> from working properly. That's why we need the xfrm_lookup for both
> cases.

You're right.  Sorry for the confusion.
-- 
Debian GNU/Linux 3.0 is out! ( http://www.debian.org/ )
Email:  Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [RFC, PATCH 4/5]: netfilter+ipsec - policy lookup

[Herbert Xu]

On Thu, Mar 18, 2004 at 05:32:23PM +0100, Patrick McHardy wrote:
>  
> @@ -661,6 +661,20 @@
>  	
>  	if ((*pskb)->dst->error)
>  		return -1;
> +
> +#ifdef CONFIG_XFRM
> +	if (!(IPCB(*pskb)->flags & IPSKB_XFRM_TRANSFORMED)) {
> +		struct xfrm_policy_afinfo *afinfo;
> +
> +		afinfo = xfrm_policy_get_afinfo(AF_INET);
> +		if (afinfo != NULL) {
> +			afinfo->decode_session(*pskb, &fl);
> +			xfrm_policy_put_afinfo(afinfo);
> +			if (xfrm_lookup(&(*pskb)->dst, &fl, (*pskb)->sk, 0) != 0)
> +				return -1;
> +		}
> +	}
> +#endif

BTW, you can xfrm4_route_forward here.
-- 
Debian GNU/Linux 3.0 is out! ( http://www.debian.org/ )
Email:  Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [RFC, PATCH 4/5]: netfilter+ipsec - policy lookup

[Patrick McHardy]

Herbert Xu wrote:
> On Thu, Mar 18, 2004 at 05:32:23PM +0100, Patrick McHardy wrote:
> 
>> 
>>@@ -661,6 +661,20 @@
>> 	
>> 	if ((*pskb)->dst->error)
>> 		return -1;
>>+
>>+#ifdef CONFIG_XFRM
>>+	if (!(IPCB(*pskb)->flags & IPSKB_XFRM_TRANSFORMED)) {
>>+		struct xfrm_policy_afinfo *afinfo;
>>+
>>+		afinfo = xfrm_policy_get_afinfo(AF_INET);
>>+		if (afinfo != NULL) {
>>+			afinfo->decode_session(*pskb, &fl);
>>+			xfrm_policy_put_afinfo(afinfo);
>>+			if (xfrm_lookup(&(*pskb)->dst, &fl, (*pskb)->sk, 0) != 0)
>>+				return -1;
>>+		}
>>+	}
>>+#endif
> 
> 
> BTW, you can xfrm4_route_forward here.

Is it correct that __xfrm_route_forward will use NULL for the sock
parameter to xfrm_lookup even if the packet is from a local socket ?

Regards
Patrick

Re: [RFC, PATCH 4/5]: netfilter+ipsec - policy lookup

[Herbert Xu]

On Mon, Mar 22, 2004 at 12:34:11AM +0100, Patrick McHardy wrote:
> Herbert Xu wrote:
> >
> >BTW, you can xfrm4_route_forward here.
> 
> Is it correct that __xfrm_route_forward will use NULL for the sock
> parameter to xfrm_lookup even if the packet is from a local socket ?

No that would be wrong as socket policies won't be applied correctly.
Forget about that idea :)
-- 
Debian GNU/Linux 3.0 is out! ( http://www.debian.org/ )
Email:  Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [RFC, PATCH 4/5]: netfilter+ipsec - policy lookup

[Patrick McHardy]

Herbert Xu wrote:
> On Mon, Mar 22, 2004 at 12:34:11AM +0100, Patrick McHardy wrote:
>
>>Is it correct that __xfrm_route_forward will use NULL for the sock
>>parameter to xfrm_lookup even if the packet is from a local socket ?
> 
> No that would be wrong as socket policies won't be applied correctly.
> Forget about that idea :)

Thanks anyway, it reminded me to check for !(dst->flags & DST_NOXFRM)
before xfrm_lookup, I'm going to change this now.

Regards
Patrick

Re: [RFC, PATCH 4/5]: netfilter+ipsec - policy lookup

[Alexander Samad]

[-- Attachment #1: Type: text/plain, Size: 642 bytes --]

Hi

Think their might be a problem with this patch.

Potientially a packet could traverse the pre, forward and the post
routing, at which point it can be SNAT'ed or MASQ'ed and then re
injected into route_me_harder.  This potiential could allow packets to
be rerouted based on the new src/dst addresses differently to the intail
packet but this new packet doesn't traverse any of the chains with the
new information.

Alex

On Thu, Mar 18, 2004 at 05:32:23PM +0100, Patrick McHardy wrote:
> This patch adds policy lookups to ip_route_me_harder and makes NAT
> reroute for any change that affects route/policy lookups.
> 



[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [RFC, PATCH 4/5]: netfilter+ipsec - policy lookup

[Patrick McHardy]

Alexander Samad wrote:
> Hi
> 
> Think their might be a problem with this patch.
> 
> Potientially a packet could traverse the pre, forward and the post
> routing, at which point it can be SNAT'ed or MASQ'ed and then re
> injected into route_me_harder.  This potiential could allow packets to
> be rerouted based on the new src/dst addresses differently to the intail
> packet but this new packet doesn't traverse any of the chains with the
> new information.

This is just as without the patches, SNAT in POST_ROUTING never causes
a packet to re-traverse the hooks. There is one minor difference,
packets which match a policy after NAT stop traversing the hooks at
NF_IP_PRI_NAT_SRC priority. I will fix this this for the final version.

Regards
Patrick

> 
> Alex
> 
> On Thu, Mar 18, 2004 at 05:32:23PM +0100, Patrick McHardy wrote:
> 
>>This patch adds policy lookups to ip_route_me_harder and makes NAT
>>reroute for any change that affects route/policy lookups.
>>
> 
> 
> 

Re: [RFC, PATCH 4/5]: netfilter+ipsec - policy lookup

[Alexander Samad]

[-- Attachment #1: Type: text/plain, Size: 1520 bytes --]

On Wed, Mar 24, 2004 at 03:39:50AM +0100, Patrick McHardy wrote:
> Alexander Samad wrote:
> >Hi
> >
> >Think their might be a problem with this patch.
> >
> >Potientially a packet could traverse the pre, forward and the post
> >routing, at which point it can be SNAT'ed or MASQ'ed and then re
> >injected into route_me_harder.  This potiential could allow packets to
> >be rerouted based on the new src/dst addresses differently to the intail
> >packet but this new packet doesn't traverse any of the chains with the
> >new information.
> 
> This is just as without the patches, SNAT in POST_ROUTING never causes
> a packet to re-traverse the hooks. There is one minor difference,
> packets which match a policy after NAT stop traversing the hooks at
> NF_IP_PRI_NAT_SRC priority. I will fix this this for the final version.

Sorry might not have made myself clear, after an SNAT with your patch
the packet is re injected into route_me_harder, thus the packet is able
to be rerouted (sent out another interface for example) 

What this would mean is a packet could meet your iptable rules with is
PRE-SNAT details and then actually behave differently once it has been
SNAT'ed (and not get checked)

Alex

> 
> Regards
> Patrick
> 
> >
> >Alex
> >
> >On Thu, Mar 18, 2004 at 05:32:23PM +0100, Patrick McHardy wrote:
> >
> >>This patch adds policy lookups to ip_route_me_harder and makes NAT
> >>reroute for any change that affects route/policy lookups.
> >>
> >
> >
> >
> 
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[RFC, PATCH 5/5]: netfilter+ipsec - policy checks

[Patrick McHardy]

[-- Attachment #1: Type: text/plain, Size: 260 bytes --]

This patch makes xfrm_policy_check locate the correct policy after NAT.
For protocols which do policy checks in their receive routines the
reference to nfct has to be kept until policy checks are done, the
other ones still drop it in ip_local_deliver_finish.


[-- Attachment #2: 05-nat-policy-check.diff --]
[-- Type: text/x-patch, Size: 5917 bytes --]

# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
#   2004/03/18 16:44:21+01:00 kaber@trash.net 
#   [NETFILTER]: Make policy checks find correct policy after NAT
# 
# net/xfrm/xfrm_policy.c
#   2004/03/18 16:44:12+01:00 kaber@trash.net +2 -0
#   [NETFILTER]: Make policy checks find correct policy after NAT
# 
# net/ipv4/udp.c
#   2004/03/18 16:44:12+01:00 kaber@trash.net +2 -0
#   [NETFILTER]: Make policy checks find correct policy after NAT
# 
# net/ipv4/tcp_ipv4.c
#   2004/03/18 16:44:12+01:00 kaber@trash.net +1 -0
#   [NETFILTER]: Make policy checks find correct policy after NAT
# 
# net/ipv4/raw.c
#   2004/03/18 16:44:12+01:00 kaber@trash.net +1 -0
#   [NETFILTER]: Make policy checks find correct policy after NAT
# 
# net/ipv4/ip_input.c
#   2004/03/18 16:44:12+01:00 kaber@trash.net +6 -8
#   [NETFILTER]: Make policy checks find correct policy after NAT
# 
# net/core/netfilter.c
#   2004/03/18 16:44:12+01:00 kaber@trash.net +43 -0
#   [NETFILTER]: Make policy checks find correct policy after NAT
# 
# include/linux/netfilter.h
#   2004/03/18 16:44:12+01:00 kaber@trash.net +16 -0
#   [NETFILTER]: Make policy checks find correct policy after NAT
# 
diff -Nru a/include/linux/netfilter.h b/include/linux/netfilter.h
--- a/include/linux/netfilter.h	Thu Mar 18 16:46:27 2004
+++ b/include/linux/netfilter.h	Thu Mar 18 16:46:27 2004
@@ -171,5 +171,21 @@
 #define NF_HOOK_COND NF_HOOK
 #endif /*CONFIG_NETFILTER*/
 
+#ifdef CONFIG_XFRM
+#ifdef CONFIG_IP_NF_NAT_NEEDED
+struct flowi;
+extern void nf_nat_decode_session4(struct sk_buff *skb, struct flowi *fl);
+
+static inline void
+nf_nat_decode_session(struct sk_buff *skb, struct flowi *fl, int family)
+{
+	if (family == AF_INET)
+		nf_nat_decode_session4(skb, fl);
+}
+#else /* CONFIG_IP_NF_NAT_NEEDED */
+#define nf_nat_decode_session(skb,fl,family)
+#endif /* CONFIG_IP_NF_NAT_NEEDED */
+#endif /* CONFIG_XFRM */
+
 #endif /*__KERNEL__*/
 #endif /*__LINUX_NETFILTER_H*/
diff -Nru a/net/core/netfilter.c b/net/core/netfilter.c
--- a/net/core/netfilter.c	Thu Mar 18 16:46:27 2004
+++ b/net/core/netfilter.c	Thu Mar 18 16:46:27 2004
@@ -739,6 +739,49 @@
 	return NF_HOOK(PF_INET, NF_IP_PRE_ROUTING, skb, skb->dev, NULL,
 	               nf_postxfrm_done);
 }
+
+#ifdef CONFIG_IP_NF_NAT_NEEDED
+#include <linux/netfilter_ipv4/ip_conntrack.h>
+#include <linux/netfilter_ipv4/ip_nat.h>
+
+void nf_nat_decode_session4(struct sk_buff *skb, struct flowi *fl)
+{
+	struct ip_conntrack *ct;
+	struct ip_conntrack_tuple *t;
+	struct ip_nat_info_manip *m;
+	unsigned int i;
+
+	if (skb->nfct == NULL)
+		return;
+	ct = (struct ip_conntrack *)skb->nfct->master;
+
+	for (i = 0; i < ct->nat.info.num_manips; i++) {
+		m = &ct->nat.info.manips[i];
+		t = &ct->tuplehash[m->direction].tuple;
+
+		switch (m->hooknum) {
+		case NF_IP_PRE_ROUTING:
+			if (m->maniptype != IP_NAT_MANIP_DST)
+				break;
+			fl->fl4_dst = t->dst.ip;
+			if (t->dst.protonum == IPPROTO_TCP ||
+			    t->dst.protonum == IPPROTO_UDP)
+				fl->fl_ip_dport = t->dst.u.tcp.port;
+			break;
+#ifdef CONFIG_IP_NF_NAT_LOCAL
+		case NF_IP_LOCAL_IN:
+			if (m->maniptype != IP_NAT_MANIP_SRC)
+				break;
+			fl->fl4_src = t->src.ip;
+			if (t->dst.protonum == IPPROTO_TCP ||
+			    t->dst.protonum == IPPROTO_UDP)
+				fl->fl_ip_sport = t->src.u.tcp.port;
+			break;
+#endif
+		}
+	}
+}
+#endif /* CONFIG_IP_NF_NAT_NEEDED */
 #endif /* CONFIG_XFRM */
 
 int skb_ip_make_writable(struct sk_buff **pskb, unsigned int writable_len)
diff -Nru a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
--- a/net/ipv4/ip_input.c	Thu Mar 18 16:46:27 2004
+++ b/net/ipv4/ip_input.c	Thu Mar 18 16:46:27 2004
@@ -206,10 +206,6 @@
 
 	__skb_pull(skb, ihl);
 
-	/* Free reference early: we don't need it any more, and it may
-           hold ip_conntrack module loaded indefinitely. */
-	nf_reset(skb);
-
         /* Point into the IP datagram, just past the header. */
         skb->h.raw = skb->data;
 
@@ -240,10 +236,12 @@
 		if (ipprot != NULL) {
 			int ret;
 
-			if (!ipprot->no_policy &&
-			    !xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb)) {
-				kfree_skb(skb);
-				goto out;
+			if (!ipprot->no_policy) {
+				if (!xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb)) {
+					kfree_skb(skb);
+					goto out;
+				}
+				nf_reset(skb);
 			}
 			ret = ipprot->handler(skb);
 			if (ret < 0) {
diff -Nru a/net/ipv4/raw.c b/net/ipv4/raw.c
--- a/net/ipv4/raw.c	Thu Mar 18 16:46:27 2004
+++ b/net/ipv4/raw.c	Thu Mar 18 16:46:27 2004
@@ -249,6 +249,7 @@
 		kfree_skb(skb);
 		return NET_RX_DROP;
 	}
+	nf_reset(skb);
 
 	skb_push(skb, skb->data - skb->nh.raw);
 
diff -Nru a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
--- a/net/ipv4/tcp_ipv4.c	Thu Mar 18 16:46:27 2004
+++ b/net/ipv4/tcp_ipv4.c	Thu Mar 18 16:46:27 2004
@@ -1785,6 +1785,7 @@
 
 	if (!xfrm4_policy_check(sk, XFRM_POLICY_IN, skb))
 		goto discard_and_relse;
+	nf_reset(skb);
 
 	if (sk_filter(sk, skb, 0))
 		goto discard_and_relse;
diff -Nru a/net/ipv4/udp.c b/net/ipv4/udp.c
--- a/net/ipv4/udp.c	Thu Mar 18 16:46:27 2004
+++ b/net/ipv4/udp.c	Thu Mar 18 16:46:27 2004
@@ -1030,6 +1030,7 @@
 		kfree_skb(skb);
 		return -1;
 	}
+	nf_reset(skb);
 
 	if (up->encap_type) {
 		/*
@@ -1195,6 +1196,7 @@
 
 	if (!xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb))
 		goto drop;
+	nf_reset(skb);
 
 	/* No socket. Drop packet silently, if checksum is wrong */
 	if (udp_checksum_complete(skb))
diff -Nru a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
--- a/net/xfrm/xfrm_policy.c	Thu Mar 18 16:46:27 2004
+++ b/net/xfrm/xfrm_policy.c	Thu Mar 18 16:46:27 2004
@@ -21,6 +21,7 @@
 #include <linux/workqueue.h>
 #include <linux/notifier.h>
 #include <linux/netdevice.h>
+#include <linux/netfilter.h>
 #include <net/xfrm.h>
 #include <net/ip.h>
 
@@ -908,6 +909,7 @@
 
 	if (_decode_session(skb, &fl, family) < 0)
 		return 0;
+	nf_nat_decode_session(skb, &fl, family);
 
 	/* First, check used SA against their selectors. */
 	if (skb->sp) {

Re: [RFC, PATCH 5/5]: netfilter+ipsec - policy checks

[David S. Miller]

On Thu, 18 Mar 2004 17:32:39 +0100
Patrick McHardy <kaber@trash.net> wrote:

> This patch makes xfrm_policy_check locate the correct policy after NAT.
> For protocols which do policy checks in their receive routines the
> reference to nfct has to be kept until policy checks are done, the
> other ones still drop it in ip_local_deliver_finish.

This patch looks fine to me.

Other than the minor comments I've made the most unhappy I am
with the input patch, and you agree it's grotty too.  Let's look
for a better solution, perhaps with new top-level SKB state,
and then we can put all of your work in after you're made the other
minor fixes I've asked for as well.

Thanks Patrick.

Re: [RFC, PATCH 5/5]: netfilter+ipsec - policy checks

[Patrick McHardy]

David S. Miller wrote:

> Other than the minor comments I've made the most unhappy I am
> with the input patch, and you agree it's grotty too.  Let's look
> for a better solution, perhaps with new top-level SKB state,
> and then we can put all of your work in after you're made the other
> minor fixes I've asked for as well.

I'm going to make the changes you suggested. Can you please explain what
you imagine with the new top-level skb state ?

Regards
Patrick