On Fri, Jul 22, 2016 at 12:05:53PM +0200, Corinna Vinschen wrote: [...]> This version doesn't build on Cygwin anymore. The reason is that > various configure tests fail. > > The culprit is the new definition of IPPORT_RESERVED to 0 in configure.ac.Sigh. How about putting it in defines.h instead? includes.h includes netinet/in.h from whence the definition of IPPORT_RESERVED is, on Cygwin at least, seems to be protected against multiple inclusion. Putting it there means only one definition in a file that we don't sync with OpenBSD. diff --git a/configure.ac b/configure.ac index 21ef389..2cd6a6f 100644 --- a/configure.ac +++ b/configure.ac @@ -589,8 +589,9 @@ case "$host" in [Define if you want to disable shadow passwords]) AC_DEFINE([NO_X11_UNIX_SOCKETS], [1], [Define if X11 doesn't support AF_UNIX sockets on that system]) - AC_DEFINE([IPPORT_RESERVED], [0], - [Cygwin has no notion of ports only accessible to superusers]) + AC_DEFINE([NO_IPPORT_RESERVED_CONCEPT], [1], + [Define if the concept of ports only accessible to + superusers isn't known]) AC_DEFINE([DISABLE_FD_PASSING], [1], [Define if your platform needs to skip post auth file descriptor passing]) diff --git a/defines.h b/defines.h index a438ddd..c099df6 100644 --- a/defines.h +++ b/defines.h @@ -43,6 +43,17 @@ enum #endif /* + * Cygwin doesn't really have a notion of reserved ports but for backward + * compatibility they define it to 1024 in netinet/in.h inside an enum. We + * don't actually want that restriction so we want to set that to zero, but + * we can't do it direct in config.h because it'll cause a conflicting + * definition the first time we include netinet/in.h. + */ +#ifdef NO_IPPORT_RESERVED_CONCEPT +#define IPPORT_RESERVED 0 +#endif + +/* * Definitions for IP type of service (ip_tos) */ #include <netinet/in_systm.h> -- Darren Tucker (dtucker at zip.com.au) GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860 37F4 9357 ECEF 11EA A6FA (new) Good judgement comes with experience. Unfortunately, the experience usually comes from bad judgement.
On Jul 22 21:37, Darren Tucker wrote:> On Fri, Jul 22, 2016 at 12:05:53PM +0200, Corinna Vinschen wrote: > [...] > > This version doesn't build on Cygwin anymore. The reason is that > > various configure tests fail. > > > > The culprit is the new definition of IPPORT_RESERVED to 0 in configure.ac. > > Sigh. > > How about putting it in defines.h instead? includes.h includes > netinet/in.h from whence the definition of IPPORT_RESERVED is, on Cygwin > at least, seems to be protected against multiple inclusion. Putting it > there means only one definition in a file that we don't sync with OpenBSD.Hmm. If that only affects Cygwin, and if defines.h is not synced anyway, what about getting rid of the configure stuff entirely? Tested counterproposal: diff --git a/configure.ac b/configure.ac index 21ef389..e64386f 100644 --- a/configure.ac +++ b/configure.ac @@ -589,8 +589,6 @@ case "$host" in [Define if you want to disable shadow passwords]) AC_DEFINE([NO_X11_UNIX_SOCKETS], [1], [Define if X11 doesn't support AF_UNIX sockets on that system]) - AC_DEFINE([IPPORT_RESERVED], [0], - [Cygwin has no notion of ports only accessible to superusers]) AC_DEFINE([DISABLE_FD_PASSING], [1], [Define if your platform needs to skip post auth file descriptor passing]) diff --git a/defines.h b/defines.h index a438ddd..d1ad6a7 100644 --- a/defines.h +++ b/defines.h @@ -43,6 +43,17 @@ enum #endif /* + * Cygwin doesn't really have a notion of reserved ports but for backward + * compatibility they define it to 1024 in netinet/in.h inside an enum. We + * don't actually want that restriction so we want to set that to zero, but + * we can't do it direct in config.h because it'll cause a conflicting + * definition the first time we include netinet/in.h. + */ +#ifdef HAVE_CYGWIN +#define IPPORT_RESERVED 0 +#endif + +/* * Definitions for IP type of service (ip_tos) */ #include <netinet/in_systm.h> As for the comment preceeding the definition, I didn't change it from your text in my proposal. However. I'd like to outline that IPPORT_RESERVED == 1024 still makes sense in terms of the implementation of bindresvport_sa and rcmd. It's not just backward compatibility. There are also applications out there which still expect this value to make sense. The *real* problem here is that OpenSSH checks for uid 0 before allowing to bind a socket to a port < IPPORT_RESERVED, rather than letting the OS decide if the current user is allowed to bind that port. From my POV this check in OpenSSH is gratuitious and it's the real reason we have this problem at all. Thanks, Corinna -- Corinna Vinschen Cygwin Maintainer Red Hat -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.mindrot.org/pipermail/openssh-unix-dev/attachments/20160722/778b6a70/attachment.bin>
On Fri, Jul 22, 2016 at 10:18 PM, Corinna Vinschen <vinschen at redhat.com> wrote: [...]> Hmm. If that only affects Cygwin, and if defines.h is not synced anyway, > what about getting rid of the configure stuff entirely? > > Tested counterproposal:Looks reasonable. It's late here so I'm going to look at it tomorrow.> As for the comment preceeding the definition, I didn't change it from > your text in my proposal. However. > > I'd like to outline that IPPORT_RESERVED == 1024 still makes sense in > terms of the implementation of bindresvport_sa and rcmd. It's not just > backward compatibility. There are also applications out there which > still expect this value to make sense.Fair point.> The *real* problem here is that OpenSSH checks for uid 0 before allowing > to bind a socket to a port < IPPORT_RESERVED, rather than letting the OS > decide if the current user is allowed to bind that port. > From my POV this check in OpenSSH is gratuitious and it's the real reason > we have this problem at all.In the case of sshd running with privsep off, the process doing the binding is still running as root and I suspect those checks date back to when it was always running as root. It could probably temporarily_use_uid() though. Thanks. -- Darren Tucker (dtucker at zip.com.au) GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860 37F4 9357 ECEF 11EA A6FA (new) Good judgement comes with experience. Unfortunately, the experience usually comes from bad judgement.