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