From b94bf874bfd0f1298911bb461aad3d8cb8666948 Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Thu, 23 Jul 2026 08:48:28 +0800 Subject: [PATCH] test(admin): pin GHSA-5354 scope guard for derived credentials (#5144) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(admin): confine service-account parent to caller scope (GHSA-5354) 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. * test(admin): pin GHSA-5354 scope guard for derived credentials The GHSA-5354 fix (#5141) added `add_service_account_parent_within_scope` and a named-boundary test, but that test only covers the self-scoped case with `req_user == req_parent_user`. A derived credential — a service account or STS token holding CreateServiceAccountAdminAction — has `req_user` (its own key) distinct from `req_parent_user` (its parent), which is the realistic attacker shape and was left unexercised. Add a test that pins the guard's allow set to exactly `owner || is_svc_acc` across that derived-credential shape, including a derived non-owner aiming at the root credential, so a later change to either the guard or the `is_svc_acc` rewrite cannot silently let a derived non-owner escape its own parent's scope. Also note at the check site that the guard is evaluated on the original `target_user`, before the derived-credential rewrite to `req_parent_user`. Refs GHSA-5354-r3w2-34m8. --- rustfs/src/admin/handlers/service_account.rs | 47 +++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/rustfs/src/admin/handlers/service_account.rs b/rustfs/src/admin/handlers/service_account.rs index 138a633b4..31aebd08d 100644 --- a/rustfs/src/admin/handlers/service_account.rs +++ b/rustfs/src/admin/handlers/service_account.rs @@ -360,7 +360,11 @@ 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. + // a root-parented (owner) service account. Evaluated on the original + // `target_user` the caller submitted, before the derived-credential rewrite + // to `req_parent_user` below, so the guard is bound to the parent actually + // requested. Its allow set is exactly `owner || is_svc_acc`. 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")); } @@ -1470,6 +1474,47 @@ mod tests { ); } + #[test] + fn ghsa_5354_scope_guard_matches_owner_or_self_scope_for_derived_credentials() { + // The handler evaluates `add_service_account_parent_within_scope` on the + // original `target_user`, before the `target_user = req_parent_user` rewrite + // taken inside the `is_svc_acc` branch. The guard's allow set must therefore + // stay exactly `owner || is_svc_acc`, where + // `is_svc_acc == (target_user == req_user || target_user == req_parent_user)`. + // + // The named-boundary test above only exercises the self-scoped case with + // `req_user == req_parent_user`. A *derived* credential has `req_user` + // (its own key) distinct from `req_parent_user` (its parent), which is the + // realistic attacker shape: a service account holding + // CreateServiceAccountAdminAction. Pin the equivalence across that shape so a + // later change to either the guard or the rewrite cannot silently let a + // derived non-owner credential escape its own parent's scope. + let root = rustfs_credentials::DEFAULT_ACCESS_KEY; + let cases: &[(&str, &str, &str, bool)] = &[ + // Derived non-owner (own key != parent) aiming at root -> deny. + (root, "attacker-sa", "alice", false), + // Derived non-owner aiming at an unrelated user -> deny. + ("bob", "attacker-sa", "alice", false), + // Derived non-owner aiming at its own parent -> allow. + ("alice", "attacker-sa", "alice", false), + // Derived non-owner aiming at its own key -> allow. + ("attacker-sa", "attacker-sa", "alice", false), + // Top-level non-owner aiming at itself -> allow. + ("alice", "alice", "alice", false), + // Owner is unrestricted, including root and a derived shape. + (root, "attacker-sa", "alice", true), + ("bob", root, root, true), + ]; + for &(target_user, req_user, req_parent_user, owner) in cases { + let is_svc_acc = target_user == req_user || target_user == req_parent_user; + assert_eq!( + add_service_account_parent_within_scope(target_user, req_user, req_parent_user, owner), + owner || is_svc_acc, + "scope guard must equal `owner || is_svc_acc` for (target={target_user}, req_user={req_user}, req_parent={req_parent_user}, owner={owner})" + ); + } + } + #[test] fn guess_user_provider_detects_builtin_accounts() { let credentials = StoredCredentials {