Attachment 3415 Details for Bug 2890 – patch including pin caching (2020)

[patch] patch including pin caching (2020)

new.patch (text/plain), 6.74 KB, created by Jakub Jelen on 2020-06-24 17:13:36 AEST

(hide)

Description:

Filename:

MIME Type:

Creator: Jakub Jelen

Created: 2020-06-24 17:13:36 AEST

Size: 6.74 KB

patch

obsolete

>commit 1e44b62c26f799bcabff78c266e4a88af176a0ae
>Author: Jakub Jelen <jjelen@redhat.com>
>Date:   Thu Oct 25 17:25:27 2018 +0200
>
>    pkcs11: Cache user PIN in ssh-agent and refresh session upon card removal
>    
>    This introduces a check in pkcs11-helper verifying the smart card is ready
>    to sign data (was not removed since last invocation). If it was, the new
>    code tries to login again using cached PIN, rather than fail hard.
>    
>    Signed-off: Jakub Jelen <jjelen@redhat.com
>    Pair-programmed-with: Nikos Mavrogiannopoulos <nmav@redhat.com>
>
>diff --git a/ssh-pkcs11-helper.c b/ssh-pkcs11-helper.c
>index d73e8357..c8fe587d 100644
>--- a/ssh-pkcs11-helper.c
>+++ b/ssh-pkcs11-helper.c
>@@ -50,7 +50,7 @@
> 
> struct pkcs11_keyinfo {
> 	struct sshkey	*key;
>-	char		*providername, *label;
>+	char		*providername, *label, *pin;
> 	TAILQ_ENTRY(pkcs11_keyinfo) next;
> };
> 
>@@ -63,7 +63,7 @@ struct sshbuf *iqueue;
> struct sshbuf *oqueue;
> 
> static void
>-add_key(struct sshkey *k, char *name, char *label)
>+add_key(struct sshkey *k, char *name, char *label, char *pin)
> {
> 	struct pkcs11_keyinfo *ki;
> 
>@@ -71,6 +71,7 @@ add_key(struct sshkey *k, char *name, char *label)
> 	ki->providername = xstrdup(name);
> 	ki->key = k;
> 	ki->label = xstrdup(label);
>+	ki->pin = xstrdup(pin);
> 	TAILQ_INSERT_TAIL(&pkcs11_keylist, ki, next);
> }
> 
>@@ -85,6 +86,8 @@ del_keys_by_name(char *name)
> 			TAILQ_REMOVE(&pkcs11_keylist, ki, next);
> 			free(ki->providername);
> 			free(ki->label);
>+			explicit_bzero(ki->pin, strlen(ki->pin));
>+			free(ki->pin);
> 			sshkey_free(ki->key);
> 			free(ki);
> 		}
>@@ -93,14 +96,18 @@ del_keys_by_name(char *name)
> 
> /* lookup matching 'private' key */
> static struct sshkey *
>-lookup_key(struct sshkey *k)
>+lookup_key(struct sshkey *k, char **pin)
> {
> 	struct pkcs11_keyinfo *ki;
> 
> 	TAILQ_FOREACH(ki, &pkcs11_keylist, next) {
> 		debug("check %p %s %s", ki, ki->providername, ki->label);
>-		if (sshkey_equal(k, ki->key))
>+		if (sshkey_equal(k, ki->key)) {
>+			if (pin != NULL) {
>+				*pin = ki->pin;
>+			}
> 			return (ki->key);
>+		}
> 	}
> 	return (NULL);
> }
>@@ -146,7 +153,7 @@ process_add(void)
> 				fatal("%s: buffer error: %s",
> 				    __func__, ssh_err(r));
> 			free(blob);
>-			add_key(keys[i], name, labels[i]);
>+			add_key(keys[i], name, labels[i], pin);
> 			free(labels[i]);
> 		}
> 	} else {
>@@ -157,6 +164,7 @@ process_add(void)
> 	}
> 	free(labels);
> 	free(keys); /* keys themselves are transferred to pkcs11_keylist */
>+	explicit_bzero(pin, strlen(pin));
> 	free(pin);
> 	free(name);
> 	send_msg(msg);
>@@ -193,6 +201,7 @@ process_sign(void)
> 	int r, ok = -1;
> 	struct sshkey *key, *found;
> 	struct sshbuf *msg;
>+	char *pin = NULL;
> 
> 	/* XXX support SHA2 signature flags */
> 	if ((r = sshbuf_get_string(iqueue, &blob, &blen)) != 0 ||
>@@ -203,10 +212,13 @@ process_sign(void)
> 	if ((r = sshkey_from_blob(blob, blen, &key)) != 0)
> 		error("%s: sshkey_from_blob: %s", __func__, ssh_err(r));
> 	else {
>-		if ((found = lookup_key(key)) != NULL) {
>+		if ((found = lookup_key(key, &pin)) != NULL) {
> #ifdef WITH_OPENSSL
> 			int ret;
> 
>+			if (pkcs11_refresh_key(found, pin) != 0) {
>+				error("%s: Failed to get key. Is smart card present?", __func__);
>+			}
> 			if (key->type == KEY_RSA) {
> 				slen = RSA_size(key->rsa);
> 				signature = xmalloc(slen);
>diff --git a/ssh-pkcs11.c b/ssh-pkcs11.c
>index f495883d..0df43a60 100644
>--- a/ssh-pkcs11.c
>+++ b/ssh-pkcs11.c
>@@ -76,6 +76,10 @@ struct pkcs11_key {
> 	int			keyid_len;
> };
> 
>+static int pkcs11_open_session(struct pkcs11_provider *p, CK_ULONG slotidx, char *pin, CK_ULONG user);
>+static int pkcs11_fetch_keys(struct pkcs11_provider *p, CK_ULONG slotidx, struct sshkey ***keysp, char ***labelsp, int *nkeys);
>+static int pkcs11_key_included(struct sshkey ***keysp, int *nkeys, struct sshkey *key);
>+
> int pkcs11_interactive = 0;
> 
> #ifdef HAVE_EC_KEY_METHOD_NEW
>@@ -342,6 +346,106 @@ pkcs11_check_obj_bool_attrib(struct pkcs11_key *k11, CK_OBJECT_HANDLE obj,
> 	return (0);
> }
> 
>+int pkcs11_key_is_present(struct pkcs11_key *k11)
>+{
>+	CK_RV			rv;
>+	CK_FUNCTION_LIST	*f;
>+	CK_SLOT_INFO		info;
>+	CK_TOKEN_INFO		tokeninfo;
>+	CK_SESSION_HANDLE	session;
>+	CK_SESSION_INFO		sessioninfo;
>+
>+	f = k11->provider->function_list;
>+	rv = f->C_GetSlotInfo(k11->slotidx, &info);
>+	if (rv != CKR_OK) {
>+		/* The cryptoki is not ready to work with this slot */
>+		return -1;
>+	}
>+	if (!(info.flags & CKF_TOKEN_PRESENT)) {
>+		return -1;
>+	}
>+
>+	rv = f->C_GetTokenInfo(k11->slotidx, &tokeninfo);
>+	if (rv != CKR_OK) {
>+		/* The cryptoki is not ready to work with this token */
>+		return -1;
>+	}
>+	/* TODO check if the fields of the tokeninfo match the stored values */
>+
>+	session = k11->provider->slotinfo[k11->slotidx].session;
>+	rv = f->C_GetSessionInfo(session, &sessioninfo);
>+	if (rv != CKR_OK) {
>+		/* The cryptoki is not ready to work with this session */
>+		return -1;
>+	}
>+	if (sessioninfo.slotID != k11->slotidx) {
>+		return -1;
>+	}
>+	return 0;
>+}
>+
>+static int pkcs11_reload_key(struct sshkey *key, struct pkcs11_key *k11, char *pin)
>+{
>+	int			r, i;
>+	struct sshkey		**keysp = NULL;
>+	int			nkeys = 0;
>+
>+	/* No need to C_CloseSession(): It is already invalidated */
>+	debug("%s: called", __func__);
>+
>+	r = pkcs11_open_session(k11->provider, k11->slotidx, pin, CKU_USER);
>+	if (r == -1)
>+		return -1;
>+
>+	/* Check that the key we are using is present in the current card */
>+	r = pkcs11_fetch_keys(k11->provider, k11->slotidx, &keysp, NULL, &nkeys);
>+	if (r < 0)
>+		return -1;
>+
>+	r = -1;
>+	if (pkcs11_key_included(&keysp, &nkeys, key) == 1)
>+		r = 0;
>+
>+	/* clean up the keys */
>+	for (i = 0; i < nkeys; i++)
>+		sshkey_free(keysp[i]);
>+	free(keysp);
>+	return r;
>+}
>+
>+int pkcs11_refresh_key(struct sshkey *key, char *pin)
>+{
>+	struct pkcs11_key	*k11;
>+
>+	debug("%s: called", __func__);
>+
>+	switch (key->type) {
>+	case KEY_RSA:
>+		if ((k11 = RSA_get_ex_data(key->rsa, rsa_idx)) == NULL) {
>+			error("RSA_get_ex_data failed for rsa %p", key->rsa);
>+			return (-1);
>+		}
>+		break;
>+	case KEY_ECDSA:
>+		if ((k11 = EC_KEY_get_ex_data(key->ecdsa, ec_key_idx)) == NULL) {
>+			error("EC_KEY_get_ex_data failed for ecdsa %p", key->ecdsa);
>+			return (-1);
>+		}
>+		break;
>+	}
>+	if (!k11->provider || !k11->provider->valid) {
>+		error("no pkcs11 (valid) provider for rsa %p", key->rsa);
>+		return (-1);
>+	}
>+
>+	if (pkcs11_key_is_present(k11) == -1)
>+		if (pkcs11_reload_key(key, k11, pin) == -1)
>+			return -1;
>+
>+	return 0;
>+}
>+
>+/* openssl callback doing the actual signing operation */
> static int
> pkcs11_get_key(struct pkcs11_key *k11, CK_MECHANISM_TYPE mech_type)
> {
>diff --git a/ssh-pkcs11.h b/ssh-pkcs11.h
>index 81f1d7c5..e3237870 100644
>--- a/ssh-pkcs11.h
>+++ b/ssh-pkcs11.h
>@@ -34,6 +34,7 @@ struct sshkey *
> 	pkcs11_destroy_keypair(char *, char *, unsigned long, unsigned char,
> 	    u_int32_t *);
> #endif
>+int	pkcs11_refresh_key(struct sshkey *, char *);
> 
> #if !defined(WITH_OPENSSL) && defined(ENABLE_PKCS11)
> #undef ENABLE_PKCS11

Actions: View | Diff

Attachments on bug 2890: 3196 | 3197 | 3369 | 3415