test(admin): pin GHSA-5354 scope guard for derived credentials (#5144)

* 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.
This commit is contained in:
Zhengchao An
2026-07-23 08:48:28 +08:00
committed by GitHub
parent 7d96ffd7fb
commit b94bf874bf
+46 -1
View File
@@ -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 {