Merge pull request #628 from LedgerHQ/fix/apa/blind_signing_app_policies

Rework blind signing policies

Author: apaillier-ledger

CHANGELOG.md

@@ -5,6 +5,17 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/)
 and this project adheres to [Semantic Versioning](http://semver.org/).
 
+## [1.11.2](https://github.com/ledgerhq/app-ethereum/compare/1.11.1...1.11.2) - 2024-08-13
+
+### Added
+
+- Blind-signing setting
+
+### Changed
+
+- Simplified blind-signing warnings on Flex & Stax
+- Restored blind-signing warning screen from < 1.11.0 on Nano devices
+
 ## [1.11.1](https://github.com/ledgerhq/app-ethereum/compare/1.11.0...1.11.1) - 2024-07-26
 
 ### Fixed
@@ -38,6 +49,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Ledger Flex support
 
 ### Removed
+
 - (clone) Flare
 - (clone) Flare Coston
 - (clone) Eth Goerli

Makefile

@@ -39,7 +39,7 @@ include ./makefile_conf/chain/$(CHAIN).mk
 
 APPVERSION_M = 1
 APPVERSION_N = 11
-APPVERSION_P = 1
+APPVERSION_P = 2
 APPVERSION = $(APPVERSION_M).$(APPVERSION_N).$(APPVERSION_P)
 
 # Application source files
@@ -156,7 +156,6 @@ DISABLE_STANDARD_APP_FILES = 1
 ########################################
 
 DEFINES += CHAINID_COINNAME=\"$(TICKER)\" CHAIN_ID=$(CHAIN_ID)
-DEFINES += BUILD_YEAR=\"$(shell date +%Y)\"
 
 # Enabled Features #
 include makefile_conf/features.mk

client/src/ledger_app_clients/ethereum/settings.py

@@ -5,22 +5,25 @@
 
 
 class SettingID(Enum):
+    BLIND_SIGNING = auto()
     VERBOSE_ENS = auto()
-    VERBOSE_EIP712 = auto()
     NONCE = auto()
+    VERBOSE_EIP712 = auto()
     DEBUG_DATA = auto()
 
 
 def get_device_settings(firmware: Firmware) -> list[SettingID]:
     if firmware == Firmware.NANOS:
         return [
+            SettingID.BLIND_SIGNING,
             SettingID.NONCE,
             SettingID.DEBUG_DATA,
         ]
     return [
+        SettingID.BLIND_SIGNING,
         SettingID.VERBOSE_ENS,
-        SettingID.VERBOSE_EIP712,
         SettingID.NONCE,
+        SettingID.VERBOSE_EIP712,
         SettingID.DEBUG_DATA,
     ]
 

src/common_ui.h

@@ -5,7 +5,8 @@
 #include <stdint.h>
 
 void ui_idle(void);
-void ui_warning_contract_data(void);
+void ui_warning_blind_signing(void);
+void ui_error_blind_signing(void);
 void ui_display_public_eth2(void);
 void ui_display_privacy_public_key(void);
 void ui_display_privacy_shared_secret(void);

src/handle_swap_sign_transaction.c

@@ -95,10 +95,8 @@ void __attribute__((noreturn)) handle_swap_sign_transaction(const chain_config_t
 
     if (N_storage.initialized != 0x01) {
         internalStorage_t storage;
-        storage.contractDetails = 0x00;
-        storage.initialized = 0x01;
-        storage.displayNonce = 0x00;
-        storage.contractDetails = 0x00;
+        explicit_bzero(&storage, sizeof(storage));
+        storage.initialized = true;
         nvm_write((void*) &N_storage, (void*) &storage, sizeof(internalStorage_t));
     }
 

src/main.c

@@ -548,14 +548,7 @@ __attribute__((noreturn)) void coin_main(libargs_t *args) {
 
                 if (!N_storage.initialized) {
                     internalStorage_t storage;
-                    storage.contractDetails = false;
-                    storage.displayNonce = false;
-#ifdef HAVE_EIP712_FULL_SUPPORT
-                    storage.verbose_eip712 = false;
-#endif
-#ifdef HAVE_DOMAIN_NAME
-                    storage.verbose_domain_name = false;
-#endif
+                    explicit_bzero(&storage, sizeof(storage));
                     storage.initialized = true;
                     nvm_write((void *) &N_storage, (void *) &storage, sizeof(internalStorage_t));
                 }

src/shared_context.h

@@ -30,6 +30,7 @@ typedef struct bip32_path_t {
 } bip32_path_t;
 
 typedef struct internalStorage_t {
+    bool dataAllowed;
     bool contractDetails;
     bool displayNonce;
 #ifdef HAVE_EIP712_FULL_SUPPORT

src/ui_callbacks.h

@@ -21,6 +21,4 @@ unsigned int io_seproxyhal_touch_eth2_address_ok(const bagl_element_t *e);
 unsigned int io_seproxyhal_touch_privacy_ok(const bagl_element_t *e);
 unsigned int io_seproxyhal_touch_privacy_cancel(const bagl_element_t *e);
 
-void ui_warning_contract_data(void);
-
 void io_seproxyhal_send_status(uint32_t sw);

src_bagl/common_ui.c

@@ -12,8 +12,12 @@ void ui_idle(void) {
     ux_flow_init(0, ux_idle_flow, NULL);
 }
 
-void ui_warning_contract_data(void) {
-    ux_flow_init(0, ux_blind_signing_flow, NULL);
+void ui_error_blind_signing(void) {
+    ux_flow_init(0, ux_error_blind_signing_flow, NULL);
+}
+
+void ui_warning_blind_signing(void) {
+    ux_flow_init(0, ux_warning_blind_signing_flow, NULL);
 }
 
 void ui_display_public_eth2(void) {

src_bagl/ui_flow.c

@@ -10,22 +10,24 @@
 
 // Reuse the strings.common.fullAmount buffer for settings displaying.
 // No risk of collision as this buffer is unused in the settings menu
-#define SETTING_VERBOSE_DOMAIN_NAME_STATE (strings.common.fullAmount + (BUF_INCREMENT * 0))
-#define SETTING_VERBOSE_EIP712_STATE      (strings.common.fullAmount + (BUF_INCREMENT * 1))
+#define SETTING_BLIND_SIGNING_STATE       (strings.common.fullAmount + (BUF_INCREMENT * 0))
+#define SETTING_VERBOSE_DOMAIN_NAME_STATE (strings.common.fullAmount + (BUF_INCREMENT * 1))
 #define SETTING_DISPLAY_NONCE_STATE       (strings.common.fullAmount + (BUF_INCREMENT * 2))
-#define SETTING_DISPLAY_DATA_STATE        (strings.common.fullAmount + (BUF_INCREMENT * 3))
+#define SETTING_VERBOSE_EIP712_STATE      (strings.common.fullAmount + (BUF_INCREMENT * 3))
+#define SETTING_DISPLAY_DATA_STATE        (strings.common.fullAmount + (BUF_INCREMENT * 4))
 
 #define BOOL_TO_STATE_STR(b) (b ? ENABLED_STR : DISABLED_STR)
 
 static void display_settings(const ux_flow_step_t* const start_step);
+static void switch_settings_blind_signing(void);
+#ifdef HAVE_DOMAIN_NAME
+static void switch_settings_verbose_domain_name(void);
+#endif  // HAVE_DOMAIN_NAME
 static void switch_settings_display_data(void);
 static void switch_settings_display_nonce(void);
 #ifdef HAVE_EIP712_FULL_SUPPORT
 static void switch_settings_verbose_eip712(void);
 #endif  // HAVE_EIP712_FULL_SUPPORT
-#ifdef HAVE_DOMAIN_NAME
-static void switch_settings_verbose_domain_name(void);
-#endif  // HAVE_DOMAIN_NAME
 
 //////////////////////////////////////////////////////////////////////
 // clang-format off
@@ -70,6 +72,26 @@ UX_FLOW(ux_idle_flow,
         FLOW_LOOP);
 
 // clang-format off
+UX_STEP_CB(
+    ux_settings_flow_blind_signing_step,
+#ifdef TARGET_NANOS
+    bnnn_paging,
+#else
+    bnnn,
+#endif
+    switch_settings_blind_signing(),
+    {
+#ifdef TARGET_NANOS
+      .title = "Blind signing",
+      .text =
+#else
+      "Blind signing",
+      "Enables transaction",
+      "blind signing",
+#endif
+      SETTING_BLIND_SIGNING_STATE
+    });
+
 #ifdef HAVE_DOMAIN_NAME
 UX_STEP_CB(
     ux_settings_flow_verbose_domain_name_step,
@@ -83,19 +105,6 @@ UX_STEP_CB(
     });
 #endif // HAVE_DOMAIN_NAME
 
-#ifdef HAVE_EIP712_FULL_SUPPORT
-UX_STEP_CB(
-    ux_settings_flow_verbose_eip712_step,
-    bnnn,
-    switch_settings_verbose_eip712(),
-    {
-      "Raw messages",
-      "Displays raw content",
-      "from EIP712 messages",
-      SETTING_VERBOSE_EIP712_STATE
-    });
-#endif // HAVE_EIP712_FULL_SUPPORT
-
 UX_STEP_CB(
     ux_settings_flow_display_nonce_step,
 #ifdef TARGET_NANOS
@@ -116,6 +125,19 @@ UX_STEP_CB(
       SETTING_DISPLAY_NONCE_STATE
     });
 
+#ifdef HAVE_EIP712_FULL_SUPPORT
+UX_STEP_CB(
+    ux_settings_flow_verbose_eip712_step,
+    bnnn,
+    switch_settings_verbose_eip712(),
+    {
+      "Raw messages",
+      "Displays raw content",
+      "from EIP712 messages",
+      SETTING_VERBOSE_EIP712_STATE
+    });
+#endif // HAVE_EIP712_FULL_SUPPORT
+
 UX_STEP_CB(
     ux_settings_flow_display_data_step,
 #ifdef TARGET_NANOS
@@ -147,17 +169,19 @@ UX_STEP_CB(
 // clang-format on
 
 UX_FLOW(ux_settings_flow,
+        &ux_settings_flow_blind_signing_step,
 #ifdef HAVE_DOMAIN_NAME
         &ux_settings_flow_verbose_domain_name_step,
 #endif  // HAVE_DOMAIN_NAME
+        &ux_settings_flow_display_nonce_step,
 #ifdef HAVE_EIP712_FULL_SUPPORT
         &ux_settings_flow_verbose_eip712_step,
 #endif  // HAVE_EIP712_FULL_SUPPORT
-        &ux_settings_flow_display_nonce_step,
         &ux_settings_flow_display_data_step,
         &ux_settings_flow_back_step);
 
 static void display_settings(const ux_flow_step_t* const start_step) {
+    strlcpy(SETTING_BLIND_SIGNING_STATE, BOOL_TO_STATE_STR(N_storage.dataAllowed), BUF_INCREMENT);
     strlcpy(SETTING_DISPLAY_DATA_STATE,
             BOOL_TO_STATE_STR(N_storage.contractDetails),
             BUF_INCREMENT);
@@ -182,6 +206,10 @@ static void toggle_setting(volatile bool* setting, const ux_flow_step_t* ui_step
     display_settings(ui_step);
 }
 
+static void switch_settings_blind_signing(void) {
+    toggle_setting(&N_storage.dataAllowed, &ux_settings_flow_blind_signing_step);
+}
+
 static void switch_settings_display_data(void) {
     toggle_setting(&N_storage.contractDetails, &ux_settings_flow_display_data_step);
 }
@@ -204,76 +232,46 @@ static void switch_settings_verbose_domain_name(void) {
 
 //////////////////////////////////////////////////////////////////////
 // clang-format off
-UX_STEP_NOCB(
-    ux_blind_signing_warning_step,
-    pbb,
-    {
-      &C_icon_warning,
 #ifdef TARGET_NANOS
-      "Transaction",
-      "not trusted",
-#else
-      "This transaction",
-      "cannot be trusted",
-#endif
-    });
-#ifndef TARGET_NANOS
-UX_STEP_NOCB(
-    ux_blind_signing_text1_step,
-    nnnn,
+UX_STEP_CB(
+    ux_error_blind_signing_step,
+    bnnn_paging,
+    ui_idle(),
     {
-      "Your Ledger cannot",
-      "decode this",
-      "transaction. If you",
-      "sign it, you could",
+      "Error",
+      "Blind signing must be enabled in Settings",
     });
-UX_STEP_NOCB(
-    ux_blind_signing_text2_step,
-    nnnn,
+#else
+UX_STEP_CB(
+    ux_error_blind_signing_step,
+    pnn,
+    ui_idle(),
     {
-      "be authorizing",
-      "malicious actions",
-      "that can drain your",
-      "wallet.",
+      &C_icon_crossmark,
+      "Blind signing must be",
+      "enabled in Settings",
     });
 #endif
+
 UX_STEP_NOCB(
-    ux_blind_signing_link_step,
-    nn,
-    {
-      "Learn more:",
-      "ledger.com/e8",
-    });
-UX_STEP_CB(
-    ux_blind_signing_accept_step,
+    ux_warning_blind_signing_warn_step,
     pbb,
-    start_signature_flow(),
     {
-      &C_icon_validate_14,
-#ifdef TARGET_NANOS
-      "Accept risk",
-      "and review",
-#else
-      "Accept risk and",
-      "review transaction",
-#endif
-    });
-UX_STEP_CB(
-    ux_blind_signing_reject_step,
-    pb,
-    report_finalize_error(),
-    {
-      &C_icon_crossmark,
-      "Reject",
+      &C_icon_warning,
+      "Blind",
+      "signing",
     });
+UX_STEP_INIT(
+   ux_warning_blind_signing_jump_step,
+   NULL,
+   NULL,
+   {
+     start_signature_flow();
+   }
+);
 // clang-format on
 
-UX_FLOW(ux_blind_signing_flow,
-        &ux_blind_signing_warning_step,
-#ifndef TARGET_NANOS
-        &ux_blind_signing_text1_step,
-        &ux_blind_signing_text2_step,
-#endif
-        &ux_blind_signing_link_step,
-        &ux_blind_signing_accept_step,
-        &ux_blind_signing_reject_step);
+UX_FLOW(ux_error_blind_signing_flow, &ux_error_blind_signing_step);
+UX_FLOW(ux_warning_blind_signing_flow,
+        &ux_warning_blind_signing_warn_step,
+        &ux_warning_blind_signing_jump_step);