Skip to content

Fix wolfCLU Fenrir#218

Open
aidangarske wants to merge 4 commits intowolfSSL:mainfrom
aidangarske:fenrir-fixes-1
Open

Fix wolfCLU Fenrir#218
aidangarske wants to merge 4 commits intowolfSSL:mainfrom
aidangarske:fenrir-fixes-1

Conversation

@aidangarske
Copy link
Copy Markdown
Member

@aidangarske aidangarske commented Apr 2, 2026

Description

F-570 Allocate +1 for null terminator in IV and key string copies
F-572 Move XMEMSET before wc_dilithium_init in sign path
F-573 Free data and hash buffers on all return paths in verify
F-574 Add ForceZero on password buffer in PKCS12
F-575 Add ForceZero on password and keyBuffer in PKCS8
F-645 Use wolfCLU_checkOutform for RSA -outform option
F-646 Add #else error handling for disabled hash algorithms in switch
F-647 Return WOLFCLU_SUCCESS from wolfCLU_setAttributes
F-648 Check wc_RsaPrivateKeyDecode return value directly
F-649 Check wc_EccPrivateKeyDecode return value directly
F-657 Map sha224 to WC_HASH_TYPE_SHA224, use else-if chain
F-658 Move XMEMSET before wc_dilithium_init in verify path
F-659 Heap-allocate large temp buffers in GenChimeraCertSign
F-660 Use XSTRCMP for exact match in config parsing
F-732 Map dgst -out to WOLFCLU_OUTFILE
F-733 Write actual signature length in ED25519 sign output
F-734 Write actual DER size in ECC private key output
F-735 Change || to && in XMSS/XMSSMT argv bounds check
F-736 Guard base64 allocation with success check in rand
F-740 Use wolfCLU_checkOutform for CRL -outform option
F-741 Remove free of borrowed EVP_PKEY from X509_get0_pubkey
F-742 Add ret+1 < argc guard before argv access (3 files)
F-1108 Return error code on file open failure in dilithium sign
F-1109 Break on BN_rand failure, guard serial number set
F-1484 Free data buffer on read failure in sign
F-1485 Add NULL check on X509_NAME_oneline return
F-1486 Refactor RSA certgen to goto cleanup for resource management
F-1487 Free keyBuf on read failure in RSA certgen
F-1488 Free keyBuf on read failure in ED25519 certgen
F-1489 Add ForceZero before freeing signer key in OCSP responder

Test coverage

Tests updated and what they cover

tests/pkey/rsa-test.sh

  • F-645 - verify -outform error message references "outform"

tests/x509/CRL-verify-test.sh

  • F-740 - verify -outform error message references "outform"

tests/encrypt/enc-test.sh

  • F-570 - encrypt with explicit hex key/IV succeeds

tests/genkey_sign_ver/genkey-sign-ver-test.sh

  • F-733 - ED25519 sig file is exactly 64 bytes
  • F-734 - ECC DER key file size is reasonable, not buffer-sized
  • F-742 - missing -inkey value fails gracefully, not segfault
  • F-735 - missing -height value fails gracefully (if XMSS compiled in)
  • F-648/F-649 — sign with empty key file returns error

tests/hash/hash-test.sh

  • F-742 - missing -in value fails gracefully, not segfault

tests/bench/bench-test.sh

  • F-742 - missing -time value fails gracefully, not segfault

tests/dgst/dgst-test.sh

  • F-732 - dgst -out creates output file and round-trips with -signature

tests/x509/x509-process-test.sh

  • F-741 - x509 -modulus -noout does not crash

tests/x509/x509-req-test.sh

  • F-657 - SHA-224 cert signature algorithm check
  • F-660 - abbreviated keyUsage "d" does not match digitalSignature
  • F-647 - req with challengePassword attribute succeeds

Fixed fsan leak for x509 that was being leaked and suppressed by command line tests
SHA-224 test assertion fails if not found now
properly error on dillithium return
Add missing force zeros for keybuf
fix ed25519 certgen cleanup on error paths

Other tests not covered by test validated by internal testing suite + code review since test paths where not hit with simple command line code tests in make check

…649,

  F-657, F-658, F-659, F-660, F-732, F-733, F-734, F-735, F-736, F-740,
  F-741, F-742, F-1108, F-1109, F-1484, F-1485, F-1486, F-1487, F-1488,
  F-1489
Copilot AI review requested due to automatic review settings April 2, 2026 23:54
@aidangarske aidangarske marked this pull request as draft April 2, 2026 23:54
@aidangarske aidangarske self-assigned this Apr 2, 2026

This comment was marked as resolved.

wolfSSL-Fenrir-bot

This comment was marked as resolved.

Copilot AI review requested due to automatic review settings April 3, 2026 17:54

This comment was marked as resolved.

- Copilot: privkey double-free — Fixed: added privkey = NULL after mid-function free at line 454
- Copilot: ForceZero NULL guard in OCSP — Fixed: added if (signerKeyDer != NULL && signerKeyDerSz > 0) guard
- Copilot: ForceZero on key buffers in GenChimeraCertSign — Fixed: added ForceZero on caKeyBuf, altCaKeyBuf, serverKeyBuf before XFREE
- Fenrir: pkey vs privkey — No change needed: pkey is a borrowed ref from X509_get0_pubkey, not owned by caller. Removing the free was correct.
- Fenrir: Missing ForceZero on heap key buffers — Same as Copilot wolfSSL#3, addressed above
- CI: switch-enum errors — Fixed: removed inner #ifdef guards on enum cases that always exist, added SM3 under #ifdef WOLFSSL_SM3, removed
WC_HASH_TYPE_MAX (duplicate value)
- CI: heap-buffer-overflow in strstr — Fixed: allocate inBufSz + 1 and null-terminate for XSTRSTR safety
- CI: heap-use-after-free — Fixed by the privkey NULL fix above
Copilot AI review requested due to automatic review settings April 3, 2026 18:05

This comment was marked as resolved.

  - Copilot: BN_bn2hex NULL guard — Added NULL check on num before calling wolfSSL_BN_bn2hex
  - Copilot: return 0 on missing args — Changed return ret to return USER_INPUT_ERROR at lines 118 and 194
  - Copilot: SHA-224 test assertion — Test now fails if sha224 is NOT found (not just if sha256 is)
  - Copilot: dilithium_init return value — Capture into ret for proper error logging
  - Security review: Missing ForceZero on keyBuf — Added ForceZero before XFREE on all keyBuf free paths in both certgen files
@aidangarske aidangarske marked this pull request as ready for review April 3, 2026 19:19
@aidangarske aidangarske requested review from Copilot and removed request for Copilot April 3, 2026 19:19
@aidangarske aidangarske requested review from lealem47 April 3, 2026 23:36
@aidangarske aidangarske assigned lealem47 and unassigned lealem47 and aidangarske Apr 3, 2026
@aidangarske aidangarske requested a review from dgarske April 6, 2026 18:57
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.

4 participants