Bug 2885

Summary:	sshd is not using chroot and privsep on default cygwin install any more
Product:	Portable OpenSSH	Reporter:	Christian.Lupien
Component:	sshd	Assignee:	Assigned to nobody <unassigned-bugs>
Status:	NEW ---
Severity:	security	CC:	djm, dtucker, vinschen
Priority:	P5
Version:	7.7p1
Hardware:	Other
OS:	Cygwin on NT/2k/Win7-11

Description Christian.Lupien 2018-07-24 02:41:44 AEST

I think that since commit d13281f2964abc5f2e535e1613c77fc61b0c53e7
sshd under cygwin is no longer using privsep by default.

I was trying to test which sshd account the program was using for privsep (I had a local and a domain sshd account). However I was unable to see any use of it (nor any errors). I looked at the "ps -a" output, also at the output of "sshd -d -d -d" and even tried strace but could not find it anywhere.

I then looked at the code. Most the places in the code where
  getuid() == 0
is used it the code is special cased for cgywin but the change introduced by the commit d13281f2964abc5f2e535e1613c77fc61b0c53e7 did not.

Therefore, privsep under cygwin seems to be disabled unless the account under which sshd is executed is forced to have uid=0 (by changing the /etc/passwd file).

I did try the forcing uid=0 in the passwd file and that worked (I saw the sshd account being used and chroot executed.)

I have not tried modifying the code and recompiling it but probably replacing (part of line 1734)
  getuid() == 0 || geteuid() == 0
with
  platform_privileged_uidswap()
would work.

Comment 1 Damien Miller 2018-07-27 13:31:16 AEST

I don't understand how commit d13281f2964 changed this behaviour - it replaced the inline getuid() checks with one performed in main() and there has never AFAIK been special-casing for Cygwin around this check.

Comment 2 Christian.Lupien 2018-08-02 03:12:34 AEST

You are right. I guess I thought it used to work and was looking at something that changed. But, as you say, that commit did not change the main behavior.

So I guess this means the behavior of chroot and prvisep account not being used on cygwin (with uid !=0, which is the default) has been this way for a long time.

It should be fixed or documented.

Comment 3 Damien Miller 2018-08-02 16:10:09 AEST

Corinna, what do you think the right behaviour is here?

Comment 4 Corinna Vinschen 2018-08-02 19:24:04 AEST

Privsep never worked fully on Cygwin.  It uses descriptor passing via AF_UNIX sockets which isn't implemented in Cygwin (yet).

So when privsep was introduced, only one half of it got used on Cygwin.
The problem is, I don't remember which part of it.

Either way, I checked the source history and it seems the code
in question here (preauth) was never used on Cygwin anyway.  The
getuid==0 check exists since 2002.

Second problem is that permanently_set_uid() is a fake on Cygwin.
Permanently changing the uid/gid only works when exec'ing a new
process.  It's never permanent while still in the same process.
This is a limitation of the Windows authorization model.

Above all, chroot is a concept not supported by Windows anyway.
Therefore the chroot emulation of Cygwin does not actually
add any security since applications can easily work around it
by using native Windows API calls rather than just Cygwin
POSIX calls, as is well-known.

Bottom line is, I'm not sure what to do here.  Preauth privsep
is not really well supportable on Cygwin.  Probably the best way
out is to remove any Cygwin-specific code here and tell the user
that this is not supported on Cygwin.

THis shouldn't affect the other part of privsep, though.


Corinna

Comment 5 Darren Tucker 2018-09-14 13:53:11 AEST

(In reply to Corinna Vinschen from comment #4)
> Privsep never worked fully on Cygwin.  It uses descriptor passing
> via AF_UNIX sockets which isn't implemented in Cygwin (yet).
> 
> So when privsep was introduced, only one half of it got used on
> Cygwin.
> The problem is, I don't remember which part of it.

If the sticking point is descriptor passing then that's used for pty allocation in post-auth privsep, so that would make preauth the working part.

There used to also be an anonymous mmap thing that got used for zlib state passing, but since preauth zlib is now gone that shouldn't be an issue.