Skip to content

Commit

Permalink
[fix] Cred number must be set to 1 when credential is chosen SK-side
Browse files Browse the repository at this point in the history
  • Loading branch information
lpascal-ledger committed Nov 4, 2024
1 parent b7899ae commit 5954961
Show file tree
Hide file tree
Showing 20 changed files with 25 additions and 15 deletions.
13 changes: 10 additions & 3 deletions src/ctap2/get_assertion/get_assertion.c
Original file line number Diff line number Diff line change
Expand Up @@ -344,18 +344,25 @@ void ctap2_get_assertion_handle(u2f_service_t *service, uint8_t *buffer, uint16_
goto exit;
}

ctap2_copy_info_on_buffers();

/* if (true) { */
/* nfc_handle_get_assertion(); */

/* } else */

if (CMD_IS_OVER_U2F_NFC) {
// No up nor uv requested, skip UX and reply immediately
ctap2_copy_info_on_buffers();

nfc_handle_get_assertion();

} else if (!ctap2AssertData->userPresenceRequired && !ctap2AssertData->pinRequired) {
// No up nor uv required, skip UX and reply immediately
get_assertion_confirm(1);
} else {
// Look for a potential rk entry if no allow list was provided
if (!ctap2AssertData->allowListPresent) {
// This value will be set to 1 further into the code, because in this case (non-NFC,
// non-RK), credential is chosen authenticator-side, *not* client-side (through
// getNextAssertion).
ctap2AssertData->availableCredentials =
rk_build_RKList_from_rpID(ctap2AssertData->rpIdHash);
if (ctap2AssertData->availableCredentials == 1) {
Expand Down
4 changes: 4 additions & 0 deletions src/ctap2/get_assertion/get_assertion_ui.c
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@ static void ctap_ux_on_user_choice(bool confirm, uint16_t idx) {
ctap2UxState = CTAP2_UX_STATE_NONE;

if (confirm) {
// As the choice is made authenticator-side, according to the spec SK should display 1 and
// only 1 matching credential. This will prevent the client to call getNextAssertion to
// discover more credentials.
globals_get_ctap2_assert_data()->availableCredentials = 1;
get_assertion_confirm(idx);
#ifdef HAVE_NBGL
app_nbgl_status("Login request signed", true, ui_idle);
Expand Down
2 changes: 1 addition & 1 deletion tests/functional/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@


def pytest_addoption(parser):
parser.addoption("--transport", default="U2F")
parser.addoption("--transport", default="U2F", choices=("U2F", "HID"))
parser.addoption("--fast", action="store_true")
parser.addoption("--ctap2_u2f_proxy", action="store_true")

Expand Down
19 changes: 9 additions & 10 deletions tests/functional/ctap2/test_get_assertion.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from utils import generate_make_credentials_params, fido_known_app


def test_get_assertion(client, test_name):
def test_get_assertion_ok(client, test_name):
compare_args = (TESTS_SPECULOS_DIR, test_name)
# This test use the fact that after a reboot of the device
# the ctap2WrappingKey should stay the same.
Expand Down Expand Up @@ -124,7 +124,7 @@ def test_get_assertion_user_refused(client, test_name):
def test_get_assertion_no_credentials(client, test_name):
compare_args = (TESTS_SPECULOS_DIR, test_name)
args = generate_make_credentials_params(client, ref=0)

rp = args.rp
# Try without allow_list
with pytest.raises(CtapError) as e:
client.ctap2.get_assertion(args.rp["id"], args.client_data_hash,
Expand All @@ -137,7 +137,7 @@ def test_get_assertion_no_credentials(client, test_name):
args = generate_make_credentials_params(client)
allow_list = [{"id": generate_random_bytes(32), "type": "public-key"}]
with pytest.raises(CtapError) as e:
client.ctap2.get_assertion(args.rp["id"], args.client_data_hash,
client.ctap2.get_assertion(rp["id"], args.client_data_hash,
allow_list,
login_type="none",
check_screens="full",
Expand Down Expand Up @@ -222,13 +222,12 @@ def test_get_assertion_allow_list_ok(client, test_name):
users_credential_data = []

# Register a first user with a random RP
t = generate_get_assertion_params(client, rp=rp)
t = generate_get_assertion_params(client)
allow_list.append({"id": t.credential_data.credential_id, "type": "public-key"})

# Register 3 users for a known RP
for idx in range(1, 4):
local_args = generate_make_credentials_params(client, ref=idx)
local_args.rp = t.args.rp
local_args = generate_make_credentials_params(client, ref=idx, rp=rp)
attestation = client.ctap2.make_credential(local_args)
credential_data = AttestedCredentialData(attestation.auth_data.credential_data)
allow_list.append({"id": credential_data.credential_id, "type": "public-key"})
Expand All @@ -241,7 +240,7 @@ def test_get_assertion_allow_list_ok(client, test_name):

# Generate get assertion request checking presented users
client_data_hash = generate_random_bytes(32)
assertion = client.ctap2.get_assertion(t.args.rp["id"], client_data_hash, allow_list,
assertion = client.ctap2.get_assertion(rp["id"], client_data_hash, allow_list,
login_type="multi",
user_accept=True,
check_users=registered_users,
Expand All @@ -250,14 +249,14 @@ def test_get_assertion_allow_list_ok(client, test_name):
select_user_idx=3)

credential_data = users_credential_data[2]
assertion.verify(client_data_hash, t.credential_data.public_key)
assertion.verify(client_data_hash, credential_data.public_key)

with pytest.raises(InvalidSignature):
credential_data = users_credential_data[1]
assertion.verify(client_data_hash, t.credential_data.public_key)
assertion.verify(client_data_hash, credential_data.public_key)

assert len(assertion.auth_data) == 37
assert sha256(t.args.rp["id"].encode()) == assertion.auth_data.rp_id_hash
assert sha256(rp["id"].encode()) == assertion.auth_data.rp_id_hash
assert assertion.auth_data.flags == AuthenticatorData.FLAG.USER_PRESENT
assert assertion.user is None
assert assertion.number_of_credentials is None
Expand Down
2 changes: 1 addition & 1 deletion tests/functional/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ def generate_random_string(length):
def generate_make_credentials_params(client,
rp=None,
rk: Optional[bool] = None,
uv=None,
uv: Optional[bool] = None,
key_params=None,
pin: Optional[bytes] = None,
pin_uv_param: Optional[bytes] = None,
Expand Down

0 comments on commit 5954961

Please sign in to comment.