upstream: expose PKCS#11 key labels/X.509 subjects as comments

Extract the key label or X.509 subject string when PKCS#11 keys
are retrieved from the token and plumb this through to places where
it may be used as a comment.

based on openssh#138
by Danielle Church

feedback and ok markus@

OpenBSD-Commit-ID: cae1fda10d9e10971dea29520916e27cfec7ca35

ssh-agent.c

@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-agent.c,v 1.252 2020/01/23 07:10:22 dtucker Exp $ */
+/* $OpenBSD: ssh-agent.c,v 1.253 2020/01/25 00:03:36 djm Exp $ */
 /*
  * Author: Tatu Ylonen <[email protected]>
  * Copyright (c) 1995 Tatu Ylonen <[email protected]>, Espoo, Finland
@@ -633,6 +633,7 @@ static void
 process_add_smartcard_key(SocketEntry *e)
 {
 	char *provider = NULL, *pin = NULL, canonical_provider[PATH_MAX];
+	char **comments = NULL;
 	int r, i, count = 0, success = 0, confirm = 0;
 	u_int seconds;
 	time_t death = 0;
@@ -682,28 +683,34 @@ process_add_smartcard_key(SocketEntry *e)
 	if (lifetime && !death)
 		death = monotime() + lifetime;
 
-	count = pkcs11_add_provider(canonical_provider, pin, &keys);
+	count = pkcs11_add_provider(canonical_provider, pin, &keys, &comments);
 	for (i = 0; i < count; i++) {
 		k = keys[i];
 		if (lookup_identity(k) == NULL) {
 			id = xcalloc(1, sizeof(Identity));
 			id->key = k;
+			keys[i] = NULL; /* transferred */
 			id->provider = xstrdup(canonical_provider);
-			id->comment = xstrdup(canonical_provider); /* XXX */
+			if (*comments[i] != '\0') {
+				id->comment = comments[i];
+				comments[i] = NULL; /* transferred */
+			} else {
+				id->comment = xstrdup(canonical_provider);
+			}
 			id->death = death;
 			id->confirm = confirm;
 			TAILQ_INSERT_TAIL(&idtab->idlist, id, next);
 			idtab->nentries++;
 			success = 1;
-		} else {
-			sshkey_free(k);
 		}
-		keys[i] = NULL;
+		sshkey_free(keys[i]);
+		free(comments[i]);
 	}
 send:
 	free(pin);
 	free(provider);
 	free(keys);
+	free(comments);
 	send_status(e, success);
 }
 

ssh-keygen.c

@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-keygen.c,v 1.391 2020/01/24 05:33:01 djm Exp $ */
+/* $OpenBSD: ssh-keygen.c,v 1.392 2020/01/25 00:03:36 djm Exp $ */
 /*
  * Author: Tatu Ylonen <[email protected]>
  * Copyright (c) 1994 Tatu Ylonen <[email protected]>, Espoo, Finland
@@ -829,13 +829,13 @@ do_download(struct passwd *pw)
 	int i, nkeys;
 	enum sshkey_fp_rep rep;
 	int fptype;
-	char *fp, *ra;
+	char *fp, *ra, **comments = NULL;
 
 	fptype = print_bubblebabble ? SSH_DIGEST_SHA1 : fingerprint_hash;
 	rep =    print_bubblebabble ? SSH_FP_BUBBLEBABBLE : SSH_FP_DEFAULT;
 
 	pkcs11_init(1);
-	nkeys = pkcs11_add_provider(pkcs11provider, NULL, &keys);
+	nkeys = pkcs11_add_provider(pkcs11provider, NULL, &keys, &comments);
 	if (nkeys <= 0)
 		fatal("cannot read public key from pkcs11");
 	for (i = 0; i < nkeys; i++) {
@@ -853,10 +853,13 @@ do_download(struct passwd *pw)
 			free(fp);
 		} else {
 			(void) sshkey_write(keys[i], stdout); /* XXX check */
-			fprintf(stdout, "\n");
+			fprintf(stdout, "%s%s\n",
+			    *(comments[i]) == '\0' ? "" : " ", comments[i]);
 		}
+		free(comments[i]);
 		sshkey_free(keys[i]);
 	}
+	free(comments);
 	free(keys);
 	pkcs11_terminate();
 	exit(0);
@@ -1703,7 +1706,8 @@ load_pkcs11_key(char *path)
 		fatal("Couldn't load CA public key \"%s\": %s",
 		    path, ssh_err(r));
 
-	nkeys = pkcs11_add_provider(pkcs11provider, identity_passphrase, &keys);
+	nkeys = pkcs11_add_provider(pkcs11provider, identity_passphrase,
+	    &keys, NULL);
 	debug3("%s: %d keys", __func__, nkeys);
 	if (nkeys <= 0)
 		fatal("cannot read public key from pkcs11");

ssh-pkcs11-client.c

@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-pkcs11-client.c,v 1.15 2019/01/21 12:53:35 djm Exp $ */
+/* $OpenBSD: ssh-pkcs11-client.c,v 1.16 2020/01/25 00:03:36 djm Exp $ */
 /*
  * Copyright (c) 2010 Markus Friedl.  All rights reserved.
  * Copyright (c) 2014 Pedro Martelletto. All rights reserved.
@@ -312,11 +312,13 @@ pkcs11_start_helper(void)
 }
 
 int
-pkcs11_add_provider(char *name, char *pin, struct sshkey ***keysp)
+pkcs11_add_provider(char *name, char *pin, struct sshkey ***keysp,
+    char ***labelsp)
 {
 	struct sshkey *k;
 	int r, type;
 	u_char *blob;
+	char *label;
 	size_t blen;
 	u_int nkeys, i;
 	struct sshbuf *msg;
@@ -338,16 +340,22 @@ pkcs11_add_provider(char *name, char *pin, struct sshkey ***keysp)
 		if ((r = sshbuf_get_u32(msg, &nkeys)) != 0)
 			fatal("%s: buffer error: %s", __func__, ssh_err(r));
 		*keysp = xcalloc(nkeys, sizeof(struct sshkey *));
+		if (labelsp)
+			*labelsp = xcalloc(nkeys, sizeof(char *));
 		for (i = 0; i < nkeys; i++) {
 			/* XXX clean up properly instead of fatal() */
 			if ((r = sshbuf_get_string(msg, &blob, &blen)) != 0 ||
-			    (r = sshbuf_skip_string(msg)) != 0)
+			    (r = sshbuf_get_cstring(msg, &label, NULL)) != 0)
 				fatal("%s: buffer error: %s",
 				    __func__, ssh_err(r));
 			if ((r = sshkey_from_blob(blob, blen, &k)) != 0)
 				fatal("%s: bad key: %s", __func__, ssh_err(r));
 			wrap_key(k);
 			(*keysp)[i] = k;
+			if (labelsp)
+				(*labelsp)[i] = label;
+			else
+				free(label);
 			free(blob);
 		}
 	} else if (type == SSH2_AGENT_FAILURE) {

ssh-pkcs11-helper.c

@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-pkcs11-helper.c,v 1.21 2019/09/06 05:23:55 djm Exp $ */
+/* $OpenBSD: ssh-pkcs11-helper.c,v 1.22 2020/01/25 00:03:36 djm Exp $ */
 /*
  * Copyright (c) 2010 Markus Friedl.  All rights reserved.
  *
@@ -50,7 +50,7 @@
 
 struct pkcs11_keyinfo {
 	struct sshkey	*key;
-	char		*providername;
+	char		*providername, *label;
 	TAILQ_ENTRY(pkcs11_keyinfo) next;
 };
 
@@ -63,13 +63,14 @@ struct sshbuf *iqueue;
 struct sshbuf *oqueue;
 
 static void
-add_key(struct sshkey *k, char *name)
+add_key(struct sshkey *k, char *name, char *label)
 {
 	struct pkcs11_keyinfo *ki;
 
 	ki = xcalloc(1, sizeof(*ki));
 	ki->providername = xstrdup(name);
 	ki->key = k;
+	ki->label = xstrdup(label);
 	TAILQ_INSERT_TAIL(&pkcs11_keylist, ki, next);
 }
 
@@ -83,6 +84,7 @@ del_keys_by_name(char *name)
 		if (!strcmp(ki->providername, name)) {
 			TAILQ_REMOVE(&pkcs11_keylist, ki, next);
 			free(ki->providername);
+			free(ki->label);
 			sshkey_free(ki->key);
 			free(ki);
 		}
@@ -96,7 +98,7 @@ lookup_key(struct sshkey *k)
 	struct pkcs11_keyinfo *ki;
 
 	TAILQ_FOREACH(ki, &pkcs11_keylist, next) {
-		debug("check %p %s", ki, ki->providername);
+		debug("check %p %s %s", ki, ki->providername, ki->label);
 		if (sshkey_equal(k, ki->key))
 			return (ki->key);
 	}
@@ -121,13 +123,14 @@ process_add(void)
 	u_char *blob;
 	size_t blen;
 	struct sshbuf *msg;
+	char **labels = NULL;
 
 	if ((msg = sshbuf_new()) == NULL)
 		fatal("%s: sshbuf_new failed", __func__);
 	if ((r = sshbuf_get_cstring(iqueue, &name, NULL)) != 0 ||
 	    (r = sshbuf_get_cstring(iqueue, &pin, NULL)) != 0)
 		fatal("%s: buffer error: %s", __func__, ssh_err(r));
-	if ((nkeys = pkcs11_add_provider(name, pin, &keys)) > 0) {
+	if ((nkeys = pkcs11_add_provider(name, pin, &keys, &labels)) > 0) {
 		if ((r = sshbuf_put_u8(msg,
 		    SSH2_AGENT_IDENTITIES_ANSWER)) != 0 ||
 		    (r = sshbuf_put_u32(msg, nkeys)) != 0)
@@ -139,19 +142,21 @@ process_add(void)
 				continue;
 			}
 			if ((r = sshbuf_put_string(msg, blob, blen)) != 0 ||
-			    (r = sshbuf_put_cstring(msg, name)) != 0)
+			    (r = sshbuf_put_cstring(msg, labels[i])) != 0)
 				fatal("%s: buffer error: %s",
 				    __func__, ssh_err(r));
 			free(blob);
-			add_key(keys[i], name);
+			add_key(keys[i], name, labels[i]);
+			free(labels[i]);
 		}
 	} else {
 		if ((r = sshbuf_put_u8(msg, SSH_AGENT_FAILURE)) != 0)
 			fatal("%s: buffer error: %s", __func__, ssh_err(r));
 		if ((r = sshbuf_put_u32(msg, -nkeys)) != 0)
 			fatal("%s: buffer error: %s", __func__, ssh_err(r));
 	}
-	free(keys);
+	free(labels);
+	free(keys); /* keys themselves are transferred to pkcs11_keylist */
 	free(pin);
 	free(name);
 	send_msg(msg);