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

[dgarske]

Six independent robustness fixes plus a benchmark mode, found running the stock examples against current SPI TPMs 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 300000) through a new XTPM_GET_TIMEMS(), so the budget no longer varies with host speed and scheduler; RSA-2048 key generation is an unbounded prime search whose duration varies by an order of magnitude for the same command, so a fixed iteration count timed out intermittently on runs that succeeded moments earlier. Ports without a monotonic clock keep the existing counter unchanged, and TPM_TIMEOUT_TRIES of zero keeps its "no cap" meaning on both paths.

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

• Examples tolerate EncryptDecrypt being disabled rather than absent. They 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 call sites that were missing the response-code masking gain it.

• ASN error codes no longer collide with TPM response codes. The ASN parser's internal error values shared the numeric space with TPM_RC_*, so a parse failure could be read as a TPM response. They are renamed and re-based, with deprecated aliases kept behind WOLFTPM_NO_DEPRECATED_ASN_RC so existing callers continue to build.

• New wolfTPM2_AllocatePCRBanks() for changing which PCR banks a TPM allocates, with examples/pcr/allocate to report and re-provision them. Parts that keep a single bank active need this to move between SHA-256 and SHA-384. Two details are worth flagging for anyone building the selection themselves:

  • TPM 2.0 Part 3 section 22.5 says a bank omitted from the TPML_PCR_SELECTION is deallocated, but Infineon parts answer TPM_RC_PCR unless every bank is named, with an all-zero pcrSelect bitmap on the ones being dropped. The wrapper mirrors the shape TPM_CAP_PCRS reports, which is the form those parts accept.
  • TPM_CAP_ALGS is not authoritative for what can be allocated; a part may advertise a hash there and have no PCR bank for it. The bank list from TPM_CAP_PCRS is used instead, so an unusable bank is rejected before the command is sent rather than silently ignored by the TPM.

  The change is staged and takes effect at the next TPM2_Startup(CLEAR) following a _TPM_Init, which no command can trigger, so the caller must power cycle. The firmware TPM gains the matching fixes: it no longer accepts a selection that would leave it with no banks at all, it honours the pcrSelect bitmap so a bank can actually be deallocated, and it no longer reports every bank as unallocated after a restart against an existing NV file.

• examples/bench/bench gains a fixed-iteration mode. -iter=[n] runs each algorithm exactly n times and reports the average with the per-iteration minimum and maximum, instead of running for a wall-clock budget. The existing duration mode is unchanged and remains the default. Operations whose cost varies run to run, such as RSA key generation and post-quantum signing, produce a single misleading figure under a short duration; the spread makes that visible.

Hardware / test status

Validated on a Raspberry Pi 5 driving SPI TPMs over spidev with no kernel TPM driver in the path, against parts that exercise every path: no SHA-1, TPM2_EncryptDecrypt disabled, a 1024-byte TPM_PT_INPUT_BUFFER, and RSA-2048 key generation whose duration varies by an order of magnitude run to run. native_test now runs to completion where it previously aborted at the first SHA-1 PCR read, examples/wrap/encrypt_decrypt skips cleanly instead of failing, and examples/bench/bench completes its post-quantum rows instead of hanging partway. A full SHA-256 to SHA-384 bank switch and back was performed on hardware across power cycles.

Also run against the firmware TPM and the simulator with no change in results: make check, run_examples.sh, 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, and both settings of TPM_TIMEOUT_TRIES are compile-verified.

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. The default is a tail estimate for the slowest command rather than a measured bound, since RSA key generation has no upper limit; a per-command duration would remove the guesswork.

wolfTPM2_AllocatePCRBanks() allocates each bank across all implemented PCRs rather than exposing per-PCR granularity, and deliberately refuses a zero-length selection, which is a legal command that would leave the TPM with no PCRs at all.

[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

[Copilot]

🟡 Changes recommended

Unresolved findings affect PQC benchmarking, SHA-1 handling, TIS timeout semantics, and signature-size validation.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (4)

examples/bench/bench.c:166

• The new signature-size guard returns BUFFER_E for ML-DSA verification when the signature exceeds the TPM input buffer, but this helper does not classify BUFFER_E as a skippable unsupported operation. The ML-DSA verify loop therefore reaches bench_asym_done with a hard error and aborts the benchmark, despite the PR's claimed completion of post-quantum rows. Handle this specific BUFFER_E in the PQC verify path (or otherwise classify only this input-buffer case) before treating it as a benchmark failure.

    return ((rc & 0xBF) == TPM_RC_SCHEME) ||
        WOLFTPM_IS_COMMAND_UNAVAILABLE_OR_DISABLED(rc);

examples/native/native_test.c:870

• This only guards the later standalone SHA-1 read/policy tests. On WOLFTPM_MICROCHIP, TEST_WRAP_DIGEST is also hard-coded to TPM_ALG_SHA1 (examples/tpm_test.h:105-110), and the earlier PCR read/extend/reset loop runs before this block (native_test.c:714-795), so a TPM without SHA-1 still aborts before reaching this new check. The SHA-1 availability check needs to cover that earlier TEST_WRAP_DIGEST path as well, or the example remains non-robust for that supported configuration.

    /* Many current TPMs allocate no SHA-1 bank; ask before selecting it. A
     * query failure is reported, not silently treated as "no bank". */
    pcrIndex = 0;
    rc = TPM2_IsPcrBankAllocated(TPM_ALG_SHA1, pcrIndex, &isAllocated);

src/tpm2_tis.c:478

• The real-time budget is still bounded by TPM_TIMEOUT_TRIES whenever the clock has not advanced during the first tries polls. With a configured nonzero value and a fast/no-sleep port, a legitimate command can time out after the iteration cap long before TPM_TIMEOUT_MS; this is not distinguishable from a merely slow clock tick and contradicts the stated real-time timeout behavior. Do not apply the iteration cap on monotonic-clock ports, or use a separate clock-failure signal rather than elapsed == 0.

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

src/tpm2_wrap.c:5835

• [High] This compares only the signature payload against TPM_PT_INPUT_BUFFER, but that property bounds the complete serialized command input. For example, a 900-byte signature on a TPM reporting 1024 bytes passes here even though the documented 176-byte command overhead makes the command at least 1076 bytes; it is then sent despite the new hang-prevention goal. Include TPM_SIG_CMD_OVERHEAD in this comparison (with the same overflow-safe arithmetic used for the other size checks). CWE-130.

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

• Files reviewed: 25/26 changed files
• Comments generated: 1
• Review effort level: Lite

[aidangarske]

Skoll Code Review

Scan type: review

Overall recommendation: REQUEST_CHANGES
Findings: 8 total — 8 posted, 0 skipped
8 finding(s) posted as inline comments (see file-level comments below)

Posted findings

• [High] Signature guard can still submit an oversized command — src/tpm2_wrap.c:5774-5863
• [High] PCR allocation discards the caller session's updated nonce — src/tpm2_wrap.c:7612-7637
• [High] A clock that stalls after advancing disables every timeout — src/tpm2_tis.c:448-481
• [High] Truncated PCR capability results are treated as complete — src/tpm2.c:7976-8001
• [High] The restore option can silently drop existing PCR banks — examples/pcr/allocate.c:42-109
• [High] The default example suite performs destructive PCR reprovisioning — examples/run_examples.sh:893-911
• [Medium] Fixed-iteration spread is omitted from symmetric rows — examples/bench/bench.c:88-138
• [Medium] Iteration mode persists across benchmark invocations — examples/bench/bench.c:419-460

Review generated by Skoll