Subject: Re: kern/7368: ipnat not rewriting PORT command 100% of time
To: Andrew Brown <atatat@atatdot.net>
From: Olaf Seibert <rhialto@polder.ubc.kun.nl>
List: netbsd-bugs
Date: 04/16/1999 14:28:14
On Thu, 15 Apr 1999, Olaf Seibert wrote:

> I found out that the NAT thinks the (TCP) checksum of the packet is
> bad. This happens in ip_proxy.c: ap_check(). Of course the checksum
> is really OK, since the other end accepts the packets. An obvious
> candidate for confusing the fr_tcpsum() function is the presence of
> options. Unfortunately, the fr_tcpsum() function still looks a bit
> bewildering to me so I can't say for sure, or suggest a fix yet.

After looking at the source, and wondering how it could ever have
worked, it dawned on me that it actually didn't. Indeed, -current has
some small but important differences in the checksum code here. I pulled
these into 1.3.3, and then it worked.

I propose that the fix below (properly cleaned up) be added to the
NetBSD-1.3.3/source/patches directory.

Pseudo diffs (diffs with stuff removed, so they probably won't patch
automatically):

*** fil.c	Fri Apr 16 11:40:11 1999
--- /home/users/rhialto/cpp/netbsd/src/sys/netinet/fil.c	Wed Feb  3 13:24:20 1999
***************
*** 1,4 ****
! /*	$NetBSD: fil.c,v 1.15.2.6 1998/11/29 03:19:18 cgd Exp $	*/
  
  /*
   * Copyright (C) 1993-1998 by Darren Reed.
--- 1,4 ----
! /*	$NetBSD: fil.c,v 1.27 1999/02/02 19:57:30 cjs Exp $	*/
  
  /*
   * Copyright (C) 1993-1998 by Darren Reed.
***************
*** 963,969 ****
  	ipp->ip_src = ip->ip_src;
  	ipp->ip_dst = ip->ip_dst;
  	ipp->ip_p = ip->ip_p;
! 	ipp->ip_len = htons(ip->ip_len);
  	m->m_len -= hlen;
  # if BSD >= 199306
  	m->m_data += hlen;
--- 976,982 ----
  	ipp->ip_src = ip->ip_src;
  	ipp->ip_dst = ip->ip_dst;
  	ipp->ip_p = ip->ip_p;
! 	ipp->ip_len = htons(ip->ip_len - hlen);
  	m->m_len -= hlen;
  # if BSD >= 199306
  	m->m_data += hlen;
***************
*** 1017,1024 ****
  	sum += *sp++;	/* ack */
  	sum += *sp++;
  	sum += *sp++;	/* off */
! 	sum += *sp;	/* win */
! 	sp += 2;	/* Skip over checksum */
  	sum += *sp++;	/* urp */
  
  # if SOLARIS
--- 1030,1037 ----
  	sum += *sp++;	/* ack */
  	sum += *sp++;
  	sum += *sp++;	/* off */
! 	sum += *sp++;	/* win */
! 	sum += *sp++;	/* sum */
  	sum += *sp++;	/* urp */
  
  # if SOLARIS
*** /home/users/rhialto/cpp/netbsd/src/sys/netinet/ip_ftp_pxy.c	Mon Nov 23 13:15:09 1998
--- ip_ftp_pxy.c	Fri Apr 16 11:40:17 1999
***************
*** 144,154 ****
  	/*
  	 * check for CR-LF at the end.
  	 */
  	if (((*s == '\r') && (*(s + 1) == '\n')) ||
  	    ((*(s - 1) == '\r') && (*s == '\n')))
  		a6 = a5 & 0xff;
  	else
  		return 0;
  	a5 >>= 8;
  	/*
  	 * Calculate new address parts for PORT command
--- 160,173 ----
  	/*
  	 * check for CR-LF at the end.
  	 */
+ #if 0
  	if (((*s == '\r') && (*(s + 1) == '\n')) ||
  	    ((*(s - 1) == '\r') && (*s == '\n')))
  		a6 = a5 & 0xff;
  	else
  		return 0;
+ #else
+ 		a6 = a5 & 0xff;
+ #endif
  	a5 >>= 8;
  	/*
  	 * Calculate new address parts for PORT command
*** ip_proxy.c	Fri Apr 16 14:20:25 1999
--- /home/users/rhialto/cpp/netbsd/src/sys/netinet/ip_proxy.c	Wed Feb  3 13:24:21 1999
***************
*** 207,213 ****
  			sum = fr_tcpsum(*(mb_t **)fin->fin_mp,
  					ip, tcp, ip->ip_len);
  #endif
! 			if (tcp->th_sum != sum) {
  				frstats[fin->fin_out].fr_tcpbad++;
  				return -1;
  			}
--- 211,217 ----
  			sum = fr_tcpsum(*(mb_t **)fin->fin_mp,
  					ip, tcp, ip->ip_len);
  #endif
! 			if (sum != 0) {
  				frstats[fin->fin_out].fr_tcpbad++;
  				return -1;
  			}
***************
*** 225,230 ****
--- 229,235 ----
  
  		if (tcp != NULL) {
  			err = ap_fixseqack(fin, ip, aps, err);
+ 			tcp->th_sum = 0;
  #if SOLARIS && defined(_KERNEL)
  			tcp->th_sum = fr_tcpsum(fin->fin_qfm, ip, tcp,
  					        ip->ip_len);

-Olaf.
--
___ Olaf 'Rhialto' Seibert - rhialto@polder.ubc. ---- Unauthorized duplication,
\X/ .kun.nl ---- while sometimes necessary, is never as good as the real thing.