Skip to content

logout user in the case that the underlying connection gets disconnected #383

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
JacobBarthelmeh wants to merge 3 commits into
base: main
Choose a base branch
from
Open

logout user in the case that the underlying connection gets disconnected #383

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1109abb
logout user in the case that the underlying connection gets disconnected
JacobBarthelmeh May 28, 2026
15cb710
add wh_Auth_Reset to force user reset and add logging messages
JacobBarthelmeh Jun 10, 2026
f19487e
fail closed now when lock fails with wh_Auth_Reset
JacobBarthelmeh Jun 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions src/wh_auth.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,25 @@ int wh_Auth_Logout(whAuthContext* context, whUserId user_id)
}


/* 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;

if (context == NULL) {
return WH_ERROR_BADARGS;
}

rc = WH_AUTH_LOCK(context);

@bigbrett bigbrett Jun 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JacobBarthelmeh Seems odd to still clear the session if you can't acquire the lock - this means someone else could be in process of reading. Perhaps the backend would get put into a weird state if synchronous access isn't respected? I'd rather just hard fail if you can't acquire the lock, and let the caller deal with it since it probably means their entire system is broken? I guess you could make the argument that the locking function really shouldn't ever fail, but kind of a bucket of worms. Thoughts?

@JacobBarthelmeh JacobBarthelmeh Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm okay with changing it to be a hard fail when the lock fails. I think that is better behavior than what I'd added for continuing to memset anyway.

@bigbrett bigbrett Jun 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JacobBarthelmeh sounds good, pls assign me when ready for re-review

if (rc == WH_ERROR_OK) {
memset(&context->user, 0, sizeof(whAuthUser));
(void)WH_AUTH_UNLOCK(context);
} /* LOCK() */
return rc;
}


/* Check on request authorization and action permissions for current user
* logged in */
int wh_Auth_CheckRequestAuthorization(whAuthContext* context, uint16_t group,
Expand Down
57 changes: 48 additions & 9 deletions src/wh_server.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,26 @@ 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) {
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,

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot Jun 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 [Low] Init-time auth SECEVENT logs fire before wh_Log_Init and are discarded · Dead error handling

The new WH_LOG(WH_LOG_LEVEL_SECEVENT, ...) calls in the auth-clearing block (lines 98 and 105) run before wh_Log_Init at line 122. After the memset at line 86, server->log.cb is NULL, so wh_Log_AddEntry returns WH_ERROR_ABORTED and these security events are silently dropped.

Fix: Move the auth stale-session clearing block to after the wh_Log_Init call so its SECEVENT messages are actually recorded.

@bigbrett bigbrett Jun 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JacobBarthelmeh this look like the log will indeed be dropped since it is after the reset. Could you address this pls? Then we can get it in

"Failed to clear auth session during server init");
return rc;
}
}
#endif /* WOLFHSM_CFG_ENABLE_AUTHENTICATION */

#ifndef WOLFHSM_CFG_NO_CRYPTO
Expand Down Expand Up @@ -177,6 +197,31 @@ 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 rc = wh_Auth_Logout(server->auth, user_id);

if (rc != WH_ERROR_OK) {
WH_LOG(&server->log, WH_LOG_LEVEL_SECEVENT,
"Auth session force-cleared on disconnect 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 on disconnect");
server->connected = connected;
return rc;
}
}
#endif /* WOLFHSM_CFG_ENABLE_AUTHENTICATION */

server->connected = connected;
return WH_ERROR_OK;
}
Expand Down Expand Up @@ -285,15 +330,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;

Expand Down
51 changes: 50 additions & 1 deletion test/wh_test_auth.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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);
Expand Down Expand Up @@ -1415,6 +1428,41 @@ int whTest_AuthTCP(whClientConfig* clientCfg)
}


#if !defined(WOLFHSM_CFG_TEST_CLIENT_ONLY_TCP) && \
defined(WOLFHSM_CFG_ENABLE_SERVER)
static int AuthAbruptDisconnect(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) && \
Expand All @@ -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(AuthAbruptDisconnect(client_ctx));

/* Verify that authorization callbacks were invoked during tests */
WH_TEST_PRINT(
Expand Down
14 changes: 14 additions & 0 deletions wolfhsm/wh_auth.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,20 @@ 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.
*
* 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. 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 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);

/**
* @brief Check authorization for an action.
*
Expand Down
Loading