[PATCH] ip_nat_mangle_udp_packet() updated

[PATCH] ip_nat_mangle_udp_packet() updated

[netfilter]

[-- Attachment #1.1: Type: text/plain, Size: 389 bytes --]

Hello Harald and all.

Find attached an updated patch for my ip_nat_mangle_udp_packet().
This function no longer gratuitously checkums the mangled UDP packet.
It only calculated a UDP checksum when the pre-mangling checksum is
non-zero.  It still pushes the packet up for IP checksum however.  I
believe this to be the correct way to do this.

Thanx,
b.

-- 
Brian J. Murrell

[-- Attachment #1.2: udp_conntrack-2.patch --]
[-- Type: text/plain, Size: 9199 bytes --]

diff -Nur --exclude '*~' --exclude '*.orig' linux-2.4.19-16mdk/include/linux/netfilter_ipv4/ip_nat_helper.h linux-2.4.19-16mdk.tmp/include/linux/netfilter_ipv4/ip_nat_helper.h
--- linux-2.4.19-16mdk/include/linux/netfilter_ipv4/ip_nat_helper.h	2002-10-09 04:52:29.000000000 -0400
+++ linux-2.4.19-16mdk.tmp/include/linux/netfilter_ipv4/ip_nat_helper.h	2002-10-17 11:07:43.000000000 -0400
@@ -50,6 +50,13 @@
 				unsigned int match_len,
 				char *rep_buffer,
 				unsigned int rep_len);
+extern int ip_nat_mangle_udp_packet(struct sk_buff **skb,
+				struct ip_conntrack *ct,
+				enum ip_conntrack_info ctinfo,
+				unsigned int match_offset,
+				unsigned int match_len,
+				char *rep_buffer,
+				unsigned int rep_len);
 extern int ip_nat_seq_adjust(struct sk_buff *skb,
 				struct ip_conntrack *ct,
 				enum ip_conntrack_info ctinfo);
diff -Nur --exclude '*~' --exclude '*.orig' linux-2.4.19-16mdk/net/ipv4/netfilter/ip_nat_helper.c linux-2.4.19-16mdk.tmp/net/ipv4/netfilter/ip_nat_helper.c
--- linux-2.4.19-16mdk/net/ipv4/netfilter/ip_nat_helper.c	2002-09-20 09:44:35.000000000 -0400
+++ linux-2.4.19-16mdk.tmp/net/ipv4/netfilter/ip_nat_helper.c	2002-10-17 11:15:35.000000000 -0400
@@ -8,6 +8,9 @@
  *		- add support for SACK adjustment 
  *	14 Mar 2002 Harald Welte <laforge@gnumonks.org>:
  *		- merge SACK support into newnat API
+ *	16 Aug 2002 Brian J. Murrell <netfilter@interlinx.bc.ca>:
+ *		- make ip_nat_resize_packet more generic (TCP and UDP)
+ *		- add ip_nat_mangle_udp_packet
  */
 #include <linux/version.h>
 #include <linux/config.h>
@@ -22,6 +25,7 @@
 #include <net/icmp.h>
 #include <net/ip.h>
 #include <net/tcp.h>
+#include <net/udp.h>
 
 #define ASSERT_READ_LOCK(x) MUST_BE_READ_LOCKED(&ip_nat_lock)
 #define ASSERT_WRITE_LOCK(x) MUST_BE_WRITE_LOCKED(&ip_nat_lock)
@@ -51,18 +55,12 @@
 		     int new_size)
 {
 	struct iphdr *iph;
-	struct tcphdr *tcph;
-	void *data;
 	int dir;
 	struct ip_nat_seq *this_way, *other_way;
 
 	DEBUGP("ip_nat_resize_packet: old_size = %u, new_size = %u\n",
 		(*skb)->len, new_size);
 
-	iph = (*skb)->nh.iph;
-	tcph = (void *)iph + iph->ihl*4;
-	data = (void *)tcph + tcph->doff*4;
-
 	dir = CTINFO2DIR(ctinfo);
 
 	this_way = &ct->nat.info.seq[dir];
@@ -84,37 +82,41 @@
 	}
 
 	iph = (*skb)->nh.iph;
-	tcph = (void *)iph + iph->ihl*4;
-	data = (void *)tcph + tcph->doff*4;
-
-	DEBUGP("ip_nat_resize_packet: Seq_offset before: ");
-	DUMP_OFFSET(this_way);
+	if (iph->protocol == IPPROTO_TCP) {
+		struct tcphdr *tcph = (void *)iph + iph->ihl*4;
+		void *data = (void *)tcph + tcph->doff*4;
+
+		DEBUGP("ip_nat_resize_packet: Seq_offset before: ");
+		DUMP_OFFSET(this_way);
+
+		LOCK_BH(&ip_nat_seqofs_lock);
+
+		/* SYN adjust. If it's uninitialized, of this is after last 
+		 * correction, record it: we don't handle more than one 
+		 * adjustment in the window, but do deal with common case of a 
+		 * retransmit */
+		if (this_way->offset_before == this_way->offset_after
+		    || before(this_way->correction_pos, ntohl(tcph->seq))) {
+			this_way->correction_pos = ntohl(tcph->seq);
+			this_way->offset_before = this_way->offset_after;
+			this_way->offset_after = (int32_t)
+				this_way->offset_before + new_size -
+				(*skb)->len;
+		}
 
-	LOCK_BH(&ip_nat_seqofs_lock);
+		UNLOCK_BH(&ip_nat_seqofs_lock);
 
-	/* SYN adjust. If it's uninitialized, of this is after last 
-	 * correction, record it: we don't handle more than one 
-	 * adjustment in the window, but do deal with common case of a 
-	 * retransmit */
-	if (this_way->offset_before == this_way->offset_after
-	    || before(this_way->correction_pos, ntohl(tcph->seq))) {
-		this_way->correction_pos = ntohl(tcph->seq);
-		this_way->offset_before = this_way->offset_after;
-		this_way->offset_after = (int32_t)
-			this_way->offset_before + new_size - (*skb)->len;
+		DEBUGP("ip_nat_resize_packet: Seq_offset after: ");
+		DUMP_OFFSET(this_way);
 	}
-
-	UNLOCK_BH(&ip_nat_seqofs_lock);
-
-	DEBUGP("ip_nat_resize_packet: Seq_offset after: ");
-	DUMP_OFFSET(this_way);
 	
 	return 1;
 }
 
 
 /* Generic function for mangling variable-length address changes inside
- * NATed connections (like the PORT XXX,XXX,XXX,XXX,XXX,XXX command in FTP).
+ * NATed TCP connections (like the PORT XXX,XXX,XXX,XXX,XXX,XXX
+ * command in FTP).
  *
  * Takes care about all the nasty sequence number changes, checksumming,
  * skb enlargement, ...
@@ -174,10 +176,11 @@
 	tcph = (void *)iph + iph->ihl*4;
 	data = (void *)tcph + tcph->doff*4;
 
-	/* move post-replacement */
-	memmove(data + match_offset + rep_len,
-		 data + match_offset + match_len,
-		 (*skb)->tail - (data + match_offset + match_len));
+	if (rep_len != match_len)
+		/* move post-replacement */
+		memmove(data + match_offset + rep_len,
+			data + match_offset + match_len,
+			(*skb)->tail - (data + match_offset + match_len));
 
 	/* insert data from buffer */
 	memcpy(data + match_offset, rep_buffer, rep_len);
@@ -207,6 +210,114 @@
 
 	return 1;
 }
+			
+/* Generic function for mangling variable-length address changes inside
+ * NATed UDP connections (like the CONNECT DATA XXXXX MESG XXXXX INDEX XXXXX
+ * command in the Amanda protocol)
+ *
+ * Takes care about all the nasty sequence number changes, checksumming,
+ * skb enlargement, ...
+ *
+ * XXX - This function could be merged with ip_nat_mangle_tcp_packet which
+ *       should be fairly easy to do.
+ */
+int 
+ip_nat_mangle_udp_packet(struct sk_buff **skb,
+			 struct ip_conntrack *ct,
+			 enum ip_conntrack_info ctinfo,
+			 unsigned int match_offset,
+			 unsigned int match_len,
+			 char *rep_buffer,
+			 unsigned int rep_len)
+{
+	struct iphdr *iph = (*skb)->nh.iph;
+	struct udphdr *udph = (void *)iph + iph->ihl * 4;
+	unsigned char *data;
+	u_int32_t udplen, newlen, newudplen;
+
+	udplen = (*skb)->len - iph->ihl*4;
+	newudplen = udplen - match_len + rep_len;
+	newlen = iph->ihl*4 + newudplen;
+
+	if (newlen > 65535) {
+		if (net_ratelimit())
+			printk("ip_nat_mangle_udp_packet: nat'ed packet "
+				"exceeds maximum packet size\n");
+		return 0;
+	}
+
+	if ((*skb)->len != newlen) {
+		if (!ip_nat_resize_packet(skb, ct, ctinfo, newlen)) {
+			printk("resize_packet failed!!\n");
+			return 0;
+		}
+	}
+
+	/* Alexey says: if a hook changes _data_ ... it can break
+	   original packet sitting in tcp queue and this is fatal */
+	if (skb_cloned(*skb)) {
+		struct sk_buff *nskb = skb_copy(*skb, GFP_ATOMIC);
+		if (!nskb) {
+			if (net_ratelimit())
+				printk("Out of memory cloning TCP packet\n");
+			return 0;
+		}
+		/* Rest of kernel will get very unhappy if we pass it
+		   a suddenly-orphaned skbuff */
+		if ((*skb)->sk)
+			skb_set_owner_w(nskb, (*skb)->sk);
+		kfree_skb(*skb);
+		*skb = nskb;
+	}
+
+	/* skb may be copied !! */
+	iph = (*skb)->nh.iph;
+	udph = (void *)iph + iph->ihl*4;
+	data = (void *)udph + sizeof(struct udphdr);
+
+	if (rep_len != match_len)
+		/* move post-replacement */
+		memmove(data + match_offset + rep_len,
+			data + match_offset + match_len,
+			(*skb)->tail - (data + match_offset + match_len));
+
+	/* insert data from buffer */
+	memcpy(data + match_offset, rep_buffer, rep_len);
+
+	/* update skb info */
+	if (newlen > (*skb)->len) {
+		DEBUGP("ip_nat_mangle_udp_packet: Extending packet by "
+			"%u to %u bytes\n", newlen - (*skb)->len, newlen);
+		skb_put(*skb, newlen - (*skb)->len);
+	} else {
+		DEBUGP("ip_nat_mangle_udp_packet: Shrinking packet from "
+			"%u to %u bytes\n", (*skb)->len, newlen);
+		skb_trim(*skb, newlen);
+	}
+
+	/* update the length of the UDP and IP packets to the new values*/
+	udph->len = htons((*skb)->len - iph->ihl*4);
+	iph->tot_len = htons(newlen);
+
+	/* fix udp checksum if udp checksum was previously calculated */
+	if ((*skb)->csum != 0) {
+		(*skb)->csum = csum_partial((char *)udph +
+					    sizeof(struct udphdr),
+					    newudplen - sizeof(struct udphdr),
+					    0);
+
+		udph->check = 0;
+		udph->check = csum_tcpudp_magic(iph->saddr, iph->daddr,
+						newudplen, IPPROTO_UDP,
+						csum_partial((char *)udph,
+							 sizeof(struct udphdr),
+							(*skb)->csum));
+	}
+
+	ip_send_check(iph);
+
+	return 1;
+}
 
 /* Adjust one found SACK option including checksum correction */
 static void
diff -Nur --exclude '*~' --exclude '*.orig' linux-2.4.19-16mdk/net/ipv4/netfilter/ip_nat_standalone.c linux-2.4.19-16mdk.tmp/net/ipv4/netfilter/ip_nat_standalone.c
--- linux-2.4.19-16mdk/net/ipv4/netfilter/ip_nat_standalone.c	2002-09-20 09:44:36.000000000 -0400
+++ linux-2.4.19-16mdk.tmp/net/ipv4/netfilter/ip_nat_standalone.c	2002-10-17 11:16:00.000000000 -0400
@@ -358,5 +358,6 @@
 EXPORT_SYMBOL(ip_nat_helper_unregister);
 EXPORT_SYMBOL(ip_nat_cheat_check);
 EXPORT_SYMBOL(ip_nat_mangle_tcp_packet);
+EXPORT_SYMBOL(ip_nat_mangle_udp_packet);
 EXPORT_SYMBOL(ip_nat_used_tuple);
 MODULE_LICENSE("GPL");
Binary files linux-2.4.19-16mdk/net/ipv4/netfilter/.ip_nat_standalone.c.swp and linux-2.4.19-16mdk.tmp/net/ipv4/netfilter/.ip_nat_standalone.c.swp differ

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

[PATCH] tcp window tracking patch with SACK support

[Jozsef Kadlecsik]

Hello,

As it was reported several times on the mailing list, the tcp window
tracking patch could cause long SMTP delays in some cases.

After dumping a lot of traffic and replaying the streams it turned
out that the book-keeping inside the patch could go out of sync when SACK
was used. In another words, SACK was not supported by the patch and it
could cause legitimate packets to be blocked.

I have committed the new tcp-window-tracking.patch with SACK support in
the cvs.

It'd be very helpful if sites which experienced the long SMTP delays could
give a try to the new version of the patch and then send a report: 'yeah,
it's seems to be OK' or 'nope, still bad'. :-)

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlec@sunserv.kfki.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics
          H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH] tcp window tracking patch with SACK support

[Harald Welte]

On Fri, Oct 18, 2002 at 01:09:43PM +0200, Jozsef Kadlecsik wrote:
> Hello,
> 
> As it was reported several times on the mailing list, the tcp window
> tracking patch could cause long SMTP delays in some cases.
> 
> After dumping a lot of traffic and replaying the streams it turned
> out that the book-keeping inside the patch could go out of sync when SACK
> was used. In another words, SACK was not supported by the patch and it
> could cause legitimate packets to be blocked.

Mmmh.. I see. 

> I have committed the new tcp-window-tracking.patch with SACK support in
> the cvs.

great, thanks.

> It'd be very helpful if sites which experienced the long SMTP delays could
> give a try to the new version of the patch and then send a report: 'yeah,
> it's seems to be OK' or 'nope, still bad'. :-)

well, I'd rather not test it on kashyyyk.netfilter.org for some time...
so I really have to rely on others for testing. Please tell me about
your erxperience with this new patch.

> Best regards,
> Jozsef

-- 
Live long and prosper
- Harald Welte / laforge@gnumonks.org               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+(*)

Re: [PATCH] tcp window tracking patch with SACK support

[Roberto Nibali]

Hello,

>>As it was reported several times on the mailing list, the tcp window
>>tracking patch could cause long SMTP delays in some cases.
>>
>>After dumping a lot of traffic and replaying the streams it turned
>>out that the book-keeping inside the patch could go out of sync when SACK
>>was used. In another words, SACK was not supported by the patch and it
>>could cause legitimate packets to be blocked.

First I wasn't able to reproduce it on a normal box with low traffic but then I 
asked our management if I could test it out on one of our main packetfilters and 
after 3 minutes I saw those strange DROPs and retransmits with interesting TCP 
flag combinations indicating the SACK. While this firewall has a steady 30Mbit/s 
  sustained filtering rate 7/24 with 8 interfaces and around 1200 rules, I 
expected some rules not to work because not only did I use the tcp window 
tracking patch but the packetfilter was still running ipchains with a 2.2.x kernel.

To get to the point: All rules were transitioned (I added the MetaFW layer for 
iptables to our fw suite) and worked very well but the SMTP ones somehow 
generated too many DROPs. I accredit this to the missing SACK support in the tcp 
window tracking patch. I wasn't able to tcpdump nor trace packets back in 
another way because of strict security constraints (don't ask).

>>It'd be very helpful if sites which experienced the long SMTP delays could
>>give a try to the new version of the patch and then send a report: 'yeah,
>>it's seems to be OK' or 'nope, still bad'. :-)
>  
> well, I'd rather not test it on kashyyyk.netfilter.org for some time...

Actually you should, because the problem will be spotted very soon and you can 
then quickly failback to the old kernel without pissing off too many people.

> so I really have to rely on others for testing. Please tell me about
> your erxperience with this new patch.

I will now (after I have more or less proven that without this SACK patch we're 
running into problems too) patch my kernel and try it again. Unfortunately I 
have only a very limited test period because if I screw up 30 fulltime engineers 
won't be able to work properly. My initial test which only lasted for 20 minutes 
cost us around $4000. I will report back as soon as I have done testing with 
this new patch.

Regards,
Roberto Nibali, ratz
-- 
echo '[q]sa[ln0=aln256%Pln256/snlbx]sb3135071790101768542287578439snlbxq' | dc

Re: [PATCH] tcp window tracking patch with SACK support

[Roberto Nibali]

Sucess report:

> I will now (after I have more or less proven that without this SACK 
> patch we're running into problems too) patch my kernel and try it again. 

Unpatched kernel              : A lot of DROPs
Patched kernel (SACK support) : _No_ DROPs which are illicit drops.

I've run the new packet filter for 16 hours and I am now evaluating 1.2Gbytes of 
logfiles. So far it looks good. It has cured the obvious problems I've had with 
the initial tcp window tracking patch.

I see now however, that there is still one open report from Brian.

Thanks,
Roberto Nibali, ratz
-- 
echo '[q]sa[ln0=aln256%Pln256/snlbx]sb3135071790101768542287578439snlbxq' | dc

Re: [PATCH] tcp window tracking patch with SACK support

[netfilter]

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

On Mon, Nov 18, 2002 at 10:48:56AM +0100, Roberto Nibali wrote:
> 
> I see now however, that there is still one open report from Brian.

Actually, I sent a second one to Jozsef by private mail just the other
day.

b.

-- 
Brian J. Murrell

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] ip_nat_mangle_udp_packet() updated

[Harald Welte]

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

On Thu, Oct 17, 2002 at 12:39:09PM -0400, netfilter@interlinx.bc.ca wrote:
> Hello Harald and all.
> 
> Find attached an updated patch for my ip_nat_mangle_udp_packet().
> This function no longer gratuitously checkums the mangled UDP packet.
> It only calculated a UDP checksum when the pre-mangling checksum is
> non-zero.  It still pushes the packet up for IP checksum however.  I
> believe this to be the correct way to do this.

This patch looks fine to me. I will put it into the pending repository
of patch-o-matic.

> Thanx,

Thank you!

-- 
Live long and prosper
- Harald Welte / laforge@gnumonks.org               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+(*)

[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]