Make fwTPM state changes transactional and harden PolicyPCR and private-blob wrapping - #593
Conversation
aidangarske
commented
Sep 8, 2026
There was a problem hiding this comment.
🟡 Changes recommended
Two correctness/security issues remain: bounds validation for TPM2B_NAME length in wrap-key derivation and missing error handling for PolicyPCR hash finalization.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR strengthens fwTPM’s security and crash-consistency by making NV-backed state mutations transactional (rollback on persistence failure), hardening PolicyPCR correctness (digest verification + PCR-change invalidation), and improving private-blob wrapping so wrapped objects are bound to their public area and use per-blob IVs.
Changes:
- Make NV journal appends/compactions act as a commit boundary, with rollback/“NV unavailable” behavior on failed commits/checkpoints.
- Harden policy/session behavior: PolicyPCR verifies caller-provided digests for real policy sessions and invalidates authorization if PCRs change.
- Rework private blob wrapping: per-blob random IV + HMAC over (IV || ciphertext), and keys derived from (parent private || child Name) to bind private blobs to the public area.
File summaries
| File | Description |
|---|---|
| wolftpm/fwtpm/fwtpm.h | Adds session/ctx fields to track PCR update counters and NV transactional state. |
| wolftpm/fwtpm/fwtpm_crypto.h | Updates crypto API signatures for public-name binding and RNG-provided IVs. |
| src/fwtpm/fwtpm_crypto.c | Implements Name-bound wrap-key derivation, per-blob IV, and updated wrap/unwrap formats. |
| src/fwtpm/fwtpm_nv.c | Implements transactional append semantics, compaction-as-commit logic, and deletion-aware compaction. |
| src/fwtpm/fwtpm_command.c | Rolls back volatile state on NV persist failures and hardens PolicyPCR behavior. |
| tests/fwtpm_unit_tests.c | Adds regression tests for NV transactional behavior, PolicyPCR verification/invalidation, and private-blob wrapping properties. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
12c83df to
c9e6529
Compare