Add dynamic key allocation support for Dilithium

[Frauschi]

This update introduces the WOLFSSL_DILITHIUM_DYNAMIC_KEYS option, allowing for dynamic memory allocation of public and private key buffers in dilithium_key objects. This change reduces memory usage by allocating buffers only when needed.

This greatly reduces dynamic memory usage during peer verification when using ML-DSA certificates, which is especially important for resource-constrained systems.

[Frauschi]

Jenkins retest this please

[dgarske]

🐺 Skoll Code Review

Overall recommendation: REQUEST_CHANGES
Findings: 4 total — 3 posted, 1 skipped

Posted findings

• [High] Heap buffer overflow in wc_dilithium_set_level ForceZero when FIPS204_DRAFT is enabled — wolfcrypt/src/dilithium.c:11017-11021
• [Medium] oqs_dilithium_make_key missing USE_INTEL_SPEEDUP +8 alignment padding — wolfcrypt/src/dilithium.c:10077-10093
• [Low] Return value of wc_dilithium_size/wc_dilithium_pub_size not validated before use as allocation size — wolfcrypt/src/dilithium.c:7673

Skipped findings

• [Medium] Duplicated dynamic key allocation pattern across 5 call sites

Review generated by Skoll via openclaw

[dgarske]

🟠 [High] Heap buffer overflow in wc_dilithium_set_level ForceZero when FIPS204_DRAFT is enabled
🚫 BLOCK bug

When WOLFSSL_DILITHIUM_FIPS204_DRAFT and WOLFSSL_DILITHIUM_DYNAMIC_KEYS are both defined, the ForceZero at line 11020 uses wc_dilithium_size(key) to determine how many bytes to zero. However, key->params has already been updated to the NEW level by dilithium_get_params(level, &key->params) at line 10992. The wc_dilithium_size() function (line 11137-11150) checks key->params->level first when FIPS204_DRAFT is defined, so if the new level is a draft level it returns the NEW level's key size, not the OLD level's size. If the new level has a larger key size than the old level (e.g., changing from ML_DSA_44 with 2560 bytes to ML_DSA_87_DRAFT with 4896 bytes), ForceZero will write 2336 bytes past the end of the heap-allocated key->k buffer, causing a heap buffer overflow. The current CI configs do not test FIPS204_DRAFT + DYNAMIC_KEYS, so this would not be caught by CI.

Suggestion:

Suggested change

	#ifdef WOLFSSL_DILITHIUM_DYNAMIC_KEYS
	Compute the old key size before `dilithium_get_params` changes `key->params`. For example:
	```c
	#if defined(WOLFSSL_DILITHIUM_DYNAMIC_KEYS) && defined(WOLFSSL_DILITHIUM_PRIVATE_KEY)
	word32 oldKeySz = 0;
	if ((key != NULL) && (key->k != NULL)) {
	int sz = wc_dilithium_size(key);
	if (sz > 0)
	oldKeySz = (word32)sz;
	}
	#endif
	...
	ret = dilithium_get_params(level, &key->params);
	...
	#ifdef WOLFSSL_DILITHIUM_DYNAMIC_KEYS
	if (key->k != NULL) {
	#ifdef WOLFSSL_DILITHIUM_PRIVATE_KEY
	ForceZero(key->k, oldKeySz);
	#endif

[dgarske]

🟡 [Medium] oqs_dilithium_make_key missing USE_INTEL_SPEEDUP +8 alignment padding
💡 SUGGEST bug

All other dynamic allocation sites in this PR add 8 extra bytes when USE_INTEL_SPEEDUP is defined (e.g., dilithium_make_key_from_seed at line 7675, wc_dilithium_import_public at line 11685, dilithium_set_priv_key at line 11789). However, oqs_dilithium_make_key allocates without the +8 padding. While USE_INTEL_SPEEDUP is unlikely to be combined with HAVE_LIBOQS in practice (the OQS and native implementations are mutually exclusive compile-time paths), this inconsistency could become a problem if shared code paths (like import/export) allocate with +8 but the OQS make_key path doesn't, and the key is later used by code expecting the extra alignment bytes.

Suggestion:

Suggested change

	#ifdef WOLFSSL_DILITHIUM_DYNAMIC_KEYS
	Add the USE_INTEL_SPEEDUP guard consistently:
	```c
	#ifdef WOLFSSL_DILITHIUM_DYNAMIC_KEYS
	if ((ret == 0) && (key->k == NULL)) {
	int secSz = wc_dilithium_size(key);
	#ifdef USE_INTEL_SPEEDUP
	key->k = (byte*)XMALLOC((word32)secSz + 8, key->heap,
	DYNAMIC_TYPE_DILITHIUM);
	#else
	key->k = (byte*)XMALLOC((word32)secSz, key->heap,
	DYNAMIC_TYPE_DILITHIUM);
	#endif

Apply the same pattern for the key->p allocation in this function.

[dgarske]

🔵 [Low] Return value of wc_dilithium_size/wc_dilithium_pub_size not validated before use as allocation size
🔧 NIT convention

wc_dilithium_size() and wc_dilithium_pub_size() return BAD_FUNC_ARG (negative) when the key level is not set. The return value is stored in an int and then cast to word32 for XMALLOC without checking for errors. A negative value cast to word32 becomes a very large value (~4 billion), causing XMALLOC to fail with NULL (which is then caught as MEMORY_E). While this won't cause incorrect behavior in practice (since the level must be set before these functions are called), the error code is misleading — it would return MEMORY_E instead of BAD_FUNC_ARG. This pattern appears at every dynamic allocation site.

Recommendation: Consider checking secSz > 0 before using it as a malloc size, or rely on the existing preconditions that guarantee the level is always set before reaching these code paths.