pinpad for ssh-agent - patch posted

[Damien Miller]

On Tue, 1 Jun 2021, Jeremy Jackson wrote:

> Hi,
> 
> I just wanted to bring attention to an earlier post:
> 
> https://lists.mindrot.org/pipermail/openssh-unix-dev/2021-January/039031.html
> 
> This does work on Ubuntu Hirsute - openssh 8.4p1 with IsoApplet + OpenSC
> 
> The effect is that
> 
> $ ssh-add -s pkcs11-opensc.so
> 
> Prompts for the PIN# from the terminal, then again immediately from the pinpad
> (Dell smartcard keyboard, the light indicates PIN entry mode).
> 
> Perhaps it should only reqire one PIN entry, from the pinpad only.
> 
> After this, ssh agent works, allowing logins without password or PIN prompts,
> including forwarded agent connections, ie ssh -A host1 ; ssh host2
> 
> It would be nice to have another mode, where PIN is not cached, and each agent
> signing operation required PIN to be entered on pinpad, perhaps with a prompt
> somewhere stating what is being singed, e.g. "ssh login to host2".
> 
> Regardless of any present shortcomings, the patch does allow use of pinpad
> with ssh; without the patch, the pinpad must be disabled by OpenSC
> configuration.

So the problem with this type of change is that none of the developers
have access to the hardware in question, and thus little way to test.
Past experience with software token emulation has been problematic too.

In this case, the following cleaned-up diff should minimise the effect
on the the non-pinpad case. Can you say whether it solves your problem?

diff --git a/ssh-pkcs11.c b/ssh-pkcs11.c
index cc8ab3a..b5f8b3c 100644
--- a/ssh-pkcs11.c
+++ b/ssh-pkcs11.c
@@ -236,6 +236,9 @@ pkcs11_login_slot(struct pkcs11_provider *provider, struct pkcs11_slotinfo *si,
 {
 	char			*pin = NULL, prompt[1024];
 	CK_RV			 rv;
+	int			 have_pinpad;
+
+	have_pinpad = si->token.flags & CKF_PROTECTED_AUTHENTICATION_PATH;
 
 	if (provider == NULL || si == NULL || !provider->valid) {
 		error("no pkcs11 (valid) provider found");
@@ -243,12 +246,11 @@ pkcs11_login_slot(struct pkcs11_provider *provider, struct pkcs11_slotinfo *si,
 	}
 
 	if (!pkcs11_interactive) {
-		error("need pin entry%s",
-		    (si->token.flags & CKF_PROTECTED_AUTHENTICATION_PATH) ?
+		error("need pin entry%s", have_pinpad ?
 		    " on reader keypad" : "");
 		return (-1);
 	}
-	if (si->token.flags & CKF_PROTECTED_AUTHENTICATION_PATH)
+	if (have_pinpad)
 		verbose("Deferring PIN entry to reader keypad.");
 	else {
 		snprintf(prompt, sizeof(prompt), "Enter PIN for '%s': ",
@@ -637,16 +639,22 @@ pkcs11_open_session(struct pkcs11_provider *p, CK_ULONG slotidx, char *pin,
 	CK_FUNCTION_LIST	*f;
 	CK_RV			rv;
 	CK_SESSION_HANDLE	session;
-	int			login_required, ret;
+	int			login_required, have_pinpad, ret;
+	size_t			pinlen;
 
 	f = p->function_list;
 	si = &p->slotinfo[slotidx];
+	pinlen = pin == NULL ? 0 : strlen(pin);
 
 	login_required = si->token.flags & CKF_LOGIN_REQUIRED;
+	have_pinpad = si->token.flags & CKF_PROTECTED_AUTHENTICATION_PATH;
+
+	if (have_pinpad && pin != NULL && pinlen == 0)
+		pin = NULL; /* prefer token PIN entry to empty TTY PIN */
 
 	/* fail early before opening session */
-	if (login_required && !pkcs11_interactive &&
-	    (pin == NULL || strlen(pin) == 0)) {
+	if (login_required && !pkcs11_interactive && !have_pinpad &&
+	    (pin == NULL || pinlen == 0)) {
 		error("pin required");
 		return (-SSH_PKCS11_ERR_PIN_REQUIRED);
 	}
@@ -655,8 +663,9 @@ pkcs11_open_session(struct pkcs11_provider *p, CK_ULONG slotidx, char *pin,
 		error("C_OpenSession failed: %lu", rv);
 		return (-1);
 	}
-	if (login_required && pin != NULL && strlen(pin) != 0) {
-		rv = f->C_Login(session, user, (u_char *)pin, strlen(pin));
+	if (login_required &&
+	    (have_pinpad || (pin != NULL && pinlen != 0))) {
+		rv = f->C_Login(session, user, (u_char *)pin, pinlen);
 		if (rv != CKR_OK && rv != CKR_USER_ALREADY_LOGGED_IN) {
 			error("C_Login failed: %lu", rv);
 			ret = (rv == CKR_PIN_LOCKED) ?