Skip to content

Commit 3b2d711

Browse files
claudeclaude
committed
XMSS: clean -Wconversion warnings and add XMSS to Wconversion CI matrix
Add --enable-xmss to all 11 existing wolfCrypt-Wconversion matrix rows so the XMSS sources (wc_xmss.c, wc_xmss_impl.c) are exercised under the same strict conversion-warning flags already enforced for ML-KEM. Add 2 new matrix rows that cover the XMSS sub-options (small, verify-only on 32-bit) which use distinct code paths. Add a test_xmss_runtime job that builds --enable-xmss and --enable-xmss=yes,small (without --disable-crypttests) and runs testwolfcrypt for each. The build-only matrix can't catch a bad cast that silently changes the wire output; the runtime job is the safety net (same pattern as test_lms_runtime in PR #14). To make the XMSS sources compile cleanly under -Wconversion -Warith-conversion -Wsign-conversion -Wcast-qual: - wc_xmss.h: U-suffix on length constants, (word32) casts inside size macros (XMSS_HASH_PRF_DATA_LEN, XMSS_CHAIN_HASH_DATA_LEN, XMSS_RAND_HASH_DATA_LEN, XMSS_RETAIN_LEN) so arithmetic stays unsigned. - wc_xmss_impl.c: - U-suffix on the local fixed-parameter constants (XMSS_WOTS_W, XMSS_HASH_PADDING_*, XMSS_PRF_M_LEN, XMSS_IDX_LEN, etc.). - In wc_xmss_msg_convert: kept word16 csum (per RFC 8391 the WOTS+ checksum field is fixed 16-bit, the algorithm relies on 2^16 wraparound), with explicit (word16) casts at every assignment to silence the warning without changing semantics. Same pattern as the LMS PR's wc_lmots_q_expand fix. - Replaced hand-rolled length expressions with the (now fixed) macros where possible (XMSS_HASH_PRF_DATA_LEN, XMSS_RAND_HASH_DATA_LEN). - Cast (byte)i at addr_buf[XMSS_ADDR_*]/addr[XMSS_ADDR_*] = i sites where i is word32 (WOTS+ chain/hash addressing). - Changed wc_idx_zero/wc_idx_update parameter from word8/int len to word32 so length flows cleanly into XMEMSET; rewrote the increment loop to use unsigned i with i > 0 termination. - Restored (word32)1U on shifts whose exponent can reach the tree height (up to 60 for XMSSMT-SHA2_60/12_256) — bare 1U is unsigned int which is undefined behaviour for shifts >= 32 on most targets. - Changed wc_xmss_bds_state_treehash_init / wc_xmss_bds_next_idx 'int i' parameter to word32, removing call-site (int) casts. - Cast at the narrowest assignment boundary throughout (e.g., `const word8 hsk = (word8)(params->sub_h - params->bds_k)`, `addr[XMSS_ADDR_LAYER] = (word32)(params->d - 1)`). - Switch (word8)min(...) cast at the narrow assignment site to silence the word32→word8 narrowing from the shared min() helper. - wc_xmss.c: - 3 → 3U in wc_RNG_GenerateBlock(rng, seed, 3 * key->params->n). - Cast public API's int msgLen/mLen to (word32) when forwarding to wc_xmss_sign / wc_xmssmt_sign / wc_xmssmt_verify; added the missing msgLen <= 0 check in wc_XmssKey_Verify (Sign already had it). Verified: 13 matrix rows build clean under the conversion flags; testwolfcrypt's "XMSS Vfy" / "XMSS" tests pass for --enable-xmss, --enable-xmss=yes,small, and --enable-xmss=yes,verify-only; bench_xmss_xmssmt parameter sets within noise vs. master. https://claude.ai/code/session_01EJmy1bKDgHseTwZ5Qqpu1g
1 parent 7b53303 commit 3b2d711

4 files changed

Lines changed: 179 additions & 139 deletions

File tree

‎.github/workflows/wolfCrypt-Wconversion.yml‎

Lines changed: 40 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,17 +18,20 @@ jobs:
1818
matrix:
1919
config: [
2020
# Add new configs here
21-
'--disable-asm --enable-cryptonly --enable-all-crypto --disable-examples --disable-benchmark --disable-crypttests --enable-mlkem CPPFLAGS="-Wconversion -Warith-conversion -Wenum-conversion -Wfloat-conversion -Wsign-conversion -Wcast-qual"',
22-
'--enable-intelasm --enable-cryptonly --enable-all-crypto --disable-examples --disable-benchmark --disable-crypttests --enable-mlkem CPPFLAGS="-Wconversion -Warith-conversion -Wenum-conversion -Wfloat-conversion -Wsign-conversion -Wcast-qual"',
23-
'--enable-smallstack --disable-asm --enable-cryptonly --enable-all-crypto --disable-examples --disable-benchmark --disable-crypttests --enable-mlkem CPPFLAGS="-Wconversion -Warith-conversion -Wenum-conversion -Wfloat-conversion -Wsign-conversion -Wcast-qual"',
24-
'--enable-smallstack --enable-intelasm --enable-cryptonly --enable-all-crypto --disable-examples --disable-benchmark --disable-crypttests --enable-mlkem CPPFLAGS="-Wconversion -Warith-conversion -Wenum-conversion -Wfloat-conversion -Wsign-conversion -Wcast-qual"',
25-
'--enable-cryptonly --enable-all-crypto --disable-examples --disable-benchmark --disable-crypttests --enable-mlkem CPPFLAGS="-Wconversion -Warith-conversion -Wenum-conversion -Wfloat-conversion -Wsign-conversion -DNO_INT128 -Wcast-qual"',
26-
'--enable-cryptonly --enable-all-crypto --disable-examples --disable-benchmark --disable-crypttests --enable-mlkem CPPFLAGS="-Wdeclaration-after-statement -Wconversion -Warith-conversion -Wenum-conversion -Wfloat-conversion -Wsign-conversion -Wcast-qual" --enable-32bit CFLAGS=-m32',
27-
'--enable-cryptonly --enable-all-crypto --disable-examples --disable-benchmark --disable-crypttests --enable-mlkem=yes,small CPPFLAGS="-Wconversion -Warith-conversion -Wenum-conversion -Wfloat-conversion -Wsign-conversion -Wcast-qual -DNO_INT128"',
28-
'--enable-cryptonly --enable-all-crypto --disable-examples --disable-benchmark --disable-crypttests --enable-mlkem=yes,no-large-code CPPFLAGS="-Wconversion -Warith-conversion -Wenum-conversion -Wfloat-conversion -Wsign-conversion -Wcast-qual -DNO_INT128"',
29-
'--enable-smallstack --enable-cryptonly --enable-all-crypto --disable-examples --disable-benchmark --disable-crypttests --enable-mlkem CPPFLAGS="-Wconversion -Warith-conversion -Wenum-conversion -Wfloat-conversion -Wsign-conversion -Wcast-qual -DNO_INT128"',
30-
'--disable-intelasm --enable-cryptonly --enable-all-crypto --disable-examples --disable-benchmark --disable-crypttests --enable-mlkem CPPFLAGS="-DWOLFSSL_MLKEM_ENCAPSULATE_SMALL_MEM -DWOLFSSL_MLKEM_MAKEKEY_SMALL_MEM -Wdeclaration-after-statement -Wconversion -Warith-conversion -Wenum-conversion -Wfloat-conversion -Wsign-conversion -Wcast-qual" --enable-32bit CFLAGS=-m32',
31-
'--disable-intelasm --enable-cryptonly --enable-all-crypto --disable-examples --disable-benchmark --disable-crypttests --enable-mlkem=yes,small CPPFLAGS="-DWOLFSSL_MLKEM_ENCAPSULATE_SMALL_MEM -DWOLFSSL_MLKEM_MAKEKEY_SMALL_MEM -Wconversion -Warith-conversion -Wenum-conversion -Wfloat-conversion -Wsign-conversion -Wcast-qual -DNO_INT128"',
21+
'--disable-asm --enable-cryptonly --enable-all-crypto --disable-examples --disable-benchmark --disable-crypttests --enable-mlkem --enable-xmss CPPFLAGS="-Wconversion -Warith-conversion -Wenum-conversion -Wfloat-conversion -Wsign-conversion -Wcast-qual"',
22+
'--enable-intelasm --enable-cryptonly --enable-all-crypto --disable-examples --disable-benchmark --disable-crypttests --enable-mlkem --enable-xmss CPPFLAGS="-Wconversion -Warith-conversion -Wenum-conversion -Wfloat-conversion -Wsign-conversion -Wcast-qual"',
23+
'--enable-smallstack --disable-asm --enable-cryptonly --enable-all-crypto --disable-examples --disable-benchmark --disable-crypttests --enable-mlkem --enable-xmss CPPFLAGS="-Wconversion -Warith-conversion -Wenum-conversion -Wfloat-conversion -Wsign-conversion -Wcast-qual"',
24+
'--enable-smallstack --enable-intelasm --enable-cryptonly --enable-all-crypto --disable-examples --disable-benchmark --disable-crypttests --enable-mlkem --enable-xmss CPPFLAGS="-Wconversion -Warith-conversion -Wenum-conversion -Wfloat-conversion -Wsign-conversion -Wcast-qual"',
25+
'--enable-cryptonly --enable-all-crypto --disable-examples --disable-benchmark --disable-crypttests --enable-mlkem --enable-xmss CPPFLAGS="-Wconversion -Warith-conversion -Wenum-conversion -Wfloat-conversion -Wsign-conversion -DNO_INT128 -Wcast-qual"',
26+
'--enable-cryptonly --enable-all-crypto --disable-examples --disable-benchmark --disable-crypttests --enable-mlkem --enable-xmss CPPFLAGS="-Wdeclaration-after-statement -Wconversion -Warith-conversion -Wenum-conversion -Wfloat-conversion -Wsign-conversion -Wcast-qual" --enable-32bit CFLAGS=-m32',
27+
'--enable-cryptonly --enable-all-crypto --disable-examples --disable-benchmark --disable-crypttests --enable-mlkem=yes,small --enable-xmss CPPFLAGS="-Wconversion -Warith-conversion -Wenum-conversion -Wfloat-conversion -Wsign-conversion -Wcast-qual -DNO_INT128"',
28+
'--enable-cryptonly --enable-all-crypto --disable-examples --disable-benchmark --disable-crypttests --enable-mlkem=yes,no-large-code --enable-xmss CPPFLAGS="-Wconversion -Warith-conversion -Wenum-conversion -Wfloat-conversion -Wsign-conversion -Wcast-qual -DNO_INT128"',
29+
'--enable-smallstack --enable-cryptonly --enable-all-crypto --disable-examples --disable-benchmark --disable-crypttests --enable-mlkem --enable-xmss CPPFLAGS="-Wconversion -Warith-conversion -Wenum-conversion -Wfloat-conversion -Wsign-conversion -Wcast-qual -DNO_INT128"',
30+
'--disable-intelasm --enable-cryptonly --enable-all-crypto --disable-examples --disable-benchmark --disable-crypttests --enable-mlkem --enable-xmss CPPFLAGS="-DWOLFSSL_MLKEM_ENCAPSULATE_SMALL_MEM -DWOLFSSL_MLKEM_MAKEKEY_SMALL_MEM -Wdeclaration-after-statement -Wconversion -Warith-conversion -Wenum-conversion -Wfloat-conversion -Wsign-conversion -Wcast-qual" --enable-32bit CFLAGS=-m32',
31+
'--disable-intelasm --enable-cryptonly --enable-all-crypto --disable-examples --disable-benchmark --disable-crypttests --enable-mlkem=yes,small --enable-xmss CPPFLAGS="-DWOLFSSL_MLKEM_ENCAPSULATE_SMALL_MEM -DWOLFSSL_MLKEM_MAKEKEY_SMALL_MEM -Wconversion -Warith-conversion -Wenum-conversion -Wfloat-conversion -Wsign-conversion -Wcast-qual -DNO_INT128"',
32+
# Dedicated rows exercising XMSS sub-options not covered by --enable-xmss default
33+
'--enable-cryptonly --enable-all-crypto --disable-examples --disable-benchmark --disable-crypttests --enable-mlkem --enable-xmss=yes,small CPPFLAGS="-Wconversion -Warith-conversion -Wenum-conversion -Wfloat-conversion -Wsign-conversion -Wcast-qual"',
34+
'--enable-cryptonly --enable-all-crypto --disable-examples --disable-benchmark --disable-crypttests --enable-mlkem --enable-xmss=yes,verify-only CPPFLAGS="-Wdeclaration-after-statement -Wconversion -Warith-conversion -Wenum-conversion -Wfloat-conversion -Wsign-conversion -Wcast-qual" --enable-32bit CFLAGS=-m32',
3235
]
3336
name: build library
3437
if: github.repository_owner == 'wolfssl'
@@ -50,3 +53,29 @@ jobs:
5053
echo "running ./configure ${{ matrix.config }}"
5154
./configure ${{ matrix.config }} || $(exit 3)
5255
make -j 4 || $(exit 4)
56+
57+
# Compiler-clean (above) doesn't catch a bad cast that silently changes
58+
# the wire output. Run testwolfcrypt's xmss_test under the same conversion
59+
# flags on the default and small XMSS variants.
60+
test_xmss_runtime:
61+
strategy:
62+
matrix:
63+
config: [
64+
'--enable-xmss CPPFLAGS="-Wconversion -Warith-conversion -Wenum-conversion -Wfloat-conversion -Wsign-conversion -Wcast-qual"',
65+
'--enable-xmss=yes,small CPPFLAGS="-Wconversion -Warith-conversion -Wenum-conversion -Wfloat-conversion -Wsign-conversion -Wcast-qual"',
66+
]
67+
name: test XMSS runtime
68+
if: github.repository_owner == 'wolfssl'
69+
runs-on: ubuntu-24.04
70+
timeout-minutes: 6
71+
steps:
72+
- uses: actions/checkout@v4
73+
name: Checkout wolfSSL
74+
75+
- name: Build and run wolfCrypt XMSS tests
76+
run: |
77+
./autogen.sh || $(exit 2)
78+
echo "running ./configure ${{ matrix.config }}"
79+
./configure ${{ matrix.config }} || $(exit 3)
80+
make -j 4 || $(exit 4)
81+
./wolfcrypt/test/testwolfcrypt || $(exit 5)

‎wolfcrypt/src/wc_xmss.c‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -749,13 +749,15 @@ static WC_INLINE int wc_xmsskey_signupdate(XmssKey* key, byte* sig,
749749
*/
750750
#ifndef WOLFSSL_WC_XMSS_SMALL
751751
if (key->is_xmssmt) {
752-
ret = wc_xmssmt_sign(state, msg, msgLen, key->sk, sig);
752+
ret = wc_xmssmt_sign(state, msg, (word32)msgLen, key->sk,
753+
sig);
753754
}
754755
else {
755-
ret = wc_xmss_sign(state, msg, msgLen, key->sk, sig);
756+
ret = wc_xmss_sign(state, msg, (word32)msgLen, key->sk,
757+
sig);
756758
}
757759
#else
758-
ret = wc_xmssmt_sign(state, msg, msgLen, key->sk, sig);
760+
ret = wc_xmssmt_sign(state, msg, (word32)msgLen, key->sk, sig);
759761
#endif
760762
if (ret == WC_NO_ERR_TRACE(KEY_EXHAUSTED_E)) {
761763
/* Signature space exhausted. */
@@ -1083,7 +1085,7 @@ int wc_XmssKey_MakeKey(XmssKey* key, WC_RNG* rng)
10831085
}
10841086
#ifdef WOLFSSL_SMALL_STACK
10851087
if (ret == 0) {
1086-
seed = (unsigned char*)XMALLOC(3 * key->params->n, NULL,
1088+
seed = (unsigned char*)XMALLOC(3U * key->params->n, NULL,
10871089
DYNAMIC_TYPE_TMP_BUFFER);
10881090
if (seed == NULL) {
10891091
ret = MEMORY_E;
@@ -1093,7 +1095,7 @@ int wc_XmssKey_MakeKey(XmssKey* key, WC_RNG* rng)
10931095

10941096
if (ret == 0) {
10951097
/* Generate three random seeds. */
1096-
ret = wc_RNG_GenerateBlock(rng, seed, 3 * key->params->n);
1098+
ret = wc_RNG_GenerateBlock(rng, seed, 3U * key->params->n);
10971099
}
10981100

10991101
if (ret == 0) {
@@ -1473,10 +1475,11 @@ int wc_XmssKey_ExportPubRaw(const XmssKey* key, byte* out, word32* outLen)
14731475
}
14741476

14751477
if (ret == 0) {
1476-
int i = 0;
1478+
word32 i = 0;
14771479
/* First copy the oid into buffer. */
14781480
for (; i < XMSS_OID_LEN; i++) {
1479-
out[XMSS_OID_LEN - i - 1] = (key->oid >> (8 * i)) & 0xFF;
1481+
out[XMSS_OID_LEN - i - 1U] =
1482+
(byte)((key->oid >> (8U * i)) & 0xFFU);
14801483
}
14811484
/* Copy the public key data into buffer after oid. */
14821485
XMEMCPY(out + XMSS_OID_LEN, key->pk, pubLen - XMSS_OID_LEN);
@@ -1603,7 +1606,7 @@ int wc_XmssKey_Verify(XmssKey* key, const byte* sig, word32 sigLen,
16031606
int ret = 0;
16041607

16051608
/* Validate parameters. */
1606-
if ((key == NULL) || (sig == NULL) || (m == NULL)) {
1609+
if ((key == NULL) || (sig == NULL) || (m == NULL) || (mLen <= 0)) {
16071610
ret = BAD_FUNC_ARG;
16081611
}
16091612
/* Validate state. */
@@ -1631,7 +1634,7 @@ int wc_XmssKey_Verify(XmssKey* key, const byte* sig, word32 sigLen,
16311634
ret = wc_xmss_state_init(state, key->params);
16321635
if (ret == 0) {
16331636
/* Verify using either XMSS^MT function as it works for both. */
1634-
ret = wc_xmssmt_verify(state, m, mLen, sig, key->pk);
1637+
ret = wc_xmssmt_verify(state, m, (word32)mLen, sig, key->pk);
16351638
/* Free state after use. */
16361639
wc_xmss_state_free(state);
16371640
}

0 commit comments

Comments
 (0)