From a41eefff41f994c5d01978f8b2dd10683403e9ba Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Fri, 18 May 2018 07:22:58 +0100 Subject: [PATCH] do_ssh2_authconn: replace s->gotit with pq_push_front. The userauth loop has always had a rather awkward feature whereby some of its branches have _already_ received an SSH_MSG_USERAUTH_SUCCESS or SSH_MSG_USERAUTH_FAILURE packet (e.g. because they have to wait for a packet that might be one of those _or_ a continuation packet of some kind), whereas other branches go back round to the top of the loop at the moment that they know one of those will be the next packet to arrive. So we had a flag 's->gotit' which tells the start of the loop whether it's already sitting on the success or failure message, or whether the first thing it should do is to crWait for one. Now that the packets are coming from a linked list, there's a simpler way to handle this: the top of the userauth loop _always_ expects to find a success/failure message as the first thing in the queue, and if any branch of the auth code finds it's already removed such a message from the queue, it can simply put it back on the front again before going back round. --- ssh.c | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/ssh.c b/ssh.c index 60ef289c..ab914ec4 100644 --- a/ssh.c +++ b/ssh.c @@ -10018,7 +10018,7 @@ static void do_ssh2_authconn(void *vctx) AUTH_TYPE_KEYBOARD_INTERACTIVE_QUIET } type; int done_service_req; - int gotit, need_pw, can_pubkey, can_passwd, can_keyb_inter; + int need_pw, can_pubkey, can_passwd, can_keyb_inter; int userpass_ret; int tried_pubkey_config, done_agent; #ifndef NO_GSSAPI @@ -10375,7 +10375,6 @@ static void do_ssh2_authconn(void *vctx) ssh2_pkt_addstring(s->pktout, "none"); /* method */ ssh2_pkt_send(ssh, s->pktout); s->type = AUTH_TYPE_NONE; - s->gotit = FALSE; s->we_are_in = FALSE; s->tried_pubkey_config = FALSE; @@ -10399,8 +10398,8 @@ static void do_ssh2_authconn(void *vctx) /* * Wait for the result of the last authentication request. */ - if (!s->gotit) - crMaybeWaitUntilV((pktin = pq_pop(&ssh->pq_ssh2_authconn)) != NULL); + crMaybeWaitUntilV((pktin = pq_pop(&ssh->pq_ssh2_authconn)) != NULL); + /* * Now is a convenient point to spew any banner material * that we've accumulated. (This should ensure that when @@ -10437,8 +10436,6 @@ static void do_ssh2_authconn(void *vctx) crStopV; } - s->gotit = FALSE; - /* * OK, we're now sitting on a USERAUTH_FAILURE message, so * we can look at the string in it and know what we can @@ -10597,8 +10594,9 @@ static void do_ssh2_authconn(void *vctx) crMaybeWaitUntilV((pktin = pq_pop(&ssh->pq_ssh2_authconn)) != NULL); if (pktin->type != SSH2_MSG_USERAUTH_PK_OK) { - /* Offer of key refused. */ - s->gotit = TRUE; + /* Offer of key refused, presumably via + * USERAUTH_FAILURE. Requeue for the next iteration. */ + pq_push_front(&ssh->pq_ssh2_authconn, pktin); } else { @@ -10733,7 +10731,7 @@ static void do_ssh2_authconn(void *vctx) crMaybeWaitUntilV((pktin = pq_pop(&ssh->pq_ssh2_authconn)) != NULL); if (pktin->type != SSH2_MSG_USERAUTH_PK_OK) { /* Key refused. Give up. */ - s->gotit = TRUE; /* reconsider message next loop */ + pq_push_front(&ssh->pq_ssh2_authconn, pktin); s->type = AUTH_TYPE_PUBLICKEY_OFFER_LOUD; continue; /* process this new message */ } @@ -10892,7 +10890,6 @@ static void do_ssh2_authconn(void *vctx) s->type = AUTH_TYPE_GSSAPI; s->tried_gssapi = TRUE; - s->gotit = TRUE; ssh->pkt_actx = SSH2_PKTCTX_GSSAPI; if (ssh->gsslib->gsslogmsg) @@ -10925,6 +10922,7 @@ static void do_ssh2_authconn(void *vctx) crMaybeWaitUntilV((pktin = pq_pop(&ssh->pq_ssh2_authconn)) != NULL); if (pktin->type != SSH2_MSG_USERAUTH_GSSAPI_RESPONSE) { logevent("GSSAPI authentication request refused"); + pq_push_front(&ssh->pq_ssh2_authconn, pktin); continue; } @@ -11019,6 +11017,7 @@ static void do_ssh2_authconn(void *vctx) logevent("GSSAPI authentication -" " bad server response"); s->gss_stat = SSH_GSS_FAILURE; + pq_push_front(&ssh->pq_ssh2_authconn, pktin); break; } ssh_pkt_getstring(pktin, &data, &len); @@ -11039,8 +11038,6 @@ static void do_ssh2_authconn(void *vctx) ssh2_gss_authpacket(ssh, s->gss_ctx, "gssapi-with-mic"); ssh2_pkt_send(ssh, s->pktout); - s->gotit = FALSE; - ssh->gsslib->release_cred(ssh->gsslib, &s->gss_ctx); continue; #endif @@ -11072,7 +11069,7 @@ static void do_ssh2_authconn(void *vctx) * at all (or, bizarrely but legally, accepts the * user without actually issuing any prompts). * Give up on it entirely. */ - s->gotit = TRUE; + pq_push_front(&ssh->pq_ssh2_authconn, pktin); s->type = AUTH_TYPE_KEYBOARD_INTERACTIVE_QUIET; s->kbd_inter_refused = TRUE; /* don't try it again */ continue; @@ -11201,7 +11198,7 @@ static void do_ssh2_authconn(void *vctx) /* * We should have SUCCESS or FAILURE now. */ - s->gotit = TRUE; + pq_push_front(&ssh->pq_ssh2_authconn, pktin); } else if (s->can_passwd) { @@ -11428,7 +11425,7 @@ static void do_ssh2_authconn(void *vctx) * In any of these cases, we go back to the top of * the loop and start again. */ - s->gotit = TRUE; + pq_push_front(&ssh->pq_ssh2_authconn, pktin); /* * We don't need the old password any more, in any