From 1109abb637de60365d4d7d1fc0b5ed3b502e3ce3 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Wed, 27 May 2026 22:02:25 -0600 Subject: [PATCH 1/3] logout user in the case that the underlying connection gets disconnected --- src/wh_server.c | 44 ++++++++++++++++++++++++++++++-------- test/wh_test_auth.c | 51 ++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 85 insertions(+), 10 deletions(-) diff --git a/src/wh_server.c b/src/wh_server.c index 2618d29b7..6fd90b562 100644 --- a/src/wh_server.c +++ b/src/wh_server.c @@ -87,6 +87,17 @@ int wh_Server_Init(whServerContext* server, whServerConfig* config) server->nvm = config->nvm; #ifdef WOLFHSM_CFG_ENABLE_AUTHENTICATION server->auth = config->auth; + /* auth context is externally owned; clear any stale session left over from + * a prior connection (Logout first so backend callback runs). */ + if (server->auth != NULL && + server->auth->user.user_id != WH_USER_ID_INVALID) { + whUserId stale_id = server->auth->user.user_id; + int logout_rc = wh_Auth_Logout(server->auth, stale_id); + if (logout_rc != WH_ERROR_OK || + server->auth->user.user_id != WH_USER_ID_INVALID) { + memset(&server->auth->user, 0, sizeof(server->auth->user)); + } + } #endif /* WOLFHSM_CFG_ENABLE_AUTHENTICATION */ #ifndef WOLFHSM_CFG_NO_CRYPTO @@ -177,6 +188,27 @@ int wh_Server_SetConnected(whServerContext *server, whCommConnected connected) return WH_ERROR_BADARGS; } +#ifdef WOLFHSM_CFG_ENABLE_AUTHENTICATION + /* Log out any active user on disconnect, including abrupt drops where + * COMM_CLOSE never arrives. */ + if (connected == WH_COMM_DISCONNECTED && + server->connected != WH_COMM_DISCONNECTED && + server->auth != NULL && + server->auth->user.user_id != WH_USER_ID_INVALID) { + whUserId user_id = server->auth->user.user_id; + int logout_rc = wh_Auth_Logout(server->auth, user_id); + + /* wh_Auth_Logout only clears the session when the backend callback + * returns WH_ERROR_OK. Force-clear if it failed or left the user set, + * so a stale identity can never carry over to the next connection + * (mirrors the defensive clear in wh_Server_Init). */ + if (logout_rc != WH_ERROR_OK || + server->auth->user.user_id != WH_USER_ID_INVALID) { + memset(&server->auth->user, 0, sizeof(server->auth->user)); + } + } +#endif /* WOLFHSM_CFG_ENABLE_AUTHENTICATION */ + server->connected = connected; return WH_ERROR_OK; } @@ -285,15 +317,9 @@ static int _wh_Server_HandleCommRequest(whServerContext* server, /* No message */ /* Process the close action */ -#ifdef WOLFHSM_CFG_ENABLE_AUTHENTICATION - /* Log out the current user when communication channel closes */ - if (server->auth != NULL && - server->auth->user.user_id != WH_USER_ID_INVALID) { - whUserId user_id = server->auth->user.user_id; - (void)wh_Auth_Logout(server->auth, user_id); - } -#endif /* WOLFHSM_CFG_ENABLE_AUTHENTICATION */ - + /* wh_Server_SetConnected logs out any active user on the transition to + * the disconnected state, so the graceful-close and abrupt-disconnect + * paths share a single authoritative logout. */ wh_Server_SetConnected(server, WH_COMM_DISCONNECTED); *out_resp_size = 0; diff --git a/test/wh_test_auth.c b/test/wh_test_auth.c index cb9e65013..6c37e5d7f 100644 --- a/test/wh_test_auth.c +++ b/test/wh_test_auth.c @@ -89,6 +89,7 @@ static whNvmContext nvm[1] = {{0}}; /* Test-specific authorization override callbacks to verify they are invoked */ static int test_checkRequestAuthorizationCalled = 0; static int test_checkKeyAuthorizationCalled = 0; +static int test_logoutCallCount = 0; static int test_CheckRequestAuthorization(void* context, int err, uint16_t user_id, uint16_t group, @@ -115,12 +116,20 @@ static int test_CheckKeyAuthorization(void* context, int err, uint16_t user_id, return err; } +/* Counts Logout calls for the abrupt-disconnect test. */ +static int test_Logout(void* context, uint16_t current_user_id, + uint16_t user_id) +{ + test_logoutCallCount++; + return wh_Auth_BaseLogout(context, current_user_id, user_id); +} + /* Auth setup following wh_posix_server pattern */ static whAuthCb default_auth_cb = { .Init = wh_Auth_BaseInit, .Cleanup = wh_Auth_BaseCleanup, .Login = wh_Auth_BaseLogin, - .Logout = wh_Auth_BaseLogout, + .Logout = test_Logout, .CheckRequestAuthorization = test_CheckRequestAuthorization, .CheckKeyAuthorization = test_CheckKeyAuthorization, .UserAdd = wh_Auth_BaseUserAdd, @@ -142,6 +151,10 @@ static int _whTest_Auth_SetupMemory(whClientContext** out_client) uint16_t out_user_id; int i; + /* Reset so logout-count assertions are self-contained across repeated + * invocations of the suite in a single process. */ + test_logoutCallCount = 0; + /* Initialize transport memory config - avoid compound literals for C90 */ tmcf->req = (whTransportMemCsr*)req_buffer; tmcf->req_size = sizeof(req_buffer); @@ -1415,6 +1428,41 @@ int whTest_AuthTCP(whClientConfig* clientCfg) } +#if !defined(WOLFHSM_CFG_TEST_CLIENT_ONLY_TCP) && \ + defined(WOLFHSM_CFG_ENABLE_SERVER) +static int _whTest_Auth_AbruptDisconnect(whClientContext* client_ctx) +{ + int32_t server_rc = 0; + whUserId user_id = WH_USER_ID_INVALID; + int logouts_before = 0; + + WH_TEST_PRINT(" Test: Abrupt disconnect calls wh_Auth_Logout\n"); + + WH_TEST_RETURN_ON_FAIL(_whTest_Auth_LoginOp( + client_ctx, WH_AUTH_METHOD_PIN, TEST_ADMIN_USERNAME, TEST_ADMIN_PIN, + strlen(TEST_ADMIN_PIN), &server_rc, &user_id)); + WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_OK); + WH_TEST_ASSERT_RETURN(user_id != WH_USER_ID_INVALID); + WH_TEST_ASSERT_RETURN(auth_ctx.user.user_id == user_id); + WH_TEST_ASSERT_RETURN(auth_ctx.user.is_active); + + logouts_before = test_logoutCallCount; + WH_TEST_RETURN_ON_FAIL( + wh_Server_SetConnected(server, WH_COMM_DISCONNECTED)); + WH_TEST_ASSERT_RETURN(test_logoutCallCount == logouts_before + 1); + WH_TEST_ASSERT_RETURN(auth_ctx.user.user_id == WH_USER_ID_INVALID); + WH_TEST_ASSERT_RETURN(!auth_ctx.user.is_active); + + /* Repeat disconnect is a no-op. */ + logouts_before = test_logoutCallCount; + WH_TEST_RETURN_ON_FAIL( + wh_Server_SetConnected(server, WH_COMM_DISCONNECTED)); + WH_TEST_ASSERT_RETURN(test_logoutCallCount == logouts_before); + + return WH_TEST_SUCCESS; +} +#endif /* memory transport */ + int whTest_AuthMEM(void) { #if !defined(WOLFHSM_CFG_TEST_CLIENT_ONLY_TCP) && \ @@ -1424,6 +1472,7 @@ int whTest_AuthMEM(void) /* Memory transport mode */ WH_TEST_RETURN_ON_FAIL(_whTest_Auth_SetupMemory(&client_ctx)); WH_TEST_RETURN_ON_FAIL(whTest_AuthTest(client_ctx)); + WH_TEST_RETURN_ON_FAIL(_whTest_Auth_AbruptDisconnect(client_ctx)); /* Verify that authorization callbacks were invoked during tests */ WH_TEST_PRINT( From 15cb7109791b45ed0c8da376cb4614d6844ae809 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Wed, 10 Jun 2026 09:56:34 -0600 Subject: [PATCH 2/3] add wh_Auth_Reset to force user reset and add logging messages --- src/wh_auth.c | 19 +++++++++++++++++++ src/wh_server.c | 22 ++++++++++++---------- test/wh_test_auth.c | 4 ++-- wolfhsm/wh_auth.h | 13 +++++++++++++ 4 files changed, 46 insertions(+), 12 deletions(-) diff --git a/src/wh_auth.c b/src/wh_auth.c index 7498a3e93..835430509 100644 --- a/src/wh_auth.c +++ b/src/wh_auth.c @@ -184,6 +184,25 @@ int wh_Auth_Logout(whAuthContext* context, whUserId user_id) } +/* returns result of locking, clears context user regardless of lock success. */ +int wh_Auth_Reset(whAuthContext* context) +{ + int rc; + + if (context == NULL) { + return WH_ERROR_BADARGS; + } + + /* Best-effort lock, but clear the session regardless */ + rc = WH_AUTH_LOCK(context); + memset(&context->user, 0, sizeof(whAuthUser)); + if (rc == WH_ERROR_OK) { + (void)WH_AUTH_UNLOCK(context); + } + return rc; +} + + /* Check on request authorization and action permissions for current user * logged in */ int wh_Auth_CheckRequestAuthorization(whAuthContext* context, uint16_t group, diff --git a/src/wh_server.c b/src/wh_server.c index 6fd90b562..a10179096 100644 --- a/src/wh_server.c +++ b/src/wh_server.c @@ -93,10 +93,12 @@ int wh_Server_Init(whServerContext* server, whServerConfig* config) server->auth->user.user_id != WH_USER_ID_INVALID) { whUserId stale_id = server->auth->user.user_id; int logout_rc = wh_Auth_Logout(server->auth, stale_id); - if (logout_rc != WH_ERROR_OK || - server->auth->user.user_id != WH_USER_ID_INVALID) { - memset(&server->auth->user, 0, sizeof(server->auth->user)); + if (logout_rc != WH_ERROR_OK) { + WH_LOG(&server->log, WH_LOG_LEVEL_SECEVENT, + "Stale auth session force-cleared during server init after " + "logout failure"); } + (void)wh_Auth_Reset(server->auth); } #endif /* WOLFHSM_CFG_ENABLE_AUTHENTICATION */ @@ -198,14 +200,14 @@ int wh_Server_SetConnected(whServerContext *server, whCommConnected connected) whUserId user_id = server->auth->user.user_id; int logout_rc = wh_Auth_Logout(server->auth, user_id); - /* wh_Auth_Logout only clears the session when the backend callback - * returns WH_ERROR_OK. Force-clear if it failed or left the user set, - * so a stale identity can never carry over to the next connection - * (mirrors the defensive clear in wh_Server_Init). */ - if (logout_rc != WH_ERROR_OK || - server->auth->user.user_id != WH_USER_ID_INVALID) { - memset(&server->auth->user, 0, sizeof(server->auth->user)); + if (logout_rc != WH_ERROR_OK) { + WH_LOG(&server->log, WH_LOG_LEVEL_SECEVENT, + "Auth session force-cleared on disconnect after logout " + "failure"); } + /* Always reset so a stale identity can never carry over to the next + * connection, even if the backend logout failed. */ + (void)wh_Auth_Reset(server->auth); } #endif /* WOLFHSM_CFG_ENABLE_AUTHENTICATION */ diff --git a/test/wh_test_auth.c b/test/wh_test_auth.c index 6c37e5d7f..2bcd3ad81 100644 --- a/test/wh_test_auth.c +++ b/test/wh_test_auth.c @@ -1430,7 +1430,7 @@ int whTest_AuthTCP(whClientConfig* clientCfg) #if !defined(WOLFHSM_CFG_TEST_CLIENT_ONLY_TCP) && \ defined(WOLFHSM_CFG_ENABLE_SERVER) -static int _whTest_Auth_AbruptDisconnect(whClientContext* client_ctx) +static int AuthAbruptDisconnect(whClientContext* client_ctx) { int32_t server_rc = 0; whUserId user_id = WH_USER_ID_INVALID; @@ -1472,7 +1472,7 @@ int whTest_AuthMEM(void) /* Memory transport mode */ WH_TEST_RETURN_ON_FAIL(_whTest_Auth_SetupMemory(&client_ctx)); WH_TEST_RETURN_ON_FAIL(whTest_AuthTest(client_ctx)); - WH_TEST_RETURN_ON_FAIL(_whTest_Auth_AbruptDisconnect(client_ctx)); + WH_TEST_RETURN_ON_FAIL(AuthAbruptDisconnect(client_ctx)); /* Verify that authorization callbacks were invoked during tests */ WH_TEST_PRINT( diff --git a/wolfhsm/wh_auth.h b/wolfhsm/wh_auth.h index 85edbc3b0..e9ef38afc 100644 --- a/wolfhsm/wh_auth.h +++ b/wolfhsm/wh_auth.h @@ -271,6 +271,19 @@ int wh_Auth_Login(whAuthContext* context, uint8_t client_id, */ int wh_Auth_Logout(whAuthContext* context, whUserId user_id); +/** + * @brief Force-clear the local session state, dropping any logged-in user. + * + * Unconditionally zeroes only the session (user) state, leaving the + * externally-owned configuration (callbacks, context, lock) intact. Does not + * invoke the backend logout callback. + * + * @param[in] context Pointer to the auth context. + * @return int WH_ERROR_OK, WH_ERROR_BADARGS for a NULL context, or the lock + * error if the lock could not be acquired (session is still cleared). + */ +int wh_Auth_Reset(whAuthContext* context); + /** * @brief Check authorization for an action. * From f19487ee0c02f33bbacb78a90ef571a757956a29 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Thu, 11 Jun 2026 10:30:02 -0600 Subject: [PATCH 3/3] fail closed now when lock fails with wh_Auth_Reset --- src/wh_auth.c | 8 ++++---- src/wh_server.c | 37 ++++++++++++++++++++++++------------- wolfhsm/wh_auth.h | 9 +++++---- 3 files changed, 33 insertions(+), 21 deletions(-) diff --git a/src/wh_auth.c b/src/wh_auth.c index 835430509..ea9743183 100644 --- a/src/wh_auth.c +++ b/src/wh_auth.c @@ -184,7 +184,8 @@ int wh_Auth_Logout(whAuthContext* context, whUserId user_id) } -/* returns result of locking, clears context user regardless of lock success. */ +/* Clears the local session under the auth lock. Fails without clearing if + * the lock can not be acquired, to avoid racing an in-flight login. */ int wh_Auth_Reset(whAuthContext* context) { int rc; @@ -193,12 +194,11 @@ int wh_Auth_Reset(whAuthContext* context) return WH_ERROR_BADARGS; } - /* Best-effort lock, but clear the session regardless */ rc = WH_AUTH_LOCK(context); - memset(&context->user, 0, sizeof(whAuthUser)); if (rc == WH_ERROR_OK) { + memset(&context->user, 0, sizeof(whAuthUser)); (void)WH_AUTH_UNLOCK(context); - } + } /* LOCK() */ return rc; } diff --git a/src/wh_server.c b/src/wh_server.c index a10179096..85727124a 100644 --- a/src/wh_server.c +++ b/src/wh_server.c @@ -89,16 +89,23 @@ int wh_Server_Init(whServerContext* server, whServerConfig* config) server->auth = config->auth; /* auth context is externally owned; clear any stale session left over from * a prior connection (Logout first so backend callback runs). */ - if (server->auth != NULL && - server->auth->user.user_id != WH_USER_ID_INVALID) { - whUserId stale_id = server->auth->user.user_id; - int logout_rc = wh_Auth_Logout(server->auth, stale_id); - if (logout_rc != WH_ERROR_OK) { - WH_LOG(&server->log, WH_LOG_LEVEL_SECEVENT, + if (server->auth != NULL) { + if (server->auth->user.user_id != WH_USER_ID_INVALID) { + whUserId stale_id = server->auth->user.user_id; + + rc = wh_Auth_Logout(server->auth, stale_id); + if (rc != WH_ERROR_OK) { + WH_LOG(&server->log, WH_LOG_LEVEL_SECEVENT, "Stale auth session force-cleared during server init after " "logout failure"); + } + } + rc = wh_Auth_Reset(server->auth); + if (rc != WH_ERROR_OK) { + WH_LOG(&server->log, WH_LOG_LEVEL_SECEVENT, + "Failed to clear auth session during server init"); + return rc; } - (void)wh_Auth_Reset(server->auth); } #endif /* WOLFHSM_CFG_ENABLE_AUTHENTICATION */ @@ -197,17 +204,21 @@ int wh_Server_SetConnected(whServerContext *server, whCommConnected connected) server->connected != WH_COMM_DISCONNECTED && server->auth != NULL && server->auth->user.user_id != WH_USER_ID_INVALID) { - whUserId user_id = server->auth->user.user_id; - int logout_rc = wh_Auth_Logout(server->auth, user_id); + whUserId user_id = server->auth->user.user_id; + int rc = wh_Auth_Logout(server->auth, user_id); - if (logout_rc != WH_ERROR_OK) { + if (rc != WH_ERROR_OK) { WH_LOG(&server->log, WH_LOG_LEVEL_SECEVENT, "Auth session force-cleared on disconnect after logout " "failure"); } - /* Always reset so a stale identity can never carry over to the next - * connection, even if the backend logout failed. */ - (void)wh_Auth_Reset(server->auth); + rc = wh_Auth_Reset(server->auth); + if (rc != WH_ERROR_OK) { + WH_LOG(&server->log, WH_LOG_LEVEL_SECEVENT, + "Failed to clear auth session on disconnect"); + server->connected = connected; + return rc; + } } #endif /* WOLFHSM_CFG_ENABLE_AUTHENTICATION */ diff --git a/wolfhsm/wh_auth.h b/wolfhsm/wh_auth.h index e9ef38afc..7ab517d59 100644 --- a/wolfhsm/wh_auth.h +++ b/wolfhsm/wh_auth.h @@ -274,13 +274,14 @@ int wh_Auth_Logout(whAuthContext* context, whUserId user_id); /** * @brief Force-clear the local session state, dropping any logged-in user. * - * Unconditionally zeroes only the session (user) state, leaving the + * Zeroes only the session (user) state under the auth lock, leaving the * externally-owned configuration (callbacks, context, lock) intact. Does not - * invoke the backend logout callback. + * invoke the backend logout callback. If the lock can not be acquired the + * session is left unchanged and the lock error is returned. * * @param[in] context Pointer to the auth context. - * @return int WH_ERROR_OK, WH_ERROR_BADARGS for a NULL context, or the lock - * error if the lock could not be acquired (session is still cleared). + * @return int WH_ERROR_OK on success, WH_ERROR_BADARGS for a NULL context, or + * the lock error if the lock could not be acquired. */ int wh_Auth_Reset(whAuthContext* context);