- 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@

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 $

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 struct identity Identity;
+typedef struct idlist Idlist;
 
-typedef int sign_cb_fn(
+struct identity {
-    Authctxt *authctxt, Key *key,
+	TAILQ_ENTRY(identity) next;
-    u_char **sigp, u_int *lenp, u_char *data, u_int datalen);
+	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;
+	Idlist keys;
-	sign_cb_fn *last_key_sign;
-	int last_key_hint;
 	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)
+	pubkey_cleanup(&authctxt);
-		ssh_close_authentication_connection(authctxt.agent);
-
 	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",
+	debug("Server accepts key: pkalg %s blen %u", pkalg, blen);
-	    pkalg, blen, authctxt->last_key, authctxt->last_key_hint);
 
-	do {
+	if ((pktype = key_type_from_name(pkalg)) == KEY_UNSPEC) {
-		if (authctxt->last_key == NULL ||
+		debug("unknown pkalg %s", pkalg);
-		    authctxt->last_key_sign == NULL) {
+		goto done;
-			debug("no last key or no sign cb");
+	}
-			break;
+	if ((key = key_from_blob(pkblob, blen)) == NULL) {
-		}
+		debug("no key from blob. pkalg %s", pkalg);
-		if ((pktype = key_type_from_name(pkalg)) == KEY_UNSPEC) {
+		goto done;
-			debug("unknown pkalg %s", pkalg);
+	}
-			break;
+	if (key->type != pktype) {
-		}
+		error("input_userauth_pk_ok: type mismatch "
-		if ((key = key_from_blob(pkblob, blen)) == NULL) {
+		    "for decoded key (received %d, expected %d)",
-			debug("no key from blob. pkalg %s", pkalg);
+		    key->type, pktype);
-			break;
+		goto done;
-		}
+	}
-		if (key->type != pktype) {
+	fp = key_fingerprint(key, SSH_FP_MD5, SSH_FP_HEX);
-			error("input_userauth_pk_ok: type mismatch "
+	debug2("input_userauth_pk_ok: fp %s", fp);
-			    "for decoded key (received %d, expected %d)",
+	xfree(fp);
-			    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);
 
+	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 @@ 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
-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 @@ 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,
+send_pubkey_test(Authctxt *authctxt, Identity *id)
-    int hint)
 {
 	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,
+ * try keys in the following order:
-    u_char *data, u_int datalen)
+ *	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;
+	Identity *id;
-	int idx, ret;
+	Idlist agent, files, *preferred;
-
+	Key *key;
-	idx = authctxt->last_key_hint;
+	AuthenticationConnection *ac;
-	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;
 	char *comment;
-	Key *k;
+	int i, found;
 
-	if (called == 0) {
+	TAILQ_INIT(&agent);	/* keys from the agent */
-		if (ssh_get_num_identities(authctxt->agent, 2) == 0)
+	TAILQ_INIT(&files);	/* keys from the config file */
-			debug2("userauth_pubkey_agent: no keys at all");
+	preferred = &authctxt->keys;
-		called = 1;
+	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);
+	/* list of keys supported by the agent */
-	if (k == NULL) {
+	if ((ac = ssh_get_authentication_connection())) {
-		debug2("userauth_pubkey_agent: no more keys");
+		for (key = ssh_get_first_identity(ac, &comment, 2);
-	} else {
+		    key != NULL;
-		debug("Offering agent key: %s", comment);
+		    key = ssh_get_next_identity(ac, &comment, 2)) {
-		xfree(comment);
+			found = 0;
-		ret = send_pubkey_test(authctxt, k, agent_sign_cb, -1);
+			TAILQ_FOREACH(id, &files, next) {
-		if (ret == 0)
+				/* agent keys from the config file are preferred */ 
-			key_free(k);
+				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 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);
 	}
-	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) {
+	while ((id = TAILQ_FIRST(&authctxt->keys))) {
-		do {
+		if (id->tried++)
-			sent = userauth_pubkey_agent(authctxt);
+			return (0);
-		} while (!sent && authctxt->agent->howmany > 0);
+		TAILQ_REMOVE(&authctxt->keys, id, next);
-	}
+		TAILQ_INSERT_TAIL(&authctxt->keys, id, next);
-	while (!sent && idx < options.num_identity_files) {
+		/*
-		key = options.identity_keys[idx];
+		 * send a test message if we have the public key. for
-		filename = options.identity_files[idx];
+		 * encrypted keys we cannot do this and have to load the
-		if (key == NULL) {
+		 * private key instead
-			debug("Trying private key: %s", filename);
+		 */
-			key = load_identity_file(filename);
+		if (id->key && id->key->type != KEY_RSA1) {
-			if (key != NULL) {
+			debug("Offering public key: %s", id->filename);
-				sent = sign_and_send_pubkey(authctxt, key,
+			sent = send_pubkey_test(authctxt, id);
-				    key_sign_cb);
+		} else if (id->key == NULL) {
-				key_free(key);
+			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);
 }
 
 /*