refactor(core/embed): cache tropic change pin counter
What the developer wrote
Authored by Ondřej Vejpustek
refactor(core/embed): cache tropic change pin counter
[no changelog]
What changed, and why it matters
This commit is a small internal refactor of the Trezor firmware's Tropic secure-element integration. It adds a software cache for a hardware-backed 'change PIN counter' so the firmware does not have to repeatedly ask the secure chip for the same value during PIN operations. There is no indication in the commit or title that this fixes a security bug; it appears to be a performance or timing-predictability improvement.
No security action required based on this commit alone. Treat as a normal refactor; continue routine review of Tropic PIN counter handling.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The change introduces two static globals (g_change_pin_counter_cached, g_is_change_pin_counter_cached) and modifies get_change_pin_counter() to return a cached value after the first successful read. It also updates update_change_pin_counter() to keep the cached value in sync when the counter is incremented, and adjusts the companion timing functions to skip adding the secure-element read latency when the cache is already populated. The logic preserves the existing counter semantics (subtracting from TROPIC_CHANGE_COUNTER_SLOT_MAX_VALUE and treating LT_L3_COUNTER_INVALID as zero).
Changed components
core/embed/sec/tropic/tropic.cInspect captured patch +35 / −13
diff --git a/core/embed/sec/tropic/tropic.c b/core/embed/sec/tropic/tropic.c
index 5ff8693d..1779fcb9 100644
--- a/core/embed/sec/tropic/tropic.c
+++ b/core/embed/sec/tropic/tropic.c
@@ -461,29 +461,40 @@ static void lt_mcounter_update_time(uint32_t *time_ms) { *time_ms += 51; }
static void lt_r_mem_data_erase_time(uint32_t *time_ms) { *time_ms += 55; }
+static uint32_t g_change_pin_counter_cached = 0;
+static bool g_is_change_pin_counter_cached = false;
+
static bool get_change_pin_counter(uint32_t *change_pin_counter) {
tropic_driver_t *drv = &g_tropic_driver;
+ if (g_is_change_pin_counter_cached) {
+ *change_pin_counter = g_change_pin_counter_cached;
+ return true;
+ }
+
lt_ret_t ret = lt_mcounter_get(&drv->handle, TROPIC_CHANGE_COUNTER_SLOT,
change_pin_counter);
- if (ret == LT_L3_COUNTER_INVALID) {
+ if (ret == LT_OK) {
+ *change_pin_counter =
+ TROPIC_CHANGE_COUNTER_SLOT_MAX_VALUE - *change_pin_counter;
+ } else if (ret == LT_L3_COUNTER_INVALID) {
// The counter has not been initialized yet
*change_pin_counter = 0;
- return true;
- }
-
- if (ret != LT_OK) {
+ } else {
return false;
}
- *change_pin_counter =
- TROPIC_CHANGE_COUNTER_SLOT_MAX_VALUE - *change_pin_counter;
+ g_change_pin_counter_cached = *change_pin_counter;
+ g_is_change_pin_counter_cached = true;
return true;
}
-static void get_change_pin_counter_time(uint32_t *time_ms) {
- lt_mcounter_get_time(time_ms);
+static void get_change_pin_counter_time(uint32_t *time_ms,
+ bool is_change_pin_counter_cached) {
+ if (!is_change_pin_counter_cached) {
+ lt_mcounter_get_time(time_ms);
+ }
}
static bool update_change_pin_counter() {
@@ -498,7 +509,8 @@ static bool update_change_pin_counter() {
if (ret != LT_OK) {
return false;
}
-
+ g_change_pin_counter_cached = 1;
+ g_is_change_pin_counter_cached = true;
return true;
}
@@ -506,6 +518,10 @@ static bool update_change_pin_counter() {
return false;
}
+ if (g_is_change_pin_counter_cached) {
+ g_change_pin_counter_cached++;
+ }
+
return true;
}
@@ -557,7 +573,7 @@ cleanup:
}
void tropic_pin_stretch_time(uint32_t *time_ms) {
- get_change_pin_counter_time(time_ms);
+ get_change_pin_counter_time(time_ms, g_is_change_pin_counter_cached);
lt_mac_and_destroy_time(time_ms);
}
@@ -602,7 +618,10 @@ cleanup:
}
void tropic_pin_reset_slots_time(uint32_t *time_ms, uint16_t pin_index) {
- get_change_pin_counter_time(time_ms);
+ // When get_change_pin_counter() is called in tropic_pin_reset_slots(), the
+ // change pin counter will have already been cached by
+ // get_change_pin_counter() in tropic_pin_stretch()
+ get_change_pin_counter_time(time_ms, true);
for (int i = 0; i <= pin_index; i++) {
lt_mac_and_destroy_time(time_ms);
}
@@ -674,7 +693,10 @@ cleanup:
void tropic_pin_set_time(uint32_t *time_ms) {
rng_fill_buffer_strong_time(time_ms);
update_change_pin_counter_time(time_ms);
- get_change_pin_counter_time(time_ms);
+ // When get_change_pin_counter() is called in tropic_pin_set(), the
+ // change pin counter will have already been cached by
+ // 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);
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.