From 2b3e51af4803e5bbf06d72994756bd3efe29b95b Mon Sep 17 00:00:00 2001 From: Sameeh Jubran Date: Thu, 26 Feb 2026 17:31:05 +0200 Subject: [PATCH] Enforce server-side client identity assignment in CommInit handshake The client_id is now configured in whCommServerConfig and assigned by the server during CommInit rather than being taken from the request. This ensures the server is the authority on client identity. Changes: - Add client_id field to whCommServerConfig - Add expected_client_id to whCommServer context - Server uses configured identity during CommInit - Update all test, benchmark, and example configs Signed-off-by: Sameeh Jubran --- benchmark/wh_bench.c | 3 + docs/src-ja/chapter03.md | 1 + docs/src/chapter03.md | 1 + .../wh_posix_server/wh_posix_server_cfg.c | 4 + src/wh_comm.c | 1 + src/wh_server.c | 27 +++++-- test/wh_test_cert.c | 1 + test/wh_test_clientserver.c | 78 +++++++++++++++++++ test/wh_test_comm.c | 4 + test/wh_test_crypto.c | 9 ++- test/wh_test_log.c | 1 + test/wh_test_multiclient.c | 2 + test/wh_test_posix_threadsafe_stress.c | 1 + test/wh_test_server_img_mgr.c | 1 + test/wh_test_she.c | 1 + test/wh_test_wolfcrypt_test.c | 1 + tools/whnvmtool/test/test_whnvmtool.c | 1 + tools/whnvmtool/whnvmtool.c | 1 + wolfhsm/wh_comm.h | 5 +- 19 files changed, 135 insertions(+), 8 deletions(-) diff --git a/benchmark/wh_bench.c b/benchmark/wh_bench.c index 1fb979360..d0e0cbb9c 100644 --- a/benchmark/wh_bench.c +++ b/benchmark/wh_bench.c @@ -863,6 +863,7 @@ static whCommServerConfig g_mem_cs_conf = { .transport_context = (void*)&g_mem_tmsc, .transport_config = (void*)&g_mem_tmcf, .server_id = 124, + .client_id = WH_BENCH_CLIENT_ID, }; /* Helper function to configure client transport based on type */ @@ -981,6 +982,7 @@ static int _configureServerTransport(whBenchTransportType transport, .transport_context = (void*)&tscShm, .transport_config = (void*)&myshmconfig, .server_id = 57, + .client_id = WH_BENCH_CLIENT_ID, }; memset(&tscShm, 0, sizeof(posixTransportShmServerContext)); @@ -1001,6 +1003,7 @@ static int _configureServerTransport(whBenchTransportType transport, .transport_context = (void*)&tscTcp, .transport_config = (void*)&mytcpconfig, .server_id = 57, + .client_id = WH_BENCH_CLIENT_ID, }; memset(&tscTcp, 0, sizeof(posixTransportTcpServerContext)); diff --git a/docs/src-ja/chapter03.md b/docs/src-ja/chapter03.md index 04daad7b8..d5ec4b1ff 100644 --- a/docs/src-ja/chapter03.md +++ b/docs/src-ja/chapter03.md @@ -132,6 +132,7 @@ whCommServerConfig commServerCfg = { .transport_context = (void*)transportMemServerCtx, .transport_config = (void*)transportMemCfg, .server_id = 456, /* 一意のサーバー識別子 */ + .client_id = 123, /* 接続するクライアントに割り当てるID */ }; /* サーバーNVMコンテキストの初期化 */ diff --git a/docs/src/chapter03.md b/docs/src/chapter03.md index 38de120d3..bc3a41473 100644 --- a/docs/src/chapter03.md +++ b/docs/src/chapter03.md @@ -130,6 +130,7 @@ whCommServerConfig commServerCfg = { .transport_context = (void*)transportMemServerCtx, .transport_config = (void*)transportMemCfg, .server_id = 456, /* unique server identifier */ + .client_id = 123, /* identity assigned to the connecting client */ }; /* Initialize the server NVM context */ diff --git a/examples/posix/wh_posix_server/wh_posix_server_cfg.c b/examples/posix/wh_posix_server/wh_posix_server_cfg.c index 754a0b821..41ae84b8a 100644 --- a/examples/posix/wh_posix_server/wh_posix_server_cfg.c +++ b/examples/posix/wh_posix_server/wh_posix_server_cfg.c @@ -62,6 +62,7 @@ int wh_PosixServer_ExampleShmDmaConfig(void* conf) s_comm.transport_context = (void*)&tscDma; s_comm.transport_config = (void*)&shmConfig; s_comm.server_id = WH_POSIX_SERVER_ID; + s_comm.client_id = WH_POSIX_CLIENT_ID; s_conf->dmaConfig = &dmaConfig; s_conf->comm_config = &s_comm; @@ -89,6 +90,7 @@ int wh_PosixServer_ExampleShmConfig(void* conf) s_comm.transport_context = (void*)&tscShm; s_comm.transport_config = (void*)&shmConfig; s_comm.server_id = WH_POSIX_SERVER_ID; + s_comm.client_id = WH_POSIX_CLIENT_ID; s_conf->comm_config = &s_comm; @@ -112,6 +114,7 @@ int wh_PosixServer_ExampleTcpConfig(void* conf) s_comm.transport_context = (void*)&tscTcp; s_comm.transport_config = (void*)&tcpConfig; s_comm.server_id = WH_POSIX_SERVER_ID; + s_comm.client_id = WH_POSIX_CLIENT_ID; s_conf->comm_config = &s_comm; @@ -176,6 +179,7 @@ int wh_PosixServer_ExampleTlsConfig(void* ctx) s_comm.transport_context = (void*)&tscTls; s_comm.transport_config = (void*)&tlsConfig; s_comm.server_id = WH_POSIX_SERVER_ID; + s_comm.client_id = WH_POSIX_CLIENT_ID; s_conf->comm_config = &s_comm; diff --git a/src/wh_comm.c b/src/wh_comm.c index b8e4f6474..93ba3bec3 100644 --- a/src/wh_comm.c +++ b/src/wh_comm.c @@ -229,6 +229,7 @@ int wh_CommServer_Init(whCommServer* context, const whCommServerConfig* config, context->transport_context = config->transport_context; context->transport_cb = config->transport_cb; context->server_id = config->server_id; + context->expected_client_id = config->client_id; if (context->transport_cb->Init != NULL) { rc = context->transport_cb->Init(context->transport_context, diff --git a/src/wh_server.c b/src/wh_server.c index 5098a2da4..45e83c9c4 100644 --- a/src/wh_server.c +++ b/src/wh_server.c @@ -100,6 +100,14 @@ int wh_Server_Init(whServerContext* server, whServerConfig* config) } #endif /* WOLFHSM_CFG_LOGGING */ +#ifdef WOLFHSM_CFG_GLOBAL_KEYS + if (config->comm_config != NULL && + config->comm_config->client_id == WH_KEYUSER_GLOBAL) { + (void)wh_Server_Cleanup(server); + return WH_ERROR_BADARGS; + } +#endif + rc = wh_CommServer_Init(server->comm, config->comm_config, wh_Server_SetConnectedCb, (void*)server); if (rc != 0) { @@ -189,23 +197,32 @@ static int _wh_Server_HandleCommRequest(whServerContext* server, wh_MessageComm_TranslateInitRequest(magic, (whMessageCommInitRequest*)req_packet, &req); + /* Use server-assigned identity from config rather than trusting + * the client-supplied value. This prevents a malicious client from + * claiming another client's identity to access its keys. */ + server->comm->client_id = server->comm->expected_client_id; + #ifdef WOLFHSM_CFG_GLOBAL_KEYS /* USER=0 is reserved for global keys, client_id must be non-zero */ - if (req.client_id == WH_KEYUSER_GLOBAL) { + if (server->comm->client_id == WH_KEYUSER_GLOBAL) { *out_resp_size = 0; return WH_ERROR_BADARGS; } #endif - /* Process the init action */ - server->comm->client_id = req.client_id; + if (req.client_id != server->comm->client_id) { + WH_LOG_F(&server->log, WH_LOG_LEVEL_SECEVENT, + "CommInit: client claimed id=0x%08X, " + "server assigned id=0x%08X", + req.client_id, server->comm->client_id); + } resp.client_id = server->comm->client_id; resp.server_id = server->comm->server_id; WH_LOG_F(&server->log, WH_LOG_LEVEL_INFO, - "CommInit: client_id=0x%08X, server_id=0x%08X", req.client_id, - resp.server_id); + "CommInit: client_id=0x%08X, server_id=0x%08X", + server->comm->client_id, resp.server_id); /* Convert the response struct */ wh_MessageComm_TranslateInitResponse(magic, diff --git a/test/wh_test_cert.c b/test/wh_test_cert.c index 04250c77c..b6baf448b 100644 --- a/test/wh_test_cert.c +++ b/test/wh_test_cert.c @@ -604,6 +604,7 @@ int whTest_CertRamSim(whTestNvmBackendType nvmType) .transport_context = (void*)tmsc, .transport_config = (void*)tmcf, .server_id = 124, + .client_id = WH_TEST_DEFAULT_CLIENT_ID, }}; /* RamSim Flash state and configuration */ uint8_t memory[FLASH_RAM_SIZE] = {0}; diff --git a/test/wh_test_clientserver.c b/test/wh_test_clientserver.c index 56107adb7..02898b5e7 100644 --- a/test/wh_test_clientserver.c +++ b/test/wh_test_clientserver.c @@ -601,6 +601,7 @@ int whTest_ClientServerSequential(whTestNvmBackendType nvmType) .transport_context = (void*)tmsc, .transport_config = (void*)tmcf, .server_id = 124, + .client_id = WH_TEST_DEFAULT_CLIENT_ID, }}; /* RamSim Flash state and configuration */ @@ -1590,6 +1591,7 @@ static int wh_ClientServer_MemThreadTest(whTestNvmBackendType nvmType) .transport_context = (void*)tmsc, .transport_config = (void*)tmcf, .server_id = 124, + .client_id = WH_TEST_DEFAULT_CLIENT_ID, }}; /* RamSim Flash state and configuration */ @@ -1675,6 +1677,7 @@ static int wh_ClientServer_PosixMemMapThreadTest(whTestNvmBackendType nvmType) .transport_context = (void*)tmsc, .transport_config = (void*)tmcf, .server_id = 124, + .client_id = WH_TEST_DEFAULT_CLIENT_ID, }}; /* RamSim Flash state and configuration */ @@ -1732,8 +1735,83 @@ static int wh_ClientServer_PosixMemMapThreadTest(whTestNvmBackendType nvmType) WOLFHSM_CFG_ENABLE_SERVER */ #if defined(WOLFHSM_CFG_ENABLE_CLIENT) && defined(WOLFHSM_CFG_ENABLE_SERVER) +static int _testCommInitIdentitySpoofing(void) +{ + /* Verify the server ignores client-claimed identity and uses its own + * configured expected_client_id during CommInit (C-8 mitigation). */ + uint8_t req[BUFFER_SIZE]; + uint8_t resp[BUFFER_SIZE]; + whTransportMemConfig tmcf[1] = {{ + .req = (whTransportMemCsr*)req, + .req_size = sizeof(req), + .resp = (whTransportMemCsr*)resp, + .resp_size = sizeof(resp), + }}; + + const uint8_t SPOOFED_CLIENT_ID = 99; + const uint8_t EXPECTED_CLIENT_ID = WH_TEST_DEFAULT_CLIENT_ID; + + /* Client claims a different identity than the server expects */ + whTransportClientCb tccb[1] = {WH_TRANSPORT_MEM_CLIENT_CB}; + whTransportMemClientContext tmcc[1] = {0}; + whCommClientConfig cc_conf[1] = {{ + .transport_cb = tccb, + .transport_context = (void*)tmcc, + .transport_config = (void*)tmcf, + .client_id = SPOOFED_CLIENT_ID, + }}; + whClientContext client[1] = {0}; + whClientConfig c_conf[1] = {{ + .comm = cc_conf, + }}; + + /* Server is configured with the legitimate client identity */ + whTransportServerCb tscb[1] = {WH_TRANSPORT_MEM_SERVER_CB}; + whTransportMemServerContext tmsc[1] = {0}; + whCommServerConfig cs_conf[1] = {{ + .transport_cb = tscb, + .transport_context = (void*)tmsc, + .transport_config = (void*)tmcf, + .server_id = 124, + .client_id = EXPECTED_CLIENT_ID, + }}; + whServerConfig s_conf[1] = {{ + .comm_config = cs_conf, + }}; + whServerContext server[1] = {0}; + + uint32_t resp_client_id = 0; + uint32_t resp_server_id = 0; + + WH_TEST_RETURN_ON_FAIL(wh_Server_Init(server, s_conf)); + WH_TEST_RETURN_ON_FAIL(wh_Client_Init(client, c_conf)); + WH_TEST_RETURN_ON_FAIL( + wh_Server_SetConnected(server, WH_COMM_CONNECTED)); + + /* Client sends CommInit claiming SPOOFED_CLIENT_ID */ + WH_TEST_RETURN_ON_FAIL(wh_Client_CommInitRequest(client)); + WH_TEST_RETURN_ON_FAIL(wh_Server_HandleRequestMessage(server)); + WH_TEST_RETURN_ON_FAIL( + wh_Client_CommInitResponse(client, &resp_client_id, &resp_server_id)); + + /* Server must use its configured identity, not the spoofed one */ + WH_TEST_ASSERT_RETURN(server->comm->client_id == EXPECTED_CLIENT_ID); + WH_TEST_ASSERT_RETURN(resp_client_id == EXPECTED_CLIENT_ID); + WH_TEST_ASSERT_RETURN(resp_client_id != SPOOFED_CLIENT_ID); + + WH_TEST_RETURN_ON_FAIL(wh_Client_CommCloseRequest(client)); + WH_TEST_RETURN_ON_FAIL(wh_Server_HandleRequestMessage(server)); + + wh_Client_Cleanup(client); + wh_Server_Cleanup(server); + + return WH_ERROR_OK; +} + int whTest_ClientServer(void) { + WH_TEST_PRINT("Testing CommInit identity spoofing prevention...\n"); + WH_TEST_ASSERT(0 == _testCommInitIdentitySpoofing()); WH_TEST_PRINT("Testing client/server sequential: mem...\n"); WH_TEST_ASSERT(0 == whTest_ClientServerSequential(WH_NVM_TEST_BACKEND_FLASH)); diff --git a/test/wh_test_comm.c b/test/wh_test_comm.c index 1a6f5be64..d479921a6 100644 --- a/test/wh_test_comm.c +++ b/test/wh_test_comm.c @@ -94,6 +94,7 @@ int whTest_CommMem(void) .transport_context = (void*)tmsc, .transport_config = (void*)tmcf, .server_id = 124, + .client_id = WH_TEST_DEFAULT_CLIENT_ID, }}; whCommServer server[1] = {0}; @@ -396,6 +397,7 @@ void wh_CommClientServer_MemThreadTest(void) .transport_context = (void*)css, .transport_config = (void*)tmcf, .server_id = 0xF, + .client_id = WH_TEST_DEFAULT_CLIENT_ID, }}; _whCommClientServerThreadTest(c_conf, s_conf); @@ -435,6 +437,7 @@ void wh_CommClientServer_ShMemThreadTest(void) .transport_context = (void*)css, .transport_config = (void*)tmcf, .server_id = 0xF, + .client_id = WH_TEST_DEFAULT_CLIENT_ID, }}; _whCommClientServerThreadTest(c_conf, s_conf); @@ -467,6 +470,7 @@ void wh_CommClientServer_TcpThreadTest(void) .transport_context = (void*)tss, .transport_config = (void*)mytcpconfig, .server_id = 0xF, + .client_id = WH_TEST_DEFAULT_CLIENT_ID, }}; _whCommClientServerThreadTest(c_conf, s_conf); diff --git a/test/wh_test_crypto.c b/test/wh_test_crypto.c index 223809273..2b0197885 100644 --- a/test/wh_test_crypto.c +++ b/test/wh_test_crypto.c @@ -5751,10 +5751,14 @@ int whTest_CryptoServerConfig(whServerConfig* config) #ifdef WOLFHSM_CFG_IS_TEST_SERVER /* keep alive for 2 user changes */ if (am_connected != WH_COMM_CONNECTED && userChange < 2) { - if (userChange == 0) + if (userChange == 0) { server->comm->client_id = ALT_CLIENT_ID; - else if (userChange == 1) + server->comm->expected_client_id = ALT_CLIENT_ID; + } + else if (userChange == 1) { server->comm->client_id = WH_TEST_DEFAULT_CLIENT_ID; + server->comm->expected_client_id = WH_TEST_DEFAULT_CLIENT_ID; + } userChange++; am_connected = WH_COMM_CONNECTED; WH_TEST_RETURN_ON_FAIL(wh_Server_SetConnected(server, am_connected)); @@ -5865,6 +5869,7 @@ static int wh_ClientServer_MemThreadTest(whTestNvmBackendType nvmType) .transport_context = (void*)tmsc, .transport_config = (void*)tmcf, .server_id = 124, + .client_id = WH_TEST_DEFAULT_CLIENT_ID, }}; /* RamSim Flash state and configuration */ diff --git a/test/wh_test_log.c b/test/wh_test_log.c index e58a4ea68..84076be1b 100644 --- a/test/wh_test_log.c +++ b/test/wh_test_log.c @@ -1466,6 +1466,7 @@ static int whTest_LogClientServerMemTransport(void) .transport_context = (void*)tmsc, .transport_config = (void*)tmcf, .server_id = 124, + .client_id = WH_TEST_DEFAULT_CLIENT_ID, }}; uint8_t memory[WH_LOG_TEST_FLASH_RAM_SIZE] = {0}; diff --git a/test/wh_test_multiclient.c b/test/wh_test_multiclient.c index 54114cf66..9ef2d6271 100644 --- a/test/wh_test_multiclient.c +++ b/test/wh_test_multiclient.c @@ -1514,6 +1514,7 @@ static int whTest_MultiClientSequential(void) .transport_context = (void*)tmsc1, .transport_config = (void*)tmcf1, .server_id = 101, + .client_id = WH_TEST_DEFAULT_CLIENT_ID, }}; whServerConfig s_conf1[1] = {{ .comm_config = cs_conf1, @@ -1532,6 +1533,7 @@ static int whTest_MultiClientSequential(void) .transport_context = (void*)tmsc2, .transport_config = (void*)tmcf2, .server_id = 102, + .client_id = WH_TEST_DEFAULT_CLIENT_ID + 1, }}; whServerConfig s_conf2[1] = {{ .comm_config = cs_conf2, diff --git a/test/wh_test_posix_threadsafe_stress.c b/test/wh_test_posix_threadsafe_stress.c index fb4c6a0fc..bfa7d06ff 100644 --- a/test/wh_test_posix_threadsafe_stress.c +++ b/test/wh_test_posix_threadsafe_stress.c @@ -640,6 +640,7 @@ static int initClientServerPair(StressTestContext* ctx, int pairIndex) pair->serverCommConfig.transport_context = &pair->serverTransportCtx; pair->serverCommConfig.transport_config = &pair->tmConfig; pair->serverCommConfig.server_id = (uint16_t)(200 + pairIndex); + pair->serverCommConfig.client_id = (uint8_t)(100 + pairIndex); /* Configure crypto context */ pair->cryptoCtx.devId = INVALID_DEVID; diff --git a/test/wh_test_server_img_mgr.c b/test/wh_test_server_img_mgr.c index dbd4e1484..675e75489 100644 --- a/test/wh_test_server_img_mgr.c +++ b/test/wh_test_server_img_mgr.c @@ -1215,6 +1215,7 @@ int whTest_ServerImgMgr(whTestNvmBackendType nvmType) .transport_context = (void*)tmsc, .transport_config = (void*)tmcf, .server_id = 124, + .client_id = WH_TEST_DEFAULT_CLIENT_ID, }}; /* RamSim Flash state and configuration */ diff --git a/test/wh_test_she.c b/test/wh_test_she.c index ea918fcd9..3c78930d3 100644 --- a/test/wh_test_she.c +++ b/test/wh_test_she.c @@ -532,6 +532,7 @@ static int wh_ClientServer_MemThreadTest(void) .transport_context = (void*)tmsc, .transport_config = (void*)tmcf, .server_id = 124, + .client_id = WH_TEST_DEFAULT_CLIENT_ID, }}; /* RamSim Flash state and configuration */ diff --git a/test/wh_test_wolfcrypt_test.c b/test/wh_test_wolfcrypt_test.c index fbe74d58c..2267dbaf7 100644 --- a/test/wh_test_wolfcrypt_test.c +++ b/test/wh_test_wolfcrypt_test.c @@ -209,6 +209,7 @@ static int wh_ClientServer_MemThreadTest(void) .transport_context = (void*)tmsc, .transport_config = (void*)tmcf, .server_id = 124, + .client_id = WH_TEST_DEFAULT_CLIENT_ID, }}; /* RamSim Flash state and configuration */ diff --git a/tools/whnvmtool/test/test_whnvmtool.c b/tools/whnvmtool/test/test_whnvmtool.c index 264f80381..03c30c132 100644 --- a/tools/whnvmtool/test/test_whnvmtool.c +++ b/tools/whnvmtool/test/test_whnvmtool.c @@ -53,6 +53,7 @@ whCommServerConfig gCommServerConfig[1] = {{ .transport_context = (void*)gTransportServerContext, .transport_config = (void*)gTransportTcpConfig, .server_id = 34, + .client_id = 1, }}; diff --git a/tools/whnvmtool/whnvmtool.c b/tools/whnvmtool/whnvmtool.c index ec8012dbf..fb69f5005 100644 --- a/tools/whnvmtool/whnvmtool.c +++ b/tools/whnvmtool/whnvmtool.c @@ -528,6 +528,7 @@ int main(int argc, char* argv[]) .transport_context = (void*)gTransportServerContext, .transport_config = (void*)gTransportTcpConfig, .server_id = 34, + .client_id = 1, }}; /* POSIX flash file NVM configuration */ diff --git a/wolfhsm/wh_comm.h b/wolfhsm/wh_comm.h index 4b77d58f7..5d23859ea 100644 --- a/wolfhsm/wh_comm.h +++ b/wolfhsm/wh_comm.h @@ -253,7 +253,8 @@ typedef struct { const whTransportServerCb* transport_cb; const void* transport_config; uint8_t server_id; - uint8_t WH_PAD[7]; + uint8_t client_id; + uint8_t WH_PAD[6]; } whCommServerConfig; /* Context structure for a server. Note the client context will track the @@ -270,6 +271,8 @@ typedef struct { uint16_t reqid; uint8_t client_id; uint8_t server_id; + uint8_t expected_client_id; + uint8_t WH_PAD[7]; } whCommServer; /* Reset the state of the server context and begin the connection to a client