Call for testing: OpenSSH 7.3

[Corinna Vinschen]

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>