[openssh-commits] [openssh] 01/01: upstream: Better handle FIDO keys on tokens that provide user

git+noreply at mindrot.org git+noreply at mindrot.org
Wed Nov 3 10:08:19 AEDT 2021


This is an automated email from the git hooks/post-receive script.

djm pushed a commit to branch master
in repository openssh.

commit f3c34df860c4c1ebddacb973954e58167d9dbade
Author: djm at openbsd.org <djm at openbsd.org>
Date:   Tue Nov 2 22:56:40 2021 +0000

    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
---
 sk-api.h    |  5 ++--
 sk-usbhid.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
 ssh-sk.c    |  4 +--
 3 files changed, 83 insertions(+), 13 deletions(-)

diff --git a/sk-api.h b/sk-api.h
index c84c6f8e..34e110b4 100644
--- 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 b9924d08..6d69d321 100644
--- 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 612530ab..bac83cd1 100644
--- 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;
 

-- 
To stop receiving notification emails like this one, please contact
djm at mindrot.org.


More information about the openssh-commits mailing list