From 01363fefcd1dfb2d29b919b42267bfcdecd28a54 Mon Sep 17 00:00:00 2001 From: Iker Pedrosa Date: Thu, 3 Oct 2024 11:51:25 +0200 Subject: [PATCH] passkey: implement preflight option Expand the passkey_child to detect whether a FIDO2 device needs a PIN and the number of attempts left for the PIN. This is needed to improve the overall user experience by providing a way for SSSD to detect these features before the user is requested to interact with them. Expected input to run passkey_child: `--preflight`, `--domain`, `--key-handle` and `--public-key`. Output: whether PIN is needed (boolean) and number of PIN attempts left (integer). Example: 1 8 Signed-off-by: Iker Pedrosa --- Makefile.am | 3 +- src/passkey_child/passkey_child.c | 6 ++ src/passkey_child/passkey_child.h | 47 ++++++++- src/passkey_child/passkey_child_assert.c | 3 +- src/passkey_child/passkey_child_common.c | 65 ++++++++++++ src/passkey_child/passkey_child_credentials.c | 16 +++ src/passkey_child/passkey_child_devices.c | 24 ++++- src/tests/cmocka/test_passkey_child.c | 98 +++++++++++++++++++ 8 files changed, 258 insertions(+), 4 deletions(-) diff --git a/Makefile.am b/Makefile.am index 93c7ce08815..40d06b27d1d 100644 --- a/Makefile.am +++ b/Makefile.am @@ -3909,7 +3909,8 @@ test_passkey_LDFLAGS = \ -Wl,-wrap,fido_assert_sig_len \ -Wl,-wrap,fido_assert_set_count \ -Wl,-wrap,fido_assert_set_authdata \ - -Wl,-wrap,fido_assert_set_sig + -Wl,-wrap,fido_assert_set_sig \ + -Wl,-wrap,fido_dev_get_retry_count test_passkey_LDADD = \ $(CMOCKA_LIBS) \ $(SSSD_LIBS) \ diff --git a/src/passkey_child/passkey_child.c b/src/passkey_child/passkey_child.c index ef9b06c65aa..0e9f7220875 100644 --- a/src/passkey_child/passkey_child.c +++ b/src/passkey_child/passkey_child.c @@ -93,6 +93,12 @@ int main(int argc, const char *argv[]) ERROR("Verification error.\n"); goto done; } + } else if (data.action == ACTION_PREFLIGHT) { + ret = preflight(&data, 1); + /* Errors are ignored, as in most cases they are due to the device not + * being connected to the system. If an error occurs, the default + * values are returned, and that is sufficient for the time being. + */ } done: diff --git a/src/passkey_child/passkey_child.h b/src/passkey_child/passkey_child.h index 853dd44d7e5..05fdae92508 100644 --- a/src/passkey_child/passkey_child.h +++ b/src/passkey_child/passkey_child.h @@ -34,13 +34,15 @@ #define USER_ID_SIZE 32 #define TIMEOUT 15 #define FREQUENCY 1 +#define MAX_PIN_RETRIES 8 enum action_opt { ACTION_NONE, ACTION_REGISTER, ACTION_AUTHENTICATE, ACTION_GET_ASSERT, - ACTION_VERIFY_ASSERT + ACTION_VERIFY_ASSERT, + ACTION_PREFLIGHT }; enum credential_type { @@ -559,4 +561,47 @@ get_assert_data(struct passkey_data *data, int timeout); errno_t verify_assert_data(struct passkey_data *data); +/** + * @brief Obtain PIN retries in the device + * + * @param[in] dev Device information + * @param[in] data passkey data + * @param[in] _pin_retries Number of PIN retries + * + * @return 0 if the PIN retries were obtained properly, + * error code otherwise. + */ +errno_t +get_device_pin_retries(fido_dev_t *dev, struct passkey_data *data, + int *_pin_retries); + +/** + * @brief Print preflight information + * + * Print user-verification and pin retries + * + * @param[in] data passkey data + * @param[in] _pin_retries Number of PIN retries + * + * @return EOK + * + */ +errno_t +print_preflight(const struct passkey_data *data, int pin_retries); + +/** + * @brief Obtain authentication data prior to processing + * + * Prepare the assertion request data, select the device to use, get the device + * options and compare them with the organization policy, get the PIN retries + * and print the preflight data. + * + * @param[in] data passkey data + * @param[in] timeout Timeout in seconds to stop looking for a device + * + * @return EOK + */ +errno_t +preflight(struct passkey_data *data, int timeout); + #endif /* __PASSKEY_CHILD_H__ */ diff --git a/src/passkey_child/passkey_child_assert.c b/src/passkey_child/passkey_child_assert.c index 5139dc82ef0..7920e6fd7fb 100644 --- a/src/passkey_child/passkey_child_assert.c +++ b/src/passkey_child/passkey_child_assert.c @@ -51,7 +51,8 @@ set_assert_client_data_hash(const struct passkey_data *data, return ENOMEM; } - if (data->action == ACTION_AUTHENTICATE) { + if (data->action == ACTION_AUTHENTICATE + || data->action == ACTION_PREFLIGHT) { ret = sss_generate_csprng_buffer(cdh, sizeof(cdh)); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, diff --git a/src/passkey_child/passkey_child_common.c b/src/passkey_child/passkey_child_common.c index 46ed725da30..510d7fe7a8c 100644 --- a/src/passkey_child/passkey_child_common.c +++ b/src/passkey_child/passkey_child_common.c @@ -177,6 +177,8 @@ parse_arguments(TALLOC_CTX *mem_ctx, int argc, const char *argv[], _("Obtain assertion data"), NULL }, {"verify-assert", 0, POPT_ARG_NONE, NULL, 'v', _("Verify assertion data"), NULL }, + {"preflight", 0, POPT_ARG_NONE, NULL, 'p', + _("Obtain authentication data prior to processing"), NULL }, {"username", 0, POPT_ARG_STRING, &data->shortname, 0, _("Shortname"), NULL }, {"domain", 0, POPT_ARG_STRING, &data->domain, 0, @@ -259,6 +261,16 @@ parse_arguments(TALLOC_CTX *mem_ctx, int argc, const char *argv[], } data->action = ACTION_VERIFY_ASSERT; break; + case 'p': + if (data->action != ACTION_NONE) { + fprintf(stderr, "\nActions are mutually exclusive and should" \ + " be used only once.\n\n"); + poptPrintUsage(pc, stderr, 0); + ret = EINVAL; + goto done; + } + data->action = ACTION_PREFLIGHT; + break; case 'q': data->quiet = true; break; @@ -419,6 +431,15 @@ check_arguments(const struct passkey_data *data) goto done; } + if (data->action == ACTION_PREFLIGHT + && (data->domain == NULL || data->public_key_list == NULL + || data->key_handle_list == NULL)) { + DEBUG(SSSDBG_OP_FAILURE, + "Too few arguments for preflight action.\n"); + ret = ERR_INPUT_PARSE; + goto done; + } + done: return ret; } @@ -887,3 +908,47 @@ verify_assert_data(struct passkey_data *data) return ret; } + +errno_t +preflight(struct passkey_data *data, int timeout) +{ + fido_assert_t *assert = NULL; + fido_dev_t *dev = NULL; + int index = 0; + int pin_retries = 0; + errno_t ret; + + ret = select_authenticator(data, timeout, &dev, &assert, &index); + if (ret != EOK) { + goto done; + } + + DEBUG(SSSDBG_TRACE_FUNC, "Comparing the device and policy options.\n"); + ret = get_device_options(dev, data); + if (ret != FIDO_OK) { + goto done; + } + + DEBUG(SSSDBG_TRACE_FUNC, "Checking the number of remaining PIN retries.\n"); + ret = get_device_pin_retries(dev, data, &pin_retries); + if (ret != FIDO_OK) { + goto done; + } + + ret = EOK; + +done: + if (ret != EOK) { + data->user_verification = FIDO_OPT_TRUE; + pin_retries = MAX_PIN_RETRIES; + } + print_preflight(data, pin_retries); + + if (dev != NULL) { + fido_dev_close(dev); + } + fido_dev_free(&dev); + fido_assert_free(&assert); + + return EOK; +} diff --git a/src/passkey_child/passkey_child_credentials.c b/src/passkey_child/passkey_child_credentials.c index e27afb411bb..2910fdf7c02 100644 --- a/src/passkey_child/passkey_child_credentials.c +++ b/src/passkey_child/passkey_child_credentials.c @@ -679,3 +679,19 @@ evp_pkey_to_eddsa_pubkey(const EVP_PKEY *evp_pkey, struct pk_data_t *_pk_data) done: return ret; } + +errno_t +print_preflight(const struct passkey_data *data, int pin_retries) +{ + bool user_verification; + + if (data->user_verification == FIDO_OPT_TRUE) { + user_verification = true; + } else { + user_verification = false; + } + + PRINT("%d\n%d\n", user_verification, pin_retries); + + return EOK; +} diff --git a/src/passkey_child/passkey_child_devices.c b/src/passkey_child/passkey_child_devices.c index 84586f8e543..d8e43ad22c7 100644 --- a/src/passkey_child/passkey_child_devices.c +++ b/src/passkey_child/passkey_child_devices.c @@ -30,7 +30,7 @@ errno_t list_devices(int timeout, fido_dev_info_t *dev_list, size_t *dev_number) { - errno_t ret; + errno_t ret = EOK; for (int i = 0; i < timeout; i += FREQUENCY) { ret = fido_dev_info_manifest(dev_list, DEVLIST_SIZE, dev_number); @@ -244,3 +244,25 @@ get_device_options(fido_dev_t *dev, struct passkey_data *_data) return ret; } + +errno_t +get_device_pin_retries(fido_dev_t *dev, struct passkey_data *data, + int *_pin_retries) +{ + int ret = EOK; + + if (data->user_verification == FIDO_OPT_TRUE) { + ret = fido_dev_get_retry_count(dev, _pin_retries); + if (ret != FIDO_OK) { + DEBUG(SSSDBG_OP_FAILURE, + "fido_dev_get_retry_count failed [%d]: %s.\n", + ret, fido_strerr(ret)); + goto done; + } + } else { + *_pin_retries = MAX_PIN_RETRIES; + } + +done: + return ret; +} \ No newline at end of file diff --git a/src/tests/cmocka/test_passkey_child.c b/src/tests/cmocka/test_passkey_child.c index b45ba9008b2..479a31479f2 100644 --- a/src/tests/cmocka/test_passkey_child.c +++ b/src/tests/cmocka/test_passkey_child.c @@ -524,6 +524,17 @@ __wrap_fido_assert_set_sig(fido_assert_t *assert, size_t idx, return ret; } +int +__wrap_fido_dev_get_retry_count(fido_dev_t *dev, int *pin_retries) +{ + int ret; + + ret = mock(); + (*pin_retries) = mock(); + + return ret; +} + /*********************** * TEST **********************/ @@ -1339,6 +1350,38 @@ void test_verify_assert_data_integration(void **state) talloc_free(tmp_ctx); } +void test_get_device_pin_retries_success(void **state) +{ + struct passkey_data data; + fido_dev_t *dev = NULL; + int pin_retries = 0; + errno_t ret; + + data.user_verification = FIDO_OPT_TRUE; + will_return(__wrap_fido_dev_get_retry_count, FIDO_OK); + will_return(__wrap_fido_dev_get_retry_count, 8); + + ret = get_device_pin_retries(dev, &data, &pin_retries); + assert_int_equal(ret, FIDO_OK); + assert_int_equal(pin_retries, 8); +} + +void test_get_device_pin_retries_failure(void **state) +{ + struct passkey_data data; + fido_dev_t *dev = NULL; + int pin_retries = 0; + errno_t ret; + + data.user_verification = FIDO_OPT_TRUE; + will_return(__wrap_fido_dev_get_retry_count, FIDO_ERR_INVALID_ARGUMENT); + will_return(__wrap_fido_dev_get_retry_count, 8); + + ret = get_device_pin_retries(dev, &data, &pin_retries); + assert_int_equal(ret, FIDO_ERR_INVALID_ARGUMENT); + assert_int_equal(pin_retries, 8); +} + static void test_parse_supp_valgrind_args(void) { /* @@ -1349,6 +1392,58 @@ static void test_parse_supp_valgrind_args(void) DEBUG_CLI_INIT(debug_level); } +void test_preflight_integration(void **state) +{ + TALLOC_CTX *tmp_ctx; + struct passkey_data data; + size_t dev_number = 3; + char *key_handle; + char *public_key; + errno_t ret; + + tmp_ctx = talloc_new(NULL); + assert_non_null(tmp_ctx); + data.action = ACTION_PREFLIGHT; + data.shortname = "user"; + data.domain = "test.com"; + key_handle = talloc_strdup(tmp_ctx, TEST_KEY_HANDLE); + public_key = talloc_strdup(tmp_ctx, TEST_ES256_PEM_PUBLIC_KEY); + data.key_handle_list = &key_handle; + data.key_handle_size = 1; + data.public_key_list = &public_key; + data.public_key_size = 1; + data.type = COSE_ES256; + data.user_verification = FIDO_OPT_TRUE; + data.user_id = NULL; + data.quiet = false; + will_return(__wrap_fido_dev_info_manifest, FIDO_OK); + will_return(__wrap_fido_dev_info_manifest, dev_number); + will_return(__wrap_fido_assert_set_rp, FIDO_OK); + will_return(__wrap_fido_assert_allow_cred, FIDO_OK); + will_return(__wrap_fido_assert_set_uv, FIDO_OK); + will_return(__wrap_fido_assert_set_clientdata_hash, FIDO_OK); + for (size_t i = 0; i < (dev_number - 1); i++) { + will_return(__wrap_fido_dev_info_path, TEST_PATH); + will_return(__wrap_fido_dev_open, FIDO_OK); + will_return(__wrap_fido_dev_is_fido2, true); + if (i == 0) { + will_return(__wrap_fido_dev_get_assert, FIDO_ERR_INVALID_SIG); + } else { + will_return(__wrap_fido_dev_get_assert, FIDO_OK); + } + } + will_return(__wrap_fido_dev_has_uv, false); + will_return(__wrap_fido_dev_has_pin, true); + will_return(__wrap_fido_dev_supports_uv, false); + will_return(__wrap_fido_dev_get_retry_count, FIDO_OK); + will_return(__wrap_fido_dev_get_retry_count, 8); + + ret = preflight(&data, 1); + assert_int_equal(ret, EOK); + + talloc_free(tmp_ctx); +} + int main(int argc, const char *argv[]) { poptContext pc; @@ -1396,6 +1491,9 @@ int main(int argc, const char *argv[]) cmocka_unit_test(test_authenticate_integration), cmocka_unit_test(test_get_assert_data_integration), cmocka_unit_test(test_verify_assert_data_integration), + cmocka_unit_test(test_get_device_pin_retries_success), + cmocka_unit_test(test_get_device_pin_retries_failure), + cmocka_unit_test(test_preflight_integration), }; /* Set debug level to invalid value so we can decide if -d 0 was used. */