diff --git a/core/.changelog.d/2205.changed b/core/.changelog.d/2205.changed new file mode 100644 index 000000000..799b01ec2 --- /dev/null +++ b/core/.changelog.d/2205.changed @@ -0,0 +1 @@ +Ignore channel ID in U2F. diff --git a/core/src/apps/webauthn/fido2.py b/core/src/apps/webauthn/fido2.py index 3790102de..0b99867c0 100644 --- a/core/src/apps/webauthn/fido2.py +++ b/core/src/apps/webauthn/fido2.py @@ -216,8 +216,8 @@ _MAX_CRED_COUNT_IN_LIST = const(10) # The CID of the last WINK command. Used to ensure that we do only one WINK at a time on any given CID. _last_wink_cid = 0 -# The CID of the last successful U2F_AUTHENTICATE check-only request. -_last_good_auth_check_cid = 0 +# Indicates whether the last U2F_AUTHENTICATE had a valid key handle. +_last_auth_valid = False class CborError(Exception): @@ -544,10 +544,7 @@ async def handle_reports(iface: io.HID) -> None: req = await read_cmd(iface) if req is None: continue - if dialog_mgr.is_busy() and req.cid not in ( - dialog_mgr.get_cid(), - _CID_BROADCAST, - ): + if not dialog_mgr.allow_cid(req.cid): resp: Cmd | None = cmd_error(req.cid, _ERR_CHANNEL_BUSY) else: resp = dispatch_cmd(req, dialog_mgr) @@ -589,6 +586,10 @@ class State: self.iface = iface self.finished = False + def allow_cid(self, cid: int) -> bool: + # Generally allow any CID, because Safari browser changes CID for every U2F message. + return True + def keepalive_status(self) -> int: # Run the keepalive loop to check for timeout, but do not send any keepalive messages. return _KEEPALIVE_STATUS_NONE @@ -633,14 +634,9 @@ class U2fState(State, ConfirmInfo): class U2fConfirmRegister(U2fState): - def __init__( - self, cid: int, iface: io.HID, req_data: bytes, cred: U2fCredential - ) -> None: - super().__init__(cid, iface, req_data, cred) - async def confirm_dialog(self) -> bool: if self._cred.rp_id_hash in _BOGUS_APPIDS: - if self.cid == _last_good_auth_check_cid: + if _last_auth_valid: await show_popup( title="U2F", subtitle="Already registered.", @@ -663,18 +659,11 @@ class U2fConfirmRegister(U2fState): def __eq__(self, other: object) -> bool: return ( - isinstance(other, U2fConfirmRegister) - and self.cid == other.cid - and self._req_data == other._req_data + isinstance(other, U2fConfirmRegister) and self._req_data == other._req_data ) class U2fConfirmAuthenticate(U2fState): - def __init__( - self, cid: int, iface: io.HID, req_data: bytes, cred: Credential - ) -> None: - super().__init__(cid, iface, req_data, cred) - def get_header(self) -> str: return "U2F Authenticate" @@ -684,7 +673,6 @@ class U2fConfirmAuthenticate(U2fState): def __eq__(self, other: object) -> bool: return ( isinstance(other, U2fConfirmAuthenticate) - and self.cid == other.cid and self._req_data == other._req_data ) @@ -709,8 +697,9 @@ class U2fUnlock(State): class Fido2State(State): - def __init__(self, cid: int, iface: io.HID) -> None: - super().__init__(cid, iface) + def allow_cid(self, cid: int) -> bool: + # In FIDO2 lock out other channels until user interaction or timeout. + return cid == self.cid def keepalive_status(self) -> int: return _KEEPALIVE_STATUS_UP_NEEDED @@ -908,6 +897,10 @@ class Fido2ConfirmGetAssertion(Fido2State, ConfirmInfo, Pageable): class Fido2ConfirmNoPin(State): + def allow_cid(self, cid: int) -> bool: + # In FIDO2 lock out other channels until user interaction or timeout. + return cid == self.cid + def timeout_ms(self) -> int: return _FIDO2_CONFIRM_TIMEOUT_MS @@ -948,9 +941,6 @@ class Fido2ConfirmNoCredentials(Fido2ConfirmGetAssertion): class Fido2ConfirmReset(Fido2State): - def __init__(self, cid: int, iface: io.HID) -> None: - super().__init__(cid, iface) - async def confirm_dialog(self) -> bool: return await confirm_webauthn_reset() @@ -987,10 +977,12 @@ class DialogManager: loop.close(self.keepalive) self._clear() - def get_cid(self) -> int: - if self.state is None: - return 0 - return self.state.cid + def allow_cid(self, cid: int) -> bool: + return ( + not self.is_busy() + or cid == _CID_BROADCAST + or (self.state is not None and self.state.allow_cid(cid)) + ) def is_busy(self) -> bool: if utime.ticks_ms() >= self.deadline: @@ -1266,6 +1258,9 @@ def msg_register_sign(challenge: bytes, cred: U2fCredential) -> bytes: def msg_authenticate(req: Msg, dialog_mgr: DialogManager) -> Cmd: + global _last_auth_valid + _last_auth_valid = False + if not config.is_unlocked(): new_state: State = U2fUnlock(req.cid, dialog_mgr.iface) dialog_mgr.set_state(new_state) @@ -1296,12 +1291,12 @@ def msg_authenticate(req: Msg, dialog_mgr: DialogManager) -> Cmd: # specific error logged in _node_from_key_handle return msg_error(req.cid, _SW_WRONG_DATA) + _last_auth_valid = True + # if _AUTH_CHECK_ONLY is requested, return, because keyhandle has been checked already if req.p1 == _AUTH_CHECK_ONLY: if __debug__: log.info(__name__, "_AUTH_CHECK_ONLY") - global _last_good_auth_check_cid - _last_good_auth_check_cid = req.cid return msg_error(req.cid, _SW_CONDITIONS_NOT_SATISFIED) # from now on, only _AUTH_ENFORCE is supported