Fenrir fixes batch 12#511
Open
aidangarske wants to merge 8 commits into
Open
Conversation
Member
aidangarske
commented
May 26, 2026
aidangarske
commented
dgarske
previously approved these changes
May 26, 2026
Member
|
@aidangarske please resolve merge conflicts. Thanks |
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.
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.
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.
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.
… Part 2 Sec.10.6.5
aidangarske
force-pushed
the
fenrir-fixes-12
branch
from
302f513 to
010ea7d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.