Skip to content

Fix private scalar handling under ECC key blinding and add CI coverage - #11233

Open
dgarske wants to merge 1 commit into
wolfSSL:masterfrom
dgarske:ecc_blind_k_fixes
Open

Fix private scalar handling under ECC key blinding and add CI coverage#11233
dgarske wants to merge 1 commit into
wolfSSL:masterfrom
dgarske:ecc_blind_k_fixes

Conversation

@dgarske

@dgarske dgarske commented Aug 21, 2026

Copy link
Copy Markdown
Member

Summary

With key blinding on, wc_ecc_key_get_priv() resolves to ecc_get_k(), which recomputes key->ku = key->k XOR key->kb and returns &key->ku. It is a read-only accessor over a regenerated scratch value: writes through it are recomputed over on the next call, and zeroing it leaves the secret itself in memory. Without WOLFSSL_ECC_BLIND_K the accessor is just (key)->k, and nothing in CI defined it, so the misuse accumulated.

What was broken

Site Effect
sakke.c wc_MakeSakkeKey() Master secret generated into scratch, so the retry loop always saw zero and generation failed with RNG_FAILURE_E. SAKKE key generation did not work at all.
sakke.c x2, eccsi.c x2 Decode/import paths read the private value into scratch.
eccsi.c "Erase v", "erase j" Ephemeral secrets erased through scratch, so never actually cleared.
src/pk_ec.c wolfssl_ec_key_int_copy() wc_ecc_key_get_priv(dst) called before dst->dp was set: NULL dereference, segfault in test_wolfSSL_EC_KEY_dup.
src/pk_ec.c SetECKeyInternal() The OpenSSL-compat private key import wrote the scalar into scratch and then tested the recomputed value, silently dropping the key. On the hot path for ECDSA_do_sign, EC_KEY_set_private_key, ECDH_compute_key and d2i_ECPrivateKey.
wc_pkcs11.c mp_forcezero() through the accessor left the private scalar in memory.
port/silabs/silabs_ecc.c Key readout written into scratch.

Approach

Rather than per-site #ifdefs, the existing ecc_get_k() / ecc_blind_k_rng() family in ecc.h gains two members, so every call site is a one-liner with no conditional compilation:

  • ecc_get_k_raw(key) - writable handle on the stored scalar; a write is followed by ecc_blind_k_rng(), which is already a no-op (0) without blinding.
  • ecc_forcezero_k(key) - erases the scalar, its blind and the scratch together; plain mp_forcezero() without blinding.

The FIPS-header compat shims in the touched files get matching fallbacks. The one remaining #ifdef is the blind copy in wolfssl_ec_key_int_copy(), where dst->dp is not yet set so a fresh blind cannot be installed.

CI

Adds all-ecc-blind-k (--enable-all CPPFLAGS=-DWOLFSSL_ECC_BLIND_K) to .github/configs/os-check-linux.json. Distinct from the existing blind-private-key entry, which sets WOLFSSL_BLIND_PRIVATE_KEY and does not imply this macro.

Testing

--enable-all --enable-lms with and without -DWOLFSSL_ECC_BLIND_K: testwolfcrypt clean and the full unit suite passes, including test_wolfSSL_EC_KEY_dup (previously a segfault), SAKKE and ECCSI. Note the blinded config did not pass on master before this batch: ecc_mulmod_test() (fixed in #11172), the EC_KEY_dup segfault, and SAKKE key generation - the last two are fixed here.

The Silicon Labs and PKCS#11 changes have no host build and were made by inspection; they follow the same pattern as the rest.

@dgarske dgarske self-assigned this Aug 21, 2026
@dgarske
dgarske requested review from SparkiDev and a lite review from Copilot August 21, 2026 15:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes incorrect use of wc_ecc_key_get_priv() when WOLFSSL_ECC_BLIND_K is enabled (where the accessor returns a regenerated scratch value rather than the stored scalar share), and adds CI coverage to ensure the blinded configuration is exercised.

Changes:

  • Add ecc_get_k_raw() (writable handle to the stored scalar share) and ecc_forcezero_k() (wipe stored shares + scratch under blinding) and update relevant call sites to stop writing/zeroing via wc_ecc_key_get_priv().
  • Harden ECC blinding internals by zero-extending operands before constant-time XORs and adding consistent “fail closed” behavior.
  • Add a Linux CI config entry that enables WOLFSSL_ECC_BLIND_K to prevent regressions.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
wolfssl/wolfcrypt/ecc.h Adds new raw-scalar accessor macro and forcezero helper API to avoid misuse of the read-only blinded accessor.
wolfcrypt/src/ecc.c Updates blinding XOR paths to ensure operands are grown/zero-extended; adds ecc_forcezero_k() implementation.
src/pk_ec.c Fixes OpenSSL-compat EC key copy/import paths to write/copy the stored scalar share and (re)install blinding safely.
wolfcrypt/src/sakke.c Fixes SAKKE keygen/import to write the stored scalar share and then install a blind.
wolfcrypt/src/eccsi.c Fixes ECCSI decode/import and ensures private material is erased via the correct helper under blinding.
wolfcrypt/src/wc_pkcs11.c Ensures ECC private scalar is actually wiped when clear is requested, even under blinding.
wolfcrypt/src/port/silabs/silabs_ecc.c Updates Silicon Labs key import path to write the stored scalar share and install a blind.
.github/configs/os-check-linux.json Adds a CI configuration that defines WOLFSSL_ECC_BLIND_K (and enables pkcs11) to cover these code paths.

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

Comment thread wolfcrypt/src/port/silabs/silabs_ecc.c Outdated
@dgarske
dgarske force-pushed the ecc_blind_k_fixes branch from 1dde6ae to addd5ea Compare August 21, 2026 15:54
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.

2 participants