[bug] Recovery of BIP-39 mnemonic from SLIP-39 is cryptographically incorrect

[alvroble]

Description

I’d like to highlight a potentially misleading feature in the SLIP-39 implementation in embit. This issue was found while developing a SLIP-39 to BIP-39 recovery feature in SeedSigner (SeedSigner/seedsigner#636). Specifically, the potential problem is in the method that allows recovery of a BIP-39 mnemonic from SLIP-39 shares:

embit/src/embit/slip39.py

Lines 353 to 359 in 9d59774

	@classmethod
	def recover_mnemonic(cls, share_mnemonics, passphrase=b""):
	"""Recovers the BIP39 mnemonic from a bunch of SLIP39 mnemonics"""
	shares = [Share.parse(m) for m in share_mnemonics]
	share_set = ShareSet(shares)
	secret = share_set.recover(passphrase)
	return mnemonic_from_bytes(secret)

According to @iancoleman/slip39#1, this process is not practically meaningful and may cause user confusion:

• SLIP-39 and BIP-39 are separate schemes. They use different algorithms to generate the master secret/seed from a mnemonic and passphrase.
• Combining SLIP-39 shares and using the result as entropy for a BIP-39 mnemonic will create a different wallet, not the original one. This could result in users thinking they have restored their original wallet, while in reality, they have generated a new, unrelated seed.
• The process from SLIP-39 shares to BIP-39 mnemonic is not cryptographically feasible. PBKDF2-SHA-512 is a one-way function.
• Allowing this feature may give users a false sense of security and could lead to accidental loss of funds.

Also see https://trezor.io/guides/backups-recovery/general-standards/slip39-faqs:

No. Converting SLIP39 wallets back to BIP39 is not possible due to fundamental differences in how both standards handle backups and security.

Request

It may be prudent to either:

• Remove this method,
• add explicit warnings in documentation and code to prevent misuse and explain the risks, or
• modify the code to recover just a BIP32 HD Key from the master secret as entropy.

[8de2fdb0]

Follow up from #91

@alvroble

Let's discuss generate_shares and recover_mnemonic

In cases where you generate a new secret you can use the bip32 seed and back it up using slip39.
I think that is the most strait forward approach.

The slip39 spec states:

SLIP-0039 can be used to back up any master secret S which satisfies the length constraints described above. However, any application implementing SLIP-0039 for backing up a BIP-0032 Hierarchical Deterministic Wallet MUST use the BIP-0032 master seed as the SLIP-0039 master secret S. To clarify, this is the initial generated seed byte sequence of 128-512 bits, which is used as the input to HMAC-SHA512 for deriving the BIP-0032 master node.

In this library you can see two functions in the bip39 class

• mnemonic_to_bytes
• mnemonic_to_seed

Please correct me if I am wrong, but it looks as if mnemonic_to_bytes creates the value S as described in bip32 by converting each word into a 10 bit value , cutting of the checksum and verifying it against S.

The mnemonic_to_seed method does the same and additionally hashes S using pbkdf2_hmac with the corresponding bip39 salt generating a new S value, basically S = pbkdf2_hmac(S, salt) lets call it S_39.

New Wallet

So if you want to create a new wallet from scratch, it seems mnemonic_to_bytes is the correct input for slip39 and looking at the generate_shares method in slip39.py that is also what is used.

Convert a used bip39 backup to a slip39 backup

If you would want to convert a bip39 backed up wallet into slip39 it depends a bit but you should use mnemonic_to_seed to retrieve the bip39 generated seed S_39 which then can be used as the input for a bip32 HD wallet, keeping in mind that if you used one ore more passwords in the pbkdf2_hmac salt you would have to create a slip39 backup for each mnemonic + password combination.

Lets also mention that we assume that the bip39 backed up wallet used bip32 with a standard derivation path, the newly created wallet would also need to use bip32 with the same derivation path scheme. Also S_39 has a length of 512 bits which will require each share to have 59 words.

Looking at the generate_shares function, the conversion from bip39 seed to slip39 backup is not supported by embit.

It is recommended to create a new slip39 wallet and transfer the funds, so embit should probably never offer this conversion.

Convert a used slip39 backup to a bip39 backup

So as you stated that is not possible, let's go through how it works:

We start with slip39 mnemonic shares and convert them into S as done in recover_mnemonic then we use S to create a bip39 mnemonic using mnemonic_from_bytes. To use the bip39 mnemonic now we would generate a seed from it using mnemonic_to_seed and get as result S_39 which is a new seed, when we use that seed as an input to bip32 we will have created a new wallet and all the addresses used with the original slip39 backed up wallet won't be found.

I believe generate_shares is used correctly and although the API is a bit shaky since we really want to make sure to use a bip32 seed as input to the slip39 backup it actually does that.

In the case of recover_mnemonic I am with you, it is not a recovery at least when talking about using it as an input for bip39, it will generate a new wallet. It is basically a recovery of the bip32 seed in bip39 mnemonic format, it is the same bit of data that is used as input for generate_shares. I think the secret should not be formatted as bip39 mnemonic, it is just misleading.

So the docstring """Recovers the BIP39 mnemonic from a bunch of SLIP39 mnemonics""", is technically right but i will make users miss the fact that a final bip39 seed is not equal to the bip32 seed in bip39 mnemonic format as returned by that function.

@jimmysong do you still remember the rationals behind the recover_mnemonic method?

[alvroble]

Thanks @8de2fdb0 for the discussion.

Converting a BIP39 wallet to SLIP39 introduces ambiguity (specially for wallets with same BIP39 mnemonic + different passphrases) and results in really large shares. But as this is not recommended approach, I believe embit should be just leaving this to the higher-level wallet software.

As for recover_mnemonic, we agree in that it's misleading, it produces a BIP39 mnemonic that results in a new wallet, not a true recovery from the SLIP39 wallet.

My two cents: I think slip39.py should focus on handling secrets and shares only. At most, implement BIP32 recovery as recommended in SLIP39 (maybe just in the test cases). Any additional logic should live in higher-level wallet software (as is done with bip39.py).