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, "[email protected]")) != 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,
> - &options.remote_forwards[i]);
> - if (options.remote_forwards[i].handle < 0) {
> - if (options.exit_on_forward_failure)
> - fatal("Could not request remote forwarding.");
> - else
> - logit("Warning: Could not request remote "
> - "forwarding.");
> - } else {
> + &options.remote_forwards[i])) >= 0) {
> client_register_global_confirm(
> ssh_confirm_remote_forward,
> &options.remote_forwards[i]);
> - }
> + forward_confirms_pending++;
> + } else if (options.exit_on_forward_failure)
> + fatal("Could not request remote forwarding.");
> + else
> + logit("Warning: Could not request remote forwarding.");
> }
>
> /* Initiate tunnel forwarding. */
> if (options.tun_open != SSH_TUNMODE_NO) {
> if ((*ifname = client_request_tun_fwd(ssh,
> options.tun_open, options.tun_local,
> - options.tun_remote)) == NULL) {
> - if (options.exit_on_forward_failure)
> - fatal("Could not request tunnel forwarding.");
> - else
> - error("Could not request tunnel forwarding.");
> - }
> + options.tun_remote, ssh_tun_confirm, NULL)) != NULL)
> + forward_confirms_pending++;
> + else if (options.exit_on_forward_failure)
> + fatal("Could not request tunnel forwarding.");
> + else
> + error("Could not request tunnel forwarding.");
> }
> }
>
