core/webauthn: Remove indistinguishable credentials from the allow list.

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
 

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

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

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()