Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure dependency between PIN and wipe code #4447

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions core/.changelog.d/4446.added
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add dependency check between PIN and wipe code.
3 changes: 3 additions & 0 deletions core/embed/rust/librust_qstr.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions core/mocks/trezortranslate_keys.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down
20 changes: 20 additions & 0 deletions core/src/apps/common/request_pin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 5 additions & 0 deletions core/src/apps/management/change_pin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand All @@ -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)

Expand Down
16 changes: 12 additions & 4 deletions core/src/apps/management/change_wipe_code.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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)

Expand Down Expand Up @@ -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,
Expand All @@ -71,15 +79,15 @@ 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,
description=TR.wipe_code__change,
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,
Expand Down
3 changes: 3 additions & 0 deletions core/translations/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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:",
Expand Down
5 changes: 4 additions & 1 deletion core/translations/order.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
6 changes: 3 additions & 3 deletions core/translations/signatures.json
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
{
"current": {
"merkle_root": "53515eead12df806f139761eddc91f2aa2d3de3b9e0eb831552167ee25897f4a",
"datetime": "2024-12-16T11:26:54.578708",
"commit": "76301b1e97ea5ce0a2e17967f44a9db2a2e905e4"
"merkle_root": "c7d0f97402f5ef98e956db244650e5a8f75e6600317fa4f23ca89b56576e4625",
"datetime": "2024-12-16T13:32:50.509631",
"commit": "d759b147d4ab621c928d0fba726d1379462e07af"
},
"history": [
{
Expand Down
35 changes: 34 additions & 1 deletion tests/device_tests/test_msg_change_wipe_code_t2.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
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

Expand Down Expand Up @@ -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)
Loading