6868716 dangling sshd authentication thread after timeout exit of monitor

6875551 remove the "authenticated" label from sshd.c

5 files changed, 42 insertions, 28 deletions

diff --git a/usr/src/cmd/ssh/Makefile.ssh-common b/usr/src/cmd/ssh/Makefile.ssh-common
index 388e13bffb..66759d12d6 100644
--- a/usr/src/cmd/ssh/Makefile.ssh-common
+++ b/usr/src/cmd/ssh/Makefile.ssh-common
@@ -30,7 +30,7 @@ TEXT_DOMAIN=SUNW_OST_OSCMD
 CFLAGS += $(CCVERBOSE)
 LDFLAGS += $(MAPFILE.NGB:%=-M%)
 
-SSH_VERSION=\"Sun_SSH_1.4\"
+SSH_VERSION=\"Sun_SSH_1.5\"
 
 C99MODE= $(C99_ENABLE)
 
diff --git a/usr/src/cmd/ssh/libssh/common/compat.c b/usr/src/cmd/ssh/libssh/common/compat.c
index e91c553a8f..6d85d6e511 100644
--- a/usr/src/cmd/ssh/libssh/common/compat.c
+++ b/usr/src/cmd/ssh/libssh/common/compat.c
@@ -111,6 +111,7 @@ compat_datafellows(const char *version)
 		{ "Sun_SSH_1.2*",	SSH_BUG_STRING_ENCODING},
 		{ "Sun_SSH_1.3*",	SSH_BUG_STRING_ENCODING},
 		{ "Sun_SSH_1.4*",	0 },
+		{ "Sun_SSH_1.5*",	0 },
 		{ "Sun_SSH_*",		0 },
 		{ "*MindTerm*",		0 },
 		{ "2.1.0*",		SSH_BUG_SIGBLOB|SSH_BUG_HMAC|
diff --git a/usr/src/cmd/ssh/libssh/common/packet.c b/usr/src/cmd/ssh/libssh/common/packet.c
index 4464cf4e18..1221db134a 100644
--- a/usr/src/cmd/ssh/libssh/common/packet.c
+++ b/usr/src/cmd/ssh/libssh/common/packet.c
@@ -933,8 +933,11 @@ packet_send(void)
  * Waits until a packet has been received, and returns its type.  Note that
  * no other data is processed until this returns, so this function should not
  * be used during the interactive session.
+ *
+ * The function is also used in the monitor to read the authentication context
+ * in aps_read_auth_context() via packet_read_seqnr(), before the monitor enters
+ * aps_monitor_loop() and starts using the process_input() function.
  */
-
 int
 packet_read_seqnr(u_int32_t *seqnr_p)
 {
@@ -980,11 +983,22 @@ packet_read_seqnr(u_int32_t *seqnr_p)
 		/* Read data from the socket. */
 		len = read(connection_in, buf, sizeof(buf));
 		if (len == 0) {
-			log("Connection closed by %.200s", get_remote_ipaddr());
+			if (packet_connection_is_on_socket())
+				log("Connection closed by %.200s",
+				    get_remote_ipaddr());
+			else
+				debug("child closed the communication pipe "
+				    "before user auth was finished");
 			fatal_cleanup();
 		}
-		if (len < 0)
-			fatal("Read from socket failed: %.100s", strerror(errno));
+		if (len < 0) {
+			if (packet_connection_is_on_socket())
+				fatal("Read from socket failed: %.100s",
+				    strerror(errno));
+			else
+				fatal("Read from communication pipe failed: "
+				    "%.100s", strerror(errno));
+		}
 		/* Append it to the buffer. */
 		packet_process_incoming(buf, len);
 	}
diff --git a/usr/src/cmd/ssh/sshd/altprivsep.c b/usr/src/cmd/ssh/sshd/altprivsep.c
index 5fb6e8905f..a2c437cb6f 100644
--- a/usr/src/cmd/ssh/sshd/altprivsep.c
+++ b/usr/src/cmd/ssh/sshd/altprivsep.c
@@ -1040,6 +1040,10 @@ void
 	return (buffer_get_string(&from_monitor, length_ptr));
 }
 
+/*
+ * Start and execute the code for the monitor which never returns from this
+ * function. The child will return and continue in the caller.
+ */
 void
 altprivsep_start_and_do_monitor(int use_engine, int inetd, int newsock,
 	int statup_pipe)
@@ -1072,12 +1076,6 @@ altprivsep_start_and_do_monitor(int use_engine, int inetd, int newsock,
 		 *  - PAM cleanup
 		 */
 
-		/*
-		 * Alarm signal handler is for our child only since that's the
-		 * one that does the authentication.
-		 */
-		(void) alarm(0);
-		(void) signal(SIGALRM, SIG_DFL);
 		/* this is for MaxStartups and the child takes care of that */
 		(void) close(statup_pipe);
 		(void) pkcs11_engine_load(use_engine);
diff --git a/usr/src/cmd/ssh/sshd/sshd.c b/usr/src/cmd/ssh/sshd/sshd.c
index 6172234454..44ab561920 100644
--- a/usr/src/cmd/ssh/sshd/sshd.c
+++ b/usr/src/cmd/ssh/sshd/sshd.c
@@ -318,15 +318,15 @@ main_sigchld_handler(int sig)
 }
 
 /*
- * Signal handler for the alarm after the login grace period has expired.
+ * Signal handler for the alarm after the login grace period has expired. This
+ * is for the (soon-to-be) unprivileged child only. The monitor gets an event on
+ * the communication pipe and exits as well.
  */
 static void
 grace_alarm_handler(int sig)
 {
-	/* XXX no idea how fix this signal handler */
-
 	/* Log error and exit. */
-	fatal("Timeout before authentication for %s", get_remote_ipaddr());
+	fatal("Timeout before authentication for %.200s", get_remote_ipaddr());
 }
 
 #ifdef HAVE_SOLARIS_CONTRACTS
@@ -1512,18 +1512,6 @@ main(int ac, char **av)
 	/* Log the connection. */
 	verbose("Connection from %.500s port %d", remote_ip, remote_port);
 
-	/*
-	 * We don\'t want to listen forever unless the other side
-	 * successfully authenticates itself.  So we set up an alarm which is
-	 * cleared after successful authentication.  A limit of zero
-	 * indicates no limit. Note that we don\'t set the alarm in debugging
-	 * mode; it is just annoying to have the server exit just when you
-	 * are about to discover the bug.
-	 */
-	(void) signal(SIGALRM, grace_alarm_handler);
-	if (!debug_flag)
-		(void) alarm(options.login_grace_time);
-
 	sshd_exchange_identification(sock_in, sock_out);
 	/*
 	 * Check that the connection comes from a privileged port.
@@ -1561,11 +1549,25 @@ main(int ac, char **av)
 	 * PKCS#11 sessions. See the PKCS#11 standard for more information on
 	 * fork safety and packet.c for information about forking with the
 	 * engine.
+	 *
+	 * Note that the monitor stays in the function while the child is the
+	 * only one that returns.
 	 */
 	altprivsep_start_and_do_monitor(options.use_openssl_engine,
 	    inetd_flag, newsock, startup_pipe);
 
 	/*
+	 * We don't want to listen forever unless the other side successfully
+	 * authenticates itself. So we set up an alarm which is cleared after
+	 * successful authentication. A limit of zero indicates no limit. Note
+	 * that we don't set the alarm in debugging mode; it is just annoying to
+	 * have the server exit just when you are about to discover the bug.
+	 */
+	(void) signal(SIGALRM, grace_alarm_handler);
+	if (!debug_flag)
+		(void) alarm(options.login_grace_time);
+
+	/*
 	 * The child is about to start the first key exchange while the monitor
 	 * stays in altprivsep_start_and_do_monitor() function.
 	 */
@@ -1581,7 +1583,6 @@ main(int ac, char **av)
 		authctxt = do_authentication();
 	}
 
-authenticated:
 	/* Authentication complete */
 	(void) alarm(0);
 	/* we no longer need an alarm handler */