Internet-of-People / slip39-rust

SLIP-0039 compatible secret sharing tool
GNU General Public License v3.0
14 stars 5 forks source link

'--required-groups 1' with multiple '--group' does not produce multiple groups in the output #3

Closed kirillkovalenko closed 4 years ago

kirillkovalenko commented 4 years ago

I wanted to split a mnemonic into 2 groups where only one is required to restore the seed. I'm running the following command:

slip39 split --password "morpheus" --entropy "shell view flock obvious believe final afraid caught page second arrow predict" --required-groups 1 --group 3of5 --group 3of6

In the result output I see

{
  "group_count": 1,
  "group_threshold": 1,
  "groups": [
    {
      "member_threshold": 3,
      "member_count": 5,
      "shares": [ ...skipped... ]
   }]
}

but I expect it to be

{
  "group_count": 2,
  "group_threshold": 1,
  "groups": [
    {
      "member_threshold": 3,
      "member_count": 5,
      "shares": [ ...skipped... ]
   },
   {
      "member_threshold": 3,
      "member_count": 6,
      "shares": [ ...skipped... ]
   }]
}

Is it a bug or am I missing something?

wigy-opensource-developer commented 4 years ago

I will check the code. I have already read again the relevant part of the specification (point 2 of design rationale of SLIP-00039) and it seems to be a valid use-case per specification and common sense, too.

wigy-opensource-developer commented 4 years ago

@kirillkovalenko I have implemented a fix on the underlying library and have a pending PR for the fix.

Notice that if the people from the 2 groups know each other, some of them could cooperate to recover the common group secret without complying to member thresholds in any of the separate groups. So although this tool does not provide a way to combine member shares from different groups, I anticipate the security in your example is similar to a single 3-of-11 group.

kirillkovalenko commented 4 years ago

I anticipate the security in your example is similar to a single 3-of-11 group.

@wigy-opensource-developer Thank you for the clarification.

I have implemented a fix on the underlying library and have a pending PR for the fix.

Can you please link the RP here for reference?

wigy-opensource-developer commented 4 years ago

Of course. The backlinks hide among the comments in this thread ☝️

kirillkovalenko commented 4 years ago

@wigy-opensource-developer ping

wigy-opensource-developer commented 4 years ago

Pong. Sorry, will have some time Monday to proceed. Upstream patch was accepted, I just need to integrate the new version, but this week was not about work.

wigy-opensource-developer commented 4 years ago

You can build the fixed version from the https://github.com/Internet-of-People/slip39-rust/tree/fix/group_threshold_one branch, but I cannot publish the version on creates.io before the upstream patch is published as well.

wigy-opensource-developer commented 4 years ago

Released the fixes as 0.1.1 on crates.io