summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authordjm@openbsd.org <djm@openbsd.org>2019-03-01 02:32:39 +0000
committerDamien Miller <djm@mindrot.org>2019-03-01 13:34:00 +1100
commit76a24b3fa193a9ca3e47a8779d497cb06500798b (patch)
treee0481606d35b25110206aefa6024588827f86996
parentde817e9dfab99473017d28cdf69e60397d00ea21 (diff)
upstream: Fix two race conditions in sshd relating to SIGHUP:
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
-rw-r--r--sshd.c114
1 files changed, 86 insertions, 28 deletions
diff --git a/sshd.c b/sshd.c
index 058260d6f..cbd3bce91 100644
--- a/sshd.c
+++ b/sshd.c
@@ -1,4 +1,4 @@
1/* $OpenBSD: sshd.c,v 1.532 2019/01/21 10:38:54 djm Exp $ */ 1/* $OpenBSD: sshd.c,v 1.533 2019/03/01 02:32:39 djm Exp $ */
2/* 2/*
3 * Author: Tatu Ylonen <ylo@cs.hut.fi> 3 * Author: Tatu Ylonen <ylo@cs.hut.fi>
4 * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland 4 * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -213,9 +213,26 @@ u_int session_id2_len = 0;
213/* record remote hostname or ip */ 213/* record remote hostname or ip */
214u_int utmp_len = HOST_NAME_MAX+1; 214u_int utmp_len = HOST_NAME_MAX+1;
215 215
216/* options.max_startup sized array of fd ints */ 216/*
217 * startup_pipes/flags are used for tracking children of the listening sshd
218 * process early in their lifespans. This tracking is needed for three things:
219 *
220 * 1) Implementing the MaxStartups limit of concurrent unauthenticated
221 * connections.
222 * 2) Avoiding a race condition for SIGHUP processing, where child processes
223 * may have listen_socks open that could collide with main listener process
224 * after it restarts.
225 * 3) Ensuring that rexec'd sshd processes have received their initial state
226 * from the parent listen process before handling SIGHUP.
227 *
228 * Child processes signal that they have completed closure of the listen_socks
229 * and (if applicable) received their rexec state by sending a char over their
230 * sock. Child processes signal that authentication has completed by closing
231 * the sock (or by exiting).
232 */
217static int *startup_pipes = NULL; 233static int *startup_pipes = NULL;
218static int startup_pipe; /* in child */ 234static int *startup_flags = NULL; /* Indicates child closed listener */
235static int startup_pipe = -1; /* in child */
219 236
220/* variables used for privilege separation */ 237/* variables used for privilege separation */
221int use_privsep = -1; 238int use_privsep = -1;
@@ -901,14 +918,9 @@ server_accept_inetd(int *sock_in, int *sock_out)
901{ 918{
902 int fd; 919 int fd;
903 920
904 startup_pipe = -1;
905 if (rexeced_flag) { 921 if (rexeced_flag) {
906 close(REEXEC_CONFIG_PASS_FD); 922 close(REEXEC_CONFIG_PASS_FD);
907 *sock_in = *sock_out = dup(STDIN_FILENO); 923 *sock_in = *sock_out = dup(STDIN_FILENO);
908 if (!debug_flag) {
909 startup_pipe = dup(REEXEC_STARTUP_PIPE_FD);
910 close(REEXEC_STARTUP_PIPE_FD);
911 }
912 } else { 924 } else {
913 *sock_in = dup(STDIN_FILENO); 925 *sock_in = dup(STDIN_FILENO);
914 *sock_out = dup(STDOUT_FILENO); 926 *sock_out = dup(STDOUT_FILENO);
@@ -1033,8 +1045,9 @@ server_accept_loop(int *sock_in, int *sock_out, int *newsock, int *config_s)
1033{ 1045{
1034 fd_set *fdset; 1046 fd_set *fdset;
1035 int i, j, ret, maxfd; 1047 int i, j, ret, maxfd;
1036 int startups = 0; 1048 int startups = 0, listening = 0, lameduck = 0;
1037 int startup_p[2] = { -1 , -1 }; 1049 int startup_p[2] = { -1 , -1 };
1050 char c = 0;
1038 struct sockaddr_storage from; 1051 struct sockaddr_storage from;
1039 socklen_t fromlen; 1052 socklen_t fromlen;
1040 pid_t pid; 1053 pid_t pid;
@@ -1048,6 +1061,7 @@ server_accept_loop(int *sock_in, int *sock_out, int *newsock, int *config_s)
1048 maxfd = listen_socks[i]; 1061 maxfd = listen_socks[i];
1049 /* pipes connected to unauthenticated childs */ 1062 /* pipes connected to unauthenticated childs */
1050 startup_pipes = xcalloc(options.max_startups, sizeof(int)); 1063 startup_pipes = xcalloc(options.max_startups, sizeof(int));
1064 startup_flags = xcalloc(options.max_startups, sizeof(int));
1051 for (i = 0; i < options.max_startups; i++) 1065 for (i = 0; i < options.max_startups; i++)
1052 startup_pipes[i] = -1; 1066 startup_pipes[i] = -1;
1053 1067
@@ -1056,8 +1070,15 @@ server_accept_loop(int *sock_in, int *sock_out, int *newsock, int *config_s)
1056 * the daemon is killed with a signal. 1070 * the daemon is killed with a signal.
1057 */ 1071 */
1058 for (;;) { 1072 for (;;) {
1059 if (received_sighup) 1073 if (received_sighup) {
1060 sighup_restart(); 1074 if (!lameduck) {
1075 debug("Received SIGHUP; waiting for children");
1076 close_listen_socks();
1077 lameduck = 1;
1078 }
1079 if (listening <= 0)
1080 sighup_restart();
1081 }
1061 free(fdset); 1082 free(fdset);
1062 fdset = xcalloc(howmany(maxfd + 1, NFDBITS), 1083 fdset = xcalloc(howmany(maxfd + 1, NFDBITS),
1063 sizeof(fd_mask)); 1084 sizeof(fd_mask));
@@ -1083,19 +1104,37 @@ server_accept_loop(int *sock_in, int *sock_out, int *newsock, int *config_s)
1083 if (ret < 0) 1104 if (ret < 0)
1084 continue; 1105 continue;
1085 1106
1086 for (i = 0; i < options.max_startups; i++) 1107 for (i = 0; i < options.max_startups; i++) {
1087 if (startup_pipes[i] != -1 && 1108 if (startup_pipes[i] == -1 ||
1088 FD_ISSET(startup_pipes[i], fdset)) { 1109 !FD_ISSET(startup_pipes[i], fdset))
1089 /* 1110 continue;
1090 * the read end of the pipe is ready 1111 switch (read(startup_pipes[i], &c, sizeof(c))) {
1091 * if the child has closed the pipe 1112 case -1:
1092 * after successful authentication 1113 if (errno == EINTR || errno == EAGAIN)
1093 * or if the child has died 1114 continue;
1094 */ 1115 if (errno != EPIPE) {
1116 error("%s: startup pipe %d (fd=%d): "
1117 "read %s", __func__, i,
1118 startup_pipes[i], strerror(errno));
1119 }
1120 /* FALLTHROUGH */
1121 case 0:
1122 /* child exited or completed auth */
1095 close(startup_pipes[i]); 1123 close(startup_pipes[i]);
1096 startup_pipes[i] = -1; 1124 startup_pipes[i] = -1;
1097 startups--; 1125 startups--;
1126 if (startup_flags[i])
1127 listening--;
1128 break;
1129 case 1:
1130 /* child has finished preliminaries */
1131 if (startup_flags[i]) {
1132 listening--;
1133 startup_flags[i] = 0;
1134 }
1135 break;
1098 } 1136 }
1137 }
1099 for (i = 0; i < num_listen_socks; i++) { 1138 for (i = 0; i < num_listen_socks; i++) {
1100 if (!FD_ISSET(listen_socks[i], fdset)) 1139 if (!FD_ISSET(listen_socks[i], fdset))
1101 continue; 1140 continue;
@@ -1149,6 +1188,7 @@ server_accept_loop(int *sock_in, int *sock_out, int *newsock, int *config_s)
1149 if (maxfd < startup_p[0]) 1188 if (maxfd < startup_p[0])
1150 maxfd = startup_p[0]; 1189 maxfd = startup_p[0];
1151 startups++; 1190 startups++;
1191 startup_flags[j] = 1;
1152 break; 1192 break;
1153 } 1193 }
1154 1194
@@ -1174,7 +1214,7 @@ server_accept_loop(int *sock_in, int *sock_out, int *newsock, int *config_s)
1174 send_rexec_state(config_s[0], cfg); 1214 send_rexec_state(config_s[0], cfg);
1175 close(config_s[0]); 1215 close(config_s[0]);
1176 } 1216 }
1177 break; 1217 return;
1178 } 1218 }
1179 1219
1180 /* 1220 /*
@@ -1183,13 +1223,14 @@ server_accept_loop(int *sock_in, int *sock_out, int *newsock, int *config_s)
1183 * parent continues listening. 1223 * parent continues listening.
1184 */ 1224 */
1185 platform_pre_fork(); 1225 platform_pre_fork();
1226 listening++;
1186 if ((pid = fork()) == 0) { 1227 if ((pid = fork()) == 0) {
1187 /* 1228 /*
1188 * Child. Close the listening and 1229 * Child. Close the listening and
1189 * max_startup sockets. Start using 1230 * max_startup sockets. Start using
1190 * the accepted socket. Reinitialize 1231 * the accepted socket. Reinitialize
1191 * logging (since our pid has changed). 1232 * logging (since our pid has changed).
1192 * We break out of the loop to handle 1233 * We return from this function to handle
1193 * the connection. 1234 * the connection.
1194 */ 1235 */
1195 platform_post_fork_child(); 1236 platform_post_fork_child();
@@ -1204,7 +1245,18 @@ server_accept_loop(int *sock_in, int *sock_out, int *newsock, int *config_s)
1204 log_stderr); 1245 log_stderr);
1205 if (rexec_flag) 1246 if (rexec_flag)
1206 close(config_s[0]); 1247 close(config_s[0]);
1207 break; 1248 else {
1249 /*
1250 * Signal parent that the preliminaries
1251 * for this child are complete. For the
1252 * re-exec case, this happens after the
1253 * child has received the rexec state
1254 * from the server.
1255 */
1256 (void)atomicio(vwrite, startup_pipe,
1257 "\0", 1);
1258 }
1259 return;
1208 } 1260 }
1209 1261
1210 /* Parent. Stay in the loop. */ 1262 /* Parent. Stay in the loop. */
@@ -1236,10 +1288,6 @@ server_accept_loop(int *sock_in, int *sock_out, int *newsock, int *config_s)
1236#endif 1288#endif
1237 explicit_bzero(rnd, sizeof(rnd)); 1289 explicit_bzero(rnd, sizeof(rnd));
1238 } 1290 }
1239
1240 /* child process check (or debug mode) */
1241 if (num_listen_socks < 0)
1242 break;
1243 } 1291 }
1244} 1292}
1245 1293
@@ -1569,8 +1617,18 @@ main(int ac, char **av)
1569 /* Fetch our configuration */ 1617 /* Fetch our configuration */
1570 if ((cfg = sshbuf_new()) == NULL) 1618 if ((cfg = sshbuf_new()) == NULL)
1571 fatal("%s: sshbuf_new failed", __func__); 1619 fatal("%s: sshbuf_new failed", __func__);
1572 if (rexeced_flag) 1620 if (rexeced_flag) {
1573 recv_rexec_state(REEXEC_CONFIG_PASS_FD, cfg); 1621 recv_rexec_state(REEXEC_CONFIG_PASS_FD, cfg);
1622 if (!debug_flag) {
1623 startup_pipe = dup(REEXEC_STARTUP_PIPE_FD);
1624 close(REEXEC_STARTUP_PIPE_FD);
1625 /*
1626 * Signal parent that this child is at a point where
1627 * they can go away if they have a SIGHUP pending.
1628 */
1629 (void)atomicio(vwrite, startup_pipe, "\0", 1);
1630 }
1631 }
1574 else if (strcasecmp(config_file_name, "none") != 0) 1632 else if (strcasecmp(config_file_name, "none") != 0)
1575 load_server_config(config_file_name, cfg); 1633 load_server_config(config_file_name, cfg);
1576 1634