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

2 files changed, 332 insertions, 253 deletions

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));