upstream: 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

OpenBSD-Commit-ID: dbd658b5950f583106d945641a634bc6562dd3a3

8 files changed, 261 insertions, 126 deletions

diff --git a/sk-api.h b/sk-api.h
index 74921d4c..c84c6f8e 100644
--- a/sk-api.h
+++ b/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
  *
@@ -66,6 +66,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 {
@@ -74,7 +76,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/sk-usbhid.c b/sk-usbhid.c
index 01412b69..b9924d08 100644
--- a/sk-usbhid.c
+++ b/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
@@ -1088,9 +1088,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");
@@ -1132,12 +1134,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);
@@ -1161,17 +1167,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;
 			}
@@ -1179,6 +1191,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:
@@ -1219,6 +1234,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);
@@ -1271,6 +1287,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/ssh-add.c b/ssh-add.c
index 92192fcf..1b41cc18 100644
--- a/ssh-add.c
+++ b/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
@@ -580,25 +580,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;
@@ -607,7 +608,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);
@@ -618,10 +619,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/ssh-keygen.c b/ssh-keygen.c
index 9b912f0a..81988683 100644
--- a/ssh-keygen.c
+++ b/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
@@ -2996,24 +2996,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");
@@ -3023,34 +3051,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. */
@@ -3062,26 +3090,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;
@@ -3089,13 +3116,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/ssh-sk-client.c b/ssh-sk-client.c
index e9325900..bcef7a5d 100644
--- a/ssh-sk-client.c
+++ b/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
  *
@@ -375,17 +375,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 ||
@@ -396,7 +423,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;
 	}
@@ -405,10 +433,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;
 		}
@@ -416,29 +445,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/ssh-sk-helper.c b/ssh-sk-helper.c
index 21a08919..b1d22631 100644
--- a/ssh-sk-helper.c
+++ b/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
  *
@@ -216,15 +216,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);
@@ -232,9 +234,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;
 	}
@@ -245,21 +247,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/ssh-sk.c b/ssh-sk.c
index 393b4ccd..612530ab 100644
--- a/ssh-sk.c
+++ b/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
  *
@@ -752,25 +752,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;
@@ -784,8 +810,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;
@@ -796,39 +823,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/ssh-sk.h b/ssh-sk.h
index 0f566bbc..89d1b662 100644
--- a/ssh-sk.h
+++ b/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 */