Bug 1003 (rport0) - Support dynamic listen port allocation for remote forwards (-R0:...)
Summary: Support dynamic listen port allocation for remote forwards (-R0:...)
Status: CLOSED FIXED
Alias: rport0
Product: Portable OpenSSH
Classification: Unclassified
Component: sshd (show other bugs)
Version: -current
Hardware: All All
: P2 enhancement
Assignee: Damien Miller
URL:
Keywords: patch
Depends on:
Blocks: V_5_2
  Show dependency treegraph
 
Reported: 2005-03-22 18:59 AEDT by Lucio Crusca
Modified: 2009-02-23 13:35 AEDT (History)
0 users

See Also:


Attachments
my patch (1.95 KB, application/x-gzip)
2005-03-22 19:02 AEDT, Lucio Crusca
no flags Details
Unpacked patch (5.67 KB, patch)
2005-04-18 15:52 AEST, Damien Miller
no flags Details | Diff
Updated patch for 4.6p1 (6.40 KB, patch)
2007-03-19 09:00 AEDT, Ron Yorston
no flags Details | Diff
implement -R0:... forwarding for client and server (10.86 KB, patch)
2009-01-21 18:56 AEDT, Damien Miller
no flags Details | Diff
revised revised patch (10.83 KB, patch)
2009-01-22 21:22 AEDT, Damien Miller
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Lucio Crusca 2005-03-22 18:59:59 AEDT
See message  
http://www.mindrot.org/pipermail/openssh-unix-dev/2005-March/022829.html 
and its follow ups
Comment 1 Lucio Crusca 2005-03-22 19:02:09 AEDT
Created attachment 858 [details]
my patch
Comment 2 Damien Miller 2005-04-18 15:52:50 AEST
Created attachment 879 [details]
Unpacked patch

Please attach future patches in uncompressed format.
Comment 3 Damien Miller 2005-04-18 15:57:47 AEST
Comment on attachment 879 [details]
Unpacked patch

Some comments:

>diff -u openssh-4.0p1/channels.c patch/channels.c
>--- openssh-4.0p1/channels.c	2005-03-01 11:24:33.000000000 +0100
>+++ patch/channels.c	2005-03-15 16:44:12.000000000 +0100
>@@ -2289,6 +2289,18 @@
> 			close(sock);
> 			continue;
> 		}
>+		/* If the specified port was 0, get the actual port number */
>+		if (*listen_port == 0) {
>+			struct sockaddr_in realport;
>+			socklen_t realportsz = (socklen_t)sizeof(realport);
>+			if (getsockname(sock, (struct sockaddr *)&realport, &realportsz) < 0) {
>+				error("getsockname: %.100s", strerror(errno));
>+				close(sock);
>+				continue;
>+			}
>+			*listen_port = (u_short)ntohs(realport.sin_port);

This will only work for AF_INET sockets - it won't work for IPv6.

>@@ -2383,8 +2395,27 @@
> 		packet_put_int(listen_port);
> 		packet_send();
> 		packet_write_wait();
>-		/* Assume that server accepts the request */
>-		success = 1;
>+		
>+		if (listen_port != 0)
>+		  /* Assume that server accepts the request */
>+		  success = 1;
>+		else {
>+			type = packet_read();

This is incorrect: you shouldn't assume that the next packet is going to be a 
reply to your request. It may be something completely unexpected, like an 
IGNORE packet or the start of a rekey. 

You should use the dispatch.c functions to catch the reply and associate it 
(by seqno) back to your request.
Comment 4 Ron Yorston 2007-03-19 09:00:12 AEDT
Created attachment 1255 [details]
Updated patch for 4.6p1

I've updated the patch to apply to 4.6p1 and have attempted to address the first issue raised in Comment #3.  Fixing the other issue is currently beyond my shaky understanding of the code.

I note that ssh accepts an argument of the form '-R 0:host:port', which is fine for the purposes of this enhancement but should otherwise be considered a bug.  I blame the && on line 1258 of readconf.c.

Also, a2port returns 0 on error, which isn't sensible if 0 is a valid port number.  A proper fix would need to address this.
Comment 5 Damien Miller 2009-01-21 18:56:55 AEDT
Created attachment 1595 [details]
implement -R0:... forwarding for client and server

This patch implements -R0:... forwarding for client and server

The client support is pretty lame: it just prints the allocated port using logit(). Ideally it would make it possible to stuff it into the environment somehow, but I think the obvious solution of eval `ssh -R0:... yourhost` is too dangerous an idiom to support.
Comment 6 Damien Miller 2009-01-22 21:22:14 AEDT
Created attachment 1596 [details]
revised revised patch

(relative to CVS -current as of 5 minutes ago)

I have just adjusted a2port() to properly discriminate between explicitly zero and invalid port numbers and this required some tweaking of this diff.
Comment 7 Damien Miller 2009-02-12 14:18:48 AEDT
patch has been applied - this will be in OpenSSH-5.2
Comment 8 Damien Miller 2009-02-23 13:35:30 AEDT
Close bugs fixed/reviewed for openssh-5.2 release