openssh unix dev - Dec 2015 - Portalbe OpenSSH's don't know agent's keys after authenticion failure with partial success

Hi,

If SSH_MSG_USERAUTH_FAILURE arrives from the server with partial success
(set to 1), in input_userauth_failure() the pubkey_cleanup() and
pubkey_prepare() calls does different thing than the first pubkey_prepare()
instead of identical.

OpenSSH versions: 6.7p1 to 7.1p1 (based on changelog the issue seems to be
introduced in 6.3p1 during fixing another bug):

* ssh(1): reset the order in which public keys are tried after partial
   authentication success.

Environment:
* an ssh-agent with a passphrase-protected private key stored in its
standard name (e.g. .ssh/id_rsa)
* a server that accepts this key but requires further authentication (auth
failure with partial success)
* and the ssh client

First the client sends the key stored in the agent, and then clears its
keys in authctx, and retries the keys. At this point the public key is
missing (.ssh/id_rsa), therefore ssh tries to ask the passphrase, even if
the key is loaded into the agent. And if the passphrase is given, it
retries the very same key.

The problem seems to be occured in pubkey_prepare(), in this line:

    options.identity_keys[i] = NULL;

If the code wants to iterate thrugh these keys after a partial success,
these keys should never
 be NULL'ed, instead these should be copied.

As a result, if I'm not mistaken, the ssh client skips all keys of the
agent.

Regrads,
Laszlo Attila TOTH

Hi,

I created a patch on the top of git repository git://
anongit.mindrot.org/openssh.git
commit 292a8dee14e5e67dcd1b49ba5c7b9023e8420d59 djm at mindrot.org upstream
commit

which fixes the issue, and OpenSSH client seems to be working fine in this
case, too. The patch is attached.


Regards,
Laszlo Attila Toth



2015-12-18 15:12 GMT+01:00 T?th, L?szl? Attila <
laszlo.attila.toth at balabit.com>:
> Hi,
>
> If SSH_MSG_USERAUTH_FAILURE arrives from the server with partial success
> (set to 1), in input_userauth_failure() the pubkey_cleanup() and
> pubkey_prepare() calls does different thing than the first pubkey_prepare()
> instead of identical.
>
> OpenSSH versions: 6.7p1 to 7.1p1 (based on changelog the issue seems to be
> introduced in 6.3p1 during fixing another bug):
>
> * ssh(1): reset the order in which public keys are tried after partial
>    authentication success.
>
> Environment:
> * an ssh-agent with a passphrase-protected private key stored in its
> standard name (e.g. .ssh/id_rsa)
> * a server that accepts this key but requires further authentication (auth
> failure with partial success)
> * and the ssh client
>
> First the client sends the key stored in the agent, and then clears its
> keys in authctx, and retries the keys. At this point the public key is
> missing (.ssh/id_rsa), therefore ssh tries to ask the passphrase, even if
> the key is loaded into the agent. And if the passphrase is given, it
> retries the very same key.
>
> The problem seems to be occured in pubkey_prepare(), in this line:
>
>     options.identity_keys[i] = NULL;
>
> If the code wants to iterate thrugh these keys after a partial success,
> these keys should never
>  be NULL'ed, instead these should be copied.
>
> As a result, if I'm not mistaken, the ssh client skips all keys of the
> agent.
>
> Regrads,
> Laszlo Attila TOTH
>
>
>
>
>
>
>
>
>
-------------- next part --------------
diff --git a/ssh.c b/ssh.c
index f9ff91f..423650b 100644
--- a/ssh.c
+++ b/ssh.c
@@ -212,7 +212,7 @@ usage(void)

 static int ssh_session(void);
 static int ssh_session2(void);
-static void load_public_identity_files(void);
+void load_public_identity_files(void);
 static void main_sigchld_handler(int);

 /* from muxclient.c */
@@ -1954,7 +1954,7 @@ ssh_session2(void)
 }

 /* Loads all IdentityFile and CertificateFile keys */
-static void
+void
 load_public_identity_files(void)
 {
 	char *filename, *cp, thishost[NI_MAXHOST];
diff --git a/sshconnect2.c b/sshconnect2.c
index 8836581..acda1f2 100644
--- a/sshconnect2.c
+++ b/sshconnect2.c
@@ -81,6 +81,8 @@ extern char *client_version_string;
 extern char *server_version_string;
 extern Options options;

+extern void load_public_identity_files(void);
+
 /*
  * SSH2 key exchange
  */
@@ -317,6 +319,7 @@ void	userauth(Authctxt *, char *);

 static int sign_and_send_pubkey(Authctxt *, Identity *);
 static void pubkey_prepare(Authctxt *);
+static void pubkey_reset(Authctxt *);
 static void pubkey_cleanup(Authctxt *);
 static Key *load_identity_file(Identity *);

@@ -563,9 +566,7 @@ input_userauth_failure(int type, u_int32_t seq, void *ctxt)

 	if (partial != 0) {
 		logit("Authenticated with partial success.");
-		/* reset state */
-		pubkey_cleanup(authctxt);
-		pubkey_prepare(authctxt);
+		pubkey_reset(authctxt);
 	}
 	debug("Authentications that can continue: %s", authlist);

@@ -1405,6 +1407,15 @@ pubkey_prepare(Authctxt *authctxt)
 	}
 }

+/* reset state */
+static void
+pubkey_reset(Authctxt *authctxt)
+{
+	pubkey_cleanup(authctxt);
+	load_public_identity_files();
+	pubkey_prepare(authctxt);
+}
+
 static void
 pubkey_cleanup(Authctxt *authctxt)
 {

On Thu, 18 Feb 2016, T?th, L?szl? Attila wrote:
> Hi,
> 
> I created a patch on the top of git repository git://
> anongit.mindrot.org/openssh.git
> commit 292a8dee14e5e67dcd1b49ba5c7b9023e8420d59 djm at mindrot.org upstream
> commit
> 
> which fixes the issue, and OpenSSH client seems to be working fine in this
> case, too. The patch is attached.
I'm not exactly clear on what the problem is here - authentication using
multiple public keys is tested in the regress/multipubkey.sh test at the
moment. Is there a case that is missing?

-d