Skip to content

wolfTPM firmware TPM on PolarFire MPF300 (Mi-V RV32 soft core) - #5

Open
dgarske wants to merge 2 commits into
mainfrom
miv_mpf300_splash
Open

wolfTPM firmware TPM on PolarFire MPF300 (Mi-V RV32 soft core)#5
dgarske wants to merge 2 commits into
mainfrom
miv_mpf300_splash

Conversation

@dgarske

@dgarske dgarske commented Jul 29, 2026

Copy link
Copy Markdown
Member

Adds an example that runs wolfTPM's Firmware TPM (fTPM) in bare-metal on MIV_RV32 RISC-V soft-core built into the fabric of a Microchip PolarFire MPF300 FPGA. The Root of Trust, entropy, and secure storage all live in the FPGA design.

Included

  • fTPM 2.0 served over UART (swtpm/mssim framing), drivable by the wolfTPM host client.
  • Hardware TRNG from the PolarFire System Controller NRBG (CoreSysServices_PF), seeding wolfCrypt's Hash-DRBG.
  • Persistent secure NV in on-die sNVM - TPM state survives power cycles, no external flash.
  • Post-quantum option: ML-DSA (Dilithium) + ML-KEM (Kyber).
  • Companion wolfCrypt test/benchmark (PQC on by default) and a hello-world bring-up.
  • Self-authored wolfSSL-licensed HAL (UART, GPIO, timer, sNVM/NRBG); no vendored BSP.

Validation

Verified on MPF300 Splash Kit hardware: TRNG non-deterministic across cold boots, NV persistence across a real power cycle, and full wolfcrypt_test PASS including ML-DSA/ML-KEM.

Use case: a strong fit for aerospace and other harsh environments. A hardware root of trust that is part of the FPGA you already qualify, with no separate TPM to source or find board space for.

@dgarske dgarske self-assigned this Jul 29, 2026
Copilot AI review requested due to automatic review settings July 29, 2026 22:26

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new bare‑metal example port demonstrating wolfTPM’s firmware TPM (fwTPM) running on a Mi‑V RV32 soft core in a Microchip PolarFire MPF300 FPGA design, including bring‑up firmware, a minimal HAL, host-side UART tooling, and detailed build/validation documentation.

Changes:

  • Add a new Microchip/miv-mpf300-splash/ example with staged bring-up (hello-world, wolfCrypt test/benchmark, fwTPM over UART).
  • Implement a self-authored bare-metal HAL (UART/GPIO/timer/newlib retarget) plus System Controller mailbox access for TRNG (NRBG nonce) and persistent sNVM-backed NV.
  • Add host-side Python clients to drive the UART transport (raw swtpm framing and mssim bridge) and document FPGA/platform setup and performance notes.

Reviewed changes

Copilot reviewed 32 out of 32 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
README.md Adds top-level entry pointing to the new MPF300 Mi‑V fwTPM example.
Microchip/miv-mpf300-splash/README.md Full documentation for staged bring-up, performance notes, TRNG/NV design, and usage.
Microchip/miv-mpf300-splash/fpga/README.md Documents FPGA-side prerequisites and how to build/program the Mi‑V soft-core platform.
Microchip/miv-mpf300-splash/firmware/wolfcrypt-test/user_settings.h wolfCrypt configuration for RV32 bare-metal, including PQC options and footprint tradeoffs.
Microchip/miv-mpf300-splash/firmware/wolfcrypt-test/miv-rv32-ram.ld Linker script for wolfCrypt test/benchmark RAM execution from LSRAM.
Microchip/miv-mpf300-splash/firmware/wolfcrypt-test/Makefile Build system for wolfcrypt_test + benchmark_test on the Mi‑V RV32 target.
Microchip/miv-mpf300-splash/firmware/wolfcrypt-test/main.c Entry point wiring console/time/RNG hooks and running wolfcrypt_test/benchmark_test.
Microchip/miv-mpf300-splash/firmware/hello/miv-rv32-ram.ld Linker script for the minimal hello-world image.
Microchip/miv-mpf300-splash/firmware/hello/Makefile Build system for the hello-world firmware.
Microchip/miv-mpf300-splash/firmware/hello/main.c Hello-world banner + LED heartbeat bring-up app.
Microchip/miv-mpf300-splash/firmware/fwtpm/user_settings.h Combined wolfSSL/wolfTPM settings for fwTPM on Mi‑V RV32, including PQC toggle.
Microchip/miv-mpf300-splash/firmware/fwtpm/miv-rv32-ram.ld Linker script for fwTPM RAM execution from LSRAM.
Microchip/miv-mpf300-splash/firmware/fwtpm/miv_sysserv.h API for CoreSysServices_PF mailbox services (nonce + sNVM read/write).
Microchip/miv-mpf300-splash/firmware/fwtpm/miv_sysserv.c Implementation of mailbox executor + nonce + sNVM page services.
Microchip/miv-mpf300-splash/firmware/fwtpm/Makefile Build system for fwTPM firmware and its platform HAL integrations.
Microchip/miv-mpf300-splash/firmware/fwtpm/main.c fwTPM server initialization, self-test, and UART command loop (raw swtpm + mssim).
Microchip/miv-mpf300-splash/firmware/fwtpm/host-client/swtpm_uart_bridge.py Bridges TCP mssim frames to UART so standard wolfTPM examples can drive the device.
Microchip/miv-mpf300-splash/firmware/fwtpm/host-client/fwtpm_uart_test.py Simple raw-swtpm UART test client (caps/PCR/random).
Microchip/miv-mpf300-splash/firmware/fwtpm/host-client/fwtpm_nv_persist_test.py TPM-level NV persistence test script over UART.
Microchip/miv-mpf300-splash/firmware/fwtpm/fwtpm_rng_sysserv.c TRNG seed hook feeding Hash-DRBG from System Controller nonce service.
Microchip/miv-mpf300-splash/firmware/fwtpm/fwtpm_nv_snvm.c Persistent sNVM-backed NV HAL with RAM shadow and write-through flush.
Microchip/miv-mpf300-splash/firmware/fwtpm/fwtpm_clock_miv.c fwTPM clock HAL using CoreTimer-based millisecond time.
Microchip/miv-mpf300-splash/firmware/common/startup.S Minimal RV32 startup (gp/sp, bss/data init, trap, constructors, main).
Microchip/miv-mpf300-splash/firmware/common/retarget.c Newlib syscall stubs and UART-backed stdout/stderr, plus simple heap.
Microchip/miv-mpf300-splash/firmware/common/miv_uart.h CoreUARTapb minimal polled driver interface.
Microchip/miv-mpf300-splash/firmware/common/miv_uart.c CoreUARTapb minimal polled driver implementation.
Microchip/miv-mpf300-splash/firmware/common/miv_time.h CoreTimer-based timekeeping interface.
Microchip/miv-mpf300-splash/firmware/common/miv_time.c CoreTimer-based monotonic ticks/millis and busy-wait delays.
Microchip/miv-mpf300-splash/firmware/common/miv_gpio.h CoreGPIO minimal output driver interface.
Microchip/miv-mpf300-splash/firmware/common/miv_gpio.c CoreGPIO minimal output driver implementation.
Microchip/miv-mpf300-splash/firmware/common/miv_board.h Board/peripheral base addresses and clock configuration defaults.
Microchip/miv-mpf300-splash/.gitignore Ignores firmware build artifacts and locally-generated FPGA project/job files.
Comments suppressed due to low confidence (1)

Microchip/miv-mpf300-splash/firmware/fwtpm/miv_sysserv.c:228

  • Same double-skip issue as the plaintext read: rdWordOffset=4U plus resp + 4 means the authenticated sNVM read will not return the correct first bytes of the stored page data.
    rc = miv_sysserv_exec(SS_CMD_SNVM_READ, (const uint8_t*)frame, 16U,
                          (uint8_t*)resp, 240U, 0U, 4U);

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread Microchip/miv-mpf300-splash/firmware/common/retarget.c
Comment thread Microchip/miv-mpf300-splash/firmware/fwtpm/miv_sysserv.c
Comment thread Microchip/miv-mpf300-splash/firmware/fwtpm/miv_sysserv.c
Comment thread Microchip/miv-mpf300-splash/firmware/fwtpm/fwtpm_nv_snvm.c Outdated
@dgarske
dgarske force-pushed the miv_mpf300_splash branch from 2d984ed to facf0bd Compare July 30, 2026 16:25

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 32 out of 32 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (8)

Microchip/miv-mpf300-splash/firmware/fwtpm/fwtpm_nv_snvm.c:71

  • When MIV_SNVM_NV_AUTH is enabled, the code falls back to a hard-coded default user secret key (MIV_SNVM_USK). This makes it easy to accidentally ship multiple images that all share the same sNVM authentication key, undermining the protection goal of authenticated-ciphertext mode.
#ifndef MIV_SNVM_USK
#define MIV_SNVM_USK { 0x77, 0x6F, 0x6C, 0x66, 0x54, 0x50, 0x4D, 0x4E, \
                       0x56, 0x6B, 0x65, 0x79 }
#endif
static const uint8_t g_nv_usk[MIV_SNVM_USK_LEN] = MIV_SNVM_USK;

Microchip/miv-mpf300-splash/firmware/fwtpm/fwtpm_nv_snvm.c:52

  • The sNVM NV region geometry (MIV_SNVM_NV_BASE_PAGE + MIV_SNVM_NV_PAGES) is not compile-time validated against the device sNVM page count (221). If a user overrides these macros to an out-of-range region, the code will attempt invalid page accesses and fail at runtime.
#ifndef MIV_SNVM_NV_BASE_PAGE
#define MIV_SNVM_NV_BASE_PAGE   128U
#endif
#ifndef MIV_SNVM_NV_PAGES
#ifdef MIV_FWTPM_PQC

Microchip/miv-mpf300-splash/firmware/fwtpm/user_settings.h:41

  • This bare-metal port doesn’t define XSLEEP_MS, while other bare-metal ports in this repo do. Without XSLEEP_MS, wolfTPM headers may fall back to nanosleep (which is typically unavailable on bare-metal/newlib and is a hard error on newer GCC), making the build toolchain-fragile.
/* ---- Platform (bare-metal, no OS/filesystem) ---- */
#define WOLFCRYPT_ONLY
#define SINGLE_THREADED
#define WOLFSSL_SMALL_STACK
#define WOLFSSL_GENERAL_ALIGNMENT   4
#define SIZEOF_LONG_LONG            8
#define WOLFSSL_USER_IO

Microchip/miv-mpf300-splash/firmware/fwtpm/miv_sysserv.c:38

  • This file conditionally includes the wolfSSL source file wolfcrypt/src/misc.c to get ForceZero(). Since the build already compiles all wolfcrypt/src/*.c, including misc.c here risks duplicate symbol definitions (and makes the build more brittle). Prefer including the public header and linking the compiled object once.
#include <wolfssl/wolfcrypt/settings.h>
#ifdef NO_INLINE
    #include <wolfssl/wolfcrypt/misc.h>   /* ForceZero */
#else
    #define WOLFSSL_MISC_INCLUDED
    #include <wolfcrypt/src/misc.c>       /* ForceZero (inline build) */
#endif

Microchip/miv-mpf300-splash/firmware/fwtpm/fwtpm_rng_sysserv.c:38

  • This file conditionally includes the wolfSSL source file wolfcrypt/src/misc.c to get ForceZero(). Since the build already compiles wolfcrypt/src/*.c, including misc.c here can introduce duplicate definitions and makes the dependency on wolfSSL internals more fragile. Prefer including the public misc.h header.
#include <wolfssl/wolfcrypt/settings.h>
#include <wolfssl/wolfcrypt/error-crypt.h>   /* RNG_FAILURE_E, BAD_FUNC_ARG */
#ifdef NO_INLINE
    #include <wolfssl/wolfcrypt/misc.h>   /* ForceZero */
#else
    #define WOLFSSL_MISC_INCLUDED
    #include <wolfcrypt/src/misc.c>       /* ForceZero (inline build) */
#endif

Microchip/miv-mpf300-splash/firmware/fwtpm/Makefile:13

  • The Makefile header says "GPLv2+", but the source headers in this directory (including user_settings.h) specify GPLv3+. This inconsistency can create licensing/compliance confusion; the Makefile header should match the actual licensing stated in the code it ships with.
# wolfTPM's tpm2_types.h XSLEEP_MS calls nanosleep, which GCC >= 14 rejects as an
# implicit declaration on bare-metal newlib, so SoftConsole is the default here.
#
# Copyright (C) 2006-2026 wolfSSL Inc. GPLv2+ (see source headers).

Microchip/miv-mpf300-splash/firmware/wolfcrypt-test/Makefile:23

  • The Makefile header says "GPLv2+", but the newly added source headers under this example specify GPLv3+. Please align the Makefile’s stated license with the actual license used by the shipped source files to avoid compliance confusion.
# user_settings.h keeps GCC 15's stricter implicit-decl check happy.)
#
# Copyright (C) 2006-2026 wolfSSL Inc. GPLv2+ (see source headers).

Microchip/miv-mpf300-splash/firmware/hello/Makefile:12

  • The Makefile header says "GPLv2+", but the source headers in this new example directory specify GPLv3+. Align the license statement in the Makefile header with the rest of the sources to avoid ambiguity.
# -march on GCC >= 12, hence ARCH = rv32imc_zicsr_zifencei. For the SoftConsole
# GCC 8.3.0 instead: make CROSS_COMPILE=riscv64-unknown-elf- ARCH=rv32imc
#
# Copyright (C) 2006-2026 wolfSSL Inc. GPLv2+ (see source headers).

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 32 out of 32 changed files in this pull request and generated 2 comments.

Suppressed comments (6)

Microchip/miv-mpf300-splash/firmware/fwtpm/miv_sysserv.c:229

  • In authenticated sNVM mode, miv_snvm_write_page_auth() copies the user secret key into the stack frame buffer and returns without scrubbing it. Since this code is part of the TPM’s trusted computing base, it’s better to zeroize the frame after the mailbox transaction completes.
    ((uint8_t*)frame)[0] = page;
    memcpy((uint8_t*)frame + 4, data, MIV_SNVM_PAGE_DATA_AUTH);
    memcpy((uint8_t*)frame + 4 + MIV_SNVM_PAGE_DATA_AUTH, usk, MIV_SNVM_USK_LEN);

    return miv_sysserv_exec(SS_CMD_SNVM_WRITE_C, (const uint8_t*)frame, 252U,

Microchip/miv-mpf300-splash/firmware/fwtpm/miv_sysserv.c:252

  • In authenticated sNVM mode, miv_snvm_read_page_auth() places the user secret key into the stack request frame but does not scrub it (or the response buffer) before returning. Scrubbing these buffers reduces key exposure in memory dumps / post-mortem debug.
    if (rc == 0) {
        memcpy(data, (uint8_t*)resp + 4, MIV_SNVM_PAGE_DATA_AUTH);
    }
    return rc;

Microchip/miv-mpf300-splash/firmware/fwtpm/host-client/swtpm_uart_bridge.py:59

  • ser_read() uses time.time() for timeout measurement. Wall-clock time can jump (NTP, manual adjustments), which can cause spurious timeouts or hangs. Use time.monotonic() for measuring elapsed time.
    t0 = time.time()
    while len(buf) < n:
        c = ser.read(n - len(buf))
        if c:
            buf += c

Microchip/miv-mpf300-splash/firmware/hello/Makefile:11

  • This Makefile header claims “GPLv2+ (see source headers)”, but the newly added source headers in this example use GPLv3-or-later wording. Please avoid hardcoding a conflicting license version in the Makefile comment (or update it to match the actual source headers).
# Copyright (C) 2006-2026 wolfSSL Inc. GPLv2+ (see source headers).

Microchip/miv-mpf300-splash/firmware/wolfcrypt-test/Makefile:22

  • This Makefile header claims “GPLv2+ (see source headers)”, but the newly added source headers in this example use GPLv3-or-later wording. Please avoid hardcoding a conflicting license version in the Makefile comment (or update it to match the actual source headers).
# Copyright (C) 2006-2026 wolfSSL Inc. GPLv2+ (see source headers).

Microchip/miv-mpf300-splash/firmware/fwtpm/Makefile:12

  • This Makefile header claims “GPLv2+ (see source headers)”, but the newly added source headers in this example use GPLv3-or-later wording. Please avoid hardcoding a conflicting license version in the Makefile comment (or update it to match the actual source headers).
# Copyright (C) 2006-2026 wolfSSL Inc. GPLv2+ (see source headers).

Comment on lines +32 to +38
#include <wolfssl/wolfcrypt/settings.h>
#ifdef NO_INLINE
#include <wolfssl/wolfcrypt/misc.h> /* ForceZero */
#else
#define WOLFSSL_MISC_INCLUDED
#include <wolfcrypt/src/misc.c> /* ForceZero (inline build) */
#endif
Comment on lines +31 to +38
#include <wolfssl/wolfcrypt/settings.h>
#include <wolfssl/wolfcrypt/error-crypt.h> /* RNG_FAILURE_E, BAD_FUNC_ARG */
#ifdef NO_INLINE
#include <wolfssl/wolfcrypt/misc.h> /* ForceZero */
#else
#define WOLFSSL_MISC_INCLUDED
#include <wolfcrypt/src/misc.c> /* ForceZero (inline build) */
#endif

@aidangarske aidangarske left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skoll Code Review

Scan type: review-security
Overall recommendation: COMMENT
Findings: 2 total — 2 posted, 0 skipped
2 finding(s) posted as inline comments (see file-level comments below)

Posted findings

  • [Medium] _sbrk() allows the heap cursor to move backwards on negative incrementsMicrochip/miv-mpf300-splash/firmware/common/retarget.c:100-108
  • [Medium] Authenticated sNVM mode ships with a shared default secret keyMicrochip/miv-mpf300-splash/firmware/fwtpm/fwtpm_nv_snvm.c:67-70

Review generated by Skoll

{
char* prev;

if (heap_ptr == NULL) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [Medium] _sbrk() allows the heap cursor to move backwards on negative increments · memory corruption

The _sbrk() implementation only enforces the upper heap bound. Because incr is signed, a negative increment can pass the check and rewind heap_ptr, allowing later allocations to reuse memory that may still be live.

Fix: Reject negative increments or add a lower-bound check against end before updating heap_ptr.

* ciphertext to this chip; provision this app-layer key uniquely and securely
* in production (e.g. fused or PUF-derived). Override the build-time default
* with -DMIV_SNVM_USK='{0x..,...}' so images do not silently share one key. */
#ifndef MIV_SNVM_USK

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [Medium] Authenticated sNVM mode ships with a shared default secret key · crypto

When MIV_SNVM_NV_AUTH is enabled, the code falls back to a compiled-in 12-byte user secret key if no build override is supplied. Images built with this default share the same application-layer authenticated sNVM key.

Fix: Remove the compiled-in fallback secret or force the build to supply a unique device/application key before authenticated sNVM mode can be enabled.

@aidangarske aidangarske left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skoll Code Review

Scan type: review-security
Overall recommendation: REQUEST_CHANGES
Findings: 7 total — 7 posted, 0 skipped
7 finding(s) posted as inline comments (see file-level comments below)

Posted findings

  • [High] Default fwTPM NV storage is plaintext and unauthenticatedMicrochip/miv-mpf300-splash/firmware/fwtpm/fwtpm_nv_snvm.c:54-74,208-240
  • [Medium] Authenticated NV mode has a shared hardcoded application keyMicrochip/miv-mpf300-splash/firmware/fwtpm/fwtpm_nv_snvm.c:63-71
  • [Medium] UART framing can permanently stall the fwTPMMicrochip/miv-mpf300-splash/firmware/fwtpm/main.c:106-117,224-248,299-318
  • [Medium] NV shadow is committed before persistent page writes succeedMicrochip/miv-mpf300-splash/firmware/fwtpm/fwtpm_nv_snvm.c:106-133,148-171
  • [Low] Local bridge accepts unbounded command lengthsMicrochip/miv-mpf300-splash/firmware/fwtpm/host-client/swtpm_uart_bridge.py:89-97
  • [Low] sNVM request and response buffers are not zeroizedMicrochip/miv-mpf300-splash/firmware/fwtpm/miv_sysserv.c:171-252
  • [Low] Mailbox and persistence failure paths lack automated coverageMicrochip/miv-mpf300-splash/firmware/fwtpm/miv_sysserv.c:55-153

Review generated by Skoll

* Controller rejects the write with SNVM_WRITE_NOT_PERMITTED (status 4). */
#ifdef MIV_SNVM_NV_AUTH
#define MIV_SNVM_NV_PAGE_DATA MIV_SNVM_PAGE_DATA_AUTH /* 236 usable/page */
/* User secret key for authenticated sNVM. The device factory key already binds

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [Medium] Authenticated NV mode has a shared hardcoded application key · Security

The new authenticated mode silently falls back to the same literal 12-byte MIV_SNVM_USK for every image unless the build overrides it. The key is compiled into the firmware and is therefore not a secret or device-unique application key. The source comment recommends provisioning a unique key, but the build does not enforce that requirement.

Fix: Remove the fallback key and fail the authenticated build unless a unique key derivation or secure provisioning mechanism is configured.

break
cmd = struct.unpack(">I", hdr)[0]

if cmd == SEND_COMMAND:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 [Low] Local bridge accepts unbounded command lengths · Security

The new localhost bridge trusts the 32-bit mssim command size and calls sock_read(conn, sz) without checking a maximum or applying a socket timeout. A local client can send a SEND_COMMAND frame with a very large size and no body, causing the single-threaded bridge to block indefinitely and deny service to the legitimate client.

Fix: Reject sizes above FWTPM_MAX_COMMAND_SIZE before reading the body and use socket read deadlines for incomplete frames.

return rc;
}

int miv_snvm_write_page(uint8_t page, const uint8_t* data)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 [Low] sNVM request and response buffers are not zeroized · Zeroization

The new sNVM HAL copies full NV pages, including hierarchy secrets and private-key blobs, into automatic frame and resp buffers. Authenticated operations also copy the user secret key into frame. These buffers are returned without clearing on success or error, unlike the nonce path which explicitly uses ForceZero, leaving sensitive copies in stack memory.

Fix: Use ForceZero or an equivalent non-elidable wipe for every sensitive mailbox frame and response on all exit paths.

#define SS_REG_MBX_RDATA 0x2CU /* mailbox read-data FIFO */
#define SS_REG_USER 0x30U /* [0] USER_BUSY, [1] USER_RDVLD */

#define SS_REQ_START 0x01U

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 [Low] Mailbox and persistence failure paths lack automated coverage · Missing Tests

The new mailbox executor and sNVM backend have no automated tests for busy timeouts, controller status failures, blank-page handling, authenticated frame geometry, partial multi-page writes, or recovery after interrupted persistence. The added host scripts exercise only successful TPM exchanges and cannot validate the APB protocol or failure behavior.

Fix: Add mocked mailbox tests and failure-injection tests for the sNVM HAL, including partial writes, power-loss recovery, timeout handling, and authentication failures.

#endif
#endif

/* NV backend mode. Default: plaintext sNVM (validated on this design; needs no

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 [High] Default fwTPM NV storage is plaintext and unauthenticated · Security

Before this PR there was no MPF300 fwTPM NV backend. After it, the default build selects plaintext sNVM and sets get_integrity_key = NULL. The fwTPM journal therefore has no platform integrity key, while SnvmLoadShadow accepts successful page contents without authentication. Persistent hierarchy seeds, authorization values, and private-key blobs can be read or tampered with by code that obtains sNVM service access. Authenticated storage is only an opt-in build flag.

Fix: Make authenticated, per-device-provisioned storage mandatory for production fwTPM builds. Supply a device-derived journal integrity key and fail initialization rather than silently running with plaintext NV.


Note: Referenced line (Microchip/miv-mpf300-splash/firmware/fwtpm/fwtpm_nv_snvm.c:54-74,208-240) is outside the diff hunk. Comment anchored to nearest changed region.

#define MSSIM_STOP 21

/* ---- raw UART byte transport over CoreUARTapb ---- */
static int UartRecv(uint8_t* buf, uint32_t sz)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [Medium] UART framing can permanently stall the fwTPM · Security

Before this PR there was no MPF300 UART command loop. After it, a sender can transmit a valid raw or mssim header with a command size within the accepted maximum and then stop sending. UartRecv blocks forever waiting for the remaining bytes, with no inter-byte or frame deadline, so the device stops servicing all subsequent TPM commands. UartDrain only handles oversized frames and can also be kept busy by a continuous stream.

Fix: Add an inter-byte/frame timeout and reset the parser on expiration. Bound malformed-frame draining and return a transport error without blocking indefinitely.


Note: Referenced line (Microchip/miv-mpf300-splash/firmware/fwtpm/main.c:106-117,224-248,299-318) is outside the diff hunk. Comment anchored to nearest changed region.

#endif
}

/* Program the sNVM pages spanning shadow bytes [first, last] from g_nv.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [Medium] NV shadow is committed before persistent page writes succeed · Logic

The new backend copies data into g_nv before SnvmFlushRange writes pages to sNVM. If a later page write fails, the HAL returns an error but the RAM shadow remains ahead of persistent storage; subsequent reads expose data that will disappear or differ after reboot. Power loss during a multi-page update has the same split-state behavior, and there is no generation or rollback mechanism.

Fix: Stage updates separately and commit the shadow only after all page writes succeed, or use a transactional/append-only journal with recovery and refuse further commands after an unrecoverable persistence failure.


Note: Referenced line (Microchip/miv-mpf300-splash/firmware/fwtpm/fwtpm_nv_snvm.c:106-133,148-171) is outside the diff hunk. Comment anchored to nearest changed region.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants