Issue with OpenSSH remote forwarding of dynamic ports

[Ron Frederick]

On Aug 15, 2013, at 5:14 PM, Ángel González <keisial at gmail.com> wrote:
> 
> On 08/08/13 06:22, Ron Frederick wrote:
>>    When a connection comes to a port for which remote forwarding has
>>    been requested, a channel is opened to forward the port to the other
>>    side.
>> 
>>       byte      SSH_MSG_CHANNEL_OPEN
>>       string    "forwarded-tcpip"
>>       uint32    sender channel
>>       uint32    initial window size
>>       uint32    maximum packet size
>>       string    address that was connected
>>       uint32    port that was connected
>>       string    originator IP address
>>       uint32    originator port
>> 
>> I was expecting "port that was connected" in this message to be the dynamically allocated port so that it would always be a unique value, but this is not the case (at least with OpenSSH's sshd). Instead, it always seems to be the "port number to bind" value passed in the original SSH_MSG_GLOBAL_REQUEST "tcpip-forward" message (which is 0 since we're asking for a dynamic port).
>> 
>> Unfortunately, I would imagine changing this behavior on the server side might break existing clients out there which are expecting to get this 0 value back in channel open requests when they set up a dynamic listener, and I don't really see a good way to resolve this. Does anyone have any suggestions?
> 
> It could pass 0 for the first forward and the real port in the next ones (which are already open), but IMHO the right thing would be to always provide the real port.

I did an experiment today and was pleasantly surprised to find that the OpenSSH client does work correctly when the real port number is returned by the server. The "address that was connected" can also be a real address. So, for instance, a request could be made with a bind address of "localhost" and the address reported back could be "127.0.0.1" for IPv4 or "::1" for IPv6. Here's some debug output of it working:

debug1: remote forward success for: listen 0, connect localhost:80
Allocated port 12345 for remote forward to localhost:80
debug1: Updating allowed port 12345 for forwarding to host localhost port 80
...
debug1: client_input_channel_open: ctype forwarded-tcpip rchan 1 win 2097152 max 32768
debug1: client_request_forwarded_tcpip: listen 127.0.0.1 port 12345, originator 127.0.0.1 port 56789
debug1: connect_next: host localhost ([::1]:80) in progress, fd=9
debug1: channel 1: new [1.2.3.4]
debug1: confirm forwarded-tcpip
debug1: channel 1: connected to localhost port 80

I also tried having the server respond with a port number which didn't match the random port returned in the channel open response, and the client properly rejected that. In the debug output, I saw:

WARNING: Server requests forwarding for unknown listen_port 12346
debug1: failure forwarded-tcpip

So, it looks like it wouldn't cause a problem for OpenSSH clients if the OpenSSH server always returned the correct port number in the "tcpip-forward" response when requests were made to bind to port 0. The change looks fairly straightforward to do this. Here's a proposed patch against version 6.2p2:

--- channels.c.orig	2012-12-02 14:50:55.000000000 -0800
+++ channels.c	2013-08-17 09:18:41.000000000 -0700
@@ -1384,6 +1384,8 @@
 {
 	int direct;
 	char buf[1024];
+	char *local_ipaddr = get_local_ipaddr(c->sock);
+	int local_port = get_sock_port(c->sock, 1);
 	char *remote_ipaddr = get_peer_ipaddr(c->sock);
 	int remote_port = get_peer_port(c->sock);
 
@@ -1398,9 +1400,9 @@
 
 	snprintf(buf, sizeof buf,
 	    "%s: listening port %d for %.100s port %d, "
-	    "connect from %.200s port %d",
+	    "connect from %.200s port %d to %.100s port %d",
 	    rtype, c->listening_port, c->path, c->host_port,
-	    remote_ipaddr, remote_port);
+	    remote_ipaddr, remote_port, local_ipaddr, local_port);
 
 	xfree(c->remote_name);
 	c->remote_name = xstrdup(buf);
@@ -1416,9 +1418,9 @@
 			packet_put_cstring(c->path);
 			packet_put_int(c->host_port);
 		} else {
-			/* listen address, port */
-			packet_put_cstring(c->path);
-			packet_put_int(c->listening_port);
+			/* connected address, port */
+			packet_put_cstring(local_ipaddr);
+			packet_put_int(local_port);
 		}
 		/* originator host and port */
 		packet_put_cstring(remote_ipaddr);
@@ -1435,6 +1437,7 @@
 		packet_send();
 	}
 	xfree(remote_ipaddr);
+	xfree(local_ipaddr);
 }
 
 static void

One minor ugliness here is that I had to use "get_sock_port()" instead of get_local_port(), as get_local_port() doesn't take a socket as an argument the way get_local_ipaddr() does and there's no equivalent function under another name. Cleaning this up would impact other files, though, and I wanted to keep the diff as clean as possible for now.
-- 
Ron Frederick
ronf at timeheart.net