openssh bugs - Apr 2007 - [Bug 1307] client disconnects if ServerAlive enabled but not implemented

http://bugzilla.mindrot.org/show_bug.cgi?id=1307

           Summary: client disconnects if ServerAlive enabled but not
                    implemented
           Product: Portable OpenSSH
           Version: 4.3p2
          Platform: Other
        OS/Version: Linux
            Status: NEW
          Severity: normal
          Priority: P2
         Component: ssh
        AssignedTo: bitbucket at mindrot.org
        ReportedBy: dave.barr at gmail.com


I'm using OpenSSH v4.3p2 against a NetApp filer.  It does NOT implement
ServerAlive*, and correctly returns SSH2_MSG_UNIMPLEMENTED if sent a
ServerAlive packet (as seen with ssh -v).

ssh unfortunately however still disconnects the client after sending
ServerAliveCountMax unanswered packets even though the server correctly
told the client that it is not implemented.

ssh should either disable ServerAlive* when the server says it is not
implemented, or perhaps treat SSH2_MSG_UNIMPLEMENTED packets as a valid
ServerAlive ack.  (My vote is the former).

(Who is also submitting an RFE to get ServerAlive implemented in
NetApp's sshd, but isn't holding his breath).




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=1307





------- Comment #1 from dtucker at zip.com.au  2007-04-17 14:39 -------
Created an attachment (id=1263)
 --> (http://bugzilla.mindrot.org/attachment.cgi?id=1263&action=view)
Reset activity counters on SSH2_MSG_UNIMPLEMENTED and SSH2_IGNORE
messages.

SSH2_MSG_UNIMPLEMENTED is handled in packet.c does not make it back up
to where it could reset the inactivity timer.  This should probably be
fixed (SSH2_MSG_IGNORE too).

Please try the attached patch (against -current but will probably apply
to 4.6p1 too).




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=1307





------- Comment #2 from dtucker at zip.com.au  2007-04-17 14:47 -------
(From update of attachment 1263)
>-			case SSH2_MSG_UNIMPLEMENTED:
>-				seqnr = packet_get_int();
>-				debug("Received SSH2_MSG_UNIMPLEMENTED for %u",
>-				    seqnr);
>-				break;
> 			default:
> 				return type;
I just realized that we could replace the break with /* FALLTHROUGH */
and save a few bytes by not having to duplicate the debug in
serverloop.c and clientloop.c




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=1307


dtucker at zip.com.au changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
Attachment #1263 is|0                           |1
           obsolete|                            |




------- Comment #3 from dtucker at zip.com.au  2007-04-17 14:55 -------
Created an attachment (id=1264)
 --> (http://bugzilla.mindrot.org/attachment.cgi?id=1264&action=view)
Simplify patch #1263.




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=1307





------- Comment #4 from djm at mindrot.org  2007-04-19 17:36 -------
The patch looks generally OK, but I'm undecided on whether
SSH2_MSG_IGNORE should tickle the keepalive watchdog. It comes down to
whether the keepalive is a unidirectional "is the other end still
alive?" or end-to-end thing "is the other end still responding to
me?".

If it is an end-to-end thing then I think SSH2_MSG_IGNORE should not
restart the keepalive timer as these may be sent by the peer as their
own keepalives, whereas SSH2_MSG_UNIMPLEMENTED will only be sent in
response to something that the local end has send (fulfilling the
end-to-end test requirement).




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=1307





------- Comment #5 from markus at openbsd.org  2007-04-19 18:16 -------
(In reply to comment #4)
> The patch looks generally OK, but I'm undecided on whether
> SSH2_MSG_IGNORE should tickle the keepalive watchdog. It comes down to
> whether the keepalive is a unidirectional "is the other end still
> alive?" or end-to-end thing "is the other end still responding to
me?".
i think it's a "does the other end still exist and send messages"
thing.




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=1307





------- Comment #6 from dtucker at zip.com.au  2007-04-19 18:38 -------
(In reply to comment #4)
> If it is an end-to-end thing then I think SSH2_MSG_IGNORE should not
> restart the keepalive timer as these may be sent by the peer as their
> own keepalives, whereas SSH2_MSG_UNIMPLEMENTED will only be sent in
> response to something that the local end has send (fulfilling the
> end-to-end test requirement).
Unless I'm misreading the code, any global request from the other end
(eg a port forward request) other than IGNORE or UNIMPLEMENTED will
reset the keepalive timer, even if it's not in response to a keepalive
packet.  I think that it IGNORE messages should reset the keepalive as
they're just as good an indication that the other end is alive as any
other packet (more, if the peer is using them for its own keepalive).




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=1307


djm at mindrot.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #1264|                            |ok+
               Flag|                            |




------- Comment #7 from djm at mindrot.org  2007-04-19 18:50 -------
(From update of attachment 1264)
>--- packet.c	17 Jan 2007 00:00:14 -0000	1.146
>+++ packet.c	17 Apr 2007 04:51:57 -0000
>@@ -967,9 +967,10 @@ packet_read_expect(int expected_type)
>  * packet_process_incoming.  If so, reads the packet; otherwise returns
>  * SSH_MSG_NONE.  This does not wait for data from the connection.
>  *
>- * SSH_MSG_DISCONNECT is handled specially here.  Also,
>- * SSH_MSG_IGNORE messages are skipped by this function and are never
returned
>- * to higher levels.
>+ * SSH_MSG_DISCONNECT is handled specially here.  Also, SSH_MSG_IGNORE
>+ * messages are skipped by this function and are never returned
>+ * to higher levels, although SSH2_MSG_IGNORE are since they are needed
>+ * for keepalives.
This comment is a little wordy and confusing. Maybe just
"SSH_MSG_IGNORE is passed up without processing for use as a
keepalive"?




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=1307





------- Comment #8 from dtucker at zip.com.au  2007-04-19 19:03 -------
(In reply to comment #7)
> This comment is a little wordy and confusing. Maybe just
> "SSH_MSG_IGNORE is passed up without processing for use as a
> keepalive"?
It's wordy because it talks about SSH_MSG_IGNORE (Protocol 1) and
SSH2_MSG_IGNORE (Protocol 2) separately because they are handled
differently.  It could probably be clearer.

David, can you confirm whether or not patch #1264 resolves the problem
for you?  Thanks.




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=1307


dtucker at zip.com.au changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Platform|Other                       |All
         OS/Version|Linux                       |All
             Status|NEW                         |ASSIGNED
OtherBugsDependingO|                            |1289, 1305
              nThis|                            |






------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.