From dd14beef075ab5b70eb6c0c434d30d99fcc7d8d5 Mon Sep 17 00:00:00 2001
From: Simon Tatham <anakin@pobox.com>
Date: Thu, 15 Nov 2018 18:12:19 +0000
Subject: [PATCH] Handle SSH_MSG_USERAUTH_GSSAPI_ERRTOK gracefully.

Jacob ran across a server which terminated a GSS userauth attempt by
sending that message before USERAUTH_FAILURE. The result was that
PuTTY interpreted the ERRTOK as indicating a failure of authentication
(so far, fair enough), and then handled it exactly as it would have
handled USERAUTH_FAILURE itself: it pushed it back on in_pq and went
back round the main userauth loop. But the thing that loop is
expecting to find at the head of in_pq is an _actual_
USERAUTH_FAILURE, not some arbitrary thing that preceded it. So this
led to some confusion, especially when the real USERAUTH_FAILURE that
immediately followed the ERRTOK got interpreted as the response to the
_next_ auth attempt.

One of the root causes is that we had no handler for ERRTOK at all. We
now do. (Though it's trivial - we ignore the content of the message
and just wait for the followup USERAUTH_FAILURE that the GSSAPI RFC
says MUST come next. Possibly there's something nicer we could do
involving handing the error token to the GSSAPI library and letting it
print a final user-facing message? But that's beyond my GSS expertise.)

But this also exposed another problem: we shouldn't be pushing _any_
packet back on in_pq unless it's actually a USERAUTH_FAILURE. Any
other unexpected packet should have _different_ confused handling. So
now that call to pq_push_front is conditionalised on the packet type,
and only triggers for USERAUTH_FAILURE proper.
---
 ssh2userauth.c | 37 +++++++++++++++++++++++++++++++++++--
 1 file changed, 35 insertions(+), 2 deletions(-)

diff --git a/ssh2userauth.c b/ssh2userauth.c
index 8e4b88dc..3c57c429 100644
--- a/ssh2userauth.c
+++ b/ssh2userauth.c
@@ -1015,11 +1015,44 @@ static void ssh2_userauth_process_queue(PacketProtocolLayer *ppl)
 
                     if (s->gss_stat == SSH_GSS_S_CONTINUE_NEEDED) {
                         crMaybeWaitUntilV((pktin = ssh2_userauth_pop(s)) != NULL);
-                        if (pktin->type != SSH2_MSG_USERAUTH_GSSAPI_TOKEN) {
+
+                        if (pktin->type == SSH2_MSG_USERAUTH_GSSAPI_ERRTOK) {
+                            /*
+                             * Per RFC 4462 section 3.9, this packet
+                             * type MUST immediately precede an
+                             * ordinary USERAUTH_FAILURE.
+                             *
+                             * We currently don't know how to do
+                             * anything with the GSSAPI error token
+                             * contained in this packet, so we ignore
+                             * it and just wait for the following
+                             * FAILURE.
+                             */
+                            crMaybeWaitUntilV(
+                                (pktin = ssh2_userauth_pop(s)) != NULL);
+                            if (pktin->type != SSH2_MSG_USERAUTH_FAILURE) {
+                                ssh_proto_error(
+                                    s->ppl.ssh, "Received unexpected packet "
+                                    "after SSH_MSG_USERAUTH_GSSAPI_ERRTOK "
+                                    "(expected SSH_MSG_USERAUTH_FAILURE): "
+                                    "type %d (%s)", pktin->type,
+                                    ssh2_pkt_type(s->ppl.bpp->pls->kctx,
+                                                  s->ppl.bpp->pls->actx,
+                                                  pktin->type));
+                                return;
+                            }
+                        }
+
+                        if (pktin->type == SSH2_MSG_USERAUTH_FAILURE) {
+                            ppl_logevent(("GSSAPI authentication failed"));
+                            s->gss_stat = SSH_GSS_FAILURE;
+                            pq_push_front(s->ppl.in_pq, pktin);
+                            break;
+                        } else if (pktin->type !=
+                                   SSH2_MSG_USERAUTH_GSSAPI_TOKEN) {
                             ppl_logevent(("GSSAPI authentication -"
                                           " bad server response"));
                             s->gss_stat = SSH_GSS_FAILURE;
-                            pq_push_front(s->ppl.in_pq, pktin);
                             break;
                         }
                         data = get_string(pktin);