diff --git a/src/pkcs11_store.c b/src/pkcs11_store.c index c7135d5d..8351d6cd 100644 --- a/src/pkcs11_store.c +++ b/src/pkcs11_store.c @@ -571,6 +571,9 @@ int wolfPKCS11_Store_Remove(int type, CK_ULONG id1, CK_ULONG id2) if (buf == NULL) return NOT_AVAILABLE_E; + /* Erase the payload before invalidating the metadata, so key + * material does not remain recoverable in flash after removal. */ + erase_object_payload(buf); delete_object((int32_t)type, (uint32_t)id1, (uint32_t)id2); return 0; } diff --git a/src/psa_store.c b/src/psa_store.c index f540deb8..8f95e965 100644 --- a/src/psa_store.c +++ b/src/psa_store.c @@ -581,6 +581,9 @@ int wolfPSA_Store_Remove(int type, unsigned long id1, unsigned long id2) if (buf == NULL) return NOT_AVAILABLE_E; + /* Erase the payload before invalidating the metadata, so key + * material does not remain recoverable in flash after removal. */ + erase_object_payload(buf); delete_object((int32_t)type, (uint32_t)id1, (uint32_t)id2); return 0; } diff --git a/tools/unit-tests/unit-pkcs11_store.c b/tools/unit-tests/unit-pkcs11_store.c index f556eecc..bf69221e 100644 --- a/tools/unit-tests/unit-pkcs11_store.c +++ b/tools/unit-tests/unit-pkcs11_store.c @@ -550,6 +550,89 @@ START_TEST(test_store_rejects_negative_len) } END_TEST +/* Removing an object must erase the payload from flash, not just + * invalidate the metadata: key material must not remain recoverable + * after the API reports a successful deletion. */ +START_TEST(test_remove_erases_payload_from_flash) +{ + const int type = DYNAMIC_TYPE_RSA; + const CK_ULONG id_tok_a = 10; + const CK_ULONG id_obj_a = 20; + const CK_ULONG id_tok_b = 30; + const CK_ULONG id_obj_b = 40; + void *store = NULL; + unsigned char key_a[256]; + unsigned char key_b[128]; + uint8_t *buf_a; + uint8_t *buf_b; + uint32_t i; + int ret; + + memset(key_a, 0xAB, sizeof(key_a)); + memset(key_b, 0xCD, sizeof(key_b)); + + ret = mmap_file(vault_path, vault_base, keyvault_size, NULL); + ck_assert_int_eq(ret, 0); + memset(vault_base, 0xEE, keyvault_size); + + /* Two live objects: A gets slot 0, B gets the adjacent slot 1 */ + ret = wolfPKCS11_Store_Open(type, id_tok_a, id_obj_a, 0, &store); + ck_assert_int_eq(ret, 0); + ret = wolfPKCS11_Store_Write(store, key_a, sizeof(key_a)); + ck_assert_int_eq(ret, (int)sizeof(key_a)); + wolfPKCS11_Store_Close(store); + + ret = wolfPKCS11_Store_Open(type, id_tok_b, id_obj_b, 0, &store); + ck_assert_int_eq(ret, 0); + ret = wolfPKCS11_Store_Write(store, key_b, sizeof(key_b)); + ck_assert_int_eq(ret, (int)sizeof(key_b)); + wolfPKCS11_Store_Close(store); + + buf_a = find_object_buffer(type, id_tok_a, id_obj_a); + buf_b = find_object_buffer(type, id_tok_b, id_obj_b); + ck_assert_ptr_nonnull(buf_a); + ck_assert_ptr_nonnull(buf_b); + ck_assert_ptr_eq(buf_a, vault_base + 2 * WOLFBOOT_SECTOR_SIZE); + ck_assert_ptr_eq(buf_b, vault_base + 2 * WOLFBOOT_SECTOR_SIZE + + KEYVAULT_OBJ_SIZE); + + /* Remove A: the payload must be erased from the raw flash */ + ret = wolfPKCS11_Store_Remove(type, id_tok_a, id_obj_a); + ck_assert_int_eq(ret, 0); + + /* The 8-byte id prefix is preserved by the erase (it keeps the slot + * identity used by the backup-recovery check); the payload region + * itself must be erased to 0xFF across the whole slot. */ + ck_assert_uint_eq(((uint32_t *)buf_a)[0], (uint32_t)id_tok_a); + ck_assert_uint_eq(((uint32_t *)buf_a)[1], (uint32_t)id_obj_a); + for (i = 2 * sizeof(uint32_t); i < KEYVAULT_OBJ_SIZE; i++) { + ck_assert_msg(buf_a[i] == 0xFF, + "Payload survives removal at slot offset %u: 0x%02x", + i, buf_a[i]); + } + + /* The sector read-modify-write must leave the neighboring object B + * intact in flash */ + for (i = 2 * sizeof(uint32_t); + i < 2 * sizeof(uint32_t) + sizeof(key_b); i++) { + ck_assert_msg(buf_b[i] == 0xCD, + "Neighbor object clobbered at slot offset %u: 0x%02x", + i, buf_b[i]); + } + ret = wolfPKCS11_Store_Open(type, id_tok_b, id_obj_b, 1, &store); + ck_assert_int_eq(ret, 0); + ret = wolfPKCS11_Store_Read(store, key_b, sizeof(key_b)); + ck_assert_int_eq(ret, (int)sizeof(key_b)); + wolfPKCS11_Store_Close(store); + + /* A is no longer addressable */ + ret = wolfPKCS11_Store_Open(type, id_tok_a, id_obj_a, 1, &store); + ck_assert_int_eq(ret, NOT_AVAILABLE_E); + ret = wolfPKCS11_Store_Remove(type, id_tok_a, id_obj_a); + ck_assert_int_eq(ret, NOT_AVAILABLE_E); +} +END_TEST + Suite *wolfboot_suite(void) { /* Suite initialization */ @@ -563,6 +646,7 @@ Suite *wolfboot_suite(void) TCase* tcase_find_bounds = tcase_create("find_bounds"); TCase* tcase_remanence = tcase_create("shorter_overwrite_erases_residual"); TCase* tcase_neg_len = tcase_create("rejects_negative_len"); + TCase* tcase_remove_erase = tcase_create("remove_erases_payload"); tcase_add_test(tcase_store_and_load_objs, test_store_and_load_objs); tcase_add_test(tcase_cross_sector_write, test_cross_sector_write_preserves_length); tcase_add_test(tcase_close, test_close_clears_handle_state); @@ -571,6 +655,7 @@ Suite *wolfboot_suite(void) tcase_add_test(tcase_find_bounds, test_find_object_search_stops_at_header_sector); tcase_add_test(tcase_remanence, test_shorter_overwrite_erases_residual_key_material); tcase_add_test(tcase_neg_len, test_store_rejects_negative_len); + tcase_add_test(tcase_remove_erase, test_remove_erases_payload_from_flash); suite_add_tcase(s, tcase_store_and_load_objs); suite_add_tcase(s, tcase_cross_sector_write); suite_add_tcase(s, tcase_close); @@ -579,6 +664,7 @@ Suite *wolfboot_suite(void) suite_add_tcase(s, tcase_find_bounds); suite_add_tcase(s, tcase_remanence); suite_add_tcase(s, tcase_neg_len); + suite_add_tcase(s, tcase_remove_erase); return s; } diff --git a/tools/unit-tests/unit-psa_store.c b/tools/unit-tests/unit-psa_store.c index 7c04b37e..08235682 100644 --- a/tools/unit-tests/unit-psa_store.c +++ b/tools/unit-tests/unit-psa_store.c @@ -337,6 +337,90 @@ START_TEST(test_store_rejects_negative_len) } END_TEST +/* Removing an object must erase the payload from flash, not just + * invalidate the metadata: key material must not remain recoverable + * after the API reports a successful deletion. */ +START_TEST(test_remove_erases_payload_from_flash) +{ + const int type = WOLFPSA_STORE_KEY; + const unsigned long id1_a = 10; + const unsigned long id2_a = 20; + const unsigned long id1_b = 30; + const unsigned long id2_b = 40; + void *store = NULL; + unsigned char key_a[256]; + unsigned char key_b[128]; + uint8_t *buf_a; + uint8_t *buf_b; + uint32_t i; + int ret; + + memset(key_a, 0xAB, sizeof(key_a)); + memset(key_b, 0xCD, sizeof(key_b)); + + ret = mmap_file("/tmp/wolfboot-unit-psa-keyvault.bin", vault_base, + keyvault_size, NULL); + ck_assert_int_eq(ret, 0); + memset(vault_base, 0xEE, keyvault_size); + + /* Two live objects: A gets slot 0, B gets the adjacent slot 1 */ + ret = wolfPSA_Store_Open(type, id1_a, id2_a, 0, &store); + ck_assert_int_eq(ret, 0); + ret = wolfPSA_Store_Write(store, key_a, sizeof(key_a)); + ck_assert_int_eq(ret, (int)sizeof(key_a)); + wolfPSA_Store_Close(store); + + ret = wolfPSA_Store_Open(type, id1_b, id2_b, 0, &store); + ck_assert_int_eq(ret, 0); + ret = wolfPSA_Store_Write(store, key_b, sizeof(key_b)); + ck_assert_int_eq(ret, (int)sizeof(key_b)); + wolfPSA_Store_Close(store); + + buf_a = find_object_buffer(type, id1_a, id2_a); + buf_b = find_object_buffer(type, id1_b, id2_b); + ck_assert_ptr_nonnull(buf_a); + ck_assert_ptr_nonnull(buf_b); + ck_assert_ptr_eq(buf_a, vault_base + 2 * WOLFBOOT_SECTOR_SIZE); + ck_assert_ptr_eq(buf_b, vault_base + 2 * WOLFBOOT_SECTOR_SIZE + + KEYVAULT_OBJ_SIZE); + + /* Remove A: the payload must be erased from the raw flash */ + ret = wolfPSA_Store_Remove(type, id1_a, id2_a); + ck_assert_int_eq(ret, 0); + + /* The 8-byte id prefix is preserved by the erase (it keeps the slot + * identity used by the backup-recovery check); the payload region + * itself must be erased to 0xFF across the whole slot. */ + ck_assert_uint_eq(((uint32_t *)buf_a)[0], (uint32_t)id1_a); + ck_assert_uint_eq(((uint32_t *)buf_a)[1], (uint32_t)id2_a); + for (i = 2 * sizeof(uint32_t); i < KEYVAULT_OBJ_SIZE; i++) { + ck_assert_msg(buf_a[i] == 0xFF, + "Payload survives removal at slot offset %u: 0x%02x", + i, buf_a[i]); + } + + /* The sector read-modify-write must leave the neighboring object B + * intact in flash */ + for (i = 2 * sizeof(uint32_t); + i < 2 * sizeof(uint32_t) + sizeof(key_b); i++) { + ck_assert_msg(buf_b[i] == 0xCD, + "Neighbor object clobbered at slot offset %u: 0x%02x", + i, buf_b[i]); + } + ret = wolfPSA_Store_Open(type, id1_b, id2_b, 1, &store); + ck_assert_int_eq(ret, 0); + ret = wolfPSA_Store_Read(store, key_b, sizeof(key_b)); + ck_assert_int_eq(ret, (int)sizeof(key_b)); + wolfPSA_Store_Close(store); + + /* A is no longer addressable */ + ret = wolfPSA_Store_Open(type, id1_a, id2_a, 1, &store); + ck_assert_int_eq(ret, NOT_AVAILABLE_E); + ret = wolfPSA_Store_Remove(type, id1_a, id2_a); + ck_assert_int_eq(ret, NOT_AVAILABLE_E); +} +END_TEST + Suite *wolfboot_suite(void) { Suite *s = suite_create("wolfBoot-psa-store"); @@ -348,6 +432,7 @@ Suite *wolfboot_suite(void) TCase *tcase_tail = tcase_create("shorter_overwrite_clears_tail"); TCase *tcase_zeroize = tcase_create("cache_commit_zeroizes_cached_sector"); TCase *tcase_neg_len = tcase_create("rejects_negative_len"); + TCase *tcase_remove_erase = tcase_create("remove_erases_payload"); tcase_add_test(tcase_write, test_cross_sector_write_preserves_length); tcase_add_test(tcase_close, test_close_clears_handle_state); @@ -357,6 +442,7 @@ Suite *wolfboot_suite(void) tcase_add_test(tcase_tail, test_shorter_overwrite_clears_tail); tcase_add_test(tcase_zeroize, test_cache_commit_zeroizes_cached_sector); tcase_add_test(tcase_neg_len, test_store_rejects_negative_len); + tcase_add_test(tcase_remove_erase, test_remove_erases_payload_from_flash); suite_add_tcase(s, tcase_write); suite_add_tcase(s, tcase_close); suite_add_tcase(s, tcase_delete); @@ -365,6 +451,7 @@ Suite *wolfboot_suite(void) suite_add_tcase(s, tcase_tail); suite_add_tcase(s, tcase_zeroize); suite_add_tcase(s, tcase_neg_len); + suite_add_tcase(s, tcase_remove_erase); return s; }