Enforce NONMODIFIABLE and TRUSTED NVM policy on keystore key commit

.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
 

src/wh_server_keystore.c

@@ -246,8 +246,30 @@ static int _KeystoreCheckPolicy(whServerContext* server, whKsOp op,
             break;
 
         case WH_KS_OP_COMMIT:
+            /* Stored flags decide, not cached, so an unchecked cache path
+             * cannot launder them; same pair as wh_Nvm_AddObjectChecked.
+             * Fetched here too, so the verdict ignores cache residency. */
+            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) {
+                    /* Unreadable flags cannot be enforced: fail the commit. */
+                    return ret;
+                }
+            }
+            if (foundInNvm &&
+                (nvmMeta.flags &
+                 (WH_NVM_FLAGS_NONMODIFIABLE | WH_NVM_FLAGS_TRUSTED))) {
+                return WH_ERROR_ACCESS;
+            }
+            break;
+
         case WH_KS_OP_REVOKE:
-            /* Always allowed */
+            /* No flag gate: _revokeKey only sets NONMODIFIABLE and clears
+             * usage flags, so revocation never grants new access. The stored
+             * object is still rewritten from the cache slot. */
             break;
         default:
             /* unknown operation */

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 suites 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 run them; without it they report SKIPPED.
+
 ## 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` |

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",

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,13 +848,149 @@ 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");
+
+    /* The key cannot be erased: wh_Nvm_DestroyObjectsChecked refuses a
+     * NONMODIFIABLE object, so only the cache slot is reclaimed here. */
+    (void)wh_Client_KeyEvict(ctx, keyId);
+
+    /* 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
      * to the std path so this suite runs the same way in every config. */
     (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;
 }
 

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
 

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

test-refactor/server/wh_test_nvm_policy.c

@@ -213,6 +213,57 @@ 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;
+}
+
 int whTest_NvmPolicyChecked(whServerContext* ctx)
 {
     if (ctx == NULL) {
@@ -223,6 +274,7 @@ 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));
 
     return WH_ERROR_OK;
 }