trezor / trezor-firmware

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

Passphrase identification #2353

Open andrewkozlik opened 2 years ago

andrewkozlik commented 2 years ago

Trezor allows handling multiple passphrases at the same time. If the user enters two different passphrases and then sees a GetAddress dialog, then they can see the path of the address, but not the passphrase it belongs to.

If more than one passphrase is active in the session cache, then Trezor should show some indication of which one is being used in GetAddress, SignTx and when confirming access to a SLIP-25 account.

How should the passphrases be identified:

Related issues:

Hannsek commented 1 year ago

We can add the "name" of the wallet on the screen where the user sees the passphrase. Then we can add the "name" of the wallet also on the account detail screen in receive flow.

"name" here stands for "hidden wallet #X"

image

matejcik commented 1 year ago

That looks reasonable, and I can't foresee any good attack scenarios either.

The only thing comes to mind, malware wants you to spend from account A, so while unlocking account A, the label shown will be account B, we cross our fingers that the user doesn't notice, then we get them to unlock account B and spend from account B, but the transaction actually spends from A (which now has the same label). This already assumes the user is going to send funds to the attacker, so it doesn't actually seem to achieve much. Maybe prove that account A exists, but not in a communicable way: given that accounts A and B can't be linked, it could have been anyone who sent that transaction.

Malware could also cause user confusion by, e.g., forcing the same label for all sessions, but that's not worse than what we have now. (and, again, banking on the user not noticing, which is kind of the same as typo-attacking the passphrase entry).

(defending against the same label used by multiple passphrases does not seem worth the effort)

With temporary labels provided by Suite, we can even sidestep the whole labeling issue and trust Suite with whatever text it submits, so if your hidden wallet is named, Trezor can show that name.

andrewkozlik commented 1 year ago

Do I understand correctly, that you are in favor of the proposal which I referred to as "Temporary text label from Suite" above? If so then there is still the question of how to store the passphrase (in the session cache or using the temporary MAC solution) and the question of what to do if the host application does not provide a label, e.g. old version of Suite or some 3rd party tool.

matejcik commented 1 year ago

yes from me, how about @Hannsek

still several open questions:

if the string is not provided, we can use "Standard wallet" as a default for no-passphrase, "Hidden wallet" for passphrased. we could have an internal counter, but I don't like scanning open sessions for the same passphrase so this counter would increment every time and you'd get "hidden wallet #7" if it's the 7th time you are opening the same passphrase.

Temporary MAC would be better in that Trezor doesn't need to spend cache memory on this. A significant drawback is that this relies on the host to provide the string every time we need it, i.e., for every currency that wants to show which wallet you're spending from. (so in other words all of them). So unfortunately putting this into session cache seems like the only viable solution.

andrewkozlik commented 1 year ago
  • how to provide the string in the first place?
    • probably in a PassphraseAck field

Sounds good.

  • should firmware recognize standard wallet by itself; i.e., if passphrase is empty but the host claims this is "Hidden Wallet Redesign Passphrase #1", should we trust it?
    • probably yes, the user could have a custom label for their standard wallet?

Agreed, makes sense.

if the string is not provided, we can use "Standard wallet" as a default for no-passphrase, "Hidden wallet" for passphrased. we could have an internal counter, but I don't like scanning open sessions for the same passphrase so this counter would increment every time and you'd get "hidden wallet #7" if it's the 7th time you are opening the same passphrase.

Also makes sense, the only problem I see is collision with an already provided label. For example Suite provides the label "Hidden wallet #2", then another app that doesn't provide labels opens a passphrased wallet and we automatically assign the same label, because it's the second passphrased wallet.

Temporary MAC would be better in that Trezor doesn't need to spend cache memory on this. A significant drawback is that this relies on the host to provide the string every time we need it, i.e., for every currency that wants to show which wallet you're spending from. (so in other words all of them). So unfortunately putting this into session cache seems like the only viable solution.

I think it's sufficient to send the label and MAC only when switching between sessions, so only in Initialize. Then it can be stored only for the active session, not for each one individually.

matejcik commented 1 year ago

more significant: how to return the MAC to the host?

if the host provides label in PassphraseAck, there is no standard response that could carry the MAC back. we'd either need to insert an aux flow, like LabelMacRequest as a response to PassphraseAck (which will break every 3rd party)

or create a special flow, say, SetWalletLabel(label="what") -> PassphraseRequest -> PassphraseAck -> WalletLabel(mac=0xabcdef) which causes some corner cases:

OTOH, we could do SetWalletLabel(label="what", mac=0xabcdef) as the obvious place to provide the mac

andrewkozlik commented 1 year ago

if the host provides label in PassphraseAck, there is no standard response that could carry the MAC back. we'd either need to insert an aux flow, like LabelMacRequest as a response to PassphraseAck (which will break every 3rd party)

if the host provides label in PassphraseAck, there is no standard response that could carry the MAC back. we'd either need to insert an aux flow, like LabelMacRequest as a response to PassphraseAck (which will break every 3rd party)

Nit: I think the LabelMacRequest would have to come after the ButtonAck, where the user confirms the passphrase on the Trezor screen.

I don't think it has to break 3rd parties. The LabelMacRequest can be sent to the host only if the host provided a label. (For unlabeled walets, i.e. for hosts that don't support the functionality, we'd have to store the wallet sequence number in the cache.)

I think the above can work fine in the scenario where the user is opening a wallet that is remembered in Suite. What seems more problematic are the following:

  1. What to do if the user is creating a new wallet but actually enters the passphrase of an already labelled wallet? ("Passphrase duplicated" -> "Continue to discovered wallet" in Suite)
  2. What to do if the user changes the wallet label in Suite after the wallet is loaded on Trezor? (This may also answer point 1.)

OTOH, we could do SetWalletLabel(label="what", mac=0xabcdef) as the obvious place to provide the mac

I don't understand. The label is coming from Suite whereas the mac is being returned by Trezor.

Hannsek commented 1 year ago

todo: https://github.com/trezor/trezor-firmware/pull/2925#issuecomment-1494088275

SecurityIsHard69420666 commented 1 year ago

Another solution is to use a deterministic graphical icon (and/or accompanying text), just like my Github icon here.