Re: Public key authentication bugs

[Tilo Eckert <tilo.eckert@xxxxxxx>]

Hi,

as requested, here is a resubmit of my properly signed patches (original
mails quoted below).

Certificate of Origin was sent separately to contributing@xxxxxxxxxx.

Regards
Tilo

Am 11.06.2015 um 16:50 schrieb Tilo Eckert:
> Hi,
> 
> I found two related bugs when trying to authenticate with
> ssh_userauth_publickey_auto() against an OpenSSH server that is
> configured to require public key authentication followed by password
> authentication, i.e. sshd_config contains the line:
> AuthenticationMethods publickey,password
> 
> Bug #1: ssh_userauth_publickey_auto() does not handle the
> SSH_AUTH_PARTIAL return code after the call to ssh_userauth_publickey()
> and therefore behaves like we had sent a bad signature that does not
> match the offered public key. So instead of SSH_AUTH_PARTIAL,
> SSH_AUTH_DENIED is returned.
> 
> Bug #2: Due to Bug #1, session->auth_auto_state was free'd because
> ssh_userauth_publickey() returned SSH_AUTH_PARTIAL, but none of the
> following ifs handled its return code. The following access to the
> 'state' variable is a use-after-free, resulting in memory corruption (in
> my case ssh_disconnect() segfaulted). So, this one is probably security
> related.
> 
> I attached a patch that fixes both bugs.
> 
> Regards,
> Tilo
> 

Am 15.06.2015 um 13:28 schrieb Tilo Eckert:
> Hi,
> 
> when calling ssh_userauth_list() after a successful partial
> authentication (e.g. public key) and another authentication method (e.g.
> password) is required next by the server, only the
> SSH_AUTH_METHOD_PASSWORD flag should be set. However, the
> SSH_AUTH_METHOD_PUBLICKEY flag is also set, even though it is not
> acceptable in the current state.
> 
> The auth_methods field in the session is reset after a failed
> authentication attempt, but not after a partial one. The attached patch
> changes it to be reset in both cases.
> 
> Regards,
> Tilo

From d3e7868f34cffaf39fa6607cfe3c1f72242826cd Mon Sep 17 00:00:00 2001
From: Tilo Eckert <tilo.eckert@xxxxxxx>
Date: Mon, 15 Jun 2015 13:12:23 +0200
Subject: [PATCH] available auth_methods must be reset on partial
 authentication

Signed-off-by: Tilo Eckert <tilo.eckert@xxxxxxx>
---
 src/auth.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/auth.c b/src/auth.c
index da8c4d9..77b99a7 100755
--- a/src/auth.c
+++ b/src/auth.c
@@ -209,8 +209,8 @@ SSH_PACKET_CALLBACK(ssh_packet_userauth_failure){
             "Access denied. Authentication that can continue: %s",
             auth_methods);
 
-    session->auth_methods = 0;
   }
+  session->auth_methods = 0;
   if (strstr(auth_methods, "password") != NULL) {
     session->auth_methods |= SSH_AUTH_METHOD_PASSWORD;
   }
-- 
2.4.3

From ecc579cf777a05807d61da2f959851159a443d8d Mon Sep 17 00:00:00 2001
From: Tilo Eckert <tilo.eckert@xxxxxxx>
Date: Thu, 11 Jun 2015 16:43:27 +0200
Subject: [PATCH] SSH_AUTH_PARTIAL is now correctly passed to the caller of
 ssh_userauth_publickey_auto().

Implicitly fixed unsafe return code handling that could result in use-after-free.

Signed-off-by: Tilo Eckert <tilo.eckert@xxxxxxx>
---
 src/auth.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)
 mode change 100644 => 100755 src/auth.c

diff --git a/src/auth.c b/src/auth.c
old mode 100644
new mode 100755
index 20cac90..da8c4d9
--- a/src/auth.c
+++ b/src/auth.c
@@ -1045,15 +1045,14 @@ int ssh_userauth_publickey_auto(ssh_session session,
                 ssh_key_free(state->privkey);
                 ssh_key_free(state->pubkey);
                 SAFE_FREE(session->auth_auto_state);
-            }
-            if (rc == SSH_AUTH_ERROR) {
-                return rc;
-            } else if (rc == SSH_AUTH_SUCCESS) {
-                SSH_LOG(SSH_LOG_INFO,
-                        "Successfully authenticated using %s",
-                        privkey_file);
+                if (rc == SSH_AUTH_SUCCESS) {
+                    SSH_LOG(SSH_LOG_INFO,
+                            "Successfully authenticated using %s",
+                            privkey_file);
+                }
                 return rc;
-            } else if (rc == SSH_AUTH_AGAIN){
+            }
+            if (rc == SSH_AUTH_AGAIN){
                 return rc;
             }
 
-- 
2.4.3