-
Notifications
You must be signed in to change notification settings - Fork 168
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
Passphrase Verification for Seed Backup Flow #291
Conversation
Also wondering if people would want this option in the SeedOptionsView. |
Still on the fence after giving this some consideration... but leaning towards "I like!" I do like the choice of having seedsigner test me on important key material... but the passphrase is already visible as I'm entering it, and it's visible once again during finalization where I also get a chance to see my new fingerprint. That fingerprint, as long as recognized, is good verification all but once in hundreds of millions... less good if only a fraction of it is recognized... personally, I want my attention on the fingerprint but others can have their preference too. Since you propose the choice to skip passphrase verification, I cannot see how it hurts, while I do recognize how it can be helpful. I do see a bug... Not in your implementation @EverydayBitcoiner, but rather in the current flow of adding a passphrase. I'll define it as "User should be able to enter an empty passphrase." which means they effectively are NOT adding a passphrase at all. This minor bug is not harmful in any way, it's just clunky. If you go to add a passphrase and then edit the passphrase to delete all chars, you cannot get back to the seed finalization step without a passphrase... whereas it's my opinion that user should be able to back all the way out and finalize the seed without it... it feels like being stuck in a loop. The work around is trivial, finalize whatever, discard and start over; it's no big deal at all... worth looking at (boyscout rule) the next time someone checks out this code and has the attention of peers' eyes. |
I'm not sure I understand this one. I think that for the rightful owner, there is no harm for them to test themselves as much as they want... until they get it right. For an attacker, they'll choose to step around this and go straight to automated brute-force attack, assuming the wrench was persuasive in getting seedqr/words AND that the fool swinging didn't realize that they might need a passphrase too.
Since I'm still on the fence, but only because of my own personal preferences, I can imagine others who want this would want this:
I've already expressed my concerns (in my naturally redundant nature) that seedsigner is dangerously "leaky" in regards to seed-backup being available everytime we load a seed... and no way to disable it; but this concern is already solved for this particular user. I'll continue to rehash my concern about this anytime it rears it's ugly head, and others can continue to ignore it at their risk and at their own peril. |
Yeah while SeedSigner does give you options to see and view the passphrase during the input process I generaly like to double check important passwords/passphrases by reentering them from my backup (whatever form that may be). I agree that the fingerprint is extremely useful for identification, but that doesn't help me know if my backed up passphrase is right until the next time I load my seed and then I'm in trouble if I wrote it down wrong.
I didn't notice that. I'll have to take a look.
You may be right, I was just trying to think through precautions and this may not be it (see below for some additional thoughts you evoked about precautions).
This is an interesting point. I'm guessing you are partially referring to your Prudently Paranoid issue #273, which I just went back and reread the comments on. My current thoughts as I've mulled over this is that it may be good to have the backup option only available right when you load/create a seed. You could have an option in the SeedOptionsMenu to verify backup (pick correct words, enter passphrase). @kdmukai makes an good point about not relying on the UI Settings for security (#273 (comment)) and that the seed backup used to load the seed is an equivalent or greater risk, which I will concede. That said, security "leak" points could be reduced with UI Settings. A UI layer of security may not be ironclad, but it could it could certainly thwart or prevent more basic attacks or security leaks. I am guessing that @kdmukai does not want to lead users to believe that the in-memory seeds are secure simply because they cannot be accessed via the GUI which is a valid concern, but I'm currently inclined towards shifting how we treat seed words to be similar to how we treat passphrases (#90 (comment)) where you get to see them and make sure you backed them up (i.e. go through the backup process) at the very start, but are then unavailable to view after that. |
Right! I get that it could motivate bad habits to rely on a "settings" parameter... but from another perspective, ability to get to seed backup, or even to access utxos via address explorer or xpub is an exceptional one-time feature that may not be wanted under normal circumstances... AND there is no way to turn these features off, even if these concerns are primary to a user's threat model. My prudently paranoid settings use-case example was a hyperbolic "what if" scenario that is likely bad practice... but I still think it could be deemed sound in some cases... and who is anyone to define another's security/threat model? I can be more redundant, this time with questions. Does seedsigner support single-sig? All of this is a non issue if I just use an m of n multisig where m is greater than 1 AND none of those seeds are also used for singlesig... but seedsigner allows less than this best practice, and even allows me to load multiple seeds... but not to protect it's own interface from leaking them if surprised. |
I think this all depends on the user so it's hard to say what is exceptional and what is normal.
I'm on board with you here though I do understand @kdmukai's concern about not giving users a false sense of security using UI settings. I do think however that UI settings, while not perfect, can improve security incrementally.
While your seedsigner may be "locked" it is not immune to compromise, but if something cannot be access via the GUI it is inherently more secure than a raw seed. That said, I think that as a best practice users should generally treat and protect their loaded seedsigner as if it was their raw seed.
Not totally true as the passphrase in unavailable through the UI. |
I agree with you on this. But since the master hdkey is on the running seedsigner, we can export xpub for new coordinator and sign for those utxos. |
I think the best bugfix here would be to allow an empty passphrase. However, some minor stupid danger in distinguishing an empty passphrase from one that just consists of invisible space(s). Maybe in the empty case, the onscreen messaging can be different. Literally: "No passphrase specified" or something like that. And maybe below that: "Fingerprint remains: This is probably all too wordy, but you get the idea. @jdlcdl, can you log this as a separate issue and maybe tackle the fix? |
@EverydayBitcoiner I'm not convinced that we need another opportunity to verify the passphrase. However, I do think it's awkward to go through the backup process for a seed that we know was loaded with a passphrase. At the very least a warning at the start of the backup process should point out that the current fingerprint (result of seed + passphrase) is different from the "naked" seed that's about to be backed up. A whole side discussion can be had about the wisdom of writing down just the "naked" seed's fingerprint on your backup or if it should reference instead/also the seed+passphrase's fingerprint. |
That seems the most reasonable/consistent approach. The only thing really giving me pause is that the UI just doesn't have room. We'd be adding "Back up seed" alongside "Finalize seed". Having three menu options is somehow exponentially more confusing than simply choosing between two options. And adding an additional step ("Backup? Or continue?") is a big no; loading a seed needs to remain as streamlined as possible. |
I did this mainly because there is some additional verification that comes from actually reentering the passphrase from wherever it is stored (paper, password manager, etc.). This is more thorough and IMO better than simply visually comparing my stored passphrase with the passphrase on the SeedSigner screen. I know that I can often make simple mistakes visually comparing things and the additional step of optionally entering the passphrase from the backup to ensure it was not accidentally written down wrong ensures the integrity of the passphrase better than visually comparing.
Yes, this one of the things that I thought about. Currently I could see someone thinking they backed up the whole thing and not know or forget they are missing the passphrase.
I was thinking about this and trying to figure out what the best option is. If the fingerprint stored with the backup is not the naked fingerprint it would give a potential adversary the information that there is a passphrase while the naked fingerprint would not. As you say, this is a whole different discussion.
This could just be a differentiation between loading a seed vs. creating a seed. Most of my concerns here would generally be during the creation and backup of a new seed. When you load a seed (enter words, scan QR code) I don't know if there should be a backup option available (only scenario I can think of where that could be useful is if you only have a QR and want the words or have the words and want the QR -> this could maybe be an option in tools). I think maybe the creation and backup of a seed should be a single flow (gonna have to be careful here though to not confuse users given that the status quo is that you can backup a key anytime). |
Lots of great discussion and possible next steps above, but specifically for the change offered here, concept NACK. Backing up your naked seed / mnemonic is just a conceptually different activity from entering and verifying potentially many different bip39 passphrases for that one mnemonic. This PR's flow suggests / potentially misleads the user into thinking that the backup represents or only works for the currently-loaded mnemonic+passphrase combo. Recommend this PR be closed, BUT very interested in pursuing some of the side changes discussed along the way. |
I added a section to the end of the seed backup flow that will offer the opportunity for a user to verify their passphrase if the seed they are backing up has a passphrase. I made this up in response to issue #90 as a way for users to verify the passphrase in the backup process while still not showing the passphrase to the user again. This should work for both the seed word backup and the QR backup flows.
One thing that I did not implement in this flow that I think may be useful is a max number of attempts at passphrase verification (like 10 or something) before the seed gets wiped from memory.
Thoughts?