From 5907bd511180587964f76ed317b46379a3d2675d Mon Sep 17 00:00:00 2001 From: Marko Bencun Date: Sat, 22 Aug 2020 00:45:13 +0200 Subject: [PATCH] rust/hww: disallow api calls according to commander_states 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. --- src/CMakeLists.txt | 2 ++ src/commander/commander.c | 13 ++----- src/rust/bitbox02-rust/src/hww/api/api.rs | 44 +++++++++++++++++++++++ src/rust/bitbox02-sys/wrapper.h | 1 + src/rust/bitbox02/src/commander.rs | 8 +++++ src/rust/bitbox02/src/lib.rs | 12 +++++++ 6 files changed, 69 insertions(+), 11 deletions(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index cc812ac9aa..edceac7b3b 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -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 diff --git a/src/commander/commander.c b/src/commander/commander.c index ee48170dae..ee28938157 100644 --- a/src/commander/commander.c +++ b/src/commander/commander.c @@ -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); diff --git a/src/rust/bitbox02-rust/src/hww/api/api.rs b/src/rust/bitbox02-rust/src/hww/api/api.rs index 5437dc7a33..295d94c772 100644 --- a/src/rust/bitbox02-rust/src/hww/api/api.rs +++ b/src/rust/bitbox02-rust/src/hww/api/api.rs @@ -67,6 +67,41 @@ fn encode(response: Response) -> Vec { 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 { @@ -114,6 +149,15 @@ pub async fn process(input: Vec) -> Vec { }) => 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. diff --git a/src/rust/bitbox02-sys/wrapper.h b/src/rust/bitbox02-sys/wrapper.h index 05ce9ed53b..b199f96096 100644 --- a/src/rust/bitbox02-sys/wrapper.h +++ b/src/rust/bitbox02-sys/wrapper.h @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include diff --git a/src/rust/bitbox02/src/commander.rs b/src/rust/bitbox02/src/commander.rs index 4d6ca6ced7..19ad92177a 100644 --- a/src/rust/bitbox02/src/commander.rs +++ b/src/rust/bitbox02/src/commander.rs @@ -44,3 +44,11 @@ pub fn commander(input: Vec) -> Vec { }; 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() } +} diff --git a/src/rust/bitbox02/src/lib.rs b/src/rust/bitbox02/src/lib.rs index d82f688626..339f803def 100644 --- a/src/rust/bitbox02/src/lib.rs +++ b/src/rust/bitbox02/src/lib.rs @@ -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;