RE: IP Masq weirdness

Joseph Gooch (mrwizard@psu.edu)
Sun, 24 Oct 1999 14:10:58 -0400


This is a multi-part message in MIME format.

------=_NextPart_000_000B_01BF1E29.98B69C20
Content-Type: text/plain;
charset="iso-8859-1"
Content-Transfer-Encoding: 7bit

> -----Original Message-----
> From: kuznet@ms2.inr.ac.ru [mailto:kuznet@ms2.inr.ac.ru]
> Sent: Saturday, October 23, 1999 7:58 AM
> To: Joseph Gooch
> Cc: jjo@mendoza.gov.ar; linux-kernel@vger.rutgers.edu
> Subject: Re: IP Masq weirdness
>
>
> Hello!
>
> > Slightly revised version.
>
> Seems, neither of version are correct.
>
> Let me to explain it from historical viewpoint. This chunk of code:
>
> if (maddr == 0)
> #endif
> maddr = inet_select_addr(dev2,
> rt->rt_gateway, RT_SCOPE_UNIVERSE);
>
> was written by me in early 2.1. That time ip_masq_* did _no_ address
> selection at all and the proposed scheme was roughly equivalent to one
> used in 2.0.
>
> That time I used policy routing rules to select arbitrary source
> address, because ip_masq/ip_fw did not supply (and do not supply now)
> this facility.
>
> ip_masq_select_addr appeared much later and its using inside ip_masq_*
> contradicts to logic of this code.
>
> What can I propose? Remove these two lines and pass to
> ip_masq_* zero maddr.
> And use ip_route_output() to select source address for
> rewriting inside
> ip_masq_*, if passed maddr is zero. Seems, this approach
> satisfies all.

Well that IS what my modified ip_masq_select_addr() ends up doing. I don't
like expanding the argument list like that, but i thought i'd follow what
was there.

It occurs to me now that by the skb passing a rtable struct, and giving
rt_src = to the actual source of the packet, it's only repeating data (as
ip_forward() is passed the skb, ip_forward can check the iph->s_addr
itself). Instead of route.c calling rt_set_nexthop() shouldn't it do a
ip_route_output() and set rt_gateway rt_src via the result?

So the question here is why should ip_forward() choose a masquerading local
address? Especially when ip_masquerade() is passed full information. Why
doesn't ip_masquerade make up its mind, as it's a masquerading specific
spiel. I follow. For that matter why pass a maddr to ip_masquerade() at
all? Just for NAT?

Take a look at my latest attempt and see if that works for you. It's
working for me (functionally) but let's see if it's in the spirit of things
:)
>
> Also, it is apparent that ip_masq_user should receive maddr as input
> address too, otherwise people who used to masquerade via
> policy routing
> are in troubles.

It looks like the code in ip_masq_user drops out if maddr is set...

if (ums->maddr)
return 0;

...but continues on to ip_route_output() if it's not, in which case
ums->maddr=rt->rt_src is correct as you originally stated.

> BTW, Juan, did you solve this problem in netfilter? Someone asked
> me recently, I did not know the answer and redirected him to you
> and Paul.
>
> Alexey
>

------=_NextPart_000_000B_01BF1E29.98B69C20
Content-Type: application/octet-stream;
name="masq.patch"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: attachment;
filename="masq.patch"

diff -ru linux-2.2.13-vanilla/net/ipv4/ip_forward.c =
linux-2.2.13-ipmasq/net/ipv4/ip_forward.c=0A=
--- linux-2.2.13-vanilla/net/ipv4/ip_forward.c Wed Oct 20 18:02:28 1999=0A=
+++ linux-2.2.13-ipmasq/net/ipv4/ip_forward.c Sun Oct 24 14:07:41 1999=0A=
@@ -10,6 +10,7 @@=0A=
* Authors: see ip.c=0A=
*=0A=
* Fixes:=0A=
+ * Joseph Gooch : Removed maddr selection for ip_masq, now done in =
ip_masq.c=0A=
* Many : Split from ip.c , see ip_input.c for =0A=
* history.=0A=
* Dave Gregorich : NULL ip_rt_put fix for multicast =0A=
@@ -168,7 +169,6 @@=0A=
* and skip the firewall checks=0A=
*/=0A=
if (iph->protocol =3D=3D IPPROTO_ICMP) {=0A=
- __u32 maddr;=0A=
#ifdef CONFIG_IP_MASQUERADE_ICMP=0A=
struct icmphdr *icmph =3D (struct icmphdr *)((char*)iph + (iph->ihl =
<< 2));=0A=
if ((icmph->type=3D=3DICMP_DEST_UNREACH)||=0A=
@@ -176,8 +176,7 @@=0A=
(icmph->type=3D=3DICMP_TIME_EXCEEDED))=0A=
{=0A=
#endif=0A=
- maddr =3D rt->rt_src;=0A=
- fw_res =3D ip_fw_masq_icmp(&skb, maddr);=0A=
+ fw_res =3D ip_fw_masquerade(&skb, 0);=0A=
if (fw_res < 0) {=0A=
kfree_skb(skb);=0A=
return -1;=0A=
@@ -187,7 +186,7 @@=0A=
/* ICMP matched - skip firewall */=0A=
goto skip_call_fw_firewall;=0A=
#ifdef CONFIG_IP_MASQUERADE_ICMP=0A=
- }=0A=
+ }=0A=
#endif =0A=
}=0A=
if (rt->rt_flags&RTCF_MASQ)=0A=
@@ -219,15 +218,11 @@=0A=
*/=0A=
if (!(IPCB(skb)->flags&IPSKB_MASQUERADED) &&=0A=
(fw_res=3D=3DFW_MASQUERADE || rt->rt_flags&RTCF_MASQ)) {=0A=
- u32 maddr;=0A=
+ u32 maddr =3D 0;=0A=
=0A=
#ifdef CONFIG_IP_ROUTE_NAT=0A=
maddr =3D (rt->rt_flags&RTCF_MASQ) ? rt->rt_src_map : 0;=0A=
-=0A=
- if (maddr =3D=3D 0)=0A=
#endif=0A=
- maddr =3D rt->rt_src;=0A=
-=0A=
if (ip_fw_masquerade(&skb, maddr) < 0) {=0A=
kfree_skb(skb);=0A=
return -1;=0A=
diff -ru linux-2.2.13-vanilla/net/ipv4/ip_masq.c =
linux-2.2.13-ipmasq/net/ipv4/ip_masq.c=0A=
--- linux-2.2.13-vanilla/net/ipv4/ip_masq.c Wed Oct 20 17:59:59 1999=0A=
+++ linux-2.2.13-ipmasq/net/ipv4/ip_masq.c Sun Oct 24 14:09:28 1999=0A=
@@ -10,6 +10,9 @@=0A=
* See ip_fw.c for original log=0A=
*=0A=
* Fixes:=0A=
+ * Joseph Gooch : Modified ip_fw_masquerade() to do a ip_route_output()=0A=
+ * (help by Dan Drown) : to choose the proper local address.=0A=
+ * (and Alexey) :=0A=
* Juan Jose Ciarlante : Modularized application masquerading (see =
ip_masq_app.c)=0A=
* Juan Jose Ciarlante : New struct ip_masq_seq that holds output/input =
delta seq.=0A=
* Juan Jose Ciarlante : Added hashed lookup by proto,maddr,mport and =
proto,saddr,sport=0A=
@@ -1141,6 +1144,22 @@=0A=
return -1;=0A=
}=0A=
=0A=
+ /* Lets determine our maddr now, shall we? */=0A=
+ if (maddr =3D=3D 0) {=0A=
+ struct rtable *rt;=0A=
+ struct rtable *skb_rt =3D (struct rtable*)skb->dst;=0A=
+ struct device *skb_dev =3D skb_rt->u.dst.dev;=0A=
+=0A=
+ if (ip_route_output(&rt, iph->daddr, 0, RT_TOS(iph->tos)|RTO_CONN, =
skb_dev?skb_dev->ifindex:0)) {=0A=
+ /* Fallback on old method */=0A=
+ maddr =3D inet_select_addr(skb_dev, skb_rt->rt_gateway, =
RT_SCOPE_UNIVERSE);=0A=
+ } else {=0A=
+ /* Route lookup succeeded */=0A=
+ maddr =3D rt->rt_src;=0A=
+ ip_rt_put(rt);=0A=
+ }=0A=
+ }=0A=
+=0A=
switch (iph->protocol) {=0A=
case IPPROTO_ICMP:=0A=
return(ip_fw_masq_icmp(skb_p, maddr));=0A=
diff -ru linux-2.2.13-vanilla/net/ipv4/ip_masq_user.c =
linux-2.2.13-ipmasq/net/ipv4/ip_masq_user.c=0A=
--- linux-2.2.13-vanilla/net/ipv4/ip_masq_user.c Wed Oct 20 17:59:59 1999=0A=
+++ linux-2.2.13-ipmasq/net/ipv4/ip_masq_user.c Sat Oct 23 00:25:48 1999=0A=
@@ -100,7 +100,7 @@=0A=
return ret;=0A=
}=0A=
dev =3D rt->u.dst.dev;=0A=
- ums->maddr =3D ip_masq_select_addr(dev, rt->rt_gateway, =
RT_SCOPE_UNIVERSE);=0A=
+ ums->maddr =3D rt->rt_src; /* Per Alexey */=0A=
=0A=
IP_MASQ_DEBUG(1-debug, "did setup maddr=3D%lX\n", ntohl(ums->maddr));=0A=
ip_rt_put(rt);=0A=

------=_NextPart_000_000B_01BF1E29.98B69C20--

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/