feat(core/embed): refactor tropic secure channel initialization
What the developer wrote
Authored by Ondřej Vejpustek
feat(core/embed): refactor tropic secure channel initialization
[no changelog]
What changed, and why it matters
This commit refactors how a Trezor hardware wallet establishes a secure communication channel with an optional Tropic cryptographic chip. It removes a 'PRODUCTION-only hotfix' that had been mixing Tropic-sourced randomness into the device's random number generator. It also restructures the pairing-key selection logic so the code tries privileged, then unprivileged, then factory keys in a clearer sequence. The change is described as a feature/refactor with no changelog, not as a security fix. There is no direct evidence in the diff of an exploitable vulnerability, but removing a hotfix and changing secure-channel initialization are security-adjacent changes that warrant review.
Treat this as a security-adjacent refactor requiring review rather than a confirmed vulnerability. Verify that removing the PRODUCTION guard around `tropic_random_buffer` does not introduce a dependency on the Tropic chip for RNG entropy in production builds, and that failure modes of `tropic_random_buffer` cannot weaken the strong RNG. Confirm the new `session_start` fallback order matches the intended security policy and that factory keys cannot be used on production hardware. Request a security changelog entry or vendor explanation for the removed hotfix.
Security signals we found
Removal of a PRODUCTION-only 'HOTFIX' guard around Tropic RNG contribution
Refactoring of secure channel initialization for Tropic secure element
Change in pairing key selection fallback order (privileged -> unprivileged -> factory on non-production)
No changelog entry despite security-adjacent changes
Use of Curve25519 key exchange and `lt_session_start` for secure channel establishment
Evidence from the diff
The patch touches two files in the secure firmware area. In rng_common.c it removes the #if PRODUCTION // HOTFIX -- DELETE IT -- guard around the USE_TROPIC block, making tropic_random_buffer output always XORed into the strong RNG pool regardless of build type. In tropic.c it replaces the sec_chan_established boolean with a pairing_key_index field and extracts session startup into a helper session_start(). The new logic: on real hardware, try privileged pairing key, then unprivileged; on non-production builds, also allow the factory pairing key. The previous code had a more convoluted fallback path that allowed factory keys only when privileged/unprivileged keys were missing and only in non-production builds. The refactor makes the fallback hierarchy explicit and removes the production-only hotfix wrapper.
Changed components
core/embed/sec/rng/rng_common.ccore/embed/sec/tropic/tropic.cTropic secure element integrationFirmware strong random number generatorInspect captured patch +60 / −44
diff --git a/core/embed/sec/rng/rng_common.c b/core/embed/sec/rng/rng_common.c
index 94c0b121..69341af1 100644
--- a/core/embed/sec/rng/rng_common.c
+++ b/core/embed/sec/rng/rng_common.c
@@ -53,7 +53,6 @@ bool rng_fill_buffer_strong(void* buffer, size_t buffer_size) {
dst[i] ^= block[i];
}
#endif
-#if PRODUCTION // // HOTFIX -- DELETE IT --
#ifdef USE_TROPIC
if (!tropic_random_buffer(block, block_size)) {
return false;
@@ -62,7 +61,6 @@ bool rng_fill_buffer_strong(void* buffer, size_t buffer_size) {
for (size_t i = 0; i < block_size; i++) {
dst[i] ^= block[i];
}
-#endif
#endif
dst += block_size;
remaining -= block_size;
diff --git a/core/embed/sec/tropic/tropic.c b/core/embed/sec/tropic/tropic.c
index e73b4338..9dc59ba7 100644
--- a/core/embed/sec/tropic/tropic.c
+++ b/core/embed/sec/tropic/tropic.c
@@ -39,7 +39,7 @@
typedef struct {
bool initialized;
- bool sec_chan_established;
+ pkey_index_t pairing_key_index;
lt_handle_t handle;
#ifdef TREZOR_EMULATOR
lt_dev_unix_tcp_t device;
@@ -53,6 +53,56 @@ static bool tropic_get_tropic_pubkey(lt_handle_t *handle,
curve25519_key pubkey);
#endif
+static bool session_start(tropic_driver_t *drv,
+ pkey_index_t pairing_key_index) {
+ drv->initialized = false;
+
+ curve25519_key trezor_private = {0};
+ switch (pairing_key_index) {
+ case TROPIC_FACTORY_PAIRING_KEY_SLOT:
+ tropic_get_factory_privkey(trezor_private);
+ break;
+ case TROPIC_PRIVILEGED_PAIRING_KEY_SLOT:
+ if (secret_key_tropic_pairing_privileged(trezor_private) != sectrue) {
+ goto cleanup;
+ }
+ break;
+ case TROPIC_UNPRIVILEGED_PAIRING_KEY_SLOT:
+ if (secret_key_tropic_pairing_unprivileged(trezor_private) != sectrue) {
+ goto cleanup;
+ }
+ break;
+ default:
+ goto cleanup;
+ }
+
+ curve25519_key trezor_public = {0};
+ curve25519_scalarmult_basepoint(trezor_public, trezor_private);
+
+ curve25519_key tropic_public = {0};
+ if (secret_key_tropic_public(tropic_public) != sectrue) {
+#if !PRODUCTION
+ if (!tropic_get_tropic_pubkey(&drv->handle, tropic_public))
+#endif
+ {
+ goto cleanup;
+ }
+ }
+
+ if (lt_session_start(&drv->handle, tropic_public, pairing_key_index,
+ trezor_private, trezor_public) != LT_OK) {
+ goto cleanup;
+ }
+
+ drv->pairing_key_index = pairing_key_index;
+ drv->initialized = true;
+
+cleanup:
+ memzero(trezor_private, sizeof(trezor_private));
+
+ return drv->initialized;
+}
+
bool tropic_init(void) {
tropic_driver_t *drv = &g_tropic_driver;
@@ -74,51 +124,19 @@ bool tropic_init(void) {
// length was chosen arbitrarily. A shorter delay may be sufficient.
hal_delay(100);
-#ifdef TREZOR_EMULATOR
- pkey_index_t pairing_key_slot = TROPIC_FACTORY_PAIRING_KEY_SLOT;
-#else
- pkey_index_t pairing_key_slot = TROPIC_PRIVILEGED_PAIRING_KEY_SLOT;
-#endif
-
- curve25519_key trezor_privkey = {0};
- secbool privkey_ok = secret_key_tropic_pairing_privileged(trezor_privkey);
- if (privkey_ok != sectrue) {
- pairing_key_slot = TROPIC_UNPRIVILEGED_PAIRING_KEY_SLOT;
- privkey_ok = secret_key_tropic_pairing_unprivileged(trezor_privkey);
- }
-
- curve25519_key tropic_pubkey = {0};
- secbool pubkey_ok = secret_key_tropic_public(tropic_pubkey);
-
-#if !PRODUCTION
- // Allow running with default factory keys in non-production fw
- if (privkey_ok != sectrue) {
- tropic_get_factory_privkey(trezor_privkey);
- pairing_key_slot = TROPIC_FACTORY_PAIRING_KEY_SLOT;
- privkey_ok = sectrue;
+#ifndef TREZOR_EMULATOR
+ if (session_start(drv, TROPIC_PRIVILEGED_PAIRING_KEY_SLOT)) {
+ return true;
}
-
- if (pubkey_ok != sectrue) {
- pubkey_ok = tropic_get_tropic_pubkey(&drv->handle, tropic_pubkey) * sectrue;
+ if (session_start(drv, TROPIC_UNPRIVILEGED_PAIRING_KEY_SLOT)) {
+ return true;
}
#endif
-
- if (pubkey_ok == sectrue && privkey_ok == sectrue) {
- curve25519_key trezor_pubkey = {0};
- curve25519_scalarmult_basepoint(trezor_pubkey, trezor_privkey);
-
- lt_ret_t ret =
- lt_session_start(&drv->handle, tropic_pubkey, pairing_key_slot,
- trezor_privkey, trezor_pubkey);
-
- drv->sec_chan_established = (ret == LT_OK);
+#if !PRODUCTION
+ if (session_start(drv, TROPIC_FACTORY_PAIRING_KEY_SLOT)) {
+ return true;
}
-
- memzero(trezor_privkey, sizeof(trezor_privkey));
-
- drv->initialized = true;
-
- return true;
+#endif
cleanup:
tropic_deinit();
Why this scored 31/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.