diff --git a/jadepy/jade_ble.py b/jadepy/jade_ble.py index a378cecc..23dae340 100644 --- a/jadepy/jade_ble.py +++ b/jadepy/jade_ble.py @@ -104,7 +104,7 @@ async def _input_stream(): # Connect - seems pretty flaky so allow retries connected = False - attempts_remaining = 3 + attempts_remaining = 5 while not connected: try: attempts_remaining -= 1 @@ -118,7 +118,9 @@ async def _input_stream(): logger.warning("BLE connection exception: '{}'".format(e)) if not attempts_remaining: logger.warning("Exhausted retries - BLE connection failed") - raise + raise JadeError(2, "Unable to connect to BLE device", + "Device name: {}, Serial number: {}".format( + self.device_name, self.serial_number or '')) # Peruse services and characteristics # Get the 'handle' of the receiving charactersitic diff --git a/jadepy/jade_serial.py b/jadepy/jade_serial.py index 37706fe6..4b17c59d 100644 --- a/jadepy/jade_serial.py +++ b/jadepy/jade_serial.py @@ -2,6 +2,7 @@ import logging from serial.tools import list_ports +from .jade_error import JadeError logger = logging.getLogger(__name__) @@ -53,7 +54,10 @@ def connect(self): assert self.ser is not None if not self.ser.is_open: - self.ser.open() + try: + self.ser.open() + except serial.serialutil.SerialException: + raise JadeError(1, "Unable to open port", self.device) # Ensure RTS and DTR are not set (as this can cause the hw to reboot) self.ser.setRTS(False) diff --git a/main/process/dashboard.c b/main/process/dashboard.c index 25a04a1d..28cd7a1b 100644 --- a/main/process/dashboard.c +++ b/main/process/dashboard.c @@ -29,6 +29,7 @@ static inline bool ble_enabled(void) { return false; } static inline bool ble_connected(void) { return false; } static inline void ble_start(void) { JADE_ASSERT(false); } +static inline void ble_stop(void) { return; } #endif #include "process/ota_defines.h" #include "process_utils.h" @@ -227,6 +228,53 @@ void build_version_info_reply(const void* ctx, CborEncoder* container); // Set flag to change PIN on next successful unlock void set_request_change_pin(bool change_pin); +static void process_get_version_info_request(jade_process_t* process) +{ + ASSERT_CURRENT_MESSAGE(process, "get_version_info"); + jade_process_reply_to_message_result(process->ctx, &process->ctx.source, build_version_info_reply); +} + +// If the user has successfully authenticated over a given connection interface, +// we stop/close the 'other' interface for security and performance/stability reasons. +// If the user is not authenticated or tied to a given interface, enable all. +static void enable_connection_interfaces(const jade_msg_source_t source) +{ + JADE_LOGD("enable_connection_interfaces(%u)", source); + + if (source == SOURCE_SERIAL || (source == SOURCE_NONE && internal_relogin_source == SOURCE_SERIAL)) { + // serial_start(); + ble_stop(); + } else if (source == SOURCE_BLE || (source == SOURCE_NONE && internal_relogin_source == SOURCE_BLE)) { + ble_start(); +#ifdef CONFIG_LOG_DEFAULT_LEVEL_NONE // Leave serial up if used for logging + // serial_stop(); +#endif + } else if (source == SOURCE_INTERNAL || (source == SOURCE_NONE && internal_relogin_source == SOURCE_INTERNAL)) { + ble_stop(); +#ifdef CONFIG_LOG_DEFAULT_LEVEL_NONE // Leave serial up if used for logging + // serial_stop(); +#endif + } else { // ie. SOURCE_NONE + JADE_ASSERT(internal_relogin_source == SOURCE_NONE); + + // Ensure serial running, and start BLE if configured but not running + // serial_start(); + +#ifdef CONFIG_BT_ENABLED + if (!ble_enabled()) { +#ifndef CONFIG_DEBUG_UNATTENDED_CI + const uint8_t ble_flags = storage_get_ble_flags(); +#else + const uint8_t ble_flags = BLE_ENABLED; +#endif + if (ble_flags & BLE_ENABLED) { + ble_start(); + } + } +#endif + } +} + // Home screen/menu update static void update_home_screen(gui_view_node_t* status_light, gui_view_node_t* status_text, gui_view_node_t* label) { @@ -299,12 +347,6 @@ static void format_pin(char* buf, const uint8_t buf_len, const uint8_t* pin, con } } -static void process_get_version_info_request(jade_process_t* process) -{ - ASSERT_CURRENT_MESSAGE(process, "get_version_info"); - jade_process_reply_to_message_result(process->ctx, &process->ctx.source, build_version_info_reply); -} - // Unpack entropy bytes from message and add to random generator static void process_add_entropy_request(jade_process_t* process) { @@ -404,6 +446,9 @@ static void dispatch_message(jade_process_t* process) // or // b) There is no PIN set (ie. no encrypted keys set, eg. new device) if ((KEYCHAIN_UNLOCKED_BY_MESSAGE_SOURCE(process) && !keychain_has_temporary()) || !keychain_has_pin()) { + // If we are about to start an OTA we stop the other/unused connection + // interface for performance, stabililty and security reasons. + enable_connection_interfaces(process->ctx.source); task_function = ota_process; } else { // Reject the message as hw locked @@ -822,19 +867,30 @@ static void handle_ble_reset(void) } } +static inline void update_ble_status_item(gui_view_node_t* ble_status_item, const bool enabled) +{ + gui_update_text(ble_status_item, enabled ? "Status: Enabled" : "Status: Disabled"); +} + +static inline void update_ble_carousel_label(gui_view_node_t* status_textbox, const bool enabled) +{ + gui_update_text(status_textbox, enabled ? "Enabled" : "Disabled"); +} + // BLE properties screen static void handle_ble(void) { uint8_t ble_flags = storage_get_ble_flags(); + bool enabled = (ble_flags & BLE_ENABLED); gui_view_node_t* ble_status_item = NULL; gui_activity_t* const act = make_ble_activity(&ble_status_item); - gui_update_text(ble_status_item, ble_enabled() ? "Status: Enabled" : "Status: Disabled"); + update_ble_status_item(ble_status_item, enabled); gui_set_current_activity(act); gui_view_node_t* status_textbox = NULL; gui_activity_t* const act_status = make_carousel_activity("Bluetooth Status", NULL, &status_textbox); - gui_update_text(status_textbox, ble_enabled() ? "Enabled" : "Disabled"); + update_ble_carousel_label(status_textbox, enabled); int32_t ev_id; while (true) { @@ -852,12 +908,11 @@ static void handle_ble(void) if (ret) { if (ev_id == BTN_BLE_STATUS) { gui_set_current_activity(act_status); - bool enable_ble = ble_enabled(); while (true) { - gui_update_text(status_textbox, enable_ble ? "Enabled" : "Disabled"); + update_ble_carousel_label(status_textbox, enabled); if (gui_activity_wait_event(act_status, GUI_EVENT, ESP_EVENT_ANY_ID, NULL, &ev_id, NULL, 0)) { if (ev_id == GUI_WHEEL_LEFT_EVENT || ev_id == GUI_WHEEL_RIGHT_EVENT) { - enable_ble = !enable_ble; // Just toggle label at this point + enabled = !enabled; // Just toggle label at this point } else if (ev_id == gui_get_click_event()) { // Done - apply ble change break; @@ -866,9 +921,15 @@ static void handle_ble(void) } // Start/stop BLE and persist pref/flags - if (enable_ble) { + if (enabled) { if (!ble_enabled()) { - ble_start(); + // Only start BLE immediately if not using some other interface + if (keychain_get_userdata() == SOURCE_NONE) { + ble_start(); + } else { + const char* message[] = { "Bluetooth will be", "started on logout", "or disconnection" }; + await_message_activity(message, 3); + } } ble_flags |= BLE_ENABLED; storage_set_ble_flags(ble_flags); @@ -879,7 +940,7 @@ static void handle_ble(void) ble_flags &= ~BLE_ENABLED; storage_set_ble_flags(ble_flags); } - gui_update_text(ble_status_item, ble_enabled() ? "Status: Enabled" : "Status: Disabled"); + update_ble_status_item(ble_status_item, enabled); } else if (ev_id == BTN_BLE_RESET_PAIRING) { handle_ble_reset(); } else if (ev_id == BTN_BLE_HELP) { @@ -2365,6 +2426,10 @@ static void display_screen(jade_process_t* process, gui_activity_t* act) // Refeed sensor entropy every time we return to dashboard screen const TickType_t tick_count = xTaskGetTickCount(); refeed_entropy(&tick_count, sizeof(tick_count)); + + // Ensure the correct/expected connection interface(s) are enabled + // depending on whether the user is authenticated and on which interface. + enable_connection_interfaces(keychain_get_userdata()); } // Display the dashboard ready or welcome screen. Await messages or user GUI input. diff --git a/test_jade.py b/test_jade.py index e73cd470..dc5b2f3a 100644 --- a/test_jade.py +++ b/test_jade.py @@ -3394,6 +3394,9 @@ def run_api_tests(jadeapi, isble, qemu, authuser=False): check_frag = has_psram and not has_ble check_mem_stats(startinfo, endinfo, check_frag=check_frag) + rslt = jadeapi.clean_reset() + assert rslt is True + # Run tests using passed interface def run_interface_tests(jadeapi, @@ -3434,9 +3437,11 @@ def run_interface_tests(jadeapi, # Sanity check selfcheck time on Jade hw (skip for qemu) # May need updating if more tests added to selfcheck.c - time_ms = jadeapi.run_remote_selfcheck() - logger.info('selfcheck time: ' + str(time_ms) + 'ms') - assert qemu or time_ms < 82500 + # Don't need to run internal tests more than once (so skip for ble) + if not isble: + time_ms = jadeapi.run_remote_selfcheck() + logger.info('selfcheck time: ' + str(time_ms) + 'ms') + assert qemu or time_ms < 82500 # Test good pinserver handshake, and also 'bad sig' pinserver test_handshake(jadeapi.jade) @@ -3484,6 +3489,9 @@ def run_interface_tests(jadeapi, check_frag = has_psram and not has_ble check_mem_stats(startinfo, endinfo, check_frag=check_frag) + rslt = jadeapi.clean_reset() + assert rslt is True + # Run all selected tests over a passed JadeAPI instance. def run_jade_tests(jadeapi, args, isble): @@ -3498,44 +3506,83 @@ def run_jade_tests(jadeapi, args, isble): run_api_tests(jadeapi, isble, args.qemu, authuser=args.authuser) -# This test should be passed 2 different connections to the same jade hw -# - both serial and ble connected at the same time. -# Test that is we auth over one, we can't do sensitive calls over the other. -def mixed_sources_test(jade1, jade2): +# This test should be passed 2 different connection details to the same jade hw +# Test that is we auth over one, we can't connect with the other. +def mixed_sources_test(serialport, bleid): + logger.info("Running 'mixed sources' Tests") + SRTIMEOUT = 3 # Use a short timeout + SCAN_TIMEOUT = 6 # Example of a 'sensitve' call path, network, expected = GET_XPUB_DATA[0] - # jade1 can unlock jade, then get xpub fine - jade1.set_mnemonic(TEST_MNEMONIC) - rslt = jade1.get_xpub(network, path) - assert rslt == expected + # 1. Authorise over serial, check BLE connection fails + with JadeAPI.create_serial(serialport, timeout=SRTIMEOUT) as jade_serial: + jade_serial.set_mnemonic(TEST_MNEMONIC) + time.sleep(1) - # jade2 gets an error about jade being locked (for them) - try: - rslt = jade2.get_xpub(network, path) - assert False, "Excepted exception from mixed sources test" - except JadeError as err: - assert err.code == JadeError.HW_LOCKED + rslt = jade_serial.get_xpub(network, path) + assert rslt == expected - # jade1 is still fine - rslt = jade1.get_xpub(network, path) - assert rslt == expected + try: + # With BLE we shouldn't even be able to scan/connect the device + with JadeAPI.create_ble(serial_number=bleid, scan_timeout=SCAN_TIMEOUT) as jade_ble: + assert False, "Expected BLE connection to fail as authorised over serial!" - # Now jade2 unlocks jade - they can get xpub but jade1 now cannot - jade2.set_mnemonic(TEST_MNEMONIC) - rslt = jade2.get_xpub(network, path) - assert rslt == expected + except JadeError as err: + assert err.code == 1 and "Unable to locate BLE device" in err.message - try: - rslt = jade1.get_xpub(network, path) - assert False, "Expected exception from mixed sources test" - except JadeError as err: - assert err.code == JadeError.HW_LOCKED + rslt = jade_serial.logout() + assert rslt is True + + # 2. Authorise over BLE, check serial connection can be made, but is limited + with JadeAPI.create_ble(serial_number=bleid) as jade_ble: + rslt = jade_ble.set_mnemonic(TEST_MNEMONIC) + assert rslt - # jade2 is still fine - rslt = jade2.get_xpub(network, path) - assert rslt == expected + rslt = jade_ble.get_xpub(network, path) + assert rslt == expected + + # With serial it can connect but can't make any 'wallet-sensitive' calls + # expect 'HW_LOCKED' error over un-authenticated connection + with JadeAPI.create_serial(serialport, timeout=SRTIMEOUT) as jade_serial: + # Can, for example, get info + info = jade_serial.get_version_info() + assert len(info) == NUM_VALUES_VERINFO + + try: + rslt = jade_serial.get_xpub(network, path) + assert False, "Expected exception from mixed sources test" + except JadeError as err: + assert err.code == JadeError.HW_LOCKED + + # Can unlock with serial, but that will break BLE connection + rslt = jade_serial.set_mnemonic(TEST_MNEMONIC) + assert rslt + + rslt = jade_serial.get_xpub(network, path) + assert rslt == expected + + # BLE connection should now be broken + try: + rslt = jade_ble.get_xpub(network, path) + assert False, "Expected exception from mixed sources test" + except AssertionError as err: + assert jade_ble.jade.impl.client is None + + rslt = jade_serial.logout() + assert rslt is True + + # 3. Check BLE re-enabled, and both interfces can be used when user not authenticated + time.sleep(1) + with JadeAPI.create_serial(serialport, timeout=SRTIMEOUT) as jade_serial: + info1 = jade_serial.get_version_info() + with JadeAPI.create_ble(serial_number=bleid) as jade_ble: + info2 = jade_ble.get_version_info() + info1 = jade_serial.get_version_info() + + rslt = jade_serial.clean_reset() + assert rslt is True # Run all selected tests over all selected backends (serial/ble) @@ -3557,12 +3604,9 @@ def run_all_jade_tests(info, args): with JadeAPI.create_ble(serial_number=bleid) as jade: run_jade_tests(jade, args, isble=True) - # 3. If also testing over serial, run the 'mixed sources' tests - if not args.skipserial: - logger.info("Running 'mixed sources' Tests") - with JadeAPI.create_serial(args.serialport, - timeout=args.serialtimeout) as jadeserial: - mixed_sources_test(jadeserial, jade) + # 3. If testing both interfaces, test cannot connect 'other' when one in use + if not args.skipserial: + mixed_sources_test(args.serialport, bleid) else: msg = "Skipping BLE tests - not enabled on the hardware" logger.warning(msg)