Date: Fri, 09 Jan 2009 09:46:06 -0800 From: Julian Elischer <julian@elischer.org> To: Max Laier <max@love2party.net> Cc: svn-src-head@freebsd.org, Adrian Chadd <adrian@freebsd.org>, src-committers@freebsd.org, svn-src-all@freebsd.org Subject: Re: svn commit: r186955 - in head/sys: conf netinet Message-ID: <49678D5E.3030600@elischer.org> In-Reply-To: <200901091802.10287.max@love2party.net> References: <200901091602.n09G2Jj1061164@svn.freebsd.org> <200901091802.10287.max@love2party.net>
next in thread | previous in thread | raw e-mail | index | archive | help
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 >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?49678D5E.3030600>