Fenrir fixes 2026 06 09#18
Merged
Merged
Conversation
…rithm wolfpsa_asymmetric_check_key only compared PSA_ALG_KEY_AGREEMENT_GET_BASE() for key-agreement policies, ignoring the KDF embedded in the key's permitted-algorithm policy. Because both psa_key_agreement() and psa_key_derivation_key_agreement() delegated to psa_raw_key_agreement() with the base algorithm only, a private key restricted to e.g. ECDH+HKDF could be used for raw key agreement (exposing the bare shared secret) or with a different KDF, bypassing PSA policy domain separation. Check the requested KDF against the policy KDF: the base must match and the KDF must match exactly, except that a raw key-agreement policy remains the most permissive form and still permits any KDF. The actual ECDH computation is factored into wolfpsa_key_agreement_secret(), which every entry point now calls with the full requested algorithm so the policy KDF is always enforced. Adds test_ka_kdf_policy_separation, which fails before this change.
…fer-size overflow psa_import_key() computed buffer_size = data_length + fixed metadata and allocated that buffer before copying data_length bytes into it. For key types without an exact-length check (RAW_DATA, HMAC, DERIVE, PASSWORD, etc.), a data_length near SIZE_MAX wrapped buffer_size to a tiny allocation, and the subsequent XMEMCPY overflowed the heap. The later casts to (int) for the storage API were also unsafe for data_length > INT_MAX. Reject data_length > INT_MAX (the convention already used by wolfpsa_validate_stored_key_data_length) before any allocation. This also guarantees buffer_size cannot overflow size_t on any supported target. Adds a regression test that imports with data_length = SIZE_MAX-4 and expects PSA_ERROR_INVALID_ARGUMENT.
The in-repo hash test only checked PSA_ALG_SHA_256, so a mutated, swapped, or removed dispatch case for any other hash algorithm (SHA-1, SHA-224, SHA-384, SHA-512, SHA-512/224, SHA-512/256, SHA3-224/256/384/512) would pass unnoticed. Add a test_hash_kat() helper that verifies both the one-shot psa_hash_compute path and the multipart setup/update/finish path against a fixed digest of "abc" for every such algorithm. Algorithms the library was not built with report PSA_ERROR_NOT_SUPPORTED and are skipped, so the same test works across the build-config matrix.
psa_hash_compare validated the algorithm and length but never checked that the reference hash pointer was non-NULL before passing it to ConstantCompare, so a caller passing hash == NULL with a matching hash_length would dereference NULL after computing the digest. Add the NULL check alongside the existing PSA_ALG_IS_HASH validation, matching the NULL handling already done in psa_hash_verify, and add a test that segfaults before the fix and returns PSA_ERROR_INVALID_ARGUMENT after.
…atch The CMAC paths in psa_mac.c (wolfpsa_mac_check_key, wolfpsa_mac_setup, psa_mac_update, wolfpsa_mac_final, psa_mac_abort) were compiled but never exercised by any test, so mutations to the block-cipher-MAC dispatch survived. Add test_cmac: a NIST SP 800-38B AES-128 CMAC known-answer test for psa_mac_compute, a good/bad-tag psa_mac_verify check, and a negative test that CMAC setup rejects a non-AES key type.
…-id collision psa_import_key auto-assigned key ids from g_next_key_id starting at 1, inside the PSA user id range [PSA_KEY_ID_USER_MIN, PSA_KEY_ID_USER_MAX] that the spec reserves for caller-specified persistent keys. An auto-assigned volatile key could thus take the same numeric id as an existing persistent record, shadowing it on read (wolfpsa_get_key_data checks volatile first) and surviving psa_destroy_key() on disk (which short-circuits after wolfpsa_volatile_remove). Start and bound the auto-assign counter at the vendor range [PSA_KEY_ID_VENDOR_MIN, PSA_KEY_ID_VENDOR_MAX] so implementation-assigned ids can never collide with user-range persistent ids.
The default user_settings.h defined WC_NO_HARDEN, which disables the constant-time / cache-attack-resistant modular exponentiation path in wolfCrypt's SP math (sp_int.c _sp_exptmod_*) and skips RSA blinding. Every PSA sign/decrypt/agreement call dispatched to wolfCrypt inherited the weakening with no runtime signal. Replace WC_NO_HARDEN with explicit hardening enables (TFM_TIMING_RESISTANT, ECC_TIMING_RESISTANT, WC_RSA_BLINDING). Simply removing WC_NO_HARDEN is insufficient: it re-triggers wolfSSL's harden #warning, which the -Werror build turns into an error, and does not by itself request RSA blinding. Enabling WC_RSA_BLINDING requires an RNG on the key for private-key operations. The RSA sign paths already pass an RNG to wc_RsaSSL_Sign/wc_RsaPSS_Sign, but wc_RsaPrivateDecrypt() takes no RNG argument, so associate one via wc_RsaSetRNG() in psa_asymmetric_decrypt_rsa(); otherwise OAEP/PKCS1v15 decrypt fail. Builds clean under -Werror and the full test suite passes (psa_api_test 92/92, including RSA OAEP/PKCS1v15 decrypt).
psa_cipher_generate_iv had no in-repo test, so several mutations to its body survived. Add test_cipher_generate_iv covering the encrypt-direction success path (generated IV has the algorithm length and decrypts a round trip), fresh randomness across operations (zero-initialized buffers catch a dropped psa_generate_random call), decrypt-direction rejection, re-generation rejection, and the buffer-too-small rejection. Verified the new test fails when the direction check, the expected_len==0 check, or the psa_generate_random call is removed/flipped. The in-function iv_attempted check is a redundant guard (psa_cipher_set_iv enforces the same state), so its removal is an equivalent mutant; the test still pins the observable BAD_STATE contract.
The exposed PSA API psa_aead_generate_nonce had no in-repo test, so its direction, double-generation, CCM-lengths, and buffer-size checks all survived mutation. Add aead_generate_nonce exercising: a successful nonce generation on an encrypt context driving a full GCM round-trip, rejection on a decrypt context, rejection on a second call, rejection for CCM without psa_aead_set_lengths, and PSA_ERROR_BUFFER_TOO_SMALL for an undersized nonce buffer.
The persistent branch of psa_import_key() opened a write handle and let the atomic rename in wolfPSA_Store_Close() silently replace any pre-existing key file at the destination path. A second import to an id already in use (e.g. two callers colliding on psa_set_key_id) destroyed the stored key material with no error returned. The PSA Crypto API requires PSA_ERROR_ALREADY_EXISTS in this case, and the volatile path already enforced it. Probe for an existing record with a read open before creating the write handle and return PSA_ERROR_ALREADY_EXISTS when one is found. Add a test that imports twice to the same persistent id and verifies the original key is preserved, and repurpose the short-write test (its re-import-overwrite path is now unreachable) to cover a failed initial create committing no key.
wolfpsa_asymmetric_check_key() matched the key's permitted-algorithm policy against the requested algorithm with exact equality, so a valid PSA wildcard policy such as PSA_ALG_ECDSA(PSA_ALG_ANY_HASH) or PSA_ALG_RSA_PSS(PSA_ALG_ANY_HASH) could not authorize a concrete hash-and-sign operation like PSA_ALG_ECDSA(PSA_ALG_SHA_256), returning PSA_ERROR_NOT_PERMITTED. Add wolfpsa_sign_alg_permitted(), which keeps exact equality as the common case and additionally accepts any concrete hash-and-sign algorithm of the same base family when the policy's hash component is the PSA_ALG_ANY_HASH wildcard, as required by the PSA Crypto API. Add a property test exercising an ECDSA any-hash policy with a concrete SHA-256 sign/verify roundtrip.
Add test_aead_rejects_corrupted_tag covering GCM, CCM, and ChaCha20-Poly1305: encrypt a known plaintext, flip one tag byte, and assert both the one-shot psa_aead_decrypt and the multipart psa_aead_verify paths return PSA_ERROR_INVALID_SIGNATURE with no accepted plaintext length. This locks down the authentication-failure return mapping in wolfpsa_aead_decrypt_final against silent auth-bypass regressions.
wolfpsa_mac_setup performed mac_length/full_length and truncation-length validation after wc_HmacSetKey/wc_InitCmac had already initialised the underlying wolfCrypt context. On those failure paths (and the post-init ret != 0 path) the code zeroed and freed the wolfpsa ctx without calling wc_HmacFree/wc_CmacFree first, leaking any heap state those structs own under WOLFSSL_ASYNC_CRYPT or hardware-backed builds. Add a wolfpsa_mac_free_underlying() helper mirroring psa_mac_abort and call it before wc_ForceZero on each post-init error path.
psa_export_key read key material into the caller-owned output buffer via wolfPSA_Store_Read but, on a short/failed read (truncated or corrupted store file, I/O error), returned PSA_ERROR_STORAGE_FAILURE without zeroing the buffer. fread leaves the bytes it managed to read in place, so partial private/secret key material could remain in caller-visible memory. The two sibling read paths (wolfpsa_get_key_data and psa_export_public_key) already wc_ForceZero their buffers on this failure; psa_export_key did not. Force-zero the buffer and clear *data_length on the short-read path, and add a regression test that truncates a persistent key store file by one byte and verifies the output buffer is zeroed and the length cleared on failure.
HashEdDSA (Ed25519ph / Ed448ph) operates on a fixed-size prehash: the 64-byte SHA-512 digest for Ed25519ph and the 64-byte SHAKE256 digest for Ed448ph. The PSA sign/verify entry points passed hash_length straight to wc_ed25519ph_*/wc_ed448ph_*, so a caller could sign or verify an arbitrary-length buffer and get a non-RFC-8032 signature that round-trips within wolfPSA but fails under a strict verifier. Reject any hash_length other than 64 with PSA_ERROR_INVALID_ARGUMENT in all four functions, matching the spec's "hash_length is not valid for the algorithm and key type" return condition. Add ed25519_bad_hash_len and ed448_bad_hash_len tests asserting a 32-byte hash is rejected.
psa_key_derivation_verify_key only rejected a wrong key type for the PBKDF2 case (expecting PSA_KEY_TYPE_PASSWORD_HASH). For HKDF, TLS12_PRF, and other non-PBKDF2 KDFs the PSA Crypto API requires the expected key to be PSA_KEY_TYPE_RAW_DATA; any other type must be rejected with PSA_ERROR_INVALID_ARGUMENT. The code silently accepted AES/HMAC/etc. keys and compared their raw bytes against the derived output. Add an else branch rejecting non-RAW_DATA expected keys for non-PBKDF2 algorithms, and extend test_kdf_verify_key_policy to assert that an AES key passed to an HKDF verify_key returns PSA_ERROR_INVALID_ARGUMENT.
The CBC_NO_PADDING, CBC_PKCS7 and ECB_NO_PADDING encrypt paths in psa_cipher_update assemble a full-block plaintext input (ctx->partial plus bytes from input) into a stack-local block[AES_BLOCK_SIZE] and returned without scrubbing it, leaving plaintext on the stack frame. Add wc_ForceZero(block, sizeof(block)) before the block goes out of scope, matching the existing scrub in the CBC-PKCS7 decrypt finish path.
Per the PSA Crypto API state machine, psa_aead_set_lengths is a one-shot setup call: once the lengths have been recorded the operation must reject a subsequent call with PSA_ERROR_BAD_STATE. The previous implementation only checked nonce/aad/input, so two back-to-back set_lengths calls (before any nonce/AAD/input) silently overwrote ad_expected/plaintext_expected. Add a ctx->lengths_set guard and a regression test.
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #18
Scan targets checked: wolfpsa-bugs, wolfpsa-src
No new issues found in the changed files. ✅
dgarske
approved these changes
Jun 9, 2026
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
4 participants
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.
661adfc F-3861: reject second psa_aead_set_lengths call with BAD_STATE
2db1421 F-3865: zeroize plaintext block on psa_cipher_update encrypt paths
de3664a F-4094: enforce PSA_KEY_TYPE_RAW_DATA for non-PBKDF2 verify_key
6a77adb F-4095: enforce 64-byte prehash length for Ed25519ph/Ed448ph sign/verify
85daf4d F-4097: zeroize export buffer on key-data short read
7ced088 F-4559: free wolfCrypt MAC context on mac_setup error paths
297a5dd F-5551: add negative AEAD authentication-failure tag tests
e068596 F-5554: honor PSA_ALG_ANY_HASH wildcard sign/verify policies
ea0e48a F-3857: reject duplicate persistent key ids in psa_import_key
63ec17d F-3858: add test coverage for psa_aead_generate_nonce
f0b3a3d F-4091: add test coverage for psa_cipher_generate_iv
af941c9 F-4552: enable side-channel hardening in the default build
9528519 F-4553: assign auto key ids from the vendor range to avoid persistent-id collision
d6d4e2f F-4557: add AES-CMAC known-answer test to cover block-cipher-MAC dispatch
4d93f95 F-5549: reject NULL reference hash in psa_hash_compare
3fd680f F-5552: add known-answer tests for non-SHA-256 hash dispatch
35ff7e3 F-5557: reject oversized data_length in psa_import_key to prevent buffer-size overflow
9368edd F-5550: enforce the full key-agreement policy, not just the base algorithm