SK API and sk-helper error/PIN passing

Allow passing a PIN via the SK API (API major crank) and let the
ssh-sk-helper API follow.

Also enhance the ssh-sk-helper API to support passing back an error
code instead of a complete reply. Will be used to signal "wrong PIN",
etc.

feedback and ok markus@

9 files changed, 165 insertions, 72 deletions

diff --git a/usr.bin/ssh/sk-api.h b/usr.bin/ssh/sk-api.h
index c651cc2a977..cff07d264a1 100644
--- a/usr.bin/ssh/sk-api.h
+++ b/usr.bin/ssh/sk-api.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: sk-api.h,v 1.4 2019/12/30 09:21:16 djm Exp $ */
+/* $OpenBSD: sk-api.h,v 1.5 2019/12/30 09:23:28 djm Exp $ */
 /*
  * Copyright (c) 2019 Google LLC
  *
@@ -57,7 +57,7 @@ struct sk_resident_key {
 	struct sk_enroll_response key;
 };
 
-#define SSH_SK_VERSION_MAJOR		0x00020000 /* current API version */
+#define SSH_SK_VERSION_MAJOR		0x00030000 /* current API version */
 #define SSH_SK_VERSION_MAJOR_MASK	0xffff0000
 
 /* Return the version of the middleware API */
@@ -65,13 +65,13 @@ uint32_t sk_api_version(void);
 
 /* Enroll a U2F key (private key generation) */
 int sk_enroll(int alg, const uint8_t *challenge, size_t challenge_len,
-    const char *application, uint8_t flags,
+    const char *application, uint8_t flags, const char *pin,
     struct sk_enroll_response **enroll_response);
 
 /* Sign a challenge */
 int sk_sign(int alg, const uint8_t *message, size_t message_len,
     const char *application, const uint8_t *key_handle, size_t key_handle_len,
-    uint8_t flags, struct sk_sign_response **sign_response);
+    uint8_t flags, const char *pin, struct sk_sign_response **sign_response);
 
 /* Enumerate all resident keys */
 int sk_load_resident_keys(const char *pin,
diff --git a/usr.bin/ssh/sk-usbhid.c b/usr.bin/ssh/sk-usbhid.c
index 1778652016a..694c4135d1a 100644
--- a/usr.bin/ssh/sk-usbhid.c
+++ b/usr.bin/ssh/sk-usbhid.c
@@ -50,7 +50,7 @@
 	} while (0)
 #endif
 
-#define SK_VERSION_MAJOR	0x00020000 /* current API version */
+#define SK_VERSION_MAJOR	0x00030000 /* current API version */
 
 /* Flags */
 #define SK_USER_PRESENCE_REQD		0x01
@@ -101,13 +101,13 @@ uint32_t sk_api_version(void);
 
 /* Enroll a U2F key (private key generation) */
 int sk_enroll(int alg, const uint8_t *challenge, size_t challenge_len,
-    const char *application, uint8_t flags,
+    const char *application, uint8_t flags, const char *pin,
     struct sk_enroll_response **enroll_response);
 
 /* Sign a challenge */
 int sk_sign(int alg, const uint8_t *message, size_t message_len,
     const char *application, const uint8_t *key_handle, size_t key_handle_len,
-    uint8_t flags, struct sk_sign_response **sign_response);
+    uint8_t flags, const char *pin, struct sk_sign_response **sign_response);
 
 /* Load resident keys */
 int sk_load_resident_keys(const char *pin,
@@ -410,7 +410,7 @@ pack_public_key(int alg, const fido_cred_t *cred,
 
 int
 sk_enroll(int alg, const uint8_t *challenge, size_t challenge_len,
-    const char *application, uint8_t flags,
+    const char *application, uint8_t flags, const char *pin,
     struct sk_enroll_response **enroll_response)
 {
 	fido_cred_t *cred = NULL;
@@ -648,7 +648,7 @@ int
 sk_sign(int alg, const uint8_t *message, size_t message_len,
     const char *application,
     const uint8_t *key_handle, size_t key_handle_len,
-    uint8_t flags, struct sk_sign_response **sign_response)
+    uint8_t flags, const char *pin, struct sk_sign_response **sign_response)
 {
 	fido_assert_t *assert = NULL;
 	fido_dev_t *dev = NULL;
diff --git a/usr.bin/ssh/ssh-keygen.c b/usr.bin/ssh/ssh-keygen.c
index eaafbf2b096..9c49eb81490 100644
--- a/usr.bin/ssh/ssh-keygen.c
+++ b/usr.bin/ssh/ssh-keygen.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-keygen.c,v 1.377 2019/12/30 09:19:52 djm Exp $ */
+/* $OpenBSD: ssh-keygen.c,v 1.378 2019/12/30 09:23:28 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1994 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -3347,7 +3347,7 @@ main(int argc, char **argv)
 		fflush(stdout);
 		if (sshsk_enroll(type, sk_provider,
 		    cert_key_id == NULL ? "ssh:" : cert_key_id,
-		    sk_flags, NULL, &private, NULL) != 0)
+		    sk_flags, NULL, NULL, &private, NULL) != 0)
 			exit(1); /* error message already printed */
 		break;
 	default:
diff --git a/usr.bin/ssh/ssh-sk-client.c b/usr.bin/ssh/ssh-sk-client.c
index 486a8fba12a..08737f1c558 100644
--- a/usr.bin/ssh/ssh-sk-client.c
+++ b/usr.bin/ssh/ssh-sk-client.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-sk-client.c,v 1.2 2019/12/30 09:21:59 djm Exp $ */
+/* $OpenBSD: ssh-sk-client.c,v 1.3 2019/12/30 09:23:28 djm Exp $ */
 /*
  * Copyright (c) 2019 Google LLC
  *
@@ -19,6 +19,7 @@
 #include <sys/socket.h>
 #include <sys/wait.h>
 
+#include <limits.h>
 #include <errno.h>
 #include <signal.h>
 #include <stdarg.h>
@@ -126,14 +127,14 @@ reap_helper(pid_t pid)
 }
 
 static int
-client_converse(struct sshbuf *req, struct sshbuf **respp)
+client_converse(struct sshbuf *req, struct sshbuf **respp, u_int msg)
 {
 	int oerrno, fd, r2, r = SSH_ERR_INTERNAL_ERROR;
+	u_int rmsg, rerr;
 	pid_t pid;
 	u_char version;
 	void (*osigchld)(int);
 	struct sshbuf *resp = NULL;
-
 	*respp = NULL;
 
 	if ((r = start_helper(&fd, &pid, &osigchld)) != 0)
@@ -162,6 +163,28 @@ client_converse(struct sshbuf *req, struct sshbuf **respp)
 		r = SSH_ERR_INVALID_FORMAT;
 		goto out;
 	}
+	if ((r = sshbuf_get_u32(resp, &rmsg)) != 0) {
+		error("%s: parse message type: %s", __func__, ssh_err(r));
+		goto out;
+	}
+	if (rmsg == SSH_SK_HELPER_ERROR) {
+		if ((r = sshbuf_get_u32(resp, &rerr)) != 0) {
+			error("%s: parse error: %s", __func__, ssh_err(r));
+			goto out;
+		}
+		debug("%s: helper returned error -%u", __func__, rerr);
+		/* OpenSSH error values are negative; encoded as -err on wire */
+		if (rerr == 0 || rerr >= INT_MAX)
+			r = SSH_ERR_INTERNAL_ERROR;
+		else
+			r = -(int)rerr;
+		goto out;
+	} else if (rmsg != msg) {
+		error("%s: helper returned incorrect message type %u, "
+		    "expecting %u", __func__, rmsg, msg);
+		r = SSH_ERR_INTERNAL_ERROR;
+		goto out;
+	}
 	/* success */
 	r = 0;
  out:
@@ -187,7 +210,7 @@ client_converse(struct sshbuf *req, struct sshbuf **respp)
 int
 sshsk_sign(const char *provider, struct sshkey *key,
     u_char **sigp, size_t *lenp, const u_char *data, size_t datalen,
-    u_int compat)
+    u_int compat, const char *pin)
 {
 	int oerrno, r = SSH_ERR_INTERNAL_ERROR;
 	char *fp = NULL;
@@ -211,7 +234,8 @@ sshsk_sign(const char *provider, struct sshkey *key,
 	    (r = sshbuf_put_cstring(req, provider)) != 0 ||
 	    (r = sshbuf_put_string(req, data, datalen)) != 0 ||
 	    (r = sshbuf_put_cstring(req, NULL)) != 0 || /* alg */
-	    (r = sshbuf_put_u32(req, compat)) != 0) {
+	    (r = sshbuf_put_u32(req, compat)) != 0 ||
+	    (r = sshbuf_put_cstring(req, pin)) != 0) {
 		error("%s: compose: %s", __func__, ssh_err(r));
 		goto out;
 	}
@@ -222,7 +246,7 @@ sshsk_sign(const char *provider, struct sshkey *key,
 		r = SSH_ERR_ALLOC_FAIL;
 		goto out;
 	}
-	if ((r = client_converse(req, &resp)) != 0)
+	if ((r = client_converse(req, &resp, SSH_SK_HELPER_SIGN)) != 0)
 		goto out;
 
 	if ((r = sshbuf_get_string(resp, sigp, lenp)) != 0) {
@@ -253,8 +277,8 @@ sshsk_sign(const char *provider, struct sshkey *key,
 
 int
 sshsk_enroll(int type, const char *provider_path, const char *application,
-    uint8_t flags, struct sshbuf *challenge_buf, struct sshkey **keyp,
-    struct sshbuf *attest)
+    uint8_t flags, const char *pin, struct sshbuf *challenge_buf,
+    struct sshkey **keyp, struct sshbuf *attest)
 {
 	int oerrno, r = SSH_ERR_INTERNAL_ERROR;
 	struct sshbuf *kbuf = NULL, *abuf = NULL, *req = NULL, *resp = NULL;
@@ -279,12 +303,13 @@ sshsk_enroll(int type, const char *provider_path, const char *application,
 	    (r = sshbuf_put_cstring(req, provider_path)) != 0 ||
 	    (r = sshbuf_put_cstring(req, application)) != 0 ||
 	    (r = sshbuf_put_u8(req, flags)) != 0 ||
+	    (r = sshbuf_put_cstring(req, pin)) != 0 ||
 	    (r = sshbuf_put_stringb(req, challenge_buf)) != 0) {
 		error("%s: compose: %s", __func__, ssh_err(r));
 		goto out;
 	}
 
-	if ((r = client_converse(req, &resp)) != 0)
+	if ((r = client_converse(req, &resp, SSH_SK_HELPER_ENROLL)) != 0)
 		goto out;
 
 	if ((r = sshbuf_get_stringb(resp, kbuf)) != 0 ||
@@ -348,7 +373,7 @@ sshsk_load_resident(const char *provider_path, const char *pin,
 		goto out;
 	}
 
-	if ((r = client_converse(req, &resp)) != 0)
+	if ((r = client_converse(req, &resp, SSH_SK_HELPER_LOAD_RESIDENT)) != 0)
 		goto out;
 
 	while (sshbuf_len(resp) != 0) {
@@ -368,6 +393,8 @@ sshsk_load_resident(const char *provider_path, const char *pin,
 			error("%s: recallocarray keys failed", __func__);
 			goto out;
 		}
+		debug("%s: keys[%zu]: %s %s", __func__,
+		    nkeys, sshkey_type(key), key->sk_application);
 		keys = tmp;
 		keys[nkeys++] = key;
 		key = NULL;
diff --git a/usr.bin/ssh/ssh-sk-helper.c b/usr.bin/ssh/ssh-sk-helper.c
index ee661fed2ee..33d7e8e8c33 100644
--- a/usr.bin/ssh/ssh-sk-helper.c
+++ b/usr.bin/ssh/ssh-sk-helper.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-sk-helper.c,v 1.5 2019/12/30 09:21:59 djm Exp $ */
+/* $OpenBSD: ssh-sk-helper.c,v 1.6 2019/12/30 09:23:28 djm Exp $ */
 /*
  * Copyright (c) 2019 Google LLC
  *
@@ -47,6 +47,33 @@
 
 extern char *__progname;
 
+static struct sshbuf *reply_error(int r, char *fmt, ...)
+    __attribute__((__format__ (printf, 2, 3)));
+
+static struct sshbuf *
+reply_error(int r, char *fmt, ...)
+{
+	char *msg;
+	va_list ap;
+	struct sshbuf *resp;
+
+	va_start(ap, fmt);
+	xvasprintf(&msg, fmt, ap);
+	va_end(ap);
+	error("%s: %s", __progname, msg);
+	free(msg);
+
+	if (r >= 0)
+		fatal("%s: invalid error code %d", __func__, r);
+
+	if ((resp = sshbuf_new()) == NULL)
+		fatal("%s: sshbuf_new failed", __progname);
+	if (sshbuf_put_u32(resp, SSH_SK_HELPER_ERROR) != 0 ||
+	    sshbuf_put_u32(resp, (u_int)-r) != 0)
+		fatal("%s: buffer error", __progname);
+	return resp;
+}
+
 static struct sshbuf *
 process_sign(struct sshbuf *req)
 {
@@ -57,13 +84,14 @@ process_sign(struct sshbuf *req)
 	const u_char *message;
 	u_char *sig;
 	size_t msglen, siglen;
-	char *provider;
+	char *provider, *pin;
 
 	if ((r = sshbuf_froms(req, &kbuf)) != 0 ||
 	    (r = sshbuf_get_cstring(req, &provider, NULL)) != 0 ||
 	    (r = sshbuf_get_string_direct(req, &message, &msglen)) != 0 ||
 	    (r = sshbuf_get_cstring(req, NULL, NULL)) != 0 || /* alg */
-	    (r = sshbuf_get_u32(req, &compat)) != 0)
+	    (r = sshbuf_get_u32(req, &compat)) != 0 ||
+	    (r = sshbuf_get_cstring(req, &pin, NULL)) != 0)
 		fatal("%s: buffer error: %s", __progname, ssh_err(r));
 	if (sshbuf_len(req) != 0)
 		fatal("%s: trailing data in request", __progname);
@@ -77,19 +105,28 @@ process_sign(struct sshbuf *req)
 	    "msg len %zu, compat 0x%lx", __progname, sshkey_type(key),
 	    provider, msglen, (u_long)compat);
 
+	if (*pin == 0) {
+		free(pin);
+		pin = NULL;
+	}
+
 	if ((r = sshsk_sign(provider, key, &sig, &siglen,
-	    message, msglen, compat)) != 0)
-		fatal("Signing failed: %s", ssh_err(r));
+	    message, msglen, compat, pin)) != 0) {
+		resp = reply_error(r, "Signing failed: %s", ssh_err(r));
+		goto out;
+	}
 
 	if ((resp = sshbuf_new()) == NULL)
 		fatal("%s: sshbuf_new failed", __progname);
 
-	if ((r = sshbuf_put_string(resp, sig, siglen)) != 0)
+	if ((r = sshbuf_put_u32(resp, SSH_SK_HELPER_SIGN)) != 0 ||
+	    (r = sshbuf_put_string(resp, sig, siglen)) != 0)
 		fatal("%s: buffer error: %s", __progname, ssh_err(r));
-
+ out:
 	sshbuf_free(kbuf);
 	free(provider);
-
+	if (pin != NULL)
+		freezero(pin, strlen(pin));
 	return resp;
 }
 
@@ -98,14 +135,12 @@ process_enroll(struct sshbuf *req)
 {
 	int r;
 	u_int type;
-	char *provider;
-	char *application;
+	char *provider, *application, *pin;
 	uint8_t flags;
 	struct sshbuf *challenge, *attest, *kbuf, *resp;
 	struct sshkey *key;
 
-	if ((resp = sshbuf_new()) == NULL ||
-	    (attest = sshbuf_new()) == NULL ||
+	if ((attest = sshbuf_new()) == NULL ||
 	    (kbuf = sshbuf_new()) == NULL)
 		fatal("%s: sshbuf_new failed", __progname);
 
@@ -113,6 +148,7 @@ process_enroll(struct sshbuf *req)
 	    (r = sshbuf_get_cstring(req, &provider, NULL)) != 0 ||
 	    (r = sshbuf_get_cstring(req, &application, NULL)) != 0 ||
 	    (r = sshbuf_get_u8(req, &flags)) != 0 ||
+	    (r = sshbuf_get_cstring(req, &pin, NULL)) != 0 ||
 	    (r = sshbuf_froms(req, &challenge)) != 0)
 		fatal("%s: buffer error: %s", __progname, ssh_err(r));
 	if (sshbuf_len(req) != 0)
@@ -124,23 +160,35 @@ process_enroll(struct sshbuf *req)
 		sshbuf_free(challenge);
 		challenge = NULL;
 	}
+	if (*pin == 0) {
+		free(pin);
+		pin = NULL;
+	}
 
-	if ((r = sshsk_enroll((int)type, provider, application, flags,
-	    challenge, &key, attest)) != 0)
-		fatal("%s: sshsk_enroll failed: %s", __progname, ssh_err(r));
+	if ((r = sshsk_enroll((int)type, provider, application, flags, pin,
+	    challenge, &key, attest)) != 0) {
+		resp = reply_error(r, "Enrollment failed: %s", ssh_err(r));
+		goto out;
+	}
 
+	if ((resp = sshbuf_new()) == NULL)
+		fatal("%s: sshbuf_new failed", __progname);
 	if ((r = sshkey_private_serialize(key, kbuf)) != 0)
 		fatal("%s: serialize private key: %s", __progname, ssh_err(r));
-	if ((r = sshbuf_put_stringb(resp, kbuf)) != 0 ||
+	if ((r = sshbuf_put_u32(resp, SSH_SK_HELPER_ENROLL)) != 0 ||
+	    (r = sshbuf_put_stringb(resp, kbuf)) != 0 ||
 	    (r = sshbuf_put_stringb(resp, attest)) != 0)
 		fatal("%s: buffer error: %s", __progname, ssh_err(r));
 
+ out:
 	sshkey_free(key);
 	sshbuf_free(kbuf);
 	sshbuf_free(attest);
 	sshbuf_free(challenge);
 	free(provider);
 	free(application);
+	if (pin != NULL)
+		freezero(pin, strlen(pin));
 
 	return resp;
 }
@@ -154,8 +202,7 @@ process_load_resident(struct sshbuf *req)
 	struct sshkey **keys = NULL;
 	size_t nkeys = 0, i;
 
-	if ((resp = sshbuf_new()) == NULL ||
-	    (kbuf = sshbuf_new()) == NULL)
+	if ((kbuf = sshbuf_new()) == NULL)
 		fatal("%s: sshbuf_new failed", __progname);
 
 	if ((r = sshbuf_get_cstring(req, &provider, NULL)) != 0 ||
@@ -164,9 +211,22 @@ process_load_resident(struct sshbuf *req)
 	if (sshbuf_len(req) != 0)
 		fatal("%s: trailing data in request", __progname);
 
-	if ((r = sshsk_load_resident(provider, pin, &keys, &nkeys)) != 0)
-		fatal("%s: sshsk_load_resident failed: %s",
-		    __progname, ssh_err(r));
+	if (*pin == 0) {
+		free(pin);
+		pin = NULL;
+	}
+
+	if ((r = sshsk_load_resident(provider, pin, &keys, &nkeys)) != 0) {
+		resp = reply_error(r, " sshsk_load_resident failed: %s",
+		    ssh_err(r));
+		goto out;
+	}
+
+	if ((resp = sshbuf_new()) == NULL)
+		fatal("%s: sshbuf_new failed", __progname);
+
+	if ((r = sshbuf_put_u32(resp, SSH_SK_HELPER_LOAD_RESIDENT)) != 0)
+		fatal("%s: buffer error: %s", __progname, ssh_err(r));
 
 	for (i = 0; i < nkeys; i++) {
 		debug("%s: key %zu %s %s", __func__, i,
@@ -180,12 +240,14 @@ process_load_resident(struct sshbuf *req)
 			fatal("%s: buffer error: %s", __progname, ssh_err(r));
 	}
 
+ out:
 	for (i = 0; i < nkeys; i++)
 		sshkey_free(keys[i]);
 	free(keys);
 	sshbuf_free(kbuf);
 	free(provider);
-	freezero(pin, strlen(pin));
+	if (pin != NULL)
+		freezero(pin, strlen(pin));
 	return resp;
 }
 
diff --git a/usr.bin/ssh/ssh-sk.c b/usr.bin/ssh/ssh-sk.c
index 2b4a9f98f39..ceedc53c449 100644
--- a/usr.bin/ssh/ssh-sk.c
+++ b/usr.bin/ssh/ssh-sk.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-sk.c,v 1.20 2019/12/30 09:21:16 djm Exp $ */
+/* $OpenBSD: ssh-sk.c,v 1.21 2019/12/30 09:23:28 djm Exp $ */
 /*
  * Copyright (c) 2019 Google LLC
  *
@@ -49,13 +49,14 @@ struct sshsk_provider {
 	/* Enroll a U2F key (private key generation) */
 	int (*sk_enroll)(int alg, const uint8_t *challenge,
 	    size_t challenge_len, const char *application, uint8_t flags,
-	    struct sk_enroll_response **enroll_response);
+	    const char *pin, struct sk_enroll_response **enroll_response);
 
 	/* Sign a challenge */
 	int (*sk_sign)(int alg, const uint8_t *message, size_t message_len,
 	    const char *application,
 	    const uint8_t *key_handle, size_t key_handle_len,
-	    uint8_t flags, struct sk_sign_response **sign_response);
+	    uint8_t flags, const char *pin,
+	    struct sk_sign_response **sign_response);
 
 	/* Enumerate resident keys */
 	int (*sk_load_resident_keys)(const char *pin,
@@ -65,11 +66,11 @@ struct sshsk_provider {
 /* Built-in version */
 int ssh_sk_enroll(int alg, const uint8_t *challenge,
     size_t challenge_len, const char *application, uint8_t flags,
-    struct sk_enroll_response **enroll_response);
+    const char *pin, struct sk_enroll_response **enroll_response);
 int ssh_sk_sign(int alg, const uint8_t *message, size_t message_len,
     const char *application,
     const uint8_t *key_handle, size_t key_handle_len,
-    uint8_t flags, struct sk_sign_response **sign_response);
+    uint8_t flags, const char *pin, struct sk_sign_response **sign_response);
 int ssh_sk_load_resident_keys(const char *pin,
     struct sk_resident_key ***rks, size_t *nrks);
 
@@ -318,8 +319,8 @@ sshsk_key_from_response(int alg, const char *application, uint8_t flags,
 
 int
 sshsk_enroll(int type, const char *provider_path, const char *application,
-    uint8_t flags, struct sshbuf *challenge_buf, struct sshkey **keyp,
-    struct sshbuf *attest)
+    uint8_t flags, const char *pin, struct sshbuf *challenge_buf,
+    struct sshkey **keyp, struct sshbuf *attest)
 {
 	struct sshsk_provider *skp = NULL;
 	struct sshkey *key = NULL;
@@ -331,8 +332,9 @@ sshsk_enroll(int type, const char *provider_path, const char *application,
 	int alg;
 
 	debug("%s: provider \"%s\", application \"%s\", flags 0x%02x, "
-	    "challenge len %zu", __func__, provider_path, application,
-	    flags, challenge_buf == NULL ? 0 : sshbuf_len(challenge_buf));
+	    "challenge len %zu%s", __func__, provider_path, application,
+	    flags, challenge_buf == NULL ? 0 : sshbuf_len(challenge_buf),
+	    (pin != NULL && *pin != '\0') ? " with-pin" : "");
 
 	*keyp = NULL;
 	if (attest)
@@ -383,7 +385,7 @@ sshsk_enroll(int type, const char *provider_path, const char *application,
 	/* XXX validate flags? */
 	/* enroll key */
 	if ((r = skp->sk_enroll(alg, challenge, challenge_len, application,
-	    flags, &resp)) != 0) {
+	    flags, pin, &resp)) != 0) {
 		error("Security key provider %s returned failure %d",
 		    provider_path, r);
 		r = SSH_ERR_INVALID_FORMAT; /* XXX error codes in API? */
@@ -496,7 +498,7 @@ sshsk_ed25519_sig(struct sk_sign_response *resp, struct sshbuf *sig)
 int
 sshsk_sign(const char *provider_path, struct sshkey *key,
     u_char **sigp, size_t *lenp, const u_char *data, size_t datalen,
-    u_int compat)
+    u_int compat, const char *pin)
 {
 	struct sshsk_provider *skp = NULL;
 	int r = SSH_ERR_INTERNAL_ERROR;
@@ -505,8 +507,9 @@ sshsk_sign(const char *provider_path, struct sshkey *key,
 	struct sshbuf *inner_sig = NULL, *sig = NULL;
 	uint8_t message[32];
 
-	debug("%s: provider \"%s\", key %s, flags 0x%02x", __func__,
-	    provider_path, sshkey_type(key), key->sk_flags);
+	debug("%s: provider \"%s\", key %s, flags 0x%02x%s", __func__,
+	    provider_path, sshkey_type(key), key->sk_flags,
+	    (pin != NULL && *pin != '\0') ? " with-pin" : "");
 
 	if (sigp != NULL)
 		*sigp = NULL;
@@ -546,7 +549,7 @@ sshsk_sign(const char *provider_path, struct sshkey *key,
 	if ((r = skp->sk_sign(alg, message, sizeof(message),
 	    key->sk_application,
 	    sshbuf_ptr(key->sk_key_handle), sshbuf_len(key->sk_key_handle),
-	    key->sk_flags, &resp)) != 0) {
+	    key->sk_flags, pin, &resp)) != 0) {
 		debug("%s: sk_sign failed with code %d", __func__, r);
 		goto out;
 	}
diff --git a/usr.bin/ssh/ssh-sk.h b/usr.bin/ssh/ssh-sk.h
index 1afe839db28..348759a9806 100644
--- a/usr.bin/ssh/ssh-sk.h
+++ b/usr.bin/ssh/ssh-sk.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-sk.h,v 1.7 2019/12/30 09:21:16 djm Exp $ */
+/* $OpenBSD: ssh-sk.h,v 1.8 2019/12/30 09:23:28 djm Exp $ */
 /*
  * Copyright (c) 2019 Google LLC
  *
@@ -21,6 +21,15 @@
 struct sshbuf;
 struct sshkey;
 
+/* Version of protocol expected from ssh-sk-helper */
+#define SSH_SK_HELPER_VERSION		3
+
+/* ssh-sk-helper messages */
+#define SSH_SK_HELPER_ERROR		0	/* Only valid H->C */
+#define SSH_SK_HELPER_SIGN		1
+#define SSH_SK_HELPER_ENROLL		2
+#define SSH_SK_HELPER_LOAD_RESIDENT	3
+
 /*
  * Enroll (generate) a new security-key hosted private key of given type
  * via the specified provider middleware.
@@ -32,8 +41,8 @@ struct sshkey;
  * information is placed there.
  */
 int sshsk_enroll(int type, const char *provider_path, const char *application,
-    uint8_t flags, struct sshbuf *challenge_buf, struct sshkey **keyp,
-    struct sshbuf *attest);
+    uint8_t flags, const char *pin, struct sshbuf *challenge_buf,
+    struct sshkey **keyp, struct sshbuf *attest);
 
 /*
  * Calculate an ECDSA_SK or ED25519_SK signature using the specified key
@@ -43,7 +52,7 @@ int sshsk_enroll(int type, const char *provider_path, const char *application,
  */
 int sshsk_sign(const char *provider_path, struct sshkey *key,
     u_char **sigp, size_t *lenp, const u_char *data, size_t datalen,
-    u_int compat);
+    u_int compat, const char *pin);
 
 /*
  * Enumerates and loads all SSH-compatible resident keys from a security
diff --git a/usr.bin/ssh/sshkey.c b/usr.bin/ssh/sshkey.c
index 69238a9e18c..7e5be5bc17f 100644
--- a/usr.bin/ssh/sshkey.c
+++ b/usr.bin/ssh/sshkey.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: sshkey.c,v 1.97 2019/12/13 19:09:10 djm Exp $ */
+/* $OpenBSD: sshkey.c,v 1.98 2019/12/30 09:23:28 djm Exp $ */
 /*
  * Copyright (c) 2000, 2001 Markus Friedl.  All rights reserved.
  * Copyright (c) 2008 Alexander von Gernler.  All rights reserved.
@@ -2718,7 +2718,7 @@ sshkey_sign(struct sshkey *key,
 	case KEY_ECDSA_SK_CERT:
 	case KEY_ECDSA_SK:
 		r = sshsk_sign(sk_provider, key, sigp, lenp, data,
-		    datalen, compat);
+		    datalen, compat, /* XXX PIN */ NULL);
 		break;
 #ifdef WITH_XMSS
 	case KEY_XMSS:
diff --git a/usr.bin/ssh/sshkey.h b/usr.bin/ssh/sshkey.h
index baf4a4cf80c..9ff5b102ec2 100644
--- a/usr.bin/ssh/sshkey.h
+++ b/usr.bin/ssh/sshkey.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: sshkey.h,v 1.43 2019/12/30 09:21:59 djm Exp $ */
+/* $OpenBSD: sshkey.h,v 1.44 2019/12/30 09:23:28 djm Exp $ */
 
 /*
  * Copyright (c) 2000, 2001 Markus Friedl.  All rights reserved.
@@ -45,14 +45,6 @@
 #define SSH_RSA_MINIMUM_MODULUS_SIZE	1024
 #define SSH_KEY_MAX_SIGN_DATA_SIZE	(1 << 20)
 
-/* Version of protocol expected from ssh-sk-helper */
-#define SSH_SK_HELPER_VERSION		2
-
-/* ssh-sk-helper messages */
-#define SSH_SK_HELPER_SIGN		1
-#define SSH_SK_HELPER_ENROLL		2
-#define SSH_SK_HELPER_LOAD_RESIDENT	3
-
 struct sshbuf;
 
 /* Key types */