Date: Sun, 13 Mar 2011 19:55:13 +0000 (UTC) From: "Bjoern A. Zeeb" <bzeeb-lists@lists.zabbadoz.net> To: Doug Barton <dougb@FreeBSD.org> Cc: FreeBSD Net <freebsd-net@freebsd.org>, Ivo Vachkov <ivo.vachkov@gmail.com> Subject: Re: Proposed patch for Port Randomization modifications according to RFC6056 Message-ID: <alpine.BSF.2.00.1103131926430.6104@ai.fobar.qr> In-Reply-To: <4D72A1A7.4040402@FreeBSD.org> References: <AANLkTi=rF%2BCYiNG7PurPtrwn-AMT9cYEe90epGAJDwDq@mail.gmail.com> <4D411CC6.1090202@gont.com.ar> <AANLkTinvg5tft8xockuuV9g5QYd36ko9qO4YCvy5bkJ1@mail.gmail.com> <4D431258.8040704@FreeBSD.org> <AANLkTimhZ_pxTGt958AX8m=%2BS=g2hqsst=GH1a99D0g1@mail.gmail.com> <4D437B13.1070405@FreeBSD.org> <AANLkTim4=xa0rfoLgt-ao30XoZkLZ1hMYzE6LsrLNcbM@mail.gmail.com> <4D518FB3.3040503@FreeBSD.org> <4D6AB2BD.50208@gont.com.ar> <4D6AB636.3030708@FreeBSD.org> <alpine.BSF.2.00.1103031222160.6104@ai.fobar.qr> <4D72A1A7.4040402@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?alpine.BSF.2.00.1103131926430.6104>