Skip to content

SLH-DSA: add to wolfCrypt -Wconversion CI matrix and clean warnings#16

Closed
Frauschi wants to merge 1 commit intomasterfrom
claude/add-slhdsa-wconversion-checks
Closed

SLH-DSA: add to wolfCrypt -Wconversion CI matrix and clean warnings#16
Frauschi wants to merge 1 commit intomasterfrom
claude/add-slhdsa-wconversion-checks

Conversation

@Frauschi
Copy link
Copy Markdown
Owner

@Frauschi Frauschi commented May 4, 2026

Summary

Third in the hash-based-signature -Wconversion cleanup series after PRs #14 (LMS) and #15 (XMSS). Independent of those branches.

  • Phase A — CI matrix. Adds --enable-slhdsa to all 11 existing wolfCrypt-Wconversion.yml build rows (each row already builds ML-KEM + all-crypto under conversion flags). Adds 4 dedicated SLH-DSA rows for sub-options the default does not cover (small, small-mem, sha2, verify-only on 32-bit -m32). Adds a new test_slhdsa_runtime job mirroring the LMS / XMSS runtime jobs that builds with crypttests and runs ./wolfcrypt/test/testwolfcrypt for five configs (default-SHAKE, small, sha2, verify-only, 32-bit). Total: 15 build rows + 5 runtime rows. timeout-minutes raised to 10 — SLH-DSA adds ~9 kLoC per row and the 32-bit + smallstack rows need headroom.

  • Phase B — clean conversion warnings in wc_slhdsa.c and the SLH-DSA paths in asn.c. Patterns reused from PRs LMS: add to wolfCrypt -Wconversion CI matrix and clean conversion warnings #14 / XMSS: add to wolfCrypt -Wconversion CI matrix and clean conversion warnings #15:

    • U-suffix on integer literals that flow into word32 size fields (2*n → 2U*n, len*n → (word32)len*n, ForceZero / XMEMCPY / wc_RNG_GenerateBlock size literals).
    • (word32)1U << z for shifts whose exponent reaches the SLH-DSA tree-height parameters (params->h up to 68 for 256s/f) — avoids 32-bit UB. All eight call sites use the same form.
    • word16 csum / word16 mask accumulators with (word16) casts at the assignment boundary for the WOTS+ checksum and base-2^b decomposition.
    • HA_*, SHAKE256_SET_*_X4, HASH_H / HASH_F macros: parenthesise operands and add (word32) / (size_t) / (byte) casts so types narrow at the macro boundary, not at every call site.
    • slhdsakey_chain_idx_x4_NN signature: byte i, byte s → word32 i, word32 s; loop counters int j → word32 j; cast at invocation.
    • slhdsakey_shake256_set_seed_ha_x4 / _hash_x4 return type int → word32.
    • XMSS / FORS tree generators: introduce const word32 wi = (word32)i; const word32 wz = (word32)z; and use wi / wz in the inner-loop offset arithmetic so each multiplication / shift is in word32 from the start.
    • word64* cast-qual fixes: const on read-only inputs in slhdsakey_hash_h_2_x4 and friends.
    • asn.c: wc_SlhDsaOidToParam((int)keyOID) and (int)outSz at the SLH-DSA MakeSignature site.

  • Critical semantic fix. slhdsakey_base_2b originally wrote baseb[j] = (byte)((total >> bits) & mask);. baseb is word16* and FORS values reach 14 bits (parameter a), so the (byte) cast silently truncated values >255 and broke verification on FORS configs. Caught by testwolfcrypt under the new test_slhdsa_runtime job. Fixed to (word16).

Branch history

  • b9a192ba — initial Wconversion cleanup + CI extension.
  • e1f54685 — review fixes: revert SLHDSA_WM1 to plain int (the U-suffix forced cascading (int)/(sword8)/(byte) casts at every signed-comparison site without saving anything); unify (word32)1U shift form across all eight sites; switch export n back to byte for consistency; deduplicate the csum word16 comment; add wc_static_assert(SLHDSA_MAX_MSG_SZ <= 255) to document the invariant the (byte)i / (byte)j truncations depend on; bump CI timeouts.

Test plan

  • All 15 matrix rows build clean under the conversion flags (verified locally for default, intelasm, smallstack, smallstack+intelasm, NO_INT128, 32-bit, mlkem-small, mlkem-no-large-code, smallstack+NO_INT128, mlkem-small-mem-32bit, mlkem-small-mem+NO_INT128, slhdsa=yes,small / small-mem / sha2 / verify-only-32bit).
  • All 5 test_slhdsa_runtime configs pass slhdsa_test() in testwolfcrypt (default, small, sha2, verify-only, 32-bit).
  • Benchmark with -DBENCH_MIN_RUNTIME_SEC=5.0F across the six SHAKE variants (128s/f, 192s/f, 256s/f) — sign / verify deltas within ±5% of master.
  • CI green on Frauschi/wolfssl.

Out of scope

  • Changing SLH-DSA public API types.
  • Adding new SLH-DSA parameter sets or hash variants.
  • Touching LMS or XMSS sources (independent PR).

https://claude.ai/code/session_01EJmy1bKDgHseTwZ5Qqpu1g

Generated by Claude Code

@claude @Frauschi
SLH-DSA: add to wolfCrypt -Wconversion CI matrix and clean warnings
d9f91c3 
Adds --enable-slhdsa to the existing 11 wolfCrypt-Wconversion build
rows and adds 4 dedicated rows for the sub-options that the default
doesn't cover (small, small-mem, sha2, verify-only on 32-bit). Adds
a test_slhdsa_runtime job mirroring test_lms_runtime / test_xmss_runtime
that builds without --disable-crypttests and runs testwolfcrypt for
five configs (default-SHAKE, small, sha2, verify-only, 32-bit).

Cleans the conversion warnings that surface in wc_slhdsa.c and the
SLH-DSA code paths in asn.c. Patterns applied (same as LMS PR #14 and
XMSS PR #15):

  - U-suffix on integer literals that flow into word32 size fields
    (SLHDSA_W, SLHDSA_WM1, 2*n -> 2U*n, len*n -> (word32)len*n,
    sk-size literals 4 and 3 -> 4U / 3U at ForceZero / XMEMCPY /
    wc_RNG_GenerateBlock call sites).
  - (word32)1U << z for shifts whose exponent reaches the SLH-DSA
    tree height parameters (params->h, params->h_m, params->d) up
    to 68 for 256s/f variants - avoids 32-bit UB.
  - word16 csum / mask accumulators with explicit (word16) casts at
    the assignment boundary for WOTS+ checksum and base-2^b
    decomposition.
  - HA_*, SHAKE256_SET_*_X4 and HASH_H/HASH_F macros: parenthesise
    operands and add (word32) / (size_t) / (byte) casts so the
    argument types narrow at the macro boundary, not at every call
    site.
  - slhdsakey_chain_idx_x4_NN signature: byte i, byte s -> word32 i,
    word32 s; loop counters int j -> word32 j; call sites cast at
    invocation.
  - slhdsakey_shake256_set_seed_ha_x4 / _hash_x4 return type
    int -> word32; locals to word32.
  - XMSS / FORS tree generators: introduce const word32 wi = (word32)i
    / wz = (word32)z and use wi/wz in the inner-loop offset
    arithmetic so each multiplication / shift is in word32 from the
    start.
  - word64* cast-qual fixes: add const where the helper only reads
    its input (slhdsakey_hash_h_2_x4 inputs, etc.).
  - asn.c: int slhDsaParam = wc_SlhDsaOidToParam((int)keyOID) and
    ret = (int)outSz at three call sites where outSz is word32.

Critical semantic fix: slhdsakey_base_2b wrote
baseb[j] = (byte)((total >> bits) & mask); baseb is word16* and
FORS values can be up to 14 bits (parameter a). The byte cast
truncated values >255 and broke verification on FORS variants.
Changed to (word16) - caught by testwolfcrypt under the new
runtime job.

Verified clean against all 15 build matrix rows (default, intelasm,
smallstack, smallstack+intelasm, NO_INT128, 32-bit, mlkem-small,
mlkem-no-large-code, smallstack+NO_INT128, mlkem-small-mem-32bit,
mlkem-small-mem+NO_INT128, slhdsa=yes,small / small-mem / sha2 /
verify-only-32bit). All five test_slhdsa_runtime configs pass
testwolfcrypt's slhdsa_test().

Benchmark with -DBENCH_MIN_RUNTIME_SEC=5.0F across the six SHAKE
variants (128s/f, 192s/f, 256s/f) shows sign / verify deltas within
+/-5% of master after this change.

https://claude.ai/code/session_01EJmy1bKDgHseTwZ5Qqpu1g

SLH-DSA Wconversion: review fixes

Address review findings from the previous commit:

- wc_slhdsa.c:1681: replace the (incorrect) mask comment with a comment
  on the actual word16 cast. baseb is word16* and FORS values reach 14
  bits (parameter a), so the original (byte) cast silently truncated.
- wc_slhdsa.c:57-59: revert SLHDSA_W / SLHDSA_WM1 to plain int. The U
  suffix forced cascaded (int)SLHDSA_WM1, (sword8)SLHDSA_WM1 and
  (byte)(SLHDSA_WM1 - x) casts at every signed-comparison site without
  saving anything. The (word16) cast at the WOTS+ csum / mask
  assignment is sufficient.
- wc_slhdsa.c: drop (int)SLHDSA_WM1 at three call sites in
  slhdsakey_chain_idx_to_max_{16,24,32}, made redundant by the WM1
  revert.
- wc_slhdsa.c: unify (word32)1U shift form across all eight call
  sites; the previous commit changed only four of them.
- wc_slhdsa.c:8305, 8340: switch ExportPrivate / ExportPublic n back
  to byte to match every other internal use of params->n.
- wc_slhdsa.c:127: add wc_static_assert(SLHDSA_MAX_MSG_SZ <= 255) to
  document the invariant that the WOTS+ chain helpers' (byte)i and
  (byte)j casts depend on (max len for current parameter sets is
  2*32+3 = 67).
- wc_slhdsa.c:4315-4320: deduplicate the csum-word16 explanatory
  comment; the second copy now defers to the first.
- wolfCrypt-Wconversion.yml: bump build_library and
  test_slhdsa_runtime timeout-minutes from 6 to 10. SLH-DSA adds
  ~9 kLoC to every row and the 32-bit multilib + smallstack rows
  need headroom.

All five representative build rows still compile clean
(default, intelasm, smallstack, sha2, small-mem, verify-only-32bit).
All five test_slhdsa_runtime configs still pass slhdsa_test().

https://claude.ai/code/session_01EJmy1bKDgHseTwZ5Qqpu1g
@Frauschi Frauschi force-pushed the claude/add-slhdsa-wconversion-checks branch from e1f5468 to d9f91c3 Compare May 4, 2026 08:06
@Frauschi Frauschi closed this May 4, 2026
@Frauschi Frauschi deleted the claude/add-slhdsa-wconversion-checks branch May 6, 2026 09:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Frauschi @claude