Fenrir fixes 2026 08 26 - #25
Merged
Merged
Conversation
wolfSSL-Fenrir-bot
previously requested changes
Aug 27, 2026
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #25
Scan targets checked: wolfpsa-bugs, wolfpsa-src
Findings: 4
4 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Reported findings require changes before merge.
psa_asymmetric_verify_ecc() rejects a raw signature whose length differs from the expected r||s width with PSA_ERROR_INVALID_ARGUMENT. That length mismatch is malformed peer signature data, not API misuse, and the same function already maps the later raw-signature conversion failure to PSA_ERROR_INVALID_SIGNATURE. Return PSA_ERROR_INVALID_SIGNATURE for the length mismatch so both malformed signature paths agree. Verified with test/psa_server/psa_ecc_sig_len_test.c: a signature one byte short and one byte long both returned PSA_ERROR_INVALID_ARGUMENT before this change and PSA_ERROR_INVALID_SIGNATURE after it.
wc_psa_get_ecc_curve_id() returned ECC_SECP256K1 and the Brainpool curve ids unconditionally, while the rest of the key validation gates those families on HAVE_ECC_KOBLITZ and HAVE_ECC_BRAINPOOL. In a build without those flags the mapping let unsupported families through to wolfCrypt, which either failed later with a less specific error or, with point validation off, interpreted the key on the default curve of the same size. Return ECC_CURVE_INVALID for the SECP_K1 256-bit case without HAVE_ECC_KOBLITZ and for the whole BRAINPOOL_P_R1 family without HAVE_ECC_BRAINPOOL (wolfCrypt's feature macro; HAVE_BRAINPOOL is defined nowhere in the tree), matching the guards in psa_asymmetric_check_key_type_supported(). Verified with test/psa_server/psa_ecc_curve_caps_test.c: in the default build (no HAVE_ECC_KOBLITZ, no HAVE_ECC_BRAINPOOL) a verify on an imported secp256k1 and a Brainpool public key returned PSA_ERROR_INVALID_SIGNATURE before this change (misinterpreted on the default curve) and PSA_ERROR_NOT_SUPPORTED after it; in a Koblitz+Brainpool build the brainpool-256 id resolves to ECC_BRAINPOOLP256R1 and the test skips the supported families.
psa_asymmetric_verify_ecc imported the X9.63 point with wc_ecc_import_x963, which infers the curve from the encoded length. For any same-size non-default family (e.g. secp256k1) the point was checked on the default curve and verification of a valid signature failed. Pin the curve with wc_ecc_import_x963_ex using the id resolved from the key attributes. Add a regression test generating a secp256k1 pair, signing a digest, and verifying with the standalone public key.
wolfpsa_key_agreement_secret imported the peer X9.63 point with wc_ecc_import_x963, which infers the curve from the coordinate size and defaults to the NIST P-256 family. A secp256k1 (or Brainpool) peer point was therefore checked on the wrong curve and raw key agreement failed. Import with wc_ecc_import_x963_ex pinned to the local key's curve: a peer point that is not on this curve now fails the import instead of being reinterpreted. The ECDH scalar multiplication also needs an RNG attached to the imported private key (wc_ecc_set_rng): under ECC_TIMING_RESISTANT, which the default user_settings.h defines, wolfCrypt's blind-k ECDH returns MISSING_RNG_E without one. Add a two-party regression test covering secp256k1 (no-op without HAVE_ECC_KOBLITZ).
In the pre-hashed dispatch, PSA_ALG_IS_HASH_ML_DSA is true for both the hedged and the deterministic family (its ~0x1ff mask covers the 0x100 family selector bit), so the first branch captured deterministic requests and signed them with a live RNG via wc_MlDsaKey_SignCtxHash, leaving the wc_MlDsaKey_SignCtxHashWithSeed branch unreachable. Signing the same digest twice produced different signatures, breaking the determinism contract. Test the hedged predicate (PSA_ALG_IS_HEDGED_HASH_ML_DSA) first so the deterministic branch is reachable. Add a regression test that signs the same digest twice with PSA_ALG_DETERMINISTIC_HASH_ML_DSA(PSA_ALG_SHA_256) and asserts byte-identical signatures.
The first wildcard block of wolfpsa_sign_alg_permitted matched with PSA_ALG_IS_HASH_ML_DSA, which is true for both the hedged and the deterministic family (its ~0x1ff mask covers the 0x100 family selector bit), and compared with the same wide mask. A key policy of PSA_ALG_HASH_ML_DSA(ANY_HASH) therefore accepted PSA_ALG_DETERMINISTIC_HASH_ML_DSA(hash) requests and vice versa, and the deterministic block below, which masks with ~0xff, could never run. Gate the first block on PSA_ALG_IS_HEDGED_HASH_ML_DSA for both the request and the policy and compare with the hash-only mask, so each family's wildcard only matches its own family. Add a regression test covering both wildcard policies against both request families.
The comment on wolfpsa_mldsa_verify said the hedged and deterministic variants 'produce identical signatures'. The signing dispatch contradicts this: hedged signing passes a live RNG while the deterministic variant passes the FIPS 204 all-zero seed, so hedged signatures differ across calls. They share the signature format and the verify helper, which is what the comment should say. Comment-only change.
The public XOF definitions were wrapped in the SHAKE128/SHAKE256 feature guard while crypto.h declares them unconditionally and wolfpsa.map exports them, so a build without both SHAKE implementations lacked the symbols and any consumer calling an XOF API failed at link time. Keep the backend implementation under the guard and add stub definitions for the five public functions in the no-backend configuration: setup reports PSA_ERROR_NOT_SUPPORTED, the operation-state calls report PSA_ERROR_BAD_STATE (no operation can ever be active), abort reports PSA_SUCCESS. Verified in a no-SHAKE build (ED448/ML-DSA/ML-KEM also disabled, which depend on SHAKE256): the test links and passes where it previously failed to link; the with-SHAKE build passes unchanged.
psa_xof_update() sized the accumulation buffer as ibuf_len + (word32)input_length, which wraps modulo 2^32: after a 2 GiB update, a second update of 2 GiB + 256 bytes computed a required capacity of 256, skipped the growth, and the copy ran past the allocation (observed hang/corruption in a regression run). The doubling loop in psa_xof_ibuf_grow() could also multiply past 0x80000000 to zero and spin forever. Size the buffer in size_t, reject a combined input above UINT32_MAX (the backend Absorb() takes word32 lengths, so more is unrepresentable anyway), and stop the doubling loop at half of UINT32_MAX so the multiply cannot wrap. Add a regression test with two updates whose combined length crosses 2^32.
psa_xof_output() computed the squeezed byte count as n_blocks * block_size in word32. For a single request of 4294967376 bytes (25565282 full 168-byte SHAKE128 blocks) the product wrapped to 80: the backend wrote the full 4 GiB, but the accounting advanced 80 bytes, so the next squeeze started 4 GiB short of where it should have and the output came out with stream segments in the wrong places. Do the accounting in size_t and squeeze in chunks: cap each backend call at UINT32_MAX / block_size blocks (the backend takes a word32 count and the byte product must stay in word32), and loop until the request is satisfied. Regression test: one 4294967376-byte output request must equal the same total served in two sub-2^32 requests; the pre-fix build fails with corrupted output, the fixed build passes.
psa_xof_set_context() returned PSA_ERROR_INVALID_ARGUMENT for SHAKE (no context field) without putting the operation in the inactive state, so psa_xof_update() and psa_xof_output() still succeeded on the same operation after the failed set_context. Route every error that occurs once the operation context has been obtained through wolfpsa_xof_fail so a rejected context aborts the operation, matching the multipart error contract used by the rest of the XOF API. Regression test: setup, rejected set_context, then update/output must report PSA_ERROR_BAD_STATE. The test branches on the setup status at runtime so it also covers no-SHAKE builds, where setup reports PSA_ERROR_NOT_SUPPORTED and the stubbed no-backend error paths are exercised instead.
RFC 4615 section 3 step 1 requires that a variable-length key of exactly 128 bits be used directly as the AES-CMAC key, while any other length is normalized with CMAC(0^128, key). The implementation always took the normalization branch, so a 16-byte password produced a PRF key (and derived keys) that disagree with every RFC 4615-conformant implementation. Take the password directly as the CMAC key when its length is WC_AES_BLOCK_SIZE and keep the zero-key CMAC normalization for all other lengths. Add regression vectors from an independent RFC 4615 reference: a 16-byte password (the flagged branch) plus 6-byte and 21-byte passwords to pin the normalization branch.
psa_key_derivation_input_key() required PSA_KEY_TYPE_DERIVE for every non-PBKDF2 secret, rejecting the algorithm-compatible key types the PSA API allows: an HMAC key for PSA_ALG_SP800_108_COUNTER_HMAC and an AES key for PSA_ALG_SP800_108_COUNTER_CMAC, whose material is the raw MAC key. Whitelist the underlying MAC key type per SP800-108 variant in addition to the generic DERIVE type; the existing checks stay in force (DERIVE/VERIFY_DERIVATION usage, key algorithm equal to the KDF algorithm, AES key sizes enforced at import). Regression test derives via input_key and compares against the same derivation with input_bytes, and pins the cross-variant rejections.
Both SP800-108 backends passed ctx->label_length and ctx->context_length to wc_HmacUpdate/wc_CmacUpdate with a direct word32 cast. A 64-bit length of UINT32_MAX+1 bytes truncated to zero and the derivation silently continued with an empty label or context; other oversized lengths used only the low 32 bits, changing the derived output without an error. Reject label or context lengths above UINT32_MAX with PSA_ERROR_INVALID_ARGUMENT at the start of each backend, using the existing wolfpsa_check_word32_length helper like the secret-length checks. Regression test feeds UINT32_MAX+1-byte label/context to both backends (previously silently accepted) and pins the small happy path.
…ide HAVE_ECC psa_generate_key guarded the entire ECC key-pair branch on HAVE_ECC, so Ed25519/Ed448/X25519/X448 generation returned PSA_ERROR_NOT_SUPPORTED in builds that compile those standalone backends without generic Weierstrass ECC. The branch scaffolding is now compiled whenever any one of the five backends is present, and only the default Weierstrass dispatch remains gated on HAVE_ECC (it falls back to NOT_SUPPORTED otherwise). The family arms keep their own backend macros. Verified with a HAVE_ECC-off build variant (shim user_settings with #undef HAVE_ECC, /tmp throwaway): before the fix all four standalone families generated -134; after, all generate SUCCESS while P-256 still returns NOT_SUPPORTED. The regression test passes in the default build and in the no-ECC variant. Note: the pre-fix no-ECC library additionally fails to link because psa_asymmetric_api.c references psa_asymmetric_sign_ecc/verify_ecc without a HAVE_ECC guard; that is a separate pre-existing gap left for the coordinator. Verification: variant build (no ECC) + psa_eddsa_mont_gen_test fail before / pass after; default build green, test passes.
…utside HAVE_ECC psa_export_public_key guarded the entire ECC branch on HAVE_ECC + HAVE_ECC_KEY_EXPORT + HAVE_ECC_KEY_IMPORT, so standalone Ed25519/Ed448/X25519/X448 key-pair export returned PSA_ERROR_NOT_SUPPORTED in builds that compile those backends without generic Weierstrass ECC, and the raw byte copy of a stored ECC public key - which needs no backend at all - was blocked as well. The public key copy is now unconditional, the key-pair arms keep their own backend macros, and only the Weierstrass arm retains the compound guard (falling back to NOT_SUPPORTED). Verified with the same HAVE_ECC-off build variant as F-11567: before the fix all five cases (four families + stored public key copy) exported -134; after, all succeed. Default build green, both F-11567 and F-11568 regression tests pass. Verification: variant build (no ECC) + psa_eddsa_mont_export_test fail before / pass after; default build green, test passes.
wolfpsa_infer_key_bits set attr->bits = (psa_key_bits_t)(data_length * 8U) for unstructured key types (HMAC, RAW_DATA, DERIVE, PASSWORD, PASSWORD_HASH, PEPPER, AES, DES, ChaCha20/XChaCha20) without checking the result fits the 16-bit psa_key_bits_t. An 8192-byte import (65536 bits) truncated to 0 bits and 8193 bytes to 8, and the import succeeded with corrupted size metadata. Reject data lengths whose bit count exceeds PSA_MAX_KEY_BITS with PSA_ERROR_INVALID_ARGUMENT before the narrowing cast. Note: the DH inference branch a few lines below has the same unchecked narrowing; left untouched here as it is outside this finding's scope (surfaced to the coordinator). Verification: psa_key_infer_bits_test fails pre-fix (8192/8193-byte imports accepted) and passes post-fix (INVALID_ARGUMENT); 8191-byte boundary still imports with bits 65528; F-11567/F-11568 tests still pass.
psa_import_key checked declared bits against data length for ChaCha20/XChaCha20, AES and DES, but not for HMAC, RAW_DATA, DERIVE, PASSWORD, PASSWORD_HASH or PEPPER: one byte imported as a 256-bit HMAC key succeeded with corrupted size metadata, and zero-length imports with a persistent lifetime were stored as zero-size keys. Two changes: the six byte-string types now require the declared size to equal the data length in bits (and to fit the 16-bit size type), and wolfpsa_infer_key_bits rejects zero-length data, since a byte-string key's size is its data length and zero is not a valid size. The PSA spec ties the size of raw keys to the data, so the declared size must match; this is a compliance hardening, judged conservatively from the in-tree headers. Verification: psa_key_declared_bits_test fails pre-fix (all six mismatch imports accepted, 0-byte persistent imports stored) and passes post-fix (INVALID_ARGUMENT); consistent imports and inference still succeed; F-8727 boundary test and both F-1156x tests still pass.
psa_export_public_key forwarded the stored key data of an ML-DSA/ML-KEM key pair to wolfpsa_mldsa_export_public()/wolfpsa_mlkem_export_public() without checking its length. The helpers expand a fixed 32-byte / 64-byte seed (wc_MlDsaKey_MakeKeyFromSeed / wc_MlKemKey_MakeKeyWithRandom), so a corrupted persistent record with a shorter seed read out of bounds. All import paths enforce the seed length, so this is defense in depth against a corrupted store record. Both key-pair arms now require the stored length to equal the expected seed size and return PSA_ERROR_DATA_INVALID otherwise. Verification: psa_pqc_export_seed_test imports a valid key, truncates the on-disk record's data length field, and re-exports: pre-fix the corrupted record expanded with SUCCESS (out-of-bounds read, no crash observed in a non-ASan build); post-fix it returns PSA_ERROR_DATA_INVALID. Well-formed records still export.
psa_cipher_encrypt wrote the generated IV into the output buffer before reading the input, so with overlapping input and output ranges (e.g. the same buffer for both) the IV clobbered unread plaintext and the operation silently encrypted the wrong data; the multipart CBC paths have the same read-after-write hazard. The in-tree PSA header makes no buffer-overlap guarantee for the cipher API, so the minimal defensible fix is to detect overlap of the input and output ranges and return PSA_ERROR_NOT_SUPPORTED rather than stage the data. Verification: psa_cipher_overlap_test fails pre-fix (in-place and partial-overlap calls returned SUCCESS) and passes post-fix (NOT_SUPPORTED); disjoint and exactly-adjacent buffers still encrypt successfully.
The buffered block handling in psa_cipher_update assembles one completed block from the partial buffer plus the first bytes of the input, writes the ciphertext to the start of the output, and only then reads the rest of the input for the full-block pass. With overlapping input and output ranges (the exact in-place pattern) that first write clobbered not-yet-read input bytes and the operation silently produced wrong ciphertext. The in-tree PSA header makes no buffer-overlap guarantee, so the minimal defensible fix is to detect overlap of the input and output ranges at the top of psa_cipher_update and return PSA_ERROR_NOT_SUPPORTED; this covers the CBC and ECB partial-block paths and every other algorithm arm with one check. Verification: psa_cipher_inplace_test fails pre-fix (all three in-place update calls returned SUCCESS) and passes post-fix (NOT_SUPPORTED); a non-overlapping 5+11-byte multipart update with a fixed IV still produces the same ciphertext as a single 16-byte update.
The partial-block local in psa_cipher_update (CBC_NO_PADDING, CBC_PKCS7 encrypt, ECB_NO_PADDING) is assembled from the buffered partial block plus fresh caller input, so it holds plaintext on the encrypt paths. The existing wc_ForceZero sat after the ret != 0 check, so a backend failure (or the build-conditional NOT_SUPPORTED returns inside the ECB scope) returned with the block still populated in the abandoned stack frame. Each scope now routes every exit - backend error, unsupported-backend returns included - through a single label that scrubs the buffer before reporting the failure, matching the cleanup pattern already used by the CBC-PKCS7 finish path. No test: stack zeroization on backend-error paths is not observable through the PSA API; the full regression test set (7 tests) still passes and the build is green under -Wall -Wextra -Werror.
psa_cipher_encrypt() and psa_cipher_decrypt() completed the whole cryptographic operation and then wrote *output_length unconditionally, so a call with output_length == NULL crashed on the final write. The multipart psa_cipher_update/psa_cipher_finish already reject a NULL output-length pointer; add the same entry validation to both one-shot wrappers (PSA_ERROR_INVALID_ARGUMENT, before setup or any processing). Verification: psa_cipher_oneshot_len_test segfaults pre-fix (exit 139) and passes post-fix (INVALID_ARGUMENT for both one-shot functions); a normal one-shot encrypt/decrypt roundtrip still succeeds.
wolfpsa_cipher_check_key restricted PSA_KEY_TYPE_DES keys to CBC_NO_PADDING and ECB_NO_PADDING, so the complete ctx->is_des3 sub-branches inside the CBC_PKCS7 arms of psa_cipher_update() and psa_cipher_finish() were unreachable dead code even though the block/padding logic is generic over block_size (8-byte blocks, 24-byte 3DES keys) and wolfCrypt provides wc_Des3_CbcEncrypt/ Decrypt. Admitting PSA_ALG_CBC_PKCS7 for DES keys activates that existing generic path instead of deleting the branches: the PSA cipher API lists CBC_PKCS7 among the algorithms a DES key may use, and the padding math is block-size independent. Verification: psa_des3_pkcs7_test fails pre-fix (every 3DES CBC_PKCS7 setup/encrypt returned NOT_SUPPORTED) and passes post- fix (oneshot 10-byte and 16-byte roundtrips plus a 3+8-byte multipart roundtrip all decrypt correctly). The test payloads are explicit byte lists sized to the exact length: string initializers in fixed-size byte arrays warn -Wunterminated-string-initialization under the test Makefile's -Werror.
…back In the CBC_PKCS7 encrypt branch of psa_cipher_finish, pad_len was computed as block_size - ctx->partial_len and then corrected with if (pad_len == 0) pad_len = block_size. psa_cipher_update keeps the encrypt-path residue strictly below one full block, so pad_len always lands in [1, block_size] and the fallback was unreachable dead code. Replace it with an explicit invariant check that fails loudly (PSA_ERROR_BAD_STATE) if a full or oversized buffered block ever reaches finish, so a future change breaking the invariant cannot silently rely on untested padding code. No dedicated test: the removed branch is unreachable by construction, so there is no observable behavior change; the 3DES PKCS7 roundtrip test (which exercises this finish path) and the full regression test set still pass.
wolfSSL-Fenrir-bot
dismissed stale reviews from themself
August 27, 2026 11:43
Fenrir's latest completed scan found no issues; clearing the prior automated change request.
Frauschi
reviewed
Aug 28, 2026
Frauschi
left a comment
Frauschi
left a comment
Contributor
There was a problem hiding this comment.
The fixes themselves look right; the comments below are mostly follow-ups in the same areas rather than objections to the changes.
Two I would like resolved before merge: the overlap guard in psa_cipher_encrypt() is stricter than the one you scoped for psa_cipher_update() and rejects in-place calls that this PR's own stream_inplace_matches_reference() asserts must work, and psa_cipher_decrypt() has no overlap check at all. The rest are small - one missed zeroize site, one missed wc_ecc_import_x963_ex, two guards that fold to constant false on 32-bit, the two new ECC curve tests compiling to no-ops in the CI configuration, and a missing <unistd.h> that breaks the test build on macOS.
wc_ForceZero lengths in psa_xof.c narrow size_t to word32, and one of the three sites was missed (src/psa_xof.c:130, src/psa_xof.c:167, src/psa_xof.c:382)
wc_ForceZero() takes a size_t (wolfssl/wolfcrypt/memory.h), so the (word32) casts added at lines 130 and 167 narrow the length only for the compiler to widen it straight back. They buy nothing, and they would truncate the wipe if the UINT32_MAX cap on ibuf_cap ever moved - which is the class of bug the size_t widening was meant to remove.
The third site at line 382 zeroes the same buffer and was left uncast, so the file is inconsistent either way. Simplest fix is to drop both casts and pass ctx->ibuf_cap directly.
Weierstrass ECDH stays silently broken when ECC_TIMING_RESISTANT is combined with WC_NO_RNG (src/psa_asymmetric_api.c:1280-1285, src/psa_asymmetric_api.c:1386-1408)
With ECC_TIMING_RESISTANT and WC_NO_RNG both set, this guard compiles the RNG out, but wolfCrypt still demands one: wc_ecc_shared_secret_gen_sync returns MISSING_RNG_E whenever ECC_TIMING_RESISTANT is defined and private_key->rng is NULL. Every generic ECDH call in that configuration then fails at the very end of the operation with whatever wc_error_to_psa_status() maps that to, instead of being reported as unsupported.
Not a regression - the pre-PR code failed the same way - but this new conditional is the natural place to make the remaining unsupported combination explicit, either a build-time #error or an early PSA_ERROR_NOT_SUPPORTED on the generic-ECC path.
psa_cipher_decrypt() had no overlap check at all: it relied on the guard inside psa_cipher_update(), which is scoped to the block modes, so CTR/CFB/OFB/CCM*-no-tag/stream decrypt with the output starting inside the ciphertext range wrote plaintext over unread ciphertext and returned SUCCESS with wrong bytes. Mirror the one-shot encrypt contract on decrypt: any overlap between the declared input and output ranges is rejected with PSA_ERROR_NOT_SUPPORTED, in every mode. The one-shot entry points do not make the stream-vs-block distinction the multipart update() makes; psa_cipher_overlap_test.c now pins that contract, including a byte-wise safe in-place CTR call that the one-shot API still refuses. Also reject a NULL output with a non-zero output_size in psa_cipher_encrypt() (pre-fix the IV copy crashed), run the overlap range test on uintptr_t in all three guards (comparing pointers into different objects with < is undefined, C99 6.5.8p5), and add decrypt-side cases to the overlap test: in-place AES CBC no-padding, AES CBC PKCS7, DES3 + PKCS7, and the CTR forward-overlap regression. psa_cipher_oneshot_len_test.c takes the NULL-output case for both one-shot entry points.
The CBC-PKCS7 decrypt path assembles the first partial block in a stack buffer and returned without scrubbing it on both error exits, and left it on the success path. The sibling CBC no-padding and PKCS7 encrypt sites already scrub through the status + goto label pattern; close the fourth site the same way. The buffer holds ciphertext rather than plaintext, so the exposure is limited, but the stated goal was zeroization on every exit.
need = ibuf_len + input_length wraps where size_t is 32 bits, so 'need > (size_t)UINT32_MAX' folds to constant false and the addition it guards still wraps there. The subtraction form holds on both word sizes; ibuf_len stays <= UINT32_MAX, so the RHS cannot underflow. Drop the (word32) casts on the two wc_ForceZero() calls at the same time: wc_ForceZero takes a size_t, so the casts narrow the length only for the compiler to widen it straight back, and they would truncate the wipe if the UINT32_MAX cap on ibuf_cap ever moved. The third ForceZero site already passed ibuf_cap uncast; all three are now consistent.
psa_asymmetric_export_public_key_ecc() was the third site that infers a curve from the encoded point length and still called the unpinned wc_ecc_import_x963. curve_id is already computed and checked against ECC_CURVE_INVALID above the call, so pass it to wc_ecc_import_x963_ex: a stored secp256k1 or Brainpool-P256 key must not be re-validated against the default curve for its coordinate size. Only reachable in a Koblitz/Brainpool-enabled build, where this is the same defect F-8722 fixed for the verify path.
psa_asymmetric.c gated the Brainpool P family on defined(HAVE_ECC) && defined(HAVE_BRAINPOOL) in two places, and HAVE_BRAINPOOL appears nowhere in the wolfSSL tree, so both conditions are permanently false and the family reports as NOT_SUPPORTED even in a build that has the curves. The new wolfpsa_get_ecc_curve_id() in the same file already uses HAVE_ECC_BRAINPOOL; correct both gates to the same macro so the file no longer gates one curve family on two differently spelled macros pointing opposite ways. No callers today, so no behaviour change.
The baseline user_settings.h enables neither HAVE_ECC_KOBLITZ nor HAVE_ECC_BRAINPOOL, so psa_ecc_verify_curve_test and psa_ecc_ecdh_curve_test compiled to a main() that printed 'skipped' and the curve-pinning fixes they guard ran with no coverage in CI. The SECP_R1 round trip now runs unconditionally in both tests (it also covers the blinding RNG plumbing under ECC_TIMING_RESISTANT in the default build); the secp256k1 and Brainpool-P256 cases run when the matching family is enabled, instead of the whole body hiding behind HAVE_ECC_KOBLITZ. A new CI lane rebuilds the library and the two tests with WOLFSSL_CUSTOM_CURVES + HAVE_ECC_KOBLITZ + HAVE_ECC_BRAINPOOL and runs them, so the non-default curve paths get exercised. The variant user_settings is created in the workspace; USER_SETTINGS_PATH must be absolute because make -C test runs with CWD=test/, where a relative path would silently resolve to the repo-root baseline. libwolfpsa.so keeps its wolfCrypt objects local (the export map ships only the psa_* API), so the curve operations run on the variant wolfCrypt compiled into the library itself.
The F-10428 narrowing of the HashML-DSA ANY_HASH wildcard to one family is right for SIGN, where hedged and deterministic signing are different operations, but it applied to VERIFY too. ML-DSA verification is family-independent: psa_mldsa.c dispatches both HashML-DSA and DeterministicHashML-DSA to the same wc_MlDsaKey_VerifyCtxHash(), so a policy of PSA_ALG_DETERMINISTIC_HASH_ML_DSA(PSA_ALG_ANY_HASH) that used to admit a HashML-DSA verify request now returns NOT_PERMITTED. Extend the verify-usage branch of wolfpsa_sign_alg_permitted() to accept either family when the hash matches (a wildcard policy of either family, or a concrete policy with the same hash), mirroring the ECDSA determinism equivalence just above it. Signing stays strict. Update the stale function-header dispatch line in psa_mldsa.c that still named PSA_ALG_IS_HASH_ML_DSA - true for both families - as the hedged branch, and pin the verify equivalence (cross-family wildcard in both directions, plus a concrete-policy negative) in psa_mldsa_any_hash_test.c.
With ECC_TIMING_RESISTANT and WC_NO_RNG both set, the new conditional compiles the RNG out of wolfpsa_key_agreement_secret(), but wolfCrypt still demands one: wc_ecc_shared_secret_gen_sync() returns MISSING_RNG_E whenever ECC_TIMING_RESISTANT is defined and the private key has no RNG attached. Every generic ECDH call in that configuration then failed at the very end of the operation with whatever wc_error_to_psa_status() maps that to. Not a regression - the pre-PR code failed the same way - but report the unsupported combination up front as PSA_ERROR_NOT_SUPPORTED, before any wolfCrypt call. Montgomery X25519/X448 is unaffected and keeps working in that configuration.
glibc declares mkdtemp in <stdlib.h> under _DEFAULT_SOURCE, but on macOS/BSD it lives only in <unistd.h>, where the file failed the -Wall -Wextra -Werror build. psa_pqc_export_seed_test.c, added in this same branch, already includes it.
Frauschi
reviewed
Aug 31, 2026
Frauschi
left a comment
Frauschi
left a comment
Contributor
There was a problem hiding this comment.
Re-reviewed after the fixes. All 14 points from the last round are addressed - I rebuilt the library and ran the six touched regression tests to confirm, and they pass.
The one I would like resolved before merge is the WC_NO_RNG guard in wolfpsa_key_agreement_secret(): it is a runtime return rather than a preprocessor exclusion, so the file still does not compile in that configuration, and the condition is narrower than the macro that actually decides it. My own earlier comment on that was mis-scoped, which is noted inline.
The rest are follow-ups in the areas the fixes touched: a -DWOLFSSL_USER_SETTINGS that leaks into every test TU rather than the two that need it, a missing wildcard guard in the new HashML-DSA verify block, psa_xof_set_context() now destroying the operation it is asked about, and two tests that reuse an output-length variable as a running offset.
wolfpsa_sign_alg_permitted() admitted a verify request whose hash is itself PSA_ALG_ANY_HASH: with a DETERMINISTIC_HASH_ML_DSA(ANY_HASH) policy, a HASH_ML_DSA(ANY_HASH) request flipped 0 -> 1 in the cross-family verify-equivalence block, where every sibling wildcard block requires the request to name a concrete hash. The dispatch rejected the request anyway (mldsa_psa_hash_to_wc() has no ANY_HASH entry, so the call failed with NOT_SUPPORTED), but the policy check is the layer that must carry the invariant. The block now requires PSA_ALG_GET_HASH(alg) != PSA_ALG_ANY_HASH, and psa_mldsa_any_hash_test.c pins both directions: a wildcard verify request is NOT_PERMITTED under a wildcard policy of either family.
wolfpsa_key_agreement_secret() compiled wc_ecc_shared_secret() under a plain HAVE_ECC guard, but wolfCrypt only declares that function when HAVE_ECC_DHE is set - which settings.h turns off in WC_NO_RNG builds (blinding needs an RNG) and in NO_ECC_DHE builds - so those builds failed with an implicit declaration instead of reporting the combination as unsupported. The old runtime guard (ECC_TIMING_RESISTANT && WC_NO_RNG) caught only the blinding case. The body is now excluded at compile time when HAVE_ECC_DHE is absent and the body-only declarations moved under the same guard. A WC_NO_RNG lane was added to build-config-matrix: it builds the psa-objects target (wolfPSA's own sources only) because the bundled wolfCrypt sources do not build under WC_NO_RNG (asn.c trips -Wnonnull), which is a wolfSSL issue outside the lane's scope. Verification: WC_NO_RNG + WC_BLINDING_NO_RNG_ACKNOWLEDGE_WEAKNESS now compiles all 28 wolfPSA sources clean (-Wall -Wextra -Werror); baseline and Koblitz+Brainpool shapes build and pass all 37 API + regression tests.
test/Makefile added -DWOLFSSL_USER_SETTINGS and -I$(USER_SETTINGS_PATH) to the shared CPPFLAGS so the curve tests could read the library's feature macros, but that layered wolfPSA's user_settings.h (WOLFCRYPT_ONLY, SINGLE_THREADED, WOLFSSL_SP_MATH_ALL) on top of the autotools configuration for every test TU. The four files that include <wolfssl/options.h> then declared wolfCrypt structs (Hmac, wc_Sha256) with a layout the linked libwolfssl may not use - latent, but a real ABI hazard. Only the three curve tests include <user_settings.h> (they branch on HAVE_ECC_KOBLITZ / HAVE_ECC_BRAINPOOL), so the flag and the include path are now target-specific to them; every other TU sees the library's own configuration via config.h, as before that change. Verification: CI shape (autotools wolfSSL, repo user_settings) - all 37 API + regression tests build and pass; Koblitz+Brainpool lane - both curve tests run the real curve paths and pass.
psa_cipher_update() reports the bytes written by that call, but test_multipart_roundtrip() in psa_des3_pkcs7_test.c reused ct_len as both the out-param and the running offset: the second update overwrote it, and the test only passed because the first update (3 bytes < 8-byte block) emitted nothing. The same pattern sat in psa_cipher_inplace_test.c (out_len2). Both tests now accumulate through a separate part_len. The des3 test splits 11 bytes as 8 + 3 instead of 3 + 8 so the first update emits the first block and the accumulation is actually exercised - with the old code that split writes the finish block over the first ciphertext block and the roundtrip fails.
psa_cipher_encrypt() rejects output == NULL with a non-zero output_size up front, but psa_cipher_decrypt() relied on the same check inside psa_cipher_update() - reached only after the IV copy and set_iv, and only because a zero-length update still hits it. The regression test exercised the weakest trigger (input exactly IV size, so the update saw zero bytes). Decrypt now carries the explicit guard, and the NULL-output test case uses a full IV plus block (32 bytes) so the call exercises the real update path.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
494a71f F-8738: check the PKCS7 partial-length invariant instead of dead fallback
2653c45 F-10433: admit CBC_PKCS7 for DES keys in cipher setup
ecde5c1 F-8736: validate output_length in one-shot cipher functions
464226c F-8741: zero the cipher partial-block buffer on every exit
3f041eb F-10430: reject overlapping input/output in multipart cipher update
2418f4e F-10429: reject overlapping input/output in one-shot cipher encrypt
ad620bb F-11590: validate stored seed length before ML-DSA/ML-KEM public export
e56db16 F-8717: validate declared bits against data length for byte-string keys
5cfb31c F-8727: reject unstructured key imports whose inferred bits overflow
4d927c7 F-11568: dispatch standalone EdDSA and Montgomery public-key export outside HAVE_ECC
0c5853f F-11567: dispatch standalone EdDSA and Montgomery key generation outside HAVE_ECC
dfa3c28 F-8729: reject SP800-108 label/context lengths above UINT32_MAX
b70b782 F-8721: accept compatible MAC keys as SP800-108 input secrets
021dbe6 F-8719: use 128-bit passwords directly in PBKDF2-AES-CMAC-PRF-128
1652a49 F-11578: abort the operation when set_context rejects a context
107e869 F-8728: keep the XOF output accounting in size_t
0acdc68 F-8713: size the XOF input buffer in size_t
71ab2c8 F-11569: define the XOF API when no SHAKE backend is built
043842a F-9418: correct the hedged/deterministic claim in verify comment
697eb91 F-10428: keep the HashML-DSA ANY_HASH wildcard within one family
21ab680 F-10427: dispatch DeterministicHashML-DSA to the seeded signer
efef222 F-8724: pin peer point to the local key's curve in ECDH
bfeee78 F-8722: pin ECC curve on public-key import in verify
32d36c7 F-10435: gate secp256k1 and Brainpool curve ids on capability flags
4f57782 F-11575: report wrong-length raw ECDSA signatures as INVALID_SIGNATURE