Subject: Re: kern/36309
To: None <ipf-bug-people@NetBSD.org, gnats-admin@netbsd.org,>
From: Darren Reed <darrenr@NetBSD.org>
List: netbsd-bugs
Date: 05/31/2007 08:40:08
The following reply was made to PR kern/36309; it has been noted by GNATS.

From: Darren Reed <darrenr@NetBSD.org>
To: Michael van Elst <mlelstv@serpens.de>
Cc: gnats-bugs@netbsd.org
Subject: Re: kern/36309
Date: Thu, 31 May 2007 08:38:00 +0000

 Martin,
 
 Yes, the regression test (ni2) expects the wrong output - I've just
 been looking at it in a bit more depth to see what the cause was and
 you're right.  My current patch is below.  I don't like relying on
 oip_dst == nat_oip in every case, lest something strange happen, like
 nat_oip == nat_outip, etc.
 
 If you can test the patch below successfully, I'll commit it.
 
 Thanks for your help.
 
 Cheers,
 Darren
 
 Index: ip_nat.c
 ===================================================================
 RCS file: /devel/CVS/IP-Filter/ip_nat.c,v
 retrieving revision 2.195.2.86
 diff -c -r2.195.2.86 ip_nat.c
 *** ip_nat.c	28 May 2007 11:56:23 -0000	2.195.2.86
 --- ip_nat.c	31 May 2007 08:20:32 -0000
 ***************
 *** 2814,2821 ****
   {
   	u_32_t sum1, sum2, sumd, sumd2;
   	struct in_addr a1, a2;
   	icmphdr_t *icmp;
 - 	int flags, dlen;
   	u_short *csump;
   	tcphdr_t *tcp;
   	nat_t *nat;
 --- 2814,2821 ----
   {
   	u_32_t sum1, sum2, sumd, sumd2;
   	struct in_addr a1, a2;
 + 	int flags, dlen, odst;
   	icmphdr_t *icmp;
   	u_short *csump;
   	tcphdr_t *tcp;
   	nat_t *nat;
 ***************
 *** 2877,2885 ****
   	 * two changes cancel each other out (if the delta for
   	 * the IP address is x, then the delta for ip_sum is minus x),
   	 * so no change in the icmp_cksum is necessary.
   	 */
 ! 
 ! 	if (nat->nat_dir == NAT_OUTBOUND) {
   		a1.s_addr = ntohl(nat->nat_inip.s_addr);
   		a2.s_addr = ntohl(oip->ip_src.s_addr);
   		oip->ip_src.s_addr = htonl(a1.s_addr);
 --- 2877,2906 ----
   	 * two changes cancel each other out (if the delta for
   	 * the IP address is x, then the delta for ip_sum is minus x),
   	 * so no change in the icmp_cksum is necessary.
 + 	 *
 + 	 * Inbound ICMP
 + 	 * ------------
 + 	 * MAP rule, SRC=a,DST=b -> SRC=c,DST=b
 + 	 * - response to outgoing packet (a,b)=>(c,b) (OIP_SRC=c,OIP_DST=b)
 + 	 * - OIP_SRC(c)=nat_outip, OIP_DST(b)=nat_oip
 + 	 *
 + 	 * RDR rule, SRC=a,DST=b -> SRC=a,DST=c
 + 	 * - response to outgoing packet (c,a)=>(b,a) (OIP_SRC=b,OIP_DST=a)
 + 	 * - OIP_SRC(b)=nat_outip, OIP_DST(a)=nat_oip
 + 	 *
 + 	 * Outbound ICMP
 + 	 * -------------
 + 	 * MAP rule, SRC=a,DST=b -> SRC=c,DST=b
 + 	 * - response to incoming packet (b,c)=>(b,a) (OIP_SRC=b,OIP_DST=a)
 + 	 * - OIP_SRC(a)=nat_oip, OIP_DST(c)=nat_inip
 + 	 *
 + 	 * RDR rule, SRC=a,DST=b -> SRC=a,DST=c
 + 	 * - response to incoming packet (a,b)=>(a,c) (OIP_SRC=a,OIP_DST=c)
 + 	 * - OIP_SRC(a)=nat_oip, OIP_DST(c)=nat_inip
 + 	 *
   	 */
 ! 	odst = (oip->ip_dst.s_addr == nat->nat_oip.s_addr) ? 1 : 0;
 ! 	if (odst == 1) {
   		a1.s_addr = ntohl(nat->nat_inip.s_addr);
   		a2.s_addr = ntohl(oip->ip_src.s_addr);
   		oip->ip_src.s_addr = htonl(a1.s_addr);
 ***************
 *** 2920,2926 ****
   		 * checksum will also need to change if there has been an
   		 * IP address change.
   		 */
 ! 		if (nat->nat_dir == NAT_OUTBOUND) {
   			sum1 = ntohs(nat->nat_inport);
   			sum2 = ntohs(tcp->th_sport);
   
 --- 2941,2947 ----
   		 * checksum will also need to change if there has been an
   		 * IP address change.
   		 */
 ! 		if (odst == 1) {
   			sum1 = ntohs(nat->nat_inport);
   			sum2 = ntohs(tcp->th_sport);
   
 ***************
 *** 2982,2988 ****
   		 */
   		orgicmp = (icmphdr_t *)dp;
   
 ! 		if (nat->nat_dir == NAT_OUTBOUND) {
   			if (orgicmp->icmp_id != nat->nat_inport) {
   
   				/*
 --- 3003,3009 ----
   		 */
   		orgicmp = (icmphdr_t *)dp;
   
 ! 		if (odst == 1) {
   			if (orgicmp->icmp_id != nat->nat_inport) {
   
   				/*