From db09c0956a531ad8c026e9b5f924ab32bfb95514 Mon Sep 17 00:00:00 2001 From: Casey Bodley Date: Thu, 1 Feb 2024 13:10:00 -0500 Subject: [PATCH] rgw/iam: Policy() takes string instead of bufferlist the constructor immediately called bufferlist::to_str() to convert it into a string; just take string so callers don't have to convert it Signed-off-by: Casey Bodley --- src/rgw/rgw_admin.cc | 12 +++---- src/rgw/rgw_auth.cc | 8 ++--- src/rgw/rgw_iam_policy.cc | 4 +-- src/rgw/rgw_iam_policy.h | 2 +- src/rgw/rgw_op.cc | 32 ++++++++--------- src/rgw/rgw_op.h | 2 +- src/rgw/rgw_polparser.cc | 2 +- src/rgw/rgw_rest_pubsub.cc | 9 ++--- src/rgw/rgw_rest_role.cc | 6 ++-- src/rgw/rgw_rest_sts.cc | 9 ++--- src/rgw/rgw_rest_user_policy.cc | 4 +-- src/test/rgw/test_rgw_iam_policy.cc | 56 ++++++++++------------------- 12 files changed, 54 insertions(+), 92 deletions(-) diff --git a/src/rgw/rgw_admin.cc b/src/rgw/rgw_admin.cc index f983ee708fd..5cef1fe0bb5 100644 --- a/src/rgw/rgw_admin.cc +++ b/src/rgw/rgw_admin.cc @@ -6780,10 +6780,9 @@ int main(int argc, const char **argv) cerr << "ERROR: assume role policy document is empty" << std::endl; return -EINVAL; } - bufferlist bl = bufferlist::static_from_string(assume_role_doc); try { const rgw::IAM::Policy p( - g_ceph_context, tenant, bl, + g_ceph_context, tenant, assume_role_doc, g_ceph_context->_conf.get_val( "rgw_policy_reject_invalid_principals")); } catch (rgw::IAM::PolicyParseException& e) { @@ -6841,9 +6840,8 @@ int main(int argc, const char **argv) return -EINVAL; } - bufferlist bl = bufferlist::static_from_string(assume_role_doc); try { - const rgw::IAM::Policy p(g_ceph_context, tenant, bl, + const rgw::IAM::Policy p(g_ceph_context, tenant, assume_role_doc, g_ceph_context->_conf.get_val( "rgw_policy_reject_invalid_principals")); } catch (rgw::IAM::PolicyParseException& e) { @@ -6928,19 +6926,17 @@ int main(int argc, const char **argv) return -EINVAL; } - bufferlist bl; if (!infile.empty()) { + bufferlist bl; int ret = read_input(infile, bl); if (ret < 0) { cerr << "ERROR: failed to read input policy document: " << cpp_strerror(-ret) << std::endl; return -ret; } perm_policy_doc = bl.to_str(); - } else { - bl = bufferlist::static_from_string(perm_policy_doc); } try { - const rgw::IAM::Policy p(g_ceph_context, tenant, bl, + const rgw::IAM::Policy p(g_ceph_context, tenant, perm_policy_doc, g_ceph_context->_conf.get_val( "rgw_policy_reject_invalid_principals")); } catch (rgw::IAM::PolicyParseException& e) { diff --git a/src/rgw/rgw_auth.cc b/src/rgw/rgw_auth.cc index b7e71f6e02d..9da2f6afc94 100644 --- a/src/rgw/rgw_auth.cc +++ b/src/rgw/rgw_auth.cc @@ -987,10 +987,9 @@ void rgw::auth::RoleApplier::load_acct_info(const DoutPrefixProvider* dpp, RGWUs void rgw::auth::RoleApplier::modify_request_state(const DoutPrefixProvider *dpp, req_state* s) const { - for (auto it: role.role_policies) { + for (const auto& policy : role.role_policies) { try { - bufferlist bl = bufferlist::static_from_string(it); - const rgw::IAM::Policy p(s->cct, role.tenant, bl, false); + const rgw::IAM::Policy p(s->cct, role.tenant, policy, false); s->iam_user_policies.push_back(std::move(p)); } catch (rgw::IAM::PolicyParseException& e) { //Control shouldn't reach here as the policy has already been @@ -1002,8 +1001,7 @@ void rgw::auth::RoleApplier::modify_request_state(const DoutPrefixProvider *dpp, if (!this->token_attrs.token_policy.empty()) { try { string policy = this->token_attrs.token_policy; - bufferlist bl = bufferlist::static_from_string(policy); - const rgw::IAM::Policy p(s->cct, role.tenant, bl, false); + const rgw::IAM::Policy p(s->cct, role.tenant, policy, false); s->session_policies.push_back(std::move(p)); } catch (rgw::IAM::PolicyParseException& e) { //Control shouldn't reach here as the policy has already been diff --git a/src/rgw/rgw_iam_policy.cc b/src/rgw/rgw_iam_policy.cc index ba8507e5017..470a3e604d2 100644 --- a/src/rgw/rgw_iam_policy.cc +++ b/src/rgw/rgw_iam_policy.cc @@ -1630,9 +1630,9 @@ ostream& operator <<(ostream& m, const Statement& s) { } Policy::Policy(CephContext* cct, const string& tenant, - const bufferlist& _text, + std::string _text, bool reject_invalid_principals) - : text(_text.to_str()) { + : text(std::move(_text)) { StringStream ss(text.data()); PolicyParser pp(cct, tenant, *this, reject_invalid_principals); auto pr = Reader{}.Parse get_iam_policy_from_attr(CephContext* cct, - map& attrs, - const string& tenant) { - auto i = attrs.find(RGW_ATTR_IAM_POLICY); - if (i != attrs.end()) { - return Policy(cct, tenant, i->second, false); +static boost::optional +get_iam_policy_from_attr(CephContext* cct, + const map& attrs, + const string& tenant) +{ + if (auto i = attrs.find(RGW_ATTR_IAM_POLICY); i != attrs.end()) { + return Policy(cct, tenant, i->second.to_str(), false); } else { return none; } @@ -352,18 +353,15 @@ get_public_access_conf_from_attr(const map& attrs) } vector get_iam_user_policy_from_attr(CephContext* cct, - map& attrs, + const map& attrs, const string& tenant) { vector policies; - if (auto it = attrs.find(RGW_ATTR_USER_POLICY); it != attrs.end()) { - bufferlist out_bl = attrs[RGW_ATTR_USER_POLICY]; - map policy_map; - decode(policy_map, out_bl); - for (auto& it : policy_map) { - bufferlist bl = bufferlist::static_from_string(it.second); - Policy p(cct, tenant, bl, false); - policies.push_back(std::move(p)); - } + if (auto bl = attrs.find(RGW_ATTR_USER_POLICY); bl != attrs.end()) { + map policy_map; + decode(policy_map, bl->second); + for (const auto& [name, policy] : policy_map) { + policies.emplace_back(cct, tenant, policy, false); + } } return policies; } @@ -8508,7 +8506,7 @@ void RGWPutBucketPolicy::execute(optional_yield y) try { const Policy p( - s->cct, s->bucket_tenant, data, + s->cct, s->bucket_tenant, data.to_str(), s->cct->_conf.get_val("rgw_policy_reject_invalid_principals")); rgw::sal::Attrs attrs(s->bucket_attrs); if (s->bucket_access_conf && diff --git a/src/rgw/rgw_op.h b/src/rgw/rgw_op.h index fa61aefc563..dd30734b682 100644 --- a/src/rgw/rgw_op.h +++ b/src/rgw/rgw_op.h @@ -2071,7 +2071,7 @@ extern int rgw_build_object_policies(const DoutPrefixProvider *dpp, rgw::sal::Dr extern void rgw_build_iam_environment(rgw::sal::Driver* driver, req_state* s); extern std::vector get_iam_user_policy_from_attr(CephContext* cct, - std::map& attrs, + const std::map& attrs, const std::string& tenant); inline int get_system_versioning_params(req_state *s, diff --git a/src/rgw/rgw_polparser.cc b/src/rgw/rgw_polparser.cc index f81eda7fe97..1bc98cabc89 100644 --- a/src/rgw/rgw_polparser.cc +++ b/src/rgw/rgw_polparser.cc @@ -26,7 +26,7 @@ bool parse(CephContext* cct, const std::string& tenant, bl.append(in); try { auto p = rgw::IAM::Policy( - cct, tenant, bl, + cct, tenant, bl.to_str(), cct->_conf.get_val("rgw_policy_reject_invalid_principals")); } catch (const rgw::IAM::PolicyParseException& e) { std::cerr << fname << ": " << e.what() << std::endl; diff --git a/src/rgw/rgw_rest_pubsub.cc b/src/rgw/rgw_rest_pubsub.cc index ec0b2ff6e86..9f02c5e73a8 100644 --- a/src/rgw/rgw_rest_pubsub.cc +++ b/src/rgw/rgw_rest_pubsub.cc @@ -86,11 +86,10 @@ bool topics_has_endpoint_secret(const rgw_pubsub_topics& topics) { } std::optional get_policy_from_text(req_state* const s, - std::string& policy_text) { - const auto bl = bufferlist::static_from_string(policy_text); + const std::string& policy_text) { try { return rgw::IAM::Policy( - s->cct, s->auth.identity->get_tenant(), bl, + s->cct, s->auth.identity->get_tenant(), policy_text, s->cct->_conf.get_val("rgw_policy_reject_invalid_principals")); } catch (rgw::IAM::PolicyParseException& e) { ldout(s->cct, 1) << "failed to parse policy: '" << policy_text @@ -122,9 +121,7 @@ int verify_topic_owner_or_policy(req_state* const s, s->err.message = "Topic was created by another user."; return -EACCES; } - // bufferlist::static_from_string wants non const string - std::string policy_text(topic.policy_text); - const auto p = get_policy_from_text(s, policy_text); + const auto p = get_policy_from_text(s, topic.policy_text); rgw::IAM::PolicyPrincipal princ_type = rgw::IAM::PolicyPrincipal::Other; const rgw::ARN arn(rgw::Partition::aws, rgw::Service::sns, zonegroup_name, s->user->get_tenant(), topic.name); diff --git a/src/rgw/rgw_rest_role.cc b/src/rgw/rgw_rest_role.cc index 0e338bc63a0..f9580b206ff 100644 --- a/src/rgw/rgw_rest_role.cc +++ b/src/rgw/rgw_rest_role.cc @@ -191,10 +191,9 @@ int RGWCreateRole::init_processing(optional_yield y) s->err.message = "Missing required element AssumeRolePolicyDocument"; return -EINVAL; } - bufferlist bl = bufferlist::static_from_string(trust_policy); try { const rgw::IAM::Policy p( - s->cct, s->user->get_tenant(), bl, + s->cct, s->user->get_tenant(), trust_policy, s->cct->_conf.get_val("rgw_policy_reject_invalid_principals")); } catch (rgw::IAM::PolicyParseException& e) { @@ -552,10 +551,9 @@ int RGWPutRolePolicy::init_processing(optional_yield y) s->err.message = "Missing required element PolicyDocument"; return -EINVAL; } - bufferlist bl = bufferlist::static_from_string(perm_policy); try { const rgw::IAM::Policy p( - s->cct, s->user->get_tenant(), bl, + s->cct, s->user->get_tenant(), perm_policy, s->cct->_conf.get_val("rgw_policy_reject_invalid_principals")); } catch (rgw::IAM::PolicyParseException& e) { diff --git a/src/rgw/rgw_rest_sts.cc b/src/rgw/rgw_rest_sts.cc index 4ec0d211687..84284a587f9 100644 --- a/src/rgw/rgw_rest_sts.cc +++ b/src/rgw/rgw_rest_sts.cc @@ -535,12 +535,11 @@ int RGWREST_STS::verify_permission(optional_yield y) return ret; } string policy = role->get_assume_role_policy(); - buffer::list bl = buffer::list::static_from_string(policy); //Parse the policy //TODO - This step should be part of Role Creation try { - const rgw::IAM::Policy p(s->cct, s->user->get_tenant(), bl, false); + const rgw::IAM::Policy p(s->cct, s->user->get_tenant(), policy, false); if (!s->principal_tags.empty()) { auto res = p.eval(s->env, *s->auth.identity, rgw::IAM::stsTagSession, boost::none); if (res != rgw::IAM::Effect::Allow) { @@ -656,10 +655,9 @@ int RGWSTSAssumeRoleWithWebIdentity::get_params() } if (! policy.empty()) { - bufferlist bl = bufferlist::static_from_string(policy); try { const rgw::IAM::Policy p( - s->cct, s->user->get_tenant(), bl, + s->cct, s->user->get_tenant(), policy, s->cct->_conf.get_val("rgw_policy_reject_invalid_principals")); } catch (rgw::IAM::PolicyParseException& e) { @@ -718,10 +716,9 @@ int RGWSTSAssumeRole::get_params() } if (! policy.empty()) { - bufferlist bl = bufferlist::static_from_string(policy); try { const rgw::IAM::Policy p( - s->cct, s->user->get_tenant(), bl, + s->cct, s->user->get_tenant(), policy, s->cct->_conf.get_val("rgw_policy_reject_invalid_principals")); } catch (rgw::IAM::PolicyParseException& e) { diff --git a/src/rgw/rgw_rest_user_policy.cc b/src/rgw/rgw_rest_user_policy.cc index e452369ccb7..b390bfb5b82 100644 --- a/src/rgw/rgw_rest_user_policy.cc +++ b/src/rgw/rgw_rest_user_policy.cc @@ -133,8 +133,6 @@ int RGWPutUserPolicy::get_params() void RGWPutUserPolicy::execute(optional_yield y) { - bufferlist bl = bufferlist::static_from_string(policy); - op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->user->get_id(), nullptr, nullptr, s->info, y); if (op_ret < 0) { @@ -144,7 +142,7 @@ void RGWPutUserPolicy::execute(optional_yield y) try { const rgw::IAM::Policy p( - s->cct, s->user->get_tenant(), bl, + s->cct, s->user->get_tenant(), policy, s->cct->_conf.get_val("rgw_policy_reject_invalid_principals")); std::map policies; if (auto it = user->get_attrs().find(RGW_ATTR_USER_POLICY); it != user->get_attrs().end()) { diff --git a/src/test/rgw/test_rgw_iam_policy.cc b/src/test/rgw/test_rgw_iam_policy.cc index 156b341ca70..1a308c0f68a 100644 --- a/src/test/rgw/test_rgw_iam_policy.cc +++ b/src/test/rgw/test_rgw_iam_policy.cc @@ -178,9 +178,7 @@ public: TEST_F(PolicyTest, Parse1) { boost::optional p; - ASSERT_NO_THROW(p = Policy(cct.get(), arbitrary_tenant, - bufferlist::static_from_string(example1), - true)); + ASSERT_NO_THROW(p = Policy(cct.get(), arbitrary_tenant, example1, true)); ASSERT_TRUE(p); EXPECT_EQ(p->text, example1); @@ -208,8 +206,7 @@ TEST_F(PolicyTest, Parse1) { } TEST_F(PolicyTest, Eval1) { - auto p = Policy(cct.get(), arbitrary_tenant, - bufferlist::static_from_string(example1), true); + auto p = Policy(cct.get(), arbitrary_tenant, example1, true); Environment e; ARN arn1(Partition::aws, Service::s3, @@ -232,9 +229,7 @@ TEST_F(PolicyTest, Eval1) { TEST_F(PolicyTest, Parse2) { boost::optional p; - ASSERT_NO_THROW(p = Policy(cct.get(), arbitrary_tenant, - bufferlist::static_from_string(example2), - true)); + ASSERT_NO_THROW(p = Policy(cct.get(), arbitrary_tenant, example2, true)); ASSERT_TRUE(p); EXPECT_EQ(p->text, example2); @@ -275,8 +270,7 @@ TEST_F(PolicyTest, Parse2) { } TEST_F(PolicyTest, Eval2) { - auto p = Policy(cct.get(), arbitrary_tenant, - bufferlist::static_from_string(example2), true); + auto p = Policy(cct.get(), arbitrary_tenant, example2, true); Environment e; auto trueacct = FakeIdentity( @@ -316,8 +310,7 @@ TEST_F(PolicyTest, Eval2) { TEST_F(PolicyTest, Parse3) { boost::optional p; - ASSERT_NO_THROW(p = Policy(cct.get(), arbitrary_tenant, - bufferlist::static_from_string(example3), true)); + ASSERT_NO_THROW(p = Policy(cct.get(), arbitrary_tenant, example3, true)); ASSERT_TRUE(p); EXPECT_EQ(p->text, example3); @@ -431,8 +424,7 @@ TEST_F(PolicyTest, Parse3) { } TEST_F(PolicyTest, Eval3) { - auto p = Policy(cct.get(), arbitrary_tenant, - bufferlist::static_from_string(example3), true); + auto p = Policy(cct.get(), arbitrary_tenant, example3, true); Environment em; Environment tr = { { "aws:MultiFactorAuthPresent", "true" } }; Environment fa = { { "aws:MultiFactorAuthPresent", "false" } }; @@ -543,8 +535,7 @@ TEST_F(PolicyTest, Eval3) { TEST_F(PolicyTest, Parse4) { boost::optional p; - ASSERT_NO_THROW(p = Policy(cct.get(), arbitrary_tenant, - bufferlist::static_from_string(example4), true)); + ASSERT_NO_THROW(p = Policy(cct.get(), arbitrary_tenant, example4, true)); ASSERT_TRUE(p); EXPECT_EQ(p->text, example4); @@ -572,8 +563,7 @@ TEST_F(PolicyTest, Parse4) { } TEST_F(PolicyTest, Eval4) { - auto p = Policy(cct.get(), arbitrary_tenant, - bufferlist::static_from_string(example4), true); + auto p = Policy(cct.get(), arbitrary_tenant, example4, true); Environment e; ARN arn1(Partition::aws, Service::iam, @@ -590,8 +580,7 @@ TEST_F(PolicyTest, Eval4) { TEST_F(PolicyTest, Parse5) { boost::optional p; - ASSERT_NO_THROW(p = Policy(cct.get(), arbitrary_tenant, - bufferlist::static_from_string(example5), true)); + ASSERT_NO_THROW(p = Policy(cct.get(), arbitrary_tenant, example5, true)); ASSERT_TRUE(p); EXPECT_EQ(p->text, example5); EXPECT_EQ(p->version, Version::v2012_10_17); @@ -619,8 +608,7 @@ TEST_F(PolicyTest, Parse5) { } TEST_F(PolicyTest, Eval5) { - auto p = Policy(cct.get(), arbitrary_tenant, - bufferlist::static_from_string(example5), true); + auto p = Policy(cct.get(), arbitrary_tenant, example5, true); Environment e; ARN arn1(Partition::aws, Service::iam, @@ -642,8 +630,7 @@ TEST_F(PolicyTest, Eval5) { TEST_F(PolicyTest, Parse6) { boost::optional p; - ASSERT_NO_THROW(p = Policy(cct.get(), arbitrary_tenant, - bufferlist::static_from_string(example6), true)); + ASSERT_NO_THROW(p = Policy(cct.get(), arbitrary_tenant, example6, true)); ASSERT_TRUE(p); EXPECT_EQ(p->text, example6); EXPECT_EQ(p->version, Version::v2012_10_17); @@ -671,8 +658,7 @@ TEST_F(PolicyTest, Parse6) { } TEST_F(PolicyTest, Eval6) { - auto p = Policy(cct.get(), arbitrary_tenant, - bufferlist::static_from_string(example6), true); + auto p = Policy(cct.get(), arbitrary_tenant, example6, true); Environment e; ARN arn1(Partition::aws, Service::iam, @@ -689,8 +675,7 @@ TEST_F(PolicyTest, Eval6) { TEST_F(PolicyTest, Parse7) { boost::optional p; - ASSERT_NO_THROW(p = Policy(cct.get(), arbitrary_tenant, - bufferlist::static_from_string(example7), true)); + ASSERT_NO_THROW(p = Policy(cct.get(), arbitrary_tenant, example7, true)); ASSERT_TRUE(p); EXPECT_EQ(p->text, example7); @@ -721,8 +706,7 @@ TEST_F(PolicyTest, Parse7) { } TEST_F(PolicyTest, Eval7) { - auto p = Policy(cct.get(), arbitrary_tenant, - bufferlist::static_from_string(example7), true); + auto p = Policy(cct.get(), arbitrary_tenant, example7, true); Environment e; auto subacct = FakeIdentity( @@ -970,8 +954,7 @@ TEST_F(IPPolicyTest, ParseIPAddress) { boost::optional p; ASSERT_NO_THROW( - p = Policy(cct.get(), arbitrary_tenant, - bufferlist::static_from_string(ip_address_full_example), true)); + p = Policy(cct.get(), arbitrary_tenant, ip_address_full_example, true)); ASSERT_TRUE(p); EXPECT_EQ(p->text, ip_address_full_example); @@ -1028,14 +1011,11 @@ TEST_F(IPPolicyTest, ParseIPAddress) { TEST_F(IPPolicyTest, EvalIPAddress) { auto allowp = - Policy(cct.get(), arbitrary_tenant, - bufferlist::static_from_string(ip_address_allow_example), true); + Policy(cct.get(), arbitrary_tenant, ip_address_allow_example, true); auto denyp = - Policy(cct.get(), arbitrary_tenant, - bufferlist::static_from_string(ip_address_deny_example), true); + Policy(cct.get(), arbitrary_tenant, ip_address_deny_example, true); auto fullp = - Policy(cct.get(), arbitrary_tenant, - bufferlist::static_from_string(ip_address_full_example), true); + Policy(cct.get(), arbitrary_tenant, ip_address_full_example, true); Environment e; Environment allowedIP, blocklistedIP, allowedIPv6, blocklistedIPv6; allowedIP.emplace("aws:SourceIp","192.168.1.2"); -- 2.39.5