Fix EdDSA lazy public-key derivation race on shared private-only keys

[sameehj]

Summary

A private-only EdDSA key (a seed-only PKCS#8 Ed25519 or Ed448 key) has no
public half after import. wolfProvider derives the public half later, on
first use. The derivation writes into the shared wolfSSL key object without
holding the key mutex. When two or more threads first use the same
EVP_PKEY at the same time, they sign or export with a partly written
public key. This produces failed or invalid signatures.

Reported by: Fenrir finding 11559.

Root cause

wc_ed25519_make_public / wc_ed448_make_public set pubKeySet when they
write to the output buffer. They do not always fill key->p. Only
wc_ed*_import_public stores the value into key->p. The old code derived
the public half in three places without the mutex:

• wp_ed25519_export_public / wp_ed448_export_public
• wp_ed25519_digest_sign / wp_ed448_digest_sign (derived before the lock;
  only wc_ed*_sign_msg was locked)
• wp_Ed25519PublicKeyToDer / wp_Ed448PublicKeyToDer

A concurrent first use could read key->p while another thread wrote it.

Fix

Derive the public half in one place, under the key mutex.

• Add a derivePub callback to wp_EcxData (set for Ed25519 and Ed448,
  NULL for X25519/X448).
• Add wp_ecx_ensure_pub(). It takes the key mutex, then calls the derive
  helper. The helper calls make_public into a local buffer, then
  import_public, which is the only call that sets key->p and pubKeySet.
• Remove the unlocked derivation from the export and DER helpers. They are
  now plain export/encode.
• Call wp_ecx_ensure_pub() from every first-use site: the sign paths,
  wp_ecx_get_params_enc_pub_key, wp_ecx_match_pub_key,
  wp_ecx_export_keypair, wp_ecx_dup, and the SPKI branch of
  wp_ecx_encode.

The SPKI-only guard in wp_ecx_encode keeps the public key out of a private
key encoding. In wp_ecx_dup, the derive runs before the key copy, so the
copy also gets a happens-before edge against a concurrent first use.

Scope: other algorithms

I reviewed the other key types for the same pattern.

• ECC also derives a public key from a private key, but it does so at
  import/decode time, while the object has one owner. No race.
• ML-DSA does not derive on first use. It reports hasPub = 0 for a
  private-only key and fails cleanly.
• RSA, DH, ECDSA, and ML-KEM have no first-use public derivation.

EdDSA was the only affected type.

Test

test_ecx_shared_key_first_use (test 195) loads one seed-only key, then uses
it from 4 threads at the same time. Half the threads sign, half export. The
signers verify against a separate public-only key, so a bad public half
cannot hide a bad signature. The workload is fixed and small, so any failure
is a defect, not timing.

• Without the fix: the test fails on every run (10/10 local runs).
• With the fix: the test passes, and the full unit suite passes.

ThreadSanitizer (the tsan job) is the reliable detector for the data race
itself. The test is the deterministic correctness guard.

Severity

Medium. The window is first use of a shared private-only key. The impact is
failed or invalid signatures, not key disclosure or forgery. A TLS load path
that warms the public key via X509_check_private_key is not affected. A
sign-only service with a raw private import and a thread pool is the
realistic case.

Commits

• test: adds the failing test.
• fix: adds the fix.

The first commit fails on its own by design (red), the second makes it pass
(green). Please merge as a unit; do not run per-commit CI or bisect across
the pair.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #478

Scan targets checked: wolfprovider-bugs, wolfprovider-src

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.

[sameehj]

Fixed in af3b531. The shared helpers now size sig[] and pub[] from WP_ECX_SHARED_SIG_SIZE / WP_ECX_SHARED_PUB_SIZE, which pick ED25519 sizes when WP_HAVE_ED448 is off, ED448 sizes when only Ed448 is built, and MAX of both when both are built — the same pattern as the readback buffers earlier in this file.

These two comments are the same compile break; the Memory management label does not apply. I did not add a CI cell for Ed25519-only wolfSSL: a unit test cannot catch a compile failure that current --enable-all-crypto jobs never hit. Verified locally that the Ed25519-only branch expands to ED25519_SIG_SIZE / ED25519_PUB_KEY_SIZE.

[sameehj]

Fixed in af3b531. The shared helpers now size sig[] and pub[] from WP_ECX_SHARED_SIG_SIZE / WP_ECX_SHARED_PUB_SIZE, which pick ED25519 sizes when WP_HAVE_ED448 is off, ED448 sizes when only Ed448 is built, and MAX of both when both are built — the same pattern as the readback buffers earlier in this file.

These two comments are the same compile break; the Memory management label does not apply. I did not add a CI cell for Ed25519-only wolfSSL: a unit test cannot catch a compile failure that current --enable-all-crypto jobs never hit. Verified locally that the Ed25519-only branch expands to ED25519_SIG_SIZE / ED25519_PUB_KEY_SIZE.

[sameehj]

Fixed in af3b531. The shared helpers now size sig[] and pub[] from WP_ECX_SHARED_SIG_SIZE / WP_ECX_SHARED_PUB_SIZE, which pick ED25519 sizes when WP_HAVE_ED448 is off, ED448 sizes when only Ed448 is built, and MAX of both when both are built — the same pattern as the readback buffers earlier in this file.

These two comments are the same compile break; the Memory management label does not apply. I did not add a CI cell for Ed25519-only wolfSSL: a unit test cannot catch a compile failure that current --enable-all-crypto jobs never hit. Verified locally that the Ed25519-only branch expands to ED25519_SIG_SIZE / ED25519_PUB_KEY_SIZE.

[sameehj]

Fixed in af3b531. The shared helpers now size sig[] and pub[] from WP_ECX_SHARED_SIG_SIZE / WP_ECX_SHARED_PUB_SIZE, which pick ED25519 sizes when WP_HAVE_ED448 is off, ED448 sizes when only Ed448 is built, and MAX of both when both are built — the same pattern as the readback buffers earlier in this file.

These two comments are the same compile break; the Memory management label does not apply. I did not add a CI cell for Ed25519-only wolfSSL: a unit test cannot catch a compile failure that current --enable-all-crypto jobs never hit. Verified locally that the Ed25519-only branch expands to ED25519_SIG_SIZE / ED25519_PUB_KEY_SIZE.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #478

Scan targets checked: wolfprovider-bugs, wolfprovider-src

Fenrir result: Approved ✅

No new issues found in the changed files.

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

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

[aidangarske]

Skoll Code Review

Scan type: review

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

Posted findings

• [Medium] Start gate does not wait for every worker — test/test_ecx.c:1341-1345
• [Medium] Concurrent wolfProvider verification is not exercised — test/test_ecx.c:1231-1237

Review generated by Skoll

[aidangarske]

Start gate does not wait for every worker

The new test broadcasts immediately after creating the threads, without confirming that every worker has reached the condition-variable wait. A delayed worker can observe start == 1 and proceed only after another worker has completed the first-use derivation. Consequently, the test is scheduler-dependent and can pass against the unfixed implementation despite claiming to release all workers together.

Fix: Track a ready-worker count under startLock. Have each worker increment it before waiting, and have the parent wait until all created workers are ready before setting start and broadcasting.

[sameehj]

Thank you, addressed.

[aidangarske]

Concurrent wolfProvider verification is not exercised

This PR adds mutex protection to wp_ed25519_digest_verify and wp_ed448_digest_verify, but the new threaded test creates its verification key in osslLibCtx and verifies through the OpenSSL default provider. The changed wolfProvider verification callbacks therefore never run concurrently, so an omitted or incorrect verify lock would remain invisible to both this test and TSan.

Fix: Add a contention case using a shared wolfProvider-backed public key and a precomputed valid signature, with multiple workers verifying concurrently.

[sameehj]

Addressed, thanks.