From 22a29880bb324983a997f6bc03484879cae1f912 Mon Sep 17 00:00:00 2001 From: Damien Miller Date: Mon, 10 May 2010 11:53:54 +1000 Subject: - djm@cvs.openbsd.org 2010/04/23 22:42:05 [session.c] set stderr to /dev/null for subsystems rather than just closing it. avoids hangs if a subsystem or shell initialisation writes to stderr. bz#1750; ok markus@ --- session.c | 83 ++++++++++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 55 insertions(+), 28 deletions(-) (limited to 'session.c') diff --git a/session.c b/session.c index e032de692..b49d08d60 100644 --- a/session.c +++ b/session.c @@ -1,4 +1,4 @@ -/* $OpenBSD: session.c,v 1.252 2010/03/07 11:57:13 dtucker Exp $ */ +/* $OpenBSD: session.c,v 1.253 2010/04/23 22:42:05 djm Exp $ */ /* * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland * All rights reserved @@ -47,6 +47,7 @@ #include #include +#include #include #ifdef HAVE_PATHS_H #include @@ -447,6 +448,9 @@ do_exec_no_pty(Session *s, const char *command) #ifdef USE_PIPES int pin[2], pout[2], perr[2]; + if (s == NULL) + fatal("do_exec_no_pty: no session"); + /* Allocate pipes for communicating with the program. */ if (pipe(pin) < 0) { error("%s: pipe in: %.100s", __func__, strerror(errno)); @@ -458,33 +462,59 @@ do_exec_no_pty(Session *s, const char *command) close(pin[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; + 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; + } } #else int inout[2], err[2]; + if (s == NULL) + fatal("do_exec_no_pty: no session"); + /* Uses socket pairs to communicate with the program. */ if (socketpair(AF_UNIX, SOCK_STREAM, 0, inout) < 0) { error("%s: socketpair #1: %.100s", __func__, strerror(errno)); 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; + 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; + } } #endif - if (s == NULL) - fatal("do_exec_no_pty: no session"); - session_proctitle(s); /* Fork the child. */ @@ -496,13 +526,15 @@ do_exec_no_pty(Session *s, const char *command) close(pin[1]); close(pout[0]); close(pout[1]); - close(perr[0]); + if (perr[0] != -1) + close(perr[0]); close(perr[1]); #else close(inout[0]); close(inout[1]); close(err[0]); - close(err[1]); + if (err[1] != -1) + close(err[1]); #endif return -1; case 0: @@ -536,7 +568,8 @@ do_exec_no_pty(Session *s, const char *command) close(pout[1]); /* Redirect stderr. */ - close(perr[0]); + if (perr[0] != -1) + close(perr[0]); if (dup2(perr[1], 2) < 0) perror("dup2 stderr"); close(perr[1]); @@ -547,7 +580,8 @@ do_exec_no_pty(Session *s, const char *command) * seem to depend on it. */ close(inout[1]); - close(err[1]); + if (err[1] != -1) + close(err[1]); if (dup2(inout[0], 0) < 0) /* stdin */ perror("dup2 stdin"); if (dup2(inout[0], 1) < 0) /* stdout (same as stdin) */ @@ -595,10 +629,6 @@ do_exec_no_pty(Session *s, const char *command) close(perr[1]); if (compat20) { - if (s->is_subsystem) { - close(perr[0]); - perr[0] = -1; - } session_set_fds(s, pin[1], pout[0], perr[0], 0); } else { /* Enter the interactive session. */ @@ -615,10 +645,7 @@ 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], - s->is_subsystem ? -1 : err[1], 0); - if (s->is_subsystem) - close(err[1]); + session_set_fds(s, inout[1], inout[1], err[1], 0); } else { server_loop(pid, inout[1], inout[1], err[1]); /* server_loop has closed inout[1] and err[1]. */ -- cgit v1.2.3 From 7aa46ec393dd193d0a45ce7a5eb49413ead8789b Mon Sep 17 00:00:00 2001 From: Damien Miller Date: Sat, 26 Jun 2010 09:37:57 +1000 Subject: - djm@cvs.openbsd.org 2010/06/18 03:16:03 [session.c] Missing check for chroot_director == "none" (we already checked against NULL); bz#1564 from Jan.Pechanec AT Sun.COM --- ChangeLog | 4 ++++ session.c | 5 +++-- 2 files changed, 7 insertions(+), 2 deletions(-) (limited to 'session.c') diff --git a/ChangeLog b/ChangeLog index 57ddea6ff..b38b3ca08 100644 --- a/ChangeLog +++ b/ChangeLog @@ -17,6 +17,10 @@ [sftp.c] unbreak ls in working directories that contains globbing characters in their pathnames. bz#1655 reported by vgiffin AT apple.com + - djm@cvs.openbsd.org 2010/06/18 03:16:03 + [session.c] + Missing check for chroot_director == "none" (we already checked against + NULL); bz#1564 from Jan.Pechanec AT Sun.COM 20100622 - (djm) [loginrec.c] crank LINFO_NAMESIZE (username length) to 512 diff --git a/session.c b/session.c index b49d08d60..7e19299cb 100644 --- a/session.c +++ b/session.c @@ -1,4 +1,4 @@ -/* $OpenBSD: session.c,v 1.253 2010/04/23 22:42:05 djm Exp $ */ +/* $OpenBSD: session.c,v 1.254 2010/06/18 03:16:03 djm Exp $ */ /* * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland * All rights reserved @@ -1819,7 +1819,8 @@ do_child(Session *s, const char *command) #ifdef HAVE_LOGIN_CAP r = login_getcapbool(lc, "requirehome", 0); #endif - if (r || options.chroot_directory == NULL) + if (r || options.chroot_directory == NULL || + strcasecmp(options.chroot_directory, "none") == 0) fprintf(stderr, "Could not chdir to home " "directory %s: %s\n", pw->pw_dir, strerror(errno)); -- cgit v1.2.3 From 1b2b61e6f87810354c9a47448fbdb483f33413b0 Mon Sep 17 00:00:00 2001 From: Damien Miller Date: Sat, 26 Jun 2010 09:47:43 +1000 Subject: - djm@cvs.openbsd.org 2010/06/22 04:59:12 [session.c] include the user name on "subsystem request for ..." log messages; bz#1571; ok dtucker@ --- ChangeLog | 4 ++++ session.c | 5 +++-- 2 files changed, 7 insertions(+), 2 deletions(-) (limited to 'session.c') diff --git a/ChangeLog b/ChangeLog index e5475328b..60b571e94 100644 --- a/ChangeLog +++ b/ChangeLog @@ -46,6 +46,10 @@ [ssh-keyscan.c] replace verbose and overflow-prone Linebuf code with read_keyfile_line() based on patch from joachim AT joachimschipper.nl; bz#1565; ok dtucker@ + - djm@cvs.openbsd.org 2010/06/22 04:59:12 + [session.c] + include the user name on "subsystem request for ..." log messages; + bz#1571; ok dtucker@ 20100622 - (djm) [loginrec.c] crank LINFO_NAMESIZE (username length) to 512 diff --git a/session.c b/session.c index 7e19299cb..5de0ac897 100644 --- a/session.c +++ b/session.c @@ -1,4 +1,4 @@ -/* $OpenBSD: session.c,v 1.254 2010/06/18 03:16:03 djm Exp $ */ +/* $OpenBSD: session.c,v 1.255 2010/06/22 04:59:12 djm Exp $ */ /* * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland * All rights reserved @@ -2165,7 +2165,8 @@ session_subsystem_req(Session *s) u_int i; packet_check_eom(); - logit("subsystem request for %.100s", subsys); + logit("subsystem request for %.100s by user %s", subsys, + s->pw->pw_name); for (i = 0; i < options.num_subsystems; i++) { if (strcmp(subsys, options.subsystem_name[i]) == 0) { -- cgit v1.2.3 From 8853ca5fc46b7b71c74baeefb9b0899c7fcfdb9a Mon Sep 17 00:00:00 2001 From: Damien Miller Date: Sat, 26 Jun 2010 10:00:14 +1000 Subject: - 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@ --- ChangeLog | 6 +++++ channels.c | 17 +++++++++---- session.c | 81 ++++++++++++++++++++++---------------------------------------- 3 files changed, 46 insertions(+), 58 deletions(-) (limited to 'session.c') diff --git a/ChangeLog b/ChangeLog index cac82b47d..22bd509ec 100644 --- a/ChangeLog +++ b/ChangeLog @@ -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 diff --git a/channels.c b/channels.c index 2f2798ddd..fe08257df 100644 --- 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 * Copyright (c) 1995 Tatu Ylonen , 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); } } } diff --git a/session.c b/session.c index 5de0ac897..71e4fbe7c 100644 --- 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 , 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); } -- cgit v1.2.3