diff --git a/src/arm_tee_psa_ipc.c b/src/arm_tee_psa_ipc.c index aaf75267..f6244585 100644 --- a/src/arm_tee_psa_ipc.c +++ b/src/arm_tee_psa_ipc.c @@ -395,7 +395,8 @@ static psa_status_t wolfboot_crypto_dispatch(const psa_invec *in_vec, return psa_generate_random((uint8_t *)out_vec[0].base, out_vec[0].len); case ARM_TEE_CRYPTO_OPEN_KEY_SID: - if (out_vec == NULL || out_len < 1) { + if (out_vec == NULL || out_len < 1 || out_vec[0].base == NULL || + out_vec[0].len < sizeof(psa_key_id_t)) { return PSA_ERROR_INVALID_ARGUMENT; } return wolfboot_psa_open_key(iov->key_id, @@ -405,7 +406,9 @@ static psa_status_t wolfboot_crypto_dispatch(const psa_invec *in_vec, return wolfboot_psa_close_key(iov->key_id); case ARM_TEE_CRYPTO_IMPORT_KEY_SID: - if (in_len < 3 || out_vec == NULL || out_len < 1) { + if (in_len < 3 || out_vec == NULL || out_len < 1 || + out_vec[0].base == NULL || + out_vec[0].len < sizeof(psa_key_id_t)) { return PSA_ERROR_INVALID_ARGUMENT; } if (in_vec[1].base == NULL || @@ -425,7 +428,9 @@ static psa_status_t wolfboot_crypto_dispatch(const psa_invec *in_vec, } case ARM_TEE_CRYPTO_GENERATE_KEY_SID: - if (in_len < 2 || out_vec == NULL || out_len < 1) { + if (in_len < 2 || out_vec == NULL || out_len < 1 || + out_vec[0].base == NULL || + out_vec[0].len < sizeof(psa_key_id_t)) { return PSA_ERROR_INVALID_ARGUMENT; } if (in_vec[1].base == NULL || @@ -478,7 +483,8 @@ static psa_status_t wolfboot_crypto_dispatch(const psa_invec *in_vec, } case ARM_TEE_CRYPTO_GET_KEY_ATTRIBUTES_SID: - if (out_vec == NULL || out_len < 1) { + if (out_vec == NULL || out_len < 1 || out_vec[0].base == NULL || + out_vec[0].len < sizeof(psa_key_attributes_t)) { return PSA_ERROR_INVALID_ARGUMENT; } return psa_get_key_attributes(iov->key_id, @@ -1006,11 +1012,13 @@ static int32_t arm_tee_psa_ps_dispatch(int32_t type, const psa_invec *in_vec, return PSA_SUCCESS; } if (type == ARM_TEE_PS_GET_SUPPORT) { - if (out_vec != NULL && out_len >= 1 && out_vec[0].base != NULL) { - uint32_t support = 0; - XMEMCPY(out_vec[0].base, &support, sizeof(support)); - out_vec[0].len = sizeof(support); + uint32_t support = 0; + if (out_vec == NULL || out_len < 1 || out_vec[0].base == NULL || + out_vec[0].len < sizeof(support)) { + return PSA_ERROR_INVALID_ARGUMENT; } + XMEMCPY(out_vec[0].base, &support, sizeof(support)); + out_vec[0].len = sizeof(support); return PSA_SUCCESS; } return PSA_ERROR_NOT_SUPPORTED; @@ -1067,13 +1075,19 @@ int32_t arm_tee_psa_call(psa_handle_t handle, int32_t type, out_vec_s[i] = out_vec[i]; } + /* Every non-NULL .base must pass the non-secure attribution check, even + * when the declared .len is zero: a descriptor is not guaranteed to be + * accessed only within .len, so a zero-length descriptor would otherwise + * smuggle a Secure pointer past validation. At least one byte is always + * checked. */ for (i = 0; i < in_len; i++) { if (in_vec_s[i].len > 0 && in_vec_s[i].base == NULL) { return PSA_ERROR_INVALID_ARGUMENT; } - if (in_vec_s[i].len > 0 && + if (in_vec_s[i].base != NULL && cmse_check_address_range((void *)in_vec_s[i].base, - in_vec_s[i].len, + in_vec_s[i].len > 0 ? + in_vec_s[i].len : 1, CMSE_NONSECURE) == NULL) { return PSA_ERROR_INVALID_ARGUMENT; } @@ -1082,9 +1096,10 @@ int32_t arm_tee_psa_call(psa_handle_t handle, int32_t type, if (out_vec_s[i].len > 0 && out_vec_s[i].base == NULL) { return PSA_ERROR_INVALID_ARGUMENT; } - if (out_vec_s[i].len > 0 && + if (out_vec_s[i].base != NULL && cmse_check_address_range(out_vec_s[i].base, - out_vec_s[i].len, + out_vec_s[i].len > 0 ? + out_vec_s[i].len : 1, CMSE_NONSECURE) == NULL) { return PSA_ERROR_INVALID_ARGUMENT; } diff --git a/tools/unit-tests/arm_cmse.h b/tools/unit-tests/arm_cmse.h index 7423acb4..1fe8446c 100644 --- a/tools/unit-tests/arm_cmse.h +++ b/tools/unit-tests/arm_cmse.h @@ -1,10 +1,13 @@ #ifndef UNIT_TEST_ARM_CMSE_H #define UNIT_TEST_ARM_CMSE_H +#include #include #define CMSE_NONSECURE 0 -#define cmse_check_address_range(ptr, size, flags) \ - ((void *)(uintptr_t)(ptr)) + +/* Provided by the unit test, so it can model a Secure region that must never + * pass a non-secure attribution check. */ +void *cmse_check_address_range(void *ptr, size_t size, int flags); #endif diff --git a/tools/unit-tests/unit-arm-tee-psa-ipc.c b/tools/unit-tests/unit-arm-tee-psa-ipc.c index d0e560a1..8b4fd04b 100644 --- a/tools/unit-tests/unit-arm-tee-psa-ipc.c +++ b/tools/unit-tests/unit-arm-tee-psa-ipc.c @@ -17,8 +17,99 @@ void wc_ForceZero(void *mem, size_t len) ForceZero(mem, len); } +/* Simulated Secure SRAM: any pointer landing in here is rejected by the CMSE + * stub below, exactly like a real Secure address fails CMSE_NONSECURE. */ +static uint8_t secure_mem[64]; + +void *cmse_check_address_range(void *ptr, size_t size, int flags) +{ + uint8_t *start = (uint8_t *)ptr; + uint8_t *end; + + (void)flags; + if (size == 0) { + size = 1; + } + end = start + size; + if (end > secure_mem && start < secure_mem + sizeof(secure_mem)) { + return NULL; + } + return ptr; +} + #include "../../src/arm_tee_psa_ipc.c" +/* Backend stubs: the tests below only exercise the IPC argument validation, + * never the crypto/attestation back ends. */ +psa_status_t psa_crypto_init(void) { return PSA_SUCCESS; } +psa_status_t psa_generate_random(uint8_t *o, size_t s) +{ (void)o; (void)s; return PSA_ERROR_NOT_SUPPORTED; } +psa_status_t psa_get_key_attributes(psa_key_id_t k, psa_key_attributes_t *a) +{ (void)k; (void)a; return PSA_ERROR_NOT_SUPPORTED; } +void psa_reset_key_attributes(psa_key_attributes_t *a) { (void)a; } +psa_status_t psa_destroy_key(psa_key_id_t k) +{ (void)k; return PSA_ERROR_NOT_SUPPORTED; } +psa_status_t psa_import_key(const psa_key_attributes_t *a, const uint8_t *d, + size_t dl, psa_key_id_t *k) +{ (void)a; (void)d; (void)dl; (void)k; return PSA_ERROR_NOT_SUPPORTED; } +psa_status_t psa_generate_key(const psa_key_attributes_t *a, psa_key_id_t *k) +{ (void)a; (void)k; return PSA_ERROR_NOT_SUPPORTED; } +psa_status_t psa_export_key(psa_key_id_t k, uint8_t *d, size_t ds, size_t *dl) +{ (void)k; (void)d; (void)ds; (void)dl; return PSA_ERROR_NOT_SUPPORTED; } +psa_status_t psa_export_public_key(psa_key_id_t k, uint8_t *d, size_t ds, + size_t *dl) +{ (void)k; (void)d; (void)ds; (void)dl; return PSA_ERROR_NOT_SUPPORTED; } +psa_status_t psa_hash_compute(psa_algorithm_t alg, const uint8_t *i, size_t il, + uint8_t *h, size_t hs, size_t *hl) +{ (void)alg; (void)i; (void)il; (void)h; (void)hs; (void)hl; + return PSA_ERROR_NOT_SUPPORTED; } +psa_status_t psa_hash_setup(psa_hash_operation_t *op, psa_algorithm_t alg) +{ (void)op; (void)alg; return PSA_ERROR_NOT_SUPPORTED; } +psa_status_t psa_hash_update(psa_hash_operation_t *op, const uint8_t *i, + size_t il) +{ (void)op; (void)i; (void)il; return PSA_ERROR_NOT_SUPPORTED; } +psa_status_t psa_hash_finish(psa_hash_operation_t *op, uint8_t *h, size_t hs, + size_t *hl) +{ (void)op; (void)h; (void)hs; (void)hl; return PSA_ERROR_NOT_SUPPORTED; } +psa_status_t psa_hash_clone(const psa_hash_operation_t *s, + psa_hash_operation_t *t) +{ (void)s; (void)t; return PSA_ERROR_NOT_SUPPORTED; } +psa_status_t psa_hash_abort(psa_hash_operation_t *op) +{ (void)op; return PSA_ERROR_NOT_SUPPORTED; } +psa_status_t psa_cipher_encrypt_setup(psa_cipher_operation_t *op, + psa_key_id_t k, psa_algorithm_t alg) +{ (void)op; (void)k; (void)alg; return PSA_ERROR_NOT_SUPPORTED; } +psa_status_t psa_cipher_decrypt_setup(psa_cipher_operation_t *op, + psa_key_id_t k, psa_algorithm_t alg) +{ (void)op; (void)k; (void)alg; return PSA_ERROR_NOT_SUPPORTED; } +psa_status_t psa_cipher_set_iv(psa_cipher_operation_t *op, const uint8_t *iv, + size_t ivl) +{ (void)op; (void)iv; (void)ivl; return PSA_ERROR_NOT_SUPPORTED; } +psa_status_t psa_cipher_update(psa_cipher_operation_t *op, const uint8_t *i, + size_t il, uint8_t *o, size_t os, size_t *ol) +{ (void)op; (void)i; (void)il; (void)o; (void)os; (void)ol; + return PSA_ERROR_NOT_SUPPORTED; } +psa_status_t psa_cipher_finish(psa_cipher_operation_t *op, uint8_t *o, + size_t os, size_t *ol) +{ (void)op; (void)o; (void)os; (void)ol; return PSA_ERROR_NOT_SUPPORTED; } +psa_status_t psa_cipher_abort(psa_cipher_operation_t *op) +{ (void)op; return PSA_ERROR_NOT_SUPPORTED; } +psa_status_t psa_sign_hash(psa_key_id_t k, psa_algorithm_t alg, + const uint8_t *h, size_t hl, uint8_t *s, size_t ss, size_t *sl) +{ (void)k; (void)alg; (void)h; (void)hl; (void)s; (void)ss; (void)sl; + return PSA_ERROR_NOT_SUPPORTED; } +psa_status_t psa_verify_hash(psa_key_id_t k, psa_algorithm_t alg, + const uint8_t *h, size_t hl, const uint8_t *s, size_t sl) +{ (void)k; (void)alg; (void)h; (void)hl; (void)s; (void)sl; + return PSA_ERROR_NOT_SUPPORTED; } +int wolfBoot_dice_get_token(const uint8_t *c, size_t cs, uint8_t *t, size_t ts, + size_t *tl) +{ (void)c; (void)cs; (void)t; (void)ts; (void)tl; return -1; } +int wolfBoot_dice_get_token_size(size_t cs, size_t *ts) +{ (void)cs; (void)ts; return -1; } +int wolfBoot_dice_get_attest_pubkey(uint8_t *b, size_t *l) +{ (void)b; (void)l; return -1; } + static void reset_ps_state(void) { memset(g_ps_entries, 0, sizeof(g_ps_entries)); @@ -164,6 +255,68 @@ START_TEST(test_ps_set_get_info_remove_success_path) } END_TEST +START_TEST(test_psa_call_rejects_secure_zero_len_outvec) +{ + psa_outvec out_vec[1]; + size_t i; + + reset_ps_state(); + memset(secure_mem, 0xA5, sizeof(secure_mem)); + + /* A zero-length descriptor pointing at Secure memory must not pass + * validation: ARM_TEE_PS_GET_SUPPORT writes through .base regardless of + * the declared length. */ + out_vec[0].base = secure_mem; + out_vec[0].len = 0; + + ck_assert_int_eq( + arm_tee_psa_call((psa_handle_t)ARM_TEE_PROTECTED_STORAGE_HANDLE, + ARM_TEE_PS_GET_SUPPORT, NULL, 0, out_vec, 1), + PSA_ERROR_INVALID_ARGUMENT); + + for (i = 0; i < sizeof(secure_mem); i++) { + ck_assert_uint_eq(secure_mem[i], 0xA5); + } +} +END_TEST + +START_TEST(test_ps_get_support_rejects_short_outvec) +{ + uint8_t buf[sizeof(uint32_t)]; + psa_outvec out_vec[1]; + + reset_ps_state(); + memset(buf, 0xA5, sizeof(buf)); + + out_vec[0].base = buf; + out_vec[0].len = sizeof(uint32_t) - 1; + + ck_assert_int_eq( + arm_tee_psa_test_ps_dispatch(ARM_TEE_PS_GET_SUPPORT, NULL, 0, + out_vec, 1), + PSA_ERROR_INVALID_ARGUMENT); + ck_assert_uint_eq(buf[0], 0xA5); +} +END_TEST + +START_TEST(test_ps_get_support_success_path) +{ + uint32_t support = 0xFFFFFFFFU; + psa_outvec out_vec[1]; + + reset_ps_state(); + out_vec[0].base = &support; + out_vec[0].len = sizeof(support); + + ck_assert_int_eq( + arm_tee_psa_call((psa_handle_t)ARM_TEE_PROTECTED_STORAGE_HANDLE, + ARM_TEE_PS_GET_SUPPORT, NULL, 0, out_vec, 1), + PSA_SUCCESS); + ck_assert_uint_eq(support, 0); + ck_assert_uint_eq(out_vec[0].len, sizeof(support)); +} +END_TEST + Suite *arm_tee_psa_ipc_suite(void) { Suite *s = suite_create("arm-tee-psa-ipc"); @@ -174,6 +327,9 @@ Suite *arm_tee_psa_ipc_suite(void) tcase_add_test(tc, test_ps_get_info_rejects_short_uid_vector); tcase_add_test(tc, test_ps_remove_rejects_short_uid_vector); tcase_add_test(tc, test_ps_set_get_info_remove_success_path); + tcase_add_test(tc, test_psa_call_rejects_secure_zero_len_outvec); + tcase_add_test(tc, test_ps_get_support_rejects_short_outvec); + tcase_add_test(tc, test_ps_get_support_success_path); suite_add_tcase(s, tc); return s;