upstream: Better handle FIDO keys on tokens that provide user

verification (UV) on the device itself, including biometric keys.

Query the token during key creation to determine whether it supports
on-token UV and, if so, clear the SSH_SK_USER_VERIFICATION_REQD flag
in the key so that ssh(1) doesn't automatically prompty for PIN later.

When making signatures with the key, query the token's capabilities
again and check whether the token is able (right now) to perform user-
verification without a PIN. If it is then the PIN prompt is bypassed
and user verification delegated to the token. If not (e.g. the token
is biometric capable, but no biometric are enrolled), then fall back
to user verification via the usual PIN prompt.

Work by Pedro Martelletto; ok myself and markus@

NB. cranks SSH_SK_VERSION_MAJOR

OpenBSD-Commit-ID: e318a8c258d9833a0b7eb0236cdb68b5143b2f27

diff --git a/sk-api.h b/sk-api.h
index c84c6f8ea9547e113de8fef237c1d20feb7f9be3..34e110b4e320aac9c13ebc925fb353bf4f2e580b 100644 (file)
--- a/sk-api.h
+++ b/sk-api.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: sk-api.h,v 1.13 2021/10/28 02:54:18 djm Exp $ */
+/* $OpenBSD: sk-api.h,v 1.14 2021/11/02 22:56:40 djm Exp $ */
 /*
  * Copyright (c) 2019 Google LLC
  *
@@ -39,6 +39,7 @@
 #define SSH_SK_ERR_DEVICE_NOT_FOUND    -4
 
 struct sk_enroll_response {
+       uint8_t flags;
        uint8_t *public_key;
        size_t public_key_len;
        uint8_t *key_handle;
@@ -76,7 +77,7 @@ struct sk_option {
        uint8_t required;
 };
 
-#define SSH_SK_VERSION_MAJOR           0x00080000 /* current API version */
+#define SSH_SK_VERSION_MAJOR           0x00090000 /* 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 b9924d0877f4d8169ec89fc23ede5e523bfb60ac..6d69d321cbd45daad7f957d23bb0e159f77ab66b 100644 (file)
--- a/sk-usbhid.c
+++ b/sk-usbhid.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: sk-usbhid.c,v 1.32 2021/10/28 02:54:18 djm Exp $ */
+/* $OpenBSD: sk-usbhid.c,v 1.33 2021/11/02 22:56:40 djm Exp $ */
 /*
  * Copyright (c) 2019 Markus Friedl
  * Copyright (c) 2020 Pedro Martelletto
@@ -112,6 +112,9 @@
                (*ps) = sig->s; \
        } while (0)
 #endif
+#ifndef FIDO_ERR_OPERATION_DENIED
+#define FIDO_ERR_OPERATION_DENIED 0x27
+#endif
 
 struct sk_usbhid {
        fido_dev_t *dev;
@@ -501,6 +504,48 @@ sk_probe(const char *application, const uint8_t *key_handle,
        return sk;
 }
 
+static int
+check_sk_options(fido_dev_t *dev, const char *opt, int *ret)
+{
+       fido_cbor_info_t *info;
+       char * const *name;
+       const bool *value;
+       size_t len;
+       int r;
+
+       *ret = -1;
+
+       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;
+       }
+       name = fido_cbor_info_options_name_ptr(info);
+       value = fido_cbor_info_options_value_ptr(info);
+       len = fido_cbor_info_options_len(info);
+       for (size_t i = 0; i < len; i++) {
+               if (!strcmp(name[i], opt)) {
+                       *ret = value[i];
+                       break;
+               }
+       }
+       fido_cbor_info_free(&info);
+       if (*ret == -1)
+               skdebug(__func__, "option %s is unknown", opt);
+       else
+               skdebug(__func__, "option %s is %s", opt, *ret ? "on" : "off");
+
+       return 0;
+}
+
 #ifdef WITH_OPENSSL
 /*
  * The key returned via fido_cred_pubkey_ptr() is in affine coordinates,
@@ -634,6 +679,7 @@ fidoerr_to_skerr(int fidoerr)
                return SSH_SK_ERR_UNSUPPORTED;
        case FIDO_ERR_PIN_REQUIRED:
        case FIDO_ERR_PIN_INVALID:
+       case FIDO_ERR_OPERATION_DENIED:
                return SSH_SK_ERR_PIN_REQUIRED;
        default:
                return -1;
@@ -687,6 +733,7 @@ sk_enroll(uint32_t alg, const uint8_t *challenge, size_t challenge_len,
        struct sk_enroll_response *response = NULL;
        size_t len;
        int credprot;
+       int internal_uv;
        int cose_alg;
        int ret = SSH_SK_ERR_GENERAL;
        int r;
@@ -809,6 +856,14 @@ sk_enroll(uint32_t alg, const uint8_t *challenge, size_t challenge_len,
                skdebug(__func__, "calloc response failed");
                goto out;
        }
+       response->flags = flags;
+       if ((flags & SSH_SK_USER_VERIFICATION_REQD)) {
+               if (check_sk_options(sk->dev, "uv", &internal_uv) == 0 &&
+                   internal_uv != -1) {
+                       /* user verification handled by token */
+                       response->flags &= ~SSH_SK_USER_VERIFICATION_REQD;
+               }
+       }
        if (pack_public_key(alg, cred, response) != 0) {
                skdebug(__func__, "pack_public_key failed");
                goto out;
@@ -991,7 +1046,7 @@ sk_sign(uint32_t alg, const uint8_t *data, size_t datalen,
        struct sk_usbhid *sk = NULL;
        struct sk_sign_response *response = NULL;
        uint8_t message[32];
-       int ret = SSH_SK_ERR_GENERAL;
+       int ret = SSH_SK_ERR_GENERAL, internal_uv;
        int r;
 
        fido_init(SSH_FIDO_INIT_ARG);
@@ -1043,11 +1098,20 @@ 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 (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 (pin == NULL && (flags & SSH_SK_USER_VERIFICATION_REQD)) {
+               if (check_sk_options(sk->dev, "uv", &internal_uv) < 0 ||
+                   internal_uv != 1) {
+                       skdebug(__func__, "check_sk_options uv");
+                       ret = SSH_SK_ERR_PIN_REQUIRED;
+                       goto out;
+               }
+               if ((r = fido_assert_set_uv(assert,
+                   FIDO_OPT_TRUE)) != FIDO_OK) {
+                       skdebug(__func__, "fido_assert_set_uv: %s",
+                           fido_strerr(r));
+                       ret = fidoerr_to_skerr(r);
+                       goto out;
+               }
        }
        if ((r = fido_dev_get_assert(sk->dev, assert, pin)) != FIDO_OK) {
                skdebug(__func__, "fido_dev_get_assert: %s", fido_strerr(r));
@@ -1084,7 +1148,7 @@ static int
 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;
+       int ret = SSH_SK_ERR_GENERAL, r = -1, internal_uv;
        fido_credman_metadata_t *metadata = NULL;
        fido_credman_rp_t *rp = NULL;
        fido_credman_rk_t *rk = NULL;
@@ -1103,6 +1167,10 @@ read_rks(struct sk_usbhid *sk, const char *pin,
                skdebug(__func__, "alloc failed");
                goto out;
        }
+       if (check_sk_options(sk->dev, "uv", &internal_uv) != 0) {
+               skdebug(__func__, "check_sk_options failed");
+               goto out;
+       }
 
        if ((r = fido_credman_get_dev_metadata(sk->dev, metadata, pin)) != 0) {
                if (r == FIDO_ERR_INVALID_COMMAND) {
@@ -1208,7 +1276,8 @@ 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)
+                       if (fido_cred_prot(cred) == FIDO_CRED_PROT_UV_REQUIRED
+                           && internal_uv == -1)
                                srk->flags |=  SSH_SK_USER_VERIFICATION_REQD;
 
                        if ((r = pack_public_key(srk->alg, cred,

diff --git a/ssh-sk.c b/ssh-sk.c
index 612530abaf11dd0ea4dd38e72ca9b9845d1fd238..bac83cd1552fa63e3e7fc390098d0871692e61a9 100644 (file)
--- a/ssh-sk.c
+++ b/ssh-sk.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-sk.c,v 1.36 2021/10/28 02:54:18 djm Exp $ */
+/* $OpenBSD: ssh-sk.c,v 1.37 2021/11/02 22:56:40 djm Exp $ */
 /*
  * Copyright (c) 2019 Google LLC
  *
@@ -534,7 +534,7 @@ sshsk_enroll(int type, const char *provider_path, const char *device,
                goto out;
        }
 
-       if ((r = sshsk_key_from_response(alg, application, flags,
+       if ((r = sshsk_key_from_response(alg, application, resp->flags,
            resp, &key)) != 0)
                goto out;