Skip to content

Handle TPMs that lack SHA-1, disable EncryptDecrypt, or run a command longer than the TIS wait budget - #607

Open
dgarske wants to merge 4 commits into
wolfSSL:masterfrom
dgarske:tpm_robustness_fixes
Open

Handle TPMs that lack SHA-1, disable EncryptDecrypt, or run a command longer than the TIS wait budget#607
dgarske wants to merge 4 commits into
wolfSSL:masterfrom
dgarske:tpm_robustness_fixes

Conversation

@dgarske

@dgarske dgarske commented Sep 12, 2026

Copy link
Copy Markdown
Member

Four independent robustness fixes, found running the stock examples against a current SPI TPM on a Raspberry Pi 5.

  • Oversized signatures no longer hang the TPM. wolfTPM2_VerifyHashTicket() and wolfTPM2_VerifyDigestSignature() now check the signature against TPM_PT_INPUT_BUFFER and return BUFFER_E before sending, rather than leaving the TPM to discover it; post-quantum signatures exceed that cap routinely and not every TPM rejects the oversized parameter cleanly, with at least one ceasing to respond until a hardware reset. The capability read is skipped at or below the 1024-byte TPM_MIN_INPUT_BUFFER floor, so RSA and ECC verifies cost no extra round trip.

  • TIS waits are bounded by real time instead of an iteration count. TPM2_TIS_WaitForStatus() and TPM2_TIS_GetBurstCount() now use TPM_TIMEOUT_MS (default 60000) through a new XTPM_GET_TIMEMS(), so the budget no longer varies with host speed and scheduler; a roughly 21 second RSA key generation previously timed out intermittently on the same command that succeeded moments earlier. Ports without a monotonic clock keep the existing counter unchanged.

  • Examples skip SHA-1 on TPMs that do not implement it. wrap_test and native_test carried hard-coded SHA-1 test vectors and PCR bank selections that abort the run with TPM_RC_HASH on an increasing share of current parts; all three sites now query TPM_CAP_ALGS first and skip with the reason printed.

  • native_test tolerates EncryptDecrypt being disabled rather than absent. It already skipped the test on TPM_RC_COMMAND_CODE, but a TPM that implements the command and ships it switched off answers TPM_RC_DISABLED; both codes now go through one helper, and the second call site gains the response-code masking it was missing.

Hardware / test status

Validated on a Raspberry Pi 5 driving an SPI TPM over spidev with no kernel TPM driver in the path, against a part that exercises all four paths: no SHA-1, TPM2_EncryptDecrypt disabled, a 1024-byte TPM_PT_INPUT_BUFFER, and roughly 21 second RSA-2048 key generation. native_test now runs to completion where it previously aborted at the first SHA-1 PCR read, and examples/bench/bench completes its post-quantum rows instead of hanging partway.

Also run against the firmware TPM and the simulator with no change in results: make check, the SPDM suite in both TCG and PSK modes, and the tpm2-tools compatibility suite. The no-clock fallback is compile-verified with -DWOLFTPM_NO_MONOTONIC_MS.

Scope

TPM_TIMEOUT_MS is a single global budget rather than a per-command duration derived from the TPM's own TPM_PT_* timeout and duration properties. That is the more correct answer and is left as follow-on work.

Copilot AI lite review requested due to automatic review settings September 12, 2026 17:09

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.

🟡 Changes recommended

Unresolved correctness, validation, coverage, and portability issues remain.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Improves TPM compatibility for oversized signatures, long-running commands, missing SHA-1 support, and disabled EncryptDecrypt commands.

Changes:

  • Adds signature input-buffer checks.
  • Uses real-time TIS wait budgets with fallback behavior.
  • Adds capability-aware handling in example tests.
File summaries
File Description
wolftpm/tpm2.h Defines the minimum TPM input-buffer size.
wolftpm/tpm2_types.h Adds timing hooks and timeout configuration.
src/tpm2_wrap.c Preflights signature sizes before verification.
src/tpm2_tis.c Applies elapsed-time TIS wait budgets.
examples/wrap/wrap_test.c Gates SHA-1-dependent verification by capability.
examples/pqc/pqc_ctrl.c Handles oversized Hash-ML-DSA verification.
examples/native/native_test.c Handles SHA-1 availability and disabled EncryptDecrypt paths.
Review details

Suppressed comments (4)

examples/native/native_test.c:118

  • [Medium][CWE-703] Returning 0 for every TPM2_GetCapability failure makes the callers print that SHA-1 is unsupported and continue. A transport error or malformed capability response can therefore skip both PCR tests and still reach Native test passed; only a successful query with no matching algorithm should be treated as an unsupported optional feature. Preserve the query error separately from the support boolean.
    if (rc != TPM_RC_SUCCESS) {
        return 0;

examples/native/native_test.c:1721

  • [Medium][CWE-754] If the second call returns TPM_RC_COMMAND_CODE or TPM_RC_DISABLED, this branch clears rc but leaves perform_EncryptDecrypt2 true and falls through to the comparison at lines 1730-1743. TPM2_EncryptDecrypt2() only populates cmdOut on success (src/tpm2.c:2770-2781), so the previous output is treated as decrypted data and the example reports TPM_RC_TESTING instead of skipping; preserve the response code through the skip or bypass that comparison.
        if (native_is_cmd_unavailable_or_disabled(rc)) { /* unsupported or disabled */
            printf("TPM2_EncryptDecrypt2: Is not a supported feature without enabling due to export controls\n");
            rc = 0;

src/tpm2_wrap.c:5777

  • [Medium][CWE-130] TPM_PT_INPUT_BUFFER is the maximum serialized command size, not a per-parameter limit. Treating sigSz <= TPM_MIN_INPUT_BUFFER as automatically safe can still send an oversized packet—for example, VerifyDigestSignature with contextSz=255 and sigSz=1024 already exceeds a 1024-byte input buffer, and a cap just above a valid PQ signature has the same issue. Compare the complete command size or reserve the fixed handle/digest/context/signature overhead before taking this shortcut.
    if (sigSz <= TPM_MIN_INPUT_BUFFER) {
        return TPM_RC_SUCCESS;

wolftpm/tpm2_types.h:734

  • [Medium] The generic FREERTOS arm expands xTaskGetTickCount() and portTICK_PERIOD_MS, but this header does not include the FreeRTOS/task headers. src/tpm2_tis.c includes only tpm2_tis.h and now necessarily expands this macro, so a normal -DFREERTOS library build can fail with undefined identifiers unless every integrator pre-includes platform headers; include the required headers or require an XTPM_GET_TIMEMS override.
    #elif defined(WOLFSSL_ESPIDF) || defined(FREERTOS)
        #define XTPM_GET_TIMEMS() \
            ((word32)xTaskGetTickCount() * (word32)portTICK_PERIOD_MS)
  • Files reviewed: 7/7 changed files
  • Comments generated: 6
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/tpm2_wrap.c
Comment thread examples/native/native_test.c
Comment thread examples/pqc/pqc_ctrl.c
Comment thread src/tpm2_wrap.c
Comment thread wolftpm/tpm2_types.h Outdated
Comment thread wolftpm/tpm2_types.h

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.

🟡 Changes recommended

Unresolved validation, fallback, and example-correctness issues remain.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (6)

examples/native/native_test.c:115

  • [Medium, CWE-20] TPM_CAP_ALGS only says that the TPM implements SHA-1; it does not say that a SHA-1 PCR bank is allocated. A TPM with SHA-1 crypto support but no SHA-1 PCR selection still takes both new branches and issues PCR_Read/PolicyPCR, which can fail and abort the test. Query TPM_CAP_PCRS and verify that the requested PCR is present in the SHA-1 selection, as test_pcr_bank_allocated does.
    in.capability = TPM_CAP_ALGS;
    in.property = alg;
    in.propertyCount = 1;

examples/native/native_test.c:118

  • [Low] A failed capability query is silently converted into “SHA-1 unsupported” (CWE-390). A transport or TPM error will therefore be printed as a skip and the native test can continue and report success instead of exposing the failure. Preserve the query return code separately from the unsupported result, as wolfTPM2_IsAlgSupported() already does, and abort/report on query failure.
    rc = TPM2_GetCapability(&in, &out);
    if (rc != TPM_RC_SUCCESS) {
        return 0;

examples/pqc/pqc_ctrl.c:505

  • [Medium, CWE-754] This branch reports PASS and returns success without verifying sig; the referenced examples/pqc/mldsa_host_verify helper is not present in the tree. A bad signature or signing regression therefore passes the example whenever the TPM input buffer is too small. Perform an actual host verification before returning success, or report this result as skipped/non-verified.
        printf("PASS  HashML-DSA-%-3s  signdigest (sig %d bytes); on-TPM "
            "verify unavailable, signature exceeds input buffer\n",
            mldsaName(ps), sigSz);
        rc = TPM_RC_SUCCESS;
        goto exit_quiet;

src/tpm2_tis.c:456

  • [Medium] The fallback is only selected when the start tick is zero; if XTPM_GET_TIMEMS() fails after a nonzero start, the implementation returns zero and unsigned subtraction wraps, making the wait expire immediately instead of falling back to the counter (CWE-834). Preserve the last valid reading or make the clock API report validity on every call before applying the elapsed-time check.
    if (to->haveStart) {
        /* unsigned subtraction stays correct across the word32 wrap */
        return ((word32)(XTPM_GET_TIMEMS() - to->start) >= TPM_TIMEOUT_MS) ?
            1 : 0;

src/tpm2_wrap.c:5824

  • [High, CWE-400] This guard covers only the two one-shot verification wrappers. wolfTPM2_VerifySequenceComplete() also appends a TPMT_SIGNATURE and is used by the PQC example and benchmark with 2420–4627-byte ML-DSA signatures; on a TPM reporting a 1024-byte input buffer it can still send the oversized command and trigger the hang this check is meant to prevent. Apply the same preflight to that wrapper or centralize it across the verification paths.
    rc = wolfTPM2_CheckSigInputBuffer(sigSz);
    if (rc != TPM_RC_SUCCESS) {
        return rc;
    }

wolftpm/tpm2_types.h:728

  • [Medium, CWE-670] When a port supplies XTPM_GET_TIMEMS, this outer guard skips the auto-detection block but never defines WOLFTPM_HAVE_MONOTONIC_MS; the TIS timeout helpers therefore compile to the old iteration-only path. The advertised custom clock hook is ineffective, including for freestanding users that provide their own clock. Treat a user-supplied hook as enabling the monotonic path before the standard-header guard, or explicitly require and document the second macro.
#if !defined(XTPM_GET_TIMEMS) && !defined(WOLFTPM_NO_MONOTONIC_MS) && \
    !defined(WOLFTPM_NO_STD_HEADERS)
  • Files reviewed: 7/7 changed files
  • Comments generated: 4
  • Review effort level: Lite

Comment thread src/tpm2_wrap.c
Comment on lines +5792 to +5793
inputBuffer = out.capabilityData.data.tpmProperties.tpmProperty[0].value;
if (inputBuffer > 0 && (UINT32)sigSz > inputBuffer) {
Comment thread wolftpm/tpm2_types.h
Comment on lines +741 to +743
#elif defined(CLOCK_MONOTONIC) || defined(__linux__)
#include <time.h>
static inline word32 XTPM_GET_TIMEMS(void)
Comment on lines +1719 to +1721
if (native_is_cmd_unavailable_or_disabled(rc)) { /* unsupported or disabled */
printf("TPM2_EncryptDecrypt2: Is not a supported feature without enabling due to export controls\n");
rc = 0;
Comment thread src/tpm2_wrap.c
Comment on lines +5786 to +5787
if (rc != TPM_RC_SUCCESS ||
out.capabilityData.data.tpmProperties.count == 0) {
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.

2 participants