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); } } diff --git a/src/wh_server_cert.c b/src/wh_server_cert.c index db0871605..5664677ff 100644 --- a/src/wh_server_cert.c +++ b/src/wh_server_cert.c @@ -1471,40 +1471,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 @@ -1515,15 +1510,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/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/src/wh_server_crypto.c b/src/wh_server_crypto.c index 5755676c3..802d13c64 100644 --- a/src/wh_server_crypto.c +++ b/src/wh_server_crypto.c @@ -4134,6 +4134,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 */ @@ -8275,6 +8279,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 */ diff --git a/test/wh_test_cert.c b/test/wh_test_cert.c index fc92d5813..1d36036d8 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" @@ -1657,6 +1659,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, + &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( diff --git a/test/wh_test_clientserver.c b/test/wh_test_clientserver.c index 56caebc70..85824d2b4 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, 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, diff --git a/test/wh_test_crypto.c b/test/wh_test_crypto.c index a5850cde5..951002e22 100644 --- a/test/wh_test_crypto.c +++ b/test/wh_test_crypto.c @@ -14783,6 +14783,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"); @@ -14856,6 +14910,223 @@ 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 */ + +#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"); diff --git a/test/wh_test_she.c b/test/wh_test_she.c index b15deddd4..f6448b939 100644 --- a/test/wh_test_she.c +++ b/test/wh_test_she.c @@ -405,6 +405,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;