Skip to content

Commit a5405f3

Browse files
move most of authentication logic into wolfHSM rather than in port, touch up for test cases
1 parent 07c88c1 commit a5405f3

6 files changed

Lines changed: 113 additions & 115 deletions

File tree

port/posix/posix_auth.c

Lines changed: 17 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -246,111 +246,32 @@ int posixAuth_Logout(void* context, uint16_t current_user_id,
246246
}
247247

248248

249-
int posixAuth_CheckRequestAuthorization(void* context, uint16_t user_id,
250-
uint16_t group, uint16_t action)
249+
int posixAuth_CheckRequestAuthorization(void* context, int err,
250+
uint16_t user_id, uint16_t group, uint16_t action)
251251
{
252-
int rc;
253-
254-
if (user_id == WH_USER_ID_INVALID) {
255-
/* allow user login request attempt and comm */
256-
if (group == WH_MESSAGE_GROUP_COMM ||
257-
(group == WH_MESSAGE_GROUP_AUTH &&
258-
action == WH_MESSAGE_AUTH_ACTION_LOGIN)) {
259-
rc = WH_ERROR_OK;
260-
}
261-
else {
262-
rc = WH_ERROR_ACCESS;
263-
}
264-
}
265-
else {
266-
int groupIndex = (group >> 8) & 0xFF;
267-
whAuthBase_User* user;
268-
269-
if (user_id > WH_AUTH_BASE_MAX_USERS) {
270-
return WH_ERROR_ACCESS;
271-
}
272-
user = &users[user_id - 1];
273-
274-
/* check if user has permissions for the group and action */
275-
276-
/* some operations a user logged in should by default have access to;
277-
* - logging out
278-
* - updating own credentials */
279-
if (group == WH_MESSAGE_GROUP_AUTH &&
280-
(action == WH_MESSAGE_AUTH_ACTION_LOGOUT ||
281-
action == WH_MESSAGE_AUTH_ACTION_USER_SET_CREDENTIALS)) {
282-
rc = WH_ERROR_OK;
283-
}
284-
else {
285-
if (user->user.permissions.groupPermissions & group) {
286-
/* Check if action is within supported range */
287-
if (action < WH_AUTH_ACTIONS_PER_GROUP) {
288-
/* Get word index and bitmask for this action */
289-
uint32_t wordAndBit = WH_AUTH_ACTION_TO_WORD_AND_BIT(action);
290-
uint32_t wordIndex = WH_AUTH_ACTION_WORD(wordAndBit);
291-
uint32_t bitmask = WH_AUTH_ACTION_BIT(wordAndBit);
292-
293-
if (wordIndex < WH_AUTH_ACTION_WORDS &&
294-
(user->user.permissions.actionPermissions[groupIndex]
295-
[wordIndex] &
296-
bitmask)) {
297-
rc = WH_ERROR_OK;
298-
}
299-
else {
300-
rc = WH_ERROR_ACCESS;
301-
}
302-
}
303-
else {
304-
rc = WH_ERROR_ACCESS;
305-
}
306-
}
307-
else {
308-
rc = WH_ERROR_ACCESS;
309-
}
310-
}
311-
}
312-
313252
(void)context;
314-
return rc;
253+
(void)user_id;
254+
(void)group;
255+
(void)action;
256+
257+
/* could override the error code here */
258+
/* the value passed in as 'err' is the current error code */
259+
return err;
315260
}
316261

317262
/* authorization check on key usage after the request has been parsed and before
318263
* the action is done */
319-
int posixAuth_CheckKeyAuthorization(void* context, uint16_t user_id,
264+
int posixAuth_CheckKeyAuthorization(void* context, int err, uint16_t user_id,
320265
uint32_t key_id, uint16_t action)
321266
{
322-
int rc = WH_ERROR_ACCESS;
323-
int i;
324-
whAuthBase_User* user;
325-
326-
if (user_id == WH_USER_ID_INVALID) {
327-
return WH_ERROR_ACCESS;
328-
}
329-
330-
if (user_id > WH_AUTH_BASE_MAX_USERS) {
331-
return WH_ERROR_NOTFOUND;
332-
}
333-
334-
user = &users[user_id - 1];
335-
336-
if (user->user.user_id == WH_USER_ID_INVALID) {
337-
return WH_ERROR_NOTFOUND;
338-
}
339-
340-
/* Check if the requested key_id is in the user's keyIds array */
341-
for (i = 0;
342-
i < user->user.permissions.keyIdCount && i < WH_AUTH_MAX_KEY_IDS;
343-
i++) {
344-
if (user->user.permissions.keyIds[i] == key_id) {
345-
rc = WH_ERROR_OK;
346-
break;
347-
}
348-
}
349-
350267
(void)context;
351-
(void)action; /* Action could be used for future fine-grained key access
352-
control */
353-
return rc;
268+
(void)user_id;
269+
(void)key_id;
270+
(void)action;
271+
272+
/* could override the error code here */
273+
/* the value passed in as 'err' is the current error code */
274+
return err;
354275
}
355276

356277

port/posix/posix_auth.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,20 +81,21 @@ int posixAuth_Logout(void* context, uint16_t current_user_id,
8181
uint16_t user_id);
8282

8383
/**
84-
* @brief Check if an action is authorized for a session.
84+
* @brief Option to override authorization check.
8585
*
8686
* @param[in] context Pointer to the auth base context.
87+
* @param[in] err The current error code set for check authorization.
8788
* @param[in] user_id The user ID to check authorization for.
8889
* @param[in] group The group to check authorization for.
8990
* @param[in] action The action to check authorization for.
9091
* @return int Returns 0 if authorized, or a negative error code on failure.
9192
*/
92-
int posixAuth_CheckRequestAuthorization(void* context, uint16_t user_id,
93+
int posixAuth_CheckRequestAuthorization(void* context, int err, uint16_t user_id,
9394
uint16_t group, uint16_t action);
9495

9596
/* authorization check on key usage after the request has been parsed and before
9697
* the action is done */
97-
int posixAuth_CheckKeyAuthorization(void* context, uint16_t user_id,
98+
int posixAuth_CheckKeyAuthorization(void* context, int err, uint16_t user_id,
9899
uint32_t key_id, uint16_t action);
99100

100101
/**

src/wh_auth.c

Lines changed: 85 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
#include "wolfhsm/wh_error.h"
4848

4949
#include "wolfhsm/wh_auth.h"
50+
#include "wolfhsm/wh_message_auth.h"
5051

5152

5253
int wh_Auth_Init(whAuthContext* context, const whAuthConfig* config)
@@ -155,17 +156,72 @@ int wh_Auth_CheckRequestAuthorization(whAuthContext* context, uint16_t group,
155156
{
156157
uint16_t user_id;
157158
int rc;
159+
whAuthUser* user;
158160

159-
if ((context == NULL) || (context->cb == NULL) ||
160-
(context->cb->CheckRequestAuthorization == NULL)) {
161+
if ((context == NULL) || (context->cb == NULL)) {
161162
return WH_ERROR_BADARGS;
162163
}
163164

164-
user_id = context->user.user_id;
165+
user = &context->user;
166+
user_id = user->user_id;
165167
/* @TODO add logging call here and with resulting return value */
166168

167-
rc = context->cb->CheckRequestAuthorization(context->context, user_id,
168-
group, action);
169+
if (user_id == WH_USER_ID_INVALID) {
170+
/* allow user login request attempt and comm */
171+
if (group == WH_MESSAGE_GROUP_COMM ||
172+
(group == WH_MESSAGE_GROUP_AUTH &&
173+
action == WH_MESSAGE_AUTH_ACTION_LOGIN)) {
174+
rc = WH_ERROR_OK;
175+
}
176+
else {
177+
rc = WH_ERROR_ACCESS;
178+
}
179+
}
180+
else {
181+
int groupIndex = (group >> 8) & 0xFF;
182+
183+
/* some operations a user logged in should by default have access to;
184+
* - logging out
185+
* - updating own credentials */
186+
if (group == WH_MESSAGE_GROUP_AUTH &&
187+
(action == WH_MESSAGE_AUTH_ACTION_LOGOUT ||
188+
action == WH_MESSAGE_AUTH_ACTION_USER_SET_CREDENTIALS)) {
189+
rc = WH_ERROR_OK;
190+
}
191+
else {
192+
if (user->permissions.groupPermissions & group) {
193+
/* Check if action is within supported range */
194+
if (action < WH_AUTH_ACTIONS_PER_GROUP) {
195+
/* Get word index and bitmask for this action */
196+
uint32_t wordAndBit = WH_AUTH_ACTION_TO_WORD_AND_BIT(action);
197+
uint32_t wordIndex = WH_AUTH_ACTION_WORD(wordAndBit);
198+
uint32_t bitmask = WH_AUTH_ACTION_BIT(wordAndBit);
199+
200+
if (wordIndex < WH_AUTH_ACTION_WORDS &&
201+
(user->permissions.actionPermissions[groupIndex]
202+
[wordIndex] &
203+
bitmask)) {
204+
rc = WH_ERROR_OK;
205+
}
206+
else {
207+
rc = WH_ERROR_ACCESS;
208+
}
209+
}
210+
else {
211+
rc = WH_ERROR_ACCESS;
212+
}
213+
}
214+
else {
215+
rc = WH_ERROR_ACCESS;
216+
}
217+
}
218+
}
219+
220+
/* allow authorization override if callback is set */
221+
if (context->cb->CheckRequestAuthorization != NULL) {
222+
rc = context->cb->CheckRequestAuthorization(context->context, rc,
223+
user_id, group, action);
224+
}
169225
return rc;
170226
}
171227

@@ -176,16 +232,36 @@ int wh_Auth_CheckKeyAuthorization(whAuthContext* context, uint32_t key_id,
176232
{
177233
uint16_t user_id;
178234
int rc;
235+
int i;
236+
whAuthUser* user;
179237

180-
if ((context == NULL) || (context->cb == NULL) ||
181-
(context->cb->CheckKeyAuthorization == NULL)) {
238+
if ((context == NULL) || (context->cb == NULL)) {
182239
return WH_ERROR_BADARGS;
183240
}
184241

185242
user_id = context->user.user_id;
243+
user = &context->user;
244+
if (user->user_id == WH_USER_ID_INVALID) {
245+
return WH_ERROR_ACCESS;
246+
}
186247

187-
rc = context->cb->CheckKeyAuthorization(context->context, user_id, key_id,
188-
action);
248+
/* Check if the requested key_id is in the user's keyIds array */
249+
for (i = 0;
250+
i < user->permissions.keyIdCount && i < WH_AUTH_MAX_KEY_IDS;
251+
i++) {
252+
if (user->permissions.keyIds[i] == key_id) {
253+
rc = WH_ERROR_OK;
254+
break;
255+
}
256+
}
257+
258+
(void)context;
259+
(void)action; /* Action could be used for future fine-grained key access
260+
control */
261+
if (context->cb->CheckKeyAuthorization != NULL) {
262+
rc = context->cb->CheckKeyAuthorization(context->context, rc,
263+
user_id, key_id, action);
264+
}
189265
return rc;
190266
}
191267

test/wh_test_auth.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1177,11 +1177,11 @@ static int CheckServerSupportsAuth(whClientContext* client_ctx)
11771177
whUserId user_id;
11781178
int isSupported = 0;
11791179

1180-
WH_TEST_RETURN_ON_FAIL(wh_Client_AuthLogin(client_ctx, WH_AUTH_METHOD_PIN,
1180+
WH_TEST_RETURN_ON_FAIL(_whTest_Auth_LoginOp(client_ctx, WH_AUTH_METHOD_PIN,
11811181
TEST_ADMIN_USERNAME, TEST_ADMIN_PIN, strlen(TEST_ADMIN_PIN), &server_rc,
11821182
&user_id));
11831183
if (server_rc != WH_AUTH_NOT_ENABLED) {
1184-
WH_TEST_RETURN_ON_FAIL(wh_Client_AuthLogout(client_ctx, user_id,
1184+
WH_TEST_RETURN_ON_FAIL(_whTest_Auth_LogoutOp(client_ctx, user_id,
11851185
&server_rc));
11861186
isSupported = 1;
11871187
}

test/wh_test_she.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ int whTest_SheClientConfig(whClientConfig* config)
179179
/* Attempt log in as an admin user for the rest of the tests */
180180
WH_TEST_RETURN_ON_FAIL(wh_Client_AuthLogin(client, WH_AUTH_METHOD_PIN,
181181
TEST_ADMIN_USERNAME, TEST_ADMIN_PIN, strlen(TEST_ADMIN_PIN),
182-
NULL, NULL));
182+
&ret, NULL));
183183
#endif /* WOLFHSM_CFG_ENABLE_AUTHENTICATION */
184184

185185
{

wolfhsm/wh_auth.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -112,12 +112,12 @@ typedef struct {
112112
int (*Logout)(void* context, whUserId current_user_id, whUserId user_id);
113113

114114

115-
/* Check if an action is authorized for a session */
116-
int (*CheckRequestAuthorization)(void* context, uint16_t user_id,
115+
/* Allow override of if action is authorized for a user */
116+
int (*CheckRequestAuthorization)(void* context, int err, uint16_t user_id,
117117
uint16_t group, uint16_t action);
118118

119-
/* Check if a key is authorized for use */
120-
int (*CheckKeyAuthorization)(void* context, uint16_t user_id,
119+
/* Allow override of if a key is authorized for use */
120+
int (*CheckKeyAuthorization)(void* context, int err, uint16_t user_id,
121121
uint32_t key_id, uint16_t action);
122122

123123
/* Add a new user */

0 commit comments

Comments
 (0)