Skip to content

Commit d111d86

Browse files
Enforce NONMODIFIABLE and TRUSTED NVM policy on keystore key commit
1 parent 67aecaa commit d111d86

10 files changed

Lines changed: 519 additions & 13 deletions

File tree

.github/workflows/build-and-test-refactor.yml

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,12 @@ jobs:
3535
fromJSON('[{"os":"ubuntu-latest","group":"pq-dma"},
3636
{"os":"ubuntu-latest","group":"wolfcrypt"},
3737
{"os":"ubuntu-latest","group":"threadsafe"},
38+
{"os":"ubuntu-latest","group":"nvm-persist"},
3839
{"os":"ubuntu-latest","group":"base"}]') ||
3940
fromJSON('[{"os":"ubuntu-latest","group":"pq-dma"},
4041
{"os":"ubuntu-latest","group":"wolfcrypt"},
4142
{"os":"ubuntu-latest","group":"threadsafe"},
43+
{"os":"ubuntu-latest","group":"nvm-persist"},
4244
{"os":"ubuntu-latest","group":"base"},
4345
{"os":"macos-latest","group":"base"}]') }}
4446
@@ -191,6 +193,17 @@ jobs:
191193
if: matrix.group == 'base'
192194
run: cd test-refactor/posix && make clean && make -j AUTH=1 NOCRYPTO=1 WOLFSSL_DIR=../../wolfssl && make run
193195

196+
# Suites that leave undeletable NVM objects. The NVM is RAM-backed and
197+
# fresh per process, so they only occupy slots within one `make run`.
198+
- name: Build and test refactor with persistent NVM artifacts
199+
if: matrix.group == 'nvm-persist'
200+
run: cd test-refactor/posix && make clean && make -j ASAN=1 PERSISTENT_NVM_ARTIFACTS=1 WOLFSSL_DIR=../../wolfssl && make run
201+
202+
# Same with DMA, so the gated suites also cover the DMA dispatch path
203+
- name: Build and test refactor with persistent NVM artifacts and DMA
204+
if: matrix.group == 'nvm-persist'
205+
run: cd test-refactor/posix && make clean && make -j DMA=1 ASAN=1 PERSISTENT_NVM_ARTIFACTS=1 WOLFSSL_DIR=../../wolfssl && make run
206+
194207
- name: Show ccache stats
195208
run: ccache -s
196209

src/wh_server_keystore.c

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,7 @@ static int _KeystoreCheckPolicy(whServerContext* server, whKsOp op,
154154
whNvmMetadata* cacheMeta = NULL;
155155
whNvmMetadata nvmMeta;
156156
whNvmFlags flags;
157+
whNvmFlags denyMask;
157158
int ret;
158159
int foundInCache = 0;
159160
int foundInNvm = 0;
@@ -247,7 +248,25 @@ static int _KeystoreCheckPolicy(whServerContext* server, whKsOp op,
247248

248249
case WH_KS_OP_COMMIT:
249250
case WH_KS_OP_REVOKE:
250-
/* Always allowed */
251+
/* Both rewrite the stored object from the cache slot, so the
252+
* stored flags decide; a cached copy cannot launder them. */
253+
if (!foundInNvm && (server->nvm != NULL)) {
254+
ret = wh_Nvm_GetMetadata(server->nvm, keyId, &nvmMeta);
255+
if (ret == WH_ERROR_OK) {
256+
foundInNvm = 1;
257+
}
258+
else if (ret != WH_ERROR_NOTFOUND) {
259+
return ret; /* unreadable flags: deny the write */
260+
}
261+
}
262+
/* Revoke leaves NONMODIFIABLE permitted so an already-revoked key
263+
* can be revoked again. */
264+
denyMask = (op == WH_KS_OP_COMMIT)
265+
? (WH_NVM_FLAGS_NONMODIFIABLE | WH_NVM_FLAGS_TRUSTED)
266+
: WH_NVM_FLAGS_TRUSTED;
267+
if (foundInNvm && ((nvmMeta.flags & denyMask) != 0)) {
268+
return WH_ERROR_ACCESS;
269+
}
251270
break;
252271
default:
253272
/* unknown operation */

test-refactor/README.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ The top-level `make` forwards to the POSIX port; `cd test-refactor/posix && make
2525

2626
Results are printed via `WOLFHSM_CFG_PRINTF` from the wolfHSM build. `test-suite.log` contains the detailed output.
2727

28+
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. Without it they are compiled out of their parent suite, which still runs and passes; unlike a whole entry point that is compiled out, no SKIPPED line is printed for them.
29+
2830
## Running the tests from an embedded target
2931
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.
3032

@@ -87,7 +89,7 @@ Translated tests:
8789
| `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) |
8890
| `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 |
8991
| `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 |
90-
| `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) |
92+
| `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` |
9193
| `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 |
9294
| `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 |
9395
| `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

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -506,7 +506,8 @@ static int _whTest_CryptoKeyUsagePolicies(whClientContext* client)
506506

507507
#if !defined(NO_AES) && defined(HAVE_AES_CBC) && \
508508
defined(WOLFHSM_CFG_TEST_ALLOW_PERSISTENT_NVM_ARTIFACTS)
509-
static int whTest_RevocationTryAESEncrypt(whKeyId keyId, WC_RNG* rng,
509+
static int whTest_RevocationTryAESEncrypt(whClientContext* client,
510+
whKeyId keyId, WC_RNG* rng,
510511
int* encryptRes)
511512
{
512513
int ret;
@@ -584,7 +585,7 @@ static int _whTest_CryptoKeyRevocationAesCbc(whClientContext* client)
584585
return ret;
585586
}
586587

587-
ret = whTest_RevocationTryAESEncrypt(keyId, rng, &encryptRes);
588+
ret = whTest_RevocationTryAESEncrypt(client, keyId, rng, &encryptRes);
588589
if (ret != 0) {
589590
WH_ERROR_PRINT("Failed to encrypt with unrevoked AES key: %d\n", ret);
590591
(void)wh_Client_KeyEvict(client, keyId);
@@ -605,7 +606,7 @@ static int _whTest_CryptoKeyRevocationAesCbc(whClientContext* client)
605606
return ret;
606607
}
607608

608-
ret = whTest_RevocationTryAESEncrypt(keyId, rng, &encryptRes);
609+
ret = whTest_RevocationTryAESEncrypt(client, keyId, rng, &encryptRes);
609610
if (ret != 0 || encryptRes != WH_ERROR_USAGE) {
610611
WH_ERROR_PRINT(
611612
"Encrypt with revoked AES key should fail (%d), got %d\n",
@@ -621,7 +622,7 @@ static int _whTest_CryptoKeyRevocationAesCbc(whClientContext* client)
621622
return ret;
622623
}
623624

624-
ret = whTest_RevocationTryAESEncrypt(keyId, rng, &encryptRes);
625+
ret = whTest_RevocationTryAESEncrypt(client, keyId, rng, &encryptRes);
625626
if (ret != 0 || encryptRes != WH_ERROR_USAGE) {
626627
WH_ERROR_PRINT(
627628
"Encrypt with revoked AES key should fail (%d), got %d\n",
@@ -653,7 +654,7 @@ static int _whTest_CryptoKeyRevocationAesCbc(whClientContext* client)
653654
(void)wc_FreeRng(rng);
654655
return ret;
655656
}
656-
ret = whTest_RevocationTryAESEncrypt(keyId, rng, &encryptRes);
657+
ret = whTest_RevocationTryAESEncrypt(client, keyId, rng, &encryptRes);
657658
if (ret != 0 || encryptRes != 0) {
658659
WH_ERROR_PRINT(
659660
"Failed to encrypt with unrevoked AES key (2nd time): %d\n", ret);
@@ -673,7 +674,7 @@ static int _whTest_CryptoKeyRevocationAesCbc(whClientContext* client)
673674
(void)wc_FreeRng(rng);
674675
return ret;
675676
}
676-
ret = whTest_RevocationTryAESEncrypt(keyId, rng, &encryptRes);
677+
ret = whTest_RevocationTryAESEncrypt(client, keyId, rng, &encryptRes);
677678
if (ret != 0 || encryptRes != WH_ERROR_USAGE) {
678679
WH_ERROR_PRINT(
679680
"Encrypt with revoked AES key should fail (%d), got %d\n",

test-refactor/client-server/wh_test_crypto_keystore.c

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,15 @@
2828
* _whTest_NonExportableKeystore - confirm WH_NVM_FLAGS_NONEXPORTABLE keys
2929
* cannot be exported while ordinary keys can
3030
* (std and DMA export paths)
31+
* _whTest_NonModifiableCommit - re-commit over a stored
32+
* WH_NVM_FLAGS_NONMODIFIABLE object is
33+
* denied, cached or not, and the stored key
34+
* and label survive the denial
35+
* _whTest_ModifiableRecommit - a key without the flag still re-commits
36+
*
37+
* _whTest_NonModifiableCommit is gated by
38+
* WOLFHSM_CFG_TEST_ALLOW_PERSISTENT_NVM_ARTIFACTS: the object it commits
39+
* cannot be erased, so it holds an NVM slot for the rest of the run.
3140
*/
3241

3342
#include "wolfhsm/wh_settings.h"
@@ -839,13 +848,155 @@ static int _whTest_NonExportableKeystore(whClientContext* ctx)
839848
return 0;
840849
}
841850

851+
#if defined(WOLFHSM_CFG_TEST_ALLOW_PERSISTENT_NVM_ARTIFACTS)
852+
/* Committing a NONMODIFIABLE key leaves an object that
853+
* wh_Nvm_DestroyObjectsChecked refuses to erase, so it occupies one NVM
854+
* slot for the rest of the run. Gated like the keypolicy revocation test. */
855+
static int _whTest_NonModifiableCommit(whClientContext* ctx)
856+
{
857+
int ret = 0;
858+
whKeyId keyId = WH_KEYID_ERASED;
859+
uint8_t key[WH_TEST_KEYSTORE_TEST_SZ] = {
860+
0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF, 0x01, 0x23, 0x45,
861+
0x67, 0x89, 0xAB, 0xCD, 0xEF, 0xFE, 0xDC, 0xBA, 0x98, 0x76, 0x54,
862+
0x32, 0x10, 0xFE, 0xDC, 0xBA, 0x98, 0x76, 0x54, 0x32, 0x10};
863+
uint8_t exportedKey[WH_TEST_KEYSTORE_TEST_SZ] = {0};
864+
uint8_t label[WH_NVM_LABEL_LEN] = "NonModifiableCommitKey";
865+
uint8_t exportedLabel[WH_NVM_LABEL_LEN] = {0};
866+
uint16_t exportedKeySize;
867+
868+
WH_TEST_PRINT("Testing non-modifiable commit enforcement...\n");
869+
870+
/* Test 1: first commit of a NONMODIFIABLE key stores it, and the commit
871+
* leaves the slot cached, so a repeat commit is an overwrite attempt. */
872+
ret = wh_Client_KeyCache(ctx, WH_NVM_FLAGS_NONMODIFIABLE, label,
873+
sizeof(label), key, sizeof(key), &keyId);
874+
if (ret != 0) {
875+
WH_ERROR_PRINT("Failed to cache non-modifiable key: %d\n", ret);
876+
return ret;
877+
}
878+
879+
ret = wh_Client_KeyCommit(ctx, keyId);
880+
if (ret != 0) {
881+
WH_ERROR_PRINT("Failed first commit of non-modifiable key: %d\n", ret);
882+
return ret;
883+
}
884+
885+
/* Test 2: re-committing over the stored non-modifiable object is denied */
886+
ret = wh_Client_KeyCommit(ctx, keyId);
887+
if (ret != WH_ERROR_ACCESS) {
888+
WH_ERROR_PRINT("Non-modifiable key was re-committed unexpectedly: %d\n",
889+
ret);
890+
return -1;
891+
}
892+
893+
WH_TEST_DEBUG_PRINT("Non-modifiable key re-commit correctly denied\n");
894+
895+
/* Test 3: the denial left the stored object intact. Evicting is allowed
896+
* because the key is committed, so the export below must freshen it back
897+
* out of NVM rather than read the surviving cache slot. */
898+
ret = wh_Client_KeyEvict(ctx, keyId);
899+
if (ret != 0) {
900+
WH_ERROR_PRINT("Failed to evict committed non-modifiable key: %d\n",
901+
ret);
902+
return ret;
903+
}
904+
905+
exportedKeySize = sizeof(exportedKey);
906+
ret = wh_Client_KeyExport(ctx, keyId, exportedLabel, sizeof(exportedLabel),
907+
exportedKey, &exportedKeySize);
908+
if (ret != 0) {
909+
WH_ERROR_PRINT("Failed to export stored non-modifiable key: %d\n", ret);
910+
return ret;
911+
}
912+
913+
if (exportedKeySize != sizeof(key) ||
914+
memcmp(key, exportedKey, exportedKeySize) != 0 ||
915+
memcmp(label, exportedLabel, sizeof(label)) != 0) {
916+
WH_ERROR_PRINT("Denied commit altered the stored key\n");
917+
return -1;
918+
}
919+
920+
WH_TEST_DEBUG_PRINT("Stored non-modifiable key unchanged after denial\n");
921+
922+
/* Evicting reclaims only the cache slot; wh_Nvm_DestroyObjectsChecked
923+
* refuses the NONMODIFIABLE object. Checked so Test 4 cannot degrade into
924+
* a repeat of Test 2 with the slot still resident. */
925+
ret = wh_Client_KeyEvict(ctx, keyId);
926+
if (ret != 0) {
927+
WH_ERROR_PRINT("Failed to evict before uncached commit check: %d\n",
928+
ret);
929+
return ret;
930+
}
931+
932+
/* Test 4: the denial does not depend on cache residency. With no slot
933+
* left, the stored flags still decide, so commit reports ACCESS rather
934+
* than the NOTFOUND raised by the missing slot. */
935+
ret = wh_Client_KeyCommit(ctx, keyId);
936+
if (ret != WH_ERROR_ACCESS) {
937+
WH_ERROR_PRINT("Uncached non-modifiable commit not denied: %d\n", ret);
938+
return -1;
939+
}
940+
941+
WH_TEST_DEBUG_PRINT("Uncached non-modifiable commit correctly denied\n");
942+
943+
WH_TEST_PRINT("NON-MODIFIABLE COMMIT TEST SUCCESS\n");
944+
return 0;
945+
}
946+
#endif /* WOLFHSM_CFG_TEST_ALLOW_PERSISTENT_NVM_ARTIFACTS */
947+
948+
static int _whTest_ModifiableRecommit(whClientContext* ctx)
949+
{
950+
int ret = 0;
951+
whKeyId keyId = WH_KEYID_ERASED;
952+
uint8_t key[WH_TEST_KEYSTORE_TEST_SZ] = {
953+
0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF, 0x01, 0x23, 0x45,
954+
0x67, 0x89, 0xAB, 0xCD, 0xEF, 0xFE, 0xDC, 0xBA, 0x98, 0x76, 0x54,
955+
0x32, 0x10, 0xFE, 0xDC, 0xBA, 0x98, 0x76, 0x54, 0x32, 0x10};
956+
uint8_t label[WH_NVM_LABEL_LEN] = "ModifiableCommitKey";
957+
958+
WH_TEST_PRINT("Testing modifiable commit is unaffected...\n");
959+
960+
/* A key without the flag still commits repeatedly */
961+
ret = wh_Client_KeyCache(ctx, WH_NVM_FLAGS_NONE, label, sizeof(label), key,
962+
sizeof(key), &keyId);
963+
if (ret != 0) {
964+
WH_ERROR_PRINT("Failed to cache modifiable key: %d\n", ret);
965+
return ret;
966+
}
967+
968+
ret = wh_Client_KeyCommit(ctx, keyId);
969+
if (ret != 0) {
970+
WH_ERROR_PRINT("Failed first commit of modifiable key: %d\n", ret);
971+
return ret;
972+
}
973+
974+
ret = wh_Client_KeyCommit(ctx, keyId);
975+
if (ret != 0) {
976+
WH_ERROR_PRINT("Failed repeat commit of modifiable key: %d\n", ret);
977+
return ret;
978+
}
979+
980+
WH_TEST_DEBUG_PRINT("Modifiable key repeat commit allowed\n");
981+
982+
/* Clean up */
983+
(void)wh_Client_KeyErase(ctx, keyId);
984+
985+
WH_TEST_PRINT("MODIFIABLE COMMIT TEST SUCCESS\n");
986+
return 0;
987+
}
988+
842989
int whTest_Crypto_Keystore(whClientContext* ctx)
843990
{
844991
/* A preceding suite may leave the DMA-preferred dispatch mode set; reset
845992
* to the std path so this suite runs the same way in every config. */
846993
(void)wh_Client_SetDmaMode(ctx, 0);
847994
WH_TEST_RETURN_ON_FAIL(_whTest_KeyCache(ctx));
848995
WH_TEST_RETURN_ON_FAIL(_whTest_NonExportableKeystore(ctx));
996+
#if defined(WOLFHSM_CFG_TEST_ALLOW_PERSISTENT_NVM_ARTIFACTS)
997+
WH_TEST_RETURN_ON_FAIL(_whTest_NonModifiableCommit(ctx));
998+
#endif
999+
WH_TEST_RETURN_ON_FAIL(_whTest_ModifiableRecommit(ctx));
8491000
return 0;
8501001
}
8511002

test-refactor/config/wolfhsm_cfg.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,8 @@
5656
#define WOLFHSM_CFG_SERVER_NVM_FLASH_LOG
5757

5858
/* WOLFHSM_CFG_TEST_ALLOW_PERSISTENT_NVM_ARTIFACTS is intentionally NOT
59-
* defined here. Not implemented yet. */
59+
* defined here: one NVM is shared by every test in a run. The persistent
60+
* NVM artifacts CI job defines it on the command line instead. */
6061

6162
#define WOLFHSM_CFG_ENABLE_TIMEOUT
6263

test-refactor/posix/Makefile

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,13 @@ ifeq ($(CRYPTO_AFFINITY),1)
152152
DEF += -DWOLFHSM_CFG_CRYPTO_AFFINITY
153153
endif
154154

155+
# Enable the test suites that leave undeletable NVM objects behind. One NVM is
156+
# shared by every test in a run, so those objects occupy slots for the rest of
157+
# the run.
158+
ifeq ($(PERSISTENT_NVM_ARTIFACTS),1)
159+
DEF += -DWOLFHSM_CFG_TEST_ALLOW_PERSISTENT_NVM_ARTIFACTS
160+
endif
161+
155162
# Build the wolfCrypt test suite as a wolfHSM client
156163
ifeq ($(TESTWOLFCRYPT),1)
157164
DEF += -DWOLFHSM_CFG_TEST_WOLFCRYPTTEST

0 commit comments

Comments
 (0)