From cb029fa9050439c38ceca8316dd93119b7c9e4a3 Mon Sep 17 00:00:00 2001 From: ciny Date: Thu, 19 Sep 2019 15:42:12 +0200 Subject: [PATCH] core: various fixes to slip39 --- core/src/apps/common/storage/recovery.py | 7 +++++-- .../apps/common/storage/recovery_shares.py | 19 ++++++++++++------- .../management/recovery_device/recover.py | 8 +++----- .../apps/management/reset_device/layout.py | 17 +++++++++++++++-- 4 files changed, 35 insertions(+), 16 deletions(-) diff --git a/core/src/apps/common/storage/recovery.py b/core/src/apps/common/storage/recovery.py index 8d4d88923..cbacc11ad 100644 --- a/core/src/apps/common/storage/recovery.py +++ b/core/src/apps/common/storage/recovery.py @@ -132,10 +132,13 @@ def fetch_slip39_remaining_shares() -> Optional[List[int]]: return None result = [] - for i in range(get_slip39_group_count()): + group_count = get_slip39_group_count() + if not group_count: + raise RuntimeError + for i in range(group_count): result.append(remaining[i]) - return result + return result[:group_count] def end_progress() -> None: diff --git a/core/src/apps/common/storage/recovery_shares.py b/core/src/apps/common/storage/recovery_shares.py index 3897b2948..2e73db1a6 100644 --- a/core/src/apps/common/storage/recovery_shares.py +++ b/core/src/apps/common/storage/recovery_shares.py @@ -9,12 +9,18 @@ if False: # Each mnemonic is stored under key = index. -def set(index: int, mnemonic: str) -> None: - common.set(common.APP_RECOVERY_SHARES, index, mnemonic.encode()) +def set(index: int, mnemonic: str, group_index: int) -> None: + common.set( + common.APP_RECOVERY_SHARES, + index + group_index * slip39.MAX_SHARE_COUNT, + mnemonic.encode(), + ) -def get(index: int) -> Optional[str]: - m = common.get(common.APP_RECOVERY_SHARES, index) +def get(index: int, group_index: int) -> Optional[str]: + m = common.get( + common.APP_RECOVERY_SHARES, index + group_index * slip39.MAX_SHARE_COUNT + ) if m: return m.decode() return None @@ -32,9 +38,8 @@ def fetch() -> List[List[str]]: def fetch_group(group_index: int) -> List[str]: mnemonics = [] - starting_index = group_index * slip39.MAX_SHARE_COUNT - for index in range(starting_index, starting_index + slip39.MAX_SHARE_COUNT): - m = get(index) + for index in range(slip39.MAX_SHARE_COUNT): + m = get(index, group_index) if m: mnemonics.append(m) diff --git a/core/src/apps/management/recovery_device/recover.py b/core/src/apps/management/recovery_device/recover.py index 4b5545234..af161626c 100644 --- a/core/src/apps/management/recovery_device/recover.py +++ b/core/src/apps/management/recovery_device/recover.py @@ -29,8 +29,6 @@ def process_slip39(words: str) -> Tuple[Optional[bytes], slip39.Share]: share = slip39.decode_mnemonic(words) remaining = storage.recovery.fetch_slip39_remaining_shares() - # TODO: move this whole logic to storage - index_with_group_offset = share.index + share.group_index * slip39.MAX_SHARE_COUNT # if this is the first share, parse and store metadata if not remaining: @@ -42,7 +40,7 @@ def process_slip39(words: str) -> Tuple[Optional[bytes], slip39.Share]: storage.recovery.set_slip39_remaining_shares( share.threshold - 1, share.group_index ) - storage.recovery_shares.set(index_with_group_offset, words) + storage.recovery_shares.set(share.index, words, share.group_index) # if share threshold and group threshold are 1 # we can calculate the secret right away @@ -58,7 +56,7 @@ def process_slip39(words: str) -> Tuple[Optional[bytes], slip39.Share]: # These should be checked by UI before so it's a Runtime exception otherwise if share.identifier != storage.recovery.get_slip39_identifier(): raise RuntimeError("Slip39: Share identifiers do not match") - if storage.recovery_shares.get(index_with_group_offset): + if storage.recovery_shares.get(share.index, share.group_index): raise RuntimeError("Slip39: This mnemonic was already entered") remaining_for_share = ( @@ -69,7 +67,7 @@ def process_slip39(words: str) -> Tuple[Optional[bytes], slip39.Share]: remaining_for_share - 1, share.group_index ) remaining[share.group_index] = remaining_for_share - 1 - storage.recovery_shares.set(index_with_group_offset, words) + storage.recovery_shares.set(share.index, words, share.group_index) if remaining.count(0) < share.group_threshold: # we need more shares diff --git a/core/src/apps/management/reset_device/layout.py b/core/src/apps/management/reset_device/layout.py index 0c0f0281b..ee133785c 100644 --- a/core/src/apps/management/reset_device/layout.py +++ b/core/src/apps/management/reset_device/layout.py @@ -168,7 +168,17 @@ async def _confirm_word( ctx, share_index, numbered_share_words, count, group_index=None ): - # TODO: duplicated words in the choice list + # remove duplicate share words so we don't offer them + duplicate_list = [] + for i in range(len(numbered_share_words)): + duplicates = [ + j for j, word in numbered_share_words if word == numbered_share_words[i][1] + ] + if len(duplicates) > 1: + duplicate_list.extend(duplicate_list[1:]) + for remove_index in sorted(set(duplicate_list), reverse=True): + numbered_share_words.pop(remove_index) + # shuffle the numbered seed half, slice off the choices we need random.shuffle(numbered_share_words) numbered_choices = numbered_share_words[: MnemonicWordSelect.NUM_OF_CHOICES] @@ -565,7 +575,10 @@ class Slip39NumInput(ui.Component): elif self.step is Slip39NumInput.SET_THRESHOLD: if self.group_id is None: first_line_text = "For recovery you need" - second_line_text = "any %s of the shares." % count + if count == self.input.max_count: + second_line_text = "all %s of the shares." % count + else: + second_line_text = "any %s of the shares." % count else: first_line_text = "The required number of " second_line_text = "shares to form Group %s." % (self.group_id + 1)