From 23b5dfd5cc86fa19bb76845700c46e3908b7caa3 Mon Sep 17 00:00:00 2001 From: jackctj117 Date: Thu, 30 Jul 2026 14:17:15 -0600 Subject: [PATCH 1/9] Report 2704 --- test/wh_test_crypto.c | 67 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/test/wh_test_crypto.c b/test/wh_test_crypto.c index 8ee35737b..ab3543bdb 100644 --- a/test/wh_test_crypto.c +++ b/test/wh_test_crypto.c @@ -17119,6 +17119,73 @@ int whTest_CryptoKeyUsagePolicies(whClientContext* client, WC_RNG* rng) #endif /* HAVE_ECC_DHE */ #endif /* HAVE_ECC */ +#ifdef HAVE_CURVE25519 + /* X25519 (Curve25519) shared secret without DERIVE flag */ + WH_TEST_PRINT(" Testing X25519 shared secret without DERIVE flag...\n"); + { + curve25519_key privKey[1]; + curve25519_key pubKey[1]; + uint8_t x25519Secret[CURVE25519_KEYSIZE] = {0}; + word32 x25519SecretLen = sizeof(x25519Secret); + whKeyId privId = WH_KEYID_ERASED; + whKeyId pubId = WH_KEYID_ERASED; + + /* Generate a private key on the server WITHOUT the derive flag */ + ret = wh_Client_Curve25519MakeCacheKey( + client, CURVE25519_KEYSIZE, &privId, WH_NVM_FLAGS_USAGE_SIGN, + (uint8_t*)"x25519-no-derive", strlen("x25519-no-derive")); + if (ret == 0) { + /* Valid peer public key (DERIVE allowed) */ + ret = wh_Client_Curve25519MakeCacheKey( + client, CURVE25519_KEYSIZE, &pubId, WH_NVM_FLAGS_USAGE_DERIVE, + (uint8_t*)"x25519-peer", strlen("x25519-peer")); + } + if (ret == 0) { + ret = wc_curve25519_init_ex(privKey, NULL, WH_CLIENT_DEVID(client)); + if (ret == 0) { + ret = wc_curve25519_init_ex(pubKey, NULL, + WH_CLIENT_DEVID(client)); + if (ret == 0) { + /* Associate the cached keyIds with the local key objects */ + ret = wh_Client_Curve25519SetKeyId(privKey, privId); + if (ret == 0) { + ret = wh_Client_Curve25519SetKeyId(pubKey, pubId); + } + + /* Must fail: private key lacks DERIVE */ + if (ret == 0) { + ret = wc_curve25519_shared_secret( + privKey, pubKey, x25519Secret, &x25519SecretLen); + if (ret == WH_ERROR_USAGE) { + WH_TEST_PRINT( + " PASS: Correctly denied key derivation\n"); + ret = 0; /* Test passed */ + } + else { + WH_ERROR_PRINT( + " FAIL: Expected WH_ERROR_USAGE, got %d\n", + ret); + ret = WH_ERROR_ABORTED; + } + } + wc_curve25519_free(pubKey); + } + wc_curve25519_free(privKey); + } + } + /* Clean up cached keys */ + if (!WH_KEYID_ISERASED(privId)) { + wh_Client_KeyEvict(client, privId); + } + if (!WH_KEYID_ISERASED(pubId)) { + wh_Client_KeyEvict(client, pubId); + } + } + if (ret != 0) { + return ret; + } +#endif /* HAVE_CURVE25519 */ + #ifdef HAVE_HKDF /* HKDF without DERIVE flag */ WH_TEST_PRINT(" Testing HKDF without DERIVE flag...\n"); From fa934331c201c4b25d7b7d669437e99d09779fcd Mon Sep 17 00:00:00 2001 From: jackctj117 Date: Thu, 30 Jul 2026 14:17:15 -0600 Subject: [PATCH 2/9] Report 5476 --- port/posix/posix_transport_shm.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/port/posix/posix_transport_shm.c b/port/posix/posix_transport_shm.c index a61863dcd..f03af7cc6 100644 --- a/port/posix/posix_transport_shm.c +++ b/port/posix/posix_transport_shm.c @@ -603,6 +603,9 @@ int posixTransportShm_ClientStaticMemDmaCallback( else if (oper == WH_DMA_OPER_CLIENT_READ_POST) { if (isInDma == 0) { uint8_t* ptr = (uint8_t*)dmaPtr + (uintptr_t)*xformedCliAddr; + /* Scrub key material before freeing. len is bounded by the temp + * buffer's XMALLOC, well within uint32_t for this transport. */ + wh_Utils_ForceZero(ptr, (uint32_t)len); XFREE(ptr, heap, DYNAMIC_TYPE_TMP_BUFFER); } } @@ -611,6 +614,9 @@ int posixTransportShm_ClientStaticMemDmaCallback( uint8_t* ptr = (uint8_t*)dmaPtr + (uintptr_t)*xformedCliAddr; memcpy((void*)clientAddr, ptr, len); /* copy results of what server wrote */ + /* Scrub key material before freeing. len is bounded by the temp + * buffer's XMALLOC, well within uint32_t for this transport. */ + wh_Utils_ForceZero(ptr, (uint32_t)len); XFREE(ptr, heap, DYNAMIC_TYPE_TMP_BUFFER); } } From fe5164b46e56075751fcc4b5fcc15a400fdbbeda Mon Sep 17 00:00:00 2001 From: jackctj117 Date: Thu, 30 Jul 2026 14:17:15 -0600 Subject: [PATCH 3/9] Report 3110 --- test/wh_test_she.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/test/wh_test_she.c b/test/wh_test_she.c index c04423d2c..ef7ac967d 100644 --- a/test/wh_test_she.c +++ b/test/wh_test_she.c @@ -414,6 +414,43 @@ int whTest_SheClientConfig(whClientConfig* config) WH_TEST_PRINT("SHE LOAD KEY UID checks SUCCESS\n"); } + /* Corrupted M3 with valid M1/M2 must be rejected; reloading with the + * restored M3 at the same counter then succeeds. */ + { + uint8_t savedM3; + + if ((ret = wh_She_GenerateLoadableKey( + SHE_TEST_VECTOR_KEY_ID, WH_SHE_MASTER_ECU_KEY_ID, 2, 0, sheUid, + vectorRawKey, vectorMasterEcuKey, messageOne, messageTwo, + messageThree, messageFour, messageFive)) != 0) { + WH_ERROR_PRINT("Failed to generate M3-test M1/M2/M3 %d\n", ret); + goto exit; + } + + savedM3 = messageThree[0]; + messageThree[0] ^= 0xFF; + ret = wh_Client_SheLoadKey(client, messageOne, messageTwo, messageThree, + outMessageFour, outMessageFive); + if (ret != WH_SHE_ERC_KEY_UPDATE_ERROR) { + WH_ERROR_PRINT("SHE LOAD KEY corrupt M3: expected " + "KEY_UPDATE_ERROR, got %d\n", + ret); + ret = WH_ERROR_ABORTED; + goto exit; + } + + messageThree[0] = savedM3; + if ((ret = wh_Client_SheLoadKey(client, messageOne, messageTwo, + messageThree, outMessageFour, + outMessageFive)) != 0) { + WH_ERROR_PRINT("SHE LOAD KEY restored M3: expected success, " + "got %d\n", + ret); + goto exit; + } + WH_TEST_PRINT("SHE LOAD KEY M3 CMAC auth SUCCESS\n"); + } + if ((ret = wh_Client_SheInitRnd(client)) != 0) { WH_ERROR_PRINT("Failed to wh_Client_SheInitRnd %d\n", ret); goto exit; From f38332d4abf7eaaefa1bc3a35d40aa0205e09958 Mon Sep 17 00:00:00 2001 From: jackctj117 Date: Thu, 30 Jul 2026 14:17:15 -0600 Subject: [PATCH 4/9] Report 2705 --- test/wh_test_crypto.c | 150 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 150 insertions(+) diff --git a/test/wh_test_crypto.c b/test/wh_test_crypto.c index ab3543bdb..37295e917 100644 --- a/test/wh_test_crypto.c +++ b/test/wh_test_crypto.c @@ -17186,6 +17186,156 @@ int whTest_CryptoKeyUsagePolicies(whClientContext* client, WC_RNG* rng) } #endif /* HAVE_CURVE25519 */ +#ifndef NO_RSA + /* RSA usage policy plus the sign/verify fallback (PUBLIC_DECRYPT falls back + * to VERIFY, PRIVATE_ENCRYPT to SIGN). */ + WH_TEST_PRINT(" Testing RSA usage policy and sign/verify fallback...\n"); + { + RsaKey rsaKey[1]; + whKeyId rsaId = WH_KEYID_ERASED; + byte rsaIn[32]; + byte rsaSig[RSA_KEY_BYTES]; + byte rsaRec[RSA_KEY_BYTES]; + int opRc; + int sigLen; + + memset(rsaIn, 0xA5, sizeof(rsaIn)); + + /* SIGN only key: PublicEncrypt needs ENCRYPT, so deny. */ + ret = + wh_Client_RsaMakeCacheKey(client, RSA_KEY_BITS, RSA_EXPONENT, + &rsaId, WH_NVM_FLAGS_USAGE_SIGN, 0, NULL); + if (ret == 0) { + ret = wc_InitRsaKey_ex(rsaKey, NULL, WH_CLIENT_DEVID(client)); + if (ret == 0) { + ret = wh_Client_RsaSetKeyId(rsaKey, rsaId); + if (ret == 0) { + opRc = wc_RsaPublicEncrypt(rsaIn, sizeof(rsaIn), rsaSig, + sizeof(rsaSig), rsaKey, rng); + if (opRc == WH_ERROR_USAGE) { + WH_TEST_PRINT(" PASS: encrypt denied without " + "ENCRYPT\n"); + } + else { + WH_ERROR_PRINT( + " FAIL: expected WH_ERROR_USAGE, got %d\n", + opRc); + ret = WH_ERROR_ABORTED; + } + } + wc_FreeRsaKey(rsaKey); + } + wh_Client_KeyEvict(client, rsaId); + } + if (ret != 0) { + return ret; + } + + /* ENCRYPT only key: Verify needs DECRYPT or VERIFY, so deny. */ + rsaId = WH_KEYID_ERASED; + ret = wh_Client_RsaMakeCacheKey(client, RSA_KEY_BITS, RSA_EXPONENT, + &rsaId, WH_NVM_FLAGS_USAGE_ENCRYPT, 0, + NULL); + if (ret == 0) { + ret = wc_InitRsaKey_ex(rsaKey, NULL, WH_CLIENT_DEVID(client)); + if (ret == 0) { + ret = wh_Client_RsaSetKeyId(rsaKey, rsaId); + if (ret == 0) { + memset(rsaSig, 0, sizeof(rsaSig)); + opRc = wc_RsaSSL_Verify(rsaSig, sizeof(rsaSig), rsaRec, + sizeof(rsaRec), rsaKey); + if (opRc == WH_ERROR_USAGE) { + WH_TEST_PRINT(" PASS: verify denied without " + "VERIFY\n"); + } + else { + WH_ERROR_PRINT( + " FAIL: expected WH_ERROR_USAGE, got %d\n", + opRc); + ret = WH_ERROR_ABORTED; + } + } + wc_FreeRsaKey(rsaKey); + } + wh_Client_KeyEvict(client, rsaId); + } + if (ret != 0) { + return ret; + } + + /* VERIFY only key: Sign needs ENCRYPT or SIGN, so deny. */ + rsaId = WH_KEYID_ERASED; + ret = wh_Client_RsaMakeCacheKey(client, RSA_KEY_BITS, RSA_EXPONENT, + &rsaId, WH_NVM_FLAGS_USAGE_VERIFY, 0, + NULL); + if (ret == 0) { + ret = wc_InitRsaKey_ex(rsaKey, NULL, WH_CLIENT_DEVID(client)); + if (ret == 0) { + ret = wh_Client_RsaSetKeyId(rsaKey, rsaId); + if (ret == 0) { + opRc = wc_RsaSSL_Sign(rsaIn, sizeof(rsaIn), rsaSig, + sizeof(rsaSig), rsaKey, rng); + if (opRc == WH_ERROR_USAGE) { + WH_TEST_PRINT(" PASS: sign denied without SIGN\n"); + } + else { + WH_ERROR_PRINT( + " FAIL: expected WH_ERROR_USAGE, got %d\n", + opRc); + ret = WH_ERROR_ABORTED; + } + } + wc_FreeRsaKey(rsaKey); + } + wh_Client_KeyEvict(client, rsaId); + } + if (ret != 0) { + return ret; + } + + /* 4) SIGN|VERIFY key: sign then verify must succeed via the fallback + * paths, proving the fallback is live rather than dead code. */ + rsaId = WH_KEYID_ERASED; + ret = wh_Client_RsaMakeCacheKey( + client, RSA_KEY_BITS, RSA_EXPONENT, &rsaId, + WH_NVM_FLAGS_USAGE_SIGN | WH_NVM_FLAGS_USAGE_VERIFY, 0, NULL); + if (ret == 0) { + ret = wc_InitRsaKey_ex(rsaKey, NULL, WH_CLIENT_DEVID(client)); + if (ret == 0) { + ret = wh_Client_RsaSetKeyId(rsaKey, rsaId); + if (ret == 0) { + sigLen = wc_RsaSSL_Sign(rsaIn, sizeof(rsaIn), rsaSig, + sizeof(rsaSig), rsaKey, rng); + if (sigLen < 0) { + WH_ERROR_PRINT(" FAIL: sign via fallback %d\n", + sigLen); + ret = WH_ERROR_ABORTED; + } + } + if (ret == 0) { + opRc = wc_RsaSSL_Verify(rsaSig, sigLen, rsaRec, + sizeof(rsaRec), rsaKey); + if (opRc != (int)sizeof(rsaIn) || + memcmp(rsaRec, rsaIn, sizeof(rsaIn)) != 0) { + WH_ERROR_PRINT(" FAIL: verify via fallback %d\n", + opRc); + ret = WH_ERROR_ABORTED; + } + else { + WH_TEST_PRINT( + " PASS: sign/verify fallback round trip\n"); + } + } + wc_FreeRsaKey(rsaKey); + } + wh_Client_KeyEvict(client, rsaId); + } + if (ret != 0) { + return ret; + } + } +#endif /* NO_RSA */ + #ifdef HAVE_HKDF /* HKDF without DERIVE flag */ WH_TEST_PRINT(" Testing HKDF without DERIVE flag...\n"); From ecd2b4c56e44a9b351988e1bddc78704f9a733b6 Mon Sep 17 00:00:00 2001 From: jackctj117 Date: Thu, 30 Jul 2026 14:17:15 -0600 Subject: [PATCH 5/9] Report 2001 --- src/wh_server_counter.c | 3 ++- test/wh_test_clientserver.c | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/wh_server_counter.c b/src/wh_server_counter.c index a681c894a..22a5b1225 100644 --- a/src/wh_server_counter.c +++ b/src/wh_server_counter.c @@ -225,7 +225,8 @@ int wh_Server_HandleCounter(whServerContext* server, uint16_t magic, } break; default: - ret = WH_ERROR_BADARGS; + *out_resp_size = 0; + ret = WH_ERROR_BADARGS; break; } diff --git a/test/wh_test_clientserver.c b/test/wh_test_clientserver.c index 56caebc70..0336177e8 100644 --- a/test/wh_test_clientserver.c +++ b/test/wh_test_clientserver.c @@ -678,6 +678,25 @@ static int _testClientCounter(whClientContext* client) wh_Client_CounterRead(client, (whNvmId)i, &counter)); } + /* Invalid counter action: the default case reports a zero length + * response, not the stale request sized buffer. */ + { + uint8_t reqbuf[8] = {0}; + uint8_t respbuf[64] = {0}; + uint16_t respGroup = 0; + uint16_t respAction = 0; + uint16_t respSz = 0xFFFF; + + WH_TEST_RETURN_ON_FAIL(wh_Client_SendRequest( + client, WH_MESSAGE_GROUP_COUNTER, 0x7F, sizeof(reqbuf), reqbuf)); + do { + rc = wh_Client_RecvResponse(client, &respGroup, &respAction, + &respSz, sizeof(respbuf), respbuf); + } while (rc == WH_ERROR_NOTREADY); + WH_TEST_ASSERT_RETURN(rc == WH_ERROR_OK); + WH_TEST_ASSERT_RETURN(respSz == 0); + } + /* Ensure NVM is empty */ WH_TEST_RETURN_ON_FAIL(rc = wh_Client_NvmGetAvailable( client, &server_rc, &avail_size, &avail_objects, From d49abfebfef348ba21829ddbf934403f9872687a Mon Sep 17 00:00:00 2001 From: jackctj117 Date: Thu, 30 Jul 2026 14:17:15 -0600 Subject: [PATCH 6/9] Report 2709 --- test/wh_test_crypto.c | 54 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/test/wh_test_crypto.c b/test/wh_test_crypto.c index 37295e917..bddc18b50 100644 --- a/test/wh_test_crypto.c +++ b/test/wh_test_crypto.c @@ -17046,6 +17046,60 @@ int whTest_CryptoKeyUsagePolicies(whClientContext* client, WC_RNG* rng) } #endif /* HAVE_ECC_SIGN */ +#ifdef HAVE_ECC_VERIFY + /* ECDSA verify without VERIFY flag */ + WH_TEST_PRINT(" Testing ECDSA verify without VERIFY flag...\n"); + { + ecc_key eccKey[1]; + uint8_t sig[ECC_MAX_SIG_SIZE] = {0}; + word32 sigLen = sizeof(sig); + uint8_t hash[WC_SHA256_DIGEST_SIZE] = {0}; + int verifyResult = 0; + + /* Cache a SIGN only key so verification is denied by usage policy + * before the signature is checked. */ + keyId = WH_KEYID_ERASED; + ret = wh_Client_EccMakeCacheKey( + client, 32, ECC_SECP256R1, &keyId, WH_NVM_FLAGS_USAGE_SIGN, + strlen("ecc-no-verify"), (uint8_t*)"ecc-no-verify"); + if (ret == 0) { + ret = wc_ecc_init_ex(eccKey, NULL, WH_CLIENT_DEVID(client)); + if (ret == 0) { + ret = wc_ecc_set_curve(eccKey, 32, ECC_SECP256R1); + if (ret == 0) { + ret = wh_Client_EccSetKeyId(eccKey, keyId); + if (ret == 0) { + ret = wc_RNG_GenerateBlock(rng, hash, sizeof(hash)); + if (ret == 0) { + /* Usage enforcement runs before verify, so a + * dummy signature is enough to kill the check. */ + ret = wc_ecc_verify_hash(sig, sigLen, hash, + sizeof(hash), + &verifyResult, eccKey); + if (ret == WH_ERROR_USAGE) { + WH_TEST_PRINT( + " PASS: Correctly denied verification\n"); + ret = 0; /* Test passed */ + } + else { + WH_ERROR_PRINT(" FAIL: Expected " + "WH_ERROR_USAGE, got %d\n", + ret); + ret = WH_ERROR_ABORTED; + } + } + } + } + wc_ecc_free(eccKey); + } + wh_Client_KeyEvict(client, keyId); + } + } + if (ret != 0) { + return ret; + } +#endif /* HAVE_ECC_VERIFY */ + #ifdef HAVE_ECC_DHE /* ECDH without DERIVE flag */ WH_TEST_PRINT(" Testing ECDH without DERIVE flag...\n"); From a72861f8aaf4168394253c01982f58efb70e6a34 Mon Sep 17 00:00:00 2001 From: jackctj117 Date: Thu, 30 Jul 2026 14:17:15 -0600 Subject: [PATCH 7/9] Report 2298 --- src/wh_server_crypto.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/wh_server_crypto.c b/src/wh_server_crypto.c index b419a4af7..b7162c8d6 100644 --- a/src/wh_server_crypto.c +++ b/src/wh_server_crypto.c @@ -4396,6 +4396,10 @@ static int _HandleCmac(whServerContext* ctx, uint16_t magic, int devId, } } WH_DEBUG_SERVER_VERBOSE("cmac end ret:%d\n", ret); + /* Scrub tmpKey and the Cmac context (AES schedule + k1/k2). Full struct + * zero avoids a wc_CmacFree double free. */ + wh_Utils_ForceZero(tmpKey, sizeof(tmpKey)); + wh_Utils_ForceZero(cmac, sizeof(cmac)); return ret; } #endif /* WOLFSSL_CMAC && !NO_AES && WOLFSSL_AES_DIRECT */ @@ -8921,6 +8925,10 @@ static int _HandleCmacDma(whServerContext* ctx, uint16_t magic, int devId, } WH_DEBUG_SERVER_VERBOSE("dma cmac end ret:%d\n", ret); + /* Scrub tmpKey and the Cmac context (AES schedule + k1/k2). Full struct + * zero avoids a wc_CmacFree double free. */ + wh_Utils_ForceZero(tmpKey, sizeof(tmpKey)); + wh_Utils_ForceZero(cmac, sizeof(cmac)); return ret; } #endif /* WOLFSSL_CMAC && !NO_AES && WOLFSSL_AES_DIRECT */ From a9c7004ec83bb2f3d117e1e5c92306a344e871ca Mon Sep 17 00:00:00 2001 From: jackctj117 Date: Thu, 30 Jul 2026 14:17:15 -0600 Subject: [PATCH 8/9] Report 4235 --- src/wh_server_cert.c | 35 +++++++++++++---------------------- test/wh_test_cert.c | 28 ++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 22 deletions(-) diff --git a/src/wh_server_cert.c b/src/wh_server_cert.c index ff6ff64b1..9f1613dfc 100644 --- a/src/wh_server_cert.c +++ b/src/wh_server_cert.c @@ -1511,40 +1511,35 @@ int wh_Server_HandleCertRequest(whServerContext* server, uint16_t magic, if (req_size != sizeof(req)) { /* Request is malformed */ - rc = WH_ERROR_ABORTED; + resp.rc = WH_ERROR_ABORTED; } - if (rc == WH_ERROR_OK) { + if (resp.rc == WH_ERROR_OK) { /* Convert request struct */ wh_MessageCert_TranslateVerifyDmaRequest( magic, (whMessageCert_VerifyDmaRequest*)req_packet, &req); /* Process client address */ - rc = wh_Server_DmaProcessClientAddress( + resp.rc = wh_Server_DmaProcessClientAddress( server, req.cert_addr, &cert_data, req.cert_len, WH_DMA_OPER_CLIENT_READ_PRE, (whServerDmaFlags){0}); - if (rc == WH_ERROR_OK) { + if (resp.rc == WH_ERROR_OK) { cert_dma_pre_ok = 1; } } - if (rc == WH_ERROR_OK) { + if (resp.rc == WH_ERROR_OK) { /* Process the verify action */ - rc = WH_SERVER_NVM_LOCK(server); - if (rc == WH_ERROR_OK) { - rc = wh_Server_CertVerifyAcert( + resp.rc = WH_SERVER_NVM_LOCK(server); + if (resp.rc == WH_ERROR_OK) { + resp.rc = wh_Server_CertVerifyAcert( server, cert_data, req.cert_len, req.trustedRootNvmId); (void)WH_SERVER_NVM_UNLOCK(server); } /* WH_SERVER_NVM_LOCK() */ - /* Signature verification error is not an error for the server, - * so propagate this error to the client in the response, - * otherwise return the error code from the verify action */ - if (rc == ASN_SIG_CONFIRM_E || rc == ASN_SIG_OID_E) { + /* A signature verification failure is reported to the client, + * not treated as a server error */ + if (resp.rc == ASN_SIG_CONFIRM_E || resp.rc == ASN_SIG_OID_E) { resp.rc = WH_ERROR_CERT_VERIFY; - rc = WH_ERROR_OK; - } - else { - resp.rc = rc; } } /* Always call POST for successful PRE, regardless of operation @@ -1555,15 +1550,11 @@ int wh_Server_HandleCertRequest(whServerContext* server, uint16_t magic, WH_DMA_OPER_CLIENT_READ_POST, (whServerDmaFlags){0}); } - /* Convert the response struct */ + /* Convert the response struct. resp.rc now holds the final status + * on every path, so translate last with no post fixup */ wh_MessageCert_TranslateSimpleResponse( magic, &resp, (whMessageCert_SimpleResponse*)resp_packet); *out_resp_size = sizeof(resp); - - /* If there was an error, return it in the response */ - if (rc != WH_ERROR_OK) { - resp.rc = rc; - } } break; #endif /* WOLFHSM_CFG_DMA */ #endif /* WOLFHSM_CFG_CERTIFICATE_MANAGER_ACERT */ diff --git a/test/wh_test_cert.c b/test/wh_test_cert.c index 926a208fe..e1aa0f1c1 100644 --- a/test/wh_test_cert.c +++ b/test/wh_test_cert.c @@ -28,6 +28,8 @@ #if defined(WOLFHSM_CFG_CERTIFICATE_MANAGER) && !defined(WOLFHSM_CFG_NO_CRYPTO) #include "wolfhsm/wh_error.h" +#include "wolfhsm/wh_message.h" +#include "wolfhsm/wh_message_cert.h" #ifdef WOLFHSM_CFG_ENABLE_SERVER #include "wolfhsm/wh_server.h" @@ -1846,6 +1848,32 @@ int whTest_CertClientAcertDma_ClientServerTestInternal(whClientContext* client) client, attrCert_der, attrCert_der_len, rootCertB_id, &out_rc)); WH_TEST_ASSERT_RETURN(out_rc == WH_ERROR_CERT_VERIFY); + /* Regression test for finding 4235. A malformed (undersized) ACERT_DMA + * request must report an error on the wire, not a false success. Send a + * raw 1 byte request with the low level API and confirm resp.rc is not OK. + */ + WH_TEST_PRINT("Sending malformed ACERT_DMA request...\n"); + { + uint8_t badReq = 0; + uint16_t rgroup, raction, rsize; + whMessageCert_SimpleResponse badResp = {0}; + + do { + rc = wh_Client_SendRequest( + client, WH_MESSAGE_GROUP_CERT, + WH_MESSAGE_CERT_ACTION_VERIFY_ACERT_DMA, sizeof(badReq), + &badReq); + } while (rc == WH_ERROR_NOTREADY); + WH_TEST_ASSERT_RETURN(rc == WH_ERROR_OK); + + do { + rc = wh_Client_RecvResponse(client, &rgroup, &raction, &rsize, + sizeof(badResp), &badResp); + } while (rc == WH_ERROR_NOTREADY); + WH_TEST_ASSERT_RETURN(rc == WH_ERROR_OK); + WH_TEST_ASSERT_RETURN(badResp.rc != WH_ERROR_OK); + } + /* Clean up - delete the trusted certificates */ WH_TEST_PRINT("Deleting trusted certificates...\n"); WH_TEST_RETURN_ON_FAIL( From e84d2b7c270350dd6fbfb55b3b05a75b6c7b5072 Mon Sep 17 00:00:00 2001 From: jackctj117 Date: Thu, 30 Jul 2026 14:17:15 -0600 Subject: [PATCH 9/9] Report 4324 --- src/wh_server_img_mgr.c | 77 +++++++++++++---- test/wh_test_server_img_mgr.c | 157 ++++++++++++++++++++++++++++++++++ wolfhsm/wh_settings.h | 9 ++ 3 files changed, 225 insertions(+), 18 deletions(-) diff --git a/src/wh_server_img_mgr.c b/src/wh_server_img_mgr.c index a20ec29e5..70f868a71 100644 --- a/src/wh_server_img_mgr.c +++ b/src/wh_server_img_mgr.c @@ -80,15 +80,55 @@ int wh_Server_ImgMgrInit(whServerImgMgrContext* context, return ret; } +/* Copy key material out of the keystore cache while holding the NVM lock, so + * the caller works from a private snapshot instead of a live cache slot that a + * concurrent evict or cache request could rewrite. */ +static int _ImgMgrCopyKeyFromKeystore(whServerContext* server, whKeyId keyId, + uint8_t* dst, size_t dstMax, + size_t* outLen) +{ + int ret; + uint8_t* keyBuf = NULL; + whNvmMetadata* keyMeta = NULL; + + if ((server == NULL) || (dst == NULL) || (outLen == NULL) || + (dstMax == 0)) { + return WH_ERROR_BADARGS; + } + + ret = WH_SERVER_NVM_LOCK(server); + if (ret != WH_ERROR_OK) { + return ret; + } + + ret = wh_Server_KeystoreFreshenKey(server, keyId, &keyBuf, &keyMeta); + if (ret == WH_ERROR_OK) { + if ((keyBuf == NULL) || (keyMeta == NULL)) { + ret = WH_ERROR_ABORTED; + } + else if ((size_t)keyMeta->len > dstMax) { + ret = WH_ERROR_BUFFER_SIZE; + } + else { + memcpy(dst, keyBuf, keyMeta->len); + *outLen = (size_t)keyMeta->len; + } + } + + (void)WH_SERVER_NVM_UNLOCK(server); + + return ret; +} + int wh_Server_ImgMgrVerifyImg(whServerImgMgrContext* context, const whServerImgMgrImg* img, whServerImgMgrVerifyResult* result) { - int ret = WH_ERROR_OK; - whServerContext* server = NULL; - uint8_t* keyBuf = NULL; - whNvmMetadata* keyMeta = NULL; - size_t keySz = 0; + int ret = WH_ERROR_OK; + whServerContext* server = NULL; + uint8_t keyBuf[WOLFHSM_CFG_SERVER_IMG_MGR_MAX_KEY_SIZE]; + const uint8_t* keyPtr = NULL; /* stays NULL for paths with no key */ + size_t keySz = 0; uint8_t sigBuf[WOLFHSM_CFG_SERVER_IMG_MGR_MAX_SIG_SIZE]; /* Buffer for signature */ whNvmMetadata sigMeta = {0}; @@ -112,12 +152,12 @@ int wh_Server_ImgMgrVerifyImg(whServerImgMgrContext* context, switch (img->imgType) { case WH_IMG_MGR_IMG_TYPE_WOLFBOOT: /* Load key from keystore, skip sig loading (sig is in header) */ - ret = wh_Server_KeystoreFreshenKey(server, img->keyId, &keyBuf, - &keyMeta); + ret = _ImgMgrCopyKeyFromKeystore(server, img->keyId, keyBuf, + sizeof(keyBuf), &keySz); if (ret != WH_ERROR_OK) { return ret; } - keySz = keyMeta->len; + keyPtr = keyBuf; /* sig/sigSz passed as NULL/0 to callback */ break; @@ -127,15 +167,8 @@ int wh_Server_ImgMgrVerifyImg(whServerImgMgrContext* context, break; case WH_IMG_MGR_IMG_TYPE_RAW: - /* Existing behavior: load key from keystore + sig from NVM */ - ret = wh_Server_KeystoreFreshenKey(server, img->keyId, &keyBuf, - &keyMeta); - if (ret != WH_ERROR_OK) { - return ret; - } - keySz = keyMeta->len; - - /* Load the signature from NVM */ + /* Load the signature from NVM first so the key snapshot is the + * last thing taken before verification */ ret = wh_Nvm_GetMetadata(server->nvm, img->sigNvmId, &sigMeta); if (ret != WH_ERROR_OK) { return ret; @@ -153,6 +186,14 @@ int wh_Server_ImgMgrVerifyImg(whServerImgMgrContext* context, } actualSigSize = sigMeta.len; sigPtr = sigBuf; + + /* Load key from keystore */ + ret = _ImgMgrCopyKeyFromKeystore(server, img->keyId, keyBuf, + sizeof(keyBuf), &keySz); + if (ret != WH_ERROR_OK) { + return ret; + } + keyPtr = keyBuf; break; default: @@ -162,7 +203,7 @@ int wh_Server_ImgMgrVerifyImg(whServerImgMgrContext* context, /* Invoke verify method callback */ if (img->verifyMethod != NULL) { result->verifyMethodResult = img->verifyMethod( - context, img, keyBuf, keySz, sigPtr, actualSigSize); + context, img, keyPtr, keySz, sigPtr, actualSigSize); } else { result->verifyMethodResult = WH_ERROR_NOHANDLER; diff --git a/test/wh_test_server_img_mgr.c b/test/wh_test_server_img_mgr.c index 53715787c..982da1f7f 100644 --- a/test/wh_test_server_img_mgr.c +++ b/test/wh_test_server_img_mgr.c @@ -1430,6 +1430,155 @@ whTest_ServerImgMgrServerCfgWolfBootCertChainRsa4096(whServerConfig* serverCfg) #endif /* WOLFHSM_CFG_CERTIFICATE_MANAGER */ #endif /* !NO_RSA */ +/* Key used to prove the verify callback gets a private copy */ +static const uint8_t testSnapshotKey[16] = {0x00, 0x11, 0x22, 0x33, 0x44, 0x55, + 0x66, 0x77, 0x88, 0x99, 0xAA, 0xBB, + 0xCC, 0xDD, 0xEE, 0xFF}; + +/* Verify method that evicts and re-caches the same keyId the way a concurrent + * request would. The key passed in must be a private snapshot, so it stays + * intact for the rest of the callback. */ +static int _ImgMgrSnapshotVerifyMethod(whServerImgMgrContext* context, + const whServerImgMgrImg* img, + const uint8_t* key, size_t keySz, + const uint8_t* sig, size_t sigSz) +{ + int ret; + whNvmMetadata meta = {0}; + uint8_t attackerKey[sizeof(testSnapshotKey)]; + + (void)sig; + (void)sigSz; + + if (context == NULL || context->server == NULL || img == NULL || + key == NULL || keySz != sizeof(testSnapshotKey)) { + return WH_ERROR_BADARGS; + } + + memset(attackerKey, 0xAA, sizeof(attackerKey)); + + ret = wh_Server_KeystoreEvictKey(context->server, img->keyId); + if (ret != WH_ERROR_OK) { + WH_ERROR_PRINT("Failed to evict key during verify: %d\n", ret); + return ret; + } + + meta.id = img->keyId; + meta.access = WH_NVM_ACCESS_ANY; + meta.flags = WH_NVM_FLAGS_NONE; + meta.len = sizeof(attackerKey); + ret = wh_Server_KeystoreCacheKey(context->server, &meta, attackerKey); + if (ret != WH_ERROR_OK) { + WH_ERROR_PRINT("Failed to cache substitute key during verify: %d\n", + ret); + return ret; + } + + if (memcmp(key, testSnapshotKey, keySz) != 0) { + WH_ERROR_PRINT("Verify key was substituted mid verification\n"); + return WH_ERROR_ABORTED; + } + + return WH_ERROR_OK; +} + +static int whTest_ServerImgMgrKeySnapshot(whServerConfig* serverCfg) +{ + int ret = 0; + whServerContext server[1] = {0}; + whServerImgMgrConfig imgMgrConfig = {0}; + whServerImgMgrContext imgMgr = {0}; + whServerImgMgrImg testImage = {0}; + whServerImgMgrVerifyResult result = {0}; + whNvmMetadata keyMeta = {0}; + whNvmMetadata sigMeta = {0}; + const whNvmId testKeyId = 1; + const whNvmId testSigNvmId = 2; + const uint8_t dummySig[4] = {0}; + + /* The callback ignores the signature, but the RAW path requires one */ + sigMeta.id = testSigNvmId; + sigMeta.access = WH_NVM_ACCESS_ANY; + sigMeta.flags = WH_NVM_FLAGS_NONE; + sigMeta.len = sizeof(dummySig); + snprintf((char*)sigMeta.label, WH_NVM_LABEL_LEN, "TestSnapshotSig"); + + ret = + wh_Nvm_AddObject(serverCfg->nvm, &sigMeta, sizeof(dummySig), dummySig); + if (ret != WH_ERROR_OK) { + WH_ERROR_PRINT("Failed to add snapshot signature to NVM: %d\n", ret); + return ret; + } + + testImage.imgType = WH_IMG_MGR_IMG_TYPE_RAW; + testImage.addr = (uintptr_t)testData; + testImage.size = sizeof(testData); + testImage.keyId = testKeyId; + testImage.sigNvmId = testSigNvmId; + testImage.verifyMethod = _ImgMgrSnapshotVerifyMethod; + testImage.verifyAction = wh_Server_ImgMgrVerifyActionDefault; + + imgMgrConfig.images = &testImage; + imgMgrConfig.imageCount = 1; + imgMgrConfig.server = server; + + ret = wh_Server_Init(server, serverCfg); + if (ret != WH_ERROR_OK) { + WH_ERROR_PRINT("Failed to initialize server: %d\n", ret); + return ret; + } + + ret = wh_Server_ImgMgrInit(&imgMgr, &imgMgrConfig); + if (ret != WH_ERROR_OK) { + WH_ERROR_PRINT("Failed to initialize image manager: %d\n", ret); + wh_Server_Cleanup(server); + return ret; + } + + keyMeta.id = testKeyId; + keyMeta.access = WH_NVM_ACCESS_ANY; + keyMeta.flags = WH_NVM_FLAGS_NONE; + keyMeta.len = sizeof(testSnapshotKey); + snprintf((char*)keyMeta.label, WH_NVM_LABEL_LEN, "TestSnapshotKey"); + + ret = + wh_Server_KeystoreCacheKey(server, &keyMeta, (uint8_t*)testSnapshotKey); + if (ret != WH_ERROR_OK) { + WH_ERROR_PRINT("Failed to cache snapshot key: %d\n", ret); + wh_Server_Cleanup(server); + return ret; + } + + ret = wh_Server_ImgMgrVerifyImg(&imgMgr, &testImage, &result); + if (ret != WH_ERROR_OK) { + WH_ERROR_PRINT("Snapshot image verification failed: %d\n", ret); + wh_Server_Cleanup(server); + return ret; + } + + if (result.verifyMethodResult != WH_ERROR_OK) { + WH_ERROR_PRINT("Snapshot verify method failed: %d\n", + result.verifyMethodResult); + wh_Server_Cleanup(server); + return result.verifyMethodResult; + } + + /* Drop the substitute key the callback left behind */ + (void)wh_Server_KeystoreEvictKey(server, testKeyId); + + ret = wh_Nvm_DestroyObjects(serverCfg->nvm, 1, &sigMeta.id); + if (ret != WH_ERROR_OK) { + WH_ERROR_PRINT("Failed to delete snapshot signature object: %d\n", ret); + wh_Server_Cleanup(server); + return ret; + } + + wh_Server_Cleanup(server); + + WH_TEST_PRINT("IMG_MGR key snapshot Test completed successfully!\n"); + return 0; +} + int whTest_ServerImgMgr(whTestNvmBackendType nvmType) { int rc = 0; @@ -1489,6 +1638,14 @@ int whTest_ServerImgMgr(whTestNvmBackendType nvmType) return rc; } + /* Verify key material is snapshotted before the verify callback runs */ + rc = whTest_ServerImgMgrKeySnapshot(s_conf); + if (rc != 0) { + WH_ERROR_PRINT("Image manager key snapshot test failed: %d\n", rc); + wh_Nvm_Cleanup(nvm); + return rc; + } + /* Run image manager server config tests for each built-in verify method */ #ifdef HAVE_ECC diff --git a/wolfhsm/wh_settings.h b/wolfhsm/wh_settings.h index 8537b9e6c..3f072d7f6 100644 --- a/wolfhsm/wh_settings.h +++ b/wolfhsm/wh_settings.h @@ -113,6 +113,9 @@ * WOLFHSM_CFG_SERVER_IMG_MGR_MAX_SIG_SIZE - Maximum signature size for image * verification Default: 512 bytes (RSA4096) * + * WOLFHSM_CFG_SERVER_IMG_MGR_MAX_KEY_SIZE - Maximum verification key size for + * image verification Default: 1200 bytes (ASN.1 RSA4096 public key) + * * WOLFHSM_CFG_DMA_CUSTOM_CLIENT_COPY - if defined, allows to setup a custom * callback to handle client to server and/or server to client memory copy * operation in DMA requests. @@ -306,6 +309,12 @@ #define WOLFHSM_CFG_SERVER_IMG_MGR_MAX_SIG_SIZE 512 #endif +/* Image manager maximum verification key size. Sized to hold an ASN.1 RSA4096 + * public key. Raise it for larger keys such as ML-DSA. */ +#ifndef WOLFHSM_CFG_SERVER_IMG_MGR_MAX_KEY_SIZE +#define WOLFHSM_CFG_SERVER_IMG_MGR_MAX_KEY_SIZE 1200 +#endif + /* WOLFHSM_CFG_CUSTOMCB_LEN - Maximum size of a customcb message. * Default: 256 */ #ifndef WOLFHSM_CFG_CUSTOMCB_LEN