refactor(core): Add secret_mcu_device_cert_*() methods.
What changed, and why it matters
This commit is a code cleanup (refactor) that moves the handling of a device certificate stored in secure memory into dedicated helper functions. It does not change what data is stored or how it is protected; it only makes the code more maintainable. There is no indication this fixes a security bug.
No security action required. Treat as normal code-quality/maintenance change. Continue standard review and testing.
Security signals we found
Refactor only: no change to security policy or trust boundary
Existing length-prefix validation is preserved and centralized
No new attack surface introduced; prodtest commands already had write/read access
Commit message explicitly labels this as a refactor and omits changelog entry
Evidence from the diff
The change introduces secret_mcu_device_cert_write(), secret_mcu_device_cert_size(), and secret_mcu_device_cert_read() in the STM32U5 secret-storage layer. These helpers centralize the existing 2-byte big-endian length prefix that was previously managed inline by the prodtest CLI commands. The prodtest commands now call these helpers instead of directly using secret_write()/secret_read(). The certificate size validation logic is preserved and moved into the helper functions.
Changed components
core/embed/sec/secret/stm32u5/secret.ccore/embed/sec/secret/inc/sec/secret.hcore/embed/projects/prodtest/cmd/prodtest_secrets.cInspect captured patch +86 / −28
diff --git a/core/embed/projects/prodtest/cmd/prodtest_secrets.c b/core/embed/projects/prodtest/cmd/prodtest_secrets.c
index 5a8bb69d..4f4f4b51 100644
--- a/core/embed/projects/prodtest/cmd/prodtest_secrets.c
+++ b/core/embed/projects/prodtest/cmd/prodtest_secrets.c
@@ -262,34 +262,26 @@ static void prodtest_secrets_certdev_write(cli_t* cli) {
#ifdef TREZOR_EMULATOR
cli_error(cli, CLI_ERROR, "Not implemented");
#else
- const size_t prefix_length = 2;
size_t certificate_length = 0;
- uint8_t prefixed_certificate[SECRET_MCU_DEVICE_CERT_SIZE] = {0};
- if (!cli_arg_hex(cli, "hex-data", prefixed_certificate + prefix_length,
- sizeof(prefixed_certificate) - prefix_length,
+ uint8_t certificate[MCU_ATTESTATION_MAX_CERT_SIZE] = {0};
+ if (!cli_arg_hex(cli, "hex-data", certificate, sizeof(certificate),
&certificate_length)) {
- if (certificate_length == sizeof(prefixed_certificate) - prefix_length) {
+ if (certificate_length == sizeof(certificate)) {
cli_error(cli, CLI_ERROR, "Certificate too long.");
} else {
cli_error(cli, CLI_ERROR, "Hexadecimal decoding error.");
}
return;
}
- prefixed_certificate[0] = (certificate_length >> 8) & 0xFF;
- prefixed_certificate[1] = certificate_length & 0xFF;
- if (!check_device_cert_chain(cli, &prefixed_certificate[prefix_length],
- certificate_length)) {
+ if (!check_device_cert_chain(cli, certificate, certificate_length)) {
// Error returned by check_device_cert_chain().
return;
}
- secbool result =
- secret_write(prefixed_certificate, SECRET_MCU_DEVICE_CERT_OFFSET,
- sizeof(prefixed_certificate));
-
- if (sectrue != result) {
- cli_error(cli, CLI_ERROR, "secret_write() failed.");
+ if (secret_mcu_device_cert_write(certificate, certificate_length) !=
+ sectrue) {
+ cli_error(cli, CLI_ERROR, "secret_mcu_device_cert_write() failed.");
return;
}
@@ -306,23 +298,16 @@ static void prodtest_secrets_certdev_read(cli_t* cli) {
#ifdef TREZOR_EMULATOR
cli_error(cli, CLI_ERROR, "Not implemented");
#else
- const size_t prefix_length = 2;
- uint8_t prefixed_certificate[SECRET_MCU_DEVICE_CERT_SIZE] = {0};
+ uint8_t certificate[MCU_ATTESTATION_MAX_CERT_SIZE] = {0};
+ size_t certificate_length = 0;
- if (secret_read(prefixed_certificate, SECRET_MCU_DEVICE_CERT_OFFSET,
- sizeof(prefixed_certificate)) != sectrue) {
- cli_error(cli, CLI_ERROR, "`secret_read()` failed.");
+ if (secret_mcu_device_cert_read(certificate, sizeof(certificate),
+ &certificate_length) != sectrue) {
+ cli_error(cli, CLI_ERROR, "secret_mcu_device_cert_read() failed.");
return;
}
- size_t certificate_length =
- prefixed_certificate[0] << 8 | prefixed_certificate[1];
-
- if (certificate_length > sizeof(prefixed_certificate) - prefix_length) {
- cli_error(cli, CLI_ERROR, "Invalid certificate data.");
- return;
- }
- cli_ok_hexdata(cli, prefixed_certificate + prefix_length, certificate_length);
+ cli_ok_hexdata(cli, certificate, certificate_length);
#endif
}
#endif
diff --git a/core/embed/sec/secret/inc/sec/secret.h b/core/embed/sec/secret/inc/sec/secret.h
index 920af316..52b3d112 100644
--- a/core/embed/sec/secret/inc/sec/secret.h
+++ b/core/embed/sec/secret/inc/sec/secret.h
@@ -136,6 +136,41 @@ secbool secret_lock(void);
*/
secbool secret_is_locked(void);
+#ifdef USE_MCU_ATTESTATION
+
+/**
+ * @brief Writes the MCU device certificate to secret storage.
+ *
+ * Encodes the certificate with a 2-byte big-endian length prefix before
+ * writing.
+ *
+ * @param cert Pointer to the certificate data.
+ * @param cert_size Length of the certificate in bytes.
+ * @return secbool sectrue on success, secfalse otherwise.
+ */
+secbool secret_mcu_device_cert_write(const uint8_t* cert, size_t cert_size);
+
+/**
+ * @brief Returns the size of the stored MCU device certificate.
+ *
+ * @param cert_size Pointer to receive the certificate size in bytes.
+ * @return secbool sectrue on success, secfalse otherwise.
+ */
+secbool secret_mcu_device_cert_size(size_t* cert_size);
+
+/**
+ * @brief Reads the MCU device certificate from secret storage.
+ *
+ * @param cert Buffer to receive the certificate data.
+ * @param max_cert_size Size of the buffer.
+ * @param cert_size Pointer to receive the actual certificate size.
+ * @return secbool sectrue on success, secfalse otherwise.
+ */
+secbool secret_mcu_device_cert_read(uint8_t* cert, size_t max_cert_size,
+ size_t* cert_size);
+
+#endif // USE_MCU_ATTESTATION
+
#ifdef LOCKABLE_BOOTLOADER
/**
diff --git a/core/embed/sec/secret/stm32u5/secret.c b/core/embed/sec/secret/stm32u5/secret.c
index ac6b47a3..fe8648d5 100644
--- a/core/embed/sec/secret/stm32u5/secret.c
+++ b/core/embed/sec/secret/stm32u5/secret.c
@@ -649,4 +649,42 @@ void secret_safety_erase(void) {
secret_bhk_regenerate();
}
+#ifdef USE_MCU_ATTESTATION
+
+secbool secret_mcu_device_cert_write(const uint8_t *cert, size_t cert_size) {
+ if (cert_size > SECRET_MCU_DEVICE_CERT_SIZE - 2) {
+ return secfalse;
+ }
+ uint8_t prefixed[SECRET_MCU_DEVICE_CERT_SIZE] = {0};
+ prefixed[0] = (cert_size >> 8) & 0xFF;
+ prefixed[1] = cert_size & 0xFF;
+ memcpy(&prefixed[2], cert, cert_size);
+ return secret_write(prefixed, SECRET_MCU_DEVICE_CERT_OFFSET,
+ sizeof(prefixed));
+}
+
+secbool secret_mcu_device_cert_size(size_t *cert_size) {
+ uint8_t prefix[2] = {0};
+ if (secret_read(prefix, SECRET_MCU_DEVICE_CERT_OFFSET, sizeof(prefix)) !=
+ sectrue) {
+ return secfalse;
+ }
+ *cert_size = prefix[0] << 8 | prefix[1];
+ if (*cert_size > SECRET_MCU_DEVICE_CERT_SIZE - 2) {
+ return secfalse;
+ }
+ return sectrue;
+}
+
+secbool secret_mcu_device_cert_read(uint8_t *cert, size_t max_cert_size,
+ size_t *cert_size) {
+ if (secret_mcu_device_cert_size(cert_size) != sectrue ||
+ *cert_size > max_cert_size) {
+ return secfalse;
+ }
+ return secret_read(cert, SECRET_MCU_DEVICE_CERT_OFFSET + 2, *cert_size);
+}
+
+#endif // USE_MCU_ATTESTATION
+
#endif // SECURE_MODE
Why this scored 18/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.