flex-dapps / lighthouse-client-ui

8 stars 3 forks source link

Seed security #11

Open paulhauner opened 3 years ago

paulhauner commented 3 years ago

Presently the user is provided the following actions for their mnemonic:

These are useful, but don't encourage a "paper only" mnemonic which is our usual advice.

I'm leaning towards disabling clipboard/download for mnemonics, but I'm keen to hear feedback for either sides.

kirk-baird commented 3 years ago

Paper here is generally the best option, though knowing people I think it is often the case that they will be lazy and not put in on paper if it's the only option. However, when there is a significant stake involved will people be less lazy?

But also looking at the laziness argument if we give people the option to save it to disk more users will end up saving unencrypted copies locally to their machine.

The download to file also makes it easier for users to save it to an external hard drive for cold storage.

gnattishness commented 3 years ago

AFAIK there are some potential scenarios where displaying the mnemonic on the screen (to be written) can result in compromise and saving to file wouldn't, but these are pretty convoluted and unlikely. E.g. the computer is safe, but there's a hardware backdoor in the monitor.

Otherwise, agreed regarding "paper only" and providing safety-by-default.

Perhaps a reasonable middle-ground could be, when displayed, to allow the mnemonic be selected and copied.

daniel-flex commented 3 years ago

I think people staking will be somewhat savvy users, and will respect the nature of whatever is put in place. Could the best option be the most secure (regardless of UX), but also offer an explanation as to why?

Also, does copying to clipboard share the copied data with all apps?

When talking about a 'paper only' mnemonic above, is that in reference to the user physically writing out the mnemonic on paper, or printing it?

AgeManning commented 3 years ago

I think we should give the option to download to file (from memory metamask still does this).

Not sure how hard it is, but with keepass when you copy to a password to the clipboard it removes it after a period of time. This could probably be dangerous if we did that here.

Maybe the option to download to file is a smaller text and has a hover-over warning.

zedt3ster commented 3 years ago

I'm in favor of disabling clipboard and file download. I think a lot of users, if given the opportunity to backup their seed phrase digitally, might not end up bothering creating a paper back up.

AgeManning commented 3 years ago

I know that for a few of my metamask wallets, especially ones for testnets, I store the backup phrases in a password database.

I think its handy to be able to restore digitally rather than typing them all in. I think users can store seed phrases securely without having to hand write them onto paper.

AgeManning commented 3 years ago

After a bit of thinking. I'm quite in favour of giving the optionality of clipboard and file download (with warnings).

If a user goes through all the warnings and doesn't save their seed phrase, or saves it in a public space, that's on them. I think pop-up warnings are a sufficient measure to prevent people doing bad things. I think removing useful options entirely assuming people will use them incorrectly is overkill.

paulhauner commented 3 years ago

I think removing useful options entirely assuming people will use them incorrectly is overkill.

FYI you can still select the seed and copy/paste it without the button.

The format is slightly odd but still usable:

01
taxi
02
update
03
project
04
chuckle
05
summer
06
insane
07
code
08
illness
09
keep
10
genre
11
smooth
12
hurt
13
knee
14
enable
15
accuse
16
income
17
mimic
18
future
19
tribe
20
review
21
anger
22
knife
23
snow
24
nose
holiman commented 3 years ago

@zedt3ster asked me to chime in :)

Personally, I think there's a benefit in being able to copy it, somehow, to store in a password manager. Perhaps there's no need for an explicity copy-to-clipboard button, as long as the text is selectable in some form. But I can see the argument from both sides, so I don't really have a strong opinion.

One thing that I also think should be present (if not already), is the thing that ledger does: After pressing "continue" on whatever dialog where you had the seed phrase up, you get challenged and must enter some random words, like "enter word 15", to demonstrate that you indeed saved it somewhere.

Also, I meant to raise this somewhere, and this is as good as anywhere, I guess.

In geth, we have sometimes encountered disk bitflips. So data was corrupted, either while in memory or when on disk. This is a nuisance, and may be a cause of problems that people have historically had with being unable to open keystores.

To counter that, we added a double-check: to ensure that after writing the file to disk, we can also read it back and decrypt with the given password. I would recommend all/any key manager to implement the same check.

paulhauner commented 3 years ago

Thanks for your input @holiman!

One thing that I also think should be present (if not already), is the thing that ledger does: After pressing "continue" on whatever dialog where you had the seed phrase up, you get challenged and must enter some random words, like "enter word 15", to demonstrate that you indeed saved it somewhere.

We have 12/24 word confirmation dialogue.

To counter that, we added a double-check: to ensure that after writing the file to disk, we can also read it back and decrypt with the given password. I would recommend all/any key manager to implement the same check.

We save the keypair to disk here and then load it from disk and decrypt it again here.

paulhauner commented 3 years ago

So it seems I need to make a call here, so I'll do that. I'm going to opt for:

The user needs to click "Acknowledge" before the download/copy action takes effect.

@daniel-flex, is this ok from your end? :)

daniel-flex commented 3 years ago

@paulhauner ok from my end - you didn't mention the print button. Presume keep it?

daniel-flex commented 3 years ago

@paulhauner have used the standard window.confirm dialog as it's a pretty safe way of doing it.