Add caller-supplied policy authorization for firmware upgrade

[dgarske]

Summary

Lets callers authorize the firmware-upgrade start command with their own platform-hierarchy policy (multi-branch PolicyOR, SHA-256/384/512) instead of the fixed library authorization. Backward compatible - no existing API changes.

Features

• New _ex APIs taking a caller-satisfied session: wolfTPM2_FirmwareUpgradeHash_ex, wolfTPM2_FirmwareUpgrade_ex, wolfTPM2_FirmwareUpgradeRecover_ex; the existing calls forward with a NULL session.
• New public wrappers: wolfTPM2_PolicyOR, wolfTPM2_SetPrimaryPolicy, wolfTPM2_PolicyCommandCodeMake, wolfTPM2_IsAlgSupported.
• Infineon: a caller session leaves the platform primary policy untouched; ST33: it replaces the default TPM_RS_PW.
• Firmware examples: non-destructive --policytest self-test and --policy/--policyor [--sha256|--sha384|--sha512] end-to-end modes; shared helpers factored into examples/firmware/firmware_policy.c.

Fixes

• wolfTPM2_PolicyOR validates branch count and each branch digest size against its buffer (out-of-bounds read, CWE-125).
• Examples skip a policy hash the TPM does not implement (via TPM_CAP_ALGS) instead of failing with TPM_RC_SIZE.
• Example arg parsing rejects unknown/extra options; the caller session is released only when not consumed by a started upgrade; a provisioned platform policy is cleared on the failure path.
• CMake builds the new shared example source.

[Copilot]

Pull request overview

This PR extends wolfTPM’s firmware-upgrade wrappers to support caller-supplied authorization sessions (e.g., policy sessions) for the vendor “firmware start” command, enabling platforms that gate upgrade behind custom platform hierarchy policies.

Changes:

• Added a wolfTPM2_PolicyOR() wrapper to satisfy policy sessions via TPM2_PolicyOR.
• Added wolfTPM2_FirmwareUpgradeHash_ex(..., startSession) and routed the legacy wolfTPM2_FirmwareUpgradeHash() through it (preserving existing behavior when startSession == NULL).
• Updated the ST33 firmware update example and documentation to demonstrate/describe policy-based authorization and a safe self-test flow.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
wolftpm/tpm2_wrap.h	Declares new wolfTPM2_PolicyOR wrapper and wolfTPM2_FirmwareUpgradeHash_ex API with caller-supplied session support.
src/tpm2_wrap.c	Implements wolfTPM2_PolicyOR and adds the _ex firmware upgrade routing + vendor start-session plumbing.
examples/firmware/st33_fw_update.c	Adds --policytest self-test to validate policy-session + PolicyOR behavior without performing an upgrade.
examples/firmware/README.md	Documents advanced policy-based firmware start authorization and the new _ex API usage.

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

Suppressed comments (5)

src/tpm2_wrap.c:11213

• Severity: Critical (CWE-121/CWE-476). tpm2_ifx_firmware_start() builds a command in a fixed-size stack buffer (TPM_SHA512_DIGEST_SIZE) and copies manifest_hash using manifest_hash_sz without validating it. Through the public wolfTPM2_FirmwareUpgradeHash_ex() API a caller can pass a larger size (or a non-NULL size with NULL manifest_hash), causing a stack overflow or NULL dereference. Validate manifest_hash pointer and size against hashAlg before constructing the command.

    int rc;
    WOLFTPM2_SESSION tpmSession;
    TPM_HANDLE sessionHandle = TPM_RH_NULL;
    int ownSession = 0;

    XMEMSET(&tpmSession, 0, sizeof(tpmSession));

src/tpm2_wrap.c:10739

• Severity: Medium (CWE-20). wolfTPM2_SetPrimaryPolicy() accepts authPolicySz > 0 with authPolicy == NULL, which would silently send an empty policy digest (size=0) while still setting hashAlg. This is an argument-validation bug that can result in unexpected policy changes or confusing TPM errors; fail fast when a non-zero size is provided without a buffer (and when clearing, enforce hashAlg==TPM_ALG_NULL implies authPolicySz==0).

This issue also appears on line 11207 of the same file.

    if (dev == NULL) {
        return BAD_FUNC_ARG;
    }
    if (authPolicySz > (word32)sizeof(in.authPolicy.buffer)) {
        return BAD_FUNC_ARG;
    }

examples/firmware/firmware_policy.c:184

• Severity: Low (CWE-476). firmware_policy_session_setup() dereferences dev and session without checking for NULL (it calls wolfTPM2_IsAlgSupported(dev, ...) and XMEMSET(session,...)). Even though this is example code, it’s a shared helper and should fail cleanly with BAD_FUNC_ARG on NULL inputs.

    if (hsz == 0 || hsz > TPM_MAX_DIGEST_SIZE) {
        return BAD_FUNC_ARG;
    }
    XMEMSET(session, 0, sizeof(*session));
    XMEMSET(&orList, 0, sizeof(orList));

examples/firmware/st33_fw_update.c:425

• Severity: Low (CWE-772). In the ST33 example, sessionStarted is set to 1 before calling wolfTPM2_FirmwareUpgrade_ex(). If that call fails before FieldUpgradeStart is issued (e.g., comms/GetCapabilities failure), cleanup will skip UnloadHandle and leak the policy session. Consider only marking the session as “consumed” on overall success; if the TPM already consumed it on a later failure, UnloadHandle may fail harmlessly and is already ignored.

        if (rc == 0) {
            if (policyMode) {
                printf("Using caller-supplied policy session\n");
                /* session is handed to FieldUpgradeStart, which consumes it */
                sessionStarted = 1;
            }
            rc = wolfTPM2_FirmwareUpgrade_ex(&dev,
                fwinfo.manifest_buf, (uint32_t)fwinfo.manifest_bufSz,
                TPM2_ST33_FwData_Cb, &fwinfo,
                policyMode ? &policySession : NULL);

examples/firmware/ifx_fw_update.c:251

• Severity: Low (CWE-772). In the Infineon example, sessionStarted is set based only on caps.opMode before the upgrade call. If the upgrade fails before reaching FieldUpgradeStart (e.g., IO/GetCapabilities errors inside the library), cleanup will skip UnloadHandle and leak the policy session. Mark the session as “consumed” only when the overall call succeeds and the path actually reaches start (opMode != 0x03).

    if (rc == 0) {
        /* The upgrade hands the session to FieldUpgradeStart, which the TPM
         * consumes as it enters upgrade mode. opMode 0x03 (finalize-only) does
         * not reach start, so the session there is not consumed. */
        if (policyMode) {

[aidangarske]

Skoll Multi-Scan Review

Modes: review + review-security

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

Posted findings

• [Medium] [review] wolfTPM2_IsAlgSupported tri-state return (1/0/rc) is easy to misuse — src/tpm2_wrap.c:1030-1056
• [Low] [review-security] Policy self-test hard-fails when TPM implements a hash wolfCrypt was not built with — examples/firmware/firmware_policy.c:153-173
• [Low] [review] Cleanup comment/guard says library 'zeroes' the session handle, but it sets TPM_RH_NULL — examples/firmware/ifx_fw_update.c:311-314; examples/firmware/st33_fw_update.c:472-475
• [Low] [review] No unit coverage for wolfTPM2_PolicyOR / wolfTPM2_IsAlgSupported success paths — tests/unit_tests.c:8200-8209
• [Info] [review-security] README describes consumed session as 'zeroed' but library sets TPM_RH_NULL — examples/firmware/README.md:270-276

Review generated by Skoll

[aidangarske]

Looks good!