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, 59 insertions, 34 deletions

diff --git a/ssh-keygen.c b/ssh-keygen.c
index 9b912f0a5..81988683f 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;
 }