Skip to content

Commit

Permalink
rust/hww: disallow api calls according to commander_states
Browse files Browse the repository at this point in the history
Moving the commander states check calls from C to Rust makes sure that
API calls handled in Rust obey the same rules, e.g. that SetDeviceName
cannot be called in the middle of a Bitcoin tx signing session.
  • Loading branch information
benma committed Aug 22, 2020
1 parent ad81783 commit 5907bd5
Show file tree
Hide file tree
Showing 6 changed files with 69 additions and 11 deletions.
2 changes: 2 additions & 0 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,8 @@ add_custom_target(rust-bindgen
--whitelist-type commander_error_t
--rustified-enum commander_error_t
--whitelist-function commander
--whitelist-function commander_states_can_call
--whitelist-function commander_states_clear_force_next
--whitelist-type BitBoxBaseRequest
--whitelist-var ".*_tag"
--whitelist-var MAX_LABEL_SIZE
Expand Down
13 changes: 2 additions & 11 deletions src/commander/commander.c
Original file line number Diff line number Diff line change
Expand Up @@ -362,17 +362,8 @@ void commander(const in_buffer_t* in_buf, buffer_t* out_buf)
commander_error_t err =
protobuf_decode(in_buf, &request) ? COMMANDER_OK : COMMANDER_ERR_INVALID_INPUT;
if (err == COMMANDER_OK) {
if (!commander_states_can_call(request.which_request)) {
err = COMMANDER_ERR_INVALID_STATE;
} else {
// Since we will process the call now, so can clear the 'force next' info.
// We do this before processing as the api call can potentially define the next api call
// to be forced.
commander_states_clear_force_next();

err = _api_process(&request, &response);
util_zero(&request, sizeof(request));
}
err = _api_process(&request, &response);
util_zero(&request, sizeof(request));
}
if (err != COMMANDER_OK) {
_report_error(&response, err);
Expand Down
44 changes: 44 additions & 0 deletions src/rust/bitbox02-rust/src/hww/api/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,41 @@ fn encode(response: Response) -> Vec<u8> {
out
}

/// Returns the field tag number of the request as defined in the .proto file. This is needed for
/// compatibility with commander_states.c, and needed as long as API calls processed in C use
/// `commmander_states_force_next()`.
fn request_tag(request: &Request) -> u32 {
use Request::*;
match request {
RandomNumber(_) => bitbox02::Request_random_number_tag,
DeviceName(_) => bitbox02::Request_device_name_tag,
DeviceLanguage(_) => bitbox02::Request_device_language_tag,
DeviceInfo(_) => bitbox02::Request_device_info_tag,
SetPassword(_) => bitbox02::Request_set_password_tag,
CreateBackup(_) => bitbox02::Request_create_backup_tag,
ShowMnemonic(_) => bitbox02::Request_show_mnemonic_tag,
BtcPub(_) => bitbox02::Request_btc_pub_tag,
BtcSignInit(_) => bitbox02::Request_btc_sign_init_tag,
BtcSignInput(_) => bitbox02::Request_btc_sign_input_tag,
BtcSignOutput(_) => bitbox02::Request_btc_sign_output_tag,
InsertRemoveSdcard(_) => bitbox02::Request_insert_remove_sdcard_tag,
CheckSdcard(_) => bitbox02::Request_check_sdcard_tag,
SetMnemonicPassphraseEnabled(_) => bitbox02::Request_set_mnemonic_passphrase_enabled_tag,
ListBackups(_) => bitbox02::Request_list_backups_tag,
RestoreBackup(_) => bitbox02::Request_restore_backup_tag,
PerformAttestation(_) => bitbox02::Request_perform_attestation_tag,
Reboot(_) => bitbox02::Request_reboot_tag,
CheckBackup(_) => bitbox02::Request_check_backup_tag,
Eth(_) => bitbox02::Request_eth_tag,
Reset(_) => bitbox02::Request_reset_tag,
RestoreFromMnemonic(_) => bitbox02::Request_restore_from_mnemonic_tag,
Bitboxbase(_) => bitbox02::Request_bitboxbase_tag,
Fingerprint(_) => bitbox02::Request_fingerprint_tag,
Btc(_) => bitbox02::Request_btc_tag,
ElectrumEncryptionKey(_) => bitbox02::Request_electrum_encryption_key_tag,
}
}

async fn api_set_device_name(
pb::SetDeviceNameRequest { name }: &pb::SetDeviceNameRequest,
) -> Response {
Expand Down Expand Up @@ -114,6 +149,15 @@ pub async fn process(input: Vec<u8>) -> Vec<u8> {
}) => request,
_ => return encode(make_error(Error::COMMANDER_ERR_INVALID_INPUT)),
};
if !bitbox02::commander::states_can_call(request_tag(&request) as u16) {
return encode(make_error(Error::COMMANDER_ERR_INVALID_STATE));
}

// Since we will process the call now, so can clear the 'force next' info.
// We do this before processing as the api call can potentially define the next api call
// to be forced.
bitbox02::commander::states_clear_force_next();

match process_api(&request).await {
Some(response) => encode(response),
// Api call not handled in Rust -> handle it in C.
Expand Down
1 change: 1 addition & 0 deletions src/rust/bitbox02-sys/wrapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include <bitboxbase/bitboxbase_screensaver.h>
#include <bitboxbase/bitboxbase_watchdog.h>
#include <commander/commander.h>
#include <commander/commander_states.h>
#include <keystore.h>
#include <memory/memory.h>
#include <platform/bitboxbase/leds.h>
Expand Down
8 changes: 8 additions & 0 deletions src/rust/bitbox02/src/commander.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,11 @@ pub fn commander(input: Vec<u8>) -> Vec<u8> {
};
output_vec
}

pub fn states_can_call(request_tag: u16) -> bool {
unsafe { bitbox02_sys::commander_states_can_call(request_tag) }
}

pub fn states_clear_force_next() {
unsafe { bitbox02_sys::commander_states_clear_force_next() }
}
12 changes: 12 additions & 0 deletions src/rust/bitbox02/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,18 @@ pub const BitBoxBaseRequest_display_status_tag: u16 =
pub const BitBoxBaseRequest_set_config_tag: u16 =
bitbox02_sys::BitBoxBaseRequest_set_config_tag as u16;

pub use bitbox02_sys::{
Request_bitboxbase_tag, Request_btc_pub_tag, Request_btc_sign_init_tag,
Request_btc_sign_input_tag, Request_btc_sign_output_tag, Request_btc_tag,
Request_check_backup_tag, Request_check_sdcard_tag, Request_create_backup_tag,
Request_device_info_tag, Request_device_language_tag, Request_device_name_tag,
Request_electrum_encryption_key_tag, Request_eth_tag, Request_fingerprint_tag,
Request_insert_remove_sdcard_tag, Request_list_backups_tag, Request_perform_attestation_tag,
Request_random_number_tag, Request_reboot_tag, Request_reset_tag, Request_restore_backup_tag,
Request_restore_from_mnemonic_tag, Request_set_mnemonic_passphrase_enabled_tag,
Request_set_password_tag, Request_show_mnemonic_tag,
};

// Use this for functions exported to "C"
#[allow(non_camel_case_types)]
pub type commander_error_t = bitbox02_sys::commander_error_t;
Expand Down

0 comments on commit 5907bd5

Please sign in to comment.