Re: ssh: Register tun channel cleanup callback when ExitOnForwardFailure is set
On Tue, Feb 18, 2020 at 09:00:13AM +1100, Damien Miller wrote:
> On Mon, 17 Feb 2020, Remi Pommarel wrote:
>
> > When remote side fails to create tun (e.g. tun device is already opened)
> > it notifies the client with an SSH2_MSG_CHANNEL_OPEN_FAILURE message and
> > channel is marked dead on client side. But because tun forward channel
> > is not an interactive channel it has no cleanup callback and is kept in
> > a Zombie state until ssh is manually terminated.
> >
> > This makes "ssh -Nw" to not fail and exit in such situation even if
> > ExitOnForwardFailure is set.
> >
> > This patch registers a cleanup callback to tun forward channel if
> > ExitOnForwardFailure is set so that "ssh -Nw" exit directly when
> > remote fails to established the tunnel correctly on its side.
>
> Please try this patch. It handles tunnel forwarding failures via
> the same logic as remote forwards.
Tried it and seems to work.
Thanks.
>
> diff --git a/clientloop.c b/clientloop.c
> index f7ce3a2..f35ccfb 100644
> --- a/clientloop.c
> +++ b/clientloop.c
> @@ -1641,7 +1641,7 @@ client_request_agent(struct ssh *ssh, const char
> *request_type, int rchan)
>
> char *
> client_request_tun_fwd(struct ssh *ssh, int tun_mode,
> -int local_tun, int remote_tun)
> +int local_tun, int remote_tun, channel_open_fn *cb, void *cbctx)
> {
> Channel *c;
> int r, fd;
> @@ -1663,6 +1663,9 @@ client_request_tun_fwd(struct ssh *ssh, int tun_mode,
> CHAN_TCP_WINDOW_DEFAULT, CHAN_TCP_PACKET_DEFAULT, 0, "tun", 1);
> c->datagram = 1;
>
> + if (cb != NULL)
> + channel_register_open_confirm(ssh, c->self, cb, cbctx);
> +
> if ((r = sshpkt_start(ssh, SSH2_MSG_CHANNEL_OPEN)) != 0 ||
> (r = sshpkt_put_cstring(ssh, "t...@openssh.com")) != 0 ||
> (r = sshpkt_put_u32(ssh, c->self)) != 0 ||
> diff --git a/clientloop.h b/clientloop.h
> index bf79c87..4604e7c 100644
> --- a/clientloop.h
> +++ b/clientloop.h
> @@ -46,7 +46,8 @@ int client_x11_get_proto(struct ssh *, const char *, const
> char *,
> void client_global_request_reply_fwd(int, u_int32_t, void *);
> void client_session2_setup(struct ssh *, int, int, int,
> const char *, struct termios *, int, struct sshbuf *, char **);
> -char *client_request_tun_fwd(struct ssh *, int, int, int);
> +char *client_request_tun_fwd(struct ssh *, int, int, int,
> +channel_open_fn *, void *);
> void client_stop_mux(void);
>
> /* Escape filter for protocol 2 sessions */
> diff --git a/ssh.c b/ssh.c
> index 314b3c5..37a693c 100644
> --- a/ssh.c
> +++ b/ssh.c
> @@ -175,7 +175,7 @@ struct sshbuf *command;
> int subsystem_flag = 0;
>
> /* # of replies received for global requests */
> -static int remote_forward_confirms_received = 0;
> +static int forward_confirms_pending = -1;
>
> /* mux.c */
> extern int muxserver_sock;
> @@ -1640,6 +1640,16 @@ fork_postauth(void)
> fatal("daemon() failed: %.200s", strerror(errno));
> }
>
> +static void
> +forwarding_success(void)
> +{
> + if (forward_confirms_pending > 0 && --forward_confirms_pending == 0) {
> + debug("All forwarding requests processed");
> + if (fork_after_authentication_flag)
> + fork_postauth();
> + }
> +}
> +
> /* Callback for remote forward global requests */
> static void
> ssh_confirm_remote_forward(struct ssh *ssh, int type, u_int32_t seq, void
> *ctxt)
> @@ -1699,11 +1709,7 @@ ssh_confirm_remote_forward(struct ssh *ssh, int type,
> u_int32_t seq, void *ctxt)
> "for listen port %d", rfwd->listen_port);
> }
> }
> - if (++remote_forward_confirms_received == options.num_remote_forwards) {
> - debug("All remote forwarding requests processed");
> - if (fork_after_authentication_flag)
> - fork_postauth();
> - }
> + forwarding_success();
> }
>
> static void
> @@ -1720,6 +1726,19 @@ ssh_stdio_confirm(struct ssh *ssh, int id, int
> success, void *arg)
> fatal("stdio forwarding failed");
> }
>
> +static void
> +ssh_tun_confirm(struct ssh *ssh, int id, int success, void *arg)
> +{
> + if (!success) {
> + error("Tunnel forwarding failed");
> + if (options.exit_on_forward_failure)
> + cleanup_exit(255);
> + }
> +
> + debug("%s: tunnel forward established, id=%d", __func__, id);
> + forwarding_success();
> +}
> +
> static void
> ssh_init_stdio_forwarding(struct ssh *ssh)
> {
> @@ -1783,32 +1802,29 @@ ssh_init_forwarding(struct ssh *ssh, char **ifname)
> options.remote_forwards[i].connect_path :
> options.remote_forwards[i].connect_host,
> options.remote_forwards[i].connect_port);
> - options.remote_forwards[i].handle =
> + if ((options.remote_forwards[i].handle =
> channel_request_remote_forwarding(ssh,
Re: ssh: Register tun channel cleanup callback when ExitOnForwardFailure is set
On Mon, 17 Feb 2020, Remi Pommarel wrote:
> When remote side fails to create tun (e.g. tun device is already opened)
> it notifies the client with an SSH2_MSG_CHANNEL_OPEN_FAILURE message and
> channel is marked dead on client side. But because tun forward channel
> is not an interactive channel it has no cleanup callback and is kept in
> a Zombie state until ssh is manually terminated.
>
> This makes "ssh -Nw" to not fail and exit in such situation even if
> ExitOnForwardFailure is set.
>
> This patch registers a cleanup callback to tun forward channel if
> ExitOnForwardFailure is set so that "ssh -Nw" exit directly when
> remote fails to established the tunnel correctly on its side.
Please try this patch. It handles tunnel forwarding failures via
the same logic as remote forwards.
diff --git a/clientloop.c b/clientloop.c
index f7ce3a2..f35ccfb 100644
--- a/clientloop.c
+++ b/clientloop.c
@@ -1641,7 +1641,7 @@ client_request_agent(struct ssh *ssh, const char
*request_type, int rchan)
char *
client_request_tun_fwd(struct ssh *ssh, int tun_mode,
-int local_tun, int remote_tun)
+int local_tun, int remote_tun, channel_open_fn *cb, void *cbctx)
{
Channel *c;
int r, fd;
@@ -1663,6 +1663,9 @@ client_request_tun_fwd(struct ssh *ssh, int tun_mode,
CHAN_TCP_WINDOW_DEFAULT, CHAN_TCP_PACKET_DEFAULT, 0, "tun", 1);
c->datagram = 1;
+ if (cb != NULL)
+ channel_register_open_confirm(ssh, c->self, cb, cbctx);
+
if ((r = sshpkt_start(ssh, SSH2_MSG_CHANNEL_OPEN)) != 0 ||
(r = sshpkt_put_cstring(ssh, "t...@openssh.com")) != 0 ||
(r = sshpkt_put_u32(ssh, c->self)) != 0 ||
diff --git a/clientloop.h b/clientloop.h
index bf79c87..4604e7c 100644
--- a/clientloop.h
+++ b/clientloop.h
@@ -46,7 +46,8 @@ intclient_x11_get_proto(struct ssh *, const char *, const
char *,
voidclient_global_request_reply_fwd(int, u_int32_t, void *);
voidclient_session2_setup(struct ssh *, int, int, int,
const char *, struct termios *, int, struct sshbuf *, char **);
-char*client_request_tun_fwd(struct ssh *, int, int, int);
+char*client_request_tun_fwd(struct ssh *, int, int, int,
+channel_open_fn *, void *);
voidclient_stop_mux(void);
/* Escape filter for protocol 2 sessions */
diff --git a/ssh.c b/ssh.c
index 314b3c5..37a693c 100644
--- a/ssh.c
+++ b/ssh.c
@@ -175,7 +175,7 @@ struct sshbuf *command;
int subsystem_flag = 0;
/* # of replies received for global requests */
-static int remote_forward_confirms_received = 0;
+static int forward_confirms_pending = -1;
/* mux.c */
extern int muxserver_sock;
@@ -1640,6 +1640,16 @@ fork_postauth(void)
fatal("daemon() failed: %.200s", strerror(errno));
}
+static void
+forwarding_success(void)
+{
+ if (forward_confirms_pending > 0 && --forward_confirms_pending == 0) {
+ debug("All forwarding requests processed");
+ if (fork_after_authentication_flag)
+ fork_postauth();
+ }
+}
+
/* Callback for remote forward global requests */
static void
ssh_confirm_remote_forward(struct ssh *ssh, int type, u_int32_t seq, void
*ctxt)
@@ -1699,11 +1709,7 @@ ssh_confirm_remote_forward(struct ssh *ssh, int type,
u_int32_t seq, void *ctxt)
"for listen port %d", rfwd->listen_port);
}
}
- if (++remote_forward_confirms_received == options.num_remote_forwards) {
- debug("All remote forwarding requests processed");
- if (fork_after_authentication_flag)
- fork_postauth();
- }
+ forwarding_success();
}
static void
@@ -1720,6 +1726,19 @@ ssh_stdio_confirm(struct ssh *ssh, int id, int success,
void *arg)
fatal("stdio forwarding failed");
}
+static void
+ssh_tun_confirm(struct ssh *ssh, int id, int success, void *arg)
+{
+ if (!success) {
+ error("Tunnel forwarding failed");
+ if (options.exit_on_forward_failure)
+ cleanup_exit(255);
+ }
+
+ debug("%s: tunnel forward established, id=%d", __func__, id);
+ forwarding_success();
+}
+
static void
ssh_init_stdio_forwarding(struct ssh *ssh)
{
@@ -1783,32 +1802,29 @@ ssh_init_forwarding(struct ssh *ssh, char **ifname)
options.remote_forwards[i].connect_path :
options.remote_forwards[i].connect_host,
options.remote_forwards[i].connect_port);
- options.remote_forwards[i].handle =
+ if ((options.remote_forwards[i].handle =
channel_request_remote_forwarding(ssh,
- _forwards[i]);
- if (options.remote_forwards[i].handle < 0) {
- if (options.exit_on_forward_failure)
- fatal("Could not request remote forwarding.");
- else
-
ssh: Register tun channel cleanup callback when ExitOnForwardFailure is set
When remote side fails to create tun (e.g. tun device is already opened)
it notifies the client with an SSH2_MSG_CHANNEL_OPEN_FAILURE message and
channel is marked dead on client side. But because tun forward channel
is not an interactive channel it has no cleanup callback and is kept in
a Zombie state until ssh is manually terminated.
This makes "ssh -Nw" to not fail and exit in such situation even if
ExitOnForwardFailure is set.
This patch registers a cleanup callback to tun forward channel if
ExitOnForwardFailure is set so that "ssh -Nw" exit directly when
remote fails to established the tunnel correctly on its side.
Index: usr.bin/ssh/clientloop.c
===
RCS file: /cvs/src/usr.bin/ssh/clientloop.c,v
retrieving revision 1.340
diff -u -p -r1.340 clientloop.c
--- usr.bin/ssh/clientloop.c2 Feb 2020 09:45:34 - 1.340
+++ usr.bin/ssh/clientloop.c17 Feb 2020 15:42:59 -
@@ -1673,6 +1673,11 @@ client_request_tun_fwd(struct ssh *ssh,
(r = sshpkt_send(ssh)) != 0)
sshpkt_fatal(ssh, r, "%s: send reply", __func__);
+ if (options.exit_on_forward_failure) {
+ channel_register_cleanup(ssh, c->self,
+ client_channel_closed, 0);
+ }
+
return ifname;
}
