Fix wolfCLU Fenrir

[aidangarske]

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

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

[Copilot]

Pull request overview

This PR hardens wolfCLU (“Fenrir”) CLI behavior by fixing argument parsing edge cases, correcting crypto/hashing/output handling, tightening resource cleanup, and adding regression tests to prevent crashes and incorrect outputs.

Changes:

• Fix correctness bugs in crypto/encoding/signing flows (e.g., SHA-224 mapping, dgst -out, ED25519/ECC output sizes, RSA/ECC key decode return checking).
• Improve robustness/security via safer argv bounds checks, more consistent cleanup/freeing, and additional ForceZero wiping of sensitive buffers.
• Expand shell-based CLI regression tests for the above behaviors (incl. crash-prevention and output validation).

Reviewed changes

Copilot reviewed 30 out of 30 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
tests/x509/x509-req-test.sh	Adds coverage for SHA-224 signature alg selection, exact-match keyUsage parsing, and config attributes (challengePassword).
tests/x509/x509-process-test.sh	Adds regression test for x509 -modulus -noout not crashing.
tests/x509/CRL-verify-test.sh	Adds -outform invalid value error-message coverage.
tests/pkey/rsa-test.sh	Adds -outform invalid value error-message coverage for RSA tool.
tests/hash/hash-test.sh	Adds missing -in value graceful-failure test.
tests/genkey_sign_ver/genkey-sign-ver-test.sh	Adds tests for ED25519 signature length, ECC DER key sizing, missing argv values, and bad key inputs.
tests/encrypt/enc-test.sh	Adds regression test for explicit hex key/IV handling (null terminator sizing).
tests/dgst/dgst-test.sh	Adds test for dgst -out creating a signature file and verifying it.
tests/bench/bench-test.sh	Adds missing -time value graceful-failure test.
src/x509/clu_x509_sign.c	Heap-allocates large temp buffers; fixes SHA224 mapping; adds subject NULL check; improves serial generation error handling.
src/x509/clu_config.c	Makes config parsing require exact matches for keyUsage/basicConstraints tokens; enables req attributes success path.
src/x509/clu_cert_setup.c	Fixes borrowed EVP_PKEY free; adds cleanup frees for key BIO/privkey.
src/tools/clu_rand.c	Guards base64 allocation/encoding behind success checks.
src/sign-verify/clu_verify.c	Frees buffers on early read failures; moves dilithium key zeroing before init.
src/sign-verify/clu_sign.c	Frees buffers on early read failures; checks RSA/ECC decode return values; fixes ED25519 output length; improves dilithium error return.
src/sign-verify/clu_sign_verify_setup.c	Adds ret+1 < argc guards before argv access for several options.
src/sign-verify/clu_dgst_setup.c	Maps dgst -out to output-file option (WOLFCLU_OUTFILE) and handles it in parsing.
src/sign-verify/clu_crl_verify.c	Uses wolfCLU_checkOutform for -outform.
src/pkey/clu_rsa.c	Uses wolfCLU_checkOutform for -outform.
src/pkcs/clu_pkcs8.c	Adds ForceZero for password and stdin key buffer.
src/pkcs/clu_pkcs12.c	Adds ForceZero for password buffer.
src/ocsp/clu_ocsp.c	Adds ForceZero before freeing responder signer key DER.
src/hash/clu_hash_setup.c	Adds ret+1 < argc guards before argv access.
src/hash/clu_alg_hash.c	Adds explicit error handling when hash algorithms are disabled at compile time.
src/genkey/clu_genkey.c	Writes actual DER size for ECC private key output.
src/genkey/clu_genkey_setup.c	Fixes XMSS/XMSSMT argv bounds check logic (`
src/crypto/clu_crypto_setup.c	Fixes XMALLOC/XSTRLCPY sizing for IV/key strings (+1 for NUL).
src/certgen/clu_certgen_rsa.c	Refactors to goto cleanup for consistent resource management; fixes keyBuf free on read failure.
src/certgen/clu_certgen_ed25519.c	Fixes keyBuf free on read failure.
src/benchmark/clu_bench_setup.c	Adds ret+1 < argc guard before argv access for -time.

---

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

[Copilot]

The new cleanup frees privkey when non-NULL, but privkey is already freed earlier in the reqFlag signing block without being set to NULL. This makes the reqFlag success path hit a double-free at function end. Consider consolidating ownership: either remove the earlier wolfSSL_EVP_PKEY_free(privkey) and rely on this cleanup block, or set privkey = NULL immediately after the earlier free so this cleanup doesn’t free it twice.

Suggested change

	if (privkey != NULL)
	if (privkey != NULL && !reqFlag)

[Copilot]

wolfCLU_ForceZero does not handle a NULL pointer (it dereferences mem unconditionally). In this cleanup block, signerKeyDer can still be NULL when earlier loading fails and jumps to cleanup, which would crash here. Guard this call with if (signerKeyDer != NULL && signerKeyDerSz > 0) (or change wolfCLU_ForceZero to safely no-op on NULL/0).

Suggested change

	wolfCLU_ForceZero(signerKeyDer, signerKeyDerSz);
	if (signerKeyDer != NULL && signerKeyDerSz > 0)
	wolfCLU_ForceZero(signerKeyDer, signerKeyDerSz);

[Copilot]

These buffers include private key material (caKeyBuf, altCaKeyBuf, serverKeyBuf, etc.) that is read from files and then freed at function exit. Now that they’re heap-allocated, freeing without securely wiping can leave key bytes in freed heap pages. Consider calling wolfCLU_ForceZero on key-containing buffers (at least caKeyBuf, altCaKeyBuf, serverKeyBuf) before XFREE to match the project’s other key/password hardening changes in this PR.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #218

Scan targets checked: wolfclu-bugs, wolfclu-src

Findings: 2
2 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.

[wolfSSL-Fenrir-bot]

🔶 [Medium] Removed pkey free without adding it to cleanup — potential memory leak
Category: Resource leaks on error paths

The PR removes wolfSSL_EVP_PKEY_free(pkey) from line 775 (inside a conditional block) but only adds wolfSSL_EVP_PKEY_free(privkey) to the cleanup section at line 849. The variable names pkey and privkey are different. If these refer to distinct WOLFSSL_EVP_PKEY objects (e.g., pkey obtained via wolfSSL_X509_get_pubkey() and privkey being the signing key), then pkey is now leaked on all paths through that conditional block. If they are aliases for the same pointer, this is fine — but the differing names suggest they may be distinct objects.

}
-            wolfSSL_EVP_PKEY_free(pkey);
        }
    }
...
+    if (privkey != NULL)
+        wolfSSL_EVP_PKEY_free(privkey);

Recommendation: Verify whether pkey and privkey are the same pointer. If they are distinct, add wolfSSL_EVP_PKEY_free(pkey) back — either inline or in the cleanup block. If they are the same, consider using a consistent variable name to avoid confusion.

[wolfSSL-Fenrir-bot]

🔶 [Medium] Missing ForceZero on heap-allocated private key buffers in GenChimeraCertSign
Category: Missing ForceZero

This PR converts 11 large stack-allocated buffers to heap allocations and adds XFREE cleanup at lines 925-942. Three of these buffers hold private key material: caKeyBuf (CA private key read from bioCaKey), altCaKeyBuf (alternative CA key from bioAltCaKey), and serverKeyBuf (server key from bioServerKey). These are freed without wolfCLU_ForceZero or ForceZero being called first, leaving key material in heap memory after deallocation.

This is inconsistent with the same PR's changes elsewhere: wolfCLU_ForceZero is added for signerKeyDer in clu_ocsp.c:887, for password in clu_pkcs12.c:237, and for both password and keyBuffer in clu_pkcs8.c:190,282. The heap-allocated key buffers in GenChimeraCertSign contain equally sensitive material but lack the same protection.

XFREE(caKeyBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
    XFREE(altCaKeyBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
    XFREE(sapkiBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
    XFREE(altSigAlgBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
    XFREE(scratchBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
    XFREE(preTbsBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
    XFREE(altSigValBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
    XFREE(derBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
    XFREE(outBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
    XFREE(caCertBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
    XFREE(serverKeyBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);

Recommendation: Add wolfCLU_ForceZero (or ForceZero) calls before XFREE for the three buffers that hold private key material:

wolfCLU_ForceZero(caKeyBuf, LARGE_TEMP_SZ);
XFREE(caKeyBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
wolfCLU_ForceZero(altCaKeyBuf, LARGE_TEMP_SZ);
XFREE(altCaKeyBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
...
wolfCLU_ForceZero(serverKeyBuf, LARGE_TEMP_SZ);
XFREE(serverKeyBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);

Guard the ForceZero calls with NULL checks since the buffers may be NULL on early error paths.