From afe10313c1fa8d478af399ee7d54c8f85503013b Mon Sep 17 00:00:00 2001 From: Damien Miller Date: Thu, 13 Jun 2024 14:35:25 +1000 Subject: [PATCH] fix PTY allocation on Cygwin, broken by sshd split Cygwin doesn't support FD passing and so used to disable post-auth privilege separation entirely because privsep requires PTY allocation to happen in the privileged monitor process with the PTY file descriptors being passed back to the unprivileged process. This brings back a minimal version of the previous special treatment for Cygwin (and any other platform that sets DISABLE_FD_PASSING): privilege separation remains enabled, but PTY allocation happens in the post-auth user process rather than the monitor. This either requires PTY allocation to not need privilege to begin with (this appears to be the case on Cygwin), or the post-auth privsep process retain privilege (other platforms that set the DISABLE_FD_PASSING option). Keeping privileges here is bad, but the non-Cygwin systems that set DISABLE_FD_PASSING are so deeply legacy that this is likely to be the least of their problems. --- session.c | 11 +++++++++++ sshd-session.c | 18 +++++++++++++++++- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/session.c b/session.c index 7cf22a722..ad05a95d4 100644 --- a/session.c +++ b/session.c @@ -103,6 +103,17 @@ #include #endif +/* + * Hack for systems that do not support FD passing: allocate PTYs directly + * without calling into the monitor. This requires either the post-auth + * privsep process retain root privileges (see the comment in + * sshd-session:privsep_postauth) or that PTY allocation doesn't require + * privileges to begin with (e.g. Cygwin). + */ +#ifdef DISABLE_FD_PASSING +#define mm_pty_allocate pty_allocate +#endif + #define IS_INTERNAL_SFTP(c) \ (!strncmp(c, INTERNAL_SFTP_NAME, sizeof(INTERNAL_SFTP_NAME) - 1) && \ (c[sizeof(INTERNAL_SFTP_NAME) - 1] == '\0' || \ diff --git a/sshd-session.c b/sshd-session.c index dbc3074fa..7ab1ea472 100644 --- a/sshd-session.c +++ b/sshd-session.c @@ -379,6 +379,21 @@ privsep_preauth(struct ssh *ssh) static void privsep_postauth(struct ssh *ssh, Authctxt *authctxt) { + int skip_privdrop = 0; + + /* + * Hack for systems that don't support FD passing: retain privileges + * in the post-auth privsep process so it can allocate PTYs directly. + * This is basically equivalent to what we did <= 9.7, which was to + * disable post-auth privsep entriely. + * Cygwin doesn't need to drop privs here although it doesn't support + * fd passing, as AFAIK PTY allocation on this platform doesn't require + * special privileges to begin with. + */ +#if defined(DISABLE_FD_PASSING) && !defined(HAVE_CYGWIN) + skip_privdrop = 1; +#endif + /* New socket pair */ monitor_reinit(pmonitor); @@ -406,7 +421,8 @@ privsep_postauth(struct ssh *ssh, Authctxt *authctxt) reseed_prngs(); /* Drop privileges */ - do_setusercontext(authctxt->pw); + if (!skip_privdrop) + do_setusercontext(authctxt->pw); /* It is safe now to apply the key state */ monitor_apply_keystate(ssh, pmonitor);