fips: keep PRIVATE_KEY_UNLOCK/op/LOCK inside a single cgo call - #59
Merged
Conversation
lealem47
requested review from
sebastian-carpenter
and
a lite review from Copilot
September 3, 2026 19:11
There was a problem hiding this comment.
🟡 Changes recommended
The current diff introduces build-breaking issues in the C preambles (undefined WC_SPKRE_F and C helpers calling PRIVATE_KEY_UNLOCK/LOCK which are not C symbols).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR fixes a FIPS-threading correctness issue by ensuring the “private-key gate” (unlock → wolfCrypt op → lock) happens within a single cgo call, preventing goroutine rescheduling onto a different OS thread mid-sequence.
Changes:
- Add per-operation static C helpers that wrap
unlock/op/lockinside one cgo call for HKDF, PBKDF2, and selected ECC operations. - Expand and clarify FIPS private-key gate documentation and threading requirements in
fips.go.
File summaries
| File | Description |
|---|---|
| hmac.go | Adds a static C HKDF wrapper intended to keep unlock/op/lock within one cgo call. |
| aes.go | Adds a static C PBKDF2 wrapper intended to keep unlock/op/lock within one cgo call. |
| ecc.go | Adds static C wrappers for ECC private export and shared secret to keep unlock/op/lock within one cgo call. |
| fips.go | Documents the FIPS private-key gate/threading behavior; adjusts the C preamble for private-key lock/unlock helpers. |
Review details
Suppressed comments (3)
fips.go:38
- The WC_SPKRE_F macro is referenced by WC_PRIVATE_KEY_LOCK/UNLOCK but its definition was removed, leaving WC_SPKRE_F undefined in the C preamble (build will fail). Reintroduce the macro (or call wolfCrypt_SetPrivateKeyReadEnable_fips directly).
// int WC_PRIVATE_KEY_LOCK(void) {
// return WC_SPKRE_F(0,WC_KEYTYPE_ALL);
// }
// int WC_PRIVATE_KEY_UNLOCK(void) {
// return WC_SPKRE_F(1,WC_KEYTYPE_ALL);
ecc.go:148
- This C helper calls PRIVATE_KEY_UNLOCK/LOCK, which are Go functions and not available to C. Switch to WC_PRIVATE_KEY_UNLOCK/WC_PRIVATE_KEY_LOCK and declare them to avoid implicit-declaration errors on clang.
// PRIVATE_KEY_UNLOCK();
// ret = wc_ecc_export_x963_ex(key, out, outLen, compressed);
// PRIVATE_KEY_LOCK();
ecc.go:158
- This C helper calls PRIVATE_KEY_UNLOCK/LOCK, which are Go functions and not available to C. Use WC_PRIVATE_KEY_UNLOCK/WC_PRIVATE_KEY_LOCK (and declare them) so the helper compiles/links correctly.
// PRIVATE_KEY_UNLOCK();
// ret = wc_ecc_shared_secret(private_key, public_key, out, outLen);
// PRIVATE_KEY_LOCK();
- Files reviewed: 4/4 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
sebastian-carpenter
requested changes
Sep 3, 2026
sebastian-carpenter
approved these changes
Sep 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A goroutine is pinned to its OS thread only for the duration of a single
cgo call. Calling PRIVATE_KEY_UNLOCK, then the wolfCrypt operation, then
PRIVATE_KEY_LOCK as three separate cgo calls therefore leaves a window in
which the scheduler can move the goroutine to another thread. The
operation then runs on a thread whose counter was never incremented and
fails with FIPS_PRIVATE_KEY_LOCKED_E (-287), while the thread that took
the unlock is left permanently unlocked.
To address this, this PR moves the unlock/crypto call/lock into one static cgo helper function