trezor / trezor-firmware

:lock: Trezor Firmware Monorepo
https://trezor.io
Other
1.33k stars 649 forks source link

Model T does not conform to CTAP 2.0 specification #3486

Open Strykar opened 8 months ago

Strykar commented 8 months ago

Describe the bug @matejcik on Matrix pointed me here after sharing https://github.com/Yubico/pam-u2f/issues/310

The Trezor Model T does not conform to the CTAP 2.0 spec. This was noticed when another U2F / FIDO2 security key was added to the same system and pam-u2f was configured to manage the systems (GDM) 2FA login.

Firmware version and revision 2.6.3

Desktop/smartphone setup (please complete the following information):

To Reproduce Steps to reproduce the behavior:

  1. Add / enroll a secondary U2F / FIDO2 security key to a system where the Model T is already plugged in and setup
  2. Enable U2F for system login via pam-u2f in /etc/pam.d/gdm-password (auth required pam_u2f.so nouserok origin=pam://hostname appid=pam://hostname debug debug_file=stderr)
  3. Restart GDM, it will take over a minute for the secondary key to start blinking (ready to auth / touch / presence)
  4. Repeat the same process above but without the Model T plugged in and the same process is instant, no delays.
  5. The linked GH pam-u2f issue has extensive logs
  6. Summary from @LDVG who was very helpful and quick to find the issue: Yes, there's little we can do to workaround this. While the CTAP 2.0 specification is not entirely straight-forward on the matter: I would expect a request which has set user presence (UP) to false and user verification (UV) to false to not require any user interaction. This pre-flight process is what pam-u2f uses to determine which authenticator to use before performing the actual authentication ceremony.

Expected behavior Co-exist with other 2FA devices

andrewkozlik commented 8 months ago

I think the motivation behind this behavior is that we don't want Trezor to reveal anything about the user's U2F/FIDO2 credentials unless the device is unlocked, so we keep the application waiting until the user unlocks their Trezor. The application keeps waiting until there is a timeout and then it tries the other authenticator. If the application attempted authentication in parallel for both authenticators at the same time, then this probably wouldn't be an issue. However, I agree that Trezor's overcautious behavior is not really in line with the U2F spec, and revealing ownership of a credential is not a big deal.

I see several options:

  1. If we don't want to reveal anything about the user's U2F/FIDO2 credentials when the device is locked and we don't want Trezor to obstruct the above scenario where the application tries authenticators in series, then when the device is locked, we can disable the U2F interface or decline the authentication request right away, claiming that the credential does not belong to Trezor. Not very user friendly, IMHO. The user will need to unlock Trezor and try to log in again, which breaks UX for single-authenticator scenarios as well as for applications which try authenticators in parallel.
  2. Reveal ownership information about the user's U2F/FIDO2 credentials even in locked state. As I mentioned, doesn't seem like a big deal.
  3. Maintain the status quo, because the problem only concerns applications which try authenticators in series.

In case of option 2, the fix for U2F seems simple. However, I don't have time to test it out:

--- a/core/src/apps/webauthn/fido2.py
+++ b/core/src/apps/webauthn/fido2.py
@@ -1310,7 +1310,7 @@ def _msg_authenticate(req: Msg, dialog_mgr: DialogManager) -> Cmd:
     data = req.data  # local_cache_attribute
     info = log.info  # local_cache_attribute

-    if not config.is_unlocked():
+    if not config.is_unlocked() and req.p1 != _AUTH_CHECK_ONLY:
         new_state: State = U2fUnlock(cid, dialog_mgr.iface)
         dialog_mgr.set_state(new_state)
         return msg_error(cid, _SW_CONDITIONS_NOT_SATISFIED)

For FIDO2 this fix is probably a bit more complex, because there is some more decoding involved and it probably also needs more thought to make sure we don't leak something sensitive.

As for severity, I wouldn't rank this issue very high, because it only concerns applications which try authenticators in series and the workaround is for the user to unlock their Trezor. IIUC, this is clearly indicated by the fact that it lights up with a PIN entry screen in the described scenario.

prusnak commented 8 months ago

I agree with the assessment of low priority. It was omitted in the user report that Trezor asks for unlock and that everything works just fine when the Trezor is unlocked.

Strykar commented 8 months ago

I think the motivation behind this behavior is that we don't want Trezor to reveal anything about the user's U2F/FIDO2 credentials unless the device is unlocked, so we keep the application waiting until the user unlocks their Trezor. The application keeps waiting until there is a timeout and then it tries the other authenticator.

Thanks for the prompt response @andrewkozlik (2) is a reasonable solution but one that I am unwilling to test as this is the only Trezor I have :disappointed:

For FIDO2 this fix is probably a bit more complex, because there is some more decoding involved and it probably also needs more thought to make sure we don't leak something sensitive.

I agree with (and support) the premise the Trezor should not advertise itself as a key owner, by default. If the patch you suggested above could be an option configurable via say the touchscreen or Trezor Suite, that could be a good middle ground, giving users the choice to enable a feature AND have it behave within spec.

As for severity, I wouldn't rank this issue very high, because it only concerns applications which try authenticators in series and the workaround is for the user to unlock their Trezor. IIUC, this is clearly indicated by the fact that it lights up with a PIN entry screen in the described scenario.

Agreed, especially if you feel fixing the FIDO bits is non-trivial. If we agree it is out of spec will this issue be kept open?

UPDATE: It appears the issue might inadvertently be fixed by just entering the PIN once on the Trezor, I will post logs here for review once I have some more info today. pam-u2f also has a nodetect option that may work with some devices.

Strykar commented 8 months ago

I agree with the assessment of low priority. It was omitted in the user report that Trezor asks for unlock and that everything works just fine when the Trezor is unlocked.

To be clear, the Model T does not ever show the PIN entry screen (the behavior you and @andrewkozlik are ascribing) when two U2F devices are connected, it stays on the "Touch to unlock" all the time. Even after I intentionally let the secondary U2F device time out by not touching it, a sign in request does not appear on the Model T.

It is only when I unplug the secondary device, sign in to the Model T that the auth request is shown. I just tested this over a few reboots to be sure. To sum up, the Model T works perfectly if it is the only U2F device on the system.

I am new to U2F, if anything was omitted, it was unintentional and purely of my own ignorance.

andrewkozlik commented 8 months ago

To be clear, the Model T does not ever show the PIN entry screen when two U2F devices are connected, it stays on the "Touch to unlock" all the time.

That's very strange and certainly sounds like a bug. From the log at https://gist.github.com/Strykar/8cf798b60dc77fdee68161386b295a9d I see that Trezor (see line 105) received a GetAssertion command (line 113) and after one minute responded with ERR_OPERATION_DENIED (line 126), which is exactly what I'd expect it to do if left untouched while showing a dialog.

  1. Can you please record the same log as above with the Trezor behaving correctly?
  2. Can you please elaborate on the lock state that causes the Trezor to behave incorrectly? I take it, that it always behaves correctly in unlocked state.
    • Does it behave incorrectly in disconnected state? (Freshly plugged in and locked.)
    • Does it behave incorrectly in manually locked state? (Unlocked at least once after plugging in and then locked by holding your finger on the homescreen.)
    • Does it behave incorrectly in autolocked state. (Unlocked at least once after plugging in and then locked by leaving untouched for the autolock time, which is 10 minutes by default.)