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

[dgarske]

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]

🟡 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.

[Copilot]

🟡 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

[Copilot]

🟡 Changes recommended

Unresolved findings affect command-size validation, timeout behavior, capability handling, and example success/error paths.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (6)

examples/native/native_test.c:906

• [Medium] (CWE-390) As above, a failure to query PCR capabilities is reported as “no SHA-1 PCR bank” and the native test continues. That hides an actual TPM/transport failure; only a successful query with isAllocated == 0 should take the skip branch.

    if (TPM2_IsPcrBankAllocated(TPM_ALG_SHA1, pcrIndex, &isAllocated) != 0 ||
            !isAllocated) {
        printf("TPM2_PolicyPCR: SHA-1 skipped (no SHA-1 PCR bank allocated)\n");

examples/wrap/wrap_test.c:293

• [Medium][CWE-390] NVStoreKey returning TPM_RC_DISABLED means the persistent write did not happen, but this new condition treats it as success and prints that the key was created at the persistent handle. The next invocation will not find that handle, so keep TPM_RC_DISABLED out of this persistence check or explicitly report and handle a transient-only key.

        if (!WOLFTPM_IS_COMMAND_UNAVAILABLE_OR_DISABLED(rc) && rc != 0) goto exit;

examples/wrap/wrap_test.c:595

• [Medium][CWE-390] The ECC NVStoreKey path has the same regression: TPM_RC_DISABLED is accepted even though no persistent object was written, followed by a misleading "Created new ... at" message. Restrict the disabled-command handling to the EncryptDecrypt feature checks or explicitly handle the non-persistent case here.

        if (!WOLFTPM_IS_COMMAND_UNAVAILABLE_OR_DISABLED(rc) && rc != 0) goto exit;

src/tpm2.c:7877

• [Medium] HASH_COUNT is the local compile-time TPML capacity, not a guarantee that the TPM has no more PCR banks. On a build where it is 2, a TPM exposing SHA-1/SHA-256/SHA-384 can set moreData and the parser truncates the capability list before this loop, so this public helper can report an allocated SHA-384/512 bank as absent. The "all assigned banks" query needs pagination/non-truncating inspection, or the API must report an incomplete query explicitly.

    in.propertyCount = HASH_COUNT; /* all assigned banks */

src/tpm2_tis.c:476

• [Medium] (CWE-682) When TPM_TIMEOUT_TRIES is 0 and the millisecond clock has not advanced between TimeoutStart and the first poll, this returns TPM_RC_TIMEOUT after a single poll. A coarse millisecond clock plus a 10us wait commonly produces elapsed == 0, so a command can fail immediately instead of receiving the full TPM_TIMEOUT_MS budget. Once a clock is active, the iteration backstop must not fire on an unchanged sample; only use it when clock failure is actually detected.

        return (to->tries <= 0 && elapsed == 0) ? 1 : 0;

src/tpm2_wrap.c:6275

• [Medium] (CWE-754) This new preflight returns BUFFER_E for oversized ML-DSA signatures, but the existing examples/bench/bench.c caller treats BUFFER_E as a hard verification failure rather than an unsupported-operation skip. On the 1024-byte-input-buffer TPM described by this PR, bench_pqc_mldsa() therefore aborts at its verify row instead of completing or skipping it, contrary to the reported hardware result. Handle this result as an explicit skip in the callers or do not add the check to this API without updating them.

    rc = wolfTPM2_CheckSigInputBuffer(sigSz);
    if (rc != TPM_RC_SUCCESS) {
        return rc;
    }

• Files reviewed: 8/8 changed files
• Comments generated: 4
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Unresolved timeout, signature validation, and caller-handling issues must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (4)

src/tpm2_wrap.c:5795

• TPM_PT_INPUT_BUFFER limits the serialized command, not just the signature field. For a TPM reporting 1024, a 900-byte signature passes this sigSz > inputBuffer test even though the command also contains headers, handles/auth, digest, and the signature encoding; it can still exceed the limit and hit the same wedge this helper is meant to prevent. Compare the signature plus overhead here (or compute the exact packet size).

    if ((UINT32)sigSz > inputBuffer) {
    #ifdef DEBUG_WOLFTPM
        printf("TPM2_VerifySignature: signature %d bytes exceeds the TPM's "
            "%u byte input buffer\n", sigSz, (unsigned int)inputBuffer);
    #endif
        return BUFFER_E;
    }

src/tpm2_wrap.c:5806

• [Medium][CWE-754] Unlike the input-buffer query above, failures and malformed responses from the TPM_PT_MAX_COMMAND_SIZE query are silently ignored and the helper returns success. If this is the tighter limit for a command, the oversized signature is still sent, so the preflight is fail-open; return the query error or TPM_RC_VALUE when the property cannot be validated.

    rc = TPM2_GetCapability(&in, &out);
    if (rc == TPM_RC_SUCCESS &&
            out.capabilityData.capability == TPM_CAP_TPM_PROPERTIES) {

wolftpm/tpm2_types.h:751

• [Medium][CWE-755] A later clock_gettime() failure returns 0 here, but the timeout code only treats a zero value at start as unavailable. Once start is nonzero, the next subtraction wraps to a huge elapsed value and the wait returns TPM_RC_TIMEOUT immediately instead of falling back to the counter as documented. Preserve a validity/error indication from the clock hook and switch to the fallback path on any failed read.

                if (clock_gettime(CLOCK_MONOTONIC, &ts) != 0) {
                    return 0;

wolftpm/tpm2_types.h:766

• [Medium] The PR description says the default TPM_TIMEOUT_MS is 60000 ms, but this defines 180000 ms. That changes the failure bound by 3x on monotonic-clock builds; align the code and description, or document why 180 seconds is the intended default.

#ifndef TPM_TIMEOUT_MS
#define TPM_TIMEOUT_MS 180000

• Files reviewed: 8/8 changed files
• Comments generated: 2
• Review effort level: Lite