Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions src/wh_auth.c
Original file line number Diff line number Diff line change
Expand Up @@ -491,8 +491,8 @@ int wh_Auth_UserGet(whAuthContext* context, const char* username,

rc = WH_AUTH_LOCK(context);
if (rc == WH_ERROR_OK) {
rc = context->cb->UserGet(context->context, username, out_user_id,
out_permissions);
rc = context->cb->UserGet(context->context, context->user.user_id,
username, out_user_id, out_permissions);

(void)WH_AUTH_UNLOCK(context);
} /* LOCK() */
Expand Down
31 changes: 26 additions & 5 deletions src/wh_auth_base.c
Original file line number Diff line number Diff line change
Expand Up @@ -437,14 +437,35 @@ int wh_Auth_BaseUserSetPermissions(void* context, uint16_t current_user_id,
}


int wh_Auth_BaseUserGet(void* context, const char* username,
whUserId* out_user_id,
int wh_Auth_BaseUserGet(void* context, uint16_t current_user_id,
const char* username, whUserId* out_user_id,
whAuthPermissions* out_permissions)
{
whAuthBase_User* user = wh_Auth_BaseFindUser(username);
if (user == NULL) {
return WH_ERROR_NOTFOUND;
whAuthBase_User* user;
int is_admin;

if (current_user_id == WH_USER_ID_INVALID ||
current_user_id > WH_AUTH_BASE_MAX_USERS) {
return WH_ERROR_ACCESS;
}

is_admin = WH_AUTH_IS_ADMIN(users[current_user_id - 1].user.permissions);

user = wh_Auth_BaseFindUser(username);
if (user == NULL || user->user.user_id == WH_USER_ID_INVALID) {
/* A non-admin caller gets the same error whether or not the name
* exists, so the response is not an account-enumeration oracle. */
if (is_admin) {
return WH_ERROR_NOTFOUND;
}
return WH_ERROR_ACCESS;
}

/* A non-admin caller may only read its own record. */
if (!is_admin && user->user.user_id != current_user_id) {
return WH_ERROR_ACCESS;
}

*out_user_id = user->user.user_id;
*out_permissions = user->user.permissions;
(void)context;
Expand Down
139 changes: 139 additions & 0 deletions test-refactor/client-server/wh_test_auth.c
Original file line number Diff line number Diff line change
Expand Up @@ -561,6 +561,140 @@ static int _whTest_AuthDeleteUser_impl(whClientContext* client)
return WH_TEST_SUCCESS;
}

/* Get User Tests */
static int _whTest_AuthUserGet_impl(whClientContext* client)
{
int32_t server_rc;
whUserId admin_id = WH_USER_ID_INVALID;
whUserId getuser_id = WH_USER_ID_INVALID;
whUserId target_id = WH_USER_ID_INVALID;
whUserId fetched_id = WH_USER_ID_INVALID;
whAuthPermissions perms;
whAuthPermissions fetched_perms;
int groupIndex = (WH_MESSAGE_GROUP_AUTH >> 8) & 0xFF;
uint32_t actionWord, actionBit;

WH_AUTH_ACTION_TO_WORD_AND_BITMASK(WH_MESSAGE_AUTH_ACTION_USER_GET,
actionWord, actionBit);

/* Login as admin to create the test users */
server_rc = 0;
WH_TEST_RETURN_ON_FAIL(
_whTest_Auth_LoginOp(client, WH_AUTH_METHOD_PIN, TEST_ADMIN_USERNAME,
TEST_ADMIN_PIN, (uint16_t)strlen(TEST_ADMIN_PIN),
&server_rc, &admin_id));
WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_OK);

/* Non-admin user holding only the AUTH group USER_GET action */
memset(&perms, 0, sizeof(perms));
WH_AUTH_SET_ALLOWED_ACTION(perms, WH_MESSAGE_GROUP_AUTH,
WH_MESSAGE_AUTH_ACTION_USER_GET);
WH_AUTH_SET_IS_ADMIN(perms, 0);
server_rc = 0;
WH_TEST_RETURN_ON_FAIL(_whTest_Auth_UserAddOp(client, "getuser", perms,
WH_AUTH_METHOD_PIN, "pass", 4,
&server_rc, &getuser_id));
WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_OK);
WH_TEST_ASSERT_RETURN(getuser_id != WH_USER_ID_INVALID);

/* Second user, whose profile the non-admin must not be able to read */
memset(&perms, 0, sizeof(perms));
server_rc = 0;
WH_TEST_RETURN_ON_FAIL(_whTest_Auth_UserAddOp(client, "gettarget", perms,
WH_AUTH_METHOD_PIN, "pass", 4,
&server_rc, &target_id));
WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_OK);
WH_TEST_ASSERT_RETURN(target_id != WH_USER_ID_INVALID);

/* Switch to the non-admin session */
server_rc = 0;
_whTest_Auth_LogoutOp(client, admin_id, &server_rc);
server_rc = 0;
WH_TEST_RETURN_ON_FAIL(_whTest_Auth_LoginOp(client, WH_AUTH_METHOD_PIN,
"getuser", "pass", 4, &server_rc,
&getuser_id));
Comment thread
yosuke-wolfssl marked this conversation as resolved.
WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_OK);

/* Test 1: Non-admin getting another user's profile */
WH_TEST_PRINT(" Test: Non-admin get of another user\n");
memset(&fetched_perms, 0, sizeof(fetched_perms));
fetched_id = WH_USER_ID_INVALID;
server_rc = 0;
WH_TEST_RETURN_ON_FAIL(_whTest_Auth_UserGetOp(
client, "gettarget", &server_rc, &fetched_id, &fetched_perms));
WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_ACCESS);
/* No target identity may leak on the denied path */
WH_TEST_ASSERT_RETURN(fetched_id == WH_USER_ID_INVALID);

/* Test 2: Non-admin getting its own profile */
WH_TEST_PRINT(" Test: Non-admin get of own user\n");
memset(&fetched_perms, 0, sizeof(fetched_perms));
fetched_id = WH_USER_ID_INVALID;
server_rc = 0;
WH_TEST_RETURN_ON_FAIL(_whTest_Auth_UserGetOp(
client, "getuser", &server_rc, &fetched_id, &fetched_perms));
WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_OK);
WH_TEST_ASSERT_RETURN(fetched_id == getuser_id);
/* The granted USER_GET action must survive the round trip; not admin */
WH_TEST_ASSERT_RETURN(fetched_perms.groupPermissions[groupIndex] != 0);
WH_TEST_ASSERT_RETURN(
(fetched_perms.actionPermissions[groupIndex][actionWord] & actionBit) !=
0);
WH_TEST_ASSERT_RETURN(WH_AUTH_IS_ADMIN(fetched_perms) == 0);

/* Test 3: Non-admin getting a name that does not exist. The error must
* match the denied case so the response is not an existence oracle. */
WH_TEST_PRINT(" Test: Non-admin get of unknown user\n");
memset(&fetched_perms, 0, sizeof(fetched_perms));
fetched_id = WH_USER_ID_INVALID;
server_rc = 0;
WH_TEST_RETURN_ON_FAIL(_whTest_Auth_UserGetOp(
client, "nosuchuser", &server_rc, &fetched_id, &fetched_perms));
WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_ACCESS);
WH_TEST_ASSERT_RETURN(fetched_id == WH_USER_ID_INVALID);

/* Test 4: Admin getting another user's profile */
WH_TEST_PRINT(" Test: Admin get of another user\n");
server_rc = 0;
_whTest_Auth_LogoutOp(client, getuser_id, &server_rc);
server_rc = 0;
WH_TEST_RETURN_ON_FAIL(
_whTest_Auth_LoginOp(client, WH_AUTH_METHOD_PIN, TEST_ADMIN_USERNAME,
TEST_ADMIN_PIN, (uint16_t)strlen(TEST_ADMIN_PIN),
&server_rc, &admin_id));
WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_OK);

memset(&fetched_perms, 0, sizeof(fetched_perms));
fetched_id = WH_USER_ID_INVALID;
server_rc = 0;
WH_TEST_RETURN_ON_FAIL(_whTest_Auth_UserGetOp(
client, "gettarget", &server_rc, &fetched_id, &fetched_perms));
WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_OK);
WH_TEST_ASSERT_RETURN(fetched_id == target_id);
/* gettarget was created with empty permissions and is not admin */
WH_TEST_ASSERT_RETURN(fetched_perms.groupPermissions[groupIndex] == 0);
WH_TEST_ASSERT_RETURN(WH_AUTH_IS_ADMIN(fetched_perms) == 0);

/* Test 5: Admin getting a name that does not exist. Unlike a non-admin,
* an admin is allowed to learn that the account is absent. */
WH_TEST_PRINT(" Test: Admin get of unknown user\n");
memset(&fetched_perms, 0, sizeof(fetched_perms));
fetched_id = WH_USER_ID_INVALID;
server_rc = 0;
WH_TEST_RETURN_ON_FAIL(_whTest_Auth_UserGetOp(
client, "nosuchuser", &server_rc, &fetched_id, &fetched_perms));
WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_NOTFOUND);
WH_TEST_ASSERT_RETURN(fetched_id == WH_USER_ID_INVALID);
Comment thread
yosuke-wolfssl marked this conversation as resolved.

/* Cleanup */
_whTest_Auth_DeleteUserByName(client, "getuser");
_whTest_Auth_DeleteUserByName(client, "gettarget");
server_rc = 0;
_whTest_Auth_LogoutOp(client, admin_id, &server_rc);

return WH_TEST_SUCCESS;
}

/* Set User Permissions Tests */
static int _whTest_AuthSetPermissions_impl(whClientContext* client)
{
Expand Down Expand Up @@ -1101,6 +1235,11 @@ int whTest_AuthDeleteUser(whClientContext* client)
return _whTest_Auth_RunBracketed(client, _whTest_AuthDeleteUser_impl);
}

int whTest_AuthUserGet(whClientContext* client)
{
return _whTest_Auth_RunBracketed(client, _whTest_AuthUserGet_impl);
}

int whTest_AuthSetPermissions(whClientContext* client)
{
return _whTest_Auth_RunBracketed(client, _whTest_AuthSetPermissions_impl);
Expand Down
2 changes: 2 additions & 0 deletions test-refactor/wh_test_list.c
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ WH_TEST_DECL(whTest_AuthLogin);
WH_TEST_DECL(whTest_AuthLogout);
WH_TEST_DECL(whTest_AuthAddUser);
WH_TEST_DECL(whTest_AuthDeleteUser);
WH_TEST_DECL(whTest_AuthUserGet);
WH_TEST_DECL(whTest_AuthSetPermissions);
WH_TEST_DECL(whTest_AuthSetCredentials);
WH_TEST_DECL(whTest_AuthRequestAuthorization);
Expand Down Expand Up @@ -159,6 +160,7 @@ const whTestCase whTestsClient[] = {
{"whTest_AuthLogout", whTest_AuthLogout},
{"whTest_AuthAddUser", whTest_AuthAddUser},
{"whTest_AuthDeleteUser", whTest_AuthDeleteUser},
{"whTest_AuthUserGet", whTest_AuthUserGet},
{"whTest_AuthSetPermissions", whTest_AuthSetPermissions},
{"whTest_AuthSetCredentials", whTest_AuthSetCredentials},
{"whTest_AuthRequestAuthorization", whTest_AuthRequestAuthorization},
Expand Down
6 changes: 5 additions & 1 deletion wolfhsm/wh_auth.h
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,8 @@ typedef struct {
whUserId user_id, whAuthPermissions permissions);

/* Get user information by username */
int (*UserGet)(void* context, const char* username, whUserId* out_user_id,
int (*UserGet)(void* context, whUserId current_user_id,
const char* username, whUserId* out_user_id,
whAuthPermissions* out_permissions);
Comment thread
yosuke-wolfssl marked this conversation as resolved.

/* Set user credentials (PIN, etc.) */
Expand Down Expand Up @@ -349,6 +350,9 @@ int wh_Auth_UserSetPermissions(whAuthContext* context, whUserId user_id,
/**
* @brief Get user information.
*
* The caller's own user id gates access: a non-admin caller may only read its
* own record, and a denied or missing lookup both return WH_ERROR_ACCESS.
*
* @param[in] context Pointer to the auth context.
* @param[in] username The username to look up.
* @param[out] out_user_id Pointer to store the user ID.
Expand Down
9 changes: 7 additions & 2 deletions wolfhsm/wh_auth_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -127,14 +127,19 @@ int wh_Auth_BaseUserSetPermissions(void* context, uint16_t current_user_id,
/**
* @brief Get user information by username.
*
* A non-admin caller may only read its own record; an admin caller may read
* any record. To a non-admin, a denied and a missing name both return
* WH_ERROR_ACCESS.
*
* @param[in] context Pointer to the auth base context.
* @param[in] current_user_id The user ID of the caller performing the lookup.
* @param[in] username The username to look up.
* @param[out] out_user_id Pointer to store the user ID.
* @param[out] out_permissions Pointer to store the user permissions.
* @return int Returns 0 on success, or a negative error code on failure.
*/
int wh_Auth_BaseUserGet(void* context, const char* username,
whUserId* out_user_id,
int wh_Auth_BaseUserGet(void* context, uint16_t current_user_id,
const char* username, whUserId* out_user_id,
whAuthPermissions* out_permissions);

/**
Expand Down
Loading