Trouble with port forwarding over proxy control socket

[ding]

Hi,

This is attempt 3 at sending this email, apologies to the admins.

I'm working on support for connecting to a control socket in the Go SSH 
package
(a continuation of https://github.com/golang/crypto/pull/205), but I'm 
having
trouble getting port forwarding to work correctly.

As far as I can tell the comment at
https://github.com/openssh/openssh-portable/blob/b652322cdc5e94f059b37a8fb87e44ccb1cdff33/channels.c#L3169
suggests that it *should* work, but looking through the code I can't see 
how.
The problem I'm hitting is that the response to the global "tcpip-forward"
request never gets forwarded downstream back over the control socket.

I can only assume that anyone currently using this is just not waiting 
for the
response? If the port is known ahead of time and you don't care about 
handling
a failure response then I guess you can just ignore it? It seems like it
definitely breaks the ability to use port 0 for remote forwarding though as
there's no way to find out which port was chosen on the server.

This seems to break with the OpenSSH client with `-O proxy` as well. Running
`ssh -R 0:127.0.0.1:<localport> -N <host>` allows connecting from a 
random port
on the server to the local port (and the log shows "Allocated port <...> for
remote forward to 127.0.0.1:<localport>"). Doing the same through a control
socket with `ssh -O proxy -R 0:127.0.0.1:<localport> -N <host>` doesn't 
output
the port to log, finding the port on the server with `lsof` and writing 
to it
results in the control master process logging "WARNING: Server requests
forwarding for unknown listen_port <...>".

Building "ssh" with the attached patch fixes the issue for me (at least 
for the
Go implementation), but including "clientloop.h" in "channels.c" seems 
wrong,
and it breaks a bunch of build targets. There's a comment above
`struct global_confirm` suggesting that it should be moved elsewhere, so I
assume that problem can be solved by moving it and the related functions 
to a
more accessible place. There could also be bigger technical problems 
with this
approach that I'm not aware of.

Before putting more effort into this I wanted to check if anyone had any
thoughts or better ideas on what the problem might be, or how to fix it?

Thanks!
-------------- next part --------------
diff --git a/channels.c b/channels.c
index 80014ff34..d389680fc 100644
--- a/channels.c
+++ b/channels.c
@@ -80,6 +80,7 @@
 #include "authfd.h"
 #include "pathnames.h"
 #include "match.h"
+#include "clientloop.h"
 
 /* XXX remove once we're satisfied there's no lurking bugs */
 /* #define DEBUG_CHANNEL_POLL 1 */
@@ -3197,6 +3198,50 @@ channel_output_poll(struct ssh *ssh)
  *    easily.
  */
 
+struct channel_proxy_confirm_ctx {
+	u_int cid;	/* channel id */
+};
+
+/* forward global request replies to downstream mux clients */
+static void
+channel_proxy_confirm_global_request(struct ssh *ssh, int type,
+	u_int32_t seq, void *ctxt)
+{
+	struct channel_proxy_confirm_ctx *ctx = ctxt;
+	Channel *downstream;
+	struct sshbuf *out = NULL;
+	const u_char *cp = NULL;
+	size_t len;
+	int r;
+
+	if ((downstream = channel_by_id(ssh, ctx->cid)) == NULL) {
+		/* no channel for reply */
+		/* c.f. mux_confirm_remote_forward() */
+		/* doesn't this result in a memory leak of the context? */
+		error_f("unknown channel");
+		return;
+	}
+	if ((out = sshbuf_new()) == NULL)
+		fatal_f("sshbuf_new");
+	/* forward the packet to muxclient */
+	cp = sshpkt_ptr(ssh, &len);
+	if (cp == NULL) {
+		error_f("no packet");
+		goto out;
+	}
+	if ((r = sshbuf_put_u8(out, 0)) != 0 ||	/* padlen */
+		(r = sshbuf_put_u8(out, type)) != 0 ||
+		(r = sshbuf_put(out, cp, len)) != 0 ||
+		(r = sshbuf_put_stringb(downstream->output, out)) != 0) {
+		error_fr(r, "forward muxclient");
+		goto out;
+	}
+
+out:
+	sshbuf_free(out);
+	free(ctx);
+}
+
 /*
  * receive packets from downstream mux clients:
  * channel callback fired on read from mux client, creates
@@ -3214,6 +3259,7 @@ channel_proxy_downstream(struct ssh *ssh, Channel *downstream)
 	size_t have;
 	int ret = -1, r;
 	u_int id, remote_id, listen_port;
+	struct channel_proxy_confirm_ctx *global_req_ctx = NULL;
 
 	/* sshbuf_dump(downstream->input, stderr); */
 	if ((r = sshbuf_get_string_direct(downstream->input, &cp, &have))
@@ -3295,7 +3341,9 @@ channel_proxy_downstream(struct ssh *ssh, Channel *downstream)
 			error_fr(r, "parse");
 			goto out;
 		}
-		if (strcmp(ctype, "tcpip-forward") != 0) {
+		/* is there a reason why cancel wasn't supported? */
+		if (strcmp(ctype, "tcpip-forward") != 0 &&
+			strcmp(ctype, "cancel-tcpip-forward") != 0) {
 			error_f("unsupported request %s", ctype);
 			goto out;
 		}
@@ -3313,6 +3361,10 @@ channel_proxy_downstream(struct ssh *ssh, Channel *downstream)
 		/* Record that connection to this host/port is permitted. */
 		permission_set_add(ssh, FORWARD_USER, FORWARD_LOCAL, "<mux>",
 		    -1, listen_host, NULL, (int)listen_port, downstream);
+		global_req_ctx = xcalloc(1, sizeof(*global_req_ctx));
+		global_req_ctx->cid = downstream->self;
+		client_register_global_confirm(
+			channel_proxy_confirm_global_request, global_req_ctx);
 		break;
 	case SSH2_MSG_CHANNEL_CLOSE:
 		if (have < 4)