From 5a30eacbbe5bad601bc6d2fe1d6ade33f8f98421 Mon Sep 17 00:00:00 2001 From: "Jamie C. Driver" Date: Thu, 25 Jan 2024 17:12:03 +0000 Subject: [PATCH] qrmode pin: change pin when logged in using QR codes Note we suppress the mid-flow pin-change confirmation in the qr case --- main/button_events.h | 1 + main/process/auth_user.c | 49 ++++++++++++++++------------- main/process/dashboard.c | 67 ++++++++++++++++++++++++++++++++++++---- main/qrmode.c | 22 +++++++------ main/qrmode.h | 2 +- main/ui/dashboard.c | 9 ++++-- 6 files changed, 110 insertions(+), 40 deletions(-) diff --git a/main/button_events.h b/main/button_events.h index 0b152680..5d9ea708 100644 --- a/main/button_events.h +++ b/main/button_events.h @@ -166,6 +166,7 @@ typedef enum { BTN_SETTINGS_OTP_EXIT, BTN_SETTINGS_BLE, BTN_SETTINGS_CHANGE_PIN, + BTN_SETTINGS_CHANGE_PIN_QR, BTN_SETTINGS_RESET, BTN_SETTINGS_EXIT, diff --git a/main/process/auth_user.c b/main/process/auth_user.c index 34324096..36db9a9c 100644 --- a/main/process/auth_user.c +++ b/main/process/auth_user.c @@ -169,7 +169,7 @@ static bool set_pin_get_aeskey(jade_process_t* process, const char* title, uint8 return pinclient_set(process, pin, pin_len, aeskey, aes_len); } -static bool get_pin_load_keys(jade_process_t* process) +static bool get_pin_load_keys(jade_process_t* process, const bool suppress_pin_change_confirmation) { JADE_ASSERT(process); @@ -185,7 +185,8 @@ static bool get_pin_load_keys(jade_process_t* process) SENSITIVE_PUSH(aeskey, sizeof(aeskey)); // Do the pinserver 'getpin' process - if (!get_pin_get_aeskey(process, "Unlock Jade", pin, sizeof(pin), aeskey, sizeof(aeskey))) { + const char* unlock_pin_msg = suppress_pin_change_confirmation ? "Enter Current PIN" : "Unlock Jade"; + if (!get_pin_get_aeskey(process, unlock_pin_msg, pin, sizeof(pin), aeskey, sizeof(aeskey))) { // Server or networking/connection error // NOTE: reply message will have already been sent JADE_LOGE("Failed to get aeskey from pinserver"); @@ -238,31 +239,33 @@ static bool get_pin_load_keys(jade_process_t* process) rslt = true; // Optionally change PIN - const char* question[] = { "Do you want to", "change your PIN?" }; - if (change_pin_requested && await_yesno_activity("Change PIN", question, 2, true, NULL)) { - uint8_t aeskey_new[AES_KEY_LEN_256]; - SENSITIVE_PUSH(aeskey_new, sizeof(aeskey_new)); - - if (set_pin_get_aeskey(process, "Enter New PIN", pin, sizeof(pin), aeskey_new, sizeof(aeskey_new))) { - JADE_LOGI("PIN changed on server"); - if (keychain_reencrypt(aeskey, sizeof(aeskey), aeskey_new, sizeof(aeskey_new))) { - const char* message[] = { "PIN changed" }; - await_message_activity(message, 1); + if (change_pin_requested) { + const char* question[] = { "Do you want to", "change your PIN?" }; + if (suppress_pin_change_confirmation || await_yesno_activity("Change PIN", question, 2, true, NULL)) { + uint8_t aeskey_new[AES_KEY_LEN_256]; + SENSITIVE_PUSH(aeskey_new, sizeof(aeskey_new)); + + if (set_pin_get_aeskey(process, "Enter New PIN", pin, sizeof(pin), aeskey_new, sizeof(aeskey_new))) { + JADE_LOGI("PIN changed on server"); + if (keychain_reencrypt(aeskey, sizeof(aeskey), aeskey_new, sizeof(aeskey_new))) { + const char* message[] = { "PIN changed" }; + await_message_activity(message, 1); + } else { + JADE_LOGE("Failed to re-encrypt with changed PIN data"); + const char* message[] = { "Failed to re-encypt key data!" }; + await_error_activity(message, 1); + } } else { - JADE_LOGE("Failed to re-encrypt with changed PIN data"); - const char* message[] = { "Failed to re-encypt key data!" }; + JADE_LOGW("Abandoned change-PIN"); + const char* message[] = { "Change-PIN abandoned" }; await_error_activity(message, 1); } - } else { - JADE_LOGW("Abandoned change-PIN"); - const char* message[] = { "Change-PIN abandoned" }; - await_error_activity(message, 1); + SENSITIVE_POP(aeskey_new); } - SENSITIVE_POP(aeskey_new); } // All good - set_request_change_pin(false); + set_request_change_pin(false); // clear flag jade_process_reply_to_message_ok(process); JADE_LOGI("Success"); @@ -351,6 +354,10 @@ void auth_user_process(void* process_ptr) } } + // Optional flag to suppress user confirmation of any pin change + bool suppress_pin_change_confirmation = false; + rpc_get_boolean("suppress_pin_change_confirmation", ¶ms, &suppress_pin_change_confirmation); + // We have five cases: // 1. Temporary - has a temporary keys in memory // - nothing to do here, just return ok (having checked message source) @@ -396,7 +403,7 @@ void auth_user_process(void* process_ptr) if (keychain_get()) { keychain_clear(); } - rslt = get_pin_load_keys(process); + rslt = get_pin_load_keys(process, suppress_pin_change_confirmation); } } else { // Jade hw is not fully initialised - if we have an 'in-memory' mnemonic diff --git a/main/process/dashboard.c b/main/process/dashboard.c index e596528b..a052ab57 100644 --- a/main/process/dashboard.c +++ b/main/process/dashboard.c @@ -41,6 +41,7 @@ static inline void ble_start(void) { JADE_ASSERT(false); } // Whether during initialisation we select USB, BLE QR etc. static jade_msg_source_t initialisation_source = SOURCE_NONE; +static jade_msg_source_t internal_relogin_source = SOURCE_NONE; static bool show_connect_screen = false; // The dynamic home screen menu @@ -159,7 +160,7 @@ gui_activity_t* make_unlocked_settings_activity(void); gui_activity_t* make_wallet_settings_activity(void); gui_activity_t* make_device_settings_activity(void); -gui_activity_t* make_authentication_activity(void); +gui_activity_t* make_authentication_activity(bool initialised_and_pin_unlocked); gui_activity_t* make_prefs_settings_activity(bool initialised_and_locked); gui_activity_t* make_display_settings_activity(void); gui_activity_t* make_info_activity(const char* fw_version); @@ -518,10 +519,26 @@ static void dispatch_message(jade_process_t* process) cleanup_jade_process(&task_process); // When the authentication process exits clear any initialisation-source. - // Also set the 'connect screen' flag if it looks like the auth failed. + // Also set the 'connect screen' flag if it looks like the auth failed, + // and handle any relogin data that may have been cached. if (task_function == auth_user_process) { show_connect_screen = (keychain_get() && keychain_get_userdata() == SOURCE_NONE); initialisation_source = SOURCE_NONE; + + if (internal_relogin_source != SOURCE_NONE) { + if (keychain_has_temporary() || !keychain_has_pin() || keychain_get_userdata() != SOURCE_INTERNAL) { + // If pin-wallet wiped (eg bad pins) or a temporary login or non-internal + // login made, wipe the internal-relogin data as it no longer applies. + internal_relogin_source = SOURCE_NONE; + } else if (keychain_get()) { + // On successful login, re-instate original login source + keychain_set(keychain_get(), internal_relogin_source, false); + internal_relogin_source = SOURCE_NONE; + } + // else bad-pin (no wallet loaded) but still have tries remaining. + // Retain re-login data until successful login or ultimate failure + // when encrypted pin-protected wallet is wiped completely. + } } } } @@ -582,9 +599,8 @@ static void offer_jade_reset(void) } // Offer to communicate with pinserver via QRs -static bool auth_qr_mode(void) +static bool auth_qr_mode_ex(const bool suppress_pin_change_confirmation) { - // Temporary login via QR - just set the message source if (keychain_has_temporary()) { keychain_set(keychain_get(), SOURCE_INTERNAL, true); @@ -606,10 +622,18 @@ static bool auth_qr_mode(void) // Start pinserver/qr handshake process initialisation_source = SOURCE_INTERNAL; show_connect_screen = true; - handle_qr_auth(); + handle_qr_auth(suppress_pin_change_confirmation); return true; } +// Offer to communicate with pinserver via QRs +static bool auth_qr_mode(void) +{ + // Standard/normal authentication using QR codes + const bool suppress_pin_change_confirmation = false; + return auth_qr_mode_ex(suppress_pin_change_confirmation); +} + // Unlock jade using qr-codes to effect communication with the pinserver static bool offer_pinserver_qr_unlock(void) { @@ -878,6 +902,30 @@ static void handle_change_pin(void) set_request_change_pin(change_pin); } +static bool handle_change_pin_qr(void) +{ + // Request/start a qr unlock + // NOTE: we suppress the pin-change confirmation mid-handling, + // as we ask up-front before the process is initiated. + const bool suppress_pin_change_confirmation = true; + const char* message[] = { "Do you want to", "change your PIN now", "using QR codes?" }; + if (!await_yesno_activity("Change PIN", message, 3, true, NULL) + || !auth_qr_mode_ex(suppress_pin_change_confirmation)) { + return false; + } + + // Cache the existing 'source' userdata so it can be re-instated after a + // successful pin-change and re-login (otherwise we'd be left in QR mode) + internal_relogin_source = keychain_get_userdata(); + + // Set flag to change pin on next successful auth/unlock and log out of + // the current session (note the qr unlock has already been initiated). + // Return to main loop to handle the qr unlock + set_request_change_pin(true); + keychain_clear(); + return true; +} + // Helper to delete a wallet registration record after user confirms static bool offer_delete_registered_wallet(const char* name, const bool is_multisig) { @@ -1882,6 +1930,9 @@ static void handle_settings(const bool startup_menu) // hw uninitialised and not unlocked/no wallet loaded (ie. no temporary signer) const bool hw_locked_uninitialised = !keychain_get() && !keychain_has_pin(); + // hw initialised and that wallet has been unlocked with PIN (ie not a temporary signer) + const bool hw_pin_unlocked = keychain_get() && keychain_has_pin() && !keychain_has_temporary(); + // NOTE: menu navigation frees prior screens, as the navigation is // potentially unbound with all the back and forward buttons. bool done = false; @@ -1946,7 +1997,7 @@ static void handle_settings(const bool startup_menu) case BTN_SETTINGS_AUTHENTICATION: case BTN_SETTINGS_OTP_EXIT: // Change to 'Authentication' menu - act = make_authentication_activity(); + act = make_authentication_activity(hw_pin_unlocked); break; case BTN_SETTINGS_OTP: @@ -1994,6 +2045,10 @@ static void handle_settings(const bool startup_menu) handle_change_pin(); break; + case BTN_SETTINGS_CHANGE_PIN_QR: + done = handle_change_pin_qr(); + break; + // NOTE: Only Jade v1.1's have brightness controls #ifdef CONFIG_BOARD_TYPE_JADE_V1_1 case BTN_SETTINGS_DISPLAY_BRIGHTNESS: diff --git a/main/qrmode.c b/main/qrmode.c index f7a20de2..e5db9bc5 100644 --- a/main/qrmode.c +++ b/main/qrmode.c @@ -1513,9 +1513,9 @@ static bool post_cancel_message(const jade_msg_source_t source) } // Locally create and post an 'auth_user' request -static bool post_auth_msg_request(const jade_msg_source_t source) +static bool post_auth_msg_request(const jade_msg_source_t source, const bool suppress_pin_change_confirmation) { - uint8_t cbor_buf[64]; + uint8_t cbor_buf[96]; CborEncoder root_encoder; cbor_encoder_init(&root_encoder, cbor_buf, sizeof(cbor_buf), 0); @@ -1530,11 +1530,12 @@ static bool post_auth_msg_request(const jade_msg_source_t source) JADE_ASSERT(cberr == CborNoError); CborEncoder params_encoder; // network - cberr = cbor_encoder_create_map(&root_map_encoder, ¶ms_encoder, 1); + cberr = cbor_encoder_create_map(&root_map_encoder, ¶ms_encoder, 2); JADE_ASSERT(cberr == CborNoError); const network_type_t restriction = keychain_get_network_type_restriction(); const char* networks = restriction == NETWORK_TYPE_TEST ? "testnet" : "mainnet"; add_string_to_map(¶ms_encoder, "network", networks); + add_boolean_to_map(¶ms_encoder, "suppress_pin_change_confirmation", suppress_pin_change_confirmation); cberr = cbor_encoder_close_container(&root_map_encoder, ¶ms_encoder); JADE_ASSERT(cberr == CborNoError); @@ -1662,9 +1663,12 @@ static bool handle_outbound_reply(outbound_message_writer_fn_t handler) } // This task is run to act as a client to Jade's normal 'auth-user' processing -static void auth_qr_client_task(void* unused) +static void auth_qr_client_task(void* ctx) { - JADE_LOGI("Starting Auth QR client task: %d", xPortGetFreeHeapSize()); + JADE_LOGI("Starting Auth QR client task: %d with ctx %p", xPortGetFreeHeapSize(), ctx); + + // NOTE: we just use ctx being NULL or not to convey boolean + const bool suppress_pin_change_confirmation = (ctx != NULL); // Only needed/expected for 'full' inititialistion with pinserver JADE_ASSERT(!keychain_has_temporary()); @@ -1676,7 +1680,7 @@ static void auth_qr_client_task(void* unused) // Post in a synthesized 'auth_user' message JADE_LOGI("Posting initial auth_user message"); - if (!post_auth_msg_request(SOURCE_INTERNAL)) { + if (!post_auth_msg_request(SOURCE_INTERNAL, suppress_pin_change_confirmation)) { JADE_LOGW("Failed to post initial auth_user message"); goto cleanup; } @@ -1708,15 +1712,15 @@ static void auth_qr_client_task(void* unused) vTaskDelete(NULL); } -void handle_qr_auth(void) +void handle_qr_auth(const bool suppress_pin_change_confirmation) { // Only needed/expected for 'full' inititialistion with pinserver JADE_ASSERT(!keychain_has_temporary()); // Start a task to run the qr client side TaskHandle_t auth_qr_client_task_handle; - const BaseType_t retval = xTaskCreatePinnedToCore(&auth_qr_client_task, "auth_qr_client_task", 4 * 1024, NULL, - JADE_TASK_PRIO_GUI, &auth_qr_client_task_handle, JADE_CORE_SECONDARY); + const BaseType_t retval = xTaskCreatePinnedToCore(&auth_qr_client_task, "auth_qr_client_task", 4 * 1024, + (void*)suppress_pin_change_confirmation, JADE_TASK_PRIO_GUI, &auth_qr_client_task_handle, JADE_CORE_SECONDARY); JADE_ASSERT_MSG( retval == pdPASS, "Failed to create auth_qr_client_task, xTaskCreatePinnedToCore() returned %d", retval); diff --git a/main/qrmode.h b/main/qrmode.h index e7dee62b..936993b5 100644 --- a/main/qrmode.h +++ b/main/qrmode.h @@ -29,6 +29,6 @@ bool await_qr_back_continue_activity( const char* message[], size_t message_size, const char* url, bool default_selection); // Start pinserver authentication via qr codes -void handle_qr_auth(void); +void handle_qr_auth(bool suppress_pin_change_confirmation); #endif /* QRMODE_H_ */ diff --git a/main/ui/dashboard.c b/main/ui/dashboard.c index 1515d7b3..a380a87f 100644 --- a/main/ui/dashboard.c +++ b/main/ui/dashboard.c @@ -361,15 +361,18 @@ gui_activity_t* make_display_settings_activity(void) return make_menu_activity("Display", hdrbtns, 2, menubtns, num_menubtns); } -gui_activity_t* make_authentication_activity(void) +gui_activity_t* make_authentication_activity(const bool initialised_and_pin_unlocked) { btn_data_t hdrbtns[] = { { .txt = "=", .font = JADE_SYMBOLS_16x16_FONT, .ev_id = BTN_SETTINGS_AUTHENTICATION_EXIT }, { .txt = NULL, .font = GUI_DEFAULT_FONT, .ev_id = GUI_BUTTON_EVENT_NONE } }; btn_data_t menubtns[] = { { .txt = "Duress PIN", .font = GUI_DEFAULT_FONT, .ev_id = BTN_SETTINGS_WALLET_ERASE_PIN }, - { .txt = "OTP", .font = GUI_DEFAULT_FONT, .ev_id = BTN_SETTINGS_OTP } }; + { .txt = "OTP", .font = GUI_DEFAULT_FONT, .ev_id = BTN_SETTINGS_OTP }, + { .txt = "Change PIN (QR)", .font = GUI_DEFAULT_FONT, .ev_id = BTN_SETTINGS_CHANGE_PIN_QR } }; - return make_menu_activity("Authentication", hdrbtns, 2, menubtns, 2); + const size_t num_btns = initialised_and_pin_unlocked ? 3 : 2; + + return make_menu_activity("Authentication", hdrbtns, 2, menubtns, num_btns); } gui_activity_t* make_otp_activity(void)