oed / seedsplit

Use shamirs secret sharing scheme to split a seed mnemonic for crypto wallets to multiple mnemonics.
MIT License
123 stars 30 forks source link

Invalid Entropy with 24 words - BIP39 #5

Closed tloriato closed 5 years ago

tloriato commented 7 years ago

I've found out that bip39 new version 2.4.0 breaks this module when working with 24 words.

You can change your package.json to download the 2.3.0 version and it should work fine.

[Related #3 ]

oed commented 7 years ago

Thanks for pointing this out. Do you know why there is a breaking change in bip39?

coop555 commented 6 years ago

I can confirm the same error with clean install latest MacOS + Homebrew even though I changed package.json to require v2.3.0 bip39 but does not help.

readline.js:1029 throw err; ^

TypeError: Invalid entropy

Note: All 4 tests pass when running 'npm test'

I can test more if someone wants me to try something, otherwise i am at a loss.

EDIT: I also just installed it on a clean Ubuntu 17.10. Same error there. 15 words work. But 24 words still fail with the Entropy error.

In Ubuntu, can't get it to pass the 4 npm tests. On MacOS, I was able to resolve the errors, but in Ubuntu, I can't. Not sure what logs might be helpful.

oed commented 6 years ago

@coop555 I suppose you get the same error when just using bip39 by itself?

coop555 commented 6 years ago

Hi. Thanks for getting back. I need help running BIP39 as a standalone node app. Not sure how to do it. I made some half hearted attempts at it today but failed.

I assume you want me to directly run bip39.mnemonicToEntropy("24 word seed") or bip39.mnemonicToSeedHex('24 word seed')

and see if it also fails, right?

oed commented 6 years ago

@coop555 yepp, that's right. In the seedsplit directory (if you cloned the git repo) just run this in your terminal:

$ node
> let bip39 = require('bip39')
> bip39.mnemonicToEntropy("20 word seed")
coop555 commented 6 years ago

OK thanks. It works.

Both the 21 word seed and longer 24 word seed returned appropriate entropy hex strings.

Then I used both resultant entropy strings in the reverse bip39.entropyToMnemonic function, and they each returned the appropriate seed. The 24 word seed worked.

Then I reran seedsplit -t 2 -s 3 with the same 24 word seed to confirm I got the same error:

Enter seed mnemonic:
readline.js:1029 throw err; ^

TypeError: Invalid entropy at Object.entropyToMnemonic (/usr/local/lib/node_modules/seedsplit/node_modules/bip39/index.js:99:34) at shards.map.shard (/usr/local/lib/node_modules/seedsplit/lib/seedsplit.js:16:18) at Array.map () at Object.split (/usr/local/lib/node_modules/seedsplit/lib/seedsplit.js:14:31) at result (/usr/local/lib/node_modules/seedsplit/bin/cli.js:29:36) at prompt.get (/usr/local/lib/node_modules/seedsplit/bin/cli.js:60:7) at /usr/local/lib/node_modules/seedsplit/node_modules/prompt/lib/prompt.js:336:32 at /usr/local/lib/node_modules/seedsplit/node_modules/async/lib/async.js:154:25 at assembler (/usr/local/lib/node_modules/seedsplit/node_modules/prompt/lib/prompt.js:333:9) at /usr/local/lib/node_modules/seedsplit/node_modules/prompt/lib/prompt.js:342:32

Anything you'd like me to try?

oed commented 6 years ago

Oh, I see where the issue is. I'm padding the resulting split entopy with zeros to get the right length to do entropyToMnemonic. The resulting string might get the wrong length for some reason. I'd have to look more closely at this next week when I'm less busy :)

coop555 commented 6 years ago

Awesome. I am glad you figured it. Let me know if there's anything else I can do to help.

mifunetoshiro commented 6 years ago

It's not because of padding with zeroes, this is because secrets.js (unmaintained and has bugs anyway) returns shards with Buffer.from(shard, 'hex').length 34, and bip39 returns an "Invalid entropy" for length > 32: https://github.com/bitcoinjs/bip39/blob/master/index.js#L99

If you change line 99 and 83 of bip39 to > 36 it works, but this is just a dirty workaround, because according to BIP-39 the entropy should be between 128-256 bits (16-32 bytes). And this also raises this issue then: https://github.com/oed/seedsplit/issues/6.

All in all, this is not an issue with seedsplit, but in the way secrets.js creates shares not compatible with bip39, and therefore making seedsplit not compatible with 24 seed words. Should change the readme to say it doesn't work with 24 word seeds.

oed commented 6 years ago

@mifunetoshiro thanks! I think it worked in an older version though :/

mifunetoshiro commented 6 years ago

Yes, until bitcoinjs enforced the entropy to 128-256 bits in bip39, 2.4.0 it seems.