Merge pull request #99 from trilitech/90-add-hwm-onoff-in-settings

Implement HWM on/off behaviour
trilitech · Apr 11, 2024 · a92013c · a92013c
2 parents 33ef9d2 + 846f39c
commit a92013c
Show file tree

Hide file tree

Showing 88 changed files with 346 additions and 195 deletions.
diff --git a/src/apdu_pubkey.c b/src/apdu_pubkey.c
@@ -73,9 +73,8 @@ int handle_get_public_key(buffer_t *cdata,
     TZ_ASSERT_NOT_NULL(cdata);
 
     global.path_with_curve.derivation_type = derivation_type;
-
     if ((cdata->size == 0u) && authorize) {
-        TZ_ASSERT(copy_bip32_path_with_curve(&global.path_with_curve, &N_data.baking_key),
+        TZ_ASSERT(copy_bip32_path_with_curve(&global.path_with_curve, &(g_hwm.baking_key)),
                   EXC_MEMORY_ERROR);
     } else {
         TZ_ASSERT(read_bip32_path(cdata, &global.path_with_curve.bip32_path), EXC_WRONG_VALUES);

diff --git a/src/apdu_query.c b/src/apdu_query.c
@@ -37,19 +37,19 @@ int handle_query_all_hwm(void) {
     uint8_t resp[5u * sizeof(uint32_t)] = {0};
     size_t offset = 0;
 
-    write_u32_be(resp, offset, N_data.hwm.main.highest_level);
+    write_u32_be(resp, offset, g_hwm.hwm.main.highest_level);
     offset += sizeof(uint32_t);
 
-    write_u32_be(resp, offset, N_data.hwm.main.highest_round);
+    write_u32_be(resp, offset, g_hwm.hwm.main.highest_round);
     offset += sizeof(uint32_t);
 
-    write_u32_be(resp, offset, N_data.hwm.test.highest_level);
+    write_u32_be(resp, offset, g_hwm.hwm.test.highest_level);
     offset += sizeof(uint32_t);
 
-    write_u32_be(resp, offset, N_data.hwm.test.highest_round);
+    write_u32_be(resp, offset, g_hwm.hwm.test.highest_round);
     offset += sizeof(uint32_t);
 
-    write_u32_be(resp, offset, N_data.main_chain_id.v);
+    write_u32_be(resp, offset, g_hwm.main_chain_id.v);
     offset += sizeof(uint32_t);
 
     return io_send_response_pointer(resp, offset, SW_OK);
@@ -59,10 +59,10 @@ int handle_query_main_hwm(void) {
     uint8_t resp[2u * sizeof(uint32_t)] = {0};
     size_t offset = 0;
 
-    write_u32_be(resp, offset, N_data.hwm.main.highest_level);
+    write_u32_be(resp, offset, g_hwm.hwm.main.highest_level);
     offset += sizeof(uint32_t);
 
-    write_u32_be(resp, offset, N_data.hwm.main.highest_round);
+    write_u32_be(resp, offset, g_hwm.hwm.main.highest_round);
     offset += sizeof(uint32_t);
 
     return io_send_response_pointer(resp, offset, SW_OK);
@@ -72,13 +72,13 @@ int handle_query_auth_key(void) {
     uint8_t resp[1u + (MAX_BIP32_PATH * sizeof(uint32_t))] = {0};
     size_t offset = 0;
 
-    uint8_t const length = N_data.baking_key.bip32_path.length;
+    uint8_t const length = g_hwm.baking_key.bip32_path.length;
 
     resp[offset] = length;
     offset++;
 
     for (uint8_t i = 0; i < length; ++i) {
-        write_u32_be(resp, offset, N_data.baking_key.bip32_path.components[i]);
+        write_u32_be(resp, offset, g_hwm.baking_key.bip32_path.components[i]);
         offset += sizeof(uint32_t);
     }
 
@@ -89,22 +89,22 @@ int handle_query_auth_key_with_curve(void) {
     uint8_t resp[2u + (MAX_BIP32_PATH * sizeof(uint32_t))] = {0};
     size_t offset = 0;
 
-    uint8_t const length = N_data.baking_key.bip32_path.length;
+    uint8_t const length = g_hwm.baking_key.bip32_path.length;
 
-    int derivation_type = unparse_derivation_type(N_data.baking_key.derivation_type);
+    int derivation_type = unparse_derivation_type(g_hwm.baking_key.derivation_type);
 
     if (derivation_type < 0) {
         return io_send_apdu_err(EXC_REFERENCED_DATA_NOT_FOUND);
     }
 
-    resp[offset] = unparse_derivation_type(N_data.baking_key.derivation_type);
+    resp[offset] = unparse_derivation_type(g_hwm.baking_key.derivation_type);
     offset++;
 
     resp[offset] = length;
     offset++;
 
     for (uint8_t i = 0; i < length; ++i) {
-        write_u32_be(resp, offset, N_data.baking_key.bip32_path.components[i]);
+        write_u32_be(resp, offset, g_hwm.baking_key.bip32_path.components[i]);
         offset += sizeof(uint32_t);
     }
 

diff --git a/src/apdu_reset.c b/src/apdu_reset.c
@@ -36,14 +36,14 @@
  * @return true
  */
 static bool ok(void) {
-    UPDATE_NVRAM(ram, {
-        ram->hwm.main.highest_level = G.reset_level;
-        ram->hwm.main.highest_round = 0;
-        ram->hwm.main.had_attestation = false;
-        ram->hwm.test.highest_level = G.reset_level;
-        ram->hwm.test.highest_round = 0;
-        ram->hwm.test.had_attestation = false;
-    });
+    g_hwm.hwm.main.highest_level = G.reset_level;
+    g_hwm.hwm.main.highest_round = 0;
+    g_hwm.hwm.main.had_attestation = false;
+    g_hwm.hwm.test.highest_level = G.reset_level;
+    g_hwm.hwm.test.highest_round = 0;
+    g_hwm.hwm.test.had_attestation = false;
+
+    UPDATE_NVRAM;
 
     // Send back the response, do not restart the event loop
     io_send_sw(SW_OK);

diff --git a/src/apdu_setup.c b/src/apdu_setup.c
@@ -42,18 +42,18 @@
  * @return true
  */
 static bool ok(void) {
-    UPDATE_NVRAM(ram, {
-        copy_bip32_path_with_curve(&ram->baking_key, &global.path_with_curve);
-        ram->main_chain_id = G.main_chain_id;
-        ram->hwm.main.highest_level = G.hwm.main;
-        ram->hwm.main.highest_round = 0;
-        ram->hwm.main.had_attestation = false;
-        ram->hwm.main.had_preattestation = false;
-        ram->hwm.test.highest_level = G.hwm.test;
-        ram->hwm.test.highest_round = 0;
-        ram->hwm.test.had_attestation = false;
-        ram->hwm.test.had_preattestation = false;
-    });
+    copy_bip32_path_with_curve(&(g_hwm.baking_key), &global.path_with_curve);
+    g_hwm.main_chain_id = G.main_chain_id;
+    g_hwm.hwm.main.highest_level = G.hwm.main;
+    g_hwm.hwm.main.highest_round = 0;
+    g_hwm.hwm.main.had_attestation = false;
+    g_hwm.hwm.main.had_preattestation = false;
+    g_hwm.hwm.test.highest_level = G.hwm.test;
+    g_hwm.hwm.test.highest_round = 0;
+    g_hwm.hwm.test.had_attestation = false;
+    g_hwm.hwm.test.had_preattestation = false;
+
+    UPDATE_NVRAM;
 
     provide_pubkey(&global.path_with_curve);
 
@@ -89,7 +89,8 @@ int handle_setup(buffer_t *cdata, derivation_type_t derivation_type) {
 }
 
 int handle_deauthorize(void) {
-    UPDATE_NVRAM(ram, { memset(&ram->baking_key, 0, sizeof(ram->baking_key)); });
+    memset(&(g_hwm.baking_key), 0, sizeof(g_hwm.baking_key));
+    UPDATE_NVRAM_VAR(baking_key);
 #ifdef HAVE_BAGL
     // Ignore calculation errors
     calculate_idle_screen_authorized_key();

diff --git a/src/apdu_sign.c b/src/apdu_sign.c
@@ -225,7 +225,7 @@ static int baking_sign_complete(bool const send_hash) {
                 case OPERATION_TAG_DELEGATION:
                     // Must be self-delegation signed by the *authorized* baking key
                     TZ_ASSERT(
-                        bip32_path_with_curve_eq(&global.path_with_curve, &N_data.baking_key) &&
+                        bip32_path_with_curve_eq(&global.path_with_curve, &g_hwm.baking_key) &&
                             // ops->signing is generated from G.bip32_path and G.curve
                             (COMPARE(G.maybe_ops.v.operation.source, G.maybe_ops.v.signing) == 0) &&
                             (COMPARE(G.maybe_ops.v.operation.destination, G.maybe_ops.v.signing) ==
@@ -238,7 +238,7 @@ static int baking_sign_complete(bool const send_hash) {
                 case OPERATION_TAG_NONE:
                     // Reveal cases
                     TZ_ASSERT(
-                        bip32_path_with_curve_eq(&global.path_with_curve, &N_data.baking_key) &&
+                        bip32_path_with_curve_eq(&global.path_with_curve, &g_hwm.baking_key) &&
                             // ops->signing is generated from G.bip32_path and G.curve
                             (COMPARE(G.maybe_ops.v.operation.source, G.maybe_ops.v.signing) == 0),
                         EXC_SECURITY);

diff --git a/src/baking_auth.c b/src/baking_auth.c
@@ -44,20 +44,20 @@ tz_exc write_high_water_mark(parsed_baking_data_t const *const in) {
 
     TZ_ASSERT(is_valid_level(in->level), EXC_WRONG_VALUES);
 
-    UPDATE_NVRAM(ram, {
-        // If the chain matches the main chain *or* the main chain is not set, then use 'main' HWM.
-        high_watermark_t volatile *const dest = select_hwm_by_chain(in->chain_id, ram);
-        TZ_ASSERT_NOT_NULL(dest);
-
-        if ((in->level > dest->highest_level) || (in->round > dest->highest_round)) {
-            dest->had_attestation = false;
-            dest->had_preattestation = false;
-        };
-        dest->highest_level = CUSTOM_MAX(in->level, dest->highest_level);
-        dest->highest_round = in->round;
-        dest->had_attestation |= in->type == BAKING_TYPE_ATTESTATION;
-        dest->had_preattestation |= in->type == BAKING_TYPE_PREATTESTATION;
-    });
+    // If the chain matches the main chain *or* the main chain is not set, then use 'main' HWM.
+    high_watermark_t *dest = select_hwm_by_chain(in->chain_id);
+    TZ_ASSERT_NOT_NULL(dest);
+
+    if ((in->level > dest->highest_level) || (in->round > dest->highest_round)) {
+        dest->had_attestation = false;
+        dest->had_preattestation = false;
+    };
+    dest->highest_level = CUSTOM_MAX(in->level, dest->highest_level);
+    dest->highest_round = in->round;
+    dest->had_attestation |= in->type == BAKING_TYPE_ATTESTATION;
+    dest->had_preattestation |= in->type == BAKING_TYPE_PREATTESTATION;
+
+    UPDATE_NVRAM_VAR(hwm);
 
 end:
     return exc;
@@ -69,14 +69,13 @@ tz_exc authorize_baking(derivation_type_t const derivation_type,
 
     TZ_ASSERT_NOT_NULL(bip32_path);
 
-    TZ_ASSERT(bip32_path->length <= NUM_ELEMENTS(N_data.baking_key.bip32_path.components),
+    TZ_ASSERT(bip32_path->length <= NUM_ELEMENTS(g_hwm.baking_key.bip32_path.components),
               EXC_WRONG_LENGTH);
 
     if (bip32_path->length != 0u) {
-        UPDATE_NVRAM(ram, {
-            ram->baking_key.derivation_type = derivation_type;
-            copy_bip32_path(&ram->baking_key.bip32_path, bip32_path);
-        });
+        g_hwm.baking_key.derivation_type = derivation_type;
+        copy_bip32_path(&g_hwm.baking_key.bip32_path, bip32_path);
+        UPDATE_NVRAM_VAR(baking_key);
     }
 
 end:
@@ -100,8 +99,7 @@ static bool is_level_authorized(parsed_baking_data_t const *const baking_info) {
         return false;
     }
 
-    high_watermark_t volatile const *const hwm =
-        select_hwm_by_chain(baking_info->chain_id, &N_data);
+    high_watermark_t *const hwm = select_hwm_by_chain(baking_info->chain_id);
     if (hwm == NULL) {
         return false;
     }
@@ -139,8 +137,8 @@ static bool is_level_authorized(parsed_baking_data_t const *const baking_info) {
 static bool is_path_authorized(derivation_type_t const derivation_type,
                                bip32_path_t const *const bip32_path) {
     return (bip32_path != NULL) && (derivation_type != DERIVATION_TYPE_UNSET) &&
-           (derivation_type == N_data.baking_key.derivation_type) && (bip32_path->length != 0u) &&
-           bip32_paths_eq(bip32_path, (const bip32_path_t *) &N_data.baking_key.bip32_path);
+           (derivation_type == g_hwm.baking_key.derivation_type) && (bip32_path->length != 0u) &&
+           bip32_paths_eq(bip32_path, (const bip32_path_t *) &g_hwm.baking_key.bip32_path);
 }
 
 tz_exc guard_baking_authorized(parsed_baking_data_t const *const baking_info,

diff --git a/src/globals.c b/src/globals.c
@@ -44,21 +44,19 @@ void clear_apdu_globals(void) {
 
 void init_globals(void) {
     memset(&global, 0, sizeof(global));
+    memcpy(&g_hwm, (const void *) (&N_data), sizeof(g_hwm));
 }
 
 void toggle_hwm(void) {
-    UPDATE_NVRAM(ram, { ram->hwm_disabled = !ram->hwm_disabled; });
+    g_hwm.hwm_disabled = !(g_hwm.hwm_disabled);
+    UPDATE_NVRAM;  // Update the NVRAM data.
 }
 
 // DO NOT TRY TO INIT THIS. This can only be written via an system call.
 // The "N_" is *significant*. It tells the linker to put this in NVRAM.
-nvram_data const N_data_real;
-
-high_watermark_t volatile *select_hwm_by_chain(chain_id_t const chain_id,
-                                               nvram_data volatile *const ram) {
-    if (ram == NULL) {
-        return NULL;
-    }
-    return ((chain_id.v == ram->main_chain_id.v) || !ram->main_chain_id.v) ? &ram->hwm.main
-                                                                           : &ram->hwm.test;
+baking_data const N_data_real;
+
+high_watermark_t *select_hwm_by_chain(chain_id_t const chain_id) {
+    return ((chain_id.v == g_hwm.main_chain_id.v) || !g_hwm.main_chain_id.v) ? &g_hwm.hwm.main
+                                                                             : &g_hwm.hwm.test;
 }
diff --git a/src/globals.h b/src/globals.h
@@ -146,18 +146,17 @@ typedef struct {
 
             apdu_hmac_state_t hmac;  ///< state used to handle hmac
         } u;
-
-        /// state used to store baking authorizing data
-        struct {
-            nvram_data new_data;  ///< Staging area for setting N_data
-        } baking_auth;
     } apdu;
+
+    baking_data hwm_data;  ///< baking HWM data in RAM
 } globals_t;
 
 extern globals_t global;
 
-extern nvram_data const N_data_real;
-#define N_data (*(volatile nvram_data *) PIC(&N_data_real))
+#define g_hwm global.hwm_data
+
+extern baking_data const N_data_real;
+#define N_data (*(volatile baking_data *) PIC(&N_data_real))
 
 /**
  * @brief Selects a HWM for a given chain id depending on the ram
@@ -167,24 +166,24 @@ extern nvram_data const N_data_real;
  *        of the ram. Selects the test HWM of the ram otherwise.
  *
  * @param chain_id: chain id
- * @param ram: ram
  * @return high_watermark_t*: selected HWM
  */
-high_watermark_t volatile *select_hwm_by_chain(chain_id_t const chain_id,
-                                               nvram_data volatile *const ram);
+high_watermark_t *select_hwm_by_chain(chain_id_t const chain_id);
+
+/**
+ * @brief Updates a single variable in NVRAM baking_data.
+ *
+ * @param variable: defines the name of the variable to be updated in NVRAM
+ */
+#define UPDATE_NVRAM_VAR(variable)                   \
+    if (!N_data_real.hwm_disabled) {                 \
+        nvm_write((void *) &(N_data.variable),       \
+                  &global.hwm_data.variable,         \
+                  sizeof(global.hwm_data.variable)); \
+    }
 
 /**
- * @brief Properly updates NVRAM data to prevent any clobbering of data
+ * @brief Properly updates an entire NVRAM struct to prevent any clobbering of data
  *
- * @param out_name: defines the name of a pointer to the nvram_data struct
- * @param body: defines the code to apply updates
  */
-#define UPDATE_NVRAM(out_name, body)                                                    \
-    ({                                                                                  \
-        nvram_data *const out_name = &global.apdu.baking_auth.new_data;                 \
-        memcpy(&global.apdu.baking_auth.new_data,                                       \
-               (nvram_data const *const) &N_data,                                       \
-               sizeof(global.apdu.baking_auth.new_data));                               \
-        body;                                                                           \
-        nvm_write((void *) &N_data, &global.apdu.baking_auth.new_data, sizeof(N_data)); \
-    })
+#define UPDATE_NVRAM nvm_write((void *) &(N_data), &global.hwm_data, sizeof(global.hwm_data));
diff --git a/src/types.h b/src/types.h
@@ -202,7 +202,7 @@ typedef struct {
     bool hwm_disabled;                   /**< Set HWM setting on/off,
                                               e.g. if you are using signer assisted HWM,
                                               no need to track HWM using Ledger.*/
-} nvram_data;
+} baking_data;
 
 #define SIGN_HASH_SIZE 32u  // TODO: Rename or use a different constant.
 

diff --git a/src/ui_bagl.c b/src/ui_bagl.c
@@ -104,7 +104,7 @@ UX_FLOW(ux_settings_flow, &ux_hwm_info, &ux_menu_back_step, FLOW_LOOP);
 void ui_settings(void) {
     hwm_status_to_string(home_context.hwm_status,
                          sizeof(home_context.hwm_status),
-                         &N_data.hwm_disabled);
+                         &g_hwm.hwm_disabled);
     ux_flow_init(0, ux_settings_flow, NULL);
 }
 
@@ -120,7 +120,7 @@ tz_exc calculate_idle_screen_chain_id(void) {
 
     TZ_ASSERT(chain_id_to_string_with_aliases(home_context.chain_id,
                                               sizeof(home_context.chain_id),
-                                              &N_data.main_chain_id) >= 0,
+                                              &g_hwm.main_chain_id) >= 0,
               EXC_WRONG_LENGTH);
 
 end:
@@ -132,15 +132,15 @@ tz_exc calculate_idle_screen_authorized_key(void) {
 
     memset(&home_context.authorized_key, 0, sizeof(home_context.authorized_key));
 
-    if (N_data.baking_key.bip32_path.length == 0u) {
+    if (g_hwm.baking_key.bip32_path.length == 0u) {
         TZ_ASSERT(copy_string(home_context.authorized_key,
                               sizeof(home_context.authorized_key),
                               "No Key Authorized"),
                   EXC_WRONG_LENGTH);
     } else {
         TZ_CHECK(bip32_path_with_curve_to_pkh_string(home_context.authorized_key,
                                                      sizeof(home_context.authorized_key),
-                                                     &N_data.baking_key));
+                                                     &g_hwm.baking_key));
     }
 
 end:
@@ -152,7 +152,7 @@ tz_exc calculate_idle_screen_hwm(void) {
 
     memset(&home_context.hwm, 0, sizeof(home_context.hwm));
 
-    TZ_ASSERT(hwm_to_string(home_context.hwm, sizeof(home_context.hwm), &N_data.hwm.main) >= 0,
+    TZ_ASSERT(hwm_to_string(home_context.hwm, sizeof(home_context.hwm), &g_hwm.hwm.main) >= 0,
               EXC_WRONG_LENGTH);
 
 end: