[Bug] BIP39 wallet with passphrase should not accept blank on re-enter the BIP39 passphrase window #603
Comments
tadeubas
changed the title
|
Not sure what you mean. When opening a wallet file, you're only asked once for the passphrase. Leaving blank here is perfectly fine and actually part of the whole idea (e.g. plausible deniability). When importing a new keystore, you're asked for the passphrase twice and you'll receive an error if the phrases do not match, including your case of leaving it blank the second time. I tried reproducing this on MacOS and Win 10.
I agree that Passphrase entered may seem confusing if you didn't even enter one. But the user should still be able to see that he was prompted for a passphrase and that the feature is enabled for the current keystore. Maybe changing this to Passphrase enabled but left blank would be best. |
|
You forgot the scenario from
The scenario you tested it asks for the BIP39 passphrase right away and give a feedback if it doesn't match. The problem is that the passphrase must be required if the user informed that the wallet has a passphrase (especially on the scenario from |
|
Oh, now I understand! That is certainly a bug and even potentially dangerous because if a user misspells his passphrase this way (the second time) he will not be aware of his mistake and may send funds to the wrong wallet. The re-enter prompt is basically completely missing here since the "second" prompt is just the default prompt as if you were regularly opening the wallet file. |
|
I think the best solution is storing the "complelete HD wallet" (The HD seed from BIP39 + passphrase) to the *.mv.db file. In order to reducing the chances of user mistyping the BIP39 passphrase when BIP 39 wallet import. I suggest user need to type twice of the BIP39 passphrase when wallet import. Just same as the of the wallet encrypt password setting scheme. The HD seed from this "BIP39 + passphrase" should only create "once" when wallet import, then save the "complelete" HD wallet to *.mv.db file. Current Sparrow wallet (1.6.5) has create a new HD seed each time from user enter passphrase when user opening the wallet file (*.mv.db file). This is completely unnecessary, and need to remove this function on the future version. User don't required to re-enter the BIP39 passphrase anymore when open the wallet file (*.mv.db) because sparrow should not create HD seed is this time. The wallet is still encrypted using the wallet password. So there is no security reduction of the sparrow wallet. |
|
@Bzvwf16WSDKoSZd8 I disagree with your solution because it removes the possibility of a plausible deniability, unless if implemented another way, like using the wallet password (BlueWallet has an app password to do plausible deniability). Now I wondered what Sparrow does when it is closed and I was using a wallet with a BIP39 passphrase... It only saves the wallet seed encrypted and then constructs the wallet with passphrase everytime in memory only when the user types the BIP39 passphrase? |
But the whole point of an optional passphrase is to act as a second factor and/or plausible deniability, which as @tadeubas already explained, would be mitigated. Software or hardware wallets should not remember your passphrase, that's part of the use case, unless you specifically want them to (e.g. Ledger Attach to PIN). |
|
@tadeubas Look at this reddit post. The user mistyping BIP39 passphrase seems to be the bigger problem than the plausible deniability at the moment. BIP39 Wallet haven't store any passphrase in plain-text. It just store the 512 bit BIP32 HD seed. def to_seed(cls, mnemonic: str, passphrase: str = "") -> bytes:
mnemonic = cls.normalize_string(mnemonic)
passphrase = cls.normalize_string(passphrase)
passphrase = "mnemonic" + passphrase
mnemonic_bytes = mnemonic.encode("utf-8")
passphrase_bytes = passphrase.encode("utf-8")
stretched = hashlib.pbkdf2_hmac(
"sha512", mnemonic_bytes, passphrase_bytes, PBKDF2_ROUNDS
)
return stretched[:64]The plausible deniability using BIP39 passphrase when wallet opening time should be disable by default and only enable manually by user. On my solution, I suggest "user need to type twice of the BIP39 passphrase when wallet import as same as the of the wallet encrypt password setting scheme to reducing the chances of user mistyping. |
|
@sutterseba This. |
|
@Bzvwf16WSDKoSZd8 Thx to let us known these users cases... I opened this issue because I was also confused about the way Sparrow deals with the BIP39 passphrase via I think the solution is up to the Sparrow main developer to decide, but definitely the way Sparrow implemented BIP39 passphrase and plausible deniability with this passphrase is causing issues with the users... maybe BIP39 passphrase must always be typed twice to avoid mistakes? Also would be good to have an eye icon to "show/hide the password" like we see on phones. Plausible deniability could be implemented with the wallet password instead of BIP39 passphrase... @craigraw what do you think would be the best way to solve this issue of the user mistyping the BIP39 passphrase? |
|
With respect to using BIP39 passphrases on Sparrow, the current design maximises security (at the expense of, for example, telling the user they have entered an incorrect passphrase). Unfortunately there is no perfect solution here. First off, it makes no sense at all to store the passphrase in the wallet file. In that case it simply acts as a 25th word, and 24 words already provide more than sufficient entropy. The point of the passphrase is to create the well known "something I have + something I know" security paradigm. In this case, the wallet file is something you have, and the passphrase is something you know. So let's put that aside. Secondly, there is an argument that Sparrow should store something derived from the passphrase (not the passphrase itself). For example, Sparrow could store the master fingerprint of the seed (derived from the BIP39 words + passphrase) which is a 4 byte value that should (with some rare collisions) identify whether an entered passphrase is correct. But there is a significant cost to doing so in reducing security by making a brute force attack easier. To explain further, a Sparrow Wallet *.mv.db file stores nothing derived from the passphrase. This means that if the wallet file is decrypted, a brute force attack to locate any funds must not only calculate the seed from the BIP39 words + passphrase, but also calculate addresses and search for any funds on the blockchain. The first part of this (deriving the seed) is computationally inexpensive, since BIP39 uses the PBKDF2 algorithm which is now very old and simple to execute, even with the prescribed 2048 rounds. (By the way, this is a issue with Electrum's wallet file encryption, but I digress). If Sparrow stored the derived master fingerprint in the wallet file, then a brute force attack could simply iterate passphrase guesses until it found one that matched the master fingerprint. Without this information however, it also needs to (at least) generate the first address and search the entire blockchain for a transaction output to it. This is not only computationally much more expensive, but also scales as the blockchain grows, giving some protection as computing resources inevitably become cheaper and brute force attacks become easier over time. Although Sparrow does not have the information to tell you if you have entered an incorrect passphrase, it can see if, on loading a wallet, the retrieved transaction history is entirely different from that stored in the wallet file. If it detects this, it does a full refresh of the wallet, discarding all stored transaction history and loading everything from the server again. So if you enter an incorrect passphrase, you will be presented with an empty wallet as soon as the refresh begins. This speaks somewhat to plausible deniability, but really is just the best Sparrow can do given it is maximising security over other aspects of passphrase use. The question you need to ask yourself is whether you are prepared to give up security for UX - which is a difficult question to answer in general, since it ultimately depends on the amount of entropy contained in a given passphrase. |
|
@craigraw thx a lot for the explanations! Without changing the way Sparrow works, which is awesome, I think that asking for the user to always type twice the BIP39 passphrase is enough (so Sparrow UI only check if the user typed 2 times the same passphrase, otherwise asks to type the passphrase the first and second time again). This would avoid mistyping by the user (the Reddit showed 2 users that lost funds because of mistyping... it is a lot harder to mistype equally 2 times the same passphrase). Also an eye option to click (show/hide) the password the user is typing would be great to avoid mistyping, this without not compromise the Sparrow security neither plausible deniability because it is just an UI change. What do you think? |
|
@craigraw But Sparrow already using a strong encryption "Argon2" (winner of the Password Hashing Competition in 2015), The situation you mentioned unlikely to happen in the most cases when the user provides a strong password. I strongly agree that security is very important, but also to strike a balance between user experience.
For case 1) For case 2) The user need to typing BIP39 passphrase on Sparrow wallet is obviously the second type of users. |
|
I given this a lot of thought, and ultimately I don't want to reduce Sparrow's potential security for ease of use. Many users may store significant amounts in software wallets (especially those using Whirlpool) and not everyone can afford hardware wallets. Further, a common security practice is "defence in depth" which in this case suggests one might choose to not rely not on the wallet password alone. Ultimately, it's a choice I don't want to take away from the user. With respect to the OP's points, Sparrow is here asking for the passphrase as per the normal wallet load sequence, which it uses even when importing from a seed. I'll need to see whether it makes sense to complicate the load process to handle this special case. I agree the password dialog should not accept an empty password, and reflecting an empty passphrase with a "No passphrase" message on the seed view dialog seems a reasonable improvement. |
|
@craigraw Thank you for the explanation. I just don't know why Sparrow can giving users a choice to disable the wallet encrytion, but can't giving users a choice to disable BIP39 passphrase plausible deniability? Both are related to security right? |
|
Yes, they are related to security, but otherwise not closely related. It is really just a convenience factor that causes some wallets to conflate passwords and passphrases. Passphrase plausible deniability is difficult to support well given the above considerations (Sparrow must receive a response from the server before it can refresh the wallet). That said, it is simple enough to achieve some form of it by just using two passphrases. Adding a further layer of configuration for no passphrase wallets to optionally request a passphrase will further complicate things, and I'm not sure this would materially improve things. |
|
As of 9022438:
|
and others
When Sparrow asks to Re-enter the BIP39 passphrase, it should not accept blank! Otherwise when you click on
Settings > Keystores > View Seed...you will see the text "Passphrase entered" but the wallet will be the same as a "No passphrase" causing misunderstanding to the user.Also the text of BIP39 passphrase window should always be the same "Re-enter", and not the "Enter" txt that apeears when you open Sparrow with an imported wallet loaded... also, none of them should accept a blank password.
Sparrow
1.6.5The text was updated successfully, but these errors were encountered: