Replace usage of NVRAM hwm_data to RAM hwm_data

- Remove HWM struct from apdu. - Create a global define g_hwm for RAM hwm_data - Move NVRAM hwm_data to RAM during app init
trilitech · Apr 8, 2024 · ca689d5 · ca689d5
1 parent b08bb1c
commit ca689d5
Show file tree

Hide file tree

Showing 10 changed files with 79 additions and 84 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,13 +36,12 @@
  * @return true
  */
 static bool ok(void) {
-    baking_hwm_data *hwm_data = &global.apdu.baking_auth.new_data;
-    hwm_data->hwm.main.highest_level = G.reset_level;
-    hwm_data->hwm.main.highest_round = 0;
-    hwm_data->hwm.main.had_attestation = false;
-    hwm_data->hwm.test.highest_level = G.reset_level;
-    hwm_data->hwm.test.highest_round = 0;
-    hwm_data->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;
 

diff --git a/src/apdu_setup.c b/src/apdu_setup.c
@@ -42,17 +42,16 @@
  * @return true
  */
 static bool ok(void) {
-    baking_hwm_data *hwm_data = &global.apdu.baking_auth.new_data;
-    copy_bip32_path_with_curve(&(hwm_data->baking_key), &global.path_with_curve);
-    hwm_data->main_chain_id = G.main_chain_id;
-    hwm_data->hwm.main.highest_level = G.hwm.main;
-    hwm_data->hwm.main.highest_round = 0;
-    hwm_data->hwm.main.had_attestation = false;
-    hwm_data->hwm.main.had_preattestation = false;
-    hwm_data->hwm.test.highest_level = G.hwm.test;
-    hwm_data->hwm.test.highest_round = 0;
-    hwm_data->hwm.test.had_attestation = false;
-    hwm_data->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;
 
@@ -90,8 +89,7 @@ int handle_setup(buffer_t *cdata, derivation_type_t derivation_type) {
 }
 
 int handle_deauthorize(void) {
-    baking_hwm_data *hwm_data = &global.apdu.baking_auth.new_data;
-    memset(&(hwm_data->baking_key), 0, sizeof(hwm_data->baking_key));
+    memset(&(g_hwm.baking_key), 0, sizeof(g_hwm.baking_key));
     UPDATE_NVRAM_VAR(baking_key);
 #ifdef HAVE_BAGL
     // Ignore calculation errors

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,9 +44,8 @@ tz_exc write_high_water_mark(parsed_baking_data_t const *const in) {
 
     TZ_ASSERT(is_valid_level(in->level), EXC_WRONG_VALUES);
 
-    baking_hwm_data *hwm_data = &global.apdu.baking_auth.new_data;
     // 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, hwm_data);
+    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)) {
@@ -70,13 +69,12 @@ 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) {
-        baking_hwm_data *out_data = &global.apdu.baking_auth.new_data;
-        out_data->baking_key.derivation_type = derivation_type;
-        copy_bip32_path(&out_data->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);
     }
 
@@ -101,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;
     }
@@ -140,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,22 +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) {
-    baking_hwm_data *out = &global.apdu.baking_auth.new_data;
-    out->hwm_disabled = !out->hwm_disabled;
-    UPDATE_NVRAM_VAR(hwm_disabled);  // Update the NVRAM data.
+    g_hwm.hwm_disabled = !(g_hwm.hwm_disabled);
+    UPDATE_NVRAM_SETTINGS(hwm_disabled);  // 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.
 baking_hwm_data const N_data_real;
 
-high_watermark_t *select_hwm_by_chain(chain_id_t const chain_id, baking_hwm_data *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;
+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,16 +146,15 @@ typedef struct {
 
             apdu_hmac_state_t hmac;  ///< state used to handle hmac
         } u;
-
-        /// state used to store baking authorizing data
-        struct {
-            baking_hwm_data new_data;  ///< baking HWM data in RAM
-        } baking_auth;
     } apdu;
+
+    baking_hwm_data hwm_data;  ///< baking HWM data in RAM
 } globals_t;
 
 extern globals_t global;
 
+#define g_hwm global.hwm_data
+
 extern baking_hwm_data const N_data_real;
 #define N_data (*(volatile baking_hwm_data *) PIC(&N_data_real))
 
@@ -167,31 +166,37 @@ extern baking_hwm_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 *select_hwm_by_chain(chain_id_t const chain_id,
-                                      baking_hwm_data volatile *const ram);
+high_watermark_t *select_hwm_by_chain(chain_id_t const chain_id);
+
+/**
+ * @brief Properly updates a single variable in NVRAM.
+ *
+ * @param variable: defines the name of the variable to be updated in NVRAM
+ */
+#define UPDATE_NVRAM_SETTINGS(variable)    \
+    nvm_write((void *) &(N_data.variable), \
+              &global.hwm_data.variable,   \
+              sizeof(global.hwm_data.variable));
 
 /**
  * @brief Properly updates a single variable in NVRAM.
  *
  * @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.apdu.baking_auth.new_data.variable,         \
-                  sizeof(global.apdu.baking_auth.new_data.variable)); \
+#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 an entire NVRAM struct to prevent any clobbering of data
  *
  */
-#define UPDATE_NVRAM                                         \
-    if (!N_data_real.hwm_disabled) {                         \
-        nvm_write((void *) &(N_data),                        \
-                  &global.apdu.baking_auth.new_data,         \
-                  sizeof(global.apdu.baking_auth.new_data)); \
+#define UPDATE_NVRAM                                                              \
+    if (!N_data_real.hwm_disabled) {                                              \
+        nvm_write((void *) &(N_data), &global.hwm_data, sizeof(global.hwm_data)); \
     }
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:

diff --git a/src/ui_nbgl.c b/src/ui_nbgl.c
@@ -77,20 +77,20 @@ static bool navigation_cb_baking(uint8_t page, nbgl_pageContent_t* content) {
     bakeInfoContents[0] = buffer[0];
     bakeInfoContents[1] = buffer[1];
     bakeInfoContents[2] = buffer[2];
-    const bool hwm_disabled = N_data.hwm_disabled;
+    const bool hwm_disabled = g_hwm.hwm_disabled;
 
     TZ_ASSERT(
-        chain_id_to_string_with_aliases(buffer[0], sizeof(buffer[0]), &N_data.main_chain_id) >= 0,
+        chain_id_to_string_with_aliases(buffer[0], sizeof(buffer[0]), &g_hwm.main_chain_id) >= 0,
         EXC_WRONG_LENGTH);
 
-    if (N_data.baking_key.bip32_path.length == 0u) {
+    if (g_hwm.baking_key.bip32_path.length == 0u) {
         TZ_ASSERT(copy_string(buffer[1], sizeof(buffer[1]), "No Key Authorized"), EXC_WRONG_LENGTH);
     } else {
         TZ_CHECK(
-            bip32_path_with_curve_to_pkh_string(buffer[1], sizeof(buffer[1]), &N_data.baking_key));
+            bip32_path_with_curve_to_pkh_string(buffer[1], sizeof(buffer[1]), &g_hwm.baking_key));
     }
 
-    TZ_ASSERT(hwm_to_string(buffer[2], sizeof(buffer[2]), &N_data.hwm.main) >= 0, EXC_WRONG_LENGTH);
+    TZ_ASSERT(hwm_to_string(buffer[2], sizeof(buffer[2]), &g_hwm.hwm.main) >= 0, EXC_WRONG_LENGTH);
 
     switch (page) {
         case 0: