Skip to content

Fenrir fixes batch 12 #511

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
aidangarske wants to merge 8 commits into
base: master
Choose a base branch
from
+209 −17
Open

Fenrir fixes batch 12 #511

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
00079a6
F-4373 — TPM2_Clear unauthenticated wipe via TPM_ST_NO_SESSIONS bypass
aidangarske May 26, 2026
08a197a
F-4374 — PolicyTicket zero-digest bypass forges PolicySigned extension
aidangarske May 26, 2026
2abe5e1
F-4375 — LoadExternal SYMCIPHER OOB write via dead bounds check
aidangarske May 26, 2026
16e3ecc
F-4742 — PolicyAuthorize zero-ticket bypass forges policyDigest chain
aidangarske May 26, 2026
6003e72
F-4170 + F-4171 — Fix TPMT_HA HMAC arm wire marshaling on sign and ve…
aidangarske May 26, 2026
c97fe5c
F-4742 — Revert zero-digest reject; NULL ticket is spec-compliant per…
aidangarske May 26, 2026
99e1b58
F-4171 — Bounds-check cmd before parsing hmacHash in VerifySequenceCo…
aidangarske May 26, 2026
010ea7d
Replace em-dashes with hyphens in comments per review
aidangarske May 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 59 additions & 14 deletions src/fwtpm/fwtpm_command.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;

Expand All @@ -14063,7 +14080,6 @@ static TPM_RC FwCmd_SignSequenceComplete(FWTPM_CTX* ctx, TPM2_Packet* cmd,
paramStart = FwRspParamsBegin(rsp, cmdTag, &paramSzPos);
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));
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down
153 changes: 150 additions & 3 deletions tests/fwtpm_unit_tests.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 */

/* ================================================================== */
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 */
/* ================================================================== */
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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 */
Expand All @@ -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");
Expand Down
Loading