upstream: restore blocking status on stdio fds before close

ssh(1) needs to set file descriptors to non-blocking mode to operate
but it was not restoring the original state on exit. This could cause
problems with fds shared with other programs via the shell, e.g.

> $ cat > test.sh << _EOF
> #!/bin/sh
> {
>         ssh -Fnone -oLogLevel=verbose ::1 hostname
>         cat /usr/share/dict/words
> } | sleep 10
> _EOF
> $ ./test.sh
> Authenticated to ::1 ([::1]:22).
> Transferred: sent 2352, received 2928 bytes, in 0.1 seconds
> Bytes per second: sent 44338.9, received 55197.4
> cat: stdout: Resource temporarily unavailable

This restores the blocking status for fds 0,1,2 (stdio) before ssh(1)
abandons/closes them.

This was reported as bz3280 and GHPR246; ok dtucker@

OpenBSD-Commit-ID: 8cc67346f05aa85a598bddf2383fcfcc3aae61ce
This commit is contained in:
djm@openbsd.org 2021-05-19 01:24:05 +00:00 committed by Damien Miller
parent c4902e1a65
commit 7be4ac8136
6 changed files with 75 additions and 65 deletions

View File

@ -1,4 +1,4 @@
/* $OpenBSD: channels.c,v 1.406 2021/04/03 06:18:40 djm Exp $ */ /* $OpenBSD: channels.c,v 1.407 2021/05/19 01:24:05 djm Exp $ */
/* /*
* Author: Tatu Ylonen <ylo@cs.hut.fi> * Author: Tatu Ylonen <ylo@cs.hut.fi>
* Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@ -333,7 +333,27 @@ channel_register_fds(struct ssh *ssh, Channel *c, int rfd, int wfd, int efd,
#endif #endif
/* enable nonblocking mode */ /* enable nonblocking mode */
if (nonblock) { c->restore_block = 0;
if (nonblock == CHANNEL_NONBLOCK_STDIO) {
/*
* Special handling for stdio file descriptors: do not set
* non-blocking mode if they are TTYs. Otherwise prepare to
* restore their blocking state on exit to avoid interfering
* with other programs that follow.
*/
if (rfd != -1 && !isatty(rfd) && fcntl(rfd, F_GETFL) == 0) {
c->restore_block |= CHANNEL_RESTORE_RFD;
set_nonblock(rfd);
}
if (wfd != -1 && !isatty(wfd) && fcntl(wfd, F_GETFL) == 0) {
c->restore_block |= CHANNEL_RESTORE_WFD;
set_nonblock(wfd);
}
if (efd != -1 && !isatty(efd) && fcntl(efd, F_GETFL) == 0) {
c->restore_block |= CHANNEL_RESTORE_EFD;
set_nonblock(efd);
}
} else if (nonblock) {
if (rfd != -1) if (rfd != -1)
set_nonblock(rfd); set_nonblock(rfd);
if (wfd != -1) if (wfd != -1)
@ -422,17 +442,23 @@ channel_find_maxfd(struct ssh_channels *sc)
} }
int int
channel_close_fd(struct ssh *ssh, int *fdp) channel_close_fd(struct ssh *ssh, Channel *c, int *fdp)
{ {
struct ssh_channels *sc = ssh->chanctxt; struct ssh_channels *sc = ssh->chanctxt;
int ret = 0, fd = *fdp; int ret, fd = *fdp;
if (fd == -1)
return 0;
if ((*fdp == c->rfd && (c->restore_block & CHANNEL_RESTORE_RFD) != 0) ||
(*fdp == c->wfd && (c->restore_block & CHANNEL_RESTORE_WFD) != 0) ||
(*fdp == c->efd && (c->restore_block & CHANNEL_RESTORE_EFD) != 0))
(void)fcntl(*fdp, F_SETFL, 0); /* restore blocking */
if (fd != -1) {
ret = close(fd); ret = close(fd);
*fdp = -1; *fdp = -1;
if (fd == sc->channel_max_fd) if (fd == sc->channel_max_fd)
channel_find_maxfd(sc); channel_find_maxfd(sc);
}
return ret; return ret;
} }
@ -442,13 +468,13 @@ channel_close_fds(struct ssh *ssh, Channel *c)
{ {
int sock = c->sock, rfd = c->rfd, wfd = c->wfd, efd = c->efd; int sock = c->sock, rfd = c->rfd, wfd = c->wfd, efd = c->efd;
channel_close_fd(ssh, &c->sock); channel_close_fd(ssh, c, &c->sock);
if (rfd != sock) if (rfd != sock)
channel_close_fd(ssh, &c->rfd); channel_close_fd(ssh, c, &c->rfd);
if (wfd != sock && wfd != rfd) if (wfd != sock && wfd != rfd)
channel_close_fd(ssh, &c->wfd); channel_close_fd(ssh, c, &c->wfd);
if (efd != sock && efd != rfd && efd != wfd) if (efd != sock && efd != rfd && efd != wfd)
channel_close_fd(ssh, &c->efd); channel_close_fd(ssh, c, &c->efd);
} }
static void static void
@ -702,7 +728,7 @@ channel_stop_listening(struct ssh *ssh)
case SSH_CHANNEL_X11_LISTENER: case SSH_CHANNEL_X11_LISTENER:
case SSH_CHANNEL_UNIX_LISTENER: case SSH_CHANNEL_UNIX_LISTENER:
case SSH_CHANNEL_RUNIX_LISTENER: case SSH_CHANNEL_RUNIX_LISTENER:
channel_close_fd(ssh, &c->sock); channel_close_fd(ssh, c, &c->sock);
channel_free(ssh, c); channel_free(ssh, c);
break; break;
} }
@ -1491,7 +1517,8 @@ channel_decode_socks5(Channel *c, struct sshbuf *input, struct sshbuf *output)
Channel * Channel *
channel_connect_stdio_fwd(struct ssh *ssh, channel_connect_stdio_fwd(struct ssh *ssh,
const char *host_to_connect, u_short port_to_connect, int in, int out) const char *host_to_connect, u_short port_to_connect,
int in, int out, int nonblock)
{ {
Channel *c; Channel *c;
@ -1499,7 +1526,7 @@ channel_connect_stdio_fwd(struct ssh *ssh,
c = channel_new(ssh, "stdio-forward", SSH_CHANNEL_OPENING, in, out, c = channel_new(ssh, "stdio-forward", SSH_CHANNEL_OPENING, in, out,
-1, CHAN_TCP_WINDOW_DEFAULT, CHAN_TCP_PACKET_DEFAULT, -1, CHAN_TCP_WINDOW_DEFAULT, CHAN_TCP_PACKET_DEFAULT,
0, "stdio-forward", /*nonblock*/0); 0, "stdio-forward", nonblock);
c->path = xstrdup(host_to_connect); c->path = xstrdup(host_to_connect);
c->host_port = port_to_connect; c->host_port = port_to_connect;
@ -1649,7 +1676,7 @@ channel_post_x11_listener(struct ssh *ssh, Channel *c,
if (c->single_connection) { if (c->single_connection) {
oerrno = errno; oerrno = errno;
debug2("single_connection: closing X11 listener."); debug2("single_connection: closing X11 listener.");
channel_close_fd(ssh, &c->sock); channel_close_fd(ssh, c, &c->sock);
chan_mark_dead(ssh, c); chan_mark_dead(ssh, c);
errno = oerrno; errno = oerrno;
} }
@ -2058,7 +2085,7 @@ channel_handle_efd_write(struct ssh *ssh, Channel *c,
return 1; return 1;
if (len <= 0) { if (len <= 0) {
debug2("channel %d: closing write-efd %d", c->self, c->efd); debug2("channel %d: closing write-efd %d", c->self, c->efd);
channel_close_fd(ssh, &c->efd); channel_close_fd(ssh, c, &c->efd);
} else { } else {
if ((r = sshbuf_consume(c->extended, len)) != 0) if ((r = sshbuf_consume(c->extended, len)) != 0)
fatal_fr(r, "channel %i: consume", c->self); fatal_fr(r, "channel %i: consume", c->self);
@ -2087,7 +2114,7 @@ channel_handle_efd_read(struct ssh *ssh, Channel *c,
return 1; return 1;
if (len <= 0) { if (len <= 0) {
debug2("channel %d: closing read-efd %d", c->self, c->efd); debug2("channel %d: closing read-efd %d", c->self, c->efd);
channel_close_fd(ssh, &c->efd); channel_close_fd(ssh, c, &c->efd);
} else if (c->extended_usage == CHAN_EXTENDED_IGNORE) } else if (c->extended_usage == CHAN_EXTENDED_IGNORE)
debug3("channel %d: discard efd", c->self); debug3("channel %d: discard efd", c->self);
else if ((r = sshbuf_put(c->extended, buf, len)) != 0) else if ((r = sshbuf_put(c->extended, buf, len)) != 0)

View File

@ -1,4 +1,4 @@
/* $OpenBSD: channels.h,v 1.137 2021/04/03 06:18:40 djm Exp $ */ /* $OpenBSD: channels.h,v 1.138 2021/05/19 01:24:05 djm Exp $ */
/* /*
* Author: Tatu Ylonen <ylo@cs.hut.fi> * Author: Tatu Ylonen <ylo@cs.hut.fi>
@ -63,6 +63,16 @@
#define CHANNEL_CANCEL_PORT_STATIC -1 #define CHANNEL_CANCEL_PORT_STATIC -1
/* nonblocking flags for channel_new */
#define CHANNEL_NONBLOCK_LEAVE 0 /* don't modify non-blocking state */
#define CHANNEL_NONBLOCK_SET 1 /* set non-blocking state */
#define CHANNEL_NONBLOCK_STDIO 2 /* set non-blocking and restore on close */
/* c->restore_block mask flags */
#define CHANNEL_RESTORE_RFD 0x01
#define CHANNEL_RESTORE_WFD 0x02
#define CHANNEL_RESTORE_EFD 0x04
/* TCP forwarding */ /* TCP forwarding */
#define FORWARD_DENY 0 #define FORWARD_DENY 0
#define FORWARD_REMOTE (1) #define FORWARD_REMOTE (1)
@ -139,6 +149,7 @@ struct Channel {
* to a matching pre-select handler. * to a matching pre-select handler.
* this way post-select handlers are not * this way post-select handlers are not
* accidentally called if a FD gets reused */ * accidentally called if a FD gets reused */
int restore_block; /* fd mask to restore blocking status */
struct sshbuf *input; /* data read from socket, to be sent over struct sshbuf *input; /* data read from socket, to be sent over
* encrypted connection */ * encrypted connection */
struct sshbuf *output; /* data received over encrypted connection for struct sshbuf *output; /* data received over encrypted connection for
@ -266,7 +277,7 @@ void channel_register_filter(struct ssh *, int, channel_infilter_fn *,
void channel_register_status_confirm(struct ssh *, int, void channel_register_status_confirm(struct ssh *, int,
channel_confirm_cb *, channel_confirm_abandon_cb *, void *); channel_confirm_cb *, channel_confirm_abandon_cb *, void *);
void channel_cancel_cleanup(struct ssh *, int); void channel_cancel_cleanup(struct ssh *, int);
int channel_close_fd(struct ssh *, int *); int channel_close_fd(struct ssh *, Channel *, int *);
void channel_send_window_changes(struct ssh *); void channel_send_window_changes(struct ssh *);
/* mux proxy support */ /* mux proxy support */
@ -313,7 +324,7 @@ Channel *channel_connect_to_port(struct ssh *, const char *, u_short,
char *, char *, int *, const char **); char *, char *, int *, const char **);
Channel *channel_connect_to_path(struct ssh *, const char *, char *, char *); Channel *channel_connect_to_path(struct ssh *, const char *, char *, char *);
Channel *channel_connect_stdio_fwd(struct ssh *, const char*, Channel *channel_connect_stdio_fwd(struct ssh *, const char*,
u_short, int, int); u_short, int, int, int);
Channel *channel_connect_by_listen_address(struct ssh *, const char *, Channel *channel_connect_by_listen_address(struct ssh *, const char *,
u_short, char *, char *); u_short, char *, char *);
Channel *channel_connect_by_listen_path(struct ssh *, const char *, Channel *channel_connect_by_listen_path(struct ssh *, const char *,

View File

@ -1,4 +1,4 @@
/* $OpenBSD: clientloop.c,v 1.362 2021/05/04 22:53:52 dtucker Exp $ */ /* $OpenBSD: clientloop.c,v 1.363 2021/05/19 01:24:05 djm Exp $ */
/* /*
* Author: Tatu Ylonen <ylo@cs.hut.fi> * Author: Tatu Ylonen <ylo@cs.hut.fi>
* Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@ -1405,14 +1405,6 @@ client_loop(struct ssh *ssh, int have_pty, int escape_char_arg,
if (have_pty) if (have_pty)
leave_raw_mode(options.request_tty == REQUEST_TTY_FORCE); leave_raw_mode(options.request_tty == REQUEST_TTY_FORCE);
/* restore blocking io */
if (!isatty(fileno(stdin)))
unset_nonblock(fileno(stdin));
if (!isatty(fileno(stdout)))
unset_nonblock(fileno(stdout));
if (!isatty(fileno(stderr)))
unset_nonblock(fileno(stderr));
/* /*
* If there was no shell or command requested, there will be no remote * If there was no shell or command requested, there will be no remote
* exit status to be returned. In that case, clear error code if the * exit status to be returned. In that case, clear error code if the

21
mux.c
View File

@ -1,4 +1,4 @@
/* $OpenBSD: mux.c,v 1.87 2021/04/03 06:18:40 djm Exp $ */ /* $OpenBSD: mux.c,v 1.88 2021/05/19 01:24:05 djm Exp $ */
/* /*
* Copyright (c) 2002-2008 Damien Miller <djm@openbsd.org> * Copyright (c) 2002-2008 Damien Miller <djm@openbsd.org>
* *
@ -452,14 +452,6 @@ mux_master_process_new_session(struct ssh *ssh, u_int rid,
if (cctx->want_tty && tcgetattr(new_fd[0], &cctx->tio) == -1) if (cctx->want_tty && tcgetattr(new_fd[0], &cctx->tio) == -1)
error_f("tcgetattr: %s", strerror(errno)); error_f("tcgetattr: %s", strerror(errno));
/* enable nonblocking unless tty */
if (!isatty(new_fd[0]))
set_nonblock(new_fd[0]);
if (!isatty(new_fd[1]))
set_nonblock(new_fd[1]);
if (!isatty(new_fd[2]))
set_nonblock(new_fd[2]);
window = CHAN_SES_WINDOW_DEFAULT; window = CHAN_SES_WINDOW_DEFAULT;
packetmax = CHAN_SES_PACKET_DEFAULT; packetmax = CHAN_SES_PACKET_DEFAULT;
if (cctx->want_tty) { if (cctx->want_tty) {
@ -469,7 +461,7 @@ mux_master_process_new_session(struct ssh *ssh, u_int rid,
nc = channel_new(ssh, "session", SSH_CHANNEL_OPENING, nc = channel_new(ssh, "session", SSH_CHANNEL_OPENING,
new_fd[0], new_fd[1], new_fd[2], window, packetmax, new_fd[0], new_fd[1], new_fd[2], window, packetmax,
CHAN_EXTENDED_WRITE, "client-session", /*nonblock*/0); CHAN_EXTENDED_WRITE, "client-session", CHANNEL_NONBLOCK_STDIO);
nc->ctl_chan = c->self; /* link session -> control channel */ nc->ctl_chan = c->self; /* link session -> control channel */
c->remote_id = nc->self; /* link control -> session channel */ c->remote_id = nc->self; /* link control -> session channel */
@ -1025,13 +1017,8 @@ mux_master_process_stdio_fwd(struct ssh *ssh, u_int rid,
} }
} }
/* enable nonblocking unless tty */ nc = channel_connect_stdio_fwd(ssh, chost, cport, new_fd[0], new_fd[1],
if (!isatty(new_fd[0])) CHANNEL_NONBLOCK_STDIO);
set_nonblock(new_fd[0]);
if (!isatty(new_fd[1]))
set_nonblock(new_fd[1]);
nc = channel_connect_stdio_fwd(ssh, chost, cport, new_fd[0], new_fd[1]);
free(chost); free(chost);
nc->ctl_chan = c->self; /* link session -> control channel */ nc->ctl_chan = c->self; /* link session -> control channel */

View File

@ -1,4 +1,4 @@
/* $OpenBSD: nchan.c,v 1.72 2021/01/27 09:26:54 djm Exp $ */ /* $OpenBSD: nchan.c,v 1.73 2021/05/19 01:24:05 djm Exp $ */
/* /*
* Copyright (c) 1999, 2000, 2001, 2002 Markus Friedl. All rights reserved. * Copyright (c) 1999, 2000, 2001, 2002 Markus Friedl. All rights reserved.
* *
@ -384,7 +384,7 @@ chan_shutdown_write(struct ssh *ssh, Channel *c)
c->istate, c->ostate, strerror(errno)); c->istate, c->ostate, strerror(errno));
} }
} else { } else {
if (channel_close_fd(ssh, &c->wfd) < 0) { if (channel_close_fd(ssh, c, &c->wfd) < 0) {
logit_f("channel %d: close() failed for " logit_f("channel %d: close() failed for "
"fd %d [i%d o%d]: %.100s", c->self, c->wfd, "fd %d [i%d o%d]: %.100s", c->self, c->wfd,
c->istate, c->ostate, strerror(errno)); c->istate, c->ostate, strerror(errno));
@ -412,7 +412,7 @@ chan_shutdown_read(struct ssh *ssh, Channel *c)
c->istate, c->ostate, strerror(errno)); c->istate, c->ostate, strerror(errno));
} }
} else { } else {
if (channel_close_fd(ssh, &c->rfd) < 0) { if (channel_close_fd(ssh, c, &c->rfd) < 0) {
logit_f("channel %d: close() failed for " logit_f("channel %d: close() failed for "
"fd %d [i%d o%d]: %.100s", c->self, c->rfd, "fd %d [i%d o%d]: %.100s", c->self, c->rfd,
c->istate, c->ostate, strerror(errno)); c->istate, c->ostate, strerror(errno));
@ -431,7 +431,7 @@ chan_shutdown_extended_read(struct ssh *ssh, Channel *c)
debug_f("channel %d: (i%d o%d sock %d wfd %d efd %d [%s])", debug_f("channel %d: (i%d o%d sock %d wfd %d efd %d [%s])",
c->self, c->istate, c->ostate, c->sock, c->rfd, c->efd, c->self, c->istate, c->ostate, c->sock, c->rfd, c->efd,
channel_format_extended_usage(c)); channel_format_extended_usage(c));
if (channel_close_fd(ssh, &c->efd) < 0) { if (channel_close_fd(ssh, c, &c->efd) < 0) {
logit_f("channel %d: close() failed for " logit_f("channel %d: close() failed for "
"extended fd %d [i%d o%d]: %.100s", c->self, c->efd, "extended fd %d [i%d o%d]: %.100s", c->self, c->efd,
c->istate, c->ostate, strerror(errno)); c->istate, c->ostate, strerror(errno));

17
ssh.c
View File

@ -1,4 +1,4 @@
/* $OpenBSD: ssh.c,v 1.556 2021/05/17 11:43:16 djm Exp $ */ /* $OpenBSD: ssh.c,v 1.557 2021/05/19 01:24:05 djm Exp $ */
/* /*
* Author: Tatu Ylonen <ylo@cs.hut.fi> * Author: Tatu Ylonen <ylo@cs.hut.fi>
* Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@ -1876,9 +1876,10 @@ ssh_init_stdio_forwarding(struct ssh *ssh)
if ((in = dup(STDIN_FILENO)) == -1 || if ((in = dup(STDIN_FILENO)) == -1 ||
(out = dup(STDOUT_FILENO)) == -1) (out = dup(STDOUT_FILENO)) == -1)
fatal("channel_connect_stdio_fwd: dup() in/out failed"); fatal_f("dup() in/out failed");
if ((c = channel_connect_stdio_fwd(ssh, options.stdio_forward_host, if ((c = channel_connect_stdio_fwd(ssh, options.stdio_forward_host,
options.stdio_forward_port, in, out)) == NULL) options.stdio_forward_port, in, out,
CHANNEL_NONBLOCK_STDIO)) == NULL)
fatal_f("channel_connect_stdio_fwd failed"); fatal_f("channel_connect_stdio_fwd failed");
channel_register_cleanup(ssh, c->self, client_cleanup_stdio_fwd, 0); channel_register_cleanup(ssh, c->self, client_cleanup_stdio_fwd, 0);
channel_register_open_confirm(ssh, c->self, ssh_stdio_confirm, NULL); channel_register_open_confirm(ssh, c->self, ssh_stdio_confirm, NULL);
@ -2074,14 +2075,6 @@ ssh_session2_open(struct ssh *ssh)
if (in == -1 || out == -1 || err == -1) if (in == -1 || out == -1 || err == -1)
fatal("dup() in/out/err failed"); fatal("dup() in/out/err failed");
/* enable nonblocking unless tty */
if (!isatty(in))
set_nonblock(in);
if (!isatty(out))
set_nonblock(out);
if (!isatty(err))
set_nonblock(err);
window = CHAN_SES_WINDOW_DEFAULT; window = CHAN_SES_WINDOW_DEFAULT;
packetmax = CHAN_SES_PACKET_DEFAULT; packetmax = CHAN_SES_PACKET_DEFAULT;
if (tty_flag) { if (tty_flag) {
@ -2091,7 +2084,7 @@ ssh_session2_open(struct ssh *ssh)
c = channel_new(ssh, c = channel_new(ssh,
"session", SSH_CHANNEL_OPENING, in, out, err, "session", SSH_CHANNEL_OPENING, in, out, err,
window, packetmax, CHAN_EXTENDED_WRITE, window, packetmax, CHAN_EXTENDED_WRITE,
"client-session", /*nonblock*/0); "client-session", CHANNEL_NONBLOCK_STDIO);
debug3_f("channel_new: %d", c->self); debug3_f("channel_new: %d", c->self);