Skip to content

Correct fwTPM hierarchy and NV behavior and clear TPM command secrets - #613

Merged
dgarske merged 12 commits into
wolfSSL:masterfrom
aidangarske:fenrir-fixes-14405-14406-14430-14431-14230-14439
Sep 29, 2026
Merged

dgarske merged 12 commits into
wolfSSL:masterfrom
aidangarske:fenrir-fixes-14405-14406-14430-14431-14230-14439

Conversation

@aidangarske

Copy link
Copy Markdown
Member
F-14405, F-14406, F-14430, F-14431, F-14230, F-14439

@aidangarske aidangarske self-assigned this Sep 28, 2026
Copilot AI lite review requested due to automatic review settings September 28, 2026 19:36
@aidangarske

Copy link
Copy Markdown
Member Author

@wolfSSL-Fenrir-bot review balanced

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Unresolved NV lifecycle and command-buffer wiping issues remain.

Review effort: Lite
Findings: 2 Medium severity

Open (2)
What changed in this PR

Corrects fwTPM hierarchy/NV behavior and strengthens TPM command-buffer secret cleanup.

Changes:

  • Adds versioned endorsement-proof derivation and legacy compatibility.
  • Updates hierarchy, Clear, Startup, and NV lifecycle handling.
  • Adds command-buffer cleanup and regression tests.
File Summary
wolftpm/​fwtpm/​fwtpm.h Adds proof-version state.
wolftpm/​fwtpm/​fwtpm_crypto.h Updates crypto helper APIs.
tests/​unit_tests.c Adds command-buffer wipe tests.
tests/​fwtpm_unit_tests.c Adds hierarchy and NV regression tests.
src/​tpm2.c Cleans command buffers after responses and errors.
src/​fwtpm/​fwtpm.c Initializes proof-version state.
src/​fwtpm/​fwtpm_nv.c Persists proof-version state.
src/​fwtpm/​fwtpm_crypto.c Updates proof and private-blob derivation.
src/​fwtpm/​fwtpm_command.c Updates Clear, Startup, NV, and hierarchy handling.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/fwtpm/fwtpm_command.c
Comment thread src/tpm2.c

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fenrir Automated Review — PR #613

Scan targets checked: wolftpm-src, wolftpm-bugs
Coverage: 7 of 9 in-scope changed file(s) opened by the reviewer; not opened: wolftpm/fwtpm/fwtpm.h, wolftpm/fwtpm/fwtpm_crypto.h

Findings: 2
2 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Reported findings require changes before merge.

Review tier: Balanced

Comment thread src/fwtpm/fwtpm_command.c
Comment thread tests/fwtpm_unit_tests.c Outdated
@aidangarske
aidangarske requested review from wolfSSL-Fenrir-bot and a lite review from Copilot September 28, 2026 20:08

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fenrir Automated Review — PR #613

Scan targets checked: wolftpm-src, wolftpm-bugs
Coverage: 1 of 1 in-scope changed file(s) opened by the reviewer

Fenrir result: Approved ✅

No new issues found in the changed files.

Advisory only — this automated result does not count as a GitHub approval.

Review tier: Lite

@wolfSSL-Fenrir-bot
wolfSSL-Fenrir-bot dismissed their stale review September 28, 2026 20:09

Fenrir's latest completed scan found no issues; clearing the prior automated change request.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Unresolved NV startup-state handling and command-buffer cleanup gaps remain.

Review effort: Lite
Findings: None

Resolved since last review (2)
Previously missed (2)

In code that hasn't changed since last review

Medium severity NV clear fails to erase stored data on Startup(CLEAR)

src/​fwtpm/​fwtpm_command.c:1078

[Medium, CWE-200] TPMA_NV_CLEAR_STCLEAR means the NV contents are cleared on Startup(CLEAR), but this branch only clears WRITTEN and the separate written flag. A subsequent partial write can make the index readable again while exposing untouched bytes from before the clear. Clear the stored data as part of this transition and preserve it in the existing rollback path if persistence fails.

Medium severity Retry path leaves command secrets after transport failure

src/​tpm2.c:653

[Medium, CWE-316] The retry-enabled variant has the same gap: a transport/SPDM failure returns at lines 629-631 before this final-attempt wipe, leaving the serialized command and its auth/plaintext parameters in the shared buffer. Wipe the command body on that early return too.

@aidangarske
aidangarske requested a review from dgarske September 28, 2026 21:01
@dgarske dgarske self-assigned this Sep 28, 2026
@dgarske
dgarske merged commit 4797a8d into wolfSSL:master Sep 29, 2026
227 checks passed
@aidangarske
aidangarske deleted the fenrir-fixes-14405-14406-14430-14431-14230-14439 branch September 29, 2026 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants