refactor(core/prodtest): revise tropic initialization
What the developer wrote
Authored by Ondřej Vejpustek
refactor(core/prodtest): revise tropic initialization
[no changelog]
What changed, and why it matters
This commit refactors how the Trezor firmware initializes and talks to the Tropic secure chip, mostly in the internal production-test tool. It removes some custom wrapper functions and switches to the vendor's libtropic library directly. The visible changes include removing a hard crash ('ensure') when a secure session cannot be started, and removing a second (apparently accidental) memzero call that was clearing the wrong key. These are code-quality and robustness improvements rather than a clear-cut security fix, but they do reduce two risky behaviors.
Treat as a hardening/refactoring commit. Review the new lt_port_init/lt_port_deinit paths for correct SPI/GPIO cleanup and confirm that removing the 'ensure' panic does not mask pairing failures in production firmware. Verify that exposing tropic_get_handle() is gated only on TREZOR_PRODTEST and not reachable in normal firmware builds. No urgent user action is indicated.
Security signals we found
Removal of unconditional device panic (ensure) on secure-channel setup failure
Correction of duplicated memzero that was clearing the wrong key buffer
Refactoring of HAL init/deinit to be invoked through libtropic port layer
Exposure of raw lt_handle_t pointer in production-test build only
No changelog entry and no vendor security disclosure
Evidence from the diff
The patch restructures Tropic initialization in core/embed/sec/tropic. Key changes: (1) tropic01.c now implements lt_port_init/lt_port_deinit and performs actual SPI/GPIO HAL setup, instead of having a separate tropic_hal_init/tropic_hal_deinit pair that was previously unused by the libtropic port layer. (2) tropic.c removes the custom tropic_get_ info wrappers and exposes tropic_get_handle() under TREZOR_PRODTEST so prodtest can call libtropic directly. (3) The removed ‘ensure(ret == LT_OK, …)’ panic in tropic_init() means a failed lt_session_start no longer crashes the device; instead sec_chan_established is simply set to false. (4) A duplicated memzero that was clearing tropic_secret_trezor_privkey twice (and never clearing tropic_secret_tropic_pubkey) is replaced by a single memzero of trezor_privkey. (5) tropic_internal.h is deleted because tropic_hal_ symbols are gone. There is no changelog entry and no explicit security framing from the vendor.
Changed components
core/embed/sec/tropic/tropic.ccore/embed/sec/tropic/stm32/tropic01.ccore/embed/sec/tropic/inc/sec/tropic.hcore/embed/projects/prodtest/cmd/prodtest_tropic.ccore/embed/sec/tropic/tropic_internal.hInspect captured patch +36 / −122
diff --git a/core/embed/projects/prodtest/cmd/prodtest_tropic.c b/core/embed/projects/prodtest/cmd/prodtest_tropic.c
index 1ab23704e..a8f7b3716 100644
--- a/core/embed/projects/prodtest/cmd/prodtest_tropic.c
+++ b/core/embed/projects/prodtest/cmd/prodtest_tropic.c
@@ -29,6 +29,8 @@
#include "memzero.h"
+#include "libtropic.h"
+
#include "secure_channel.h"
typedef enum {
@@ -49,8 +51,10 @@ static void prodtest_tropic_get_riscv_fw_version(cli_t* cli) {
return;
}
- uint8_t version[TROPIC_RISCV_FW_SIZE];
- if (!tropic_get_riscv_fw_version(version, sizeof(version))) {
+ lt_handle_t* handle = tropic_get_handle();
+
+ uint8_t version[LT_L2_GET_INFO_RISCV_FW_SIZE] = {0};
+ if (lt_get_info_riscv_fw_ver(handle, version, sizeof(version)) != LT_OK) {
cli_error(cli, CLI_ERROR, "Unable to get RISCV FW version");
}
@@ -64,8 +68,10 @@ static void prodtest_tropic_get_spect_fw_version(cli_t* cli) {
return;
}
- uint8_t version[TROPIC_SPECT_FW_SIZE];
- if (!tropic_get_spect_fw_version(version, sizeof(version))) {
+ lt_handle_t* handle = tropic_get_handle();
+
+ uint8_t version[LT_L2_GET_INFO_SPECT_FW_SIZE];
+ if (!lt_get_info_spect_fw_ver(handle, version, sizeof(version))) {
cli_error(cli, CLI_ERROR, "Unable to get SPECT FW version");
}
@@ -79,8 +85,10 @@ static void prodtest_tropic_get_chip_id(cli_t* cli) {
return;
}
- uint8_t chip_id[TROPIC_CHIP_ID_SIZE];
- if (!tropic_get_chip_id(chip_id, sizeof(chip_id))) {
+ lt_handle_t* handle = tropic_get_handle();
+
+ uint8_t chip_id[LT_L2_GET_INFO_CHIP_ID_SIZE];
+ if (!lt_get_info_chip_id(handle, chip_id, sizeof(chip_id))) {
cli_error(cli, CLI_ERROR, "Unable to get CHIP ID");
}
diff --git a/core/embed/sec/tropic/inc/sec/tropic.h b/core/embed/sec/tropic/inc/sec/tropic.h
index f0e840453..116b1984c 100644
--- a/core/embed/sec/tropic/inc/sec/tropic.h
+++ b/core/embed/sec/tropic/inc/sec/tropic.h
@@ -23,19 +23,14 @@
#ifdef KERNEL_MODE
-#define TROPIC_CHIP_ID_SIZE 128
-#define TROPIC_RISCV_FW_SIZE 4
-#define TROPIC_SPECT_FW_SIZE 4
-
bool tropic_init(void);
void tropic_deinit(void);
-bool tropic_get_spect_fw_version(uint8_t* version_buffer, uint16_t max_len);
-
-bool tropic_get_riscv_fw_version(uint8_t* version_buffer, uint16_t max_len);
-
-bool tropic_get_chip_id(uint8_t* chip_id, uint16_t max_len);
+#ifdef TREZOR_PRODTEST
+#include "libtropic.h"
+lt_handle_t* tropic_get_handle(void);
+#endif
#endif
diff --git a/core/embed/sec/tropic/stm32/tropic01.c b/core/embed/sec/tropic/stm32/tropic01.c
index c16f9faa7..de51f48cb 100644
--- a/core/embed/sec/tropic/stm32/tropic01.c
+++ b/core/embed/sec/tropic/stm32/tropic01.c
@@ -41,11 +41,11 @@ void tropic01_reset(void) {
systick_delay_ms(10);
}
-bool tropic_hal_init(void) {
+lt_ret_t lt_port_init(lt_handle_t *h) {
tropic01_hal_driver_t *drv = &g_tropic01_hal_driver;
if (drv->initialized) {
- return true;
+ return LT_OK;
}
GPIO_InitTypeDef GPIO_InitStructure = {0};
@@ -118,10 +118,10 @@ bool tropic_hal_init(void) {
drv->initialized = true;
- return true;
+ return LT_OK;
}
-void tropic_hal_deinit(void) {
+lt_ret_t lt_port_deinit(lt_handle_t *h) {
tropic01_hal_driver_t *drv = &g_tropic01_hal_driver;
if (drv->spi.Instance != NULL) {
@@ -139,18 +139,6 @@ void tropic_hal_deinit(void) {
HAL_GPIO_DeInit(TROPIC01_SPI_MOSI_PORT, TROPIC01_SPI_MOSI_PIN);
HAL_GPIO_DeInit(TROPIC01_PWR_PORT, TROPIC01_PWR_PIN);
- memset(drv, 0, sizeof(*drv));
-}
-
-lt_ret_t lt_port_init(lt_handle_t *h) {
- UNUSED(h);
- // no action, as we initialize separately
- return LT_OK;
-}
-
-lt_ret_t lt_port_deinit(lt_handle_t *h) {
- UNUSED(h);
- // no action, as we deinitialize separately
return LT_OK;
}
diff --git a/core/embed/sec/tropic/tropic.c b/core/embed/sec/tropic/tropic.c
index 09fda3c10..2d21a06ca 100644
--- a/core/embed/sec/tropic/tropic.c
+++ b/core/embed/sec/tropic/tropic.c
@@ -29,7 +29,6 @@
#include "ed25519-donna/ed25519.h"
#include "memzero.h"
-#include "tropic_internal.h"
#define PKEY_INDEX_BYTE PAIRING_KEY_SLOT_INDEX_0
@@ -48,38 +47,28 @@ bool tropic_init(void) {
return true;
}
- curve25519_key tropic_secret_tropic_pubkey = {0};
- curve25519_key tropic_secret_trezor_privkey = {0};
-
- if (!tropic_hal_init()) {
- goto cleanup;
- }
+ curve25519_key tropic_pubkey = {0};
+ curve25519_key trezor_privkey = {0};
if (lt_init(&drv->handle) != LT_OK) {
- tropic_hal_deinit();
goto cleanup;
}
- secbool pubkey_ok = secret_key_tropic_public(tropic_secret_tropic_pubkey);
- secbool privkey_ok =
- secret_key_tropic_pairing_privileged(tropic_secret_trezor_privkey);
+ secbool pubkey_ok = secret_key_tropic_public(tropic_pubkey);
+ secbool privkey_ok = secret_key_tropic_pairing_privileged(trezor_privkey);
if (pubkey_ok == sectrue && privkey_ok == sectrue) {
- uint8_t trezor_pubkey[32] = {0};
- curve25519_scalarmult_basepoint(trezor_pubkey,
- tropic_secret_trezor_privkey);
+ curve25519_key trezor_pubkey = {0};
+ curve25519_scalarmult_basepoint(trezor_pubkey, trezor_privkey);
- lt_ret_t ret = lt_session_start(
- &drv->handle, tropic_secret_tropic_pubkey, PKEY_INDEX_BYTE,
- tropic_secret_trezor_privkey, trezor_pubkey);
+ lt_ret_t ret =
+ lt_session_start(&drv->handle, tropic_pubkey, PKEY_INDEX_BYTE,
+ trezor_privkey, trezor_pubkey);
- // todo delete the ensure
- ensure((ret == LT_OK) * sectrue, "lt_session_start failed");
drv->sec_chan_established = (ret == LT_OK);
}
- memzero(tropic_secret_trezor_privkey, sizeof(tropic_secret_trezor_privkey));
- memzero(tropic_secret_trezor_privkey, sizeof(tropic_secret_tropic_pubkey));
+ memzero(trezor_privkey, sizeof(trezor_privkey));
drv->initialized = true;
@@ -92,58 +81,18 @@ cleanup:
void tropic_deinit(void) {
tropic_driver_t *drv = &g_tropic_driver;
-
- if (drv->handle.device != NULL) {
- lt_deinit(&drv->handle);
- }
-
- tropic_hal_deinit();
-
+ lt_deinit(&drv->handle);
memset(drv, 0, sizeof(*drv));
}
-bool tropic_get_spect_fw_version(uint8_t *version_buffer, uint16_t max_len) {
- tropic_driver_t *drv = &g_tropic_driver;
-
- if (!drv->initialized) {
- return false;
- }
-
- if (LT_OK != lt_get_info_spect_fw_ver(&drv->handle, (uint8_t *)version_buffer,
- max_len)) {
- return false;
- }
-
- return true;
-}
-
-bool tropic_get_riscv_fw_version(uint8_t *version_buffer, uint16_t max_len) {
- tropic_driver_t *drv = &g_tropic_driver;
-
- if (!drv->initialized) {
- return false;
- }
-
- if (LT_OK != lt_get_info_riscv_fw_ver(&drv->handle, (uint8_t *)version_buffer,
- max_len)) {
- return false;
- }
-
- return true;
-}
-
-bool tropic_get_chip_id(uint8_t *chip_id, uint16_t max_len) {
+lt_handle_t *tropic_get_handle(void) {
tropic_driver_t *drv = &g_tropic_driver;
if (!drv->initialized) {
- return false;
+ return NULL;
}
- if (LT_OK != lt_get_info_chip_id(&drv->handle, (uint8_t *)chip_id, max_len)) {
- return false;
- }
-
- return true;
+ return &drv->handle;
}
bool tropic_ping(const uint8_t *msg_out, uint8_t *msg_in, uint16_t msg_len) {
diff --git a/core/embed/sec/tropic/tropic_internal.h b/core/embed/sec/tropic/tropic_internal.h
deleted file mode 100644
index 6a43ce1d2..000000000
--- a/core/embed/sec/tropic/tropic_internal.h
+++ /dev/null
@@ -1,26 +0,0 @@
-/*
- * This file is part of the Trezor project, https://trezor.io/
- *
- * Copyright (c) SatoshiLabs
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program. If not, see <http://www.gnu.org/licenses/>.
- */
-
-#pragma once
-
-#include <trezor_types.h>
-
-bool tropic_hal_init(void);
-
-void tropic_hal_deinit(void);
Why this scored 25/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.