weak password hashing used

[undeath]

Electrum is using relatively weak password hashing to protect the encrypted wallet.

https://github.com/spesmilo/electrum/blob/f7b14cb27f42700672168c250fd561e8f00e17d5/lib/storage.py#L122

It does use the (kind of outdated) PBKDF2 key derivation with no salt and a quite low iteration count. Wouldn't it be more sensible to save PBKDF2 settings in the wallet file, especially including a random salt and ideally a user-adjustable high iteration count (>10000)?

Omitting a random salt makes Electrum wallets prone to rainbow table attacks and the low, fixed iteration count is making the creation of such tables even easier. I realise there is still the work of doing the EC decryption but this is allowing unnecessary shortcuts for an attacker.

[dabura667]

This is only protecting the metadata. The private keys are protected with a different function.

[undeath]

You are correct. The snipped quoted above is only used if the wallet is encrypted. But the same password is used to encrypt the wallet (storage) itself and its private key(s) (keystore): https://github.com/spesmilo/electrum/blob/f03cb757131cd5bcef376ccc77b9214e5837a230/lib/base_wizard.py#L354-L357

The function to encrypt the private keys themselves is indeed even weaker and only using two iterations of SHA-256 without any salting (used in pw_encode/pw_decode): https://github.com/spesmilo/electrum/blob/f03cb757131cd5bcef376ccc77b9214e5837a230/lib/bitcoin.py#L244-L247

[SomberNight]

(from https://github.com/spesmilo/electrum/pull/4838)

Currently, when keystore encryption is enabled, the symmetric key used to encrypt the seed/private keys is sha256d(password). (If storage encryption is also enabled, then the whole wallet file is encrypted again, there the key derivation uses PBKDF2 with 1024 iterations of sha512)

Making the KDF more expensive is blocked on https://github.com/spesmilo/electrum/issues/4909 It's more complicated than I had thought... :/

Perhaps it would make sense to use the same KDF in both cases. Anyway, to change the storage KDF, metadata needs to be put in the header of the encrypted file. To change the keystore KDF, metadata needs to be put in the (decrypted) storage.

[SomberNight]

Anyway, to change the storage KDF, metadata needs to be put in the header of the encrypted file.

See https://github.com/spesmilo/electrum/issues/5999. If we had a proper header for the wallet file, we could just put the properties of the KDF of storage encryption into it (algorithm, number of iterations, salt, etc), and easily make it stronger without further backwards compatibility changes.