oed / seedsplit

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

This is not a BIP - so if you lose this code, your keys are useless #6

Open dagelf opened 6 years ago

dagelf commented 6 years ago

Something that someone using this might not consider is the possibility of this code disappearing, rendering your keys useless - unless you also store the algorithm along with those keys.

The solution is to turn this into a BIP, so that the protocol will be preserved and maintained. Until that happens it is ill advised to use this - rather use n of m multisig wallets, each with its own passphrase.

oed commented 6 years ago

Agreed. You should not trust that this code will be on github forever. If you want to use it you can download an keep a copy of the code for yourself. Unfortunately I don't really have the time to create a BIP, if someone else does it I'll be all for it!

prusnak commented 6 years ago

work in progress: https://github.com/satoshilabs/slips/blob/master/slip-0039.md

oed commented 6 years ago

@prusnak that's awesome! Please keep us updated :blush:

AndreasGassmann commented 3 years ago

I know this is an old issue, but I think it's important to clarify.

@prusnak, I read some discussions around SLIP-0039 and I think it does not allow to go from mnemonic to shares and back again. SLIP-0039 only generates shares for you that can be combined to get a BIP32 root key (and any combination of shares will give you the same BIP32 root key), but you can never have a "single mnemonic representation" of it. Is this correct? If yes, then I don't think SLIP-0039 matches the use case of this project.

I was very excited about the SLIP-0039 proposal when I first heard about it, but if there really is no way of having one single mnemonic representation, I don't think it's useful for most of the projects that want to use Shamir's Secret Sharing.

oed commented 3 years ago

@AndreasGassmann What use cases do you have in mind that SLIP-0039 doesn't cover? For me the use of it in the new Trezor hardware covers my use case, which means I'm likely to abandon this project. Huge props to @prusnak for making this feature a part of Trezor btw 🎉 🙏

If you're interested in using sss more as a low level component you should check out something like this: https://github.com/3box/sss-wasm

AndreasGassmann commented 3 years ago

The use case that this tool handles: Taking a standard BIP39 mnemonic and "splitting it", so those splits can later be combined again to get the original BIP39 mnemonic.

SLIP-0039 only works with wallets that have SLIP-0039 support (which, as far as I'm aware, is only Trezor).

But a tool like yours that takes a standard BIP39 mnemonic as input can be used together with any BIP39 compatible wallet. So it's up to the user to either only store his BIP39 mnemonic, keep both the BIP39 mnemonic and SSS shares, or destroy the original BIP39 mnemonic and only store the SSS shares.

SLIP-0039 doesn't give you the first 2 options.

I was very excited when I first heard about SLIP-0039 because it has some interesting features, like the groups with individual thresholds. But the incompatibility with BIP39 mnemonics is a dealbreaker for me, because the user of a wallet has to choose between a regular BIP39 mnemonic, or SSS. It's not possible to have both.

There are some discussions around this problem in the following issues:

https://github.com/BlockchainCommons/bc-lethekit/issues/38 https://github.com/iancoleman/slip39/issues/1#issuecomment-563213494

prusnak commented 3 years ago

There are very good reasosn why we disallowed BIP39 splitting into SLIP39 - these are documented here: https://github.com/satoshilabs/slips/blob/master/slip-0039.md#user-content-bip39compatibility

AndreasGassmann commented 3 years ago

@prusnak I don't really want this to evolve into another discussion about BIP39 <=> SLIP-0039 compatibility.

All I wanted to say is that the reference to SLIP-0039 here is misleading, because it suggests that it will support the use case that this very tool is built around:

Seedsplit lets you split a mnemonic seed into a selected number of shards which are also encoded as mnemonics. n-of-m shards can then be combined in order to get the initial mnemonic seed.

Which is not the case.

The title of this issue expresses concerns that there is no standard for "splitting BIP39 into shards", and the reference to SLIP-0039 suggests that this is an effort to solve that, which it is not. It solves a similar, but ultimately different use case.

I want people to be aware that there is currently no BIP/SLIP in the works to address this specific use case. A brief look at SLIP-0039 might suggest that it does, because it's so similar. I personally only discovered the incompatibility with BIP39 when I was already working on integrating SLIP39 into our wallet (which was before the BIP39 compatibility section was added to SLIP-0039). Sadly, we will now continue to use our own, custom, seed split for the time being.


I would still like to take a minute and address your reasons for disallowing BIP39 splitting into SLIP39, because I strongly disagree with most of them.

prusnak commented 3 years ago

All I wanted to say is that the reference to SLIP-0039 here is misleading,

I agree. The goals of SLIP-0039 and your project are very different.

secinthenet commented 3 years ago

@AndreasGassmann @prusnak as I just commented in https://github.com/BlockchainCommons/bc-lethekit/issues/38#issuecomment-751904641:

SLIP39 doesn't specify what secret you split. The doc only says "SSS splits a master secret, such as the master seed S for Hierarchical Deterministic Wallets described in BIP-0032". So, using the master seed S is the example given (and the implementation in Trezor as I understand it), but another option it to split the initial entropy in BIP39, making it possible to round trip between the two. That is, you can use the following scheme to make SLIP39 compatible:

SLIP39 shards <--> BIP39 entropy <--> BIP39 mnemonic

@prusnak did I get anything wrong here? @AndreasGassmann have you considered using SLIP39 to split the BIP39 initial entropy, so that you can actually convert between the two? I know this risks confusing users if SLIP39 is used inconsistently to derive keys, but it doesn't really goes against the spec as far as I can tell.

AndreasGassmann commented 3 years ago

@secinthenet I'm not sure if that would actually work. From what I remember, I don't think it's that easy.

But even if it was, it goes against the goals of SLIP-0039. The authors of SLIP-0039 have been very outspoken about NOT supporting this exact use cases. As mentioned in my comment above, I strongly disagree with this decision.

The problem I see is that even if we find a way of making it work for our use case, it still will not be part of the actual standard, so we will end up with 2 "different" SLIP-0039 implementations, which wouldn't be compatible with each other. And that would cause a lot of confusion.

The ideal solution would be if the SLIP-0039 spec was changed to be compatible with BIP-39, but that is most likely not going to happen. Even though most people want to have BIP-39 <=> SLIP-0039 compatibility. So the next best solution would be to start a separate BIP to create a BIP-39 compatible SSS.

prusnak commented 3 years ago

@prusnak did I get anything wrong here?

You assume BIP39 and SLIP39 passphrases are empty. Once you start using variable passphrases with the seeds you'll end up with different secrets.

secinthenet commented 3 years ago

@prusnak did I get anything wrong here?

You assume BIP39 and SLIP39 passphrases are empty. Once you start using variable passphrases with the seeds you'll end up with different secrets.

Right, thanks for the correction. Would it make sense to emphasize more strongly in the spec of SLIP39 that it should be used to split the BIP32 master seed when used in the context of HD wallets? otherwise, I think there's a risk someone will it in a way that is incompatible with other implementations. Since the terminology is a bit overloaded here, I want to be clear that "BIP32 master seed" is the marked part in the BIP32 diagram:

image

secinthenet commented 3 years ago

@secinthenet I'm not sure if that would actually work. From what I remember, I don't think it's that easy.

Indeed, it seems the passphrases are the problem here, since SLIP39 specifies how they should be used, and it's incompatible with how they're used in BIP39.

But even if it was, it goes against the goals of SLIP-0039. The authors of SLIP-0039 have been very outspoken about NOT supporting this exact use cases. As mentioned in my comment above, I strongly disagree with this decision.

The problem I see is that even if we find a way of making it work for our use case, it still will not be part of the actual standard, so we will end up with 2 "different" SLIP-0039 implementations, which wouldn't be compatible with each other. And that would cause a lot of confusion.

The ideal solution would be if the SLIP-0039 spec was changed to be compatible with BIP-39, but that is most likely not going to happen. Even though most people want to have BIP-39 <=> SLIP-0039 compatibility. So the next best solution would be to start a separate BIP to create a BIP-39 compatible SSS.

Yes, I agree it would be better to define a new standard, even if it's very similar in terms of crypto, so that there's no confusion with SLIP39. BlockchainCommons are working on one called SSKR: https://github.com/BlockchainCommons/bc-sskr

prusnak commented 3 years ago

Would it make sense to emphasize more strongly in the spec of SLIP39 that it should be used to split the BIP32 master seed when used in the context of HD wallets?

@secinthenet Please send a pull request to SLIP39 if you have some wording in mind. Thank you!

secinthenet commented 3 years ago

@prusnak thanks, I just sent a PR: https://github.com/satoshilabs/slips/pull/1044