Call for testing: OpenSSH-5.9

[Damien Miller]

On Mon, 29 Aug 2011, Darren Tucker wrote:

> On Mon, Aug 29, 2011 at 3:34 PM, Tim Rice <tim at multitalents.net> wrote:
> [...]
> > +#ifdef POLL_USES_FD
> > +       if (setrlimit(RLIMIT_NOFILE, &rl_one) == -1)
> > +#else
> >        if (setrlimit(RLIMIT_NOFILE, &rl_zero) == -1)
> > +#endif
> 
> Problem is this destroys a lot of the value of the rlimit sandbox,
> since a compromised slave can close all descriptors and open a single
> new one with connect() and, eg, use it to probe serves on localhost or
> beyond the machine in question.
> 
> One thing we could do to mitigate this is to have the monitoring
> SIGKILL the slave if the socketpair closes.  It's racy so there's a
> window where a compromised slave could potentially make a connection.

The below diff does this. IMO it would be better to turn the sandbox
off for the platforms that don't support it for this release.

Index: sshd.c
===================================================================
RCS file: /cvs/src/usr.bin/ssh/sshd.c,v
retrieving revision 1.385
diff -u -p -r1.385 sshd.c
--- sshd.c	23 Jun 2011 09:34:13 -0000	1.385
+++ sshd.c	29 Aug 2011 06:29:32 -0000
@@ -222,6 +222,7 @@ int startup_pipe;		/* in child */
 /* variables used for privilege separation */
 int use_privsep = -1;
 struct monitor *pmonitor = NULL;
+int privsep_is_preauth = 1;
 
 /* global authentication context */
 Authctxt *the_authctxt = NULL;
@@ -637,10 +638,13 @@ privsep_preauth(Authctxt *authctxt)
 
 		/* Wait for the child's exit status */
 		while (waitpid(pid, &status, 0) < 0) {
+			pmonitor->m_pid = -1;
 			if (errno != EINTR)
 				fatal("%s: waitpid: %s", __func__,
 				    strerror(errno));
 		}
+		pmonitor->m_pid = -1;
+		privsep_is_preauth = 0;
 		if (WIFEXITED(status)) {
 			if (WEXITSTATUS(status) != 0)
 				fatal("%s: preauth child exited with status %d",
@@ -2217,7 +2221,14 @@ do_ssh2_kex(void)
 void
 cleanup_exit(int i)
 {
-	if (the_authctxt)
+	if (the_authctxt) {
 		do_cleanup(the_authctxt);
+		if (privsep_is_preauth && pmonitor->m_pid > 1) {
+			debug("Killing privsep child %d", pmonitor->m_pid);
+			if (kill(SIGKILL, pmonitor->m_pid) != 0)
+				error("%s: kill(%d): %s", __func__,
+				    pmonitor->m_pid, strerror(errno));
+		}
+	}
 	_exit(i);
 }