diff --git a/src/wh_nvm.c b/src/wh_nvm.c index a9b67a1c8..f276ebb83 100644 --- a/src/wh_nvm.c +++ b/src/wh_nvm.c @@ -349,7 +349,8 @@ int wh_Nvm_DestroyObjectsChecked(whNvmContext* context, whNvmId list_count, for (i = 0; i < list_count; i++) { ret = wh_Nvm_CheckPolicy(context, WH_NVM_OP_DESTROY, id_list[i], NULL); - if (ret != WH_ERROR_OK) { + /* An absent id has no policy to enforce and is not an error */ + if ((ret != WH_ERROR_OK) && (ret != WH_ERROR_NOTFOUND)) { return ret; } } diff --git a/src/wh_nvm_flash.c b/src/wh_nvm_flash.c index c2f3bc11c..3447f3cb8 100644 --- a/src/wh_nvm_flash.c +++ b/src/wh_nvm_flash.c @@ -1145,6 +1145,7 @@ int wh_NvmFlash_DestroyObjects(void* c, whNvmId list_count, int entry = 0; int src_part = 0; int dest_part = 0; + int any_marked = 0; uint32_t dest_object = 0; uint32_t dest_data = 0; @@ -1173,10 +1174,18 @@ int wh_NvmFlash_DestroyObjects(void* c, whNvmId list_count, &entry); if ((ret == 0) && (entry >= 0)) { d->objects[entry].state.status = NF_STATUS_DATA_BAD; + any_marked = 1; } } while (entry >= 0); } + /* Nothing matched a non-empty list: replicating would just rewrite the + * partition unchanged, so skip the flash wear. A zero list_count is a + * compaction request and must still replicate. */ + if ((list_count > 0) && (any_marked == 0)) { + return WH_ERROR_OK; + } + /* Blank check the inactive partition and erase if not blank */ ret = nfPartition_BlankCheck(context, dest_part); if (ret == WH_ERROR_NOTBLANK) { diff --git a/src/wh_server_keystore.c b/src/wh_server_keystore.c index b08143c67..0b1d8e51d 100644 --- a/src/wh_server_keystore.c +++ b/src/wh_server_keystore.c @@ -1275,16 +1275,20 @@ int wh_Server_KeystoreEraseKeyChecked(whServerContext* server, whNvmId keyId) } #endif /* WOLFHSM_CFG_HWKEYSTORE */ - /* remove the key from the cache if present, enforcing policy */ + /* NOTFOUND means the key was not cached, whether it is absent entirely or + * lives only in NVM; both are fine. Any other error must not be masked by + * the destroy below. */ ret = wh_Server_KeystoreEvictKeyChecked(server, keyId); + if ((ret != WH_ERROR_OK) && (ret != WH_ERROR_NOTFOUND)) { + return ret; + } - /* With no NVM, the cache eviction above is the whole erase; return its - * result so policy and not-found errors still propagate. */ + /* Nothing left to destroy is a successful erase, matching + * wh_Server_KeystoreEraseKey */ if (server->nvm == NULL) { - return ret; + return WH_ERROR_OK; } - /* destroy the object */ return wh_Nvm_DestroyObjectsChecked(server->nvm, 1, &keyId); } diff --git a/test-refactor/server/wh_test_nvm_optional.c b/test-refactor/server/wh_test_nvm_optional.c index 306d725f1..274304db4 100644 --- a/test-refactor/server/wh_test_nvm_optional.c +++ b/test-refactor/server/wh_test_nvm_optional.c @@ -266,10 +266,10 @@ static int _RunNvmOptionalChecks(whServerContext* server) WH_ERROR_NOTFOUND == wh_Server_KeystoreReadKey(server, localId, outMeta, outKey, &outSz)); - /* EraseKeyChecked enforces policy and, with no NVM, returns the cache - * eviction's status. A missing key reports NOTFOUND -- unlike the - * non-checked EraseKey above, which treats "nothing to destroy" as OK. */ - WH_TEST_ASSERT_RETURN(WH_ERROR_NOTFOUND == + /* EraseKeyChecked enforces policy but, like the non-checked EraseKey + * above, treats "nothing to erase" as OK. This holds whether or not NVM + * is attached. */ + WH_TEST_ASSERT_RETURN(WH_ERROR_OK == wh_Server_KeystoreEraseKeyChecked(server, missingId)); /* On a primed, policy-permissive key, EraseKeyChecked succeeds cache-only diff --git a/test-refactor/server/wh_test_nvm_policy.c b/test-refactor/server/wh_test_nvm_policy.c new file mode 100644 index 000000000..8e1208148 --- /dev/null +++ b/test-refactor/server/wh_test_nvm_policy.c @@ -0,0 +1,230 @@ +/* + * Copyright (C) 2026 wolfSSL Inc. + * + * This file is part of wolfHSM. + * + * wolfHSM is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * wolfHSM is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with wolfHSM. If not, see . + */ +/* + * test-refactor/server/wh_test_nvm_policy.c + * + * Server-side test that the policy-checked keystore APIs report denials + * correctly when NVM is attached. Complements wh_test_nvm_optional.c, which + * covers the same APIs with the NVM detached. + */ + +#include "wolfhsm/wh_settings.h" + +#include +#include + +#include "wolfhsm/wh_error.h" +#include "wolfhsm/wh_common.h" +#include "wolfhsm/wh_nvm.h" +#include "wolfhsm/wh_server.h" +#include "wolfhsm/wh_server_keystore.h" + +#include "wh_test_common.h" +#include "wh_test_list.h" + +#if defined(WOLFHSM_CFG_ENABLE_SERVER) && !defined(WOLFHSM_CFG_NO_CRYPTO) + +#define WH_TEST_NVMPOL_KEYLEN (32) + +/* A destroy batch naming an absent id must still destroy the present ones */ +static int _whTest_NvmPolicyDestroyBatchMissingId(whServerContext* server) +{ + whNvmMetadata meta[1]; + whNvmMetadata outMeta[1]; + uint8_t data[] = {0xA1, 0xB2, 0xC3, 0xD4}; + whNvmId present = 0x0310; + whNvmId absent = 0x0311; + whNvmId list[2]; + + list[0] = absent; + list[1] = present; + + memset(meta, 0, sizeof(meta)); + meta->id = present; + meta->len = (whNvmSize)sizeof(data); + meta->access = WH_NVM_ACCESS_ANY; + WH_TEST_ASSERT_RETURN( + WH_ERROR_OK == + wh_Nvm_AddObjectChecked(server->nvm, meta, sizeof(data), data)); + + WH_TEST_ASSERT_RETURN( + WH_ERROR_OK == wh_Nvm_DestroyObjectsChecked(server->nvm, 2, list)); + + WH_TEST_ASSERT_RETURN( + WH_ERROR_NOTFOUND == + wh_Nvm_GetMetadata(server->nvm, present, outMeta)); + + return WH_ERROR_OK; +} + +/* Destroying a list whose ids are all absent must not replicate the partition. + * Reclaimable space is the observable proof, since a replication compacts it + * away. */ +static int _whTest_NvmPolicyDestroyAllAbsentNoChurn(whServerContext* server) +{ + whNvmMetadata meta[1]; + uint8_t data[] = {0x5A, 0x6B, 0x7C, 0x8D}; + whNvmId id = 0x0320; + whNvmId absent[2]; + uint32_t availSize; + uint32_t reclaimBefore; + uint32_t reclaimAfter; + whNvmId availObjs; + whNvmId reclaimObjsBefore; + whNvmId reclaimObjsAfter; + + absent[0] = 0x0321; + absent[1] = 0x0322; + + /* Overwrite the object so the old copy becomes reclaimable */ + memset(meta, 0, sizeof(meta)); + meta->id = id; + meta->len = (whNvmSize)sizeof(data); + meta->access = WH_NVM_ACCESS_ANY; + WH_TEST_ASSERT_RETURN( + WH_ERROR_OK == + wh_Nvm_AddObjectChecked(server->nvm, meta, sizeof(data), data)); + WH_TEST_ASSERT_RETURN( + WH_ERROR_OK == + wh_Nvm_AddObjectChecked(server->nvm, meta, sizeof(data), data)); + + WH_TEST_ASSERT_RETURN( + WH_ERROR_OK == + wh_Nvm_GetAvailable(server->nvm, &availSize, &availObjs, + &reclaimBefore, &reclaimObjsBefore)); + if (reclaimObjsBefore == 0) { + /* Backend exposes no reclaimable space, so no-churn is not observable + * here; pass without claiming it was verified. */ + (void)wh_Nvm_DestroyObjectsChecked(server->nvm, 1, &id); + WH_TEST_PRINT(" no-churn NOT verified on this backend " + "(no reclaimable space to observe)\n"); + return WH_ERROR_OK; + } + + WH_TEST_ASSERT_RETURN( + WH_ERROR_OK == wh_Nvm_DestroyObjectsChecked(server->nvm, 2, absent)); + + /* Untouched reclaimable space proves the backend was never called */ + WH_TEST_ASSERT_RETURN( + WH_ERROR_OK == + wh_Nvm_GetAvailable(server->nvm, &availSize, &availObjs, &reclaimAfter, + &reclaimObjsAfter)); + WH_TEST_ASSERT_RETURN(reclaimObjsAfter == reclaimObjsBefore); + WH_TEST_ASSERT_RETURN(reclaimAfter == reclaimBefore); + + /* Zero-count compaction, by contrast, does reach the backend and clears + * reclaimable space, pinning the guard that lets it through */ + WH_TEST_ASSERT_RETURN( + WH_ERROR_OK == wh_Nvm_DestroyObjectsChecked(server->nvm, 0, NULL)); + WH_TEST_ASSERT_RETURN( + WH_ERROR_OK == + wh_Nvm_GetAvailable(server->nvm, &availSize, &availObjs, &reclaimAfter, + &reclaimObjsAfter)); + WH_TEST_ASSERT_RETURN(reclaimObjsAfter == 0); + + /* Clean up the surviving object */ + WH_TEST_ASSERT_RETURN( + WH_ERROR_OK == wh_Nvm_DestroyObjectsChecked(server->nvm, 1, &id)); + + return WH_ERROR_OK; +} + +/* Erasing a key that is absent from both cache and NVM is a successful erase, + * matching the non-checked wh_Server_KeystoreEraseKey. */ +static int _whTest_NvmPolicyMissingKeyEraseSucceeds(whServerContext* server) +{ + whKeyId missingId; + + missingId = WH_MAKE_KEYID(WH_KEYTYPE_CRYPTO, WH_TEST_DEFAULT_CLIENT_ID, + 0x32); + + WH_TEST_ASSERT_RETURN(WH_ERROR_OK == + wh_Server_KeystoreEraseKeyChecked(server, missingId)); + WH_TEST_ASSERT_RETURN(WH_ERROR_OK == + wh_Server_KeystoreEraseKey(server, missingId)); + + return WH_ERROR_OK; +} + +/* A revoked key that was never committed lives only in the cache, so the NVM + * destroy step finds nothing. The erase must still report the cache eviction's + * policy denial rather than the NVM layer's "nothing to destroy" success. */ +static int _whTest_NvmPolicyRevokedCacheOnlyEraseDenied(whServerContext* server) +{ + whNvmMetadata meta[1]; + whNvmMetadata outMeta[1]; + uint8_t keyData[WH_TEST_NVMPOL_KEYLEN]; + uint8_t outKey[WH_TEST_NVMPOL_KEYLEN]; + uint32_t outSz; + whKeyId revokeId; + int i; + + for (i = 0; i < (int)sizeof(keyData); i++) { + keyData[i] = (uint8_t)(i + 1); + } + + revokeId = WH_MAKE_KEYID(WH_KEYTYPE_CRYPTO, WH_TEST_DEFAULT_CLIENT_ID, + 0x31); + + /* Cache the key without committing it, so it never reaches NVM */ + memset(meta, 0, sizeof(meta)); + meta->id = revokeId; + meta->len = (whNvmSize)sizeof(keyData); + meta->flags = WH_NVM_FLAGS_USAGE_ANY; + meta->access = WH_NVM_ACCESS_ANY; + WH_TEST_ASSERT_RETURN(WH_ERROR_OK == + wh_Server_KeystoreCacheKey(server, meta, keyData)); + + /* Revoking marks the cached copy NONMODIFIABLE; with the key absent from + * NVM there is nothing to commit. */ + WH_TEST_ASSERT_RETURN(WH_ERROR_OK == + wh_Server_KeystoreRevokeKey(server, revokeId)); + + /* The denial must survive, even though the NVM destroy would succeed */ + WH_TEST_ASSERT_RETURN(WH_ERROR_ACCESS == + wh_Server_KeystoreEraseKeyChecked(server, revokeId)); + + /* Nothing was erased: the key is still readable */ + outSz = sizeof(outKey); + WH_TEST_ASSERT_RETURN( + WH_ERROR_OK == + wh_Server_KeystoreReadKey(server, revokeId, outMeta, outKey, &outSz)); + WH_TEST_ASSERT_RETURN(outSz == (uint32_t)sizeof(keyData)); + + /* Force-remove the revoked key (EraseKeyChecked could not) */ + (void)wh_Server_KeystoreEvictKey(server, revokeId); + + return WH_ERROR_OK; +} + +int whTest_NvmPolicyChecked(whServerContext* ctx) +{ + if (ctx == NULL) { + return WH_ERROR_BADARGS; + } + + WH_TEST_RETURN_ON_FAIL(_whTest_NvmPolicyDestroyBatchMissingId(ctx)); + WH_TEST_RETURN_ON_FAIL(_whTest_NvmPolicyDestroyAllAbsentNoChurn(ctx)); + WH_TEST_RETURN_ON_FAIL(_whTest_NvmPolicyMissingKeyEraseSucceeds(ctx)); + WH_TEST_RETURN_ON_FAIL(_whTest_NvmPolicyRevokedCacheOnlyEraseDenied(ctx)); + + return WH_ERROR_OK; +} + +#endif /* WOLFHSM_CFG_ENABLE_SERVER && !WOLFHSM_CFG_NO_CRYPTO */ diff --git a/test-refactor/wh_test_list.c b/test-refactor/wh_test_list.c index 70f7d0396..59318de44 100644 --- a/test-refactor/wh_test_list.c +++ b/test-refactor/wh_test_list.c @@ -53,6 +53,7 @@ WH_TEST_DECL(whTest_CertReadRejectsServerOnly); WH_TEST_DECL(whTest_HwKeystoreServer); WH_TEST_DECL(whTest_ServerImgMgr); WH_TEST_DECL(whTest_NvmOptional); +WH_TEST_DECL(whTest_NvmPolicyChecked); WH_TEST_DECL(whTest_ClientCerts); WH_TEST_DECL(whTest_Counter); WH_TEST_DECL(whTest_Crypto_Aes); @@ -116,6 +117,7 @@ const whTestCase whTestsServer[] = { { "whTest_CertReadRejectsServerOnly", whTest_CertReadRejectsServerOnly }, { "whTest_ServerImgMgr", whTest_ServerImgMgr }, { "whTest_NvmOptional", whTest_NvmOptional }, + { "whTest_NvmPolicyChecked", whTest_NvmPolicyChecked }, { "whTest_SheMasterEcuKeyFallback", whTest_SheMasterEcuKeyFallback }, { "whTest_SheReqSizeChecking", whTest_SheReqSizeChecking }, { "whTest_HwKeystoreServer", whTest_HwKeystoreServer }, diff --git a/test/wh_test_nvmflags.c b/test/wh_test_nvmflags.c index 00ec332c4..b327c9159 100644 --- a/test/wh_test_nvmflags.c +++ b/test/wh_test_nvmflags.c @@ -31,6 +31,10 @@ #define TEST_NVM_ID_NONDESTROYABLE 0x0003 #define TEST_NVM_ID_NONMOD_DMA 0x0004 #define TEST_NVM_ID_NONDESTROYABLE_DMA 0x0005 +#define TEST_NVM_ID_BATCH_A 0x0006 +#define TEST_NVM_ID_BATCH_B 0x0007 +#define TEST_NVM_ID_BATCH_ABSENT 0x0008 +#define TEST_NVM_ID_BATCH_PLAIN 0x0009 #define TEST_KEY_ID_NONMOD 0x0001 #define TEST_KEY_ID_MODIFIABLE 0x0002 #define TEST_KEY_ID_PROMOTE 0x0003 @@ -197,6 +201,54 @@ static int _testNonExportableNvmAccess(whClientContext* client) return 0; } +/* A destroy batch naming an absent id must still destroy the present ones. + * Leaves no artifacts, so this runs regardless of the persistent-artifact + * gate below. */ +static int _testNvmDestroyBatchMissingIdTolerated(whClientContext* client) +{ + int32_t server_rc; + whNvmId out_id; + whNvmAccess out_access; + whNvmFlags out_flags; + whNvmSize out_len; + uint8_t data[] = {0x13, 0x24, 0x35, 0x46}; + uint8_t label[] = "BATCH"; + uint8_t labelLen = (uint8_t)strlen((const char*)label); + whNvmId list[3] = {TEST_NVM_ID_BATCH_A, TEST_NVM_ID_BATCH_ABSENT, + TEST_NVM_ID_BATCH_B}; + + WH_TEST_PRINT("Testing NVM destroy batch containing an absent id...\n"); + + WH_TEST_RETURN_ON_FAIL(wh_Client_NvmAddObject( + client, TEST_NVM_ID_BATCH_A, WH_NVM_ACCESS_ANY, 0, labelLen, label, + sizeof(data), data, &server_rc)); + WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_OK); + + WH_TEST_RETURN_ON_FAIL(wh_Client_NvmAddObject( + client, TEST_NVM_ID_BATCH_B, WH_NVM_ACCESS_ANY, 0, labelLen, label, + sizeof(data), data, &server_rc)); + WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_OK); + + /* The absent id must not abort the batch */ + WH_TEST_RETURN_ON_FAIL( + wh_Client_NvmDestroyObjects(client, 3, list, &server_rc)); + WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_OK); + + /* Both present objects must actually be gone */ + WH_TEST_RETURN_ON_FAIL(wh_Client_NvmGetMetadata( + client, TEST_NVM_ID_BATCH_A, &server_rc, &out_id, &out_access, + &out_flags, &out_len, 0, NULL)); + WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_NOTFOUND); + + WH_TEST_RETURN_ON_FAIL(wh_Client_NvmGetMetadata( + client, TEST_NVM_ID_BATCH_B, &server_rc, &out_id, &out_access, + &out_flags, &out_len, 0, NULL)); + WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_NOTFOUND); + + WH_TEST_PRINT(" NVM destroy batch with absent id tolerated: PASS\n"); + return WH_ERROR_OK; +} + #if defined(WOLFHSM_CFG_TEST_ALLOW_PERSISTENT_NVM_ARTIFACTS) static int _testNvmNonmodifiableNoOverwrite(whClientContext* client) { @@ -263,6 +315,9 @@ static int _testNvmNondestroyableModifyNoDestroy(whClientContext* client) uint8_t readData[sizeof(data2)] = {0}; whNvmSize readLen = sizeof(readData); whNvmId list[1] = {TEST_NVM_ID_NONDESTROYABLE}; + whNvmId batchList[2] = {TEST_NVM_ID_BATCH_PLAIN, + TEST_NVM_ID_NONDESTROYABLE}; + whNvmId plainList[1] = {TEST_NVM_ID_BATCH_PLAIN}; uint8_t label[] = "NDY"; uint8_t labelLen = (uint8_t)strlen((const char*)label); @@ -300,6 +355,30 @@ static int _testNvmNondestroyableModifyNoDestroy(whClientContext* client) WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_OK); WH_TEST_ASSERT_RETURN(memcmp(readData, data2, sizeof(data2)) == 0); + /* A policy violation anywhere in the batch must abort the whole batch, + * leaving the permitted object in the same batch untouched. */ + WH_TEST_RETURN_ON_FAIL(wh_Client_NvmAddObject( + client, TEST_NVM_ID_BATCH_PLAIN, WH_NVM_ACCESS_ANY, 0, labelLen, label, + sizeof(data1), data1, &server_rc)); + WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_OK); + + WH_TEST_RETURN_ON_FAIL( + wh_Client_NvmDestroyObjects(client, 2, batchList, &server_rc)); + WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_ACCESS); + + memset(readData, 0, sizeof(readData)); + readLen = sizeof(readData); + WH_TEST_RETURN_ON_FAIL(wh_Client_NvmRead(client, TEST_NVM_ID_BATCH_PLAIN, 0, + sizeof(data1), &server_rc, + &readLen, readData)); + WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_OK); + WH_TEST_ASSERT_RETURN(memcmp(readData, data1, sizeof(data1)) == 0); + + /* The permitted object destroys fine on its own */ + WH_TEST_RETURN_ON_FAIL( + wh_Client_NvmDestroyObjects(client, 1, plainList, &server_rc)); + WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_OK); + WH_TEST_PRINT( " NVM NONDESTROYABLE modify allowed, destroy denied: PASS\n"); return WH_ERROR_OK; @@ -680,6 +759,11 @@ int whTest_NvmFlags(whClientContext* client) if (ret != WH_ERROR_OK) return ret; + WH_TEST_PRINT("\n=== NVM Object Tests (destroy batch tolerance) ===\n"); + + ret = _testNvmDestroyBatchMissingIdTolerated(client); + if (ret != WH_ERROR_OK) + return ret; #if defined(WOLFHSM_CFG_TEST_ALLOW_PERSISTENT_NVM_ARTIFACTS) WH_TEST_PRINT("--- NVM Object Tests (NONMODIFIABLE/NODESTROYABLE) ---\n"); diff --git a/wolfhsm/wh_nvm.h b/wolfhsm/wh_nvm.h index 9acbcf9fa..4eedd49c7 100644 --- a/wolfhsm/wh_nvm.h +++ b/wolfhsm/wh_nvm.h @@ -391,12 +391,17 @@ int wh_Nvm_DestroyObjects(whNvmContext* context, whNvmId list_count, * or is a server-only key (WH_NVM_FLAGS_TRUSTED). If so, returns an access * error without destroying any objects. * + * As with wh_Nvm_DestroyObjects(), IDs in the list that are not present do not + * cause an error: an absent object carries no flags to enforce and is skipped. + * * @param[in] context Pointer to the NVM context. Must not be NULL. * @param[in] list_count Number of IDs in the list. * @param[in] id_list Array of object IDs to destroy. - * @return int WH_ERROR_OK on success. + * @return int WH_ERROR_OK on success, including when some or all IDs are + * not present. * WH_ERROR_ACCESS if any object is non-modifiable, - * non-destroyable, or a server-only key. + * non-destroyable, or a server-only key. No + * objects are destroyed. * WH_ERROR_BADARGS if context is NULL, not initialized, or * id_list is NULL with non-zero list_count. * Other negative error codes on backend failure. diff --git a/wolfhsm/wh_server_keystore.h b/wolfhsm/wh_server_keystore.h index d34588e7c..3e15333e8 100644 --- a/wolfhsm/wh_server_keystore.h +++ b/wolfhsm/wh_server_keystore.h @@ -180,7 +180,14 @@ int wh_Server_KeystoreEraseKey(whServerContext* server, whNvmId keyId); /** * @brief Erase a key with policy enforcement * - * Runs keystore policy checks before evicting/destroying. + * Runs keystore policy checks before evicting/destroying. An absent key is a + * successful erase, whether or not NVM is configured. + * + * @param[in] server Server context + * @param[in] keyId Key ID to erase + * @return 0 on success, including when the key does not exist. WH_ERROR_ACCESS + * if policy denies the erase (the persisted key stays; a cached copy + * may still be evicted). Other error codes on failure. */ int wh_Server_KeystoreEraseKeyChecked(whServerContext* server, whNvmId keyId);