flense SSHv1 support from ssh-agent, considerably simplifying it
authordjm <djm@openbsd.org>
Sun, 30 Apr 2017 23:29:10 +0000 (23:29 +0000)
committerdjm <djm@openbsd.org>
Sun, 30 Apr 2017 23:29:10 +0000 (23:29 +0000)
ok markus

usr.bin/ssh/ssh-agent.c

index ae94880..bb6e325 100644 (file)
@@ -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
@@ -105,13 +105,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;
 
@@ -158,21 +158,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
@@ -186,12 +174,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);
        }
@@ -228,34 +215,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));
@@ -279,27 +256,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;
        }
@@ -327,70 +301,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);
@@ -402,24 +358,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;
@@ -427,15 +378,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;
@@ -445,12 +390,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;
@@ -486,12 +427,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);
@@ -552,17 +493,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));
@@ -574,13 +512,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)
@@ -600,8 +537,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;
                }
        }
@@ -622,17 +558,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);
@@ -650,9 +584,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)
@@ -666,25 +599,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);
@@ -722,10 +651,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 */
@@ -741,24 +669,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: