Bug 2529 - direct-streamlocal channel open doesn't match PROTOCOL documentation
Summary: direct-streamlocal channel open doesn't match PROTOCOL documentation
Status: CLOSED FIXED
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: ssh (show other bugs)
Version: -current
Hardware: All All
: P5 normal
Assignee: Assigned to nobody
URL:
Keywords:
Depends on:
Blocks: V_7_3
  Show dependency treegraph
 
Reported: 2016-01-22 16:49 AEDT by Ron Frederick
Modified: 2016-08-02 10:42 AEST (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ron Frederick 2016-01-22 16:49:54 AEDT
I attempted to implement support for direct-streamlocal@openssh.com channels in my own SSH implementation and found that the documentation in the OpenSSH PROTOCOL file doesn't match the implementation in OpenSSH. The PROTOCOL file describes the channel open message as:

        byte            SSH_MSG_CHANNEL_OPEN
        string          "direct-streamlocal@openssh.com"
        uint32          sender channel
        uint32          initial window size
        uint32          maximum packet size
        string          socket path

However, the implementation tacks some additional data to the end:

                packet_start(SSH2_MSG_CHANNEL_OPEN);
                packet_put_cstring(rtype);
                packet_put_int(c->self);
                packet_put_int(c->local_window_max);
                packet_put_int(c->local_maxpacket);
                if (strcmp(rtype, "direct-tcpip") == 0) {
                        /* target host, port */
                        packet_put_cstring(c->path);
                        packet_put_int(c->host_port);
                } else if (strcmp(rtype, "direct-streamlocal@openssh.com") == 0) {
                        /* target path */
                        packet_put_cstring(c->path);
                } else if (strcmp(rtype, "forwarded-streamlocal@openssh.com") == 0) {
                        /* listen path */
                        packet_put_cstring(c->path);
                } else {
                        /* listen address, port */
                        packet_put_cstring(c->path);
                        packet_put_int(local_port);
                }
                if (strcmp(rtype, "forwarded-streamlocal@openssh.com") == 0) {
                        /* reserved for future owner/mode info */
                        packet_put_cstring("");
                } else {
-->                     /* originator host and port */
-->                     packet_put_cstring(remote_ipaddr);
-->                     packet_put_int((u_int)remote_port);
                }
                packet_send();

It correctly special-cases forwarded-streamlocal, but for all other cases (including direct-streamlocal) it appends the remote IP add and port, even though these values are not applicable in the direct-streamlocal case.

This may be difficult to fix in a backward-compatible manner, since the code in serverloop.c actually seems to be expecting to get a host & port:

        target = packet_get_string(NULL);
        originator = packet_get_string(NULL);
        originator_port = packet_get_int();
        packet_check_eom();

So, perhaps the right thing here is to update the documentation in PROTOCOL to match the current implementation. It seems odd to send this information when it looks like it will always be an empty string and a port of zero, though, especially given that "port" information makes no sense for this type of connection.
Comment 1 Damien Miller 2016-02-11 13:26:41 AEDT
I think the suggested fix (updating PROTOCOL) is probably the best one given the circumstances. What do you think, Todd?
Comment 2 Damien Miller 2016-02-26 14:44:27 AEDT
Retarget to openssh-7.3
Comment 3 Damien Miller 2016-02-26 14:47:19 AEDT
Retarget to openssh-7.3
Comment 4 Damien Miller 2016-04-08 16:36:25 AEST
I've adjusted the PROTOCOL documentation to read:

        byte            SSH_MSG_CHANNEL_OPEN
        string          "direct-streamlocal@openssh.com"
        uint32          sender channel
        uint32          initial window size
        uint32          maximum packet size
        string          socket path
+       string          reserved
+       uint32          reserved

Maybe we could make something useful out of them in the future.

The updated PROTOCOL will be in the openssh-7.3 release.
Comment 5 Damien Miller 2016-08-02 10:42:37 AEST
Close all resolved bugs after 7.3p1 release