Skip to content

Fix expected callback behavior for ECC/Dilithium for Free Callbacks#9962

Merged
dgarske merged 6 commits intowolfSSL:masterfrom
night1rider:ecc-dilithium-callback-free-fix
Mar 13, 2026
Merged

Fix expected callback behavior for ECC/Dilithium for Free Callbacks#9962
dgarske merged 6 commits intowolfSSL:masterfrom
night1rider:ecc-dilithium-callback-free-fix

Conversation

@night1rider
Copy link
Contributor

Description

A crypto callback is a replacement for the wolfSSL function: returning 0 means the callback handled cleanup, only CRYPTOCB_UNAVAILABLE signals fallback to software.

wc_ecc_free and wc_dilithium_free ignored the wc_CryptoCb_Free return value, breaking this convention established by AES and SHA free handlers.

Without this fix, callback authors who only clean up their hardware resource and return 0 have wolfSSL do the software cleanup of the rest of the context.

If the return check is added later to match convention, wolfSSL would return early on 0, silently skipping software cleanup and possibly leaking memory, a that would be hard to detect on embedded devices.

This PR also updates PKCS11 free handlers to return CRYPTOCB_UNAVAILABLE after HSM cleanup, adds ECC/dilithium free test cases, and adds CI coverage.

The original addition of the ECC Free Callback: #9780
The original addition of the Dilithium Free Callback: #9836

…et passed to hit callback paths when configured and that Dilithium will be retested in the callback section of the wolfCrypt test.
… it respects the return code of the callback
@night1rider night1rider force-pushed the ecc-dilithium-callback-free-fix branch from c00ee29 to 1625b29 Compare March 12, 2026 21:14
…turn CRYPTOCB_UNAVAILABLE after attempting the context free so the caller still does software cleanup on the rest of the context that the callback does not handle.
Copy link
Contributor

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

Aligns ECC and Dilithium “free” crypto callback behavior with the existing wolfCrypt convention: callbacks return 0 when they fully handled cleanup, and only CRYPTOCB_UNAVAILABLE requests software fallback cleanup. This prevents inconsistent cleanup behavior across crypto backends (software vs HW/HSM via callbacks).

Changes:

  • Update wc_ecc_free() and wc_dilithium_free() to honor wc_CryptoCb_Free() return values (early-return unless CRYPTOCB_UNAVAILABLE).
  • Adjust PKCS#11 free callback handling to return CRYPTOCB_UNAVAILABLE after HSM object release so software cleanup still occurs.
  • Extend tests/CI to cover Dilithium free-callback paths and run a new CI config that enables the relevant callback features.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
wolfcrypt/src/ecc.c Honor crypto-callback free return value to decide whether to run software cleanup.
wolfcrypt/src/dilithium.c Same as ECC: respect callback return code for free behavior.
wolfcrypt/src/wc_pkcs11.c Return CRYPTOCB_UNAVAILABLE from PKCS#11 free handlers to ensure software cleanup still runs.
wolfcrypt/test/test.c Run Dilithium tests under cryptocb test path and add cryptocb free handling for ECC/Dilithium in the test callback; update Dilithium inits to *_init_ex(..., devId).
.github/workflows/os-check.yml Add CI matrix entry to exercise cryptocb + pkcallbacks + dilithium configuration.
Comments suppressed due to low confidence (1)

wolfcrypt/test/test.c:47004

  • On wc_dilithium_init_ex() failure this returns immediately, skipping the out: cleanup path. In WOLFSSL_SMALL_STACK && !WOLFSSL_NO_MALLOC builds that leaks msg, key, and pubExported. Route this error through the existing out: label (or free the allocated buffers) before returning.

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

You can also share your feedback on Copilot code review. Take the survey.

@night1rider night1rider self-assigned this Mar 12, 2026
@night1rider night1rider added the For This Release Release version 5.9.0 label Mar 12, 2026
@dgarske dgarske merged commit 00cd1a7 into wolfSSL:master Mar 13, 2026
461 checks passed
@Frauschi
Copy link
Contributor

Thanks for this fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

For This Release Release version 5.9.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants