Skip to content

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

Merged
aidangarske merged 2 commits intowolfSSL:masterfrom
dgarske:native_ecc_curves
Apr 23, 2026
Merged

Enhance the TPM 2 native_test to include additional ECC curves (P384)#492
aidangarske merged 2 commits intowolfSSL:masterfrom
dgarske:native_ecc_curves

Conversation

@dgarske
Copy link
Copy Markdown
Member

@dgarske dgarske commented Apr 22, 2026

No description provided.

@dgarske dgarske self-assigned this Apr 22, 2026
Copilot AI review requested due to automatic review settings April 22, 2026 20:34
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

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.

Comment thread examples/native/native_test.c
Comment thread examples/native/native_test.c
Comment thread examples/native/native_test.c Outdated
Comment thread examples/native/native_test.c
@dgarske dgarske force-pushed the native_ecc_curves branch from bbb465a to 664ad70 Compare April 23, 2026 14:26
@dgarske dgarske assigned wolfSSL-Bot and aidangarske and unassigned dgarske Apr 23, 2026
@dgarske dgarske requested a review from aidangarske April 23, 2026 15:14
Copy link
Copy Markdown
Member

@aidangarske aidangarske left a comment

Choose a reason for hiding this comment

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

Skoll Code Review

Scan type: review-security
Overall recommendation: COMMENT
Findings: 4 total — 3 posted, 1 skipped
3 finding(s) posted as inline comments (see file-level comments below)

Posted findings

  • [Medium] TPM_RC_VALUE treated as 'curve unsupported' for every TPM call masks genuine regressionsexamples/native/native_test.c:68-79
  • [Low] Refactor weakens Create/Load failure detection: previously-hard-failed error codes are now silent skipsexamples/native/native_test.c:144-178
  • [Low] goto done after rc=-1 still triggers caller's if (rc != 0) goto exit but prints a confusing skip message pathexamples/native/native_test.c:229-238

Skipped findings

  • [Info] P-384 key created with SHA-256 nameAlg and scheme hash may be rejected by strict TPMs

Review generated by Skoll

Comment thread examples/native/native_test.c Outdated
Comment thread examples/native/native_test.c
Comment thread examples/native/native_test.c
@dgarske dgarske requested a review from aidangarske April 23, 2026 17:20
Copy link
Copy Markdown
Member

@aidangarske aidangarske left a comment

Choose a reason for hiding this comment

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

Skoll Code Review

Scan type: review
Overall recommendation: COMMENT
Findings: 5 total — 5 posted, 0 skipped
5 finding(s) posted as inline comments (see file-level comments below)

Posted findings

  • [Medium] The new P-384 path can pass without ever exercising the added regression checkexamples/native/native_test.c:145-149,168-172,190-194,215-219,244-248,268-272
  • [Medium] P-384 key configured with SHA-256 nameAlg/scheme — deviates from repo conventionexamples/native/native_test.c:131-140
  • [Low] Two parallel arrays must stay in sync — one-step-off error waiting to happenexamples/native/native_test.c:444-447
  • [Low] Loop bound uses magic literal instead of array lengthexamples/native/native_test.c:1323
  • [Low] zCompare uses TPM2B_PUBLIC_KEY_RSA to hold an ECC x-coordinateexamples/native/native_test.c:96

Review generated by Skoll

Comment thread examples/native/native_test.c
Comment thread examples/native/native_test.c Outdated
Comment thread examples/native/native_test.c Outdated
Comment thread examples/native/native_test.c
Comment thread examples/native/native_test.c Outdated
@dgarske dgarske force-pushed the native_ecc_curves branch from b774260 to 28c30ee Compare April 23, 2026 18:12
@aidangarske aidangarske self-requested a review April 23, 2026 21:36
@aidangarske aidangarske merged commit 72bd518 into wolfSSL:master Apr 23, 2026
123 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants