===================================================================
RCS file: /cvsrepo/anoncvs/cvs/src/usr.bin/ssh/ssh-pkcs11.c,v
retrieving revision 1.46
retrieving revision 1.47
diff -u -r1.46 -r1.47
--- src/usr.bin/ssh/ssh-pkcs11.c	2019/10/01 10:22:53	1.46
+++ src/usr.bin/ssh/ssh-pkcs11.c	2020/01/25 00:03:36	1.47
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-pkcs11.c,v 1.46 2019/10/01 10:22:53 djm Exp $ */
+/* $OpenBSD: ssh-pkcs11.c,v 1.47 2020/01/25 00:03:36 djm Exp $ */
 /*
  * Copyright (c) 2010 Markus Friedl.  All rights reserved.
  * Copyright (c) 2014 Pedro Martelletto. All rights reserved.
@@ -880,23 +880,28 @@
 	return (key);
 }
 
-static struct sshkey *
+static int
 pkcs11_fetch_x509_pubkey(struct pkcs11_provider *p, CK_ULONG slotidx,
-    CK_OBJECT_HANDLE *obj)
+    CK_OBJECT_HANDLE *obj, struct sshkey **keyp, char **labelp)
 {
 	CK_ATTRIBUTE		 cert_attr[3];
 	CK_SESSION_HANDLE	 session;
 	CK_FUNCTION_LIST	*f = NULL;
 	CK_RV			 rv;
 	X509			*x509 = NULL;
+	X509_NAME		*x509_name = NULL;
 	EVP_PKEY		*evp;
 	RSA			*rsa = NULL;
 	EC_KEY			*ec = NULL;
 	struct sshkey		*key = NULL;
 	int			 i;
 	int			 nid;
-	const u_char		 *cp;
+	const u_char		*cp;
+	char			*subject = NULL;
 
+	*keyp = NULL;
+	*labelp = NULL;
+
 	memset(&cert_attr, 0, sizeof(cert_attr));
 	cert_attr[0].type = CKA_ID;
 	cert_attr[1].type = CKA_SUBJECT;
@@ -909,7 +914,7 @@
 	rv = f->C_GetAttributeValue(session, *obj, cert_attr, 3);
 	if (rv != CKR_OK) {
 		error("C_GetAttributeValue failed: %lu", rv);
-		return (NULL);
+		return -1;
 	}
 
 	/*
@@ -920,7 +925,7 @@
 	if (cert_attr[1].ulValueLen == 0 ||
 	    cert_attr[2].ulValueLen == 0) {
 		error("invalid attribute length");
-		return (NULL);
+		return -1;
 	}
 
 	/* allocate buffers for attributes */
@@ -932,44 +937,45 @@
 	rv = f->C_GetAttributeValue(session, *obj, cert_attr, 3);
 	if (rv != CKR_OK) {
 		error("C_GetAttributeValue failed: %lu", rv);
-		goto fail;
+		goto out;
 	}
 
-	x509 = X509_new();
-	if (x509 == NULL) {
-		error("x509_new failed");
-		goto fail;
-	}
+	/* Decode DER-encoded cert subject */
+	cp = cert_attr[2].pValue;
+	if ((x509_name = d2i_X509_NAME(NULL, &cp,
+	    cert_attr[1].ulValueLen)) == NULL ||
+	    (subject = X509_NAME_oneline(x509_name, NULL, 0)) == NULL)
+		subject = xstrdup("invalid subject");
+	X509_NAME_free(x509_name);
 
 	cp = cert_attr[2].pValue;
-	if (d2i_X509(&x509, &cp, cert_attr[2].ulValueLen) == NULL) {
+	if ((x509 = d2i_X509(NULL, &cp, cert_attr[2].ulValueLen)) == NULL) {
 		error("d2i_x509 failed");
-		goto fail;
+		goto out;
 	}
 
-	evp = X509_get_pubkey(x509);
-	if (evp == NULL) {
+	if ((evp = X509_get_pubkey(x509)) == NULL) {
 		error("X509_get_pubkey failed");
-		goto fail;
+		goto out;
 	}
 
 	if (EVP_PKEY_base_id(evp) == EVP_PKEY_RSA) {
 		if (EVP_PKEY_get0_RSA(evp) == NULL) {
 			error("invalid x509; no rsa key");
-			goto fail;
+			goto out;
 		}
 		if ((rsa = RSAPublicKey_dup(EVP_PKEY_get0_RSA(evp))) == NULL) {
 			error("RSAPublicKey_dup failed");
-			goto fail;
+			goto out;
 		}
 
 		if (pkcs11_rsa_wrap(p, slotidx, &cert_attr[0], rsa))
-			goto fail;
+			goto out;
 
 		key = sshkey_new(KEY_UNSPEC);
 		if (key == NULL) {
 			error("sshkey_new failed");
-			goto fail;
+			goto out;
 		}
 
 		key->rsa = rsa;
@@ -979,26 +985,26 @@
 	} else if (EVP_PKEY_base_id(evp) == EVP_PKEY_EC) {
 		if (EVP_PKEY_get0_EC_KEY(evp) == NULL) {
 			error("invalid x509; no ec key");
-			goto fail;
+			goto out;
 		}
 		if ((ec = EC_KEY_dup(EVP_PKEY_get0_EC_KEY(evp))) == NULL) {
 			error("EC_KEY_dup failed");
-			goto fail;
+			goto out;
 		}
 
 		nid = sshkey_ecdsa_key_to_nid(ec);
 		if (nid < 0) {
 			error("couldn't get curve nid");
-			goto fail;
+			goto out;
 		}
 
 		if (pkcs11_ecdsa_wrap(p, slotidx, &cert_attr[0], ec))
-			goto fail;
+			goto out;
 
 		key = sshkey_new(KEY_UNSPEC);
 		if (key == NULL) {
 			error("sshkey_new failed");
-			goto fail;
+			goto out;
 		}
 
 		key->ecdsa = ec;
@@ -1006,17 +1012,24 @@
 		key->type = KEY_ECDSA;
 		key->flags |= SSHKEY_FLAG_EXT;
 		ec = NULL;	/* now owned by key */
-	} else
+	} else {
 		error("unknown certificate key type");
-
-fail:
+		goto out;
+	}
+ out:
 	for (i = 0; i < 3; i++)
 		free(cert_attr[i].pValue);
 	X509_free(x509);
 	RSA_free(rsa);
 	EC_KEY_free(ec);
-
-	return (key);
+	if (key == NULL) {
+		free(subject);
+		return -1;
+	}
+	/* success */
+	*keyp = key;
+	*labelp = subject;
+	return 0;
 }
 
 #if 0
@@ -1037,7 +1050,7 @@
  */
 static int
 pkcs11_fetch_certs(struct pkcs11_provider *p, CK_ULONG slotidx,
-    struct sshkey ***keysp, int *nkeys)
+    struct sshkey ***keysp, char ***labelsp, int *nkeys)
 {
 	struct sshkey		*key = NULL;
 	CK_OBJECT_CLASS		 key_class;
@@ -1048,6 +1061,7 @@
 	CK_OBJECT_HANDLE	 obj;
 	CK_ULONG		 n = 0;
 	int			 ret = -1;
+	char			*label;
 
 	memset(&key_attr, 0, sizeof(key_attr));
 	memset(&obj, 0, sizeof(obj));
@@ -1089,18 +1103,19 @@
 			goto fail;
 		}
 
+		key = NULL;
+		label = NULL;
 		switch (ck_cert_type) {
 		case CKC_X_509:
-			key = pkcs11_fetch_x509_pubkey(p, slotidx, &obj);
+			if (pkcs11_fetch_x509_pubkey(p, slotidx, &obj,
+			    &key, &label) != 0) {
+				error("failed to fetch key");
+				continue;
+			}
 			break;
 		default:
-			/* XXX print key type? */
-			key = NULL;
-			error("skipping unsupported certificate type");
-		}
-
-		if (key == NULL) {
-			error("failed to fetch key");
+			error("skipping unsupported certificate type %lu",
+			    ck_cert_type);
 			continue;
 		}
 
@@ -1111,6 +1126,11 @@
 			*keysp = xrecallocarray(*keysp, *nkeys,
 			    *nkeys + 1, sizeof(struct sshkey *));
 			(*keysp)[*nkeys] = key;
+			if (labelsp != NULL) {
+				*labelsp = xrecallocarray(*labelsp, *nkeys,
+				    *nkeys + 1, sizeof(char *));
+				(*labelsp)[*nkeys] = xstrdup((char *)label);
+			}
 			*nkeys = *nkeys + 1;
 			debug("have %d keys", *nkeys);
 		}
@@ -1134,11 +1154,11 @@
  */
 static int
 pkcs11_fetch_keys(struct pkcs11_provider *p, CK_ULONG slotidx,
-    struct sshkey ***keysp, int *nkeys)
+    struct sshkey ***keysp, char ***labelsp, int *nkeys)
 {
 	struct sshkey		*key = NULL;
 	CK_OBJECT_CLASS		 key_class;
-	CK_ATTRIBUTE		 key_attr[1];
+	CK_ATTRIBUTE		 key_attr[2];
 	CK_SESSION_HANDLE	 session;
 	CK_FUNCTION_LIST	*f = NULL;
 	CK_RV			 rv;
@@ -1165,6 +1185,7 @@
 
 	while (1) {
 		CK_KEY_TYPE	ck_key_type;
+		CK_UTF8CHAR	label[256];
 
 		rv = f->C_FindObjects(session, &obj, 1, &n);
 		if (rv != CKR_OK) {
@@ -1179,13 +1200,18 @@
 		key_attr[0].type = CKA_KEY_TYPE;
 		key_attr[0].pValue = &ck_key_type;
 		key_attr[0].ulValueLen = sizeof(ck_key_type);
+		key_attr[1].type = CKA_LABEL;
+		key_attr[1].pValue = &label;
+		key_attr[1].ulValueLen = sizeof(label) - 1;
 
-		rv = f->C_GetAttributeValue(session, obj, key_attr, 1);
+		rv = f->C_GetAttributeValue(session, obj, key_attr, 2);
 		if (rv != CKR_OK) {
 			error("C_GetAttributeValue failed: %lu", rv);
 			goto fail;
 		}
 
+		label[key_attr[1].ulValueLen] = '\0';
+
 		switch (ck_key_type) {
 		case CKK_RSA:
 			key = pkcs11_fetch_rsa_pubkey(p, slotidx, &obj);
@@ -1211,6 +1237,11 @@
 			*keysp = xrecallocarray(*keysp, *nkeys,
 			    *nkeys + 1, sizeof(struct sshkey *));
 			(*keysp)[*nkeys] = key;
+			if (labelsp != NULL) {
+				*labelsp = xrecallocarray(*labelsp, *nkeys,
+				    *nkeys + 1, sizeof(char *));
+				(*labelsp)[*nkeys] = xstrdup((char *)label);
+			}
 			*nkeys = *nkeys + 1;
 			debug("have %d keys", *nkeys);
 		}
@@ -1415,7 +1446,8 @@
  * keyp is provided, fetch keys.
  */
 static int
-pkcs11_register_provider(char *provider_id, char *pin, struct sshkey ***keyp,
+pkcs11_register_provider(char *provider_id, char *pin,
+    struct sshkey ***keyp, char ***labelsp,
     struct pkcs11_provider **providerp, CK_ULONG user)
 {
 	int nkeys, need_finalize = 0;
@@ -1434,6 +1466,8 @@
 
 	if (keyp != NULL)
 		*keyp = NULL;
+	if (labelsp != NULL)
+		*labelsp = NULL;
 
 	if (pkcs11_provider_lookup(provider_id) != NULL) {
 		debug("%s: provider already registered: %s",
@@ -1531,8 +1565,8 @@
 		if ((ret = pkcs11_open_session(p, i, pin, user)) != 0 ||
 		    keyp == NULL)
 			continue;
-		pkcs11_fetch_keys(p, i, keyp, &nkeys);
-		pkcs11_fetch_certs(p, i, keyp, &nkeys);
+		pkcs11_fetch_keys(p, i, keyp, labelsp, &nkeys);
+		pkcs11_fetch_certs(p, i, keyp, labelsp, &nkeys);
 		if (nkeys == 0 && !p->slotinfo[i].logged_in &&
 		    pkcs11_interactive) {
 			/*
@@ -1544,8 +1578,8 @@
 				error("login failed");
 				continue;
 			}
-			pkcs11_fetch_keys(p, i, keyp, &nkeys);
-			pkcs11_fetch_certs(p, i, keyp, &nkeys);
+			pkcs11_fetch_keys(p, i, keyp, labelsp, &nkeys);
+			pkcs11_fetch_certs(p, i, keyp, labelsp, &nkeys);
 		}
 	}
 
@@ -1576,12 +1610,14 @@
  * fails if provider already exists
  */
 int
-pkcs11_add_provider(char *provider_id, char *pin, struct sshkey ***keyp)
+pkcs11_add_provider(char *provider_id, char *pin, struct sshkey ***keyp,
+    char ***labelsp)
 {
 	struct pkcs11_provider *p = NULL;
 	int nkeys;
 
-	nkeys = pkcs11_register_provider(provider_id, pin, keyp, &p, CKU_USER);
+	nkeys = pkcs11_register_provider(provider_id, pin, keyp, labelsp,
+	    &p, CKU_USER);
 
 	/* no keys found or some other error, de-register provider */
 	if (nkeys <= 0 && p != NULL) {
@@ -1613,8 +1649,8 @@
 
 	if ((p = pkcs11_provider_lookup(provider_id)) != NULL)
 		debug("%s: provider \"%s\" available", __func__, provider_id);
-	else if ((ret = pkcs11_register_provider(provider_id, pin, NULL, &p,
-	    CKU_SO)) < 0) {
+	else if ((ret = pkcs11_register_provider(provider_id, pin, NULL, NULL,
+	    &p, CKU_SO)) < 0) {
 		debug("%s: could not register provider %s", __func__,
 		    provider_id);
 		goto out;
@@ -1685,7 +1721,7 @@
 
 	if ((p = pkcs11_provider_lookup(provider_id)) != NULL) {
 		debug("%s: using provider \"%s\"", __func__, provider_id);
-	} else if (pkcs11_register_provider(provider_id, pin, NULL, &p,
+	} else if (pkcs11_register_provider(provider_id, pin, NULL, NULL, &p,
 	    CKU_SO) < 0) {
 		debug("%s: could not register provider %s", __func__,
 		    provider_id);