ECN target bug report

ECN target bug report

[Andrea Rossato]

below you will find a coupe of emails sent to this list at the end of 
september.

the first one states that there is a problem with tcp checksum in the 
case a packet had been stripped of ecn bits. The problem was also 
reported by Graham Murray in Agust.

the answer was that this is due to tcpdump getting a cloned copy of the 
packet:
now, if I send tcp packet stripped with -ecn-tcp-remove to a box and i 
dump packets there, tcp checksum is incorrect and the box will be not 
respondig. If I remove the rule, packets are getting there with the 
correct checksum and the box responds.

What's interesting is that if I put these rules:

iptables -A OUTPUT -t mangle -o ppp0 -p tcp -d my.host.org --dport 80 -j 
ECN --ecn-tcp-remove
iptables -A OUTPUT -o ppp0 -p tcp -d my.host.org --dport 80 -m unclean 
-j DROP

packets will be actually dropped! something strange for being normal, 
isn't it? or iptables treats as unclean ecn stripped packets, and this 
is supposed to be normal? anyway iptables seems not to be the only one, 
so ECN target is actually preatty useless.

(using linux-2.4.20 and iptables-1.2.7a)

Thanks for you attention.
Andrea



> Subject:-j ECN --ecn-tcp-remove seems to be mangling the TCP checksum...
> From: netfilter@horizon.com
> Date: 27 Sep 2002 06:27:53 -0000
> To: netfilter@lists.netfilter.org

> bash-2.05b# iptables -t mangle -A fix-ecn -d 1.1.1.1 -p tcp -j ECN --ecn-tcp-remove
> bash-2.05b# echo 1 > /proc/sys/net/ipv4/tcp_ecn ; telnet 1.1.1.1 80
> 01:52:20.662338 science.horizon.com.11058 > 1.1.1.1.www: S [bad tcp cksum bf40!] 2655433521:2655433521(0) win 5840 <mss 1460,sackOK,timestamp 14290984 0,nop,wscale 0> (DF) [tos 0x10]  (ttl 64, id 41753, len 60)
>                          4510 003c a319 4000 4006 716c c023 6401
>                          0101 0101 2b32 0050 9e46 b331 0000 0000
>                          a002 16d0 3c55 0000 0204 05b4 0402 080a
>                          00da 1028 0000 0000 0103 0300
> 
> Now I'll turn tcp_ecn off again:
> bash-2.05b# echo 0 > /proc/sys/net/ipv4/tcp_ecn ; telnet 1.1.1.1 80
> 01:52:36.771155 science.horizon.com.11059 > 1.1.1.1.www: S [tcp sum ok] 2671050014:2671050014(0) win 5840 <mss 1460,sackOK,timestamp 14292595 0,nop,wscale 0> (DF) [tos 0x10]  (ttl 64, id 60269, len 60)
>                          4510 003c eb6d 4000 4006 2918 c023 6401
>                          0101 0101 2b33 0050 9f34 fd1e 0000 0000
>                          a002 16d0 2bed 0000 0204 05b4 0402 080a
>                          00da 1673 0000 0000 0103 0300
> 
> Notice the bad tcp checksum in the third case.

> Subject: Re: -j ECN --ecn-tcp-remove seems to be mangling the TCP checksum...
> From: Maciej Soltysiak <solt@dns.toxicfilms.tv>
> Date: Mon, 30 Sep 2002 11:55:56 +0200 (CEST)
> To: netfilter@horizon.com
> CC: netfilter@lists.netfilter.org

>>> Is this a bug?  The ipt_ECN.c file is
>>> ipt_ECN.c,v 1.4 2002/08/05 19:36:51 laforge Exp
> 
> No it is not. Do the same with a remote host.
> Send a ECNstripped packets to some other host, and tcpdump there.
> The checksum will be ok.
> It is the problem with tcpdump getting a cloned copy of the packet,
> read the RR's FIXME notes in netfilter sources about it.
> 
> I noticed that too, once, and thought it's a checksum calculation bug.
> Maciej Soltysiak

Re: ECN target bug report

[Andrea Rossato]

[-- Attachment #1: Type: text/plain, Size: 1685 bytes --]

attached you will find 2 patches. the first one is just a hack that 
provides a solution, the second is an attempt to prove the existence of 
the problem.
this is far from being the solution, it's just a workaround: checksum of 
the packets will be recalculated from scratch. this means that unclean 
packets with ec e cwr bits set will be cleaned after passing through 
this rule. that's the reason for partial checksum recalculation: if a 
packet was unclean it must remain as such.

The problem, as far as I can see it, could be located in csum_partial 
(arch/i386/checksum.S): i'm not a kernel hacker (i'm a lawyer, a legal 
scholar actually), but i do not see any mistake in the way partial 
checksum is carried out in tcp_etc_set. anyway checksum after partial or 
total recalculation differ. That's a fact. Evidence of the fact  can be 
gained with the second patch: in this case the kernel will log the 
checksum after partial recalculation and after total recalculation (that 
means that two calculations will take place). The two values differ!

now, is this the right place for such a bug submission? is this code 
manteined? should i refer to the kernel mailing list?

If someone could give me directions I would very much appreciate. I 
would like to see this problem solved.
Thanks for your attention.

1. check the bug:
echo 1 /proc/sys/net/ipv4/tcp_ecn
iptables -A OUTPUT -t mangle -o ppp0 -p tcp -d my.host.org --dport 80 -j 
ECN --ecn-tcp-remove
iptables -A OUTPUT -o ppp0 -p tcp -d my.host.org --dport 80 -m unclean 
-j DROP
packets will be dropped


2. apply one of the patches and try again:
packets will get though and the connection will be established.

andrea

[-- Attachment #2: ecn_csum.patch --]
[-- Type: text/plain, Size: 1224 bytes --]

--- linux-2.4.20/net/ipv4/netfilter/ipt_ECN.c.orig	2002-12-07 23:22:37.000000000 +0100
+++ linux-2.4.20/net/ipv4/netfilter/ipt_ECN.c	2002-12-09 10:13:56.000000000 +0100
@@ -11,6 +11,7 @@
 #include <linux/skbuff.h>
 #include <linux/ip.h>
 #include <net/checksum.h>
+#include <net/tcp.h>
 
 #include <linux/netfilter_ipv4/ip_tables.h>
 #include <linux/netfilter_ipv4/ipt_ECN.h>
@@ -62,6 +63,7 @@
 	struct tcphdr *tcph = (void *) iph + iph->ihl * 4;
 	u_int16_t *tcpflags = (u_int16_t *)tcph + 6;
 	u_int16_t diffs[2];
+	u_int32_t tcplen;
 
 	/* raw socket (tcpdump) may have clone of incoming
 	 * skb: don't disturb it --RR */
@@ -87,13 +89,14 @@
 	}
 	
 	if (diffs[0] != *tcpflags) {
-		diffs[0] = htons(diffs[0]) ^ 0xFFFF;
-		diffs[1] = htons(*tcpflags);
-		tcph->check = csum_fold(csum_partial((char *)diffs,
-		                                    sizeof(diffs),
-		                                    tcph->check^0xFFFF));
-		(*pskb)->nfcache |= NFC_ALTERED;
 
+		tcplen = (*pskb)->len - iph->ihl*4;
+		tcph->check = 0;
+		tcph->check = tcp_v4_check(tcph, tcplen, iph->saddr, iph->daddr,
+				   csum_partial((char *)tcph, tcph->doff*4,
+					   (*pskb)->csum));
+		(*pskb)->nfcache |= NFC_ALTERED;
+	
 		return 1;
 	}
 

[-- Attachment #3: ecn_csum_with_debug_info.patch --]
[-- Type: text/plain, Size: 1260 bytes --]

--- linux-2.4.20/net/ipv4/netfilter/ipt_ECN.c.orig	2002-12-09 10:44:03.000000000 +0100
+++ linux-2.4.20/net/ipv4/netfilter/ipt_ECN.c	2002-12-09 10:48:46.000000000 +0100
@@ -11,6 +11,7 @@
 #include <linux/skbuff.h>
 #include <linux/ip.h>
 #include <net/checksum.h>
+#include <net/tcp.h>
 
 #include <linux/netfilter_ipv4/ip_tables.h>
 #include <linux/netfilter_ipv4/ipt_ECN.h>
@@ -62,6 +63,7 @@
 	struct tcphdr *tcph = (void *) iph + iph->ihl * 4;
 	u_int16_t *tcpflags = (u_int16_t *)tcph + 6;
 	u_int16_t diffs[2];
+	u_int32_t tcplen;
 
 	/* raw socket (tcpdump) may have clone of incoming
 	 * skb: don't disturb it --RR */
@@ -92,6 +94,17 @@
 		tcph->check = csum_fold(csum_partial((char *)diffs,
 		                                    sizeof(diffs),
 		                                    tcph->check^0xFFFF));
+
+		printk(KERN_WARNING "ECN: checksum after partial recalculation \"%x\"\n", tcph->check );
+		
+		tcplen = (*pskb)->len - iph->ihl*4;
+		tcph->check = 0;
+		tcph->check = tcp_v4_check(tcph, tcplen, iph->saddr, iph->daddr,
+				   csum_partial((char *)tcph, tcph->doff*4,
+					   (*pskb)->csum));
+
+		printk(KERN_WARNING "ECN: checksum after total recalculation \"%x\"\n", tcph->check );
+
 		(*pskb)->nfcache |= NFC_ALTERED;
 
 		return 1;

Re: ECN target bug report

[Andrea Rossato]

[-- Attachment #1: Type: text/plain, Size: 340 bytes --]

attached you will find what could be a suitable, even though temporary, 
solution for ECN target.
a packet with ec and cwr bits set and a bad checksum will not be 
processed. If the checksum is good the bits will be stripped.
andrea

-- 


~  GnuGP public key for arossato@istitutocolli.org
~      http://www.istitutocolli.org/pubkey.asc



[-- Attachment #2: ecn_checksum.patch --]
[-- Type: text/plain, Size: 1520 bytes --]

--- linux-2.4.20/net/ipv4/netfilter/ipt_ECN.c.orig	2002-12-09 10:44:03.000000000 +0100
+++ linux-2.4.20/net/ipv4/netfilter/ipt_ECN.c	2002-12-09 17:16:11.000000000 +0100
@@ -11,6 +11,7 @@
 #include <linux/skbuff.h>
 #include <linux/ip.h>
 #include <net/checksum.h>
+#include <net/tcp.h>
 
 #include <linux/netfilter_ipv4/ip_tables.h>
 #include <linux/netfilter_ipv4/ipt_ECN.h>
@@ -62,6 +63,7 @@
 	struct tcphdr *tcph = (void *) iph + iph->ihl * 4;
 	u_int16_t *tcpflags = (u_int16_t *)tcph + 6;
 	u_int16_t diffs[2];
+	u_int32_t tcplen;
 
 	/* raw socket (tcpdump) may have clone of incoming
 	 * skb: don't disturb it --RR */
@@ -74,6 +76,15 @@
 		iph = (*pskb)->nh.iph;
 	}
 
+
+	/* Checksum invalid?  Ignore. */
+	/* FIXME: Source route IP option packets --RR */
+	tcplen = (*pskb)->len - iph->ihl*4;
+	if (tcp_v4_check(tcph, tcplen, iph->saddr, iph->daddr,
+			 csum_partial((char *) tcph, tcplen, 0))) {
+		return 0;
+	}
+
 	diffs[0] = *tcpflags;
 
 	if (einfo->operation & IPT_ECN_OP_SET_ECE
@@ -87,13 +98,12 @@
 	}
 	
 	if (diffs[0] != *tcpflags) {
-		diffs[0] = htons(diffs[0]) ^ 0xFFFF;
-		diffs[1] = htons(*tcpflags);
-		tcph->check = csum_fold(csum_partial((char *)diffs,
-		                                    sizeof(diffs),
-		                                    tcph->check^0xFFFF));
+		tcph->check = 0;
+		tcph->check = tcp_v4_check(tcph, tcplen, iph->saddr, iph->daddr,
+				   csum_partial((char *)tcph, tcph->doff*4,
+					   (*pskb)->csum));
 		(*pskb)->nfcache |= NFC_ALTERED;
-
+	
 		return 1;
 	}
 

Re: ECN target bug report

[Andrea Rossato]

[-- Attachment #1: Type: text/plain, Size: 262 bytes --]

attached you will find what could be a suitable, even though temporary,
solution for ECN target.
a packet with ec and cwr bits set and a bad checksum will not be
processed. If the checksum is good the bits will be stripped and a new 
checksum calculated.
andrea

[-- Attachment #2: ecn_checksum.patch --]
[-- Type: text/plain, Size: 1521 bytes --]

--- linux-2.4.20/net/ipv4/netfilter/ipt_ECN.c.orig	2002-12-09 10:44:03.000000000 +0100
+++ linux-2.4.20/net/ipv4/netfilter/ipt_ECN.c	2002-12-09 17:16:11.000000000 +0100
@@ -11,6 +11,7 @@
 #include <linux/skbuff.h>
 #include <linux/ip.h>
 #include <net/checksum.h>
+#include <net/tcp.h>
 
 #include <linux/netfilter_ipv4/ip_tables.h>
 #include <linux/netfilter_ipv4/ipt_ECN.h>
@@ -62,6 +63,7 @@
 	struct tcphdr *tcph = (void *) iph + iph->ihl * 4;
 	u_int16_t *tcpflags = (u_int16_t *)tcph + 6;
 	u_int16_t diffs[2];
+	u_int32_t tcplen;
 
 	/* raw socket (tcpdump) may have clone of incoming
 	 * skb: don't disturb it --RR */
@@ -74,6 +76,15 @@
 		iph = (*pskb)->nh.iph;
 	}
 
+
+	/* Checksum invalid?  Ignore. */
+	/* FIXME: Source route IP option packets --RR */
+	tcplen = (*pskb)->len - iph->ihl*4;
+	if (tcp_v4_check(tcph, tcplen, iph->saddr, iph->daddr,
+			 csum_partial((char *) tcph, tcplen, 0))) {
+		return 0;
+	}
+
 	diffs[0] = *tcpflags;
 
 	if (einfo->operation & IPT_ECN_OP_SET_ECE
@@ -87,13 +98,12 @@
 	}
 	
 	if (diffs[0] != *tcpflags) {
-		diffs[0] = htons(diffs[0]) ^ 0xFFFF;
-		diffs[1] = htons(*tcpflags);
-		tcph->check = csum_fold(csum_partial((char *)diffs,
-		                                    sizeof(diffs),
-		                                    tcph->check^0xFFFF));
+		tcph->check = 0;
+		tcph->check = tcp_v4_check(tcph, tcplen, iph->saddr, iph->daddr,
+				   csum_partial((char *)tcph, tcph->doff*4,
+					   (*pskb)->csum));
 		(*pskb)->nfcache |= NFC_ALTERED;
-
+	
 		return 1;
 	}