On Fri, Aug 03, 2001 at 01:46:21PM +1000, Rusty Russell wrote:
> Argh... there *is* a check in ip_conntrack_proto_tcp, but it checks
> against tcph->doff, not sizeof(struct tcphdr)!
Sorry Rusty, but check on sizeof(struct tcphdr) is IMHO wrong, again.
- scenario a
Imagine the case, where we have the first 18 bytes of the tcp header,
including the checksum, but not including the urgp. Result: Your check
decides the packet is too small to recalculate the checksum, so the packet
will get sent out with the old checksum :(
- scenario b
Imagine the case, where we only have the first couple of bytes of the inner
packet, not including the checksum of the tcp header. Result: Your check
decides the patcket is too small and the code will thus not change the port
numbers inside the tcp header. The icmp packet gets forwarded, reaches the
destination host. The destination host parses the tcp header, reads out the
port number, and reads the wrong, un-nat'ed one.
Please look at the attached patch, which is the new proposed fix, as in
current netfilter CVS patch-o-matic (tcp_manip_pkt.patch).
> Rusty.
--
Live long and prosper
- Harald Welte / laforge@xxxxxxxxxxxx http://www.gnumonks.org
============================================================================
GCS/E/IT d- s-: a-- C+++ UL++++$ P+++ L++++$ E--- W- N++ o? K- w--- O- M-
V-- PS+ PE-- Y+ PGP++ t++ 5-- !X !R tv-- b+++ DI? !D G+ e* h+ r% y+(*)
--- linux-2.4.7-mb/net/ipv4/netfilter/ip_nat_proto_tcp.c 2001/07/31
15:37:45 1.1
+++ linux-2.4.7-mb/net/ipv4/netfilter/ip_nat_proto_tcp.c 2001/07/31
17:35:20
@@ -92,10 +104,17 @@
oldip = iph->daddr;
portptr = &hdr->dest;
}
- hdr->check = ip_nat_cheat_check(~oldip, manip->ip,
+
+ /* this could be a inner header returned in icmp packet; in such
+ cases we cannot update the checksum field since it is outside of
+ the 64 bits of transport layer headers typically included */
+ if(((void *)&hdr->check + sizeof(hdr->check) - (void *)iph) <= len) {
+ hdr->check = ip_nat_cheat_check(~oldip, manip->ip,
ip_nat_cheat_check(*portptr ^ 0xFFFF,
manip->u.tcp.port,
hdr->check));
+ }
+
*portptr = manip->u.tcp.port;
}
|