Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redesign Passphrase #1

Closed
slush0 opened this issue Mar 5, 2018 · 21 comments
Closed

Redesign Passphrase #1

slush0 opened this issue Mar 5, 2018 · 21 comments
Assignees
Labels
core Trezor Core firmware. Runs on Trezor Model T and T2B1. T1B1 legacy Trezor One trezorlib Python library and the command line trezorctl tool.
Milestone

Comments

@slush0
Copy link
Contributor

slush0 commented Mar 5, 2018

Make the workflow same for client for both non-passphrase device and passphrase device. Currently, client receives state only when passphrase is enabled.

@prusnak
Copy link
Member

prusnak commented Mar 16, 2019

IMG_20181029_135355

@prusnak
Copy link
Member

prusnak commented Mar 16, 2019

We can ask for the passphrase 2x to avoid typos. And only ask for the passphrase once when the state is provided and we can detect these typos.

@prusnak
Copy link
Member

prusnak commented Mar 16, 2019

Another option is to show passphrase on the device (if the user wants to - i.e. dialog Show passphrase?) after it has been entered.

@tsusanka
Copy link
Contributor

Another option is to show passphrase on the device (if the user wants to - i.e. dialog Show passphrase?) after it has been entered.

Or add an icon that shows the passphrase (as is used sometimes). I'm not sure it can fit on the screen though.

Peek 2019-03-16 22-38

@prusnak prusnak transferred this issue from trezor/trezor-core Apr 15, 2019
@tsusanka tsusanka added this to the backlog milestone Apr 15, 2019
@prusnak prusnak added architecture core Trezor Core firmware. Runs on Trezor Model T and T2B1. T1B1 legacy Trezor One trezorlib Python library and the command line trezorctl tool. labels Apr 15, 2019
@prusnak prusnak modified the milestones: backlog, backlog-legacy Apr 15, 2019
@prusnak prusnak changed the title Rethink PassphraseStateRequest Redesign Passphrase May 17, 2019
@prusnak prusnak modified the milestones: backlog-legacy, backlog May 17, 2019
@prusnak
Copy link
Member

prusnak commented May 17, 2019

Related to Protocol redesign: #79

@andrewkozlik
Copy link
Contributor

andrewkozlik commented May 17, 2019

Some good points about passphrase design were made here: https://www.reddit.com/r/Bitcoin/comments/bp0nqs/trezor_or_ledger_which_one_is_better/enof517?utm_source=share&utm_medium=web2x

To summarize:

  • Make it possible to easily switch between passphrases on the fly.
  • Get rid of enable/disable passphrase as a configuration option. There should be no indication of whether in the past the device was used with a (non-empty) passphrase or not.
  • Use a 2nd PIN to unlock the device and automatically enter a preconfigured passphrase.

I think the first two points can be solved by adding a "Switch passphrase" button to the web wallet. As for the third point, I actually think that this is in contradiction with plausible deniability. For one thing, there is likely to be some side-channel information, which could reveal whether the primary or secondary PIN was entered. So in general I am not too keen on supporting multiple PINs. However, I would like to make it possible to configure a default passphrase which would be automatically used upon start-up. The reason is that people may want to use a passphrase for various reasons:

  • Plausible deniability, i.e. to have a real wallet and a decoy wallet.
  • Providing additional protection, because they do not trust the PIN and hardware to protect their seed if the wallet is stolen.
  • Providing additional protection in case their paper backup is stolen.

Say you are worried about protecting your paper backup but you trust the hardware and PIN to provide sufficient protection. In that case entering a passphrase during every start-up can be a real burden, which is why you'd like to configure it.

@prusnak
Copy link
Member

prusnak commented May 17, 2019

I strongly agree with the first and second Reddit suggestions. Strongly Disagree with the third one.

I would like to make it possible to configure a default passphrase which would be automatically used upon start-up.

Where would this passphrase be stored? I think it is antipattern to store it anywhere. For various usability and security reasons.

@matejcik
Copy link
Contributor

agree with @prusnak. Mainly because if passphrase is stored, it becomes another piece of "static" data that you don't remember after a month of not needing it, and you need to back it up somehow. Either you add it to the seed (which defeats point (3)), or you handle it separately (which gives you a brand new point of failure).

@andrewkozlik
Copy link
Contributor

Where would this passphrase be stored? I think it is antipattern to store it anywhere. For various usability and security reasons.

It would be stored encrypted in the storage just like the mnemonic. This doesn't prevent you from using more passphrases. For example, you can have:

  • passphrase1 to access your regular accounts which you use often and where you have say around $1000, and
  • passphrase2 to access your savings accounts which you use rarely and where you have larger sums.

You configure passphrase1 to be stored on Trezor and used as default, whereas passphrase2 is not stored anywhere. That way you can easily access your regular accounts. If somebody steals your paper backup, they are not able to access neither your regular accounts nor your savings accounts. If they steal your Trezor and guess the PIN or extract the sensitive data by some means, then they gain access to your regular accounts but not to your savings accounts. So this feature merely provides a way to fine-tune your risk/usability tradeoff.

@prusnak
Copy link
Member

prusnak commented May 17, 2019

I don't like that, honestly. It introduces confusion and people will surely forget the stored passphrase.

@tsusanka
Copy link
Contributor

Consider splitting up reset device and backup device as part of this issue, because it is a breaking change for wallet / third parties as well.

@tsusanka
Copy link
Contributor

tsusanka commented Mar 30, 2020

Two QA scenarios I have forgotten:

For 2.3.0:

  • Enable the "passphrase always on device" setting using trezorctl set passphrase enabled -f.
  • Test against both Wallet and Suite that the passphrase is always requested on the device.

For 1.9.0 and 2.3.0:

  • Test basic passphrase usage in Electrum.

@sorooris
Copy link
Contributor

sorooris commented Apr 7, 2020

  1. QA OK

2.3.0 (0b7a844)
Suite: bc1225231da00906c40a46d8cf5eac32bdfda206
Wallet (stage): 59e011aabb0d494ba569028e3eb8a2ad51c9eabc
Next Wallet (stage): 08826e1e8555a83d84eeedb55b5995c508ed8518

Passphrase was correctly requested only on the device, per session.

  1. QA OK

2.3.0 (0b7a844)
1.9.0 (0b7a844)
Electrum 3.3.8

Passphrase was correctly prompted only once per session, tested send & receive, sign & verify.

A trivial visual bug was present in the passphrase entry prompt in the app.
image

@tsusanka
Copy link
Contributor

tsusanka commented Apr 7, 2020

The visual bug should be fixed in Electrum's master, se spesmilo/electrum#6064 (comment) . So I consider this done 🎉

isis-mc pushed a commit to isis-mc/trezor-firmware that referenced this issue Oct 22, 2021
 * ADD a custom group element contraction function to work around
   non-roundtripping of donna's normal `ge25519_pack()` and
   `ge25519_unpack_negative_vartime()` functions.
 * ADD a test for roundtripping.
 * FIXES part of trezor#1.
isis-mc pushed a commit to isis-mc/trezor-firmware that referenced this issue Oct 22, 2021
This test passes but the test for encoding a basepoint still does not
pass!  Argggghghggh.

 * FIXES part of trezor#1.  Not really.  But it helps narrow down the issue.
mmilata pushed a commit that referenced this issue Aug 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Trezor Core firmware. Runs on Trezor Model T and T2B1. T1B1 legacy Trezor One trezorlib Python library and the command line trezorctl tool.
Projects
None yet
Development

No branches or pull requests

9 participants