- 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@
diff --git a/ChangeLog b/ChangeLog
index 795bae3..f02114c 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 d329604..74d699f 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 @@
 
 typedef struct Authctxt Authctxt;
 typedef struct Authmethod Authmethod;
+typedef struct identity Identity;
+typedef struct idlist Idlist;
 
-typedef int sign_cb_fn(
-    Authctxt *authctxt, Key *key,
-    u_char **sigp, u_int *lenp, u_char *data, u_int datalen);
+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 @@
 	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 @@
 
 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 @@
 
 	/* 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 @@
 	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 @@
 {
 	Authctxt *authctxt = ctxt;
 	Key *key = NULL;
+	Identity *id = NULL;
 	Buffer b;
 	int pktype, sent = 0;
 	u_int alen, blen;
@@ -382,40 +390,33 @@
 	}
 	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");
-			break;
-		}
-		sent = sign_and_send_pubkey(authctxt, key,
-		   authctxt->last_key_sign);
-	} while (0);
+	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;
+		}
+	}
+done:
 	if (key != NULL)
 		key_free(key);
 	xfree(pkalg);
@@ -428,7 +429,6 @@
 	/* try another method if we did not send a packet */
 	if (sent == 0)
 		userauth(authctxt, NULL);
-
 }
 
 int
@@ -547,18 +547,35 @@
 {
 	/* 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
-sign_and_send_pubkey(Authctxt *authctxt, Key *k, sign_cb_fn *sign_callback)
+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, Identity *id)
 {
 	Buffer b;
 	u_char *blob, *signature;
@@ -569,7 +586,7 @@
 
 	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 @@
 	} 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 @@
 		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 @@
 }
 
 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 @@
 	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 @@
 	return private;
 }
 
-static int
-identity_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)
 {
-	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)
-{
-	return key_sign(key, sigp, lenp, data, datalen);
-}
-
-static int
-userauth_pubkey_agent(Authctxt *authctxt)
-{
-	static int called = 0;
-	int ret = 0;
+	Identity *id;
+	Idlist agent, files, *preferred;
+	Key *key;
+	AuthenticationConnection *ac;
 	char *comment;
-	Key *k;
+	int i, found;
 
-	if (called == 0) {
-		if (ssh_get_num_identities(authctxt->agent, 2) == 0)
-			debug2("userauth_pubkey_agent: no keys at all");
-		called = 1;
+	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);
 	}
-	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);
+	/* 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;
 	}
-	if (ret == 0)
-		debug2("userauth_pubkey_agent: no message sent");
-	return ret;
+	/* 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 void
+pubkey_cleanup(Authctxt *authctxt)
+{
+	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);
+	}
 }
 
 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);
 }
 
 /*