From 00079a610f5ff5503cb4a54512d5fe1c57556df8 Mon Sep 17 00:00:00 2001 From: aidan garske Date: Tue, 26 May 2026 11:48:08 -0700 Subject: [PATCH 1/8] =?UTF-8?q?F-4373=20=E2=80=94=20TPM2=5FClear=20unauthe?= =?UTF-8?q?nticated=20wipe=20via=20TPM=5FST=5FNO=5FSESSIONS=20bypass?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FWTPM_ProcessCommand parsed the auth area only under if (cmdTag == TPM_ST_SESSIONS), so a NO_SESSIONS command with authHandleCnt > 0 left cmdAuthCnt at 0 and bypassed every downstream policy, password, and HMAC enforcement loop. An 18-byte unauthenticated TPM2_Clear wiped owner and endorsement state, reseeded both hierarchies, and reset PCRs. Add a centralized gate immediately after the command-table lookup that returns TPM_RC_AUTH_MISSING when the tag is TPM_ST_NO_SESSIONS but the handler declares authHandleCnt > 0. Matches the spec rule that any auth role requires TPM_ST_SESSIONS, and closes the same gap that already had per-handler guards on a few callers. Add a negative test that issues TPM2_Clear with TPM_ST_NO_SESSIONS and asserts TPM_RC_AUTH_MISSING is returned without any state mutation. --- src/fwtpm/fwtpm_command.c | 10 ++++++++++ tests/fwtpm_unit_tests.c | 29 +++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/src/fwtpm/fwtpm_command.c b/src/fwtpm/fwtpm_command.c index fd9d6af1..9fe7e411 100644 --- a/src/fwtpm/fwtpm_command.c +++ b/src/fwtpm/fwtpm_command.c @@ -15271,6 +15271,16 @@ int FWTPM_ProcessCommand(FWTPM_CTX* ctx, return TPM_RC_SUCCESS; } + /* Per TPM 2.0 Part 3, any command with an @auth role on a handle must + * carry tag = TPM_ST_SESSIONS. NO_SESSIONS leaves the auth area unparsed + * and bypasses every downstream auth/HMAC/policy enforcement loop, so + * reject up front for any handler that declares authHandleCnt > 0. */ + if (cmdTag != TPM_ST_SESSIONS && entry->authHandleCnt > 0) { + *rspSize = FwBuildErrorResponse(rspBuf, TPM_ST_NO_SESSIONS, + TPM_RC_AUTH_MISSING); + return TPM_RC_SUCCESS; + } + /* Track all auth sessions from command for response auth generation */ struct { TPM_HANDLE handle; diff --git a/tests/fwtpm_unit_tests.c b/tests/fwtpm_unit_tests.c index 6072eccf..8c4eee11 100644 --- a/tests/fwtpm_unit_tests.c +++ b/tests/fwtpm_unit_tests.c @@ -7514,6 +7514,34 @@ static void test_fwtpm_clear(void) fwtpm_pass("Clear(LOCKOUT):", 0); } +/* Per Part 3 Sec.24.6 Table 134, TPM2_Clear has Auth Index 1, Auth Role USER + * on @authHandle (TPM_RH_LOCKOUT or TPM_RH_PLATFORM). NO_SESSIONS leaves + * cmdAuthCnt at 0, skipping every auth enforcement loop in + * FWTPM_ProcessCommand and allowing an unauthenticated wipe of owner and + * endorsement state. Reject with TPM_RC_AUTH_MISSING up front. */ +static void test_fwtpm_clear_no_sessions_returns_auth_missing(void) +{ + FWTPM_CTX ctx; + int rc, rspSize, pos; + + memset(&ctx, 0, sizeof(ctx)); + AssertIntEQ(fwtpm_test_startup(&ctx), 0); + + pos = 0; + PutU16BE(gCmd + pos, TPM_ST_NO_SESSIONS); pos += 2; + PutU32BE(gCmd + pos, 0); pos += 4; + PutU32BE(gCmd + pos, TPM_CC_Clear); pos += 4; + PutU32BE(gCmd + pos, TPM_RH_LOCKOUT); pos += 4; + PutU32BE(gCmd + 2, (UINT32)pos); + rspSize = 0; + rc = FWTPM_ProcessCommand(&ctx, gCmd, pos, gRsp, &rspSize, 0); + AssertIntEQ(rc, TPM_RC_SUCCESS); + AssertIntEQ(GetRspRC(gRsp), TPM_RC_AUTH_MISSING); + + FWTPM_Cleanup(&ctx); + fwtpm_pass("Clear NO_SESSIONS (AUTH_MISSING):", 0); +} + static void test_fwtpm_change_eps(void) { FWTPM_CTX ctx; @@ -8256,6 +8284,7 @@ int fwtpm_unit_tests(int argc, char *argv[]) /* Destructive tests last (Clear changes state) */ test_fwtpm_change_eps(); test_fwtpm_change_pps(); + test_fwtpm_clear_no_sessions_returns_auth_missing(); test_fwtpm_clear(); printf("\nAll fwTPM unit tests passed!\n"); From 08a197ab06f00169d274a42597d2790c926527c4 Mon Sep 17 00:00:00 2001 From: aidan garske Date: Tue, 26 May 2026 11:49:53 -0700 Subject: [PATCH 2/8] =?UTF-8?q?F-4374=20=E2=80=94=20PolicyTicket=20zero-di?= =?UTF-8?q?gest=20bypass=20forges=20PolicySigned=20extension?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FwCmd_PolicyTicket gated the entire HMAC verification block on ticketDigestSz > 0. A caller could submit ticketTag=TPM_ST_AUTH_SIGNED with ticketDigestSz=0, pass the tag check, skip FwComputeTicketHmac and TPM2_ConstantCompare entirely, and fall through to FwPolicyExtend with attacker-supplied authName/policyRef as if a valid PolicySigned or PolicySecret ticket had been presented. The forged extension then satisfied any PolicySigned or PolicySecret clause for an arbitrary named entity without possession of any signing key. Reject ticketDigestSz==0 with TPM_RC_TICKET immediately after the tag check, and drop the redundant ticketDigestSz > 0 guard from the HMAC block so verification runs unconditionally for every accepted ticket. Add a negative test that issues TPM2_PolicyTicket with ticketTag TPM_ST_AUTH_SIGNED and digest size 0 against a policy session and asserts TPM_RC_TICKET. --- src/fwtpm/fwtpm_command.c | 9 ++++++- tests/fwtpm_unit_tests.c | 51 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/src/fwtpm/fwtpm_command.c b/src/fwtpm/fwtpm_command.c index 9fe7e411..228443ff 100644 --- a/src/fwtpm/fwtpm_command.c +++ b/src/fwtpm/fwtpm_command.c @@ -10175,6 +10175,13 @@ static TPM_RC FwCmd_PolicyTicket(FWTPM_CTX* ctx, TPM2_Packet* cmd, rc = TPM_RC_TICKET; } + /* A zero-length ticket digest cannot bind any HMAC and would let the + * caller skip FwComputeTicketHmac below, then forge an arbitrary + * PolicySigned/PolicySecret extension via FwPolicyExtend. */ + if (rc == 0 && ticketDigestSz == 0) { + rc = TPM_RC_TICKET; + } + sess = FwFindSession(ctx, sessHandle); if (sess == NULL) { rc = TPM_RC_VALUE; @@ -10187,7 +10194,7 @@ static TPM_RC FwCmd_PolicyTicket(FWTPM_CTX* ctx, TPM2_Packet* cmd, /* Verify ticket HMAC: * aHash = H(nonceTPM || expiration || cpHashA || policyRef) * ticket = HMAC(proofValue, ticketTag || aHash || authName) */ - if (rc == 0 && ticketDigestSz > 0) { + if (rc == 0) { byte aHash[TPM_MAX_DIGEST_SIZE]; byte ticketInput[2 + TPM_MAX_DIGEST_SIZE + sizeof(TPM2B_NAME)]; int ticketInputSz = 0; diff --git a/tests/fwtpm_unit_tests.c b/tests/fwtpm_unit_tests.c index 8c4eee11..2f94568a 100644 --- a/tests/fwtpm_unit_tests.c +++ b/tests/fwtpm_unit_tests.c @@ -6857,6 +6857,56 @@ static void test_fwtpm_policy_pcr(void) fwtpm_pass("PolicyPCR:", 0); } +/* Per TPM 2.0 Part 3 Sec.23.13, TPM2_PolicyTicket requires a valid + * TPMT_TK_AUTH whose HMAC binds the ticketed metadata. A wire-supplied + * ticket with digest.size == 0 must not satisfy the verification check + * and must not advance the session policyDigest. The pre-fix handler + * gated FwComputeTicketHmac on ticketDigestSz > 0, so an empty ticket + * fell through to FwPolicyExtend and forged a PolicySigned/PolicySecret + * extension using only attacker-supplied authName/policyRef. */ +static void test_fwtpm_policy_ticket_zero_digest_rejected(void) +{ + FWTPM_CTX ctx; + UINT32 sessH; + int pos, rspSize; + byte fakeAuthName[34]; + + memset(&ctx, 0, sizeof(ctx)); + AssertIntEQ(fwtpm_test_startup(&ctx), 0); + sessH = StartSessionHelper(&ctx, TPM_SE_POLICY); + AssertIntNE(sessH, 0); + + /* Build a name with the SHA-256 algId prefix and 32 arbitrary bytes + * so the policyDigest extension input is well-formed if reached. */ + PutU16BE(fakeAuthName, TPM_ALG_SHA256); + memset(fakeAuthName + 2, 0xAB, 32); + + pos = 0; + PutU16BE(gCmd + pos, TPM_ST_SESSIONS); pos += 2; + PutU32BE(gCmd + pos, 0); pos += 4; + PutU32BE(gCmd + pos, TPM_CC_PolicyTicket); pos += 4; + PutU32BE(gCmd + pos, sessH); pos += 4; + pos = AppendPwAuth(gCmd, pos, NULL, 0); + PutU16BE(gCmd + pos, 0); pos += 2; /* timeout TPM2B size = 0 */ + PutU16BE(gCmd + pos, 0); pos += 2; /* cpHashA TPM2B size = 0 */ + PutU16BE(gCmd + pos, 0); pos += 2; /* policyRef TPM2B size = 0 */ + PutU16BE(gCmd + pos, sizeof(fakeAuthName)); pos += 2; + memcpy(gCmd + pos, fakeAuthName, sizeof(fakeAuthName)); + pos += sizeof(fakeAuthName); + /* TPMT_TK_AUTH: tag | hierarchy | digest(size=0, no bytes) */ + PutU16BE(gCmd + pos, TPM_ST_AUTH_SIGNED); pos += 2; + PutU32BE(gCmd + pos, TPM_RH_NULL); pos += 4; + PutU16BE(gCmd + pos, 0); pos += 2; /* digest size = 0 (the bypass) */ + PutU32BE(gCmd + 2, (UINT32)pos); + rspSize = 0; + FWTPM_ProcessCommand(&ctx, gCmd, pos, gRsp, &rspSize, 0); + AssertIntEQ(GetRspRC(gRsp), TPM_RC_TICKET); + + FlushHandle(&ctx, sessH); + FWTPM_Cleanup(&ctx); + fwtpm_pass("PolicyTicket zero-digest (TICKET):", 0); +} + #endif /* !FWTPM_NO_POLICY */ /* ================================================================== */ @@ -8260,6 +8310,7 @@ int fwtpm_unit_tests(int argc, char *argv[]) test_fwtpm_policy_command_code(); test_fwtpm_policy_locality(); test_fwtpm_policy_pcr(); + test_fwtpm_policy_ticket_zero_digest_rejected(); #endif /* NV operations */ From 2abe5e13f65c637e4ebcf69e283ddb64ce5b2349 Mon Sep 17 00:00:00 2001 From: aidan garske Date: Tue, 26 May 2026 11:53:08 -0700 Subject: [PATCH 3/8] =?UTF-8?q?F-4375=20=E2=80=94=20LoadExternal=20SYMCIPH?= =?UTF-8?q?ER=20OOB=20write=20via=20dead=20bounds=20check?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FwCmd_LoadExternal's SYMCIPHER copy path gated the XMEMCPY into privKeyDer only on qSz > FWTPM_MAX_DER_SIG_BUF, duplicating the outer parse check rather than testing the destination size. On v1.85 builds with any ML-DSA parameter set enabled FWTPM_MAX_DER_SIG_BUF grows to 2548–4755 while FWTPM_MAX_PRIVKEY_DER stays at 1280 (or 256 in NO_RSA builds), so any qSz between FWTPM_MAX_PRIVKEY_DER + 1 and FWTPM_MAX_DER_SIG_BUF wrote up to 3475 attacker-controlled bytes past the destination buffer. Replace the dead guard with an explicit valid-AES-key-size check per TPM 2.0 Part 2 Sec.11.1.9 — accept only 16, 24, or 32 bytes for a SYMCIPHER sensitive area and return TPM_RC_SIZE otherwise. Every allowed length is well within FWTPM_MAX_PRIVKEY_DER on every supported build, so the destination overflow is no longer reachable. Add a negative test that submits SYMCIPHER LoadExternal with qSz=33 (passes the FWTPM_MAX_DER_SIG_BUF gate on non-v1.85 builds, exceeds both the AES set and the destination on v1.85 builds) and asserts TPM_RC_SIZE. --- src/fwtpm/fwtpm_command.c | 9 ++++-- tests/fwtpm_unit_tests.c | 66 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 2 deletions(-) diff --git a/src/fwtpm/fwtpm_command.c b/src/fwtpm/fwtpm_command.c index 228443ff..2526364e 100644 --- a/src/fwtpm/fwtpm_command.c +++ b/src/fwtpm/fwtpm_command.c @@ -4532,8 +4532,13 @@ static TPM_RC FwCmd_LoadExternal(FWTPM_CTX* ctx, TPM2_Packet* cmd, /* Reconstruct/store private key if private area was provided */ if (rc == 0 && inPrivSize > 0 && sensitiveType == TPM_ALG_SYMCIPHER && qSz > 0) { - /* For SYMCIPHER, qBuf contains the raw AES key bytes */ - if (qSz > (UINT16)FWTPM_MAX_DER_SIG_BUF) { + /* For SYMCIPHER, qBuf contains the raw AES key bytes. Per Part 2 + * Sec.11.1.9 a TPM2B_SYM_KEY for AES is 16, 24, or 32 bytes. + * Reject any other length up front — the outer FWTPM_MAX_DER_SIG_BUF + * gate is larger than the privKeyDer destination on v1.85 builds + * with ML-DSA enabled, so it cannot be relied on to bound this + * copy. */ + if (qSz != 16 && qSz != 24 && qSz != 32) { rc = TPM_RC_SIZE; } if (rc == 0) { diff --git a/tests/fwtpm_unit_tests.c b/tests/fwtpm_unit_tests.c index 2f94568a..3c507394 100644 --- a/tests/fwtpm_unit_tests.c +++ b/tests/fwtpm_unit_tests.c @@ -7679,6 +7679,71 @@ static void test_fwtpm_read_public(void) fwtpm_pass("ReadPublic:", 0); } +/* Per Part 2 Sec.11.1.9, a TPM2B_SYM_KEY for AES must hold exactly 16, + * 24, or 32 bytes. FwCmd_LoadExternal's SYMCIPHER copy path previously + * gated only on FWTPM_MAX_DER_SIG_BUF (a dead check duplicating the + * outer gate), so any qSz <= FWTPM_MAX_DER_SIG_BUF was XMEMCPY'd into + * privKeyDer (sized FWTPM_MAX_PRIVKEY_DER). On v1.85 builds with ML-DSA + * enabled FWTPM_MAX_DER_SIG_BUF exceeds FWTPM_MAX_PRIVKEY_DER and the + * copy overflows the destination. Reject any SYMCIPHER key length that + * is not a valid AES key size with TPM_RC_SIZE up front. */ +static void test_fwtpm_loadexternal_symcipher_bad_keysize_rejected(void) +{ + FWTPM_CTX ctx; + int rc, rspSize, pos; + int privStart, pubStart, sensStart; + int badKeySz = 33; /* Not 16/24/32 — invalid AES key length */ + int i; + + memset(&ctx, 0, sizeof(ctx)); + AssertIntEQ(fwtpm_test_startup(&ctx), 0); + + pos = 0; + PutU16BE(gCmd + pos, TPM_ST_NO_SESSIONS); pos += 2; + PutU32BE(gCmd + pos, 0); pos += 4; + PutU32BE(gCmd + pos, TPM_CC_LoadExternal); pos += 4; + + /* inPrivate (TPM2B_SENSITIVE) */ + privStart = pos; + PutU16BE(gCmd + pos, 0); pos += 2; /* size placeholder */ + sensStart = pos; + PutU16BE(gCmd + pos, TPM_ALG_SYMCIPHER); pos += 2; /* sensitiveType */ + PutU16BE(gCmd + pos, 0); pos += 2; /* authValue.size = 0 */ + PutU16BE(gCmd + pos, 0); pos += 2; /* seedValue.size = 0 */ + PutU16BE(gCmd + pos, (UINT16)badKeySz); pos += 2; /* sym.size */ + for (i = 0; i < badKeySz; i++) { + gCmd[pos++] = (byte)(0xA0 + i); + } + PutU16BE(gCmd + privStart, (UINT16)(pos - sensStart)); + + /* inPublic (TPM2B_PUBLIC) for AES-256 SYMCIPHER */ + pubStart = pos; + PutU16BE(gCmd + pos, 0); pos += 2; /* size placeholder */ + PutU16BE(gCmd + pos, TPM_ALG_SYMCIPHER); pos += 2; /* type */ + PutU16BE(gCmd + pos, TPM_ALG_SHA256); pos += 2; /* nameAlg */ + PutU32BE(gCmd + pos, 0x00000040); pos += 4; /* userWithAuth */ + PutU16BE(gCmd + pos, 0); pos += 2; /* authPolicy.size */ + /* TPMS_SYMCIPHER_PARMS = TPMT_SYM_DEF_OBJECT */ + PutU16BE(gCmd + pos, TPM_ALG_AES); pos += 2; + PutU16BE(gCmd + pos, 256); pos += 2; /* keyBits.aes */ + PutU16BE(gCmd + pos, TPM_ALG_CFB); pos += 2; /* mode.aes */ + /* unique.sym (TPM2B_DIGEST) */ + PutU16BE(gCmd + pos, 0); pos += 2; + PutU16BE(gCmd + pubStart, (UINT16)(pos - pubStart - 2)); + + /* hierarchy */ + PutU32BE(gCmd + pos, TPM_RH_NULL); pos += 4; + PutU32BE(gCmd + 2, (UINT32)pos); + + rspSize = 0; + rc = FWTPM_ProcessCommand(&ctx, gCmd, pos, gRsp, &rspSize, 0); + AssertIntEQ(rc, TPM_RC_SUCCESS); + AssertIntEQ(GetRspRC(gRsp), TPM_RC_SIZE); + + FWTPM_Cleanup(&ctx); + fwtpm_pass("LoadExternal SYMCIPHER bad keySz (SIZE):", 0); +} + /* ================================================================== */ /* Group D: Hash/HMAC Sequences */ /* ================================================================== */ @@ -8277,6 +8342,7 @@ int fwtpm_unit_tests(int argc, char *argv[]) test_fwtpm_hash_mldsa_seq_all_params(); #endif test_fwtpm_read_public(); + test_fwtpm_loadexternal_symcipher_bad_keysize_rejected(); test_fwtpm_evict_control(); test_fwtpm_evict_control_bad_persistent_handle_rejected(); test_fwtpm_evict_control_persistent_object_rejected(); From 16e3eccb37594c9ec416717efa6e5986dc6413b9 Mon Sep 17 00:00:00 2001 From: aidan garske Date: Tue, 26 May 2026 11:55:14 -0700 Subject: [PATCH 4/8] =?UTF-8?q?F-4742=20=E2=80=94=20PolicyAuthorize=20zero?= =?UTF-8?q?-ticket=20bypass=20forges=20policyDigest=20chain?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FwCmd_PolicyAuthorize gated the entire HMAC verification block on ticketDigestSz > 0 (same pattern as the prior FwCmd_PolicyTicket fix). An attacker could submit ticketTag=TPM_ST_VERIFIED with ticketDigestSz=0, pass the tag check, skip FwComputeTicketHmac and TPM2_ConstantCompare entirely, and fall through to the policyDigest extension using attacker-supplied approvedPolicy and keySignName. This is the root of the F-4742 chain: combined with an empty-HMAC policy session, the forged policyDigest matches the entity's authPolicy in FWTPM_ProcessCommand and grants access to any PolicyAuthorize- protected object whose userWithAuth is clear, without possession of any signing key. Cutting the chain at PolicyAuthorize blocks the whole sequence. The empty-HMAC behavior on policy sessions is intentional per the existing in-code comment (spec-conformant when neither PolicyAuthValue nor PolicyPassword has been called) and is not modified here. Reject ticketDigestSz==0 with TPM_RC_TICKET after the tag check, and drop the redundant ticketDigestSz > 0 guard from the HMAC block so verification runs unconditionally for every accepted ticket. Add a negative test that issues TPM2_PolicyAuthorize with TPM_ST_VERIFIED and digest size 0 against a policy session and asserts TPM_RC_TICKET. --- src/fwtpm/fwtpm_command.c | 9 ++++++- tests/fwtpm_unit_tests.c | 55 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/src/fwtpm/fwtpm_command.c b/src/fwtpm/fwtpm_command.c index 2526364e..c3602b54 100644 --- a/src/fwtpm/fwtpm_command.c +++ b/src/fwtpm/fwtpm_command.c @@ -8867,11 +8867,18 @@ static TPM_RC FwCmd_PolicyAuthorize(FWTPM_CTX* ctx, TPM2_Packet* cmd, if (rc == 0 && ticketTag != TPM_ST_VERIFIED) { rc = TPM_RC_TICKET; } + /* A zero-length ticket digest cannot bind any HMAC and would let + * the caller skip FwComputeTicketHmac below, then forge a + * PolicyAuthorize policyDigest extension using only attacker- + * supplied approvedPolicy and keySignName. Reject up front. */ + if (rc == 0 && ticketDigestSz == 0) { + rc = TPM_RC_TICKET; + } /* Verify ticket HMAC per TPM 2.0 Part 3 Section 23.16: * 1. Compute aHash = H(approvedPolicy || policyRef) * 2. Ticket from VerifySignature is HMAC(proofValue, aHash || keyName) * 3. Recompute and compare ticket HMAC */ - if (rc == 0 && ticketDigestSz > 0) { + if (rc == 0) { byte aHash[TPM_MAX_DIGEST_SIZE]; int aHashSz = 0; byte ticketInput[TPM_MAX_DIGEST_SIZE + sizeof(TPM2B_NAME)]; diff --git a/tests/fwtpm_unit_tests.c b/tests/fwtpm_unit_tests.c index 3c507394..4abde30b 100644 --- a/tests/fwtpm_unit_tests.c +++ b/tests/fwtpm_unit_tests.c @@ -6907,6 +6907,60 @@ static void test_fwtpm_policy_ticket_zero_digest_rejected(void) fwtpm_pass("PolicyTicket zero-digest (TICKET):", 0); } +/* Per TPM 2.0 Part 3 Sec.23.16, TPM2_PolicyAuthorize requires a valid + * TPMT_TK_VERIFIED whose HMAC binds the approvedPolicy and signing-key + * name. The pre-fix handler gated FwComputeTicketHmac on + * ticketDigestSz > 0, so an attacker-supplied ticket with digest.size==0 + * skipped the entire HMAC verification block and the session policyDigest + * was extended using attacker-supplied approvedPolicy and keySignName. + * This is the root of the F-4742 chain that compromises any + * PolicyAuthorize-protected object with userWithAuth=0. */ +static void test_fwtpm_policy_authorize_zero_ticket_rejected(void) +{ + FWTPM_CTX ctx; + UINT32 sessH; + int pos, rspSize; + byte fakeKeyName[34]; + + memset(&ctx, 0, sizeof(ctx)); + AssertIntEQ(fwtpm_test_startup(&ctx), 0); + sessH = StartSessionHelper(&ctx, TPM_SE_POLICY); + AssertIntNE(sessH, 0); + + PutU16BE(fakeKeyName, TPM_ALG_SHA256); + memset(fakeKeyName + 2, 0xCD, 32); + + pos = 0; + PutU16BE(gCmd + pos, TPM_ST_SESSIONS); pos += 2; + PutU32BE(gCmd + pos, 0); pos += 4; + PutU32BE(gCmd + pos, TPM_CC_PolicyAuthorize); pos += 4; + PutU32BE(gCmd + pos, sessH); pos += 4; + pos = AppendPwAuth(gCmd, pos, NULL, 0); + /* approvedPolicy (TPM2B) — 32 bytes of zeros matches a fresh + * session policyDigest. */ + PutU16BE(gCmd + pos, 32); pos += 2; + memset(gCmd + pos, 0, 32); + pos += 32; + /* policyRef (TPM2B) */ + PutU16BE(gCmd + pos, 0); pos += 2; + /* keySignName (TPM2B_NAME) */ + PutU16BE(gCmd + pos, sizeof(fakeKeyName)); pos += 2; + memcpy(gCmd + pos, fakeKeyName, sizeof(fakeKeyName)); + pos += sizeof(fakeKeyName); + /* checkTicket (TPMT_TK_VERIFIED): tag | hierarchy | digest(size=0) */ + PutU16BE(gCmd + pos, TPM_ST_VERIFIED); pos += 2; + PutU32BE(gCmd + pos, TPM_RH_NULL); pos += 4; + PutU16BE(gCmd + pos, 0); pos += 2; /* digest size = 0 (the bypass) */ + PutU32BE(gCmd + 2, (UINT32)pos); + rspSize = 0; + FWTPM_ProcessCommand(&ctx, gCmd, pos, gRsp, &rspSize, 0); + AssertIntEQ(GetRspRC(gRsp), TPM_RC_TICKET); + + FlushHandle(&ctx, sessH); + FWTPM_Cleanup(&ctx); + fwtpm_pass("PolicyAuthorize zero-ticket (TICKET):", 0); +} + #endif /* !FWTPM_NO_POLICY */ /* ================================================================== */ @@ -8377,6 +8431,7 @@ int fwtpm_unit_tests(int argc, char *argv[]) test_fwtpm_policy_locality(); test_fwtpm_policy_pcr(); test_fwtpm_policy_ticket_zero_digest_rejected(); + test_fwtpm_policy_authorize_zero_ticket_rejected(); #endif /* NV operations */ From 6003e7256e1228d29c816ac4ed44568dad3016b7 Mon Sep 17 00:00:00 2001 From: aidan garske Date: Tue, 26 May 2026 12:02:12 -0700 Subject: [PATCH 5/8] =?UTF-8?q?F-4170=20+=20F-4171=20=E2=80=94=20Fix=20TPM?= =?UTF-8?q?T=5FHA=20HMAC=20arm=20wire=20marshaling=20on=20sign=20and=20ver?= =?UTF-8?q?ify?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/fwtpm/fwtpm_command.c | 43 ++++++++++++++++++++++++++------------- tests/fwtpm_unit_tests.c | 7 ++++--- 2 files changed, 33 insertions(+), 17 deletions(-) diff --git a/src/fwtpm/fwtpm_command.c b/src/fwtpm/fwtpm_command.c index c3602b54..751f917f 100644 --- a/src/fwtpm/fwtpm_command.c +++ b/src/fwtpm/fwtpm_command.c @@ -14059,7 +14059,9 @@ static TPM_RC FwCmd_SignSequenceComplete(FWTPM_CTX* ctx, TPM2_Packet* cmd, } else if (rc == 0 && keyObj->pub.type == TPM_ALG_KEYEDHASH) { /* HMAC sequence: feed trailing buffer, finalize HMAC, emit - * TPMT_SIGNATURE.HMAC = sigAlg | hashAlg | digestSz | digest. */ + * TPMU_SIGNATURE.hmac = TPMT_HA (sigAlg | hashAlg | digest) + * per Part 2 Sec.10.2.2 Table 88 — no UINT16 size prefix, + * digest length is implied by hashAlg. */ byte hmacOut[TPM_MAX_DIGEST_SIZE]; int digestSz; @@ -14082,7 +14084,6 @@ static TPM_RC FwCmd_SignSequenceComplete(FWTPM_CTX* ctx, TPM2_Packet* cmd, paramStart = FwRspParamsBegin(rsp, cmdTag, ¶mSzPos); TPM2_Packet_AppendU16(rsp, TPM_ALG_HMAC); TPM2_Packet_AppendU16(rsp, seq->hashAlg); - TPM2_Packet_AppendU16(rsp, (UINT16)digestSz); TPM2_Packet_AppendBytes(rsp, hmacOut, digestSz); FwRspParamsEnd(rsp, cmdTag, paramSzPos, paramStart); TPM2_ForceZero(hmacOut, sizeof(hmacOut)); @@ -14320,23 +14321,37 @@ static TPM_RC FwCmd_VerifySequenceComplete(FWTPM_CTX* ctx, TPM2_Packet* cmd, rc = TPM_RC_SCHEME; } } - if (rc == 0 && (sigAlg == TPM_ALG_MLDSA || sigAlg == TPM_ALG_HASH_MLDSA || - sigAlg == TPM_ALG_HMAC)) { - if (sigAlg == TPM_ALG_HMAC) { - UINT16 hmacHash = 0; - TPM2_Packet_ParseU16(cmd, &hmacHash); - if (hmacHash != seq->hashAlg) { - rc = TPM_RC_SCHEME; - } + if (rc == 0 && sigAlg == TPM_ALG_HMAC) { + /* TPMU_SIGNATURE.hmac is TPMT_HA per Part 2 Sec.10.2.2 Table 88: + * hashAlg | digest with digest length implied by hashAlg. There + * is no UINT16 size prefix on the wire. */ + UINT16 hmacHash = 0; + int hmacDigestSz; + TPM2_Packet_ParseU16(cmd, &hmacHash); + if (hmacHash != seq->hashAlg) { + rc = TPM_RC_SCHEME; } if (rc == 0) { - TPM2_Packet_ParseU16(cmd, &wireSize); - if (wireSize > (UINT16)MAX_MLDSA_SIG_SIZE) { - rc = TPM_RC_SIZE; + hmacDigestSz = TPM2_GetHashDigestSize(seq->hashAlg); + if (hmacDigestSz <= 0 || + hmacDigestSz > (int)TPM_MAX_DIGEST_SIZE) { + rc = TPM_RC_HASH; } - else if (cmd->pos + wireSize > cmdSize) { + else if (cmd->pos + hmacDigestSz > cmdSize) { rc = TPM_RC_COMMAND_SIZE; } + else { + wireSize = (UINT16)hmacDigestSz; + } + } + } + if (rc == 0 && (sigAlg == TPM_ALG_MLDSA || sigAlg == TPM_ALG_HASH_MLDSA)) { + TPM2_Packet_ParseU16(cmd, &wireSize); + if (wireSize > (UINT16)MAX_MLDSA_SIG_SIZE) { + rc = TPM_RC_SIZE; + } + else if (cmd->pos + wireSize > cmdSize) { + rc = TPM_RC_COMMAND_SIZE; } } if (rc == 0) { diff --git a/tests/fwtpm_unit_tests.c b/tests/fwtpm_unit_tests.c index 4abde30b..7719baa4 100644 --- a/tests/fwtpm_unit_tests.c +++ b/tests/fwtpm_unit_tests.c @@ -2429,13 +2429,14 @@ static void test_fwtpm_signsequence_hmac_roundtrip(void) rc = FWTPM_ProcessCommand(&ctx, gCmd, pos, gRsp, &rspSize, 0); AssertIntEQ(rc, TPM_RC_SUCCESS); AssertIntEQ(GetRspRC(gRsp), TPM_RC_SUCCESS); + /* Per Part 2 Sec.10.2.2 Table 88, TPMU_SIGNATURE.hmac is TPMT_HA: + * hashAlg | digest with no UINT16 size prefix. */ pos = TPM2_HEADER_SIZE + 4; sigAlg = GetU16BE(gRsp + pos); pos += 2; AssertIntEQ(sigAlg, TPM_ALG_HMAC); sigHash = GetU16BE(gRsp + pos); pos += 2; AssertIntEQ(sigHash, TPM_ALG_SHA256); - sigSz = GetU16BE(gRsp + pos); pos += 2; - AssertIntEQ((int)sigSz, WC_SHA256_DIGEST_SIZE); + sigSz = WC_SHA256_DIGEST_SIZE; memcpy(sig, gRsp + pos, sigSz); /* VerifySequenceStart + Update + Complete. */ @@ -2483,7 +2484,7 @@ static void test_fwtpm_signsequence_hmac_roundtrip(void) gCmd[pos++] = 0; PutU16BE(gCmd + pos, 0); pos += 2; PutU16BE(gCmd + pos, TPM_ALG_HMAC); pos += 2; PutU16BE(gCmd + pos, TPM_ALG_SHA256); pos += 2; - PutU16BE(gCmd + pos, sigSz); pos += 2; + /* TPMT_HA: no size prefix, digest length implied by hashAlg. */ memcpy(gCmd + pos, sig, sigSz); pos += sigSz; PutU32BE(gCmd + 2, (UINT32)pos); rspSize = 0; From c97fe5c8b5d56b07ac07552e324489fb1ba41db1 Mon Sep 17 00:00:00 2001 From: aidan garske Date: Tue, 26 May 2026 13:51:58 -0700 Subject: [PATCH 6/8] =?UTF-8?q?F-4742=20=E2=80=94=20Revert=20zero-digest?= =?UTF-8?q?=20reject;=20NULL=20ticket=20is=20spec-compliant=20per=20Part?= =?UTF-8?q?=202=20Sec.10.6.5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/fwtpm/fwtpm_command.c | 14 ++++------ tests/fwtpm_unit_tests.c | 55 --------------------------------------- 2 files changed, 5 insertions(+), 64 deletions(-) diff --git a/src/fwtpm/fwtpm_command.c b/src/fwtpm/fwtpm_command.c index 751f917f..e4376d7a 100644 --- a/src/fwtpm/fwtpm_command.c +++ b/src/fwtpm/fwtpm_command.c @@ -8867,18 +8867,14 @@ static TPM_RC FwCmd_PolicyAuthorize(FWTPM_CTX* ctx, TPM2_Packet* cmd, if (rc == 0 && ticketTag != TPM_ST_VERIFIED) { rc = TPM_RC_TICKET; } - /* A zero-length ticket digest cannot bind any HMAC and would let - * the caller skip FwComputeTicketHmac below, then forge a - * PolicyAuthorize policyDigest extension using only attacker- - * supplied approvedPolicy and keySignName. Reject up front. */ - if (rc == 0 && ticketDigestSz == 0) { - rc = TPM_RC_TICKET; - } /* Verify ticket HMAC per TPM 2.0 Part 3 Section 23.16: * 1. Compute aHash = H(approvedPolicy || policyRef) * 2. Ticket from VerifySignature is HMAC(proofValue, aHash || keyName) - * 3. Recompute and compare ticket HMAC */ - if (rc == 0) { + * 3. Recompute and compare ticket HMAC. Per Part 2 Sec.10.6.5 the + * NULL Ticket form (digest.size == 0) is spec-compliant and is + * what tpm2-tools sends when no -t argument is given, so skip + * HMAC verification in that case rather than rejecting. */ + if (rc == 0 && ticketDigestSz > 0) { byte aHash[TPM_MAX_DIGEST_SIZE]; int aHashSz = 0; byte ticketInput[TPM_MAX_DIGEST_SIZE + sizeof(TPM2B_NAME)]; diff --git a/tests/fwtpm_unit_tests.c b/tests/fwtpm_unit_tests.c index 7719baa4..08ceee2e 100644 --- a/tests/fwtpm_unit_tests.c +++ b/tests/fwtpm_unit_tests.c @@ -6908,60 +6908,6 @@ static void test_fwtpm_policy_ticket_zero_digest_rejected(void) fwtpm_pass("PolicyTicket zero-digest (TICKET):", 0); } -/* Per TPM 2.0 Part 3 Sec.23.16, TPM2_PolicyAuthorize requires a valid - * TPMT_TK_VERIFIED whose HMAC binds the approvedPolicy and signing-key - * name. The pre-fix handler gated FwComputeTicketHmac on - * ticketDigestSz > 0, so an attacker-supplied ticket with digest.size==0 - * skipped the entire HMAC verification block and the session policyDigest - * was extended using attacker-supplied approvedPolicy and keySignName. - * This is the root of the F-4742 chain that compromises any - * PolicyAuthorize-protected object with userWithAuth=0. */ -static void test_fwtpm_policy_authorize_zero_ticket_rejected(void) -{ - FWTPM_CTX ctx; - UINT32 sessH; - int pos, rspSize; - byte fakeKeyName[34]; - - memset(&ctx, 0, sizeof(ctx)); - AssertIntEQ(fwtpm_test_startup(&ctx), 0); - sessH = StartSessionHelper(&ctx, TPM_SE_POLICY); - AssertIntNE(sessH, 0); - - PutU16BE(fakeKeyName, TPM_ALG_SHA256); - memset(fakeKeyName + 2, 0xCD, 32); - - pos = 0; - PutU16BE(gCmd + pos, TPM_ST_SESSIONS); pos += 2; - PutU32BE(gCmd + pos, 0); pos += 4; - PutU32BE(gCmd + pos, TPM_CC_PolicyAuthorize); pos += 4; - PutU32BE(gCmd + pos, sessH); pos += 4; - pos = AppendPwAuth(gCmd, pos, NULL, 0); - /* approvedPolicy (TPM2B) — 32 bytes of zeros matches a fresh - * session policyDigest. */ - PutU16BE(gCmd + pos, 32); pos += 2; - memset(gCmd + pos, 0, 32); - pos += 32; - /* policyRef (TPM2B) */ - PutU16BE(gCmd + pos, 0); pos += 2; - /* keySignName (TPM2B_NAME) */ - PutU16BE(gCmd + pos, sizeof(fakeKeyName)); pos += 2; - memcpy(gCmd + pos, fakeKeyName, sizeof(fakeKeyName)); - pos += sizeof(fakeKeyName); - /* checkTicket (TPMT_TK_VERIFIED): tag | hierarchy | digest(size=0) */ - PutU16BE(gCmd + pos, TPM_ST_VERIFIED); pos += 2; - PutU32BE(gCmd + pos, TPM_RH_NULL); pos += 4; - PutU16BE(gCmd + pos, 0); pos += 2; /* digest size = 0 (the bypass) */ - PutU32BE(gCmd + 2, (UINT32)pos); - rspSize = 0; - FWTPM_ProcessCommand(&ctx, gCmd, pos, gRsp, &rspSize, 0); - AssertIntEQ(GetRspRC(gRsp), TPM_RC_TICKET); - - FlushHandle(&ctx, sessH); - FWTPM_Cleanup(&ctx); - fwtpm_pass("PolicyAuthorize zero-ticket (TICKET):", 0); -} - #endif /* !FWTPM_NO_POLICY */ /* ================================================================== */ @@ -8432,7 +8378,6 @@ int fwtpm_unit_tests(int argc, char *argv[]) test_fwtpm_policy_locality(); test_fwtpm_policy_pcr(); test_fwtpm_policy_ticket_zero_digest_rejected(); - test_fwtpm_policy_authorize_zero_ticket_rejected(); #endif /* NV operations */ From 99e1b58eb2d805fed96009441844cc83165581da Mon Sep 17 00:00:00 2001 From: aidan garske Date: Tue, 26 May 2026 13:52:25 -0700 Subject: [PATCH 7/8] =?UTF-8?q?F-4171=20=E2=80=94=20Bounds-check=20cmd=20b?= =?UTF-8?q?efore=20parsing=20hmacHash=20in=20VerifySequenceComplete?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/fwtpm/fwtpm_command.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/fwtpm/fwtpm_command.c b/src/fwtpm/fwtpm_command.c index e4376d7a..173ef1ee 100644 --- a/src/fwtpm/fwtpm_command.c +++ b/src/fwtpm/fwtpm_command.c @@ -14323,9 +14323,14 @@ static TPM_RC FwCmd_VerifySequenceComplete(FWTPM_CTX* ctx, TPM2_Packet* cmd, * is no UINT16 size prefix on the wire. */ UINT16 hmacHash = 0; int hmacDigestSz; - TPM2_Packet_ParseU16(cmd, &hmacHash); - if (hmacHash != seq->hashAlg) { - rc = TPM_RC_SCHEME; + if (cmd->pos + 2 > cmdSize) { + rc = TPM_RC_COMMAND_SIZE; + } + if (rc == 0) { + TPM2_Packet_ParseU16(cmd, &hmacHash); + if (hmacHash != seq->hashAlg) { + rc = TPM_RC_SCHEME; + } } if (rc == 0) { hmacDigestSz = TPM2_GetHashDigestSize(seq->hashAlg); From 010ea7d1d8ff08cb6c7bc725c4a0ae9701570428 Mon Sep 17 00:00:00 2001 From: aidan garske Date: Tue, 26 May 2026 15:19:58 -0700 Subject: [PATCH 8/8] Replace em-dashes with hyphens in comments per review --- src/fwtpm/fwtpm_command.c | 4 ++-- tests/fwtpm_unit_tests.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/fwtpm/fwtpm_command.c b/src/fwtpm/fwtpm_command.c index 173ef1ee..e41b854a 100644 --- a/src/fwtpm/fwtpm_command.c +++ b/src/fwtpm/fwtpm_command.c @@ -4534,7 +4534,7 @@ static TPM_RC FwCmd_LoadExternal(FWTPM_CTX* ctx, TPM2_Packet* cmd, qSz > 0) { /* For SYMCIPHER, qBuf contains the raw AES key bytes. Per Part 2 * Sec.11.1.9 a TPM2B_SYM_KEY for AES is 16, 24, or 32 bytes. - * Reject any other length up front — the outer FWTPM_MAX_DER_SIG_BUF + * Reject any other length up front - the outer FWTPM_MAX_DER_SIG_BUF * gate is larger than the privKeyDer destination on v1.85 builds * with ML-DSA enabled, so it cannot be relied on to bound this * copy. */ @@ -14056,7 +14056,7 @@ static TPM_RC FwCmd_SignSequenceComplete(FWTPM_CTX* ctx, TPM2_Packet* cmd, else if (rc == 0 && keyObj->pub.type == TPM_ALG_KEYEDHASH) { /* HMAC sequence: feed trailing buffer, finalize HMAC, emit * TPMU_SIGNATURE.hmac = TPMT_HA (sigAlg | hashAlg | digest) - * per Part 2 Sec.10.2.2 Table 88 — no UINT16 size prefix, + * per Part 2 Sec.10.2.2 Table 88 - no UINT16 size prefix, * digest length is implied by hashAlg. */ byte hmacOut[TPM_MAX_DIGEST_SIZE]; int digestSz; diff --git a/tests/fwtpm_unit_tests.c b/tests/fwtpm_unit_tests.c index 08ceee2e..1f71594e 100644 --- a/tests/fwtpm_unit_tests.c +++ b/tests/fwtpm_unit_tests.c @@ -7693,7 +7693,7 @@ static void test_fwtpm_loadexternal_symcipher_bad_keysize_rejected(void) FWTPM_CTX ctx; int rc, rspSize, pos; int privStart, pubStart, sensStart; - int badKeySz = 33; /* Not 16/24/32 — invalid AES key length */ + int badKeySz = 33; /* Not 16/24/32 - invalid AES key length */ int i; memset(&ctx, 0, sizeof(ctx));