From 2a771f480b867f76a8e7d4b182b83730be065c57 Mon Sep 17 00:00:00 2001 From: Brett Nicholas <7547222+bigbrett@users.noreply.github.com> Date: Tue, 3 Nov 2026 08:52:51 -0700 Subject: [PATCH] add server-side expected client ID check and callback to enforce additional binding constraints --- docs/src/3-Quickstart.md | 3 +- docs/src/5-Features.md | 14 +- src/wh_comm.c | 1 + src/wh_server.c | 36 ++- .../misc/wh_test_client_id_binding.c | 263 ++++++++++++++++++ test-refactor/wh_test_list.c | 2 + test/wh_test_clientserver.c | 206 ++++++++++++++ wolfhsm/wh_comm.h | 7 +- wolfhsm/wh_server.h | 15 + 9 files changed, 542 insertions(+), 5 deletions(-) create mode 100644 test-refactor/misc/wh_test_client_id_binding.c diff --git a/docs/src/3-Quickstart.md b/docs/src/3-Quickstart.md index a2768ae6c..aa96e112e 100644 --- a/docs/src/3-Quickstart.md +++ b/docs/src/3-Quickstart.md @@ -145,6 +145,7 @@ int main(void) .transport_context = (void*)transportCtx, .transport_config = (void*)transportCfg, .server_id = 1, /* server identifier */ + .client_id = 1, /* only client 1 may use this channel */ }}; /* 3. Build the NVM context on top of the NVM-flash layer and flash driver */ @@ -222,7 +223,7 @@ The transport is responsible for moving raw bytes between the client and the ser The communication layer sits directly above the transport and implements the request/response protocol: message framing, sequence numbers, and endianness handling. The server binds its transport into a `whCommServerConfig`, providing the callback table, the transport context, the transport config, and a `server_id`. The client's `whCommClientConfig` is analogous but carries a `client_id` instead. -The `client_id` is significant: the server learns each client's identifier during the connection handshake, and uses it to keep per-client key caches and other resources isolated from one another. The request/response protocol these structures drive is the **split-transaction, non-blocking** model described in [Client/Server Communication](4-Architecture.md#clientserver-communication). +The `client_id` is significant: the client sends it during the connection handshake, and the server uses it to keep per-client key caches and other resources isolated from one another. Setting `client_id` in the server's `whCommServerConfig` as well binds the channel to that client, and the server refuses a handshake that asks for any other ID. Leave it at 0 if you don't know in advance which client will use the channel. [Client Identity](5-Features.md#client-identity) explains how the server can still check the ID in that case. The request/response protocol these structures drive is the **split-transaction, non-blocking** model described in [Client/Server Communication](4-Architecture.md#clientserver-communication). ### NVM Configuration diff --git a/docs/src/5-Features.md b/docs/src/5-Features.md index ee50ce5e7..29699cc14 100644 --- a/docs/src/5-Features.md +++ b/docs/src/5-Features.md @@ -34,6 +34,7 @@ This chapter provides a detailed overview of the high level features that wolfHS - [Attribute Certificate (Acert) Support](#attribute-certificate-acert-support) - [Communication Layer and Transports](#communication-layer-and-transports) - [Communication Layer](#communication-layer) + - [Client Identity](#client-identity) - [Transport Backends](#transport-backends) - [DMA Support](#dma-support) - [DMA Dispatch Mode (`wh_Client_SetDmaMode`)](#dma-dispatch-mode-wh_client_setdmamode) @@ -331,7 +332,7 @@ The server-side `whKeyId` packs three fields into its 16 bits: - **USER** (middle 4 bits): the owning client. Value `0` is reserved for the global-key namespace when `WOLFHSM_CFG_GLOBAL_KEYS` is enabled. - **ID** (low 8 bits): the number the client chose. -The USER field is what gives each client its own private key space. The server fills it with the connection's client id (assigned at init and checked against `WH_CLIENT_ID_MAX`), so when two clients both use "key 5" they map to different `whKeyId` values and cannot touch each other's key. Every client sees the same `[1, 255]` range, and the ranges never overlap. +The USER field is what gives each client its own private key space. The server fills it with the connection's client id (bound at COMM INIT and checked against `WH_CLIENT_ID_MAX`), so when two clients both use "key 5" they map to different `whKeyId` values and cannot touch each other's key. Every client sees the same `[1, 255]` range, and the ranges never overlap. This only holds against a client that requests another client's ID when the server binds client IDs to connections, as described in [Client Identity](#client-identity). Turning to the cache itself: it is statically allocated inside the server context (wolfHSM uses no dynamic memory) and sized at build time by two pairs of macros: @@ -607,6 +608,17 @@ The comm layer itself is **stateless** in the protocol sense: each request/respo The comm layer is also **non-blocking and split-transaction** end-to-end: `wh_CommClient_SendRequest` and `wh_CommClient_RecvResponse` return `WH_ERROR_NOTREADY` rather than waiting on the transport, and the same convention is propagated up through every higher-level client API. Transport errors below this layer surface as either `WH_ERROR_NOTREADY` (retry) or `WH_ERROR_ABORTED` (fatal — clean up and reinitialize the connection), and the higher-level APIs distinguish between the two so applications can implement appropriate retry or recovery logic. See [Client/Server Communication](4-Architecture.md#clientserver-communication) for the broader request/response model that this layer implements. +### Client Identity + +The client ID bound at COMM INIT selects the per-client namespace for keys, NVM objects, counters, certificates, and SHE slots, so the way the server binds that ID determines how well clients are kept apart. The client sends its `client_id` in COMM INIT. By default the server checks that it is in the range 1 to `WH_CLIENT_ID_MAX` and accepts it, which means any client that can reach a channel can ask for any client ID. + +Two server-side controls bind the client ID to the connection instead. They can be used on their own or together. + +- **Assigned client ID**: set `client_id` in the server's `whCommServerConfig` to the client that channel serves. The server then refuses a COMM INIT that requests any other ID, including a second COMM INIT later in the same session. This fits transports where each channel belongs to one client, such as a protected shared memory region per core. If you don't know in advance which client will use a channel, for example on a TLS listener that serves several clients, leave `client_id` at 0. The server then skips this check and leaves the decision to the callback below. +- **COMM INIT callback**: set `commInitCb` in `whServerConfig`. The server calls it on every COMM INIT after its own checks, passing the requested client ID and `commInitCbArg`, and refuses the request if it returns anything other than `WH_ERROR_OK`. Use it when the transport knows who is connected, for example to accept only the client ID that belongs to an authenticated TLS peer. When the channel also has an assigned client ID, the callback only sees requests for that ID. + +A refused COMM INIT leaves the connection's current binding unchanged, and the client sees `WH_ERROR_ABORTED`. If `client_id` is 0 and no callback is set, the server accepts any client ID in range. + ### Transport Backends The transport interface — `whTransportClientCb` on the client side and `whTransportServerCb` on the server side — is a four-function callback table (`Init`, `Send`, `Recv`, `Cleanup`) that delivers complete, MTU-sized packets between client and server. The comm layer speaks only through this interface, so wolfHSM is integrated into a new platform by supplying a transport implementation rather than by modifying the core library. Transports are expected to deliver packets reliably and in order, one at a time and up to `WH_COMM_MTU` bytes; framing, sequencing, dispatch, and policy enforcement are all handled above. diff --git a/src/wh_comm.c b/src/wh_comm.c index d595e9b8d..e8574bd2f 100644 --- a/src/wh_comm.c +++ b/src/wh_comm.c @@ -346,6 +346,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->assigned_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 2e88f4f2b..b2296ed9f 100644 --- a/src/wh_server.c +++ b/src/wh_server.c @@ -85,8 +85,16 @@ int wh_Server_Init(whServerContext* server, whServerConfig* config) return WH_ERROR_BADARGS; } + /* An assigned client ID must be one a client is allowed to bind */ + if ((config->comm_config != NULL) && + (config->comm_config->client_id > WH_CLIENT_ID_MAX)) { + return WH_ERROR_BADARGS; + } + memset(server, 0, sizeof(*server)); - server->nvm = config->nvm; + server->nvm = config->nvm; + server->commInitCb = config->commInitCb; + server->commInitCbArg = config->commInitCbArg; #ifndef WOLFHSM_CFG_NO_CRYPTO server->crypto = config->crypto; @@ -298,6 +306,32 @@ static int _wh_Server_HandleCommRequest(whServerContext* server, return WH_ERROR_BADARGS; } + /* A channel with an assigned client ID only accepts that ID. A + * refused request leaves the current binding unchanged. */ + if ((server->comm->assigned_client_id != 0) && + (req.client_id != server->comm->assigned_client_id)) { + WH_LOG_F(&server->log, WH_LOG_LEVEL_SECEVENT, + "CommInit: refused client_id=0x%08X, channel is assigned " + "client_id=0x%08X", + req.client_id, + (unsigned int)server->comm->assigned_client_id); + *out_resp_size = 0; + return WH_ERROR_ACCESS; + } + + /* Let the server application apply its own rules */ + if (server->commInitCb != NULL) { + rc = server->commInitCb(server, (uint8_t)req.client_id, + server->commInitCbArg); + if (rc != WH_ERROR_OK) { + WH_LOG_F(&server->log, WH_LOG_LEVEL_SECEVENT, + "CommInit: callback refused client_id=0x%08X: %d", + req.client_id, rc); + *out_resp_size = 0; + return rc; + } + } + /* Process the init action */ server->comm->client_id = req.client_id; diff --git a/test-refactor/misc/wh_test_client_id_binding.c b/test-refactor/misc/wh_test_client_id_binding.c new file mode 100644 index 000000000..62f26d4e5 --- /dev/null +++ b/test-refactor/misc/wh_test_client_id_binding.c @@ -0,0 +1,263 @@ +/* + * 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/misc/wh_test_client_id_binding.c + * + * Server-side client ID binding: a client ID assigned to the channel in the + * server comm config, and the server COMM INIT callback. + */ + +#include "wolfhsm/wh_settings.h" + +#if defined(WOLFHSM_CFG_ENABLE_CLIENT) && defined(WOLFHSM_CFG_ENABLE_SERVER) + +#include +#include + +#include "wolfhsm/wh_error.h" +#include "wolfhsm/wh_comm.h" +#include "wolfhsm/wh_keyid.h" +#include "wolfhsm/wh_transport_mem.h" +#include "wolfhsm/wh_client.h" +#include "wolfhsm/wh_server.h" + +#include "wh_test_common.h" +#include "wh_test_list.h" + +enum { + BUFFER_SIZE = sizeof(whTransportMemCsr) + sizeof(whCommHeader) + + WOLFHSM_CFG_COMM_DATA_LEN, +}; + +/* COMM INIT callback state */ +typedef struct { + int calls; + int refuseRc; /* returned for any ID other than allowClientId */ + uint8_t allowClientId; /* only ID accepted, 0 accepts any */ + uint8_t lastClientId; +} whTestCommInitCbState; + +static int _commInitCb(whServerContext* server, uint8_t clientId, void* arg) +{ + whTestCommInitCbState* state = (whTestCommInitCbState*)arg; + + if ((server == NULL) || (state == NULL)) { + return WH_ERROR_BADARGS; + } + state->calls++; + state->lastClientId = clientId; + if ((state->allowClientId != 0) && (clientId != state->allowClientId)) { + return state->refuseRc; + } + return WH_ERROR_OK; +} + +/* Start a connected client and server on the memory transport */ +static int _pairInit(whClientContext* client, whClientConfig* c_conf, + whServerContext* server, whServerConfig* s_conf) +{ + WH_TEST_RETURN_ON_FAIL(wh_Server_Init(server, s_conf)); + WH_TEST_RETURN_ON_FAIL(wh_Server_SetConnected(server, WH_COMM_CONNECTED)); + WH_TEST_RETURN_ON_FAIL(wh_Client_Init(client, c_conf)); + return WH_ERROR_OK; +} + +static int _pairCleanup(whClientContext* client, whServerContext* server) +{ + WH_TEST_RETURN_ON_FAIL(wh_Server_Cleanup(server)); + WH_TEST_RETURN_ON_FAIL(wh_Client_Cleanup(client)); + return WH_ERROR_OK; +} + +/* Drive one COMM INIT with clientId. A refused request gets an empty + * response, which the client reports as WH_ERROR_ABORTED, and must leave the + * server's binding unchanged. */ +static int _commInit(whClientContext* client, whServerContext* server, + uint8_t clientId, int expectOk) +{ + uint32_t respClientId = 0; + uint32_t respServerId = 0; + uint8_t priorId = server->comm->client_id; + int rc; + + client->comm->client_id = clientId; + WH_TEST_RETURN_ON_FAIL(wh_Client_CommInitRequest(client)); + WH_TEST_RETURN_ON_FAIL(wh_Server_HandleRequestMessage(server)); + rc = wh_Client_CommInitResponse(client, &respClientId, &respServerId); + + if (expectOk) { + WH_TEST_ASSERT_RETURN(rc == WH_ERROR_OK); + WH_TEST_ASSERT_RETURN(respClientId == clientId); + WH_TEST_ASSERT_RETURN(server->comm->client_id == clientId); + } + else { + WH_TEST_ASSERT_RETURN(rc == WH_ERROR_ABORTED); + WH_TEST_ASSERT_RETURN(server->comm->client_id == priorId); + } + return WH_ERROR_OK; +} + +/* End the session with COMM CLOSE, then start a new one on the same server + * context */ +static int _reconnect(whClientContext* client, whServerContext* server) +{ + WH_TEST_RETURN_ON_FAIL(wh_Client_CommCloseRequest(client)); + WH_TEST_RETURN_ON_FAIL(wh_Server_HandleRequestMessage(server)); + WH_TEST_RETURN_ON_FAIL(wh_Client_CommCloseResponse(client)); + WH_TEST_ASSERT_RETURN(server->comm->client_id == 0); + WH_TEST_RETURN_ON_FAIL(wh_Server_SetConnected(server, WH_COMM_CONNECTED)); + return WH_ERROR_OK; +} + +static int _whTest_ClientIdBinding(void) +{ + const uint8_t assignedId = WH_TEST_DEFAULT_CLIENT_ID + 1; + const uint8_t otherId = WH_TEST_DEFAULT_CLIENT_ID + 2; + + /* Transport memory configuration */ + static uint8_t req[BUFFER_SIZE]; + static uint8_t resp[BUFFER_SIZE]; + whTransportMemConfig tmcf[1] = {{ + .req = (whTransportMemCsr*)req, + .req_size = sizeof(req), + .resp = (whTransportMemCsr*)resp, + .resp_size = sizeof(resp), + }}; + + /* Client configuration/contexts */ + 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 = WH_TEST_DEFAULT_CLIENT_ID, + }}; + whClientConfig c_conf[1] = {{ + .comm = cc_conf, + }}; + static whClientContext client[1]; + + /* Server configuration/contexts. COMM INIT needs no NVM or crypto. */ + 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, + }}; + whServerConfig s_conf[1] = {{ + .comm_config = cs_conf, + }}; + static whServerContext server[1]; + + whTestCommInitCbState cbState; + + /* An assigned ID that no client can bind is a config error */ + cs_conf->client_id = WH_CLIENT_ID_MAX + 1; + WH_TEST_ASSERT_RETURN(WH_ERROR_BADARGS == wh_Server_Init(server, s_conf)); + + /* The largest client ID is a valid assignment */ + cs_conf->client_id = WH_CLIENT_ID_MAX; + WH_TEST_RETURN_ON_FAIL(_pairInit(client, c_conf, server, s_conf)); + WH_TEST_RETURN_ON_FAIL(_commInit(client, server, WH_CLIENT_ID_MAX, 1)); + WH_TEST_RETURN_ON_FAIL(_pairCleanup(client, server)); + + /* Assigned ID: only that ID can bind, including later in the session */ + cs_conf->client_id = assignedId; + WH_TEST_RETURN_ON_FAIL(_pairInit(client, c_conf, server, s_conf)); + WH_TEST_RETURN_ON_FAIL(_commInit(client, server, otherId, 0)); + /* Still unbound */ + WH_TEST_ASSERT_RETURN(server->comm->client_id == 0); + WH_TEST_RETURN_ON_FAIL(_commInit(client, server, assignedId, 1)); + WH_TEST_RETURN_ON_FAIL(_commInit(client, server, otherId, 0)); + WH_TEST_ASSERT_RETURN(server->comm->client_id == assignedId); + /* The range check still applies */ + WH_TEST_RETURN_ON_FAIL(_commInit(client, server, 0, 0)); + WH_TEST_RETURN_ON_FAIL(_commInit(client, server, assignedId, 1)); + /* The assigned ID still applies after a reconnect */ + WH_TEST_RETURN_ON_FAIL(_reconnect(client, server)); + WH_TEST_RETURN_ON_FAIL(_commInit(client, server, otherId, 0)); + WH_TEST_RETURN_ON_FAIL(_commInit(client, server, assignedId, 1)); + WH_TEST_RETURN_ON_FAIL(_pairCleanup(client, server)); + + /* Callback only: the callback decides which ID can bind */ + memset(&cbState, 0, sizeof(cbState)); + cbState.allowClientId = assignedId; + cbState.refuseRc = WH_ERROR_ACCESS; + cs_conf->client_id = 0; + s_conf->commInitCb = _commInitCb; + s_conf->commInitCbArg = &cbState; + WH_TEST_RETURN_ON_FAIL(_pairInit(client, c_conf, server, s_conf)); + WH_TEST_RETURN_ON_FAIL(_commInit(client, server, assignedId, 1)); + WH_TEST_ASSERT_RETURN(cbState.calls == 1); + WH_TEST_ASSERT_RETURN(cbState.lastClientId == assignedId); + WH_TEST_RETURN_ON_FAIL(_commInit(client, server, otherId, 0)); + WH_TEST_ASSERT_RETURN(cbState.calls == 2); + WH_TEST_ASSERT_RETURN(cbState.lastClientId == otherId); + WH_TEST_ASSERT_RETURN(server->comm->client_id == assignedId); + /* Out of range IDs are refused before the callback runs */ + WH_TEST_RETURN_ON_FAIL( + _commInit(client, server, (uint8_t)(WH_CLIENT_ID_MAX + 1), 0)); + WH_TEST_ASSERT_RETURN(cbState.calls == 2); + /* The callback still runs after a reconnect */ + WH_TEST_RETURN_ON_FAIL(_reconnect(client, server)); + WH_TEST_RETURN_ON_FAIL(_commInit(client, server, otherId, 0)); + WH_TEST_ASSERT_RETURN(cbState.calls == 3); + WH_TEST_RETURN_ON_FAIL(_commInit(client, server, assignedId, 1)); + WH_TEST_ASSERT_RETURN(cbState.calls == 4); + /* Any value other than WH_ERROR_OK refuses, including a positive one */ + cbState.refuseRc = 1; + WH_TEST_RETURN_ON_FAIL(_commInit(client, server, otherId, 0)); + WH_TEST_ASSERT_RETURN(cbState.calls == 5); + WH_TEST_RETURN_ON_FAIL(_pairCleanup(client, server)); + + /* Assigned ID and callback: the assigned ID is checked first, and the + * callback can still refuse the assigned ID */ + memset(&cbState, 0, sizeof(cbState)); + cbState.allowClientId = otherId; + cbState.refuseRc = WH_ERROR_ACCESS; + cs_conf->client_id = assignedId; + WH_TEST_RETURN_ON_FAIL(_pairInit(client, c_conf, server, s_conf)); + WH_TEST_RETURN_ON_FAIL(_commInit(client, server, otherId, 0)); + WH_TEST_ASSERT_RETURN(cbState.calls == 0); + WH_TEST_RETURN_ON_FAIL(_commInit(client, server, assignedId, 0)); + WH_TEST_ASSERT_RETURN(cbState.calls == 1); + cbState.allowClientId = assignedId; + WH_TEST_RETURN_ON_FAIL(_commInit(client, server, assignedId, 1)); + WH_TEST_ASSERT_RETURN(cbState.calls == 2); + WH_TEST_RETURN_ON_FAIL(_pairCleanup(client, server)); + + return WH_ERROR_OK; +} + +int whTest_ClientIdBinding(void* ctx) +{ + (void)ctx; + + WH_TEST_PRINT( + "Testing server-assigned client ID and COMM INIT callback...\n"); + WH_TEST_RETURN_ON_FAIL(_whTest_ClientIdBinding()); + + return WH_ERROR_OK; +} + +#endif /* WOLFHSM_CFG_ENABLE_CLIENT && WOLFHSM_CFG_ENABLE_SERVER */ diff --git a/test-refactor/wh_test_list.c b/test-refactor/wh_test_list.c index 754c0d53f..731fd3000 100644 --- a/test-refactor/wh_test_list.c +++ b/test-refactor/wh_test_list.c @@ -42,6 +42,7 @@ /* Test declarations and weak skip implementations. */ WH_TEST_DECL(whTest_CacheHelpers); WH_TEST_DECL(whTest_ClientDevId); +WH_TEST_DECL(whTest_ClientIdBinding); WH_TEST_DECL(whTest_Comm); WH_TEST_DECL(whTest_CryptoAffinity); WH_TEST_DECL(whTest_CryptoAesGcmDmaAad); @@ -119,6 +120,7 @@ WH_TEST_DECL(whTest_AuthRequestAuthorization); const whTestCase whTestsMisc[] = { {"whTest_CacheHelpers", whTest_CacheHelpers}, {"whTest_ClientDevId", whTest_ClientDevId}, + {"whTest_ClientIdBinding", whTest_ClientIdBinding}, {"whTest_Comm", whTest_Comm}, {"whTest_Dma", whTest_Dma}, {"whTest_DmaNonCoherent", whTest_DmaNonCoherent}, diff --git a/test/wh_test_clientserver.c b/test/wh_test_clientserver.c index 6a8f21e33..06a99e05a 100644 --- a/test/wh_test_clientserver.c +++ b/test/wh_test_clientserver.c @@ -1434,6 +1434,210 @@ int whTest_ClientServerSequential(whTestNvmBackendType nvmType) return ret; } + +/* COMM INIT callback state for the client ID binding test */ +typedef struct { + int calls; + int refuseRc; /* returned for any ID other than allowClientId */ + uint8_t allowClientId; /* only ID accepted, 0 accepts any */ + uint8_t lastClientId; +} whTestCommInitCbState; + +static int _testCommInitCb(whServerContext* server, uint8_t clientId, void* arg) +{ + whTestCommInitCbState* state = (whTestCommInitCbState*)arg; + + if ((server == NULL) || (state == NULL)) { + return WH_ERROR_BADARGS; + } + state->calls++; + state->lastClientId = clientId; + if ((state->allowClientId != 0) && (clientId != state->allowClientId)) { + return state->refuseRc; + } + return WH_ERROR_OK; +} + +/* Start a connected client and server on the memory transport */ +static int _testClientIdPairInit(whClientContext* client, + whClientConfig* c_conf, + whServerContext* server, + whServerConfig* s_conf) +{ + WH_TEST_RETURN_ON_FAIL(wh_Server_Init(server, s_conf)); + WH_TEST_RETURN_ON_FAIL(wh_Server_SetConnected(server, WH_COMM_CONNECTED)); + WH_TEST_RETURN_ON_FAIL(wh_Client_Init(client, c_conf)); + return WH_ERROR_OK; +} + +static int _testClientIdPairCleanup(whClientContext* client, + whServerContext* server) +{ + WH_TEST_RETURN_ON_FAIL(wh_Server_Cleanup(server)); + WH_TEST_RETURN_ON_FAIL(wh_Client_Cleanup(client)); + return WH_ERROR_OK; +} + +/* End the session with COMM CLOSE, then start a new one on the same server + * context */ +static int _testClientIdReconnect(whClientContext* client, + whServerContext* server) +{ + WH_TEST_RETURN_ON_FAIL(wh_Client_CommCloseRequest(client)); + WH_TEST_RETURN_ON_FAIL(wh_Server_HandleRequestMessage(server)); + WH_TEST_RETURN_ON_FAIL(wh_Client_CommCloseResponse(client)); + WH_TEST_ASSERT_RETURN(server->comm->client_id == 0); + WH_TEST_RETURN_ON_FAIL(wh_Server_SetConnected(server, WH_COMM_CONNECTED)); + return WH_ERROR_OK; +} + +/* Server-side client ID binding: a client ID assigned to the channel in the + * server comm config, and the server COMM INIT callback */ +static int _testClientServerClientIdBinding(void) +{ + const uint8_t assignedId = WH_TEST_DEFAULT_CLIENT_ID + 1; + const uint8_t otherId = WH_TEST_DEFAULT_CLIENT_ID + 2; + + /* Transport memory configuration */ + 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), + }}; + + /* Client configuration/contexts */ + 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 = WH_TEST_DEFAULT_CLIENT_ID, + }}; + whClientConfig c_conf[1] = {{ + .comm = cc_conf, + }}; + whClientContext client[1] = {0}; + + /* Server configuration/contexts. COMM INIT needs no NVM or crypto. */ + 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, + }}; + whServerConfig s_conf[1] = {{ + .comm_config = cs_conf, + }}; + whServerContext server[1] = {0}; + + whTestCommInitCbState cbState; + + WH_TEST_PRINT("Testing client/server: server-assigned client ID...\n"); + + /* An assigned ID that no client can bind is a config error */ + cs_conf->client_id = WH_CLIENT_ID_MAX + 1; + WH_TEST_ASSERT_RETURN(WH_ERROR_BADARGS == wh_Server_Init(server, s_conf)); + + /* The largest client ID is a valid assignment */ + cs_conf->client_id = WH_CLIENT_ID_MAX; + WH_TEST_RETURN_ON_FAIL( + _testClientIdPairInit(client, c_conf, server, s_conf)); + WH_TEST_RETURN_ON_FAIL( + _testInitClientIdBoundary(client, server, WH_CLIENT_ID_MAX, 1)); + WH_TEST_RETURN_ON_FAIL(_testClientIdPairCleanup(client, server)); + + /* Assigned ID: only that ID can bind, including later in the session */ + cs_conf->client_id = assignedId; + WH_TEST_RETURN_ON_FAIL( + _testClientIdPairInit(client, c_conf, server, s_conf)); + WH_TEST_RETURN_ON_FAIL( + _testInitClientIdBoundary(client, server, otherId, 0)); + /* Still unbound */ + WH_TEST_ASSERT_RETURN(server->comm->client_id == 0); + WH_TEST_RETURN_ON_FAIL( + _testInitClientIdBoundary(client, server, assignedId, 1)); + WH_TEST_RETURN_ON_FAIL( + _testInitClientIdBoundary(client, server, otherId, 0)); + WH_TEST_ASSERT_RETURN(server->comm->client_id == assignedId); + /* The range check still applies */ + WH_TEST_RETURN_ON_FAIL(_testInitClientIdBoundary(client, server, 0, 0)); + WH_TEST_RETURN_ON_FAIL( + _testInitClientIdBoundary(client, server, assignedId, 1)); + /* The assigned ID still applies after a reconnect */ + WH_TEST_RETURN_ON_FAIL(_testClientIdReconnect(client, server)); + WH_TEST_RETURN_ON_FAIL( + _testInitClientIdBoundary(client, server, otherId, 0)); + WH_TEST_RETURN_ON_FAIL( + _testInitClientIdBoundary(client, server, assignedId, 1)); + WH_TEST_RETURN_ON_FAIL(_testClientIdPairCleanup(client, server)); + + WH_TEST_PRINT("Testing client/server: COMM INIT callback...\n"); + + /* Callback only: the callback decides which ID can bind */ + memset(&cbState, 0, sizeof(cbState)); + cbState.allowClientId = assignedId; + cbState.refuseRc = WH_ERROR_ACCESS; + cs_conf->client_id = 0; + s_conf->commInitCb = _testCommInitCb; + s_conf->commInitCbArg = &cbState; + WH_TEST_RETURN_ON_FAIL( + _testClientIdPairInit(client, c_conf, server, s_conf)); + WH_TEST_RETURN_ON_FAIL( + _testInitClientIdBoundary(client, server, assignedId, 1)); + WH_TEST_ASSERT_RETURN(cbState.calls == 1); + WH_TEST_ASSERT_RETURN(cbState.lastClientId == assignedId); + WH_TEST_RETURN_ON_FAIL( + _testInitClientIdBoundary(client, server, otherId, 0)); + WH_TEST_ASSERT_RETURN(cbState.calls == 2); + WH_TEST_ASSERT_RETURN(cbState.lastClientId == otherId); + WH_TEST_ASSERT_RETURN(server->comm->client_id == assignedId); + /* Out of range IDs are refused before the callback runs */ + WH_TEST_RETURN_ON_FAIL( + _testInitClientIdBoundary(client, server, WH_CLIENT_ID_MAX + 1, 0)); + WH_TEST_ASSERT_RETURN(cbState.calls == 2); + /* The callback still runs after a reconnect */ + WH_TEST_RETURN_ON_FAIL(_testClientIdReconnect(client, server)); + WH_TEST_RETURN_ON_FAIL( + _testInitClientIdBoundary(client, server, otherId, 0)); + WH_TEST_ASSERT_RETURN(cbState.calls == 3); + WH_TEST_RETURN_ON_FAIL( + _testInitClientIdBoundary(client, server, assignedId, 1)); + WH_TEST_ASSERT_RETURN(cbState.calls == 4); + /* Any value other than WH_ERROR_OK refuses, including a positive one */ + cbState.refuseRc = 1; + WH_TEST_RETURN_ON_FAIL( + _testInitClientIdBoundary(client, server, otherId, 0)); + WH_TEST_ASSERT_RETURN(cbState.calls == 5); + WH_TEST_RETURN_ON_FAIL(_testClientIdPairCleanup(client, server)); + + /* Assigned ID and callback: the assigned ID is checked first, and the + * callback can still refuse the assigned ID */ + memset(&cbState, 0, sizeof(cbState)); + cbState.allowClientId = otherId; + cbState.refuseRc = WH_ERROR_ACCESS; + cs_conf->client_id = assignedId; + WH_TEST_RETURN_ON_FAIL( + _testClientIdPairInit(client, c_conf, server, s_conf)); + WH_TEST_RETURN_ON_FAIL( + _testInitClientIdBoundary(client, server, otherId, 0)); + WH_TEST_ASSERT_RETURN(cbState.calls == 0); + WH_TEST_RETURN_ON_FAIL( + _testInitClientIdBoundary(client, server, assignedId, 0)); + WH_TEST_ASSERT_RETURN(cbState.calls == 1); + cbState.allowClientId = assignedId; + WH_TEST_RETURN_ON_FAIL( + _testInitClientIdBoundary(client, server, assignedId, 1)); + WH_TEST_ASSERT_RETURN(cbState.calls == 2); + WH_TEST_RETURN_ON_FAIL(_testClientIdPairCleanup(client, server)); + + return WH_ERROR_OK; +} #endif /* WOLFHSM_CFG_ENABLE_CLIENT && WOLFHSM_CFG_ENABLE_SERVER */ #ifdef WOLFHSM_CFG_ENABLE_CLIENT @@ -2145,6 +2349,8 @@ int whTest_ClientServer(void) WH_TEST_PRINT("Testing client/server sequential: mem...\n"); WH_TEST_ASSERT(0 == whTest_ClientServerSequential(WH_NVM_TEST_BACKEND_FLASH)); + WH_TEST_ASSERT(0 == _testClientServerClientIdBinding()); + #if defined(WOLFHSM_CFG_SERVER_NVM_FLASH_LOG) WH_TEST_PRINT("Testing client/server sequential: mem + flash log...\n"); WH_TEST_ASSERT(0 == diff --git a/wolfhsm/wh_comm.h b/wolfhsm/wh_comm.h index 878ff20d0..bd58597b9 100644 --- a/wolfhsm/wh_comm.h +++ b/wolfhsm/wh_comm.h @@ -289,7 +289,8 @@ typedef struct { const whTransportServerCb* transport_cb; const void* transport_config; uint8_t server_id; - uint8_t WH_PAD[7]; + uint8_t client_id; /* Client ID this channel expects to serve */ + uint8_t WH_PAD[6]; } whCommServerConfig; /* Context structure for a server. Note the client context will track the @@ -304,8 +305,10 @@ typedef struct { uint8_t* data; int initialized; uint16_t reqid; - uint8_t client_id; + uint8_t client_id; /* ID bound by the last accepted COMM INIT */ uint8_t server_id; + uint8_t assigned_client_id; /* expected clientID from config. 0 if unset */ + uint8_t WH_PAD[7]; } whCommServer; /* Reset the state of the server context and begin the connection to a client diff --git a/wolfhsm/wh_server.h b/wolfhsm/wh_server.h index a3da0ef3a..81bab9337 100644 --- a/wolfhsm/wh_server.h +++ b/wolfhsm/wh_server.h @@ -140,6 +140,17 @@ typedef struct { } whServerDmaContext; #endif /* WOLFHSM_CFG_DMA */ +/** Server COMM INIT callback */ + +/* Called on every COMM INIT after the server's own client ID checks, so the + * server application can add its own rules, such as matching the ID to the + * authenticated transport peer. If the channel has an assigned client ID, the + * callback only sees requests for that ID. arg is whServerConfig.commInitCbArg. + * Return WH_ERROR_OK to accept clientId. Any other value refuses the COMM INIT. + */ +typedef int (*whServerCommInitCb)(whServerContext* server, uint8_t clientId, + void* arg); + /** Server config and context */ typedef struct whServerConfig_t { @@ -171,6 +182,8 @@ typedef struct whServerConfig_t { #if defined(WOLFHSM_CFG_CERTIFICATE_MANAGER) && !defined(WOLFHSM_CFG_NO_CRYPTO) whServerCertConfig* certConfig; /* optional; NULL = no verify callback */ #endif /* WOLFHSM_CFG_CERTIFICATE_MANAGER && !WOLFHSM_CFG_NO_CRYPTO */ + whServerCommInitCb commInitCb; /* optional, NULL for no extra checks */ + void* commInitCbArg; /* passed to commInitCb */ } whServerConfig; @@ -184,6 +197,8 @@ struct whServerContext_t { whHwKeystoreContext* hwKeystore; #endif /* WOLFHSM_CFG_HWKEYSTORE */ whCommServer comm[1]; + whServerCommInitCb commInitCb; + void* commInitCbArg; #ifndef WOLFHSM_CFG_NO_CRYPTO whServerCryptoContext* crypto; int devId;