From owner-svn-src-head@FreeBSD.ORG Fri Jan 9 18:14:25 2009 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 14D941065674; Fri, 9 Jan 2009 18:14:25 +0000 (UTC) (envelope-from prvs=julian=25380973a@elischer.org) Received: from smtp-outbound.ironport.com (smtp-outbound.ironport.com [63.251.108.112]) by mx1.freebsd.org (Postfix) with ESMTP id DB1148FC0C; Fri, 9 Jan 2009 18:14:24 +0000 (UTC) (envelope-from prvs=julian=25380973a@elischer.org) Received: from unknown (HELO julian-mac.elischer.org) ([10.251.60.87]) by smtp-outbound.ironport.com with ESMTP; 09 Jan 2009 09:46:08 -0800 Message-ID: <49678D5E.3030600@elischer.org> Date: Fri, 09 Jan 2009 09:46:06 -0800 From: Julian Elischer User-Agent: Thunderbird 2.0.0.19 (Macintosh/20081209) MIME-Version: 1.0 To: Max Laier References: <200901091602.n09G2Jj1061164@svn.freebsd.org> <200901091802.10287.max@love2party.net> In-Reply-To: <200901091802.10287.max@love2party.net> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Cc: svn-src-head@freebsd.org, Adrian Chadd , src-committers@freebsd.org, svn-src-all@freebsd.org Subject: Re: svn commit: r186955 - in head/sys: conf netinet X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 09 Jan 2009 18:14:25 -0000 Max Laier wrote: > On Friday 09 January 2009 17:02:19 Adrian Chadd wrote: >> Author: adrian >> Date: Fri Jan 9 16:02:19 2009 >> New Revision: 186955 >> URL: http://svn.freebsd.org/changeset/base/186955 >> >> Log: >> Implement a new IP option (not compiled/enabled by default) to allow >> applications to specify a non-local IP address when bind()'ing a socket >> to a local endpoint. > > That's a *socket* option ... you had me very worried there for a moment ;) I > don't quite see why you'd hide these under a build time option - having the > sysctl defaulting to off under CTLFLAG_SECURE seems good enough - if people > disagree - make it a boot time tuneable, but I certainly don't see why you > should have to rebuild the kernel for a minor thing like this. It certainly > isn't performance critical. because it can be a big security hole and you do not want people to have it available on the average machine. Also because purists complained about it. You'll notice that the compile option enables the sysctl, which is used to turn on and off the capacity to do this per socket. so the admin can disable it, but I felt a lot more comfortable having it not compiled in by default. > > Some nit picking below ... > >> Modified: head/sys/netinet/in_pcb.c >> @@ -346,7 +347,11 @@ in_pcbbind_setup(struct inpcb *inp, stru >> } else if (sin->sin_addr.s_addr != INADDR_ANY) { >> sin->sin_port = 0; /* yech... */ >> bzero(&sin->sin_zero, sizeof(sin->sin_zero)); >> - if (ifa_ifwithaddr((struct sockaddr *)sin) == 0) >> + if ( >> +#if defined(IP_NONLOCALBIND) >> + ((inp->inp_flags & INP_NONLOCALOK) == 0) && >> +#endif >> + (ifa_ifwithaddr((struct sockaddr *)sin) == 0)) >> return (EADDRNOTAVAIL); >> } >> laddr = sin->sin_addr; > > This logic is really hard to get a first glance. Esp. the not NON...OK part. > Maybe a comment is called for here - or is this just me being confused? I think it's just you :-) "NONLOCAL" is one word > >> Modified: head/sys/netinet/ip_output.c >> @@ -866,6 +873,13 @@ ip_ctloutput(struct socket *so, struct s >> return (error); >> } >> >> +#if defined(IP_NONLOCALBIND) >> + case IP_NONLOCALOK: >> + if (! ip_nonlocalok) { >> + error = ENOPROTOOPT; >> + break; >> + } >> +#endif >