Skip to content

Commit 0050bdd

Browse files
JacobBarthelmehJacobBarthelmeh
committed
add locks around user login / modify sections of code for thread safety
1 parent 31b7a90 commit 0050bdd

5 files changed

Lines changed: 169 additions & 19 deletions

File tree

‎.github/workflows/build-and-test.yml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,3 +87,7 @@ jobs:
8787
# Build and test with AUTH=1 and ASAN
8888
- name: Build and test with AUTH ASAN
8989
run: cd test && make clean && make -j AUTH=1 ASAN=1 WOLFSSL_DIR=../wolfssl && make run
90+
91+
# Build and test with AUTH=1 and THREADSAFE
92+
- name: Build and test with AUTH THREADSAFE ASAN
93+
run: cd test && make clean && make -j AUTH=1 THREADSAFE=1 ASAN=1 WOLFSSL_DIR=../wolfssl && make run

‎port/posix/posix_auth.c‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,10 +48,9 @@ typedef struct whAuthBase_User {
4848
unsigned char credentials[WH_AUTH_BASE_MAX_CREDENTIALS_LEN];
4949
uint16_t credentials_len;
5050
} whAuthBase_User;
51-
/* TODO: Thread safety - The global users array is not protected by any
52-
* synchronization mechanism. In a multi-threaded environment, concurrent
53-
* access could lead to race conditions. Consider adding appropriate locking
54-
* mechanisms (mutex, rwlock) to protect concurrent access. */
51+
/* The global users array is protected by the auth context lock when
52+
* WOLFHSM_CFG_THREADSAFE is defined. Locking is performed by the wh_Auth_*
53+
* wrapper functions in wh_auth.c. */
5554
static whAuthBase_User users[WH_AUTH_BASE_MAX_USERS];
5655

5756
#if defined(WOLFHSM_CFG_CERTIFICATE_MANAGER) && !defined(WOLFHSM_CFG_NO_CRYPTO)

‎src/wh_auth.c‎

Lines changed: 120 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,22 @@ int wh_Auth_Init(whAuthContext* context, const whAuthConfig* config)
6262
context->context = config->context;
6363
memset(&context->user, 0, sizeof(whAuthUser));
6464

65+
#ifdef WOLFHSM_CFG_THREADSAFE
66+
/* Initialize the lock for thread-safe auth operations */
67+
rc = wh_Lock_Init(&context->lock, config->lockConfig);
68+
if (rc != WH_ERROR_OK) {
69+
context->cb = NULL;
70+
context->context = NULL;
71+
return rc;
72+
}
73+
#endif /* WOLFHSM_CFG_THREADSAFE */
74+
6575
if (context->cb != NULL && context->cb->Init != NULL) {
6676
rc = context->cb->Init(context->context, config->config);
6777
if (rc != WH_ERROR_OK) {
78+
#ifdef WOLFHSM_CFG_THREADSAFE
79+
(void)wh_Lock_Cleanup(&context->lock);
80+
#endif
6881
context->cb = NULL;
6982
context->context = NULL;
7083
}
@@ -76,14 +89,24 @@ int wh_Auth_Init(whAuthContext* context, const whAuthConfig* config)
7689

7790
int wh_Auth_Cleanup(whAuthContext* context)
7891
{
92+
int rc = WH_ERROR_OK;
93+
7994
if ((context == NULL) || (context->cb == NULL)) {
8095
return WH_ERROR_BADARGS;
8196
}
8297

8398
if (context->cb->Cleanup == NULL) {
8499
return WH_ERROR_ABORTED;
85100
}
86-
return context->cb->Cleanup(context->context);
101+
102+
rc = context->cb->Cleanup(context->context);
103+
104+
#ifdef WOLFHSM_CFG_THREADSAFE
105+
/* Cleanup the lock for thread-safe auth operations */
106+
(void)wh_Lock_Cleanup(&context->lock);
107+
#endif /* WOLFHSM_CFG_THREADSAFE */
108+
109+
return rc;
87110
}
88111

89112

@@ -109,6 +132,11 @@ int wh_Auth_Login(whAuthContext* context, uint8_t client_id,
109132
return WH_ERROR_BADARGS;
110133
}
111134

135+
rc = WH_AUTH_LOCK(context);
136+
if (rc != WH_ERROR_OK) {
137+
return rc;
138+
}
139+
112140
/* allowing only one user logged in to an open connection at a time */
113141
if (context->user.user_id != WH_USER_ID_INVALID) {
114142
*loggedIn = 0;
@@ -125,6 +153,7 @@ int wh_Auth_Login(whAuthContext* context, uint8_t client_id,
125153
}
126154
}
127155

156+
(void)WH_AUTH_UNLOCK(context);
128157
return rc;
129158
}
130159

@@ -138,14 +167,19 @@ int wh_Auth_Logout(whAuthContext* context, whUserId user_id)
138167
return WH_ERROR_BADARGS;
139168
}
140169

141-
rc = context->cb->Logout(context->context, context->user.user_id, user_id);
170+
rc = WH_AUTH_LOCK(context);
142171
if (rc != WH_ERROR_OK) {
143172
return rc;
144173
}
145174

146-
/* Clear the user context */
147-
memset(&context->user, 0, sizeof(whAuthUser));
148-
return WH_ERROR_OK;
175+
rc = context->cb->Logout(context->context, context->user.user_id, user_id);
176+
if (rc == WH_ERROR_OK) {
177+
/* Clear the user context */
178+
memset(&context->user, 0, sizeof(whAuthUser));
179+
}
180+
181+
(void)WH_AUTH_UNLOCK(context);
182+
return rc;
149183
}
150184

151185

@@ -271,51 +305,91 @@ int wh_Auth_UserAdd(whAuthContext* context, const char* username,
271305
whAuthMethod method, const void* credentials,
272306
uint16_t credentials_len)
273307
{
308+
int rc;
309+
274310
if ((context == NULL) || (context->cb == NULL) ||
275311
(context->cb->UserAdd == NULL)) {
276312
return WH_ERROR_BADARGS;
277313
}
278314

279-
return context->cb->UserAdd(context->context, username, out_user_id,
280-
permissions, method, credentials,
281-
credentials_len);
315+
rc = WH_AUTH_LOCK(context);
316+
if (rc != WH_ERROR_OK) {
317+
return rc;
318+
}
319+
320+
rc = context->cb->UserAdd(context->context, username, out_user_id,
321+
permissions, method, credentials,
322+
credentials_len);
323+
324+
(void)WH_AUTH_UNLOCK(context);
325+
return rc;
282326
}
283327

284328

285329
int wh_Auth_UserDelete(whAuthContext* context, whUserId user_id)
286330
{
331+
int rc;
332+
287333
if ((context == NULL) || (context->cb == NULL) ||
288334
(context->cb->UserDelete == NULL)) {
289335
return WH_ERROR_BADARGS;
290336
}
291337

292-
return context->cb->UserDelete(context->context, context->user.user_id,
293-
user_id);
338+
rc = WH_AUTH_LOCK(context);
339+
if (rc != WH_ERROR_OK) {
340+
return rc;
341+
}
342+
343+
rc = context->cb->UserDelete(context->context, context->user.user_id,
344+
user_id);
345+
346+
(void)WH_AUTH_UNLOCK(context);
347+
return rc;
294348
}
295349

296350

297351
int wh_Auth_UserSetPermissions(whAuthContext* context, whUserId user_id,
298352
whAuthPermissions permissions)
299353
{
354+
int rc;
355+
300356
if ((context == NULL) || (context->cb == NULL) ||
301357
(context->cb->UserSetPermissions == NULL)) {
302358
return WH_ERROR_BADARGS;
303359
}
304360

305-
return context->cb->UserSetPermissions(
361+
rc = WH_AUTH_LOCK(context);
362+
if (rc != WH_ERROR_OK) {
363+
return rc;
364+
}
365+
366+
rc = context->cb->UserSetPermissions(
306367
context->context, context->user.user_id, user_id, permissions);
368+
369+
(void)WH_AUTH_UNLOCK(context);
370+
return rc;
307371
}
308372

309373
int wh_Auth_UserGet(whAuthContext* context, const char* username,
310374
whUserId* out_user_id, whAuthPermissions* out_permissions)
311375
{
376+
int rc;
377+
312378
if ((context == NULL) || (context->cb == NULL) ||
313379
(context->cb->UserGet == NULL)) {
314380
return WH_ERROR_BADARGS;
315381
}
316382

317-
return context->cb->UserGet(context->context, username, out_user_id,
318-
out_permissions);
383+
rc = WH_AUTH_LOCK(context);
384+
if (rc != WH_ERROR_OK) {
385+
return rc;
386+
}
387+
388+
rc = context->cb->UserGet(context->context, username, out_user_id,
389+
out_permissions);
390+
391+
(void)WH_AUTH_UNLOCK(context);
392+
return rc;
319393
}
320394

321395
int wh_Auth_UserSetCredentials(whAuthContext* context, whUserId user_id,
@@ -325,12 +399,44 @@ int wh_Auth_UserSetCredentials(whAuthContext* context, whUserId user_id,
325399
const void* new_credentials,
326400
uint16_t new_credentials_len)
327401
{
402+
int rc;
403+
328404
if ((context == NULL) || (context->cb == NULL) ||
329405
(context->cb->UserSetCredentials == NULL)) {
330406
return WH_ERROR_BADARGS;
331407
}
332408

333-
return context->cb->UserSetCredentials(
409+
rc = WH_AUTH_LOCK(context);
410+
if (rc != WH_ERROR_OK) {
411+
return rc;
412+
}
413+
414+
rc = context->cb->UserSetCredentials(
334415
context->context, user_id, method, current_credentials,
335416
current_credentials_len, new_credentials, new_credentials_len);
417+
418+
(void)WH_AUTH_UNLOCK(context);
419+
return rc;
420+
}
421+
422+
423+
/********** Lock/Unlock Functions for Thread Safety *************************/
424+
425+
#ifdef WOLFHSM_CFG_THREADSAFE
426+
int wh_Auth_Lock(whAuthContext* auth)
427+
{
428+
if (auth == NULL) {
429+
return WH_ERROR_BADARGS;
430+
}
431+
return wh_Lock_Acquire(&auth->lock);
432+
}
433+
434+
435+
int wh_Auth_Unlock(whAuthContext* auth)
436+
{
437+
if (auth == NULL) {
438+
return WH_ERROR_BADARGS;
439+
}
440+
return wh_Lock_Release(&auth->lock);
336441
}
442+
#endif /* WOLFHSM_CFG_THREADSAFE */

‎test/wh_test_posix_threadsafe_stress.c‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,11 @@
3939

4040
#include "wolfhsm/wh_settings.h"
4141

42+
/* Note: pthread_barrier_t is not available on macOS, so skip this test */
4243
#if defined(WOLFHSM_CFG_THREADSAFE) && defined(WOLFHSM_CFG_TEST_POSIX) && \
4344
defined(WOLFHSM_CFG_GLOBAL_KEYS) && defined(WOLFHSM_CFG_ENABLE_CLIENT) && \
44-
defined(WOLFHSM_CFG_ENABLE_SERVER) && !defined(WOLFHSM_CFG_NO_CRYPTO)
45+
defined(WOLFHSM_CFG_ENABLE_SERVER) && !defined(WOLFHSM_CFG_NO_CRYPTO) && \
46+
!defined(__APPLE__)
4547

4648
#include <stdint.h>
4749
#include <stdio.h>

‎wolfhsm/wh_auth.h‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,10 @@
4242
#include "wolfhsm/wh_common.h"
4343
#include "wolfhsm/wh_message.h" /* for WH_NUMBER_OF_GROUPS */
4444

45+
#ifdef WOLFHSM_CFG_THREADSAFE
46+
#include "wolfhsm/wh_lock.h"
47+
#endif
48+
4549
/** Auth Manager Types */
4650

4751
/* User identifier type */
@@ -150,6 +154,9 @@ typedef struct whAuthContext_t {
150154
whAuthCb* cb;
151155
whAuthUser user;
152156
void* context;
157+
#ifdef WOLFHSM_CFG_THREADSAFE
158+
whLock lock; /* Lock for serializing auth operations */
159+
#endif
153160
} whAuthContext;
154161

155162
/* Simple helper configuration structure associated with an Auth Manager
@@ -158,6 +165,9 @@ typedef struct whAuthConfig_t {
158165
whAuthCb* cb;
159166
void* context;
160167
void* config;
168+
#ifdef WOLFHSM_CFG_THREADSAFE
169+
whLockConfig* lockConfig; /* Lock configuration for thread safety */
170+
#endif
161171
} whAuthConfig;
162172

163173
/** Public Auth Manager API Functions */
@@ -298,4 +308,33 @@ int wh_Auth_UserSetCredentials(whAuthContext* context, whUserId user_id,
298308
uint16_t current_credentials_len,
299309
const void* new_credentials,
300310
uint16_t new_credentials_len);
311+
312+
#ifdef WOLFHSM_CFG_THREADSAFE
313+
/**
314+
* @brief Acquires the auth lock.
315+
*
316+
* @param[in] auth Pointer to the auth context. Must not be NULL.
317+
* @return int WH_ERROR_OK on success.
318+
* WH_ERROR_BADARGS if auth is NULL.
319+
* Other negative error codes on lock acquisition failure.
320+
*/
321+
int wh_Auth_Lock(whAuthContext* auth);
322+
323+
/**
324+
* @brief Releases the auth lock.
325+
*
326+
* @param[in] auth Pointer to the auth context. Must not be NULL.
327+
* @return int WH_ERROR_OK on success.
328+
* WH_ERROR_BADARGS if auth is NULL.
329+
* Other negative error codes on lock release failure.
330+
*/
331+
int wh_Auth_Unlock(whAuthContext* auth);
332+
333+
#define WH_AUTH_LOCK(auth) wh_Auth_Lock(auth)
334+
#define WH_AUTH_UNLOCK(auth) wh_Auth_Unlock(auth)
335+
#else
336+
#define WH_AUTH_LOCK(auth) (WH_ERROR_OK)
337+
#define WH_AUTH_UNLOCK(auth) (WH_ERROR_OK)
338+
#endif
339+
301340
#endif /* !WOLFHSM_WH_AUTH_H_ */

0 commit comments

Comments
 (0)