Skip to content

Commit e1f5468

Browse files
committed
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
1 parent b9a192b commit e1f5468

2 files changed

Lines changed: 34 additions & 30 deletions

File tree

.github/workflows/wolfCrypt-Wconversion.yml

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,9 @@ jobs:
3838
name: build library
3939
if: github.repository_owner == 'wolfssl'
4040
runs-on: ubuntu-24.04
41-
# This should be a safe limit for the tests to run.
42-
timeout-minutes: 6
41+
# SLH-DSA adds ~9 kLoC to every row; the 32-bit multilib + smallstack
42+
# rows need extra headroom to stay clear of CI flakes.
43+
timeout-minutes: 10
4344
steps:
4445
- uses: actions/checkout@v4
4546
name: Checkout wolfSSL
@@ -73,7 +74,7 @@ jobs:
7374
name: test SLH-DSA runtime
7475
if: github.repository_owner == 'wolfssl'
7576
runs-on: ubuntu-24.04
76-
timeout-minutes: 6
77+
timeout-minutes: 10
7778
steps:
7879
- uses: actions/checkout@v4
7980
name: Checkout wolfSSL

wolfcrypt/src/wc_slhdsa.c

Lines changed: 30 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,9 @@ static cpuid_flags_t cpuid_flags = WC_CPUID_INITIALIZER;
5454

5555

5656
/* Winternitz number. */
57-
#define SLHDSA_W 16U
57+
#define SLHDSA_W 16
5858
/* Number of iterations of hashing itself from Winternitz number. */
59-
#define SLHDSA_WM1 (SLHDSA_W - 1U)
59+
#define SLHDSA_WM1 (SLHDSA_W - 1)
6060

6161

6262
#ifndef WOLFSSL_SLHDSA_PARAM_NO_256
@@ -126,6 +126,11 @@ static cpuid_flags_t cpuid_flags = WC_CPUID_INITIALIZER;
126126
/* Maximum message size in nibbles. */
127127
#define SLHDSA_MAX_MSG_SZ ((2 * SLHDSA_MAX_N) + 3)
128128

129+
/* SLH-DSA WOTS+ length: len = len_1 + len_2 = 2*n + 3 (for w=16). The chain
130+
* helpers below pass loop indices and chain steps through (byte) casts; this
131+
* assertion documents the invariant they rely on. */
132+
wc_static_assert(SLHDSA_MAX_MSG_SZ <= 255);
133+
129134
#ifndef WOLFSSL_SLHDSA_PARAM_NO_256F
130135
/* Maximum number of bytes to produce from digest of message. */
131136
#define SLHDSA_MAX_MD 49
@@ -1666,17 +1671,18 @@ static void slhdsakey_base_2b(const byte* x, byte b, byte outLen, word16* baseb)
16661671
int i = 0;
16671672
int bits = 0;
16681673
int total = 0;
1669-
/* mask is word16: b is small (<= 8 in valid call sites), so the mask
1670-
* fits in 16 bits. Cast at the assignment to keep -Wconversion quiet
1671-
* without changing the algorithm. */
1672-
word16 mask = (word16)((1U << b) - 1U);
1674+
word16 mask = (word16)((1 << b) - 1);
16731675

16741676
for (j = 0; j < outLen; j++) {
16751677
while (bits < b) {
16761678
total = (total << 8) + x[i++];
16771679
bits += 8;
16781680
}
16791681
bits -= b;
1682+
/* baseb is word16*: b can be up to a (the FORS height parameter),
1683+
* which reaches 14 for the 256-bit parameter sets. A (byte) cast
1684+
* here would silently truncate any value > 255 and break FORS
1685+
* verification. Cast to word16 to match the destination width. */
16801686
baseb[j] = (word16)((total >> bits) & mask);
16811687
}
16821688
}
@@ -3800,8 +3806,8 @@ static int slhdsakey_chain_idx_to_max_16(SlhDsaKey* key, const byte* sig,
38003806
if (cnt > 3) {
38013807
XMEMCPY(node + 3 * 16, sig + idx[3] * 16, 16);
38023808
}
3803-
if (j != (int)SLHDSA_WM1) {
3804-
ret = slhdsakey_chain_idx_x4_16(node, (word32)j, (word32)((int)SLHDSA_WM1 - j), pk_seed,
3809+
if (j != SLHDSA_WM1) {
3810+
ret = slhdsakey_chain_idx_x4_16(node, (word32)j, (word32)(SLHDSA_WM1 - j), pk_seed,
38053811
addr, idx, key->heap);
38063812
}
38073813
}
@@ -3874,8 +3880,8 @@ static int slhdsakey_chain_idx_to_max_24(SlhDsaKey* key, const byte* sig,
38743880
if (cnt > 3) {
38753881
XMEMCPY(node + 3 * 24, sig + idx[3] * 24, 24);
38763882
}
3877-
if (j != (int)SLHDSA_WM1) {
3878-
ret = slhdsakey_chain_idx_x4_24(node, (word32)j, (word32)((int)SLHDSA_WM1 - j), pk_seed,
3883+
if (j != SLHDSA_WM1) {
3884+
ret = slhdsakey_chain_idx_x4_24(node, (word32)j, (word32)(SLHDSA_WM1 - j), pk_seed,
38793885
addr, idx, key->heap);
38803886
}
38813887
}
@@ -3948,8 +3954,8 @@ static int slhdsakey_chain_idx_to_max_32(SlhDsaKey* key, const byte* sig,
39483954
if (cnt > 3) {
39493955
XMEMCPY(node + 3 * 32, sig + idx[3] * 32, 32);
39503956
}
3951-
if (j != (int)SLHDSA_WM1) {
3952-
ret = slhdsakey_chain_idx_x4_32(node, (word32)j, (word32)((int)SLHDSA_WM1 - j), pk_seed,
3957+
if (j != SLHDSA_WM1) {
3958+
ret = slhdsakey_chain_idx_x4_32(node, (word32)j, (word32)(SLHDSA_WM1 - j), pk_seed,
39533959
addr, idx, key->heap);
39543960
}
39553961
}
@@ -4311,11 +4317,8 @@ static int slhdsakey_wots_pk_from_sig(SlhDsaKey* key, const byte* sig,
43114317
int i;
43124318
byte msg[SLHDSA_MAX_MSG_SZ];
43134319

4314-
/* Step 1: Start csum at 0
4315-
*
4316-
* csum is word16: per FIPS 205 Algorithm 7 the WOTS+ checksum field
4317-
* is a fixed-width 16-bit integer. Cast at every assignment to
4318-
* silence -Wconversion without changing semantics. */
4320+
/* Step 1: Start csum at 0. See slhdsakey_wots_sign() for the rationale
4321+
* behind the (word16) casts on each csum / msg assignment. */
43194322
csum = 0;
43204323
/* Step 3: For each byte in message. */
43214324
for (i = 0; i < n * 2; i += 2) {
@@ -4746,7 +4749,7 @@ static int slhdsakey_ht_sign(SlhDsaKey* key, const byte* pk_fors,
47464749
byte len = key->params->len;
47474750
byte d = key->params->d;
47484751
int j;
4749-
word32 mask = ((word32)1 << h_m) - 1;
4752+
word32 mask = ((word32)1U << h_m) - 1U;
47504753

47514754
/* Step 1: Set address to all zeros. */
47524755
HA_Init(adrs);
@@ -4846,7 +4849,7 @@ static int slhdsakey_ht_verify(SlhDsaKey* key, const byte* m,
48464849
byte d = key->params->d;
48474850
int j;
48484851
/* For Step 6. */
4849-
word32 mask = ((word32)1 << h_m) - 1;
4852+
word32 mask = ((word32)1U << h_m) - 1U;
48504853

48514854
/* Step 1: Set address to all zeros. */
48524855
HA_Init(adrs);
@@ -6663,7 +6666,7 @@ static void slhdsakey_set_ha_from_md(SlhDsaKey* key, const byte* md,
66636666
/* Step 9/12: Mask off any extra high bits. */
66646667
bits = key->params->h - (key->params->h / key->params->d);
66656668
if (bits < 64) {
6666-
t[1] &= ((word32)1 << (bits - 32)) - 1;
6669+
t[1] &= ((word32)1U << (bits - 32)) - 1U;
66676670
}
66686671

66696672
/* Step 8/11: Get pointer to tree leaf index data. */
@@ -6672,7 +6675,7 @@ static void slhdsakey_set_ha_from_md(SlhDsaKey* key, const byte* md,
66726675
ato32(p, l);
66736676
/* Step 10/13: Mask off any extra high bits. */
66746677
bits = key->params->h / key->params->d;
6675-
*l &= ((word32)1 << bits) - 1;
6678+
*l &= ((word32)1U << bits) - 1U;
66766679

66776680
/* Step 11/14: Set the tree index into address. */
66786681
HA_SetTreeAddress(adrs, t);
@@ -8301,11 +8304,11 @@ int wc_SlhDsaKey_ExportPrivate(SlhDsaKey* key, byte* priv, word32* privLen)
83018304
ret = BAD_LENGTH_E;
83028305
}
83038306
else {
8304-
word32 n = key->params->n;
8307+
byte n = key->params->n;
83058308

83068309
/* Copy data out and return length. */
8307-
XMEMCPY(priv, key->sk, n * 4U);
8308-
*privLen = n * 4U;
8310+
XMEMCPY(priv, key->sk, (word32)n * 4U);
8311+
*privLen = (word32)n * 4U;
83098312
}
83108313

83118314
return ret;
@@ -8336,11 +8339,11 @@ int wc_SlhDsaKey_ExportPublic(SlhDsaKey* key, byte* pub, word32* pubLen)
83368339
ret = BAD_LENGTH_E;
83378340
}
83388341
else {
8339-
word32 n = key->params->n;
8342+
byte n = key->params->n;
83408343

83418344
/* Copy data out and return length. */
8342-
XMEMCPY(pub, key->sk + n * 2U, n * 2U);
8343-
*pubLen = n * 2U;
8345+
XMEMCPY(pub, key->sk + (word32)n * 2U, (word32)n * 2U);
8346+
*pubLen = (word32)n * 2U;
83448347
}
83458348

83468349
return ret;

0 commit comments

Comments
 (0)