- djm@cvs.openbsd.org 2007/09/04 03:21:03

     [clientloop.c monitor.c monitor_fdpass.c monitor_fdpass.h]
     [monitor_wrap.c ssh.c]
     make file descriptor passing code return an error rather than call fatal()
     when it encounters problems, and use this to make session multiplexing
     masters survive slaves failing to pass all stdio FDs; ok markus@

7 files changed, 76 insertions, 38 deletions

diff --git a/ChangeLog b/ChangeLog
index f5ee63e4f..c0a927051 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -19,6 +19,12 @@
    - djm@cvs.openbsd.org 2007/08/23 03:23:26
      [sshconnect.c]
      Execute ProxyCommands with $SHELL rather than /bin/sh unconditionally
+   - djm@cvs.openbsd.org 2007/09/04 03:21:03
+     [clientloop.c monitor.c monitor_fdpass.c monitor_fdpass.h]
+     [monitor_wrap.c ssh.c]
+     make file descriptor passing code return an error rather than call fatal()
+     when it encounters problems, and use this to make session multiplexing
+     masters survive slaves failing to pass all stdio FDs; ok markus@
 
 20070914
  - (dtucker) [openbsd-compat/bsd-asprintf.c] Plug mem leak in error path.
@@ -3216,4 +3222,4 @@
    OpenServer 6 and add osr5bigcrypt support so when someone migrates
    passwords between UnixWare and OpenServer they will still work. OK dtucker@
 
-$Id: ChangeLog,v 1.4747 2007/09/17 01:58:04 djm Exp $
+$Id: ChangeLog,v 1.4748 2007/09/17 02:04:08 djm Exp $
diff --git a/clientloop.c b/clientloop.c
index b57fda042..7a61cb74d 100644
--- a/clientloop.c
+++ b/clientloop.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: clientloop.c,v 1.181 2007/08/15 08:14:46 markus Exp $ */
+/* $OpenBSD: clientloop.c,v 1.182 2007/09/04 03:21:03 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -722,7 +722,7 @@ client_process_control(fd_set *readset)
         struct sockaddr_storage addr;
         struct confirm_ctx *cctx;
         char *cmd;
-        u_int i, len, env_len, command, flags;
+        u_int i, j, len, env_len, command, flags;
         uid_t euid;
         gid_t egid;
 
@@ -870,9 +870,22 @@ client_process_control(fd_set *readset)
         xfree(cmd);
 
         /* Gather fds from client */
-        new_fd[0] = mm_receive_fd(client_fd);
-        new_fd[1] = mm_receive_fd(client_fd);
-        new_fd[2] = mm_receive_fd(client_fd);
+        for(i = 0; i < 3; i++) {
+                if ((new_fd[i] = mm_receive_fd(client_fd)) == -1) {
+                        error("%s: failed to receive fd %d from slave",
+                            __func__, i);
+                        for (j = 0; j < i; j++)
+                                close(new_fd[j]);
+                        for (j = 0; j < env_len; j++)
+                                xfree(cctx->env[j]);
+                        if (env_len > 0)
+                                xfree(cctx->env);
+                        xfree(cctx->term);
+                        buffer_free(&cctx->cmd);
+                        xfree(cctx);
+                        return;
+                }
+        }
 
         debug2("%s: got fds stdin %d, stdout %d, stderr %d", __func__,
             new_fd[0], new_fd[1], new_fd[2]);
diff --git a/monitor.c b/monitor.c
index 08c7ea3cb..1fe1fb56f 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: monitor.c,v 1.91 2007/05/17 20:52:13 djm Exp $ */
+/* $OpenBSD: monitor.c,v 1.92 2007/09/04 03:21:03 djm Exp $ */
 /*
  * Copyright 2002 Niels Provos <provos@citi.umich.edu>
  * Copyright 2002 Markus Friedl <markus@openbsd.org>
@@ -1314,8 +1314,9 @@ mm_answer_pty(int sock, Buffer *m)
 
         mm_request_send(sock, MONITOR_ANS_PTY, m);
 
-        mm_send_fd(sock, s->ptyfd);
-        mm_send_fd(sock, s->ttyfd);
+        if (mm_send_fd(sock, s->ptyfd) == -1 ||
+            mm_send_fd(sock, s->ttyfd) == -1)
+                fatal("%s: send fds failed", __func__);
 
         /* make sure nothing uses fd 0 */
         if ((fd0 = open(_PATH_DEVNULL, O_RDONLY)) < 0)
diff --git a/monitor_fdpass.c b/monitor_fdpass.c
index 9f8e9cd55..a572302e8 100644
--- a/monitor_fdpass.c
+++ b/monitor_fdpass.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: monitor_fdpass.c,v 1.12 2006/08/03 03:34:42 deraadt Exp $ */
+/* $OpenBSD: monitor_fdpass.c,v 1.13 2007/09/04 03:21:03 djm Exp $ */
 /*
  * Copyright 2001 Niels Provos <provos@citi.umich.edu>
  * All rights reserved.
@@ -40,7 +40,7 @@
 #include "log.h"
 #include "monitor_fdpass.h"
 
-void
+int
 mm_send_fd(int sock, int fd)
 {
 #if defined(HAVE_SENDMSG) && (defined(HAVE_ACCRIGHTS_IN_MSGHDR) || defined(HAVE_CONTROL_IN_MSGHDR))
@@ -72,15 +72,21 @@ mm_send_fd(int sock, int fd)
         msg.msg_iov = &vec;
         msg.msg_iovlen = 1;
 
-        if ((n = sendmsg(sock, &msg, 0)) == -1)
-                fatal("%s: sendmsg(%d): %s", __func__, fd,
+        if ((n = sendmsg(sock, &msg, 0)) == -1) {
+                error("%s: sendmsg(%d): %s", __func__, fd,
                     strerror(errno));
-        if (n != 1)
-                fatal("%s: sendmsg: expected sent 1 got %ld",
+                return -1;
+        }
+
+        if (n != 1) {
+                error("%s: sendmsg: expected sent 1 got %ld",
                     __func__, (long)n);
+                return -1;
+        }
+        return 0;
 #else
-        fatal("%s: UsePrivilegeSeparation=yes not supported",
-            __func__);
+        error("%s: file descriptor passing not supported", __func__);
+        return -1;
 #endif
 }
 
@@ -111,29 +117,39 @@ mm_receive_fd(int sock)
         msg.msg_controllen = sizeof(tmp);
 #endif
 
-        if ((n = recvmsg(sock, &msg, 0)) == -1)
-                fatal("%s: recvmsg: %s", __func__, strerror(errno));
-        if (n != 1)
-                fatal("%s: recvmsg: expected received 1 got %ld",
+        if ((n = recvmsg(sock, &msg, 0)) == -1) {
+                error("%s: recvmsg: %s", __func__, strerror(errno));
+                return -1;
+        }
+        if (n != 1) {
+                error("%s: recvmsg: expected received 1 got %ld",
                     __func__, (long)n);
+                return -1;
+        }
 
 #ifdef HAVE_ACCRIGHTS_IN_MSGHDR
-        if (msg.msg_accrightslen != sizeof(fd))
-                fatal("%s: no fd", __func__);
+        if (msg.msg_accrightslen != sizeof(fd)) {
+                error("%s: no fd", __func__);
+                return -1;
+        }
 #else
         cmsg = CMSG_FIRSTHDR(&msg);
-        if (cmsg == NULL)
-                fatal("%s: no message header", __func__);
+        if (cmsg == NULL) {
+                error("%s: no message header", __func__);
+                return -1;
+        }
 #ifndef BROKEN_CMSG_TYPE
-        if (cmsg->cmsg_type != SCM_RIGHTS)
-                fatal("%s: expected type %d got %d", __func__,
+        if (cmsg->cmsg_type != SCM_RIGHTS) {
+                error("%s: expected type %d got %d", __func__,
                     SCM_RIGHTS, cmsg->cmsg_type);
+                return -1;
+        }
 #endif
         fd = (*(int *)CMSG_DATA(cmsg));
 #endif
         return fd;
 #else
-        fatal("%s: UsePrivilegeSeparation=yes not supported",
-            __func__);
+        error("%s: file descriptor passing not supported", __func__);
+        return -1;
 #endif
 }
diff --git a/monitor_fdpass.h b/monitor_fdpass.h
index 12c67ec2d..a4b1f6358 100644
--- a/monitor_fdpass.h
+++ b/monitor_fdpass.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: monitor_fdpass.h,v 1.3 2006/03/25 22:22:43 djm Exp $ */
+/* $OpenBSD: monitor_fdpass.h,v 1.4 2007/09/04 03:21:03 djm Exp $ */
 
 /*
  * Copyright 2002 Niels Provos <provos@citi.umich.edu>
@@ -28,7 +28,7 @@
 #ifndef _MM_FDPASS_H_
 #define _MM_FDPASS_H_
 
-void mm_send_fd(int, int);
+int mm_send_fd(int, int);
 int mm_receive_fd(int);
 
 #endif /* _MM_FDPASS_H_ */
diff --git a/monitor_wrap.c b/monitor_wrap.c
index edf2814e5..36154be4d 100644
--- a/monitor_wrap.c
+++ b/monitor_wrap.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: monitor_wrap.c,v 1.57 2007/06/07 19:37:34 pvalchev Exp $ */
+/* $OpenBSD: monitor_wrap.c,v 1.58 2007/09/04 03:21:03 djm Exp $ */
 /*
  * Copyright 2002 Niels Provos <provos@citi.umich.edu>
  * Copyright 2002 Markus Friedl <markus@openbsd.org>
@@ -688,8 +688,9 @@ mm_pty_allocate(int *ptyfd, int *ttyfd, char *namebuf, size_t namebuflen)
         buffer_append(&loginmsg, msg, strlen(msg));
         xfree(msg);
 
-        *ptyfd = mm_receive_fd(pmonitor->m_recvfd);
-        *ttyfd = mm_receive_fd(pmonitor->m_recvfd);
+        if ((*ptyfd = mm_receive_fd(pmonitor->m_recvfd)) == -1 ||
+            (*ttyfd = mm_receive_fd(pmonitor->m_recvfd)) == -1)
+                fatal("%s: receive fds failed", __func__);
 
         /* Success */
         return (1);
diff --git a/ssh.c b/ssh.c
index d3a7ffc9b..7f8ea0d17 100644
--- a/ssh.c
+++ b/ssh.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh.c,v 1.301 2007/08/07 07:32:53 djm Exp $ */
+/* $OpenBSD: ssh.c,v 1.302 2007/09/04 03:21:03 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -1426,9 +1426,10 @@ control_client(const char *path)
         if (ssh_msg_send(sock, SSHMUX_VER, &m) == -1)
                 fatal("%s: msg_send", __func__);
 
-        mm_send_fd(sock, STDIN_FILENO);
-        mm_send_fd(sock, STDOUT_FILENO);
-        mm_send_fd(sock, STDERR_FILENO);
+        if (mm_send_fd(sock, STDIN_FILENO) == -1 ||
+            mm_send_fd(sock, STDOUT_FILENO) == -1 ||
+            mm_send_fd(sock, STDERR_FILENO) == -1)
+                fatal("%s: send fds failed", __func__);
 
         /* Wait for reply, so master has a chance to gather ttymodes */
         buffer_clear(&m);