refactor(core/embed): factor out `generate_correct_mac_and_destroy_output()`
What the developer wrote
Authored by Ondřej Vejpustek
refactor(core/embed): factor out `generate_correct_mac_and_destroy_output()`
[no changelog]
What changed, and why it matters
This commit is a code cleanup: it moves two repeated calls to a hardware security function into a new helper function. The actual operations performed on the device are unchanged. There is no indication this fixes or introduces a security bug.
No security action required. Treat as ordinary refactoring.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch refactors tropic_pin_set() and tropic_pin_set_time() in core/embed/sec/tropic/tropic.c. It introduces generate_correct_mac_and_destroy_output() which calls lt_mac_and_destroy() twice: first with a reset key, then with the actual input. This exactly matches the previous inline sequence. The timing helper similarly sums the same two lt_mac_and_destroy_time() calls. The only functional difference is that the second lt_mac_and_destroy() now writes to output instead of digest, and the subsequent hmac_sha256() uses output rather than digest. Because output and digest were both local buffers of the same size and the value written by the second call was previously stored into digest and is now stored into output, the downstream HMAC input is the same value as before. Therefore behavior is preserved.
Changed components
core/embed/sec/tropic/tropic.cInspect captured patch +23 / −10
diff --git a/core/embed/sec/tropic/tropic.c b/core/embed/sec/tropic/tropic.c
index 229f5841c..cfeb0f118 100644
--- a/core/embed/sec/tropic/tropic.c
+++ b/core/embed/sec/tropic/tropic.c
@@ -643,6 +643,24 @@ void tropic_pin_reset_slots_time(uint32_t *time_ms, uint16_t pin_index) {
}
}
+static lt_ret_t generate_correct_mac_and_destroy_output(
+ lt_handle_t *handle, uint16_t slot_index,
+ const uint8_t reset_key[TROPIC_MAC_AND_DESTROY_SIZE],
+ const uint8_t input[TROPIC_MAC_AND_DESTROY_SIZE],
+ uint8_t output[TROPIC_MAC_AND_DESTROY_SIZE]) {
+ lt_ret_t res = lt_mac_and_destroy(handle, slot_index, reset_key, output);
+ if (res != LT_OK) {
+ return res;
+ }
+
+ return lt_mac_and_destroy(handle, slot_index, input, output);
+}
+
+static void generate_correct_mac_and_destroy_output_time(uint32_t *time_ms) {
+ lt_mac_and_destroy_time(time_ms);
+ lt_mac_and_destroy_time(time_ms);
+}
+
bool tropic_pin_set(
tropic_ui_progress_t ui_progress,
uint8_t stretched_pins[PIN_MAX_TRIES][TROPIC_MAC_AND_DESTROY_SIZE],
@@ -675,20 +693,16 @@ bool tropic_pin_set(
mac_and_destroy_slot_t slot_index =
get_mac_and_destroy_slot(i, change_pin_counter);
- if (lt_mac_and_destroy(&drv->handle, slot_index, reset_key, output) !=
- LT_OK) {
- goto cleanup;
- }
-
hmac_sha256(stretched_pins[i], TROPIC_MAC_AND_DESTROY_SIZE, NULL, 0,
digest);
- if (lt_mac_and_destroy(&drv->handle, slot_index, digest, digest) != LT_OK) {
+ if (generate_correct_mac_and_destroy_output(
+ &drv->handle, slot_index, reset_key, digest, output) != LT_OK) {
goto cleanup;
}
- hmac_sha256(stretched_pins[i], TROPIC_MAC_AND_DESTROY_SIZE, digest,
- sizeof(digest), stretched_pins[i]);
+ hmac_sha256(stretched_pins[i], TROPIC_MAC_AND_DESTROY_SIZE, output,
+ sizeof(output), stretched_pins[i]);
if (lt_mac_and_destroy(&drv->handle, slot_index, reset_key, output) !=
LT_OK) {
@@ -714,8 +728,7 @@ void tropic_pin_set_time(uint32_t *time_ms) {
// update_change_pin_counter() in tropic_pin_set()
get_change_pin_counter_time(time_ms, true);
for (int i = 0; i < PIN_MAX_TRIES; i++) {
- lt_mac_and_destroy_time(time_ms);
- lt_mac_and_destroy_time(time_ms);
+ generate_correct_mac_and_destroy_output_time(time_ms);
lt_mac_and_destroy_time(time_ms);
}
}
Why this scored 11/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.