Skip to content

Commit 724cafe

Browse files
JacobBarthelmehJacobBarthelmeh
committed
add admin boolean to permissions, aditional delete user test case, unique max credentials, add force zero to utils
1 parent ab3bfca commit 724cafe

7 files changed

Lines changed: 197 additions & 65 deletions

File tree

‎src/wh_auth_base.c‎

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929

3030
#include "wolfhsm/wh_common.h"
3131
#include "wolfhsm/wh_error.h"
32+
#include "wolfhsm/wh_utils.h"
3233

3334
#include "wolfhsm/wh_message.h"
3435
#include "wolfhsm/wh_message_auth.h"
@@ -70,6 +71,7 @@ int wh_Auth_BaseInit(void* context, const void* config)
7071
int wh_Auth_BaseCleanup(void* context)
7172
{
7273
(void)context;
74+
wh_Utils_ForceZero(users, sizeof(users));
7375
return WH_ERROR_OK;
7476
}
7577

@@ -231,16 +233,20 @@ int wh_Auth_BaseLogout(void* context, uint16_t current_user_id,
231233
{
232234
whAuthBase_User* user;
233235

234-
if (user_id == WH_USER_ID_INVALID) {
236+
if (current_user_id == WH_USER_ID_INVALID ||
237+
user_id == WH_USER_ID_INVALID) {
235238
return WH_ERROR_BADARGS;
236239
}
237240

238-
if (user_id > WH_AUTH_BASE_MAX_USERS) {
241+
if (current_user_id > WH_AUTH_BASE_MAX_USERS ||
242+
user_id > WH_AUTH_BASE_MAX_USERS) {
239243
return WH_ERROR_NOTFOUND;
240244
}
241245

242-
/* @TODO there likely should be restrictions here on who can logout who */
243-
(void)current_user_id;
246+
if (current_user_id != user_id &&
247+
!WH_AUTH_IS_ADMIN(users[current_user_id - 1].user.permissions)) {
248+
return WH_ERROR_ACCESS;
249+
}
244250

245251
user = &users[user_id - 1];
246252
user->user.is_active = false;
@@ -348,14 +354,23 @@ int wh_Auth_BaseUserDelete(void* context, uint16_t current_user_id,
348354
return WH_ERROR_NOTFOUND;
349355
}
350356

357+
if (current_user_id == WH_USER_ID_INVALID ||
358+
current_user_id > WH_AUTH_BASE_MAX_USERS) {
359+
return WH_ERROR_BADARGS;
360+
}
361+
362+
/* Only allow an admin user to delete users */
363+
if (!WH_AUTH_IS_ADMIN(users[current_user_id - 1].user.permissions)) {
364+
return WH_ERROR_ACCESS;
365+
}
366+
351367
user = &users[user_id - 1];
352368
if (user->user.user_id == WH_USER_ID_INVALID) {
353369
return WH_ERROR_NOTFOUND;
354370
}
355371

356372
memset(user, 0, sizeof(whAuthBase_User));
357373
(void)context;
358-
(void)current_user_id;
359374
return WH_ERROR_OK;
360375
}
361376

@@ -369,6 +384,11 @@ int wh_Auth_BaseUserSetPermissions(void* context, uint16_t current_user_id,
369384
return WH_ERROR_NOTFOUND;
370385
}
371386

387+
/* Only allow an admin user to change permissions */
388+
if (!WH_AUTH_IS_ADMIN(users[current_user_id - 1].user.permissions)) {
389+
return WH_ERROR_ACCESS;
390+
}
391+
372392
user = &users[user_id - 1];
373393
if (user->user.user_id == WH_USER_ID_INVALID) {
374394
return WH_ERROR_NOTFOUND;
@@ -387,7 +407,6 @@ int wh_Auth_BaseUserSetPermissions(void* context, uint16_t current_user_id,
387407
}
388408
}
389409
(void)context;
390-
(void)current_user_id;
391410
return WH_ERROR_OK;
392411
}
393412

‎src/wh_client_auth.c‎

Lines changed: 54 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040

4141
#include "wolfhsm/wh_client.h"
4242
#include "wolfhsm/wh_auth.h"
43+
#include "wolfhsm/wh_utils.h"
4344

4445
/* Does not find the user name in the list, only verifies that the user name is
4546
* not too long and not null. */
@@ -64,6 +65,7 @@ int wh_Client_AuthLoginRequest(whClientContext* c, whAuthMethod method,
6465
whMessageAuth_LoginRequest* msg = (whMessageAuth_LoginRequest*)buffer;
6566
uint8_t* msg_auth_data = buffer + sizeof(*msg);
6667
size_t msg_size;
68+
int rc;
6769

6870
if (c == NULL) {
6971
return WH_ERROR_BADARGS;
@@ -73,7 +75,7 @@ int wh_Client_AuthLoginRequest(whClientContext* c, whAuthMethod method,
7375
return WH_ERROR_BADARGS;
7476
}
7577

76-
if (auth_data_len > WH_MESSAGE_AUTH_MAX_CREDENTIALS_LEN) {
78+
if (auth_data_len > WH_MESSAGE_AUTH_LOGIN_MAX_AUTH_DATA_LEN) {
7779
return WH_ERROR_BADARGS;
7880
}
7981

@@ -94,9 +96,14 @@ int wh_Client_AuthLoginRequest(whClientContext* c, whAuthMethod method,
9496
memcpy(msg_auth_data, auth_data, auth_data_len);
9597
}
9698

97-
return wh_Client_SendRequest(c, WH_MESSAGE_GROUP_AUTH,
98-
WH_MESSAGE_AUTH_ACTION_LOGIN,
99-
(uint16_t)msg_size, buffer);
99+
rc = wh_Client_SendRequest(c, WH_MESSAGE_GROUP_AUTH,
100+
WH_MESSAGE_AUTH_ACTION_LOGIN,
101+
(uint16_t)msg_size, buffer);
102+
103+
/* Zeroize sensitive credential data before returning */
104+
wh_Utils_ForceZero(buffer, sizeof(buffer));
105+
106+
return rc;
100107
}
101108

102109
int wh_Client_AuthLoginResponse(whClientContext* c, int32_t* out_rc,
@@ -252,6 +259,7 @@ int wh_Client_AuthUserAddRequest(whClientContext* c, const char* username,
252259
whMessageAuth_UserAddRequest* msg = (whMessageAuth_UserAddRequest*)buffer;
253260
uint8_t* msg_credentials = buffer + sizeof(*msg);
254261
size_t msg_size;
262+
int rc = WH_ERROR_OK;
255263

256264
if (c == NULL) {
257265
return WH_ERROR_BADARGS;
@@ -266,28 +274,41 @@ int wh_Client_AuthUserAddRequest(whClientContext* c, const char* username,
266274

267275
if (wh_MessageAuth_FlattenPermissions(&permissions, msg->permissions,
268276
sizeof(msg->permissions)) != 0) {
269-
return WH_ERROR_BUFFER_SIZE;
270-
}
271-
272-
msg->method = method;
273-
msg->credentials_len = credentials_len;
274-
if (credentials_len > 0) {
275-
if (credentials == NULL) {
276-
return WH_ERROR_BADARGS;
277+
rc = WH_ERROR_BUFFER_SIZE;
278+
}
279+
else {
280+
msg->method = method;
281+
msg->credentials_len = credentials_len;
282+
if (credentials_len > 0) {
283+
if (credentials == NULL) {
284+
rc = WH_ERROR_BADARGS;
285+
}
286+
else if (credentials_len >
287+
WH_MESSAGE_AUTH_USERADD_MAX_CREDENTIALS_LEN) {
288+
rc = WH_ERROR_BUFFER_SIZE;
289+
}
290+
else {
291+
memcpy(msg_credentials, credentials, credentials_len);
292+
}
277293
}
278-
if (credentials_len > WH_MESSAGE_AUTH_MAX_CREDENTIALS_LEN) {
279-
return WH_ERROR_BUFFER_SIZE;
294+
295+
if (rc == WH_ERROR_OK) {
296+
msg_size = sizeof(*msg) + credentials_len;
297+
if (msg_size > WOLFHSM_CFG_COMM_DATA_LEN) {
298+
rc = WH_ERROR_BUFFER_SIZE;
299+
}
300+
else {
301+
rc = wh_Client_SendRequest(c, WH_MESSAGE_GROUP_AUTH,
302+
WH_MESSAGE_AUTH_ACTION_USER_ADD,
303+
(uint16_t)msg_size, buffer);
304+
}
280305
}
281-
memcpy(msg_credentials, credentials, credentials_len);
282306
}
283307

284-
msg_size = sizeof(*msg) + credentials_len;
285-
if (msg_size > WOLFHSM_CFG_COMM_DATA_LEN) {
286-
return WH_ERROR_BUFFER_SIZE;
287-
}
288-
return wh_Client_SendRequest(c, WH_MESSAGE_GROUP_AUTH,
289-
WH_MESSAGE_AUTH_ACTION_USER_ADD,
290-
(uint16_t)msg_size, buffer);
308+
/* Zeroize sensitive credential data before returning */
309+
wh_Utils_ForceZero(buffer, sizeof(buffer));
310+
311+
return rc;
291312
}
292313

293314
int wh_Client_AuthUserAddResponse(whClientContext* c, int32_t* out_rc,
@@ -597,15 +618,16 @@ int wh_Client_AuthUserSetCredentialsRequest(
597618
uint8_t* msg_current_creds = buffer + sizeof(*msg);
598619
uint8_t* msg_new_creds = msg_current_creds + current_credentials_len;
599620
uint16_t total_size;
621+
int rc;
600622

601623
if (c == NULL) {
602624
return WH_ERROR_BADARGS;
603625
}
604626

605-
if (current_credentials_len > WH_MESSAGE_AUTH_MAX_CREDENTIALS_LEN) {
627+
if (current_credentials_len > WH_MESSAGE_AUTH_SETCREDS_MAX_CREDENTIALS_LEN) {
606628
return WH_ERROR_BUFFER_SIZE;
607629
}
608-
if (new_credentials_len > WH_MESSAGE_AUTH_MAX_CREDENTIALS_LEN) {
630+
if (new_credentials_len > WH_MESSAGE_AUTH_SETCREDS_MAX_CREDENTIALS_LEN) {
609631
return WH_ERROR_BUFFER_SIZE;
610632
}
611633
if (current_credentials_len > 0 && current_credentials == NULL) {
@@ -635,9 +657,14 @@ int wh_Client_AuthUserSetCredentialsRequest(
635657
memcpy(msg_new_creds, new_credentials, new_credentials_len);
636658
}
637659

638-
return wh_Client_SendRequest(c, WH_MESSAGE_GROUP_AUTH,
639-
WH_MESSAGE_AUTH_ACTION_USER_SET_CREDENTIALS,
640-
total_size, buffer);
660+
rc = wh_Client_SendRequest(c, WH_MESSAGE_GROUP_AUTH,
661+
WH_MESSAGE_AUTH_ACTION_USER_SET_CREDENTIALS,
662+
total_size, buffer);
663+
664+
/* Zeroize sensitive credential data before returning */
665+
wh_Utils_ForceZero(buffer, sizeof(buffer));
666+
667+
return rc;
641668
}
642669

643670
int wh_Client_AuthUserSetCredentialsResponse(whClientContext* c,

‎src/wh_message_auth.c‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ int wh_MessageAuth_TranslateLoginRequest(
7676
WH_T16(magic, dest_header, src_header, auth_data_len);
7777

7878
expected_size = (uint16_t)(header_size + dest_header->auth_data_len);
79-
if (dest_header->auth_data_len > WH_MESSAGE_AUTH_MAX_CREDENTIALS_LEN ||
79+
if (dest_header->auth_data_len > WH_MESSAGE_AUTH_LOGIN_MAX_AUTH_DATA_LEN ||
8080
src_size < expected_size) {
8181
return WH_ERROR_BADARGS;
8282
}
@@ -126,8 +126,9 @@ int wh_MessageAuth_FlattenPermissions(whAuthPermissions* permissions,
126126
return WH_ERROR_BADARGS;
127127
}
128128

129-
/* Serialize groupPermissions array (WH_NUMBER_OF_GROUPS bytes) */
130-
for (i = 0; i < WH_NUMBER_OF_GROUPS; i++) {
129+
/* Serialize groupPermissions array (WH_NUMBER_OF_GROUPS + 1 bytes)
130+
* The last byte is the admin flag */
131+
for (i = 0; i < WH_NUMBER_OF_GROUPS + 1; i++) {
131132
buffer[idx++] = permissions->groupPermissions[i];
132133
}
133134

@@ -181,8 +182,9 @@ int wh_MessageAuth_UnflattenPermissions(uint8_t* buffer, uint16_t buffer_len,
181182
return WH_ERROR_BADARGS;
182183
}
183184

184-
/* Deserialize groupPermissions array (WH_NUMBER_OF_GROUPS bytes) */
185-
for (i = 0; i < WH_NUMBER_OF_GROUPS; i++) {
185+
/* Deserialize groupPermissions array (WH_NUMBER_OF_GROUPS + 1 bytes)
186+
* The last byte is the admin flag */
187+
for (i = 0; i < WH_NUMBER_OF_GROUPS + 1; i++) {
186188
permissions->groupPermissions[i] = buffer[idx++];
187189
}
188190

@@ -253,7 +255,7 @@ int wh_MessageAuth_TranslateUserAddRequest(
253255
WH_T16(magic, dest_header, src_header, credentials_len);
254256

255257
expected_size = (uint16_t)(header_size + dest_header->credentials_len);
256-
if (dest_header->credentials_len > WH_MESSAGE_AUTH_MAX_CREDENTIALS_LEN ||
258+
if (dest_header->credentials_len > WH_MESSAGE_AUTH_USERADD_MAX_CREDENTIALS_LEN ||
257259
src_size < expected_size) {
258260
return WH_ERROR_BUFFER_SIZE;
259261
}
@@ -362,10 +364,11 @@ int wh_MessageAuth_TranslateUserSetCredentialsRequest(
362364
WH_T16(magic, dest_header, src_header, new_credentials_len);
363365

364366
if (src_header->current_credentials_len >
365-
WH_MESSAGE_AUTH_MAX_CREDENTIALS_LEN) {
367+
WH_MESSAGE_AUTH_SETCREDS_MAX_CREDENTIALS_LEN) {
366368
return WH_ERROR_BUFFER_SIZE;
367369
}
368-
if (src_header->new_credentials_len > WH_MESSAGE_AUTH_MAX_CREDENTIALS_LEN) {
370+
if (src_header->new_credentials_len >
371+
WH_MESSAGE_AUTH_SETCREDS_MAX_CREDENTIALS_LEN) {
369372
return WH_ERROR_BUFFER_SIZE;
370373
}
371374

‎src/wh_server_auth.c‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,10 @@ int wh_Server_HandleAuthRequest(whServerContext* server, uint16_t magic,
6565
switch (action) {
6666

6767
case WH_MESSAGE_AUTH_ACTION_LOGIN: {
68-
whMessageAuth_LoginRequest req = {0};
69-
whMessageAuth_LoginResponse resp = {0};
70-
int loggedIn = 0;
71-
uint8_t auth_data[WH_MESSAGE_AUTH_MAX_CREDENTIALS_LEN] = {0};
68+
whMessageAuth_LoginRequest req = {0};
69+
whMessageAuth_LoginResponse resp = {0};
70+
int loggedIn = 0;
71+
uint8_t auth_data[WH_MESSAGE_AUTH_LOGIN_MAX_AUTH_DATA_LEN] = {0};
7272

7373
rc = wh_MessageAuth_TranslateLoginRequest(
7474
magic, req_packet, req_size, &req, auth_data);
@@ -118,10 +118,10 @@ int wh_Server_HandleAuthRequest(whServerContext* server, uint16_t magic,
118118
} break;
119119

120120
case WH_MESSAGE_AUTH_ACTION_USER_ADD: {
121-
whMessageAuth_UserAddRequest req = {0};
122-
whMessageAuth_UserAddResponse resp = {0};
123-
whAuthPermissions permissions = {0};
124-
uint8_t credentials[WH_MESSAGE_AUTH_MAX_CREDENTIALS_LEN] = {0};
121+
whMessageAuth_UserAddRequest req = {0};
122+
whMessageAuth_UserAddResponse resp = {0};
123+
whAuthPermissions permissions = {0};
124+
uint8_t credentials[WH_MESSAGE_AUTH_USERADD_MAX_CREDENTIALS_LEN] = {0};
125125

126126
rc = wh_MessageAuth_TranslateUserAddRequest(
127127
magic, req_packet, req_size, &req, credentials);
@@ -223,10 +223,10 @@ int wh_Server_HandleAuthRequest(whServerContext* server, uint16_t magic,
223223
} break;
224224

225225
case WH_MESSAGE_AUTH_ACTION_USER_SET_CREDENTIALS: {
226-
whMessageAuth_UserSetCredentialsRequest req_header = {0};
227-
uint8_t current_creds[WH_MESSAGE_AUTH_MAX_CREDENTIALS_LEN] = {0};
228-
uint8_t new_creds[WH_MESSAGE_AUTH_MAX_CREDENTIALS_LEN] = {0};
229-
whMessageAuth_SimpleResponse resp = {0};
226+
whMessageAuth_UserSetCredentialsRequest req_header = {0};
227+
uint8_t current_creds[WH_MESSAGE_AUTH_SETCREDS_MAX_CREDENTIALS_LEN] = {0};
228+
uint8_t new_creds[WH_MESSAGE_AUTH_SETCREDS_MAX_CREDENTIALS_LEN] = {0};
229+
whMessageAuth_SimpleResponse resp = {0};
230230
uint16_t min_size = sizeof(whMessageAuth_UserSetCredentialsRequest);
231231

232232
if (req_size < min_size) {

0 commit comments

Comments
 (0)