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 cc3bffad8..2ef8367b9 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: