- djm@cvs.openbsd.org 2010/06/25 07:20:04

[channels.c session.c] bz#1750: fix requirement for /dev/null inside ChrootDirectory for internal-sftp accidentally introduced in r1.253 by removing the code that opens and dup /dev/null to stderr and modifying the channels code to read stderr but discard it instead; ok markus@
2010-06-26 10:00:14 +10:00 · 2010-06-26 10:00:14 +10:00 · 8853ca5fc4
parent 232cfb1b1d
commit 8853ca5fc4
3 changed files with 46 additions and 58 deletions
--- a/6
+++ b/6
@ -57,6 +57,12 @@
     [channels.c mux.c readconf.c readconf.h ssh.h]
     bz#1327: remove hardcoded limit of 100 permitopen clauses and port
     forwards per direction; ok markus@ stevesk@
+   - djm@cvs.openbsd.org 2010/06/25 07:20:04
+     [channels.c session.c]
+     bz#1750: fix requirement for /dev/null inside ChrootDirectory for
+     internal-sftp accidentally introduced in r1.253 by removing the code
+     that opens and dup /dev/null to stderr and modifying the channels code
+     to read stderr but discard it instead; ok markus@

 20100622
 - (djm) [loginrec.c] crank LINFO_NAMESIZE (username length) to 512
--- a/channels.c
+++ b/channels.c
@ -1,4 +1,4 @@
-/* $OpenBSD: channels.c,v 1.305 2010/06/25 07:14:45 djm Exp $ */
+/* $OpenBSD: channels.c,v 1.306 2010/06/25 07:20:04 djm Exp $ */
 /*
 * Author: Tatu Ylonen <ylo@cs.hut.fi>
 * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@ -839,8 +839,9 @@ channel_pre_open(Channel *c, fd_set *readset, fd_set *writeset)
 		if (c->extended_usage == CHAN_EXTENDED_WRITE &&
 		    buffer_len(&c->extended) > 0)
 			FD_SET(c->efd, writeset);
-		else if (!(c->flags & CHAN_EOF_SENT) &&
-		    c->extended_usage == CHAN_EXTENDED_READ &&
+		else if (c->efd != -1 && !(c->flags & CHAN_EOF_SENT) &&
+		    (c->extended_usage == CHAN_EXTENDED_READ ||
+		    c->extended_usage == CHAN_EXTENDED_IGNORE) &&
 		    buffer_len(&c->extended) < c->remote_window)
 			FD_SET(c->efd, readset);
 	}
@ -1756,7 +1757,9 @@ channel_handle_efd(Channel *c, fd_set *readset, fd_set *writeset)
 				buffer_consume(&c->extended, len);
 				c->local_consumed += len;
 			}
-		} else if (c->extended_usage == CHAN_EXTENDED_READ &&
+		} else if (c->efd != -1 &&
+		    (c->extended_usage == CHAN_EXTENDED_READ ||
+		    c->extended_usage == CHAN_EXTENDED_IGNORE) &&
 		    (c->detach_close || FD_ISSET(c->efd, readset))) {
 			len = read(c->efd, buf, sizeof(buf));
 			debug2("channel %d: read %d from efd %d",
@ -1769,7 +1772,11 @@ channel_handle_efd(Channel *c, fd_set *readset, fd_set *writeset)
 				    c->self, c->efd);
 				channel_close_fd(&c->efd);
 			} else {
-				buffer_append(&c->extended, buf, len);
+				if (c->extended_usage == CHAN_EXTENDED_IGNORE) {
+					debug3("channel %d: discard efd",
+					    c->self);
+				} else
+					buffer_append(&c->extended, buf, len);
 			}
 		}
 	}
--- a/session.c
+++ b/session.c
@ -1,4 +1,4 @@
-/* $OpenBSD: session.c,v 1.255 2010/06/22 04:59:12 djm Exp $ */
+/* $OpenBSD: session.c,v 1.256 2010/06/25 07:20:04 djm Exp $ */
 /*
 * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
 *                    All rights reserved
@ -105,7 +105,7 @@
 /* func */

 Session *session_new(void);
-void	session_set_fds(Session *, int, int, int, int);
+void	session_set_fds(Session *, int, int, int, int, int);
 void	session_pty_cleanup(Session *);
 void	session_proctitle(Session *);
 int	session_setup_x11fwd(Session *);
@ -462,27 +462,14 @@ do_exec_no_pty(Session *s, const char *command)
 		close(pin[1]);
 		return -1;
 	}
-	if (s->is_subsystem) {
-	    	if ((perr[1] = open(_PATH_DEVNULL, O_WRONLY)) == -1) {
-			error("%s: open(%s): %s", __func__, _PATH_DEVNULL,
-			    strerror(errno));
-			close(pin[0]);
-			close(pin[1]);
-			close(pout[0]);
-			close(pout[1]);
-			return -1;
-		}
-		perr[0] = -1;
-	} else {
-		if (pipe(perr) < 0) {
-			error("%s: pipe err: %.100s", __func__,
-			    strerror(errno));
-			close(pin[0]);
-			close(pin[1]);
-			close(pout[0]);
-			close(pout[1]);
-			return -1;
-		}
+	if (pipe(perr) < 0) {
+		error("%s: pipe err: %.100s", __func__,
+		    strerror(errno));
+		close(pin[0]);
+		close(pin[1]);
+		close(pout[0]);
+		close(pout[1]);
+		return -1;
 	}
 #else
 	int inout[2], err[2];
@ -495,23 +482,12 @@ do_exec_no_pty(Session *s, const char *command)
 		error("%s: socketpair #1: %.100s", __func__, strerror(errno));
 		return -1;
 	}
-	if (s->is_subsystem) {
-	    	if ((err[0] = open(_PATH_DEVNULL, O_WRONLY)) == -1) {
-			error("%s: open(%s): %s", __func__, _PATH_DEVNULL,
-			    strerror(errno));
-			close(inout[0]);
-			close(inout[1]);
-			return -1;
-		}
-		err[1] = -1;
-	} else {
-		if (socketpair(AF_UNIX, SOCK_STREAM, 0, err) < 0) {
-			error("%s: socketpair #2: %.100s", __func__,
-			    strerror(errno));
-			close(inout[0]);
-			close(inout[1]);
-			return -1;
-		}
+	if (socketpair(AF_UNIX, SOCK_STREAM, 0, err) < 0) {
+		error("%s: socketpair #2: %.100s", __func__,
+		    strerror(errno));
+		close(inout[0]);
+		close(inout[1]);
+		return -1;
 	}
 #endif

@ -526,15 +502,13 @@ do_exec_no_pty(Session *s, const char *command)
 		close(pin[1]);
 		close(pout[0]);
 		close(pout[1]);
-		if (perr[0] != -1)
-			close(perr[0]);
+		close(perr[0]);
 		close(perr[1]);
 #else
 		close(inout[0]);
 		close(inout[1]);
 		close(err[0]);
-		if (err[1] != -1)
-			close(err[1]);
+		close(err[1]);
 #endif
 		return -1;
 	case 0:
@ -568,8 +542,7 @@ do_exec_no_pty(Session *s, const char *command)
 		close(pout[1]);

 		/* Redirect stderr. */
-		if (perr[0] != -1)
-			close(perr[0]);
+		close(perr[0]);
 		if (dup2(perr[1], 2) < 0)
 			perror("dup2 stderr");
 		close(perr[1]);
@ -580,8 +553,7 @@ do_exec_no_pty(Session *s, const char *command)
 		 * seem to depend on it.
 		 */
 		close(inout[1]);
-		if (err[1] != -1)
-			close(err[1]);
+		close(err[1]);
 		if (dup2(inout[0], 0) < 0)	/* stdin */
 			perror("dup2 stdin");
 		if (dup2(inout[0], 1) < 0)	/* stdout (same as stdin) */
@ -629,7 +601,8 @@ do_exec_no_pty(Session *s, const char *command)
 	close(perr[1]);

 	if (compat20) {
-		session_set_fds(s, pin[1], pout[0], perr[0], 0);
+		session_set_fds(s, pin[1], pout[0], perr[0],
+		    s->is_subsystem, 0);
 	} else {
 		/* Enter the interactive session. */
 		server_loop(pid, pin[1], pout[0], perr[0]);
@ -645,7 +618,8 @@ do_exec_no_pty(Session *s, const char *command)
 	 * handle the case that fdin and fdout are the same.
 	 */
 	if (compat20) {
-		session_set_fds(s, inout[1], inout[1], err[1], 0);
+		session_set_fds(s, inout[1], inout[1], err[1],
+		    s->is_subsystem, 0);
 	} else {
 		server_loop(pid, inout[1], inout[1], err[1]);
 		/* server_loop has closed inout[1] and err[1]. */
@ -767,7 +741,7 @@ do_exec_pty(Session *s, const char *command)
 	s->ptymaster = ptymaster;
 	packet_set_interactive(1);
 	if (compat20) {
-		session_set_fds(s, ptyfd, fdout, -1, 1);
+		session_set_fds(s, ptyfd, fdout, -1, 1, 1);
 	} else {
 		server_loop(pid, ptyfd, fdout, -1);
 		/* server_loop _has_ closed ptyfd and fdout. */
@ -2348,7 +2322,8 @@ session_input_channel_req(Channel *c, const char *rtype)
 }

 void
-session_set_fds(Session *s, int fdin, int fdout, int fderr, int is_tty)
+session_set_fds(Session *s, int fdin, int fdout, int fderr, int ignore_fderr,
+    int is_tty)
 {
 	if (!compat20)
 		fatal("session_set_fds: called for proto != 2.0");
@ -2360,7 +2335,7 @@ session_set_fds(Session *s, int fdin, int fdout, int fderr, int is_tty)
 		fatal("no channel for session %d", s->self);
 	channel_set_fds(s->chanid,
 	    fdout, fdin, fderr,
-	    fderr == -1 ? CHAN_EXTENDED_IGNORE : CHAN_EXTENDED_READ,
+	    ignore_fderr ? CHAN_EXTENDED_IGNORE : CHAN_EXTENDED_READ,
 	    1, is_tty, CHAN_SES_WINDOW_DEFAULT);
 }