From a3f17928863011a267dc4a7272f37708a02a9f0c Mon Sep 17 00:00:00 2001 From: Marco Oliverio Date: Thu, 9 Jul 2026 16:10:37 +0200 Subject: [PATCH 1/4] mldsa: fix usage flags in ml-dsa DMA test --- test/wh_test_crypto.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/wh_test_crypto.c b/test/wh_test_crypto.c index a5850cde5..db18c9341 100644 --- a/test/wh_test_crypto.c +++ b/test/wh_test_crypto.c @@ -12184,9 +12184,11 @@ int whTestCrypto_MlDsaVerifyOnlyDma(whClientContext* ctx, int devId, } } /* Import the key into wolfHSM via the wolfCrypt structure. This is the - * DMA-only verify test, so always import via the DMA path. */ + * DMA-only verify test, so always import via the DMA path. The key must + * carry the verify usage flag, which the DMA verify handler enforces. */ if (ret == 0) { - ret = wh_Client_MlDsaImportKeyDma(ctx, key, &keyId, 0, 0, NULL); + ret = wh_Client_MlDsaImportKeyDma(ctx, key, &keyId, + WH_NVM_FLAGS_USAGE_VERIFY, 0, NULL); if (ret == WH_ERROR_OK) { evictKey = 1; } From 283662b24c4c625e1ac688032b66e3d5947603cc Mon Sep 17 00:00:00 2001 From: Marco Oliverio Date: Tue, 30 Jun 2026 10:07:08 +0200 Subject: [PATCH 2/4] widetree: add and consolidate locking in crypto layer The crypto layer uses the keystore cache/nvm without locking. While the keystore cache is shared only for global keys, the NVM is shared across all clients for any keyId, so operations that might touch the NVM layer must be locked. Also, check + usage type of operations (check enforcement flags, then use the key) must held the lock over the all operation, otherwise a key can change flags between check and usage. This can happen only with global keys. --- src/wh_server_cert.c | 8 + src/wh_server_crypto.c | 1218 ++++++++++------- src/wh_server_keystore.c | 61 +- test-refactor/posix/Makefile | 22 +- .../posix/wh_test_keygen_unique_id.c | 699 ++++++++++ .../posix/wh_test_keygen_unique_id.h | 34 + test-refactor/posix/wh_test_keyread_race.c | 651 +++++++++ test-refactor/posix/wh_test_keyread_race.h | 34 + test-refactor/posix/wh_test_posix_main.c | 16 + test/wh_test_crypto.c | 4 +- wolfhsm/wh_server_crypto.h | 41 + wolfhsm/wh_server_keystore.h | 48 + wolfhsm/wh_settings.h | 14 + 13 files changed, 2334 insertions(+), 516 deletions(-) create mode 100644 test-refactor/posix/wh_test_keygen_unique_id.c create mode 100644 test-refactor/posix/wh_test_keygen_unique_id.h create mode 100644 test-refactor/posix/wh_test_keyread_race.c create mode 100644 test-refactor/posix/wh_test_keyread_race.h diff --git a/src/wh_server_cert.c b/src/wh_server_cert.c index db0871605..f72257c04 100644 --- a/src/wh_server_cert.c +++ b/src/wh_server_cert.c @@ -481,11 +481,19 @@ static int _verifyChainAgainstCmStore( } /* This is the leaf cert, so if requested, cache the public key */ else if (flags & WH_CERT_FLAGS_CACHE_LEAF_PUBKEY) { + /* The whole id allocation + cache import chain below is already + * atomic: every caller of this function reaches it through the + * cert request dispatch, which holds WH_SERVER_NVM_LOCK across + * the entire verify call. The NVM lock is non-recursive, so we + * must NOT re-acquire it here -- use the unlocked GetUniqueId + + * GetCacheSlotChecked building blocks under the caller's lock. + */ /* If the keyId is erased, get a unique key id for the public * key. Otherwise cache the key using the provided keyId */ if (WH_KEYID_ISERASED(*inout_keyId)) { rc = wh_Server_KeystoreGetUniqueId(server, inout_keyId); if (rc != WH_ERROR_OK) { + wc_FreeDecodedCert(&dc); return rc; } } diff --git a/src/wh_server_crypto.c b/src/wh_server_crypto.c index 5755676c3..bbe9e88ce 100644 --- a/src/wh_server_crypto.c +++ b/src/wh_server_crypto.c @@ -227,6 +227,21 @@ static int _HandleMlKemDecapsDma(whServerContext* ctx, uint16_t magic, #endif /* WOLFHSM_CFG_DMA */ #endif /* WOLFSSL_HAVE_MLKEM */ +static void _CryptoEvictKeyLocked(whServerContext* ctx, whKeyId keyId) +{ + int ret; + + if ((ctx == NULL) || WH_KEYID_ISERASED(keyId)) { + return; + } + + ret = WH_SERVER_NVM_LOCK(ctx); + if (ret == WH_ERROR_OK) { + (void)wh_Server_KeystoreEvictKey(ctx, keyId); + (void)WH_SERVER_NVM_UNLOCK(ctx); + } +} + /** Public server crypto functions */ #ifndef NO_RSA @@ -298,6 +313,34 @@ int wh_Server_CacheExportRsaKey(whServerContext* ctx, whKeyId keyId, return ret; } +int wh_Server_CacheExportRsaKeyEnforce(whServerContext* ctx, whKeyId keyId, + whNvmFlags requiredUsage, RsaKey* key) +{ + uint8_t* cacheBuf; + whNvmMetadata* cacheMeta; + int ret; + + if ((ctx == NULL) || (key == NULL) || (WH_KEYID_ISERASED(keyId))) { + return WH_ERROR_BADARGS; + } + /* Freshen, check usage and deserialize under one hold of the NVM lock so + * the policy verdict, the metadata length and the key bytes all come from + * the same snapshot of the shared cache slot. This matter for the global + * shared cache. */ + ret = WH_SERVER_NVM_LOCK(ctx); + if (ret == WH_ERROR_OK) { + ret = wh_Server_KeystoreFreshenKey(ctx, keyId, &cacheBuf, &cacheMeta); + if (ret == WH_ERROR_OK) { + ret = wh_Server_KeystoreEnforceKeyUsage(cacheMeta, requiredUsage); + } + if (ret == WH_ERROR_OK) { + ret = wh_Crypto_RsaDeserializeKeyDer(cacheMeta->len, cacheBuf, key); + } + (void)WH_SERVER_NVM_UNLOCK(ctx); + } + return ret; +} + #ifdef WOLFSSL_KEY_GEN static int _HandleRsaKeyGen(whServerContext* ctx, uint16_t magic, int devId, const void* cryptoDataIn, uint16_t inSize, @@ -356,21 +399,24 @@ static int _HandleRsaKeyGen(whServerContext* ctx, uint16_t magic, int devId, } else { /* Must import the key into the cache and return keyid */ - if (WH_KEYID_ISERASED(key_id)) { - /* Generate a new id */ - ret = wh_Server_KeystoreGetUniqueId(ctx, &key_id); - WH_DEBUG_SERVER_VERBOSE("RsaKeyGen UniqueId: keyId:%u, ret:%d\n", key_id, ret); - if (ret != WH_ERROR_OK) { - /* Early return on unique ID generation failure */ - wc_FreeRsaKey(rsa); - return ret; + /* Hold the NVM lock so id allocation and cache import are + * atomic with respect to other server contexts under + * THREADSAFE. This matter for the shared global cache. */ + ret = WH_SERVER_NVM_LOCK(ctx); + if (ret == WH_ERROR_OK) { + if (WH_KEYID_ISERASED(key_id)) { + /* Generate a new id */ + ret = wh_Server_KeystoreGetUniqueId(ctx, &key_id); + WH_DEBUG_SERVER_VERBOSE( + "RsaKeyGen UniqueId: keyId:%u, ret:%d\n", key_id, + ret); } - } - - if (ret == 0) { - ret = wh_Server_CacheImportRsaKey(ctx, rsa, key_id, flags, - label_size, label); - } + if (ret == WH_ERROR_OK) { + ret = wh_Server_CacheImportRsaKey( + ctx, rsa, key_id, flags, label_size, label); + } + (void)WH_SERVER_NVM_UNLOCK(ctx); + } /* WH_SERVER_NVM_LOCK() */ WH_DEBUG_SERVER_VERBOSE("RsaKeyGen CacheKeyRsa: keyId:%u, ret:%d\n", key_id, ret); if (ret == 0) { res.keyId = wh_KeyId_TranslateToClient(key_id); @@ -448,66 +494,60 @@ static int _HandleRsaFunction(whServerContext* ctx, uint16_t magic, int devId, return BAD_FUNC_ARG; } - /* Validate key usage policy based on RSA operation type */ - if (!WH_KEYID_ISERASED(key_id)) { - whNvmFlags requiredUsage = WH_NVM_FLAGS_NONE; - switch (op_type) { - case RSA_PUBLIC_ENCRYPT: - case RSA_PRIVATE_ENCRYPT: - requiredUsage = WH_NVM_FLAGS_USAGE_ENCRYPT; - break; - case RSA_PUBLIC_DECRYPT: - case RSA_PRIVATE_DECRYPT: - requiredUsage = WH_NVM_FLAGS_USAGE_DECRYPT; - break; - } - ret = wh_Server_KeystoreFindEnforceKeyUsage(ctx, key_id, requiredUsage); - if (ret != WH_ERROR_OK) { - /* Currently wolfCrypt doesn't have a way for crypto callbacks to - distinguish if a low level RSA operation (like encrypt/decrypt) is - being performed as part of a higher level operation like - sign/verify. Until that information is propagated to the - callback, the usage flags are treated as equivalent. */ - if (ret == WH_ERROR_USAGE) { - if (op_type == RSA_PUBLIC_DECRYPT) { - /* Decrypt usage flag wasn't set so this might be a verify - * operation. Attempt to enforce against the verify flag */ - ret = wh_Server_KeystoreFindEnforceKeyUsage( - ctx, key_id, WH_NVM_FLAGS_USAGE_VERIFY); - } - else if (op_type == RSA_PRIVATE_ENCRYPT) { - /* Encrypt usage flag wasn't set so this might be a sign - * operation. Attempt to enforce against the sign flag */ - ret = wh_Server_KeystoreFindEnforceKeyUsage( - ctx, key_id, WH_NVM_FLAGS_USAGE_SIGN); - } - } - if (ret != WH_ERROR_OK) { - goto cleanup; - } - } + /* Determine the required key usage based on the RSA operation type */ + whNvmFlags requiredUsage = WH_NVM_FLAGS_NONE; + switch (op_type) { + case RSA_PUBLIC_ENCRYPT: + case RSA_PRIVATE_ENCRYPT: + requiredUsage = WH_NVM_FLAGS_USAGE_ENCRYPT; + break; + case RSA_PUBLIC_DECRYPT: + case RSA_PRIVATE_DECRYPT: + requiredUsage = WH_NVM_FLAGS_USAGE_DECRYPT; + break; } /* init rsa key */ ret = wc_InitRsaKey_ex(rsa, NULL, devId); - /* load the key from the keystore */ + /* load the key from the keystore, enforcing the usage policy against the + * same locked snapshot of the key that is exported */ if (ret == 0) { - ret = wh_Server_CacheExportRsaKey(ctx, key_id, rsa); - WH_DEBUG_SERVER_VERBOSE("CacheExportRsaKey keyid:%u, ret:%d\n", key_id, ret); - if (ret == 0) { - /* do the rsa operation */ - ret = wc_RsaFunction(in, in_len, out, &out_len, - op_type, rsa, ctx->crypto->rng); - WH_DEBUG_SERVER_VERBOSE("RsaFunction in:%p %u, out:%p, opType:%d, outLen:%d, ret:%d\n", - in, in_len, out, op_type, out_len, ret); - } - /* free the key */ - wc_FreeRsaKey(rsa); + ret = wh_Server_CacheExportRsaKeyEnforce(ctx, key_id, requiredUsage, rsa); + if (ret == WH_ERROR_USAGE) { + /* Currently wolfCrypt doesn't have a way for crypto callbacks to + distinguish if a low level RSA operation (like encrypt/decrypt) is + being performed as part of a higher level operation like + sign/verify. Until that information is propagated to the + callback, the usage flags are treated as equivalent. */ + if (op_type == RSA_PUBLIC_DECRYPT) { + /* Decrypt usage flag wasn't set so this might be a verify + * operation. Attempt to enforce against the verify flag */ + ret = wh_Server_CacheExportRsaKeyEnforce( + ctx, key_id, WH_NVM_FLAGS_USAGE_VERIFY, rsa); + } + else if (op_type == RSA_PRIVATE_ENCRYPT) { + /* Encrypt usage flag wasn't set so this might be a sign + * operation. Attempt to enforce against the sign flag */ + ret = wh_Server_CacheExportRsaKeyEnforce( + ctx, key_id, WH_NVM_FLAGS_USAGE_SIGN, rsa); + } + } + WH_DEBUG_SERVER_VERBOSE("CacheExportRsaKey keyid:%u, ret:%d\n", key_id, + ret); + if (ret == 0) { + /* do the rsa operation */ + ret = wc_RsaFunction(in, in_len, out, &out_len, op_type, rsa, + ctx->crypto->rng); + WH_DEBUG_SERVER_VERBOSE( + "RsaFunction in:%p %u, out:%p, opType:%d, outLen:%d, ret:%d\n", in, + in_len, out, op_type, out_len, ret); + } + /* free the key */ + wc_FreeRsaKey(rsa); } -cleanup: if (evict != 0) { /* User requested to evict from cache, even if the call failed */ - (void)wh_Server_KeystoreEvictKey(ctx, key_id); + _CryptoEvictKeyLocked(ctx, key_id); } if (ret == 0) { whMessageCrypto_RsaResponse res; @@ -550,9 +590,10 @@ static int _HandleRsaGetSize(whServerContext* ctx, uint16_t magic, int devId, /* init rsa key */ ret = wc_InitRsaKey_ex(rsa, NULL, devId); - /* load the key from the keystore */ + /* load the key from the keystore (no usage requirement for size query) */ if (ret == 0) { - ret = wh_Server_CacheExportRsaKey(ctx, key_id, rsa); + ret = wh_Server_CacheExportRsaKeyEnforce(ctx, key_id, WH_NVM_FLAGS_NONE, + rsa); /* get the size */ if (ret == 0) { key_size = wc_RsaEncryptSize(rsa); @@ -566,7 +607,7 @@ static int _HandleRsaGetSize(whServerContext* ctx, uint16_t magic, int devId, WH_DEBUG_SERVER_VERBOSE("evicting temp key:%x options:%u evict:%u\n", key_id, options, evict); /* User requested to evict from cache, even if the call failed */ - (void)wh_Server_KeystoreEvictKey(ctx, key_id); + _CryptoEvictKeyLocked(ctx, key_id); } if (ret == 0) { res.keySize = key_size; @@ -641,6 +682,34 @@ int wh_Server_EccKeyCacheExport(whServerContext* ctx, whKeyId keyId, } return ret; } + +int wh_Server_EccKeyCacheExportEnforce(whServerContext* ctx, whKeyId keyId, + whNvmFlags requiredUsage, ecc_key* key) +{ + uint8_t* cacheBuf; + whNvmMetadata* cacheMeta; + int ret; + + if ((ctx == NULL) || (key == NULL) || WH_KEYID_ISERASED(keyId)) { + return WH_ERROR_BADARGS; + } + /* Freshen, check usage and deserialize under one hold of the NVM lock so + * the policy verdict, the metadata length and the key bytes all come from + * the same snapshot of the shared cache slot. This mattters for the shared + * global cache. */ + ret = WH_SERVER_NVM_LOCK(ctx); + if (ret == WH_ERROR_OK) { + ret = wh_Server_KeystoreFreshenKey(ctx, keyId, &cacheBuf, &cacheMeta); + if (ret == WH_ERROR_OK) { + ret = wh_Server_KeystoreEnforceKeyUsage(cacheMeta, requiredUsage); + } + if (ret == WH_ERROR_OK) { + ret = wh_Crypto_EccDeserializeKeyDer(cacheBuf, cacheMeta->len, key); + } + (void)WH_SERVER_NVM_UNLOCK(ctx); + } + return ret; +} #endif /* HAVE_ECC */ #ifdef HAVE_ED25519 @@ -698,6 +767,37 @@ int wh_Server_CacheExportEd25519Key(whServerContext* ctx, whKeyId keyId, } return ret; } + +int wh_Server_CacheExportEd25519KeyEnforce(whServerContext* ctx, whKeyId keyId, + whNvmFlags requiredUsage, + ed25519_key* key) +{ + uint8_t* cacheBuf = NULL; + whNvmMetadata* cacheMeta; + int ret; + + if ((ctx == NULL) || (key == NULL) || WH_KEYID_ISERASED(keyId)) { + return WH_ERROR_BADARGS; + } + + /* Freshen, check usage and deserialize under one hold of the NVM lock so + * the policy verdict, the metadata length and the key bytes all come from + * the same snapshot of the shared cache slot. This mattters for the shared + * global cache. */ + ret = WH_SERVER_NVM_LOCK(ctx); + if (ret == WH_ERROR_OK) { + ret = wh_Server_KeystoreFreshenKey(ctx, keyId, &cacheBuf, &cacheMeta); + if (ret == WH_ERROR_OK) { + ret = wh_Server_KeystoreEnforceKeyUsage(cacheMeta, requiredUsage); + } + if (ret == WH_ERROR_OK) { + ret = wh_Crypto_Ed25519DeserializeKeyDer(cacheBuf, cacheMeta->len, + key); + } + (void)WH_SERVER_NVM_UNLOCK(ctx); + } + return ret; +} #endif /* HAVE_ED25519 */ #ifdef HAVE_CURVE25519 @@ -762,6 +862,38 @@ int wh_Server_CacheExportCurve25519Key(whServerContext* server, whKeyId keyId, } return ret; } + +int wh_Server_CacheExportCurve25519KeyEnforce(whServerContext* server, + whKeyId keyId, + whNvmFlags requiredUsage, + curve25519_key* key) +{ + uint8_t* cacheBuf; + whNvmMetadata* cacheMeta; + int ret; + + if ((server == NULL) || (key == NULL) || (WH_KEYID_ISERASED(keyId))) { + return WH_ERROR_BADARGS; + } + /* Freshen, check usage and deserialize under one hold of the NVM lock so + * the policy verdict, the metadata length and the key bytes all come from + * the same snapshot of the shared cache slot. This mattters for the shared + * global cache. */ + ret = WH_SERVER_NVM_LOCK(server); + if (ret == WH_ERROR_OK) { + ret = + wh_Server_KeystoreFreshenKey(server, keyId, &cacheBuf, &cacheMeta); + if (ret == WH_ERROR_OK) { + ret = wh_Server_KeystoreEnforceKeyUsage(cacheMeta, requiredUsage); + } + if (ret == WH_ERROR_OK) { + ret = wh_Crypto_Curve25519DeserializeKey(cacheBuf, cacheMeta->len, + key); + } + (void)WH_SERVER_NVM_UNLOCK(server); + } + return ret; +} #endif /* HAVE_CURVE25519 */ #ifdef WOLFSSL_HAVE_MLDSA @@ -829,6 +961,38 @@ int wh_Server_MlDsaKeyCacheExport(whServerContext* ctx, whKeyId keyId, } return ret; } + +int wh_Server_MlDsaKeyCacheExportEnforce(whServerContext* ctx, whKeyId keyId, + whNvmFlags requiredUsage, + wc_MlDsaKey* key) +{ + uint8_t* cacheBuf; + whNvmMetadata* cacheMeta; + int ret; + + if ((ctx == NULL) || (key == NULL) || (WH_KEYID_ISERASED(keyId))) { + return WH_ERROR_BADARGS; + } + + /* Freshen, check usage and deserialize under one hold of the NVM lock so + * the policy verdict, the metadata length and the key bytes all come from + * the same snapshot of the shared cache slot. This mattters for the shared + * global cache. */ + ret = WH_SERVER_NVM_LOCK(ctx); + if (ret == WH_ERROR_OK) { + ret = wh_Server_KeystoreFreshenKey(ctx, keyId, &cacheBuf, &cacheMeta); + if (ret == WH_ERROR_OK) { + ret = wh_Server_KeystoreEnforceKeyUsage(cacheMeta, requiredUsage); + } + if (ret == WH_ERROR_OK) { + ret = + wh_Crypto_MlDsaDeserializeKeyDer(cacheBuf, cacheMeta->len, key); + WH_DEBUG_SERVER_VERBOSE("keyId:%u, ret:%d\n", keyId, ret); + } + (void)WH_SERVER_NVM_UNLOCK(ctx); + } + return ret; +} #endif /* WOLFSSL_HAVE_MLDSA */ #ifdef WOLFSSL_HAVE_MLKEM @@ -883,6 +1047,37 @@ int wh_Server_MlKemKeyCacheExport(whServerContext* ctx, whKeyId keyId, } return ret; } + +int wh_Server_MlKemKeyCacheExportEnforce(whServerContext* ctx, whKeyId keyId, + whNvmFlags requiredUsage, + MlKemKey* key) +{ + uint8_t* cacheBuf; + whNvmMetadata* cacheMeta; + int ret; + + if ((ctx == NULL) || (key == NULL) || (WH_KEYID_ISERASED(keyId))) { + return WH_ERROR_BADARGS; + } + + /* Freshen, check usage and deserialize under one hold of the NVM lock so + * the policy verdict, the metadata length and the key bytes all come from + * the same snapshot of the shared cache slot. This mattters for the shared + * global cache. */ + ret = WH_SERVER_NVM_LOCK(ctx); + if (ret == WH_ERROR_OK) { + ret = wh_Server_KeystoreFreshenKey(ctx, keyId, &cacheBuf, &cacheMeta); + if (ret == WH_ERROR_OK) { + ret = wh_Server_KeystoreEnforceKeyUsage(cacheMeta, requiredUsage); + } + if (ret == WH_ERROR_OK) { + ret = wh_Crypto_MlKemDeserializeKey(cacheBuf, cacheMeta->len, key); + WH_DEBUG_SERVER_VERBOSE("keyId:%u, ret:%d\n", keyId, ret); + } + (void)WH_SERVER_NVM_UNLOCK(ctx); + } + return ret; +} #endif /* WOLFSSL_HAVE_MLKEM */ /* The sign path (and its slot callbacks) is unavailable in verify-only builds; @@ -1210,20 +1405,24 @@ static int _HandleEccKeyGen(whServerContext* ctx, uint16_t magic, int devId, /* Must import the key into the cache and return keyid */ res_size = 0; - if (WH_KEYID_ISERASED(key_id)) { - /* Generate a new id */ - ret = wh_Server_KeystoreGetUniqueId(ctx, &key_id); - WH_DEBUG_SERVER("UniqueId: keyId:%u, ret:%d\n", key_id, ret); - if (ret != WH_ERROR_OK) { - /* Early return on unique ID generation failure */ - wc_ecc_free(key); - return ret; + /* Hold the NVM lock so id allocation and cache import are + * atomic with respect to other server contexts under + * THREADSAFE. This mattters for the shared + * global cache. */ + ret = WH_SERVER_NVM_LOCK(ctx); + if (ret == WH_ERROR_OK) { + if (WH_KEYID_ISERASED(key_id)) { + /* Generate a new id */ + ret = wh_Server_KeystoreGetUniqueId(ctx, &key_id); + WH_DEBUG_SERVER("UniqueId: keyId:%u, ret:%d\n", key_id, + ret); } - } - if (ret == 0) { - ret = wh_Server_EccKeyCacheImport(ctx, key, key_id, flags, - label_size, label); - } + if (ret == WH_ERROR_OK) { + ret = wh_Server_EccKeyCacheImport( + ctx, key, key_id, flags, label_size, label); + } + (void)WH_SERVER_NVM_UNLOCK(ctx); + } /* WH_SERVER_NVM_LOCK() */ WH_DEBUG_SERVER("CacheImport: keyId:%u, ret:%d\n", key_id, ret); /* TODO: RSA has the following, should we do the same? */ /* @@ -1281,15 +1480,6 @@ static int _HandleEccSharedSecret(whServerContext* ctx, uint16_t magic, whNvmFlags flags = (whNvmFlags)req.flags; int cache = !(flags & WH_NVM_FLAGS_EPHEMERAL); - /* Validate key usage policy for key derivation (private key) */ - if (!WH_KEYID_ISERASED(prv_key_id)) { - ret = wh_Server_KeystoreFindEnforceKeyUsage(ctx, prv_key_id, - WH_NVM_FLAGS_USAGE_DERIVE); - if (ret != WH_ERROR_OK) { - goto cleanup; - } - } - /* Response message */ byte* res_out = (byte*)cryptoDataOut + sizeof(whMessageCrypto_EcdhResponse); @@ -1305,12 +1495,15 @@ static int _HandleEccSharedSecret(whServerContext* ctx, uint16_t magic, /* set rng */ ret = wc_ecc_set_rng(prv_key, ctx->crypto->rng); if (ret == 0) { - /* load the private key */ - ret = wh_Server_EccKeyCacheExport(ctx, prv_key_id, prv_key); - } - if (ret == WH_ERROR_OK) { - /* load the public key */ - ret = wh_Server_EccKeyCacheExport(ctx, pub_key_id, pub_key); + /* load the private key, enforcing the derive usage policy + * against the same locked snapshot that is exported */ + ret = wh_Server_EccKeyCacheExportEnforce( + ctx, prv_key_id, WH_NVM_FLAGS_USAGE_DERIVE, prv_key); + if (ret == WH_ERROR_OK) { + /* load the public key (no usage requirement) */ + ret = wh_Server_EccKeyCacheExportEnforce( + ctx, pub_key_id, WH_NVM_FLAGS_NONE, pub_key); + } } if (ret == WH_ERROR_OK) { /* make shared secret */ @@ -1356,14 +1549,13 @@ static int _HandleEccSharedSecret(whServerContext* ctx, uint16_t magic, } } } -cleanup: if (evict_pub) { /* User requested to evict from cache, even if the call failed */ - (void)wh_Server_KeystoreEvictKey(ctx, pub_key_id); + _CryptoEvictKeyLocked(ctx, pub_key_id); } if (evict_prv) { /* User requested to evict from cache, even if the call failed */ - (void)wh_Server_KeystoreEvictKey(ctx, prv_key_id); + _CryptoEvictKeyLocked(ctx, prv_key_id); } if (ret == 0) { whMessageCrypto_EcdhResponse res; @@ -1423,15 +1615,6 @@ static int _HandleEccSign(whServerContext* ctx, uint16_t magic, int devId, uint32_t options = req.options; int evict = !!(options & WH_MESSAGE_CRYPTO_ECCSIGN_OPTIONS_EVICT); - /* Validate key usage policy for signing */ - if (!WH_KEYID_ISERASED(key_id)) { - ret = wh_Server_KeystoreFindEnforceKeyUsage(ctx, key_id, - WH_NVM_FLAGS_USAGE_SIGN); - if (ret != WH_ERROR_OK) { - goto cleanup; - } - } - /* Response message */ byte* res_out = (byte*)cryptoDataOut + sizeof(whMessageCrypto_EccSignResponse); @@ -1442,8 +1625,10 @@ static int _HandleEccSign(whServerContext* ctx, uint16_t magic, int devId, /* init private key */ ret = wc_ecc_init_ex(key, NULL, devId); if (ret == 0) { - /* load the private key */ - ret = wh_Server_EccKeyCacheExport(ctx, key_id, key); + /* load the private key, enforcing the sign usage policy against the + * same locked snapshot that is exported */ + ret = wh_Server_EccKeyCacheExportEnforce(ctx, key_id, + WH_NVM_FLAGS_USAGE_SIGN, key); if (ret == WH_ERROR_OK) { WH_DEBUG_SERVER_VERBOSE("EccSign: key_id=%x, in_len=%u, res_len=%u, ret=%d\n", key_id, (unsigned)in_len, (unsigned)res_len, ret); @@ -1455,10 +1640,9 @@ static int _HandleEccSign(whServerContext* ctx, uint16_t magic, int devId, } wc_ecc_free(key); } -cleanup: if (evict != 0) { /* typecasting to void so that not overwrite ret */ - (void)wh_Server_KeystoreEvictKey(ctx, key_id); + _CryptoEvictKeyLocked(ctx, key_id); } if (ret == 0) { whMessageCrypto_EccSignResponse res; @@ -1518,15 +1702,6 @@ static int _HandleEccVerify(whServerContext* ctx, uint16_t magic, int devId, int export_pub_key = !!(options & WH_MESSAGE_CRYPTO_ECCVERIFY_OPTIONS_EXPORTPUB); - /* Validate key usage policy for verification */ - if (!WH_KEYID_ISERASED(key_id)) { - ret = wh_Server_KeystoreFindEnforceKeyUsage(ctx, key_id, - WH_NVM_FLAGS_USAGE_VERIFY); - if (ret != WH_ERROR_OK) { - goto cleanup; - } - } - /* Response message */ byte* res_pub = (uint8_t*)(cryptoDataOut) + sizeof(whMessageCrypto_EccVerifyResponse); @@ -1538,8 +1713,10 @@ static int _HandleEccVerify(whServerContext* ctx, uint16_t magic, int devId, /* init public key */ ret = wc_ecc_init_ex(key, NULL, devId); if (ret == 0) { - /* load the public key */ - ret = wh_Server_EccKeyCacheExport(ctx, key_id, key); + /* load the public key, enforcing the verify usage policy against the + * same locked snapshot that is exported */ + ret = wh_Server_EccKeyCacheExportEnforce( + ctx, key_id, WH_NVM_FLAGS_USAGE_VERIFY, key); if (ret == WH_ERROR_OK) { /* verify the signature */ ret = wc_ecc_verify_hash(req_sig, sig_len, req_hash, hash_len, @@ -1566,10 +1743,9 @@ static int _HandleEccVerify(whServerContext* ctx, uint16_t magic, int devId, wc_ecc_free(key); } -cleanup: if (evict != 0) { /* User requested to evict from cache, even if the call failed */ - (void)wh_Server_KeystoreEvictKey(ctx, key_id); + _CryptoEvictKeyLocked(ctx, key_id); } if (ret == 0) { res.pubSz = pub_size; @@ -1721,6 +1897,30 @@ int wh_Server_CmacKdfKeyCacheImport(whServerContext* ctx, } #endif /* HAVE_CMAC_KDF */ +#if defined(HAVE_HKDF) || defined(HAVE_CMAC_KDF) +/* Bound on KDF inputs supplied by key ID. + * + * When a KDF input (HKDF IKM, CMAC-KDF salt or Z) is named by key ID rather + * than passed inline, the handler copies the key out of the cache slot into a + * private buffer and releases the NVM lock before calling wolfCrypt. The copy + * is what makes the usage-policy verdict and the key bytes come from the same + * snapshot; deriving straight from the live slot instead would either race a + * concurrent evict/overwrite or require holding the NVM lock across wolfCrypt. + * + * The consequence is that a cached KDF input larger than its private buffer is + * rejected with WH_ERROR_NOSPACE. WOLFHSM_CFG_SERVER_KDF_MAX_KEY_SIZE bounds + * the IKM and Z buffers; it covers every secret wolfHSM can itself cache as a + * derive input, the largest being a P-521 ECDH shared secret at MAX_ECC_BYTES, + * + */ +#ifdef HAVE_ECC +WH_UTILS_STATIC_ASSERT( + WOLFHSM_CFG_SERVER_KDF_MAX_KEY_SIZE >= MAX_ECC_BYTES, + "WOLFHSM_CFG_SERVER_KDF_MAX_KEY_SIZE too small to hold an ECDH shared " + "secret as a cached KDF input"); +#endif /* HAVE_ECC */ +#endif /* HAVE_HKDF || HAVE_CMAC_KDF */ + #ifdef HAVE_HKDF static int _HandleHkdf(whServerContext* ctx, uint16_t magic, int devId, const void* cryptoDataIn, uint16_t inSize, @@ -1776,38 +1976,39 @@ static int _HandleHkdf(whServerContext* ctx, uint16_t magic, int devId, const uint8_t* info = salt + saltSz; /* Buffer for cached key if needed */ - uint8_t* cachedKeyBuf = NULL; - whNvmMetadata* cachedKeyMeta = NULL; + uint8_t cachedKeyBuf[WOLFHSM_CFG_SERVER_KDF_MAX_KEY_SIZE]; + whNvmMetadata cachedKeyMeta[1]; + + /* Get pointer to where output data would be stored (after response struct). + * Declared before the first goto so no jump skips an initialization. */ + uint8_t* out = + (uint8_t*)cryptoDataOut + sizeof(whMessageCrypto_HkdfResponse); + uint16_t max_size = (uint16_t)(WOLFHSM_CFG_COMM_DATA_LEN - + ((uint8_t*)out - (uint8_t*)cryptoDataOut)); /* Check if we should use cached key as input */ if (inKeySz == 0 && !WH_KEYID_ISERASED(keyIdIn)) { - /* Grab references to key in the cache */ - ret = wh_Server_KeystoreFreshenKey(ctx, keyIdIn, &cachedKeyBuf, - &cachedKeyMeta); + /* Copy the derive input key into a private buffer, enforcing the + * derive usage policy against the same locked snapshot that is read. + * HKDF then runs on the private copy after the lock is released. + * An IKM larger than cachedKeyBuf yields WH_ERROR_NOSPACE; see the + * bound on cached KDF inputs documented above _HandleHkdf(). */ + uint32_t cachedKeyLen = sizeof(cachedKeyBuf); + ret = wh_Server_KeystoreReadKeyEnforce( + ctx, keyIdIn, WH_NVM_FLAGS_USAGE_DERIVE, cachedKeyMeta, + cachedKeyBuf, &cachedKeyLen); if (ret != WH_ERROR_OK) { - return ret; - } - /* Validate key usage policy for key derivation (input key) */ - ret = wh_Server_KeystoreEnforceKeyUsage(cachedKeyMeta, - WH_NVM_FLAGS_USAGE_DERIVE); - if (ret != WH_ERROR_OK) { - return ret; + goto cleanup; } /* Update inKey pointer and size to use cached key */ inKey = cachedKeyBuf; inKeySz = cachedKeyMeta->len; } - /* Get pointer to where output data would be stored (after response struct) - */ - uint8_t* out = - (uint8_t*)cryptoDataOut + sizeof(whMessageCrypto_HkdfResponse); - uint16_t max_size = (uint16_t)(WOLFHSM_CFG_COMM_DATA_LEN - - ((uint8_t*)out - (uint8_t*)cryptoDataOut)); - /* Check if output size is valid */ if (outSz > max_size) { - return WH_ERROR_BADARGS; + ret = WH_ERROR_BADARGS; + goto cleanup; } /* Generate the key into the output buffer */ @@ -1823,20 +2024,22 @@ static int _HandleHkdf(whServerContext* ctx, uint16_t magic, int devId, } else { /* Must import the key into the cache and return keyid */ - if (WH_KEYID_ISERASED(key_id)) { - /* Generate a new id */ - ret = wh_Server_KeystoreGetUniqueId(ctx, &key_id); - WH_DEBUG_SERVER_VERBOSE("HkdfKeyGen UniqueId: keyId:%u, ret:%d\n", key_id, ret); - if (ret != WH_ERROR_OK) { - /* Early return on unique ID generation failure */ - return ret; + /* Hold the NVM lock so id allocation and cache import are atomic + * with respect to other server contexts under THREADSAFE. */ + ret = WH_SERVER_NVM_LOCK(ctx); + if (ret == WH_ERROR_OK) { + if (WH_KEYID_ISERASED(key_id)) { + /* Generate a new id */ + ret = wh_Server_KeystoreGetUniqueId(ctx, &key_id); + WH_DEBUG_SERVER_VERBOSE( + "HkdfKeyGen UniqueId: keyId:%u, ret:%d\n", key_id, ret); } - } - - if (ret == 0) { - ret = wh_Server_HkdfKeyCacheImport(ctx, out, outSz, key_id, - flags, label_size, label); - } + if (ret == WH_ERROR_OK) { + ret = wh_Server_HkdfKeyCacheImport( + ctx, out, outSz, key_id, flags, label_size, label); + } + (void)WH_SERVER_NVM_UNLOCK(ctx); + } /* WH_SERVER_NVM_LOCK() */ WH_DEBUG_SERVER_VERBOSE("HkdfKeyGen CacheImport: keyId:%u, ret:%d\n", key_id, ret); if (ret == WH_ERROR_OK) { res.keyIdOut = wh_KeyId_TranslateToClient(key_id); @@ -1857,6 +2060,9 @@ static int _HandleHkdf(whServerContext* ctx, uint16_t magic, int devId, } } +cleanup: + /* The IKM copy is plaintext key material, so don't leave it on the stack */ + wc_ForceZero(cachedKeyBuf, sizeof(cachedKeyBuf)); return ret; } #endif /* HAVE_HKDF */ @@ -1914,25 +2120,35 @@ static int _HandleCmacKdf(whServerContext* ctx, uint16_t magic, int devId, const uint8_t* z = salt + saltSz; const uint8_t* fixedInfo = z + zSz; - uint8_t* cachedSaltBuf = NULL; - whNvmMetadata* cachedSaltMeta = NULL; - uint8_t* cachedZBuf = NULL; - whNvmMetadata* cachedZMeta = NULL; + /* The salt is a CMAC key, so wolfCrypt accepts only an AES key size here */ + uint8_t cachedSaltBuf[AES_256_KEY_SIZE]; + whNvmMetadata cachedSaltMeta[1]; + uint8_t cachedZBuf[WOLFHSM_CFG_SERVER_KDF_MAX_KEY_SIZE]; + whNvmMetadata cachedZMeta[1]; + + /* Declared before the first goto so no jump skips an initialization */ + uint8_t* out = + (uint8_t*)cryptoDataOut + sizeof(whMessageCrypto_CmacKdfResponse); + uint16_t max_size = (uint16_t)(WOLFHSM_CFG_COMM_DATA_LEN - + ((uint8_t*)out - (uint8_t*)cryptoDataOut)); if (saltSz == 0) { if (WH_KEYID_ISERASED(saltKeyId)) { - return WH_ERROR_BADARGS; - } - ret = wh_Server_KeystoreFreshenKey(ctx, saltKeyId, &cachedSaltBuf, - &cachedSaltMeta); - if (ret != WH_ERROR_OK) { - return ret; + ret = WH_ERROR_BADARGS; + goto cleanup; } - /* Validate key usage policy for cached salt */ - ret = wh_Server_KeystoreEnforceKeyUsage(cachedSaltMeta, - WH_NVM_FLAGS_USAGE_DERIVE); + /* Copy the derive salt into a private buffer, enforcing the derive + * usage policy against the same locked snapshot that is read. The KDF + * then runs on the private copy after the lock is released. + * A salt too big for cachedSaltBuf yields WH_ERROR_NOSPACE, where it + * would otherwise reach wc_KDA_KDF_twostep_cmac() and be rejected as + * BAD_FUNC_ARG. Both are failures; only the code differs. */ + uint32_t cachedSaltLen = sizeof(cachedSaltBuf); + ret = wh_Server_KeystoreReadKeyEnforce( + ctx, saltKeyId, WH_NVM_FLAGS_USAGE_DERIVE, cachedSaltMeta, + cachedSaltBuf, &cachedSaltLen); if (ret != WH_ERROR_OK) { - return ret; + goto cleanup; } salt = cachedSaltBuf; saltSz = cachedSaltMeta->len; @@ -1940,34 +2156,33 @@ static int _HandleCmacKdf(whServerContext* ctx, uint16_t magic, int devId, if (zSz == 0) { if (WH_KEYID_ISERASED(zKeyId)) { - return WH_ERROR_BADARGS; - } - ret = wh_Server_KeystoreFreshenKey(ctx, zKeyId, &cachedZBuf, - &cachedZMeta); - if (ret != WH_ERROR_OK) { - return ret; + ret = WH_ERROR_BADARGS; + goto cleanup; } - /* Validate key usage policy for key derivation (Z key) */ - ret = wh_Server_KeystoreEnforceKeyUsage(cachedZMeta, - WH_NVM_FLAGS_USAGE_DERIVE); + /* Copy the derive Z value into a private buffer, enforcing the derive + * usage policy against the same locked snapshot that is read. The KDF + * then runs on the private copy after the lock is released. + * A Z larger than cachedZBuf yields WH_ERROR_NOSPACE; see the bound on + * cached KDF inputs documented above _HandleHkdf(). */ + uint32_t cachedZLen = sizeof(cachedZBuf); + ret = wh_Server_KeystoreReadKeyEnforce( + ctx, zKeyId, WH_NVM_FLAGS_USAGE_DERIVE, cachedZMeta, cachedZBuf, + &cachedZLen); if (ret != WH_ERROR_OK) { - return ret; + goto cleanup; } z = cachedZBuf; zSz = cachedZMeta->len; } if ((salt == NULL) || (z == NULL) || (outSz == 0)) { - return WH_ERROR_BADARGS; + ret = WH_ERROR_BADARGS; + goto cleanup; } - uint8_t* out = - (uint8_t*)cryptoDataOut + sizeof(whMessageCrypto_CmacKdfResponse); - uint16_t max_size = (uint16_t)(WOLFHSM_CFG_COMM_DATA_LEN - - ((uint8_t*)out - (uint8_t*)cryptoDataOut)); - if (outSz > max_size) { - return WH_ERROR_BADARGS; + ret = WH_ERROR_BADARGS; + goto cleanup; } ret = wc_KDA_KDF_twostep_cmac(salt, saltSz, z, zSz, @@ -1980,15 +2195,19 @@ static int _HandleCmacKdf(whServerContext* ctx, uint16_t magic, int devId, res.outSz = outSz; } else { - if (WH_KEYID_ISERASED(keyIdOut)) { - ret = wh_Server_KeystoreGetUniqueId(ctx, &keyIdOut); - if (ret != WH_ERROR_OK) { - return ret; + /* Hold the NVM lock so id allocation and cache import are atomic + * with respect to other server contexts under THREADSAFE. */ + ret = WH_SERVER_NVM_LOCK(ctx); + if (ret == WH_ERROR_OK) { + if (WH_KEYID_ISERASED(keyIdOut)) { + ret = wh_Server_KeystoreGetUniqueId(ctx, &keyIdOut); } - } - - ret = wh_Server_CmacKdfKeyCacheImport(ctx, out, outSz, keyIdOut, - flags, label_size, label); + if (ret == WH_ERROR_OK) { + ret = wh_Server_CmacKdfKeyCacheImport( + ctx, out, outSz, keyIdOut, flags, label_size, label); + } + (void)WH_SERVER_NVM_UNLOCK(ctx); + } /* WH_SERVER_NVM_LOCK() */ if (ret == WH_ERROR_OK) { res.keyIdOut = wh_KeyId_TranslateToClient(keyIdOut); res.outSz = 0; @@ -2005,6 +2224,11 @@ static int _HandleCmacKdf(whServerContext* ctx, uint16_t magic, int devId, } } +cleanup: + /* The salt and Z copies are plaintext derive secrets, so don't leave them + * on the stack */ + wc_ForceZero(cachedSaltBuf, sizeof(cachedSaltBuf)); + wc_ForceZero(cachedZBuf, sizeof(cachedZBuf)); return ret; } #endif /* HAVE_CMAC_KDF */ @@ -2060,22 +2284,23 @@ static int _HandleCurve25519KeyGen(whServerContext* ctx, uint16_t magic, else { ser_size = 0; /* Must import the key into the cache and return keyid */ - if (WH_KEYID_ISERASED(key_id)) { - /* Generate a new id */ - ret = wh_Server_KeystoreGetUniqueId(ctx, &key_id); - WH_DEBUG_SERVER("UniqueId: keyId:%u, ret:%d\n", - key_id, ret); - if (ret != WH_ERROR_OK) { - /* Early return on unique ID generation failure */ - wc_curve25519_free(key); - return ret; + /* Hold the NVM lock so id allocation and cache import are + * atomic with respect to other server contexts under + * THREADSAFE. */ + ret = WH_SERVER_NVM_LOCK(ctx); + if (ret == WH_ERROR_OK) { + if (WH_KEYID_ISERASED(key_id)) { + /* Generate a new id */ + ret = wh_Server_KeystoreGetUniqueId(ctx, &key_id); + WH_DEBUG_SERVER("UniqueId: keyId:%u, ret:%d\n", key_id, + ret); } - } - - if (ret == 0) { - ret = wh_Server_CacheImportCurve25519Key( - ctx, key, key_id, flags, label_size, label); - } + if (ret == WH_ERROR_OK) { + ret = wh_Server_CacheImportCurve25519Key( + ctx, key, key_id, flags, label_size, label); + } + (void)WH_SERVER_NVM_UNLOCK(ctx); + } /* WH_SERVER_NVM_LOCK() */ WH_DEBUG_SERVER_VERBOSE("CacheImport: keyId:%u, ret:%d\n", key_id, ret); } @@ -2133,15 +2358,6 @@ static int _HandleCurve25519SharedSecret(whServerContext* ctx, uint16_t magic, whNvmFlags flags = (whNvmFlags)req.flags; int cache = !(flags & WH_NVM_FLAGS_EPHEMERAL); - /* Validate key usage policy for key derivation (private key) */ - if (!WH_KEYID_ISERASED(prv_key_id)) { - ret = wh_Server_KeystoreFindEnforceKeyUsage(ctx, prv_key_id, - WH_NVM_FLAGS_USAGE_DERIVE); - if (ret != WH_ERROR_OK) { - goto cleanup; - } - } - /* Response message */ uint8_t* res_out = (uint8_t*)cryptoDataOut + sizeof(whMessageCrypto_Curve25519Response); @@ -2162,10 +2378,15 @@ static int _HandleCurve25519SharedSecret(whServerContext* ctx, uint16_t magic, } #endif if (ret == 0) { - ret = wh_Server_CacheExportCurve25519Key(ctx, prv_key_id, priv); - } - if (ret == 0) { - ret = wh_Server_CacheExportCurve25519Key(ctx, pub_key_id, pub); + /* load the private key, enforcing the derive usage policy + * against the same locked snapshot that is exported */ + ret = wh_Server_CacheExportCurve25519KeyEnforce( + ctx, prv_key_id, WH_NVM_FLAGS_USAGE_DERIVE, priv); + if (ret == WH_ERROR_OK) { + /* load the public key (no usage requirement) */ + ret = wh_Server_CacheExportCurve25519KeyEnforce( + ctx, pub_key_id, WH_NVM_FLAGS_NONE, pub); + } } if (ret == 0) { ret = wc_curve25519_shared_secret_ex(priv, pub, res_out, @@ -2210,14 +2431,13 @@ static int _HandleCurve25519SharedSecret(whServerContext* ctx, uint16_t magic, } } } -cleanup: if (evict_pub) { /* User requested to evict from cache, even if the call failed */ - (void)wh_Server_KeystoreEvictKey(ctx, pub_key_id); + _CryptoEvictKeyLocked(ctx, pub_key_id); } if (evict_prv) { /* User requested to evict from cache, even if the call failed */ - (void)wh_Server_KeystoreEvictKey(ctx, prv_key_id); + _CryptoEvictKeyLocked(ctx, prv_key_id); } if (ret == 0) { uint16_t payload_len; @@ -2283,17 +2503,20 @@ static int _HandleEd25519KeyGen(whServerContext* ctx, uint16_t magic, int devId, } else { ser_size = 0; - if (WH_KEYID_ISERASED(key_id)) { - ret = wh_Server_KeystoreGetUniqueId(ctx, &key_id); - if (ret != WH_ERROR_OK) { - wc_ed25519_free(key); - return ret; + /* Hold the NVM lock so id allocation and cache import are + * atomic with respect to other server contexts under + * THREADSAFE. */ + ret = WH_SERVER_NVM_LOCK(ctx); + if (ret == WH_ERROR_OK) { + if (WH_KEYID_ISERASED(key_id)) { + ret = wh_Server_KeystoreGetUniqueId(ctx, &key_id); } - } - if (ret == 0) { - ret = wh_Server_CacheImportEd25519Key( - ctx, key, key_id, flags, label_size, label); - } + if (ret == WH_ERROR_OK) { + ret = wh_Server_CacheImportEd25519Key( + ctx, key, key_id, flags, label_size, label); + } + (void)WH_SERVER_NVM_UNLOCK(ctx); + } /* WH_SERVER_NVM_LOCK() */ } } wc_ed25519_free(key); @@ -2364,21 +2587,16 @@ static int _HandleEd25519Sign(whServerContext* ctx, uint16_t magic, int devId, uint8_t* req_ctx = req_msg + msg_len; int evict = !!(req.options & WH_MESSAGE_CRYPTO_ED25519_SIGN_OPTIONS_EVICT); - if (!WH_KEYID_ISERASED(key_id)) { - ret = wh_Server_KeystoreFindEnforceKeyUsage(ctx, key_id, - WH_NVM_FLAGS_USAGE_SIGN); - if (ret != WH_ERROR_OK) { - goto cleanup; - } - } - uint8_t* res_sig = (uint8_t*)cryptoDataOut + sizeof(whMessageCrypto_Ed25519SignResponse); word32 sig_len = sizeof(sig); ret = wc_ed25519_init_ex(key, NULL, devId); if (ret == 0) { - ret = wh_Server_CacheExportEd25519Key(ctx, key_id, key); + /* load the private key, enforcing the sign usage policy against the + * same locked snapshot that is exported */ + ret = wh_Server_CacheExportEd25519KeyEnforce( + ctx, key_id, WH_NVM_FLAGS_USAGE_SIGN, key); if (ret == WH_ERROR_OK) { ret = wc_ed25519_sign_msg_ex(req_msg, msg_len, sig, &sig_len, key, (byte)req.type, req_ctx, @@ -2395,10 +2613,9 @@ static int _HandleEd25519Sign(whServerContext* ctx, uint16_t magic, int devId, memcpy(res_sig, sig, sig_len); } -cleanup: if (evict) { /* User requested to evict from cache, even if the call failed */ - (void)wh_Server_KeystoreEvictKey(ctx, key_id); + _CryptoEvictKeyLocked(ctx, key_id); } if (ret == 0) { @@ -2465,19 +2682,14 @@ static int _HandleEd25519Verify(whServerContext* ctx, uint16_t magic, int devId, int evict = !!(req.options & WH_MESSAGE_CRYPTO_ED25519_VERIFY_OPTIONS_EVICT); - if (!WH_KEYID_ISERASED(key_id)) { - ret = wh_Server_KeystoreFindEnforceKeyUsage(ctx, key_id, - WH_NVM_FLAGS_USAGE_VERIFY); - if (ret != WH_ERROR_OK) { - goto cleanup; - } - } - int result = 0; ret = wc_ed25519_init_ex(key, NULL, devId); if (ret == 0) { - ret = wh_Server_CacheExportEd25519Key(ctx, key_id, key); + /* load the public key, enforcing the verify usage policy against the + * same locked snapshot that is exported */ + ret = wh_Server_CacheExportEd25519KeyEnforce( + ctx, key_id, WH_NVM_FLAGS_USAGE_VERIFY, key); if (ret == WH_ERROR_OK) { ret = wc_ed25519_verify_msg_ex(req_sig, sig_len, req_msg, msg_len, &result, key, (byte)req.type, @@ -2486,9 +2698,8 @@ static int _HandleEd25519Verify(whServerContext* ctx, uint16_t magic, int devId, wc_ed25519_free(key); } -cleanup: if (evict != 0) { - (void)wh_Server_KeystoreEvictKey(ctx, key_id); + _CryptoEvictKeyLocked(ctx, key_id); } if (ret == 0) { @@ -2545,14 +2756,6 @@ static int _HandleEd25519SignDma(whServerContext* ctx, uint16_t magic, WH_KEYTYPE_CRYPTO, ctx->comm->client_id, req.keyId); int evict = !!(req.options & WH_MESSAGE_CRYPTO_ED25519_SIGN_OPTIONS_EVICT); - if (!WH_KEYID_ISERASED(key_id)) { - ret = wh_Server_KeystoreFindEnforceKeyUsage(ctx, key_id, - WH_NVM_FLAGS_USAGE_SIGN); - if (ret != WH_ERROR_OK) { - goto cleanup; - } - } - memset(&res, 0, sizeof(res)); sigLen = req.sig.sz; @@ -2573,7 +2776,10 @@ static int _HandleEd25519SignDma(whServerContext* ctx, uint16_t magic, if (ret == WH_ERROR_OK) { ret = wc_ed25519_init_ex(key, NULL, devId); if (ret == 0) { - ret = wh_Server_CacheExportEd25519Key(ctx, key_id, key); + /* load the private key, enforcing the sign usage policy against + * the same locked snapshot that is exported */ + ret = wh_Server_CacheExportEd25519KeyEnforce( + ctx, key_id, WH_NVM_FLAGS_USAGE_SIGN, key); if (ret == WH_ERROR_OK) { ret = wc_ed25519_sign_msg_ex(msgAddr, req.msg.sz, sigAddr, &sigLen, key, (byte)req.type, @@ -2596,9 +2802,8 @@ static int _HandleEd25519SignDma(whServerContext* ctx, uint16_t magic, ctx, (uintptr_t)req.msg.addr, &msgAddr, req.msg.sz, WH_DMA_OPER_CLIENT_READ_POST, (whServerDmaFlags){0}); -cleanup: if (evict != 0) { - (void)wh_Server_KeystoreEvictKey(ctx, key_id); + _CryptoEvictKeyLocked(ctx, key_id); } if (ret == WH_ERROR_OK) { @@ -2653,14 +2858,6 @@ static int _HandleEd25519VerifyDma(whServerContext* ctx, uint16_t magic, int evict = !!(req.options & WH_MESSAGE_CRYPTO_ED25519_VERIFY_OPTIONS_EVICT); - if (!WH_KEYID_ISERASED(key_id)) { - ret = wh_Server_KeystoreFindEnforceKeyUsage(ctx, key_id, - WH_NVM_FLAGS_USAGE_VERIFY); - if (ret != WH_ERROR_OK) { - goto cleanup; - } - } - memset(&res, 0, sizeof(res)); ret = wh_Server_DmaProcessClientAddress( @@ -2681,7 +2878,10 @@ static int _HandleEd25519VerifyDma(whServerContext* ctx, uint16_t magic, if (ret == WH_ERROR_OK) { ret = wc_ed25519_init_ex(key, NULL, devId); if (ret == 0) { - ret = wh_Server_CacheExportEd25519Key(ctx, key_id, key); + /* load the public key, enforcing the verify usage policy against + * the same locked snapshot that is exported */ + ret = wh_Server_CacheExportEd25519KeyEnforce( + ctx, key_id, WH_NVM_FLAGS_USAGE_VERIFY, key); if (ret == WH_ERROR_OK) { int verified = 0; ret = wc_ed25519_verify_msg_ex( @@ -2702,9 +2902,8 @@ static int _HandleEd25519VerifyDma(whServerContext* ctx, uint16_t magic, ctx, (uintptr_t)req.sig.addr, &sigAddr, req.sig.sz, WH_DMA_OPER_CLIENT_READ_POST, (whServerDmaFlags){0}); -cleanup: if (evict != 0) { - (void)wh_Server_KeystoreEvictKey(ctx, key_id); + _CryptoEvictKeyLocked(ctx, key_id); } if (ret == WH_ERROR_OK) { @@ -2729,8 +2928,8 @@ static int _HandleAesCtr(whServerContext* ctx, uint16_t magic, int devId, Aes aes[1] = {0}; whMessageCrypto_AesCtrRequest req; whMessageCrypto_AesCtrResponse res; - uint8_t* cachedKey = NULL; - whNvmMetadata* keyMeta = NULL; + uint8_t cachedKey[AES_256_KEY_SIZE]; + whNvmMetadata keyMeta[1]; if (inSize < sizeof(whMessageCrypto_AesCtrRequest)) { return WH_ERROR_BADARGS; @@ -2771,13 +2970,14 @@ static int _HandleAesCtr(whServerContext* ctx, uint16_t magic, int devId, WH_DEBUG_VERBOSE_HEXDUMP("[AesCtr] tmp ", tmp, AES_BLOCK_SIZE); /* Freshen key and validate usage policy if key is not erased */ if (!WH_KEYID_ISERASED(key_id)) { - ret = wh_Server_KeystoreFreshenKey(ctx, key_id, &cachedKey, &keyMeta); - if (ret == WH_ERROR_OK) { - /* Validate key usage policy */ - ret = wh_Server_KeystoreEnforceKeyUsage( - keyMeta, enc != 0 ? WH_NVM_FLAGS_USAGE_ENCRYPT - : WH_NVM_FLAGS_USAGE_DECRYPT); - } + /* Copy the key + metadata into private buffers, enforcing the usage + * policy against the same locked snapshot that is read. The crypto + * below then runs entirely on the private copy. */ + uint32_t cachedKeyLen = sizeof(cachedKey); + ret = wh_Server_KeystoreReadKeyEnforce( + ctx, key_id, + enc != 0 ? WH_NVM_FLAGS_USAGE_ENCRYPT : WH_NVM_FLAGS_USAGE_DECRYPT, + keyMeta, cachedKey, &cachedKeyLen); if (ret == WH_ERROR_OK) { /* override the incoming values with cached key */ key = cachedKey; @@ -2849,6 +3049,7 @@ static int _HandleAesCtr(whServerContext* ctx, uint16_t magic, int devId, ret = wh_MessageCrypto_TranslateAesCtrResponse( magic, &res, (whMessageCrypto_AesCtrResponse*)cryptoDataOut); } + wc_ForceZero(cachedKey, sizeof(cachedKey)); return ret; } @@ -2869,8 +3070,8 @@ static int _HandleAesCtrDma(whServerContext* ctx, uint16_t magic, int devId, word32 outSz = 0; whKeyId keyId; - uint8_t* cachedKey = NULL; - whNvmMetadata* keyMeta = NULL; + uint8_t cachedKey[AES_256_KEY_SIZE]; + whNvmMetadata keyMeta[1]; (void)seq; @@ -2922,14 +3123,15 @@ static int _HandleAesCtrDma(whServerContext* ctx, uint16_t magic, int devId, /* Freshen key and validate usage policy if key is not erased */ if (!WH_KEYID_ISERASED(keyId)) { - ret = wh_Server_KeystoreFreshenKey(ctx, keyId, &cachedKey, - &keyMeta); - if (ret == WH_ERROR_OK) { - /* Validate key usage policy */ - ret = wh_Server_KeystoreEnforceKeyUsage( - keyMeta, enc != 0 ? WH_NVM_FLAGS_USAGE_ENCRYPT - : WH_NVM_FLAGS_USAGE_DECRYPT); - } + /* Copy the key + metadata into private buffers, enforcing the + * usage policy against the same locked snapshot that is read. The + * crypto below then runs entirely on the private copy. */ + uint32_t cachedKeyLen = sizeof(cachedKey); + ret = wh_Server_KeystoreReadKeyEnforce( + ctx, keyId, + enc != 0 ? WH_NVM_FLAGS_USAGE_ENCRYPT + : WH_NVM_FLAGS_USAGE_DECRYPT, + keyMeta, cachedKey, &cachedKeyLen); if (ret == WH_ERROR_OK) { key = cachedKey; keyLen = keyMeta->len; @@ -3045,6 +3247,7 @@ static int _HandleAesCtrDma(whServerContext* ctx, uint16_t magic, int devId, *outSize = sizeof(whMessageCrypto_AesCtrDmaResponse) + AES_IV_SIZE + AES_BLOCK_SIZE; + wc_ForceZero(cachedKey, sizeof(cachedKey)); return ret; } #endif /* WOLFHSM_CFG_DMA */ @@ -3059,8 +3262,8 @@ static int _HandleAesEcb(whServerContext* ctx, uint16_t magic, int devId, Aes aes[1] = {0}; whMessageCrypto_AesEcbRequest req; whMessageCrypto_AesEcbResponse res; - uint8_t* cachedKey = NULL; - whNvmMetadata* keyMeta = NULL; + uint8_t cachedKey[AES_256_KEY_SIZE]; + whNvmMetadata keyMeta[1]; if (inSize < sizeof(whMessageCrypto_AesEcbRequest)) { return WH_ERROR_BADARGS; @@ -3098,13 +3301,14 @@ static int _HandleAesEcb(whServerContext* ctx, uint16_t magic, int devId, /* Freshen key and validate usage policy if key is not erased */ if (!WH_KEYID_ISERASED(key_id)) { - ret = wh_Server_KeystoreFreshenKey(ctx, key_id, &cachedKey, &keyMeta); - if (ret == WH_ERROR_OK) { - /* Validate key usage policy */ - ret = wh_Server_KeystoreEnforceKeyUsage( - keyMeta, enc != 0 ? WH_NVM_FLAGS_USAGE_ENCRYPT - : WH_NVM_FLAGS_USAGE_DECRYPT); - } + /* Copy the key + metadata into private buffers, enforcing the usage + * policy against the same locked snapshot that is read. The crypto + * below then runs entirely on the private copy. */ + uint32_t cachedKeyLen = sizeof(cachedKey); + ret = wh_Server_KeystoreReadKeyEnforce( + ctx, key_id, + enc != 0 ? WH_NVM_FLAGS_USAGE_ENCRYPT : WH_NVM_FLAGS_USAGE_DECRYPT, + keyMeta, cachedKey, &cachedKeyLen); if (ret == WH_ERROR_OK) { /* override the incoming values with cached key */ key = cachedKey; @@ -3158,6 +3362,7 @@ static int _HandleAesEcb(whServerContext* ctx, uint16_t magic, int devId, ret = wh_MessageCrypto_TranslateAesEcbResponse( magic, &res, (whMessageCrypto_AesEcbResponse*)cryptoDataOut); } + wc_ForceZero(cachedKey, sizeof(cachedKey)); return ret; } @@ -3177,8 +3382,8 @@ static int _HandleAesEcbDma(whServerContext* ctx, uint16_t magic, int devId, word32 outSz = 0; whKeyId keyId; - uint8_t* cachedKey = NULL; - whNvmMetadata* keyMeta = NULL; + uint8_t cachedKey[AES_256_KEY_SIZE]; + whNvmMetadata keyMeta[1]; (void)seq; @@ -3219,14 +3424,15 @@ static int _HandleAesEcbDma(whServerContext* ctx, uint16_t magic, int devId, /* Freshen key and validate usage policy if key is not erased */ if (!WH_KEYID_ISERASED(keyId)) { - ret = wh_Server_KeystoreFreshenKey(ctx, keyId, &cachedKey, - &keyMeta); - if (ret == WH_ERROR_OK) { - /* Validate key usage policy */ - ret = wh_Server_KeystoreEnforceKeyUsage( - keyMeta, req.enc != 0 ? WH_NVM_FLAGS_USAGE_ENCRYPT - : WH_NVM_FLAGS_USAGE_DECRYPT); - } + /* Copy the key + metadata into private buffers, enforcing the + * usage policy against the same locked snapshot that is read. The + * crypto below then runs entirely on the private copy. */ + uint32_t cachedKeyLen = sizeof(cachedKey); + ret = wh_Server_KeystoreReadKeyEnforce( + ctx, keyId, + req.enc != 0 ? WH_NVM_FLAGS_USAGE_ENCRYPT + : WH_NVM_FLAGS_USAGE_DECRYPT, + keyMeta, cachedKey, &cachedKeyLen); if (ret == WH_ERROR_OK) { key = cachedKey; keyLen = keyMeta->len; @@ -3325,6 +3531,7 @@ static int _HandleAesEcbDma(whServerContext* ctx, uint16_t magic, int devId, magic, &res, (whMessageCrypto_AesEcbDmaResponse*)cryptoDataOut); *outSize = sizeof(whMessageCrypto_AesEcbDmaResponse); + wc_ForceZero(cachedKey, sizeof(cachedKey)); return ret; } #endif /* WOLFHSM_CFG_DMA */ @@ -3339,8 +3546,8 @@ static int _HandleAesCbc(whServerContext* ctx, uint16_t magic, int devId, Aes aes[1] = {0}; whMessageCrypto_AesCbcRequest req; whMessageCrypto_AesCbcResponse res; - uint8_t* cachedKey = NULL; - whNvmMetadata* keyMeta = NULL; + uint8_t cachedKey[AES_256_KEY_SIZE]; + whNvmMetadata keyMeta[1]; /* Validate minimum size */ if (inSize < sizeof(whMessageCrypto_AesCbcRequest)) { @@ -3382,13 +3589,14 @@ static int _HandleAesCbc(whServerContext* ctx, uint16_t magic, int devId, WH_DEBUG_VERBOSE_HEXDUMP("[AesCbc] IV", iv, AES_BLOCK_SIZE); /* Freshen key and validate usage policy if key is not erased */ if (!WH_KEYID_ISERASED(key_id)) { - ret = wh_Server_KeystoreFreshenKey(ctx, key_id, &cachedKey, &keyMeta); - if (ret == WH_ERROR_OK) { - /* Validate key usage policy */ - ret = wh_Server_KeystoreEnforceKeyUsage( - keyMeta, enc != 0 ? WH_NVM_FLAGS_USAGE_ENCRYPT - : WH_NVM_FLAGS_USAGE_DECRYPT); - } + /* Copy the key + metadata into private buffers, enforcing the usage + * policy against the same locked snapshot that is read. The crypto + * below then runs entirely on the private copy. */ + uint32_t cachedKeyLen = sizeof(cachedKey); + ret = wh_Server_KeystoreReadKeyEnforce( + ctx, key_id, + enc != 0 ? WH_NVM_FLAGS_USAGE_ENCRYPT : WH_NVM_FLAGS_USAGE_DECRYPT, + keyMeta, cachedKey, &cachedKeyLen); if (ret == WH_ERROR_OK) { /* override the incoming values with cached key */ key = cachedKey; @@ -3443,6 +3651,7 @@ static int _HandleAesCbc(whServerContext* ctx, uint16_t magic, int devId, ret = wh_MessageCrypto_TranslateAesCbcResponse( magic, &res, (whMessageCrypto_AesCbcResponse*)cryptoDataOut); } + wc_ForceZero(cachedKey, sizeof(cachedKey)); return ret; } @@ -3462,8 +3671,8 @@ static int _HandleAesCbcDma(whServerContext* ctx, uint16_t magic, int devId, word32 outSz = 0; whKeyId keyId; - uint8_t* cachedKey = NULL; - whNvmMetadata* keyMeta = NULL; + uint8_t cachedKey[AES_256_KEY_SIZE]; + whNvmMetadata keyMeta[1]; (void)seq; @@ -3511,14 +3720,15 @@ static int _HandleAesCbcDma(whServerContext* ctx, uint16_t magic, int devId, /* Freshen key and validate usage policy if key is not erased */ if (!WH_KEYID_ISERASED(keyId)) { - ret = wh_Server_KeystoreFreshenKey(ctx, keyId, &cachedKey, - &keyMeta); - if (ret == WH_ERROR_OK) { - /* Validate key usage policy */ - ret = wh_Server_KeystoreEnforceKeyUsage( - keyMeta, enc != 0 ? WH_NVM_FLAGS_USAGE_ENCRYPT - : WH_NVM_FLAGS_USAGE_DECRYPT); - } + /* Copy the key + metadata into private buffers, enforcing the + * usage policy against the same locked snapshot that is read. The + * crypto below then runs entirely on the private copy. */ + uint32_t cachedKeyLen = sizeof(cachedKey); + ret = wh_Server_KeystoreReadKeyEnforce( + ctx, keyId, + enc != 0 ? WH_NVM_FLAGS_USAGE_ENCRYPT + : WH_NVM_FLAGS_USAGE_DECRYPT, + keyMeta, cachedKey, &cachedKeyLen); if (ret == WH_ERROR_OK) { key = cachedKey; keyLen = keyMeta->len; @@ -3617,6 +3827,7 @@ static int _HandleAesCbcDma(whServerContext* ctx, uint16_t magic, int devId, magic, &res, (whMessageCrypto_AesCbcDmaResponse*)cryptoDataOut); *outSize = sizeof(whMessageCrypto_AesCbcDmaResponse) + AES_IV_SIZE; + wc_ForceZero(cachedKey, sizeof(cachedKey)); return ret; } #endif /* WOLFHSM_CFG_DMA */ @@ -3629,8 +3840,8 @@ static int _HandleAesGcm(whServerContext* ctx, uint16_t magic, int devId, { int ret = WH_ERROR_OK; Aes aes[1] = {0}; - uint8_t* cachedKey = NULL; - whNvmMetadata* keyMeta = NULL; + uint8_t cachedKey[AES_256_KEY_SIZE]; + whNvmMetadata keyMeta[1]; /* Validate minimum size */ if (inSize < sizeof(whMessageCrypto_AesGcmRequest)) { @@ -3691,14 +3902,16 @@ static int _HandleAesGcm(whServerContext* ctx, uint16_t magic, int devId, /* Freshen key and validate usage policy if key is not erased */ if (!WH_KEYID_ISERASED(key_id)) { - ret = wh_Server_KeystoreFreshenKey(ctx, key_id, &cachedKey, &keyMeta); - WH_DEBUG_SERVER_VERBOSE("AesGcm FreshenKey key_id:%u ret:%d\n", key_id, ret); - if (ret == WH_ERROR_OK) { - /* Validate key usage policy */ - ret = wh_Server_KeystoreEnforceKeyUsage( - keyMeta, enc != 0 ? WH_NVM_FLAGS_USAGE_ENCRYPT - : WH_NVM_FLAGS_USAGE_DECRYPT); - } + /* Copy the key + metadata into private buffers, enforcing the usage + * policy against the same locked snapshot that is read. The crypto + * below then runs entirely on the private copy. */ + uint32_t cachedKeyLen = sizeof(cachedKey); + ret = wh_Server_KeystoreReadKeyEnforce( + ctx, key_id, + enc != 0 ? WH_NVM_FLAGS_USAGE_ENCRYPT : WH_NVM_FLAGS_USAGE_DECRYPT, + keyMeta, cachedKey, &cachedKeyLen); + WH_DEBUG_SERVER_VERBOSE("AesGcm ReadKey key_id:%u ret:%d\n", key_id, + ret); if (ret == WH_ERROR_OK) { /* override the incoming values with cached key */ key = cachedKey; @@ -3767,6 +3980,7 @@ static int _HandleAesGcm(whServerContext* ctx, uint16_t magic, int devId, ret = wh_MessageCrypto_TranslateAesGcmResponse( magic, &res, (whMessageCrypto_AesGcmResponse*)cryptoDataOut); } + wc_ForceZero(cachedKey, sizeof(cachedKey)); return ret; } @@ -3787,8 +4001,8 @@ static int _HandleAesGcmDma(whServerContext* ctx, uint16_t magic, int devId, word32 outSz = 0; whKeyId keyId; - uint8_t* cachedKey = NULL; - whNvmMetadata* keyMeta = NULL; + uint8_t cachedKey[AES_256_KEY_SIZE]; + whNvmMetadata keyMeta[1]; (void)seq; @@ -3839,14 +4053,15 @@ static int _HandleAesGcmDma(whServerContext* ctx, uint16_t magic, int devId, /* Freshen key and validate usage policy if key is not erased */ if (!WH_KEYID_ISERASED(keyId)) { - ret = wh_Server_KeystoreFreshenKey(ctx, keyId, &cachedKey, - &keyMeta); - if (ret == WH_ERROR_OK) { - /* Validate key usage policy */ - ret = wh_Server_KeystoreEnforceKeyUsage( - keyMeta, enc != 0 ? WH_NVM_FLAGS_USAGE_ENCRYPT - : WH_NVM_FLAGS_USAGE_DECRYPT); - } + /* Copy the key + metadata into private buffers, enforcing the + * usage policy against the same locked snapshot that is read. The + * crypto below then runs entirely on the private copy. */ + uint32_t cachedKeyLen = sizeof(cachedKey); + ret = wh_Server_KeystoreReadKeyEnforce( + ctx, keyId, + enc != 0 ? WH_NVM_FLAGS_USAGE_ENCRYPT + : WH_NVM_FLAGS_USAGE_DECRYPT, + keyMeta, cachedKey, &cachedKeyLen); if (ret == WH_ERROR_OK) { key = cachedKey; keyLen = keyMeta->len; @@ -3962,6 +4177,7 @@ static int _HandleAesGcmDma(whServerContext* ctx, uint16_t magic, int devId, magic, &res, (whMessageCrypto_AesGcmDmaResponse*)cryptoDataOut); *outSize = sizeof(whMessageCrypto_AesGcmDmaResponse) + res.authTagSz; + wc_ForceZero(cachedKey, sizeof(cachedKey)); return ret; } #endif /* WOLFHSM_CFG_DMA */ @@ -3988,17 +4204,17 @@ static int _CmacResolveKey(whServerContext* ctx, const uint8_t* requestKey, whKeyId keyId = wh_KeyId_TranslateFromClient( WH_KEYTYPE_CRYPTO, ctx->comm->client_id, clientKeyId); - /* Validate key usage policy - CMAC accepts sign or verify */ - ret = wh_Server_KeystoreFindEnforceKeyUsage(ctx, keyId, - WH_NVM_FLAGS_USAGE_SIGN); + /* Copy the key into the caller's private buffer, enforcing the usage + * policy against the same locked snapshot that is read. CMAC accepts + * sign or verify usage, so retry with verify on a usage failure; each + * attempt is an atomic policy-check + read. */ + uint32_t reqKeyLen = *outKeyLen; + ret = wh_Server_KeystoreReadKeyEnforce( + ctx, keyId, WH_NVM_FLAGS_USAGE_SIGN, NULL, outKey, outKeyLen); if (ret == WH_ERROR_USAGE) { - ret = wh_Server_KeystoreFindEnforceKeyUsage( - ctx, keyId, WH_NVM_FLAGS_USAGE_VERIFY); - } - - if (ret == WH_ERROR_OK) { - ret = - wh_Server_KeystoreReadKey(ctx, keyId, NULL, outKey, outKeyLen); + *outKeyLen = reqKeyLen; + ret = wh_Server_KeystoreReadKeyEnforce( + ctx, keyId, WH_NVM_FLAGS_USAGE_VERIFY, NULL, outKey, outKeyLen); } if (ret == WH_ERROR_OK) { @@ -4133,6 +4349,7 @@ static int _HandleCmac(whServerContext* ctx, uint16_t magic, int devId, *outSize = sizeof(res) + res.outSz; } } + wc_ForceZero(tmpKey, sizeof(tmpKey)); WH_DEBUG_SERVER_VERBOSE("cmac end ret:%d\n", ret); return ret; } @@ -4646,22 +4863,24 @@ static int _HandleMlDsaKeyGen(whServerContext* ctx, uint16_t magic, int devId, /* Must import the key into the cache and return keyid */ res_size = 0; - if (WH_KEYID_ISERASED(key_id)) { - /* Generate a new id */ - ret = wh_Server_KeystoreGetUniqueId(ctx, &key_id); - WH_DEBUG_SERVER("UniqueId: keyId:%u, ret:%d\n", - key_id, ret); - if (ret != WH_ERROR_OK) { - /* Early return on unique ID generation failure - */ - wc_MlDsaKey_Free(key); - return ret; + /* Hold the NVM lock so id allocation and cache import + * are atomic with respect to other server contexts + * under THREADSAFE. */ + ret = WH_SERVER_NVM_LOCK(ctx); + if (ret == WH_ERROR_OK) { + if (WH_KEYID_ISERASED(key_id)) { + /* Generate a new id */ + ret = + wh_Server_KeystoreGetUniqueId(ctx, &key_id); + WH_DEBUG_SERVER("UniqueId: keyId:%u, ret:%d\n", + key_id, ret); } - } - if (ret == 0) { - ret = wh_Server_MlDsaKeyCacheImport( - ctx, key, key_id, flags, label_size, label); - } + if (ret == WH_ERROR_OK) { + ret = wh_Server_MlDsaKeyCacheImport( + ctx, key, key_id, flags, label_size, label); + } + (void)WH_SERVER_NVM_UNLOCK(ctx); + } /* WH_SERVER_NVM_LOCK() */ WH_DEBUG_SERVER("CacheImport: keyId:%u, ret:%d\n", key_id, ret); } @@ -4721,15 +4940,6 @@ static int _HandleMlDsaSign(whServerContext* ctx, uint16_t magic, int devId, uint32_t options = req.options; int evict = !!(options & WH_MESSAGE_CRYPTO_MLDSA_SIGN_OPTIONS_EVICT); - /* Validate key usage policy for signing */ - if (!WH_KEYID_ISERASED(key_id)) { - ret = wh_Server_KeystoreFindEnforceKeyUsage(ctx, key_id, - WH_NVM_FLAGS_USAGE_SIGN); - if (ret != WH_ERROR_OK) { - goto cleanup; - } - } - /* Validate input length against available data to prevent buffer overread */ if (inSize < sizeof(whMessageCrypto_MlDsaSignRequest)) { @@ -4757,8 +4967,10 @@ static int _HandleMlDsaSign(whServerContext* ctx, uint16_t magic, int devId, /* init private key */ ret = wc_MlDsaKey_Init(key, NULL, devId); if (ret == 0) { - /* load the private key */ - ret = wh_Server_MlDsaKeyCacheExport(ctx, key_id, key); + /* load the private key, enforcing the sign usage policy against the + * same locked snapshot that is exported */ + ret = wh_Server_MlDsaKeyCacheExportEnforce( + ctx, key_id, WH_NVM_FLAGS_USAGE_SIGN, key); if (ret == WH_ERROR_OK) { /* sign the input using appropriate FIPS 204 API */ if (preHashType != WC_HASH_TYPE_NONE) { @@ -4774,10 +4986,9 @@ static int _HandleMlDsaSign(whServerContext* ctx, uint16_t magic, int devId, } wc_MlDsaKey_Free(key); } -cleanup: if (evict != 0) { /* User requested to evict from cache, even if the call failed */ - (void)wh_Server_KeystoreEvictKey(ctx, key_id); + _CryptoEvictKeyLocked(ctx, key_id); } if (ret == 0) { res.sz = res_len; @@ -4828,15 +5039,6 @@ static int _HandleMlDsaVerify(whServerContext* ctx, uint16_t magic, int devId, (uint8_t*)(cryptoDataIn) + sizeof(whMessageCrypto_MlDsaVerifyRequest); int evict = !!(options & WH_MESSAGE_CRYPTO_MLDSA_VERIFY_OPTIONS_EVICT); - /* Validate key usage policy for verification */ - if (!WH_KEYID_ISERASED(key_id)) { - ret = wh_Server_KeystoreFindEnforceKeyUsage(ctx, key_id, - WH_NVM_FLAGS_USAGE_VERIFY); - if (ret != WH_ERROR_OK) { - goto cleanup; - } - } - /* Validate lengths against available payload (overflow-safe) */ if (inSize < sizeof(whMessageCrypto_MlDsaVerifyRequest)) { return WH_ERROR_BADARGS; @@ -4862,8 +5064,10 @@ static int _HandleMlDsaVerify(whServerContext* ctx, uint16_t magic, int devId, /* init public key */ ret = wc_MlDsaKey_Init(key, NULL, devId); if (ret == 0) { - /* load the public key */ - ret = wh_Server_MlDsaKeyCacheExport(ctx, key_id, key); + /* load the public key, enforcing the verify usage policy against the + * same locked snapshot that is exported */ + ret = wh_Server_MlDsaKeyCacheExportEnforce( + ctx, key_id, WH_NVM_FLAGS_USAGE_VERIFY, key); if (ret == WH_ERROR_OK) { /* verify the signature using appropriate FIPS 204 API */ if (preHashType != WC_HASH_TYPE_NONE) { @@ -4879,10 +5083,9 @@ static int _HandleMlDsaVerify(whServerContext* ctx, uint16_t magic, int devId, } wc_MlDsaKey_Free(key); } -cleanup: if (evict != 0) { /* User requested to evict from cache, even if the call failed */ - (void)wh_Server_KeystoreEvictKey(ctx, key_id); + _CryptoEvictKeyLocked(ctx, key_id); } if (ret == 0) { res.res = result; @@ -4995,14 +5198,20 @@ static int _HandleMlKemKeyGen(whServerContext* ctx, uint16_t magic, int devId, &res_size); } else { - if (WH_KEYID_ISERASED(key_id)) { - ret = wh_Server_KeystoreGetUniqueId(ctx, &key_id); - } + /* Hold the NVM lock so id allocation and cache import are + * atomic with respect to other server contexts under + * THREADSAFE. */ + ret = WH_SERVER_NVM_LOCK(ctx); if (ret == WH_ERROR_OK) { - ret = wh_Server_MlKemKeyCacheImport(ctx, key, key_id, - req.flags, label_size, - req.label); - } + if (WH_KEYID_ISERASED(key_id)) { + ret = wh_Server_KeystoreGetUniqueId(ctx, &key_id); + } + if (ret == WH_ERROR_OK) { + ret = wh_Server_MlKemKeyCacheImport( + ctx, key, key_id, req.flags, label_size, req.label); + } + (void)WH_SERVER_NVM_UNLOCK(ctx); + } /* WH_SERVER_NVM_LOCK() */ } } wc_MlKemKey_Free(key); @@ -5061,14 +5270,6 @@ static int _HandleMlKemEncaps(whServerContext* ctx, uint16_t magic, int devId, req.keyId); evict = !!(req.options & WH_MESSAGE_CRYPTO_MLKEM_ENCAPS_OPTIONS_EVICT); - if (!WH_KEYID_ISERASED(key_id)) { - ret = wh_Server_KeystoreFindEnforceKeyUsage(ctx, key_id, - WH_NVM_FLAGS_USAGE_DERIVE); - if (ret != WH_ERROR_OK) { - goto cleanup; - } - } - if (!_IsMlKemLevelSupported((int)req.level)) { ret = WH_ERROR_BADARGS; goto cleanup; @@ -5077,7 +5278,10 @@ static int _HandleMlKemEncaps(whServerContext* ctx, uint16_t magic, int devId, ret = wc_MlKemKey_Init(key, (int)req.level, NULL, devId); if (ret == 0) { keyInited = 1; - ret = wh_Server_MlKemKeyCacheExport(ctx, key_id, key); + /* Export the key, enforcing the derive usage policy against the same + * locked snapshot that is exported */ + ret = wh_Server_MlKemKeyCacheExportEnforce( + ctx, key_id, WH_NVM_FLAGS_USAGE_DERIVE, key); } /* Verify the exported key matches the requested level */ @@ -5122,7 +5326,7 @@ static int _HandleMlKemEncaps(whServerContext* ctx, uint16_t magic, int devId, } cleanup: if (evict != 0) { - (void)wh_Server_KeystoreEvictKey(ctx, key_id); + _CryptoEvictKeyLocked(ctx, key_id); } return ret; #endif /* WOLFSSL_MLKEM_NO_ENCAPSULATE */ @@ -5169,14 +5373,6 @@ static int _HandleMlKemDecaps(whServerContext* ctx, uint16_t magic, int devId, req.keyId); evict = !!(req.options & WH_MESSAGE_CRYPTO_MLKEM_DECAPS_OPTIONS_EVICT); - if (!WH_KEYID_ISERASED(key_id)) { - ret = wh_Server_KeystoreFindEnforceKeyUsage(ctx, key_id, - WH_NVM_FLAGS_USAGE_DERIVE); - if (ret != WH_ERROR_OK) { - goto cleanup; - } - } - if (!_IsMlKemLevelSupported((int)req.level)) { ret = WH_ERROR_BADARGS; goto cleanup; @@ -5192,7 +5388,10 @@ static int _HandleMlKemDecaps(whServerContext* ctx, uint16_t magic, int devId, ret = wc_MlKemKey_Init(key, (int)req.level, NULL, devId); if (ret == WH_ERROR_OK) { keyInited = 1; - ret = wh_Server_MlKemKeyCacheExport(ctx, key_id, key); + /* Export the key, enforcing the derive usage policy against the same + * locked snapshot that is exported */ + ret = wh_Server_MlKemKeyCacheExportEnforce( + ctx, key_id, WH_NVM_FLAGS_USAGE_DERIVE, key); } /* Verify the exported key matches the requested level */ @@ -5232,7 +5431,7 @@ static int _HandleMlKemDecaps(whServerContext* ctx, uint16_t magic, int devId, } cleanup: if (evict != 0) { - (void)wh_Server_KeystoreEvictKey(ctx, key_id); + _CryptoEvictKeyLocked(ctx, key_id); } return ret; #endif /* WOLFSSL_MLKEM_NO_DECAPSULATE */ @@ -6189,30 +6388,33 @@ static int _HandleMlDsaKeyGenDma(whServerContext* ctx, uint16_t magic, whKeyId keyId = wh_KeyId_TranslateFromClient( WH_KEYTYPE_CRYPTO, ctx->comm->client_id, req.keyId); - if (WH_KEYID_ISERASED(keyId)) { - /* Generate a new id */ - ret = wh_Server_KeystoreGetUniqueId(ctx, &keyId); - WH_DEBUG_SERVER("UniqueId: keyId:%u, ret:%d\n", - keyId, ret); - if (ret != WH_ERROR_OK) { - /* Early return on unique ID generation failure - */ - wc_MlDsaKey_Free(key); - return ret; + /* Hold the NVM lock so id allocation and cache import + * are atomic with respect to other server contexts + * under THREADSAFE. */ + ret = WH_SERVER_NVM_LOCK(ctx); + if (ret == WH_ERROR_OK) { + if (WH_KEYID_ISERASED(keyId)) { + /* Generate a new id */ + ret = + wh_Server_KeystoreGetUniqueId(ctx, &keyId); + WH_DEBUG_SERVER("UniqueId: keyId:%u, ret:%d\n", + keyId, ret); } - } - - if (ret == 0) { - ret = wh_Server_MlDsaKeyCacheImport( - ctx, key, keyId, req.flags, req.labelSize, - req.label); - WH_DEBUG_SERVER("CacheImport: keyId:%u, ret:%d\n", - keyId, ret); - if (ret == 0) { - res.keyId = wh_KeyId_TranslateToClient(keyId); - res.keySize = keySize; + if (ret == WH_ERROR_OK) { + ret = wh_Server_MlDsaKeyCacheImport( + ctx, key, keyId, req.flags, req.labelSize, + req.label); + WH_DEBUG_SERVER( + "CacheImport: keyId:%u, ret:%d\n", keyId, + ret); + if (ret == 0) { + res.keyId = + wh_KeyId_TranslateToClient(keyId); + res.keySize = keySize; + } } - } + (void)WH_SERVER_NVM_UNLOCK(ctx); + } /* WH_SERVER_NVM_LOCK() */ } } } @@ -6297,7 +6499,11 @@ static int _HandleMlDsaSignDma(whServerContext* ctx, uint16_t magic, int devId, if (ret == 0) { /* Export key from cache */ /* TODO: sanity check security level against key pulled from cache? */ - ret = wh_Server_MlDsaKeyCacheExport(ctx, key_id, key); + /* Export the key, enforcing the sign usage policy against the same + * locked snapshot that is exported. The non-DMA sign handler enforces + * the same policy. */ + ret = wh_Server_MlDsaKeyCacheExportEnforce( + ctx, key_id, WH_NVM_FLAGS_USAGE_SIGN, key); if (ret == 0) { /* Process client message buffer address */ ret = wh_Server_DmaProcessClientAddress( @@ -6342,17 +6548,16 @@ static int _HandleMlDsaSignDma(whServerContext* ctx, uint16_t magic, int devId, (whServerDmaFlags){0}); } } - - /* Evict key if requested */ - if (evict) { - /* User requested to evict from cache, even if the call failed - */ - (void)wh_Server_KeystoreEvictKey(ctx, key_id); - } } wc_MlDsaKey_Free(key); } + /* Evict key if requested */ + if (evict) { + /* User requested to evict from cache, even if the call failed */ + _CryptoEvictKeyLocked(ctx, key_id); + } + if (ret == 0) { /* Set response signature length */ res.sigLen = sigLen; @@ -6431,8 +6636,11 @@ static int _HandleMlDsaVerifyDma(whServerContext* ctx, uint16_t magic, return ret; } - /* Export key from cache */ - ret = wh_Server_MlDsaKeyCacheExport(ctx, key_id, key); + /* Export the key, enforcing the verify usage policy against the same + * locked snapshot that is exported. The non-DMA verify handler enforces + * the same policy. */ + ret = wh_Server_MlDsaKeyCacheExportEnforce(ctx, key_id, + WH_NVM_FLAGS_USAGE_VERIFY, key); if (ret == 0) { /* Process client signature buffer address */ ret = wh_Server_DmaProcessClientAddress( @@ -6474,12 +6682,12 @@ static int _HandleMlDsaVerifyDma(whServerContext* ctx, uint16_t magic, (whServerDmaFlags){0}); } } + } - /* Evict key if requested */ - if (evict) { - /* User requested to evict from cache, even if the call failed */ - (void)wh_Server_KeystoreEvictKey(ctx, key_id); - } + /* Evict key if requested */ + if (evict) { + /* User requested to evict from cache, even if the call failed */ + _CryptoEvictKeyLocked(ctx, key_id); } if (ret == 0) { @@ -6631,18 +6839,25 @@ static int _HandleMlKemKeyGenDma(whServerContext* ctx, uint16_t magic, whKeyId keyId = wh_KeyId_TranslateFromClient( WH_KEYTYPE_CRYPTO, ctx->comm->client_id, req.keyId); - if (WH_KEYID_ISERASED(keyId)) { - ret = wh_Server_KeystoreGetUniqueId(ctx, &keyId); - } + /* Hold the NVM lock so id allocation and cache import are + * atomic with respect to other server contexts under + * THREADSAFE. */ + ret = WH_SERVER_NVM_LOCK(ctx); if (ret == WH_ERROR_OK) { - ret = wh_Server_MlKemKeyCacheImport( - ctx, key, keyId, req.flags, req.labelSize, - req.label); + if (WH_KEYID_ISERASED(keyId)) { + ret = wh_Server_KeystoreGetUniqueId(ctx, &keyId); + } if (ret == WH_ERROR_OK) { - res.keyId = wh_KeyId_TranslateToClient(keyId); - res.keySize = keySize; + ret = wh_Server_MlKemKeyCacheImport( + ctx, key, keyId, req.flags, req.labelSize, + req.label); + if (ret == WH_ERROR_OK) { + res.keyId = wh_KeyId_TranslateToClient(keyId); + res.keySize = keySize; + } } - } + (void)WH_SERVER_NVM_UNLOCK(ctx); + } /* WH_SERVER_NVM_LOCK() */ } } wc_MlKemKey_Free(key); @@ -6704,14 +6919,6 @@ static int _HandleMlKemEncapsDma(whServerContext* ctx, uint16_t magic, ctx->comm->client_id, req.keyId); evict = !!(req.options & WH_MESSAGE_CRYPTO_MLKEM_ENCAPS_OPTIONS_EVICT); - if (!WH_KEYID_ISERASED(key_id)) { - ret = wh_Server_KeystoreFindEnforceKeyUsage(ctx, key_id, - WH_NVM_FLAGS_USAGE_DERIVE); - if (ret != WH_ERROR_OK) { - goto cleanup; - } - } - if (!_IsMlKemLevelSupported((int)req.level)) { ret = WH_ERROR_BADARGS; goto cleanup; @@ -6720,7 +6927,10 @@ static int _HandleMlKemEncapsDma(whServerContext* ctx, uint16_t magic, ret = wc_MlKemKey_Init(key, (int)req.level, NULL, devId); if (ret == WH_ERROR_OK) { keyInited = 1; - ret = wh_Server_MlKemKeyCacheExport(ctx, key_id, key); + /* Export the key, enforcing the derive usage policy against the same + * locked snapshot that is exported */ + ret = wh_Server_MlKemKeyCacheExportEnforce( + ctx, key_id, WH_NVM_FLAGS_USAGE_DERIVE, key); } /* Verify the exported key matches the requested level */ @@ -6788,7 +6998,7 @@ static int _HandleMlKemEncapsDma(whServerContext* ctx, uint16_t magic, } cleanup: if (evict != 0) { - (void)wh_Server_KeystoreEvictKey(ctx, key_id); + _CryptoEvictKeyLocked(ctx, key_id); } (void)wh_MessageCrypto_TranslateMlKemEncapsDmaResponse( @@ -6841,14 +7051,6 @@ static int _HandleMlKemDecapsDma(whServerContext* ctx, uint16_t magic, ctx->comm->client_id, req.keyId); evict = !!(req.options & WH_MESSAGE_CRYPTO_MLKEM_DECAPS_OPTIONS_EVICT); - if (!WH_KEYID_ISERASED(key_id)) { - ret = wh_Server_KeystoreFindEnforceKeyUsage(ctx, key_id, - WH_NVM_FLAGS_USAGE_DERIVE); - if (ret != WH_ERROR_OK) { - goto cleanup; - } - } - if (!_IsMlKemLevelSupported((int)req.level)) { ret = WH_ERROR_BADARGS; goto cleanup; @@ -6857,7 +7059,10 @@ static int _HandleMlKemDecapsDma(whServerContext* ctx, uint16_t magic, ret = wc_MlKemKey_Init(key, (int)req.level, NULL, devId); if (ret == WH_ERROR_OK) { keyInited = 1; - ret = wh_Server_MlKemKeyCacheExport(ctx, key_id, key); + /* Export the key, enforcing the derive usage policy against the same + * locked snapshot that is exported */ + ret = wh_Server_MlKemKeyCacheExportEnforce( + ctx, key_id, WH_NVM_FLAGS_USAGE_DERIVE, key); } /* Verify the exported key matches the requested level */ @@ -6916,7 +7121,7 @@ static int _HandleMlKemDecapsDma(whServerContext* ctx, uint16_t magic, } cleanup: if (evict != 0) { - (void)wh_Server_KeystoreEvictKey(ctx, key_id); + _CryptoEvictKeyLocked(ctx, key_id); } (void)wh_MessageCrypto_TranslateMlKemDecapsDmaResponse( @@ -7425,7 +7630,7 @@ static int _HandleLmsVerifyDma(whServerContext* ctx, uint16_t magic, int devId, } if ((req.options & WH_MESSAGE_CRYPTO_STATEFUL_SIG_OPTIONS_EVICT) != 0) { - (void)wh_Server_KeystoreEvictKey(ctx, keyId); + _CryptoEvictKeyLocked(ctx, keyId); } (void)wh_MessageCrypto_TranslatePqcStatefulSigVerifyDmaResponse( @@ -7915,7 +8120,7 @@ static int _HandleXmssVerifyDma(whServerContext* ctx, uint16_t magic, } if ((req.options & WH_MESSAGE_CRYPTO_STATEFUL_SIG_OPTIONS_EVICT) != 0) { - (void)wh_Server_KeystoreEvictKey(ctx, keyId); + _CryptoEvictKeyLocked(ctx, keyId); } (void)wh_MessageCrypto_TranslatePqcStatefulSigVerifyDmaResponse( @@ -8274,6 +8479,7 @@ static int _HandleCmacDma(whServerContext* ctx, uint16_t magic, int devId, } } + wc_ForceZero(tmpKey, sizeof(tmpKey)); WH_DEBUG_SERVER_VERBOSE("dma cmac end ret:%d\n", ret); return ret; } diff --git a/src/wh_server_keystore.c b/src/wh_server_keystore.c index 90cfb56be..1e89d7657 100644 --- a/src/wh_server_keystore.c +++ b/src/wh_server_keystore.c @@ -1064,6 +1064,43 @@ int wh_Server_KeystoreReadKeyChecked(whServerContext* server, whKeyId keyId, return wh_Server_KeystoreReadKey(server, keyId, outMeta, out, outSz); } +int wh_Server_KeystoreReadKeyEnforce(whServerContext* server, whKeyId keyId, + whNvmFlags requiredUsage, + whNvmMetadata* outMeta, uint8_t* out, + uint32_t* outSz) +{ + int ret; + whNvmMetadata meta[1]; + + if ((server == NULL) || (outSz == NULL)) { + return WH_ERROR_BADARGS; + } + + /* Copy the key and check its usage flags under one hold of the NVM lock + * so the policy verdict and the key material come from the same snapshot: + * another server context cannot erase/re-cache the key between the check + * and the read. */ + ret = WH_SERVER_NVM_LOCK(server); + if (ret == WH_ERROR_OK) { + ret = wh_Server_KeystoreReadKey(server, keyId, meta, out, outSz); + if (ret == WH_ERROR_OK) { + ret = wh_Server_KeystoreEnforceKeyUsage(meta, requiredUsage); + if (ret == WH_ERROR_OK) { + if (outMeta != NULL) { + memcpy((uint8_t*)outMeta, (uint8_t*)meta, + sizeof(whNvmMetadata)); + } + } + else if (out != NULL) { + /* Don't hand back key material that failed the policy check */ + memset(out, 0, *outSz); + } + } + (void)WH_SERVER_NVM_UNLOCK(server); + } + return ret; +} + int wh_Server_KeystoreEvictKey(whServerContext* server, whNvmId keyId) { int ret = 0; @@ -3216,6 +3253,10 @@ int wh_Server_KeystoreEnforceKeyUsage(const whNvmMetadata* meta, return WH_ERROR_USAGE; } +/* May be deprecated soon: see wh_server_keystore.h. Enforcing here and using + * the key in a later lock scope leaves a TOCTOU window; prefer + * wh_Server_KeystoreReadKeyEnforce or the typed wh_Server_*CacheExport*Enforce + * wrappers. */ int wh_Server_KeystoreFindEnforceKeyUsage(whServerContext* server, whKeyId keyId, whNvmFlags requiredUsage) @@ -3228,14 +3269,20 @@ int wh_Server_KeystoreFindEnforceKeyUsage(whServerContext* server, return WH_ERROR_BADARGS; } - /* Freshen the key to obtain the metadata */ - ret = wh_Server_KeystoreFreshenKey(server, keyId, NULL, &meta); - if (ret != WH_ERROR_OK) { - return ret; + /* Freshen the key and read its metadata under the NVM lock so the shared + * cache slot cannot be evicted or overwritten by another server context + * while we inspect the usage flags. FreshenKey hands back a pointer into + * the shared slot, so the policy check must complete before we unlock. */ + ret = WH_SERVER_NVM_LOCK(server); + if (ret == WH_ERROR_OK) { + ret = wh_Server_KeystoreFreshenKey(server, keyId, NULL, &meta); + if (ret == WH_ERROR_OK) { + /* Enforce the usage policy with the obtained metadata */ + ret = wh_Server_KeystoreEnforceKeyUsage(meta, requiredUsage); + } + (void)WH_SERVER_NVM_UNLOCK(server); } - - /* Enforce the usage policy with the obtained metadata */ - return wh_Server_KeystoreEnforceKeyUsage(meta, requiredUsage); + return ret; } #endif /* !WOLFHSM_CFG_NO_CRYPTO && WOLFHSM_CFG_ENABLE_SERVER */ diff --git a/test-refactor/posix/Makefile b/test-refactor/posix/Makefile index 3169736ad..19cfc6350 100644 --- a/test-refactor/posix/Makefile +++ b/test-refactor/posix/Makefile @@ -85,6 +85,18 @@ ifeq ($(ASAN),1) LDFLAGS += -fsanitize=address endif +# ThreadSanitizer. Enables the acquire/release transport shims in the +# concurrent tests (WOLFHSM_CFG_TEST_STRESS_TSAN) so TSAN's analysis stays on +# keystore/NVM locking rather than transport false positives. +ifeq ($(TSAN),1) + ifeq ($(ASAN),1) + $(error TSAN and ASAN cannot be used together) + endif + CFLAGS += -fsanitize=thread -fPIE + LDFLAGS += -fsanitize=thread -pie + DEF += -DWOLFSSL_NO_FENCE -DWOLFHSM_CFG_TEST_STRESS_TSAN +endif + # Enable threadsafe mode, adding lock protection to shared structures ifeq ($(THREADSAFE),1) DEF += -DWOLFHSM_CFG_THREADSAFE @@ -285,10 +297,18 @@ coverage-json: --filter '\.\./\.\./wolfhsm/.*' \ --json $(OUT) +# Under TSAN: fail fast and non-zero on any detected race, using the shared +# suppressions (wolfCrypt internals + SHM transport). +ifeq ($(TSAN),1) + RUN_ENV = TSAN_OPTIONS="halt_on_error=1:exitcode=66:suppressions=$(TEST_DIR)/tsan.supp" +else + RUN_ENV = +endif + # Send the full test output to test-suite.log, not just the summary check run: build_app @rm -f test-suite.log - @$(BUILD_DIR)/$(BIN).elf 2>&1 \ + @$(RUN_ENV) $(BUILD_DIR)/$(BIN).elf 2>&1 \ | tee test-suite.log \ | grep --line-buffered -E \ '^whTest_|^All [0-9]+ tests passed|^[0-9]+ passed, [0-9]+ skipped'; \ diff --git a/test-refactor/posix/wh_test_keygen_unique_id.c b/test-refactor/posix/wh_test_keygen_unique_id.c new file mode 100644 index 000000000..916842a7b --- /dev/null +++ b/test-refactor/posix/wh_test_keygen_unique_id.c @@ -0,0 +1,699 @@ +/* + * 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/posix/wh_test_keygen_unique_id.c + * + * Concurrent unique-id allocation test for the crypto keygen handlers in + * src/wh_server_crypto.c. + * + * KU_NUM_CLIENTS client/server pairs share a single locked NVM. For each + * supported algorithm, all clients issue a barrier-aligned cache keygen + * with an ERASED global key id, so every server auto-allocates an id from + * the shared global namespace. Each round asserts that all successful + * keygens returned distinct ids. + * + * Requires WOLFHSM_CFG_THREADSAFE and WOLFHSM_CFG_GLOBAL_KEYS: global keys + * (USER == 0) live in the one shared server->nvm->globalCache, which is + * what makes the allocations contend. Under TSAN (TSAN=1) any unserialized + * access to that cache is additionally reported as a data race. + */ + +#include "wolfhsm/wh_settings.h" + +/* pthread_barrier_t is unavailable on macOS. */ +#if defined(WOLFHSM_CFG_THREADSAFE) && defined(WOLFHSM_CFG_TEST_POSIX) && \ + defined(WOLFHSM_CFG_GLOBAL_KEYS) && defined(WOLFHSM_CFG_ENABLE_CLIENT) && \ + defined(WOLFHSM_CFG_ENABLE_SERVER) && !defined(WOLFHSM_CFG_NO_CRYPTO) && \ + !defined(__APPLE__) +#define WH_KU_ENABLED +#endif + +#include "wh_test_common.h" +#include "wh_test_list.h" /* WH_TEST_SKIPPED */ +#include "wh_test_keygen_unique_id.h" + +#ifndef WH_KU_ENABLED + +int whTest_KeygenUniqueIdConcurrent(void* ctx) +{ + (void)ctx; + return WH_TEST_SKIPPED; +} + +#else /* WH_KU_ENABLED */ + +#include +#include +#include +#include + +#include "wolfhsm/wh_error.h" +#include "wolfhsm/wh_comm.h" +#include "wolfhsm/wh_transport_mem.h" +#include "wolfhsm/wh_client.h" +#include "wolfhsm/wh_client_crypto.h" +#include "wolfhsm/wh_server.h" +#include "wolfhsm/wh_nvm.h" +#include "wolfhsm/wh_nvm_flash.h" +#include "wolfhsm/wh_flash_ramsim.h" +#include "wolfhsm/wh_lock.h" +#include "wolfhsm/wh_keyid.h" + +#include "port/posix/posix_lock.h" + +#include "wolfssl/wolfcrypt/settings.h" +#include "wolfssl/wolfcrypt/types.h" +#include "wolfssl/wolfcrypt/error-crypt.h" +#ifdef HAVE_CURVE25519 +#include "wolfssl/wolfcrypt/curve25519.h" +#endif +#ifdef HAVE_ECC +#include "wolfssl/wolfcrypt/ecc.h" +#endif +#ifdef HAVE_DILITHIUM +#include "wolfssl/wolfcrypt/dilithium.h" +#endif + +/* TSAN transport shims: the mem transport's notify counter establishes + * happens-before between client and server, but TSAN can't see it. + * Annotating the send/recv pairs keeps the analysis on keystore/NVM + * locking instead of transport false positives. */ +#ifdef WOLFHSM_CFG_TEST_STRESS_TSAN + +#ifndef __has_feature +#define __has_feature(x) 0 +#endif +#if !defined(__SANITIZE_THREAD__) && !__has_feature(thread_sanitizer) +#error ThreadSanitizer not enabled for this build +#endif + +#include + +static int ku_SendRequest(void* c, uint16_t len, const void* data) +{ + whTransportMemContext* ctx = (whTransportMemContext*)c; + __tsan_release((void*)ctx->req); + return wh_TransportMem_SendRequest(c, len, data); +} +static int ku_RecvResponse(void* c, uint16_t* out_len, void* data) +{ + whTransportMemContext* ctx = (whTransportMemContext*)c; + int rc = wh_TransportMem_RecvResponse(c, out_len, data); + if (rc == WH_ERROR_OK) { + __tsan_acquire((void*)ctx->resp); + } + return rc; +} +static int ku_RecvRequest(void* c, uint16_t* out_len, void* data) +{ + whTransportMemContext* ctx = (whTransportMemContext*)c; + int rc = wh_TransportMem_RecvRequest(c, out_len, data); + if (rc == WH_ERROR_OK) { + __tsan_acquire((void*)ctx->req); + } + return rc; +} +static int ku_SendResponse(void* c, uint16_t len, const void* data) +{ + whTransportMemContext* ctx = (whTransportMemContext*)c; + __tsan_release((void*)ctx->resp); + return wh_TransportMem_SendResponse(c, len, data); +} + +static const whTransportClientCb clientTransportCb = { + .Init = wh_TransportMem_InitClear, + .Send = ku_SendRequest, + .Recv = ku_RecvResponse, + .Cleanup = wh_TransportMem_Cleanup, +}; +static const whTransportServerCb serverTransportCb = { + .Init = wh_TransportMem_Init, + .Recv = ku_RecvRequest, + .Send = ku_SendResponse, + .Cleanup = wh_TransportMem_Cleanup, +}; + +#else /* !WOLFHSM_CFG_TEST_STRESS_TSAN */ + +static const whTransportClientCb clientTransportCb = WH_TRANSPORT_MEM_CLIENT_CB; +static const whTransportServerCb serverTransportCb = WH_TRANSPORT_MEM_SERVER_CB; + +#endif /* WOLFHSM_CFG_TEST_STRESS_TSAN */ + +#define KU_ATOMIC_LOAD(ptr) __atomic_load_n((ptr), __ATOMIC_ACQUIRE) +#define KU_ATOMIC_STORE(ptr, v) __atomic_store_n((ptr), (v), __ATOMIC_RELEASE) +#define KU_ATOMIC_ADD(ptr, v) __atomic_add_fetch((ptr), (v), __ATOMIC_ACQ_REL) + +/* Four concurrent client/server pairs sharing one NVM. Four fits the + * regular key cache (9 slots) and over-subscribes the big-key cache (3), + * which is fine: only *successful* keygens must have distinct ids. */ +#define KU_NUM_CLIENTS 4 + +#define KU_FLASH_RAM_SIZE (1024 * 1024) +#define KU_FLASH_SECTOR_SIZE (128 * 1024) +#define KU_FLASH_PAGE_SIZE 8 + +/* Auto-id keygen messages are tiny: params in, key id out. */ +#define KU_BUFFER_SIZE 4096 + +/* Rounds per algorithm; the slower big-key keygens use fewer to bound + * wall-clock. */ +#define KU_ROUNDS_SMALL 100 +#define KU_ROUNDS_BIG 24 + +/* Each thunk issues one blocking cache keygen for a GLOBAL key with an + * ERASED id, so the server auto-allocates an id from the shared global + * namespace. *outId receives it in client (flagged) format, suitable for + * wh_Client_KeyEvict(). */ +typedef int (*kuGenFn)(whClientContext* client, whKeyId* outId); + +#define KU_ERASED_GLOBAL WH_CLIENT_KEYID_MAKE_GLOBAL(0) + +#ifdef HAVE_CURVE25519 +static int kuGen_Curve25519(whClientContext* client, whKeyId* outId) +{ + whKeyId id = KU_ERASED_GLOBAL; + int rc = wh_Client_Curve25519MakeCacheKey( + client, (uint16_t)CURVE25519_KEYSIZE, &id, WH_NVM_FLAGS_NONE, NULL, 0); + *outId = id; + return rc; +} +#endif + +#ifdef HAVE_ED25519 +static int kuGen_Ed25519(whClientContext* client, whKeyId* outId) +{ + whKeyId id = KU_ERASED_GLOBAL; + int rc = + wh_Client_Ed25519MakeCacheKey(client, &id, WH_NVM_FLAGS_NONE, 0, NULL); + *outId = id; + return rc; +} +#endif + +#if defined(HAVE_ECC) && defined(WOLFSSL_KEY_GEN) +static int kuGen_Ecc(whClientContext* client, whKeyId* outId) +{ + whKeyId id = KU_ERASED_GLOBAL; + int rc = wh_Client_EccMakeCacheKey(client, 32, ECC_SECP256R1, &id, + WH_NVM_FLAGS_NONE, 0, NULL); + *outId = id; + return rc; +} +#endif + +#if defined(HAVE_DILITHIUM) && !defined(WOLFSSL_DILITHIUM_NO_MAKE_KEY) +static int kuGen_MlDsa(whClientContext* client, whKeyId* outId) +{ + whKeyId id = KU_ERASED_GLOBAL; + int rc = wh_Client_MlDsaMakeCacheKey(client, 0, WC_ML_DSA_44, &id, + WH_NVM_FLAGS_NONE, 0, NULL); + *outId = id; + return rc; +} +#endif + +typedef struct { + const char* name; + kuGenFn gen; + int rounds; +} KuAlgo; + +static const KuAlgo kuAlgos[] = { +#ifdef HAVE_CURVE25519 + {"Curve25519", kuGen_Curve25519, KU_ROUNDS_SMALL}, +#endif +#ifdef HAVE_ED25519 + {"Ed25519", kuGen_Ed25519, KU_ROUNDS_SMALL}, +#endif +#if defined(HAVE_ECC) && defined(WOLFSSL_KEY_GEN) + {"ECC", kuGen_Ecc, KU_ROUNDS_SMALL}, +#endif +#if defined(HAVE_DILITHIUM) && !defined(WOLFSSL_DILITHIUM_NO_MAKE_KEY) + {"ML-DSA", kuGen_MlDsa, KU_ROUNDS_BIG}, +#endif +}; + +#define KU_NUM_ALGOS ((int)(sizeof(kuAlgos) / sizeof(kuAlgos[0]))) + +struct KuContext; + +typedef struct { + uint8_t reqBuf[KU_BUFFER_SIZE]; + uint8_t respBuf[KU_BUFFER_SIZE]; + whTransportMemConfig tmConfig; + + whTransportMemClientContext clientTransportCtx; + whCommClientConfig clientCommConfig; + whClientContext client; + whClientConfig clientConfig; + + whTransportMemServerContext serverTransportCtx; + whCommServerConfig serverCommConfig; + whServerContext server; + whServerConfig serverConfig; + whServerCryptoContext cryptoCtx; + + pthread_t clientThread; + pthread_t serverThread; + int idx; + + struct KuContext* shared; +} KuPair; + +typedef struct KuContext { + /* Shared, locked NVM */ + uint8_t flashMemory[KU_FLASH_RAM_SIZE]; + whFlashRamsimCtx flashCtx; + whFlashRamsimCfg flashCfg; + whNvmFlashContext nvmFlashCtx; + whNvmFlashConfig nvmFlashCfg; + whNvmContext nvm; + whNvmConfig nvmCfg; + + posixLockContext nvmLockCtx; + pthread_mutexattr_t mutexAttr; + posixLockConfig posixLockCfg; + whLockConfig lockCfg; + + KuPair pairs[KU_NUM_CLIENTS]; + + /* Servers stay up across every algorithm; client threads are respawned + * per algorithm. A readiness counter rather than a barrier, so a server + * that fails to spawn does not hang the others. */ + volatile int serversReady; + volatile int serverError; + volatile int stopFlag; + + /* Per-round client synchronization (clients only) */ + pthread_barrier_t roundStart; + pthread_barrier_t roundMid; + pthread_barrier_t roundEnd; + + /* Current algorithm under test */ + kuGenFn genFn; + int rounds; + const char* algoName; + + /* Per-round results, indexed by client */ + volatile whKeyId roundIds[KU_NUM_CLIENTS]; + volatile int roundRc[KU_NUM_CLIENTS]; + + /* Outcome counters */ + volatile int collisions; + volatile int hardErrors; + volatile int successes; +} KuContext; + +/* The context is large (1 MB flash buffer); keep it off the thread stack. */ +static KuContext g_ku; + +static int kuInitSharedNvm(KuContext* ctx) +{ + static whFlashCb flashCb = WH_FLASH_RAMSIM_CB; + static whNvmCb nvmCb = WH_NVM_FLASH_CB; + static whLockCb lockCb = POSIX_LOCK_CB; + int rc; + + memset(ctx->flashMemory, 0xFF, sizeof(ctx->flashMemory)); + + ctx->flashCfg.size = KU_FLASH_RAM_SIZE; + ctx->flashCfg.sectorSize = KU_FLASH_SECTOR_SIZE; + ctx->flashCfg.pageSize = KU_FLASH_PAGE_SIZE; + ctx->flashCfg.erasedByte = 0xFF; + ctx->flashCfg.memory = ctx->flashMemory; + + ctx->nvmFlashCfg.cb = &flashCb; + ctx->nvmFlashCfg.context = &ctx->flashCtx; + ctx->nvmFlashCfg.config = &ctx->flashCfg; + + rc = wh_NvmFlash_Init(&ctx->nvmFlashCtx, &ctx->nvmFlashCfg); + if (rc != WH_ERROR_OK) { + WH_ERROR_PRINT("NVM flash init failed: %d\n", rc); + return rc; + } + + /* Error-checking mutex catches lock misuse (double-unlock, etc.). */ + memset(&ctx->nvmLockCtx, 0, sizeof(ctx->nvmLockCtx)); + pthread_mutexattr_init(&ctx->mutexAttr); + pthread_mutexattr_settype(&ctx->mutexAttr, PTHREAD_MUTEX_ERRORCHECK); + ctx->posixLockCfg.attr = &ctx->mutexAttr; + ctx->lockCfg.cb = &lockCb; + ctx->lockCfg.context = &ctx->nvmLockCtx; + ctx->lockCfg.config = &ctx->posixLockCfg; + + ctx->nvmCfg.cb = &nvmCb; + ctx->nvmCfg.context = &ctx->nvmFlashCtx; + ctx->nvmCfg.config = &ctx->nvmFlashCfg; + ctx->nvmCfg.lockConfig = &ctx->lockCfg; + + rc = wh_Nvm_Init(&ctx->nvm, &ctx->nvmCfg); + if (rc != WH_ERROR_OK) { + WH_ERROR_PRINT("NVM init failed: %d\n", rc); + return rc; + } + return WH_ERROR_OK; +} + +static int kuInitPair(KuContext* ctx, int idx) +{ + KuPair* pair = &ctx->pairs[idx]; + int rc; + + pair->idx = idx; + pair->shared = ctx; + + pair->tmConfig.req = (whTransportMemCsr*)pair->reqBuf; + pair->tmConfig.req_size = sizeof(pair->reqBuf); + pair->tmConfig.resp = (whTransportMemCsr*)pair->respBuf; + pair->tmConfig.resp_size = sizeof(pair->respBuf); + + memset(&pair->clientTransportCtx, 0, sizeof(pair->clientTransportCtx)); + pair->clientCommConfig.transport_cb = &clientTransportCb; + pair->clientCommConfig.transport_context = &pair->clientTransportCtx; + pair->clientCommConfig.transport_config = &pair->tmConfig; + /* client_id must be in [1, WH_CLIENT_ID_MAX]; distinct per pair. */ + pair->clientCommConfig.client_id = (uint8_t)(1 + idx); + pair->clientConfig.comm = &pair->clientCommConfig; + + memset(&pair->serverTransportCtx, 0, sizeof(pair->serverTransportCtx)); + pair->serverCommConfig.transport_cb = &serverTransportCb; + pair->serverCommConfig.transport_context = &pair->serverTransportCtx; + pair->serverCommConfig.transport_config = &pair->tmConfig; + pair->serverCommConfig.server_id = (uint16_t)(200 + idx); + + rc = wc_InitRng_ex(pair->cryptoCtx.rng, NULL, INVALID_DEVID); + if (rc != 0) { + WH_ERROR_PRINT("RNG init failed for pair %d: %d\n", idx, rc); + return rc; + } + + /* All servers share the one locked NVM. */ + pair->serverConfig.comm_config = &pair->serverCommConfig; + pair->serverConfig.nvm = &ctx->nvm; + pair->serverConfig.crypto = &pair->cryptoCtx; + pair->serverConfig.devId = INVALID_DEVID; + + /* Init the client here (main thread) to avoid concurrent wolfCrypt + * init/register from the worker threads. */ + rc = wh_Client_Init(&pair->client, &pair->clientConfig); + if (rc != WH_ERROR_OK) { + WH_ERROR_PRINT("Client %d init failed: %d\n", idx, rc); + wc_FreeRng(pair->cryptoCtx.rng); + return rc; + } + return WH_ERROR_OK; +} + +static void kuCleanupPair(KuPair* pair) +{ + wh_Client_Cleanup(&pair->client); + wc_FreeRng(pair->cryptoCtx.rng); +} + +/* Serve requests for one pair until stopFlag is set. */ +static void* kuServerThread(void* arg) +{ + KuPair* pair = (KuPair*)arg; + KuContext* ctx = pair->shared; + int rc; + + rc = wh_Server_Init(&pair->server, &pair->serverConfig); + if (rc != WH_ERROR_OK) { + WH_ERROR_PRINT("Server %d init failed: %d\n", pair->idx, rc); + KU_ATOMIC_STORE(&ctx->serverError, 1); + KU_ATOMIC_ADD(&ctx->serversReady, 1); + return NULL; + } + rc = wh_Server_SetConnected(&pair->server, WH_COMM_CONNECTED); + if (rc != WH_ERROR_OK) { + WH_ERROR_PRINT("Server %d SetConnected failed: %d\n", pair->idx, rc); + KU_ATOMIC_STORE(&ctx->serverError, 1); + wh_Server_Cleanup(&pair->server); + KU_ATOMIC_ADD(&ctx->serversReady, 1); + return NULL; + } + + /* Announce readiness; main waits for all servers before clients run. */ + KU_ATOMIC_ADD(&ctx->serversReady, 1); + + while (!KU_ATOMIC_LOAD(&ctx->stopFlag)) { + rc = wh_Server_HandleRequestMessage(&pair->server); + if (rc == WH_ERROR_NOTREADY) { + sched_yield(); + } + /* Other per-request errors surface to the client as response codes. */ + } + + wh_Server_Cleanup(&pair->server); + return NULL; +} + +/* Per-round detector, run by client 0 once every keygen has landed. */ +static void kuCheckRound(KuContext* ctx, int round) +{ + int i; + int j; + + for (i = 0; i < KU_NUM_CLIENTS; i++) { + int rc = ctx->roundRc[i]; + if (rc == WH_ERROR_OK) { + KU_ATOMIC_ADD(&ctx->successes, 1); + } + else if (rc != WH_ERROR_NOSPACE) { + /* NOSPACE is expected: the global id/cache space is finite under + * contention. Any other failure invalidates the round. */ + KU_ATOMIC_ADD(&ctx->hardErrors, 1); + WH_ERROR_PRINT("%s round %d: client %d keygen failed: %d\n", + ctx->algoName, round, i, rc); + } + } + + /* Two successful concurrent keygens must never auto-allocate the same + * id. */ + for (i = 0; i < KU_NUM_CLIENTS; i++) { + if (ctx->roundRc[i] != WH_ERROR_OK) { + continue; + } + for (j = i + 1; j < KU_NUM_CLIENTS; j++) { + if (ctx->roundRc[j] != WH_ERROR_OK) { + continue; + } + if (ctx->roundIds[i] == ctx->roundIds[j]) { + KU_ATOMIC_ADD(&ctx->collisions, 1); + WH_ERROR_PRINT( + "%s round %d: id collision 0x%04X (clients %d and %d)\n", + ctx->algoName, round, (unsigned)ctx->roundIds[i], i, j); + } + } + } +} + +/* Run ctx->rounds barrier-aligned keygens for the current algorithm. */ +static void* kuClientThread(void* arg) +{ + KuPair* pair = (KuPair*)arg; + KuContext* ctx = pair->shared; + int round; + + for (round = 0; round < ctx->rounds; round++) { + whKeyId id = WH_KEYID_ERASED; + int rc; + + /* Line all clients up so their id allocations overlap. */ + pthread_barrier_wait(&ctx->roundStart); + + rc = ctx->genFn(&pair->client, &id); + ctx->roundRc[pair->idx] = rc; + ctx->roundIds[pair->idx] = (rc == WH_ERROR_OK) ? id : WH_KEYID_ERASED; + + /* All ids recorded before the detector reads them. */ + pthread_barrier_wait(&ctx->roundMid); + + if (pair->idx == 0) { + kuCheckRound(ctx, round); + } + + /* Drop this round's key so the next round starts from a clean cache + * (best effort: a colliding id may already be gone). */ + if (rc == WH_ERROR_OK) { + (void)wh_Client_KeyEvict(&pair->client, id); + } + + /* Hold everyone until this round's keys are evicted. */ + pthread_barrier_wait(&ctx->roundEnd); + } + return NULL; +} + +int whTest_KeygenUniqueIdConcurrent(void* ctx_arg) +{ + KuContext* ctx = &g_ku; + int i; + int a; + int rc; + int result = WH_TEST_SUCCESS; + int serversUp = 0; + int nvmInited = 0; + int pairsInited = 0; + int barriersInited = 0; + + (void)ctx_arg; + + if (KU_NUM_ALGOS == 0) { + return WH_TEST_SKIPPED; + } + + memset(ctx, 0, sizeof(*ctx)); + + WH_TEST_PRINT(" Concurrent keygen unique-id: %d clients, %d algo(s)\n", + KU_NUM_CLIENTS, KU_NUM_ALGOS); + + rc = wolfCrypt_Init(); + if (rc != 0) { + WH_ERROR_PRINT("wolfCrypt_Init failed: %d\n", rc); + return rc; + } + + rc = kuInitSharedNvm(ctx); + if (rc != WH_ERROR_OK) { + result = rc; + goto out; + } + nvmInited = 1; + + for (i = 0; i < KU_NUM_CLIENTS; i++) { + rc = kuInitPair(ctx, i); + if (rc != WH_ERROR_OK) { + result = rc; + goto out; + } + pairsInited = i + 1; + } + + /* Round barriers synchronize the client threads only. */ + if (pthread_barrier_init(&ctx->roundStart, NULL, KU_NUM_CLIENTS) != 0 || + pthread_barrier_init(&ctx->roundMid, NULL, KU_NUM_CLIENTS) != 0 || + pthread_barrier_init(&ctx->roundEnd, NULL, KU_NUM_CLIENTS) != 0) { + WH_ERROR_PRINT("barrier init failed\n"); + result = WH_ERROR_ABORTED; + goto out; + } + barriersInited = 1; + + /* Bring up the server threads and wait until all are connected. */ + for (i = 0; i < KU_NUM_CLIENTS; i++) { + rc = pthread_create(&ctx->pairs[i].serverThread, NULL, kuServerThread, + &ctx->pairs[i]); + if (rc != 0) { + WH_ERROR_PRINT("server thread %d create failed: %d\n", i, rc); + KU_ATOMIC_STORE(&ctx->stopFlag, 1); + result = WH_ERROR_ABORTED; + goto join_servers; + } + serversUp = i + 1; + } + while (KU_ATOMIC_LOAD(&ctx->serversReady) < serversUp) { + sched_yield(); + } + + if (KU_ATOMIC_LOAD(&ctx->serverError)) { + WH_ERROR_PRINT("a server failed to start\n"); + result = WH_ERROR_ABORTED; + goto stop_servers; + } + + /* Run each algorithm: spawn client threads, run all rounds, join. */ + for (a = 0; a < KU_NUM_ALGOS; a++) { + /* Snapshot so the per-algorithm line reports deltas; the counters + * stay cumulative for the final pass/fail check. */ + int okBefore = KU_ATOMIC_LOAD(&ctx->successes); + int collisionsBefore = KU_ATOMIC_LOAD(&ctx->collisions); + + ctx->genFn = kuAlgos[a].gen; + ctx->rounds = kuAlgos[a].rounds; + ctx->algoName = kuAlgos[a].name; + memset((void*)ctx->roundIds, 0, sizeof(ctx->roundIds)); + memset((void*)ctx->roundRc, 0, sizeof(ctx->roundRc)); + + for (i = 0; i < KU_NUM_CLIENTS; i++) { + rc = pthread_create(&ctx->pairs[i].clientThread, NULL, + kuClientThread, &ctx->pairs[i]); + if (rc != 0) { + WH_ERROR_PRINT("client thread %d create failed: %d\n", i, rc); + /* A short barrier party would hang the started clients; join + * them first, then abort. Creation failure is not expected. */ + result = WH_ERROR_ABORTED; + for (--i; i >= 0; i--) { + pthread_join(ctx->pairs[i].clientThread, NULL); + } + goto stop_servers; + } + } + for (i = 0; i < KU_NUM_CLIENTS; i++) { + pthread_join(ctx->pairs[i].clientThread, NULL); + } + + WH_TEST_PRINT( + " %-10s: %d rounds x %d clients, %d ok, %d collision(s)\n", + kuAlgos[a].name, kuAlgos[a].rounds, KU_NUM_CLIENTS, + KU_ATOMIC_LOAD(&ctx->successes) - okBefore, + KU_ATOMIC_LOAD(&ctx->collisions) - collisionsBefore); + } + +stop_servers: + KU_ATOMIC_STORE(&ctx->stopFlag, 1); + +join_servers: + for (i = 0; i < serversUp; i++) { + pthread_join(ctx->pairs[i].serverThread, NULL); + } + + if (result == WH_TEST_SUCCESS) { + if (KU_ATOMIC_LOAD(&ctx->collisions) != 0) { + WH_ERROR_PRINT("FAILED: %d unique-id collision(s) detected\n", + KU_ATOMIC_LOAD(&ctx->collisions)); + result = WH_ERROR_ABORTED; + } + else if (KU_ATOMIC_LOAD(&ctx->hardErrors) != 0) { + WH_ERROR_PRINT("FAILED: %d keygen error(s) during the run\n", + KU_ATOMIC_LOAD(&ctx->hardErrors)); + result = WH_ERROR_ABORTED; + } + } + +out: + if (barriersInited) { + pthread_barrier_destroy(&ctx->roundStart); + pthread_barrier_destroy(&ctx->roundMid); + pthread_barrier_destroy(&ctx->roundEnd); + } + for (i = 0; i < pairsInited; i++) { + kuCleanupPair(&ctx->pairs[i]); + } + if (nvmInited) { + pthread_mutexattr_destroy(&ctx->mutexAttr); + wh_Nvm_Cleanup(&ctx->nvm); + } + wolfCrypt_Cleanup(); + + return result; +} + +#endif /* WH_KU_ENABLED */ diff --git a/test-refactor/posix/wh_test_keygen_unique_id.h b/test-refactor/posix/wh_test_keygen_unique_id.h new file mode 100644 index 000000000..82f2772b8 --- /dev/null +++ b/test-refactor/posix/wh_test_keygen_unique_id.h @@ -0,0 +1,34 @@ +/* + * 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/posix/wh_test_keygen_unique_id.h + * + * Concurrent keygen unique-id test. See the .c file for details. + */ + +#ifndef WH_TEST_KEYGEN_UNIQUE_ID_H_ +#define WH_TEST_KEYGEN_UNIQUE_ID_H_ + +/* Self-contained: spins up its own shared NVM and N client/server pairs. + * Matches the whTestGroup_RunOne() entry-point contract; ctx is unused. + * Returns WH_TEST_SUCCESS, WH_TEST_SKIPPED when the required build features + * are absent, or a negative error code on failure. */ +int whTest_KeygenUniqueIdConcurrent(void* ctx); + +#endif /* WH_TEST_KEYGEN_UNIQUE_ID_H_ */ diff --git a/test-refactor/posix/wh_test_keyread_race.c b/test-refactor/posix/wh_test_keyread_race.c new file mode 100644 index 000000000..aa1c15f4b --- /dev/null +++ b/test-refactor/posix/wh_test_keyread_race.c @@ -0,0 +1,651 @@ +/* + * 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/posix/wh_test_keyread_race.c + * + * Concurrent cached-key read test for the crypto request handlers in + * src/wh_server_crypto.c. Companion to wh_test_keygen_unique_id.c, which + * covers the write side. + * + * KR_NUM_CLIENTS client/server pairs share a single locked NVM, pre-seeded + * with KR_NUM_KEYS committed global AES keys -- more keys than the shared + * cache has slots, so most reads miss and reload from NVM. Each key is a + * distinct constant byte and therefore encrypts a fixed plaintext to a + * distinct, pre-computed ciphertext. Clients run barrier-aligned rounds, + * each asking its server to AES-ECB encrypt that plaintext under a rotating + * key id and comparing against the expected ciphertext; a mismatch means + * the handler read the wrong key material out of the cache. + * + * Requires WOLFHSM_CFG_THREADSAFE and WOLFHSM_CFG_GLOBAL_KEYS: global keys + * (USER == 0) live in the one shared server->nvm->globalCache, so the reads + * contend on the same slots. Under TSAN (TSAN=1) any unserialized access to + * that cache is additionally reported as a data race. + */ + +#include "wolfhsm/wh_settings.h" + +/* pthread_barrier_t is unavailable on macOS. AES-ECB with a server-cached + * key is the operation under test, so gate on it too. */ +#if defined(WOLFHSM_CFG_THREADSAFE) && defined(WOLFHSM_CFG_TEST_POSIX) && \ + defined(WOLFHSM_CFG_GLOBAL_KEYS) && defined(WOLFHSM_CFG_ENABLE_CLIENT) && \ + defined(WOLFHSM_CFG_ENABLE_SERVER) && !defined(WOLFHSM_CFG_NO_CRYPTO) && \ + !defined(NO_AES) && defined(HAVE_AES_ECB) && !defined(__APPLE__) +#define WH_KR_ENABLED +#endif + +#include "wh_test_common.h" +#include "wh_test_list.h" /* WH_TEST_SKIPPED */ +#include "wh_test_keyread_race.h" + +#ifndef WH_KR_ENABLED + +int whTest_KeyReadRace(void* ctx) +{ + (void)ctx; + return WH_TEST_SKIPPED; +} + +#else /* WH_KR_ENABLED */ + +#include +#include +#include +#include + +#include "wolfhsm/wh_error.h" +#include "wolfhsm/wh_comm.h" +#include "wolfhsm/wh_transport_mem.h" +#include "wolfhsm/wh_client.h" +#include "wolfhsm/wh_client_crypto.h" +#include "wolfhsm/wh_server.h" +#include "wolfhsm/wh_nvm.h" +#include "wolfhsm/wh_nvm_flash.h" +#include "wolfhsm/wh_flash_ramsim.h" +#include "wolfhsm/wh_lock.h" +#include "wolfhsm/wh_keyid.h" +#include "wolfhsm/wh_common.h" + +#include "port/posix/posix_lock.h" + +#include "wolfssl/wolfcrypt/settings.h" +#include "wolfssl/wolfcrypt/types.h" +#include "wolfssl/wolfcrypt/error-crypt.h" +#include "wolfssl/wolfcrypt/aes.h" + +/* TSAN transport shims: the mem transport's notify counter establishes + * happens-before between client and server, but TSAN can't see it. + * Annotating the send/recv pairs keeps the analysis on keystore/NVM + * locking instead of transport false positives. */ +#ifdef WOLFHSM_CFG_TEST_STRESS_TSAN + +#ifndef __has_feature +#define __has_feature(x) 0 +#endif +#if !defined(__SANITIZE_THREAD__) && !__has_feature(thread_sanitizer) +#error ThreadSanitizer not enabled for this build +#endif + +#include + +static int kr_SendRequest(void* c, uint16_t len, const void* data) +{ + whTransportMemContext* ctx = (whTransportMemContext*)c; + __tsan_release((void*)ctx->req); + return wh_TransportMem_SendRequest(c, len, data); +} +static int kr_RecvResponse(void* c, uint16_t* out_len, void* data) +{ + whTransportMemContext* ctx = (whTransportMemContext*)c; + int rc = wh_TransportMem_RecvResponse(c, out_len, data); + if (rc == WH_ERROR_OK) { + __tsan_acquire((void*)ctx->resp); + } + return rc; +} +static int kr_RecvRequest(void* c, uint16_t* out_len, void* data) +{ + whTransportMemContext* ctx = (whTransportMemContext*)c; + int rc = wh_TransportMem_RecvRequest(c, out_len, data); + if (rc == WH_ERROR_OK) { + __tsan_acquire((void*)ctx->req); + } + return rc; +} +static int kr_SendResponse(void* c, uint16_t len, const void* data) +{ + whTransportMemContext* ctx = (whTransportMemContext*)c; + __tsan_release((void*)ctx->resp); + return wh_TransportMem_SendResponse(c, len, data); +} + +static const whTransportClientCb clientTransportCb = { + .Init = wh_TransportMem_InitClear, + .Send = kr_SendRequest, + .Recv = kr_RecvResponse, + .Cleanup = wh_TransportMem_Cleanup, +}; +static const whTransportServerCb serverTransportCb = { + .Init = wh_TransportMem_Init, + .Recv = kr_RecvRequest, + .Send = kr_SendResponse, + .Cleanup = wh_TransportMem_Cleanup, +}; + +#else /* !WOLFHSM_CFG_TEST_STRESS_TSAN */ + +static const whTransportClientCb clientTransportCb = WH_TRANSPORT_MEM_CLIENT_CB; +static const whTransportServerCb serverTransportCb = WH_TRANSPORT_MEM_SERVER_CB; + +#endif /* WOLFHSM_CFG_TEST_STRESS_TSAN */ + +#define KR_ATOMIC_LOAD(ptr) __atomic_load_n((ptr), __ATOMIC_ACQUIRE) +#define KR_ATOMIC_STORE(ptr, v) __atomic_store_n((ptr), (v), __ATOMIC_RELEASE) +#define KR_ATOMIC_ADD(ptr, v) __atomic_add_fetch((ptr), (v), __ATOMIC_ACQ_REL) + +/* Four concurrent client/server pairs sharing one NVM. */ +#define KR_NUM_CLIENTS 4 + +/* More keys than the shared small-key cache has slots (16 by default), so + * most reads miss and race to claim a slot and reload from NVM, but within + * the NVM directory capacity (WOLFHSM_CFG_NVM_OBJECT_COUNT, default 32). */ +#define KR_NUM_KEYS 24 + +/* AES-128: a key size every AES build supports. Each key is a distinct + * constant byte, so one wrong key byte changes the whole ciphertext block + * by the cipher's avalanche. */ +#define KR_KEYSZ AES_128_KEY_SIZE + +/* Rounds per client, each barrier-aligned so the server-side reads overlap. */ +#define KR_ROUNDS 400 + +#define KR_FLASH_RAM_SIZE (1024 * 1024) +#define KR_FLASH_SECTOR_SIZE (128 * 1024) +#define KR_FLASH_PAGE_SIZE 8 + +/* One 16-byte block in, key referenced by id. */ +#define KR_BUFFER_SIZE 4096 + +/* Distinct nonzero constant per key. Keys 0..KR_NUM_KEYS-1 -> 0x01..0x18. */ +#define KR_KEYBYTE(i) ((uint8_t)(0x01 + (i))) +/* Server-internal id of the i-th seeded global key (ids start at 1). */ +#define KR_SERVER_KEYID(i) \ + WH_MAKE_KEYID(WH_KEYTYPE_CRYPTO, WH_KEYUSER_GLOBAL, (whNvmId)((i) + 1)) +/* Client-facing id the client passes to reference that same global key. */ +#define KR_CLIENT_KEYID(i) WH_CLIENT_KEYID_MAKE_GLOBAL((whKeyId)((i) + 1)) + +struct KrContext; + +typedef struct { + uint8_t reqBuf[KR_BUFFER_SIZE]; + uint8_t respBuf[KR_BUFFER_SIZE]; + whTransportMemConfig tmConfig; + + whTransportMemClientContext clientTransportCtx; + whCommClientConfig clientCommConfig; + whClientContext client; + whClientConfig clientConfig; + + whTransportMemServerContext serverTransportCtx; + whCommServerConfig serverCommConfig; + whServerContext server; + whServerConfig serverConfig; + whServerCryptoContext cryptoCtx; + + pthread_t clientThread; + pthread_t serverThread; + int idx; + + struct KrContext* shared; +} KrPair; + +typedef struct KrContext { + /* Shared, locked NVM */ + uint8_t flashMemory[KR_FLASH_RAM_SIZE]; + whFlashRamsimCtx flashCtx; + whFlashRamsimCfg flashCfg; + whNvmFlashContext nvmFlashCtx; + whNvmFlashConfig nvmFlashCfg; + whNvmContext nvm; + whNvmConfig nvmCfg; + + posixLockContext nvmLockCtx; + pthread_mutexattr_t mutexAttr; + posixLockConfig posixLockCfg; + whLockConfig lockCfg; + + KrPair pairs[KR_NUM_CLIENTS]; + + /* Fixed plaintext block and the per-key ciphertext oracle, computed once + * at setup with software AES. */ + uint8_t plaintext[AES_BLOCK_SIZE]; + uint8_t expected[KR_NUM_KEYS][AES_BLOCK_SIZE]; + + /* A readiness counter rather than a barrier, so a server that fails to + * spawn does not hang the others. */ + volatile int serversReady; + volatile int serverError; + volatile int stopFlag; + + /* Per-round client synchronization */ + pthread_barrier_t roundStart; + pthread_barrier_t roundEnd; + + /* Outcome counters, per client to avoid cross-thread contention. */ + volatile int mismatches[KR_NUM_CLIENTS]; + volatile int hardErrors[KR_NUM_CLIENTS]; + volatile int successes[KR_NUM_CLIENTS]; +} KrContext; + +/* The context is large (1 MB flash buffer); keep it off the thread stack. */ +static KrContext g_kr; + +static int krInitSharedNvm(KrContext* ctx) +{ + static whFlashCb flashCb = WH_FLASH_RAMSIM_CB; + static whNvmCb nvmCb = WH_NVM_FLASH_CB; + static whLockCb lockCb = POSIX_LOCK_CB; + int rc; + + memset(ctx->flashMemory, 0xFF, sizeof(ctx->flashMemory)); + + ctx->flashCfg.size = KR_FLASH_RAM_SIZE; + ctx->flashCfg.sectorSize = KR_FLASH_SECTOR_SIZE; + ctx->flashCfg.pageSize = KR_FLASH_PAGE_SIZE; + ctx->flashCfg.erasedByte = 0xFF; + ctx->flashCfg.memory = ctx->flashMemory; + + ctx->nvmFlashCfg.cb = &flashCb; + ctx->nvmFlashCfg.context = &ctx->flashCtx; + ctx->nvmFlashCfg.config = &ctx->flashCfg; + + rc = wh_NvmFlash_Init(&ctx->nvmFlashCtx, &ctx->nvmFlashCfg); + if (rc != WH_ERROR_OK) { + WH_ERROR_PRINT("NVM flash init failed: %d\n", rc); + return rc; + } + + /* Error-checking mutex catches lock misuse (double-unlock, etc.). */ + memset(&ctx->nvmLockCtx, 0, sizeof(ctx->nvmLockCtx)); + pthread_mutexattr_init(&ctx->mutexAttr); + pthread_mutexattr_settype(&ctx->mutexAttr, PTHREAD_MUTEX_ERRORCHECK); + ctx->posixLockCfg.attr = &ctx->mutexAttr; + ctx->lockCfg.cb = &lockCb; + ctx->lockCfg.context = &ctx->nvmLockCtx; + ctx->lockCfg.config = &ctx->posixLockCfg; + + ctx->nvmCfg.cb = &nvmCb; + ctx->nvmCfg.context = &ctx->nvmFlashCtx; + ctx->nvmCfg.config = &ctx->nvmFlashCfg; + ctx->nvmCfg.lockConfig = &ctx->lockCfg; + + rc = wh_Nvm_Init(&ctx->nvm, &ctx->nvmCfg); + if (rc != WH_ERROR_OK) { + WH_ERROR_PRINT("NVM init failed: %d\n", rc); + return rc; + } + return WH_ERROR_OK; +} + +/* Commit KR_NUM_KEYS distinct global AES keys straight to NVM (not the + * cache), and compute the ciphertext each produces for the fixed plaintext. */ +static int krSeedKeys(KrContext* ctx) +{ + uint8_t data[KR_KEYSZ]; + whNvmMetadata meta; + Aes aes[1]; + int i; + int rc; + + for (i = 0; i < AES_BLOCK_SIZE; i++) { + ctx->plaintext[i] = (uint8_t)(0xA0 + i); + } + + for (i = 0; i < KR_NUM_KEYS; i++) { + memset(data, KR_KEYBYTE(i), sizeof(data)); + + memset(&meta, 0, sizeof(meta)); + meta.id = KR_SERVER_KEYID(i); + meta.access = WH_NVM_ACCESS_ANY; + meta.flags = WH_NVM_FLAGS_USAGE_ANY; + meta.len = KR_KEYSZ; + + rc = wh_Nvm_AddObject(&ctx->nvm, &meta, sizeof(data), data); + if (rc != WH_ERROR_OK) { + WH_ERROR_PRINT("seed key %d add failed: %d\n", i, rc); + return rc; + } + + /* Independent software-AES oracle for this key. */ + rc = wc_AesInit(aes, NULL, INVALID_DEVID); + if (rc == 0) { + rc = wc_AesSetKey(aes, data, sizeof(data), NULL, AES_ENCRYPTION); + } + if (rc == 0) { + rc = wc_AesEcbEncrypt(aes, ctx->expected[i], ctx->plaintext, + AES_BLOCK_SIZE); + } + (void)wc_AesFree(aes); + if (rc != 0) { + WH_ERROR_PRINT("oracle encrypt for key %d failed: %d\n", i, rc); + return WH_ERROR_ABORTED; + } + } + return WH_ERROR_OK; +} + +static int krInitPair(KrContext* ctx, int idx) +{ + KrPair* pair = &ctx->pairs[idx]; + int rc; + + pair->idx = idx; + pair->shared = ctx; + + pair->tmConfig.req = (whTransportMemCsr*)pair->reqBuf; + pair->tmConfig.req_size = sizeof(pair->reqBuf); + pair->tmConfig.resp = (whTransportMemCsr*)pair->respBuf; + pair->tmConfig.resp_size = sizeof(pair->respBuf); + + memset(&pair->clientTransportCtx, 0, sizeof(pair->clientTransportCtx)); + pair->clientCommConfig.transport_cb = &clientTransportCb; + pair->clientCommConfig.transport_context = &pair->clientTransportCtx; + pair->clientCommConfig.transport_config = &pair->tmConfig; + /* client_id must be in [1, WH_CLIENT_ID_MAX]; distinct per pair. */ + pair->clientCommConfig.client_id = (uint8_t)(1 + idx); + pair->clientConfig.comm = &pair->clientCommConfig; + + memset(&pair->serverTransportCtx, 0, sizeof(pair->serverTransportCtx)); + pair->serverCommConfig.transport_cb = &serverTransportCb; + pair->serverCommConfig.transport_context = &pair->serverTransportCtx; + pair->serverCommConfig.transport_config = &pair->tmConfig; + pair->serverCommConfig.server_id = (uint16_t)(200 + idx); + + rc = wc_InitRng_ex(pair->cryptoCtx.rng, NULL, INVALID_DEVID); + if (rc != 0) { + WH_ERROR_PRINT("RNG init failed for pair %d: %d\n", idx, rc); + return rc; + } + + /* All servers share the one locked NVM. */ + pair->serverConfig.comm_config = &pair->serverCommConfig; + pair->serverConfig.nvm = &ctx->nvm; + pair->serverConfig.crypto = &pair->cryptoCtx; + pair->serverConfig.devId = INVALID_DEVID; + + /* Init the client here (main thread) to avoid concurrent wolfCrypt + * init/register from the worker threads. */ + rc = wh_Client_Init(&pair->client, &pair->clientConfig); + if (rc != WH_ERROR_OK) { + WH_ERROR_PRINT("Client %d init failed: %d\n", idx, rc); + wc_FreeRng(pair->cryptoCtx.rng); + return rc; + } + return WH_ERROR_OK; +} + +static void krCleanupPair(KrPair* pair) +{ + wh_Client_Cleanup(&pair->client); + wc_FreeRng(pair->cryptoCtx.rng); +} + +/* Serve requests for one pair until stopFlag is set. */ +static void* krServerThread(void* arg) +{ + KrPair* pair = (KrPair*)arg; + KrContext* ctx = pair->shared; + int rc; + + rc = wh_Server_Init(&pair->server, &pair->serverConfig); + if (rc != WH_ERROR_OK) { + WH_ERROR_PRINT("Server %d init failed: %d\n", pair->idx, rc); + KR_ATOMIC_STORE(&ctx->serverError, 1); + KR_ATOMIC_ADD(&ctx->serversReady, 1); + return NULL; + } + rc = wh_Server_SetConnected(&pair->server, WH_COMM_CONNECTED); + if (rc != WH_ERROR_OK) { + WH_ERROR_PRINT("Server %d SetConnected failed: %d\n", pair->idx, rc); + KR_ATOMIC_STORE(&ctx->serverError, 1); + wh_Server_Cleanup(&pair->server); + KR_ATOMIC_ADD(&ctx->serversReady, 1); + return NULL; + } + + /* Announce readiness; main waits for all servers before clients run. */ + KR_ATOMIC_ADD(&ctx->serversReady, 1); + + while (!KR_ATOMIC_LOAD(&ctx->stopFlag)) { + rc = wh_Server_HandleRequestMessage(&pair->server); + if (rc == WH_ERROR_NOTREADY) { + sched_yield(); + } + /* Other per-request errors surface to the client as response codes. */ + } + + wh_Server_Cleanup(&pair->server); + return NULL; +} + +/* Run KR_ROUNDS barrier-aligned AES-ECB encrypts against a rotating shared + * global key, checking each ciphertext against the oracle. */ +static void* krClientThread(void* arg) +{ + KrPair* pair = (KrPair*)arg; + KrContext* ctx = pair->shared; + int round; + + for (round = 0; round < KR_ROUNDS; round++) { + /* Rotate the key per client so all keys cycle through the shared + * cache and evict one another. */ + int keyIdx = (round * KR_NUM_CLIENTS + pair->idx) % KR_NUM_KEYS; + whKeyId cid = KR_CLIENT_KEYID(keyIdx); + uint8_t out[AES_BLOCK_SIZE]; + Aes aes[1]; + int rc; + + memset(out, 0, sizeof(out)); + + /* Line all clients up so their server-side reads overlap. */ + pthread_barrier_wait(&ctx->roundStart); + + /* INVALID_DEVID: the request goes through the explicit client API + * below, not the wolfCrypt cryptocb, so the Aes struct only has to + * carry the key id. */ + rc = wc_AesInit(aes, NULL, INVALID_DEVID); + if (rc == 0) { + rc = wh_Client_AesSetKeyId(aes, cid); + } + if (rc == 0) { + rc = wh_Client_AesEcb(&pair->client, aes, 1, ctx->plaintext, + AES_BLOCK_SIZE, out); + } + (void)wc_AesFree(aes); + + if (rc != 0) { + /* Not a mismatch, but it invalidates the round. */ + KR_ATOMIC_ADD(&ctx->hardErrors[pair->idx], 1); + WH_ERROR_PRINT("round %d: client %d ECB key 0x%04X failed: %d\n", + round, pair->idx, (unsigned)cid, rc); + } + else if (memcmp(out, ctx->expected[keyIdx], AES_BLOCK_SIZE) != 0) { + /* Ciphertext for some other key: the server read the wrong key + * material out of the shared cache. */ + KR_ATOMIC_ADD(&ctx->mismatches[pair->idx], 1); + WH_ERROR_PRINT( + "round %d: client %d key 0x%04X ciphertext mismatch\n", round, + pair->idx, (unsigned)cid); + } + else { + KR_ATOMIC_ADD(&ctx->successes[pair->idx], 1); + } + + /* Hold everyone until every read in this round is done. */ + pthread_barrier_wait(&ctx->roundEnd); + } + return NULL; +} + +int whTest_KeyReadRace(void* ctx_arg) +{ + KrContext* ctx = &g_kr; + int i; + int rc; + int result = WH_TEST_SUCCESS; + int serversUp = 0; + int nvmInited = 0; + int pairsInited = 0; + int barriersInited = 0; + int clientsStarted = 0; + int totalMiss = 0; + int totalHard = 0; + int totalSuccess = 0; + + (void)ctx_arg; + + memset(ctx, 0, sizeof(*ctx)); + + WH_TEST_PRINT(" Concurrent key read: %d clients, %d keys, %d rounds\n", + KR_NUM_CLIENTS, KR_NUM_KEYS, KR_ROUNDS); + + rc = wolfCrypt_Init(); + if (rc != 0) { + WH_ERROR_PRINT("wolfCrypt_Init failed: %d\n", rc); + return rc; + } + + rc = krInitSharedNvm(ctx); + if (rc != WH_ERROR_OK) { + result = rc; + goto out; + } + nvmInited = 1; + + rc = krSeedKeys(ctx); + if (rc != WH_ERROR_OK) { + result = rc; + goto out; + } + + for (i = 0; i < KR_NUM_CLIENTS; i++) { + rc = krInitPair(ctx, i); + if (rc != WH_ERROR_OK) { + result = rc; + goto out; + } + pairsInited = i + 1; + } + + /* Round barriers synchronize the client threads only. */ + if (pthread_barrier_init(&ctx->roundStart, NULL, KR_NUM_CLIENTS) != 0 || + pthread_barrier_init(&ctx->roundEnd, NULL, KR_NUM_CLIENTS) != 0) { + WH_ERROR_PRINT("barrier init failed\n"); + result = WH_ERROR_ABORTED; + goto out; + } + barriersInited = 1; + + /* Bring up the server threads and wait until all are connected. */ + for (i = 0; i < KR_NUM_CLIENTS; i++) { + rc = pthread_create(&ctx->pairs[i].serverThread, NULL, krServerThread, + &ctx->pairs[i]); + if (rc != 0) { + WH_ERROR_PRINT("server thread %d create failed: %d\n", i, rc); + KR_ATOMIC_STORE(&ctx->stopFlag, 1); + result = WH_ERROR_ABORTED; + goto join_servers; + } + serversUp = i + 1; + } + while (KR_ATOMIC_LOAD(&ctx->serversReady) < serversUp) { + sched_yield(); + } + + if (KR_ATOMIC_LOAD(&ctx->serverError)) { + WH_ERROR_PRINT("a server failed to start\n"); + result = WH_ERROR_ABORTED; + goto stop_servers; + } + + /* Spawn the client threads, run all rounds, join. */ + for (i = 0; i < KR_NUM_CLIENTS; i++) { + rc = pthread_create(&ctx->pairs[i].clientThread, NULL, krClientThread, + &ctx->pairs[i]); + if (rc != 0) { + WH_ERROR_PRINT("client thread %d create failed: %d\n", i, rc); + /* A short barrier party would hang the started clients; join + * them first, then abort. Creation failure is not expected. */ + result = WH_ERROR_ABORTED; + for (--i; i >= 0; i--) { + pthread_join(ctx->pairs[i].clientThread, NULL); + } + goto stop_servers; + } + clientsStarted = i + 1; + } + for (i = 0; i < clientsStarted; i++) { + pthread_join(ctx->pairs[i].clientThread, NULL); + } + +stop_servers: + KR_ATOMIC_STORE(&ctx->stopFlag, 1); + +join_servers: + for (i = 0; i < serversUp; i++) { + pthread_join(ctx->pairs[i].serverThread, NULL); + } + + if (result == WH_TEST_SUCCESS) { + for (i = 0; i < KR_NUM_CLIENTS; i++) { + totalMiss += KR_ATOMIC_LOAD(&ctx->mismatches[i]); + totalHard += KR_ATOMIC_LOAD(&ctx->hardErrors[i]); + totalSuccess += KR_ATOMIC_LOAD(&ctx->successes[i]); + } + WH_TEST_PRINT(" reads=%d, mismatches=%d, hardErrors=%d\n", + totalSuccess, totalMiss, totalHard); + if (totalMiss != 0) { + WH_ERROR_PRINT("FAILED: %d ciphertext mismatch(es) detected\n", + totalMiss); + result = WH_ERROR_ABORTED; + } + else if (totalHard != 0) { + WH_ERROR_PRINT("FAILED: %d read error(s) during the run\n", + totalHard); + result = WH_ERROR_ABORTED; + } + } + +out: + if (barriersInited) { + pthread_barrier_destroy(&ctx->roundStart); + pthread_barrier_destroy(&ctx->roundEnd); + } + for (i = 0; i < pairsInited; i++) { + krCleanupPair(&ctx->pairs[i]); + } + if (nvmInited) { + pthread_mutexattr_destroy(&ctx->mutexAttr); + wh_Nvm_Cleanup(&ctx->nvm); + } + wolfCrypt_Cleanup(); + + return result; +} + +#endif /* WH_KR_ENABLED */ diff --git a/test-refactor/posix/wh_test_keyread_race.h b/test-refactor/posix/wh_test_keyread_race.h new file mode 100644 index 000000000..9dc7cd649 --- /dev/null +++ b/test-refactor/posix/wh_test_keyread_race.h @@ -0,0 +1,34 @@ +/* + * 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/posix/wh_test_keyread_race.h + * + * Concurrent cached-key read test. See the .c file for details. + */ + +#ifndef WH_TEST_KEYREAD_RACE_H_ +#define WH_TEST_KEYREAD_RACE_H_ + +/* Self-contained: spins up its own shared NVM and N client/server pairs. + * Matches the whTestGroup_RunOne() entry-point contract; ctx is unused. + * Returns WH_TEST_SUCCESS, WH_TEST_SKIPPED when the required build features + * are absent, or a negative error code on failure. */ +int whTest_KeyReadRace(void* ctx); + +#endif /* WH_TEST_KEYREAD_RACE_H_ */ diff --git a/test-refactor/posix/wh_test_posix_main.c b/test-refactor/posix/wh_test_posix_main.c index 756ed6041..7cca15b97 100644 --- a/test-refactor/posix/wh_test_posix_main.c +++ b/test-refactor/posix/wh_test_posix_main.c @@ -47,6 +47,8 @@ #include "wh_test_posix_client.h" #include "wh_test_posix_server.h" +#include "wh_test_keygen_unique_id.h" +#include "wh_test_keyread_race.h" /* POSIX-only thread-safety stress test. Called directly rather * than through the suite runner: the legacy test owns its own @@ -284,6 +286,20 @@ int main(void) if (rc != 0 && rc != WH_TEST_SKIPPED && miscRc == 0) { miscRc = rc; } + /* Concurrent keygen and cached-key read tests. Both are + * self-contained (own shared NVM + client/server pairs), so they run + * in this pre-server slot rather than against the live + * _server/_client, and both skip themselves unless THREADSAFE + + * GLOBAL_KEYS + crypto are compiled in. */ + rc = whTestGroup_RunOne("whTest_KeygenUniqueIdConcurrent", + whTest_KeygenUniqueIdConcurrent, NULL); + if (rc != 0 && rc != WH_TEST_SKIPPED && miscRc == 0) { + miscRc = rc; + } + rc = whTestGroup_RunOne("whTest_KeyReadRace", whTest_KeyReadRace, NULL); + if (rc != 0 && rc != WH_TEST_SKIPPED && miscRc == 0) { + miscRc = rc; + } } rc = pthread_create(&sthread, NULL, _serverThread, NULL); diff --git a/test/wh_test_crypto.c b/test/wh_test_crypto.c index db18c9341..65b3eb4b3 100644 --- a/test/wh_test_crypto.c +++ b/test/wh_test_crypto.c @@ -12184,8 +12184,8 @@ int whTestCrypto_MlDsaVerifyOnlyDma(whClientContext* ctx, int devId, } } /* Import the key into wolfHSM via the wolfCrypt structure. This is the - * DMA-only verify test, so always import via the DMA path. The key must - * carry the verify usage flag, which the DMA verify handler enforces. */ + * DMA-only verify test, so always import via the DMA path. The verify + * usage flag is required: the DMA verify handler enforces it. */ if (ret == 0) { ret = wh_Client_MlDsaImportKeyDma(ctx, key, &keyId, WH_NVM_FLAGS_USAGE_VERIFY, 0, NULL); diff --git a/wolfhsm/wh_server_crypto.h b/wolfhsm/wh_server_crypto.h index 739172d37..5bf90b748 100644 --- a/wolfhsm/wh_server_crypto.h +++ b/wolfhsm/wh_server_crypto.h @@ -64,6 +64,12 @@ int wh_Server_CacheImportRsaKey(whServerContext* ctx, RsaKey* key, /* Restore a RsaKey from a server key cache */ int wh_Server_CacheExportRsaKey(whServerContext* ctx, whKeyId keyId, RsaKey* key); + +/* As wh_Server_CacheExportRsaKey, but checks the required usage flags against + * the same locked snapshot of the key. Acquires the NVM lock internally; must + * not be called with it held (the lock is non-recursive). */ +int wh_Server_CacheExportRsaKeyEnforce(whServerContext* ctx, whKeyId keyId, + whNvmFlags requiredUsage, RsaKey* key); #endif /* !NO_RSA */ #ifdef HAVE_ECC @@ -72,6 +78,12 @@ int wh_Server_EccKeyCacheImport(whServerContext* ctx, ecc_key* key, int wh_Server_EccKeyCacheExport(whServerContext* ctx, whKeyId keyId, ecc_key* key); + +/* As wh_Server_EccKeyCacheExport, but checks the required usage flags against + * the same locked snapshot of the key. Acquires the NVM lock internally; must + * not be called with it held (the lock is non-recursive). */ +int wh_Server_EccKeyCacheExportEnforce(whServerContext* ctx, whKeyId keyId, + whNvmFlags requiredUsage, ecc_key* key); #endif #ifdef HAVE_ED25519 @@ -81,6 +93,13 @@ int wh_Server_CacheImportEd25519Key(whServerContext* ctx, ed25519_key* key, int wh_Server_CacheExportEd25519Key(whServerContext* ctx, whKeyId keyId, ed25519_key* key); + +/* As wh_Server_CacheExportEd25519Key, but checks the required usage flags + * against the same locked snapshot of the key. Acquires the NVM lock + * internally; must not be called with it held (the lock is non-recursive). */ +int wh_Server_CacheExportEd25519KeyEnforce(whServerContext* ctx, whKeyId keyId, + whNvmFlags requiredUsage, + ed25519_key* key); #endif /* HAVE_ED25519 */ #ifdef HAVE_CURVE25519 @@ -92,6 +111,14 @@ int wh_Server_CacheImportCurve25519Key(whServerContext* server, /* Restore a curve25519_key from a server key cache */ int wh_Server_CacheExportCurve25519Key(whServerContext* server, whKeyId keyId, curve25519_key* key); + +/* As wh_Server_CacheExportCurve25519Key, but checks the required usage flags + * against the same locked snapshot of the key. Acquires the NVM lock + * internally; must not be called with it held (the lock is non-recursive). */ +int wh_Server_CacheExportCurve25519KeyEnforce(whServerContext* server, + whKeyId keyId, + whNvmFlags requiredUsage, + curve25519_key* key); #endif /* HAVE_CURVE25519 */ #ifdef WOLFSSL_HAVE_MLDSA @@ -102,6 +129,13 @@ int wh_Server_MlDsaKeyCacheImport(whServerContext* ctx, wc_MlDsaKey* key, /* Restore a wc_MlDsaKey from a server key cache */ int wh_Server_MlDsaKeyCacheExport(whServerContext* ctx, whKeyId keyId, wc_MlDsaKey* key); + +/* As wh_Server_MlDsaKeyCacheExport, but checks the required usage flags + * against the same locked snapshot of the key. Acquires the NVM lock + * internally; must not be called with it held (the lock is non-recursive). */ +int wh_Server_MlDsaKeyCacheExportEnforce(whServerContext* ctx, whKeyId keyId, + whNvmFlags requiredUsage, + wc_MlDsaKey* key); #endif /* WOLFSSL_HAVE_MLDSA */ #ifdef WOLFSSL_HAVE_MLKEM @@ -112,6 +146,13 @@ int wh_Server_MlKemKeyCacheImport(whServerContext* ctx, MlKemKey* key, /* Restore a MlKemKey from a server key cache */ int wh_Server_MlKemKeyCacheExport(whServerContext* ctx, whKeyId keyId, MlKemKey* key); + +/* As wh_Server_MlKemKeyCacheExport, but checks the required usage flags + * against the same locked snapshot of the key. Acquires the NVM lock + * internally; must not be called with it held (the lock is non-recursive). */ +int wh_Server_MlKemKeyCacheExportEnforce(whServerContext* ctx, whKeyId keyId, + whNvmFlags requiredUsage, + MlKemKey* key); #endif /* WOLFSSL_HAVE_MLKEM */ /* Store raw key bytes into a server key cache slot with optional metadata. diff --git a/wolfhsm/wh_server_keystore.h b/wolfhsm/wh_server_keystore.h index d34588e7c..69458bec5 100644 --- a/wolfhsm/wh_server_keystore.h +++ b/wolfhsm/wh_server_keystore.h @@ -130,6 +130,40 @@ int wh_Server_KeystoreReadKeyChecked(whServerContext* server, whKeyId keyId, whNvmMetadata* outMeta, uint8_t* out, uint32_t* outSz); +/** + * @brief Atomically read a key and enforce its usage policy + * + * Reads the key (as wh_Server_KeystoreReadKey) and checks the required usage + * flags against the same snapshot of the key, all under the NVM lock, so the + * policy checked can never belong to a different key generation than the key + * material returned. On a usage-policy failure the output buffer is cleared. + * + * Acquires WH_SERVER_NVM_LOCK internally under WOLFHSM_CFG_THREADSAFE. The + * lock is non-recursive: callers that already hold it (e.g. the SHE or cert + * request dispatch) must use wh_Server_KeystoreReadKey plus + * wh_Server_KeystoreEnforceKeyUsage directly instead. + * + * This copies the key out rather than handing back a pointer into the cache + * slot, which is what lets the caller use the key after the lock is dropped. + * The copy is bounded by the caller's buffer, so a key larger than *outSz is + * rejected with WH_ERROR_NOSPACE and nothing is written. + * + * @param[in] server Server context + * @param[in] keyId Key ID to read + * @param[in] requiredUsage Usage flags the key must have (may be + * WH_NVM_FLAGS_NONE for no usage requirement) + * @param[out] outMeta Key metadata (can be NULL) + * @param[out] out Buffer to store key data (can be NULL) + * @param[in,out] outSz Input: size of out buffer; Output: key size + * @return WH_ERROR_OK on success, WH_ERROR_USAGE if the key lacks the + * required usage flags, WH_ERROR_NOSPACE if the key does not fit in + * out, other error codes on read failure + */ +int wh_Server_KeystoreReadKeyEnforce(whServerContext* server, whKeyId keyId, + whNvmFlags requiredUsage, + whNvmMetadata* outMeta, uint8_t* out, + uint32_t* outSz); + /** * @brief Remove a key from cache * @@ -280,11 +314,25 @@ int wh_Server_KeystoreEnforceKeyUsage(const whNvmMetadata* meta, /** * Validates that a key has the required usage policy flags set * + * NOTE: this function may be deprecated soon. The policy check ends when it + * unlocks, so a caller that afterwards exports or uses the key does so in a + * separate lock scope and races an update to the key's usage flags. Prefer a + * primitive that enforces and reads under a single lock: + * wh_Server_KeystoreReadKeyEnforce for raw key bytes, or the typed + * wh_Server_*CacheExport*Enforce wrappers in wh_server_crypto.h. This function + * remains only for callers that genuinely need a standalone policy check and + * never touch the key material. + * * This function enforces key usage policies by checking that the specified * key has all the required usage flags set in its metadata. It retrieves * the key metadata from the cache or NVM storage and performs a bitwise * check against the required flags. * + * Acquires WH_SERVER_NVM_LOCK internally under WOLFHSM_CFG_THREADSAFE. The + * lock is non-recursive: callers that already hold it (e.g. the SHE or cert + * request dispatch) must use wh_Server_KeystoreFreshenKey plus + * wh_Server_KeystoreEnforceKeyUsage directly instead. + * * @param server Pointer to the server context * @param keyId The translated server keyId (after client keyId translation) * @param requiredUsage The required usage policy flags (e.g., diff --git a/wolfhsm/wh_settings.h b/wolfhsm/wh_settings.h index 1139fee9c..354953abf 100644 --- a/wolfhsm/wh_settings.h +++ b/wolfhsm/wh_settings.h @@ -84,6 +84,10 @@ * WOLFHSM_CFG_SERVER_KEYCACHE_BUFSIZE - Size of each key in RAM * Default: 1200 * + * WOLFHSM_CFG_SERVER_KDF_MAX_KEY_SIZE - Largest cached key usable as a KDF + * input (HKDF IKM, CMAC-KDF Z) + * Default: 256 + * * WOLFHSM_CFG_SERVER_CUSTOMCB_COUNT - Number of additional callbacks * Default: 8 * @@ -466,6 +470,16 @@ #endif /* WOLFHSM_CFG_KEYWRAP */ +#if defined(HAVE_HKDF) || defined(HAVE_CMAC_KDF) + +/* Largest cached key the server will accept as a KDF input supplied by key ID + * */ +#ifndef WOLFHSM_CFG_SERVER_KDF_MAX_KEY_SIZE +#define WOLFHSM_CFG_SERVER_KDF_MAX_KEY_SIZE 256 +#endif + +#endif /* HAVE_HKDF || HAVE_CMAC_KDF */ + #if defined(WOLFHSM_CFG_CERTIFICATE_MANAGER_ACERT) #if !defined(WOLFSSL_ACERT) || !defined(WOLFSSL_ASN_TEMPLATE) #error \ From e3a6cb0e68c06b4085df64f9e2638a26a3f445fb Mon Sep 17 00:00:00 2001 From: Marco Oliverio Date: Thu, 9 Jul 2026 19:37:05 +0200 Subject: [PATCH 3/4] test: add USAGE flag in DSA tests --- test-refactor/client-server/wh_test_crypto_mldsa.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test-refactor/client-server/wh_test_crypto_mldsa.c b/test-refactor/client-server/wh_test_crypto_mldsa.c index 4b69738bd..5e027f81e 100644 --- a/test-refactor/client-server/wh_test_crypto_mldsa.c +++ b/test-refactor/client-server/wh_test_crypto_mldsa.c @@ -916,9 +916,11 @@ static int _whTest_CryptoMlDsaVerifyOnlyDma(whClientContext* ctx) } } /* Import the key into wolfHSM via the wolfCrypt structure. This is the - * DMA-only verify test, so always import via the DMA path. */ + * DMA-only verify test, so always import via the DMA path. The key must + * carry the verify usage flag, which the DMA verify handler enforces. */ if (ret == 0) { - ret = wh_Client_MlDsaImportKeyDma(ctx, key, &keyId, 0, 0, NULL); + ret = wh_Client_MlDsaImportKeyDma(ctx, key, &keyId, + WH_NVM_FLAGS_USAGE_VERIFY, 0, NULL); if (ret == WH_ERROR_OK) { evictKey = 1; } From 2bd49683cf833c15823637acb942786e9bf9561b Mon Sep 17 00:00:00 2001 From: Marco Oliverio Date: Thu, 9 Jul 2026 19:53:47 +0200 Subject: [PATCH 4/4] formatting fixes --- src/wh_server_crypto.c | 67 +++++++++++++++++++++--------------------- wolfhsm/wh_settings.h | 3 +- 2 files changed, 35 insertions(+), 35 deletions(-) diff --git a/src/wh_server_crypto.c b/src/wh_server_crypto.c index bbe9e88ce..e39f47624 100644 --- a/src/wh_server_crypto.c +++ b/src/wh_server_crypto.c @@ -325,8 +325,8 @@ int wh_Server_CacheExportRsaKeyEnforce(whServerContext* ctx, whKeyId keyId, } /* Freshen, check usage and deserialize under one hold of the NVM lock so * the policy verdict, the metadata length and the key bytes all come from - * the same snapshot of the shared cache slot. This matter for the global - * shared cache. */ + * the same snapshot of the shared cache slot. This matters for the global + * shared cache. */ ret = WH_SERVER_NVM_LOCK(ctx); if (ret == WH_ERROR_OK) { ret = wh_Server_KeystoreFreshenKey(ctx, keyId, &cacheBuf, &cacheMeta); @@ -401,7 +401,7 @@ static int _HandleRsaKeyGen(whServerContext* ctx, uint16_t magic, int devId, /* Must import the key into the cache and return keyid */ /* Hold the NVM lock so id allocation and cache import are * atomic with respect to other server contexts under - * THREADSAFE. This matter for the shared global cache. */ + * THREADSAFE. This matters for the shared global cache. */ ret = WH_SERVER_NVM_LOCK(ctx); if (ret == WH_ERROR_OK) { if (WH_KEYID_ISERASED(key_id)) { @@ -512,42 +512,43 @@ static int _HandleRsaFunction(whServerContext* ctx, uint16_t magic, int devId, /* load the key from the keystore, enforcing the usage policy against the * same locked snapshot of the key that is exported */ if (ret == 0) { - ret = wh_Server_CacheExportRsaKeyEnforce(ctx, key_id, requiredUsage, rsa); - if (ret == WH_ERROR_USAGE) { - /* Currently wolfCrypt doesn't have a way for crypto callbacks to - distinguish if a low level RSA operation (like encrypt/decrypt) is - being performed as part of a higher level operation like - sign/verify. Until that information is propagated to the - callback, the usage flags are treated as equivalent. */ - if (op_type == RSA_PUBLIC_DECRYPT) { + ret = + wh_Server_CacheExportRsaKeyEnforce(ctx, key_id, requiredUsage, rsa); + if (ret == WH_ERROR_USAGE) { + /* Currently wolfCrypt doesn't have a way for crypto callbacks to + distinguish if a low level RSA operation (like encrypt/decrypt) is + being performed as part of a higher level operation like + sign/verify. Until that information is propagated to the + callback, the usage flags are treated as equivalent. */ + if (op_type == RSA_PUBLIC_DECRYPT) { /* Decrypt usage flag wasn't set so this might be a verify * operation. Attempt to enforce against the verify flag */ ret = wh_Server_CacheExportRsaKeyEnforce( ctx, key_id, WH_NVM_FLAGS_USAGE_VERIFY, rsa); - } - else if (op_type == RSA_PRIVATE_ENCRYPT) { + } + else if (op_type == RSA_PRIVATE_ENCRYPT) { /* Encrypt usage flag wasn't set so this might be a sign * operation. Attempt to enforce against the sign flag */ ret = wh_Server_CacheExportRsaKeyEnforce( ctx, key_id, WH_NVM_FLAGS_USAGE_SIGN, rsa); + } } - } - WH_DEBUG_SERVER_VERBOSE("CacheExportRsaKey keyid:%u, ret:%d\n", key_id, - ret); - if (ret == 0) { - /* do the rsa operation */ - ret = wc_RsaFunction(in, in_len, out, &out_len, op_type, rsa, - ctx->crypto->rng); - WH_DEBUG_SERVER_VERBOSE( - "RsaFunction in:%p %u, out:%p, opType:%d, outLen:%d, ret:%d\n", in, - in_len, out, op_type, out_len, ret); - } - /* free the key */ - wc_FreeRsaKey(rsa); + WH_DEBUG_SERVER_VERBOSE("CacheExportRsaKey keyid:%u, ret:%d\n", key_id, + ret); + if (ret == 0) { + /* do the rsa operation */ + ret = wc_RsaFunction(in, in_len, out, &out_len, op_type, rsa, + ctx->crypto->rng); + WH_DEBUG_SERVER_VERBOSE( + "RsaFunction in:%p %u, out:%p, opType:%d, outLen:%d, ret:%d\n", + in, in_len, out, op_type, out_len, ret); + } + /* free the key */ + wc_FreeRsaKey(rsa); } if (evict != 0) { /* User requested to evict from cache, even if the call failed */ - _CryptoEvictKeyLocked(ctx, key_id); + _CryptoEvictKeyLocked(ctx, key_id); } if (ret == 0) { whMessageCrypto_RsaResponse res; @@ -695,7 +696,7 @@ int wh_Server_EccKeyCacheExportEnforce(whServerContext* ctx, whKeyId keyId, } /* Freshen, check usage and deserialize under one hold of the NVM lock so * the policy verdict, the metadata length and the key bytes all come from - * the same snapshot of the shared cache slot. This mattters for the shared + * the same snapshot of the shared cache slot. This matters for the shared * global cache. */ ret = WH_SERVER_NVM_LOCK(ctx); if (ret == WH_ERROR_OK) { @@ -782,7 +783,7 @@ int wh_Server_CacheExportEd25519KeyEnforce(whServerContext* ctx, whKeyId keyId, /* Freshen, check usage and deserialize under one hold of the NVM lock so * the policy verdict, the metadata length and the key bytes all come from - * the same snapshot of the shared cache slot. This mattters for the shared + * the same snapshot of the shared cache slot. This matters for the shared * global cache. */ ret = WH_SERVER_NVM_LOCK(ctx); if (ret == WH_ERROR_OK) { @@ -877,7 +878,7 @@ int wh_Server_CacheExportCurve25519KeyEnforce(whServerContext* server, } /* Freshen, check usage and deserialize under one hold of the NVM lock so * the policy verdict, the metadata length and the key bytes all come from - * the same snapshot of the shared cache slot. This mattters for the shared + * the same snapshot of the shared cache slot. This matters for the shared * global cache. */ ret = WH_SERVER_NVM_LOCK(server); if (ret == WH_ERROR_OK) { @@ -976,7 +977,7 @@ int wh_Server_MlDsaKeyCacheExportEnforce(whServerContext* ctx, whKeyId keyId, /* Freshen, check usage and deserialize under one hold of the NVM lock so * the policy verdict, the metadata length and the key bytes all come from - * the same snapshot of the shared cache slot. This mattters for the shared + * the same snapshot of the shared cache slot. This matters for the shared * global cache. */ ret = WH_SERVER_NVM_LOCK(ctx); if (ret == WH_ERROR_OK) { @@ -1062,7 +1063,7 @@ int wh_Server_MlKemKeyCacheExportEnforce(whServerContext* ctx, whKeyId keyId, /* Freshen, check usage and deserialize under one hold of the NVM lock so * the policy verdict, the metadata length and the key bytes all come from - * the same snapshot of the shared cache slot. This mattters for the shared + * the same snapshot of the shared cache slot. This matters for the shared * global cache. */ ret = WH_SERVER_NVM_LOCK(ctx); if (ret == WH_ERROR_OK) { @@ -1407,7 +1408,7 @@ static int _HandleEccKeyGen(whServerContext* ctx, uint16_t magic, int devId, res_size = 0; /* Hold the NVM lock so id allocation and cache import are * atomic with respect to other server contexts under - * THREADSAFE. This mattters for the shared + * THREADSAFE. This matters for the shared * global cache. */ ret = WH_SERVER_NVM_LOCK(ctx); if (ret == WH_ERROR_OK) { diff --git a/wolfhsm/wh_settings.h b/wolfhsm/wh_settings.h index 354953abf..a6054c62b 100644 --- a/wolfhsm/wh_settings.h +++ b/wolfhsm/wh_settings.h @@ -472,8 +472,7 @@ #if defined(HAVE_HKDF) || defined(HAVE_CMAC_KDF) -/* Largest cached key the server will accept as a KDF input supplied by key ID - * */ +/* Largest cached key the server accepts as a KDF input supplied by key ID */ #ifndef WOLFHSM_CFG_SERVER_KDF_MAX_KEY_SIZE #define WOLFHSM_CFG_SERVER_KDF_MAX_KEY_SIZE 256 #endif