Accepting request 684224 from home:vitezslav_cizek:branches:network

- Fix two race conditions in sshd relating to SIGHUP (bsc#1119183)
  * 0001-upstream-Fix-two-race-conditions-in-sshd-relating-to.patch

OBS-URL: https://build.opensuse.org/request/show/684224
OBS-URL: https://build.opensuse.org/package/show/network/openssh?expand=0&rev=184

0001-upstream-Fix-two-race-conditions-in-sshd-relating-to.patch

@@ -0,0 +1,252 @@
+From 76a24b3fa193a9ca3e47a8779d497cb06500798b Mon Sep 17 00:00:00 2001
+From: "djm@openbsd.org" <djm@openbsd.org>
+Date: Fri, 1 Mar 2019 02:32:39 +0000
+Subject: upstream: Fix two race conditions in sshd relating to SIGHUP:
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+1. Recently-forked child processes will briefly remain listening to
+  listen_socks. If the main server sshd process completes its restart
+  via execv() before these sockets are closed by the child processes
+  then it can fail to listen at the desired addresses/ports and/or
+  fail to restart.
+
+2. When a SIGHUP is received, there may be forked child processes that
+  are awaiting their reexecution state. If the main server sshd
+  process restarts before passing this state, these child processes
+  will yield errors and use a fallback path of reading the current
+  sshd_config from the filesystem rather than use the one that sshd
+  was started with.
+
+To fix both of these cases, we reuse the startup_pipes that are shared
+between the main server sshd and forked children. Previously this was
+used solely to implement tracking of pre-auth child processes for
+MaxStartups, but this extends the messaging over these pipes to include
+a child->parent message that the parent process is safe to restart. This
+message is sent from the child after it has completed its preliminaries:
+closing listen_socks and receiving its reexec state.
+
+bz#2953, reported by Michal Koutný; ok markus@ dtucker@
+
+OpenBSD-Commit-ID: 7df09eacfa3ce13e9a7b1e9f17276ecc924d65ab
+---
+ sshd.c | 114 +++++++++++++++++++++++++++++++++++++++++++++++++----------------
+ 1 file changed, 86 insertions(+), 28 deletions(-)
+
+Index: openssh-7.9p1/sshd.c
+===================================================================
+--- openssh-7.9p1.orig/sshd.c	2019-03-11 15:26:34.532966127 +0100
++++ openssh-7.9p1/sshd.c	2019-03-11 16:05:21.242748303 +0100
+@@ -240,9 +240,26 @@ u_int session_id2_len = 0;
+ /* record remote hostname or ip */
+ u_int utmp_len = HOST_NAME_MAX+1;
+ 
+-/* options.max_startup sized array of fd ints */
++/*
++ * startup_pipes/flags are used for tracking children of the listening sshd
++ * process early in their lifespans. This tracking is needed for three things:
++ *
++ * 1) Implementing the MaxStartups limit of concurrent unauthenticated
++ *    connections.
++ * 2) Avoiding a race condition for SIGHUP processing, where child processes
++ *    may have listen_socks open that could collide with main listener process
++ *    after it restarts.
++ * 3) Ensuring that rexec'd sshd processes have received their initial state
++ *    from the parent listen process before handling SIGHUP.
++ *
++ * Child processes signal that they have completed closure of the listen_socks
++ * and (if applicable) received their rexec state by sending a char over their
++ * sock. Child processes signal that authentication has completed by closing
++ * the sock (or by exiting).
++ */
+ int *startup_pipes = NULL;
+-int startup_pipe;		/* in child */
++static int *startup_flags = NULL;	/* Indicates child closed listener */
++static int startup_pipe = -1;		/* in child */
+ 
+ /* variables used for privilege separation */
+ int use_privsep = -1;
+@@ -1081,14 +1098,9 @@ server_accept_inetd(int *sock_in, int *s
+ {
+ 	int fd;
+ 
+-	startup_pipe = -1;
+ 	if (rexeced_flag) {
+ 		close(REEXEC_CONFIG_PASS_FD);
+ 		*sock_in = *sock_out = dup(STDIN_FILENO);
+-		if (!debug_flag) {
+-			startup_pipe = dup(REEXEC_STARTUP_PIPE_FD);
+-			close(REEXEC_STARTUP_PIPE_FD);
+-		}
+ 	} else {
+ 		*sock_in = dup(STDIN_FILENO);
+ 		*sock_out = dup(STDOUT_FILENO);
+@@ -1213,8 +1225,9 @@ server_accept_loop(int *sock_in, int *so
+ {
+ 	fd_set *fdset;
+ 	int i, j, ret, maxfd;
+-	int startups = 0;
++	int startups = 0, listening = 0, lameduck = 0;
+ 	int startup_p[2] = { -1 , -1 };
++	char c = 0;
+ 	struct sockaddr_storage from;
+ 	socklen_t fromlen;
+ 	pid_t pid;
+@@ -1228,6 +1241,7 @@ server_accept_loop(int *sock_in, int *so
+ 			maxfd = listen_socks[i];
+ 	/* pipes connected to unauthenticated childs */
+ 	startup_pipes = xcalloc(options.max_startups, sizeof(int));
++	startup_flags = xcalloc(options.max_startups, sizeof(int));
+ 	for (i = 0; i < options.max_startups; i++)
+ 		startup_pipes[i] = -1;
+ 
+@@ -1236,8 +1250,15 @@ server_accept_loop(int *sock_in, int *so
+ 	 * the daemon is killed with a signal.
+ 	 */
+ 	for (;;) {
+-		if (received_sighup)
+-			sighup_restart();
++		if (received_sighup) {
++			if (!lameduck) {
++				debug("Received SIGHUP; waiting for children");
++				close_listen_socks();
++				lameduck = 1;
++			}
++			if (listening <= 0)
++				sighup_restart();
++		}
+ 		free(fdset);
+ 		fdset = xcalloc(howmany(maxfd + 1, NFDBITS),
+ 		    sizeof(fd_mask));
+@@ -1264,19 +1285,37 @@ server_accept_loop(int *sock_in, int *so
+ 		if (ret < 0)
+ 			continue;
+ 
+-		for (i = 0; i < options.max_startups; i++)
+-			if (startup_pipes[i] != -1 &&
+-			    FD_ISSET(startup_pipes[i], fdset)) {
+-				/*
+-				 * the read end of the pipe is ready
+-				 * if the child has closed the pipe
+-				 * after successful authentication
+-				 * or if the child has died
+-				 */
++		for (i = 0; i < options.max_startups; i++) {
++			if (startup_pipes[i] == -1 ||
++			    !FD_ISSET(startup_pipes[i], fdset))
++				continue;
++			switch (read(startup_pipes[i], &c, sizeof(c))) {
++			case -1:
++				if (errno == EINTR || errno == EAGAIN)
++					continue;
++				if (errno != EPIPE) {
++					error("%s: startup pipe %d (fd=%d): "
++					    "read %s", __func__, i,
++					    startup_pipes[i], strerror(errno));
++				}
++				/* FALLTHROUGH */
++			case 0:
++				/* child exited or completed auth */
+ 				close(startup_pipes[i]);
+ 				startup_pipes[i] = -1;
+ 				startups--;
++				if (startup_flags[i])
++					listening--;
++				break;
++			case 1:
++				/* child has finished preliminaries */
++				if (startup_flags[i]) {
++					listening--;
++					startup_flags[i] = 0;
++				}
++				break;
+ 			}
++		}
+ 		for (i = 0; i < num_listen_socks; i++) {
+ 			if (!FD_ISSET(listen_socks[i], fdset))
+ 				continue;
+@@ -1330,6 +1369,7 @@ server_accept_loop(int *sock_in, int *so
+ 					if (maxfd < startup_p[0])
+ 						maxfd = startup_p[0];
+ 					startups++;
++					startup_flags[j] = 1;
+ 					break;
+ 				}
+ 			if(!(--re_seeding_counter)) {
+@@ -1359,7 +1399,7 @@ server_accept_loop(int *sock_in, int *so
+ 					send_rexec_state(config_s[0], cfg);
+ 					close(config_s[0]);
+ 				}
+-				break;
++				return;
+ 			}
+ 
+ 			/*
+@@ -1368,13 +1408,14 @@ server_accept_loop(int *sock_in, int *so
+ 			 * parent continues listening.
+ 			 */
+ 			platform_pre_fork();
++			listening++;
+ 			if ((pid = fork()) == 0) {
+ 				/*
+ 				 * Child.  Close the listening and
+ 				 * max_startup sockets.  Start using
+ 				 * the accepted socket. Reinitialize
+ 				 * logging (since our pid has changed).
+-				 * We break out of the loop to handle
++				 * We return from this function to handle
+ 				 * the connection.
+ 				 */
+ 				platform_post_fork_child();
+@@ -1389,7 +1430,18 @@ server_accept_loop(int *sock_in, int *so
+ 				    log_stderr);
+ 				if (rexec_flag)
+ 					close(config_s[0]);
+-				break;
++				else {
++					/*
++					 * Signal parent that the preliminaries
++					 * for this child are complete. For the
++					 * re-exec case, this happens after the
++					 * child has received the rexec state
++					 * from the server.
++					 */
++					(void)atomicio(vwrite, startup_pipe,
++					    "\0", 1);
++				}
++				return;
+ 			}
+ 
+ 			/* Parent.  Stay in the loop. */
+@@ -1421,10 +1473,6 @@ server_accept_loop(int *sock_in, int *so
+ #endif
+ 			explicit_bzero(rnd, sizeof(rnd));
+ 		}
+-
+-		/* child process check (or debug mode) */
+-		if (num_listen_socks < 0)
+-			break;
+ 	}
+ }
+ 
+@@ -1760,8 +1808,18 @@ main(int ac, char **av)
+ 	/* Fetch our configuration */
+ 	if ((cfg = sshbuf_new()) == NULL)
+ 		fatal("%s: sshbuf_new failed", __func__);
+-	if (rexeced_flag)
++	if (rexeced_flag) {
+ 		recv_rexec_state(REEXEC_CONFIG_PASS_FD, cfg);
++		if (!debug_flag) {
++			startup_pipe = dup(REEXEC_STARTUP_PIPE_FD);
++			close(REEXEC_STARTUP_PIPE_FD);
++			/*
++			 * Signal parent that this child is at a point where
++			 * they can go away if they have a SIGHUP pending.
++			 */
++			(void)atomicio(vwrite, startup_pipe, "\0", 1);
++		}
++	}
+ 	else if (strcasecmp(config_file_name, "none") != 0)
+ 		load_server_config(config_file_name, cfg);
+ 

openssh.changes

@@ -1,3 +1,9 @@
+-------------------------------------------------------------------
+Mon Mar 11 15:06:17 UTC 2019 - Vítězslav Čížek <vcizek@suse.com>
+
+- Fix two race conditions in sshd relating to SIGHUP (bsc#1119183)
+  * 0001-upstream-Fix-two-race-conditions-in-sshd-relating-to.patch
+
 -------------------------------------------------------------------
 Thu Feb 28 19:20:58 UTC 2019 - Vítězslav Čížek <vcizek@suse.com>
 

openssh.spec

@@ -104,6 +104,7 @@ Patch36:        openssh-CVE-2019-6109-sanitize-scp-filenames.patch
 Patch37:        openssh-CVE-2019-6109-force-progressmeter-update.patch
 Patch38:        openssh-CVE-2019-6111-scp-client-wildcard.patch
 Patch39:        openssh-7.9p1-brace-expansion.patch
+Patch40:        0001-upstream-Fix-two-race-conditions-in-sshd-relating-to.patch
 BuildRequires:  audit-devel
 BuildRequires:  autoconf
 BuildRequires:  groff