Skip to content

Commit 8e1cf1f

Browse files
benmabenma
committed
api: centralize state check
We had two separate checks to decide if we will allow a protobuf message API call to proceed: in hww.rs (forbid if initialized and locked) and in api.rs in the can_call() function that defines which states the API call can run in. This makes it a bit hard to follow as split into two locations, and it also prevents us from introducing a protobuf call that is only allowed to run if the device is initialized but still locked. `Unlock` would be such a useful new API call if we want to allow entering the passphrase on the host. The hww.rs check passed if `!initialized || !locked`, which means the same as `uninitialized || seeded || unlocked`. We change the `can_call` state checks to include this condition. For this we split the local `Initialized` state into `InitializedAndLocked` and `InitializedAndUnlocked`. For each line in can_call, the two conditions are combined. The equivalent state check in the end is simply to convert all `Initialized` checks to `InitializedAndUnlocked`. The `true` entries become explicit as they can't include `InitializedAndLocked` (e.g. can't change the name using `DeviceName` if initialized but not yet unlocked).
1 parent 5fa56cc commit 8e1cf1f

File tree

2 files changed

+56
-28
lines changed

2 files changed

+56
-28
lines changed

‎src/rust/bitbox02-rust/src/hww.rs

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -119,12 +119,6 @@ pub async fn process_packet(usb_in: Vec<u8>) -> Vec<u8> {
119119
_ => (),
120120
}
121121

122-
// No other message than the attestation and unlock calls shall pass until the device is
123-
// unlocked or ready to be initialized.
124-
if bitbox02::memory::is_initialized() && bitbox02::keystore::is_locked() {
125-
return Vec::new();
126-
}
127-
128122
let mut out = [OP_STATUS_SUCCESS].to_vec();
129123
match noise::process(usb_in, &mut out).await {
130124
Ok(()) => out,
@@ -417,7 +411,14 @@ mod tests {
417411

418412
// Can't reboot when initialized but locked.
419413
bitbox02::keystore::lock();
420-
assert!(make_request(reboot_request.encode_to_vec().as_ref()).is_err());
414+
let response_encoded = make_request(&reboot_request.encode_to_vec()).unwrap();
415+
let response = crate::pb::Response::decode(&response_encoded[..]).unwrap();
416+
assert_eq!(
417+
response,
418+
crate::pb::Response {
419+
response: Some(api::error::make_error(api::error::Error::InvalidState))
420+
},
421+
);
421422

422423
// Unlock.
423424
assert_eq!(

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

Lines changed: 48 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -94,11 +94,17 @@ fn can_call(request: &Request) -> bool {
9494
Uninitialized,
9595
// Seeded (password defined, seed created/loaded).
9696
Seeded,
97-
// Initialized (seed backuped up on SD card).
98-
Initialized,
97+
// InitializedAndLocked (seed backuped up on SD card, keystore locked).
98+
InitializedAndLocked,
99+
// InitializedAndUnlocked (seed backuped up on SD card, keystore unlocked).
100+
InitializedAndUnlocked,
99101
}
100102
let state: State = if bitbox02::memory::is_initialized() {
101-
State::Initialized
103+
if bitbox02::keystore::is_locked() {
104+
State::InitializedAndLocked
105+
} else {
106+
State::InitializedAndUnlocked
107+
}
102108
} else if bitbox02::memory::is_seeded() {
103109
State::Seeded
104110
} else {
@@ -108,33 +114,54 @@ fn can_call(request: &Request) -> bool {
108114
match request {
109115
// Deprecated call, last used in v1.0.0.
110116
Request::PerformAttestation(_) => false,
111-
Request::DeviceInfo(_) => true,
112-
Request::Reboot(_) => true,
113-
Request::DeviceName(_) => true,
114-
Request::DeviceLanguage(_) => true,
115-
Request::CheckSdcard(_) => true,
116-
Request::InsertRemoveSdcard(_) => true,
117-
Request::ListBackups(_) => true,
117+
Request::DeviceInfo(_) => matches!(
118+
state,
119+
State::Uninitialized | State::Seeded | State::InitializedAndUnlocked
120+
),
121+
Request::Reboot(_) => matches!(
122+
state,
123+
State::Uninitialized | State::Seeded | State::InitializedAndUnlocked
124+
),
125+
Request::DeviceName(_) => matches!(
126+
state,
127+
State::Uninitialized | State::Seeded | State::InitializedAndUnlocked
128+
),
129+
Request::DeviceLanguage(_) => matches!(
130+
state,
131+
State::Uninitialized | State::Seeded | State::InitializedAndUnlocked
132+
),
133+
Request::CheckSdcard(_) => matches!(
134+
state,
135+
State::Uninitialized | State::Seeded | State::InitializedAndUnlocked
136+
),
137+
Request::InsertRemoveSdcard(_) => matches!(
138+
state,
139+
State::Uninitialized | State::Seeded | State::InitializedAndUnlocked
140+
),
141+
Request::ListBackups(_) => matches!(
142+
state,
143+
State::Uninitialized | State::Seeded | State::InitializedAndUnlocked
144+
),
118145
Request::SetPassword(_) => matches!(state, State::Uninitialized | State::Seeded),
119146
Request::RestoreBackup(_) => matches!(state, State::Uninitialized | State::Seeded),
120147
Request::RestoreFromMnemonic(_) => matches!(state, State::Uninitialized | State::Seeded),
121-
Request::CreateBackup(_) => matches!(state, State::Seeded | State::Initialized),
122-
Request::ShowMnemonic(_) => matches!(state, State::Seeded | State::Initialized),
123-
Request::Fingerprint(_) => matches!(state, State::Initialized),
124-
Request::ElectrumEncryptionKey(_) => matches!(state, State::Initialized),
148+
Request::CreateBackup(_) => matches!(state, State::Seeded | State::InitializedAndUnlocked),
149+
Request::ShowMnemonic(_) => matches!(state, State::Seeded | State::InitializedAndUnlocked),
150+
Request::Fingerprint(_) => matches!(state, State::InitializedAndUnlocked),
151+
Request::ElectrumEncryptionKey(_) => matches!(state, State::InitializedAndUnlocked),
125152
Request::BtcPub(_) | Request::Btc(_) | Request::BtcSignInit(_) => {
126-
matches!(state, State::Initialized)
153+
matches!(state, State::InitializedAndUnlocked)
127154
}
128155
// These are streamed asynchronously using the `next_request()` primitive in
129156
// bitcoin/signtx.rs and are not handled directly.
130157
Request::BtcSignInput(_) | Request::BtcSignOutput(_) => false,
131158

132-
Request::CheckBackup(_) => matches!(state, State::Initialized),
133-
Request::SetMnemonicPassphraseEnabled(_) => matches!(state, State::Initialized),
134-
Request::Eth(_) => matches!(state, State::Initialized),
135-
Request::Reset(_) => matches!(state, State::Initialized),
136-
Request::Cardano(_) => matches!(state, State::Initialized),
137-
Request::Bip85(_) => matches!(state, State::Initialized),
159+
Request::CheckBackup(_) => matches!(state, State::InitializedAndUnlocked),
160+
Request::SetMnemonicPassphraseEnabled(_) => matches!(state, State::InitializedAndUnlocked),
161+
Request::Eth(_) => matches!(state, State::InitializedAndUnlocked),
162+
Request::Reset(_) => matches!(state, State::InitializedAndUnlocked),
163+
Request::Cardano(_) => matches!(state, State::InitializedAndUnlocked),
164+
Request::Bip85(_) => matches!(state, State::InitializedAndUnlocked),
138165
}
139166
}
140167

0 commit comments

Comments
 (0)