openssh bugs - Dec 2012 - [Bug 2055] New: channel_setup_fwd_listener return value used incorrectly

https://bugzilla.mindrot.org/show_bug.cgi?id=2055

            Bug ID: 2055
           Summary: channel_setup_fwd_listener return value used
                    incorrectly
    Classification: Unclassified
           Product: Portable OpenSSH
           Version: 6.1p1
          Hardware: Other
                OS: Linux
            Status: NEW
          Severity: enhancement
          Priority: P5
         Component: ssh
          Assignee: unassigned-bugs at mindrot.org
          Reporter: mathieu.lacage at gmail.com

It looks like the code for channel_setup_fwd_listener is written to
return 1 on success, and 0 on failure. The user of this function from
mux.c (indirectly thru channel_setup_local_fwd_listener) appears to be
assuming a negative value on failure and 0 on success (the standard
unix semantics)

i.e., none of the users of this function report port allocation errors
correctly which means that it is not possible to reliably detect port
allocation failure when setting up port allocations thru ssh -S /tmp/x
O forward. i.e. it sucks :/

How to reproduce:

ssh -L1040:localhost:1000  localhost &
ssh -M -S /tmp/x -NT localhost &
ssh -S /tmp/x -L1040:localhost:1000 -O forward localhost

What should happen:
from reading the source code in mux.c, I believe that the last command
should print something along the lines of: "Master refused forwarding
request: Port forwarding failed"

What happens:
no output from last command that appears to succeed.

I suspect that somewhere along the lifetime of this code, the callee
was changed to return its success/failure differently but the callers
were not updated.

The fix is fairly obvious (although I did not bother to recompile/test
it): 

in mux.c, change 
                if (channel_setup_local_fwd_listener(fwd.listen_host,
                    fwd.listen_port, fwd.connect_host,
fwd.connect_port,
                    options.gateway_ports) < 0) {

to:
                if (channel_setup_local_fwd_listener(fwd.listen_host,
                    fwd.listen_port, fwd.connect_host,
fwd.connect_port,
                    options.gateway_ports) == 0) {

Note: from looking at the code, I believe that -R is immune to this
problem. -D should suffer from the same problem although I did not test
it.

Enjoy!

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2055

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |djm at mindrot.org
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED
             Blocks|                            |2035

--- Comment #1 from Damien Miller <djm at mindrot.org> ---
Good catch. This has been fixed and will be in openssh-6.2

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2055

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #2 from Damien Miller <djm at mindrot.org> ---
mark bugs closed by openssh-6.2 release as CLOSED

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.