Fix user enumeration vulnerability

Apply upstream patch to delay bailout for invalid authenticating user
until after the packet containing the request has been fully parsed.

Closes: #906236

7 files changed, 189 insertions, 18 deletions

diff --git a/auth2-gss.c b/auth2-gss.c
index fd411d3a7..88bc3ae7b 100644
--- a/auth2-gss.c
+++ b/auth2-gss.c
@@ -104,9 +104,6 @@ userauth_gssapi(struct ssh *ssh)
         u_int len;
         u_char *doid = NULL;
 
-        if (!authctxt->valid || authctxt->user == NULL)
-                return (0);
-
         mechs = packet_get_int();
         if (mechs == 0) {
                 debug("Mechanism negotiation is not supported");
@@ -137,6 +134,12 @@ userauth_gssapi(struct ssh *ssh)
                 return (0);
         }
 
+        if (!authctxt->valid || authctxt->user == NULL) {
+                debug2("%s: disabled because of invalid user", __func__);
+                free(doid);
+                return (0);
+        }
+
         if (GSS_ERROR(PRIVSEP(ssh_gssapi_server_ctx(&ctxt, &goid)))) {
                 if (ctxt != NULL)
                         ssh_gssapi_delete_ctx(&ctxt);
diff --git a/auth2-hostbased.c b/auth2-hostbased.c
index 8996f7e05..82a7dcdae 100644
--- a/auth2-hostbased.c
+++ b/auth2-hostbased.c
@@ -67,10 +67,6 @@ userauth_hostbased(struct ssh *ssh)
         size_t alen, blen, slen;
         int r, pktype, authenticated = 0;
 
-        if (!authctxt->valid) {
-                debug2("%s: disabled because of invalid user", __func__);
-                return 0;
-        }
         /* XXX use sshkey_froms() */
         if ((r = sshpkt_get_cstring(ssh, &pkalg, &alen)) != 0 ||
             (r = sshpkt_get_string(ssh, &pkblob, &blen)) != 0 ||
@@ -118,6 +114,11 @@ userauth_hostbased(struct ssh *ssh)
                 goto done;
         }
 
+        if (!authctxt->valid || authctxt->user == NULL) {
+                debug2("%s: disabled because of invalid user", __func__);
+                goto done;
+        }
+
         if ((b = sshbuf_new()) == NULL)
                 fatal("%s: sshbuf_new failed", __func__);
         /* reconstruct packet */
diff --git a/auth2-pubkey.c b/auth2-pubkey.c
index 8024b1d6a..a9272b97f 100644
--- a/auth2-pubkey.c
+++ b/auth2-pubkey.c
@@ -89,19 +89,15 @@ userauth_pubkey(struct ssh *ssh)
 {
         Authctxt *authctxt = ssh->authctxt;
         struct passwd *pw = authctxt->pw;
-        struct sshbuf *b;
+        struct sshbuf *b = NULL;
         struct sshkey *key = NULL;
-        char *pkalg, *userstyle = NULL, *key_s = NULL, *ca_s = NULL;
-        u_char *pkblob, *sig, have_sig;
+        char *pkalg = NULL, *userstyle = NULL, *key_s = NULL, *ca_s = NULL;
+        u_char *pkblob = NULL, *sig = NULL, have_sig;
         size_t blen, slen;
         int r, pktype;
         int authenticated = 0;
         struct sshauthopt *authopts = NULL;
 
-        if (!authctxt->valid) {
-                debug2("%s: disabled because of invalid user", __func__);
-                return 0;
-        }
         if ((r = sshpkt_get_u8(ssh, &have_sig)) != 0 ||
             (r = sshpkt_get_cstring(ssh, &pkalg, NULL)) != 0 ||
             (r = sshpkt_get_string(ssh, &pkblob, &blen)) != 0)
@@ -168,6 +164,11 @@ userauth_pubkey(struct ssh *ssh)
                                 fatal("%s: sshbuf_put_string session id: %s",
                                     __func__, ssh_err(r));
                 }
+                if (!authctxt->valid || authctxt->user == NULL) {
+                        debug2("%s: disabled because of invalid user",
+                            __func__);
+                        goto done;
+                }
                 /* reconstruct packet */
                 xasprintf(&userstyle, "%s%s%s", authctxt->user,
                     authctxt->style ? ":" : "",
@@ -184,7 +185,6 @@ userauth_pubkey(struct ssh *ssh)
 #ifdef DEBUG_PK
                 sshbuf_dump(b, stderr);
 #endif
-
                 /* test for correct signature */
                 authenticated = 0;
                 if (PRIVSEP(user_key_allowed(ssh, pw, key, 1, &authopts)) &&
@@ -193,7 +193,6 @@ userauth_pubkey(struct ssh *ssh)
                         authenticated = 1;
                 }
                 sshbuf_free(b);
-                free(sig);
                 auth2_record_key(authctxt, authenticated, key);
         } else {
                 debug("%s: test pkalg %s pkblob %s%s%s",
@@ -204,6 +203,11 @@ userauth_pubkey(struct ssh *ssh)
                 if ((r = sshpkt_get_end(ssh)) != 0)
                         fatal("%s: %s", __func__, ssh_err(r));
 
+                if (!authctxt->valid || authctxt->user == NULL) {
+                        debug2("%s: disabled because of invalid user",
+                            __func__);
+                        goto done;
+                }
                 /* XXX fake reply and always send PK_OK ? */
                 /*
                  * XXX this allows testing whether a user is allowed
@@ -237,6 +241,7 @@ done:
         free(pkblob);
         free(key_s);
         free(ca_s);
+        free(sig);
         return authenticated;
 }
 
diff --git a/debian/.git-dpm b/debian/.git-dpm
index 0f4069a2f..40345f1a3 100644
--- a/debian/.git-dpm
+++ b/debian/.git-dpm
@@ -1,6 +1,6 @@
 # see git-dpm(1) from git-dpm package
-60256f28189c3d0650a78e737eb0ca4753478a4b
-60256f28189c3d0650a78e737eb0ca4753478a4b
+c4ca1497658e0508e8595ad74978c07bc92a18e3
+c4ca1497658e0508e8595ad74978c07bc92a18e3
 ed6ae9c1a014a08ff5db3d768f01f2e427eeb476
 ed6ae9c1a014a08ff5db3d768f01f2e427eeb476
 openssh_7.7p1.orig.tar.gz
diff --git a/debian/changelog b/debian/changelog
index 15024f76b..d9de16199 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,11 @@
+openssh (1:7.7p1-4) UNRELEASED; urgency=high
+
+  * Apply upstream patch to delay bailout for invalid authenticating user
+    until after the packet containing the request has been fully parsed
+    (closes: #906236).
+
+ -- Colin Watson <cjwatson@debian.org>  Fri, 17 Aug 2018 12:30:13 +0100
+
 openssh (1:7.7p1-3) unstable; urgency=medium
 
   [ Colin Watson ]
diff --git a/debian/patches/series b/debian/patches/series
index 9f89f7347..e1eb16773 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -25,3 +25,4 @@ seccomp-s390-flock-ipc.patch
 seccomp-getuid-geteuid.patch
 seccomp-s390-ioctl-ep11-crypto.patch
 upstream-relax-checking-of-authorized_keys-environme.patch
+upstream-delay-bailout-for-invalid-authenticating-user.patch
diff --git a/debian/patches/upstream-delay-bailout-for-invalid-authenticating-user.patch b/debian/patches/upstream-delay-bailout-for-invalid-authenticating-user.patch
new file mode 100644
index 000000000..737a9f48d
--- /dev/null
+++ b/debian/patches/upstream-delay-bailout-for-invalid-authenticating-user.patch
@@ -0,0 +1,153 @@
+From c4ca1497658e0508e8595ad74978c07bc92a18e3 Mon Sep 17 00:00:00 2001
+From: "djm@openbsd.org" <djm@openbsd.org>
+Date: Tue, 31 Jul 2018 03:10:27 +0000
+Subject: upstream: delay bailout for invalid authenticating user
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+... until after the packet containing the request has been fully parsed.
+Reported by Dariusz Tytko and Michał Sajdak; ok deraadt
+
+OpenBSD-Commit-ID: b4891882fbe413f230fe8ac8a37349b03bd0b70d
+
+Origin: backport, http://anongit.mindrot.org/openssh.git/commit/?id=74287f5df9966a0648b4a68417451dd18f079ab8
+Bug-Debian: https://bugs.debian.org/906236
+Last-Update: 2018-08-17
+
+Patch-Name: upstream-delay-bailout-for-invalid-authenticating-user.patch
+---
+ auth2-gss.c       |  9 ++++++---
+ auth2-hostbased.c |  9 +++++----
+ auth2-pubkey.c    | 23 ++++++++++++++---------
+ 3 files changed, 25 insertions(+), 16 deletions(-)
+
+diff --git a/auth2-gss.c b/auth2-gss.c
+index fd411d3a7..88bc3ae7b 100644
+--- a/auth2-gss.c
++++ b/auth2-gss.c
+@@ -104,9 +104,6 @@ userauth_gssapi(struct ssh *ssh)
+        u_int len;
+        u_char *doid = NULL;
+ 
+-       if (!authctxt->valid || authctxt->user == NULL)
+-               return (0);
+-
+        mechs = packet_get_int();
+        if (mechs == 0) {
+                debug("Mechanism negotiation is not supported");
+@@ -137,6 +134,12 @@ userauth_gssapi(struct ssh *ssh)
+                return (0);
+        }
+ 
++       if (!authctxt->valid || authctxt->user == NULL) {
++               debug2("%s: disabled because of invalid user", __func__);
++               free(doid);
++               return (0);
++       }
++
+        if (GSS_ERROR(PRIVSEP(ssh_gssapi_server_ctx(&ctxt, &goid)))) {
+                if (ctxt != NULL)
+                        ssh_gssapi_delete_ctx(&ctxt);
+diff --git a/auth2-hostbased.c b/auth2-hostbased.c
+index 8996f7e05..82a7dcdae 100644
+--- a/auth2-hostbased.c
++++ b/auth2-hostbased.c
+@@ -67,10 +67,6 @@ userauth_hostbased(struct ssh *ssh)
+        size_t alen, blen, slen;
+        int r, pktype, authenticated = 0;
+ 
+-       if (!authctxt->valid) {
+-               debug2("%s: disabled because of invalid user", __func__);
+-               return 0;
+-       }
+        /* XXX use sshkey_froms() */
+        if ((r = sshpkt_get_cstring(ssh, &pkalg, &alen)) != 0 ||
+            (r = sshpkt_get_string(ssh, &pkblob, &blen)) != 0 ||
+@@ -118,6 +114,11 @@ userauth_hostbased(struct ssh *ssh)
+                goto done;
+        }
+ 
++       if (!authctxt->valid || authctxt->user == NULL) {
++               debug2("%s: disabled because of invalid user", __func__);
++               goto done;
++       }
++
+        if ((b = sshbuf_new()) == NULL)
+                fatal("%s: sshbuf_new failed", __func__);
+        /* reconstruct packet */
+diff --git a/auth2-pubkey.c b/auth2-pubkey.c
+index 8024b1d6a..a9272b97f 100644
+--- a/auth2-pubkey.c
++++ b/auth2-pubkey.c
+@@ -89,19 +89,15 @@ userauth_pubkey(struct ssh *ssh)
+ {
+        Authctxt *authctxt = ssh->authctxt;
+        struct passwd *pw = authctxt->pw;
+-       struct sshbuf *b;
++       struct sshbuf *b = NULL;
+        struct sshkey *key = NULL;
+-       char *pkalg, *userstyle = NULL, *key_s = NULL, *ca_s = NULL;
+-       u_char *pkblob, *sig, have_sig;
++       char *pkalg = NULL, *userstyle = NULL, *key_s = NULL, *ca_s = NULL;
++       u_char *pkblob = NULL, *sig = NULL, have_sig;
+        size_t blen, slen;
+        int r, pktype;
+        int authenticated = 0;
+        struct sshauthopt *authopts = NULL;
+ 
+-       if (!authctxt->valid) {
+-               debug2("%s: disabled because of invalid user", __func__);
+-               return 0;
+-       }
+        if ((r = sshpkt_get_u8(ssh, &have_sig)) != 0 ||
+            (r = sshpkt_get_cstring(ssh, &pkalg, NULL)) != 0 ||
+            (r = sshpkt_get_string(ssh, &pkblob, &blen)) != 0)
+@@ -168,6 +164,11 @@ userauth_pubkey(struct ssh *ssh)
+                                fatal("%s: sshbuf_put_string session id: %s",
+                                    __func__, ssh_err(r));
+                }
++               if (!authctxt->valid || authctxt->user == NULL) {
++                       debug2("%s: disabled because of invalid user",
++                           __func__);
++                       goto done;
++               }
+                /* reconstruct packet */
+                xasprintf(&userstyle, "%s%s%s", authctxt->user,
+                    authctxt->style ? ":" : "",
+@@ -184,7 +185,6 @@ userauth_pubkey(struct ssh *ssh)
+ #ifdef DEBUG_PK
+                sshbuf_dump(b, stderr);
+ #endif
+-
+                /* test for correct signature */
+                authenticated = 0;
+                if (PRIVSEP(user_key_allowed(ssh, pw, key, 1, &authopts)) &&
+@@ -193,7 +193,6 @@ userauth_pubkey(struct ssh *ssh)
+                        authenticated = 1;
+                }
+                sshbuf_free(b);
+-               free(sig);
+                auth2_record_key(authctxt, authenticated, key);
+        } else {
+                debug("%s: test pkalg %s pkblob %s%s%s",
+@@ -204,6 +203,11 @@ userauth_pubkey(struct ssh *ssh)
+                if ((r = sshpkt_get_end(ssh)) != 0)
+                        fatal("%s: %s", __func__, ssh_err(r));
+ 
++               if (!authctxt->valid || authctxt->user == NULL) {
++                       debug2("%s: disabled because of invalid user",
++                           __func__);
++                       goto done;
++               }
+                /* XXX fake reply and always send PK_OK ? */
+                /*
+                 * XXX this allows testing whether a user is allowed
+@@ -237,6 +241,7 @@ done:
+        free(pkblob);
+        free(key_s);
+        free(ca_s);
++       free(sig);
+        return authenticated;
+ }
+