From cedcb3773c9d604566af304893bd50b023e0bd71 Mon Sep 17 00:00:00 2001 From: Seena Fallah Date: Thu, 17 Apr 2025 14:55:00 +0200 Subject: [PATCH] rgw: SysReqApplier overrides is_admin_of based on impersonation SysReqApplier now returns true for is_admin_of() when the requester was a system user and was not impersonating any user/role using rgwx-perm-check-uid or rgwx-perm-check-role. Signed-off-by: Seena Fallah (cherry picked from commit 0e650ea276669c2c6bb236f27db07910754cc220) --- src/rgw/rgw_auth.cc | 3 ++- src/rgw/rgw_auth.h | 3 ++- src/rgw/rgw_auth_filters.h | 33 ++++++++++++++++++++------ src/rgw/rgw_auth_s3.h | 10 ++++---- src/rgw/rgw_rest_s3.cc | 47 ++++++++++++++++++++++++++++++++++---- src/rgw/rgw_rest_s3.h | 1 + src/rgw/rgw_swift_auth.cc | 4 ++-- src/rgw/rgw_swift_auth.h | 5 ++-- 8 files changed, 84 insertions(+), 22 deletions(-) diff --git a/src/rgw/rgw_auth.cc b/src/rgw/rgw_auth.cc index c255e1ac365..14e096f22c5 100644 --- a/src/rgw/rgw_auth.cc +++ b/src/rgw/rgw_auth.cc @@ -1315,7 +1315,8 @@ rgw::auth::AnonymousEngine::authenticate(const DoutPrefixProvider* dpp, const re auto apl = \ apl_factory->create_apl_local(cct, s, std::move(user), std::nullopt, {}, rgw::auth::LocalApplier::NO_SUBUSER, - std::nullopt, rgw::auth::LocalApplier::NO_ACCESS_KEY); + std::nullopt, rgw::auth::LocalApplier::NO_ACCESS_KEY, + false /* is_impersonating */); return result_t::grant(std::move(apl)); } } diff --git a/src/rgw/rgw_auth.h b/src/rgw/rgw_auth.h index eeb00615a2b..a7a09b36b21 100644 --- a/src/rgw/rgw_auth.h +++ b/src/rgw/rgw_auth.h @@ -765,7 +765,8 @@ public: std::vector policies, const std::string& subuser, const std::optional& perm_mask, - const std::string& access_key_id) const = 0; + const std::string& access_key_id, + bool is_impersonating) const = 0; }; }; diff --git a/src/rgw/rgw_auth_filters.h b/src/rgw/rgw_auth_filters.h index d15bb54661c..dc6e5f7ce84 100644 --- a/src/rgw/rgw_auth_filters.h +++ b/src/rgw/rgw_auth_filters.h @@ -240,7 +240,8 @@ class SysReqApplier : public DecoratedApplier { CephContext* const cct; rgw::sal::Driver* driver; const RGWHTTPArgs& args; - mutable boost::tribool is_system; + mutable boost::tribool is_system = boost::logic::indeterminate; + mutable bool is_impersonating; mutable std::optional effective_owner; mutable std::optional effective_tenant; @@ -249,12 +250,17 @@ public: SysReqApplier(CephContext* const cct, rgw::sal::Driver* driver, const req_state* const s, - U&& decoratee) + U&& decoratee, + bool is_impersonating = false) : DecoratedApplier(std::forward(decoratee)), cct(cct), driver(driver), args(s->info.args), - is_system(boost::logic::indeterminate) { + is_impersonating(is_impersonating) { + if (is_impersonating) { + // we only accept impersonated requests from a system user + is_system = true; + } } void to_str(std::ostream& out) const override; @@ -275,6 +281,13 @@ public: return DecoratedApplier::get_tenant(); } + bool is_admin_of(const rgw_owner& o) const override { + if (is_system && !is_impersonating) { + return true; + } + + return DecoratedApplier::is_admin_of(o); + } }; template @@ -292,8 +305,13 @@ template auto SysReqApplier::load_acct_info(const DoutPrefixProvider* dpp) const -> std::unique_ptr { std::unique_ptr user = DecoratedApplier::load_acct_info(dpp); - is_system = user->get_info().system; + // skip loading the account info if we already have it through impersonation + if (is_impersonating) { + return user; + } + + is_system = user->get_info().system; if (is_system) { //ldpp_dout(dpp, 20) << "system request" << dendl; @@ -320,7 +338,7 @@ auto SysReqApplier::load_acct_info(const DoutPrefixProvider* dpp) const -> st throw -EACCES; } effective_tenant = info.tenant; - } + } } } return user; @@ -344,8 +362,9 @@ template static inline SysReqApplier add_sysreq(CephContext* const cct, rgw::sal::Driver* driver, const req_state* const s, - T&& t) { - return SysReqApplier(cct, driver, s, std::forward(t)); + T&& t, + bool is_impersonating = false) { + return SysReqApplier(cct, driver, s, std::forward(t), is_impersonating); } } /* namespace auth */ diff --git a/src/rgw/rgw_auth_s3.h b/src/rgw/rgw_auth_s3.h index b6fc6ef1e08..edc208e9567 100644 --- a/src/rgw/rgw_auth_s3.h +++ b/src/rgw/rgw_auth_s3.h @@ -60,10 +60,11 @@ class STSAuthStrategy : public rgw::auth::Strategy, std::vector policies, const std::string& subuser, const std::optional& perm_mask, - const std::string& access_key_id) const override { + const std::string& access_key_id, + bool is_impersonating) const override { auto apl = rgw::auth::add_sysreq(cct, driver, s, LocalApplier(cct, std::move(user), std::move(account), std::move(policies), - subuser, perm_mask, access_key_id)); + subuser, perm_mask, access_key_id), is_impersonating); return aplptr_t(new decltype(apl)(std::move(apl))); } @@ -182,10 +183,11 @@ class AWSAuthStrategy : public rgw::auth::Strategy, std::vector policies, const std::string& subuser, const std::optional& perm_mask, - const std::string& access_key_id) const override { + const std::string& access_key_id, + bool is_impersonating) const override { auto apl = rgw::auth::add_sysreq(cct, driver, s, LocalApplier(cct, std::move(user), std::move(account), std::move(policies), - subuser, perm_mask, access_key_id)); + subuser, perm_mask, access_key_id), is_impersonating); /* TODO(rzarzynski): replace with static_ptr. */ return aplptr_t(new decltype(apl)(std::move(apl))); } diff --git a/src/rgw/rgw_rest_s3.cc b/src/rgw/rgw_rest_s3.cc index 9f12e790337..424dd64e766 100644 --- a/src/rgw/rgw_rest_s3.cc +++ b/src/rgw/rgw_rest_s3.cc @@ -6855,7 +6855,7 @@ rgw::auth::s3::LocalEngine::authenticate( if (s->op_type == RGW_OP_OPTIONS_CORS) { auto apl = apl_factory->create_apl_local( cct, s, std::move(user), std::move(account), std::move(policies), - k.subuser, std::nullopt, access_key_id); + k.subuser, std::nullopt, access_key_id, false /* is_impersonating */); return result_t::grant(std::move(apl), completer_factory(k.key)); } @@ -6874,9 +6874,46 @@ rgw::auth::s3::LocalEngine::authenticate( return result_t::deny(-ERR_SIGNATURE_NO_MATCH); } - auto apl = apl_factory->create_apl_local( - cct, s, std::move(user), std::move(account), std::move(policies), - k.subuser, std::nullopt, access_key_id); + aplptr_t apl; + + // if this is a system request and we have rgwx-perm-check-uid passed, + // we do impersonation + if (user->get_info().system) { + const std::string perm_check_uid = s->info.args.get(RGW_SYS_PARAM_PREFIX "perm-check-uid"); + if (!perm_check_uid.empty()) { + auto perm_check_user = driver->get_user(rgw_user(perm_check_uid)); + if (int r = perm_check_user->load_user(dpp, y); r < 0) { + ldpp_dout(dpp, 0) << "ERROR: unable to load user info for " + << perm_check_uid << dendl; + return result_t::deny(r); + } + + account.reset(); + policies.clear(); + // load account and policies for the impersonated user + int ret = load_account_and_policies(dpp, y, driver, perm_check_user->get_info(), + perm_check_user->get_attrs(), account, policies); + if (ret < 0) { + ldpp_dout(dpp, 0) << "ERROR: unable to load account and policies for " + << perm_check_uid << dendl; + return result_t::deny(ret); + } + + apl = apl_factory->create_apl_local( + cct, s, std::move(perm_check_user), std::move(account), std::move(policies), + rgw::auth::LocalApplier::NO_SUBUSER, std::nullopt, rgw::auth::LocalApplier::NO_ACCESS_KEY, + true /* is_impersonating */); + } + } + + if (!apl) { + // if we don't have impersonation, use the original user + // and the original access key id for logging + apl = apl_factory->create_apl_local( + cct, s, std::move(user), std::move(account), std::move(policies), + k.subuser, std::nullopt, access_key_id, false /* is_impersonating */); + } + return result_t::grant(std::move(apl), completer_factory(k.key)); } @@ -7112,7 +7149,7 @@ rgw::auth::s3::STSEngine::authenticate( string subuser; auto apl = local_apl_factory->create_apl_local( cct, s, std::move(user), std::move(account), std::move(policies), - subuser, token.perm_mask, std::string(_access_key_id)); + subuser, token.perm_mask, std::string(_access_key_id), false /* is_impersonating */); return result_t::grant(std::move(apl), completer_factory(token.secret_access_key)); } } diff --git a/src/rgw/rgw_rest_s3.h b/src/rgw/rgw_rest_s3.h index e8fdc69751c..62445b03270 100644 --- a/src/rgw/rgw_rest_s3.h +++ b/src/rgw/rgw_rest_s3.h @@ -1159,6 +1159,7 @@ public: }; class LocalEngine : public AWSEngine { + typedef rgw::auth::IdentityApplier::aplptr_t aplptr_t; rgw::sal::Driver* driver; const rgw::auth::LocalApplier::Factory* const apl_factory; diff --git a/src/rgw/rgw_swift_auth.cc b/src/rgw/rgw_swift_auth.cc index febad481b51..1a763243115 100644 --- a/src/rgw/rgw_swift_auth.cc +++ b/src/rgw/rgw_swift_auth.cc @@ -525,7 +525,7 @@ ExternalTokenEngine::authenticate(const DoutPrefixProvider* dpp, auto apl = apl_factory->create_apl_local( cct, s, std::move(user), std::move(account), std::move(policies), extract_swift_subuser(swift_user), - std::nullopt, LocalApplier::NO_ACCESS_KEY); + std::nullopt, LocalApplier::NO_ACCESS_KEY, false /* is_impersonating */); return result_t::grant(std::move(apl)); } @@ -688,7 +688,7 @@ SignedTokenEngine::authenticate(const DoutPrefixProvider* dpp, auto apl = apl_factory->create_apl_local( cct, s, std::move(user), std::move(account), std::move(policies), extract_swift_subuser(swift_user), - std::nullopt, LocalApplier::NO_ACCESS_KEY); + std::nullopt, LocalApplier::NO_ACCESS_KEY, false /* is_impersonating */); return result_t::grant(std::move(apl)); } diff --git a/src/rgw/rgw_swift_auth.h b/src/rgw/rgw_swift_auth.h index c27a24a2619..eb34984398c 100644 --- a/src/rgw/rgw_swift_auth.h +++ b/src/rgw/rgw_swift_auth.h @@ -243,12 +243,13 @@ class DefaultStrategy : public rgw::auth::Strategy, std::vector policies, const std::string& subuser, const std::optional& perm_mask, - const std::string& access_key_id) const override { + const std::string& access_key_id, + bool is_impersonating) const override { auto apl = \ rgw::auth::add_3rdparty(driver, rgw_user(s->account_name), rgw::auth::add_sysreq(cct, driver, s, LocalApplier(cct, std::move(user), std::move(account), std::move(policies), - subuser, perm_mask, access_key_id))); + subuser, perm_mask, access_key_id), is_impersonating)); /* TODO(rzarzynski): replace with static_ptr. */ return aplptr_t(new decltype(apl)(std::move(apl))); } -- 2.39.5