Skip to content

Commit 9346c7a

Browse files
JacobBarthelmehJacobBarthelmeh
committed
add constant compare utils function, permissions helper macros, fixes to auth tranlation layer, admin user add restriction, duplicate user name restriction
1 parent 2e66b0e commit 9346c7a

File tree

12 files changed

+389
-282
lines changed

12 files changed

+389
-282
lines changed

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,3 +88,7 @@ jobs:
8888
# Build and test with AUTH=1 and THREADSAFE
8989
- name: Build and test with AUTH THREADSAFE ASAN
9090
run: cd test && make clean && make -j AUTH=1 THREADSAFE=1 ASAN=1 WOLFSSL_DIR=../wolfssl && make run
91+
92+
# Build and test with AUTH=1 and NOCRYPTO=1 (auth on, crypto off)
93+
- name: Build and test with AUTH NOCRYPTO
94+
run: cd test && make clean && make -j AUTH=1 NOCRYPTO=1 WOLFSSL_DIR=../wolfssl && make run

‎Makefile‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
export DEBUG
55
export DEBUG_VERBOSE
6+
export AUTH
67

78
all: test benchmark tools examples
89

‎examples/demo/client/wh_demo_client_auth.c‎

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -351,19 +351,7 @@ static int wh_DemoClient_AuthUserSetPermissions(whClientContext* clientContext)
351351

352352
/* Enable CRYPTO group and all CRYPTO actions */
353353
memset(&permissions, 0, sizeof(permissions));
354-
355-
/* Enable all CRYPTO actions by setting all bits in all words, an example of
356-
* a CRYPTO action is WC_ALGO_TYPE_CIPHER or WC_ALGO_TYPE_PK */
357-
{
358-
int groupIndex = (WH_MESSAGE_GROUP_CRYPTO >> 8) & 0xFF;
359-
int wordIndex;
360-
/* Enable access to CRYPTO group */
361-
permissions.groupPermissions[groupIndex] = 1;
362-
/* Set all action bits for CRYPTO group (allows all actions) */
363-
for (wordIndex = 0; wordIndex < WH_AUTH_ACTION_WORDS; wordIndex++) {
364-
permissions.actionPermissions[groupIndex][wordIndex] = 0xFFFFFFFF;
365-
}
366-
}
354+
WH_AUTH_SET_ALLOWED_GROUP(permissions, WH_MESSAGE_GROUP_CRYPTO);
367355

368356
rc = wh_Client_AuthUserSetPermissions(clientContext, userId, permissions,
369357
&serverRc);

‎src/wh_auth.c‎

Lines changed: 128 additions & 142 deletions
Original file line numberDiff line numberDiff line change
@@ -133,27 +133,25 @@ int wh_Auth_Login(whAuthContext* context, uint8_t client_id,
133133
}
134134

135135
rc = WH_AUTH_LOCK(context);
136-
if (rc != WH_ERROR_OK) {
137-
return rc;
138-
}
139-
140-
/* allowing only one user logged in to an open connection at a time */
141-
if (context->user.user_id != WH_USER_ID_INVALID) {
142-
*loggedIn = 0;
143-
rc = WH_ERROR_OK; /* login attempt happened but failed */
144-
}
145-
else {
146-
rc = context->cb->Login(context->context, client_id, method, username,
147-
auth_data, auth_data_len, &out_user_id,
148-
&out_permissions, loggedIn);
149-
if (rc == WH_ERROR_OK && *loggedIn) {
150-
context->user.user_id = out_user_id;
151-
context->user.permissions = out_permissions;
152-
context->user.is_active = true;
136+
if (rc == WH_ERROR_OK) {
137+
/* allowing only one user logged in to an open connection at a time */
138+
if (context->user.user_id != WH_USER_ID_INVALID) {
139+
*loggedIn = 0;
140+
rc = WH_ERROR_OK; /* login attempt happened but failed */
141+
}
142+
else {
143+
rc = context->cb->Login(context->context, client_id, method,
144+
username, auth_data, auth_data_len,
145+
&out_user_id, &out_permissions, loggedIn);
146+
if (rc == WH_ERROR_OK && *loggedIn) {
147+
context->user.user_id = out_user_id;
148+
context->user.permissions = out_permissions;
149+
context->user.is_active = true;
150+
}
153151
}
154-
}
155152

156-
(void)WH_AUTH_UNLOCK(context);
153+
(void)WH_AUTH_UNLOCK(context);
154+
} /* LOCK() */
157155
return rc;
158156
}
159157

@@ -168,17 +166,16 @@ int wh_Auth_Logout(whAuthContext* context, whUserId user_id)
168166
}
169167

170168
rc = WH_AUTH_LOCK(context);
171-
if (rc != WH_ERROR_OK) {
172-
return rc;
173-
}
174-
175-
rc = context->cb->Logout(context->context, context->user.user_id, user_id);
176169
if (rc == WH_ERROR_OK) {
177-
/* Clear the user context */
178-
memset(&context->user, 0, sizeof(whAuthUser));
179-
}
170+
rc = context->cb->Logout(context->context, context->user.user_id,
171+
user_id);
172+
if (rc == WH_ERROR_OK) {
173+
/* Clear the user context */
174+
memset(&context->user, 0, sizeof(whAuthUser));
175+
}
180176

181-
(void)WH_AUTH_UNLOCK(context);
177+
(void)WH_AUTH_UNLOCK(context);
178+
} /* LOCK() */
182179
return rc;
183180
}
184181

@@ -197,56 +194,54 @@ int wh_Auth_CheckRequestAuthorization(whAuthContext* context, uint16_t group,
197194
}
198195

199196
rc = WH_AUTH_LOCK(context);
200-
if (rc != WH_ERROR_OK) {
201-
return rc;
202-
}
203-
204-
user = &context->user;
205-
user_id = user->user_id;
206-
/* @TODO add logging call here and with resulting return value */
207-
208-
if (user_id == WH_USER_ID_INVALID) {
209-
/* allow user login request attempt and comm */
210-
if (group == WH_MESSAGE_GROUP_COMM ||
211-
(group == WH_MESSAGE_GROUP_AUTH &&
212-
action == WH_MESSAGE_AUTH_ACTION_LOGIN)) {
213-
rc = WH_ERROR_OK;
214-
}
215-
else {
216-
rc = WH_ERROR_ACCESS;
217-
}
218-
}
219-
else {
220-
int groupIndex = (group >> 8) & 0xFF;
221-
222-
/* some operations a user logged in should by default have access to;
223-
* - logging out
224-
* - updating own credentials */
225-
if (group == WH_MESSAGE_GROUP_AUTH &&
226-
(action == WH_MESSAGE_AUTH_ACTION_LOGOUT ||
227-
action == WH_MESSAGE_AUTH_ACTION_USER_SET_CREDENTIALS)) {
228-
rc = WH_ERROR_OK;
197+
if (rc == WH_ERROR_OK) {
198+
user = &context->user;
199+
user_id = user->user_id;
200+
/* @TODO add logging call here and with resulting return value */
201+
202+
if (user_id == WH_USER_ID_INVALID) {
203+
/* allow user login request attempt and comm */
204+
if (group == WH_MESSAGE_GROUP_COMM ||
205+
(group == WH_MESSAGE_GROUP_AUTH &&
206+
action == WH_MESSAGE_AUTH_ACTION_LOGIN)) {
207+
rc = WH_ERROR_OK;
208+
}
209+
else {
210+
rc = WH_ERROR_ACCESS;
211+
}
229212
}
230213
else {
231-
/* Validate groupIndex is within bounds */
232-
if (groupIndex >= WH_NUMBER_OF_GROUPS || groupIndex < 0) {
233-
rc = WH_ERROR_ACCESS;
214+
int groupIndex = (group >> 8) & 0xFF;
215+
216+
/* A user logged in should by default have access to logging out */
217+
if (group == WH_MESSAGE_GROUP_AUTH &&
218+
action == WH_MESSAGE_AUTH_ACTION_LOGOUT) {
219+
rc = WH_ERROR_OK;
234220
}
235-
else if (user->permissions.groupPermissions[groupIndex]) {
236-
/* Check if action is within supported range */
237-
if (action < WH_AUTH_ACTIONS_PER_GROUP) {
238-
/* Get word index and bitmask for this action */
239-
uint32_t wordIndex;
240-
uint32_t bitmask;
241-
242-
WH_AUTH_ACTION_TO_WORD_AND_BITMASK(action, wordIndex,
243-
bitmask);
244-
245-
if (wordIndex < WH_AUTH_ACTION_WORDS &&
246-
(user->permissions
247-
.actionPermissions[groupIndex][wordIndex] &
248-
bitmask)) {
249-
rc = WH_ERROR_OK;
221+
else {
222+
/* Validate groupIndex is within bounds */
223+
if (groupIndex >= WH_NUMBER_OF_GROUPS || groupIndex < 0) {
224+
rc = WH_ERROR_ACCESS;
225+
}
226+
else if (user->permissions.groupPermissions[groupIndex]) {
227+
/* Check if action is within supported range */
228+
if (action < WH_AUTH_ACTIONS_PER_GROUP) {
229+
/* Get word index and bitmask for this action */
230+
uint32_t wordIndex;
231+
uint32_t bitmask;
232+
233+
WH_AUTH_ACTION_TO_WORD_AND_BITMASK(action, wordIndex,
234+
bitmask);
235+
236+
if (wordIndex < WH_AUTH_ACTION_WORDS &&
237+
(user->permissions
238+
.actionPermissions[groupIndex][wordIndex] &
239+
bitmask)) {
240+
rc = WH_ERROR_OK;
241+
}
242+
else {
243+
rc = WH_ERROR_ACCESS;
244+
}
250245
}
251246
else {
252247
rc = WH_ERROR_ACCESS;
@@ -256,19 +251,15 @@ int wh_Auth_CheckRequestAuthorization(whAuthContext* context, uint16_t group,
256251
rc = WH_ERROR_ACCESS;
257252
}
258253
}
259-
else {
260-
rc = WH_ERROR_ACCESS;
261-
}
262254
}
263-
}
264-
265-
(void)WH_AUTH_UNLOCK(context);
266255

267-
/* allow authorization override if callback is set */
268-
if (context->cb->CheckRequestAuthorization != NULL) {
269-
rc = context->cb->CheckRequestAuthorization(context->context, rc,
270-
user_id, group, action);
271-
}
256+
/* allow authorization override if callback is set */
257+
if (context->cb->CheckRequestAuthorization != NULL) {
258+
rc = context->cb->CheckRequestAuthorization(context->context, rc,
259+
user_id, group, action);
260+
}
261+
(void)WH_AUTH_UNLOCK(context);
262+
} /* LOCK() */
272263
return rc;
273264
}
274265

@@ -287,35 +278,32 @@ int wh_Auth_CheckKeyAuthorization(whAuthContext* context, uint32_t key_id,
287278
}
288279

289280
rc = WH_AUTH_LOCK(context);
290-
if (rc != WH_ERROR_OK) {
291-
return rc;
292-
}
293-
294-
/* Reset rc to default access denied after successful lock */
295-
rc = WH_ERROR_ACCESS;
296-
297-
user_id = context->user.user_id;
298-
user = &context->user;
299-
if (user->user_id == WH_USER_ID_INVALID) {
300-
(void)WH_AUTH_UNLOCK(context);
301-
return WH_ERROR_ACCESS;
302-
}
303-
304-
/* Check if the requested key_id is in the user's keyIds array */
305-
for (i = 0; i < user->permissions.keyIdCount && i < WH_AUTH_MAX_KEY_IDS;
306-
i++) {
307-
if (user->permissions.keyIds[i] == key_id) {
308-
rc = WH_ERROR_OK;
309-
break;
281+
if (rc == WH_ERROR_OK) {
282+
/* Reset rc to default access denied after successful lock */
283+
rc = WH_ERROR_ACCESS;
284+
285+
user_id = context->user.user_id;
286+
user = &context->user;
287+
if (user->user_id == WH_USER_ID_INVALID) {
288+
(void)WH_AUTH_UNLOCK(context);
289+
return WH_ERROR_ACCESS;
310290
}
311-
}
312291

313-
(void)WH_AUTH_UNLOCK(context);
292+
/* Check if the requested key_id is in the user's keyIds array */
293+
for (i = 0; i < user->permissions.keyIdCount && i < WH_AUTH_MAX_KEY_IDS;
294+
i++) {
295+
if (user->permissions.keyIds[i] == key_id) {
296+
rc = WH_ERROR_OK;
297+
break;
298+
}
299+
}
314300

315-
if (context->cb->CheckKeyAuthorization != NULL) {
316-
rc = context->cb->CheckKeyAuthorization(context->context, rc, user_id,
317-
key_id, action);
318-
}
301+
if (context->cb->CheckKeyAuthorization != NULL) {
302+
rc = context->cb->CheckKeyAuthorization(context->context, rc,
303+
user_id, key_id, action);
304+
}
305+
(void)WH_AUTH_UNLOCK(context);
306+
} /* LOCK() */
319307
return rc;
320308
}
321309

@@ -334,15 +322,19 @@ int wh_Auth_UserAdd(whAuthContext* context, const char* username,
334322
}
335323

336324
rc = WH_AUTH_LOCK(context);
337-
if (rc != WH_ERROR_OK) {
338-
return rc;
339-
}
340-
341-
rc =
342-
context->cb->UserAdd(context->context, username, out_user_id,
325+
if (rc == WH_ERROR_OK) {
326+
/* only an admin level user can add another admin level user */
327+
if (WH_AUTH_IS_ADMIN(permissions) &&
328+
!WH_AUTH_IS_ADMIN(context->user.permissions)) {
329+
rc = WH_AUTH_PERMISSION_ERROR;
330+
}
331+
else {
332+
rc =
333+
context->cb->UserAdd(context->context, username, out_user_id,
343334
permissions, method, credentials, credentials_len);
344-
345-
(void)WH_AUTH_UNLOCK(context);
335+
}
336+
(void)WH_AUTH_UNLOCK(context);
337+
} /* LOCK() */
346338
return rc;
347339
}
348340

@@ -357,14 +349,12 @@ int wh_Auth_UserDelete(whAuthContext* context, whUserId user_id)
357349
}
358350

359351
rc = WH_AUTH_LOCK(context);
360-
if (rc != WH_ERROR_OK) {
361-
return rc;
362-
}
363-
364-
rc = context->cb->UserDelete(context->context, context->user.user_id,
365-
user_id);
352+
if (rc == WH_ERROR_OK) {
353+
rc = context->cb->UserDelete(context->context, context->user.user_id,
354+
user_id);
366355

367-
(void)WH_AUTH_UNLOCK(context);
356+
(void)WH_AUTH_UNLOCK(context);
357+
} /* LOCK() */
368358
return rc;
369359
}
370360

@@ -402,14 +392,12 @@ int wh_Auth_UserGet(whAuthContext* context, const char* username,
402392
}
403393

404394
rc = WH_AUTH_LOCK(context);
405-
if (rc != WH_ERROR_OK) {
406-
return rc;
407-
}
408-
409-
rc = context->cb->UserGet(context->context, username, out_user_id,
410-
out_permissions);
395+
if (rc == WH_ERROR_OK) {
396+
rc = context->cb->UserGet(context->context, username, out_user_id,
397+
out_permissions);
411398

412-
(void)WH_AUTH_UNLOCK(context);
399+
(void)WH_AUTH_UNLOCK(context);
400+
} /* LOCK() */
413401
return rc;
414402
}
415403

@@ -428,15 +416,13 @@ int wh_Auth_UserSetCredentials(whAuthContext* context, whUserId user_id,
428416
}
429417

430418
rc = WH_AUTH_LOCK(context);
431-
if (rc != WH_ERROR_OK) {
432-
return rc;
433-
}
434-
435-
rc = context->cb->UserSetCredentials(
436-
context->context, user_id, method, current_credentials,
437-
current_credentials_len, new_credentials, new_credentials_len);
419+
if (rc == WH_ERROR_OK) {
420+
rc = context->cb->UserSetCredentials(
421+
context->context, user_id, method, current_credentials,
422+
current_credentials_len, new_credentials, new_credentials_len);
438423

439-
(void)WH_AUTH_UNLOCK(context);
424+
(void)WH_AUTH_UNLOCK(context);
425+
} /* LOCK() */
440426
return rc;
441427
}
442428

0 commit comments

Comments
 (0)