multi: Add seed passphrase. #35
Conversation
dcr/loader.go
Outdated
| tweakedSeed = func() []byte { return applyPass(ts[:]) } | ||
| } else { | ||
| tweakedSeed = func() []byte { return seed } | ||
| tweakedSeed = func() []byte { return applyPass(seed) } |
There was a problem hiding this comment.
The tweakedSeed func is kind of ugly now, want to clean this up I think.
dcr/loader.go
Outdated
| if len(wd.EncryptedSeedPass) != 0 { | ||
| encSeedPass, err := hex.DecodeString(wd.EncryptedSeedPass) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("unable to decode encrypted seed pass: %v", err) | ||
| } | ||
| seedPass, err = DecryptData(encSeedPass, params.Pass) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("unable to decrypt wallet seed pass: %v", err) | ||
| } | ||
| } |
There was a problem hiding this comment.
there appears to be no way to specify the wallet seed password unless it exists encrypted in the recovery data. Maybe that's intentional, but it doesn't seem all too useful to me (i would expect passwords to not need to be saved with the other seed data, and to only be provided as user input when needed).
There was a problem hiding this comment.
i think i see why, though i'm still not sure i like it.
effectively, the encryption password becomes the required user input. and instead of only needing to recover by seed words, now you must keep all of the auxiliary data around as well for the encrypted password. without it, if you only have your seed words, you will never be able to recover your real wallet seed.
unless the password is always a constant (like it is in the tests), and the only reason this is done is to expand the key using pbkdf2 to 64 bytes. but then why would we encrypt the password...?
There was a problem hiding this comment.
There is a feature to restore a wallet from saved data. This was done so that wallet backups could leave out the dcrwallet database, because they say it is too big. So we only save the "walletdata" in cake backups. Without the seed passphrase, if one was provided, we could not restore from the walletdata.
We also have to provide the mnemonic when requested, so we need to have the original seed.
I thought saving the seed as is before modification and saving the passphrase separately is the least complicated solution.
There was a problem hiding this comment.
is the seed passphrase different from the passphrase used to unlock the wallet?
except for some potentially far-off compatibility concerns, i'm just not seeing how it's useful to have this saved.
There was a problem hiding this comment.
is the seed passphrase different from the passphrase used to unlock the wallet?
Yes. The same seed can make different wallets based on some data only you know. So if someone finds your mnemonic I guess, they still can't get your seed.
It's being saved so we can restore from the walletdata file. Would you prefer we saved the altered seed?
There was a problem hiding this comment.
no it's fine if it's intentionally being used to generate different seeds from the same mnemonic, as long as you are really really sure you will never forget the various different passwords. i just find it rather unconventional.
There was a problem hiding this comment.
Yeah idk its this thing https://github.com/bitcoin/bips/blob/master/bip-0039.mediawiki#from-mnemonic-to-seed
Looking again it seems to say we always apply the PBKDF2 even with no password. We cant do that with the 15 word seed but I guess we should with the new ones.
JoeGruffins
force-pushed
the
mnemonicpassphrase
branch
from
3888c84 to
f0963c0
Compare
JoeGruffins
force-pushed
the
mnemonicpassphrase
branch
from
f0963c0 to
d6d973d
Compare
|
Now erroring if a passphrase is supplied with 15 words. Also always applying for 12 and 24 word https://github.com/decred/libwallet/compare/f0963c09cf459e0a9b957fffc73b8082e51209dd..d6d973d93d9c069e06e6b3508845b11178928f7d |
JoeGruffins
force-pushed
the
mnemonicpassphrase
branch
from
d6d973d to
17fa8e1
Compare
|
change EncryptedSeedPass -> EncryptedSeedPassHex for clarity |
2 participants
closes #34