NetBSD Problem Report #50198

From prlw1@cam.ac.uk  Thu Sep  3 17:13:12 2015
Return-Path: <prlw1@cam.ac.uk>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.netbsd.org", Issuer "Postmaster NetBSD.org" (verified OK))
	by mollari.NetBSD.org (Postfix) with ESMTPS id E71E4A6554
	for <gnats-bugs@gnats.NetBSD.org>; Thu,  3 Sep 2015 17:13:12 +0000 (UTC)
Message-Id: <20150903171258.GA8456@quark>
Date: Thu, 3 Sep 2015 18:12:58 +0100
From: Patrick Welche <prlw1@cam.ac.uk>
Reply-To: prlw1@cam.ac.uk
To: gnats-bugs@NetBSD.org
Cc: prlw1@cam.ac.uk
Subject: SIOCGNATL broken in IPFilter 5

>Number:         50198
>Category:       kern
>Synopsis:       SIOCGNATL broken in IPFilter 5
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    ipf-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Sep 03 17:15:00 +0000 2015
>Closed-Date:    
>Last-Modified:  Mon Oct 10 01:38:48 +0000 2016
>Originator:     Patrick Welche
>Release:        NetBSD 7.0_RC3
>Organization:
>Environment:
Tested 5.2_STABLE/i386 vs 7.0_RC3/i386 (and -current/i386)
>Description:
ipnat SIOCGNATL from IPFilter v5 doesn't look up redirected destination
IP addresses correctly, which breaks transparent proxies such as squid.

IPFilter 4 in NetBSD 5 returns:
           real: 192.168.204.87:80
IPFilter 5 in NetBSD 7 returns:
           real: 127.0.0.1:1234
>How-To-Repeat:
Set up a trivial redirect rule:

# cat /etc/ipnat.conf
rdr xennet0 0/0 port 80 -> 127.0.0.1 port 1234 tcp

and run the following server, which listens on port 1234, and does a
SIOCGNATL call:

================================================================================
#include <sys/ioctl.h>
#include <sys/socket.h>
#include <sys/time.h>

#include <arpa/inet.h>
#include <netinet/in.h>
#include <netinet/ipl.h>
#include <netinet/ip_fil.h>
#include <netinet/ip_nat.h>

#include <err.h>
#include <fcntl.h>
#include <stdio.h>
#include <string.h>
#include <unistd.h>

#define MYADDR "127.0.0.1"
#define MYPORT 1234

int main()
{
	int i, sock, con, natfd;
	struct sockaddr_storage connstore;
	struct sockaddr_in lo, *conn;
	socklen_t connlen;
	ipfobj_t obj;
	natlookup_t nat;

	connlen   = sizeof(connstore);
	conn   = (struct sockaddr_in *)&connstore;

	natfd = open("/dev/ipnat", O_RDONLY);
	if (natfd == -1)
		err(1, "/dev/ipnat");

	if (inet_aton(MYADDR, &lo.sin_addr) == 0)
		errx(1,"inet_aton received invalid string");

	lo.sin_len    = sizeof(lo);
	lo.sin_family = AF_INET;
	lo.sin_port   = htons(MYPORT);
	memset(lo.sin_zero, 0, sizeof(lo.sin_zero));

	sock = socket(PF_INET, SOCK_STREAM, IPPROTO_TCP);
	if (bind(sock, (struct sockaddr *)&lo, sizeof(lo)) == -1)
		err(1, "bind");
	if (listen(sock, 10) == -1)
		err(1, "listen");

	printf("IPFilter version %u\n", IPFILTER_VERSION);
	printf("listening on   : %s:%d\n", MYADDR, MYPORT);
	for (i = 0; i < 1; ++i) {
		con = accept(sock, (struct sockaddr *)conn, &connlen);
		if (con == -1)
			err(1, "accept");
		printf("connection from: %s:%hu\n", inet_ntoa(conn->sin_addr),
			ntohs(conn->sin_port));

		memset(&nat, 0, sizeof(nat));
		nat.nl_outip   = conn->sin_addr;
		nat.nl_outport = conn->sin_port;
		nat.nl_inip    = lo.sin_addr;
		nat.nl_inport  = lo.sin_port;
		nat.nl_flags   = IPN_TCP;
#if IPFILTER_VERSION >= 5000003
		nat.nl_v       = 4;
#endif

		memset(&obj, 0, sizeof(obj));
		obj.ipfo_rev = IPFILTER_VERSION;
		obj.ipfo_size = sizeof(nat);
		obj.ipfo_ptr = &nat;
		obj.ipfo_type = IPFOBJ_NATLOOKUP;

		if (ioctl(natfd, SIOCGNATL, &obj) == -1)
			err(1, "SIOCGNATL");

		printf("           real: %s:%d\n", inet_ntoa(nat.nl_realip),
			ntohs(nat.nl_realport));

		close(con);
	}

	if (close(natfd) == -1)
		err(1, "close nat");
	if (close(sock) == -1)
		err(1, "close server");

	return 0;
}
================================================================================

Add a few debug printfs in ip_nat.c:

================================================================================
NetBSD 5 patch:
diff --git a/sys/dist/ipf/netinet/ip_nat.c b/sys/dist/ipf/netinet/ip_nat.c
index 9b30e07..5c5f2a8 100644
--- a/sys/dist/ipf/netinet/ip_nat.c
+++ b/sys/dist/ipf/netinet/ip_nat.c
@@ -3599,6 +3599,15 @@ natlookup_t *np;
 					np->nl_realip, np->nl_outip))) {
 			np->nl_inip = nat->nat_inip;
 			np->nl_inport = nat->nat_inport;
+printf("%s IPN_IN:\n", __func__);
+printf("  inip: %s:%d\n",
+	inet_ntoa(nat->nat_inip), ntohs(nat->nat_inport));
+printf(" outip: %s:%d\n",
+	inet_ntoa(nat->nat_outip), ntohs(nat->nat_outport));
+printf("   oip: %s:%d\n",
+	inet_ntoa(nat->nat_oip), ntohs(nat->nat_oport));
+printf("==> in: %s:%d\n",
+	inet_ntoa(np->nl_inip), ntohs(np->nl_inport));
 		}
 	} else {
 		/*
@@ -3623,6 +3632,15 @@ natlookup_t *np;

 			np->nl_realip = nat->nat_outip;
 			np->nl_realport = nat->nat_outport;
+printf("%s !IPN_IN:\n", __func__);
+printf("  inip: %s:%d\n",
+	inet_ntoa(nat->nat_inip), ntohs(nat->nat_inport));
+printf(" outip: %s:%d\n",
+	inet_ntoa(nat->nat_outip), ntohs(nat->nat_outport));
+printf("   oip: %s:%d\n",
+	inet_ntoa(nat->nat_oip), ntohs(nat->nat_oport));
+printf("> real: %s:%d\n",
+	inet_ntoa(np->nl_realip), ntohs(np->nl_realport));
 		}
  	}

================================================================================
NetBSD 7 patch:
diff --git a/sys/external/bsd/ipf/netinet/ip_nat.c b/sys/external/bsd/ipf/netinet/ip_nat.c
index 0fa9033..b202a8e 100644
--- a/sys/external/bsd/ipf/netinet/ip_nat.c
+++ b/sys/external/bsd/ipf/netinet/ip_nat.c
@@ -4605,6 +4606,17 @@ ipf_nat_lookupredir(ipf_main_softc_t *softc, natlookup_t *np)
 					    np->nl_realip, np->nl_outip))) {
 			np->nl_inip = nat->nat_odstip;
 			np->nl_inport = nat->nat_odport;
+printf("%s IPN_IN:\n", __func__);
+printf("     osrc: %s:%d",
+	intoa(nat->nat_osrcaddr), ntohs(nat->nat_osport));
+printf(" odst: %s:%d\n",
+	intoa(nat->nat_odstaddr), ntohs(nat->nat_odport));
+printf("     nsrc: %s:%d",
+	intoa(nat->nat_nsrcaddr), ntohs(nat->nat_nsport));
+printf(" ndst: %s:%d\n",
+	intoa(nat->nat_ndstaddr), ntohs(nat->nat_ndport));
+printf("===> in  : %s:%d\n",
+	intoa(np->nl_inip.s_addr), ntohs(np->nl_inport));
 		}
 	} else {
 		/*
@@ -4627,8 +4639,25 @@ ipf_nat_lookupredir(ipf_main_softc_t *softc, natlookup_t *np)
 				}
 			}

+#define XXXPWHACK 0
+#if XXXPWHACK
+			np->nl_realip = nat->nat_odstip;
+			np->nl_realport = nat->nat_odport;
+#else
 			np->nl_realip = nat->nat_ndstip;
 			np->nl_realport = nat->nat_ndport;
+#endif
+printf("%s !IPN_IN:\n", __func__);
+printf("     osrc: %s:%d",
+	intoa(nat->nat_osrcaddr), ntohs(nat->nat_osport));
+printf(" odst: %s:%d\n",
+	intoa(nat->nat_odstaddr), ntohs(nat->nat_odport));
+printf("     nsrc: %s:%d",
+	intoa(nat->nat_nsrcaddr), ntohs(nat->nat_nsport));
+printf(" ndst: %s:%d\n",
+	intoa(nat->nat_ndstaddr), ntohs(nat->nat_ndport));
+printf("===> real: %s:%d\n",
+	intoa(np->nl_realip.s_addr), ntohs(np->nl_realport));
 		}
  	}

================================================================================

Now telnet in to port 80 from another computer (192.168.204.1), and
observe the different results (running on console, hence kernel printfs
are interleaved):

================================================================================
netbsd5# uname -rm
5.2_STABLE i386
netbsd5# ./ipfbug
IPFilter version 4012900
listening on   : 127.0.0.1:1234
connection from: 192.168.204.1:65392
nat_lookupredir !IPN_IN:
  inip: 127.0.0.1:1234
 outip: 192.168.204.87:80
   oip: 192.168.204.1:65392
> real: 192.168.204.87:80
           real: 192.168.204.87:80
netbsd5# ipnat -l
List of active MAP/Redirect filters:
rdr xennet0 0.0.0.0/0 port 80 -> 127.0.0.1 port 1234 tcp

List of active sessions:
RDR 127.0.0.1       1234  <- -> 192.168.204.87  80    [192.168.204.1 65392]
================================================================================
netbsd7# uname -rm
7.0_RC3 i386
netbsd7# ./ipfbug
IPFilter version 5010200
listening on   : 127.0.0.1:1234
connection from: 192.168.204.1:65386
ipf_nat_lookupredir !IPN_IN:
     osrc: 192.168.204.1:65386 odst: 192.168.204.86:80
     nsrc: 192.168.204.1:65386 ndst: 127.0.0.1:1234
===> real: 127.0.0.1:1234
           real: 127.0.0.1:1234
netbsd7# ipnat -l
List of active MAP/Redirect filters:
rdr xennet0 0/0 port 80 -> 127.0.0.1/32 port 1234 tcp

List of active sessions:
RDR 127.0.0.1       1234  <- -> 192.168.204.86  80    [192.168.204.1 65386]
================================================================================
>Fix:
I think ipf_nat_lookupredir() is simply returning the wrong variables.
Flipping XXXPWHACK to 1 should give the correct answer.
Something similar will be necessary in the untested IPN_IN case.


There is also an upgrade issue: IPFilter 5 grew a nl_v member, hence the
necessity of

#if IPFILTER_VERSION >= 5000003
                nat.nl_v       = 4;
#endif

in my toy server above. This means that any transparent proxy servers
written against IPFilter 4 will stop working on an upgrade to IPFilter 5.

It might be worth adding a "case 0:" to print a warning and fall through
to "case 4:" in ip_nat.c "switch (nl.nl_v)", rather than assume IPv0.

>Release-Note:

>Audit-Trail:

Responsible-Changed-From-To: kern-bug-people->ipf-bug-people
Responsible-Changed-By: dholland@NetBSD.org
Responsible-Changed-When: Sun, 06 Sep 2015 00:04:34 +0000
Responsible-Changed-Why:
ipf has its own bug people


From: =?UTF-8?Q?Egerv=c3=a1ry_Gergely?= <gergely@egervary.hu>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: kern/50198: SIOCGNATL broken in IPFilter 5
Date: Fri, 30 Sep 2016 18:04:23 +0200

 > ipnat SIOCGNATL from IPFilter v5 doesn't look up redirected
 > destination IP addresses correctly, which breaks transparent proxies
 > such as squid.
 >
 > IPFilter 4 in NetBSD 5 returns:
 >            real: 192.168.204.87:80
 > IPFilter 5 in NetBSD 7 returns:
 >            real: 127.0.0.1:1234
 >
 > I think ipf_nat_lookupredir() is simply returning the wrong variables.

 I can confirm this, the patch below fixes it:

 --- ip_nat.c.orig       2016-04-30 05:33:02.000000000 +0200
 +++ ip_nat.c    2016-09-30 17:16:03.000000000 +0200
 @@ -4620,8 +4620,10 @@
                                 }
                         }

 -                       np->nl_realip = nat->nat_ndstip;
 -                       np->nl_realport = nat->nat_ndport;
 +                       np->nl_realip = nat->nat_odstip;
 +                       np->nl_realport = nat->nat_odport;
                 }
         }

 Please commit a fix.
 (hi Darren, are You here?)

 Thank You,
 Gergely EGERVARY

From: Stephen Borrill <sborrill@NetBSD.org>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: kern/50198: SIOCGNATL broken in IPFilter 5
Date: Tue, 4 Oct 2016 16:03:11 +0100

 Fixed in this commit and pullups to netbsd-7 requested.

 -------- Forwarded Message --------
 Subject: CVS commit: src/sys/external/bsd/ipf/netinet
 Date: Tue, 4 Oct 2016 14:36:46 +0000
 From: Stephen Borrill <sborrill@netbsd.org>
 Reply-To: sborrill@netbsd.org
 To: source-changes@NetBSD.org

 Module Name:	src
 Committed By:	sborrill
 Date:		Tue Oct  4 14:36:46 UTC 2016

 Modified Files:
 	src/sys/external/bsd/ipf/netinet: ip_nat.c ip_nat6.c

 Log Message:
 Fix lookup of original destination address when using a redirect rule.
 This is required for transparent proxying by squid, for example.


 To generate a diff of this commit:
 cvs rdiff -u -r1.16 -r1.17 src/sys/external/bsd/ipf/netinet/ip_nat.c
 cvs rdiff -u -r1.9 -r1.10 src/sys/external/bsd/ipf/netinet/ip_nat6.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.


 -- 
 Stephen

State-Changed-From-To: open->closed
State-Changed-By: sborrill@NetBSD.org
State-Changed-When: Tue, 04 Oct 2016 15:23:45 +0000
State-Changed-Why:
Committed fix and requested pullups


From: Stephen Borrill <sborrill@NetBSD.org>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: kern/50198: Fwd: Re: [squid-users] intercept + IPv6 + IPFilter
 5.1
Date: Thu, 6 Oct 2016 08:50:11 +0100

 Patch may not be correct after all, re-opening PR and stalling pullup
 request.


 -------- Forwarded Message --------
 Subject: Re: [squid-users] intercept + IPv6 + IPFilter 5.1
 Date: Wed, 5 Oct 2016 20:49:54 +0200
 From: Egerváry Gergely <gergely@egervary.hu>
 To: squid-users@lists.squid-cache.org

 >> Should "intercept" work with IPv6 on NetBSD 7-STABLE and IPFilter 5.1?

 Okay, we have "fixed" Squid interception, and IPFilter in the kernel,
 and now it's working good. But did we do it in the right way?

 While reading ip_nat.c in IPFilter, I found that SIOCGNATL - and its
 function called ipf_nat_lookupredir() - is a frontend to two functions:
 ipf_nat_inlookup() and ipf_nat_outlookup().

 We are now calling SIOCGNATL to use ipf_nat_outlookup(). But should not
 we call it to use ipf_nat_inlookup() instead?

 In Squid, we are working with 3 different addresses:
 - source IP:port of the connection (browser client)
 - real destination IP:port (the target web server)
 - interception destination IP:port (Squid itself)

 In IPFilter, the terminology is different: "real" refers to the
 original source, not the original destination.

 In my understanding, on redirect (RDR) rules, where we know the
 original source address and the rewrited destination address, we should
 use ipf_nat_inlookup() to get the original destination address.

 ipf_nat_outlookup() should be used on source-NAT (MAP) scenarios,
 what we don't need for Squid.

 If that's true, IPFilter was correct - we have to revert our IPFilter
 patches - and modify Intercept.cc instead.

 See IPFilter source code comments below:

 ========
 Function: ipf_nat_inlookup
 Returns: nat_t* - NULL == no match, else pointer to matching NAT entry
 Parameters:
 fin(I) - pointer to packet information
 flags(I) - NAT flags for this packet
 p(I) - protocol for this packet
 src(I) - source IP address
 mapdst(I) - destination IP address

 Lookup a nat entry based on the mapped destination ip address/port
 and real source address/port. We use this lookup when receiving a
 packet, we're looking for a table entry, based on the destination
 address.

 ========
 Function: ipf_nat_outlookup
 Returns: nat_t* - NULL == no match, else pointer to matching NAT entry
 Parameters:
 fin(I) - pointer to packet information
 flags(I) - NAT flags for this packet
 p(I) - protocol for this packet
 src(I) - source IP address
 dst(I) - destination IP address
 rw(I) - 1 == write lock on held, 0 == read lock.

 Lookup a nat entry based on the source 'real' ip address/port
 and destination address/port. We use this lookup when sending a packet
 out, we're looking for a table entry, based on the source address.

 ========

 See full ip_nat.c source code here:

 http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/external/bsd/ipf/netinet/ip_nat.c?rev=1.16&content-type=text/x-cvsweb-markup

 Thank you,
 -- 
 Gergely EGERVARY

 _______________________________________________
 squid-users mailing list
 squid-users@lists.squid-cache.org
 http://lists.squid-cache.org/listinfo/squid-users


 -- 
 Stephen

State-Changed-From-To: closed->open
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Mon, 10 Oct 2016 01:38:48 +0000
State-Changed-Why:
sborrill@ said he was going to reopen it, but didn't.
what's the current state?


>Unformatted:

NetBSD Home
NetBSD PR Database Search

(Contact us) $NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2014 The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.