From f17400e7389bf1fff7939690e4bfe1caad1e7486 Mon Sep 17 00:00:00 2001 From: Matt Johnston <matt@ucc.asn.au> Date: Mon, 11 Oct 2021 15:42:14 +0800 Subject: [PATCH] Replace ChanType.sepfds with Channel.bidir_fd This handles the case where a svrchansess has separate FDs for nopty, but a single FD for pty mode. The use of sepfds was also previously incorrect for X11 and agent forwarding --- channel.h | 4 +++- cli-agentfwd.c | 2 +- cli-chansession.c | 3 +-- cli-tcpfwd.c | 6 ++---- common-channel.c | 6 ++++-- svr-agentfwd.c | 1 - svr-chansession.c | 3 ++- svr-tcpfwd.c | 2 -- svr-x11fwd.c | 1 - 9 files changed, 13 insertions(+), 15 deletions(-) diff --git a/channel.h b/channel.h index 4c6fe54..6aae9ea 100644 --- a/channel.h +++ b/channel.h @@ -60,6 +60,9 @@ struct Channel { int readfd; /* read from insecure side, written to wire */ int errfd; /* used like writefd or readfd, depending if it's client or server. Doesn't exactly belong here, but is cleaner here */ + int bidir_fd; /* a boolean indicating that writefd/readfd are the same + file descriptor (bidirectional), such as a network socket or PTY. + That is handled differently when closing FDs */ circbuffer *writebuf; /* data from the wire, for local consumption. Can be initially NULL */ circbuffer *extrabuf; /* extended-data for the program - used like writebuf @@ -87,7 +90,6 @@ struct Channel { struct ChanType { - int sepfds; /* Whether this channel has separate pipes for in/out or not */ const char *name; /* Sets up the channel */ int (*inithandler)(struct Channel*); diff --git a/cli-agentfwd.c b/cli-agentfwd.c index 00454dc..6fb5c4b 100644 --- a/cli-agentfwd.c +++ b/cli-agentfwd.c @@ -47,7 +47,6 @@ static int new_agent_chan(struct Channel * channel); const struct ChanType cli_chan_agent = { - 0, /* sepfds */ "auth-agent@openssh.com", new_agent_chan, NULL, @@ -94,6 +93,7 @@ static int new_agent_chan(struct Channel * channel) { channel->readfd = fd; channel->writefd = fd; + channel->bidir_fd = 1; return 0; } diff --git a/cli-chansession.c b/cli-chansession.c index cfd3401..14a2779 100644 --- a/cli-chansession.c +++ b/cli-chansession.c @@ -46,7 +46,6 @@ static int cli_init_netcat(struct Channel *channel); static void cli_tty_setup(void); const struct ChanType clichansess = { - 0, /* sepfds */ "session", /* name */ cli_initchansess, /* inithandler */ NULL, /* checkclosehandler */ @@ -344,6 +343,7 @@ static int cli_init_stdpipe_sess(struct Channel *channel) { setnonblocking(STDERR_FILENO); channel->extrabuf = cbuf_new(opts.recv_window); + channel->bidir_fd = 0; return 0; } @@ -383,7 +383,6 @@ static int cli_initchansess(struct Channel *channel) { #if DROPBEAR_CLI_NETCAT static const struct ChanType cli_chan_netcat = { - 0, /* sepfds */ "direct-tcpip", cli_init_netcat, /* inithandler */ NULL, diff --git a/cli-tcpfwd.c b/cli-tcpfwd.c index 4ab7748..e61a0c3 100644 --- a/cli-tcpfwd.c +++ b/cli-tcpfwd.c @@ -35,7 +35,6 @@ static int newtcpforwarded(struct Channel * channel); const struct ChanType cli_chan_tcpremote = { - 1, /* sepfds */ "forwarded-tcpip", newtcpforwarded, NULL, @@ -51,7 +50,6 @@ static int cli_localtcp(const char* listenaddr, const char* remoteaddr, unsigned int remoteport); static const struct ChanType cli_chan_tcplocal = { - 1, /* sepfds */ "direct-tcpip", tcp_prio_inithandler, NULL, @@ -275,10 +273,10 @@ static int newtcpforwarded(struct Channel * channel) { } channel->prio = DROPBEAR_CHANNEL_PRIO_UNKNOWABLE; - + snprintf(portstring, sizeof(portstring), "%u", fwd->connectport); channel->conn_pending = connect_remote(fwd->connectaddr, portstring, channel_connect_done, channel, NULL, NULL); - + err = SSH_OPEN_IN_PROGRESS; out: diff --git a/common-channel.c b/common-channel.c index 03b3312..5c8d9bc 100644 --- a/common-channel.c +++ b/common-channel.c @@ -339,6 +339,7 @@ void channel_connect_done(int result, int sock, void* user_data, const char* err if (result == DROPBEAR_SUCCESS) { channel->readfd = channel->writefd = sock; + channel->bidir_fd = 1; channel->conn_pending = NULL; send_msg_channel_open_confirmation(channel, channel->recvwindow, channel->recvmaxpacket); @@ -1039,7 +1040,7 @@ static void close_chan_fd(struct Channel *channel, int fd, int how) { int closein = 0, closeout = 0; - if (channel->type->sepfds) { + if (channel->bidir_fd) { TRACE(("SHUTDOWN(%d, %d)", fd, how)) shutdown(fd, how); if (how == 0) { @@ -1069,7 +1070,7 @@ static void close_chan_fd(struct Channel *channel, int fd, int how) { /* if we called shutdown on it and all references are gone, then we * need to close() it to stop it lingering */ - if (channel->type->sepfds && channel->readfd == FD_CLOSED + if (channel->bidir_fd && channel->readfd == FD_CLOSED && channel->writefd == FD_CLOSED && channel->errfd == FD_CLOSED) { TRACE(("CLOSE (finally) of %d", fd)) m_close(fd); @@ -1102,6 +1103,7 @@ int send_msg_channel_open_init(int fd, const struct ChanType *type) { chan->writefd = chan->readfd = fd; ses.maxfd = MAX(ses.maxfd, fd); + chan->bidir_fd = 1; chan->await_open = 1; diff --git a/svr-agentfwd.c b/svr-agentfwd.c index ac9475f..a8941ea 100644 --- a/svr-agentfwd.c +++ b/svr-agentfwd.c @@ -186,7 +186,6 @@ void svr_agentcleanup(struct ChanSess * chansess) { } static const struct ChanType chan_svr_agent = { - 0, /* sepfds */ "auth-agent@openssh.com", NULL, NULL, diff --git a/svr-chansession.c b/svr-chansession.c index 9c26204..e19c8df 100644 --- a/svr-chansession.c +++ b/svr-chansession.c @@ -64,7 +64,6 @@ static void send_msg_chansess_exitsignal(const struct Channel * channel, static void get_termmodes(const struct ChanSess *chansess); const struct ChanType svrchansess = { - 0, /* sepfds */ "session", /* name */ newchansess, /* inithandler */ sesscheckclose, /* checkclosehandler */ @@ -769,6 +768,7 @@ static int noptycommand(struct Channel *channel, struct ChanSess *chansess) { ses.maxfd = MAX(ses.maxfd, channel->writefd); ses.maxfd = MAX(ses.maxfd, channel->readfd); ses.maxfd = MAX(ses.maxfd, channel->errfd); + channel->bidir_fd = 0; addchildpid(chansess, chansess->pid); @@ -895,6 +895,7 @@ static int ptycommand(struct Channel *channel, struct ChanSess *chansess) { channel->readfd = chansess->master; /* don't need to set stderr here */ ses.maxfd = MAX(ses.maxfd, chansess->master); + channel->bidir_fd = 1; setnonblocking(chansess->master); diff --git a/svr-tcpfwd.c b/svr-tcpfwd.c index f3bab25..9c1edd4 100644 --- a/svr-tcpfwd.c +++ b/svr-tcpfwd.c @@ -59,7 +59,6 @@ static int newtcpdirect(struct Channel * channel); #if DROPBEAR_SVR_REMOTETCPFWD static const struct ChanType svr_chan_tcpremote = { - 1, /* sepfds */ "forwarded-tcpip", tcp_prio_inithandler, NULL, @@ -241,7 +240,6 @@ out: #if DROPBEAR_SVR_LOCALTCPFWD const struct ChanType svr_chan_tcpdirect = { - 1, /* sepfds */ "direct-tcpip", newtcpdirect, /* init */ NULL, /* checkclose */ diff --git a/svr-x11fwd.c b/svr-x11fwd.c index 460db4c..353cb12 100644 --- a/svr-x11fwd.c +++ b/svr-x11fwd.c @@ -211,7 +211,6 @@ static int x11_inithandler(struct Channel *channel) { } static const struct ChanType chan_x11 = { - 0, /* sepfds */ "x11", x11_inithandler, /* inithandler */ NULL, /* checkclose */