From 65a1ecf0599efc004e38d146fdd18c5bf8d1155e Mon Sep 17 00:00:00 2001 From: Ajinkya Rajandekar <145996984+ajinkyaraj-23@users.noreply.github.com> Date: Tue, 16 Jul 2024 15:39:22 +0100 Subject: [PATCH] Change blindsign flow according to guideline. Remove blindsign button from stax --- app/src/apdu_sign.c | 10 ++- app/src/globals.h | 5 +- app/src/ui_commons.h | 9 +++ app/src/ui_home_nbgl.c | 17 +---- app/src/ui_stream_nbgl.c | 64 ++++++++++--------- tests/integration/touch/test_basic.py | 6 -- .../touch/test_blindsign_too_deep.py | 9 ++- 7 files changed, 56 insertions(+), 64 deletions(-) diff --git a/app/src/apdu_sign.c b/app/src/apdu_sign.c index 501c1f219..6c5842856 100644 --- a/app/src/apdu_sign.c +++ b/app/src/apdu_sign.c @@ -633,7 +633,7 @@ reviewChoice(bool confirm) if (confirm) { nbgl_useCaseStatus("TRANSACTION\nSIGNED", true, accept_blindsign_cb); } else { - tz_reject_ui(); + tz_reject(); } FUNC_LEAVE(); @@ -665,6 +665,8 @@ void continue_blindsign_cb(void) { FUNC_ENTER(("void")); + nbgl_operationType_t op = TYPE_TRANSACTION; + op |= BLIND_OPERATION; useCaseTagValueList.pairs = NULL; useCaseTagValueList.callback = getTagValuePair; @@ -672,11 +674,7 @@ continue_blindsign_cb(void) useCaseTagValueList.nbPairs = 2; useCaseTagValueList.smallCaseForValue = false; useCaseTagValueList.wrapping = false; - infoLongPress.icon = &C_tezos; - infoLongPress.text = "Sign transaction?"; - infoLongPress.longPressText = "Hold to sign"; - nbgl_useCaseStaticReview(&useCaseTagValueList, &infoLongPress, - "Reject transaction", reviewChoice); + nbgl_useCaseReview(op, &useCaseTagValueList, &C_tezos, REVIEW("Transaction"),NULL, SIGN("Transaction"), reviewChoice); FUNC_LEAVE(); } diff --git a/app/src/globals.h b/app/src/globals.h index 7d4b0846f..e7f68e517 100644 --- a/app/src/globals.h +++ b/app/src/globals.h @@ -20,7 +20,6 @@ limitations under the License. */ #pragma once - #include #include #include @@ -116,6 +115,10 @@ typedef struct { bagl_element_t bagls[5 + TZ_SCREEN_LINES_11PX]; } ux; /// Config for history screens for nano devices. #endif + +#ifdef HAVE_NBGL + char error_code[10]; /// Error codes to be displayed in blindsigning. +#endif } globals_t; /* Settings */ diff --git a/app/src/ui_commons.h b/app/src/ui_commons.h index 72d446c1e..9912a1d73 100644 --- a/app/src/ui_commons.h +++ b/app/src/ui_commons.h @@ -41,3 +41,12 @@ #define REGULAR BAGL_FONT_OPEN_SANS_REGULAR_11px | BAGL_FONT_ALIGNMENT_CENTER #define BOLD BAGL_FONT_OPEN_SANS_EXTRABOLD_11px | BAGL_FONT_ALIGNMENT_CENTER #endif // HAVE_BAGL + +#define SIGN_BUTTON "Hold to sign" +#define REJECT_BUTTON "Reject" +#define SIGN(msg) "Sign " msg "?" +#define REVIEW(msg) "Review " msg +#define REJECT(msg) "Reject " msg +#define REJECT_QUESTION(msg) REJECT(msg) "?" +#define REJECT_CONFIRM_BUTTON "Yes, reject" +#define RESUME(msg) "Go back to " msg \ No newline at end of file diff --git a/app/src/ui_home_nbgl.c b/app/src/ui_home_nbgl.c index c4e96c1a5..cf718b653 100644 --- a/app/src/ui_home_nbgl.c +++ b/app/src/ui_home_nbgl.c @@ -40,11 +40,9 @@ static const char *const infoContents[] enum { EXPERT_MODE_TOKEN = FIRST_USER_TOKEN, - BLIND_SIGNING_TOKEN }; enum { EXPERT_MODE_TOKEN_ID = 0, - BLIND_SIGNING_TOKEN_ID, SETTINGS_SWITCHES_NB }; @@ -59,12 +57,7 @@ controls_callback(int token, __attribute__((unused)) uint8_t index, __attribute__((unused)) int page) { uint8_t switch_value; - if (token == BLIND_SIGNING_TOKEN) { - switch_value = !N_settings.blindsigning; - toggle_blindsigning(); - switches[BLIND_SIGNING_TOKEN_ID].initState - = (nbgl_state_t)(switch_value); - } else if (token == EXPERT_MODE_TOKEN) { + if (token == EXPERT_MODE_TOKEN) { switch_value = !N_settings.expert_mode; toggle_expert_mode(); switches[EXPERT_MODE_TOKEN_ID].initState @@ -96,14 +89,6 @@ initSettings(void) switches[EXPERT_MODE_TOKEN_ID].subText = "Enable expert mode signing"; switches[EXPERT_MODE_TOKEN_ID].token = EXPERT_MODE_TOKEN; switches[EXPERT_MODE_TOKEN_ID].tuneId = TUNE_TAP_CASUAL; - - switches[BLIND_SIGNING_TOKEN_ID].initState - = (nbgl_state_t)(N_settings.blindsigning); - switches[BLIND_SIGNING_TOKEN_ID].text = "Blind signing"; - switches[BLIND_SIGNING_TOKEN_ID].subText - = "Enable transaction blind signing"; - switches[BLIND_SIGNING_TOKEN_ID].token = BLIND_SIGNING_TOKEN; - switches[BLIND_SIGNING_TOKEN_ID].tuneId = TUNE_TAP_CASUAL; } void diff --git a/app/src/ui_stream_nbgl.c b/app/src/ui_stream_nbgl.c index faee5e741..882780ba1 100644 --- a/app/src/ui_stream_nbgl.c +++ b/app/src/ui_stream_nbgl.c @@ -89,31 +89,38 @@ start_blindsign(void) FUNC_LEAVE(); } -static void -blindsign_splash(void) -{ +static void blindsign_choice(bool confirm) { TZ_PREAMBLE(("void")); - nbgl_useCaseReviewStart( - &C_Important_Circle_64px, "Blind signing", - "This transaction can not be securely interpreted by Ledger Stax. It " - "might put your assets at risk.", - "Reject transaction", start_blindsign, tz_reject_ui); - + if (confirm) { + start_blindsign(); + } else { + tz_reject_ui(); + } TZ_POSTAMBLE; } -static void -handle_blindsigning(bool confirm) -{ +static void blindsign_splash(bool confirm) { TZ_PREAMBLE(("void")); if (confirm) { - if (!N_settings.blindsigning) { - toggle_blindsigning(); - } - nbgl_useCaseStatus("BLIND SIGNING\nENABLED", true, blindsign_splash); - } else { tz_reject_ui(); + } else { + const char *blindsign_msg = + "Your Ledger cannot decode this transaction. If you sign it, you could be authorizing " + "malicious actions that can drain your wallet.\n"; + const char *learn_more_msg = '\n Learn More: tinyurl.com/ledger-Tezos'; + char total_blindsign_msg[150]= {'\0'}; + memcpy(total_blindsign_msg, blindsign_msg, sizeof(blindsign_msg)); + memcpy(total_blindsign_msg + sizeof(blindsign_msg), global.error_code, sizeof(global.error_code) ); + memcpy(total_blindsign_msg + sizeof(blindsign_msg) + sizeof(global.error_code), learn_more_msg, sizeof(learn_more_msg)); + nbgl_useCaseChoice( + &C_Important_Circle_64px, + "The transaction cannot be trusted", + total_blindsign_msg, + "I accept the risk", + "Reject transaction", + blindsign_choice); } + TZ_POSTAMBLE; } @@ -122,22 +129,19 @@ switch_to_blindsigning(__attribute__((unused)) const char *err_type, const char *err_code) { TZ_PREAMBLE(("void")); - PRINTF("[DEBUG] refill_error: global.step = %d\n", global.step); + PRINTF("[DEBUG] refill_error: global.step = %d\n %s", global.step, err_code); TZ_ASSERT(EXC_UNEXPECTED_STATE, global.step == ST_CLEAR_SIGN); global.keys.apdu.sign.step = SIGN_ST_WAIT_USER_INPUT; global.step = ST_BLIND_SIGN; - if (N_settings.blindsigning) { - nbgl_useCaseReviewStart(&C_Important_Circle_64px, - "Blind signing required:\nParsing Error", - err_code, "Reject transaction", - blindsign_splash, tz_reject_ui); - } else { - nbgl_useCaseChoice(&C_Important_Circle_64px, - "Enable blind signing to authorize this " - "transaction:\nParsing Error", - err_code, "Enable blind signing", - "Reject transaction", handle_blindsigning); - } + memcpy(global.error_code, err_code, sizeof(global.error_code)); + + nbgl_useCaseChoice( + &C_Important_Circle_64px, + "Security risk detected", + "It may not be safe to sign this transaction. To continue, you'll need to review the risk.", + "Back to safety", + "Review risk", + blindsign_splash); TZ_POSTAMBLE; } diff --git a/tests/integration/touch/test_basic.py b/tests/integration/touch/test_basic.py index d7c0648ef..43e215c43 100755 --- a/tests/integration/touch/test_basic.py +++ b/tests/integration/touch/test_basic.py @@ -23,13 +23,7 @@ app.welcome.settings() app.assert_settings() - app.settings.toggle_blindsigning() - app.assert_settings(blindsigning=True) - app.settings.toggle_expert_mode() - app.assert_settings(blindsigning=True, expert_mode=True) - - app.settings.toggle_blindsigning() app.assert_settings(expert_mode=True) app.settings.toggle_expert_mode() diff --git a/tests/integration/touch/test_blindsign_too_deep.py b/tests/integration/touch/test_blindsign_too_deep.py index c65c7e032..23083b484 100755 --- a/tests/integration/touch/test_blindsign_too_deep.py +++ b/tests/integration/touch/test_blindsign_too_deep.py @@ -31,11 +31,10 @@ app.assert_screen("tbtd_review_0") app.review.next() - app.assert_screen("too_deep_enable_blindsign") - with app.fading_screen("blindsign_enabled"): - app.review.enable_blindsign.confirm() - - app.assert_screen("blindsign_warning") + app.assert_screen("unsafe_operation_warning_1") + app.review.next() + app.assert_screen("unsafe_operation_warning_2") + app.review.next() with app.fading_screen("loading_operation"): app.review.next() app.send_apdu("800f82001211020000000c02000000070200000002002a")