Repository navigation
SLH-DSA: build with NO_ASN and leave out unused hash families - #11894
Conversation
wc_slhdsa.c included asn.h unconditionally and compiled its DER key encode/decode functions without a NO_ASN guard. asn.h needs an mp_int type, so a minimal verify-only build with no big-int math had to compile asn.c, sp_int.c and wolfmath.c just to link. Guard the include, the DER functions and their prototypes on NO_ASN. A NO_ASN + WOLFCRYPT_ONLY build also drops misc.c's c32toa/ato32, which SLH-DSA uses, so keep them for SLH-DSA as is already done for XMSS and LMS. The SHA2/SHAKE hash dispatch was decided at run time from the parameter set, so a SHA2-128s-only build with WOLFSSL_SLHDSA_NO_SHAKE still referenced SHA-512, which only the category 3 and 5 SHA2 sets use, and SHAKE-256. Compile the SHAKE code only when a SHAKE parameter set is built, and the SHA-512 branches only when a SHA2-192/256 set is (new WC_SLHDSA_NEED_SHA512). With a single family the hash macros call it directly, and SlhDsaKey holds only the hash state that is used. The pre-hash buffer bound now follows the hashes that are compiled in, so a SHA-256-only build no longer fails its static assert. The MC/DC white-box tests that call these static functions directly are guarded the same way. SLH-DSA-SHA2-128s verify-only on Cortex-M33 (-Os, Thumb-2 asm, gc-sections): wc_slhdsa plus linked hash code goes from 28,197 to 7,629 bytes, and SlhDsaKey from 1,160 to 480 bytes. Verified on QEMU mps2-an505 and on an STM32U585.
|
@claude review |
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #11894
Scan targets checked: wolfcrypt-src, wolfcrypt-bugs, wolfssl-src, wolfssl-bugs
Coverage: 3 of 5 in-scope changed file(s) opened by the reviewer; not opened: tests/unit-mcdc/test_slhdsa_hash_fault_whitebox.c, wolfssl/wolfcrypt/settings.h
Fenrir result: Approved ✅
No new issues found in the changed files.
Advisory only — this automated result does not count as a GitHub approval.
Review tier: Lite
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. A human should still look at it, because it changes the SLH-DSA crypto code's hash dispatch and the public SlhDsaKey layout across several new build-macro combinations.
What was reviewed:
- The
#ifdef'd if/else chains in the SHA2 helpers and in Init/Free/sign/verify. With SHAKE or SHA-512 compiled out, noelseis left dangling, andnis either guarded or(void)'d. - The single-family
HASH_*andHASH_T_*macros, and the SHA-256-sizedblock,digestand HMAC buffers in builds without SHA-512. - The
SLHDSA_LARGEST_APPROVED_PHM_LENstatic assert, and theNO_ASNguards on the DER functions and their prototypes. I also looked at--disable-asnunit tests that still call the DER APIs and ruled that out.
Extended reasoning...
The change affects wc_slhdsa.c, wc_slhdsa.h and settings.h in wolfCrypt, plus the MC/DC whitebox tests. It adds compile-time guards that remove the SHAKE code, the SHA-512 code or the DER/ASN code from SLH-DSA builds. This is crypto signing and verification code, and the size of the public SlhDsaKey struct now depends on WOLFSSL_SLHDSA_NO_SHAKE and WC_SLHDSA_NEED_SHA512. If the library and an application are built with different macros, the struct layouts will not match. The edits are mechanical and I found no bugs, but there are many macro combinations and the surface is security-sensitive, so I deferred instead of approving.
Frauschi
left a comment
Frauschi
left a comment
There was a problem hiding this comment.
Nice footprint win. Built and ran testwolfcrypt on a cryptonly --disable-asn config, a SHAKE + SHA2-128 build and a gcc-16 SHA2-128s verify-only build; all pass. A few small things inline, nothing blocking.
Review follow-ups for the NO_ASN and hash family gating: - SLHDSA_IS_SHA2() is now a constant when only SHA2 parameter sets are compiled in. The runtime family checks then fold away like the HASH_* macros, and no SHA2-only path can fall through to a SHAKE branch that is compiled out. - Add SLHDSA_SHA2_USE_SHA256(n) so the SHA2 helpers keep a plain if/else with only the SHA-512 arm under WC_SLHDSA_NEED_SHA512, instead of an #ifdef'd if and a bare block. - Replace the pre-hash size static assert, which now mirrored WC_MAX_DIGEST_SIZE, with a check in slhdsakey_validate_prehash() that the expected PHM fits in WC_MAX_DIGEST_SIZE. - Update the white-box test rationale: in 128-bit-only builds the SHA-512 decisions are compiled out rather than unreachable.
With only SHA2 parameter sets compiled in, SLHDSA_IS_SHA2() is constant, so the hash family re-init in wc_SlhDsaKey_PrivateKeyDecode() and wc_SlhDsaKey_PublicKeyDecode() and the SHAKE pre-hash choice in slhdsa_test() became dead code that clang -Wunreachable-code reports. The family can only change when both are compiled in, so compile them only then.
|
tests good with multi-test with |
Without SHA-512 the SHA-256 branch is the only one left, so the function read as if control could reach its end without returning a value. Take the SHA-512 case as an early return instead and finish with the SHA-256 update, and say in the SLHDSA_SHA2_USE_SHA256() comment that it is constant true when no category 3 or 5 SHA2 set is compiled in.
Description
Two footprint problems in
wc_slhdsa.c, found while measuring post-quantum verify-only builds on Cortex-M33:NO_ASN.asn.hwas included unconditionally and the DER encode/decode functions had noNO_ASNguard.asn.hneeds anmp_inttype, so a minimal verify-only build (no RSA/ECC/DH, no big-int math) had to compileasn.c,sp_int.candwolfmath.cjust to link. The include, the DER functions and their prototypes are now guarded onNO_ASN.settings.halso keeps misc.c'sc32toa/ato32for SLH-DSA whenNO_ASN+WOLFCRYPT_ONLYwould drop them, as it already does for XMSS and LMS.SHA2_128Senabled andWOLFSSL_SLHDSA_NO_SHAKEdefined, the SHA-512 paths (used only by the category 3 and 5 SHA2 sets) and the SHAKE paths stayed referenced. Now:WC_SLHDSA_NEED_SHA512).HASH_*macros call it directly andSLHDSA_IS_SHA2()is a constant, so no runtime family check is left.SlhDsaKeyonly holds the hash state that is used.slhdsakey_validate_prehash()that the expected digest fits inWC_MAX_DIGEST_SIZE.The MC/DC white-box tests in
tests/unit-mcdc/that call these static functions directly are guarded the same way.SLH-DSA-SHA2-128s verify-only (
WOLFSSL_WC_SLHDSA_SMALL+WOLFSSL_WC_SLHDSA_SMALL_MEM), arm-none-eabi-gcc 15.3-mcpu=cortex-m33 -Os, Thumb-2 asm,--gc-sections:wc_slhdsa.otextwc_slhdsa+ hash code after gc-sectionssizeof(SlhDsaKey)The "this PR" column is the
NO_ASNbuild with only SHA-256. Built with this PR, master's config, which still compiles the extra sources, comes to 15,440 bytes, because nothing references them any more. SLH-DSA-SHAKE-128s is unchanged.Testing
make checkwith./configure --enable-slhdsa,--enable-slhdsa=yes,sha2and--enable-slhdsa=verify-only,small,128s,sha2(macOS arm64, from a git checkout so-Werroris on).wolfcrypt/test/testwolfcryptwith:--enable-slhdsa=sha2-128s,=verify-only,small,sha2-128s,=sha2-192sand=128s,sha2-128s,small.--enable-cryptonly --disable-asn --disable-rsa --disable-ecc --disable-dh, with=verify-only,sha2-128s, the default, and=yes,sha2.--enable-intelasmbuilds (CC="clang -arch x86_64", run under Rosetta) with=sha2-128s,=yes,sha2and=verify-only,sha2-192s: build, thentestwolfcrypt.test_slhdsa_whiteboxandtest_slhdsa_hash_fault_whitebox, built and run the wayrun-whitebox-smoke.shdoes, against static=sha2-128s,=verify-only,sha2-128sand=yes,sha2libraries.user_settings.hbuilds of SHA2-128s and SHAKE-128s verify-only, includingNO_ASNbuilds that compile onlywc_slhdsa.cand the one hash they use. Each verifies a host-generated signature and rejects a tampered one.NO_ASNSHA2-128s verifier verifies a signature and rejects a tampered one, using 852 bytes of stack and 12.1 M cycles at the 4 MHz reset clock.Not covered by this PR:
NO_ASNand SLH-DSA still fails insrc/internal.c, becausewc_SlhDsaOidToParamis only declared when ASN is enabled. That build also failed before this change, earlier, inwc_slhdsa.c.configurestill requires SHA-512 and HMAC for any SHA2 parameter set.Checklist