From 61a16fa3b48bdd36c78dd5da6165e1d682dc99dc Mon Sep 17 00:00:00 2001 From: Yosuke Shimizu Date: Fri, 24 Jul 2026 16:11:01 +0900 Subject: [PATCH] Enforce NONMODIFIABLE and TRUSTED NVM policy on keystore key commit --- .github/workflows/build-and-test-refactor.yml | 13 + src/wh_server_keystore.c | 21 +- test-refactor/README.md | 4 +- .../client-server/wh_test_crypto_keypolicy.c | 13 +- .../client-server/wh_test_crypto_keystore.c | 151 ++++++++++ test-refactor/config/wolfhsm_cfg.h | 3 +- test-refactor/posix/Makefile | 7 + test-refactor/server/wh_test_nvm_policy.c | 272 ++++++++++++++++++ wolfhsm/wh_client.h | 8 +- wolfhsm/wh_server_keystore.h | 20 +- 10 files changed, 499 insertions(+), 13 deletions(-) diff --git a/.github/workflows/build-and-test-refactor.yml b/.github/workflows/build-and-test-refactor.yml index a9213b423..452bffc39 100644 --- a/.github/workflows/build-and-test-refactor.yml +++ b/.github/workflows/build-and-test-refactor.yml @@ -35,10 +35,12 @@ jobs: fromJSON('[{"os":"ubuntu-latest","group":"pq-dma"}, {"os":"ubuntu-latest","group":"wolfcrypt"}, {"os":"ubuntu-latest","group":"threadsafe"}, + {"os":"ubuntu-latest","group":"nvm-persist"}, {"os":"ubuntu-latest","group":"base"}]') || fromJSON('[{"os":"ubuntu-latest","group":"pq-dma"}, {"os":"ubuntu-latest","group":"wolfcrypt"}, {"os":"ubuntu-latest","group":"threadsafe"}, + {"os":"ubuntu-latest","group":"nvm-persist"}, {"os":"ubuntu-latest","group":"base"}, {"os":"macos-latest","group":"base"}]') }} @@ -191,6 +193,17 @@ jobs: if: matrix.group == 'base' run: cd test-refactor/posix && make clean && make -j AUTH=1 NOCRYPTO=1 WOLFSSL_DIR=../../wolfssl && make run + # Suites that leave undeletable NVM objects. The NVM is RAM-backed and + # fresh per process, so they only occupy slots within one `make run`. + - name: Build and test refactor with persistent NVM artifacts + if: matrix.group == 'nvm-persist' + run: cd test-refactor/posix && make clean && make -j ASAN=1 PERSISTENT_NVM_ARTIFACTS=1 WOLFSSL_DIR=../../wolfssl && make run + + # Same with DMA, so the gated suites also cover the DMA dispatch path + - name: Build and test refactor with persistent NVM artifacts and DMA + if: matrix.group == 'nvm-persist' + run: cd test-refactor/posix && make clean && make -j DMA=1 ASAN=1 PERSISTENT_NVM_ARTIFACTS=1 WOLFSSL_DIR=../../wolfssl && make run + - name: Show ccache stats run: ccache -s diff --git a/src/wh_server_keystore.c b/src/wh_server_keystore.c index 94512ef1f..38e0fa95d 100644 --- a/src/wh_server_keystore.c +++ b/src/wh_server_keystore.c @@ -154,6 +154,7 @@ static int _KeystoreCheckPolicy(whServerContext* server, whKsOp op, whNvmMetadata* cacheMeta = NULL; whNvmMetadata nvmMeta; whNvmFlags flags; + whNvmFlags denyMask; int ret; int foundInCache = 0; int foundInNvm = 0; @@ -247,7 +248,25 @@ static int _KeystoreCheckPolicy(whServerContext* server, whKsOp op, case WH_KS_OP_COMMIT: case WH_KS_OP_REVOKE: - /* Always allowed */ + /* Both rewrite the stored object from the cache slot, so the + * stored flags decide; a cached copy cannot launder them. */ + if (!foundInNvm && (server->nvm != NULL)) { + ret = wh_Nvm_GetMetadata(server->nvm, keyId, &nvmMeta); + if (ret == WH_ERROR_OK) { + foundInNvm = 1; + } + else if (ret != WH_ERROR_NOTFOUND) { + return ret; /* unreadable flags: deny the write */ + } + } + /* Revoke leaves NONMODIFIABLE permitted so an already-revoked key + * can be revoked again. */ + denyMask = (op == WH_KS_OP_COMMIT) + ? (WH_NVM_FLAGS_NONMODIFIABLE | WH_NVM_FLAGS_TRUSTED) + : WH_NVM_FLAGS_TRUSTED; + if (foundInNvm && ((nvmMeta.flags & denyMask) != 0)) { + return WH_ERROR_ACCESS; + } break; default: /* unknown operation */ diff --git a/test-refactor/README.md b/test-refactor/README.md index d7811ce39..a86145487 100644 --- a/test-refactor/README.md +++ b/test-refactor/README.md @@ -25,6 +25,8 @@ The top-level `make` forwards to the POSIX port; `cd test-refactor/posix && make Results are printed via `WOLFHSM_CFG_PRINTF` from the wolfHSM build. `test-suite.log` contains the detailed output. +Some sub-tests are opt-in because they leave NVM objects that cannot be erased, so they occupy slots for the rest of the run. Build with `make PERSISTENT_NVM_ARTIFACTS=1` to define `WOLFHSM_CFG_TEST_ALLOW_PERSISTENT_NVM_ARTIFACTS` and include them. + ## Running the tests from an embedded target To run the tests on a target device, create an application running on the client or server that runs the tests from `main()`. See sections on adding ports and tests. @@ -87,7 +89,7 @@ Translated tests: | `wh_test_crypto.c::whTest_CryptoKeyUsagePolicies` (AES CTR/ECB/GCM subset) | `client-server/wh_test_crypto_aes.c::whTest_CryptoAesKeyUsagePolicies` | Client | AES-CTR/ECB/GCM key usage enforcement (non-DMA and DMA variants) | | `wh_test_crypto.c::whTestCrypto_LmsCryptoCb` | `client-server/wh_test_crypto_lms.c::whTest_Crypto_Lms` | Client | DMA-only LMS generate/durability/sign/verify, public-key export+import, private export/import rejection, and the server-only `WH_NVM_FLAGS_TRUSTED` keygen strip regression. Gated by `WOLFHSM_CFG_DMA && WOLFSSL_HAVE_LMS && !WOLFSSL_LMS_VERIFY_ONLY`; reports SKIPPED otherwise | | `wh_test_crypto.c::whTestCrypto_XmssCryptoCb` | `client-server/wh_test_crypto_xmss.c::whTest_Crypto_Xmss` | Client | DMA-only XMSS generate/durability/sign/verify, public-key export+import, private export/import rejection, and the server-only `WH_NVM_FLAGS_TRUSTED` keygen strip regression. Gated by `WOLFHSM_CFG_DMA && WOLFSSL_HAVE_XMSS && !WOLFSSL_XMSS_VERIFY_ONLY`; reports SKIPPED otherwise | -| `wh_test_crypto.c::{whTest_KeyCache, whTest_NonExportableKeystore}` | `client-server/wh_test_crypto_keystore.c::whTest_Crypto_Keystore` | Client | Key-cache lifecycle (cache/export, evict, commit/erase, cross-cache eviction/replacement, NVM-backed eviction) and non-exportable-flag enforcement; std and DMA export paths. The `WOLFHSM_CFG_IS_TEST_SERVER` multi-client user-exclusion path is dropped (needs two client contexts) | +| `wh_test_crypto.c::{whTest_KeyCache, whTest_NonExportableKeystore}` | `client-server/wh_test_crypto_keystore.c::whTest_Crypto_Keystore` | Client | Key-cache lifecycle (cache/export, evict, commit/erase, cross-cache eviction/replacement, NVM-backed eviction) and non-exportable-flag enforcement; std and DMA export paths. The `WOLFHSM_CFG_IS_TEST_SERVER` multi-client user-exclusion path is dropped (needs two client contexts). Adds `_whTest_NonModifiableCommit` (re-commit over a stored `WH_NVM_FLAGS_NONMODIFIABLE` object is denied whether or not the slot is still cached; gated by `WOLFHSM_CFG_TEST_ALLOW_PERSISTENT_NVM_ARTIFACTS` since the committed object cannot be erased) and the ungated `_whTest_ModifiableRecommit` | | `wh_test_clientserver.c` (echo and server-info paths) | `client-server/wh_test_echo.c::whTest_Echo`, `client-server/wh_test_server_info.c::whTest_ServerInfo` | Client | pthread test ported, sequential test dropped | | `wh_test_clientserver.c` (NVM CRUD + OOB read clamping paths) | `client-server/wh_test_nvm_ops.c::{whTest_NvmCrud, whTest_NvmReadOob}` | Client | each test cleans up its own slots; OOB test covers UINT16_MAX overflow regression | | `wh_test_clientserver.c` (NVM DMA CRUD path) | `client-server/wh_test_nvm_dma.c::whTest_NvmCrudDma` | Client | gated on `WOLFHSM_CFG_DMA` | diff --git a/test-refactor/client-server/wh_test_crypto_keypolicy.c b/test-refactor/client-server/wh_test_crypto_keypolicy.c index 07a1919a4..359931b48 100644 --- a/test-refactor/client-server/wh_test_crypto_keypolicy.c +++ b/test-refactor/client-server/wh_test_crypto_keypolicy.c @@ -506,7 +506,8 @@ static int _whTest_CryptoKeyUsagePolicies(whClientContext* client) #if !defined(NO_AES) && defined(HAVE_AES_CBC) && \ defined(WOLFHSM_CFG_TEST_ALLOW_PERSISTENT_NVM_ARTIFACTS) -static int whTest_RevocationTryAESEncrypt(whKeyId keyId, WC_RNG* rng, +static int whTest_RevocationTryAESEncrypt(whClientContext* client, + whKeyId keyId, WC_RNG* rng, int* encryptRes) { int ret; @@ -584,7 +585,7 @@ static int _whTest_CryptoKeyRevocationAesCbc(whClientContext* client) return ret; } - ret = whTest_RevocationTryAESEncrypt(keyId, rng, &encryptRes); + ret = whTest_RevocationTryAESEncrypt(client, keyId, rng, &encryptRes); if (ret != 0) { WH_ERROR_PRINT("Failed to encrypt with unrevoked AES key: %d\n", ret); (void)wh_Client_KeyEvict(client, keyId); @@ -605,7 +606,7 @@ static int _whTest_CryptoKeyRevocationAesCbc(whClientContext* client) return ret; } - ret = whTest_RevocationTryAESEncrypt(keyId, rng, &encryptRes); + ret = whTest_RevocationTryAESEncrypt(client, keyId, rng, &encryptRes); if (ret != 0 || encryptRes != WH_ERROR_USAGE) { WH_ERROR_PRINT( "Encrypt with revoked AES key should fail (%d), got %d\n", @@ -621,7 +622,7 @@ static int _whTest_CryptoKeyRevocationAesCbc(whClientContext* client) return ret; } - ret = whTest_RevocationTryAESEncrypt(keyId, rng, &encryptRes); + ret = whTest_RevocationTryAESEncrypt(client, keyId, rng, &encryptRes); if (ret != 0 || encryptRes != WH_ERROR_USAGE) { WH_ERROR_PRINT( "Encrypt with revoked AES key should fail (%d), got %d\n", @@ -653,7 +654,7 @@ static int _whTest_CryptoKeyRevocationAesCbc(whClientContext* client) (void)wc_FreeRng(rng); return ret; } - ret = whTest_RevocationTryAESEncrypt(keyId, rng, &encryptRes); + ret = whTest_RevocationTryAESEncrypt(client, keyId, rng, &encryptRes); if (ret != 0 || encryptRes != 0) { WH_ERROR_PRINT( "Failed to encrypt with unrevoked AES key (2nd time): %d\n", ret); @@ -673,7 +674,7 @@ static int _whTest_CryptoKeyRevocationAesCbc(whClientContext* client) (void)wc_FreeRng(rng); return ret; } - ret = whTest_RevocationTryAESEncrypt(keyId, rng, &encryptRes); + ret = whTest_RevocationTryAESEncrypt(client, keyId, rng, &encryptRes); if (ret != 0 || encryptRes != WH_ERROR_USAGE) { WH_ERROR_PRINT( "Encrypt with revoked AES key should fail (%d), got %d\n", diff --git a/test-refactor/client-server/wh_test_crypto_keystore.c b/test-refactor/client-server/wh_test_crypto_keystore.c index 604026e3a..adda65964 100644 --- a/test-refactor/client-server/wh_test_crypto_keystore.c +++ b/test-refactor/client-server/wh_test_crypto_keystore.c @@ -28,6 +28,15 @@ * _whTest_NonExportableKeystore - confirm WH_NVM_FLAGS_NONEXPORTABLE keys * cannot be exported while ordinary keys can * (std and DMA export paths) + * _whTest_NonModifiableCommit - re-commit over a stored + * WH_NVM_FLAGS_NONMODIFIABLE object is + * denied, cached or not, and the stored key + * and label survive the denial + * _whTest_ModifiableRecommit - a key without the flag still re-commits + * + * _whTest_NonModifiableCommit is gated by + * WOLFHSM_CFG_TEST_ALLOW_PERSISTENT_NVM_ARTIFACTS: the object it commits + * cannot be erased, so it holds an NVM slot for the rest of the run. */ #include "wolfhsm/wh_settings.h" @@ -839,6 +848,144 @@ static int _whTest_NonExportableKeystore(whClientContext* ctx) return 0; } +#if defined(WOLFHSM_CFG_TEST_ALLOW_PERSISTENT_NVM_ARTIFACTS) +/* Committing a NONMODIFIABLE key leaves an object that + * wh_Nvm_DestroyObjectsChecked refuses to erase, so it occupies one NVM + * slot for the rest of the run. Gated like the keypolicy revocation test. */ +static int _whTest_NonModifiableCommit(whClientContext* ctx) +{ + int ret = 0; + whKeyId keyId = WH_KEYID_ERASED; + uint8_t key[WH_TEST_KEYSTORE_TEST_SZ] = { + 0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF, 0x01, 0x23, 0x45, + 0x67, 0x89, 0xAB, 0xCD, 0xEF, 0xFE, 0xDC, 0xBA, 0x98, 0x76, 0x54, + 0x32, 0x10, 0xFE, 0xDC, 0xBA, 0x98, 0x76, 0x54, 0x32, 0x10}; + uint8_t exportedKey[WH_TEST_KEYSTORE_TEST_SZ] = {0}; + uint8_t label[WH_NVM_LABEL_LEN] = "NonModifiableCommitKey"; + uint8_t exportedLabel[WH_NVM_LABEL_LEN] = {0}; + uint16_t exportedKeySize; + + WH_TEST_PRINT("Testing non-modifiable commit enforcement...\n"); + + /* Test 1: first commit of a NONMODIFIABLE key stores it, and the commit + * leaves the slot cached, so a repeat commit is an overwrite attempt. */ + ret = wh_Client_KeyCache(ctx, WH_NVM_FLAGS_NONMODIFIABLE, label, + sizeof(label), key, sizeof(key), &keyId); + if (ret != 0) { + WH_ERROR_PRINT("Failed to cache non-modifiable key: %d\n", ret); + return ret; + } + + ret = wh_Client_KeyCommit(ctx, keyId); + if (ret != 0) { + WH_ERROR_PRINT("Failed first commit of non-modifiable key: %d\n", ret); + return ret; + } + + /* Test 2: re-committing over the stored non-modifiable object is denied */ + ret = wh_Client_KeyCommit(ctx, keyId); + if (ret != WH_ERROR_ACCESS) { + WH_ERROR_PRINT("Non-modifiable key was re-committed unexpectedly: %d\n", + ret); + return -1; + } + + WH_TEST_DEBUG_PRINT("Non-modifiable key re-commit correctly denied\n"); + + /* Test 3: the denial left the stored object intact. Evicting is allowed + * because the key is committed, so the export below must freshen it back + * out of NVM rather than read the surviving cache slot. */ + ret = wh_Client_KeyEvict(ctx, keyId); + if (ret != 0) { + WH_ERROR_PRINT("Failed to evict committed non-modifiable key: %d\n", + ret); + return ret; + } + + exportedKeySize = sizeof(exportedKey); + ret = wh_Client_KeyExport(ctx, keyId, exportedLabel, sizeof(exportedLabel), + exportedKey, &exportedKeySize); + if (ret != 0) { + WH_ERROR_PRINT("Failed to export stored non-modifiable key: %d\n", ret); + return ret; + } + + if (exportedKeySize != sizeof(key) || + memcmp(key, exportedKey, exportedKeySize) != 0 || + memcmp(label, exportedLabel, sizeof(label)) != 0) { + WH_ERROR_PRINT("Denied commit altered the stored key\n"); + return -1; + } + + WH_TEST_DEBUG_PRINT("Stored non-modifiable key unchanged after denial\n"); + + /* Evicting reclaims only the cache slot; wh_Nvm_DestroyObjectsChecked + * refuses the NONMODIFIABLE object. Checked so Test 4 cannot degrade into + * a repeat of Test 2 with the slot still resident. */ + ret = wh_Client_KeyEvict(ctx, keyId); + if (ret != 0) { + WH_ERROR_PRINT("Failed to evict before uncached commit check: %d\n", + ret); + return ret; + } + + /* Test 4: the denial does not depend on cache residency. With no slot + * left, the stored flags still decide, so commit reports ACCESS rather + * than the NOTFOUND raised by the missing slot. */ + ret = wh_Client_KeyCommit(ctx, keyId); + if (ret != WH_ERROR_ACCESS) { + WH_ERROR_PRINT("Uncached non-modifiable commit not denied: %d\n", ret); + return -1; + } + + WH_TEST_DEBUG_PRINT("Uncached non-modifiable commit correctly denied\n"); + + WH_TEST_PRINT("NON-MODIFIABLE COMMIT TEST SUCCESS\n"); + return 0; +} +#endif /* WOLFHSM_CFG_TEST_ALLOW_PERSISTENT_NVM_ARTIFACTS */ + +static int _whTest_ModifiableRecommit(whClientContext* ctx) +{ + int ret = 0; + whKeyId keyId = WH_KEYID_ERASED; + uint8_t key[WH_TEST_KEYSTORE_TEST_SZ] = { + 0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF, 0x01, 0x23, 0x45, + 0x67, 0x89, 0xAB, 0xCD, 0xEF, 0xFE, 0xDC, 0xBA, 0x98, 0x76, 0x54, + 0x32, 0x10, 0xFE, 0xDC, 0xBA, 0x98, 0x76, 0x54, 0x32, 0x10}; + uint8_t label[WH_NVM_LABEL_LEN] = "ModifiableCommitKey"; + + WH_TEST_PRINT("Testing modifiable commit is unaffected...\n"); + + /* A key without the flag still commits repeatedly */ + ret = wh_Client_KeyCache(ctx, WH_NVM_FLAGS_NONE, label, sizeof(label), key, + sizeof(key), &keyId); + if (ret != 0) { + WH_ERROR_PRINT("Failed to cache modifiable key: %d\n", ret); + return ret; + } + + ret = wh_Client_KeyCommit(ctx, keyId); + if (ret != 0) { + WH_ERROR_PRINT("Failed first commit of modifiable key: %d\n", ret); + return ret; + } + + ret = wh_Client_KeyCommit(ctx, keyId); + if (ret != 0) { + WH_ERROR_PRINT("Failed repeat commit of modifiable key: %d\n", ret); + return ret; + } + + WH_TEST_DEBUG_PRINT("Modifiable key repeat commit allowed\n"); + + /* Clean up */ + (void)wh_Client_KeyErase(ctx, keyId); + + WH_TEST_PRINT("MODIFIABLE COMMIT TEST SUCCESS\n"); + return 0; +} + int whTest_Crypto_Keystore(whClientContext* ctx) { /* A preceding suite may leave the DMA-preferred dispatch mode set; reset @@ -846,6 +993,10 @@ int whTest_Crypto_Keystore(whClientContext* ctx) (void)wh_Client_SetDmaMode(ctx, 0); WH_TEST_RETURN_ON_FAIL(_whTest_KeyCache(ctx)); WH_TEST_RETURN_ON_FAIL(_whTest_NonExportableKeystore(ctx)); +#if defined(WOLFHSM_CFG_TEST_ALLOW_PERSISTENT_NVM_ARTIFACTS) + WH_TEST_RETURN_ON_FAIL(_whTest_NonModifiableCommit(ctx)); +#endif + WH_TEST_RETURN_ON_FAIL(_whTest_ModifiableRecommit(ctx)); return 0; } diff --git a/test-refactor/config/wolfhsm_cfg.h b/test-refactor/config/wolfhsm_cfg.h index ac428acfe..22afb4191 100644 --- a/test-refactor/config/wolfhsm_cfg.h +++ b/test-refactor/config/wolfhsm_cfg.h @@ -56,7 +56,8 @@ #define WOLFHSM_CFG_SERVER_NVM_FLASH_LOG /* WOLFHSM_CFG_TEST_ALLOW_PERSISTENT_NVM_ARTIFACTS is intentionally NOT - * defined here. Not implemented yet. */ + * defined here: one NVM is shared by every test in a run. The persistent + * NVM artifacts CI job defines it on the command line instead. */ #define WOLFHSM_CFG_ENABLE_TIMEOUT diff --git a/test-refactor/posix/Makefile b/test-refactor/posix/Makefile index 89673960d..ffd563f04 100644 --- a/test-refactor/posix/Makefile +++ b/test-refactor/posix/Makefile @@ -152,6 +152,13 @@ ifeq ($(CRYPTO_AFFINITY),1) DEF += -DWOLFHSM_CFG_CRYPTO_AFFINITY endif +# Enable the test suites that leave undeletable NVM objects behind. One NVM is +# shared by every test in a run, so those objects occupy slots for the rest of +# the run. +ifeq ($(PERSISTENT_NVM_ARTIFACTS),1) + DEF += -DWOLFHSM_CFG_TEST_ALLOW_PERSISTENT_NVM_ARTIFACTS +endif + # Build the wolfCrypt test suite as a wolfHSM client ifeq ($(TESTWOLFCRYPT),1) DEF += -DWOLFHSM_CFG_TEST_WOLFCRYPTTEST diff --git a/test-refactor/server/wh_test_nvm_policy.c b/test-refactor/server/wh_test_nvm_policy.c index 8e1208148..b0846c98d 100644 --- a/test-refactor/server/wh_test_nvm_policy.c +++ b/test-refactor/server/wh_test_nvm_policy.c @@ -213,6 +213,273 @@ static int _whTest_NvmPolicyRevokedCacheOnlyEraseDenied(whServerContext* server) return WH_ERROR_OK; } +/* Committing over a stored TRUSTED object must be denied even when the cache + * slot's own flags lack the flag. Only the unchecked cache path (keywrap + * unwrap-and-cache, SHE) can produce that pairing. */ +static int _whTest_NvmPolicyCommitTrustedDenied(whServerContext* server) +{ + whNvmMetadata meta[1]; + uint8_t kek[WH_TEST_NVMPOL_KEYLEN]; + uint8_t forged[WH_TEST_NVMPOL_KEYLEN]; + uint8_t stored[WH_TEST_NVMPOL_KEYLEN]; + whKeyId kekId; + int i; + + for (i = 0; i < (int)sizeof(kek); i++) { + kek[i] = (uint8_t)(0x40 + i); + forged[i] = 0xFF; + } + + kekId = WH_MAKE_KEYID(WH_KEYTYPE_CRYPTO, WH_TEST_DEFAULT_CLIENT_ID, 0x33); + + /* Provision the trusted KEK the way boot code or whnvmtool would */ + memset(meta, 0, sizeof(meta)); + meta->id = kekId; + meta->len = (whNvmSize)sizeof(kek); + meta->flags = WH_NVM_FLAGS_TRUSTED; + meta->access = WH_NVM_ACCESS_ANY; + WH_TEST_ASSERT_RETURN( + WH_ERROR_OK == wh_Nvm_AddObject(server->nvm, meta, sizeof(kek), kek)); + + /* Cache other bytes under the same id with the trusted flag cleared */ + meta->flags = WH_NVM_FLAGS_USAGE_ANY; + WH_TEST_ASSERT_RETURN(WH_ERROR_OK == + wh_Server_KeystoreCacheKey(server, meta, forged)); + + WH_TEST_ASSERT_RETURN(WH_ERROR_ACCESS == + wh_Server_KeystoreCommitKeyChecked(server, kekId)); + + /* The stored KEK is untouched */ + memset(stored, 0, sizeof(stored)); + WH_TEST_ASSERT_RETURN(WH_ERROR_OK == + wh_Nvm_Read(server->nvm, kekId, 0, + (whNvmSize)sizeof(stored), stored)); + WH_TEST_ASSERT_RETURN(memcmp(stored, kek, sizeof(kek)) == 0); + + /* Unchecked teardown: the checked paths refuse a trusted object */ + (void)wh_Server_KeystoreEvictKey(server, kekId); + WH_TEST_ASSERT_RETURN(WH_ERROR_OK == + wh_Nvm_DestroyObjects(server->nvm, 1, &kekId)); + + return WH_ERROR_OK; +} + +/* The other half of the commit deny mask. The client-driven test for this lives + * behind the persistence gate, since only the unchecked NVM API can remove the + * object again, so cover it here too and run in every configuration. */ +static int _whTest_NvmPolicyCommitNonModifiableDenied(whServerContext* server) +{ + whNvmMetadata meta[1]; + uint8_t keyData[WH_TEST_NVMPOL_KEYLEN]; + uint8_t replacement[WH_TEST_NVMPOL_KEYLEN]; + uint8_t stored[WH_TEST_NVMPOL_KEYLEN]; + whKeyId keyId; + int i; + + for (i = 0; i < (int)sizeof(keyData); i++) { + keyData[i] = (uint8_t)(0xA0 + i); + replacement[i] = 0xDD; + } + + keyId = WH_MAKE_KEYID(WH_KEYTYPE_CRYPTO, WH_TEST_DEFAULT_CLIENT_ID, 0x36); + + memset(meta, 0, sizeof(meta)); + meta->id = keyId; + meta->len = (whNvmSize)sizeof(keyData); + meta->flags = WH_NVM_FLAGS_NONMODIFIABLE; + meta->access = WH_NVM_ACCESS_ANY; + WH_TEST_ASSERT_RETURN(WH_ERROR_OK == wh_Nvm_AddObject(server->nvm, meta, + sizeof(keyData), + keyData)); + + /* Cache a replacement under the same id with the flag cleared */ + meta->flags = WH_NVM_FLAGS_USAGE_ANY; + WH_TEST_ASSERT_RETURN(WH_ERROR_OK == + wh_Server_KeystoreCacheKey(server, meta, replacement)); + + WH_TEST_ASSERT_RETURN(WH_ERROR_ACCESS == + wh_Server_KeystoreCommitKeyChecked(server, keyId)); + + /* The stored object is untouched */ + memset(stored, 0, sizeof(stored)); + WH_TEST_ASSERT_RETURN(WH_ERROR_OK == + wh_Nvm_Read(server->nvm, keyId, 0, + (whNvmSize)sizeof(stored), stored)); + WH_TEST_ASSERT_RETURN(memcmp(stored, keyData, sizeof(keyData)) == 0); + + /* Unchecked teardown: the checked destroy refuses a non-modifiable object */ + (void)wh_Server_KeystoreEvictKey(server, keyId); + WH_TEST_ASSERT_RETURN(WH_ERROR_OK == + wh_Nvm_DestroyObjects(server->nvm, 1, &keyId)); + + return WH_ERROR_OK; +} + +/* Both writers must fail closed when the stored flags cannot be read. A + * backend without the optional GetMetadata callback makes wh_Nvm_GetMetadata + * report WH_ERROR_ABORTED, which they must propagate instead of allowing. */ +static int _whTest_NvmPolicyUnreadableFlagsDenied(whServerContext* server) +{ + whNvmMetadata meta[1]; + static whNvmCb blindCb; + whNvmCb* savedCb; + uint8_t keyData[WH_TEST_NVMPOL_KEYLEN]; + whKeyId keyId; + int commitRet; + int revokeRet; + int i; + + for (i = 0; i < (int)sizeof(keyData); i++) { + keyData[i] = (uint8_t)(0x60 + i); + } + + keyId = WH_MAKE_KEYID(WH_KEYTYPE_CRYPTO, WH_TEST_DEFAULT_CLIENT_ID, 0x34); + + memset(meta, 0, sizeof(meta)); + meta->id = keyId; + 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)); + + /* The real backend, minus the optional metadata callback. Static, so a + * later edit that returns before the restore below cannot leave the shared + * context pointing into a dead stack frame. */ + savedCb = server->nvm->cb; + blindCb = *savedCb; + blindCb.GetMetadata = NULL; + server->nvm->cb = &blindCb; + + commitRet = wh_Server_KeystoreCommitKeyChecked(server, keyId); + revokeRet = wh_Server_KeystoreRevokeKey(server, keyId); + + /* Restore before asserting so a failure cannot leave the shared context + * with the stub callback table */ + server->nvm->cb = savedCb; + + WH_TEST_ASSERT_RETURN(WH_ERROR_ABORTED == commitRet); + WH_TEST_ASSERT_RETURN(WH_ERROR_ABORTED == revokeRet); + + /* Neither denial wrote anything: the key is still cache-only */ + WH_TEST_ASSERT_RETURN(WH_ERROR_NOTFOUND == + wh_Nvm_GetMetadata(server->nvm, keyId, meta)); + + (void)wh_Server_KeystoreEvictKey(server, keyId); + + return WH_ERROR_OK; +} + +/* Revoke rewrites the stored object from the cache slot, so it is gated on the + * stored TRUSTED flag exactly as commit is. Without the gate a laundered cache + * slot replaces the KEK bytes and drops the flag. */ +static int _whTest_NvmPolicyRevokeTrustedDenied(whServerContext* server) +{ + whNvmMetadata meta[1]; + whNvmMetadata storedMeta[1]; + uint8_t kek[WH_TEST_NVMPOL_KEYLEN]; + uint8_t forged[WH_TEST_NVMPOL_KEYLEN]; + uint8_t stored[WH_TEST_NVMPOL_KEYLEN]; + whKeyId kekId; + int i; + + for (i = 0; i < (int)sizeof(kek); i++) { + kek[i] = (uint8_t)(0x80 + i); + forged[i] = 0xEE; + } + + kekId = WH_MAKE_KEYID(WH_KEYTYPE_CRYPTO, WH_TEST_DEFAULT_CLIENT_ID, 0x35); + + /* Provision the trusted KEK the way boot code or whnvmtool would */ + memset(meta, 0, sizeof(meta)); + meta->id = kekId; + meta->len = (whNvmSize)sizeof(kek); + meta->flags = WH_NVM_FLAGS_TRUSTED; + meta->access = WH_NVM_ACCESS_ANY; + WH_TEST_ASSERT_RETURN( + WH_ERROR_OK == wh_Nvm_AddObject(server->nvm, meta, sizeof(kek), kek)); + + /* Cache other bytes under the same id with the trusted flag cleared */ + meta->flags = WH_NVM_FLAGS_USAGE_ANY; + WH_TEST_ASSERT_RETURN(WH_ERROR_OK == + wh_Server_KeystoreCacheKey(server, meta, forged)); + + WH_TEST_ASSERT_RETURN(WH_ERROR_ACCESS == + wh_Server_KeystoreRevokeKey(server, kekId)); + + /* The stored KEK keeps both its bytes and its trusted flag */ + memset(stored, 0, sizeof(stored)); + WH_TEST_ASSERT_RETURN(WH_ERROR_OK == + wh_Nvm_Read(server->nvm, kekId, 0, + (whNvmSize)sizeof(stored), stored)); + WH_TEST_ASSERT_RETURN(memcmp(stored, kek, sizeof(kek)) == 0); + WH_TEST_ASSERT_RETURN(WH_ERROR_OK == + wh_Nvm_GetMetadata(server->nvm, kekId, storedMeta)); + WH_TEST_ASSERT_RETURN((storedMeta->flags & WH_NVM_FLAGS_TRUSTED) != 0); + + /* Unchecked teardown: the checked paths refuse a trusted object */ + (void)wh_Server_KeystoreEvictKey(server, kekId); + WH_TEST_ASSERT_RETURN(WH_ERROR_OK == + wh_Nvm_DestroyObjects(server->nvm, 1, &kekId)); + + return WH_ERROR_OK; +} + +/* The allow side of the same mask: revoke omits NONMODIFIABLE so a revoked, + * committed key can be revoked again. Asserted against the stored object that + * commit refuses, so the two arms are pinned on identical input. */ +static int _whTest_NvmPolicyRevokeNonModifiableAllowed(whServerContext* server) +{ + whNvmMetadata meta[1]; + whNvmMetadata storedMeta[1]; + uint8_t keyData[WH_TEST_NVMPOL_KEYLEN]; + whKeyId keyId; + int i; + + for (i = 0; i < (int)sizeof(keyData); i++) { + keyData[i] = (uint8_t)(0xC0 + i); + } + + keyId = WH_MAKE_KEYID(WH_KEYTYPE_CRYPTO, WH_TEST_DEFAULT_CLIENT_ID, 0x37); + + /* Commit an ordinary key, then revoke it: the revoke marks the stored + * object NONMODIFIABLE */ + memset(meta, 0, sizeof(meta)); + meta->id = keyId; + 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)); + WH_TEST_ASSERT_RETURN( + WH_ERROR_OK == wh_Server_KeystoreCommitKeyChecked(server, keyId)); + WH_TEST_ASSERT_RETURN(WH_ERROR_OK == + wh_Server_KeystoreRevokeKey(server, keyId)); + + WH_TEST_ASSERT_RETURN(WH_ERROR_OK == + wh_Nvm_GetMetadata(server->nvm, keyId, storedMeta)); + WH_TEST_ASSERT_RETURN((storedMeta->flags & WH_NVM_FLAGS_NONMODIFIABLE) != 0); + + /* Commit is now refused on that object, but revoke is not */ + WH_TEST_ASSERT_RETURN( + WH_ERROR_ACCESS == wh_Server_KeystoreCommitKeyChecked(server, keyId)); + WH_TEST_ASSERT_RETURN(WH_ERROR_OK == + wh_Server_KeystoreRevokeKey(server, keyId)); + + /* Still permitted once the cache slot is gone and the stored flags alone + * decide */ + (void)wh_Server_KeystoreEvictKey(server, keyId); + WH_TEST_ASSERT_RETURN(WH_ERROR_OK == + wh_Server_KeystoreRevokeKey(server, keyId)); + + /* Unchecked teardown: the checked destroy refuses a revoked object */ + (void)wh_Server_KeystoreEvictKey(server, keyId); + WH_TEST_ASSERT_RETURN(WH_ERROR_OK == + wh_Nvm_DestroyObjects(server->nvm, 1, &keyId)); + + return WH_ERROR_OK; +} + int whTest_NvmPolicyChecked(whServerContext* ctx) { if (ctx == NULL) { @@ -223,6 +490,11 @@ int whTest_NvmPolicyChecked(whServerContext* 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)); + WH_TEST_RETURN_ON_FAIL(_whTest_NvmPolicyCommitTrustedDenied(ctx)); + WH_TEST_RETURN_ON_FAIL(_whTest_NvmPolicyCommitNonModifiableDenied(ctx)); + WH_TEST_RETURN_ON_FAIL(_whTest_NvmPolicyUnreadableFlagsDenied(ctx)); + WH_TEST_RETURN_ON_FAIL(_whTest_NvmPolicyRevokeTrustedDenied(ctx)); + WH_TEST_RETURN_ON_FAIL(_whTest_NvmPolicyRevokeNonModifiableAllowed(ctx)); return WH_ERROR_OK; } diff --git a/wolfhsm/wh_client.h b/wolfhsm/wh_client.h index e20475ec5..0c072f8a4 100644 --- a/wolfhsm/wh_client.h +++ b/wolfhsm/wh_client.h @@ -955,7 +955,9 @@ int wh_Client_KeyCommitResponse(whClientContext* c); * @param[in] c Pointer to the client context. * @param[in] keyId Key ID to be committed. Committing a key means making it * persistent in non-volatile memory. - * @return int Returns 0 on success, or a negative error code on failure. + * @return int 0 on success, else a negative error code. WH_ERROR_ACCESS if a + * non-modifiable or server-trusted object already exists under keyId. + * WH_ERROR_ABORTED if the stored flags are unreadable. */ int wh_Client_KeyCommit(whClientContext* c, whNvmId keyId); @@ -1035,7 +1037,9 @@ int wh_Client_KeyRevokeResponse(whClientContext* c); * * @param[in] c Pointer to the client context. * @param[in] keyId Key ID to be revoked. - * @return int Returns 0 on success, or a negative error code on failure. + * @return int 0 on success, else a negative error code. WH_ERROR_ACCESS if the + * stored object carries WH_NVM_FLAGS_TRUSTED. WH_ERROR_ABORTED if the stored + * flags are unreadable. */ int wh_Client_KeyRevoke(whClientContext* c, whKeyId keyId); diff --git a/wolfhsm/wh_server_keystore.h b/wolfhsm/wh_server_keystore.h index 89f153c13..01c5ea8e7 100644 --- a/wolfhsm/wh_server_keystore.h +++ b/wolfhsm/wh_server_keystore.h @@ -196,7 +196,16 @@ int wh_Server_KeystoreCommitKey(whServerContext* server, whNvmId keyId); /** * @brief Commit a cached key to NVM with policy enforcement * - * Runs keystore policy checks before committing. + * Runs keystore policy checks before committing. Caller must hold the NVM lock. + * + * @param[in] server Server context + * @param[in] keyId Key ID to commit + * @return WH_ERROR_OK on success + * @return WH_ERROR_ACCESS if an NVM object already exists under keyId and + * carries WH_NVM_FLAGS_NONMODIFIABLE or WH_NVM_FLAGS_TRUSTED + * @return WH_ERROR_NOTFOUND if the key is not resident in the cache + * @return the NVM layer's error, typically WH_ERROR_ABORTED, if the stored + * flags cannot be read */ int wh_Server_KeystoreCommitKeyChecked(whServerContext* server, whNvmId keyId); @@ -228,7 +237,14 @@ int wh_Server_KeystoreEraseKeyChecked(whServerContext* server, whNvmId keyId); /** * @brief Revoke a key (clears usage and marks non-modifiable) * - * Placeholder implementation for key revocation. + * Caller must hold the NVM lock + * + * @param[in] server Server context + * @param[in] keyId Key ID to revoke + * @return 0 on success. WH_ERROR_ACCESS if the stored object carries + * WH_NVM_FLAGS_TRUSTED. The NVM layer's error, typically + * WH_ERROR_ABORTED, if the stored flags cannot be read. Other error + * codes on failure. */ int wh_Server_KeystoreRevokeKey(whServerContext* server, whKeyId keyId);