feat(core): Ignore channel ID in U2F.

Safari browser changes the CID for every single operation. We need to
keep up the same U2F dialog for different CIDs as long as the dialog
corresponds to the same request data.
pull/2848/head
Andrew Kozlik 1 year ago committed by Andrew Kozlik
parent ccf08df07d
commit deb38a2db5

@ -0,0 +1 @@
Ignore channel ID in U2F.

@ -211,8 +211,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. # 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 _last_wink_cid = 0
# The CID of the last successful U2F_AUTHENTICATE check-only request. # Indicates whether the last U2F_AUTHENTICATE had a valid key handle.
_last_good_auth_check_cid = 0 _last_auth_valid = False
class CborError(Exception): class CborError(Exception):
@ -545,10 +545,7 @@ async def handle_reports(iface: HID) -> None:
req = await _read_cmd(iface) req = await _read_cmd(iface)
if req is None: if req is None:
continue continue
if dialog_mgr.is_busy() and req.cid not in ( if not dialog_mgr.allow_cid(req.cid):
dialog_mgr.get_cid(),
_CID_BROADCAST,
):
resp: Cmd | None = cmd_error(req.cid, _ERR_CHANNEL_BUSY) resp: Cmd | None = cmd_error(req.cid, _ERR_CHANNEL_BUSY)
else: else:
resp = _dispatch_cmd(req, dialog_mgr) resp = _dispatch_cmd(req, dialog_mgr)
@ -617,6 +614,10 @@ class State:
self.iface = iface self.iface = iface
self.finished = False 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: def keepalive_status(self) -> int:
# Run the keepalive loop to check for timeout, but do not send any keepalive messages. # Run the keepalive loop to check for timeout, but do not send any keepalive messages.
return _KEEPALIVE_STATUS_NONE return _KEEPALIVE_STATUS_NONE
@ -657,14 +658,9 @@ class U2fState(State):
class U2fConfirmRegister(U2fState): class U2fConfirmRegister(U2fState):
def __init__(
self, cid: int, iface: HID, req_data: bytes, cred: U2fCredential
) -> None:
super().__init__(cid, iface, req_data, cred)
async def confirm_dialog(self) -> bool: async def confirm_dialog(self) -> bool:
if self._cred.rp_id_hash in _BOGUS_APPIDS: if self._cred.rp_id_hash in _BOGUS_APPIDS:
if self.cid == _last_good_auth_check_cid: if _last_auth_valid:
await show_popup( await show_popup(
"U2F", "U2F",
"This device is already registered with this application.", "This device is already registered with this application.",
@ -684,23 +680,17 @@ class U2fConfirmRegister(U2fState):
def __eq__(self, other: object) -> bool: def __eq__(self, other: object) -> bool:
return ( return (
isinstance(other, U2fConfirmRegister) isinstance(other, U2fConfirmRegister) and self._req_data == other._req_data
and self.cid == other.cid
and self._req_data == other._req_data
) )
class U2fConfirmAuthenticate(U2fState): class U2fConfirmAuthenticate(U2fState):
def __init__(self, cid: int, iface: HID, req_data: bytes, cred: Credential) -> None:
super().__init__(cid, iface, req_data, cred)
async def confirm_dialog(self) -> bool: async def confirm_dialog(self) -> bool:
return await _confirm_fido("U2F Authenticate", self._cred) return await _confirm_fido("U2F Authenticate", self._cred)
def __eq__(self, other: object) -> bool: def __eq__(self, other: object) -> bool:
return ( return (
isinstance(other, U2fConfirmAuthenticate) isinstance(other, U2fConfirmAuthenticate)
and self.cid == other.cid
and self._req_data == other._req_data and self._req_data == other._req_data
) )
@ -725,8 +715,9 @@ class U2fUnlock(State):
class Fido2State(State): class Fido2State(State):
def __init__(self, cid: int, iface: HID) -> None: def allow_cid(self, cid: int) -> bool:
super().__init__(cid, iface) # In FIDO2 lock out other channels until user interaction or timeout.
return cid == self.cid
def keepalive_status(self) -> int: def keepalive_status(self) -> int:
return _KEEPALIVE_STATUS_UP_NEEDED return _KEEPALIVE_STATUS_UP_NEEDED
@ -906,6 +897,10 @@ class Fido2ConfirmGetAssertion(Fido2State):
class Fido2ConfirmNoPin(State): 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: def timeout_ms(self) -> int:
return _FIDO2_CONFIRM_TIMEOUT_MS return _FIDO2_CONFIRM_TIMEOUT_MS
@ -946,9 +941,6 @@ class Fido2ConfirmNoCredentials(Fido2ConfirmGetAssertion):
class Fido2ConfirmReset(Fido2State): class Fido2ConfirmReset(Fido2State):
def __init__(self, cid: int, iface: HID) -> None:
super().__init__(cid, iface)
async def confirm_dialog(self) -> bool: async def confirm_dialog(self) -> bool:
from trezor.ui.layouts.fido import confirm_fido_reset from trezor.ui.layouts.fido import confirm_fido_reset
@ -989,10 +981,12 @@ class DialogManager:
loop.close(self.keepalive) loop.close(self.keepalive)
self._clear() self._clear()
def get_cid(self) -> int: def allow_cid(self, cid: int) -> bool:
if self.state is None: return (
return 0 not self.is_busy()
return self.state.cid or cid == _CID_BROADCAST
or (self.state is not None and self.state.allow_cid(cid))
)
def is_busy(self) -> bool: def is_busy(self) -> bool:
if utime.ticks_ms() >= self.deadline: if utime.ticks_ms() >= self.deadline:
@ -1296,6 +1290,9 @@ def _msg_register_sign(challenge: bytes, cred: U2fCredential) -> bytes:
def _msg_authenticate(req: Msg, dialog_mgr: DialogManager) -> Cmd: def _msg_authenticate(req: Msg, dialog_mgr: DialogManager) -> Cmd:
global _last_auth_valid
_last_auth_valid = False
cid = req.cid # local_cache_attribute cid = req.cid # local_cache_attribute
data = req.data # local_cache_attribute data = req.data # local_cache_attribute
info = log.info # local_cache_attribute info = log.info # local_cache_attribute
@ -1330,12 +1327,12 @@ def _msg_authenticate(req: Msg, dialog_mgr: DialogManager) -> Cmd:
# specific error logged in _node_from_key_handle # specific error logged in _node_from_key_handle
return msg_error(cid, _SW_WRONG_DATA) return msg_error(cid, _SW_WRONG_DATA)
_last_auth_valid = True
# if _AUTH_CHECK_ONLY is requested, return, because keyhandle has been checked already # if _AUTH_CHECK_ONLY is requested, return, because keyhandle has been checked already
if req.p1 == _AUTH_CHECK_ONLY: if req.p1 == _AUTH_CHECK_ONLY:
if __debug__: if __debug__:
info(__name__, "_AUTH_CHECK_ONLY") info(__name__, "_AUTH_CHECK_ONLY")
global _last_good_auth_check_cid
_last_good_auth_check_cid = cid
return msg_error(cid, _SW_CONDITIONS_NOT_SATISFIED) return msg_error(cid, _SW_CONDITIONS_NOT_SATISFIED)
# from now on, only _AUTH_ENFORCE is supported # from now on, only _AUTH_ENFORCE is supported

Loading…
Cancel
Save