From 7da3d84cff39ccf091e20202159244b9a57277a0 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 5 Aug 2026 15:09:25 +0200 Subject: [PATCH 1/9] F-6970: add negative test for CBC-PKCS7 padding validation test_cipher_cbc_pkcs7_multipart_decrypt and friends only exercised valid-padding roundtrips, so no test ever expected PSA_ERROR_INVALID_PADDING from psa_cipher_finish. Add test_cipher_cbc_pkcs7_reject_invalid_padding: encrypt a one-block plaintext, flip one bit in the ciphertext block preceding the padding block (which, via CBC chaining P_last = Dec(C_last) XOR C_prev, deterministically corrupts exactly one byte of the recovered padding while pad_len itself stays valid), and assert psa_cipher_finish rejects it. Verified this fails if the padding-byte comparison loop is deleted. --- test/psa_server/psa_api_test.c | 121 +++++++++++++++++++++++++++++++++ 1 file changed, 121 insertions(+) diff --git a/test/psa_server/psa_api_test.c b/test/psa_server/psa_api_test.c index fe6029d..c5417be 100644 --- a/test/psa_server/psa_api_test.c +++ b/test/psa_server/psa_api_test.c @@ -2183,6 +2183,121 @@ static int test_cipher_cbc_pkcs7_multipart_decrypt(void) return result; } +/* F-6970: corrupting a ciphertext byte in the block preceding the final + * padding block must flip exactly one recovered padding byte, which the + * PKCS7 padding-byte comparison loop in psa_cipher_finish() must catch and + * reject with PSA_ERROR_INVALID_PADDING. */ +static int test_cipher_cbc_pkcs7_reject_invalid_padding(void) +{ + static const uint8_t key[16] = { + 0x2b,0x7e,0x15,0x16,0x28,0xae,0xd2,0xa6, + 0xab,0xf7,0x15,0x88,0x09,0xcf,0x4f,0x3c + }; + static const uint8_t iv[16] = { + 0x00,0x01,0x02,0x03,0x04,0x05,0x06,0x07, + 0x08,0x09,0x0a,0x0b,0x0c,0x0d,0x0e,0x0f + }; + /* Exactly one AES block: encryption appends a full extra block of + * 0x10 padding bytes, so ciphertext[16..31] decrypts to sixteen + * bytes of value 0x10. */ + static const uint8_t plaintext[16] = { + 0x30,0x31,0x32,0x33,0x34,0x35,0x36,0x37, + 0x38,0x39,0x61,0x62,0x63,0x64,0x65,0x66 + }; + uint8_t ciphertext[sizeof(plaintext) + 16]; + uint8_t decrypted[sizeof(plaintext) + 16]; + size_t ciphertext_len = 0; + size_t part_len = 0; + size_t finish_len = 0; + size_t dec_part_len = 0; + size_t dec_finish_len = 0; + psa_key_id_t key_id = 0; + psa_key_attributes_t attrs = psa_key_attributes_init(); + psa_cipher_operation_t op = psa_cipher_operation_init(); + psa_status_t st; + int result = TEST_FAIL; + + psa_set_key_type(&attrs, PSA_KEY_TYPE_AES); + psa_set_key_bits(&attrs, 128); + psa_set_key_usage_flags(&attrs, PSA_KEY_USAGE_ENCRYPT | PSA_KEY_USAGE_DECRYPT); + psa_set_key_algorithm(&attrs, PSA_ALG_CBC_PKCS7); + + st = psa_import_key(&attrs, key, sizeof(key), &key_id); + if (check_status(st, "psa_import_key(AES PKCS7 bad pad)") != TEST_OK) goto cleanup; + + st = psa_cipher_encrypt_setup(&op, key_id, PSA_ALG_CBC_PKCS7); + if (st == PSA_ERROR_NOT_SUPPORTED) { + result = TEST_SKIPPED; + goto cleanup; + } + if (check_status(st, "psa_cipher_encrypt_setup(PKCS7 bad pad)") != TEST_OK) { + goto cleanup; + } + st = psa_cipher_set_iv(&op, iv, sizeof(iv)); + if (check_status(st, "psa_cipher_set_iv(PKCS7 bad pad)") != TEST_OK) { + goto cleanup; + } + st = psa_cipher_update(&op, plaintext, sizeof(plaintext), + ciphertext, sizeof(ciphertext), &part_len); + if (check_status(st, "psa_cipher_update(PKCS7 bad pad enc)") != TEST_OK) { + goto cleanup; + } + ciphertext_len += part_len; + st = psa_cipher_finish(&op, ciphertext + ciphertext_len, + sizeof(ciphertext) - ciphertext_len, &finish_len); + if (check_status(st, "psa_cipher_finish(PKCS7 bad pad enc)") != TEST_OK) { + goto cleanup; + } + ciphertext_len += finish_len; + (void)psa_cipher_abort(&op); + if (check_true(ciphertext_len == sizeof(plaintext) + 16, + "psa_cipher_encrypt(PKCS7 bad pad) length") != TEST_OK) { + goto cleanup; + } + + /* Flip one bit in the block preceding the padding block. Since + * P_last = Dec(C_last) XOR C_prev in CBC mode, this changes exactly + * one byte of the recovered padding block from 0x10 to 0x11, while + * the other fifteen bytes -- including the pad_len byte itself -- + * stay 0x10. Only the per-byte comparison loop can detect this. */ + ciphertext[3] ^= 0x01; + + op = psa_cipher_operation_init(); + st = psa_cipher_decrypt_setup(&op, key_id, PSA_ALG_CBC_PKCS7); + if (check_status(st, "psa_cipher_decrypt_setup(PKCS7 bad pad)") != TEST_OK) { + goto cleanup; + } + st = psa_cipher_set_iv(&op, iv, sizeof(iv)); + if (check_status(st, "psa_cipher_set_iv(PKCS7 bad pad dec)") != TEST_OK) { + goto cleanup; + } + st = psa_cipher_update(&op, ciphertext, ciphertext_len, + decrypted, sizeof(decrypted), &dec_part_len); + if (check_status(st, "psa_cipher_update(PKCS7 bad pad dec)") != TEST_OK) { + goto cleanup; + } + st = psa_cipher_finish(&op, decrypted + dec_part_len, + sizeof(decrypted) - dec_part_len, &dec_finish_len); + if (check_true(st == PSA_ERROR_INVALID_PADDING, + "psa_cipher_finish(PKCS7 bad pad) status") != TEST_OK) { + goto cleanup; + } + (void)psa_cipher_abort(&op); + + st = psa_destroy_key(key_id); + if (check_status(st, "psa_destroy_key(AES PKCS7 bad pad)") != TEST_OK) return TEST_FAIL; + + key_id = 0; + result = TEST_OK; + +cleanup: + (void)psa_cipher_abort(&op); + if (key_id != 0) { + (void)psa_destroy_key(key_id); + } + return result; +} + static int test_cipher_cbc_pkcs7_decrypt_update_small_output(void) { static const uint8_t key[16] = { @@ -8332,6 +8447,12 @@ int main(int argc, char** argv) return TEST_FAIL; } } + if (only == NULL || strcmp(only, "cipher_cbc_pkcs7_reject_invalid_padding") == 0) { + if (run_named_test("cipher_cbc_pkcs7_reject_invalid_padding", + test_cipher_cbc_pkcs7_reject_invalid_padding) == TEST_FAIL) { + return TEST_FAIL; + } + } if (only == NULL || strcmp(only, "cipher_error_state") == 0) { if (run_named_test("cipher_error_state", test_cipher_error_aborts_operation) == TEST_FAIL) { From 934c0d6019280c923f34762f4729d0dea0e5936f Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 5 Aug 2026 15:14:38 +0200 Subject: [PATCH 2/9] F-6253: add negative test for psa_decapsulate DECRYPT usage policy test_missing_usage only covered psa_encapsulate lacking ENCRYPT and decapsulating with a public key (wrong type). No test exercised the key_usage & PSA_KEY_USAGE_DECRYPT check in psa_decapsulate itself, so removing or inverting that check would go undetected. Add test_decapsulate_missing_usage: generate an ML-KEM key pair with ENCRYPT but not DECRYPT, encapsulate successfully, then assert psa_decapsulate returns PSA_ERROR_NOT_PERMITTED. --- test/psa_server/psa_mlkem_test.c | 54 ++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/test/psa_server/psa_mlkem_test.c b/test/psa_server/psa_mlkem_test.c index 70db0a1..1e97175 100644 --- a/test/psa_server/psa_mlkem_test.c +++ b/test/psa_server/psa_mlkem_test.c @@ -677,6 +677,59 @@ static int test_missing_usage(void) return 0; } +/* -------------------------------------------------------------------------- + * Case 6b: key pair without DECRYPT usage: psa_decapsulate must be rejected + * with PSA_ERROR_NOT_PERMITTED, even though the ciphertext itself is valid + * (obtained via a successful psa_encapsulate on the same key). + * -------------------------------------------------------------------------- */ +static int test_decapsulate_missing_usage(void) +{ + psa_key_attributes_t kp_attrs = psa_key_attributes_init(); + psa_key_attributes_t ss_attrs; + psa_key_id_t kp_no_dec = PSA_KEY_ID_NULL; + psa_key_id_t ss_enc = PSA_KEY_ID_NULL; + psa_key_id_t ss_dec = PSA_KEY_ID_NULL; + uint8_t ct[768]; /* ML-KEM-512 */ + size_t ct_len = 0; + psa_status_t st; + + /* Generate a key pair WITHOUT PSA_KEY_USAGE_DECRYPT. */ + psa_set_key_type(&kp_attrs, PSA_KEY_TYPE_ML_KEM_KEY_PAIR); + psa_set_key_bits(&kp_attrs, 512); + psa_set_key_usage_flags(&kp_attrs, + PSA_KEY_USAGE_ENCRYPT | PSA_KEY_USAGE_EXPORT); + psa_set_key_algorithm(&kp_attrs, PSA_ALG_ML_KEM); + + st = psa_generate_key(&kp_attrs, &kp_no_dec); + if (check_status(st, "decap-missing-usage generate kp") != 0) return 1; + + ss_attrs = make_ss_attrs(); + st = psa_encapsulate(kp_no_dec, PSA_ALG_ML_KEM, &ss_attrs, + &ss_enc, ct, sizeof(ct), &ct_len); + if (check_status(st, "decap-missing-usage encapsulate") != 0) { + (void)psa_destroy_key(kp_no_dec); + return 1; + } + + ss_attrs = make_ss_attrs(); + st = psa_decapsulate(kp_no_dec, PSA_ALG_ML_KEM, ct, ct_len, + &ss_attrs, &ss_dec); + if (check_status_eq(st, PSA_ERROR_NOT_PERMITTED, + "decapsulate without DECRYPT") != 0) { + if (ss_dec != PSA_KEY_ID_NULL) + (void)psa_destroy_key(ss_dec); + (void)psa_destroy_key(ss_enc); + (void)psa_destroy_key(kp_no_dec); + return 1; + } + + (void)psa_destroy_key(ss_enc); + (void)psa_destroy_key(kp_no_dec); + + printf("PASS: test_decapsulate_missing_usage\n"); + return 0; +} + /* -------------------------------------------------------------------------- * Case 7: import a 64-byte seed as ML_KEM_KEY_PAIR. * - bits=0 -> PSA_ERROR_INVALID_ARGUMENT (ambiguous). @@ -860,6 +913,7 @@ int main(void) if (test_tampered_ciphertext_implicit_rejection() != 0) return 1; if (test_wrong_ciphertext_length() != 0) return 1; if (test_missing_usage() != 0) return 1; + if (test_decapsulate_missing_usage() != 0) return 1; if (test_import_seed() != 0) return 1; if (test_buffer_too_small() != 0) return 1; From 898d16475b6f78d4770e86f0ba9cd1fd3d05e178 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 5 Aug 2026 15:16:44 +0200 Subject: [PATCH 3/9] F-6252: add negative test for KEK algorithm-policy check in key wrap/unwrap psa_wrap_key/psa_unwrap_key reject a KEK whose permitted algorithm is not PSA_ALG_KW (or is PSA_ALG_NONE), even if it carries WRAP/UNWRAP usage. No existing test exercised this branch since every KEK in the suite was imported with its permitted algorithm equal to the alg passed in. Add a test importing KEKs with PSA_ALG_CTR and PSA_ALG_NONE to pin the PSA_ERROR_NOT_PERMITTED behavior on both entry points. --- test/psa_server/psa_key_wrap_test.c | 96 +++++++++++++++++++++++++++++ 1 file changed, 96 insertions(+) diff --git a/test/psa_server/psa_key_wrap_test.c b/test/psa_server/psa_key_wrap_test.c index 15e0651..6ffde1e 100644 --- a/test/psa_server/psa_key_wrap_test.c +++ b/test/psa_server/psa_key_wrap_test.c @@ -38,6 +38,9 @@ * 6. PSA_ALG_KWP → PSA_ERROR_NOT_SUPPORTED. * 7. Corrupted wrapped data (one byte flipped) → PSA_ERROR_INVALID_SIGNATURE. * 8. Output buffer one byte too small → PSA_ERROR_BUFFER_TOO_SMALL. + * 9. KEK whose permitted algorithm does not authorize PSA_ALG_KW (either + * PSA_ALG_NONE or PSA_ALG_CTR, despite carrying WRAP|UNWRAP usage) → + * PSA_ERROR_NOT_PERMITTED from both psa_wrap_key and psa_unwrap_key. */ #include "psa_api_test_user_settings.h" @@ -568,6 +571,98 @@ static int test_buffer_too_small(void) return ret; } +/* ------------------------------------------------------------------------- + * Test 9: KEK permitted-algorithm policy (wrap_alg != alg) → + * PSA_ERROR_NOT_PERMITTED + * + * A KEK carrying PSA_KEY_USAGE_WRAP|UNWRAP but whose permitted algorithm is + * not PSA_ALG_KW must be rejected by both psa_wrap_key and psa_unwrap_key, + * even though the usage-flag and key-type checks would otherwise pass. This + * covers both the "!= alg" mismatch (PSA_ALG_CTR) and the PSA_ALG_NONE case. + * ---------------------------------------------------------------------- */ +static int test_wrap_alg_policy(void) +{ + psa_key_id_t kek_mismatch = PSA_KEY_ID_NULL; + psa_key_id_t kek_none = PSA_KEY_ID_NULL; + psa_key_id_t target = PSA_KEY_ID_NULL; + psa_key_id_t dummy_key = PSA_KEY_ID_NULL; + psa_key_attributes_t tattr = psa_key_attributes_init(); + psa_key_attributes_t rattr = psa_key_attributes_init(); + uint8_t wrapped[32]; + size_t wrapped_len = 0; + psa_status_t st; + int ret = 0; + + /* KEK permitted for AES-CTR, not PSA_ALG_KW */ + kek_mismatch = import_aes_key("alg_policy/kek_mismatch", + kKekRfc3394, sizeof(kKekRfc3394), + PSA_KEY_USAGE_WRAP | PSA_KEY_USAGE_UNWRAP, + PSA_ALG_CTR); + if (kek_mismatch == PSA_KEY_ID_NULL) { ret = 1; goto done; } + + /* KEK with no permitted algorithm at all */ + kek_none = import_aes_key("alg_policy/kek_none", + kKekRfc3394, sizeof(kKekRfc3394), + PSA_KEY_USAGE_WRAP | PSA_KEY_USAGE_UNWRAP, + PSA_ALG_NONE); + if (kek_none == PSA_KEY_ID_NULL) { ret = 1; goto done; } + + /* Target key */ + psa_set_key_type(&tattr, PSA_KEY_TYPE_AES); + psa_set_key_bits(&tattr, 128); + psa_set_key_usage_flags(&tattr, PSA_KEY_USAGE_EXPORT); + psa_set_key_algorithm(&tattr, PSA_ALG_KW); + + st = psa_import_key(&tattr, kPlainRfc3394, sizeof(kPlainRfc3394), &target); + if (expect_status("alg_policy/import_target", st, PSA_SUCCESS) != 0) { + ret = 1; goto done; + } + + /* psa_wrap_key with an AES-CTR-only KEK */ + st = psa_wrap_key(kek_mismatch, PSA_ALG_KW, target, + wrapped, sizeof(wrapped), &wrapped_len); + if (expect_status("alg_policy/wrap_mismatch", + st, PSA_ERROR_NOT_PERMITTED) != 0) { + ret = 1; goto done; + } + + /* psa_wrap_key with a PSA_ALG_NONE KEK */ + st = psa_wrap_key(kek_none, PSA_ALG_KW, target, + wrapped, sizeof(wrapped), &wrapped_len); + if (expect_status("alg_policy/wrap_none", + st, PSA_ERROR_NOT_PERMITTED) != 0) { + ret = 1; goto done; + } + + /* psa_unwrap_key with an AES-CTR-only KEK */ + psa_set_key_type(&rattr, PSA_KEY_TYPE_AES); + psa_set_key_bits(&rattr, 128); + psa_set_key_usage_flags(&rattr, PSA_KEY_USAGE_EXPORT); + psa_set_key_algorithm(&rattr, PSA_ALG_KW); + + st = psa_unwrap_key(&rattr, kek_mismatch, PSA_ALG_KW, + kCipherRfc3394, sizeof(kCipherRfc3394), &dummy_key); + if (expect_status("alg_policy/unwrap_mismatch", + st, PSA_ERROR_NOT_PERMITTED) != 0) { + ret = 1; goto done; + } + + /* psa_unwrap_key with a PSA_ALG_NONE KEK */ + st = psa_unwrap_key(&rattr, kek_none, PSA_ALG_KW, + kCipherRfc3394, sizeof(kCipherRfc3394), &dummy_key); + if (expect_status("alg_policy/unwrap_none", + st, PSA_ERROR_NOT_PERMITTED) != 0) { + ret = 1; goto done; + } + +done: + destroy_if_valid(target); + destroy_if_valid(kek_mismatch); + destroy_if_valid(kek_none); + destroy_if_valid(dummy_key); + return ret; +} + /* ------------------------------------------------------------------------- * main * ---------------------------------------------------------------------- */ @@ -589,6 +684,7 @@ int main(void) if (test_kwp_not_supported() != 0) return 1; if (test_corrupt_wrapped() != 0) return 1; if (test_buffer_too_small() != 0) return 1; + if (test_wrap_alg_policy() != 0) return 1; printf("PSA key wrap test: OK\n"); return 0; From dc1a7f56f550b163178665644d62824defdf8113 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 5 Aug 2026 15:18:36 +0200 Subject: [PATCH 4/9] F-6073: add multipart test coverage for psa_hash_verify psa_hash_verify was a live public API with no test in the repo, leaving its length check and ConstantCompare deletable by mutation without any test failure. Add setup/update/verify coverage for the correct-digest, one-byte-flipped, and wrong-length cases, mirroring the existing one-shot psa_hash_compare test. --- test/psa_server/psa_api_test.c | 54 ++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/test/psa_server/psa_api_test.c b/test/psa_server/psa_api_test.c index c5417be..2ae7be6 100644 --- a/test/psa_server/psa_api_test.c +++ b/test/psa_server/psa_api_test.c @@ -315,6 +315,57 @@ static int test_hash_compare(void) return TEST_OK; } +static int test_hash_verify(void) +{ + static const uint8_t msg[] = "abc"; + /* SHA-256("abc") */ + static const uint8_t sha256_abc[WC_SHA256_DIGEST_SIZE] = { + 0xba, 0x78, 0x16, 0xbf, 0x8f, 0x01, 0xcf, 0xea, + 0x41, 0x41, 0x40, 0xde, 0x5d, 0xae, 0x22, 0x23, + 0xb0, 0x03, 0x61, 0xa3, 0x96, 0x17, 0x7a, 0x9c, + 0xb4, 0x10, 0xff, 0x61, 0xf2, 0x00, 0x15, 0xad, + }; + uint8_t bad_hash[WC_SHA256_DIGEST_SIZE]; + psa_hash_operation_t op; + psa_status_t st; + + /* Success path: correct digest via multipart setup/update/verify must + * return PSA_SUCCESS. */ + op = psa_hash_operation_init(); + st = psa_hash_setup(&op, PSA_ALG_SHA_256); + if (check_status(st, "psa_hash_setup(verify correct)") != TEST_OK) return TEST_FAIL; + st = psa_hash_update(&op, msg, sizeof(msg) - 1); + if (check_status(st, "psa_hash_update(verify correct)") != TEST_OK) return TEST_FAIL; + st = psa_hash_verify(&op, sha256_abc, sizeof(sha256_abc)); + if (check_status(st, "psa_hash_verify correct") != TEST_OK) return TEST_FAIL; + + /* Mismatch path: one byte flipped must return PSA_ERROR_INVALID_SIGNATURE. + * Catches mutation of the ConstantCompare check. */ + memcpy(bad_hash, sha256_abc, sizeof(bad_hash)); + bad_hash[0] ^= 0x01; + op = psa_hash_operation_init(); + st = psa_hash_setup(&op, PSA_ALG_SHA_256); + if (check_status(st, "psa_hash_setup(verify mismatch)") != TEST_OK) return TEST_FAIL; + st = psa_hash_update(&op, msg, sizeof(msg) - 1); + if (check_status(st, "psa_hash_update(verify mismatch)") != TEST_OK) return TEST_FAIL; + st = psa_hash_verify(&op, bad_hash, sizeof(bad_hash)); + if (check_true(st == PSA_ERROR_INVALID_SIGNATURE, + "psa_hash_verify mismatch") != TEST_OK) return TEST_FAIL; + + /* Wrong reference length must return PSA_ERROR_INVALID_SIGNATURE. + * Catches mutation of the hash_length != computed_hash_length check. */ + op = psa_hash_operation_init(); + st = psa_hash_setup(&op, PSA_ALG_SHA_256); + if (check_status(st, "psa_hash_setup(verify wrong length)") != TEST_OK) return TEST_FAIL; + st = psa_hash_update(&op, msg, sizeof(msg) - 1); + if (check_status(st, "psa_hash_update(verify wrong length)") != TEST_OK) return TEST_FAIL; + st = psa_hash_verify(&op, sha256_abc, sizeof(sha256_abc) - 1); + if (check_true(st == PSA_ERROR_INVALID_SIGNATURE, + "psa_hash_verify wrong length") != TEST_OK) return TEST_FAIL; + + return TEST_OK; +} + static int check_status_or_skip(psa_status_t st, const char* what) { if (st == PSA_ERROR_NOT_SUPPORTED) { @@ -8310,6 +8361,9 @@ int main(int argc, char** argv) if (only == NULL || strcmp(only, "hash_compare") == 0) { if (run_named_test("hash_compare", test_hash_compare) == TEST_FAIL) return TEST_FAIL; } + if (only == NULL || strcmp(only, "hash_verify") == 0) { + if (run_named_test("hash_verify", test_hash_verify) == TEST_FAIL) return TEST_FAIL; + } if (only == NULL || strcmp(only, "hash_error_state") == 0) { if (run_named_test("hash_error_state", test_hash_error_aborts_operation) == TEST_FAIL) { From 26e2d1d2be55796f203222940289607d2380d680 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 5 Aug 2026 15:20:26 +0200 Subject: [PATCH 5/9] F-6072: add mismatch-value coverage for KDF verify_bytes/verify_key test_kdf_verify_key_policy only exercised the matching-value path of psa_key_derivation_verify_bytes/verify_key, so a mutation that made the ConstantCompare mismatch branch unreachable (always PSA_SUCCESS) would go undetected. Add HKDF cases that flip one byte of the expected value and assert PSA_ERROR_INVALID_SIGNATURE for both APIs. --- test/psa_server/psa_api_test.c | 67 ++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/test/psa_server/psa_api_test.c b/test/psa_server/psa_api_test.c index 2ae7be6..ac2e897 100644 --- a/test/psa_server/psa_api_test.c +++ b/test/psa_server/psa_api_test.c @@ -6987,9 +6987,11 @@ static int test_kdf_verify_key_policy(void) uint8_t pbkdf2_expected[16]; psa_key_derivation_operation_t op = psa_key_derivation_operation_init(); psa_key_attributes_t attrs = psa_key_attributes_init(); + uint8_t hkdf_bad_expected[16]; psa_key_id_t hkdf_verify_key = 0; psa_key_id_t hkdf_no_usage_key = 0; psa_key_id_t hkdf_wrong_type_key = 0; + psa_key_id_t hkdf_mismatch_key = 0; psa_key_id_t pbkdf2_verify_key = 0; psa_key_id_t pbkdf2_wrong_type_key = 0; psa_status_t st; @@ -7050,6 +7052,68 @@ static int test_kdf_verify_key_policy(void) } op = psa_key_derivation_operation_init(); + /* Mismatch path: one byte flipped must return PSA_ERROR_INVALID_SIGNATURE. + * Catches mutation of the ConstantCompare check in verify_bytes/verify_key. */ + memcpy(hkdf_bad_expected, hkdf_expected, sizeof(hkdf_bad_expected)); + hkdf_bad_expected[0] ^= 0x01u; + + st = psa_key_derivation_setup(&op, PSA_ALG_HKDF(PSA_ALG_SHA_256)); + if (check_status(st, "psa_key_derivation_setup(HKDF verify_bytes mismatch)") != TEST_OK) { + goto cleanup; + } + st = psa_key_derivation_input_bytes(&op, PSA_KEY_DERIVATION_INPUT_SECRET, + hkdf_secret, sizeof(hkdf_secret)); + if (check_status(st, "psa_key_derivation_input_bytes(SECRET verify_bytes mismatch)") != TEST_OK) { + goto cleanup; + } + st = psa_key_derivation_input_bytes(&op, PSA_KEY_DERIVATION_INPUT_INFO, + hkdf_info, sizeof(hkdf_info) - 1u); + if (check_status(st, "psa_key_derivation_input_bytes(INFO verify_bytes mismatch)") != TEST_OK) { + goto cleanup; + } + st = psa_key_derivation_verify_bytes(&op, hkdf_bad_expected, sizeof(hkdf_bad_expected)); + if (check_true(st == PSA_ERROR_INVALID_SIGNATURE, + "psa_key_derivation_verify_bytes rejects mismatched value") != TEST_OK) { + goto cleanup; + } + st = psa_key_derivation_abort(&op); + if (check_status(st, "psa_key_derivation_abort(HKDF verify_bytes mismatch)") != TEST_OK) { + goto cleanup; + } + op = psa_key_derivation_operation_init(); + + psa_set_key_type(&attrs, PSA_KEY_TYPE_RAW_DATA); + psa_set_key_usage_flags(&attrs, PSA_KEY_USAGE_VERIFY_DERIVATION); + st = psa_import_key(&attrs, hkdf_bad_expected, sizeof(hkdf_bad_expected), &hkdf_mismatch_key); + if (check_status(st, "psa_import_key(HKDF verify_key mismatch key)") != TEST_OK) { + goto cleanup; + } + + st = psa_key_derivation_setup(&op, PSA_ALG_HKDF(PSA_ALG_SHA_256)); + if (check_status(st, "psa_key_derivation_setup(HKDF verify_key mismatch)") != TEST_OK) { + goto cleanup; + } + st = psa_key_derivation_input_bytes(&op, PSA_KEY_DERIVATION_INPUT_SECRET, + hkdf_secret, sizeof(hkdf_secret)); + if (check_status(st, "psa_key_derivation_input_bytes(SECRET verify_key mismatch)") != TEST_OK) { + goto cleanup; + } + st = psa_key_derivation_input_bytes(&op, PSA_KEY_DERIVATION_INPUT_INFO, + hkdf_info, sizeof(hkdf_info) - 1u); + if (check_status(st, "psa_key_derivation_input_bytes(INFO verify_key mismatch)") != TEST_OK) { + goto cleanup; + } + st = psa_key_derivation_verify_key(&op, hkdf_mismatch_key); + if (check_true(st == PSA_ERROR_INVALID_SIGNATURE, + "psa_key_derivation_verify_key rejects mismatched value") != TEST_OK) { + goto cleanup; + } + st = psa_key_derivation_abort(&op); + if (check_status(st, "psa_key_derivation_abort(HKDF verify_key mismatch)") != TEST_OK) { + goto cleanup; + } + op = psa_key_derivation_operation_init(); + psa_reset_key_attributes(&attrs); psa_set_key_type(&attrs, PSA_KEY_TYPE_RAW_DATA); psa_set_key_usage_flags(&attrs, PSA_KEY_USAGE_EXPORT); @@ -7229,6 +7293,9 @@ static int test_kdf_verify_key_policy(void) if (hkdf_wrong_type_key != 0) { (void)psa_destroy_key(hkdf_wrong_type_key); } + if (hkdf_mismatch_key != 0) { + (void)psa_destroy_key(hkdf_mismatch_key); + } if (hkdf_verify_key != 0) { (void)psa_destroy_key(hkdf_verify_key); } From 7067e3ebb64a876ebfbe67e3a13d783c80c8b1e1 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 5 Aug 2026 15:31:35 +0200 Subject: [PATCH 6/9] F-6071: fix RSA PKCS#1v1.5 hashed-verify hash binding and add test coverage psa_asymmetric_verify_rsa() compared the padding-stripped output of wc_RsaSSL_Verify_ex2() directly against the caller's raw hash for PSA_ALG_RSA_PKCS1V15_SIGN(hash). Since wc_RsaUnPad_ex() only removes the PKCS#1 v1.5 padding and does not strip the DER DigestInfo wrapper that wc_EncodeSignature() added on the sign side, the recovered value is the full DigestInfo, never equal in length to the raw hash: verification of a validly-signed value always failed. Fix it by re-encoding the expected DigestInfo (same as the sign path) and comparing against that, matching PSA_ALG_RSA_PKCS1V15_SIGN_RAW and RSA-PSS which already round-trip correctly. Add negative test coverage (signature-substitution across RAW, hashed PKCS#1v1.5 and PSS) that exercises the hash-binding checks this finding flagged as untested. --- src/psa_rsa.c | 32 ++++++-- test/psa_server/psa_api_test.c | 129 +++++++++++++++++++++++++++++++++ 2 files changed, 156 insertions(+), 5 deletions(-) diff --git a/src/psa_rsa.c b/src/psa_rsa.c index fedc051..40a55c9 100644 --- a/src/psa_rsa.c +++ b/src/psa_rsa.c @@ -283,19 +283,41 @@ psa_status_t psa_asymmetric_verify_rsa(psa_key_type_t key_type, ret = wc_RsaSSL_Verify_ex(signature, (word32)signature_length, decoded, (word32)sizeof(decoded), &rsa_key, WC_RSA_PKCSV15_PAD); + + if (ret > 0) { + if ((size_t)ret != hash_length || + ConstantCompare(decoded, hash, (int)hash_length) != 0) { + ret = SIG_VERIFY_E; + } + } } else { + /* wc_RsaSSL_Verify_ex2() only strips the PKCS#1 v1.5 padding; + * the recovered value is still the DER-encoded DigestInfo that + * was signed, so it must be compared against a freshly encoded + * DigestInfo for the caller-supplied hash, not against the raw + * hash bytes. */ + byte encoded[RSA_MAX_SIZE/8]; + int hash_oid = wc_GetCTC_HashOID(hash_type); + int encoded_len = (hash_oid > 0) ? + (int)wc_EncodeSignature(encoded, hash, (word32)hash_length, + hash_oid) : 0; + ret = wc_RsaSSL_Verify_ex2(signature, (word32)signature_length, decoded, (word32)sizeof(decoded), &rsa_key, WC_RSA_PKCSV15_PAD, hash_type); - } - if (ret > 0) { - if ((size_t)ret != hash_length || - ConstantCompare(decoded, hash, (int)hash_length) != 0) { - ret = SIG_VERIFY_E; + if (ret > 0) { + if (encoded_len <= 0 || (size_t)ret != (size_t)encoded_len || + ConstantCompare(decoded, encoded, encoded_len) != 0) { + ret = SIG_VERIFY_E; + } + else { + ret = (int)hash_length; + } } + wc_ForceZero(encoded, sizeof(encoded)); } wc_ForceZero(decoded, sizeof(decoded)); } diff --git a/test/psa_server/psa_api_test.c b/test/psa_server/psa_api_test.c index ac2e897..9f156d7 100644 --- a/test/psa_server/psa_api_test.c +++ b/test/psa_server/psa_api_test.c @@ -5650,6 +5650,129 @@ static int test_asym_verify_rejects_bad_signatures(void) return TEST_OK; } +static int test_hash_verify_rejects_substituted_hash(psa_key_type_t type, + size_t bits, + psa_algorithm_t alg, + const uint8_t* hash_a, + const uint8_t* hash_b, + size_t hash_len, + size_t expected_sig_len, + const char* label) +{ + uint8_t sig[128]; + size_t sig_len = 0; + psa_key_id_t key_id = PSA_KEY_ID_NULL; + psa_key_attributes_t attrs = psa_key_attributes_init(); + psa_status_t st; + int result = TEST_FAIL; + + psa_set_key_type(&attrs, type); + psa_set_key_bits(&attrs, bits); + psa_set_key_usage_flags(&attrs, + PSA_KEY_USAGE_SIGN_HASH | PSA_KEY_USAGE_VERIFY_HASH); + psa_set_key_algorithm(&attrs, alg); + + st = psa_generate_key(&attrs, &key_id); + if (st == PSA_ERROR_NOT_SUPPORTED) { + return TEST_SKIPPED; + } + if (check_status(st, label) != TEST_OK) { + return TEST_FAIL; + } + + st = psa_sign_hash(key_id, alg, hash_a, hash_len, sig, sizeof(sig), &sig_len); + if (check_status(st, "psa_sign_hash(substituted hash setup)") != TEST_OK) { + goto cleanup; + } + if (check_true(sig_len == expected_sig_len, + "psa_sign_hash(substituted hash setup) length") != TEST_OK) { + goto cleanup; + } + + st = psa_verify_hash(key_id, alg, hash_a, hash_len, sig, sig_len); + if (check_status(st, "psa_verify_hash(genuine hash)") != TEST_OK) { + goto cleanup; + } + + /* A genuine signature over hash_a must not verify against a different + * hash_b: PKCS#1 v1.5 rejection surfaces as PSA_ERROR_INVALID_SIGNATURE, + * while RSA-PSS's padding check reports the mismatch as + * PSA_ERROR_INVALID_PADDING; either way psa_verify_hash() must not + * report success. */ + st = psa_verify_hash(key_id, alg, hash_b, hash_len, sig, sig_len); + if (check_true(st != PSA_SUCCESS, + "psa_verify_hash rejects signature over substituted hash") != TEST_OK) { + printf(" %s expected rejection, got PSA_SUCCESS\n", label); + goto cleanup; + } + + result = TEST_OK; + +cleanup: + if (key_id != PSA_KEY_ID_NULL) { + psa_status_t destroy_st = psa_destroy_key(key_id); + if (result == TEST_OK && + check_status(destroy_st, "psa_destroy_key(substituted hash test)") != TEST_OK) { + result = TEST_FAIL; + } + } + return result; +} + +static int test_rsa_verify_rejects_bad_signatures(void) +{ + static const uint8_t hash_a[WC_SHA256_DIGEST_SIZE] = { + 0x9f,0x86,0xd0,0x81,0x88,0x4c,0x7d,0x65, + 0x9a,0x2f,0xea,0xa0,0xc5,0x5a,0xd0,0x15, + 0xa3,0xbf,0x4f,0x1b,0x2b,0x0b,0x82,0x2c, + 0xd1,0x5d,0x6c,0x15,0xb0,0xf0,0x0a,0x08 + }; + static const uint8_t hash_b[WC_SHA256_DIGEST_SIZE] = { + 0x00,0x01,0x02,0x03,0x04,0x05,0x06,0x07, + 0x08,0x09,0x0a,0x0b,0x0c,0x0d,0x0e,0x0f, + 0x10,0x11,0x12,0x13,0x14,0x15,0x16,0x17, + 0x18,0x19,0x1a,0x1b,0x1c,0x1d,0x1e,0x1f + }; + int result; + + /* RSA signature corruption is exercised generically via the modular + * exponentiation input, so a flipped byte usually fails PKCS#1 padding + * validation itself (a different, still-safe error) rather than the + * hash-binding comparison covered by this finding. The substitution + * checks below keep the signature/padding well-formed and only vary + * the caller-supplied hash, which is what actually exercises the + * hash-binding check at psa_rsa.c's ConstantCompare()/check_padding() + * calls. */ + result = test_hash_verify_rejects_substituted_hash( + PSA_KEY_TYPE_RSA_KEY_PAIR, 1024, + PSA_ALG_RSA_PKCS1V15_SIGN(PSA_ALG_SHA_256), + hash_a, hash_b, sizeof(hash_a), 128u, + "psa_generate_key(RSA PKCS1v15 substituted hash)"); + if (result == TEST_FAIL) { + return TEST_FAIL; + } + + result = test_hash_verify_rejects_substituted_hash( + PSA_KEY_TYPE_RSA_KEY_PAIR, 1024, + PSA_ALG_RSA_PKCS1V15_SIGN_RAW, + hash_a, hash_b, sizeof(hash_a), 128u, + "psa_generate_key(RSA PKCS1v15 raw substituted hash)"); + if (result == TEST_FAIL) { + return TEST_FAIL; + } + + result = test_hash_verify_rejects_substituted_hash( + PSA_KEY_TYPE_RSA_KEY_PAIR, 1024, + PSA_ALG_RSA_PSS(PSA_ALG_SHA_256), + hash_a, hash_b, sizeof(hash_a), 128u, + "psa_generate_key(RSA PSS substituted hash)"); + if (result == TEST_FAIL) { + return TEST_FAIL; + } + + return TEST_OK; +} + static int test_asym_requires_verify_usage(void) { static const uint8_t hash[WC_SHA256_DIGEST_SIZE] = { @@ -8816,6 +8939,12 @@ int main(int argc, char** argv) return TEST_FAIL; } } + if (only == NULL || strcmp(only, "rsa_verify_bad_signature") == 0) { + if (run_named_test("rsa_verify_bad_signature", + test_rsa_verify_rejects_bad_signatures) == TEST_FAIL) { + return TEST_FAIL; + } + } if (only == NULL || strcmp(only, "asym_requires_verify_usage") == 0) { if (run_named_test("asym_requires_verify_usage", test_asym_requires_verify_usage) == TEST_FAIL) { From c2958ca4c1bae61a8c4c376f34763677117f86f9 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 5 Aug 2026 15:54:28 +0200 Subject: [PATCH 7/9] Bound hash_length before wc_EncodeSignature in RSA sign/verify Copilot flagged the encoded[RSA_MAX_SIZE/8] buffer added in 7067e3e as a stack-usage concern. It is worse than that: wc_EncodeSignature() writes a DER header plus hash_length bytes with no bound of its own, and hash_length is caller-supplied all the way down. wolfpsa_verify_hash_worker() only null-checks it and psa_asymmetric_verify_rsa() only range-checks it against word32, so the buffer size was the overflow bound rather than a headroom margin. The verify path is the regression: the DigestInfo encode happens before wc_RsaSSL_Verify_ex2(), so psa_verify_hash() with a public key and an over-long hash_length overruns the 384-byte stack buffer without needing a valid signature. ASan, on this branch before the fix: ERROR: AddressSanitizer: stack-buffer-overflow WRITE of size 4096 wc_EncodeSignature asn.c:16193 psa_asymmetric_verify_rsa src/psa_rsa.c:303 psa_verify_hash src/psa_asymmetric_api.c:713 [496, 880) 'encoded' (line 300) Size the buffer as Copilot suggested (PSA_HASH_MAX_SIZE + 32, matching the sign path) and add the range check that makes that sizing safe. psa_asymmetric_sign_rsa() has the same unbounded encode into a 96-byte heap buffer at psa_rsa.c:185. That one predates this branch and reproduces on master at 62f2931; it needs SIGN_HASH usage, so it is lower severity, but it is the same defect and is guarded here too. Add test_rsa_pkcs1v15_rejects_oversized_hash_length covering both entry points. The verify case signs a correctly sized hash first so the signature carries valid PKCS#1 padding, otherwise the call fails padding validation before reaching the guard and would pass for the wrong reason. Confirmed the test trips ASan when the guards are reverted. Also abort the hash operation on the update-failure paths of test_hash_verify_multipart, per Copilot's other comment, matching the existing convention. --- src/psa_rsa.c | 17 ++++++- test/psa_server/psa_api_test.c | 91 ++++++++++++++++++++++++++++++++-- 2 files changed, 103 insertions(+), 5 deletions(-) diff --git a/src/psa_rsa.c b/src/psa_rsa.c index 40a55c9..bb1e3ba 100644 --- a/src/psa_rsa.c +++ b/src/psa_rsa.c @@ -174,6 +174,14 @@ psa_status_t psa_asymmetric_sign_rsa(psa_key_type_t key_type, wc_FreeRsaKey(&rsa_key); return PSA_ERROR_NOT_SUPPORTED; } + /* wc_EncodeSignature() writes the DER header plus hash_length + * bytes into sig_input without bounding the write itself, so + * reject a hash that cannot fit before allocating. */ + if (hash_length > PSA_HASH_MAX_SIZE) { + wc_FreeRng(&rng); + wc_FreeRsaKey(&rsa_key); + return PSA_ERROR_INVALID_ARGUMENT; + } sig_input = (byte*)XMALLOC(PSA_HASH_MAX_SIZE + 32, NULL, DYNAMIC_TYPE_TMP_BUFFER); if (sig_input == NULL) { @@ -297,9 +305,14 @@ psa_status_t psa_asymmetric_verify_rsa(psa_key_type_t key_type, * was signed, so it must be compared against a freshly encoded * DigestInfo for the caller-supplied hash, not against the raw * hash bytes. */ - byte encoded[RSA_MAX_SIZE/8]; + byte encoded[PSA_HASH_MAX_SIZE + 32]; int hash_oid = wc_GetCTC_HashOID(hash_type); - int encoded_len = (hash_oid > 0) ? + /* wc_EncodeSignature() writes the DER header plus hash_length + * bytes with no bound of its own, and hash_length is caller + * supplied, so it must be range-checked against the destination + * before encoding. */ + int encoded_len = (hash_oid > 0 && + hash_length <= PSA_HASH_MAX_SIZE) ? (int)wc_EncodeSignature(encoded, hash, (word32)hash_length, hash_oid) : 0; diff --git a/test/psa_server/psa_api_test.c b/test/psa_server/psa_api_test.c index 9f156d7..c1dc2d5 100644 --- a/test/psa_server/psa_api_test.c +++ b/test/psa_server/psa_api_test.c @@ -335,7 +335,10 @@ static int test_hash_verify(void) st = psa_hash_setup(&op, PSA_ALG_SHA_256); if (check_status(st, "psa_hash_setup(verify correct)") != TEST_OK) return TEST_FAIL; st = psa_hash_update(&op, msg, sizeof(msg) - 1); - if (check_status(st, "psa_hash_update(verify correct)") != TEST_OK) return TEST_FAIL; + if (check_status(st, "psa_hash_update(verify correct)") != TEST_OK) { + (void)psa_hash_abort(&op); + return TEST_FAIL; + } st = psa_hash_verify(&op, sha256_abc, sizeof(sha256_abc)); if (check_status(st, "psa_hash_verify correct") != TEST_OK) return TEST_FAIL; @@ -347,7 +350,10 @@ static int test_hash_verify(void) st = psa_hash_setup(&op, PSA_ALG_SHA_256); if (check_status(st, "psa_hash_setup(verify mismatch)") != TEST_OK) return TEST_FAIL; st = psa_hash_update(&op, msg, sizeof(msg) - 1); - if (check_status(st, "psa_hash_update(verify mismatch)") != TEST_OK) return TEST_FAIL; + if (check_status(st, "psa_hash_update(verify mismatch)") != TEST_OK) { + (void)psa_hash_abort(&op); + return TEST_FAIL; + } st = psa_hash_verify(&op, bad_hash, sizeof(bad_hash)); if (check_true(st == PSA_ERROR_INVALID_SIGNATURE, "psa_hash_verify mismatch") != TEST_OK) return TEST_FAIL; @@ -358,7 +364,10 @@ static int test_hash_verify(void) st = psa_hash_setup(&op, PSA_ALG_SHA_256); if (check_status(st, "psa_hash_setup(verify wrong length)") != TEST_OK) return TEST_FAIL; st = psa_hash_update(&op, msg, sizeof(msg) - 1); - if (check_status(st, "psa_hash_update(verify wrong length)") != TEST_OK) return TEST_FAIL; + if (check_status(st, "psa_hash_update(verify wrong length)") != TEST_OK) { + (void)psa_hash_abort(&op); + return TEST_FAIL; + } st = psa_hash_verify(&op, sha256_abc, sizeof(sha256_abc) - 1); if (check_true(st == PSA_ERROR_INVALID_SIGNATURE, "psa_hash_verify wrong length") != TEST_OK) return TEST_FAIL; @@ -5719,6 +5728,75 @@ static int test_hash_verify_rejects_substituted_hash(psa_key_type_t type, return result; } +/* psa_sign_hash/psa_verify_hash pass the caller-supplied hash_length straight + * through to psa_asymmetric_sign_rsa()/psa_asymmetric_verify_rsa(), which feed + * it to wc_EncodeSignature(). That helper writes the DER header plus + * hash_length bytes into a fixed-size destination without bounding the write + * itself, so an over-long hash_length overruns the buffer (stack on the verify + * side, heap on the sign side) before any signature check happens. Pin the + * length guards on both entry points. */ +static int test_rsa_pkcs1v15_rejects_oversized_hash_length(void) +{ + static uint8_t big_hash[PSA_HASH_MAX_SIZE + 512]; + uint8_t sig[512]; + size_t sig_len = 0; + psa_key_id_t key_id = PSA_KEY_ID_NULL; + psa_key_attributes_t attrs = psa_key_attributes_init(); + psa_algorithm_t alg = PSA_ALG_RSA_PKCS1V15_SIGN(PSA_ALG_SHA_256); + psa_status_t st; + int result = TEST_FAIL; + + memset(big_hash, 0x41, sizeof(big_hash)); + + psa_set_key_type(&attrs, PSA_KEY_TYPE_RSA_KEY_PAIR); + psa_set_key_bits(&attrs, 2048); + psa_set_key_usage_flags(&attrs, + PSA_KEY_USAGE_SIGN_HASH | PSA_KEY_USAGE_VERIFY_HASH); + psa_set_key_algorithm(&attrs, alg); + + st = psa_generate_key(&attrs, &key_id); + if (st == PSA_ERROR_NOT_SUPPORTED) { + return TEST_SKIPPED; + } + if (check_status(st, "psa_generate_key(RSA oversized hash_length)") + != TEST_OK) { + return TEST_FAIL; + } + + /* Sign side: must be refused, not encoded into the temporary buffer. */ + st = psa_sign_hash(key_id, alg, big_hash, sizeof(big_hash), + sig, sizeof(sig), &sig_len); + if (check_true(st == PSA_ERROR_INVALID_ARGUMENT, + "psa_sign_hash rejects oversized hash_length") != TEST_OK) { + goto cleanup; + } + + /* Verify side: produce a well-formed signature over a correctly sized + * hash first. Without it the bogus signature fails PKCS#1 padding before + * the length guard is reached, and the case would pass for the wrong + * reason. With valid padding the DigestInfo encode is reached, so the + * length guard is what must reject. */ + st = psa_sign_hash(key_id, alg, big_hash, WC_SHA256_DIGEST_SIZE, + sig, sizeof(sig), &sig_len); + if (check_status(st, "psa_sign_hash(valid hash for verify guard)") + != TEST_OK) { + goto cleanup; + } + st = psa_verify_hash(key_id, alg, big_hash, sizeof(big_hash), + sig, sig_len); + if (check_true(st == PSA_ERROR_INVALID_SIGNATURE, + "psa_verify_hash rejects oversized hash_length") + != TEST_OK) { + goto cleanup; + } + + result = TEST_OK; + +cleanup: + psa_destroy_key(key_id); + return result; +} + static int test_rsa_verify_rejects_bad_signatures(void) { static const uint8_t hash_a[WC_SHA256_DIGEST_SIZE] = { @@ -8945,6 +9023,13 @@ int main(int argc, char** argv) return TEST_FAIL; } } + if (only == NULL || strcmp(only, "rsa_oversized_hash_length") == 0) { + if (run_named_test("rsa_oversized_hash_length", + test_rsa_pkcs1v15_rejects_oversized_hash_length) + == TEST_FAIL) { + return TEST_FAIL; + } + } if (only == NULL || strcmp(only, "asym_requires_verify_usage") == 0) { if (run_named_test("asym_requires_verify_usage", test_asym_requires_verify_usage) == TEST_FAIL) { From d304d0948e0aa9ecb4c0f39e3e095606c24ee2d5 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 6 Aug 2026 09:34:22 +0200 Subject: [PATCH 8/9] Address review: distinct RSA verify statuses, digest bound, test rigour Status codes (src/psa_rsa.c): the hashed PKCS#1 v1.5 verify branch folded an unsupported hash OID, an unencodable hash_length and an encode failure into encoded_len == 0, all reported as PSA_ERROR_INVALID_SIGNATURE. The sign path returns NOT_SUPPORTED and INVALID_ARGUMENT for the same two conditions, so the entry points disagreed on identical bad input, and a build problem was indistinguishable from an attack. Split them into explicit early-return guards mirroring the sign path; an encode failure now reports INSUFFICIENT_MEMORY, since wc_EncodeSignature() returning 0 means its internal allocation failed. This also lifts the buffer-filling call out of the ternary initializer. Digest bound: both guards used PSA_HASH_MAX_SIZE, derived from the PSA_WANT_ALG_SHA* set, while wc_GetCTC_HashOID() reflects the wolfCrypt build. A Zephyr config selecting only CONFIG_PSA_WANT_ALG_SHA_256 gets PSA_HASH_MAX_SIZE 32 while wolfCrypt still encodes SHA-512, so a valid 64-byte digest would be rejected. Bound both guards and both DigestInfo buffers by WC_MAX_DIGEST_SIZE via one shared WOLFPSA_RSA_DIGESTINFO_MAX. test_rsa_pkcs1v15_rejects_oversized_hash_length: the verify half asserted INVALID_SIGNATURE, which the vulnerable code also returns once the overrun DigestInfo fails the following length comparison, so without a sanitizer it passed against the bug it was written to pin. It now asserts INVALID_ARGUMENT. Verified: with the guard removed but the buffer enlarged so no overflow occurs, the assertion fails on its own. test_hash_verify_rejects_substituted_hash: propagate skips instead of returning TEST_OK when every sub-case skipped, which reported a pass having executed zero assertions; treat NOT_SUPPORTED from psa_sign_hash as a skip so a build without WC_RSA_PSS skips rather than fails; assert the exact expected status per algorithm instead of any non-success, and print it on mismatch; size sig from PSA_SIGNATURE_MAX_SIZE and move to 2048-bit keys for consistency with the sibling test. Renamed the caller and its selector to rsa_verify_substituted_hash, since it substitutes hashes rather than corrupting signatures. Also drop an internal finding ID from a test comment and add the missing psa_reset_key_attributes() before the HKDF mismatch-key import. --- src/psa_rsa.c | 60 ++++++++++++++++----- test/psa_server/psa_api_test.c | 96 +++++++++++++++++++++++----------- 2 files changed, 113 insertions(+), 43 deletions(-) diff --git a/src/psa_rsa.c b/src/psa_rsa.c index bb1e3ba..b7bf893 100644 --- a/src/psa_rsa.c +++ b/src/psa_rsa.c @@ -47,6 +47,18 @@ int wc_psa_get_rsa_padding(psa_algorithm_t alg); int wc_psa_get_hash_type(psa_algorithm_t alg); +/* Scratch size for the PKCS#1 v1.5 DigestInfo that wc_EncodeSignature() + * builds: the digest itself plus the DER header prepended to it. + * + * Bounded by WC_MAX_DIGEST_SIZE rather than PSA_HASH_MAX_SIZE because + * wc_GetCTC_HashOID() reflects what wolfCrypt was compiled with, while + * PSA_HASH_MAX_SIZE is derived from the PSA_WANT_ALG_SHA* set. The two can + * diverge - a Zephyr build selecting only CONFIG_PSA_WANT_ALG_SHA_256 gets + * PSA_HASH_MAX_SIZE 32 while wolfCrypt still returns a valid OID for + * SHA-512 - and sizing from the PSA constant would reject digests the + * implementation can legitimately encode. */ +#define WOLFPSA_RSA_DIGESTINFO_MAX (WC_MAX_DIGEST_SIZE + 32) + /* Only the PSS and OAEP paths use MGF1; guard the helper with the same * condition as its callers so a config with neither (e.g. RSA sign/verify with * PKCS#1 v1.5 only) does not trip -Werror=unused-function. */ @@ -177,19 +189,19 @@ psa_status_t psa_asymmetric_sign_rsa(psa_key_type_t key_type, /* wc_EncodeSignature() writes the DER header plus hash_length * bytes into sig_input without bounding the write itself, so * reject a hash that cannot fit before allocating. */ - if (hash_length > PSA_HASH_MAX_SIZE) { + if (hash_length > WC_MAX_DIGEST_SIZE) { wc_FreeRng(&rng); wc_FreeRsaKey(&rsa_key); return PSA_ERROR_INVALID_ARGUMENT; } - sig_input = (byte*)XMALLOC(PSA_HASH_MAX_SIZE + 32, NULL, + sig_input = (byte*)XMALLOC(WOLFPSA_RSA_DIGESTINFO_MAX, NULL, DYNAMIC_TYPE_TMP_BUFFER); if (sig_input == NULL) { wc_FreeRng(&rng); wc_FreeRsaKey(&rsa_key); return PSA_ERROR_INSUFFICIENT_MEMORY; } - sig_input_alloc_len = PSA_HASH_MAX_SIZE + 32; + sig_input_alloc_len = WOLFPSA_RSA_DIGESTINFO_MAX; sig_input_len = wc_EncodeSignature(sig_input, hash, (word32)hash_length, hash_oid); } @@ -305,16 +317,40 @@ psa_status_t psa_asymmetric_verify_rsa(psa_key_type_t key_type, * was signed, so it must be compared against a freshly encoded * DigestInfo for the caller-supplied hash, not against the raw * hash bytes. */ - byte encoded[PSA_HASH_MAX_SIZE + 32]; - int hash_oid = wc_GetCTC_HashOID(hash_type); + byte encoded[WOLFPSA_RSA_DIGESTINFO_MAX]; + int hash_oid; + int encoded_len; + + /* Report configuration and argument failures as themselves, + * matching the sign path, rather than letting them fall through + * to SIG_VERIFY_E. An unsupported hash or an unencodable digest + * is not a signature mismatch, and folding them together makes a + * build problem indistinguishable from an attack. */ + hash_oid = wc_GetCTC_HashOID(hash_type); + if (hash_oid <= 0) { + wc_ForceZero(decoded, sizeof(decoded)); + wc_FreeRsaKey(&rsa_key); + return PSA_ERROR_NOT_SUPPORTED; + } /* wc_EncodeSignature() writes the DER header plus hash_length * bytes with no bound of its own, and hash_length is caller - * supplied, so it must be range-checked against the destination - * before encoding. */ - int encoded_len = (hash_oid > 0 && - hash_length <= PSA_HASH_MAX_SIZE) ? - (int)wc_EncodeSignature(encoded, hash, (word32)hash_length, - hash_oid) : 0; + * supplied, so it must be range-checked before encoding. */ + if (hash_length > WC_MAX_DIGEST_SIZE) { + wc_ForceZero(decoded, sizeof(decoded)); + wc_FreeRsaKey(&rsa_key); + return PSA_ERROR_INVALID_ARGUMENT; + } + encoded_len = (int)wc_EncodeSignature(encoded, hash, + (word32)hash_length, + hash_oid); + /* Unsigned return type, so 0 is the only error indication; it + * means the internal allocation failed, not a bad signature. */ + if (encoded_len <= 0) { + wc_ForceZero(encoded, sizeof(encoded)); + wc_ForceZero(decoded, sizeof(decoded)); + wc_FreeRsaKey(&rsa_key); + return PSA_ERROR_INSUFFICIENT_MEMORY; + } ret = wc_RsaSSL_Verify_ex2(signature, (word32)signature_length, decoded, (word32)sizeof(decoded), @@ -322,7 +358,7 @@ psa_status_t psa_asymmetric_verify_rsa(psa_key_type_t key_type, hash_type); if (ret > 0) { - if (encoded_len <= 0 || (size_t)ret != (size_t)encoded_len || + if ((size_t)ret != (size_t)encoded_len || ConstantCompare(decoded, encoded, encoded_len) != 0) { ret = SIG_VERIFY_E; } diff --git a/test/psa_server/psa_api_test.c b/test/psa_server/psa_api_test.c index c1dc2d5..53b071d 100644 --- a/test/psa_server/psa_api_test.c +++ b/test/psa_server/psa_api_test.c @@ -2243,9 +2243,9 @@ static int test_cipher_cbc_pkcs7_multipart_decrypt(void) return result; } -/* F-6970: corrupting a ciphertext byte in the block preceding the final - * padding block must flip exactly one recovered padding byte, which the - * PKCS7 padding-byte comparison loop in psa_cipher_finish() must catch and +/* Corrupting a ciphertext byte in the block preceding the final padding + * block must flip exactly one recovered padding byte, which the PKCS7 + * padding-byte comparison loop in psa_cipher_finish() must catch and * reject with PSA_ERROR_INVALID_PADDING. */ static int test_cipher_cbc_pkcs7_reject_invalid_padding(void) { @@ -5666,9 +5666,10 @@ static int test_hash_verify_rejects_substituted_hash(psa_key_type_t type, const uint8_t* hash_b, size_t hash_len, size_t expected_sig_len, + psa_status_t expected_st, const char* label) { - uint8_t sig[128]; + uint8_t sig[PSA_SIGNATURE_MAX_SIZE]; size_t sig_len = 0; psa_key_id_t key_id = PSA_KEY_ID_NULL; psa_key_attributes_t attrs = psa_key_attributes_init(); @@ -5690,6 +5691,13 @@ static int test_hash_verify_rejects_substituted_hash(psa_key_type_t type, } st = psa_sign_hash(key_id, alg, hash_a, hash_len, sig, sizeof(sig), &sig_len); + /* Key generation succeeds for any RSA algorithm, so an algorithm the + * build does not implement (RSA-PSS without WC_RSA_PSS) only surfaces + * here. That is a skip, not a failure. */ + if (st == PSA_ERROR_NOT_SUPPORTED) { + result = TEST_SKIPPED; + goto cleanup; + } if (check_status(st, "psa_sign_hash(substituted hash setup)") != TEST_OK) { goto cleanup; } @@ -5704,14 +5712,16 @@ static int test_hash_verify_rejects_substituted_hash(psa_key_type_t type, } /* A genuine signature over hash_a must not verify against a different - * hash_b: PKCS#1 v1.5 rejection surfaces as PSA_ERROR_INVALID_SIGNATURE, - * while RSA-PSS's padding check reports the mismatch as - * PSA_ERROR_INVALID_PADDING; either way psa_verify_hash() must not - * report success. */ + * hash_b. The exact status is deterministic per algorithm, so pin it: + * PKCS#1 v1.5 rejection surfaces as PSA_ERROR_INVALID_SIGNATURE, while + * RSA-PSS's padding check reports the mismatch as + * PSA_ERROR_INVALID_PADDING. Accepting any non-success status here would + * also accept a regression that broke psa_verify_hash outright. */ st = psa_verify_hash(key_id, alg, hash_b, hash_len, sig, sig_len); - if (check_true(st != PSA_SUCCESS, + if (check_true(st == expected_st, "psa_verify_hash rejects signature over substituted hash") != TEST_OK) { - printf(" %s expected rejection, got PSA_SUCCESS\n", label); + printf(" %s expected 0x%08x, got 0x%08x\n", label, + (unsigned)expected_st, (unsigned)st); goto cleanup; } @@ -5720,7 +5730,7 @@ static int test_hash_verify_rejects_substituted_hash(psa_key_type_t type, cleanup: if (key_id != PSA_KEY_ID_NULL) { psa_status_t destroy_st = psa_destroy_key(key_id); - if (result == TEST_OK && + if (result != TEST_FAIL && check_status(destroy_st, "psa_destroy_key(substituted hash test)") != TEST_OK) { result = TEST_FAIL; } @@ -5737,8 +5747,8 @@ static int test_hash_verify_rejects_substituted_hash(psa_key_type_t type, * length guards on both entry points. */ static int test_rsa_pkcs1v15_rejects_oversized_hash_length(void) { - static uint8_t big_hash[PSA_HASH_MAX_SIZE + 512]; - uint8_t sig[512]; + static uint8_t big_hash[WC_MAX_DIGEST_SIZE + 512]; + uint8_t sig[PSA_SIGNATURE_MAX_SIZE]; size_t sig_len = 0; psa_key_id_t key_id = PSA_KEY_ID_NULL; psa_key_attributes_t attrs = psa_key_attributes_init(); @@ -5771,11 +5781,16 @@ static int test_rsa_pkcs1v15_rejects_oversized_hash_length(void) goto cleanup; } - /* Verify side: produce a well-formed signature over a correctly sized - * hash first. Without it the bogus signature fails PKCS#1 padding before - * the length guard is reached, and the case would pass for the wrong - * reason. With valid padding the DigestInfo encode is reached, so the - * length guard is what must reject. */ + /* Verify side: sign a correctly sized hash first, so the signature is + * one that would otherwise verify. The guard must still reject on + * hash_length alone, and must do so with the same status as the sign + * path rather than folding into a signature mismatch. + * + * PSA_ERROR_INVALID_ARGUMENT is what makes this assertion discriminating. + * Deleting the guard lets wc_EncodeSignature() overrun the DigestInfo + * buffer and then yields PSA_ERROR_INVALID_SIGNATURE from the following + * length comparison, so asserting INVALID_SIGNATURE here would pass + * against the vulnerable implementation in a build without a sanitizer. */ st = psa_sign_hash(key_id, alg, big_hash, WC_SHA256_DIGEST_SIZE, sig, sizeof(sig), &sig_len); if (check_status(st, "psa_sign_hash(valid hash for verify guard)") @@ -5784,7 +5799,7 @@ static int test_rsa_pkcs1v15_rejects_oversized_hash_length(void) } st = psa_verify_hash(key_id, alg, big_hash, sizeof(big_hash), sig, sig_len); - if (check_true(st == PSA_ERROR_INVALID_SIGNATURE, + if (check_true(st == PSA_ERROR_INVALID_ARGUMENT, "psa_verify_hash rejects oversized hash_length") != TEST_OK) { goto cleanup; @@ -5797,7 +5812,7 @@ static int test_rsa_pkcs1v15_rejects_oversized_hash_length(void) return result; } -static int test_rsa_verify_rejects_bad_signatures(void) +static int test_rsa_verify_rejects_substituted_hashes(void) { static const uint8_t hash_a[WC_SHA256_DIGEST_SIZE] = { 0x9f,0x86,0xd0,0x81,0x88,0x4c,0x7d,0x65, @@ -5812,6 +5827,7 @@ static int test_rsa_verify_rejects_bad_signatures(void) 0x18,0x19,0x1a,0x1b,0x1c,0x1d,0x1e,0x1f }; int result; + int ran = 0; /* RSA signature corruption is exercised generically via the modular * exponentiation input, so a flipped byte usually fails PKCS#1 padding @@ -5820,35 +5836,52 @@ static int test_rsa_verify_rejects_bad_signatures(void) * checks below keep the signature/padding well-formed and only vary * the caller-supplied hash, which is what actually exercises the * hash-binding check at psa_rsa.c's ConstantCompare()/check_padding() - * calls. */ + * calls. + * + * Each sub-case can skip independently (no RSA key generation, or a + * build without WC_RSA_PSS), so count what actually ran: reporting + * TEST_OK when every case skipped would silently disable exactly the + * regression this test exists to catch. */ result = test_hash_verify_rejects_substituted_hash( - PSA_KEY_TYPE_RSA_KEY_PAIR, 1024, + PSA_KEY_TYPE_RSA_KEY_PAIR, 2048, PSA_ALG_RSA_PKCS1V15_SIGN(PSA_ALG_SHA_256), - hash_a, hash_b, sizeof(hash_a), 128u, + hash_a, hash_b, sizeof(hash_a), 256u, + PSA_ERROR_INVALID_SIGNATURE, "psa_generate_key(RSA PKCS1v15 substituted hash)"); if (result == TEST_FAIL) { return TEST_FAIL; } + if (result == TEST_OK) { + ran++; + } result = test_hash_verify_rejects_substituted_hash( - PSA_KEY_TYPE_RSA_KEY_PAIR, 1024, + PSA_KEY_TYPE_RSA_KEY_PAIR, 2048, PSA_ALG_RSA_PKCS1V15_SIGN_RAW, - hash_a, hash_b, sizeof(hash_a), 128u, + hash_a, hash_b, sizeof(hash_a), 256u, + PSA_ERROR_INVALID_SIGNATURE, "psa_generate_key(RSA PKCS1v15 raw substituted hash)"); if (result == TEST_FAIL) { return TEST_FAIL; } + if (result == TEST_OK) { + ran++; + } result = test_hash_verify_rejects_substituted_hash( - PSA_KEY_TYPE_RSA_KEY_PAIR, 1024, + PSA_KEY_TYPE_RSA_KEY_PAIR, 2048, PSA_ALG_RSA_PSS(PSA_ALG_SHA_256), - hash_a, hash_b, sizeof(hash_a), 128u, + hash_a, hash_b, sizeof(hash_a), 256u, + PSA_ERROR_INVALID_PADDING, "psa_generate_key(RSA PSS substituted hash)"); if (result == TEST_FAIL) { return TEST_FAIL; } + if (result == TEST_OK) { + ran++; + } - return TEST_OK; + return (ran > 0) ? TEST_OK : TEST_SKIPPED; } static int test_asym_requires_verify_usage(void) @@ -7283,6 +7316,7 @@ static int test_kdf_verify_key_policy(void) } op = psa_key_derivation_operation_init(); + psa_reset_key_attributes(&attrs); psa_set_key_type(&attrs, PSA_KEY_TYPE_RAW_DATA); psa_set_key_usage_flags(&attrs, PSA_KEY_USAGE_VERIFY_DERIVATION); st = psa_import_key(&attrs, hkdf_bad_expected, sizeof(hkdf_bad_expected), &hkdf_mismatch_key); @@ -9017,9 +9051,9 @@ int main(int argc, char** argv) return TEST_FAIL; } } - if (only == NULL || strcmp(only, "rsa_verify_bad_signature") == 0) { - if (run_named_test("rsa_verify_bad_signature", - test_rsa_verify_rejects_bad_signatures) == TEST_FAIL) { + if (only == NULL || strcmp(only, "rsa_verify_substituted_hash") == 0) { + if (run_named_test("rsa_verify_substituted_hash", + test_rsa_verify_rejects_substituted_hashes) == TEST_FAIL) { return TEST_FAIL; } } From d6ba2872e9a467a83697a41e2c254ced2db26a59 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 6 Aug 2026 12:39:28 +0200 Subject: [PATCH 9/9] Bind RSA PKCS#1 v1.5 hash_length to the algorithm's digest length Four review findings, all in the RSA sign/verify pair. hash_length was bounded only by WC_MAX_DIGEST_SIZE, which is the buffer precondition but not the semantic one. PSA requires the digest to be exactly PSA_HASH_LENGTH(PSA_ALG_SIGN_GET_HASH(alg)). Without that check wc_EncodeSignature() wraps a 20-byte digest in a DigestInfo carrying the SHA-256 OID, and because the verify path re-encodes the same way, the malformed structure round-tripped: sign(20-byte hash under SHA-256) -> 0 verify(same 20-byte hash) -> 0 which violates RFC 8017 A.2.4. Previously only the sign side was lax and hashed verify always failed, so this became observable only once verify started working. Both guards now check the exact length and keep the WC_MAX_DIGEST_SIZE term as the buffer backstop, since the two constants can diverge. Confirmed the same input now returns INVALID_ARGUMENT on both entry points while the correct 32-byte length still round-trips. RSA-PSS reported a hash mismatch as PSA_ERROR_INVALID_PADDING, which psa_verify_hash does not define; the spec requires INVALID_SIGNATURE for any signature that does not verify. The previous commit's test pinned the non-conformant status as expected behaviour, which would have left the psa-arch-tests job as the only thing able to catch it. Normalize the padding-check failure to SIG_VERIFY_E and assert INVALID_SIGNATURE for all three sub-cases. WOLFPSA_RSA_DIGESTINFO_MAX used a hand-picked +32 header margin, below wolfCrypt's own declared worst case. It is safe for every OID currently encodable (real overhead is 19-21 bytes) but rests on inspection of the OID table rather than on a published bound - the same class of accidental margin the guard was added to remove. Use MAX_DER_DIGEST_ASN_SZ, taking the buffer from 96 to 100 bytes. The sign path still assigned wc_EncodeSignature()'s return straight into sig_input_len while verify checked it, so an allocation failure surfaced as INVALID_ARGUMENT via wc_RsaSSL_Sign(sig_input, 0, ...) on one side and INSUFFICIENT_MEMORY on the other. Mirror the verify-side check. Extend test_rsa_pkcs1v15_rejects_oversized_hash_length with the under-sized case on both entry points. --- src/psa_rsa.c | 74 ++++++++++++++++++++++++++-------- test/psa_server/psa_api_test.c | 37 ++++++++++++++--- 2 files changed, 88 insertions(+), 23 deletions(-) diff --git a/src/psa_rsa.c b/src/psa_rsa.c index b7bf893..bfa8837 100644 --- a/src/psa_rsa.c +++ b/src/psa_rsa.c @@ -50,14 +50,21 @@ int wc_psa_get_hash_type(psa_algorithm_t alg); /* Scratch size for the PKCS#1 v1.5 DigestInfo that wc_EncodeSignature() * builds: the digest itself plus the DER header prepended to it. * - * Bounded by WC_MAX_DIGEST_SIZE rather than PSA_HASH_MAX_SIZE because - * wc_GetCTC_HashOID() reflects what wolfCrypt was compiled with, while - * PSA_HASH_MAX_SIZE is derived from the PSA_WANT_ALG_SHA* set. The two can - * diverge - a Zephyr build selecting only CONFIG_PSA_WANT_ALG_SHA_256 gets - * PSA_HASH_MAX_SIZE 32 while wolfCrypt still returns a valid OID for - * SHA-512 - and sizing from the PSA constant would reject digests the - * implementation can legitimately encode. */ -#define WOLFPSA_RSA_DIGESTINFO_MAX (WC_MAX_DIGEST_SIZE + 32) + * The digest term is WC_MAX_DIGEST_SIZE rather than PSA_HASH_MAX_SIZE + * because wc_GetCTC_HashOID() reflects what wolfCrypt was compiled with, + * while PSA_HASH_MAX_SIZE is derived from the PSA_WANT_ALG_SHA* set. The + * two can diverge - a Zephyr build selecting only + * CONFIG_PSA_WANT_ALG_SHA_256 gets PSA_HASH_MAX_SIZE 32 while wolfCrypt + * still returns a valid OID for SHA-512 - and sizing from the PSA constant + * would reject digests the implementation can legitimately encode. + * + * The header term is wolfCrypt's own worst case for the SEQUENCE + + * AlgorithmIdentifier + OCTET STRING header, so the bound tracks the + * library instead of resting on the overhead the current OID table happens + * to produce. wc_EncodeSignature() sizes with SizeASN_Items() and then + * writes via SetASN_Items() with no bound of its own, so this buffer is + * the only limit on that write. */ +#define WOLFPSA_RSA_DIGESTINFO_MAX (WC_MAX_DIGEST_SIZE + MAX_DER_DIGEST_ASN_SZ) /* Only the PSS and OAEP paths use MGF1; guard the helper with the same * condition as its callers so a config with neither (e.g. RSA sign/verify with @@ -129,6 +136,7 @@ psa_status_t psa_asymmetric_sign_rsa(psa_key_type_t key_type, int padding; int hash_type; int hash_oid; + size_t expected_hash_len; byte* sig_input = NULL; word32 sig_input_len = 0; word32 sig_input_alloc_len = 0; @@ -186,10 +194,18 @@ psa_status_t psa_asymmetric_sign_rsa(psa_key_type_t key_type, wc_FreeRsaKey(&rsa_key); return PSA_ERROR_NOT_SUPPORTED; } - /* wc_EncodeSignature() writes the DER header plus hash_length - * bytes into sig_input without bounding the write itself, so - * reject a hash that cannot fit before allocating. */ - if (hash_length > WC_MAX_DIGEST_SIZE) { + /* PSA requires the digest to be exactly as long as the hash + * named by the algorithm; anything else is INVALID_ARGUMENT. + * wc_EncodeSignature() would otherwise happily wrap a 20-byte + * digest in a SHA-256 DigestInfo, and since the verify path + * re-encodes the same way that malformed structure would + * round-trip to success in violation of RFC 8017 A.2.4. The + * WC_MAX_DIGEST_SIZE term is retained as the buffer + * precondition: wc_EncodeSignature() does not bound its own + * write, and the two constants can diverge. */ + expected_hash_len = PSA_HASH_LENGTH(PSA_ALG_SIGN_GET_HASH(alg)); + if (expected_hash_len == 0 || hash_length != expected_hash_len || + hash_length > WC_MAX_DIGEST_SIZE) { wc_FreeRng(&rng); wc_FreeRsaKey(&rsa_key); return PSA_ERROR_INVALID_ARGUMENT; @@ -204,6 +220,16 @@ psa_status_t psa_asymmetric_sign_rsa(psa_key_type_t key_type, sig_input_alloc_len = WOLFPSA_RSA_DIGESTINFO_MAX; sig_input_len = wc_EncodeSignature(sig_input, hash, (word32)hash_length, hash_oid); + /* Unsigned return type, so 0 is the only error indication; it + * means the internal allocation failed, not a bad argument. + * Mirrors the verify path so both report the same status. */ + if (sig_input_len == 0) { + wc_ForceZero(sig_input, sig_input_alloc_len); + XFREE(sig_input, NULL, DYNAMIC_TYPE_TMP_BUFFER); + wc_FreeRng(&rng); + wc_FreeRsaKey(&rsa_key); + return PSA_ERROR_INSUFFICIENT_MEMORY; + } } ret = wc_RsaSSL_Sign(sig_input, sig_input_len, signature, @@ -254,7 +280,8 @@ psa_status_t psa_asymmetric_verify_rsa(psa_key_type_t key_type, word32 idx = 0; int padding; int hash_type; - + size_t expected_hash_len; + (void)key_bits; /* Check if key type is RSA public key or key pair */ @@ -332,10 +359,15 @@ psa_status_t psa_asymmetric_verify_rsa(psa_key_type_t key_type, wc_FreeRsaKey(&rsa_key); return PSA_ERROR_NOT_SUPPORTED; } - /* wc_EncodeSignature() writes the DER header plus hash_length - * bytes with no bound of its own, and hash_length is caller - * supplied, so it must be range-checked before encoding. */ - if (hash_length > WC_MAX_DIGEST_SIZE) { + /* Same bound as the sign path: PSA requires the digest to match + * the length of the hash named by the algorithm, and the + * WC_MAX_DIGEST_SIZE term is the buffer precondition for the + * unbounded write wc_EncodeSignature() performs. Accepting a + * mismatched length here would let a DigestInfo whose OID and + * digest length disagree verify successfully. */ + expected_hash_len = PSA_HASH_LENGTH(PSA_ALG_SIGN_GET_HASH(alg)); + if (expected_hash_len == 0 || hash_length != expected_hash_len || + hash_length > WC_MAX_DIGEST_SIZE) { wc_ForceZero(decoded, sizeof(decoded)); wc_FreeRsaKey(&rsa_key); return PSA_ERROR_INVALID_ARGUMENT; @@ -399,6 +431,14 @@ psa_status_t psa_asymmetric_verify_rsa(psa_key_type_t key_type, if (ret == 0) { ret = (int)hash_length; } + else { + /* The padding check is the PSS signature check, so a failure + * here is a signature mismatch. Reporting wolfCrypt's + * BAD_PADDING_E verbatim would surface as + * PSA_ERROR_INVALID_PADDING, which psa_verify_hash does not + * define; the spec requires PSA_ERROR_INVALID_SIGNATURE. */ + ret = SIG_VERIFY_E; + } } wc_ForceZero(decoded, sizeof(decoded)); #else diff --git a/test/psa_server/psa_api_test.c b/test/psa_server/psa_api_test.c index 53b071d..70a0bc5 100644 --- a/test/psa_server/psa_api_test.c +++ b/test/psa_server/psa_api_test.c @@ -5712,11 +5712,10 @@ static int test_hash_verify_rejects_substituted_hash(psa_key_type_t type, } /* A genuine signature over hash_a must not verify against a different - * hash_b. The exact status is deterministic per algorithm, so pin it: - * PKCS#1 v1.5 rejection surfaces as PSA_ERROR_INVALID_SIGNATURE, while - * RSA-PSS's padding check reports the mismatch as - * PSA_ERROR_INVALID_PADDING. Accepting any non-success status here would - * also accept a regression that broke psa_verify_hash outright. */ + * hash_b. All three algorithms must report PSA_ERROR_INVALID_SIGNATURE, + * the only status the PSA spec defines for a signature that does not + * verify. Accepting any non-success status here would also accept a + * regression that broke psa_verify_hash outright. */ st = psa_verify_hash(key_id, alg, hash_b, hash_len, sig, sig_len); if (check_true(st == expected_st, "psa_verify_hash rejects signature over substituted hash") != TEST_OK) { @@ -5805,6 +5804,32 @@ static int test_rsa_pkcs1v15_rejects_oversized_hash_length(void) goto cleanup; } + /* Under-sized digest: a 20-byte hash under SHA-256 fits every buffer, so + * only the exact-length check rejects it. Without that check + * wc_EncodeSignature() wraps it in a DigestInfo carrying the SHA-256 OID + * around a 20-byte OCTET STRING - malformed per RFC 8017 A.2.4 - and + * because verify re-encodes the same way, the pair round-trips to + * PSA_SUCCESS instead of being refused. */ + st = psa_sign_hash(key_id, alg, big_hash, 20u, + sig, sizeof(sig), &sig_len); + if (check_true(st == PSA_ERROR_INVALID_ARGUMENT, + "psa_sign_hash rejects undersized hash_length") != TEST_OK) { + goto cleanup; + } + + st = psa_sign_hash(key_id, alg, big_hash, WC_SHA256_DIGEST_SIZE, + sig, sizeof(sig), &sig_len); + if (check_status(st, "psa_sign_hash(valid hash for undersized guard)") + != TEST_OK) { + goto cleanup; + } + st = psa_verify_hash(key_id, alg, big_hash, 20u, sig, sig_len); + if (check_true(st == PSA_ERROR_INVALID_ARGUMENT, + "psa_verify_hash rejects undersized hash_length") + != TEST_OK) { + goto cleanup; + } + result = TEST_OK; cleanup: @@ -5872,7 +5897,7 @@ static int test_rsa_verify_rejects_substituted_hashes(void) PSA_KEY_TYPE_RSA_KEY_PAIR, 2048, PSA_ALG_RSA_PSS(PSA_ALG_SHA_256), hash_a, hash_b, sizeof(hash_a), 256u, - PSA_ERROR_INVALID_PADDING, + PSA_ERROR_INVALID_SIGNATURE, "psa_generate_key(RSA PSS substituted hash)"); if (result == TEST_FAIL) { return TEST_FAIL;