Possible issue with stdio forwarding

[Iain Morgan]

On Thu, Jan 28, 2010 at 22:33:43 -0600, Iain Morgan wrote:
> On Thu, Jan 28, 2010 at 20:01:08 -0600, Damien Miller wrote:
> > On Thu, 28 Jan 2010, Iain Morgan wrote:
> > 
> > > Greetings,
> > > 
> > > I've been doing a little testing with the stdio forwarding support added
> > > in recent snapshots and have encountered one possible issue. First, I
> > > should say that this feature generally seems to work. However, I haven't
> > > been able to get it to work when connecting to a server running
> > > SSH.COM's product.
> > > 
[snip]
> > 
> > I think this is the culprit. From session.c:
> > 
> >   1366  static void
> >   1367  port_open_helper(Channel *c, char *rtype)
> >   1368  {
> >   1369          int direct;
> >   1370          char buf[1024];
> >   1371          char *remote_ipaddr = get_peer_ipaddr(c->sock);
> >   1372          int remote_port = get_peer_port(c->sock);
> > ...
> >   1386                  packet_start(SSH2_MSG_CHANNEL_OPEN);
> >   1387                  packet_put_cstring(rtype);
> >   1388                  packet_put_int(c->self);
> >   1389                  packet_put_int(c->local_window_max);
> >   1390                  packet_put_int(c->local_maxpacket);
> > ...
> >   1400                  /* originator host and port */
> >   1401                  packet_put_cstring(remote_ipaddr);
> >   1402                  packet_put_int((u_int)remote_port);
> >   1403                  packet_send();
> > 
> > get_peer_ipaddr() will return "UNKNOWN" for stdio connections and 
> > get_peer_port() will return -1. OpenSSH just logs these values (they
> > are client supplied, so untrustworthy for any serious use), but Tectia
> > is probably sanity checking them. Could you try this diff?
> > 
> > Index: channels.c
> > ===================================================================
> > RCS file: /cvs/src/usr.bin/ssh/channels.c,v
> > retrieving revision 1.302
> > diff -u -p -r1.302 channels.c
> > --- channels.c	26 Jan 2010 01:28:35 -0000	1.302
> > +++ channels.c	29 Jan 2010 01:59:34 -0000
> > @@ -1371,6 +1371,13 @@ port_open_helper(Channel *c, char *rtype
> >  	char *remote_ipaddr = get_peer_ipaddr(c->sock);
> >  	int remote_port = get_peer_port(c->sock);
> >  
> > +	if (remote_port == -1) {
> > +		/* Fake something */
> > +		xfree(remote_ipaddr);
> > +		remote_ipaddr = xstrdup("127.0.0.1");
> > +		remote_port = 1;
> > +	}
> > +
> >  	direct = (strcmp(rtype, "direct-tcpip") == 0);
> >  
> >  	snprintf(buf, sizeof buf,
> > 
> > I'm not sure this is the right solution. E.g. it doesn't try to match
> > the AF of the target forward, but maybe it doesn't matter.
> > 
> > -d
> 
> I just tested it and it works. I'll do some further testing tomorrow.
> 
> Thanks
> 
> -- 
> Iain Morgan

Further testing against the 20100130 snapshot with the above patch
applied confirms that it fixes the issue. No adverse effects have been
detected.


As an aside, I should mention that I am seeing chan_read_failed messages
apparently from the master session when using multiplexing. These occur
when the slave session exits. Here's an example message:

	channel 11: chan_read_failed for istate 3

It's not a big issue, but it may disturb some users.

-- 
Iain Morgan