openssh bugs - Aug 2014 - [Bug 2265] New: ServerAlive{Interval,CountMax} ignored if using an active -R or -L tunnel

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

            Bug ID: 2265
           Summary: ServerAlive{Interval,CountMax} ignored if using an
                    active -R or -L tunnel
           Product: Portable OpenSSH
           Version: -current
          Hardware: All
                OS: All
            Status: NEW
          Severity: normal
          Priority: P5
         Component: ssh
          Assignee: unassigned-bugs at mindrot.org
          Reporter: openssh at orib.net

Scenario:

1. Set up a local socket server that sends data slowly enough so that
buffers would take hours to fill up:

  $ (until false; do echo -n X; sleep 2; done) | nc -l 8000 &

2. Connect through an unreliable connection, asking to detect a broken
connection within 10 seconds (5 second "alive" signals, 2 missing
maximum)

  $ ssh -R 8001:127.0.0.1:8000 \
        -o 'ServerAliveInterval 5' -o 'ServerAliveCountMax 2' \
        -o 'ProxyCommand nc 127.0.0.1 22' \
        127.0.0.1 'telnet 127.0.0.1 8001'

(this assumes you can ssh into localhost using either a password or
public key authentication)

3. Observe that indeed, you are getting 'X' printed every 2 seconds,
through the ssh tunnel.

4. Suspend the intermediate proxy - in another terminal / screen
session (or after backgrounding the ssh command above), do:

   $ pkill -STOP -xf 'nc 127.0.0.1 22'

5. Wait 10 seconds for ServerAlive detection to kick in. Or 10 hours.
ServerAlive detection never actually kicks in.

6. Tear down everything (it is enough to Ctrl-C the ssh command)

7. Repeat steps 1-5, this time, with 'sleep 2' replaced by 'sleep
30'.
This time, ServerAlive detection kicks in as expected.

This happens on every openssh version I've tried (All on linux, the
versions on ubuntu 8.04, 10.04, 10.10, 12.04, 14.04), and is still in
current from browsing the source code.

The problem is the "ServerAlive" logic (and I assume, also the
ClientAlive logic on the server side - though I haven't verified that
yet): A connection is deemed "alive" if the select() waiting for data
did not time out. 

However, it should be deemed alive only if there has been data on the
ssh connection itself - not the local ends of a -L / -R tunnel and
whatever other local sockets might be waited upon with select(). 
As the above example shows, even though the connection to the server is
effectively dead, it will not be detected.

This setup is artificial, and is easier to debug than a real world
setting. It includes:

- the ssh server
- an intermediate pipe ('nc 127.0.0.1 22') that can be kill -STOPped
without dropping the connection
- the ssh client
- a slow server that trickles data through a tunnel

In a real world scenario, the intermediate pipe is likely to be an
unreliable network connection (e.g. an intermediate router somewhere
along the way that is not directly connected to a client interface -
and that stops routing traffic in the middle of the session). If this
is the case, then eventually the ssh client will have a TCP timeout (2
mins, usually) and detect the broken connection -- which is why I
suppose this was not previously reported. However, if there is no
indication the intermediate connection died (like in the example I gave
above), then the ssh client will hang forever, despite the
"ServerAlive*" settings.

As I mentioned, this likely applies to the sshd, ClientAliveInterval,
ClientAliveCountMax respectively, though I haven't verified it.

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

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

openssh at orib.net changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |openssh at orib.net

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

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

--- Comment #1 from openssh at orib.net ---
Note that in some circumstances this can be leveraged into a
denial-of-service attack - if an attacker is able to disconnect a
remote connection and feed data locally at the same time, they can
avoid new data coming in.

(I found this out while investigating what looked like a DOS but
eventually wasn't)

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

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

jxraynor at gmail.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jxraynor at gmail.com

--- Comment #2 from jxraynor at gmail.com ---
The patch sent to the mailing list here:

https://lists.mindrot.org/pipermail/openssh-unix-dev/2020-May/038522.html

...will fix this issue.  However, the patch is currently in limbo,
neither accepted nor rejected.

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

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

Darren Tucker <dtucker at dtucker.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dtucker at dtucker.net

--- Comment #3 from Darren Tucker <dtucker at dtucker.net> ---
Created attachment 3417
  --> https://bugzilla.mindrot.org/attachment.cgi?id=3417&action=edit
ServerAliveInterval doesn't work if client keeps trying to send data

Patch in question for commenting.

-- 
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=2265

Darren Tucker <dtucker at dtucker.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |3162


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=3162
[Bug 3162] Tracking bug for 8.4 release
-- 
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=2265

--- Comment #4 from Darren Tucker <dtucker at dtucker.net> ---
Comment on attachment 3417
  --> https://bugzilla.mindrot.org/attachment.cgi?id=3417
ServerAliveInterval doesn't work if client keeps trying to send data

Looks mostly ok, there's a couple of long lines and one comment:
>+		timeout_secs = server_alive_time - now;
>+		if (timeout_secs < 0)
>+			timeout_secs = 0;
This can be a MAXIMUM(..) which is shorter and consistent with the rest
of the code.

I'll attach an updated patch shortly.

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

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

Darren Tucker <dtucker at dtucker.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #3417|0                           |1
        is obsolete|                            |
                 CC|                            |djm at mindrot.org
   Attachment #3419|                            |ok?(djm at mindrot.org)
              Flags|                            |

--- Comment #5 from Darren Tucker <dtucker at dtucker.net> ---
Created attachment 3419
  --> https://bugzilla.mindrot.org/attachment.cgi?id=3419&action=edit
Make ServerAlive behave correctly during client port forward activity

-- 
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=2265

Darren Tucker <dtucker at dtucker.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #3419|ok?(djm at mindrot.org)        |
              Flags|                            |
   Attachment #3419|0                           |1
        is obsolete|                            |
   Attachment #3420|                            |ok?(djm at mindrot.org)
              Flags|                            |

--- Comment #6 from Darren Tucker <dtucker at dtucker.net> ---
Created attachment 3420
  --> https://bugzilla.mindrot.org/attachment.cgi?id=3420&action=edit
Move the ServerAlive scheduling into a helper function.

To me this is a bit easier to read.

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

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

Darren Tucker <dtucker at dtucker.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #3420|0                           |1
        is obsolete|                            |
   Attachment #3420|ok?(djm at mindrot.org)        |
              Flags|                            |
   Attachment #3421|                            |ok?(djm at mindrot.org)
              Flags|                            |

--- Comment #7 from Darren Tucker <dtucker at dtucker.net> ---
Created attachment 3421
  --> https://bugzilla.mindrot.org/attachment.cgi?id=3421&action=edit
Move the ServerAlive scheduling into a helper function.

fix typo

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

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

Darren Tucker <dtucker at dtucker.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #8 from Darren Tucker <dtucker at dtucker.net> ---
(modified) patch applied and and will be in the 8.4 release.  Thanks
for the report and patch.

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

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

Jordan Macdonald <macdjord at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |macdjord at gmail.com

-- 
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=2265

Damien Miller <djm at mindrot.org> changed:

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

--- Comment #9 from Damien Miller <djm at mindrot.org> ---
close bugs that were resolved in OpenSSH 8.5 release cycle

-- 
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=2265

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #3421|ok?(djm at mindrot.org)        |
              Flags|                            |

-- 
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.