Fix stale session permissions after wh_Auth_UserSetPermissions

Author: yosuke-wolfssl

docs/src/5-Features.md

@@ -1201,6 +1201,17 @@ Credential updates through `wh_Auth_UserSetCredentials` add a further check on t
 
 `wh_Auth_UserDelete` and `wh_Auth_UserSetPermissions` remain admin-only operations in the base backend.
 
+Independently of the backend, the core refuses any `wh_Auth_UserSetPermissions` call from a non-admin session whose supplied permissions **carry** the admin flag. The check is absolute rather than a promotion check: the core cannot see whether the target already held the flag, so it refuses even a request that would merely preserve an existing admin. That is the only permission policy the core enforces when changing an existing user. Subset limits on the remaining group, action, and key-id bits — and admin **revocation**, including demotion of the last remaining admin — are delegated to the backend. The core is handed only the target's `whUserId` and has no way to read that user's current permissions (`UserGet` is keyed by username), so it can enforce absolute rules but not rules relative to the target's existing state. A backend that allows non-admin permission edits is responsible for its own escalation and lockout policy.
+
+### Permission Changes and Live Sessions
+
+A permission change made through `wh_Auth_UserSetPermissions` takes effect on the calling session immediately: when the target `user_id` is the user logged in on that context, the cached session permissions are refreshed with the supplied values, so a revoked group or action bit is enforced on the very next request rather than at the next login. The cache mirrors what the caller supplied, so a backend that stores something other than the permissions it was handed leaves the session cache diverged from its own record.
+
+Two consequences are worth planning for:
+
+- **Self-demotion is immediate and can be self-locking.** A session that clears its own group or action bits loses those capabilities at once. If it clears the `USER_SET_PERMISSIONS` action, it can no longer restore itself and must log out and back in as a sufficiently privileged user.
+- **Other sessions are not affected.** Permissions are cached per `whAuthContext`, and there is no cross-context notification. A user logged in on another connection keeps its existing permissions until it logs in again.
+
 ### Pluggable Backend
 
 The authentication manager does not own the user database itself. All operations that read or modify user state — login, user add/delete, permission updates, credential updates — are dispatched through a `whAuthCb` callback table that the application supplies at server initialization. The storage backend is therefore a port-time decision: an in-memory table for development, an NVM-backed store for production, or a connector to an external identity service.

src/wh_auth.c

@@ -472,8 +472,24 @@ int wh_Auth_UserSetPermissions(whAuthContext* context, whUserId user_id,
         return rc;
     }
 
-    rc = context->cb->UserSetPermissions(
-        context->context, context->user.user_id, user_id, permissions);
+    /* A non-admin session can not set the admin flag, no matter what the
+     * backend policy allows. Absolute check; the target's current flag is
+     * not visible here. */
+    if (!WH_AUTH_IS_ADMIN(context->user.permissions) &&
+        WH_AUTH_IS_ADMIN(permissions)) {
+        rc = WH_AUTH_PERMISSION_ERROR;
+    }
+    else {
+        rc = context->cb->UserSetPermissions(
+            context->context, context->user.user_id, user_id, permissions);
+
+        /* Keep the live session cache in sync when a logged-in user changes
+         * its own permissions; authorization reads only this cache. */
+        if ((rc == WH_ERROR_OK) && (user_id == context->user.user_id) &&
+            (context->user.user_id != WH_USER_ID_INVALID)) {
+            context->user.permissions = permissions;
+        }
+    }
 
     (void)WH_AUTH_UNLOCK(context);
     return rc;

test-refactor/client-server/wh_test_auth.c

@@ -568,8 +568,20 @@ static int _whTest_AuthSetPermissions_impl(whClientContext* client)
     whUserId          user_id;
     whAuthPermissions perms, new_perms;
     whAuthPermissions fetched_perms;
+    whAuthPermissions full_perms, demoted_perms;
+    whAuthPermissions nonadmin_perms, escalate_perms;
     whUserId          fetched_user_id = WH_USER_ID_INVALID;
+    whUserId          probe_id        = WH_USER_ID_INVALID;
+    whUserId          nonadmin_id     = WH_USER_ID_INVALID;
     int32_t           get_rc          = 0;
+    int32_t           probe_server_rc = 0;
+    int32_t           plain_set_rc    = 0;
+    int32_t           self_grant_rc   = 0;
+    int32_t           other_grant_rc  = 0;
+    int32_t           target_get_rc   = 0;
+    int               probe_rc        = 0;
+    int               nonadmin_admin  = 0;
+    int               target_admin    = 0;
 
     /* Login as admin first */
     whAuthPermissions admin_perms;
@@ -643,6 +655,151 @@ static int _whTest_AuthSetPermissions_impl(whClientContext* client)
         WH_TEST_ASSERT_RETURN(permissions_match);
     }
 
+    /* Test 2c: self-demote must bind the live session. Admin clears its own
+     * USER_ADD bit (keeping admin + SET_PERMISSIONS so it can restore). */
+    WH_TEST_PRINT("  Test: Self-demote binds live session\n");
+    memset(&full_perms, 0, sizeof(full_perms));
+    fetched_user_id = WH_USER_ID_INVALID;
+    get_rc          = 0;
+    WH_TEST_RETURN_ON_FAIL(_whTest_Auth_UserGetOp(client, TEST_ADMIN_USERNAME,
+                                                  &get_rc, &fetched_user_id,
+                                                  &full_perms));
+    WH_TEST_ASSERT_RETURN(get_rc == WH_ERROR_OK);
+    WH_TEST_ASSERT_RETURN(fetched_user_id == admin_id);
+
+    demoted_perms = full_perms;
+    WH_AUTH_CLEAR_ALLOWED_ACTION(demoted_perms, WH_MESSAGE_GROUP_AUTH,
+                                 WH_MESSAGE_AUTH_ACTION_USER_ADD);
+    server_rc = 0;
+    WH_TEST_RETURN_ON_FAIL(
+        _whTest_Auth_UserSetPermsOp(client, admin_id, demoted_perms,
+                                    &server_rc));
+    WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_OK);
+
+    /* The revoked bit must now deny the admin's own UserAdd. Capture the
+     * probe result, restore, and only then assert -- a failing assert here
+     * must not leave the stored admin record demoted for later tests. */
+    memset(&perms, 0, sizeof(perms));
+    probe_server_rc = 0;
+    probe_id        = WH_USER_ID_INVALID;
+    probe_rc = _whTest_Auth_UserAddOp(client, "selfdemote_probe", perms,
+                                      WH_AUTH_METHOD_PIN, "pass", 4,
+                                      &probe_server_rc, &probe_id);
+
+    /* Restore full perms before asserting the probe outcome. */
+    server_rc = 0;
+    WH_TEST_RETURN_ON_FAIL(
+        _whTest_Auth_UserSetPermsOp(client, admin_id, full_perms, &server_rc));
+    WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_OK);
+
+    /* If the probe wrongly succeeded it consumed a user slot, so drop it
+     * before asserting to keep the 5-slot table clean for later tests. */
+    _whTest_Auth_DeleteUserByName(client, "selfdemote_probe");
+
+    /* A denial is a well-formed response, so the client call itself is OK */
+    WH_TEST_ASSERT_RETURN(probe_rc == WH_ERROR_OK);
+    WH_TEST_ASSERT_RETURN(probe_server_rc != WH_ERROR_OK);
+    WH_TEST_ASSERT_RETURN(probe_id == WH_USER_ID_INVALID);
+
+    /* The same session can add again after the restore. */
+    memset(&perms, 0, sizeof(perms));
+    server_rc = 0;
+    probe_id  = WH_USER_ID_INVALID;
+    WH_TEST_RETURN_ON_FAIL(_whTest_Auth_UserAddOp(client, "selfdemote_probe",
+                                                  perms, WH_AUTH_METHOD_PIN,
+                                                  "pass", 4, &server_rc,
+                                                  &probe_id));
+    WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_OK);
+    WH_TEST_ASSERT_RETURN(probe_id != WH_USER_ID_INVALID);
+    _whTest_Auth_DeleteUserByName(client, "selfdemote_probe");
+
+    /* Changing a different user's permissions leaves the caller's own
+     * session untouched: admin can still add after demoting testuser3. */
+    memset(&new_perms, 0, sizeof(new_perms));
+    server_rc = 0;
+    WH_TEST_RETURN_ON_FAIL(
+        _whTest_Auth_UserSetPermsOp(client, user_id, new_perms, &server_rc));
+    WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_OK);
+
+    memset(&perms, 0, sizeof(perms));
+    server_rc = 0;
+    probe_id  = WH_USER_ID_INVALID;
+    WH_TEST_RETURN_ON_FAIL(_whTest_Auth_UserAddOp(client, "selfdemote_probe2",
+                                                  perms, WH_AUTH_METHOD_PIN,
+                                                  "pass", 4, &server_rc,
+                                                  &probe_id));
+    WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_OK);
+    WH_TEST_ASSERT_RETURN(probe_id != WH_USER_ID_INVALID);
+    _whTest_Auth_DeleteUserByName(client, "selfdemote_probe2");
+
+    /* Test 2d: a non-admin session can not hand out the admin flag. */
+    WH_TEST_PRINT("  Test: Non-admin cannot grant admin permissions\n");
+    memset(&nonadmin_perms, 0, sizeof(nonadmin_perms));
+    WH_AUTH_SET_ALLOWED_ACTION(nonadmin_perms, WH_MESSAGE_GROUP_AUTH,
+                               WH_MESSAGE_AUTH_ACTION_USER_SET_PERMISSIONS);
+    WH_AUTH_SET_IS_ADMIN(nonadmin_perms, 0);
+    server_rc   = 0;
+    nonadmin_id = WH_USER_ID_INVALID;
+    WH_TEST_RETURN_ON_FAIL(_whTest_Auth_UserAddOp(
+        client, "setperms_nonadmin", nonadmin_perms, WH_AUTH_METHOD_PIN, "pass",
+        4, &server_rc, &nonadmin_id));
+    WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_OK);
+    WH_TEST_ASSERT_RETURN(nonadmin_id != WH_USER_ID_INVALID);
+
+    _whTest_Auth_LogoutOp(client, admin_id, &server_rc);
+    server_rc = 0;
+    WH_TEST_RETURN_ON_FAIL(_whTest_Auth_LoginOp(client, WH_AUTH_METHOD_PIN,
+                                                "setperms_nonadmin", "pass", 4,
+                                                &server_rc, &nonadmin_id));
+    WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_OK);
+
+    /* Capture the outcomes first; asserting here would return with the
+     * non-admin session still live. */
+    escalate_perms = nonadmin_perms;
+    WH_AUTH_SET_IS_ADMIN(escalate_perms, 1);
+    WH_TEST_RETURN_ON_FAIL(_whTest_Auth_UserSetPermsOp(
+        client, nonadmin_id, nonadmin_perms, &plain_set_rc));
+    WH_TEST_RETURN_ON_FAIL(_whTest_Auth_UserSetPermsOp(
+        client, nonadmin_id, escalate_perms, &self_grant_rc));
+    WH_TEST_RETURN_ON_FAIL(_whTest_Auth_UserSetPermsOp(
+        client, user_id, escalate_perms, &other_grant_rc));
+
+    _whTest_Auth_LogoutOp(client, nonadmin_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, 4, &server_rc, &admin_id));
+    WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_OK);
+
+    /* The backend refuses the plain call; the admin flag is refused earlier,
+     * in the core, whatever the target. */
+    WH_TEST_ASSERT_RETURN(plain_set_rc == WH_ERROR_ACCESS);
+    WH_TEST_ASSERT_RETURN(self_grant_rc == WH_AUTH_PERMISSION_ERROR);
+    WH_TEST_ASSERT_RETURN(other_grant_rc == WH_AUTH_PERMISSION_ERROR);
+
+    /* Neither stored record may have picked up the admin flag. */
+    memset(&fetched_perms, 0, sizeof(fetched_perms));
+    fetched_user_id = WH_USER_ID_INVALID;
+    get_rc          = 0;
+    WH_TEST_RETURN_ON_FAIL(_whTest_Auth_UserGetOp(client, "setperms_nonadmin",
+                                                  &get_rc, &fetched_user_id,
+                                                  &fetched_perms));
+    nonadmin_admin = WH_AUTH_IS_ADMIN(fetched_perms) ? 1 : 0;
+
+    memset(&fetched_perms, 0, sizeof(fetched_perms));
+    fetched_user_id = WH_USER_ID_INVALID;
+    target_get_rc   = 0;
+    WH_TEST_RETURN_ON_FAIL(_whTest_Auth_UserGetOp(
+        client, "testuser3", &target_get_rc, &fetched_user_id, &fetched_perms));
+    target_admin = WH_AUTH_IS_ADMIN(fetched_perms) ? 1 : 0;
+
+    _whTest_Auth_DeleteUserByName(client, "setperms_nonadmin");
+
+    WH_TEST_ASSERT_RETURN(get_rc == WH_ERROR_OK);
+    WH_TEST_ASSERT_RETURN(target_get_rc == WH_ERROR_OK);
+    WH_TEST_ASSERT_RETURN(!nonadmin_admin);
+    WH_TEST_ASSERT_RETURN(!target_admin);
+
     /* Test 3: Set user permissions for non-existent user */
     WH_TEST_PRINT("  Test: Set user permissions for non-existent user\n");
     memset(&new_perms, 0, sizeof(new_perms));

wolfhsm/wh_auth.h

@@ -338,6 +338,10 @@ int wh_Auth_UserDelete(whAuthContext* context, whUserId user_id);
 /**
  * @brief Set user permissions.
  *
+ * On success, a change targeting this context's logged-in user also refreshes
+ * its cached session permissions, so it binds immediately. A non-admin session
+ * supplying the admin flag is refused with WH_AUTH_PERMISSION_ERROR.
+ *
  * @param[in] context Pointer to the auth context.
  * @param[in] user_id The user ID to set permissions for.
  * @param[in] permissions The new permissions to set.