Add SLH-DSA Support - #455

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

🐺 Skoll Code Review

Overall recommendation: REQUEST_CHANGES
Findings: 26 total — 15 posted, 11 skipped

Posted findings

• [High] Examples are check_PROGRAMS so every build configuration runs them via make check, and CI's example steps silently depend on that — examples/include.am:22-32
• [High] pqc_openssl_example hard-codes SLH-DSA-SHA2-128f, which is absent whenever WOLFPROV_HAVE_SLHDSA is set but SHA2 sets are not built — examples/pqc_openssl_example.c:210-218
• [Medium] wp_encrypt_key_pkcs8: reports the output buffer size as the encrypted length, and ignores the RNG lock result its sibling now checks — src/wp_internal.c:1110-1133
• [Medium] Deterministic seed handling: seedLen survives gen_set_params failure paths, and validator and consumer compute the expected length differently — src/wp_slhdsa_kmgmt.c:1159-1174
• [Medium] wp_slhdsa_import flag handling: asserts hasPub instead of verifying it, and wipes hasPub/hasPriv on failure — src/wp_slhdsa_kmgmt.c:690-702
• [Medium] Signature context state (context string, deterministic, entropy, encoding) persists across re-init — src/wp_slhdsa_sig.c:284-291
• [Medium] Uninitialized locals may break the build under the project's own -Werror hardening — src/wp_slhdsa_sig.c:338
• [Medium] SLH-DSA key mutex covers only sign/verify and the refcount: keymgmt readers, import writers, and lock scope are all unguarded — src/wp_slhdsa_kmgmt.c:432
• [Medium] wp_dh_sync_priv_to_key mutates the shared inner DhKey unlocked, and wp_dh_encode_epki depends on the size function having run first — src/wp_dh_kmgmt.c:2789-2837
• [Low] OSSL_SIGNATURE_PARAM_DETERMINISTIC is sent as unsigned int but advertised and read as int — test/test_slhdsa.c:1139
• [Low] wp_slhdsa_free refcount handling: unsynchronized read, permanent leak when wc_LockMutex fails, and up_ref/free test the mutex result differently — src/wp_slhdsa_kmgmt.c:363-391
• [Low] SLH-DSA export and get_params report the wrong outcome when a key component is absent — src/wp_slhdsa_kmgmt.c:805-875
• [Low] wp_slhdsa_gen_init_base calls wc_FreeRng on an RNG that wc_InitRng may never have initialized — src/wp_slhdsa_kmgmt.c:1067-1091
• [Low] Encoder dispatch binds OSSL_FUNC_ENCODER_IMPORT_OBJECT to a function with a mismatched signature — src/wp_slhdsa_kmgmt.c:1978
• [Info] Preprocessor hygiene in the new SLH-DSA files: dead version guards, a redundant #ifndef, a dropped WP_HAVE_PKCS8_ENC guard, and a stale #endif — src/wp_slhdsa_kmgmt.c:1985-1996

Review generated by Skoll via Claude/Codex

🐺 Skoll Code Review

Overall recommendation: APPROVE
Findings: 24 total — 20 posted, 4 skipped

Posted findings

• [Medium] wp_slhdsa_export_object treats the decoder reference as the key pointer instead of dereferencing it — src/wp_slhdsa_kmgmt.c:1917-1922
• [Medium] wp_slhdsa_export invokes the caller-supplied paramCb while still holding keyMutex — src/wp_slhdsa_kmgmt.c:898-957
• [Medium] hasPub/hasPriv bookkeeping is inconsistent: wp_slhdsa_dup never derives them from the key flags, and wp_slhdsa_import clears them on failures that never touched the key — src/wp_slhdsa_kmgmt.c:483-491
• [Medium] SLH-DSA test coverage gaps: the OpenSSL 3.5+ message API, the SIGNATURE param branch, verify-only builds, and the pure/raw rejection path are all untested — test/test_slhdsa.c:29-30
• [Medium] One-shot sign/verify rejects messages over 64 MiB using the streaming-buffer cap — src/wp_slhdsa_sig.c:378-383
• [Medium] wp_dh_fix_pki_len only corrects the 0x81 short form, and indexes keyData[1]/[2] with no minimum-length guard — src/wp_dh_kmgmt.c:2690-2695
• [Medium] New refuse-cipher-on-traditional guard in ECC/RSA is untested and turns a common CLI invocation into a silent failure — src/wp_rsa_kmgmt.c:3558-3564
• [Medium] PQC example still hardcodes relative ".libs" while openssl_example.c was fixed to resolve from argv[0] — examples/pqc_openssl_example.c:200-217
• [Medium] The SHAKE/SHA2 parameter-set split is not respected outside the example: the interop algorithm list is ungated and the README still recommends plain --enable-slhdsa — test/standalone/tests/pqc_interop/test_pqc_interop.c:1011-1018
• [Medium] The new DH key mutex is one-sided: it covers the import but none of the readers, and one encode path skips the sync helper entirely — src/wp_dh_kmgmt.c:2584-2590, 2656-2687
• [Low] wp_slhdsa_encode locks before any NULL check on slhdsa, and ends with an unreachable second unlock — src/wp_slhdsa_kmgmt.c:1778-1789
• [Low] wp_slhdsa_get_params now reports success with return_size 0 when the component is absent, diverging from wp_mldsa_get_params — src/wp_slhdsa_kmgmt.c:1042-1044
• [Low] SLH-DSA keymgmt provides no OSSL_FUNC_KEYMGMT_VALIDATE despite wc_SlhDsaKey_CheckKey being available — src/wp_slhdsa_kmgmt.c:1357-1383
• [Low] wp_cipher_from_params does not clear cipher/cipherName on the non-UTF8 param path, contradicting its own new comment — src/wp_internal.c:925-927
• [Low] wp_encrypt_key_pkcs8 assigns the wolfSSL-returned length without checking it fits the caller's buffer — src/wp_internal.c:1142
• [Low] verifySig is cleared in verify_message_init but not in the shared wp_slhdsa_init, which the re-init fix otherwise now resets — src/wp_slhdsa_sig.c:631-640
• [Low] PKCS#8 helper hygiene: raw HAVE_PKCS8/NO_PWDBASED test instead of the new WP_HAVE_PKCS8_ENC macro, and an RNG unlock coupled to the shared ok flag — src/wp_internal.c:994
• [Low] Doc comment on wp_mlkem_kem_encapsulate contradicts the new NULL-output behaviour — src/wp_mlkem_kem.c:186
• [Low] Rejected SLH-DSA signing randomizer left in the signature context without cleansing — src/wp_slhdsa_sig.c:778-798
• [Low] wp_slhdsa_free permanently abandons the key object when the refcount mutex cannot be acquired — src/wp_slhdsa_kmgmt.c:373-402

Review generated by Skoll via Claude/Codex