F-7450: Wipe the ECCSI SSK in wc_FreeEccsiKey by stenslae · Pull Request #11998 · wolfSSL/wolfssl

stenslae · 2026-11-04T20:13:39Z

Description

Updated wc_FreeEccsiKey to call mp_forcezero() on key->ssk and key->tmp instead of mp_free()
Added test_wc_Eccsi_FreeZeroizesSsk to test_eccsi.c to verify that the SSK buffer is zeroed out before being released back to the heap.

Testing

Built --enable-all-crypto --enable-heapmath and passed test suite with updated tests.

Checklist

added tests
updated/added doxygen
updated appropriate READMEs
Updated manual and documentation

stenslae · 2026-11-04T20:13:50Z

@claude review

Copilot

🟡 Changes recommended

The test’s allocator hooks can cause invalid accesses or frees during ECC cache cleanup, and its wipe watcher falsely fails when memory-zero checking adds an allocation prefix.

2 open issues

What changed in this PR

This PR strengthens ECCSI key cleanup by wiping sensitive multiprecision values and adds a regression test for secret signing key (SSK) zeroization.

Changes:

Replace mp_free() with mp_forcezero() for the SSK and temporary value.
Add and register a heap-math test that inspects the SSK buffer before release.

File	Description
`wolfcrypt/src/eccsi.c`	Wipes the SSK and temporary value during key cleanup.
`tests/api/test_eccsi.h`	Declares and registers the zeroization test.
`tests/api/test_eccsi.c`	Adds allocator hooks to verify the SSK is wiped before release.

_{Review effort: Balanced}

github-actions · 2026-11-04T20:18:24Z

MemBrowse Memory Report · Explore what changed →

zephyr-frdm_rw612

FLASH: log_const_area -8 B, rodata -8 B, text +1,592 B (+0.6%, 284,040 B / 67,108,864 B, total: 0% used)

No memory changes detected for:

_{Catch memory regressions in CI with MemBrowse. Free for open source.}

claude

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline finding on the test, I also checked the library change in wc_FreeEccsiKey. mp_forcezero exists for all three math backends: sp_int, tfm and integer.c. In heap math it still calls mp_free after zeroing, so switching from mp_free does not leak the ssk/tmp buffers.

Extended reasoning...

The change in wolfcrypt/src/eccsi.c replaces mp_free with mp_forcezero for the ECCSI SSK and tmp, and adds an allocator-hook test. I checked that mp_forcezero is defined for every math backend and still frees heap memory in integer.c.

wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #11998

Scan targets checked: wolfcrypt-src, wolfcrypt-bugs, wolfssl-bugs
Not reviewed (no scan target covers them): tests/api/test_eccsi.h
Coverage: 2 of 2 in-scope changed file(s) opened by the reviewer

Fenrir result: Approved ✅

No new issues found in the changed files.

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

Review tier: Lite

stenslae · 2026-11-04T20:22:44Z

@claude review

Copilot

🟢 Approval recommended

No active review issues remain for the ECCSI key-cleanup changes and accompanying zeroization regression test.

No open issues 🎉

2 resolved since last review

_{Review effort: Balanced}

wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #11998

Scan targets checked: wolfssl-bugs
Unchanged since last review (not re-run): wolfcrypt-src, wolfcrypt-bugs
Not reviewed (no scan target covers them): tests/api/test_eccsi.h
Coverage: 1 of 1 in-scope changed file(s) opened by the reviewer

Fenrir result: Approved ✅

No new issues found in the changed files.

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

Review tier: Lite

claude

Code review found no issues

No high-confidence issues detected in this change.

This review covers commit 0747760, which is no longer the latest commit on this pull request; later commits are not covered by it.

wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #11998

Scan targets checked: wolfssl-bugs
Unchanged since last review (not re-run): wolfcrypt-src, wolfcrypt-bugs
Not reviewed (no scan target covers them): tests/api/test_eccsi.h
Coverage: 0 of 1 in-scope changed file(s) opened by the reviewer; not opened: tests/api/test_eccsi.c

Fenrir result: Approved ✅

No new issues found in the changed files.

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

Review tier: Lite

Copilot

🟡 Changes recommended

The test guard includes heap-math FREERTOS and FREERTOS_TCP configurations without WOLFSSL_TRACK_MEMORY, where XFREE bypasses callbacks and causes false failures even when the SSK is correctly wiped.

1 open issue

Exclude FreeRTOS heap configurations from the XFREE test guard · New

_{Review effort: Balanced}

wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #11998

Scan targets checked: wolfssl-bugs
Unchanged since last review (not re-run): wolfcrypt-src, wolfcrypt-bugs
Not reviewed (no scan target covers them): tests/api/test_eccsi.h
Coverage: 1 of 1 in-scope changed file(s) opened by the reviewer

Fenrir result: Approved ✅

No new issues found in the changed files.

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

Review tier: Lite

Copilot

🟡 Changes recommended

The callback-based test incorrectly runs under HAVE_IO_POOL or WOLFSSL_EMBOS, where XFREE bypasses allocator callbacks and causes false failures despite correct zeroization.

1 open issue

Exclude allocator configurations that bypass zeroization callbacks · New

1 resolved since last review

Exclude FreeRTOS heap configurations from the XFREE test guard

_{Review effort: Balanced}

Copilot

🟢 Approval recommended

No active review issues are reported for the ECCSI key-cleanup hardening and SSK zeroization regression test.

No open issues 🎉

1 resolved since last review

Exclude allocator configurations that bypass zeroization callbacks

_{Review effort: Balanced}

wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #11998

Scan targets checked: wolfssl-bugs
Unchanged since last review (not re-run): wolfcrypt-src, wolfcrypt-bugs
Not reviewed (no scan target covers them): tests/api/test_eccsi.h
Coverage: 1 of 1 in-scope changed file(s) opened by the reviewer

Fenrir result: Approved ✅

No new issues found in the changed files.

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

Review tier: Lite

SparkiDev · 2026-11-05T01:27:22Z

-        mp_free(&key->ssk);
-        mp_free(&key->tmp);
+        mp_forcezero(&key->ssk);
+        mp_forcezero(&key->tmp);


When does key->tmp hold sensitive data.

It doesn't, so I put it back to mp_free. Only the SSK is wiped now.

SparkiDev · 2026-11-05T01:30:05Z

 #ifdef WOLFCRYPT_ECCSI_CLIENT
-        mp_free(&key->ssk);
-        mp_free(&key->tmp);
+        mp_forcezero(&key->ssk);


Add a WOLFSSL_CHECK_MEM_ZERO check.
When private data is put into key->ssk or key->tmp, call mp_memzero_add().

key->tmp only holds public values, so it does not need wiping.

stenslae · 2026-11-06T20:23:38Z

@claude review

Copilot

🟢 Approval recommended

No active review issues are reported for the SSK memory-zeroization change.

No open issues 🎉

_{Review effort: Balanced}

claude

Code review found no issues

No high-confidence issues detected in this change.

wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #11998

Scan targets checked: wolfcrypt-src, wolfcrypt-bugs, wolfssl-bugs
Not reviewed (no scan target covers them): tests/api/test_eccsi.h
Coverage: 2 of 2 in-scope changed file(s) opened by the reviewer

Fenrir result: Approved ✅

No new issues found in the changed files.

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

Review tier: Max

stenslae requested a review from wolfSSL-Fenrir-bot November 4, 2026 20:13

stenslae self-assigned this Nov 4, 2026

Copilot AI balanced review requested due to automatic review settings November 4, 2026 20:13

Copilot started reviewing on behalf of stenslae November 4, 2026 20:14

View session

Copilot AI reviewed Nov 4, 2026

View reviewed changes

Comment thread tests/api/test_eccsi.c Resolved

Comment thread tests/api/test_eccsi.c Resolved

claude Bot reviewed Nov 4, 2026

View reviewed changes

Comment thread tests/api/test_eccsi.c Resolved

wolfSSL-Fenrir-bot reviewed Nov 4, 2026

View reviewed changes

stenslae force-pushed the fix/eccsi-free-forcezero-ssk branch from 70dd9b8 to 0747760 Compare November 4, 2026 20:22

stenslae requested review from wolfSSL-Fenrir-bot and a balanced review from Copilot November 4, 2026 20:22

Copilot started reviewing on behalf of stenslae November 4, 2026 20:22

View session

Copilot AI reviewed Nov 4, 2026

View reviewed changes

wolfSSL-Fenrir-bot reviewed Nov 4, 2026

View reviewed changes

stenslae force-pushed the fix/eccsi-free-forcezero-ssk branch from 0747760 to 841150b Compare November 4, 2026 20:27

stenslae requested review from wolfSSL-Fenrir-bot and a balanced review from Copilot November 4, 2026 20:27

claude Bot reviewed Nov 4, 2026

View reviewed changes

Copilot started reviewing on behalf of stenslae November 4, 2026 20:27

View session

wolfSSL-Fenrir-bot reviewed Nov 4, 2026

View reviewed changes

Copilot AI reviewed Nov 4, 2026

View reviewed changes

Comment thread tests/api/test_eccsi.c Resolved

stenslae force-pushed the fix/eccsi-free-forcezero-ssk branch from 841150b to 683359b Compare November 4, 2026 20:35

stenslae requested review from wolfSSL-Fenrir-bot and a balanced review from Copilot November 4, 2026 20:35

Copilot started reviewing on behalf of stenslae November 4, 2026 20:35

View session

wolfSSL-Fenrir-bot reviewed Nov 4, 2026

View reviewed changes

Copilot AI reviewed Nov 4, 2026

View reviewed changes

Comment thread tests/api/test_eccsi.c Resolved

stenslae force-pushed the fix/eccsi-free-forcezero-ssk branch from 683359b to df9ce1a Compare November 4, 2026 21:09

stenslae requested review from wolfSSL-Fenrir-bot and a balanced review from Copilot November 4, 2026 21:09

Copilot started reviewing on behalf of stenslae November 4, 2026 21:09

View session

Copilot AI reviewed Nov 4, 2026

View reviewed changes

wolfSSL-Fenrir-bot reviewed Nov 4, 2026

View reviewed changes

stenslae assigned wolfSSL-Bot and unassigned stenslae Nov 4, 2026

SparkiDev requested changes Nov 5, 2026

View reviewed changes

SparkiDev assigned stenslae Nov 5, 2026

Emma Stensland added 2 commits November 6, 2026 13:16

F-7450: Wipe the ECCSI SSK in wc_FreeEccsiKey

8dce99d

F-7450: Mem-zero check the ECCSI SSK; free tmp normally

d0dfa93

key->tmp only holds public values, so it does not need wiping.

stenslae force-pushed the fix/eccsi-free-forcezero-ssk branch from df9ce1a to d0dfa93 Compare November 6, 2026 20:17

stenslae requested review from wolfSSL-Fenrir-bot and a balanced review from Copilot November 6, 2026 20:23

Copilot started reviewing on behalf of stenslae November 6, 2026 20:23

View session

Copilot AI reviewed Nov 6, 2026

View reviewed changes

claude Bot reviewed Nov 6, 2026

View reviewed changes

wolfSSL-Fenrir-bot reviewed Nov 6, 2026

View reviewed changes

Conversation

stenslae commented Nov 4, 2026

Description

Testing

Checklist

Uh oh!

stenslae commented Nov 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Nov 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

MemBrowse Memory Report · Explore what changed →

zephyr-frdm_rw612

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #11998

Uh oh!

stenslae commented Nov 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #11998

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #11998

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #11998

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #11998

Uh oh!

SparkiDev Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

stenslae Nov 6, 2026

Choose a reason for hiding this comment

Uh oh!

SparkiDev Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

stenslae commented Nov 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

github-actions Bot commented Nov 4, 2026 •

edited

Loading