2 files changed, 199 insertions, 147 deletions

diff --git a/ChangeLog b/ChangeLog
index 795bae3c4..f02114c01 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -45,6 +45,14 @@
    - markus@cvs.openbsd.org 2003/05/11 20:30:25
      [channels.c clientloop.c serverloop.c session.c ssh.c]
      make channel_new() strdup the 'remote_name' (not the caller); ok theo
+   - markus@cvs.openbsd.org 2003/05/12 16:55:37
+     [sshconnect2.c]
+     for pubkey authentication try the user keys in the following order:
+        1. agent keys that are found in the config file
+        2. other agent keys
+        3. keys that are only listed in the config file
+     this helps when an agent has many keys, where the server might
+     close the connection before the correct key is used. report & ok pb@
 
 20030512
  - (djm) Redhat spec: Don't install profile.d scripts when not 
@@ -1432,4 +1440,4 @@
      save auth method before monitor_reset_key_state(); bugzilla bug #284;
      ok provos@
 
-$Id: ChangeLog,v 1.2688 2003/05/14 03:45:42 djm Exp $
+$Id: ChangeLog,v 1.2689 2003/05/14 03:46:00 djm Exp $
diff --git a/sshconnect2.c b/sshconnect2.c
index d32960447..74d699ff2 100644
--- a/sshconnect2.c
+++ b/sshconnect2.c
@@ -23,7 +23,7 @@
  */
 
 #include "includes.h"
-RCSID("$OpenBSD: sshconnect2.c,v 1.116 2003/04/08 20:21:29 itojun Exp $");
+RCSID("$OpenBSD: sshconnect2.c,v 1.117 2003/05/12 16:55:37 markus Exp $");
 
 #include "ssh.h"
 #include "ssh2.h"
@@ -141,10 +141,18 @@ ssh_kex2(char *host, struct sockaddr *hostaddr)
 
 typedef struct Authctxt Authctxt;
 typedef struct Authmethod Authmethod;
-
-typedef int sign_cb_fn(
-    Authctxt *authctxt, Key *key,
-    u_char **sigp, u_int *lenp, u_char *data, u_int datalen);
+typedef struct identity Identity;
+typedef struct idlist Idlist;
+
+struct identity {
+        TAILQ_ENTRY(identity) next;
+        AuthenticationConnection *ac;   /* set if agent supports key */
+        Key     *key;                   /* public/private key */
+        char    *filename;              /* comment for agent-only keys */
+        int     tried;
+        int     isprivate;              /* key points to the private key */
+};
+TAILQ_HEAD(idlist, identity);
 
 struct Authctxt {
         const char *server_user;
@@ -155,9 +163,7 @@ struct Authctxt {
         int success;
         char *authlist;
         /* pubkey */
-        Key *last_key;
-        sign_cb_fn *last_key_sign;
-        int last_key_hint;
+        Idlist keys;
         AuthenticationConnection *agent;
         /* hostbased */
         Sensitive *sensitive;
@@ -187,8 +193,11 @@ int	userauth_hostbased(Authctxt *);
 
 void    userauth(Authctxt *, char *);
 
-static int sign_and_send_pubkey(Authctxt *, Key *, sign_cb_fn *);
+static int sign_and_send_pubkey(Authctxt *, Identity *);
 static void clear_auth_state(Authctxt *);
+static void pubkey_prepare(Authctxt *);
+static void pubkey_cleanup(Authctxt *);
+static Key *load_identity_file(char *);
 
 static Authmethod *authmethod_get(char *authlist);
 static Authmethod *authmethod_lookup(const char *name);
@@ -251,7 +260,7 @@ ssh_userauth2(const char *local_user, const char *server_user, char *host,
 
         /* setup authentication context */
         memset(&authctxt, 0, sizeof(authctxt));
-        authctxt.agent = ssh_get_authentication_connection();
+        pubkey_prepare(&authctxt);
         authctxt.server_user = server_user;
         authctxt.local_user = local_user;
         authctxt.host = host;
@@ -273,9 +282,7 @@ ssh_userauth2(const char *local_user, const char *server_user, char *host,
         dispatch_set(SSH2_MSG_USERAUTH_BANNER, &input_userauth_banner);
         dispatch_run(DISPATCH_BLOCK, &authctxt.success, &authctxt);     /* loop until success */
 
-        if (authctxt.agent != NULL)
-                ssh_close_authentication_connection(authctxt.agent);
-
+        pubkey_cleanup(&authctxt);
         debug("Authentication succeeded (%s).", authctxt.method->name);
 }
 void
@@ -360,6 +367,7 @@ input_userauth_pk_ok(int type, u_int32_t seq, void *ctxt)
 {
         Authctxt *authctxt = ctxt;
         Key *key = NULL;
+        Identity *id = NULL;
         Buffer b;
         int pktype, sent = 0;
         u_int alen, blen;
@@ -382,40 +390,33 @@ input_userauth_pk_ok(int type, u_int32_t seq, void *ctxt)
         }
         packet_check_eom();
 
-        debug("Server accepts key: pkalg %s blen %u lastkey %p hint %d",
-            pkalg, blen, authctxt->last_key, authctxt->last_key_hint);
+        debug("Server accepts key: pkalg %s blen %u", pkalg, blen);
 
-        do {
-                if (authctxt->last_key == NULL ||
-                    authctxt->last_key_sign == NULL) {
-                        debug("no last key or no sign cb");
-                        break;
-                }
-                if ((pktype = key_type_from_name(pkalg)) == KEY_UNSPEC) {
-                        debug("unknown pkalg %s", pkalg);
-                        break;
-                }
-                if ((key = key_from_blob(pkblob, blen)) == NULL) {
-                        debug("no key from blob. pkalg %s", pkalg);
-                        break;
-                }
-                if (key->type != pktype) {
-                        error("input_userauth_pk_ok: type mismatch "
-                            "for decoded key (received %d, expected %d)",
-                            key->type, pktype);
-                        break;
-                }
-                fp = key_fingerprint(key, SSH_FP_MD5, SSH_FP_HEX);
-                debug2("input_userauth_pk_ok: fp %s", fp);
-                xfree(fp);
-                if (!key_equal(key, authctxt->last_key)) {
-                        debug("key != last_key");
+        if ((pktype = key_type_from_name(pkalg)) == KEY_UNSPEC) {
+                debug("unknown pkalg %s", pkalg);
+                goto done;
+        }
+        if ((key = key_from_blob(pkblob, blen)) == NULL) {
+                debug("no key from blob. pkalg %s", pkalg);
+                goto done;
+        }
+        if (key->type != pktype) {
+                error("input_userauth_pk_ok: type mismatch "
+                    "for decoded key (received %d, expected %d)",
+                    key->type, pktype);
+                goto done;
+        }
+        fp = key_fingerprint(key, SSH_FP_MD5, SSH_FP_HEX);
+        debug2("input_userauth_pk_ok: fp %s", fp);
+        xfree(fp);
+
+        TAILQ_FOREACH(id, &authctxt->keys, next) {
+                if (key_equal(key, id->key)) {
+                        sent = sign_and_send_pubkey(authctxt, id);
                         break;
                 }
-                sent = sign_and_send_pubkey(authctxt, key,
-                   authctxt->last_key_sign);
-        } while (0);
-
+        }
+done:
         if (key != NULL)
                 key_free(key);
         xfree(pkalg);
@@ -428,7 +429,6 @@ input_userauth_pk_ok(int type, u_int32_t seq, void *ctxt)
         /* try another method if we did not send a packet */
         if (sent == 0)
                 userauth(authctxt, NULL);
-
 }
 
 int
@@ -547,18 +547,35 @@ clear_auth_state(Authctxt *authctxt)
 {
         /* XXX clear authentication state */
         dispatch_set(SSH2_MSG_USERAUTH_PASSWD_CHANGEREQ, NULL);
+}
 
-        if (authctxt->last_key != NULL && authctxt->last_key_hint == -1) {
-                debug3("clear_auth_state: key_free %p", authctxt->last_key);
-                key_free(authctxt->last_key);
-        }
-        authctxt->last_key = NULL;
-        authctxt->last_key_hint = -2;
-        authctxt->last_key_sign = NULL;
+static int
+identity_sign(Identity *id, u_char **sigp, u_int *lenp,
+    u_char *data, u_int datalen)
+{
+        Key *prv;
+        int ret;
+
+        /* the agent supports this key */
+        if (id->ac)
+                return (ssh_agent_sign(id->ac, id->key, sigp, lenp,
+                    data, datalen));
+        /*
+         * we have already loaded the private key or
+         * the private key is stored in external hardware
+         */
+        if (id->isprivate || (id->key->flags & KEY_FLAG_EXT))
+                return (key_sign(id->key, sigp, lenp, data, datalen));
+        /* load the private key from the file */
+        if ((prv = load_identity_file(id->filename)) == NULL)
+                return (-1);
+        ret = key_sign(prv, sigp, lenp, data, datalen);
+        key_free(prv);
+        return (ret);
 }
 
 static int
-sign_and_send_pubkey(Authctxt *authctxt, Key *k, sign_cb_fn *sign_callback)
+sign_and_send_pubkey(Authctxt *authctxt, Identity *id)
 {
         Buffer b;
         u_char *blob, *signature;
@@ -569,7 +586,7 @@ sign_and_send_pubkey(Authctxt *authctxt, Key *k, sign_cb_fn *sign_callback)
 
         debug3("sign_and_send_pubkey");
 
-        if (key_to_blob(k, &blob, &bloblen) == 0) {
+        if (key_to_blob(id->key, &blob, &bloblen) == 0) {
                 /* we cannot handle this key */
                 debug3("sign_and_send_pubkey: cannot handle key");
                 return 0;
@@ -594,12 +611,12 @@ sign_and_send_pubkey(Authctxt *authctxt, Key *k, sign_cb_fn *sign_callback)
         } else {
                 buffer_put_cstring(&b, authctxt->method->name);
                 buffer_put_char(&b, have_sig);
-                buffer_put_cstring(&b, key_ssh_name(k));
+                buffer_put_cstring(&b, key_ssh_name(id->key));
         }
         buffer_put_string(&b, blob, bloblen);
 
         /* generate signature */
-        ret = (*sign_callback)(authctxt, k, &signature, &slen,
+        ret = identity_sign(id, &signature, &slen,
             buffer_ptr(&b), buffer_len(&b));
         if (ret == -1) {
                 xfree(blob);
@@ -619,7 +636,7 @@ sign_and_send_pubkey(Authctxt *authctxt, Key *k, sign_cb_fn *sign_callback)
                 buffer_put_cstring(&b, authctxt->method->name);
                 buffer_put_char(&b, have_sig);
                 if (!(datafellows & SSH_BUG_PKAUTH))
-                        buffer_put_cstring(&b, key_ssh_name(k));
+                        buffer_put_cstring(&b, key_ssh_name(id->key));
                 buffer_put_string(&b, blob, bloblen);
         }
         xfree(blob);
@@ -643,23 +660,19 @@ sign_and_send_pubkey(Authctxt *authctxt, Key *k, sign_cb_fn *sign_callback)
 }
 
 static int
-send_pubkey_test(Authctxt *authctxt, Key *k, sign_cb_fn *sign_callback,
-    int hint)
+send_pubkey_test(Authctxt *authctxt, Identity *id)
 {
         u_char *blob;
         u_int bloblen, have_sig = 0;
 
         debug3("send_pubkey_test");
 
-        if (key_to_blob(k, &blob, &bloblen) == 0) {
+        if (key_to_blob(id->key, &blob, &bloblen) == 0) {
                 /* we cannot handle this key */
                 debug3("send_pubkey_test: cannot handle key");
                 return 0;
         }
         /* register callback for USERAUTH_PK_OK message */
-        authctxt->last_key_sign = sign_callback;
-        authctxt->last_key_hint = hint;
-        authctxt->last_key = k;
         dispatch_set(SSH2_MSG_USERAUTH_PK_OK, &input_userauth_pk_ok);
 
         packet_start(SSH2_MSG_USERAUTH_REQUEST);
@@ -668,7 +681,7 @@ send_pubkey_test(Authctxt *authctxt, Key *k, sign_cb_fn *sign_callback,
         packet_put_cstring(authctxt->method->name);
         packet_put_char(have_sig);
         if (!(datafellows & SSH_BUG_PKAUTH))
-                packet_put_cstring(key_ssh_name(k));
+                packet_put_cstring(key_ssh_name(id->key));
         packet_put_string(blob, bloblen);
         xfree(blob);
         packet_send();
@@ -713,103 +726,134 @@ load_identity_file(char *filename)
         return private;
 }
 
-static int
-identity_sign_cb(Authctxt *authctxt, Key *key, u_char **sigp, u_int *lenp,
-    u_char *data, u_int datalen)
-{
-        Key *private;
-        int idx, ret;
-
-        idx = authctxt->last_key_hint;
-        if (idx < 0)
-                return -1;
-
-        /* private key is stored in external hardware */
-        if (options.identity_keys[idx]->flags & KEY_FLAG_EXT)
-                return key_sign(options.identity_keys[idx], sigp, lenp, data, datalen);
-
-        private = load_identity_file(options.identity_files[idx]);
-        if (private == NULL)
-                return -1;
-        ret = key_sign(private, sigp, lenp, data, datalen);
-        key_free(private);
-        return ret;
-}
-
-static int
-agent_sign_cb(Authctxt *authctxt, Key *key, u_char **sigp, u_int *lenp,
-    u_char *data, u_int datalen)
-{
-        return ssh_agent_sign(authctxt->agent, key, sigp, lenp, data, datalen);
-}
-
-static int
-key_sign_cb(Authctxt *authctxt, Key *key, u_char **sigp, u_int *lenp,
-    u_char *data, u_int datalen)
+/*
+ * try keys in the following order:
+ *      1. agent keys that are found in the config file
+ *      2. other agent keys
+ *      3. keys that are only listed in the config file
+ */
+static void
+pubkey_prepare(Authctxt *authctxt)
 {
-        return key_sign(key, sigp, lenp, data, datalen);
+        Identity *id;
+        Idlist agent, files, *preferred;
+        Key *key;
+        AuthenticationConnection *ac;
+        char *comment;
+        int i, found;
+
+        TAILQ_INIT(&agent);     /* keys from the agent */
+        TAILQ_INIT(&files);     /* keys from the config file */
+        preferred = &authctxt->keys;
+        TAILQ_INIT(preferred);  /* preferred order of keys */
+
+        /* list of keys stored in the filesystem */
+        for (i = 0; i < options.num_identity_files; i++) {
+                key = options.identity_keys[i];
+                if (key && key->type == KEY_RSA1)
+                        continue;
+                options.identity_keys[i] = NULL;
+                id = xmalloc(sizeof(*id));
+                memset(id, 0, sizeof(*id));
+                id->key = key;
+                id->filename = xstrdup(options.identity_files[i]);
+                TAILQ_INSERT_TAIL(&files, id, next);
+        }
+        /* list of keys supported by the agent */
+        if ((ac = ssh_get_authentication_connection())) {
+                for (key = ssh_get_first_identity(ac, &comment, 2);
+                    key != NULL;
+                    key = ssh_get_next_identity(ac, &comment, 2)) {
+                        found = 0;
+                        TAILQ_FOREACH(id, &files, next) {
+                                /* agent keys from the config file are preferred */ 
+                                if (key_equal(key, id->key)) {
+                                        key_free(key);
+                                        xfree(comment);
+                                        TAILQ_REMOVE(&files, id, next);
+                                        TAILQ_INSERT_TAIL(preferred, id, next);
+                                        id->ac = ac;
+                                        found = 1;
+                                        break;
+                                }
+                        }
+                        if (!found) {
+                                id = xmalloc(sizeof(*id));
+                                memset(id, 0, sizeof(*id));
+                                id->key = key;
+                                id->filename = comment;
+                                id->ac = ac;
+                                TAILQ_INSERT_TAIL(&agent, id, next);
+                        }
+                }
+                /* append remaining agent keys */
+                for (id = TAILQ_FIRST(&agent); id; id = TAILQ_FIRST(&agent)) {
+                        TAILQ_REMOVE(&agent, id, next);
+                        TAILQ_INSERT_TAIL(preferred, id, next);
+                }
+                authctxt->agent = ac;
+        }
+        /* append remaining keys from the config file */
+        for (id = TAILQ_FIRST(&files); id; id = TAILQ_FIRST(&files)) {
+                TAILQ_REMOVE(&files, id, next);
+                TAILQ_INSERT_TAIL(preferred, id, next);
+        }
+        TAILQ_FOREACH(id, preferred, next) {
+                debug2("key: %s (%p)", id->filename, id->key);
+        }
 }
 
-static int
-userauth_pubkey_agent(Authctxt *authctxt)
+static void
+pubkey_cleanup(Authctxt *authctxt)
 {
-        static int called = 0;
-        int ret = 0;
-        char *comment;
-        Key *k;
-
-        if (called == 0) {
-                if (ssh_get_num_identities(authctxt->agent, 2) == 0)
-                        debug2("userauth_pubkey_agent: no keys at all");
-                called = 1;
+        Identity *id;
+
+        if (authctxt->agent != NULL)
+                ssh_close_authentication_connection(authctxt->agent);
+        for (id = TAILQ_FIRST(&authctxt->keys); id;
+            id = TAILQ_FIRST(&authctxt->keys)) {
+                TAILQ_REMOVE(&authctxt->keys, id, next);
+                if (id->key)
+                        key_free(id->key);
+                if (id->filename)
+                        xfree(id->filename);
+                xfree(id);
         }
-        k = ssh_get_next_identity(authctxt->agent, &comment, 2);
-        if (k == NULL) {
-                debug2("userauth_pubkey_agent: no more keys");
-        } else {
-                debug("Offering agent key: %s", comment);
-                xfree(comment);
-                ret = send_pubkey_test(authctxt, k, agent_sign_cb, -1);
-                if (ret == 0)
-                        key_free(k);
-        }
-        if (ret == 0)
-                debug2("userauth_pubkey_agent: no message sent");
-        return ret;
 }
 
 int
 userauth_pubkey(Authctxt *authctxt)
 {
-        static int idx = 0;
+        Identity *id;
         int sent = 0;
-        Key *key;
-        char *filename;
 
-        if (authctxt->agent != NULL) {
-                do {
-                        sent = userauth_pubkey_agent(authctxt);
-                } while (!sent && authctxt->agent->howmany > 0);
-        }
-        while (!sent && idx < options.num_identity_files) {
-                key = options.identity_keys[idx];
-                filename = options.identity_files[idx];
-                if (key == NULL) {
-                        debug("Trying private key: %s", filename);
-                        key = load_identity_file(filename);
-                        if (key != NULL) {
-                                sent = sign_and_send_pubkey(authctxt, key,
-                                    key_sign_cb);
-                                key_free(key);
+        while ((id = TAILQ_FIRST(&authctxt->keys))) {
+                if (id->tried++)
+                        return (0);
+                TAILQ_REMOVE(&authctxt->keys, id, next);
+                TAILQ_INSERT_TAIL(&authctxt->keys, id, next);
+                /*
+                 * send a test message if we have the public key. for
+                 * encrypted keys we cannot do this and have to load the
+                 * private key instead
+                 */
+                if (id->key && id->key->type != KEY_RSA1) {
+                        debug("Offering public key: %s", id->filename);
+                        sent = send_pubkey_test(authctxt, id);
+                } else if (id->key == NULL) {
+                        debug("Trying private key: %s", id->filename);
+                        id->key = load_identity_file(id->filename);
+                        if (id->key != NULL) {
+                                id->isprivate = 1;
+                                sent = sign_and_send_pubkey(authctxt, id);
+                                key_free(id->key);
+                                id->key = NULL;
                         }
-                } else if (key->type != KEY_RSA1) {
-                        debug("Offering public key: %s", filename);
-                        sent = send_pubkey_test(authctxt, key,
-                            identity_sign_cb, idx);
                 }
-                idx++;
+                if (sent)
+                        return (sent);
         }
-        return sent;
+        return (0);
 }
 
 /*