diff --git a/src/wh_auth.c b/src/wh_auth.c index 95db35106..3e232ac93 100644 --- a/src/wh_auth.c +++ b/src/wh_auth.c @@ -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() */ diff --git a/src/wh_auth_base.c b/src/wh_auth_base.c index 9b49f7001..5ce389946 100644 --- a/src/wh_auth_base.c +++ b/src/wh_auth_base.c @@ -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; diff --git a/test-refactor/client-server/wh_test_auth.c b/test-refactor/client-server/wh_test_auth.c index 34eb55dac..c0628fa8e 100644 --- a/test-refactor/client-server/wh_test_auth.c +++ b/test-refactor/client-server/wh_test_auth.c @@ -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)); + 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); + + /* 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) { @@ -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); diff --git a/test-refactor/wh_test_list.c b/test-refactor/wh_test_list.c index 70f7d0396..7785c97e3 100644 --- a/test-refactor/wh_test_list.c +++ b/test-refactor/wh_test_list.c @@ -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); @@ -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}, diff --git a/wolfhsm/wh_auth.h b/wolfhsm/wh_auth.h index 26834d0a9..ab3db3b8b 100644 --- a/wolfhsm/wh_auth.h +++ b/wolfhsm/wh_auth.h @@ -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); /* Set user credentials (PIN, etc.) */ @@ -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. diff --git a/wolfhsm/wh_auth_base.h b/wolfhsm/wh_auth_base.h index 0f966b3cd..30222155c 100644 --- a/wolfhsm/wh_auth_base.h +++ b/wolfhsm/wh_auth_base.h @@ -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); /**