upstream commit

flense SSHv1 support from ssh-agent, considerably
simplifying it

ok markus

Upstream-ID: 71d772cdcefcb29f76e01252e8361e6fc2dfc365

1 files changed, 90 insertions, 162 deletions

diff --git a/ssh-agent.c b/ssh-agent.c
index cc3bffad..2ef8367b 100644
--- a/ssh-agent.c
+++ b/ssh-agent.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-agent.c,v 1.220 2017/04/30 23:18:44 djm Exp $ */
+/* $OpenBSD: ssh-agent.c,v 1.221 2017/04/30 23:29:10 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -118,13 +118,13 @@ typedef struct identity {
 	u_int confirm;
 } Identity;
 
-typedef struct {
+struct idtable {
 	int nentries;
 	TAILQ_HEAD(idqueue, identity) idlist;
-} Idtab;
+};
 
-/* private key table, one per protocol version */
-Idtab idtable[3];
+/* private key table */
+struct idtable *idtab;
 
 int max_fd = 0;
 
@@ -171,21 +171,9 @@ close_socket(SocketEntry *e)
 static void
 idtab_init(void)
 {
-	int i;
-
-	for (i = 0; i <=2; i++) {
-		TAILQ_INIT(&idtable[i].idlist);
-		idtable[i].nentries = 0;
-	}
-}
-
-/* return private key table for requested protocol version */
-static Idtab *
-idtab_lookup(int version)
-{
-	if (version < 1 || version > 2)
-		fatal("internal error, bad protocol version %d", version);
-	return &idtable[version];
+	idtab = xcalloc(1, sizeof(*idtab));
+	TAILQ_INIT(&idtab->idlist);
+	idtab->nentries = 0;
 }
 
 static void
@@ -199,12 +187,11 @@ free_identity(Identity *id)
 
 /* return matching private key for given public key */
 static Identity *
-lookup_identity(struct sshkey *key, int version)
+lookup_identity(struct sshkey *key)
 {
 	Identity *id;
 
-	Idtab *tab = idtab_lookup(version);
-	TAILQ_FOREACH(id, &tab->idlist, next) {
+	TAILQ_FOREACH(id, &idtab->idlist, next) {
 		if (sshkey_equal(key, id->key))
 			return (id);
 	}
@@ -241,34 +228,24 @@ send_status(SocketEntry *e, int success)
 
 /* send list of supported public keys to 'client' */
 static void
-process_request_identities(SocketEntry *e, int version)
+process_request_identities(SocketEntry *e)
 {
-	Idtab *tab = idtab_lookup(version);
 	Identity *id;
 	struct sshbuf *msg;
 	int r;
-	u_char *blob;
-	size_t blen;
 
 	if ((msg = sshbuf_new()) == NULL)
 		fatal("%s: sshbuf_new failed", __func__);
-	if ((r = sshbuf_put_u8(msg, (version == 1) ?
-	    SSH_AGENT_RSA_IDENTITIES_ANSWER :
-	    SSH2_AGENT_IDENTITIES_ANSWER)) != 0 ||
-	    (r = sshbuf_put_u32(msg, tab->nentries)) != 0)
+	if ((r = sshbuf_put_u8(msg, SSH2_AGENT_IDENTITIES_ANSWER)) != 0 ||
+	    (r = sshbuf_put_u32(msg, idtab->nentries)) != 0)
 		fatal("%s: buffer error: %s", __func__, ssh_err(r));
-	TAILQ_FOREACH(id, &tab->idlist, next) {
-		if ((r = sshkey_to_blob(id->key, &blob, &blen)) != 0) {
-			error("%s: sshkey_to_blob: %s", __func__,
+	TAILQ_FOREACH(id, &idtab->idlist, next) {
+		if ((r = sshkey_puts(id->key, msg)) != 0 ||
+		    (r = sshbuf_put_cstring(msg, id->comment)) != 0) {
+			error("%s: put key/comment: %s", __func__,
 			    ssh_err(r));
 			continue;
 		}
-		if ((r = sshbuf_put_string(msg, blob, blen)) != 0)
-			fatal("%s: buffer error: %s",
-			    __func__, ssh_err(r));
-		free(blob);
-		if ((r = sshbuf_put_cstring(msg, id->comment)) != 0)
-			fatal("%s: buffer error: %s", __func__, ssh_err(r));
 	}
 	if ((r = sshbuf_put_stringb(e->output, msg)) != 0)
 		fatal("%s: buffer error: %s", __func__, ssh_err(r));
@@ -292,27 +269,24 @@ agent_decode_alg(struct sshkey *key, u_int flags)
 static void
 process_sign_request2(SocketEntry *e)
 {
-	u_char *blob, *data, *signature = NULL;
-	size_t blen, dlen, slen = 0;
+	const u_char *data;
+	u_char *signature = NULL;
+	size_t dlen, slen = 0;
 	u_int compat = 0, flags;
 	int r, ok = -1;
 	struct sshbuf *msg;
-	struct sshkey *key;
+	struct sshkey *key = NULL;
 	struct identity *id;
 
 	if ((msg = sshbuf_new()) == NULL)
 		fatal("%s: sshbuf_new failed", __func__);
-	if ((r = sshbuf_get_string(e->request, &blob, &blen)) != 0 ||
-	    (r = sshbuf_get_string(e->request, &data, &dlen)) != 0 ||
+	if ((r = sshkey_froms(e->request, &key)) != 0 ||
+	    (r = sshbuf_get_string_direct(e->request, &data, &dlen)) != 0 ||
 	    (r = sshbuf_get_u32(e->request, &flags)) != 0)
 		fatal("%s: buffer error: %s", __func__, ssh_err(r));
 	if (flags & SSH_AGENT_OLD_SIGNATURE)
 		compat = SSH_BUG_SIGBLOB;
-	if ((r = sshkey_from_blob(blob, blen, &key)) != 0) {
-		error("%s: cannot parse key blob: %s", __func__, ssh_err(r));
-		goto send;
-	}
-	if ((id = lookup_identity(key, 2)) == NULL) {
+	if ((id = lookup_identity(key)) == NULL) {
 		verbose("%s: %s key not found", __func__, sshkey_type(key));
 		goto send;
 	}
@@ -340,70 +314,52 @@ process_sign_request2(SocketEntry *e)
 		fatal("%s: buffer error: %s", __func__, ssh_err(r));
 
 	sshbuf_free(msg);
-	free(data);
-	free(blob);
 	free(signature);
 }
 
 /* shared */
 static void
-process_remove_identity(SocketEntry *e, int version)
+process_remove_identity(SocketEntry *e)
 {
-	size_t blen;
 	int r, success = 0;
 	struct sshkey *key = NULL;
-	u_char *blob;
+	Identity *id;
 
-	switch (version) {
-	case 2:
-		if ((r = sshbuf_get_string(e->request, &blob, &blen)) != 0)
-			fatal("%s: buffer error: %s", __func__, ssh_err(r));
-		if ((r = sshkey_from_blob(blob, blen, &key)) != 0)
-			error("%s: sshkey_from_blob failed: %s",
-			    __func__, ssh_err(r));
-		free(blob);
-		break;
+	if ((r = sshkey_froms(e->request, &key)) != 0) {
+		error("%s: get key: %s", __func__, ssh_err(r));
+		goto done;
 	}
-	if (key != NULL) {
-		Identity *id = lookup_identity(key, version);
-		if (id != NULL) {
-			/*
-			 * We have this key.  Free the old key.  Since we
-			 * don't want to leave empty slots in the middle of
-			 * the array, we actually free the key there and move
-			 * all the entries between the empty slot and the end
-			 * of the array.
-			 */
-			Idtab *tab = idtab_lookup(version);
-			if (tab->nentries < 1)
-				fatal("process_remove_identity: "
-				    "internal error: tab->nentries %d",
-				    tab->nentries);
-			TAILQ_REMOVE(&tab->idlist, id, next);
-			free_identity(id);
-			tab->nentries--;
-			success = 1;
-		}
-		sshkey_free(key);
+	if ((id = lookup_identity(key)) == NULL) {
+		debug("%s: key not found", __func__);
+		goto done;
 	}
+	/* We have this key, free it. */
+	if (idtab->nentries < 1)
+		fatal("%s: internal error: nentries %d",
+		    __func__, idtab->nentries);
+	TAILQ_REMOVE(&idtab->idlist, id, next);
+	free_identity(id);
+	idtab->nentries--;
+	sshkey_free(key);
+	success = 1;
+ done:
 	send_status(e, success);
 }
 
 static void
-process_remove_all_identities(SocketEntry *e, int version)
+process_remove_all_identities(SocketEntry *e)
 {
-	Idtab *tab = idtab_lookup(version);
 	Identity *id;
 
 	/* Loop over all identities and clear the keys. */
-	for (id = TAILQ_FIRST(&tab->idlist); id;
-	    id = TAILQ_FIRST(&tab->idlist)) {
-		TAILQ_REMOVE(&tab->idlist, id, next);
+	for (id = TAILQ_FIRST(&idtab->idlist); id;
+	    id = TAILQ_FIRST(&idtab->idlist)) {
+		TAILQ_REMOVE(&idtab->idlist, id, next);
 		free_identity(id);
 	}
 
 	/* Mark that there are no identities. */
-	tab->nentries = 0;
+	idtab->nentries = 0;
 
 	/* Send success. */
 	send_status(e, 1);
@@ -415,24 +371,19 @@ reaper(void)
 {
 	time_t deadline = 0, now = monotime();
 	Identity *id, *nxt;
-	int version;
-	Idtab *tab;
-
-	for (version = 1; version < 3; version++) {
-		tab = idtab_lookup(version);
-		for (id = TAILQ_FIRST(&tab->idlist); id; id = nxt) {
-			nxt = TAILQ_NEXT(id, next);
-			if (id->death == 0)
-				continue;
-			if (now >= id->death) {
-				debug("expiring key '%s'", id->comment);
-				TAILQ_REMOVE(&tab->idlist, id, next);
-				free_identity(id);
-				tab->nentries--;
-			} else
-				deadline = (deadline == 0) ? id->death :
-				    MINIMUM(deadline, id->death);
-		}
+
+	for (id = TAILQ_FIRST(&idtab->idlist); id; id = nxt) {
+		nxt = TAILQ_NEXT(id, next);
+		if (id->death == 0)
+			continue;
+		if (now >= id->death) {
+			debug("expiring key '%s'", id->comment);
+			TAILQ_REMOVE(&idtab->idlist, id, next);
+			free_identity(id);
+			idtab->nentries--;
+		} else
+			deadline = (deadline == 0) ? id->death :
+			    MINIMUM(deadline, id->death);
 	}
 	if (deadline == 0 || deadline <= now)
 		return 0;
@@ -440,15 +391,9 @@ reaper(void)
 		return (deadline - now);
 }
 
-/*
- * XXX this and the corresponding serialisation function probably belongs
- * in key.c
- */
-
 static void
-process_add_identity(SocketEntry *e, int version)
+process_add_identity(SocketEntry *e)
 {
-	Idtab *tab = idtab_lookup(version);
 	Identity *id;
 	int success = 0, confirm = 0;
 	u_int seconds;
@@ -458,12 +403,8 @@ process_add_identity(SocketEntry *e, int version)
 	u_char ctype;
 	int r = SSH_ERR_INTERNAL_ERROR;
 
-	switch (version) {
-	case 2:
-		r = sshkey_private_deserialize(e->request, &k);
-		break;
-	}
-	if (r != 0 || k == NULL ||
+	if ((r = sshkey_private_deserialize(e->request, &k)) != 0 ||
+	    k == NULL ||
 	    (r = sshbuf_get_cstring(e->request, &comment, NULL)) != 0) {
 		error("%s: decode private key: %s", __func__, ssh_err(r));
 		goto err;
@@ -499,12 +440,12 @@ process_add_identity(SocketEntry *e, int version)
 	success = 1;
 	if (lifetime && !death)
 		death = monotime() + lifetime;
-	if ((id = lookup_identity(k, version)) == NULL) {
+	if ((id = lookup_identity(k)) == NULL) {
 		id = xcalloc(1, sizeof(Identity));
 		id->key = k;
-		TAILQ_INSERT_TAIL(&tab->idlist, id, next);
+		TAILQ_INSERT_TAIL(&idtab->idlist, id, next);
 		/* Increment the number of identities. */
-		tab->nentries++;
+		idtab->nentries++;
 	} else {
 		sshkey_free(k);
 		free(id->comment);
@@ -565,17 +506,14 @@ process_lock_agent(SocketEntry *e, int lock)
 }
 
 static void
-no_identities(SocketEntry *e, u_int type)
+no_identities(SocketEntry *e)
 {
 	struct sshbuf *msg;
 	int r;
 
 	if ((msg = sshbuf_new()) == NULL)
 		fatal("%s: sshbuf_new failed", __func__);
-	if ((r = sshbuf_put_u8(msg,
-	    (type == SSH_AGENTC_REQUEST_RSA_IDENTITIES) ?
-	    SSH_AGENT_RSA_IDENTITIES_ANSWER :
-	    SSH2_AGENT_IDENTITIES_ANSWER)) != 0 ||
+	if ((r = sshbuf_put_u8(msg, SSH2_AGENT_IDENTITIES_ANSWER)) != 0 ||
 	    (r = sshbuf_put_u32(msg, 0)) != 0 ||
 	    (r = sshbuf_put_stringb(e->output, msg)) != 0)
 		fatal("%s: buffer error: %s", __func__, ssh_err(r));
@@ -587,13 +525,12 @@ static void
 process_add_smartcard_key(SocketEntry *e)
 {
 	char *provider = NULL, *pin, canonical_provider[PATH_MAX];
-	int r, i, version, count = 0, success = 0, confirm = 0;
+	int r, i, count = 0, success = 0, confirm = 0;
 	u_int seconds;
 	time_t death = 0;
 	u_char type;
 	struct sshkey **keys = NULL, *k;
 	Identity *id;
-	Idtab *tab;
 
 	if ((r = sshbuf_get_cstring(e->request, &provider, NULL)) != 0 ||
 	    (r = sshbuf_get_cstring(e->request, &pin, NULL)) != 0)
@@ -613,8 +550,7 @@ process_add_smartcard_key(SocketEntry *e)
 			confirm = 1;
 			break;
 		default:
-			error("process_add_smartcard_key: "
-			    "Unknown constraint type %d", type);
+			error("%s: Unknown constraint type %d", __func__, type);
 			goto send;
 		}
 	}
@@ -635,17 +571,15 @@ process_add_smartcard_key(SocketEntry *e)
 	count = pkcs11_add_provider(canonical_provider, pin, &keys);
 	for (i = 0; i < count; i++) {
 		k = keys[i];
-		version = 2;
-		tab = idtab_lookup(version);
-		if (lookup_identity(k, version) == NULL) {
+		if (lookup_identity(k) == NULL) {
 			id = xcalloc(1, sizeof(Identity));
 			id->key = k;
 			id->provider = xstrdup(canonical_provider);
 			id->comment = xstrdup(canonical_provider); /* XXX */
 			id->death = death;
 			id->confirm = confirm;
-			TAILQ_INSERT_TAIL(&tab->idlist, id, next);
-			tab->nentries++;
+			TAILQ_INSERT_TAIL(&idtab->idlist, id, next);
+			idtab->nentries++;
 			success = 1;
 		} else {
 			sshkey_free(k);
@@ -663,9 +597,8 @@ static void
 process_remove_smartcard_key(SocketEntry *e)
 {
 	char *provider = NULL, *pin = NULL, canonical_provider[PATH_MAX];
-	int r, version, success = 0;
+	int r, success = 0;
 	Identity *id, *nxt;
-	Idtab *tab;
 
 	if ((r = sshbuf_get_cstring(e->request, &provider, NULL)) != 0 ||
 	    (r = sshbuf_get_cstring(e->request, &pin, NULL)) != 0)
@@ -679,25 +612,21 @@ process_remove_smartcard_key(SocketEntry *e)
 	}
 
 	debug("%s: remove %.100s", __func__, canonical_provider);
-	for (version = 1; version < 3; version++) {
-		tab = idtab_lookup(version);
-		for (id = TAILQ_FIRST(&tab->idlist); id; id = nxt) {
-			nxt = TAILQ_NEXT(id, next);
-			/* Skip file--based keys */
-			if (id->provider == NULL)
-				continue;
-			if (!strcmp(canonical_provider, id->provider)) {
-				TAILQ_REMOVE(&tab->idlist, id, next);
-				free_identity(id);
-				tab->nentries--;
-			}
+	for (id = TAILQ_FIRST(&idtab->idlist); id; id = nxt) {
+		nxt = TAILQ_NEXT(id, next);
+		/* Skip file--based keys */
+		if (id->provider == NULL)
+			continue;
+		if (!strcmp(canonical_provider, id->provider)) {
+			TAILQ_REMOVE(&idtab->idlist, id, next);
+			free_identity(id);
+			idtab->nentries--;
 		}
 	}
 	if (pkcs11_del_provider(canonical_provider) == 0)
 		success = 1;
 	else
-		error("process_remove_smartcard_key:"
-		    " pkcs11_del_provider failed");
+		error("%s: pkcs11_del_provider failed", __func__);
 send:
 	free(provider);
 	send_status(e, success);
@@ -735,10 +664,9 @@ process_message(SocketEntry *e)
 	if (locked && type != SSH_AGENTC_UNLOCK) {
 		sshbuf_reset(e->request);
 		switch (type) {
-		case SSH_AGENTC_REQUEST_RSA_IDENTITIES:
 		case SSH2_AGENTC_REQUEST_IDENTITIES:
 			/* send empty lists */
-			no_identities(e, type);
+			no_identities(e);
 			break;
 		default:
 			/* send a fail message for all other request types */
@@ -754,24 +682,24 @@ process_message(SocketEntry *e)
 		process_lock_agent(e, type == SSH_AGENTC_LOCK);
 		break;
 	case SSH_AGENTC_REMOVE_ALL_RSA_IDENTITIES:
-		process_remove_all_identities(e, 1); /* safe for !WITH_SSH1 */
+		process_remove_all_identities(e); /* safe for !WITH_SSH1 */
 		break;
 	/* ssh2 */
 	case SSH2_AGENTC_SIGN_REQUEST:
 		process_sign_request2(e);
 		break;
 	case SSH2_AGENTC_REQUEST_IDENTITIES:
-		process_request_identities(e, 2);
+		process_request_identities(e);
 		break;
 	case SSH2_AGENTC_ADD_IDENTITY:
 	case SSH2_AGENTC_ADD_ID_CONSTRAINED:
-		process_add_identity(e, 2);
+		process_add_identity(e);
 		break;
 	case SSH2_AGENTC_REMOVE_IDENTITY:
-		process_remove_identity(e, 2);
+		process_remove_identity(e);
 		break;
 	case SSH2_AGENTC_REMOVE_ALL_IDENTITIES:
-		process_remove_all_identities(e, 2);
+		process_remove_all_identities(e);
 		break;
 #ifdef ENABLE_PKCS11
 	case SSH_AGENTC_ADD_SMARTCARD_KEY: