openssh unix dev - Nov 2016 - [PATCH] ssh-pkcs11: allow providing unconditional pin code for PKCS11

Some HSM's such as Safenet Network HSM do not allow searching for keys
unauthenticated. To support such devices provide a mechanism for users
to provide a pin code that is always used to automatically log in to
the HSM when using PKCS11.

The pin code is read from a file specified by the environment variable
SSH_PKCS11_PINFILE if it is set.

Tested against Safenet Network HSM.
---
 ssh-pkcs11.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/ssh-pkcs11.c b/ssh-pkcs11.c
index aaf712d..f75b201 100644
--- a/ssh-pkcs11.c
+++ b/ssh-pkcs11.c
@@ -42,6 +42,8 @@
 #include "ssh-pkcs11.h"
 #include "xmalloc.h"
 
+#define SSH_MAX_PKCS11_PIN_BYTES 128
+
 struct pkcs11_slotinfo {
 	CK_TOKEN_INFO		token;
 	CK_SESSION_HANDLE	session;
@@ -216,6 +218,36 @@ pkcs11_find(struct pkcs11_provider *p, CK_ULONG slotidx,
CK_ATTRIBUTE *attr,
 	return (ret);
 }
 
+/* read pin from a file specified in SSH_PKCS11_PINFILE if one exists */
+char *
+pkcs11_read_pinfile()
+{
+	FILE *f;
+	char *pinfilename;
+	char buf[SSH_MAX_PKCS11_PIN_BYTES];
+	int i;
+
+	if ((pinfilename = getenv("SSH_PKCS11_PINFILE")) == NULL)
+		return NULL;
+	if ((f = fopen(pinfilename, "r")) == NULL) {
+		debug("failed to read SSH_PKCS11_PINFILE");
+		return NULL;
+	}
+	if (fgets(buf, SSH_MAX_PKCS11_PIN_BYTES, f) == NULL)
+		return NULL;
+	fclose(f);
+
+	/* truncate first line and ignore the rest */
+	for (i = 0; buf[i] && i < SSH_MAX_PKCS11_PIN_BYTES; i++) {
+		if (buf[i] == '\n' || buf[i] == '\r') {
+			buf[i] = '\0';
+			break;
+		}
+	}
+
+	return xstrdup(buf);
+}
+
 /* openssl callback doing the actual signing operation */
 static int
 pkcs11_rsa_private_encrypt(int flen, const u_char *from, u_char *to, RSA *rsa,
@@ -575,6 +607,9 @@ pkcs11_add_provider(char *provider_id, char *pin, struct
sshkey ***keyp)
 	CK_TOKEN_INFO *token;
 	CK_ULONG i;
 
+	if (!pin)
+		pin = pkcs11_read_pinfile();
+
 	*keyp = NULL;
 	if (pkcs11_provider_lookup(provider_id) != NULL) {
 		debug("%s: provider already registered: %s",

Juha-Matti Tapio wrote:
> +pkcs11_read_pinfile()
..
> +	if ((f = fopen(pinfilename, "r")) == NULL) {
> +		debug("failed to read SSH_PKCS11_PINFILE");
> +		return NULL;
> +	}
> +	if (fgets(buf, SSH_MAX_PKCS11_PIN_BYTES, f) == NULL)
> +		return NULL;
This leaks the open file on error.

> +	fclose(f);
> +
> +	/* truncate first line and ignore the rest */
> +	for (i = 0; buf[i] && i < SSH_MAX_PKCS11_PIN_BYTES; i++) {
> +		if (buf[i] == '\n' || buf[i] == '\r') {
> +			buf[i] = '\0';
> +			break;
> +		}
> +	}
Why not just a call to strtok() ?

> +	return xstrdup(buf);
Does this memory get freed somewhere?


I don't think this patch is ready yet, and I don't know if this
functionality is really desirable. Would it be an option to store the
pin itself in the environment? HSM seems a bit pointless if a PIN
must be stored in plain text on disk..


//Peter

On 11/16/2016 10:31 AM, Juha-Matti Tapio wrote:
> Some HSM's such as Safenet Network HSM do not allow searching for keys
> unauthenticated. To support such devices provide a mechanism for users
> to provide a pin code that is always used to automatically log in to
> the HSM when using PKCS11.
>
> The pin code is read from a file specified by the environment variable
> SSH_PKCS11_PINFILE if it is set.
Don't we have PKCS#11 URI [1] to handle this? Without re-inventing wheel 
again? Wider implemenation would solve also other pains in PKCS#11 
waters in OpenSSH (choosing single key from a card -- alternative to 
IdentityFile, using p11kit, ...), though it would need some work to 
implement in OpenSSH, but as I can observe, PKCS#11 is not a biggest 
priority. Though I am having a look into that.

[1] https://tools.ietf.org/html/rfc7512

Regards,

-- 
Jakub Jelen
Software Engineer
Security Technologies
Red Hat

On Mon, Nov 21, 2016 at 09:05:23AM +0100, Jakub Jelen
wrote:
> On 11/16/2016 10:31 AM, Juha-Matti Tapio wrote:
> > Some HSM's such as Safenet Network HSM do not allow searching for
keys
> > unauthenticated. To support such devices provide a mechanism for users
> > to provide a pin code that is always used to automatically log in to
> > the HSM when using PKCS11.
> > 
> > The pin code is read from a file specified by the environment variable
> > SSH_PKCS11_PINFILE if it is set.
> Don't we have PKCS#11 URI [1] to handle this? Without re-inventing
wheel
> again? Wider implemenation would solve also other pains in PKCS#11 waters
in
> OpenSSH (choosing single key from a card -- alternative to IdentityFile,
> using p11kit, ...), though it would need some work to implement in OpenSSH,
> but as I can observe, PKCS#11 is not a biggest priority. Though I am having
> a look into that.
I think PKCS#11 URI support would be an excellent way to do it and
being able to choose the key would be a definite improvement. I
am not sure how much effort it would take but in principle I think it
would be the cleanest way to solve the issues.