Skip to content

Commit efe6bac

Browse files
committed
workflow: default to digits in password entry when using Optiga
The Optiga securechip implementation limits password unlock attempts to 10 at the secure chip config level, whereas in the ATECC, the securechip-imposed limit is ~700k. For this reason, when using the Optiga, users can safely use passwords consisting only of digits, so we default to the digits keyboard in that case. Letters are still available through the keyboard switcher.
1 parent 5ffaae4 commit efe6bac

File tree

10 files changed

+55
-24
lines changed

10 files changed

+55
-24
lines changed

src/rust/bitbox02-rust/src/hww/api/error.rs

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,17 +48,23 @@ impl core::convert::From<crate::workflow::cancel::Error> for Error {
4848
}
4949
}
5050

51+
impl core::convert::From<crate::workflow::password::EnterError> for Error {
52+
fn from(error: crate::workflow::password::EnterError) -> Self {
53+
match error {
54+
crate::workflow::password::EnterError::Memory => Error::Memory,
55+
crate::workflow::password::EnterError::Cancelled => Error::UserAbort,
56+
}
57+
}
58+
}
59+
5160
impl core::convert::From<crate::workflow::password::EnterTwiceError> for Error {
5261
fn from(error: crate::workflow::password::EnterTwiceError) -> Self {
5362
match error {
5463
crate::workflow::password::EnterTwiceError::DoNotMatch => {
5564
// For backwards compatibility.
5665
Error::Generic
5766
}
58-
crate::workflow::password::EnterTwiceError::Cancelled => {
59-
// Added in v9.13.0.
60-
Error::UserAbort
61-
}
67+
crate::workflow::password::EnterTwiceError::EnterError(err) => err.into(),
6268
}
6369
}
6470
}
@@ -108,6 +114,7 @@ impl core::convert::From<UnlockError> for Error {
108114
fn from(error: UnlockError) -> Self {
109115
match error {
110116
UnlockError::UserAbort => Error::UserAbort,
117+
UnlockError::Memory => Error::Memory,
111118
UnlockError::IncorrectPassword | UnlockError::Generic => Error::Generic,
112119
}
113120
}

src/rust/bitbox02-rust/src/hww/api/restore.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ pub async fn from_mnemonic(
140140
})
141141
.await?;
142142
}
143-
Err(password::EnterTwiceError::Cancelled) => return Err(Error::UserAbort),
143+
Err(err @ password::EnterTwiceError::EnterError(_)) => return Err(err.into()),
144144
Ok(password) => break password,
145145
}
146146
};

src/rust/bitbox02-rust/src/workflow/password.rs

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,12 @@ pub enum PasswordType {
3434
Bip39Passphrase,
3535
}
3636

37+
#[derive(Debug)]
38+
pub enum EnterError {
39+
Memory,
40+
Cancelled,
41+
}
42+
3743
/// If `can_cancel` is `Yes`, the workflow can be cancelled.
3844
/// If it is no, the result is always `Ok(())`.
3945
///
@@ -47,7 +53,7 @@ pub async fn enter(
4753
title: &str,
4854
password_type: PasswordType,
4955
can_cancel: CanCancel,
50-
) -> Result<zeroize::Zeroizing<String>, Error> {
56+
) -> Result<zeroize::Zeroizing<String>, EnterError> {
5157
let params = trinary_input_string::Params {
5258
title,
5359
hide: true,
@@ -56,14 +62,23 @@ pub async fn enter(
5662
PasswordType::Bip39Passphrase => true,
5763
},
5864
longtouch: true,
65+
default_to_digits: match password_type {
66+
PasswordType::DevicePassword => {
67+
match bitbox02::memory::get_securechip_type().map_err(|_| EnterError::Memory)? {
68+
bitbox02::memory::SecurechipType::Atecc => false,
69+
bitbox02::memory::SecurechipType::Optiga => true,
70+
}
71+
}
72+
PasswordType::Bip39Passphrase => false,
73+
},
5974
..Default::default()
6075
};
6176

6277
loop {
6378
match hal.ui().enter_string(&params, can_cancel, "").await {
64-
o @ Ok(_) => return o,
79+
Ok(pw) => return Ok(pw),
6580
Err(Error::Cancelled) => match prompt_cancel(hal).await {
66-
Ok(()) => return Err(Error::Cancelled),
81+
Ok(()) => return Err(EnterError::Cancelled),
6782
Err(confirm::UserAbort) => {}
6883
},
6984
}
@@ -72,14 +87,12 @@ pub async fn enter(
7287

7388
pub enum EnterTwiceError {
7489
DoNotMatch,
75-
Cancelled,
90+
EnterError(EnterError),
7691
}
7792

78-
impl core::convert::From<Error> for EnterTwiceError {
79-
fn from(error: Error) -> Self {
80-
match error {
81-
Error::Cancelled => EnterTwiceError::Cancelled,
82-
}
93+
impl core::convert::From<EnterError> for EnterTwiceError {
94+
fn from(error: EnterError) -> Self {
95+
EnterTwiceError::EnterError(error)
8396
}
8497
}
8598

@@ -126,7 +139,7 @@ pub async fn enter_twice(
126139
{
127140
Ok(()) => break,
128141
Err(confirm::UserAbort) => match prompt_cancel(hal).await {
129-
Ok(()) => return Err(EnterTwiceError::Cancelled),
142+
Ok(()) => return Err(EnterTwiceError::EnterError(EnterError::Cancelled)),
130143
Err(confirm::UserAbort) => {}
131144
},
132145
}

src/rust/bitbox02-rust/src/workflow/unlock.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,16 @@ async fn confirm_mnemonic_passphrase(
5555
pub enum UnlockError {
5656
UserAbort,
5757
IncorrectPassword,
58+
Memory,
5859
Generic,
5960
}
6061

61-
impl core::convert::From<super::cancel::Error> for UnlockError {
62-
fn from(_error: super::cancel::Error) -> Self {
63-
UnlockError::UserAbort
62+
impl core::convert::From<password::EnterError> for UnlockError {
63+
fn from(error: password::EnterError) -> Self {
64+
match error {
65+
password::EnterError::Cancelled => UnlockError::UserAbort,
66+
password::EnterError::Memory => UnlockError::Memory,
67+
}
6468
}
6569
}
6670

@@ -119,7 +123,7 @@ pub async fn unlock_bip39(hal: &mut impl crate::hal::Hal) {
119123
password::CanCancel::No,
120124
)
121125
.await
122-
.expect("not cancelable");
126+
.expect("not cancelable and does not call memory functions");
123127

124128
if let Ok(()) = confirm_mnemonic_passphrase(hal, mnemonic_passphrase.as_str()).await {
125129
break;

src/rust/bitbox02/src/ui/types.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ pub struct TrinaryInputStringParams<'a> {
107107
pub special_chars: bool,
108108
pub longtouch: bool,
109109
pub cancel_is_backbutton: bool,
110+
pub default_to_digits: bool,
110111
}
111112

112113
impl<'a> TrinaryInputStringParams<'a> {
@@ -138,6 +139,7 @@ impl<'a> TrinaryInputStringParams<'a> {
138139
special_chars: self.special_chars,
139140
longtouch: self.longtouch,
140141
cancel_is_backbutton: self.cancel_is_backbutton,
142+
default_to_digits: self.default_to_digits,
141143
})
142144
}
143145
}

src/ui/components/keyboard_switch.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,7 @@ static component_functions_t _component_functions = {
152152
component_t* keyboard_switch_create(
153153
slider_location_t location,
154154
bool special_chars,
155+
bool default_to_digits,
155156
component_t* parent)
156157
{
157158
component_t* keyboard_switch = malloc(sizeof(component_t));
@@ -167,7 +168,7 @@ component_t* keyboard_switch_create(
167168
memset(ks_data, 0, sizeof(keyboard_switch_data_t));
168169

169170
ks_data->location = location;
170-
ks_data->mode = LOWER_CASE;
171+
ks_data->mode = default_to_digits ? DIGITS : LOWER_CASE;
171172
ks_data->active = false;
172173
ks_data->special_chars = special_chars;
173174

src/ui/components/keyboard_switch.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,14 @@ typedef enum { LOWER_CASE, UPPER_CASE, DIGITS, SPECIAL_CHARS } keyboard_mode_t;
2525
/**
2626
* Creates a keyboard switch component.
2727
* @param[in] location The slider location.
28-
* @param[in] make special chars keyboard mode available.
28+
* @param[in] special_chars make special chars keyboard mode available.
29+
* @param[in] default_to_digits start with the digits keyboard instead of the lowercase keyboard.
2930
* @param[in] parent The parent component.
3031
*/
3132
component_t* keyboard_switch_create(
3233
slider_location_t location,
3334
bool special_chars,
35+
bool default_to_digits,
3436
component_t* parent);
3537

3638
/**

src/ui/components/trinary_input_string.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -490,8 +490,8 @@ component_t* trinary_input_string_create(
490490
ui_util_add_sub_component(component, data->confirm_component);
491491

492492
if (params->wordlist == NULL && !params->number_input) {
493-
data->keyboard_switch_component =
494-
keyboard_switch_create(top_slider, params->special_chars, component);
493+
data->keyboard_switch_component = keyboard_switch_create(
494+
top_slider, params->special_chars, params->default_to_digits, component);
495495
ui_util_add_sub_component(component, data->keyboard_switch_component);
496496
}
497497

src/ui/components/trinary_input_string.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ typedef struct {
3838
bool longtouch;
3939
// whether the cancel button should be rendered as a back button instead of as a cross.
4040
bool cancel_is_backbutton;
41+
// start with the digits keyboard instead of the lowercase keyboard.
42+
bool default_to_digits;
4143
} trinary_input_string_params_t;
4244

4345
/********************************** Create Instance **********************************/

test/unit-test/test_ui_components.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ static void test_ui_components_keyboard_switch(void** state)
130130
{
131131
component_t* mock_component = mock_component_create();
132132

133-
component_t* keyboard_switch = keyboard_switch_create(top_slider, true, mock_component);
133+
component_t* keyboard_switch = keyboard_switch_create(top_slider, true, false, mock_component);
134134
assert_non_null(keyboard_switch);
135135
assert_ui_component_functions(keyboard_switch);
136136
keyboard_switch->f->cleanup(keyboard_switch);

0 commit comments

Comments
 (0)