[dgarske]

This PR fixes the fwTPM issues #648-#654, plus related defects in the same code paths. Each fix is its own commit with a regression test:

• Parameter encryption: include the authorized entity's authValue in the key for every authorization session, bound or not, matching the reference TPM (fwTPM: parameter-encryption key leaves out the authValue when the session isn't bound to the authorized entity #648).
• ECDSA: pad R and S to the curve width (fwTPM: ECDSA signatureR/signatureS are not padded to the curve size #649), and sign an all-zero digest when wolfCrypt allows it (WC_ALLOW_ECC_ZERO_HASH), as the reference TPM does; otherwise return TPM_RC_VALUE on the digest parameter instead of TPM_RC_FAILURE (fwTPM: TPM2_Sign answers TPM_RC_FAILURE for an all-zero ECDSA digest #654).
• RSA: use the correct SHA-1 and SHA-3 hash/MGF1 pairs for OAEP and PSS, refuse OAEP and PSS with a hash that has no wolfCrypt MGF1 selector, and advertise the enabled SHA-1, SHA-3 and SM3 algorithms (fwTPM: RSA-OAEP with SHA-1 uses MGF1-SHA-256, and TPM_CAP_ALGS doesn't list SHA-1 #650).
• Capabilities: report the complete fixed TPM_PT properties and live session, object, persistent and NV counts (fwTPM: TPM_CAP_TPM_PROPERTIES leaves out fixed properties, including TPM_PT_CONTEXT_SYM_SIZE #651).
• Clock and orderly state: checkpoint Clock and orderly counters to NV with power-loss recovery, preserve Clock safety across Shutdown, and resume Clock from its NV checkpoint after a platform power cycle (fwTPM: TPM_CAP_TPM_PROPERTIES leaves out fixed properties, including TPM_PT_CONTEXT_SYM_SIZE #651). A build without NV reports Clock unsafe after a power cycle.
• Contexts: ContextSave stores an authenticated object snapshot so ContextLoad works after a flush, and session context loads validate the saved handle and hierarchy (fwTPM: ContextLoad of a flushed object fails, so fwTPM can't run behind a resource manager #652). Each ContextLoad now takes its own object slot, as on a reference TPM, so the autoconf fwtpm_server build defaults to 7 transient object slots for tools that run without a resource manager. An object loaded from a legacy persistent record, whose hierarchy is unknown, saves with a proof bound to both the owner and endorsement seeds.
• TestParms: reject unsupported RSA sizes, hashes the crypto backend lacks and RSA schemes without an MGF1 selector, and return parameter-numbered errors (fwTPM: TPM2_TestParms errors carry no parameter number #653). CreatePrimary, Create and CreateLoaded apply the same RSA size and curve checks to inPublic, and reject an RSA decrypt key whose nameAlg has no MGF1 selector.

Related fixes:

• Apply command HMAC verification consistently to every session in the authorization area.
• Hash and StirRandom parse their parameters after the authorization area and return session-framed responses.
• TPM_CAP_HANDLES includes hash, HMAC, event, sign and verify sequence handles.
• wolfTPM2_SignDigest and wolfTPM2_SignSequenceComplete normalize ECDSA scalars to the curve width, as wolfTPM2_SignHashScheme already does.
• TPM2_Sign reports a digest size mismatch on parameter 1.

Notes:

• New FWTPM_Clock_PowerOn() for hosts that power cycle within one FWTPM_CTX. Embedded hosts must call FWTPM_Clock_Update() at least every FWTPM_CLOCK_POLL_MS.
• Older fwTPM firmware cannot read the extended orderly-shutdown NV marker, so a downgrade needs journal conversion.
• MAX_CONTEXT_SIZE defaults to 4096 when MAX_RSA_KEY_BITS exceeds 2048 and to 8192 with PQC, since an object context now carries the private key. Define it to keep a fixed size.
• New CI job runs tpm2-pkcs11 (tpm2_ptool and OpenSC pkcs11-tool) against fwtpm_server, directly and through tpm2-abrmd, for the classic and v1.85 builds.

Fixes #648, Fixes #649, Fixes #650, Fixes #651, Fixes #652, Fixes #653, Fixes #654

[AlexLaroche]

Thanks @dgarske, I tested #656 (0d35f1d) against master (d7f73b5) and it resolves #648–#653. #654 is mostly resolved: see the note below.

Reproductions. Both servers were built with --enable-fwtpm-only --enable-swtpm on wolfSSL v5.9.4-stable, with AddressSanitizer. Each case uses the ESYS program from its issue.

Issue	master d7f73b5	#656
#648 Unseal through an encrypting session (plain, and salted+bound)	64 bytes, wrong	64 bytes, correct
#649 ECDSA r/s of 200 signatures each (P-256 child, P-521 child, P-256 primary)	1/0, 95/107, 2/1 short	none short
#650 RSA_Decrypt OAEP-SHA1 (MGF1-SHA1) of an OpenSSL ciphertext	TPM_RC_FAILURE	decrypts, plaintext matches
#650 TPM_CAP_ALGS	no SHA-1	SHA-1 and SHA3-256/384/512 listed
#651 TPM_CAP_TPM_PROPERTIES from TPM_PT_FIXED	19 properties	46, including TPM_PT_CONTEXT_HASH/_SYM/_SYM_SIZE (SHA-256, AES, 256)
#652 ContextSave, FlushContext, ContextLoad of a loaded key	TPM_RC_HANDLE on load	loads
#653 TestParms RSA keyBits 1000	0x087	0x1c4, the same as libtpms
#654 ECDSA of an all-zero digest	TPM_RC_FAILURE	TPM_RC_VALUE on parameter 1

End to end. I also ran the tpm2-pkcs11 stack you mention in the PR's notes. That was released tpm2-pkcs11 1.10.1 on tpm2-tss 4.2.0, driven by a C# PKCS#11 suite of 642 cases. It ran on both tpm2-pkcs11 store backends (esysdb and FAPI), and on both the classic and --enable-v185 fwTPM builds:

• On master: tpm2-pkcs11 doesn't get past C_Initialize, because TestParms answers a bare TPM_RC_CURVE (fwTPM: TPM2_TestParms errors carry no parameter number #653).
• With Fixes for Firmware TPM issues and related session, clock and signature defects #656 connected directly: all four configurations pass, with 0 failures.
• With Fixes for Firmware TPM issues and related session, clock and signature defects #656 behind tpm2-abrmd: the classic build passes on both store backends, with 0 failures. I didn't run the --enable-v185 build this way. Until now fwTPM couldn't run behind a resource manager (fwTPM: ContextLoad of a flushed object fails, so fwTPM can't run behind a resource manager #652).

These results are without any of the local patches I had been applying to fwTPM for #648–#652, so I can drop them.

One note on #654. A clear TPM_RC_VALUE on the digest parameter is a real improvement, because a client no longer sees the TPM as failed. libtpms and the TCG reference implementation still sign e = 0, so a PKCS#11 C_Sign of an empty message with CKM_ECDSA works on those and fails on fwTPM. I'm fine with closing #654 on this. If wolfCrypt later accepts a zero hash, signing it would match the other TPMs.

[dgarske]

Hi @AlexLaroche thank you for that feedback! I've pushed updates. Can you take another look? Thanks, David Garske, wolfSSL

[embhorn]

Skoll Multi-Scan Review

Modes: review + review-security

Overall recommendation: REQUEST_CHANGES
Findings: 6 total — 6 posted, 0 skipped
6 finding(s) posted as inline comments (see file-level comments below)

Posted findings

• [High] [review] Legacy persistent-object children can no longer be context-saved — src/fwtpm/fwtpm_command.c:5844-5848
• [High] [review] Any signing failure on a zero digest becomes a parameter error — src/fwtpm/fwtpm_crypto.c:3990-3995
• [High] [review] RSA SHA-3 schemes silently use MGF1-SHA256 — src/fwtpm/fwtpm_crypto.c:94-107
• [Medium] [review-security] Fresh TPM reports Clock as unsafe for its first update interval — src/fwtpm/fwtpm.c:41-47
• [Medium] [review-security] OAEP now selects MGF1 from the scheme hash — src/fwtpm/fwtpm_crypto.c:85-88
• [Medium] [review-security] SHA-3 RSA falls back to MGF1-SHA256 when its selector is unavailable — src/fwtpm/fwtpm_crypto.c:94-108

Review generated by Skoll

[AlexLaroche]

Hi @AlexLaroche thank you for that feedback! I've pushed updates. Can you take another look? Thanks, David Garske, wolfSSL

Thanks @dgarske! I retested #656 at ad2e978, again with no local patches. Everything still holds, and #654 is now fully resolved.

Reproductions. As before: fwtpm_server built with --enable-fwtpm-only --enable-swtpm on wolfSSL v5.9.4-stable, with AddressSanitizer, and each issue's ESYS program. No sanitizer reports.

Issue	ad2e978
#648 Unseal through an encrypting session (plain, and salted+bound)	64 bytes, correct
#649 ECDSA r/s of 200 signatures each (P-256 child, P-521 child, P-256 primary)	none short
#650 RSA_Decrypt OAEP-SHA1 (MGF1-SHA1) of an OpenSSL ciphertext	decrypts, plaintext matches
#650 TPM_CAP_ALGS	SHA-1 and SHA3-256/384/512 listed
#651 TPM_CAP_TPM_PROPERTIES from TPM_PT_FIXED	46 properties
#652 ContextSave, FlushContext, ContextLoad of a loaded key	loads
#653 TestParms RSA keyBits 1000	0x1c4, the same as libtpms
#654 ECDSA of an all-zero digest, wolfSSL defaults	TPM_RC_VALUE on parameter 1
#654 ECDSA of an all-zero digest, wolfSSL with -DWC_ALLOW_ECC_ZERO_HASH	signs; OpenSSL verifies the signature against the key's public point

Thanks for the WC_ALLOW_ECC_ZERO_HASH path. With it, a PKCS#11 C_Sign of an empty message with CKM_ECDSA works on fwTPM as it does on libtpms and the TCG reference implementation.

End to end. The C# PKCS#11 suite (642 cases) ran through tpm2-pkcs11, now at master a5156c5, on tpm2-tss 4.2.0. It covered both store backends (esysdb and FAPI) and both the classic and --enable-v185 builds, connected directly and behind tpm2-abrmd. All 8 configurations pass, with 0 failures. That includes the --enable-v185 build behind tpm2-abrmd, which I hadn't run last time.

One small note on the new digest-size check in TPM2_Sign (fwtpm_command.c, "Part 3 Sec.20.2.1"). It returns a bare TPM_RC_SIZE, while the TCG reference implementation returns TPM_RCS_SIZE + RC_Sign_digest, which names parameter 1 (Sign.c). The reference also checks the size only when no validation ticket is supplied. I found this by reading the code, not by running it. It doesn't affect anything above.

[AlexLaroche]

One more finding on ad2e978, in the same area as #653: object creation with a key size or curve the build doesn't support.

TPM2_CreatePrimary with an RSA key size the build doesn't support returns TPM_RC_FAILURE, so a client reads it as a TPM failure. TestParms already gives the right code for the same template since #653. The TPM keeps working afterwards (GetRandom succeeds). This comes from an ESYS program run against both servers, with a signing key in the null hierarchy:

	fwTPM ad2e978	libtpms
RSA 3072, TestParms	0x1c4	success
RSA 3072, CreatePrimary	0x101 (TPM_RC_FAILURE)	success
RSA 4096, TestParms	0x1c4	0x1c4
RSA 4096, CreatePrimary	0x101 (TPM_RC_FAILURE)	0x2c4 (TPM_RC_VALUE, parameter 2)
P-224, TestParms	0x1e6	success
P-224, CreatePrimary	0x0a6 (TPM_RC_CURVE, no parameter number)	success

From the code, CreatePrimary and Create don't check keyBits before wc_MakeRsaKey, and FwGenerateRsaKey maps the wolfCrypt error to TPM_RC_FAILURE. Running the same check TestParms does would give 0x2c4 for the RSA sizes and 0x2e6 (TPM_RC_CURVE on inPublic) for P-224. I only ran CreatePrimary; Create goes through the same FwGenerateRsaKey.

[dgarske]

Hi @AlexLaroche , thanks for the feedback! Can you review the latest work? Thanks, David Garske, wolfSSL

[embhorn]

Skoll Code Review

Scan type: review

Overall recommendation: REQUEST_CHANGES
Findings: 3 total — 3 posted, 0 skipped
3 finding(s) posted as inline comments (see file-level comments below)

Posted findings

• [High] No-NV power cycle reports a rolled-back Clock as safe — src/fwtpm/fwtpm.c:265-268
• [High] RSA seed operations accept a parent with no usable MGF1 — src/fwtpm/fwtpm_crypto.c:105-108
• [High] Clock compaction can replace the durable checkpoint with a different RAM value — src/fwtpm/fwtpm_nv.c:2968-2976

Review generated by Skoll

[embhorn]

Skoll Code Review

Scan type: review

Overall recommendation: REQUEST_CHANGES
Findings: 2 total — 2 posted, 0 skipped
2 finding(s) posted as inline comments (see file-level comments below)

Posted findings

• [High] Socket polling timeout disconnects valid slow clients — src/fwtpm/fwtpm_io.c:100-103
• [Medium] HMAC sweep accepts unrelated command errors as proof — tests/fwtpm_unit_tests.c:17308-17312

Review generated by Skoll

[embhorn]

Socket polling timeout disconnects valid slow clients

The new one-second receive and send timeouts make a pause within a valid TCP frame return a timeout error. SocketRecv and SocketSend treat every non-interruption error as fatal, and the server closes that client. A client that sends part of a command, pauses for over one second, then sends the rest now loses its connection; previously the server waited for the remaining bytes. The clock poll interval does not establish a deadline for completing a client frame.

Suggested fix: Treat socket timeouts as polling events and preserve each client's partial frame while the server continues clock updates; close on EOF or an actual connection error.
Basis: Linux socket(7), SO_RCVTIMEO and SO_SNDTIMEO specifies that a timed-out blocking transfer returns EAGAIN or EWOULDBLOCK, and Microsoft Winsock getsockopt specifies WSAETIMEDOUT for a receive timeout.

[embhorn]

HMAC sweep accepts unrelated command errors as proof

The new sweep supplies TPM_RH_OWNER for every handle and the same two-byte parameter to every command, then accepts any non-success response. Many commands can reject those handles or parameters before HMAC verification, so the test can pass even if their HMAC checks are bypassed. The single positive control covers only GetRandom.

Suggested fix: Use valid fixtures for command families intended to reach HMAC verification, and assert an authentication error for the corrupted HMAC; consult TPM 2.0 Part 1 §19.6 for the expected authorization behavior.

[embhorn]

Skoll Code Review

Scan type: review

Overall recommendation: REQUEST_CHANGES
Findings: 2 total — 2 posted, 0 skipped
2 finding(s) posted as inline comments (see file-level comments below)

Posted findings

• [High] Reset the server Time epoch on platform power-on — src/fwtpm/fwtpm_main.c:340-341
• [Medium] Correct the Clock rollback claim in the new API documentation — wolftpm/fwtpm/fwtpm.h:1094-1096

Review generated by Skoll

[embhorn]

Reset the server Time epoch on platform power-on

The new host clock HAL subtracts bootMs, but bootMs is set only when the server starts. After a platform POWER_OFF followed by POWER_ON in the same process, FWTPM_Clock_PowerOn() adjusts Clock while the HAL still includes the time spent powered off. The PR now reports that HAL value as Time in ReadClock, PolicyCounterTimer and GetTime attestations. A ReadClock after the power cycle therefore reports Time continuing from the previous power period. The new power-cycle tests check Clock and safe, but do not check Time.

Suggested fix: Reset the standalone HAL's Time epoch on each platform POWER_ON, independently of the durable Clock checkpoint, and test ReadClock across a same-process power cycle.
Basis: TCG TPM 2.0 Library Part 2, §10.10.6 TPMS_TIME_INFO requires Time to reset when power to its circuit is reestablished: https://trustedcomputinggroup.org/wp-content/uploads/Trusted-Platform-Module-2.0-Library-Part-2-Structures_Version-185_pub.pdf

[dgarske]

Fixed in 828271c8: FWTPM_Clock_PowerOn records the HAL value as the Time base, so Time restarts at zero on every POWER_ON for any clock HAL. ReadClock, PolicyCounterTimer and GetTime use it. The power-cycle test now keeps the HAL running across POWER_OFF and checks Time.

[embhorn]

Correct the Clock rollback claim in the new API documentation

The new public API comment says Clock cannot roll back, but the implementation resumes from the last NV checkpoint after an unclean power loss. That checkpoint can precede a value already returned to a caller; the new recovery test explicitly asserts this case. The safe=NO flag communicates the resulting uncertainty, so the unqualified API claim may mislead integrators.

Suggested fix: State that an unclean power loss can make Clock resume below a previously reported value, with safe=NO until recovery.
Basis: TCG TPM 2.0 Library Part 2, §10.10.5 TPMS_CLOCK_INFO defines safe in terms of whether a greater Clock value was previously reported: https://trustedcomputinggroup.org/wp-content/uploads/Trusted-Platform-Module-2.0-Library-Part-2-Structures_Version-185_pub.pdf

Suggestion:

Suggested change

	FWTPM_CTX. Clock restarts from its last NV checkpoint, so powered-off
	FWTPM_CTX. Clock resumes from its last NV checkpoint and powered-off
	time is not counted. After an unclean shutdown, Clock may roll back
	and reports unsafe until the recovery interval has passed.

[dgarske]

Fixed in 828271c8 with the suggested wording, plus a note that Time restarts at zero.