From owner-freebsd-net@FreeBSD.ORG Mon Jul 21 18:27:04 2008 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 988B01065675; Mon, 21 Jul 2008 18:27:04 +0000 (UTC) (envelope-from mgrooms@shrew.net) Received: from shrew.net (shrew.net [206.223.169.85]) by mx1.freebsd.org (Postfix) with ESMTP id 54E2B8FC08; Mon, 21 Jul 2008 18:27:04 +0000 (UTC) (envelope-from mgrooms@shrew.net) Received: from localhost (wm-ca.hub.org [206.223.169.82]) by shrew.net (Postfix) with ESMTP id E79E279E30E; Mon, 21 Jul 2008 13:27:03 -0500 (CDT) Received: from shrew.net ([206.223.169.85]) by localhost (mx1.hub.org [206.223.169.82]) (amavisd-new, port 10024) with ESMTP id 67507-06; Mon, 21 Jul 2008 18:27:03 +0000 (UTC) Received: from hole.shrew.net (cpe-70-113-206-103.austin.res.rr.com [70.113.206.103]) by shrew.net (Postfix) with ESMTP id 91C2E79E307; Mon, 21 Jul 2008 13:27:03 -0500 (CDT) Received: from [10.22.200.30] ([10.22.200.30]) by hole.shrew.net (8.14.2/8.14.2) with ESMTP id m6LIR0bq070726; Mon, 21 Jul 2008 13:27:00 -0500 (CDT) (envelope-from mgrooms@shrew.net) Message-ID: <4884D4F9.4050707@shrew.net> Date: Mon, 21 Jul 2008 13:27:05 -0500 From: Matthew Grooms User-Agent: Thunderbird 2.0.0.14 (Windows/20080421) MIME-Version: 1.0 To: vanhu@freebsd.org, Sam Leffler References: <4884C28F.4020402@shrew.net> In-Reply-To: <4884C28F.4020402@shrew.net> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: freebsd-net@freebsd.org Subject: Re: FreeBSD NAT-T patch integration [CFR/CFT] X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 21 Jul 2008 18:27:04 -0000 On Mon, Jul 21, 2008 at 10:31:10AM +0200, VANHULLEBUS Yvan wrote: > > After some more testing, I found another issue: in udp4_espdecap(), > when payload <= sizeof(uint64_t) + sizeof(struct esp), packet should > not be discarded, but just returned for normal processing. > I noticed this too. But the only situation that I could think of where a valid ISAKMP packet will be smaller than this is a NAT-T keep-alive. These are handled previously in the code path so I don't think there is an issue from a functional standpoint. > And I also have doubts about a change in udp_ctloutput(), in the > switch statement which process optval and searches for an > UDP_ENCAP_ESPINUDP* flag. > > The way you changed it forces a flags cleanup anytime. > I don't see why someone would set both UDP_ENCAP_ESPINUDP and > UDP_ENCAP_ESPINUDP_NON_IKE, but as I was tracking down a problem, I > changed it again to be processed "the old way" to ensure it was not > the source of the issue. > It should be disallowed as in Sams patch. Allowing them to be mixed would cause problems using any of the patches I have seen. There is no way to distinguish between a Draft 00/01 ISAKMP packet and an RFC ESP packet without matching the port value to a SAD NAT-T mapping. And as you mentioned, I also don't see why anyone would try to set them both. There should never be a situation where you need to evaluate a NON-ESP NAT-T marker on an ISAKMP socket, only NON-ISAKMP markers. On a related note, I noticed the patch unconditionally uses a source port of 500 when processing outbound Draft 00/01 packets. Should this value be obtained from the SAD NAT-T mapping to support an IKE daemon bound to a non standard port? Thanks, -Matthew