From 9866f68d86482b3de7e0058a908727c3815e72b9 Mon Sep 17 00:00:00 2001
From: Zhengchao An <anzhengchao@gmail.com>
Date: Thu, 23 Jul 2026 07:43:30 +0800
Subject: [PATCH] fix(admin): confine service-account parent to caller scope
 (GHSA-5354) (#5141)

AddServiceAccount gated creation only on CreateServiceAccountAdminAction and did not constrain the targetUser (the new service account's parent) to the caller's own scope. Combined with the deliberate root-credential exemption in the existence check, a non-owner principal holding that admin action could create a service account parented to the root credential, which then authenticates as owner via prepare_service_account_auth. This is the direct-handler analogue of the ImportIam parent-scope check (GHSA-566f), which already enforces the invariant via imported_service_account_parent_allowed.

Enforce the same invariant through add_service_account_parent_within_scope: a non-owner may create a service account only for itself (or, for a derived credential, its own parent); owners retain cross-user creation. Add named regression test ghsa_5354_non_owner_service_account_parent_confined_to_scope.

Refs GHSA-5354-r3w2-34m8.
---
 rustfs/src/admin/handlers/service_account.rs | 62 ++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/rustfs/src/admin/handlers/service_account.rs b/rustfs/src/admin/handlers/service_account.rs
index 1b27192f2..138a633b4 100644
--- a/rustfs/src/admin/handlers/service_account.rs
+++ b/rustfs/src/admin/handlers/service_account.rs
@@ -108,6 +108,23 @@ fn is_service_account_owner_of(caller: &StoredCredentials, target_parent_user: &
     caller_parent == target_parent_user
 }
 
+/// GHSA-5354: whether a caller resolving to `owner` may create a service account
+/// parented to `target_user`, given the caller's own key (`req_user`) and its
+/// effective parent (`req_parent_user`, which equals `req_user` for a top-level
+/// user or the parent for a derived credential).
+///
+/// The `CreateServiceAccountAdminAction` permission gates *whether* a caller may
+/// create service accounts, not *for whom*. A non-owner is therefore confined to
+/// its own scope; only an owner may target another user — including the root
+/// credential, whose existence check is deliberately lenient. Without this a
+/// caller holding only that action could pass `target_user = <root access key>`
+/// and mint a root-parented service account that authenticates as owner via
+/// `prepare_service_account_auth`, a full-takeover privilege escalation. Mirrors
+/// `imported_service_account_parent_allowed` on the ImportIam path (GHSA-566f).
+fn add_service_account_parent_within_scope(target_user: &str, req_user: &str, req_parent_user: &str, owner: bool) -> bool {
+    owner || target_user == req_user || target_user == req_parent_user
+}
+
 /// Fail-closed ownership check used by DeleteServiceAccount for a caller that
 /// lacks the RemoveServiceAccount admin action.
 ///
@@ -342,6 +359,12 @@ impl Operation for AddServiceAccount {
 
         let is_svc_acc = target_user == req_user || target_user == req_parent_user;
 
+        // GHSA-5354: confine a non-owner caller to its own scope so it cannot mint
+        // a root-parented (owner) service account. See the helper's doc comment.
+        if !add_service_account_parent_within_scope(&target_user, &req_user, &req_parent_user, owner) {
+            return Err(s3_error!(AccessDenied, "service account parent is outside requester scope"));
+        }
+
         let mut target_groups = None;
         let mut opts = NewServiceAccountOpts {
             access_key: create_req.access_key,
@@ -1408,6 +1431,45 @@ mod tests {
         );
     }
 
+    #[test]
+    fn ghsa_5354_non_owner_service_account_parent_confined_to_scope() {
+        // Regression (GHSA-5354): a caller holding CreateServiceAccountAdminAction
+        // but resolving to owner=false must not be able to parent a new service
+        // account to the root credential (or any other user). A root-parented SA
+        // authenticates as owner, so allowing this is a full-takeover escalation.
+        let root = rustfs_credentials::DEFAULT_ACCESS_KEY; // "rustfsadmin"
+
+        // Attack: non-owner "alice" targets root as the parent -> DENY.
+        assert!(
+            !add_service_account_parent_within_scope(root, "alice", "alice", false),
+            "non-owner must not parent a service account to root"
+        );
+        // Non-owner targeting an unrelated user -> DENY.
+        assert!(
+            !add_service_account_parent_within_scope("bob", "alice", "alice", false),
+            "non-owner must not parent a service account to another user"
+        );
+        // Self-scoped creation (default target == own key) -> ALLOW.
+        assert!(
+            add_service_account_parent_within_scope("alice", "alice", "alice", false),
+            "non-owner may create a service account for itself"
+        );
+        // Derived credential targeting its own parent -> ALLOW.
+        assert!(
+            add_service_account_parent_within_scope("parent-user", "svc-key", "parent-user", false),
+            "derived credential may create a service account under its own parent"
+        );
+        // Owner may target any parent, including root (cross-user creation intact).
+        assert!(
+            add_service_account_parent_within_scope(root, root, root, true),
+            "owner retains cross-user / root-parent creation"
+        );
+        assert!(
+            add_service_account_parent_within_scope("bob", root, root, true),
+            "owner may create a service account for another user"
+        );
+    }
+
     #[test]
     fn guess_user_provider_detects_builtin_accounts() {
         let credentials = StoredCredentials {