From owner-freebsd-net@FreeBSD.ORG Sun Mar 13 19:55:49 2011 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 D3393106566C for ; Sun, 13 Mar 2011 19:55:49 +0000 (UTC) (envelope-from bzeeb-lists@lists.zabbadoz.net) Received: from mx1.sbone.de (bird.sbone.de [46.4.1.90]) by mx1.freebsd.org (Postfix) with ESMTP id 56A388FC12 for ; Sun, 13 Mar 2011 19:55:48 +0000 (UTC) Received: from mail.sbone.de (mail.sbone.de [IPv6:fde9:577b:c1a9:31::2013:587]) (using TLSv1 with cipher ADH-CAMELLIA256-SHA (256/256 bits)) (No client certificate requested) by mx1.sbone.de (Postfix) with ESMTPS id 3DDF025D388E; Sun, 13 Mar 2011 19:55:17 +0000 (UTC) Received: from content-filter.sbone.de (content-filter.sbone.de [IPv6:fde9:577b:c1a9:31::2013:2742]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.sbone.de (Postfix) with ESMTPS id 4DF73159ABC3; Sun, 13 Mar 2011 19:55:16 +0000 (UTC) X-Virus-Scanned: amavisd-new at sbone.de Received: from mail.sbone.de ([IPv6:fde9:577b:c1a9:31::2013:587]) by content-filter.sbone.de (content-filter.sbone.de [fde9:577b:c1a9:31::2013:2742]) (amavisd-new, port 10024) with ESMTP id 7+xe0A5Piopb; Sun, 13 Mar 2011 19:55:15 +0000 (UTC) Received: from nv.sbone.de (nv.sbone.de [IPv6:fde9:577b:c1a9:31::2013:138]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.sbone.de (Postfix) with ESMTPSA id CA485159ABCE; Sun, 13 Mar 2011 19:55:14 +0000 (UTC) Date: Sun, 13 Mar 2011 19:55:13 +0000 (UTC) From: "Bjoern A. Zeeb" To: Doug Barton In-Reply-To: <4D72A1A7.4040402@FreeBSD.org> Message-ID: References: <4D411CC6.1090202@gont.com.ar> <4D431258.8040704@FreeBSD.org> <4D437B13.1070405@FreeBSD.org> <4D518FB3.3040503@FreeBSD.org> <4D6AB2BD.50208@gont.com.ar> <4D6AB636.3030708@FreeBSD.org> <4D72A1A7.4040402@FreeBSD.org> X-OpenPGP-Key: 0x14003F198FEFA3E77207EE8D2B58B8F83CCF1842 MIME-Version: 1.0 Content-Type: TEXT/PLAIN; format=flowed; charset=US-ASCII Cc: FreeBSD Net , Ivo Vachkov Subject: Re: Proposed patch for Port Randomization modifications according to RFC6056 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: Sun, 13 Mar 2011 19:55:49 -0000 On Sat, 5 Mar 2011, Doug Barton wrote: Hi, as you may have noticed, I had committed logical upfront changes to the current code this weekend, to make it easier for anyone to later understand what happened, when looking at revision history. I have updated the patch for HEAD and it can be found here: http://people.freebsd.org/~bz/20110313-01-rfc6056.diff Looking through the functional changes and the RFC while working on the ip(4) man page update I though noticed that several alogrithms could need implementation improvements. Given the replies I thought the functional review had long happened. Could someone please go and verify: Algo 2: - could we miss a port due to the % missing the +1? Algo 3: - we do not increment *lastport (next_ephemeral) as suggested in the RFC. Algo 3+4+5: - check the %s for +1 needed as well - factor out the system boot time changes (as suggested in the RFC) to a VNET_SYSINIT? Algo 3+4: - doing a secret key per request in my view changes the suggested per (laddr, faddr, fport, secret) from a 3-tuple to a 4-tuple. What implications does that have aoart from cost? Could we per section 3.4 of the RFC use ipport_tick() to change it, getting one/two arc4random() calls out of the function as well? Algo 5: - when calculation *lastport = shouldn't first be *lastport instead? Re-randomizing that value every time instead of once on boottime, does change the algorithm, doesn't it? We might need to re-set some of the then "global" values when we switch algos from the sysctl handler. We should take care of that. In various places the first things we do, due to the classic way we did it, are: a) check count, which for the first run must never return the error in our implementation disabling the dorandom for only one port left. b) increment *lastport, which is not always suggested c) as a result of b) have to check the range which otherwise would have to be fine because of the % operation. Would it make sense to shuffle the code around a bit to be closer to the RFC, use a break for the result on tmpinp to escape the loop, etc? Also I'd like to point out that we still do not always use the random port for TCP per: 441 /* 442 * For UDP, use random port allocation as long as the user 443 * allows it. For TCP (and as of yet unknown) connections, 444 * use random port allocation only if the user allows it AND 445 * ipport_tick() allows it. 446 */ 447 if (V_ipport_randomized && 448 (!V_ipport_stoprandom || pcbinfo == &V_udbinfo)) 449 dorandom = 1; 450 else 451 dorandom = 0; Do we want to change that decision, or do we want to defer the change in a similar way to defer the change of the default algo -- understand performance (vs. security) impact first? /bz -- Bjoern A. Zeeb You have to have visions! Stop bit received. Insert coin for new address family.