Checking for disconnects within ssh_auth_response_termination()?

[Karl Scott <karlscottbg@xxxxxxxxx>]

Hey folks,

I've been working with libssh for some time now, and saw that the function
that ends up being looped over from within
ssh_handle_packets_termination(), in src/session.c, during an
authentication attempt is often ssh_auth_response_termination().

I noticed that in this small function:













*static int ssh_auth_response_termination(void *user){  ssh_session
session=(ssh_session)user;  switch(session->auth_state){    case
SSH_AUTH_STATE_NONE:    case SSH_AUTH_STATE_KBDINT_SENT:    case
SSH_AUTH_STATE_GSSAPI_REQUEST_SENT:    case SSH_AUTH_STATE_GSSAPI_TOKEN:
case SSH_AUTH_STATE_GSSAPI_MIC_SENT:      return 0;    default:      return
1;  }}*

We do not check for session disconnects, so if the session disconnects
while we are looping over this, we won't stop looping until we simply time
out.

Is this intended, as maybe the session will reconnect somehow while we are
looping in here? If it is not the case that the session might reconnect,
may I propose this patch that I am currently using internally:

--- src/auth.c    2016-02-22 23:16:40.000000000 -0800
+++ src/auth.modified.c    2017-10-19 12:35:06.774460055 -0700
@@ -79,6 +79,10 @@

 static int ssh_auth_response_termination(void *user){
   ssh_session session=(ssh_session)user;
+  if (ssh_is_connected(session) == 0) {
+      session->auth_state = SSH_AUTH_STATE_FAILED;
+      return 1; /* don't wait to time out if the session disconnects */
+  }
   switch(session->auth_state){
     case SSH_AUTH_STATE_NONE:
     case SSH_AUTH_STATE_KBDINT_SENT:

I am happy to submit this patch formally, but I wanted to bounce it off of
the community first. I may be overlooking something here. See any problems?

Thank you,

Karl