From 1e70dc3285fc9b4f6454975acb81e8702c23dd89 Mon Sep 17 00:00:00 2001
From: "djm@openbsd.org" <djm@openbsd.org>
Date: Wed, 13 May 2020 09:57:17 +0000
Subject: upstream: always call fido_init(); previous behaviour only called

fido_init() when SK_DEBUG was defined. Harmless with current libfido2, but
this isn't guaranteed in the future.

OpenBSD-Commit-ID: c7ea20ff2bcd98dd12015d748d3672d4f01f0864
---
 sk-usbhid.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

(limited to 'sk-usbhid.c')

diff --git a/sk-usbhid.c b/sk-usbhid.c
index 25250824d..8097cc7f4 100644
--- a/sk-usbhid.c
+++ b/sk-usbhid.c
@@ -57,6 +57,12 @@
 
 /* #define SK_DEBUG 1 */
 
+#ifdef SK_DEBUG
+#define SSH_FIDO_INIT_ARG	FIDO_DEBUG
+#else
+#define SSH_FIDO_INIT_ARG	0
+#endif
+
 #define MAX_FIDO_DEVICES	256
 
 /* Compatibility with OpenSSH 1.0.x */
@@ -459,9 +465,8 @@ sk_enroll(uint32_t alg, const uint8_t *challenge, size_t challenge_len,
 	int r;
 	char *device = NULL;
 
-#ifdef SK_DEBUG
-	fido_init(FIDO_DEBUG);
-#endif
+	fido_init(SSH_FIDO_INIT_ARG);
+
 	if (enroll_response == NULL) {
 		skdebug(__func__, "enroll_response == NULL");
 		goto out;
@@ -749,9 +754,7 @@ sk_sign(uint32_t alg, const uint8_t *data, size_t datalen,
 	int ret = SSH_SK_ERR_GENERAL;
 	int r;
 
-#ifdef SK_DEBUG
-	fido_init(FIDO_DEBUG);
-#endif
+	fido_init(SSH_FIDO_INIT_ARG);
 
 	if (sign_response == NULL) {
 		skdebug(__func__, "sign_response == NULL");
@@ -995,6 +998,8 @@ sk_load_resident_keys(const char *pin, struct sk_option **options,
 	*rksp = NULL;
 	*nrksp = 0;
 
+	fido_init(SSH_FIDO_INIT_ARG);
+
 	if (check_sign_load_resident_options(options, &device) != 0)
 		goto out; /* error already logged */
 	if (device != NULL) {
-- 
cgit v1.2.3


From 2ad7b7e46408dbebf2a4efc4efd75a9544197d57 Mon Sep 17 00:00:00 2001
From: "djm@openbsd.org" <djm@openbsd.org>
Date: Wed, 13 May 2020 10:08:02 +0000
Subject: upstream: Enable credProtect extension when generating a resident

key.

The FIDO 2.1 Client to Authenticator Protocol introduced a "credProtect"
feature to better protect resident keys. This option allows (amone other
possibilities) requiring a PIN prior to all operations that may retrieve
the key handle.

Patch by Pedro Martelletto; ok djm and markus

OpenBSD-Commit-ID: 013bc06a577dcaa66be3913b7f183eb8cad87e73
---
 sk-usbhid.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

(limited to 'sk-usbhid.c')

diff --git a/sk-usbhid.c b/sk-usbhid.c
index 8097cc7f4..4d64550c9 100644
--- a/sk-usbhid.c
+++ b/sk-usbhid.c
@@ -449,6 +449,43 @@ check_enroll_options(struct sk_option **options, char **devicep,
 	return 0;
 }
 
+static int
+check_sk_extensions(fido_dev_t *dev, const char *ext, int *ret)
+{
+	fido_cbor_info_t *info;
+	char * const *ptr;
+	size_t len;
+	int r;
+
+	*ret = 0;
+
+	if (!fido_dev_is_fido2(dev)) {
+		skdebug(__func__, "device is not fido2");
+		return 0;
+	}
+	if ((info = fido_cbor_info_new()) == NULL) {
+		skdebug(__func__, "fido_cbor_info_new failed");
+		return -1;
+	}
+	if ((r = fido_dev_get_cbor_info(dev, info)) != FIDO_OK) {
+		skdebug(__func__, "fido_dev_get_cbor_info: %s", fido_strerr(r));
+		fido_cbor_info_free(&info);
+		return -1;
+	}
+	ptr = fido_cbor_info_extensions_ptr(info);
+	len = fido_cbor_info_extensions_len(info);
+	for (size_t i = 0; i < len; i++) {
+		if (!strcmp(ptr[i], ext)) {
+			*ret = 1;
+			break;
+		}
+	}
+	fido_cbor_info_free(&info);
+	skdebug(__func__, "extension %s %s", ext, *ret ? "present" : "absent");
+
+	return 0;
+}
+
 int
 sk_enroll(uint32_t alg, const uint8_t *challenge, size_t challenge_len,
     const char *application, uint8_t flags, const char *pin,
@@ -460,6 +497,7 @@ sk_enroll(uint32_t alg, const uint8_t *challenge, size_t challenge_len,
 	uint8_t user_id[32];
 	struct sk_enroll_response *response = NULL;
 	size_t len;
+	int credprot;
 	int cose_alg;
 	int ret = SSH_SK_ERR_GENERAL;
 	int r;
@@ -532,6 +570,25 @@ sk_enroll(uint32_t alg, const uint8_t *challenge, size_t challenge_len,
 		skdebug(__func__, "fido_dev_open: %s", fido_strerr(r));
 		goto out;
 	}
+	if ((flags & SSH_SK_RESIDENT_KEY) != 0) {
+		if (check_sk_extensions(dev, "credProtect", &credprot) < 0) {
+			skdebug(__func__, "check_sk_extensions failed");
+			goto out;
+		}
+		if (credprot == 0) {
+			skdebug(__func__, "refusing to create unprotected "
+			    "resident key");
+			ret = SSH_SK_ERR_UNSUPPORTED;
+			goto out;
+		}
+		if ((r = fido_cred_set_prot(cred,
+		    FIDO_CRED_PROT_UV_OPTIONAL_WITH_ID)) != FIDO_OK) {
+			skdebug(__func__, "fido_cred_set_prot: %s",
+			    fido_strerr(r));
+			ret = fidoerr_to_skerr(r);
+			goto out;
+		}
+	}
 	if ((r = fido_dev_make_cred(dev, cred, pin)) != FIDO_OK) {
 		skdebug(__func__, "fido_dev_make_cred: %s", fido_strerr(r));
 		ret = fidoerr_to_skerr(r);
-- 
cgit v1.2.3


From d7d753e2979f2d3c904b03a08d30856cd2a6e892 Mon Sep 17 00:00:00 2001
From: "deraadt@openbsd.org" <deraadt@openbsd.org>
Date: Wed, 13 May 2020 22:38:41 +0000
Subject: upstream: we are still aiming for pre-C99 ...

OpenBSD-Commit-ID: a240fc9cbe60bc4e6c3d24d022eb4ab01fe1cb38
---
 sk-usbhid.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'sk-usbhid.c')

diff --git a/sk-usbhid.c b/sk-usbhid.c
index 4d64550c9..2a573caa2 100644
--- a/sk-usbhid.c
+++ b/sk-usbhid.c
@@ -454,7 +454,7 @@ check_sk_extensions(fido_dev_t *dev, const char *ext, int *ret)
 {
 	fido_cbor_info_t *info;
 	char * const *ptr;
-	size_t len;
+	size_t len, i;
 	int r;
 
 	*ret = 0;
@@ -474,7 +474,7 @@ check_sk_extensions(fido_dev_t *dev, const char *ext, int *ret)
 	}
 	ptr = fido_cbor_info_extensions_ptr(info);
 	len = fido_cbor_info_extensions_len(info);
-	for (size_t i = 0; i < len; i++) {
+	for (i = 0; i < len; i++) {
 		if (!strcmp(ptr[i], ext)) {
 			*ret = 1;
 			break;
-- 
cgit v1.2.3


From 9b8ad93824c682ce841f53f3b5762cef4e7cc4dc Mon Sep 17 00:00:00 2001
From: "djm@openbsd.org" <djm@openbsd.org>
Date: Thu, 27 Aug 2020 01:06:18 +0000
Subject: upstream: support for user-verified FIDO keys

FIDO2 supports a notion of "user verification" where the user is
required to demonstrate their identity to the token before particular
operations (e.g. signing). Typically this is done by authenticating
themselves using a PIN that has been set on the token.

This adds support for generating and using user verified keys where
the verification happens via PIN (other options might be added in the
future, but none are in common use now). Practically, this adds
another key generation option "verify-required" that yields a key that
requires a PIN before each authentication.

feedback markus@ and Pedro Martelletto; ok markus@

OpenBSD-Commit-ID: 57fd461e4366f87c47502c5614ec08573e6d6a15
---
 krl.c          |  7 ++++---
 monitor.c      |  4 ++--
 monitor_wrap.c |  4 ++--
 monitor_wrap.h |  5 +++--
 sk-usbhid.c    | 38 ++++++++++++++++++++++++++------------
 ssh-agent.c    |  5 +++--
 ssh-keygen.1   | 22 +++++++++++++++++++---
 ssh-keygen.c   | 55 +++++++++++++++++++++++++++++++++++++++----------------
 ssh-keysign.c  |  4 ++--
 ssh_api.c      | 10 +++++-----
 sshconnect2.c  | 37 ++++++++++++++++++++++++-------------
 sshd.c         |  8 ++++----
 sshkey.c       | 20 +++++++++++---------
 sshkey.h       | 11 ++++++-----
 sshsig.c       | 16 +++++++++-------
 sshsig.h       |  8 +++++---
 16 files changed, 164 insertions(+), 90 deletions(-)

(limited to 'sk-usbhid.c')

diff --git a/krl.c b/krl.c
index c431f7047..3a69b636a 100644
--- a/krl.c
+++ b/krl.c
@@ -14,7 +14,7 @@
  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $OpenBSD: krl.c,v 1.50 2020/04/03 05:48:57 djm Exp $ */
+/* $OpenBSD: krl.c,v 1.51 2020/08/27 01:06:18 djm Exp $ */
 
 #include "includes.h"
 
@@ -812,9 +812,10 @@ ssh_krl_to_blob(struct ssh_krl *krl, struct sshbuf *buf,
 		if ((r = sshbuf_put_u8(buf, KRL_SECTION_SIGNATURE)) != 0 ||
 		    (r = sshkey_puts(sign_keys[i], buf)) != 0)
 			goto out;
-
+		/* XXX support sk-* keys */
 		if ((r = sshkey_sign(sign_keys[i], &sblob, &slen,
-		    sshbuf_ptr(buf), sshbuf_len(buf), NULL, NULL, 0)) != 0)
+		    sshbuf_ptr(buf), sshbuf_len(buf), NULL, NULL,
+		    NULL, 0)) != 0)
 			goto out;
 		KRL_DBG(("%s: signature sig len %zu", __func__, slen));
 		if ((r = sshbuf_put_string(buf, sblob, slen)) != 0)
diff --git a/monitor.c b/monitor.c
index 0107a7eba..7c3e6aafe 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: monitor.c,v 1.212 2020/07/07 02:47:21 deraadt Exp $ */
+/* $OpenBSD: monitor.c,v 1.213 2020/08/27 01:06:18 djm Exp $ */
 /*
  * Copyright 2002 Niels Provos <provos@citi.umich.edu>
  * Copyright 2002 Markus Friedl <markus@openbsd.org>
@@ -679,7 +679,7 @@ mm_answer_sign(struct ssh *ssh, int sock, struct sshbuf *m)
 
 	if ((key = get_hostkey_by_index(keyid)) != NULL) {
 		if ((r = sshkey_sign(key, &signature, &siglen, p, datlen, alg,
-		    options.sk_provider, compat)) != 0)
+		    options.sk_provider, NULL, compat)) != 0)
 			fatal("%s: sshkey_sign failed: %s",
 			    __func__, ssh_err(r));
 	} else if ((key = get_hostkey_public_by_index(keyid, ssh)) != NULL &&
diff --git a/monitor_wrap.c b/monitor_wrap.c
index 001a8fa1c..5e38d83eb 100644
--- a/monitor_wrap.c
+++ b/monitor_wrap.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: monitor_wrap.c,v 1.117 2019/12/15 18:57:30 djm Exp $ */
+/* $OpenBSD: monitor_wrap.c,v 1.118 2020/08/27 01:06:18 djm Exp $ */
 /*
  * Copyright 2002 Niels Provos <provos@citi.umich.edu>
  * Copyright 2002 Markus Friedl <markus@openbsd.org>
@@ -216,7 +216,7 @@ mm_choose_dh(int min, int nbits, int max)
 int
 mm_sshkey_sign(struct ssh *ssh, struct sshkey *key, u_char **sigp, size_t *lenp,
     const u_char *data, size_t datalen, const char *hostkey_alg,
-    const char *sk_provider, u_int compat)
+    const char *sk_provider, const char *sk_pin, u_int compat)
 {
 	struct kex *kex = *pmonitor->m_pkex;
 	struct sshbuf *m;
diff --git a/monitor_wrap.h b/monitor_wrap.h
index 23ab096aa..0db38c206 100644
--- a/monitor_wrap.h
+++ b/monitor_wrap.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: monitor_wrap.h,v 1.44 2019/11/25 00:51:37 djm Exp $ */
+/* $OpenBSD: monitor_wrap.h,v 1.45 2020/08/27 01:06:18 djm Exp $ */
 
 /*
  * Copyright 2002 Niels Provos <provos@citi.umich.edu>
@@ -46,7 +46,8 @@ int mm_is_monitor(void);
 DH *mm_choose_dh(int, int, int);
 #endif
 int mm_sshkey_sign(struct ssh *, struct sshkey *, u_char **, size_t *,
-    const u_char *, size_t, const char *, const char *, u_int compat);
+    const u_char *, size_t, const char *, const char *,
+    const char *, u_int compat);
 void mm_inform_authserv(char *, char *);
 struct passwd *mm_getpwnamallow(struct ssh *, const char *);
 char *mm_auth2_read_banner(void);
diff --git a/sk-usbhid.c b/sk-usbhid.c
index 2a573caa2..1dd834883 100644
--- a/sk-usbhid.c
+++ b/sk-usbhid.c
@@ -163,7 +163,8 @@ pick_first_device(void)
 /* Check if the specified key handle exists on a given device. */
 static int
 try_device(fido_dev_t *dev, const uint8_t *message, size_t message_len,
-    const char *application, const uint8_t *key_handle, size_t key_handle_len)
+    const char *application, const uint8_t *key_handle, size_t key_handle_len,
+    uint8_t flags, const char *pin)
 {
 	fido_assert_t *assert = NULL;
 	int r = FIDO_ERR_INTERNAL;
@@ -191,7 +192,7 @@ try_device(fido_dev_t *dev, const uint8_t *message, size_t message_len,
 		skdebug(__func__, "fido_assert_up: %s", fido_strerr(r));
 		goto out;
 	}
-	r = fido_dev_get_assert(dev, assert, NULL);
+	r = fido_dev_get_assert(dev, assert, pin);
 	skdebug(__func__, "fido_dev_get_assert: %s", fido_strerr(r));
 	if (r == FIDO_ERR_USER_PRESENCE_REQUIRED) {
 		/* U2F tokens may return this */
@@ -206,7 +207,8 @@ try_device(fido_dev_t *dev, const uint8_t *message, size_t message_len,
 /* Iterate over configured devices looking for a specific key handle */
 static fido_dev_t *
 find_device(const char *path, const uint8_t *message, size_t message_len,
-    const char *application, const uint8_t *key_handle, size_t key_handle_len)
+    const char *application, const uint8_t *key_handle, size_t key_handle_len,
+    uint8_t flags, const char *pin)
 {
 	fido_dev_info_t *devlist = NULL;
 	fido_dev_t *dev = NULL;
@@ -260,7 +262,7 @@ find_device(const char *path, const uint8_t *message, size_t message_len,
 			continue;
 		}
 		if (try_device(dev, message, message_len, application,
-		    key_handle, key_handle_len) == 0) {
+		    key_handle, key_handle_len, flags, pin) == 0) {
 			skdebug(__func__, "found key");
 			break;
 		}
@@ -570,19 +572,23 @@ sk_enroll(uint32_t alg, const uint8_t *challenge, size_t challenge_len,
 		skdebug(__func__, "fido_dev_open: %s", fido_strerr(r));
 		goto out;
 	}
-	if ((flags & SSH_SK_RESIDENT_KEY) != 0) {
+	if ((flags & (SSH_SK_RESIDENT_KEY|SSH_SK_USER_VERIFICATION_REQD)) != 0) {
 		if (check_sk_extensions(dev, "credProtect", &credprot) < 0) {
 			skdebug(__func__, "check_sk_extensions failed");
 			goto out;
 		}
 		if (credprot == 0) {
 			skdebug(__func__, "refusing to create unprotected "
-			    "resident key");
+			    "resident/verify-required key");
 			ret = SSH_SK_ERR_UNSUPPORTED;
 			goto out;
 		}
-		if ((r = fido_cred_set_prot(cred,
-		    FIDO_CRED_PROT_UV_OPTIONAL_WITH_ID)) != FIDO_OK) {
+		if ((flags & SSH_SK_USER_VERIFICATION_REQD))
+			credprot = FIDO_CRED_PROT_UV_REQUIRED;
+		else
+			credprot = FIDO_CRED_PROT_UV_OPTIONAL_WITH_ID;
+
+		if ((r = fido_cred_set_prot(cred, credprot)) != FIDO_OK) {
 			skdebug(__func__, "fido_cred_set_prot: %s",
 			    fido_strerr(r));
 			ret = fidoerr_to_skerr(r);
@@ -826,7 +832,7 @@ sk_sign(uint32_t alg, const uint8_t *data, size_t datalen,
 		goto out;
 	}
 	if ((dev = find_device(device, message, sizeof(message),
-	    application, key_handle, key_handle_len)) == NULL) {
+	    application, key_handle, key_handle_len, flags, pin)) == NULL) {
 		skdebug(__func__, "couldn't find device for key handle");
 		goto out;
 	}
@@ -855,8 +861,15 @@ sk_sign(uint32_t alg, const uint8_t *data, size_t datalen,
 		skdebug(__func__, "fido_assert_set_up: %s", fido_strerr(r));
 		goto out;
 	}
-	if ((r = fido_dev_get_assert(dev, assert, NULL)) != FIDO_OK) {
+	if (pin == NULL && (flags & SSH_SK_USER_VERIFICATION_REQD) &&
+	    (r = fido_assert_set_uv(assert, FIDO_OPT_TRUE)) != FIDO_OK) {
+		skdebug(__func__, "fido_assert_set_uv: %s", fido_strerr(r));
+		ret = FIDO_ERR_PIN_REQUIRED;
+		goto out;
+	}
+	if ((r = fido_dev_get_assert(dev, assert, pin)) != FIDO_OK) {
 		skdebug(__func__, "fido_dev_get_assert: %s", fido_strerr(r));
+		ret = fidoerr_to_skerr(r);
 		goto out;
 	}
 	if ((response = calloc(1, sizeof(*response))) == NULL) {
@@ -978,8 +991,9 @@ read_rks(const char *devpath, const char *pin,
 				continue;
 			}
 			skdebug(__func__, "Device %s RP \"%s\" slot %zu: "
-			    "type %d", devpath, fido_credman_rp_id(rp, i), j,
-			    fido_cred_type(cred));
+			    "type %d flags 0x%02x prot 0x%02x", devpath,
+			    fido_credman_rp_id(rp, i), j, fido_cred_type(cred),
+			    fido_cred_flags(cred), fido_cred_prot(cred));
 
 			/* build response entry */
 			if ((srk = calloc(1, sizeof(*srk))) == NULL ||
diff --git a/ssh-agent.c b/ssh-agent.c
index 5f7ac8b91..d24a63089 100644
--- a/ssh-agent.c
+++ b/ssh-agent.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-agent.c,v 1.262 2020/07/05 23:59:45 djm Exp $ */
+/* $OpenBSD: ssh-agent.c,v 1.263 2020/08/27 01:06:18 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -407,9 +407,10 @@ process_sign_request2(SocketEntry *e)
 			    sshkey_type(id->key), fp);
 		}
 	}
+	/* XXX support PIN required FIDO keys */
 	if ((r = sshkey_sign(id->key, &signature, &slen,
 	    data, dlen, agent_decode_alg(key, flags),
-	    id->sk_provider, compat)) != 0) {
+	    id->sk_provider, NULL, compat)) != 0) {
 		error("%s: sshkey_sign: %s", __func__, ssh_err(r));
 		goto send;
 	}
diff --git a/ssh-keygen.1 b/ssh-keygen.1
index 9198a511f..7e0558fe1 100644
--- a/ssh-keygen.1
+++ b/ssh-keygen.1
@@ -1,4 +1,4 @@
-.\"	$OpenBSD: ssh-keygen.1,v 1.205 2020/07/15 07:50:46 solene Exp $
+.\"	$OpenBSD: ssh-keygen.1,v 1.206 2020/08/27 01:06:18 djm Exp $
 .\"
 .\" Author: Tatu Ylonen <ylo@cs.hut.fi>
 .\" Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -35,7 +35,7 @@
 .\" (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
 .\" THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 .\"
-.Dd $Mdocdate: July 15 2020 $
+.Dd $Mdocdate: August 27 2020 $
 .Dt SSH-KEYGEN 1
 .Os
 .Sh NAME
@@ -511,6 +511,12 @@ A username to be associated with a resident key,
 overriding the empty default username.
 Specifying a username may be useful when generating multiple resident keys
 for the same application name.
+.It Cm verify-required
+Indicate that this private key should require user verification for
+each signature.
+Not all FIDO tokens support support this option.
+Currently PIN authentication is the only supported verification method,
+but other methods may be supported in the future.
 .It Cm write-attestation Ns = Ns Ar path
 May be used at key generation time to record the attestation certificate
 returned from FIDO tokens during key generation.
@@ -961,7 +967,7 @@ by
 Allows X11 forwarding.
 .Pp
 .It Ic no-touch-required
-Do not require signatures made using this key require demonstration
+Do not require signatures made using this key include demonstration
 of user presence (e.g. by having the user touch the authenticator).
 This option only makes sense for the FIDO authenticator algorithms
 .Cm ecdsa-sk
@@ -974,6 +980,16 @@ The
 .Ar address_list
 is a comma-separated list of one or more address/netmask pairs in CIDR
 format.
+.Pp
+.It Ic verify-required
+Require signatures made using this key indicate that the user was first
+verified.
+This option only makes sense for the FIDO authenticator algorithms
+.Cm ecdsa-sk
+and
+.Cm ed25519-sk .
+Currently PIN authentication is the only supported verification method,
+but other methods may be supported in the future.
 .El
 .Pp
 At present, no standard options are valid for host keys.
diff --git a/ssh-keygen.c b/ssh-keygen.c
index cc092368e..89ef9a143 100644
--- a/ssh-keygen.c
+++ b/ssh-keygen.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-keygen.c,v 1.415 2020/08/03 02:53:51 djm Exp $ */
+/* $OpenBSD: ssh-keygen.c,v 1.416 2020/08/27 01:06:18 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1994 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -589,7 +589,7 @@ do_convert_private_ssh2(struct sshbuf *b)
 
 	/* try the key */
 	if (sshkey_sign(key, &sig, &slen, data, sizeof(data),
-	    NULL, NULL, 0) != 0 ||
+	    NULL, NULL, NULL, 0) != 0 ||
 	    sshkey_verify(key, sig, slen, data, sizeof(data),
 	    NULL, 0, NULL) != 0) {
 		sshkey_free(key);
@@ -1727,7 +1727,8 @@ load_pkcs11_key(char *path)
 static int
 agent_signer(struct sshkey *key, u_char **sigp, size_t *lenp,
     const u_char *data, size_t datalen,
-    const char *alg, const char *provider, u_int compat, void *ctx)
+    const char *alg, const char *provider, const char *pin,
+    u_int compat, void *ctx)
 {
 	int *agent_fdp = (int *)ctx;
 
@@ -1744,7 +1745,7 @@ do_ca_sign(struct passwd *pw, const char *ca_key_path, int prefer_agent,
 	u_int n;
 	struct sshkey *ca, *public;
 	char valid[64], *otmp, *tmp, *cp, *out, *comment;
-	char *ca_fp = NULL, **plist = NULL;
+	char *ca_fp = NULL, **plist = NULL, *pin = NULL;
 	struct ssh_identitylist *agent_ids;
 	size_t j;
 	struct notifier_ctx *notifier = NULL;
@@ -1785,6 +1786,12 @@ do_ca_sign(struct passwd *pw, const char *ca_key_path, int prefer_agent,
 	} else {
 		/* CA key is assumed to be a private key on the filesystem */
 		ca = load_identity(tmp, NULL);
+		if (sshkey_is_sk(ca) &&
+		    (ca->sk_flags & SSH_SK_USER_VERIFICATION_REQD)) {
+			if ((pin = read_passphrase("Enter PIN for CA key: ",
+			    RP_ALLOW_STDIN)) == NULL)
+				fatal("%s: couldn't read PIN", __func__);
+		}
 	}
 	free(tmp);
 
@@ -1844,7 +1851,7 @@ do_ca_sign(struct passwd *pw, const char *ca_key_path, int prefer_agent,
 
 		if (agent_fd != -1 && (ca->flags & SSHKEY_FLAG_EXT) != 0) {
 			if ((r = sshkey_certify_custom(public, ca,
-			    key_type_name, sk_provider, agent_signer,
+			    key_type_name, sk_provider, NULL, agent_signer,
 			    &agent_fd)) != 0)
 				fatal("Couldn't certify key %s via agent: %s",
 				    tmp, ssh_err(r));
@@ -1856,7 +1863,7 @@ do_ca_sign(struct passwd *pw, const char *ca_key_path, int prefer_agent,
 				    sshkey_type(ca), ca_fp);
 			}
 			r = sshkey_certify(public, ca, key_type_name,
-			    sk_provider);
+			    sk_provider, pin);
 			notify_complete(notifier);
 			if (r != 0)
 				fatal("Couldn't certify key %s: %s",
@@ -1890,6 +1897,8 @@ do_ca_sign(struct passwd *pw, const char *ca_key_path, int prefer_agent,
 		if (cert_serial_autoinc)
 			cert_serial++;
 	}
+	if (pin != NULL)
+		freezero(pin, strlen(pin));
 	free(ca_fp);
 #ifdef ENABLE_PKCS11
 	pkcs11_terminate();
@@ -2526,6 +2535,7 @@ sign_one(struct sshkey *signkey, const char *filename, int fd,
 	struct sshbuf *sigbuf = NULL, *abuf = NULL;
 	int r = SSH_ERR_INTERNAL_ERROR, wfd = -1, oerrno;
 	char *wfile = NULL, *asig = NULL, *fp = NULL;
+	char *pin = NULL, *prompt = NULL;
 
 	if (!quiet) {
 		if (fd == STDIN_FILENO)
@@ -2533,17 +2543,25 @@ sign_one(struct sshkey *signkey, const char *filename, int fd,
 		else
 			fprintf(stderr, "Signing file %s\n", filename);
 	}
-	if (signer == NULL && sshkey_is_sk(signkey) &&
-	    (signkey->sk_flags & SSH_SK_USER_PRESENCE_REQD)) {
-		if ((fp = sshkey_fingerprint(signkey, fingerprint_hash,
-		    SSH_FP_DEFAULT)) == NULL)
-			fatal("%s: sshkey_fingerprint failed", __func__);
-		fprintf(stderr, "Confirm user presence for key %s %s\n",
-		    sshkey_type(signkey), fp);
-		free(fp);
+	if (signer == NULL && sshkey_is_sk(signkey)) {
+		if ((signkey->sk_flags & SSH_SK_USER_VERIFICATION_REQD)) {
+			xasprintf(&prompt, "Enter PIN for %s key: ",
+			    sshkey_type(signkey));
+			if ((pin = read_passphrase(prompt,
+			    RP_ALLOW_STDIN)) == NULL)
+				fatal("%s: couldn't read PIN", __func__);
+		}
+		if ((signkey->sk_flags & SSH_SK_USER_PRESENCE_REQD)) {
+			if ((fp = sshkey_fingerprint(signkey, fingerprint_hash,
+			    SSH_FP_DEFAULT)) == NULL)
+				fatal("%s: fingerprint failed", __func__);
+			fprintf(stderr, "Confirm user presence for key %s %s\n",
+			    sshkey_type(signkey), fp);
+			free(fp);
+		}
 	}
-	if ((r = sshsig_sign_fd(signkey, NULL, sk_provider, fd, sig_namespace,
-	    &sigbuf, signer, signer_ctx)) != 0) {
+	if ((r = sshsig_sign_fd(signkey, NULL, sk_provider, pin,
+	    fd, sig_namespace, &sigbuf, signer, signer_ctx)) != 0) {
 		error("Signing %s failed: %s", filename, ssh_err(r));
 		goto out;
 	}
@@ -2591,7 +2609,10 @@ sign_one(struct sshkey *signkey, const char *filename, int fd,
 	r = 0;
  out:
 	free(wfile);
+	free(prompt);
 	free(asig);
+	if (pin != NULL)
+		freezero(pin, strlen(pin));
 	sshbuf_free(abuf);
 	sshbuf_free(sigbuf);
 	if (wfd != -1)
@@ -3554,6 +3575,8 @@ main(int argc, char **argv)
 		for (i = 0; i < nopts; i++) {
 			if (strcasecmp(opts[i], "no-touch-required") == 0) {
 				sk_flags &= ~SSH_SK_USER_PRESENCE_REQD;
+			} else if (strcasecmp(opts[i], "verify-required") == 0) {
+				sk_flags |= SSH_SK_USER_VERIFICATION_REQD;
 			} else if (strcasecmp(opts[i], "resident") == 0) {
 				sk_flags |= SSH_SK_RESIDENT_KEY;
 			} else if (strncasecmp(opts[i], "device=", 7) == 0) {
diff --git a/ssh-keysign.c b/ssh-keysign.c
index 3e3ea3e14..7991e0f01 100644
--- a/ssh-keysign.c
+++ b/ssh-keysign.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-keysign.c,v 1.63 2019/11/18 16:10:05 naddy Exp $ */
+/* $OpenBSD: ssh-keysign.c,v 1.64 2020/08/27 01:06:18 djm Exp $ */
 /*
  * Copyright (c) 2002 Markus Friedl.  All rights reserved.
  *
@@ -278,7 +278,7 @@ main(int argc, char **argv)
 	}
 
 	if ((r = sshkey_sign(keys[i], &signature, &slen, data, dlen,
-	    NULL, NULL, 0)) != 0)
+	    NULL, NULL, NULL, 0)) != 0)
 		fatal("sshkey_sign failed: %s", ssh_err(r));
 	free(data);
 
diff --git a/ssh_api.c b/ssh_api.c
index a0358d4be..129404b20 100644
--- a/ssh_api.c
+++ b/ssh_api.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh_api.c,v 1.20 2020/07/01 16:28:31 markus Exp $ */
+/* $OpenBSD: ssh_api.c,v 1.21 2020/08/27 01:06:18 djm Exp $ */
 /*
  * Copyright (c) 2012 Markus Friedl.  All rights reserved.
  *
@@ -54,7 +54,7 @@ int	_ssh_host_key_sign(struct ssh *, struct sshkey *, struct sshkey *,
  */
 int	use_privsep = 0;
 int	mm_sshkey_sign(struct sshkey *, u_char **, u_int *,
-    const u_char *, u_int, const char *, const char *, u_int);
+    const u_char *, u_int, const char *, const char *, const char *, u_int);
 
 #ifdef WITH_OPENSSL
 DH	*mm_choose_dh(int, int, int);
@@ -66,8 +66,8 @@ u_int session_id2_len = 0;
 
 int
 mm_sshkey_sign(struct sshkey *key, u_char **sigp, u_int *lenp,
-    const u_char *data, u_int datalen, const char *alg, const char *sk_provider,
-    u_int compat)
+    const u_char *data, u_int datalen, const char *alg,
+    const char *sk_provider, const char *sk_pin, u_int compat)
 {
 	return (-1);
 }
@@ -567,5 +567,5 @@ _ssh_host_key_sign(struct ssh *ssh, struct sshkey *privkey,
     const u_char *data, size_t dlen, const char *alg)
 {
 	return sshkey_sign(privkey, signature, slen, data, dlen,
-	    alg, NULL, ssh->compat);
+	    alg, NULL, NULL, ssh->compat);
 }
diff --git a/sshconnect2.c b/sshconnect2.c
index 74946da0d..347e348c6 100644
--- a/sshconnect2.c
+++ b/sshconnect2.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: sshconnect2.c,v 1.324 2020/06/27 13:39:09 bket Exp $ */
+/* $OpenBSD: sshconnect2.c,v 1.325 2020/08/27 01:06:18 djm Exp $ */
 /*
  * Copyright (c) 2000 Markus Friedl.  All rights reserved.
  * Copyright (c) 2008 Damien Miller.  All rights reserved.
@@ -1175,7 +1175,7 @@ identity_sign(struct identity *id, u_char **sigp, size_t *lenp,
 	struct sshkey *sign_key = NULL, *prv = NULL;
 	int r = SSH_ERR_INTERNAL_ERROR;
 	struct notifier_ctx *notifier = NULL;
-	char *fp = NULL;
+	char *fp = NULL, *pin = NULL, *prompt = NULL;
 
 	*sigp = NULL;
 	*lenp = 0;
@@ -1204,20 +1204,28 @@ identity_sign(struct identity *id, u_char **sigp, size_t *lenp,
 			goto out;
 		}
 		sign_key = prv;
-		if (sshkey_is_sk(sign_key) &&
-		    (sign_key->sk_flags & SSH_SK_USER_PRESENCE_REQD)) {
-			/* XXX match batch mode should just skip these keys? */
-			if ((fp = sshkey_fingerprint(sign_key,
-			    options.fingerprint_hash, SSH_FP_DEFAULT)) == NULL)
-				fatal("%s: sshkey_fingerprint", __func__);
-			notifier = notify_start(options.batch_mode,
-			    "Confirm user presence for key %s %s",
-			    sshkey_type(sign_key), fp);
-			free(fp);
+		if (sshkey_is_sk(sign_key)) {
+			if ((sign_key->sk_flags &
+			    SSH_SK_USER_VERIFICATION_REQD)) {
+				xasprintf(&prompt, "Enter PIN for %s key %s: ",
+				    sshkey_type(sign_key), id->filename);
+				pin = read_passphrase(prompt, 0);
+			}
+			if ((sign_key->sk_flags & SSH_SK_USER_PRESENCE_REQD)) {
+				/* XXX should batch mode just skip these? */
+				if ((fp = sshkey_fingerprint(sign_key,
+				    options.fingerprint_hash,
+				    SSH_FP_DEFAULT)) == NULL)
+					fatal("%s: fingerprint", __func__);
+				notifier = notify_start(options.batch_mode,
+				    "Confirm user presence for key %s %s",
+				    sshkey_type(sign_key), fp);
+				free(fp);
+			}
 		}
 	}
 	if ((r = sshkey_sign(sign_key, sigp, lenp, data, datalen,
-	    alg, options.sk_provider, compat)) != 0) {
+	    alg, options.sk_provider, pin, compat)) != 0) {
 		debug("%s: sshkey_sign: %s", __func__, ssh_err(r));
 		goto out;
 	}
@@ -1232,6 +1240,9 @@ identity_sign(struct identity *id, u_char **sigp, size_t *lenp,
 	/* success */
 	r = 0;
  out:
+	free(prompt);
+	if (pin != NULL)
+		freezero(pin, strlen(pin));
 	notify_complete(notifier);
 	sshkey_free(prv);
 	return r;
diff --git a/sshd.c b/sshd.c
index d9a159f6a..8aa7f3df6 100644
--- a/sshd.c
+++ b/sshd.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: sshd.c,v 1.560 2020/07/03 10:12:26 markus Exp $ */
+/* $OpenBSD: sshd.c,v 1.561 2020/08/27 01:06:19 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -2338,19 +2338,19 @@ sshd_hostkey_sign(struct ssh *ssh, struct sshkey *privkey,
 	if (use_privsep) {
 		if (privkey) {
 			if (mm_sshkey_sign(ssh, privkey, signature, slenp,
-			    data, dlen, alg, options.sk_provider,
+			    data, dlen, alg, options.sk_provider, NULL,
 			    ssh->compat) < 0)
 				fatal("%s: privkey sign failed", __func__);
 		} else {
 			if (mm_sshkey_sign(ssh, pubkey, signature, slenp,
-			    data, dlen, alg, options.sk_provider,
+			    data, dlen, alg, options.sk_provider, NULL,
 			    ssh->compat) < 0)
 				fatal("%s: pubkey sign failed", __func__);
 		}
 	} else {
 		if (privkey) {
 			if (sshkey_sign(privkey, signature, slenp, data, dlen,
-			    alg, options.sk_provider, ssh->compat) < 0)
+			    alg, options.sk_provider, NULL, ssh->compat) < 0)
 				fatal("%s: privkey sign failed", __func__);
 		} else {
 			if ((r = ssh_agent_sign(auth_sock, pubkey,
diff --git a/sshkey.c b/sshkey.c
index 10b9e4676..ac451f1a8 100644
--- a/sshkey.c
+++ b/sshkey.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: sshkey.c,v 1.110 2020/06/24 15:07:33 markus Exp $ */
+/* $OpenBSD: sshkey.c,v 1.111 2020/08/27 01:06:19 djm Exp $ */
 /*
  * Copyright (c) 2000, 2001 Markus Friedl.  All rights reserved.
  * Copyright (c) 2008 Alexander von Gernler.  All rights reserved.
@@ -2727,7 +2727,7 @@ int
 sshkey_sign(struct sshkey *key,
     u_char **sigp, size_t *lenp,
     const u_char *data, size_t datalen,
-    const char *alg, const char *sk_provider, u_int compat)
+    const char *alg, const char *sk_provider, const char *sk_pin, u_int compat)
 {
 	int was_shielded = sshkey_is_shielded(key);
 	int r2, r = SSH_ERR_INTERNAL_ERROR;
@@ -2766,7 +2766,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, /* XXX PIN */ NULL);
+		    datalen, compat, sk_pin);
 		break;
 #ifdef WITH_XMSS
 	case KEY_XMSS:
@@ -2888,7 +2888,8 @@ sshkey_drop_cert(struct sshkey *k)
 /* Sign a certified key, (re-)generating the signed certblob. */
 int
 sshkey_certify_custom(struct sshkey *k, struct sshkey *ca, const char *alg,
-    const char *sk_provider, sshkey_certify_signer *signer, void *signer_ctx)
+    const char *sk_provider, const char *sk_pin,
+    sshkey_certify_signer *signer, void *signer_ctx)
 {
 	struct sshbuf *principals = NULL;
 	u_char *ca_blob = NULL, *sig_blob = NULL, nonce[32];
@@ -3026,7 +3027,7 @@ sshkey_certify_custom(struct sshkey *k, struct sshkey *ca, const char *alg,
 
 	/* Sign the whole mess */
 	if ((ret = signer(ca, &sig_blob, &sig_len, sshbuf_ptr(cert),
-	    sshbuf_len(cert), alg, sk_provider, 0, signer_ctx)) != 0)
+	    sshbuf_len(cert), alg, sk_provider, sk_pin, 0, signer_ctx)) != 0)
 		goto out;
 	/* Check and update signature_type against what was actually used */
 	if ((ret = sshkey_get_sigtype(sig_blob, sig_len, &sigtype)) != 0)
@@ -3056,19 +3057,20 @@ sshkey_certify_custom(struct sshkey *k, struct sshkey *ca, const char *alg,
 static int
 default_key_sign(struct sshkey *key, u_char **sigp, size_t *lenp,
     const u_char *data, size_t datalen,
-    const char *alg, const char *sk_provider, u_int compat, void *ctx)
+    const char *alg, const char *sk_provider, const char *sk_pin,
+    u_int compat, void *ctx)
 {
 	if (ctx != NULL)
 		return SSH_ERR_INVALID_ARGUMENT;
 	return sshkey_sign(key, sigp, lenp, data, datalen, alg,
-	    sk_provider, compat);
+	    sk_provider, sk_pin, compat);
 }
 
 int
 sshkey_certify(struct sshkey *k, struct sshkey *ca, const char *alg,
-    const char *sk_provider)
+    const char *sk_provider, const char *sk_pin)
 {
-	return sshkey_certify_custom(k, ca, alg, sk_provider,
+	return sshkey_certify_custom(k, ca, alg, sk_provider, sk_pin,
 	    default_key_sign, NULL);
 }
 
diff --git a/sshkey.h b/sshkey.h
index 9c1d4f637..2d8b62497 100644
--- a/sshkey.h
+++ b/sshkey.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: sshkey.h,v 1.45 2020/04/08 00:08:46 djm Exp $ */
+/* $OpenBSD: sshkey.h,v 1.46 2020/08/27 01:06:19 djm Exp $ */
 
 /*
  * Copyright (c) 2000, 2001 Markus Friedl.  All rights reserved.
@@ -200,12 +200,13 @@ size_t	 sshkey_format_cert_validity(const struct sshkey_cert *,
 int	 sshkey_check_cert_sigtype(const struct sshkey *, const char *);
 
 int	 sshkey_certify(struct sshkey *, struct sshkey *,
-    const char *, const char *);
+    const char *, const char *, const char *);
 /* Variant allowing use of a custom signature function (e.g. for ssh-agent) */
 typedef int sshkey_certify_signer(struct sshkey *, u_char **, size_t *,
-    const u_char *, size_t, const char *, const char *, u_int, void *);
+    const u_char *, size_t, const char *, const char *, const char *,
+    u_int, void *);
 int	 sshkey_certify_custom(struct sshkey *, struct sshkey *, const char *,
-    const char *, sshkey_certify_signer *, void *);
+    const char *, const char *, sshkey_certify_signer *, void *);
 
 int		 sshkey_ecdsa_nid_from_name(const char *);
 int		 sshkey_curve_name_to_nid(const char *);
@@ -234,7 +235,7 @@ int	 sshkey_plain_to_blob(const struct sshkey *, u_char **, size_t *);
 int	 sshkey_putb_plain(const struct sshkey *, struct sshbuf *);
 
 int	 sshkey_sign(struct sshkey *, u_char **, size_t *,
-    const u_char *, size_t, const char *, const char *, u_int);
+    const u_char *, size_t, const char *, const char *, const char *, u_int);
 int	 sshkey_verify(const struct sshkey *, const u_char *, size_t,
     const u_char *, size_t, const char *, u_int, struct sshkey_sig_details **);
 int	 sshkey_check_sigtype(const u_char *, size_t, const char *);
diff --git a/sshsig.c b/sshsig.c
index 15f9cead6..658b8c852 100644
--- a/sshsig.c
+++ b/sshsig.c
@@ -151,7 +151,7 @@ done:
 
 static int
 sshsig_wrap_sign(struct sshkey *key, const char *hashalg,
-    const char *sk_provider, const struct sshbuf *h_message,
+    const char *sk_provider, const char *sk_pin, const struct sshbuf *h_message,
     const char *sig_namespace, struct sshbuf **out,
     sshsig_signer *signer, void *signer_ctx)
 {
@@ -185,14 +185,14 @@ sshsig_wrap_sign(struct sshkey *key, const char *hashalg,
 	if (signer != NULL) {
 		if ((r = signer(key, &sig, &slen,
 		    sshbuf_ptr(tosign), sshbuf_len(tosign),
-		    sign_alg, sk_provider, 0, signer_ctx)) != 0) {
+		    sign_alg, sk_provider, sk_pin, 0, signer_ctx)) != 0) {
 			error("Couldn't sign message: %s", ssh_err(r));
 			goto done;
 		}
 	} else {
 		if ((r = sshkey_sign(key, &sig, &slen,
 		    sshbuf_ptr(tosign), sshbuf_len(tosign),
-		    sign_alg, sk_provider, 0)) != 0) {
+		    sign_alg, sk_provider, sk_pin, 0)) != 0) {
 			error("Couldn't sign message: %s", ssh_err(r));
 			goto done;
 		}
@@ -430,7 +430,8 @@ hash_buffer(const struct sshbuf *m, const char *hashalg, struct sshbuf **bp)
 }
 
 int
-sshsig_signb(struct sshkey *key, const char *hashalg, const char *sk_provider,
+sshsig_signb(struct sshkey *key, const char *hashalg,
+    const char *sk_provider, const char *sk_pin,
     const struct sshbuf *message, const char *sig_namespace,
     struct sshbuf **out, sshsig_signer *signer, void *signer_ctx)
 {
@@ -445,7 +446,7 @@ sshsig_signb(struct sshkey *key, const char *hashalg, const char *sk_provider,
 		error("%s: hash_buffer failed: %s", __func__, ssh_err(r));
 		goto out;
 	}
-	if ((r = sshsig_wrap_sign(key, hashalg, sk_provider, b,
+	if ((r = sshsig_wrap_sign(key, hashalg, sk_provider, sk_pin, b,
 	    sig_namespace, out, signer, signer_ctx)) != 0)
 		goto out;
 	/* success */
@@ -558,7 +559,8 @@ hash_file(int fd, const char *hashalg, struct sshbuf **bp)
 }
 
 int
-sshsig_sign_fd(struct sshkey *key, const char *hashalg, const char *sk_provider,
+sshsig_sign_fd(struct sshkey *key, const char *hashalg,
+    const char *sk_provider, const char *sk_pin,
     int fd, const char *sig_namespace, struct sshbuf **out,
     sshsig_signer *signer, void *signer_ctx)
 {
@@ -573,7 +575,7 @@ sshsig_sign_fd(struct sshkey *key, const char *hashalg, const char *sk_provider,
 		error("%s: hash_file failed: %s", __func__, ssh_err(r));
 		return r;
 	}
-	if ((r = sshsig_wrap_sign(key, hashalg, sk_provider, b,
+	if ((r = sshsig_wrap_sign(key, hashalg, sk_provider, sk_pin, b,
 	    sig_namespace, out, signer, signer_ctx)) != 0)
 		goto out;
 	/* success */
diff --git a/sshsig.h b/sshsig.h
index 63cc1ad1a..44157bc2a 100644
--- a/sshsig.h
+++ b/sshsig.h
@@ -23,7 +23,8 @@ struct sshsigopt;
 struct sshkey_sig_details;
 
 typedef int sshsig_signer(struct sshkey *, u_char **, size_t *,
-    const u_char *, size_t, const char *, const char *, u_int, void *);
+    const u_char *, size_t, const char *, const char *, const char *,
+    u_int, void *);
 
 /* Buffer-oriented API */
 
@@ -33,7 +34,7 @@ typedef int sshsig_signer(struct sshkey *, u_char **, size_t *,
  * out is populated with the detached signature, or NULL on failure.
  */
 int sshsig_signb(struct sshkey *key, const char *hashalg,
-    const char *sk_provider, const struct sshbuf *message,
+    const char *sk_provider, const char *sk_pin, const struct sshbuf *message,
     const char *sig_namespace, struct sshbuf **out,
     sshsig_signer *signer, void *signer_ctx);
 
@@ -54,7 +55,8 @@ int sshsig_verifyb(struct sshbuf *signature,
  * out is populated with the detached signature, or NULL on failure.
  */
 int sshsig_sign_fd(struct sshkey *key, const char *hashalg,
-    const char *sk_provider, int fd, const char *sig_namespace,
+    const char *sk_provider, const char *sk_pin,
+    int fd, const char *sig_namespace,
     struct sshbuf **out, sshsig_signer *signer, void *signer_ctx);
 
 /*
-- 
cgit v1.2.3


From 642e06d0df983fa2af85126cf4b23440bb2985bf Mon Sep 17 00:00:00 2001
From: "djm@openbsd.org" <djm@openbsd.org>
Date: Thu, 27 Aug 2020 01:07:51 +0000
Subject: upstream: major rework of FIDO token selection logic

When PINs are in use and multiple FIDO tokens are attached to a host, we
cannot just blast requests at all attached tokens with the PIN specified
as this will cause the per-token PIN failure counter to increment. If
this retry counter hits the token's limit (usually 3 attempts), then the
token will lock itself and render all (web and SSH) of its keys invalid.
We don't want this.

So this reworks the key selection logic for the specific case of
multiple keys being attached. When multiple keys are attached and the
operation requires a PIN, then the user must touch the key that they
wish to use first in order to identify it.

This may require multiple touches, but only if there are multiple keys
attached AND (usually) the operation requires a PIN. The usual case of a
single key attached should be unaffected.

Work by Pedro Martelletto; ok myself and markus@

OpenBSD-Commit-ID: 637d3049ced61b7a9ee796914bbc4843d999a864
---
 sk-usbhid.c  | 578 +++++++++++++++++++++++++++++++++--------------------------
 ssh-keygen.c |   7 +-
 2 files changed, 332 insertions(+), 253 deletions(-)

(limited to 'sk-usbhid.c')

diff --git a/sk-usbhid.c b/sk-usbhid.c
index 1dd834883..2efb377c5 100644
--- a/sk-usbhid.c
+++ b/sk-usbhid.c
@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2019 Markus Friedl
+ * Copyright (c) 2020 Pedro Martelletto
  *
  * Permission to use, copy, modify, and distribute this software for any
  * purpose with or without fee is hereby granted, provided that the above
@@ -43,6 +44,7 @@
 #ifndef SK_STANDALONE
 # include "log.h"
 # include "xmalloc.h"
+# include "misc.h"
 /*
  * If building as part of OpenSSH, then rename exported functions.
  * This must be done before including sk-api.h.
@@ -63,7 +65,10 @@
 #define SSH_FIDO_INIT_ARG	0
 #endif
 
-#define MAX_FIDO_DEVICES	256
+#define MAX_FIDO_DEVICES	8
+#define FIDO_POLL_MS		50
+#define SELECT_MS		15000
+#define POLL_SLEEP_NS		200000000
 
 /* Compatibility with OpenSSH 1.0.x */
 #if (OPENSSL_VERSION_NUMBER < 0x10100000L)
@@ -74,6 +79,11 @@
 	} while (0)
 #endif
 
+struct sk_usbhid {
+	fido_dev_t *dev;
+	char *path;
+};
+
 /* Return the version of the middleware API */
 uint32_t sk_api_version(void);
 
@@ -127,54 +137,185 @@ sk_api_version(void)
 	return SSH_SK_VERSION_MAJOR;
 }
 
-/* Select the first identified FIDO device attached to the system */
-static char *
-pick_first_device(void)
+static struct sk_usbhid *
+sk_open(const char *path)
 {
-	char *ret = NULL;
-	fido_dev_info_t *devlist = NULL;
-	size_t olen = 0;
+	struct sk_usbhid *sk;
 	int r;
-	const fido_dev_info_t *di;
 
-	if ((devlist = fido_dev_info_new(1)) == NULL) {
-		skdebug(__func__, "fido_dev_info_new failed");
-		goto out;
+	if (path == NULL) {
+		skdebug(__func__, "path == NULL");
+		return NULL;
 	}
-	if ((r = fido_dev_info_manifest(devlist, 1, &olen)) != FIDO_OK) {
-		skdebug(__func__, "fido_dev_info_manifest failed: %s",
+	if ((sk = calloc(1, sizeof(*sk))) == NULL) {
+		skdebug(__func__, "calloc sk failed");
+		return NULL;
+	}
+	if ((sk->path = strdup(path)) == NULL) {
+		skdebug(__func__, "strdup path failed");
+		free(sk);
+		return NULL;
+	}
+	if ((sk->dev = fido_dev_new()) == NULL) {
+		skdebug(__func__, "fido_dev_new failed");
+		free(sk->path);
+		free(sk);
+		return NULL;
+	}
+	if ((r = fido_dev_open(sk->dev, sk->path)) != FIDO_OK) {
+		skdebug(__func__, "fido_dev_open %s failed: %s", sk->path,
 		    fido_strerr(r));
-		goto out;
+		fido_dev_free(&sk->dev);
+		free(sk->path);
+		free(sk);
+		return NULL;
 	}
-	if (olen != 1) {
-		skdebug(__func__, "fido_dev_info_manifest bad len %zu", olen);
-		goto out;
+	return sk;
+}
+
+static void
+sk_close(struct sk_usbhid *sk)
+{
+	if (sk == NULL)
+		return;
+	fido_dev_cancel(sk->dev); /* cancel any pending operation */
+	fido_dev_close(sk->dev);
+	fido_dev_free(&sk->dev);
+	free(sk->path);
+	free(sk);
+}
+
+static struct sk_usbhid **
+sk_openv(const fido_dev_info_t *devlist, size_t ndevs, size_t *nopen)
+{
+	const fido_dev_info_t *di;
+	struct sk_usbhid **skv;
+	size_t i;
+
+	*nopen = 0;
+	if ((skv = calloc(ndevs, sizeof(*skv))) == NULL) {
+		skdebug(__func__, "calloc skv failed");
+		return NULL;
 	}
-	di = fido_dev_info_ptr(devlist, 0);
-	if ((ret = strdup(fido_dev_info_path(di))) == NULL) {
-		skdebug(__func__, "fido_dev_info_path failed");
-		goto out;
+	for (i = 0; i < ndevs; i++) {
+		if ((di = fido_dev_info_ptr(devlist, i)) == NULL)
+			skdebug(__func__, "fido_dev_info_ptr failed");
+		else if ((skv[*nopen] = sk_open(fido_dev_info_path(di))) == NULL)
+			skdebug(__func__, "sk_open failed");
+		else
+			(*nopen)++;
 	}
- out:
-	fido_dev_info_free(&devlist, 1);
-	return ret;
+	if (*nopen == 0) {
+		for (i = 0; i < ndevs; i++)
+			sk_close(skv[i]);
+		free(skv);
+		skv = NULL;
+	}
+
+	return skv;
+}
+
+static void
+sk_closev(struct sk_usbhid **skv, size_t nsk)
+{
+	size_t i;
+
+	for (i = 0; i < nsk; i++)
+		sk_close(skv[i]);
+	free(skv);
 }
 
-/* Check if the specified key handle exists on a given device. */
 static int
-try_device(fido_dev_t *dev, const uint8_t *message, size_t message_len,
-    const char *application, const uint8_t *key_handle, size_t key_handle_len,
-    uint8_t flags, const char *pin)
+sk_touch_begin(struct sk_usbhid **skv, size_t nsk)
+{
+	size_t i, ok = 0;
+	int r;
+
+	for (i = 0; i < nsk; i++)
+		if ((r = fido_dev_get_touch_begin(skv[i]->dev)) != FIDO_OK)
+			skdebug(__func__, "fido_dev_get_touch_begin %s failed:"
+			    " %s", skv[i]->path, fido_strerr(r));
+		else
+			ok++;
+
+	return ok ? 0 : -1;
+}
+
+static int
+sk_touch_poll(struct sk_usbhid **skv, size_t nsk, int *touch, size_t *idx)
+{
+	struct timespec ts_pause;
+	size_t npoll, i;
+	int r;
+
+	ts_pause.tv_sec = 0;
+	ts_pause.tv_nsec = POLL_SLEEP_NS;
+	nanosleep(&ts_pause, NULL);
+	npoll = nsk;
+	for (i = 0; i < nsk; i++) {
+		if (skv[i] == NULL)
+			continue; /* device discarded */
+		skdebug(__func__, "polling %s", skv[i]->path);
+		if ((r = fido_dev_get_touch_status(skv[i]->dev, touch,
+		    FIDO_POLL_MS)) != FIDO_OK) {
+			skdebug(__func__, "fido_dev_get_touch_status %s: %s",
+			    skv[i]->path, fido_strerr(r));
+			sk_close(skv[i]); /* discard device */
+			skv[i] = NULL;
+			if (--npoll == 0) {
+				skdebug(__func__, "no device left to poll");
+				return -1;
+			}
+		} else if (*touch) {
+			*idx = i;
+			return 0;
+		}
+	}
+	*touch = 0;
+	return 0;
+}
+
+/* Calculate SHA256(m) */
+static int
+sha256_mem(const void *m, size_t mlen, u_char *d, size_t dlen)
+{
+#ifdef WITH_OPENSSL
+	u_int mdlen;
+#endif
+
+	if (dlen != 32)
+		return -1;
+#ifdef WITH_OPENSSL
+	mdlen = dlen;
+	if (!EVP_Digest(m, mlen, d, &mdlen, EVP_sha256(), NULL))
+		return -1;
+#else
+	SHA256Data(m, mlen, d);
+#endif
+	return 0;
+}
+
+/* Check if the specified key handle exists on a given sk. */
+static int
+sk_try(const struct sk_usbhid *sk, const char *application,
+    const uint8_t *key_handle, size_t key_handle_len)
 {
 	fido_assert_t *assert = NULL;
+	/* generate an invalid signature on FIDO2 tokens */
+	const char *data = "";
+	uint8_t message[32];
 	int r = FIDO_ERR_INTERNAL;
 
+	if (sha256_mem(data, strlen(data), message, sizeof(message)) != 0) {
+		skdebug(__func__, "hash message failed");
+		goto out;
+	}
 	if ((assert = fido_assert_new()) == NULL) {
 		skdebug(__func__, "fido_assert_new failed");
 		goto out;
 	}
 	if ((r = fido_assert_set_clientdata_hash(assert, message,
-	    message_len)) != FIDO_OK) {
+	    sizeof(message))) != FIDO_OK) {
 		skdebug(__func__, "fido_assert_set_clientdata_hash: %s",
 		    fido_strerr(r));
 		goto out;
@@ -192,7 +333,7 @@ try_device(fido_dev_t *dev, const uint8_t *message, size_t message_len,
 		skdebug(__func__, "fido_assert_up: %s", fido_strerr(r));
 		goto out;
 	}
-	r = fido_dev_get_assert(dev, assert, pin);
+	r = fido_dev_get_assert(sk->dev, assert, NULL);
 	skdebug(__func__, "fido_dev_get_assert: %s", fido_strerr(r));
 	if (r == FIDO_ERR_USER_PRESENCE_REQUIRED) {
 		/* U2F tokens may return this */
@@ -204,77 +345,110 @@ try_device(fido_dev_t *dev, const uint8_t *message, size_t message_len,
 	return r != FIDO_OK ? -1 : 0;
 }
 
-/* Iterate over configured devices looking for a specific key handle */
-static fido_dev_t *
-find_device(const char *path, const uint8_t *message, size_t message_len,
-    const char *application, const uint8_t *key_handle, size_t key_handle_len,
-    uint8_t flags, const char *pin)
+static struct sk_usbhid *
+sk_select_by_cred(const fido_dev_info_t *devlist, size_t ndevs,
+    const char *application, const uint8_t *key_handle, size_t key_handle_len)
 {
-	fido_dev_info_t *devlist = NULL;
-	fido_dev_t *dev = NULL;
-	size_t devlist_len = 0, i;
-	int r;
-
-	if (path != NULL) {
-		if ((dev = fido_dev_new()) == NULL) {
-			skdebug(__func__, "fido_dev_new failed");
-			return NULL;
-		}
-		if ((r = fido_dev_open(dev, path)) != FIDO_OK) {
-			skdebug(__func__, "fido_dev_open failed");
-			fido_dev_free(&dev);
-			return NULL;
+	struct sk_usbhid **skv, *sk;
+	size_t skvcnt, i;
+
+	if ((skv = sk_openv(devlist, ndevs, &skvcnt)) == NULL) {
+		skdebug(__func__, "sk_openv failed");
+		return NULL;
+	}
+	sk = NULL;
+	for (i = 0; i < skvcnt; i++)
+		if (sk_try(skv[i], application, key_handle,
+		    key_handle_len) == 0) {
+			sk = skv[i];
+			skv[i] = NULL;
+			skdebug(__func__, "found key in %s", sk->path);
+			break;
 		}
-		return dev;
-	}
+	sk_closev(skv, skvcnt);
+	return sk;
+}
 
-	if ((devlist = fido_dev_info_new(MAX_FIDO_DEVICES)) == NULL) {
-		skdebug(__func__, "fido_dev_info_new failed");
+static struct sk_usbhid *
+sk_select_by_touch(const fido_dev_info_t *devlist, size_t ndevs)
+{
+	struct sk_usbhid **skv, *sk;
+	struct timeval tv_start, tv_now, tv_delta;
+	size_t skvcnt, idx;
+	int touch, ms_remain;
+
+	if ((skv = sk_openv(devlist, ndevs, &skvcnt)) == NULL) {
+		skdebug(__func__, "sk_openv failed");
+		return NULL;
+	}
+	sk = NULL;
+	if (skvcnt < 2) {
+		if (skvcnt == 1) {
+			/* single candidate */
+			sk = skv[0];
+			skv[0] = NULL;
+		}
 		goto out;
 	}
-	if ((r = fido_dev_info_manifest(devlist, MAX_FIDO_DEVICES,
-	    &devlist_len)) != FIDO_OK) {
-		skdebug(__func__, "fido_dev_info_manifest: %s", fido_strerr(r));
+	if (sk_touch_begin(skv, skvcnt) == -1) {
+		skdebug(__func__, "sk_touch_begin failed");
 		goto out;
 	}
-
-	skdebug(__func__, "found %zu device(s)", devlist_len);
-
-	for (i = 0; i < devlist_len; i++) {
-		const fido_dev_info_t *di = fido_dev_info_ptr(devlist, i);
-
-		if (di == NULL) {
-			skdebug(__func__, "fido_dev_info_ptr %zu failed", i);
-			continue;
-		}
-		if ((path = fido_dev_info_path(di)) == NULL) {
-			skdebug(__func__, "fido_dev_info_path %zu failed", i);
-			continue;
-		}
-		skdebug(__func__, "trying device %zu: %s", i, path);
-		if ((dev = fido_dev_new()) == NULL) {
-			skdebug(__func__, "fido_dev_new failed");
-			continue;
-		}
-		if ((r = fido_dev_open(dev, path)) != FIDO_OK) {
-			skdebug(__func__, "fido_dev_open failed");
-			fido_dev_free(&dev);
-			continue;
+	monotime_tv(&tv_start);
+	do {
+		if (sk_touch_poll(skv, skvcnt, &touch, &idx) == -1) {
+			skdebug(__func__, "sk_touch_poll failed");
+			goto out;
 		}
-		if (try_device(dev, message, message_len, application,
-		    key_handle, key_handle_len, flags, pin) == 0) {
-			skdebug(__func__, "found key");
-			break;
+		if (touch) {
+			sk = skv[idx];
+			skv[idx] = NULL;
+			goto out;
 		}
-		fido_dev_close(dev);
-		fido_dev_free(&dev);
-	}
+		monotime_tv(&tv_now);
+		timersub(&tv_now, &tv_start, &tv_delta);
+		ms_remain = SELECT_MS - tv_delta.tv_sec * 1000 -
+		    tv_delta.tv_usec / 1000;
+	} while (ms_remain >= FIDO_POLL_MS);
+	skdebug(__func__, "timeout");
+out:
+	sk_closev(skv, skvcnt);
+	return sk;
+}
 
- out:
-	if (devlist != NULL)
-		fido_dev_info_free(&devlist, MAX_FIDO_DEVICES);
+static struct sk_usbhid *
+sk_probe(const char *application, const uint8_t *key_handle,
+    size_t key_handle_len)
+{
+	struct sk_usbhid *sk;
+	fido_dev_info_t *devlist;
+	size_t ndevs;
+	int r;
 
-	return dev;
+	if ((devlist = fido_dev_info_new(MAX_FIDO_DEVICES)) == NULL) {
+		skdebug(__func__, "fido_dev_info_new failed");
+		return NULL;
+	}
+	if ((r = fido_dev_info_manifest(devlist, MAX_FIDO_DEVICES,
+	    &ndevs)) != FIDO_OK) {
+		skdebug(__func__, "fido_dev_info_manifest failed: %s",
+		    fido_strerr(r));
+		fido_dev_info_free(&devlist, MAX_FIDO_DEVICES);
+		return NULL;
+	}
+	skdebug(__func__, "%zu device(s) detected", ndevs);
+	if (ndevs == 0) {
+		sk = NULL;
+	} else if (application != NULL && key_handle != NULL) {
+		skdebug(__func__, "selecting sk by cred");
+		sk = sk_select_by_cred(devlist, ndevs, application, key_handle,
+		    key_handle_len);
+	} else {
+		skdebug(__func__, "selecting sk by touch");
+		sk = sk_select_by_touch(devlist, ndevs);
+	}
+	fido_dev_info_free(&devlist, MAX_FIDO_DEVICES);
+	return sk;
 }
 
 #ifdef WITH_OPENSSL
@@ -451,52 +625,15 @@ check_enroll_options(struct sk_option **options, char **devicep,
 	return 0;
 }
 
-static int
-check_sk_extensions(fido_dev_t *dev, const char *ext, int *ret)
-{
-	fido_cbor_info_t *info;
-	char * const *ptr;
-	size_t len, i;
-	int r;
-
-	*ret = 0;
-
-	if (!fido_dev_is_fido2(dev)) {
-		skdebug(__func__, "device is not fido2");
-		return 0;
-	}
-	if ((info = fido_cbor_info_new()) == NULL) {
-		skdebug(__func__, "fido_cbor_info_new failed");
-		return -1;
-	}
-	if ((r = fido_dev_get_cbor_info(dev, info)) != FIDO_OK) {
-		skdebug(__func__, "fido_dev_get_cbor_info: %s", fido_strerr(r));
-		fido_cbor_info_free(&info);
-		return -1;
-	}
-	ptr = fido_cbor_info_extensions_ptr(info);
-	len = fido_cbor_info_extensions_len(info);
-	for (i = 0; i < len; i++) {
-		if (!strcmp(ptr[i], ext)) {
-			*ret = 1;
-			break;
-		}
-	}
-	fido_cbor_info_free(&info);
-	skdebug(__func__, "extension %s %s", ext, *ret ? "present" : "absent");
-
-	return 0;
-}
-
 int
 sk_enroll(uint32_t alg, const uint8_t *challenge, size_t challenge_len,
     const char *application, uint8_t flags, const char *pin,
     struct sk_option **options, struct sk_enroll_response **enroll_response)
 {
 	fido_cred_t *cred = NULL;
-	fido_dev_t *dev = NULL;
 	const uint8_t *ptr;
 	uint8_t user_id[32];
+	struct sk_usbhid *sk = NULL;
 	struct sk_enroll_response *response = NULL;
 	size_t len;
 	int credprot;
@@ -511,12 +648,12 @@ sk_enroll(uint32_t alg, const uint8_t *challenge, size_t challenge_len,
 		skdebug(__func__, "enroll_response == NULL");
 		goto out;
 	}
+	*enroll_response = NULL;
 	memset(user_id, 0, sizeof(user_id));
-	if (check_enroll_options(options, &device,
-	    user_id, sizeof(user_id)) != 0)
+	if (check_enroll_options(options, &device, user_id,
+	    sizeof(user_id)) != 0)
 		goto out; /* error already logged */
 
-	*enroll_response = NULL;
 	switch(alg) {
 #ifdef WITH_OPENSSL
 	case SSH_SK_ECDSA:
@@ -530,12 +667,15 @@ sk_enroll(uint32_t alg, const uint8_t *challenge, size_t challenge_len,
 		skdebug(__func__, "unsupported key type %d", alg);
 		goto out;
 	}
-	if (device == NULL && (device = pick_first_device()) == NULL) {
-		ret = SSH_SK_ERR_DEVICE_NOT_FOUND;
-		skdebug(__func__, "pick_first_device failed");
+	if (device != NULL)
+		sk = sk_open(device);
+	else
+		sk = sk_probe(NULL, NULL, 0);
+	if (sk == NULL) {
+		skdebug(__func__, "failed to find sk");
 		goto out;
 	}
-	skdebug(__func__, "using device %s", device);
+	skdebug(__func__, "using device %s", sk->path);
 	if ((cred = fido_cred_new()) == NULL) {
 		skdebug(__func__, "fido_cred_new failed");
 		goto out;
@@ -564,22 +704,11 @@ sk_enroll(uint32_t alg, const uint8_t *challenge, size_t challenge_len,
 		skdebug(__func__, "fido_cred_set_rp: %s", fido_strerr(r));
 		goto out;
 	}
-	if ((dev = fido_dev_new()) == NULL) {
-		skdebug(__func__, "fido_dev_new failed");
-		goto out;
-	}
-	if ((r = fido_dev_open(dev, device)) != FIDO_OK) {
-		skdebug(__func__, "fido_dev_open: %s", fido_strerr(r));
-		goto out;
-	}
 	if ((flags & (SSH_SK_RESIDENT_KEY|SSH_SK_USER_VERIFICATION_REQD)) != 0) {
-		if (check_sk_extensions(dev, "credProtect", &credprot) < 0) {
-			skdebug(__func__, "check_sk_extensions failed");
-			goto out;
-		}
-		if (credprot == 0) {
-			skdebug(__func__, "refusing to create unprotected "
-			    "resident/verify-required key");
+		if (!fido_dev_supports_cred_prot(sk->dev)) {
+			skdebug(__func__, "%s does not support credprot, "
+			    "refusing to create unprotected "
+			    "resident/verify-required key", sk->path);
 			ret = SSH_SK_ERR_UNSUPPORTED;
 			goto out;
 		}
@@ -595,7 +724,7 @@ sk_enroll(uint32_t alg, const uint8_t *challenge, size_t challenge_len,
 			goto out;
 		}
 	}
-	if ((r = fido_dev_make_cred(dev, cred, pin)) != FIDO_OK) {
+	if ((r = fido_dev_make_cred(sk->dev, cred, pin)) != FIDO_OK) {
 		skdebug(__func__, "fido_dev_make_cred: %s", fido_strerr(r));
 		ret = fidoerr_to_skerr(r);
 		goto out;
@@ -662,13 +791,8 @@ sk_enroll(uint32_t alg, const uint8_t *challenge, size_t challenge_len,
 		free(response->attestation_cert);
 		free(response);
 	}
-	if (dev != NULL) {
-		fido_dev_close(dev);
-		fido_dev_free(&dev);
-	}
-	if (cred != NULL) {
-		fido_cred_free(&cred);
-	}
+	sk_close(sk);
+	fido_cred_free(&cred);
 	return ret;
 }
 
@@ -782,26 +906,6 @@ check_sign_load_resident_options(struct sk_option **options, char **devicep)
 	return 0;
 }
 
-/* Calculate SHA256(m) */
-static int
-sha256_mem(const void *m, size_t mlen, u_char *d, size_t dlen)
-{
-#ifdef WITH_OPENSSL
-	u_int mdlen;
-#endif
-
-	if (dlen != 32)
-		return -1;
-#ifdef WITH_OPENSSL
-	mdlen = dlen;
-	if (!EVP_Digest(m, mlen, d, &mdlen, EVP_sha256(), NULL))
-		return -1;
-#else
-	SHA256Data(m, mlen, d);
-#endif
-	return 0;
-}
-
 int
 sk_sign(uint32_t alg, const uint8_t *data, size_t datalen,
     const char *application,
@@ -811,7 +915,7 @@ sk_sign(uint32_t alg, const uint8_t *data, size_t datalen,
 {
 	fido_assert_t *assert = NULL;
 	char *device = NULL;
-	fido_dev_t *dev = NULL;
+	struct sk_usbhid *sk = NULL;
 	struct sk_sign_response *response = NULL;
 	uint8_t message[32];
 	int ret = SSH_SK_ERR_GENERAL;
@@ -831,9 +935,14 @@ sk_sign(uint32_t alg, const uint8_t *data, size_t datalen,
 		skdebug(__func__, "hash message failed");
 		goto out;
 	}
-	if ((dev = find_device(device, message, sizeof(message),
-	    application, key_handle, key_handle_len, flags, pin)) == NULL) {
-		skdebug(__func__, "couldn't find device for key handle");
+	if (device != NULL)
+		sk = sk_open(device);
+	else if (pin != NULL || (flags & SSH_SK_USER_VERIFICATION_REQD))
+		sk = sk_probe(NULL, NULL, 0);
+	else
+		sk = sk_probe(application, key_handle, key_handle_len);
+	if (sk == NULL) {
+		skdebug(__func__, "failed to find sk");
 		goto out;
 	}
 	if ((assert = fido_assert_new()) == NULL) {
@@ -867,7 +976,7 @@ sk_sign(uint32_t alg, const uint8_t *data, size_t datalen,
 		ret = FIDO_ERR_PIN_REQUIRED;
 		goto out;
 	}
-	if ((r = fido_dev_get_assert(dev, assert, pin)) != FIDO_OK) {
+	if ((r = fido_dev_get_assert(sk->dev, assert, pin)) != FIDO_OK) {
 		skdebug(__func__, "fido_dev_get_assert: %s", fido_strerr(r));
 		ret = fidoerr_to_skerr(r);
 		goto out;
@@ -893,22 +1002,16 @@ sk_sign(uint32_t alg, const uint8_t *data, size_t datalen,
 		free(response->sig_s);
 		free(response);
 	}
-	if (dev != NULL) {
-		fido_dev_close(dev);
-		fido_dev_free(&dev);
-	}
-	if (assert != NULL) {
-		fido_assert_free(&assert);
-	}
+	sk_close(sk);
+	fido_assert_free(&assert);
 	return ret;
 }
 
 static int
-read_rks(const char *devpath, const char *pin,
+read_rks(struct sk_usbhid *sk, const char *pin,
     struct sk_resident_key ***rksp, size_t *nrksp)
 {
 	int ret = SSH_SK_ERR_GENERAL, r = -1;
-	fido_dev_t *dev = NULL;
 	fido_credman_metadata_t *metadata = NULL;
 	fido_credman_rp_t *rp = NULL;
 	fido_credman_rk_t *rk = NULL;
@@ -916,30 +1019,25 @@ read_rks(const char *devpath, const char *pin,
 	const fido_cred_t *cred;
 	struct sk_resident_key *srk = NULL, **tmp;
 
-	if ((dev = fido_dev_new()) == NULL) {
-		skdebug(__func__, "fido_dev_new failed");
-		return ret;
-	}
-	if ((r = fido_dev_open(dev, devpath)) != FIDO_OK) {
-		skdebug(__func__, "fido_dev_open %s failed: %s",
-		    devpath, fido_strerr(r));
-		fido_dev_free(&dev);
-		return ret;
+	if (pin == NULL) {
+		skdebug(__func__, "no PIN specified");
+		ret = SSH_SK_ERR_PIN_REQUIRED;
+		goto out;
 	}
 	if ((metadata = fido_credman_metadata_new()) == NULL) {
 		skdebug(__func__, "alloc failed");
 		goto out;
 	}
 
-	if ((r = fido_credman_get_dev_metadata(dev, metadata, pin)) != 0) {
+	if ((r = fido_credman_get_dev_metadata(sk->dev, metadata, pin)) != 0) {
 		if (r == FIDO_ERR_INVALID_COMMAND) {
 			skdebug(__func__, "device %s does not support "
-			    "resident keys", devpath);
+			    "resident keys", sk->path);
 			ret = 0;
 			goto out;
 		}
 		skdebug(__func__, "get metadata for %s failed: %s",
-		    devpath, fido_strerr(r));
+		    sk->path, fido_strerr(r));
 		ret = fidoerr_to_skerr(r);
 		goto out;
 	}
@@ -950,14 +1048,14 @@ read_rks(const char *devpath, const char *pin,
 		skdebug(__func__, "alloc rp failed");
 		goto out;
 	}
-	if ((r = fido_credman_get_dev_rp(dev, rp, pin)) != 0) {
+	if ((r = fido_credman_get_dev_rp(sk->dev, rp, pin)) != 0) {
 		skdebug(__func__, "get RPs for %s failed: %s",
-		    devpath, fido_strerr(r));
+		    sk->path, fido_strerr(r));
 		goto out;
 	}
 	nrp = fido_credman_rp_count(rp);
 	skdebug(__func__, "Device %s has resident keys for %zu RPs",
-	    devpath, nrp);
+	    sk->path, nrp);
 
 	/* Iterate over RP IDs that have resident keys */
 	for (i = 0; i < nrp; i++) {
@@ -974,10 +1072,10 @@ read_rks(const char *devpath, const char *pin,
 			skdebug(__func__, "alloc rk failed");
 			goto out;
 		}
-		if ((r = fido_credman_get_dev_rk(dev, fido_credman_rp_id(rp, i),
-		    rk, pin)) != 0) {
+		if ((r = fido_credman_get_dev_rk(sk->dev,
+		    fido_credman_rp_id(rp, i), rk, pin)) != 0) {
 			skdebug(__func__, "get RKs for %s slot %zu failed: %s",
-			    devpath, i, fido_strerr(r));
+			    sk->path, i, fido_strerr(r));
 			goto out;
 		}
 		nrk = fido_credman_rk_count(rk);
@@ -991,7 +1089,7 @@ read_rks(const char *devpath, const char *pin,
 				continue;
 			}
 			skdebug(__func__, "Device %s RP \"%s\" slot %zu: "
-			    "type %d flags 0x%02x prot 0x%02x", devpath,
+			    "type %d flags 0x%02x prot 0x%02x", sk->path,
 			    fido_credman_rp_id(rp, i), j, fido_cred_type(cred),
 			    fido_cred_flags(cred), fido_cred_prot(cred));
 
@@ -1050,8 +1148,6 @@ read_rks(const char *devpath, const char *pin,
 	}
 	fido_credman_rp_free(&rp);
 	fido_credman_rk_free(&rk);
-	fido_dev_close(dev);
-	fido_dev_free(&dev);
 	fido_credman_metadata_free(&metadata);
 	return ret;
 }
@@ -1061,11 +1157,11 @@ sk_load_resident_keys(const char *pin, struct sk_option **options,
     struct sk_resident_key ***rksp, size_t *nrksp)
 {
 	int ret = SSH_SK_ERR_GENERAL, r = -1;
-	fido_dev_info_t *devlist = NULL;
-	size_t i, ndev = 0, nrks = 0;
-	const fido_dev_info_t *di;
+	size_t i, nrks = 0;
 	struct sk_resident_key **rks = NULL;
+	struct sk_usbhid *sk = NULL;
 	char *device = NULL;
+
 	*rksp = NULL;
 	*nrksp = 0;
 
@@ -1073,40 +1169,19 @@ sk_load_resident_keys(const char *pin, struct sk_option **options,
 
 	if (check_sign_load_resident_options(options, &device) != 0)
 		goto out; /* error already logged */
-	if (device != NULL) {
-		skdebug(__func__, "trying %s", device);
-		if ((r = read_rks(device, pin, &rks, &nrks)) != 0) {
-			skdebug(__func__, "read_rks failed for %s", device);
-			ret = r;
-			goto out;
-		}
-	} else {
-		/* Try all devices */
-		if ((devlist = fido_dev_info_new(MAX_FIDO_DEVICES)) == NULL) {
-			skdebug(__func__, "fido_dev_info_new failed");
-			goto out;
-		}
-		if ((r = fido_dev_info_manifest(devlist,
-		    MAX_FIDO_DEVICES, &ndev)) != FIDO_OK) {
-			skdebug(__func__, "fido_dev_info_manifest failed: %s",
-			    fido_strerr(r));
-			goto out;
-		}
-		for (i = 0; i < ndev; i++) {
-			if ((di = fido_dev_info_ptr(devlist, i)) == NULL) {
-				skdebug(__func__, "no dev info at %zu", i);
-				continue;
-			}
-			skdebug(__func__, "trying %s", fido_dev_info_path(di));
-			if ((r = read_rks(fido_dev_info_path(di), pin,
-			    &rks, &nrks)) != 0) {
-				skdebug(__func__, "read_rks failed for %s",
-				    fido_dev_info_path(di));
-				/* remember last error */
-				ret = r;
-				continue;
-			}
-		}
+	if (device != NULL)
+		sk = sk_open(device);
+	else
+		sk = sk_probe(NULL, NULL, 0);
+	if (sk == NULL) {
+		skdebug(__func__, "failed to find sk");
+		goto out;
+	}
+	skdebug(__func__, "trying %s", sk->path);
+	if ((r = read_rks(sk, pin, &rks, &nrks)) != 0) {
+		skdebug(__func__, "read_rks failed for %s", sk->path);
+		ret = r;
+		goto out;
 	}
 	/* success, unless we have no keys but a specific error */
 	if (nrks > 0 || ret == SSH_SK_ERR_GENERAL)
@@ -1116,7 +1191,7 @@ sk_load_resident_keys(const char *pin, struct sk_option **options,
 	rks = NULL;
 	nrks = 0;
  out:
-	free(device);
+	sk_close(sk);
 	for (i = 0; i < nrks; i++) {
 		free(rks[i]->application);
 		freezero(rks[i]->key.public_key, rks[i]->key.public_key_len);
@@ -1124,7 +1199,6 @@ sk_load_resident_keys(const char *pin, struct sk_option **options,
 		freezero(rks[i], sizeof(*rks[i]));
 	}
 	free(rks);
-	fido_dev_info_free(&devlist, MAX_FIDO_DEVICES);
 	return ret;
 }
 
diff --git a/ssh-keygen.c b/ssh-keygen.c
index 89ef9a143..1d6234c1c 100644
--- a/ssh-keygen.c
+++ b/ssh-keygen.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-keygen.c,v 1.416 2020/08/27 01:06:18 djm Exp $ */
+/* $OpenBSD: ssh-keygen.c,v 1.417 2020/08/27 01:07:51 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1994 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -3632,6 +3632,11 @@ main(int argc, char **argv)
 				fatal("Too many incorrect PINs");
 			passphrase = read_passphrase("Enter PIN for "
 			    "authenticator: ", RP_ALLOW_STDIN);
+			if (!quiet) {
+				printf("You may need to touch your "
+				    "authenticator (again) to authorize "
+				    "key generation.\n");
+			}
 		}
 		if (passphrase != NULL) {
 			freezero(passphrase, strlen(passphrase));
-- 
cgit v1.2.3


From b649b3daa6d4b8ebe1bd6de69b3db5d2c03c9af0 Mon Sep 17 00:00:00 2001
From: "djm@openbsd.org" <djm@openbsd.org>
Date: Thu, 27 Aug 2020 01:08:19 +0000
Subject: upstream: preserve verify-required for resident FIDO keys

When downloading a resident, verify-required key from a FIDO token,
preserve the verify-required in the private key that is written to
disk. Previously we weren't doing that because of lack of support
in the middleware API.

from Pedro Martelletto; ok markus@ and myself

OpenBSD-Commit-ID: 201c46ccdd227cddba3d64e1bdbd082afa956517
---
 sk-api.h    | 5 +++--
 sk-usbhid.c | 6 ++++--
 ssh-sk.c    | 5 +++--
 3 files changed, 10 insertions(+), 6 deletions(-)

(limited to 'sk-usbhid.c')

diff --git a/sk-api.h b/sk-api.h
index 1ecaa3537..cc32cd4cc 100644
--- a/sk-api.h
+++ b/sk-api.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: sk-api.h,v 1.9 2020/04/28 04:02:29 djm Exp $ */
+/* $OpenBSD: sk-api.h,v 1.10 2020/08/27 01:08:19 djm Exp $ */
 /*
  * Copyright (c) 2019 Google LLC
  *
@@ -63,6 +63,7 @@ struct sk_resident_key {
 	size_t slot;
 	char *application;
 	struct sk_enroll_response key;
+	uint8_t flags;
 };
 
 struct sk_option {
@@ -71,7 +72,7 @@ struct sk_option {
 	uint8_t required;
 };
 
-#define SSH_SK_VERSION_MAJOR		0x00050000 /* current API version */
+#define SSH_SK_VERSION_MAJOR		0x00060000 /* current API version */
 #define SSH_SK_VERSION_MAJOR_MASK	0xffff0000
 
 /* Return the version of the middleware API */
diff --git a/sk-usbhid.c b/sk-usbhid.c
index 2efb377c5..0305683fe 100644
--- a/sk-usbhid.c
+++ b/sk-usbhid.c
@@ -1104,8 +1104,7 @@ read_rks(struct sk_usbhid *sk, const char *pin,
 			}
 
 			srk->key.key_handle_len = fido_cred_id_len(cred);
-			memcpy(srk->key.key_handle,
-			    fido_cred_id_ptr(cred),
+			memcpy(srk->key.key_handle, fido_cred_id_ptr(cred),
 			    srk->key.key_handle_len);
 
 			switch (fido_cred_type(cred)) {
@@ -1121,6 +1120,9 @@ read_rks(struct sk_usbhid *sk, const char *pin,
 				goto out; /* XXX free rk and continue */
 			}
 
+			if (fido_cred_prot(cred) == FIDO_CRED_PROT_UV_REQUIRED)
+				srk->flags |=  SSH_SK_USER_VERIFICATION_REQD;
+
 			if ((r = pack_public_key(srk->alg, cred,
 			    &srk->key)) != 0) {
 				skdebug(__func__, "pack public key failed");
diff --git a/ssh-sk.c b/ssh-sk.c
index 1afb205f8..89478aff0 100644
--- a/ssh-sk.c
+++ b/ssh-sk.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-sk.c,v 1.30 2020/04/28 04:02:29 djm Exp $ */
+/* $OpenBSD: ssh-sk.c,v 1.31 2020/08/27 01:08:19 djm Exp $ */
 /*
  * Copyright (c) 2019 Google LLC
  *
@@ -769,8 +769,9 @@ sshsk_load_resident(const char *provider_path, const char *device,
 		default:
 			continue;
 		}
-		/* XXX where to get flags? */
 		flags = SSH_SK_USER_PRESENCE_REQD|SSH_SK_RESIDENT_KEY;
+		if ((rks[i]->flags & SSH_SK_USER_VERIFICATION_REQD))
+			flags |= SSH_SK_USER_VERIFICATION_REQD;
 		if ((r = sshsk_key_from_response(rks[i]->alg,
 		    rks[i]->application, flags, &rks[i]->key, &key)) != 0)
 			goto out;
-- 
cgit v1.2.3


From bbcc858ded3fbc46abfa7760e40389e3ca93884c Mon Sep 17 00:00:00 2001
From: Damien Miller <djm@mindrot.org>
Date: Thu, 27 Aug 2020 12:37:12 +1000
Subject: degrade semi-gracefully when libfido2 is too old

---
 configure.ac | 11 ++++++++---
 sk-usbhid.c  | 21 +++++++++++++++++++++
 2 files changed, 29 insertions(+), 3 deletions(-)

(limited to 'sk-usbhid.c')

diff --git a/configure.ac b/configure.ac
index 0f15ef2bf..756e26572 100644
--- a/configure.ac
+++ b/configure.ac
@@ -3180,9 +3180,14 @@ if test "x$enable_sk" = "xyes" -a "x$enable_sk_internal" = "xyes" ; then
 		], [ AC_MSG_ERROR([no usable libfido2 found]) ],
 		[ $OTHERLIBS ]
 	)
-	AC_CHECK_LIB([fido2], [fido_cred_set_prot], [],
-		[ AC_MSG_ERROR([libfido2 missing fido_cred_set_prot; please use libfido2 >= 1.4.0]) ],
-	)
+	saved_LIBS="$LIBS"
+	LIBS="$LIBS $LIBFIDO2"
+	AC_CHECK_FUNCS([ \
+		fido_cred_set_prot \
+		fido_dev_get_touch_status \
+		fido_dev_supports_cred_prot \
+	])
+	LIBS="$saved_LIBS"
 	AC_CHECK_HEADER([fido.h], [],
 		AC_MSG_ERROR([missing fido.h from libfido2]))
 	AC_CHECK_HEADER([fido/credman.h], [],
diff --git a/sk-usbhid.c b/sk-usbhid.c
index 0305683fe..0b11e40aa 100644
--- a/sk-usbhid.c
+++ b/sk-usbhid.c
@@ -41,6 +41,17 @@
 #include <fido.h>
 #include <fido/credman.h>
 
+/* backwards compat for libfido2 */
+#ifndef HAVE_FIDO_DEV_SUPPORTS_CRED_PROT
+#define fido_dev_supports_cred_prot(x) (0)
+#endif
+#ifndef HAVE_FIDO_DEV_GET_TOUCH_BEGIN
+#define fido_dev_get_touch_begin(x) (FIDO_ERR_UNSUPPORTED_OPTION)
+#endif
+#ifndef HAVE_FIDO_DEV_GET_TOUCH_STATUS
+#define fido_dev_get_touch_status(x, y, z) (FIDO_ERR_UNSUPPORTED_OPTION)
+#endif
+
 #ifndef SK_STANDALONE
 # include "log.h"
 # include "xmalloc.h"
@@ -377,6 +388,11 @@ sk_select_by_touch(const fido_dev_info_t *devlist, size_t ndevs)
 	size_t skvcnt, idx;
 	int touch, ms_remain;
 
+#ifndef HAVE_FIDO_DEV_GET_TOUCH_STATUS
+	skdebug(__func__, "libfido2 version does not support a feature needed for multiple tokens. Please upgrade to >=1.5.0");
+	return NULL;
+#endif
+
 	if ((skv = sk_openv(devlist, ndevs, &skvcnt)) == NULL) {
 		skdebug(__func__, "sk_openv failed");
 		return NULL;
@@ -705,6 +721,11 @@ sk_enroll(uint32_t alg, const uint8_t *challenge, size_t challenge_len,
 		goto out;
 	}
 	if ((flags & (SSH_SK_RESIDENT_KEY|SSH_SK_USER_VERIFICATION_REQD)) != 0) {
+#ifndef HAVE_FIDO_DEV_SUPPORTS_CRED_PROT
+		skdebug(__func__, "libfido2 version does not support a feature required for this operation. Please upgrade to >=1.5.0");
+		ret = SSH_SK_ERR_UNSUPPORTED;
+		goto out;
+#endif
 		if (!fido_dev_supports_cred_prot(sk->dev)) {
 			skdebug(__func__, "%s does not support credprot, "
 			    "refusing to create unprotected "
-- 
cgit v1.2.3


From b969072cc3d62d05cb41bc6d6f3c22c764ed932f Mon Sep 17 00:00:00 2001
From: "djm@openbsd.org" <djm@openbsd.org>
Date: Thu, 27 Aug 2020 09:43:28 +0000
Subject: upstream: skip a bit more FIDO token selection logic when only a

single token is attached.

with Pedro Martelletto

OpenBSD-Commit-ID: e4a324bd9814227ec1faa8cb619580e661cca9ac
---
 sk-usbhid.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

(limited to 'sk-usbhid.c')

diff --git a/sk-usbhid.c b/sk-usbhid.c
index 0b11e40aa..98e8ccf10 100644
--- a/sk-usbhid.c
+++ b/sk-usbhid.c
@@ -367,8 +367,13 @@ sk_select_by_cred(const fido_dev_info_t *devlist, size_t ndevs,
 		skdebug(__func__, "sk_openv failed");
 		return NULL;
 	}
+	if (skvcnt == 1) {
+		sk = skv[0];
+		skv[0] = NULL;
+		goto out;
+	}
 	sk = NULL;
-	for (i = 0; i < skvcnt; i++)
+	for (i = 0; i < skvcnt; i++) {
 		if (sk_try(skv[i], application, key_handle,
 		    key_handle_len) == 0) {
 			sk = skv[i];
@@ -376,6 +381,8 @@ sk_select_by_cred(const fido_dev_info_t *devlist, size_t ndevs,
 			skdebug(__func__, "found key in %s", sk->path);
 			break;
 		}
+	}
+ out:
 	sk_closev(skv, skvcnt);
 	return sk;
 }
-- 
cgit v1.2.3


From ce178be0d954b210c958bc2b9e998cd6a7aa73a9 Mon Sep 17 00:00:00 2001
From: Damien Miller <djm@mindrot.org>
Date: Thu, 27 Aug 2020 20:01:52 +1000
Subject: tweak back-compat for older libfido2

---
 configure.ac |  1 +
 sk-usbhid.c  | 26 ++++++++++++++++++++------
 2 files changed, 21 insertions(+), 6 deletions(-)

(limited to 'sk-usbhid.c')

diff --git a/configure.ac b/configure.ac
index 756e26572..7386d742d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -3183,6 +3183,7 @@ if test "x$enable_sk" = "xyes" -a "x$enable_sk_internal" = "xyes" ; then
 	saved_LIBS="$LIBS"
 	LIBS="$LIBS $LIBFIDO2"
 	AC_CHECK_FUNCS([ \
+		fido_cred_prot \
 		fido_cred_set_prot \
 		fido_dev_get_touch_status \
 		fido_dev_supports_cred_prot \
diff --git a/sk-usbhid.c b/sk-usbhid.c
index 98e8ccf10..9b1f4b707 100644
--- a/sk-usbhid.c
+++ b/sk-usbhid.c
@@ -42,6 +42,12 @@
 #include <fido/credman.h>
 
 /* backwards compat for libfido2 */
+#ifndef HAVE_FIDO_CRED_PROD
+#define fido_cred_prot(x) (0)
+#endif
+#ifndef HAVE_FIDO_CRED_SET_PROT
+#define fido_cred_set_prot(x, y) (FIDO_ERR_UNSUPPORTED_OPTION)
+#endif
 #ifndef HAVE_FIDO_DEV_SUPPORTS_CRED_PROT
 #define fido_dev_supports_cred_prot(x) (0)
 #endif
@@ -51,6 +57,12 @@
 #ifndef HAVE_FIDO_DEV_GET_TOUCH_STATUS
 #define fido_dev_get_touch_status(x, y, z) (FIDO_ERR_UNSUPPORTED_OPTION)
 #endif
+#ifndef FIDO_CRED_PROT_UV_REQUIRED
+#define FIDO_CRED_PROT_UV_REQUIRED 0
+#endif
+#ifndef FIDO_CRED_PROT_UV_OPTIONAL_WITH_ID
+#define FIDO_CRED_PROT_UV_OPTIONAL_WITH_ID 0
+#endif
 
 #ifndef SK_STANDALONE
 # include "log.h"
@@ -395,11 +407,6 @@ sk_select_by_touch(const fido_dev_info_t *devlist, size_t ndevs)
 	size_t skvcnt, idx;
 	int touch, ms_remain;
 
-#ifndef HAVE_FIDO_DEV_GET_TOUCH_STATUS
-	skdebug(__func__, "libfido2 version does not support a feature needed for multiple tokens. Please upgrade to >=1.5.0");
-	return NULL;
-#endif
-
 	if ((skv = sk_openv(devlist, ndevs, &skvcnt)) == NULL) {
 		skdebug(__func__, "sk_openv failed");
 		return NULL;
@@ -413,6 +420,11 @@ sk_select_by_touch(const fido_dev_info_t *devlist, size_t ndevs)
 		}
 		goto out;
 	}
+#ifndef HAVE_FIDO_DEV_GET_TOUCH_STATUS
+	skdebug(__func__, "libfido2 version does not support a feature needed for multiple tokens. Please upgrade to >=1.5.0");
+	goto out;
+#endif
+
 	if (sk_touch_begin(skv, skvcnt) == -1) {
 		skdebug(__func__, "sk_touch_begin failed");
 		goto out;
@@ -728,10 +740,12 @@ sk_enroll(uint32_t alg, const uint8_t *challenge, size_t challenge_len,
 		goto out;
 	}
 	if ((flags & (SSH_SK_RESIDENT_KEY|SSH_SK_USER_VERIFICATION_REQD)) != 0) {
-#ifndef HAVE_FIDO_DEV_SUPPORTS_CRED_PROT
+#if !defined(HAVE_FIDO_DEV_SUPPORTS_CRED_PROT) || \
+    !defined(HAVE_FIDO_CRED_SET_PROT)
 		skdebug(__func__, "libfido2 version does not support a feature required for this operation. Please upgrade to >=1.5.0");
 		ret = SSH_SK_ERR_UNSUPPORTED;
 		goto out;
+		credprot = 0; (void)credprot; /* avoid warning */
 #endif
 		if (!fido_dev_supports_cred_prot(sk->dev)) {
 			skdebug(__func__, "%s does not support credprot, "
-- 
cgit v1.2.3


From 39e88aeff9c7cb6862b37ad1a87a03ebbb38c233 Mon Sep 17 00:00:00 2001
From: "djm@openbsd.org" <djm@openbsd.org>
Date: Mon, 31 Aug 2020 00:17:41 +0000
Subject: upstream: Add RCS IDs to the few files that are missing them; from

Pedro Martelletto

OpenBSD-Commit-ID: 39aa37a43d0c75ec87f1659f573d3b5867e4a3b3
---
 PROTOCOL.agent  | 2 ++
 PROTOCOL.sshsig | 1 +
 PROTOCOL.u2f    | 1 +
 sk-usbhid.c     | 1 +
 sshsig.c        | 1 +
 sshsig.h        | 1 +
 6 files changed, 7 insertions(+)

(limited to 'sk-usbhid.c')

diff --git a/PROTOCOL.agent b/PROTOCOL.agent
index da3381942..6947b46cd 100644
--- a/PROTOCOL.agent
+++ b/PROTOCOL.agent
@@ -2,4 +2,6 @@ This file used to contain a description of the SSH agent protocol
 implemented by OpenSSH. It has since been superseded by an Internet-
 draft that is available from:
 
+$OpenBSD: PROTOCOL.agent,v 1.13 2020/08/31 00:17:41 djm Exp $
+
 https://tools.ietf.org/html/draft-miller-ssh-agent-02
diff --git a/PROTOCOL.sshsig b/PROTOCOL.sshsig
index db18458fa..78457ddfc 100644
--- a/PROTOCOL.sshsig
+++ b/PROTOCOL.sshsig
@@ -97,3 +97,4 @@ the reserved field if it is not empty.
 The data is concatenated and passed to the SSH signing
 function.
 
+$OpenBSD: PROTOCOL.sshsig,v 1.4 2020/08/31 00:17:41 djm Exp $
diff --git a/PROTOCOL.u2f b/PROTOCOL.u2f
index fd31ea4ef..5b8a06277 100644
--- a/PROTOCOL.u2f
+++ b/PROTOCOL.u2f
@@ -368,3 +368,4 @@ In OpenSSH, the middleware will be invoked by using a similar mechanism to
 ssh-pkcs11-helper to provide address-space containment of the
 middleware from ssh-agent.
 
+$OpenBSD: PROTOCOL.u2f,v 1.25 2020/08/31 00:17:41 djm Exp $
diff --git a/sk-usbhid.c b/sk-usbhid.c
index 9b1f4b707..7c8ff8d98 100644
--- a/sk-usbhid.c
+++ b/sk-usbhid.c
@@ -1,3 +1,4 @@
+/* $OpenBSD: sk-usbhid.c,v 1.25 2020/08/31 00:17:41 djm Exp $ */
 /*
  * Copyright (c) 2019 Markus Friedl
  * Copyright (c) 2020 Pedro Martelletto
diff --git a/sshsig.c b/sshsig.c
index 658b8c852..0bd7e5cb7 100644
--- a/sshsig.c
+++ b/sshsig.c
@@ -1,3 +1,4 @@
+/* $OpenBSD: sshsig.c,v 1.17 2020/08/31 00:17:41 djm Exp $ */
 /*
  * Copyright (c) 2019 Google LLC
  *
diff --git a/sshsig.h b/sshsig.h
index 44157bc2a..67794a971 100644
--- a/sshsig.h
+++ b/sshsig.h
@@ -1,3 +1,4 @@
+/* $OpenBSD: sshsig.h,v 1.9 2020/08/31 00:17:41 djm Exp $ */
 /*
  * Copyright (c) 2019 Google LLC
  *
-- 
cgit v1.2.3


From c1c44eeecddf093a7983bd91e70b446de789b363 Mon Sep 17 00:00:00 2001
From: pedro martelletto <pedro@ambientworks.net>
Date: Tue, 1 Sep 2020 17:01:55 +0200
Subject: configure.ac: fix libfido2 back-compat

- HAVE_FIDO_CRED_PROD -> HAVE_FIDO_CRED_PROT;
- check for fido_dev_get_touch_begin(), so that
  HAVE_FIDO_DEV_GET_TOUCH_BEGIN gets defined.
---
 configure.ac | 1 +
 sk-usbhid.c  | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

(limited to 'sk-usbhid.c')

diff --git a/configure.ac b/configure.ac
index 7386d742d..9ae199bc7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -3185,6 +3185,7 @@ if test "x$enable_sk" = "xyes" -a "x$enable_sk_internal" = "xyes" ; then
 	AC_CHECK_FUNCS([ \
 		fido_cred_prot \
 		fido_cred_set_prot \
+		fido_dev_get_touch_begin \
 		fido_dev_get_touch_status \
 		fido_dev_supports_cred_prot \
 	])
diff --git a/sk-usbhid.c b/sk-usbhid.c
index 7c8ff8d98..de85b2cb3 100644
--- a/sk-usbhid.c
+++ b/sk-usbhid.c
@@ -43,7 +43,7 @@
 #include <fido/credman.h>
 
 /* backwards compat for libfido2 */
-#ifndef HAVE_FIDO_CRED_PROD
+#ifndef HAVE_FIDO_CRED_PROT
 #define fido_cred_prot(x) (0)
 #endif
 #ifndef HAVE_FIDO_CRED_SET_PROT
-- 
cgit v1.2.3


From c76773524179cb654ff838dd43ba1ddb155bafaa Mon Sep 17 00:00:00 2001
From: "djm@openbsd.org" <djm@openbsd.org>
Date: Wed, 9 Sep 2020 03:08:01 +0000
Subject: upstream: when writing an attestation blob for a FIDO key, record all

the data needed to verify the attestation. Previously we were missing the
"authenticator data" that is included in the signature.

spotted by Ian Haken
feedback Pedro Martelletto and Ian Haken; ok markus@

OpenBSD-Commit-ID: 8439896e63792b2db99c6065dd9a45eabbdb7e0a
---
 PROTOCOL.u2f | 98 +++++++++++-------------------------------------------------
 sk-api.h     |  6 ++--
 sk-usbhid.c  | 13 +++++++-
 ssh-keygen.1 |  9 +++---
 ssh-keygen.c | 44 ++++++++++++++++-----------
 ssh-sk.c     | 44 ++++++++++++++++++---------
 6 files changed, 96 insertions(+), 118 deletions(-)

(limited to 'sk-usbhid.c')

diff --git a/PROTOCOL.u2f b/PROTOCOL.u2f
index 5b8a06277..f8ca56b11 100644
--- a/PROTOCOL.u2f
+++ b/PROTOCOL.u2f
@@ -154,6 +154,16 @@ by trusted hardware before it will issue a certificate. To support this
 case, OpenSSH optionally allows retaining the attestation information
 at the time of key generation. It will take the following format:
 
+	string		"ssh-sk-attest-v01"
+	string		attestation certificate
+	string		enrollment signature
+	string		authenticator data (CBOR encoded)
+	uint32		reserved flags
+	string		reserved string
+
+A previous version of this format, emitted prior to OpenSSH 8.4 omitted
+the authenticator data.
+
 	string		"ssh-sk-attest-v00"
 	string		attestation certificate
 	string		enrollment signature
@@ -267,87 +277,15 @@ regress testing. For this reason, OpenSSH shall support a dynamically-
 loaded middleware libraries to communicate with security keys, but offer
 support for the common case of USB HID security keys internally.
 
-The middleware library need only expose a handful of functions:
-
-	#define SSH_SK_VERSION_MAJOR		0x00050000 /* API version */
-	#define SSH_SK_VERSION_MAJOR_MASK	0xffff0000
-
-	/* Flags */
-	#define SSH_SK_USER_PRESENCE_REQD	0x01
-	#define SSH_SK_USER_VERIFICATION_REQD	0x04
-	#define SSH_SK_RESIDENT_KEY		0x20
-
-	/* Algs */
-	#define SSH_SK_ECDSA                   0x00
-	#define SSH_SK_ED25519                 0x01
-
-	/* Error codes */
-	#define SSH_SK_ERR_GENERAL		-1
-	#define SSH_SK_ERR_UNSUPPORTED		-2
-	#define SSH_SK_ERR_PIN_REQUIRED		-3
-	#define SSH_SK_ERR_DEVICE_NOT_FOUND	-4
-
-	struct sk_enroll_response {
-		uint8_t *public_key;
-		size_t public_key_len;
-		uint8_t *key_handle;
-		size_t key_handle_len;
-		uint8_t *signature;
-		size_t signature_len;
-		uint8_t *attestation_cert;
-		size_t attestation_cert_len;
-	};
-
-	struct sk_sign_response {
-		uint8_t flags;
-		uint32_t counter;
-		uint8_t *sig_r;
-		size_t sig_r_len;
-		uint8_t *sig_s;
-		size_t sig_s_len;
-	};
-
-	struct sk_resident_key {
-		uint32_t alg;
-		size_t slot;
-		char *application;
-		struct sk_enroll_response key;
-	};
-
-	struct sk_option {
-		char *name;
-		char *value;
-		uint8_t important;
-	};
-
-	/* Return the version of the middleware API */
-	uint32_t sk_api_version(void);
-
-	/* Enroll a U2F key (private key generation) */
-	int sk_enroll(uint32_t alg,
-	    const uint8_t *challenge, size_t challenge_len,
-	    const char *application, uint8_t flags, const char *pin,
-	    struct sk_option **options,
-	    struct sk_enroll_response **enroll_response);
-
-	/* Sign a challenge */
-	int sk_sign(uint32_t 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, const char *pin, struct sk_option **options,
-	    struct sk_sign_response **sign_response);
-
-	/* Enumerate all resident keys */
-	int sk_load_resident_keys(const char *pin, struct sk_option **options,
-	    struct sk_resident_key ***rks, size_t *nrks);
-
-The SSH_SK_VERSION_MAJOR should be incremented for each incompatible
+The middleware library need only expose a handful of functions and
+numbers listed in sk-api.h. Included in the defined numbers is a
+SSH_SK_VERSION_MAJOR that should be incremented for each incompatible
 API change.
 
-The options may be used to pass miscellaneous options to the middleware
-as a NULL-terminated array of pointers to struct sk_option. The middleware
-may ignore unsupported or unknown options unless the "important" flag is
-set, in which case it should return failure if an unsupported option is
+miscellaneous options may be passed to the middleware as a NULL-
+terminated array of pointers to struct sk_option. The middleware may
+ignore unsupported or unknown options unless the "required" flag is set,
+in which case it should return failure if an unsupported option is
 requested.
 
 At present the following options names are supported:
@@ -368,4 +306,4 @@ In OpenSSH, the middleware will be invoked by using a similar mechanism to
 ssh-pkcs11-helper to provide address-space containment of the
 middleware from ssh-agent.
 
-$OpenBSD: PROTOCOL.u2f,v 1.25 2020/08/31 00:17:41 djm Exp $
+$OpenBSD: PROTOCOL.u2f,v 1.26 2020/09/09 03:08:01 djm Exp $
diff --git a/sk-api.h b/sk-api.h
index cc32cd4cc..df17ca540 100644
--- a/sk-api.h
+++ b/sk-api.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: sk-api.h,v 1.10 2020/08/27 01:08:19 djm Exp $ */
+/* $OpenBSD: sk-api.h,v 1.11 2020/09/09 03:08:01 djm Exp $ */
 /*
  * Copyright (c) 2019 Google LLC
  *
@@ -47,6 +47,8 @@ struct sk_enroll_response {
 	size_t signature_len;
 	uint8_t *attestation_cert;
 	size_t attestation_cert_len;
+	uint8_t *authdata;
+	size_t authdata_len;
 };
 
 struct sk_sign_response {
@@ -72,7 +74,7 @@ struct sk_option {
 	uint8_t required;
 };
 
-#define SSH_SK_VERSION_MAJOR		0x00060000 /* current API version */
+#define SSH_SK_VERSION_MAJOR		0x00070000 /* current API version */
 #define SSH_SK_VERSION_MAJOR_MASK	0xffff0000
 
 /* Return the version of the middleware API */
diff --git a/sk-usbhid.c b/sk-usbhid.c
index de85b2cb3..007c59644 100644
--- a/sk-usbhid.c
+++ b/sk-usbhid.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: sk-usbhid.c,v 1.25 2020/08/31 00:17:41 djm Exp $ */
+/* $OpenBSD: sk-usbhid.c,v 1.26 2020/09/09 03:08:01 djm Exp $ */
 /*
  * Copyright (c) 2019 Markus Friedl
  * Copyright (c) 2020 Pedro Martelletto
@@ -822,6 +822,16 @@ sk_enroll(uint32_t alg, const uint8_t *challenge, size_t challenge_len,
 		memcpy(response->attestation_cert, ptr, len);
 		response->attestation_cert_len = len;
 	}
+	if ((ptr = fido_cred_authdata_ptr(cred)) != NULL) {
+		len = fido_cred_authdata_len(cred);
+		debug3("%s: authdata len=%zu", __func__, len);
+		if ((response->authdata = calloc(1, len)) == NULL) {
+			skdebug(__func__, "calloc authdata failed");
+			goto out;
+		}
+		memcpy(response->authdata, ptr, len);
+		response->authdata_len = len;
+	}
 	*enroll_response = response;
 	response = NULL;
 	ret = 0;
@@ -832,6 +842,7 @@ sk_enroll(uint32_t alg, const uint8_t *challenge, size_t challenge_len,
 		free(response->key_handle);
 		free(response->signature);
 		free(response->attestation_cert);
+		free(response->authdata);
 		free(response);
 	}
 	sk_close(sk);
diff --git a/ssh-keygen.1 b/ssh-keygen.1
index c51a0d9c8..3ae596caa 100644
--- a/ssh-keygen.1
+++ b/ssh-keygen.1
@@ -1,4 +1,4 @@
-.\"	$OpenBSD: ssh-keygen.1,v 1.208 2020/08/27 06:15:22 jmc Exp $
+.\"	$OpenBSD: ssh-keygen.1,v 1.209 2020/09/09 03:08:01 djm Exp $
 .\"
 .\" Author: Tatu Ylonen <ylo@cs.hut.fi>
 .\" Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -35,7 +35,7 @@
 .\" (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
 .\" THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 .\"
-.Dd $Mdocdate: August 27 2020 $
+.Dd $Mdocdate: September 9 2020 $
 .Dt SSH-KEYGEN 1
 .Os
 .Sh NAME
@@ -520,9 +520,10 @@ Not all FIDO tokens support this option.
 Currently PIN authentication is the only supported verification method,
 but other methods may be supported in the future.
 .It Cm write-attestation Ns = Ns Ar path
-May be used at key generation time to record the attestation certificate
+May be used at key generation time to record the attestation data
 returned from FIDO tokens during key generation.
-By default this information is discarded.
+Please note that this information is potentially sensitive.
+By default, this information is discarded.
 .El
 .Pp
 The
diff --git a/ssh-keygen.c b/ssh-keygen.c
index 64cee4de0..a12b79a56 100644
--- a/ssh-keygen.c
+++ b/ssh-keygen.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-keygen.c,v 1.419 2020/08/27 09:46:04 djm Exp $ */
+/* $OpenBSD: ssh-keygen.c,v 1.420 2020/09/09 03:08:01 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1994 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -3071,6 +3071,27 @@ do_download_sk(const char *skprovider, const char *device)
 	return ret;
 }
 
+static void
+save_attestation(struct sshbuf *attest, const char *path)
+{
+	mode_t omask;
+	int r;
+
+	if (path == NULL)
+		return; /* nothing to do */
+	if (attest == NULL || sshbuf_len(attest) == 0)
+		fatal("Enrollment did not return attestation data");
+	omask = umask(077);
+	r = sshbuf_write_file(path, attest);
+	umask(omask);
+	if (r != 0)
+		fatal("Unable to write attestation data \"%s\": %s", path,
+		    ssh_err(r));
+	if (!quiet)
+		printf("Your FIDO attestation certificate has been saved in "
+		    "%s\n", path);
+}
+
 static void
 usage(void)
 {
@@ -3137,7 +3158,7 @@ main(int argc, char **argv)
 	unsigned long long cert_serial = 0;
 	char *identity_comment = NULL, *ca_key_path = NULL, **opts = NULL;
 	char *sk_application = NULL, *sk_device = NULL, *sk_user = NULL;
-	char *sk_attestaion_path = NULL;
+	char *sk_attestation_path = NULL;
 	struct sshbuf *challenge = NULL, *attest = NULL;
 	size_t i, nopts = 0;
 	u_int32_t bits = 0;
@@ -3593,7 +3614,7 @@ main(int argc, char **argv)
 				}
 			} else if (strncasecmp(opts[i],
 			    "write-attestation=", 18) == 0) {
-				sk_attestaion_path = opts[i] + 18;
+				sk_attestation_path = opts[i] + 18;
 			} else if (strncasecmp(opts[i],
 			    "application=", 12) == 0) {
 				sk_application = xstrdup(opts[i] + 12);
@@ -3715,20 +3736,9 @@ main(int argc, char **argv)
 		free(fp);
 	}
 
-	if (sk_attestaion_path != NULL) {
-		if (attest == NULL || sshbuf_len(attest) == 0) {
-			fatal("Enrollment did not return attestation "
-			    "certificate");
-		}
-		if ((r = sshbuf_write_file(sk_attestaion_path, attest)) != 0) {
-			fatal("Unable to write attestation certificate "
-			    "\"%s\": %s", sk_attestaion_path, ssh_err(r));
-		}
-		if (!quiet) {
-			printf("Your FIDO attestation certificate has been "
-			    "saved in %s\n", sk_attestaion_path);
-		}
-	}
+	if (sk_attestation_path != NULL)
+		save_attestation(attest, sk_attestation_path);
+
 	sshbuf_free(attest);
 	sshkey_free(public);
 
diff --git a/ssh-sk.c b/ssh-sk.c
index 89478aff0..1455df635 100644
--- a/ssh-sk.c
+++ b/ssh-sk.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-sk.c,v 1.31 2020/08/27 01:08:19 djm Exp $ */
+/* $OpenBSD: ssh-sk.c,v 1.32 2020/09/09 03:08:02 djm Exp $ */
 /*
  * Copyright (c) 2019 Google LLC
  *
@@ -174,6 +174,7 @@ sshsk_free_enroll_response(struct sk_enroll_response *r)
 	freezero(r->public_key, r->public_key_len);
 	freezero(r->signature, r->signature_len);
 	freezero(r->attestation_cert, r->attestation_cert_len);
+	freezero(r->authdata, r->authdata_len);
 	freezero(r, sizeof(*r));
 }
 
@@ -419,6 +420,31 @@ make_options(const char *device, const char *user_id,
 	return ret;
 }
 
+
+static int
+fill_attestation_blob(const struct sk_enroll_response *resp,
+    struct sshbuf *attest)
+{
+	int r;
+
+	if (attest == NULL)
+		return 0; /* nothing to do */
+	if ((r = sshbuf_put_cstring(attest, "ssh-sk-attest-v01")) != 0 ||
+	    (r = sshbuf_put_string(attest,
+	    resp->attestation_cert, resp->attestation_cert_len)) != 0 ||
+	    (r = sshbuf_put_string(attest,
+	    resp->signature, resp->signature_len)) != 0 ||
+	    (r = sshbuf_put_string(attest,
+	    resp->authdata, resp->authdata_len)) != 0 ||
+	    (r = sshbuf_put_u32(attest, 0)) != 0 || /* resvd flags */
+	    (r = sshbuf_put_string(attest, NULL, 0)) != 0 /* resvd */) {
+		error("%s: buffer error: %s", __func__, ssh_err(r));
+		return r;
+	}
+	/* success */
+	return 0;
+}
+
 int
 sshsk_enroll(int type, const char *provider_path, const char *device,
     const char *application, const char *userid, uint8_t flags,
@@ -506,19 +532,9 @@ sshsk_enroll(int type, const char *provider_path, const char *device,
 		goto out;
 
 	/* Optionally fill in the attestation information */
-	if (attest != NULL) {
-		if ((r = sshbuf_put_cstring(attest,
-		    "ssh-sk-attest-v00")) != 0 ||
-		    (r = sshbuf_put_string(attest,
-		    resp->attestation_cert, resp->attestation_cert_len)) != 0 ||
-		    (r = sshbuf_put_string(attest,
-		    resp->signature, resp->signature_len)) != 0 ||
-		    (r = sshbuf_put_u32(attest, 0)) != 0 || /* resvd flags */
-		    (r = sshbuf_put_string(attest, NULL, 0)) != 0 /* resvd */) {
-			error("%s: buffer error: %s", __func__, ssh_err(r));
-			goto out;
-		}
-	}
+	if ((r = fill_attestation_blob(resp, attest)) != 0)
+		goto out;
+
 	/* success */
 	*keyp = key;
 	key = NULL; /* transferred */
-- 
cgit v1.2.3