diff options
author | Damien Miller <djm@cvs.openbsd.org> | 2021-10-28 02:54:19 +0000 |
---|---|---|
committer | Damien Miller <djm@cvs.openbsd.org> | 2021-10-28 02:54:19 +0000 |
commit | 420c94d7f6fbcb6c49b835beca8ead9cbd3b8d5c (patch) | |
tree | c710d95d7185badf90baae9336c8675e6b2d7401 /usr.bin/ssh | |
parent | 8f81ef6a2593393370afb38500867aa786a77873 (diff) |
When downloading resident keys from a FIDO token, pass back the
user ID that was used when the key was created and append it to
the filename the key is written to (if it is not the default).
Avoids keys being clobbered if the user created multiple
resident keys with the same application string but different
user IDs.
feedback Pedro Martelletto; ok markus
NB. increments SSH_SK_VERSION_MAJOR
Diffstat (limited to 'usr.bin/ssh')
-rw-r--r-- | usr.bin/ssh/sk-api.h | 6 | ||||
-rw-r--r-- | usr.bin/ssh/sk-usbhid.c | 35 | ||||
-rw-r--r-- | usr.bin/ssh/ssh-add.c | 28 | ||||
-rw-r--r-- | usr.bin/ssh/ssh-keygen.c | 93 | ||||
-rw-r--r-- | usr.bin/ssh/ssh-sk-client.c | 88 | ||||
-rw-r--r-- | usr.bin/ssh/ssh-sk-helper.c | 33 | ||||
-rw-r--r-- | usr.bin/ssh/ssh-sk.c | 90 | ||||
-rw-r--r-- | usr.bin/ssh/ssh-sk.h | 14 |
8 files changed, 261 insertions, 126 deletions
diff --git a/usr.bin/ssh/sk-api.h b/usr.bin/ssh/sk-api.h index 6a8b03a6ac3..556964b5bdd 100644 --- a/usr.bin/ssh/sk-api.h +++ b/usr.bin/ssh/sk-api.h @@ -1,4 +1,4 @@ -/* $OpenBSD: sk-api.h,v 1.12 2021/02/18 02:15:07 djm Exp $ */ +/* $OpenBSD: sk-api.h,v 1.13 2021/10/28 02:54:18 djm Exp $ */ /* * Copyright (c) 2019 Google LLC * @@ -64,6 +64,8 @@ struct sk_resident_key { char *application; struct sk_enroll_response key; uint8_t flags; + uint8_t *user_id; + size_t user_id_len; }; struct sk_option { @@ -72,7 +74,7 @@ struct sk_option { uint8_t required; }; -#define SSH_SK_VERSION_MAJOR 0x00070000 /* current API version */ +#define SSH_SK_VERSION_MAJOR 0x00080000 /* current API version */ #define SSH_SK_VERSION_MAJOR_MASK 0xffff0000 /* Return the version of the middleware API */ diff --git a/usr.bin/ssh/sk-usbhid.c b/usr.bin/ssh/sk-usbhid.c index 440d4bc5339..c2358075600 100644 --- a/usr.bin/ssh/sk-usbhid.c +++ b/usr.bin/ssh/sk-usbhid.c @@ -1,4 +1,4 @@ -/* $OpenBSD: sk-usbhid.c,v 1.31 2021/10/01 04:50:36 djm Exp $ */ +/* $OpenBSD: sk-usbhid.c,v 1.32 2021/10/28 02:54:18 djm Exp $ */ /* * Copyright (c) 2019 Markus Friedl * Copyright (c) 2020 Pedro Martelletto @@ -1038,9 +1038,11 @@ read_rks(struct sk_usbhid *sk, const char *pin, fido_credman_metadata_t *metadata = NULL; fido_credman_rp_t *rp = NULL; fido_credman_rk_t *rk = NULL; - size_t i, j, nrp, nrk; + size_t i, j, nrp, nrk, user_id_len; const fido_cred_t *cred; + const char *rp_id, *rp_name, *user_name; struct sk_resident_key *srk = NULL, **tmp; + const u_char *user_id; if (pin == NULL) { skdebug(__func__, "no PIN specified"); @@ -1082,12 +1084,16 @@ read_rks(struct sk_usbhid *sk, const char *pin, /* Iterate over RP IDs that have resident keys */ for (i = 0; i < nrp; i++) { + rp_id = fido_credman_rp_id(rp, i); + rp_name = fido_credman_rp_name(rp, i); skdebug(__func__, "rp %zu: name=\"%s\" id=\"%s\" hashlen=%zu", - i, fido_credman_rp_name(rp, i), fido_credman_rp_id(rp, i), + i, rp_name == NULL ? "(none)" : rp_name, + rp_id == NULL ? "(none)" : rp_id, fido_credman_rp_id_hash_len(rp, i)); /* Skip non-SSH RP IDs */ - if (strncasecmp(fido_credman_rp_id(rp, i), "ssh:", 4) != 0) + if (rp_id == NULL || + strncasecmp(fido_credman_rp_id(rp, i), "ssh:", 4) != 0) continue; fido_credman_rk_free(&rk); @@ -1111,17 +1117,23 @@ read_rks(struct sk_usbhid *sk, const char *pin, skdebug(__func__, "no RK in slot %zu", j); continue; } - skdebug(__func__, "Device %s RP \"%s\" slot %zu: " - "type %d flags 0x%02x prot 0x%02x", sk->path, - fido_credman_rp_id(rp, i), j, fido_cred_type(cred), + if ((user_name = fido_cred_user_name(cred)) == NULL) + user_name = ""; + user_id = fido_cred_user_id_ptr(cred); + user_id_len = fido_cred_user_id_len(cred); + skdebug(__func__, "Device %s RP \"%s\" user \"%s\" " + "uidlen %zu slot %zu: type %d flags 0x%02x " + "prot 0x%02x", sk->path, rp_id, user_name, + user_id_len, j, fido_cred_type(cred), fido_cred_flags(cred), fido_cred_prot(cred)); /* build response entry */ if ((srk = calloc(1, sizeof(*srk))) == NULL || (srk->key.key_handle = calloc(1, fido_cred_id_len(cred))) == NULL || - (srk->application = strdup(fido_credman_rp_id(rp, - i))) == NULL) { + (srk->application = strdup(rp_id)) == NULL || + (user_id_len > 0 && + (srk->user_id = calloc(1, user_id_len)) == NULL)) { skdebug(__func__, "alloc sk_resident_key"); goto out; } @@ -1129,6 +1141,9 @@ read_rks(struct sk_usbhid *sk, const char *pin, srk->key.key_handle_len = fido_cred_id_len(cred); memcpy(srk->key.key_handle, fido_cred_id_ptr(cred), srk->key.key_handle_len); + srk->user_id_len = user_id_len; + if (srk->user_id_len != 0) + memcpy(srk->user_id, user_id, srk->user_id_len); switch (fido_cred_type(cred)) { case COSE_ES256: @@ -1169,6 +1184,7 @@ read_rks(struct sk_usbhid *sk, const char *pin, free(srk->application); freezero(srk->key.public_key, srk->key.public_key_len); freezero(srk->key.key_handle, srk->key.key_handle_len); + freezero(srk->user_id, srk->user_id_len); freezero(srk, sizeof(*srk)); } fido_credman_rp_free(&rp); @@ -1221,6 +1237,7 @@ sk_load_resident_keys(const char *pin, struct sk_option **options, free(rks[i]->application); freezero(rks[i]->key.public_key, rks[i]->key.public_key_len); freezero(rks[i]->key.key_handle, rks[i]->key.key_handle_len); + freezero(rks[i]->user_id, rks[i]->user_id_len); freezero(rks[i], sizeof(*rks[i])); } free(rks); diff --git a/usr.bin/ssh/ssh-add.c b/usr.bin/ssh/ssh-add.c index bca50bb05a0..116b5a87cdc 100644 --- a/usr.bin/ssh/ssh-add.c +++ b/usr.bin/ssh/ssh-add.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssh-add.c,v 1.160 2021/04/03 06:18:41 djm Exp $ */ +/* $OpenBSD: ssh-add.c,v 1.161 2021/10/28 02:54:18 djm Exp $ */ /* * Author: Tatu Ylonen <ylo@cs.hut.fi> * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland @@ -573,25 +573,26 @@ lock_agent(int agent_fd, int lock) static int load_resident_keys(int agent_fd, const char *skprovider, int qflag) { - struct sshkey **keys; - size_t nkeys, i; + struct sshsk_resident_key **srks; + size_t nsrks, i; + struct sshkey *key; int r, ok = 0; char *fp; pass = read_passphrase("Enter PIN for authenticator: ", RP_ALLOW_STDIN); - if ((r = sshsk_load_resident(skprovider, NULL, pass, - &keys, &nkeys)) != 0) { + if ((r = sshsk_load_resident(skprovider, NULL, pass, 0, + &srks, &nsrks)) != 0) { error_r(r, "Unable to load resident keys"); return r; } - for (i = 0; i < nkeys; i++) { - if ((fp = sshkey_fingerprint(keys[i], + for (i = 0; i < nsrks; i++) { + key = srks[i]->key; + if ((fp = sshkey_fingerprint(key, fingerprint_hash, SSH_FP_DEFAULT)) == NULL) fatal_f("sshkey_fingerprint failed"); - if ((r = ssh_add_identity_constrained(agent_fd, keys[i], "", + if ((r = ssh_add_identity_constrained(agent_fd, key, "", lifetime, confirm, maxsign, skprovider)) != 0) { - error("Unable to add key %s %s", - sshkey_type(keys[i]), fp); + error("Unable to add key %s %s", sshkey_type(key), fp); free(fp); ok = r; continue; @@ -600,7 +601,7 @@ load_resident_keys(int agent_fd, const char *skprovider, int qflag) ok = 1; if (!qflag) { fprintf(stderr, "Resident identity added: %s %s\n", - sshkey_type(keys[i]), fp); + sshkey_type(key), fp); if (lifetime != 0) { fprintf(stderr, "Lifetime set to %d seconds\n", lifetime); @@ -611,10 +612,9 @@ load_resident_keys(int agent_fd, const char *skprovider, int qflag) } } free(fp); - sshkey_free(keys[i]); } - free(keys); - if (nkeys == 0) + sshsk_free_resident_keys(srks, nsrks); + if (nsrks == 0) return SSH_ERR_KEY_NOT_FOUND; return ok == 1 ? 0 : ok; } diff --git a/usr.bin/ssh/ssh-keygen.c b/usr.bin/ssh/ssh-keygen.c index 7745addaee0..d7dcf858451 100644 --- a/usr.bin/ssh/ssh-keygen.c +++ b/usr.bin/ssh/ssh-keygen.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssh-keygen.c,v 1.438 2021/10/02 03:17:01 dtucker Exp $ */ +/* $OpenBSD: ssh-keygen.c,v 1.439 2021/10/28 02:54:18 djm Exp $ */ /* * Author: Tatu Ylonen <ylo@cs.hut.fi> * Copyright (c) 1994 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland @@ -2973,24 +2973,52 @@ passphrase_again: return passphrase1; } -static const char * -skip_ssh_url_preamble(const char *s) +static char * +sk_suffix(const char *application, const uint8_t *user, size_t userlen) { - if (strncmp(s, "ssh://", 6) == 0) - return s + 6; - else if (strncmp(s, "ssh:", 4) == 0) - return s + 4; - return s; + char *ret, *cp; + size_t slen, i; + + /* Trim off URL-like preamble */ + if (strncmp(application, "ssh://", 6) == 0) + ret = xstrdup(application + 6); + else if (strncmp(application, "ssh:", 4) == 0) + ret = xstrdup(application + 4); + else + ret = xstrdup(application); + + /* Count trailing zeros in user */ + for (i = 0; i < userlen; i++) { + if (user[userlen - i - 1] != 0) + break; + } + if (i >= userlen) + return ret; /* user-id was default all-zeros */ + + /* Append user-id, escaping non-UTF-8 characters */ + slen = userlen - i; + if (asmprintf(&cp, INT_MAX, NULL, "%.*s", (int)slen, user) == -1) + fatal_f("asmprintf failed"); + /* Don't emit a user-id that contains path or control characters */ + if (strchr(cp, '/') != NULL || strstr(cp, "..") != NULL || + strchr(cp, '\\') != NULL) { + free(cp); + cp = tohex(user, slen); + } + xextendf(&ret, "_", "%s", cp); + free(cp); + return ret; } static int do_download_sk(const char *skprovider, const char *device) { - struct sshkey **keys; - size_t nkeys, i; + struct sshsk_resident_key **srks; + size_t nsrks, i; int r, ret = -1; char *fp, *pin = NULL, *pass = NULL, *path, *pubpath; const char *ext; + struct sshkey *key; if (skprovider == NULL) fatal("Cannot download keys without provider"); @@ -3000,34 +3028,34 @@ do_download_sk(const char *skprovider, const char *device) printf("You may need to touch your authenticator " "to authorize key download.\n"); } - if ((r = sshsk_load_resident(skprovider, device, pin, - &keys, &nkeys)) != 0) { + if ((r = sshsk_load_resident(skprovider, device, pin, 0, + &srks, &nsrks)) != 0) { if (pin != NULL) freezero(pin, strlen(pin)); error_r(r, "Unable to load resident keys"); return -1; } - if (nkeys == 0) + if (nsrks == 0) logit("No keys to download"); if (pin != NULL) freezero(pin, strlen(pin)); - for (i = 0; i < nkeys; i++) { - if (keys[i]->type != KEY_ECDSA_SK && - keys[i]->type != KEY_ED25519_SK) { + for (i = 0; i < nsrks; i++) { + key = srks[i]->key; + if (key->type != KEY_ECDSA_SK && key->type != KEY_ED25519_SK) { error("Unsupported key type %s (%d)", - sshkey_type(keys[i]), keys[i]->type); + sshkey_type(key), key->type); continue; } - if ((fp = sshkey_fingerprint(keys[i], - fingerprint_hash, SSH_FP_DEFAULT)) == NULL) + if ((fp = sshkey_fingerprint(key, fingerprint_hash, + SSH_FP_DEFAULT)) == NULL) fatal_f("sshkey_fingerprint failed"); debug_f("key %zu: %s %s %s (flags 0x%02x)", i, - sshkey_type(keys[i]), fp, keys[i]->sk_application, - keys[i]->sk_flags); - ext = skip_ssh_url_preamble(keys[i]->sk_application); + sshkey_type(key), fp, key->sk_application, key->sk_flags); + ext = sk_suffix(key->sk_application, + srks[i]->user_id, srks[i]->user_id_len); xasprintf(&path, "id_%s_rk%s%s", - keys[i]->type == KEY_ECDSA_SK ? "ecdsa_sk" : "ed25519_sk", + key->type == KEY_ECDSA_SK ? "ecdsa_sk" : "ed25519_sk", *ext == '\0' ? "" : "_", ext); /* If the file already exists, ask the user to confirm. */ @@ -3039,26 +3067,25 @@ do_download_sk(const char *skprovider, const char *device) /* Save the key with the application string as the comment */ if (pass == NULL) pass = private_key_passphrase(); - if ((r = sshkey_save_private(keys[i], path, pass, - keys[i]->sk_application, private_key_format, + if ((r = sshkey_save_private(key, path, pass, + key->sk_application, private_key_format, openssh_format_cipher, rounds)) != 0) { error_r(r, "Saving key \"%s\" failed", path); free(path); break; } if (!quiet) { - printf("Saved %s key%s%s to %s\n", - sshkey_type(keys[i]), + printf("Saved %s key%s%s to %s\n", sshkey_type(key), *ext != '\0' ? " " : "", - *ext != '\0' ? keys[i]->sk_application : "", + *ext != '\0' ? key->sk_application : "", path); } /* Save public key too */ xasprintf(&pubpath, "%s.pub", path); free(path); - if ((r = sshkey_save_public(keys[i], pubpath, - keys[i]->sk_application)) != 0) { + if ((r = sshkey_save_public(key, pubpath, + key->sk_application)) != 0) { error_r(r, "Saving public key \"%s\" failed", pubpath); free(pubpath); break; @@ -3066,13 +3093,11 @@ do_download_sk(const char *skprovider, const char *device) free(pubpath); } - if (i >= nkeys) + if (i >= nsrks) ret = 0; /* success */ if (pass != NULL) freezero(pass, strlen(pass)); - for (i = 0; i < nkeys; i++) - sshkey_free(keys[i]); - free(keys); + sshsk_free_resident_keys(srks, nsrks); return ret; } diff --git a/usr.bin/ssh/ssh-sk-client.c b/usr.bin/ssh/ssh-sk-client.c index d928bda88f6..afd808025fc 100644 --- a/usr.bin/ssh/ssh-sk-client.c +++ b/usr.bin/ssh/ssh-sk-client.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssh-sk-client.c,v 1.9 2021/04/03 06:18:41 djm Exp $ */ +/* $OpenBSD: ssh-sk-client.c,v 1.10 2021/10/28 02:54:18 djm Exp $ */ /* * Copyright (c) 2019 Google LLC * @@ -365,17 +365,44 @@ sshsk_enroll(int type, const char *provider_path, const char *device, return r; } +static void +sshsk_free_resident_key(struct sshsk_resident_key *srk) +{ + if (srk == NULL) + return; + sshkey_free(srk->key); + freezero(srk->user_id, srk->user_id_len); + free(srk); +} + + +void +sshsk_free_resident_keys(struct sshsk_resident_key **srks, size_t nsrks) +{ + size_t i; + + if (srks == NULL || nsrks == 0) + return; + + for (i = 0; i < nsrks; i++) + sshsk_free_resident_key(srks[i]); + free(srks); +} + int sshsk_load_resident(const char *provider_path, const char *device, - const char *pin, struct sshkey ***keysp, size_t *nkeysp) + const char *pin, u_int flags, struct sshsk_resident_key ***srksp, + size_t *nsrksp) { int oerrno, r = SSH_ERR_INTERNAL_ERROR; struct sshbuf *kbuf = NULL, *req = NULL, *resp = NULL; - struct sshkey *key = NULL, **keys = NULL, **tmp; - size_t i, nkeys = 0; + struct sshkey *key = NULL; + struct sshsk_resident_key *srk = NULL, **srks = NULL, **tmp; + u_char *userid = NULL; + size_t userid_len = 0, nsrks = 0; - *keysp = NULL; - *nkeysp = 0; + *srksp = NULL; + *nsrksp = 0; if ((resp = sshbuf_new()) == NULL || (kbuf = sshbuf_new()) == NULL || @@ -386,7 +413,8 @@ sshsk_load_resident(const char *provider_path, const char *device, if ((r = sshbuf_put_cstring(req, provider_path)) != 0 || (r = sshbuf_put_cstring(req, device)) != 0 || - (r = sshbuf_put_cstring(req, pin)) != 0) { + (r = sshbuf_put_cstring(req, pin)) != 0 || + (r = sshbuf_put_u32(req, flags)) != 0) { error_fr(r, "compose"); goto out; } @@ -395,10 +423,11 @@ sshsk_load_resident(const char *provider_path, const char *device, goto out; while (sshbuf_len(resp) != 0) { - /* key, comment */ + /* key, comment, user_id */ if ((r = sshbuf_get_stringb(resp, kbuf)) != 0 || - (r = sshbuf_get_cstring(resp, NULL, NULL)) != 0) { - error_fr(r, "parse signature"); + (r = sshbuf_get_cstring(resp, NULL, NULL)) != 0 || + (r = sshbuf_get_string(resp, &userid, &userid_len)) != 0) { + error_fr(r, "parse"); r = SSH_ERR_INVALID_FORMAT; goto out; } @@ -406,29 +435,40 @@ sshsk_load_resident(const char *provider_path, const char *device, error_fr(r, "decode key"); goto out; } - if ((tmp = recallocarray(keys, nkeys, nkeys + 1, - sizeof(*keys))) == NULL) { - error_f("recallocarray keys failed"); + if ((srk = calloc(1, sizeof(*srk))) == NULL) { + error_f("calloc failed"); goto out; } - debug_f("keys[%zu]: %s %s", nkeys, sshkey_type(key), - key->sk_application); - keys = tmp; - keys[nkeys++] = key; + srk->key = key; key = NULL; + srk->user_id = userid; + srk->user_id_len = userid_len; + userid = NULL; + userid_len = 0; + if ((tmp = recallocarray(srks, nsrks, nsrks + 1, + sizeof(*srks))) == NULL) { + error_f("recallocarray keys failed"); + goto out; + } + debug_f("srks[%zu]: %s %s uidlen %zu", nsrks, + sshkey_type(srk->key), srk->key->sk_application, + srk->user_id_len); + srks = tmp; + srks[nsrks++] = srk; + srk = NULL; } /* success */ r = 0; - *keysp = keys; - *nkeysp = nkeys; - keys = NULL; - nkeys = 0; + *srksp = srks; + *nsrksp = nsrks; + srks = NULL; + nsrks = 0; out: oerrno = errno; - for (i = 0; i < nkeys; i++) - sshkey_free(keys[i]); - free(keys); + sshsk_free_resident_key(srk); + sshsk_free_resident_keys(srks, nsrks); + freezero(userid, userid_len); sshkey_free(key); sshbuf_free(kbuf); sshbuf_free(req); diff --git a/usr.bin/ssh/ssh-sk-helper.c b/usr.bin/ssh/ssh-sk-helper.c index aacf3d797d0..8b40d1d5c7c 100644 --- a/usr.bin/ssh/ssh-sk-helper.c +++ b/usr.bin/ssh/ssh-sk-helper.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssh-sk-helper.c,v 1.11 2020/10/18 11:32:02 djm Exp $ */ +/* $OpenBSD: ssh-sk-helper.c,v 1.12 2021/10/28 02:54:18 djm Exp $ */ /* * Copyright (c) 2019 Google LLC * @@ -213,15 +213,17 @@ process_load_resident(struct sshbuf *req) int r; char *provider, *pin, *device; struct sshbuf *kbuf, *resp; - struct sshkey **keys = NULL; - size_t nkeys = 0, i; + struct sshsk_resident_key **srks = NULL; + size_t nsrks = 0, i; + u_int flags; if ((kbuf = sshbuf_new()) == NULL) fatal("%s: sshbuf_new failed", __progname); if ((r = sshbuf_get_cstring(req, &provider, NULL)) != 0 || (r = sshbuf_get_cstring(req, &device, NULL)) != 0 || - (r = sshbuf_get_cstring(req, &pin, NULL)) != 0) + (r = sshbuf_get_cstring(req, &pin, NULL)) != 0 || + (r = sshbuf_get_u32(req, &flags)) != 0) fatal_r(r, "%s: parse", __progname); if (sshbuf_len(req) != 0) fatal("%s: trailing data in request", __progname); @@ -229,9 +231,9 @@ process_load_resident(struct sshbuf *req) null_empty(&device); null_empty(&pin); - if ((r = sshsk_load_resident(provider, device, pin, - &keys, &nkeys)) != 0) { - resp = reply_error(r, " sshsk_load_resident failed: %s", + if ((r = sshsk_load_resident(provider, device, pin, flags, + &srks, &nsrks)) != 0) { + resp = reply_error(r, "sshsk_load_resident failed: %s", ssh_err(r)); goto out; } @@ -242,21 +244,22 @@ process_load_resident(struct sshbuf *req) if ((r = sshbuf_put_u32(resp, SSH_SK_HELPER_LOAD_RESIDENT)) != 0) fatal_r(r, "%s: compose", __progname); - for (i = 0; i < nkeys; i++) { - debug_f("key %zu %s %s", i, sshkey_type(keys[i]), - keys[i]->sk_application); + for (i = 0; i < nsrks; i++) { + debug_f("key %zu %s %s uidlen %zu", i, + sshkey_type(srks[i]->key), srks[i]->key->sk_application, + srks[i]->user_id_len); sshbuf_reset(kbuf); - if ((r = sshkey_private_serialize(keys[i], kbuf)) != 0) + if ((r = sshkey_private_serialize(srks[i]->key, kbuf)) != 0) fatal_r(r, "%s: encode key", __progname); if ((r = sshbuf_put_stringb(resp, kbuf)) != 0 || - (r = sshbuf_put_cstring(resp, "")) != 0) /* comment */ + (r = sshbuf_put_cstring(resp, "")) != 0 || /* comment */ + (r = sshbuf_put_string(resp, srks[i]->user_id, + srks[i]->user_id_len)) != 0) fatal_r(r, "%s: compose key", __progname); } out: - for (i = 0; i < nkeys; i++) - sshkey_free(keys[i]); - free(keys); + sshsk_free_resident_keys(srks, nsrks); sshbuf_free(kbuf); free(provider); if (pin != NULL) diff --git a/usr.bin/ssh/ssh-sk.c b/usr.bin/ssh/ssh-sk.c index d764161a192..9e33d71aca4 100644 --- a/usr.bin/ssh/ssh-sk.c +++ b/usr.bin/ssh/ssh-sk.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssh-sk.c,v 1.35 2021/02/26 00:16:58 djm Exp $ */ +/* $OpenBSD: ssh-sk.c,v 1.36 2021/10/28 02:54:18 djm Exp $ */ /* * Copyright (c) 2019 Google LLC * @@ -733,25 +733,51 @@ sshsk_free_sk_resident_keys(struct sk_resident_key **rks, size_t nrks) free(rks); } +static void +sshsk_free_resident_key(struct sshsk_resident_key *srk) +{ + if (srk == NULL) + return; + sshkey_free(srk->key); + freezero(srk->user_id, srk->user_id_len); + free(srk); +} + + +void +sshsk_free_resident_keys(struct sshsk_resident_key **srks, size_t nsrks) +{ + size_t i; + + if (srks == NULL || nsrks == 0) + return; + + for (i = 0; i < nsrks; i++) + sshsk_free_resident_key(srks[i]); + free(srks); +} + int sshsk_load_resident(const char *provider_path, const char *device, - const char *pin, struct sshkey ***keysp, size_t *nkeysp) + const char *pin, u_int flags, struct sshsk_resident_key ***srksp, + size_t *nsrksp) { struct sshsk_provider *skp = NULL; int r = SSH_ERR_INTERNAL_ERROR; struct sk_resident_key **rks = NULL; - size_t i, nrks = 0, nkeys = 0; - struct sshkey *key = NULL, **keys = NULL, **tmp; - uint8_t flags; + size_t i, nrks = 0, nsrks = 0; + struct sshkey *key = NULL; + struct sshsk_resident_key *srk = NULL, **srks = NULL, **tmp; + uint8_t sk_flags; struct sk_option **opts = NULL; debug_f("provider \"%s\"%s", provider_path, (pin != NULL && *pin != '\0') ? ", have-pin": ""); - if (keysp == NULL || nkeysp == NULL) + if (srksp == NULL || nsrksp == NULL) return SSH_ERR_INVALID_ARGUMENT; - *keysp = NULL; - *nkeysp = 0; + *srksp = NULL; + *nsrksp = 0; if ((r = make_options(device, NULL, &opts)) != 0) goto out; @@ -765,8 +791,9 @@ sshsk_load_resident(const char *provider_path, const char *device, goto out; } for (i = 0; i < nrks; i++) { - debug3_f("rk %zu: slot = %zu, alg = %d, application = \"%s\"", - i, rks[i]->slot, rks[i]->alg, rks[i]->application); + debug3_f("rk %zu: slot %zu, alg %d, app \"%s\", uidlen %zu", + i, rks[i]->slot, rks[i]->alg, rks[i]->application, + rks[i]->user_id_len); /* XXX need better filter here */ if (strncmp(rks[i]->application, "ssh:", 4) != 0) continue; @@ -777,39 +804,50 @@ sshsk_load_resident(const char *provider_path, const char *device, default: continue; } - flags = SSH_SK_USER_PRESENCE_REQD|SSH_SK_RESIDENT_KEY; + sk_flags = SSH_SK_USER_PRESENCE_REQD|SSH_SK_RESIDENT_KEY; if ((rks[i]->flags & SSH_SK_USER_VERIFICATION_REQD)) - flags |= SSH_SK_USER_VERIFICATION_REQD; + sk_flags |= SSH_SK_USER_VERIFICATION_REQD; if ((r = sshsk_key_from_response(rks[i]->alg, - rks[i]->application, flags, &rks[i]->key, &key)) != 0) + rks[i]->application, sk_flags, &rks[i]->key, &key)) != 0) + goto out; + if ((srk = calloc(1, sizeof(*srk))) == NULL) { + error_f("calloc failed"); + r = SSH_ERR_ALLOC_FAIL; goto out; - if ((tmp = recallocarray(keys, nkeys, nkeys + 1, + } + srk->key = key; + key = NULL; /* transferred */ + if ((srk->user_id = calloc(1, rks[i]->user_id_len)) == NULL) { + error_f("calloc failed"); + r = SSH_ERR_ALLOC_FAIL; + goto out; + } + memcpy(srk->user_id, rks[i]->user_id, rks[i]->user_id_len); + srk->user_id_len = rks[i]->user_id_len; + if ((tmp = recallocarray(srks, nsrks, nsrks + 1, sizeof(*tmp))) == NULL) { error_f("recallocarray failed"); r = SSH_ERR_ALLOC_FAIL; goto out; } - keys = tmp; - keys[nkeys++] = key; - key = NULL; + srks = tmp; + srks[nsrks++] = srk; + srk = NULL; /* XXX synthesise comment */ } /* success */ - *keysp = keys; - *nkeysp = nkeys; - keys = NULL; - nkeys = 0; + *srksp = srks; + *nsrksp = nsrks; + srks = NULL; + nsrks = 0; r = 0; out: sshsk_free_options(opts); sshsk_free(skp); sshsk_free_sk_resident_keys(rks, nrks); sshkey_free(key); - if (nkeys != 0) { - for (i = 0; i < nkeys; i++) - sshkey_free(keys[i]); - free(keys); - } + sshsk_free_resident_key(srk); + sshsk_free_resident_keys(srks, nsrks); return r; } diff --git a/usr.bin/ssh/ssh-sk.h b/usr.bin/ssh/ssh-sk.h index 0f566bbc385..89d1b662785 100644 --- a/usr.bin/ssh/ssh-sk.h +++ b/usr.bin/ssh/ssh-sk.h @@ -1,4 +1,4 @@ -/* $OpenBSD: ssh-sk.h,v 1.10 2020/01/10 23:43:26 djm Exp $ */ +/* $OpenBSD: ssh-sk.h,v 1.11 2021/10/28 02:54:18 djm Exp $ */ /* * Copyright (c) 2019 Google LLC * @@ -31,6 +31,12 @@ struct sk_option; #define SSH_SK_HELPER_ENROLL 2 #define SSH_SK_HELPER_LOAD_RESIDENT 3 +struct sshsk_resident_key { + struct sshkey *key; + uint8_t *user_id; + size_t user_id_len; +}; + /* * Enroll (generate) a new security-key hosted private key of given type * via the specified provider middleware. @@ -63,7 +69,11 @@ int sshsk_sign(const char *provider_path, struct sshkey *key, * Returns 0 on success or a ssherr.h error code on failure. */ int sshsk_load_resident(const char *provider_path, const char *device, - const char *pin, struct sshkey ***keysp, size_t *nkeysp); + const char *pin, u_int flags, struct sshsk_resident_key ***srksp, + size_t *nsrksp); + +/* Free an array of sshsk_resident_key (as returned from sshsk_load_resident) */ +void sshsk_free_resident_keys(struct sshsk_resident_key **srks, size_t nsrks); #endif /* _SSH_SK_H */ |