diff --git a/core/.changelog.d/4084.changed b/core/.changelog.d/4084.changed new file mode 100644 index 00000000000..4a8225382d9 --- /dev/null +++ b/core/.changelog.d/4084.changed @@ -0,0 +1 @@ +Increase maximum passphrase length to 128 bytes. diff --git a/core/embed/rust/src/ui/component/base.rs b/core/embed/rust/src/ui/component/base.rs index 9b24c44fd19..083e21d38a8 100644 --- a/core/embed/rust/src/ui/component/base.rs +++ b/core/embed/rust/src/ui/component/base.rs @@ -1,7 +1,7 @@ -use heapless::Vec; +use heapless::{String, Vec}; use crate::{ - strutil::{ShortString, TString}, + strutil::TString, time::Duration, ui::{ button_request::{ButtonRequest, ButtonRequestCode}, @@ -592,6 +592,8 @@ impl EventCtx { } } +pub type FlowMsgText = String<128>; + /// Component::Msg for component parts of a swipe flow. Converting results of /// different screens to a shared type makes things easier to work with. /// @@ -603,7 +605,7 @@ pub enum FlowMsg { Cancelled, Info, Choice(usize), - Text(ShortString), + Text(FlowMsgText), } #[cfg(feature = "micropython")] diff --git a/core/embed/rust/src/ui/component/text/common.rs b/core/embed/rust/src/ui/component/text/common.rs index 34d5015cd5e..6d5bb2f5e43 100644 --- a/core/embed/rust/src/ui/component/text/common.rs +++ b/core/embed/rust/src/ui/component/text/common.rs @@ -1,6 +1,6 @@ -use crate::{ - strutil::ShortString, - ui::{component::EventCtx, util::ResultExt}, +use crate::ui::{ + component::{base::FlowMsgText, EventCtx}, + util::ResultExt, }; /// Reified editing operations of `TextBox`. @@ -16,13 +16,13 @@ pub enum TextEdit { /// operations over it. Text ops usually take a `EventCtx` to request a paint /// pass in case of any state modification. pub struct TextBox { - text: ShortString, + text: FlowMsgText, } impl TextBox { /// Create a new `TextBox` with content `text`. pub fn new(text: &str, max_len: usize) -> Self { - let text = unwrap!(ShortString::try_from(text)); + let text = unwrap!(FlowMsgText::try_from(text)); debug_assert!(text.capacity() >= max_len); Self { text } } diff --git a/core/embed/rust/src/ui/layout/obj.rs b/core/embed/rust/src/ui/layout/obj.rs index 4ec68214a4a..be99651921e 100644 --- a/core/embed/rust/src/ui/layout/obj.rs +++ b/core/embed/rust/src/ui/layout/obj.rs @@ -371,7 +371,7 @@ impl LayoutObj { } pub fn new_root(root: impl LayoutMaybeTrace + 'static) -> Result, Error> { - // SAFETY: This is a Python object and hase a base as first element + // SAFETY: This is a Python object and has a base as first element unsafe { Gc::new_with_custom_finaliser(Self { base: Self::obj_type().as_base(), diff --git a/core/embed/rust/src/ui/layout_bolt/component/keyboard/passphrase.rs b/core/embed/rust/src/ui/layout_bolt/component/keyboard/passphrase.rs index 5bdde78d831..885c1482d69 100644 --- a/core/embed/rust/src/ui/layout_bolt/component/keyboard/passphrase.rs +++ b/core/embed/rust/src/ui/layout_bolt/component/keyboard/passphrase.rs @@ -48,7 +48,7 @@ const KEYBOARD: [[&str; KEY_COUNT]; PAGE_COUNT] = [ ["_<>", ".:@", "/|\\", "!()", "+%&", "-[]", "?{}", ",'`", ";\"~", "$^="], ]; -const MAX_LENGTH: usize = 50; +const MAX_LENGTH: usize = 128; const INPUT_AREA_HEIGHT: i16 = ScrollBar::DOT_SIZE + 9; impl PassphraseKeyboard { diff --git a/core/embed/rust/src/ui/layout_caesar/component/changing_text.rs b/core/embed/rust/src/ui/layout_caesar/component/changing_text.rs index fb8ac492e64..bd5172275e0 100644 --- a/core/embed/rust/src/ui/layout_caesar/component/changing_text.rs +++ b/core/embed/rust/src/ui/layout_caesar/component/changing_text.rs @@ -1,12 +1,9 @@ -use crate::{ - strutil::ShortString, - ui::{ - component::{Component, Event, EventCtx, Never, Pad}, - display::Font, - geometry::{Alignment, Point, Rect}, - shape::{self, Renderer}, - util::long_line_content_with_ellipsis, - }, +use crate::ui::{ + component::{base::FlowMsgText, Component, Event, EventCtx, Never, Pad}, + display::Font, + geometry::{Alignment, Point, Rect}, + shape::{self, Renderer}, + util::long_line_content_with_ellipsis, }; use super::theme; @@ -16,7 +13,7 @@ use super::theme; /// and without being affected by other components. pub struct ChangingTextLine { pad: Pad, - text: ShortString, + text: FlowMsgText, font: Font, /// Whether to show the text. Can be disabled. show_content: bool, @@ -29,7 +26,7 @@ pub struct ChangingTextLine { impl ChangingTextLine { pub fn new(text: &str, font: Font, alignment: Alignment, max_len: usize) -> Self { - let text = unwrap!(ShortString::try_from(text)); + let text = unwrap!(FlowMsgText::try_from(text)); debug_assert!(text.capacity() >= max_len); Self { pad: Pad::with_background(theme::BG), diff --git a/core/embed/rust/src/ui/layout_caesar/component/input_methods/passphrase.rs b/core/embed/rust/src/ui/layout_caesar/component/input_methods/passphrase.rs index b0335b75733..6b698e47dfb 100644 --- a/core/embed/rust/src/ui/layout_caesar/component/input_methods/passphrase.rs +++ b/core/embed/rust/src/ui/layout_caesar/component/input_methods/passphrase.rs @@ -1,9 +1,12 @@ use crate::{ - strutil::{ShortString, TString}, + strutil::TString, translations::TR, trezorhal::random, ui::{ - component::{text::common::TextBox, Child, Component, ComponentExt, Event, EventCtx}, + component::{ + base::FlowMsgText, text::common::TextBox, Child, Component, ComponentExt, Event, + EventCtx, + }, display::Icon, geometry::Rect, shape::Renderer, @@ -26,7 +29,7 @@ enum ChoiceCategory { SpecialSymbol, } -const MAX_PASSPHRASE_LENGTH: usize = 50; +const MAX_PASSPHRASE_LENGTH: usize = 128; const DIGITS: &str = "0123456789"; const LOWERCASE_LETTERS: &str = "abcdefghijklmnopqrstuvwxyz"; @@ -291,17 +294,17 @@ impl PassphraseEntry { fn update_passphrase_dots(&mut self, ctx: &mut EventCtx) { debug_assert!({ - let s = ShortString::new(); + let s = FlowMsgText::new(); s.capacity() >= MAX_PASSPHRASE_LENGTH }); let text_to_show = if self.show_plain_passphrase { - unwrap!(ShortString::try_from(self.passphrase())) + unwrap!(FlowMsgText::try_from(self.passphrase())) } else if self.is_empty() { - unwrap!(ShortString::try_from("")) + unwrap!(FlowMsgText::try_from("")) } else { // Showing asterisks and possibly the last digit. - let mut dots = ShortString::new(); + let mut dots = FlowMsgText::new(); for _ in 0..self.textbox.len() - 1 { unwrap!(dots.push('*')); } diff --git a/core/embed/rust/src/ui/layout_delizia/component/keyboard/passphrase.rs b/core/embed/rust/src/ui/layout_delizia/component/keyboard/passphrase.rs index b7a760d81dc..ebbe3620d70 100644 --- a/core/embed/rust/src/ui/layout_delizia/component/keyboard/passphrase.rs +++ b/core/embed/rust/src/ui/layout_delizia/component/keyboard/passphrase.rs @@ -1,10 +1,12 @@ use crate::{ - strutil::{ShortString, TString}, + strutil::TString, translations::TR, ui::{ component::{ - base::ComponentExt, swipe_detect::SwipeConfig, text::common::TextBox, Component, Event, - EventCtx, Label, Maybe, Never, Swipe, + base::{ComponentExt, FlowMsgText}, + swipe_detect::SwipeConfig, + text::common::TextBox, + Component, Event, EventCtx, Label, Maybe, Never, Swipe, }, display, geometry::{Alignment, Direction, Grid, Insets, Offset, Rect}, @@ -27,7 +29,7 @@ use core::cell::Cell; use num_traits::ToPrimitive; pub enum PassphraseKeyboardMsg { - Confirmed(ShortString), + Confirmed(FlowMsgText), Cancelled, } @@ -101,7 +103,7 @@ const KEYBOARD: [[&str; KEY_COUNT]; PAGE_COUNT] = [ ["_<>", ".:@", "/|\\", "!()", "+%&", "-[]", "?{}", ",'`", ";\"~", "$^="], ]; -const MAX_LENGTH: usize = 50; +const MAX_LENGTH: usize = 128; const CONFIRM_BTN_INSETS: Insets = Insets::new(5, 0, 5, 0); const CONFIRM_EMPTY_BTN_MARGIN_RIGHT: i16 = 7; @@ -338,12 +340,12 @@ impl Component for PassphraseKeyboard { // Confirm button was clicked, we're done. if let Some(ButtonMsg::Clicked) = self.confirm_empty_btn.event(ctx, event) { return Some(PassphraseKeyboardMsg::Confirmed(unwrap!( - ShortString::try_from(self.passphrase()) + FlowMsgText::try_from(self.passphrase()) ))); } if let Some(ButtonMsg::Clicked) = self.confirm_btn.event(ctx, event) { return Some(PassphraseKeyboardMsg::Confirmed(unwrap!( - ShortString::try_from(self.passphrase()) + FlowMsgText::try_from(self.passphrase()) ))); } if let Some(ButtonMsg::Clicked) = self.cancel_btn.event(ctx, event) { diff --git a/core/embed/rust/src/ui/layout_delizia/flow/request_passphrase.rs b/core/embed/rust/src/ui/layout_delizia/flow/request_passphrase.rs index 99eb358bd55..640f8a9bf57 100644 --- a/core/embed/rust/src/ui/layout_delizia/flow/request_passphrase.rs +++ b/core/embed/rust/src/ui/layout_delizia/flow/request_passphrase.rs @@ -1,9 +1,8 @@ use crate::{ error, - strutil::ShortString, translations::TR, ui::{ - component::ComponentExt, + component::{base::FlowMsgText, ComponentExt}, flow::{ base::{Decision, DecisionBuilder as _}, FlowController, FlowMsg, SwipeFlow, @@ -44,7 +43,7 @@ impl FlowController for RequestPassphrase { (Self::Keypad, FlowMsg::Cancelled) => self.return_msg(FlowMsg::Cancelled), (Self::ConfirmEmpty, FlowMsg::Cancelled) => Self::Keypad.goto(), (Self::ConfirmEmpty, FlowMsg::Confirmed) => { - self.return_msg(FlowMsg::Text(ShortString::new())) + self.return_msg(FlowMsg::Text(FlowMsgText::new())) } _ => self.do_nothing(), } diff --git a/core/src/apps/common/passphrase.py b/core/src/apps/common/passphrase.py index ef8bb5b1850..48996a08fc2 100644 --- a/core/src/apps/common/passphrase.py +++ b/core/src/apps/common/passphrase.py @@ -3,7 +3,7 @@ import storage.device as storage_device from trezor.wire import DataError -_MAX_PASSPHRASE_LEN = const(50) +_MAX_PASSPHRASE_LEN = const(128) def is_enabled() -> bool: diff --git a/python/src/trezorlib/client.py b/python/src/trezorlib/client.py index 4e432bd0123..6702abea52e 100644 --- a/python/src/trezorlib/client.py +++ b/python/src/trezorlib/client.py @@ -38,7 +38,7 @@ LOG = logging.getLogger(__name__) -MAX_PASSPHRASE_LENGTH = 50 +MAX_PASSPHRASE_LENGTH = 128 MAX_PIN_LENGTH = 50 PASSPHRASE_ON_DEVICE = object() diff --git a/tests/click_tests/common.py b/tests/click_tests/common.py index 8d32078d45b..b96822dfbd4 100644 --- a/tests/click_tests/common.py +++ b/tests/click_tests/common.py @@ -27,6 +27,32 @@ class CommonPass: EMPTY_ADDRESS = "mvbu1Gdy8SUjTenqerxUaZyYjmveZvt33q" + LONGEST = 64 * "da" + LONGEST_ADDRESS = "mn13QW58ziH2obHPZmGnAhBrqkbYf3LRpo" + + ALMOST_LONGEST = LONGEST[:-1] + ALMOST_LONGEST_ADDRESS = "n48uTdBpsWzNb26ZNysXgkQiocYpR37D2m" + + TOO_LONG = LONGEST + "d" + TOO_LONG_ADDRESS = LONGEST_ADDRESS + + VECTORS = ( # passphrase, address + (SHORT, SHORT_ADDRESS), + (WITH_SPACE, WITH_SPACE_ADDRESS), + (RANDOM_25, RANDOM_25_ADDRESS), + (ALMOST_LONGEST, ALMOST_LONGEST_ADDRESS), + (LONGEST, LONGEST_ADDRESS), + ) + + +assert len(CommonPass.ALMOST_LONGEST) == 127 +assert len(CommonPass.LONGEST) == 128 +assert len(CommonPass.TOO_LONG) == 129 +assert CommonPass.LONGEST_ADDRESS != CommonPass.ALMOST_LONGEST_ADDRESS +assert CommonPass.LONGEST_ADDRESS == CommonPass.TOO_LONG_ADDRESS +# TODO: show some UI message when length reaches the limit? +# (it currently disabled typing and greys out the buttons) + class PassphraseCategory(Enum): MENU = "MENU" diff --git a/tests/click_tests/test_passphrase_bolt.py b/tests/click_tests/test_passphrase_bolt.py index 8f490c03098..e0312d7726c 100644 --- a/tests/click_tests/test_passphrase_bolt.py +++ b/tests/click_tests/test_passphrase_bolt.py @@ -45,24 +45,6 @@ TT_CATEGORY = PassphraseCategory.LOWERCASE TT_COORDS_PREV: buttons.Coords = (0, 0) -# Testing the maximum length is really 50 -# TODO: show some UI message when length reaches 50? -# (it currently disabled typing and greys out the buttons) - -DA_50 = 25 * "da" -DA_50_ADDRESS = "mg5L2i8HZKUvceK1sfmGHhE4gichFSsdvm" -assert len(DA_50) == 50 - -DA_49 = DA_50[:-1] -DA_49_ADDRESS = "mxrB75ydMS3ZzqmYKK28fj4bNMEx7dDw6e" -assert len(DA_49) == 49 -assert DA_49_ADDRESS != DA_50_ADDRESS - -DA_51 = DA_50 + "d" -DA_51_ADDRESS = DA_50_ADDRESS -assert len(DA_51) == 51 -assert DA_51_ADDRESS == DA_50_ADDRESS - @contextmanager def prepare_passphrase_dialogue( @@ -151,16 +133,7 @@ def delete_char(debug: "DebugLink") -> None: debug.click(coords) -VECTORS = ( # passphrase, address - (CommonPass.SHORT, CommonPass.SHORT_ADDRESS), - (CommonPass.WITH_SPACE, CommonPass.WITH_SPACE_ADDRESS), - (CommonPass.RANDOM_25, CommonPass.RANDOM_25_ADDRESS), - (DA_49, DA_49_ADDRESS), - (DA_50, DA_50_ADDRESS), -) - - -@pytest.mark.parametrize("passphrase, address", VECTORS) +@pytest.mark.parametrize("passphrase, address", CommonPass.VECTORS) @pytest.mark.setup_client(passphrase=True) def test_passphrase_input( device_handler: "BackgroundDeviceHandler", passphrase: str, address: str @@ -171,10 +144,10 @@ def test_passphrase_input( @pytest.mark.setup_client(passphrase=True) -def test_passphrase_input_over_50_chars(device_handler: "BackgroundDeviceHandler"): - with prepare_passphrase_dialogue(device_handler, DA_51_ADDRESS) as debug: # type: ignore - input_passphrase(debug, DA_51, check=False) - assert debug.read_layout().passphrase() == DA_50 +def test_passphrase_input_too_long(device_handler: "BackgroundDeviceHandler"): + with prepare_passphrase_dialogue(device_handler, CommonPass.TOO_LONG_ADDRESS) as debug: # type: ignore + input_passphrase(debug, CommonPass.TOO_LONG, check=False) + assert debug.read_layout().passphrase() == CommonPass.LONGEST enter_passphrase(debug) @@ -293,6 +266,7 @@ def test_cycle_through_last_character( # Checks that we can cycle through the last (50th) passphrase character # (was a bug previously) with prepare_passphrase_dialogue(device_handler) as debug: - passphrase = DA_49 + "i" # for i we need to cycle through "ghi" three times + # for "i" we need to cycle through "ghi" three times + passphrase = CommonPass.ALMOST_LONGEST + "i" input_passphrase(debug, passphrase) enter_passphrase(debug) diff --git a/tests/click_tests/test_passphrase_caesar.py b/tests/click_tests/test_passphrase_caesar.py index 57685451ba0..6f29be76bd7 100644 --- a/tests/click_tests/test_passphrase_caesar.py +++ b/tests/click_tests/test_passphrase_caesar.py @@ -186,16 +186,7 @@ def cancel(debug: "DebugLink") -> None: delete_char(debug) -VECTORS = ( # passphrase, address - (CommonPass.SHORT, CommonPass.SHORT_ADDRESS), - (CommonPass.WITH_SPACE, CommonPass.WITH_SPACE_ADDRESS), - (CommonPass.RANDOM_25, CommonPass.RANDOM_25_ADDRESS), - (AAA_49, AAA_49_ADDRESS), - (AAA_50, AAA_50_ADDRESS), -) - - -@pytest.mark.parametrize("passphrase, address", VECTORS) +@pytest.mark.parametrize("passphrase, address", CommonPass.VECTORS) @pytest.mark.setup_client(passphrase=True) def test_passphrase_input( device_handler: "BackgroundDeviceHandler", passphrase: str, address: str @@ -207,22 +198,22 @@ def test_passphrase_input( @pytest.mark.setup_client(passphrase=True) -def test_passphrase_input_over_50_chars(device_handler: "BackgroundDeviceHandler"): - with prepare_passphrase_dialogue(device_handler, AAA_51_ADDRESS) as debug: # type: ignore - # First 50 chars - input_passphrase(debug, AAA_51[:-1]) +def test_passphrase_input_too_long(device_handler: "BackgroundDeviceHandler"): + with prepare_passphrase_dialogue(device_handler, CommonPass.TOO_LONG_ADDRESS) as debug: # type: ignore + # First 128 chars + input_passphrase(debug, CommonPass.TOO_LONG[:-1]) layout = debug.read_layout() - assert AAA_51[:-1] in layout.passphrase() + assert CommonPass.TOO_LONG[:-1] in layout.passphrase() show_passphrase(debug) # Over-limit character - press_char(debug, AAA_51[-1]) + press_char(debug, CommonPass.TOO_LONG[-1]) # No change layout = debug.read_layout() - assert AAA_51[:-1] in layout.passphrase() - assert AAA_51 not in layout.passphrase() + assert CommonPass.TOO_LONG[:-1] in layout.passphrase() + assert CommonPass.TOO_LONG not in layout.passphrase() show_passphrase(debug) enter_passphrase(debug) diff --git a/tests/click_tests/test_passphrase_delizia.py b/tests/click_tests/test_passphrase_delizia.py index 85bdc371735..267acac252d 100644 --- a/tests/click_tests/test_passphrase_delizia.py +++ b/tests/click_tests/test_passphrase_delizia.py @@ -54,22 +54,6 @@ KEYBOARD_CATEGORY = PassphraseCategory.LOWERCASE COORDS_PREV: buttons.Coords = (0, 0) -# Testing the maximum length is really 50 - -DA_50 = 25 * "da" -DA_50_ADDRESS = "mg5L2i8HZKUvceK1sfmGHhE4gichFSsdvm" -assert len(DA_50) == 50 - -DA_49 = DA_50[:-1] -DA_49_ADDRESS = "mxrB75ydMS3ZzqmYKK28fj4bNMEx7dDw6e" -assert len(DA_49) == 49 -assert DA_49_ADDRESS != DA_50_ADDRESS - -DA_51 = DA_50 + "d" -DA_51_ADDRESS = DA_50_ADDRESS -assert len(DA_51) == 51 -assert DA_51_ADDRESS == DA_50_ADDRESS - def get_passphrase_choices(char: str) -> tuple[str, ...]: if char in " *#": @@ -182,16 +166,7 @@ def delete_char(debug: "DebugLink") -> None: debug.click(coords) -VECTORS = ( # passphrase, address - (CommonPass.SHORT, CommonPass.SHORT_ADDRESS), - (CommonPass.WITH_SPACE, CommonPass.WITH_SPACE_ADDRESS), - (CommonPass.RANDOM_25, CommonPass.RANDOM_25_ADDRESS), - (DA_49, DA_49_ADDRESS), - (DA_50, DA_50_ADDRESS), -) - - -@pytest.mark.parametrize("passphrase, address", VECTORS) +@pytest.mark.parametrize("passphrase, address", CommonPass.VECTORS) @pytest.mark.setup_client(passphrase=True) def test_passphrase_input( device_handler: "BackgroundDeviceHandler", passphrase: str, address: str @@ -202,10 +177,10 @@ def test_passphrase_input( @pytest.mark.setup_client(passphrase=True) -def test_passphrase_input_over_50_chars(device_handler: "BackgroundDeviceHandler"): - with prepare_passphrase_dialogue(device_handler, DA_51_ADDRESS) as debug: # type: ignore - input_passphrase(debug, DA_51, check=False) - assert debug.read_layout().passphrase() == DA_50 +def test_passphrase_input_over_max_length(device_handler: "BackgroundDeviceHandler"): + with prepare_passphrase_dialogue(device_handler, CommonPass.TOO_LONG_ADDRESS) as debug: # type: ignore + input_passphrase(debug, CommonPass.TOO_LONG, check=False) + assert debug.read_layout().passphrase() == CommonPass.LONGEST enter_passphrase(debug) @@ -323,9 +298,10 @@ def test_passphrase_dollar_sign_deletion( def test_cycle_through_last_character( device_handler: "BackgroundDeviceHandler", ): - # Checks that we can cycle through the last (50th) passphrase character + # Checks that we can cycle through the last (128th) passphrase character # (was a bug previously) with prepare_passphrase_dialogue(device_handler) as debug: - passphrase = DA_49 + "i" # for i we need to cycle through "ghi" three times + # for "i" we need to cycle through "ghi" three times + passphrase = CommonPass.ALMOST_LONGEST + "i" input_passphrase(debug, passphrase) enter_passphrase(debug) diff --git a/tests/device_tests/test_session_id_and_passphrase.py b/tests/device_tests/test_session_id_and_passphrase.py index 51a2c0731fd..d85e5802b6c 100644 --- a/tests/device_tests/test_session_id_and_passphrase.py +++ b/tests/device_tests/test_session_id_and_passphrase.py @@ -18,7 +18,7 @@ import pytest -from trezorlib import device, exceptions, messages +from trezorlib import device, exceptions, messages, models from trezorlib.debuglink import LayoutType from trezorlib.debuglink import TrezorClientDebugLink as Client from trezorlib.exceptions import TrezorFailure @@ -369,14 +369,17 @@ def call(passphrase: str, expected_result: bool): assert expected_result is False, "Call should have succeeded" assert e.code == FailureType.DataError - # 50 is ok - call(passphrase="A" * 50, expected_result=True) - # 51 is not - call(passphrase="A" * 51, expected_result=False) - # "š" has two bytes - 48x A and "š" should be fine (50 bytes) - call(passphrase="A" * 48 + "š", expected_result=True) - # "š" has two bytes - 49x A and "š" should not (51 bytes) - call(passphrase="A" * 49 + "š", expected_result=False) + max_len = 50 if client.model is models.T1B1 else 128 + + # N is ok + call(passphrase="A" * max_len, expected_result=True) + # N+1 is not + call(passphrase="A" * (max_len + 1), expected_result=False) + + # "š" has two bytes - (N-2)x A and "š" should be fine (N bytes) + call(passphrase="A" * (max_len - 2) + "š", expected_result=True) + # "š" has two bytes - (N-1)x A and "š" should not (N+1 bytes) + call(passphrase="A" * (max_len - 1) + "š", expected_result=False) @pytest.mark.models("core")