diff --git a/src/fwtpm/fwtpm_command.c b/src/fwtpm/fwtpm_command.c index fd9d6af1..e41b854a 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) { @@ -8865,7 +8870,10 @@ static TPM_RC FwCmd_PolicyAuthorize(FWTPM_CTX* ctx, TPM2_Packet* cmd, /* 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 */ + * 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; @@ -10175,6 +10183,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 +10202,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; @@ -14040,7 +14055,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; @@ -14063,7 +14080,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)); @@ -14301,23 +14317,42 @@ 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; + 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; + 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) { - 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) { @@ -15271,6 +15306,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..1f71594e 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; @@ -6857,6 +6858,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 */ /* ================================================================== */ @@ -7514,6 +7565,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; @@ -7601,6 +7680,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 */ /* ================================================================== */ @@ -8199,6 +8343,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(); @@ -8232,6 +8377,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 */ @@ -8256,6 +8402,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");