Skip to content

Commit

Permalink
passkey: omit user-verification
Browse files Browse the repository at this point in the history
If user-verification is disabled and the key doesn't support it, then
omit it. Otherwise, the authentication will produce an error and the
user will be unable to authenticate.

I have also added a unit-test to check this condition.

Signed-off-by: Iker Pedrosa <[email protected]>
(cherry picked from commit a8daf97)
  • Loading branch information
ikerexxe authored and justin-stephenson committed Sep 18, 2023
1 parent 6cedcf2 commit dd8b469
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 0 deletions.
1 change: 1 addition & 0 deletions Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -3860,6 +3860,7 @@ test_passkey_LDFLAGS = \
-Wl,-wrap,fido_dev_open \
-Wl,-wrap,fido_dev_has_uv \
-Wl,-wrap,fido_dev_has_pin \
-Wl,-wrap,fido_dev_supports_uv \
-Wl,-wrap,fido_dev_make_cred \
-Wl,-wrap,fido_cred_x5c_ptr \
-Wl,-wrap,fido_cred_verify \
Expand Down
12 changes: 12 additions & 0 deletions src/passkey_child/passkey_child_devices.c
Original file line number Diff line number Diff line change
Expand Up @@ -201,10 +201,12 @@ get_device_options(fido_dev_t *dev, struct passkey_data *_data)
{
bool has_pin;
bool has_uv;
bool supports_uv;
errno_t ret;

has_uv = fido_dev_has_uv(dev);
has_pin = fido_dev_has_pin(dev);
supports_uv = fido_dev_supports_uv(dev);

if (_data->user_verification == FIDO_OPT_TRUE && has_pin != true
&& has_uv != true) {
Expand All @@ -226,6 +228,16 @@ get_device_options(fido_dev_t *dev, struct passkey_data *_data)
goto done;
}

if (_data->user_verification == FIDO_OPT_FALSE
&& supports_uv == false) {
DEBUG(SSSDBG_CONF_SETTINGS,
"Policy disabled user-verification but the key doesn't support "
"it. Thus, omitting the user-verification.\n");
_data->user_verification = FIDO_OPT_OMIT;
ret = EOK;
goto done;
}

ret = EOK;

done:
Expand Down
32 changes: 32 additions & 0 deletions src/tests/cmocka/test_passkey_child.c
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,16 @@ __wrap_fido_dev_has_pin(fido_dev_t *dev)
return ret;
}

bool
__wrap_fido_dev_supports_uv(fido_dev_t *dev)
{
bool ret;

ret = mock();

return ret;
}

int
__wrap_fido_dev_make_cred(fido_dev_t *dev, fido_cred_t *cred, const char *pin)
{
Expand Down Expand Up @@ -1090,6 +1100,7 @@ void test_get_device_options(void **state)
ts->data.user_verification = FIDO_OPT_TRUE;
will_return(__wrap_fido_dev_has_uv, true);
will_return(__wrap_fido_dev_has_pin, true);
will_return(__wrap_fido_dev_supports_uv, true);

ret = get_device_options(dev, &ts->data);

Expand All @@ -1106,12 +1117,30 @@ void test_get_device_options_user_verification_unmatch(void **state)
ts->data.user_verification = FIDO_OPT_TRUE;
will_return(__wrap_fido_dev_has_uv, false);
will_return(__wrap_fido_dev_has_pin, false);
will_return(__wrap_fido_dev_supports_uv, false);

ret = get_device_options(dev, &ts->data);

assert_int_equal(ret, EINVAL);
}

void test_get_device_options_user_verification_false_not_supported(void **state)
{
struct test_state *ts = talloc_get_type_abort(*state, struct test_state);
fido_dev_t *dev = NULL;
errno_t ret;

ts->data.user_verification = FIDO_OPT_FALSE;
will_return(__wrap_fido_dev_has_uv, false);
will_return(__wrap_fido_dev_has_pin, false);
will_return(__wrap_fido_dev_supports_uv, false);

ret = get_device_options(dev, &ts->data);

assert_int_equal(ret, FIDO_OK);
assert_int_equal(ts->data.user_verification, FIDO_OPT_OMIT);
}

void test_request_assert(void **state)
{
struct test_state *ts = talloc_get_type_abort(*state, struct test_state);
Expand Down Expand Up @@ -1206,6 +1235,7 @@ void test_authenticate_integration(void **state)
}
will_return(__wrap_fido_dev_has_uv, false);
will_return(__wrap_fido_dev_has_pin, false);
will_return(__wrap_fido_dev_supports_uv, false);
will_return(__wrap_fido_assert_set_uv, FIDO_OK);
will_return(__wrap_fido_assert_set_clientdata_hash, FIDO_OK);
will_return(__wrap_fido_dev_has_uv, false);
Expand Down Expand Up @@ -1256,6 +1286,7 @@ void test_get_assert_data_integration(void **state)
}
will_return(__wrap_fido_dev_has_uv, false);
will_return(__wrap_fido_dev_has_pin, false);
will_return(__wrap_fido_dev_supports_uv, false);
will_return(__wrap_fido_assert_set_uv, FIDO_OK);
will_return(__wrap_fido_dev_has_uv, false);
will_return(__wrap_fido_dev_has_pin, false);
Expand Down Expand Up @@ -1358,6 +1389,7 @@ int main(int argc, const char *argv[])
cmocka_unit_test_setup_teardown(test_get_authenticator_data_multiple_keys_assert_not_found, setup, teardown),
cmocka_unit_test_setup_teardown(test_get_device_options, setup, teardown),
cmocka_unit_test_setup_teardown(test_get_device_options_user_verification_unmatch, setup, teardown),
cmocka_unit_test_setup_teardown(test_get_device_options_user_verification_false_not_supported, setup, teardown),
cmocka_unit_test_setup_teardown(test_request_assert, setup, teardown),
cmocka_unit_test_setup_teardown(test_verify_assert, setup, teardown),
cmocka_unit_test_setup_teardown(test_verify_assert_failed, setup, teardown),
Expand Down

0 comments on commit dd8b469

Please sign in to comment.