openssh bugs - Jul 2014 - [Bug 2252] New: RekeyLimit breaks ClientAlive

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

            Bug ID: 2252
           Summary: RekeyLimit breaks ClientAlive
           Product: Portable OpenSSH
           Version: 6.6p1
          Hardware: All
                OS: All
            Status: NEW
          Severity: normal
          Priority: P5
         Component: sshd
          Assignee: unassigned-bugs at mindrot.org
          Reporter: wiwi at progon.net

If RekeyLimit is enabled, ClientAlive messages will not be sent, ever!

Problem seems to be:

serverloop.c: 
   wait_until_can_do_something(...)

max_time_milliseconds is set to the remaining time to a rekey. 

client_alive_scheduled never gets set, as max_time_milliseconds!=0:

if (compat20 &&
  max_time_milliseconds == 0 && options.client_alive_interval) {
    client_alive_scheduled = 1;
    max_time_milliseconds      (u_int64_t)options.client_alive_interval * 1000;
  }

The if clause might need changed to something like this:

if (compat20 &&
  max_time_milliseconds > ((u_int64_t)options.client_alive_interval *
1000)  && 
  options.client_alive_interval) {
 ...

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

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

Garrett Lee <glee at ciena.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |glee at ciena.com

--- Comment #1 from Garrett Lee <glee at ciena.com> ---
I am also experiencing this problem and investigating an appropriate
fix.  Thank you for point out a possible fix, it saved me the time of
having to dig around through the code. 

Looking at your proposed fix, I am wondering about the case where
max_time_milliseconds actually is value 0.  I think if the rekey-time
is 0 then with your proposed code change it will skip the
tcp-keepalive.

Do you suppose the following code change would cover cases when rekey
time is zero and cases where it is non-zero?

    if (compat20 &&
            options.client_alive_interval &&
        ((max_time_milliseconds == 0) || (max_time_milliseconds >
((u_int64_t)options.client_alive_interval * 1000))) ) {
...

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

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

--- Comment #2 from Garrett Lee <glee at ciena.com> ---
(In reply to Garrett Lee from comment #1)
This bug is about SSH ClientAlive which I erroneously referred to as
'tcp-keepalive' in my previous comment.

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

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

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |djm at mindrot.org

--- Comment #3 from Damien Miller <djm at mindrot.org> ---
It will take a little more than just adjusting the first test in
wait_until_can_do_something(). Any timeout return from select() is
interpreted as "time to send a keepalive!", so we would need to keep a
proper deadline instead

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

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |2403

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

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |2443

--- Comment #4 from Damien Miller <djm at mindrot.org> ---
Retarget pending bugs to openssh-7.1

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

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|2403                        |

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

Darren Tucker <dtucker at zip.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |2451


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=2451
[Bug 2451] Bugs intended to be fixed in 7.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=2252

Darren Tucker <dtucker at zip.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|2443                        |


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=2443
[Bug 2443] Bugs intended to be fixed for OpenSSH 7.1
-- 
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=2252

Roland Friedwagner <r.friedwagner at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |r.friedwagner 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=2252

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned-bugs at mindrot.org |djm at mindrot.org
                 CC|                            |dtucker at zip.com.au
             Status|NEW                         |ASSIGNED
   Attachment #2793|                            |ok?(dtucker at zip.com.au)
              Flags|                            |

--- Comment #5 from Damien Miller <djm at mindrot.org> ---
Created attachment 2793
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2793&action=edit
fix rekey/clientalive interaction

This fixes the rekey/clientalive interaction, though a little
inexactly. 

It will work correctly if clientalivetimeout is less than the timed
rekeylimit, but will send extra client alive pings if the rekeylimit
timeout is smaller. In practice, I don't think this is a huge problem
since rekey timeouts are likely to be long and extra client alive
packets are quite harmless.

Fixing it properly would require a more comprehensive timekeeping
system to separately manage the rekey and client-alive deadlines.

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

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|2451                        |2543


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=2451
[Bug 2451] Bugs intended to be fixed in 7.2
https://bugzilla.mindrot.org/show_bug.cgi?id=2543
[Bug 2543] Tracking bug for OpenSSH 7.3 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=2252

Damien Miller <djm at mindrot.org> changed:

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

--- Comment #6 from Damien Miller <djm at mindrot.org> ---
Fixed - this will be in openssh-7.3

commit cb34a5a98043bbd7bfb8c88fb0fe0da562de811d
Author: djm at openbsd.org <djm at openbsd.org>
Date:   Fri Mar 4 03:35:44 2016 +0000

    upstream commit

    fix ClientAliveInterval when a time-based RekeyLimit is
     set; previously keepalive packets were not being sent. bz#2252
report and
     analysis by Christian Wittenhorst and Garrett Lee feedback and ok
dtucker@

    Upstream-ID: d48f9deadd35fdacdd5106b41bb07630ddd4aa81

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

Darren Tucker <dtucker at zip.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |calestyo at scientia.net

--- Comment #7 from Darren Tucker <dtucker at zip.com.au> ---
*** Bug 2572 has been marked as a duplicate of this bug. ***

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

Damien Miller <djm at mindrot.org> changed:

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

--- Comment #8 from Damien Miller <djm at mindrot.org> ---
Close all resolved bugs after 7.3p1 release

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

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #2793|ok?(dtucker at dtucker.net)    |
              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.