Fix wolfCrypt refcount race P521 primary derivation and policy session auth bypass

[aidangarske]

F-9161, F-9162, F-9187

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Pull request overview

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

Suppressed comments (3)

src/tpm2.c:753

• Severity: Medium (CWE-362). In TPM2_WolfCrypt_Cleanup, if wc_LockMutex(&gHwLock) fails, the function continues and updates gWolfCryptRefCount (and may call wolfCrypt_Cleanup) without synchronization, reintroducing the refcount race in the error path. Prefer failing closed here (return early) rather than proceeding unlocked.

#if !defined(WOLFTPM_NO_LOCK) && !defined(SINGLE_THREADED) && \
    defined(WOLFSSL_MUTEX_INITIALIZER)
    int locked = (wc_LockMutex(&gHwLock) == 0);
#endif

src/tpm2.c:699

• Severity: Medium (CWE-362). The wolfCrypt init/cleanup refcount is only mutex-protected when WOLFSSL_MUTEX_INITIALIZER is defined. In multi-threaded builds where that macro is not defined, gWolfCryptRefCount is still incremented/decremented without any synchronization, so the refcount race this PR aims to fix can still occur (double-init/double-cleanup). Consider either adding a thread-safe refcount mechanism for that configuration (e.g., an atomic/once primitive) or explicitly disallowing multi-threaded builds without a statically-initialized mutex.

#if !defined(WOLFTPM_NO_LOCK) && !defined(SINGLE_THREADED) && \
    defined(WOLFSSL_MUTEX_INITIALIZER)
    /* gHwLock is statically initialized, so it can guard the reference count
     * before wolfCrypt is initialized */
    if (wc_LockMutex(&gHwLock) != 0)
        return TPM_RC_FAILURE;
#endif

src/fwtpm/fwtpm_command.c:16888

• Consider adding a unit test that exercises this new policy-session failure-closed path (e.g., attempt to authorize a handle whose authPolicy cannot be resolved with a TPM_SE_POLICY session and empty HMAC, and assert TPM_RC_POLICY_FAIL). This helps prevent regressions of the auth-bypass class this change is addressing.

            else if (authPolicy == NULL) {
                /* A policy session cannot authorize a handle whose authPolicy
                 * cannot be resolved (for example hash/sign sequence handles);
                 * fail closed per TPM 2.0 Part 1 Sec. 19.7. */
            #ifdef DEBUG_WOLFTPM
                printf("fwTPM: Policy session rejected for handle 0x%x with "
                    "unresolved authPolicy (CC=0x%x)\n", entityH, cmdCode);
            #endif
                *rspSize = FwBuildErrorResponse(rspBuf, rspCap,
                    TPM_ST_NO_SESSIONS, TPM_RC_POLICY_FAIL);
                return TPM_RC_SUCCESS;
            }