From 03dc88cbf7230a7196808d06b84f60eae99c8c90 Mon Sep 17 00:00:00 2001 From: Stepan Snigirev Date: Sun, 10 Dec 2023 17:46:29 +0100 Subject: [PATCH 1/9] ask for overwrite if something on smartcard --- src/keystore/memorycard.py | 33 +++++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/src/keystore/memorycard.py b/src/keystore/memorycard.py index 640393b..e9cfffe 100644 --- a/src/keystore/memorycard.py +++ b/src/keystore/memorycard.py @@ -4,14 +4,12 @@ from .javacard.util import get_connection from platform import CriticalErrorWipeImmediately import platform -from rng import get_random_bytes from embit import bip39 from helpers import tagged_hash, aead_encrypt, aead_decrypt import hmac -from gui.screens import Alert, Progress, Menu, MnemonicScreen, Prompt +from gui.screens import Alert, Progress, Menu, Prompt import asyncio from io import BytesIO -from uscard import SmartcardException from binascii import hexlify import lvgl as lv @@ -228,6 +226,21 @@ def _set_pin(self, pin): async def save_mnemonic(self): await self.check_card(check_pin=True) + key_saved, encrypted, decryptable, same_mnemonic = self.get_secret_info() + if key_saved: + if not decryptable: + msg = "Some data saved to the card,\nbut it can't be decrypted" + elif same_mnemonic: + msg = "Same mnemonic is saved to the card\nin " + msg += "encrypted" if encrypted else "plaintext" + msg += " form.\n" + else: + msg = "A different mnemonic is saved to the card" + confirm = await self.show(Prompt("Overwrite existing data?", + "\n%s" % msg + "\n\nContinue?" + )) + if not confirm: + return encrypt = await self.show(Prompt("Encrypt the secret?", "\nIf you encrypt the secret on the card " "it will only work with this device.\n\n" @@ -407,10 +420,7 @@ async def storage_menu(self): else: raise KeyStoreError("Invalid menu") - async def show_card_info(self): - note = "Card fingerprint: %s" % self.hexid - version = "%s v%s" % (self.applet.NAME, self.applet.version) - platform = self.applet.platform + def get_secret_info(self): data = self.applet.get_secret() key_saved = len(data) > 0 encrypted = True @@ -422,8 +432,15 @@ async def show_card_info(self): if "entropy" in d: self._is_key_saved = True same_mnemonic = (self.mnemonic == bip39.mnemonic_from_bytes(d["entropy"])) - except KeyStoreError as e: + except KeyStoreError: decryptable = False + return key_saved, encrypted, decryptable, same_mnemonic + + async def show_card_info(self): + note = "Card fingerprint: %s" % self.hexid + version = "%s v%s" % (self.applet.NAME, self.applet.version) + platform = self.applet.platform + key_saved, encrypted, decryptable, same_mnemonic = self.get_secret_info() # yes = lv.SYMBOL.OK+" Yes" # no = lv.SYMBOL.CLOSE+" No" yes = "Yes" From 1e143aa38fbf9ffd21a6e427b82dbcd71425b1da Mon Sep 17 00:00:00 2001 From: moneymanolis Date: Wed, 13 Dec 2023 20:59:39 +0100 Subject: [PATCH 2/9] fix: only show success screen if user did not cancel --- src/keystore/memorycard.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/keystore/memorycard.py b/src/keystore/memorycard.py index e9cfffe..cbba9ba 100644 --- a/src/keystore/memorycard.py +++ b/src/keystore/memorycard.py @@ -259,6 +259,13 @@ async def save_mnemonic(self): self._is_key_saved = True # check it's ok await self.load_mnemonic() + await self.show( + Alert( + "Success!", + "Your key is stored on the smartcard now.", + button_text="OK", + ) + ) @property def is_key_saved(self): @@ -372,13 +379,6 @@ async def storage_menu(self): return False elif menuitem == 0: await self.save_mnemonic() - await self.show( - Alert( - "Success!", - "Your key is stored on the smartcard now.", - button_text="OK", - ) - ) elif menuitem == 1: await self.load_mnemonic() await self.show( From 55ab4cd45cd5a6f8d7420bd0b54c6dcd58f14975 Mon Sep 17 00:00:00 2001 From: moneymanolis Date: Sat, 16 Dec 2023 17:11:12 +0100 Subject: [PATCH 3/9] add warning style --- src/gui/common.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/gui/common.py b/src/gui/common.py index e78e3d4..753d32b 100644 --- a/src/gui/common.py +++ b/src/gui/common.py @@ -115,6 +115,9 @@ def init_styles(dark=True): lv.style_copy(styles["small"], styles["hint"]) styles["small"].text.color = ctxt + styles["warning"] = lv.style_t() + lv.style_copy(styles["warning"], th.style.label.prim) + styles["warning"].text.color = lv.color_hex(0xFF9A00) def add_label(text, y=PADDING, scr=None, style=None, width=None): """Helper functions that creates a title-styled label""" From dfafc7e5b4e1aed3507de16e3576f6b65cf4ef32 Mon Sep 17 00:00:00 2001 From: moneymanolis Date: Sat, 16 Dec 2023 17:11:34 +0100 Subject: [PATCH 4/9] add warning option to prompt class --- src/gui/screens/prompt.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/gui/screens/prompt.py b/src/gui/screens/prompt.py index e5081c7..05e8261 100644 --- a/src/gui/screens/prompt.py +++ b/src/gui/screens/prompt.py @@ -6,7 +6,7 @@ class Prompt(Screen): def __init__(self, title="Are you sure?", message="Make a choice", - confirm_text="Confirm", cancel_text="Cancel", note=None): + confirm_text="Confirm", cancel_text="Cancel", note=None, warning=None): super().__init__() self.title = add_label(title, scr=self, style="title") if note is not None: @@ -25,3 +25,10 @@ def __init__(self, title="Are you sure?", message="Make a choice", on_release(cb_with_args(self.set_value, True)), scr=self, ) + + if warning: + self.warning = add_label(warning, scr=self, style="warning") + # Align warning text + y_pos = self.cancel_button.get_y() - 60 # above the buttons + x_pos = self.get_width() // 2 - self.warning.get_width() // 2 # in the center of the prompt + self.warning.set_pos(x_pos, y_pos) From 4a5a1d19707b378e0e452112e9a1518f33ae25ef Mon Sep 17 00:00:00 2001 From: moneymanolis Date: Sat, 16 Dec 2023 17:56:54 +0100 Subject: [PATCH 5/9] improve copy on overwriting data screen + add warning text --- src/keystore/memorycard.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/keystore/memorycard.py b/src/keystore/memorycard.py index cbba9ba..1cd643c 100644 --- a/src/keystore/memorycard.py +++ b/src/keystore/memorycard.py @@ -229,15 +229,19 @@ async def save_mnemonic(self): key_saved, encrypted, decryptable, same_mnemonic = self.get_secret_info() if key_saved: if not decryptable: - msg = "Some data saved to the card,\nbut it can't be decrypted" + msg = ("There is data on the card, but its nature is unknown since we are unable to decrypt it.\n\n" + "Thus, we cannot confirm whether a mnemonic is already saved on your card or if it matches the one you are about to save.") elif same_mnemonic: - msg = "Same mnemonic is saved to the card\nin " - msg += "encrypted" if encrypted else "plaintext" - msg += " form.\n" + msg = ("The mnemonic you are about to save is already stored on the smart card.\n" + "If you proceed, the existing data will be overwritten with the same mnemonic.\n\n" + "This can be useful if you want to store this mnemonic in a different form (plaintext vs. encrypted) on the card.\n\n" + "Currently, your mnemonic is saved in {} form on the card.".format('encrypted' if encrypted else 'plaintext')) else: - msg = "A different mnemonic is saved to the card" - confirm = await self.show(Prompt("Overwrite existing data?", - "\n%s" % msg + "\n\nContinue?" + msg = ("A different mnemonic is already saved on the card.\n\n" + "Continuing will replace the existing mnemonic with the one you are about to save.") + + confirm = await self.show(Prompt("Overwrite data?", + "\n%s" % msg + "\n\nDo you want to continue?", 'Continue', warning="Irreversibly overwrite the data on the card" )) if not confirm: return From 714ed198237e662da1fc5ead308507c79435e23e Mon Sep 17 00:00:00 2001 From: moneymanolis Date: Sat, 16 Dec 2023 18:58:14 +0100 Subject: [PATCH 6/9] add icon to Prompt and add warning icon if there is a warning --- src/gui/screens/prompt.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/gui/screens/prompt.py b/src/gui/screens/prompt.py index 05e8261..8b3a758 100644 --- a/src/gui/screens/prompt.py +++ b/src/gui/screens/prompt.py @@ -17,6 +17,7 @@ def __init__(self, title="Are you sure?", message="Make a choice", self.page.set_size(480, 600) self.message = add_label(message, scr=self.page) self.page.align(self.title, lv.ALIGN.OUT_BOTTOM_MID, 0, 0) + self.icon = lv.label(self) (self.cancel_button, self.confirm_button) = add_button_pair( cancel_text, @@ -28,7 +29,13 @@ def __init__(self, title="Are you sure?", message="Make a choice", if warning: self.warning = add_label(warning, scr=self, style="warning") - # Align warning text + self.icon.set_text(lv.SYMBOL.WARNING) + + # Align warning text y_pos = self.cancel_button.get_y() - 60 # above the buttons x_pos = self.get_width() // 2 - self.warning.get_width() // 2 # in the center of the prompt self.warning.set_pos(x_pos, y_pos) + + # Align warning icon to the left of the title + self.icon.align(self.title, lv.ALIGN.IN_LEFT_MID, 90, 0) + From 312f9d12d9d252fcf5885f3439ed744cf80d1aa0 Mon Sep 17 00:00:00 2001 From: moneymanolis Date: Sat, 16 Dec 2023 19:51:23 +0100 Subject: [PATCH 7/9] rename key_saved to data_saved + change copy to "Card has data" --- src/keystore/memorycard.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/keystore/memorycard.py b/src/keystore/memorycard.py index 1cd643c..d85121c 100644 --- a/src/keystore/memorycard.py +++ b/src/keystore/memorycard.py @@ -226,8 +226,8 @@ def _set_pin(self, pin): async def save_mnemonic(self): await self.check_card(check_pin=True) - key_saved, encrypted, decryptable, same_mnemonic = self.get_secret_info() - if key_saved: + data_saved, encrypted, decryptable, same_mnemonic = self.get_secret_info() + if data_saved: if not decryptable: msg = ("There is data on the card, but its nature is unknown since we are unable to decrypt it.\n\n" "Thus, we cannot confirm whether a mnemonic is already saved on your card or if it matches the one you are about to save.") @@ -426,11 +426,11 @@ async def storage_menu(self): def get_secret_info(self): data = self.applet.get_secret() - key_saved = len(data) > 0 + data_saved = len(data) > 0 encrypted = True decryptable = True same_mnemonic = False - if key_saved: + if data_saved: try: d, encrypted = self.parse_data(data) if "entropy" in d: @@ -438,13 +438,13 @@ def get_secret_info(self): same_mnemonic = (self.mnemonic == bip39.mnemonic_from_bytes(d["entropy"])) except KeyStoreError: decryptable = False - return key_saved, encrypted, decryptable, same_mnemonic + return data_saved, encrypted, decryptable, same_mnemonic async def show_card_info(self): note = "Card fingerprint: %s" % self.hexid version = "%s v%s" % (self.applet.NAME, self.applet.version) platform = self.applet.platform - key_saved, encrypted, decryptable, same_mnemonic = self.get_secret_info() + data_saved, encrypted, decryptable, same_mnemonic = self.get_secret_info() # yes = lv.SYMBOL.OK+" Yes" # no = lv.SYMBOL.CLOSE+" No" yes = "Yes" @@ -454,9 +454,9 @@ async def show_card_info(self): "Implementation: %s" % platform, "Version: %s" % version, "\n#7f8fa4 KEY INFO: #", - "Key saved: " + (yes if key_saved else no), + "Card has data: " + (yes if data_saved else no), ] - if key_saved: + if data_saved: if decryptable: props.append("Same as current key: " + (yes if same_mnemonic else no)) props.append("Encrypted: " + (yes if encrypted else no)) From 34ec0a51c6a7c857bbb06edc5ae75532f4fb4231 Mon Sep 17 00:00:00 2001 From: moneymanolis Date: Sat, 16 Dec 2023 20:05:09 +0100 Subject: [PATCH 8/9] fix: initialize an empty icon label --- src/gui/screens/prompt.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/gui/screens/prompt.py b/src/gui/screens/prompt.py index 8b3a758..206bc83 100644 --- a/src/gui/screens/prompt.py +++ b/src/gui/screens/prompt.py @@ -17,7 +17,9 @@ def __init__(self, title="Are you sure?", message="Make a choice", self.page.set_size(480, 600) self.message = add_label(message, scr=self.page) self.page.align(self.title, lv.ALIGN.OUT_BOTTOM_MID, 0, 0) + # Initialize an empty icon label. It will display nothing until a symbol is set. self.icon = lv.label(self) + self.icon.set_text("") (self.cancel_button, self.confirm_button) = add_button_pair( cancel_text, @@ -29,6 +31,7 @@ def __init__(self, title="Are you sure?", message="Make a choice", if warning: self.warning = add_label(warning, scr=self, style="warning") + # Display warning symbol in the icon label self.icon.set_text(lv.SYMBOL.WARNING) # Align warning text From c87d12fb4027336b4382a1115621236b5bd35b22 Mon Sep 17 00:00:00 2001 From: moneymanolis Date: Sat, 16 Dec 2023 20:24:10 +0100 Subject: [PATCH 9/9] make "Keep as plain text" the CTA btn and change copy to explain the risks of encrypting the secret with device --- src/keystore/memorycard.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/keystore/memorycard.py b/src/keystore/memorycard.py index d85121c..24cd01f 100644 --- a/src/keystore/memorycard.py +++ b/src/keystore/memorycard.py @@ -242,18 +242,19 @@ async def save_mnemonic(self): confirm = await self.show(Prompt("Overwrite data?", "\n%s" % msg + "\n\nDo you want to continue?", 'Continue', warning="Irreversibly overwrite the data on the card" - )) + )) if not confirm: return - encrypt = await self.show(Prompt("Encrypt the secret?", + keep_as_plain_text = await self.show(Prompt("Encrypt the secret?", "\nIf you encrypt the secret on the card " - "it will only work with this device.\n\n" - "Otherwise it will be readable on any device " + "it will only work with the device you are currently using.\n\n" + "If you keep it as plain text, it will be readable on any Specter DIY device " "after you enter the PIN code.\n\n" - "Keep in mind that with encryption enabled " - "wiping the device makes the secret unusable!", - confirm_text="Yes, encrypt", - cancel_text="Keep as plain text")) + "Activating encryption means that if the device is wiped, the stored secret on the card becomes inaccessible.", + confirm_text="Keep as plain text", + cancel_text="Encrypt", + )) + encrypt = not keep_as_plain_text self.show_loader("Saving secret to the card...") d = self.serialize_data( {"entropy": bip39.mnemonic_to_bytes(self.mnemonic)},