From 53c1ef9ce086356d2ee126135b71ca5ad9e3897d Mon Sep 17 00:00:00 2001 From: Peter Cai Date: Sun, 13 Nov 2022 21:12:45 -0500 Subject: [PATCH 1/3] libfido2-util: Disable pre-flight checks for credentials with UV According to the FIDO2 spec, tokens may not support pre-flight checks for credentials requiring UV, at least not without at least `pinUvAuthParam` or `uv = true`. Originally, in #25268, this was handled by passing a PIN to satisfy `pinUvAuthParams`, but this is not ideal, since `pinUvAuthParam` can be obtained from either a PIN or a UV verification. Forcing the user to enter the PIN here (which is often just the fallback option on UV devices) is no better than just trying out each device with the actual assertion request. As a result, this commit disables pre-flight checks when the credential requires UV, and instead reverts to the old behavior (trying out each device and each key slot, requiring multiple user interactions) for this type of credentials. --- src/shared/libfido2-util.c | 56 ++++++++++---------------------------- 1 file changed, 15 insertions(+), 41 deletions(-) diff --git a/src/shared/libfido2-util.c b/src/shared/libfido2-util.c index a2bf14ec48..b1eb4a0e3c 100644 --- a/src/shared/libfido2-util.c +++ b/src/shared/libfido2-util.c @@ -262,7 +262,6 @@ static int fido2_is_cred_in_specific_token( const char *rp_id, const void *cid, size_t cid_size, - char **pins, Fido2EnrollFlags flags) { assert(path); @@ -296,6 +295,16 @@ static int fido2_is_cred_in_specific_token( if (r < 0) return r; + /* FIDO2 devices may not support pre-flight requests with UV, at least not + * without user interaction [1]. As a result, let's just return -EOPNOTSUPP + * here and go ahead with trying the unlock directly. + * Reference: + * 1: https://fidoalliance.org/specs/fido-v2.1-ps-20210615/fido-client-to-authenticator-protocol-v2.1-ps-20210615.html#sctn-getAssert-authnr-alg + * See section 7.4 */ + if (has_uv && FLAGS_SET(flags, FIDO2ENROLL_UV)) + return log_error_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), + "Pre-flight requests with UV are unsupported"); + /* According to CTAP 2.1 specification, to do pre-flight we need to set up option to false * with optionally pinUvAuthParam in assertion[1]. But for authenticator that doesn't support * user presence, once up option is present, the authenticator may return CTAP2_ERR_UNSUPPORTED_OPTION[2]. @@ -312,28 +321,7 @@ static int fido2_is_cred_in_specific_token( return log_error_errno(SYNTHETIC_ERRNO(EIO), "Failed to set assertion user presence: %s", sym_fido_strerr(r)); - - /* According to CTAP 2.1 specification, if the credential is marked as userVerificationRequired, - * we may pass pinUvAuthParam parameter or set uv option to true in order to check whether the - * credential is in the token. Here we choose to use PIN (pinUvAuthParam) to achieve this. - * If we don't do that, the authenticator will remove the credential from the applicable - * credentials list, hence CTAP2_ERR_NO_CREDENTIALS error will be returned. - * Please see - * https://fidoalliance.org/specs/fido-v2.1-ps-20210615/fido-client-to-authenticator-protocol-v2.1-ps-20210615.html#sctn-getAssert-authnr-alg - * step 7.4 for detailed information. - */ - if (FLAGS_SET(flags, FIDO2ENROLL_UV) && has_uv) { - if (strv_isempty(pins)) - r = FIDO_ERR_PIN_REQUIRED; - else - STRV_FOREACH(i, pins) { - r = sym_fido_dev_get_assert(d, a, *i); - if (r != FIDO_ERR_PIN_INVALID) - break; - } - - } else - r = sym_fido_dev_get_assert(d, a, NULL); + r = sym_fido_dev_get_assert(d, a, NULL); switch (r) { case FIDO_OK: @@ -632,26 +620,12 @@ int fido2_use_hmac_hash( goto finish; } - r = fido2_is_cred_in_specific_token(path, rp_id, cid, cid_size, pins, required); - /* We handle PIN related errors here since we have used PIN in the check. - * If PIN error happens, we can avoid pin retries decreased the second time. */ - if (IN_SET(r, -ENOANO, /* → pin required */ - -ENOLCK, /* → pin incorrect */ - -EOWNERDEAD)) { /* → uv blocked */ - /* If it's not the last device, try next one */ - if (i < found - 1) - continue; - - /* -EOWNERDEAD is returned when uv is blocked. Map it to EAGAIN so users can reinsert - * the token and try again. */ - if (r == -EOWNERDEAD) - r = -EAGAIN; - goto finish; - } else if (r == -ENODEV) /* not a FIDO2 device or lacking HMAC-SECRET extension */ + r = fido2_is_cred_in_specific_token(path, rp_id, cid, cid_size, required); + if (r == -ENODEV) /* not a FIDO2 device or lacking HMAC-SECRET extension */ continue; - else if (r < 0) + if (r < 0) log_error_errno(r, "Failed to determine whether the credential is in the token, trying anyway: %m"); - else if (r == 0) { + if (r == 0) { log_debug("The credential is not in the token %s, skipping.", path); continue; } From 5d2c1ce4e4c9f903b5c064f67a59c2e0b0dbd037 Mon Sep 17 00:00:00 2001 From: Peter Cai Date: Sun, 13 Nov 2022 21:58:43 -0500 Subject: [PATCH 2/3] libfido2-util: Perform pre-flight checks as well when a specific device path is given This prevents unnecessary user interactions when `fido2-device` is set to something other than `auto` -- a case overlooked in the original PR #23577 (and later #25268). We do not move pre-flight checks to `fido2_use_hmac_hash_specific_token` because the behaviors are different between different cases: when the device path is NULL, we try to automatically choose the correct device, in which case pre-flight errors should be "soft" errors, without spamming the tty with error outputs; but when a specific device path is given, a pre-flight request that determined the non-existence of the credential should be treated the same as a failed assertion request. --- src/shared/libfido2-util.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/shared/libfido2-util.c b/src/shared/libfido2-util.c index b1eb4a0e3c..aa4905c7da 100644 --- a/src/shared/libfido2-util.c +++ b/src/shared/libfido2-util.c @@ -584,8 +584,21 @@ int fido2_use_hmac_hash( if (r < 0) return log_error_errno(r, "FIDO2 support is not installed."); - if (device) + if (device) { + r = fido2_is_cred_in_specific_token(device, rp_id, cid, cid_size, required); + if (r == -ENODEV) /* not a FIDO2 device or lacking HMAC-SECRET extension */ + return log_error_errno(r, + "%s is not a FIDO2 device or it lacks support for HMAC-SECRET.", device); + if (r == 0) + /* The caller is expected to attempt other key slots in this case, + * therefore, do not spam the console with error logs here. */ + return log_debug_errno(SYNTHETIC_ERRNO(EBADSLT), + "The credential is not in the token %s.", device); + if (r < 0) + log_error_errno(r, "Failed to determine whether the credential is in the token, trying anyway: %m"); + return fido2_use_hmac_hash_specific_token(device, rp_id, salt, salt_size, cid, cid_size, pins, required, ret_hmac, ret_hmac_size); + } di = sym_fido_dev_info_new(allocated); if (!di) From 2a469016e392159d435c1f0afed3740fe784a8b3 Mon Sep 17 00:00:00 2001 From: Peter Cai Date: Wed, 23 Nov 2022 08:43:22 -0500 Subject: [PATCH 3/3] libfido2-util: Refactor pre-flight failure handling `fido2_is_cred_in_specific_token()` should simply not return error codes for non-fatal errors. For example, `-ENODEV` can be safely translated to a `false` return value. When the pre-flight request is not supported, we should simply return true to instruct the caller to attempt to use the device anyway. All error codes returned by the funtion should now be fatal and logged at error level. Non-fatal errors should only appear in debug logs. --- src/shared/libfido2-util.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/src/shared/libfido2-util.c b/src/shared/libfido2-util.c index aa4905c7da..aa2d8b3ff1 100644 --- a/src/shared/libfido2-util.c +++ b/src/shared/libfido2-util.c @@ -284,6 +284,10 @@ static int fido2_is_cred_in_specific_token( "Failed to open FIDO2 device %s: %s", path, sym_fido_strerr(r)); r = verify_features(d, path, LOG_ERR, NULL, NULL, &has_up, &has_uv); + if (r == -ENODEV) { /* Not a FIDO2 device or lacking HMAC-SECRET extension */ + log_debug_errno(r, "%s is not a FIDO2 device, or it lacks the hmac-secret extension", path); + return false; + } if (r < 0) return r; @@ -296,14 +300,15 @@ static int fido2_is_cred_in_specific_token( return r; /* FIDO2 devices may not support pre-flight requests with UV, at least not - * without user interaction [1]. As a result, let's just return -EOPNOTSUPP + * without user interaction [1]. As a result, let's just return true * here and go ahead with trying the unlock directly. * Reference: * 1: https://fidoalliance.org/specs/fido-v2.1-ps-20210615/fido-client-to-authenticator-protocol-v2.1-ps-20210615.html#sctn-getAssert-authnr-alg * See section 7.4 */ - if (has_uv && FLAGS_SET(flags, FIDO2ENROLL_UV)) - return log_error_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), - "Pre-flight requests with UV are unsupported"); + if (has_uv && FLAGS_SET(flags, FIDO2ENROLL_UV)) { + log_debug("Pre-flight requests with UV are unsupported, device: %s", path); + return true; + } /* According to CTAP 2.1 specification, to do pre-flight we need to set up option to false * with optionally pinUvAuthParam in assertion[1]. But for authenticator that doesn't support @@ -586,16 +591,13 @@ int fido2_use_hmac_hash( if (device) { r = fido2_is_cred_in_specific_token(device, rp_id, cid, cid_size, required); - if (r == -ENODEV) /* not a FIDO2 device or lacking HMAC-SECRET extension */ - return log_error_errno(r, - "%s is not a FIDO2 device or it lacks support for HMAC-SECRET.", device); if (r == 0) /* The caller is expected to attempt other key slots in this case, * therefore, do not spam the console with error logs here. */ return log_debug_errno(SYNTHETIC_ERRNO(EBADSLT), "The credential is not in the token %s.", device); if (r < 0) - log_error_errno(r, "Failed to determine whether the credential is in the token, trying anyway: %m"); + return log_error_errno(r, "Token returned error during pre-flight: %m"); return fido2_use_hmac_hash_specific_token(device, rp_id, salt, salt_size, cid, cid_size, pins, required, ret_hmac, ret_hmac_size); } @@ -634,10 +636,10 @@ int fido2_use_hmac_hash( } r = fido2_is_cred_in_specific_token(path, rp_id, cid, cid_size, required); - if (r == -ENODEV) /* not a FIDO2 device or lacking HMAC-SECRET extension */ - continue; - if (r < 0) - log_error_errno(r, "Failed to determine whether the credential is in the token, trying anyway: %m"); + if (r < 0) { + log_error_errno(r, "Token returned error during pre-flight: %m"); + goto finish; + } if (r == 0) { log_debug("The credential is not in the token %s, skipping.", path); continue;