From c919ce1d565e4b7d1f5ebfeaaf9412b04d69e1fe Mon Sep 17 00:00:00 2001 From: Lukas Bielesch Date: Thu, 12 Dec 2024 17:55:56 +0100 Subject: [PATCH] chore(core): ensure dependency between PIN and wipe code --- core/.changelog.d/4446.added | 1 + core/embed/rust/librust_qstr.h | 3 ++ .../generated/translated_string.rs | 9 +++++ core/mocks/trezortranslate_keys.pyi | 3 ++ core/src/apps/common/request_pin.py | 20 +++++++++++ core/src/apps/management/change_pin.py | 5 +++ core/src/apps/management/change_wipe_code.py | 16 ++++++--- core/translations/cs.json | 3 ++ core/translations/de.json | 3 ++ core/translations/en.json | 3 ++ core/translations/es.json | 3 ++ core/translations/fr.json | 3 ++ core/translations/it.json | 3 ++ core/translations/order.json | 5 ++- core/translations/pt.json | 3 ++ core/translations/signatures.json | 6 ++-- core/translations/tr.json | 3 ++ .../test_msg_change_wipe_code_t2.py | 35 ++++++++++++++++++- 18 files changed, 118 insertions(+), 9 deletions(-) create mode 100644 core/.changelog.d/4446.added diff --git a/core/.changelog.d/4446.added b/core/.changelog.d/4446.added new file mode 100644 index 0000000000..f6fdead7c1 --- /dev/null +++ b/core/.changelog.d/4446.added @@ -0,0 +1 @@ +Add dependency check between PIN and wipe code. diff --git a/core/embed/rust/librust_qstr.h b/core/embed/rust/librust_qstr.h index 3197b9ff12..5a60dfbad5 100644 --- a/core/embed/rust/librust_qstr.h +++ b/core/embed/rust/librust_qstr.h @@ -389,6 +389,8 @@ static void _librust_qstrs(void) { MP_QSTR_pin__tries_left; MP_QSTR_pin__turn_off; MP_QSTR_pin__turn_on; + MP_QSTR_pin__wipe_code_exists_description; + MP_QSTR_pin__wipe_code_exists_title; MP_QSTR_pin__wrong_pin; MP_QSTR_plurals__contains_x_keys; MP_QSTR_plurals__lock_after_x_hours; @@ -739,6 +741,7 @@ static void _librust_qstrs(void) { MP_QSTR_wipe_code__info; MP_QSTR_wipe_code__invalid; MP_QSTR_wipe_code__mismatch; + MP_QSTR_wipe_code__pin_not_set_description; MP_QSTR_wipe_code__reenter; MP_QSTR_wipe_code__reenter_to_confirm; MP_QSTR_wipe_code__title_check; diff --git a/core/embed/rust/src/translations/generated/translated_string.rs b/core/embed/rust/src/translations/generated/translated_string.rs index e8da45f6eb..f739eefe2e 100644 --- a/core/embed/rust/src/translations/generated/translated_string.rs +++ b/core/embed/rust/src/translations/generated/translated_string.rs @@ -1382,6 +1382,9 @@ pub enum TranslatedString { misc__enable_labeling = 973, // "Enable labeling?" #[cfg(feature = "universal_fw")] ethereum__unknown_contract_address_short = 974, // "Unknown contract address." + wipe_code__pin_not_set_description = 975, // "PIN must be set before enabling wipe code." + pin__wipe_code_exists_description = 976, // "Wipe code must be turned off before turnig off PIN protection." + pin__wipe_code_exists_title = 977, // "Wipe code set" } impl TranslatedString { @@ -2758,6 +2761,9 @@ impl TranslatedString { Self::misc__enable_labeling => "Enable labeling?", #[cfg(feature = "universal_fw")] Self::ethereum__unknown_contract_address_short => "Unknown contract address.", + Self::wipe_code__pin_not_set_description => "PIN must be set before enabling wipe code.", + Self::pin__wipe_code_exists_description => "Wipe code must be turned off before turnig off PIN protection.", + Self::pin__wipe_code_exists_title => "Wipe code set", } } @@ -4135,6 +4141,9 @@ impl TranslatedString { Qstr::MP_QSTR_misc__enable_labeling => Some(Self::misc__enable_labeling), #[cfg(feature = "universal_fw")] Qstr::MP_QSTR_ethereum__unknown_contract_address_short => Some(Self::ethereum__unknown_contract_address_short), + Qstr::MP_QSTR_wipe_code__pin_not_set_description => Some(Self::wipe_code__pin_not_set_description), + Qstr::MP_QSTR_pin__wipe_code_exists_description => Some(Self::pin__wipe_code_exists_description), + Qstr::MP_QSTR_pin__wipe_code_exists_title => Some(Self::pin__wipe_code_exists_title), _ => None, } } diff --git a/core/mocks/trezortranslate_keys.pyi b/core/mocks/trezortranslate_keys.pyi index 8454099bfd..d55d9b1e0d 100644 --- a/core/mocks/trezortranslate_keys.pyi +++ b/core/mocks/trezortranslate_keys.pyi @@ -541,6 +541,8 @@ class TR: pin__tries_left: str = "tries left" pin__turn_off: str = "Are you sure you want to turn off PIN protection?" pin__turn_on: str = "Turn on PIN protection?" + pin__wipe_code_exists_description: str = "Wipe code must be turned off before turnig off PIN protection." + pin__wipe_code_exists_title: str = "Wipe code set" pin__wrong_pin: str = "Wrong PIN" plurals__contains_x_keys: str = "key|keys" plurals__lock_after_x_hours: str = "hour|hours" @@ -908,6 +910,7 @@ class TR: wipe_code__info: str = "Wipe code can be used to erase all data from this device." wipe_code__invalid: str = "Invalid wipe code" wipe_code__mismatch: str = "The wipe codes you entered do not match." + wipe_code__pin_not_set_description: str = "PIN must be set before enabling wipe code." wipe_code__reenter: str = "Re-enter wipe code" wipe_code__reenter_to_confirm: str = "Please re-enter wipe code to confirm." wipe_code__title_check: str = "Check wipe code" diff --git a/core/src/apps/common/request_pin.py b/core/src/apps/common/request_pin.py index 988d828733..7162b29504 100644 --- a/core/src/apps/common/request_pin.py +++ b/core/src/apps/common/request_pin.py @@ -147,3 +147,23 @@ async def error_pin_matches_wipe_code() -> NoReturn: exc=wire.PinInvalid, ) assert False + + +async def error_pin_not_set() -> NoReturn: + await show_error_and_raise( + "warning_pin_not_set", + TR.wipe_code__pin_not_set_description, + TR.homescreen__title_pin_not_set, + exc=wire.ActionCancelled, + ) + assert False + + +async def error_wipe_code_exists() -> NoReturn: + await show_error_and_raise( + "wipe_code_exists", + TR.pin__wipe_code_exists_description, + TR.pin__wipe_code_exists_title, + exc=wire.ActionCancelled, + ) + assert False diff --git a/core/src/apps/management/change_pin.py b/core/src/apps/management/change_pin.py index cf0cc95179..dc606ed425 100644 --- a/core/src/apps/management/change_pin.py +++ b/core/src/apps/management/change_pin.py @@ -16,6 +16,7 @@ async def change_pin(msg: ChangePin) -> Success: from apps.common.request_pin import ( error_pin_invalid, error_pin_matches_wipe_code, + error_wipe_code_exists, request_pin_and_sd_salt, request_pin_confirm, ) @@ -26,6 +27,10 @@ async def change_pin(msg: ChangePin) -> Success: # confirm that user wants to change the pin await _require_confirm_change_pin(msg) + # Do not allow to remove the PIN if the wipe code is set. + if msg.remove and config.has_wipe_code(): + await error_wipe_code_exists() + # get old pin curpin, salt = await request_pin_and_sd_salt(TR.pin__enter) diff --git a/core/src/apps/management/change_wipe_code.py b/core/src/apps/management/change_wipe_code.py index b5256fd459..6818fb10db 100644 --- a/core/src/apps/management/change_wipe_code.py +++ b/core/src/apps/management/change_wipe_code.py @@ -15,7 +15,11 @@ async def change_wipe_code(msg: ChangeWipeCode) -> Success: from trezor.ui.layouts import show_success from trezor.wire import NotInitialized - from apps.common.request_pin import error_pin_invalid, request_pin_and_sd_salt + from apps.common.request_pin import ( + error_pin_invalid, + error_pin_not_set, + request_pin_and_sd_salt, + ) if not is_initialized(): raise NotInitialized("Device is not initialized") @@ -24,6 +28,10 @@ async def change_wipe_code(msg: ChangeWipeCode) -> Success: has_wipe_code = config.has_wipe_code() await _require_confirm_action(msg, has_wipe_code) + # Do not allow to set the wipe code if the PIN is not set. + if not config.has_pin(): + await error_pin_not_set() + # Get the unlocking PIN. pin, salt = await request_pin_and_sd_salt(TR.pin__enter) @@ -62,7 +70,7 @@ def _require_confirm_action( from trezor.ui.layouts import confirm_action, confirm_set_new_pin from trezor.wire import ProcessError - if msg.remove and has_wipe_code: + if msg.remove and has_wipe_code: # removing wipe code return confirm_action( "disable_wipe_code", TR.wipe_code__title_settings, @@ -71,7 +79,7 @@ def _require_confirm_action( prompt_screen=True, ) - if not msg.remove and has_wipe_code: + if not msg.remove and has_wipe_code: # changing wipe code return confirm_action( "change_wipe_code", TR.wipe_code__title_settings, @@ -79,7 +87,7 @@ def _require_confirm_action( verb=TR.buttons__change, ) - if not msg.remove and not has_wipe_code: + if not msg.remove and not has_wipe_code: # setting new wipe code return confirm_set_new_pin( "set_wipe_code", TR.wipe_code__title_settings, diff --git a/core/translations/cs.json b/core/translations/cs.json index 28df4065ce..67a0e74b07 100644 --- a/core/translations/cs.json +++ b/core/translations/cs.json @@ -586,6 +586,8 @@ "pin__turn_off": "Opravdu chcete vypnout ochranu PIN kódem?", "pin__turn_on": "Zapnout ochranu PIN kódem?", "pin__wrong_pin": "Nesprávný PIN", + "pin__wipe_code_exists_title": "Aktivní kód pro vymazání", + "pin__wipe_code_exists_description": "Pro vypnutí ochrany PIN kódem musí být vypnut kód pro vymazání.", "plurals__contains_x_keys": "klíč|klíčů", "plurals__lock_after_x_hours": "hod.|hod.", "plurals__lock_after_x_milliseconds": "ms|ms", @@ -960,6 +962,7 @@ "wipe_code__turn_off": "Vypnout ochranu kódem pro vymazání?", "wipe_code__turn_on": "Zapnout ochranu kódem pro vymazání?", "wipe_code__wipe_code_mismatch": "Neshoda kódu vymazání", + "wipe_code__pin_not_set_description": "Pro nastavení kódu pro vymazání musí být aktivní PIN", "word_count__title": "Počet slov", "words__account": "Účet", "words__account_colon": "Účet:", diff --git a/core/translations/de.json b/core/translations/de.json index d412cd9885..5b9da85b7e 100644 --- a/core/translations/de.json +++ b/core/translations/de.json @@ -586,6 +586,8 @@ "pin__turn_off": "Möchtest du den PIN-Schutz wirklich deaktivieren?", "pin__turn_on": "PIN-Schutz aktivieren?", "pin__wrong_pin": "Falsche PIN", + "pin__wipe_code_exists_title": "Aktiver Löschcode", + "pin__wipe_code_exists_description": "Zum Deaktivieren des PIN-Schutzes muss der Löschcode deaktiviert sein.", "plurals__contains_x_keys": "Key|Keys", "plurals__lock_after_x_hours": "Stunde|Stunden", "plurals__lock_after_x_milliseconds": "Millisekunde|Millisekunden", @@ -960,6 +962,7 @@ "wipe_code__turn_off": "Löschcode-Schutz deaktivieren?", "wipe_code__turn_on": "Löschcode-Schutz aktivieren?", "wipe_code__wipe_code_mismatch": "Löschcode-Konflikt", + "wipe_code__pin_not_set_description": "Um den Löschcode einzurichten, muss eine PIN aktiviert sein.", "word_count__title": "Anzahl der wörter", "words__account": "Konto", "words__account_colon": "Konto:", diff --git a/core/translations/en.json b/core/translations/en.json index 02edf7b9fe..08b19f8f0b 100644 --- a/core/translations/en.json +++ b/core/translations/en.json @@ -544,6 +544,8 @@ "pin__turn_off": "Are you sure you want to turn off PIN protection?", "pin__turn_on": "Turn on PIN protection?", "pin__wrong_pin": "Wrong PIN", + "pin__wipe_code_exists_title": "Wipe code set", + "pin__wipe_code_exists_description": "Wipe code must be turned off before turnig off PIN protection.", "plurals__contains_x_keys": "key|keys", "plurals__lock_after_x_hours": "hour|hours", "plurals__lock_after_x_milliseconds": "millisecond|milliseconds", @@ -918,6 +920,7 @@ "wipe_code__turn_off": "Turn off wipe code protection?", "wipe_code__turn_on": "Turn on wipe code protection?", "wipe_code__wipe_code_mismatch": "Wipe code mismatch", + "wipe_code__pin_not_set_description": "PIN must be set before enabling wipe code.", "word_count__title": "Number of words", "words__account": "Account", "words__account_colon": "Account:", diff --git a/core/translations/es.json b/core/translations/es.json index d258dc9a21..a36732b514 100644 --- a/core/translations/es.json +++ b/core/translations/es.json @@ -586,6 +586,8 @@ "pin__turn_off": "¿Quieres desactivar la protección con PIN?", "pin__turn_on": "¿Activar la protección con PIN?", "pin__wrong_pin": "PIN incorrecto", + "pin__wipe_code_exists_title": "Código de borrar activo", + "pin__wipe_code_exists_description": "Para desactivar la protección con PIN, el código de borrar debe estar desactivado.", "plurals__contains_x_keys": "clave|claves", "plurals__lock_after_x_hours": "hora|horas", "plurals__lock_after_x_milliseconds": "milisegundo|milisegundos", @@ -960,6 +962,7 @@ "wipe_code__turn_off": "¿Desactivar protección del código de borrar?", "wipe_code__turn_on": "¿Activar la protección del código de borrar?", "wipe_code__wipe_code_mismatch": "Cód.borrar no coincide.", + "wipe_code__pin_not_set_description": "Para configurar el código de borrar, debe haber un PIN activo.", "word_count__title": "Nro.palabras", "words__account": "Cuenta", "words__account_colon": "Cuenta:", diff --git a/core/translations/fr.json b/core/translations/fr.json index 4830585069..b9b220c658 100644 --- a/core/translations/fr.json +++ b/core/translations/fr.json @@ -586,6 +586,8 @@ "pin__turn_off": "Voulez-vous vraiment désactiver la prot. par PIN ?", "pin__turn_on": "Activer la prot. par PIN ?", "pin__wrong_pin": "Mauvais PIN", + "pin__wipe_code_exists_title": "Code d'eff. actif", + "pin__wipe_code_exists_description": "Pour désactiver la protection par code PIN, le code d'eff. doit être désactivé.", "plurals__contains_x_keys": "touche|touches", "plurals__lock_after_x_hours": "heure|heures", "plurals__lock_after_x_milliseconds": "milliseconde|millisecondes", @@ -960,6 +962,7 @@ "wipe_code__turn_off": "Désactiver la prot. par code d'eff. ?", "wipe_code__turn_on": "Activer la prot. par code d'eff. ?", "wipe_code__wipe_code_mismatch": "Erreur de code d'eff.", + "wipe_code__pin_not_set_description": "Pour configurer le code d'eff., un code PIN doit être actif.", "word_count__title": "Nbr de mots", "words__account": "Compte", "words__account_colon": "Compte:", diff --git a/core/translations/it.json b/core/translations/it.json index f06f00bde5..094fb19289 100644 --- a/core/translations/it.json +++ b/core/translations/it.json @@ -586,6 +586,8 @@ "pin__turn_off": "Disattivare la protezione con PIN?", "pin__turn_on": "Attivare la protez. con PIN?", "pin__wrong_pin": "PIN errato", + "pin__wipe_code_exists_title": "Codice elim. attivo", + "pin__wipe_code_exists_description": "Per disattivare la protezione con PIN, il codice elim. deve essere disattivato.", "plurals__contains_x_keys": "chiave|chiavi", "plurals__lock_after_x_hours": "ora|ore", "plurals__lock_after_x_milliseconds": "millisecondo|millisecondi", @@ -960,6 +962,7 @@ "wipe_code__turn_off": "Disattivare protez. con codice elim.?", "wipe_code__turn_on": "Attivare protez. con codice elim.?", "wipe_code__wipe_code_mismatch": "Codice elim. non corr.", + "wipe_code__pin_not_set_description": "Per configurare il codice elim., un PIN deve essere attivo.", "word_count__title": "Numero di parole", "words__account": "Conto", "words__account_colon": "Conto:", diff --git a/core/translations/order.json b/core/translations/order.json index 6fee8d6de7..5da29fe331 100644 --- a/core/translations/order.json +++ b/core/translations/order.json @@ -973,5 +973,8 @@ "971": "instructions__view_all_data", "972": "ethereum__interaction_contract", "973": "misc__enable_labeling", - "974": "ethereum__unknown_contract_address_short" + "974": "ethereum__unknown_contract_address_short", + "975": "wipe_code__pin_not_set_description", + "976": "pin__wipe_code_exists_description", + "977": "pin__wipe_code_exists_title" } diff --git a/core/translations/pt.json b/core/translations/pt.json index cfc7d82382..b2cc1670b7 100644 --- a/core/translations/pt.json +++ b/core/translations/pt.json @@ -585,6 +585,8 @@ "pin__turn_off": "Deseja desligar a proteção por PIN?", "pin__turn_on": "Ligar proteção por PIN?", "pin__wrong_pin": "PIN incorreto", + "pin__wipe_code_exists_title": "Código de limpeza ativo", + "pin__wipe_code_exists_description": "Para desativar a proteção por PIN, o código de limpeza deve estar desativado.", "plurals__contains_x_keys": "tecla|teclas", "plurals__lock_after_x_hours": "hora|horas", "plurals__lock_after_x_milliseconds": "milissegundo|milissegundos", @@ -959,6 +961,7 @@ "wipe_code__turn_off": "Desativar a proteção do código de limpeza?", "wipe_code__turn_on": "Ativar a proteção do código de limpeza?", "wipe_code__wipe_code_mismatch": "Cód. limp. não coincide", + "wipe_code__pin_not_set_description": "Para configurar o código de limpeza, é necessário ter um PIN ativo.", "word_count__title": "Número de palavras", "words__account": "Conta", "words__account_colon": "Conta:", diff --git a/core/translations/signatures.json b/core/translations/signatures.json index 7c7b10a022..0bf2cac1cc 100644 --- a/core/translations/signatures.json +++ b/core/translations/signatures.json @@ -1,8 +1,8 @@ { "current": { - "merkle_root": "53515eead12df806f139761eddc91f2aa2d3de3b9e0eb831552167ee25897f4a", - "datetime": "2024-12-16T11:26:54.578708", - "commit": "76301b1e97ea5ce0a2e17967f44a9db2a2e905e4" + "merkle_root": "b4a9f4626da2b0a07d43bd807db2da01335069525ba692b4a094331d8bf62539", + "datetime": "2024-12-16T13:26:07.096128", + "commit": "78cce0ba04436b2ec8a72d00d157a1dd37055572" }, "history": [ { diff --git a/core/translations/tr.json b/core/translations/tr.json index 7f61dbf270..30880419f1 100644 --- a/core/translations/tr.json +++ b/core/translations/tr.json @@ -539,6 +539,8 @@ "pin__turn_off": "PIN korumayı kapatmak istediğinizden emin misiniz?", "pin__turn_on": "PIN kodu koruması açılsın mı?", "pin__wrong_pin": "Yanlış PIN kodu", + "pin__wipe_code_exists_title": "Aktif silme kodu", + "pin__wipe_code_exists_description": "PIN korumasını devre dışı bırakmak için silme kodu kapatılmalıdır.", "plurals__contains_x_keys": "anahtar|anahtarlar", "plurals__lock_after_x_hours": "saat|saatler", "plurals__lock_after_x_milliseconds": "milisaniye|milisaniyeler", @@ -881,6 +883,7 @@ "wipe_code__turn_off": "Silme kodu koruması kapatılsın mı?", "wipe_code__turn_on": "Silme kodu koruması açılsın mı?", "wipe_code__wipe_code_mismatch": "Silme kod. uyumsuzluğu", + "wipe_code__pin_not_set_description": "Silme kodunu ayarlamak için bir PIN etkin olmalıdır.", "word_count__title": "Keli̇me sayisi", "words__account": "Hesap", "words__account_colon": "Hesap:", diff --git a/tests/device_tests/test_msg_change_wipe_code_t2.py b/tests/device_tests/test_msg_change_wipe_code_t2.py index 27ca3922b5..fb18d60064 100644 --- a/tests/device_tests/test_msg_change_wipe_code_t2.py +++ b/tests/device_tests/test_msg_change_wipe_code_t2.py @@ -20,7 +20,7 @@ from trezorlib import btc, device, messages from trezorlib.client import MAX_PIN_LENGTH, PASSPHRASE_TEST_PATH from trezorlib.debuglink import LayoutType from trezorlib.debuglink import TrezorClientDebugLink as Client -from trezorlib.exceptions import TrezorFailure +from trezorlib.exceptions import Cancelled, TrezorFailure from ..input_flows import InputFlowNewCodeMismatch @@ -167,3 +167,36 @@ def test_set_pin_to_wipe_code(client: Client): ) client.use_pin_sequence([WIPE_CODE4, WIPE_CODE4]) device.change_pin(client) + + +def test_set_wipe_code_without_pin(client: Client): + # Make sure that both PIN and wipe code are not set. + assert client.features.pin_protection is False + assert client.features.wipe_code_protection is False + # Expect an error when trying to set the wipe code without a PIN being turned on. + with pytest.raises(Cancelled): + device.change_wipe_code(client) + + +@pytest.mark.setup_client(pin=PIN4) +def test_set_remove_pin_without_removing_wipe_code(client: Client): + _ensure_unlocked(client, PIN4) + # Make sure the PIN is set. + assert client.features.pin_protection is True + # Make sure the wipe code is not set. + assert client.features.wipe_code_protection is False + + # Set wipe code + with client: + client.use_pin_sequence([PIN4, WIPE_CODE4, WIPE_CODE4]) + device.change_wipe_code(client) + + client.init_device() + # Make sure the wipe code is set. + assert client.features.wipe_code_protection is True + + # Remove PIN without wipe code being turned off. + with client: + # Expect an error when trying to remove PIN with enabled wipe code. + with pytest.raises(Cancelled): + device.change_pin(client, remove=True)