diff --git a/core/src/apps/webauthn/credential.py b/core/src/apps/webauthn/credential.py index aaaf74191..5417b6ad5 100644 --- a/core/src/apps/webauthn/credential.py +++ b/core/src/apps/webauthn/credential.py @@ -60,6 +60,9 @@ class Credential: self.rp_id_hash = b"" # type: bytes self.user_id = None # type: Optional[bytes] + def __lt__(self, other: "Credential") -> bool: + raise NotImplementedError + def app_name(self) -> str: raise NotImplementedError diff --git a/core/src/apps/webauthn/fido2.py b/core/src/apps/webauthn/fido2.py index 61b57f247..5b1c6248f 100644 --- a/core/src/apps/webauthn/fido2.py +++ b/core/src/apps/webauthn/fido2.py @@ -27,7 +27,7 @@ from apps.webauthn.resident_credentials import ( ) if False: - from typing import Any, Coroutine, Iterable, List, Optional, Tuple + from typing import Any, Coroutine, Iterable, Iterator, List, Optional, Tuple _CID_BROADCAST = const(0xFFFFFFFF) # broadcast channel id @@ -1305,8 +1305,7 @@ def cbor_error(cid: int, code: int) -> Cmd: def credentials_from_descriptor_list( descriptor_list: List[dict], rp_id_hash: bytes -) -> List[Credential]: - cred_list = [] +) -> Iterator[Credential]: for credential_descriptor in descriptor_list: credential_type = credential_descriptor["type"] if not isinstance(credential_type, str): @@ -1319,10 +1318,26 @@ def credentials_from_descriptor_list( raise TypeError try: cred = Credential.from_bytes(credential_id, rp_id_hash) - cred_list.append(cred) except Exception: - pass + continue + + yield cred + +def distinguishable_cred_list(credentials: Iterable[Credential]) -> List[Credential]: + """Reduces the input to a list of credentials which can be distinguished by + the user. It is assumed that all input credentials share the same RP ID.""" + cred_list = [] # type: List[Credential] + for cred in credentials: + for i, prev_cred in enumerate(cred_list): + if prev_cred.account_name() == cred.account_name(): + # Among indistinguishable FIDO2 credentials prefer the newest. + # Among U2F credentials prefer the first in the input. + if isinstance(cred, Fido2Credential) and cred < prev_cred: + cred_list[i] = cred + break + else: + cred_list.append(cred) return cred_list @@ -1369,7 +1384,8 @@ def cbor_make_credential(req: Cmd, dialog_mgr: DialogManager) -> Optional[Cmd]: # Check if any of the credential descriptors in the exclude list belong to this authenticator. exclude_list = param.get(_MAKECRED_CMD_EXCLUDE_LIST, []) - if credentials_from_descriptor_list(exclude_list, rp_id_hash): + excluded_creds = credentials_from_descriptor_list(exclude_list, rp_id_hash) + if not utils.is_empty_iterator(excluded_creds): # This authenticator is already registered. if not dialog_mgr.set_state( Fido2ConfirmExcluded(req.cid, dialog_mgr.iface, cred) @@ -1537,7 +1553,8 @@ def cbor_get_assertion(req: Cmd, dialog_mgr: DialogManager) -> Optional[Cmd]: allow_list = param.get(_GETASSERT_CMD_ALLOW_LIST, []) if allow_list: # Get all credentials from the allow list that belong to this authenticator. - cred_list = credentials_from_descriptor_list(allow_list, rp_id_hash) + allowed_creds = credentials_from_descriptor_list(allow_list, rp_id_hash) + cred_list = distinguishable_cred_list(allowed_creds) for cred in cred_list: if cred.rp_id is None: cred.rp_id = rp_id diff --git a/core/src/trezor/utils.py b/core/src/trezor/utils.py index 04e683e9e..0232238b2 100644 --- a/core/src/trezor/utils.py +++ b/core/src/trezor/utils.py @@ -152,3 +152,12 @@ def truncate_utf8(string: str, max_bytes: int) -> str: i -= 1 return data[:i].decode() + + +def is_empty_iterator(i: Iterator) -> bool: + try: + next(i) + except StopIteration: + return True + else: + return False diff --git a/core/tests/test_apps.webauthn.credential.py b/core/tests/test_apps.webauthn.credential.py index a2fb008f1..78085d5ba 100644 --- a/core/tests/test_apps.webauthn.credential.py +++ b/core/tests/test_apps.webauthn.credential.py @@ -1,7 +1,8 @@ from common import * import storage from apps.common import mnemonic -from apps.webauthn.credential import Fido2Credential, NAME_MAX_LENGTH +from apps.webauthn.credential import Fido2Credential, U2fCredential, NAME_MAX_LENGTH +from apps.webauthn.fido2 import distinguishable_cred_list from trezor.crypto.curve import nist256p1 from trezor.crypto.hashlib import sha256 @@ -74,5 +75,50 @@ class TestCredential(unittest.TestCase): self.assertEqual(cred.user_name, "a" * (NAME_MAX_LENGTH - 1)) self.assertEqual(cred.user_display_name, "a" * NAME_MAX_LENGTH) + def test_allow_list_processing(self): + a1 = Fido2Credential() + a1.user_id = b"user-a" + a1.user_name = "user-a" + a1.creation_time = 1 + + a2 = Fido2Credential() + a2.user_id = b"user-a" + a2.user_display_name = "User A" + a2.creation_time = 3 + + a3 = Fido2Credential() + a3.user_id = b"user-a" + a3.user_name = "User A" + a3.creation_time = 4 + + b1 = Fido2Credential() + b1.user_id = b"user-b" + b1.creation_time = 2 + + b2 = Fido2Credential() + b2.user_id = b"user-b" + b2.creation_time = 5 + + b3 = Fido2Credential() + b3.user_id = b"user-b" + b3.creation_time = 5 + + c1 = U2fCredential() + + c2 = U2fCredential() + + self.assertEqual(sorted(distinguishable_cred_list([a1, a2, a3, b1, b2, c1, c2])), [b2, a3, a1, c1]) + self.assertEqual(sorted(distinguishable_cred_list([c2, c1, b2, b1, a3, a2, a1])), [b2, a3, a1, c2]) + + # Test input by creation time. + self.assertEqual(sorted(distinguishable_cred_list([b2, a3, c1, a2, b1, a1, c2])), [b2, a3, a1, c1]) + self.assertEqual(sorted(distinguishable_cred_list([c2, a1, b1, a2, c1, a3, b2])), [b2, a3, a1, c2]) + + # Test duplicities. + self.assertEqual(sorted(distinguishable_cred_list([c1, a1, a1, c2, c1])), [a1, c1]) + self.assertEqual(sorted(distinguishable_cred_list([b2, b3])), [b2]) + self.assertEqual(sorted(distinguishable_cred_list([b3, b2])), [b3]) + + if __name__ == '__main__': unittest.main()