[PATCH] Fix MASQUERADE

[PATCH] Fix MASQUERADE

[Rusty Russell]

Please test.  Also resets assured bit.

Name: Change MASQUERADE to Use Device Address Directly
Status: Untested
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

Instead of doing a dubious route lookup, just use the first IP address
of the (dynamic) interface.  Also, reset assured bit so after masq
connections can be cleaned up if memory pressure.

diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .2156-linux-2.6.9-rc1-bk12/net/ipv4/netfilter/ipt_MASQUERADE.c .2156-linux-2.6.9-rc1-bk12.updated/net/ipv4/netfilter/ipt_MASQUERADE.c
--- .2156-linux-2.6.9-rc1-bk12/net/ipv4/netfilter/ipt_MASQUERADE.c	2004-08-25 09:54:25.000000000 +1000
+++ .2156-linux-2.6.9-rc1-bk12.updated/net/ipv4/netfilter/ipt_MASQUERADE.c	2004-09-08 00:15:05.000000000 +1000
@@ -82,7 +82,6 @@ masquerade_target(struct sk_buff **pskb,
 	const struct ip_nat_multi_range *mr;
 	struct ip_nat_multi_range newrange;
 	u_int32_t newsrc;
-	struct rtable *rt;
 
 	IP_NF_ASSERT(hooknum == NF_IP_POST_ROUTING);
 
@@ -96,36 +95,12 @@ masquerade_target(struct sk_buff **pskb,
 	                    || ctinfo == IP_CT_RELATED + IP_CT_IS_REPLY));
 
 	mr = targinfo;
-
-	{
-		struct flowi fl = { .nl_u = { .ip4_u =
-					      { .daddr = (*pskb)->nh.iph->daddr,
-						.tos = (RT_TOS((*pskb)->nh.iph->tos) |
-							RTO_CONN),
-#ifdef CONFIG_IP_ROUTE_FWMARK
-						.fwmark = (*pskb)->nfmark
-#endif
-					      } } };
-		if (ip_route_output_key(&rt, &fl) != 0) {
-			/* Funky routing can do this. */
-			if (net_ratelimit())
-				printk("MASQUERADE:"
-				       " No route: Rusty's brain broke!\n");
-			return NF_DROP;
-		}
-		if (rt->u.dst.dev != out) {
-			if (net_ratelimit())
-				printk("MASQUERADE:"
-				       " Route sent us somewhere else.\n");
-			ip_rt_put(rt);
-			return NF_DROP;
-		}
+	newsrc = inet_select_addr(out, 0, RT_SCOPE_UNIVERSE);
+	if (!newsrc) {
+		printk("MASQUERADE: %s ate my IP address\n", out->name);
+		return NF_DROP;
 	}
 
-	newsrc = rt->rt_src;
-	DEBUGP("newsrc = %u.%u.%u.%u\n", NIPQUAD(newsrc));
-	ip_rt_put(rt);
-
 	WRITE_LOCK(&masq_lock);
 	ct->nat.masq_index = out->ifindex;
 	WRITE_UNLOCK(&masq_lock);
@@ -157,6 +132,18 @@ device_cmp(const struct ip_conntrack *i,
 	return ret;
 }
 
+static inline int
+connect_unassure(const struct ip_conntrack *i, void *_ina)
+{
+	struct in_ifaddr *ina = _ina;
+
+	/* We reset the ASSURED bit on all connections, so they will
+	 * get reaped under memory pressure. */
+	if (i->nat.masq_index == ina->ifa_dev->dev->ifindex)
+		clear_bit(IPS_ASSURED_BIT, (unsigned long *)&i->status);
+	return 0;
+}
+
 static int masq_inet_event(struct notifier_block *this,
 			   unsigned long event,
 			   void *ptr)
@@ -166,6 +153,8 @@ static int masq_inet_event(struct notifi
 	 * entries. */
 	if (event == NETDEV_UP)
 		ip_ct_selective_cleanup(device_cmp, ptr);
+	else if (event == NETDEV_DOWN)
+		ip_ct_selective_cleanup(connect_unassure, ptr);
 
 	return NOTIFY_DONE;
 }

-- 
Anyone who quotes me in their signature is an idiot -- Rusty Russell

Re: [PATCH] Fix MASQUERADE

[David S. Miller]

On Wed, 08 Sep 2004 00:20:09 +1000
Rusty Russell <rusty@rustcorp.com.au> wrote:

> Please test.  Also resets assured bit.
> 
> Name: Change MASQUERADE to Use Device Address Directly
> Status: Untested
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
> 
> Instead of doing a dubious route lookup, just use the first IP address
> of the (dynamic) interface.  Also, reset assured bit so after masq
> connections can be cleaned up if memory pressure.

Didn't Herbert Xu et al. come to the conclusion that this isn't
the way to fix this and that using inet_select_addr() had
some problems especially in policy routing situations?

Re: [PATCH] Fix MASQUERADE

[Herbert Xu]

On Tue, Sep 07, 2004 at 08:09:44AM -0700, David S. Miller wrote:
>
> Didn't Herbert Xu et al. come to the conclusion that this isn't
> the way to fix this and that using inet_select_addr() had
> some problems especially in policy routing situations?

No that was Julian Anastasov IIRC.

I agree with Rusty and Harald that MASQUERADE should be as simple as
possible.  So if you need complex routing to determine the correct
source address, then you should use SNAT.

Now I haven't actually seen Rusty's patch but if it just uses
inet_select_addr then it's definitely the way to go.

Cheers,
-- 
Visit Openswan at http://www.openswan.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: [PATCH] Fix MASQUERADE

[David S. Miller]

On Wed, 8 Sep 2004 07:30:36 +1000
Herbert Xu <herbert@gondor.apana.org.au> wrote:

> On Tue, Sep 07, 2004 at 08:09:44AM -0700, David S. Miller wrote:
> >
> > Didn't Herbert Xu et al. come to the conclusion that this isn't
> > the way to fix this and that using inet_select_addr() had
> > some problems especially in policy routing situations?
> 
> No that was Julian Anastasov IIRC.
> 
> I agree with Rusty and Harald that MASQUERADE should be as simple as
> possible.  So if you need complex routing to determine the correct
> source address, then you should use SNAT.
> 
> Now I haven't actually seen Rusty's patch but if it just uses
> inet_select_addr then it's definitely the way to go.

Hmmm, ok.  Rusty, please resend to me under private cover.