fruzam / android-wired-tether

Automatically exported from code.google.com/p/android-wired-tether
0 stars 0 forks source link

Patch: Clamp MSS to fix broken fragmentation #31

Open GoogleCodeExporter opened 8 years ago

GoogleCodeExporter commented 8 years ago
This patch adds a netfilter rule to allow tethering with ISPs that don't 
support fragmented packets, but do advertise an appropraite path MTU.

Background: some ISPs don't support IP fragmentation, either because they're 
afraid of IP fragmentation attacks, or their routers are braindead.  Usually 
this isn't an issue for connecting clients because their DHCP responses include 
an interface MTU equal to the path MTU.  In response, the client ensures during 
TCP handshakes that connections use an MSS that results in TCP/IP packets never 
being fragmented.

However, problems occur when the connecting "client" is a NAT router (such as a 
tethering phone).  Often the phone-tethered host interface (e.g., USB RNDIS, 
802.11, etc.) uses a preset MTU (usually 1500) that exceeds the path MTU 
advertised by the ISP.  The host will open TCP connections with too-large of an 
MSS, and inevitably large packets will become fragmented and lost.  The fix is 
to have the router (phone) mangle TCP handshakes to clamp the MSS such that 
generated TCP/IP packets never exceed the path MTU.  This patch implements such 
a fix as described in the iptables(8) man page.

Now, I've not-yet witnessed this problem in practice when tethering--either 
because my ISP does support fragmentation, or because the path MTU (1400 or 
1492) isn't low enough to cause fragmentation in practice.  That said, I have 
encoutered this problem with wired-broadband ISPs.  Since the fix is trivial, 
it's probably a good idea to implement as I'm sure someone, somewhere, gets 
this wrong.

Also, I found that the "iptables" binary included with v1.4 doesn't support the 
"--clamp-mss-to-pmtu" option to the TCPMSS target.  I'm not actually sure where 
this executable comes from as there's no source included nor a pointer to it 
(which, I suspect is an unintentional compliance issue with the GPLv2, which 
iptables is released under--this should probably be addressed as well).  I 
assume it's a stock AOSP build, from Donut maybe?

In any event, it works with the updated iptables version that came with my 
phone, which I think is just a stock build from Eclair AOSP.  So in addition to 
the patch, the "iptables" binary would have to be updated as well.

Thanks!

Original issue reported on code.google.com by mkas...@gmail.com on 19 Dec 2010 at 7:30

Attachments:

GoogleCodeExporter commented 8 years ago
Thanks for this patch. I will add you enhancements in the next days.
And ... this is a "sister-project" from "wireless tether" - so, I'm going to 
take over these enhancements to wireless tether as well.

Original comment by harald....@gmail.com on 7 Jan 2011 at 12:44

GoogleCodeExporter commented 8 years ago
I recently tried this on a G2 and discovered that the "iptables -t mangle" 
commands didn't work as the Vision's stock kernel (verified in source, and I 
believe HTC devices in general) doesn't include Netfilter mangle support.

So you probably want to implement this such that the "iptables -t mangle" 
operations are non-fatal on error and don't complain to the user.  It would be 
great to have on devices that do support NF mangle out of the box, but it's no 
big deal otherwise.

And yes, wired-tether is my primary interest although I did recently modify 
wireless-tether's edify script to clamp MSS as well.  I can create an 
issue/patch for that if you'd like, although the carryover is straight forward 
and I figured it might be a bit redundant.

Original comment by mkas...@gmail.com on 8 Jan 2011 at 6:58

GoogleCodeExporter commented 8 years ago
Yes. A patch for edify would be nice!
Well ... a check if a certain kernel-option (for "-t mangle") was compiled into 
the kernel will be needed. 

Original comment by harald....@gmail.com on 10 Jan 2011 at 11:29

GoogleCodeExporter commented 8 years ago
So iptables returns "3" (internally called VERSION_PROBLEM) when one attempts 
to use a feature unsupported by the kernel.  Attached is an updated patch that 
ignores "iptables -t mangle" errors if it returns value 3.  This makes the MSS 
clamp optional, it will work for phones that have mangle table support, and 
it's ignored on phones that don't.

The patch isn't quite as clean as I would like.  Unfortuately the multiple 
tests of "returncode == 0" when an iptables error occurs prevents the patch 
from being simpler.  But that's OK.

There's two outstanding issues:

As mentioned previously, the iptables binary needs to be updated to support the 
"--clamp-mss-to-pmtu" option, which it does in the Eclair & Froyo AOSP trees.  
I'm guessing it doesn't in Donut and earlier.

Also, when I test new builds of wired-tether, I see that the binaries are not 
automatically updated on a program update (although I keep the same version 
code), and I have to uninstall/reinstall completely to get them to update.  If 
this is the case in update of the official version, then it will error for 
everyone until they do a reinstall/binary update.  So if it doesn't exist, a 
mechanism to update the iptables binary on program update would be needed to 
make this smooth.

And yes, I have a patch for wireless-tether that I'll commit shortly as an 
"issue" in the appropriate place.

Original comment by mkas...@gmail.com on 18 Jan 2011 at 8:38

Attachments:

GoogleCodeExporter commented 8 years ago
Wireless-tether issue page/patch:
http://code.google.com/p/android-wifi-tether/issues/detail?id=816

Original comment by mkas...@gmail.com on 18 Jan 2011 at 8:50