Enhance the TPM 2 native_test to include additional ECC curves (P384)

[dgarske]

No description provided.

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Extends the TPM2 native test ECDH/ZGen coverage to exercise additional ECC curves (notably P-384) and refactors the ECDH/ZGen sequence into a reusable helper with “graceful skip” behavior when TPM features are unsupported.

Changes:

• Added a curve-aware ECDH/EC_Ephemeral/ZGen_2Phase test helper and an “unsupported curve/command” classifier.
• Updated TPM2_Native_TestArgs to run the ECDH/ZGen test flow for both P-256 and P-384 and validate coordinate sizes.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

scheme.scheme is set to TPM_ALG_ECDH, but the code writes the hash algorithm into scheme.details.ecdsa.hashAlg. This is the wrong union member for the selected scheme and can result in incorrect parameters being sent to the TPM. Use the union field corresponding to ECDH (e.g., scheme.details.ecdh.hashAlg in typical TPMT_ECC_SCHEME layouts) or omit the hashAlg entirely if the ECDH scheme does not require it in this library’s API.

Suggested change

	cmdIn.create.inPublic.publicArea.parameters.eccDetail.scheme.details.ecdsa.hashAlg
	cmdIn.create.inPublic.publicArea.parameters.eccDetail.scheme.details.ecdh.hashAlg

[Copilot]

This relies on struct layout overlap within a union to preserve cmdOut.ecdh.pubPoint after writing cmdOut.ecdhZ. Reading a different union member than the one most recently written is undefined/unspecified in standard C for unrelated struct types (and can break under different compilers/flags). Store cmdOut.ecdh.pubPoint into a separate local variable before calling TPM2_ECDH_ZGen, and then pass that saved copy into ZGen_2Phase.

Suggested change

	/* ECDH ZGen (compute shared secret). Note: in the cmdOut union,
	* ECDH_ZGen_Out.outPoint overlaps ECDH_KeyGen_Out.zPoint but leaves
	* ECDH_KeyGen_Out.pubPoint intact, so cmdOut.ecdh.pubPoint is still
	* valid after this call for use by ZGen_2Phase below. */
	XMEMSET(&cmdIn.ecdhZ, 0, sizeof(cmdIn.ecdhZ));
	cmdIn.ecdhZ.keyHandle = eccKey.handle;
	cmdIn.ecdhZ.inPoint = cmdOut.ecdh.pubPoint;
	rc = TPM2_ECDH_ZGen(&cmdIn.ecdhZ, &cmdOut.ecdhZ);
	/* Preserve the generated public point before reusing the cmdOut union
	* for ECDH_ZGen output. */
	{
	TPM2B_ECC_POINT ecdhPubPoint = cmdOut.ecdh.pubPoint;
	/* ECDH ZGen (compute shared secret). */
	XMEMSET(&cmdIn.ecdhZ, 0, sizeof(cmdIn.ecdhZ));
	cmdIn.ecdhZ.keyHandle = eccKey.handle;
	cmdIn.ecdhZ.inPoint = ecdhPubPoint;
	rc = TPM2_ECDH_ZGen(&cmdIn.ecdhZ, &cmdOut.ecdhZ);
	}

[Copilot]

Treating any TPM_RC_VALUE as “unsupported curve/command” is very broad and can downgrade real test failures into skips (e.g., malformed parameters, unexpected TPM behavior). Consider narrowing the TPM_RC_VALUE skip logic to only the specific command(s) and parameter(s) where you’ve observed this behavior (e.g., only TPM2_Create with ECC curve selection), or gate it behind a more explicit check so incorrect inputs still fail loudly.

[Copilot]

cmdIn is a union reused for multiple command input structs; writing only flushHandle without first clearing cmdIn.flushCtx risks carrying stale bytes from a prior union member into the FlushContext_In struct (even if today’s implementation only reads flushHandle). Prefer to XMEMSET(&cmdIn.flushCtx, 0, sizeof(cmdIn.flushCtx)) before setting flushHandle for robustness.

Suggested change

	if (eccKey.handle != TPM_RH_NULL) {
	if (eccKey.handle != TPM_RH_NULL) {
	XMEMSET(&cmdIn.flushCtx, 0, sizeof(cmdIn.flushCtx));