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

1 files changed, 64 insertions, 26 deletions

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;
 }