kern/60889 - zero IP id change issues in 5.2RC2
Andre Oppermann
andre at freebsd.org
Wed Jan 7 14:48:35 PST 2004
Richard,
I've attached a patch that fixes the problem with FIN/ACK and one more
case which got it wrong. I also fixed the host byte order problem in
ip_output.c for the normal ip_id incrementor.
Some comments and questions:
1. Do you think it is neccessary to do a htons() on the randomized
ip_id too? I'd say yes if there is a case where it has to
monotonically increase afterwards. Does it?
2. I have a Win2k machine but have check out how I can get tcp header
compression to work with my Cisco AS5300 (if it doesn't do that by
default). Will I see the problem when I do a download from a FreeBSD
5.2RC2 machine or do I have to use the Windoze as router sending
packets upwards?`
3. There are indeed devices clearing the DF bit. For example Cisco
is recommending this in it's trouble shooting section for broadband
access via DSL/L2TP where the MTU is lower than 1500 because of the
tunnelling overhead. Make a route-map to unset the DF bit and apply
it to the incoming interface.
If these questions are answered I can prepare the final patch and
commit it pending review and re@ approval.
--
Andre
(Note: tabs converted to whitespace because of c&p)
Index: ip_output.c
===================================================================
RCS file: /home/ncvs/src/sys/netinet/ip_output.c,v
retrieving revision 1.203
diff -u -p -r1.203 ip_output.c
--- ip_output.c 20 Nov 2003 20:07:38 -0000 1.203
+++ ip_output.c 7 Jan 2004 22:43:54 -0000
@@ -246,7 +246,7 @@ ip_output(struct mbuf *m0, struct mbuf *
#ifdef RANDOM_IP_ID
ip->ip_id = ip_randomid();
#else
- ip->ip_id = ip_id++;
+ ip->ip_id = htons(ip_id++);
#endif
} else {
ip->ip_off = IP_DF;
Index: tcp_subr.c
===================================================================
RCS file: /home/ncvs/src/sys/netinet/tcp_subr.c,v
retrieving revision 1.172
diff -u -p -r1.172 tcp_subr.c
--- tcp_subr.c 6 Jan 2004 23:29:46 -0000 1.172
+++ tcp_subr.c 7 Jan 2004 22:43:55 -0000
@@ -459,6 +459,8 @@ tcp_respond(tp, ipgen, th, m, ack, seq,
tlen += sizeof (struct tcpiphdr);
ip->ip_len = tlen;
ip->ip_ttl = ip_defttl;
+ if (path_mtu_discovery)
+ ip->ip_off |= IP_DF;
}
m->m_len = tlen;
m->m_pkthdr.len = tlen;
@@ -1733,6 +1735,8 @@ tcp_twrespond(struct tcptw *tw, struct s
m->m_pkthdr.csum_flags = CSUM_TCP;
m->m_pkthdr.csum_data = offsetof(struct tcphdr, th_sum);
ip->ip_len = m->m_pkthdr.len;
+ if (path_mtu_discovery)
+ ip->ip_off |= IP_DF;
error = ip_output(m, inp->inp_options, NULL,
(tw->tw_so_options & SO_DONTROUTE), NULL, inp);
}
Richard Wendland wrote:
>
> I've been asked (for freebsd-bugs) to open a discussion about PR
> kern/60889 on freebsd-net, to decide if a recent change to IP should be
> reversed before 5.2-RELEASE (scheduled this month) to give more time
> for some serious issues and risks my PR has raised to be considered
> and tested. My proposal is to revert to 5.1 behaviour for now.
>
> The recent change is to emit a zero fragmentation id when DF is set, with
> the objective of improving privacy from external id sequence observation,
> an issue raised by Steve Bellovin's paper in IMW'02.
>
> I've identified 4 problems with this change:
>
> 1) Even with path_mtu_discovery set, for some reason TCP emits FIN-ACK
> without DF. This causes two problems for this change:
>
> a) Currently the change doesn't really meet its objective for TCP,
> it just means FIN-ACK must be observed.
>
> b) Because now just one id is consumed per TCP connection on the
> FIN-ACK, for most/many systems id becomes a close approximate
> count of the number of TCP connections it has made, which external
> observers can see, not possible before this change. To me this
> seems more of a privacy issue for all FreeBSD users than the NAT
> issue in Steve Bellovin's paper this change seeks to solve.
>
> 2) Linux made exactly this change some time ago, around Linux 2.4.4,
> but was forced to back-out the change because (I think) of practical
> connectivity issues related to the comment in include/net/ip.h
> ip_select_ident() where it now implements an incrementing ip_id for DF:
>
> /* This is only to work around buggy Windows95/2000
> * VJ compression implementations. If the ID field
> * does not change, they drop every other packet in
> * a TCP stream using header compression.
> */
>
> I'm not aware that anyone checked that this buggy Windows95/2000 VJ
> compression problem is no longer an issue in practice. I doubt that
> the large web hosters who use FreeBSD would be best-pleased if they
> ran into this for even just a few users - especially as there is no
> config option to disable this change.
>
> 3) This change causes ip_id for non-DF to be output in native
> byte order in ip_output.c. Unfortunately ip_id is still output in
> Network Byte Order in ip_mroute.c and raw_ip.c, so this change risks
> little-endian machines emitting the same fragmentation id at about
> the same time from these different modules, rather than the usual
> 64k cycle; creating a small but real risk of re-assembly errors.
> [This isn't in my PR, I've only just noticed it.]
>
> I also have a suspicion that some middle-boxes (like HTTP load-balancers)
> may clear DF without setting a fresh IP id - clearing DF would save them
> the bother of routing ICMP fragmentation needed back to the source server.
> If this is so this is another problem this change could show up which
> may cause re-assembly errors. I know the bug is elsewhere, but it would
> still become a practical problem for some FreeBSD users.
>
> So before going with this change I think four things need to be done:
>
> 1) TCP changed so FIN-ACK goes out with DF if path_mtu_discovery set.
>
> 2) Tests with Windows95/2000 TCP VJ compression (RFC1144) run.
>
> 3) ip_id should be emitted in the same byte order everywhere.
>
> 4) The change made a config option, so sites can disable it should
> they run into problems, just as RANDOM_IP_ID is an option.
>
> This all seems too much to do for 5.2-RELEASE, and as I think the problems
> and risks sufficiently serious, I propose reversing the change until this
> can be done. We need a quick decision on this to get it into 5.2-RELEASE.
>
> The PR has some more detail and tcpdump output demonstrating the
> issue:
>
> http://www.freebsd.org/cgi/query-pr.cgi?pr=kern/60889
>
> The change to be reversed can be seen at:
>
> http://www.freebsd.org/cgi/cvsweb.cgi/src/sys/netinet/ip_output.c#rev1.189
> http://www.freebsd.org/cgi/cvsweb.cgi/src/sys/netinet/ip_output.c.diff?r1=1.188&r2=1.189
>
> NB If anyone can easily run tests with Windows95/2000 TCP VJ compression that
> would be great (using tcpdump to see if there is abnormal retransmission).
>
> NB2 If anyone knows why FIN-ACK goes out without DF that would be helpful.
> I've quickly looked at the source, and I can't see why. It's emitted
> as TCP moves from FIN_WAIT_n state to TIME_WAIT probably at line 3091 of
> tcp_input.c with a call to tcp_output(). tcp_output() always appears to
> set IP_DF at line 998 of tcp_output.c, if path_mtu_discovery is enabled.
> A puzzle.
>
> Thanks to Boris Staeblow <bs at dva.in-berlin.de> and Tim Rylance
> <tkr at puffball.demon.co.uk> for highlighting this change and helping me
> diagnose the issues.
>
> Richard
> --
> Richard Wendland richard at wendland.org.uk
> _______________________________________________
> freebsd-net at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-net
> To unsubscribe, send any mail to "freebsd-net-unsubscribe at freebsd.org"
More information about the freebsd-net
mailing list