]> git.apps.os.sepia.ceph.com Git - ceph-ci.git/commitdiff
rgw/iam: Policy() takes string instead of bufferlist
authorCasey Bodley <cbodley@redhat.com>
Thu, 1 Feb 2024 18:10:00 +0000 (13:10 -0500)
committerCasey Bodley <cbodley@redhat.com>
Wed, 10 Apr 2024 17:09:15 +0000 (13:09 -0400)
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 <cbodley@redhat.com>
12 files changed:
src/rgw/rgw_admin.cc
src/rgw/rgw_auth.cc
src/rgw/rgw_iam_policy.cc
src/rgw/rgw_iam_policy.h
src/rgw/rgw_op.cc
src/rgw/rgw_op.h
src/rgw/rgw_polparser.cc
src/rgw/rgw_rest_pubsub.cc
src/rgw/rgw_rest_role.cc
src/rgw/rgw_rest_sts.cc
src/rgw/rgw_rest_user_policy.cc
src/test/rgw/test_rgw_iam_policy.cc

index f983ee708fdb2340a02d3336138fb1f15136631f..5cef1fe0bb5b312901368a0f56c80f7f1336fd72 100644 (file)
@@ -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<bool>(
            "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<bool>(
                                   "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<bool>(
                                   "rgw_policy_reject_invalid_principals"));
       } catch (rgw::IAM::PolicyParseException& e) {
index b7e71f6e02d7f2a8b0e3a325b5619db0cf6f5229..9da2f6afc949613996f67d3d0cba91f4685d4bfc 100644 (file)
@@ -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
index ba8507e501742f015f1a24c6b2ec8993983e6678..470a3e604d2def38abd842fe4d6db5f7862ac564 100644 (file)
@@ -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<kParseNumbersAsStringsFlag |
index 35d5a5698c5611b70ad6d01c1e8b5c5276f7b7ef..5eac17d3a94d6169991191bff38f8fce26ca36d2 100644 (file)
@@ -546,7 +546,7 @@ struct Policy {
   // be false when reading a stored bucket policy so as not to break
   // backwards configuration.
   Policy(CephContext* cct, const std::string& tenant,
-        const bufferlist& text,
+        std::string text,
         bool reject_invalid_principals);
 
   Effect eval(const Environment& e,
index c188e9e6f412fbc9cbc1831d36356a724ea7f779..cb8bbae5d99bbd6bc76a73cc672431aeb17ddd86 100644 (file)
@@ -323,12 +323,13 @@ static int get_obj_policy_from_attr(const DoutPrefixProvider *dpp,
 }
 
 
-static boost::optional<Policy> get_iam_policy_from_attr(CephContext* cct,
-                                                       map<string, bufferlist>& 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<Policy>
+get_iam_policy_from_attr(CephContext* cct,
+                         const map<string, bufferlist>& 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<string, bufferlist>& attrs)
 }
 
 vector<Policy> get_iam_user_policy_from_attr(CephContext* cct,
-                        map<string, bufferlist>& attrs,
+                        const map<string, bufferlist>& attrs,
                         const string& tenant) {
   vector<Policy> policies;
-  if (auto it = attrs.find(RGW_ATTR_USER_POLICY); it != attrs.end()) {
-   bufferlist out_bl = attrs[RGW_ATTR_USER_POLICY];
-   map<string, string> 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<string, string> 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<bool>("rgw_policy_reject_invalid_principals"));
     rgw::sal::Attrs attrs(s->bucket_attrs);
     if (s->bucket_access_conf &&
index fa61aefc5632464f167b6d652da2eeb064db4ac5..dd30734b682099e9adaef8184b8f4e1dff396038 100644 (file)
@@ -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<rgw::IAM::Policy> get_iam_user_policy_from_attr(CephContext* cct,
-                        std::map<std::string, bufferlist>& attrs,
+                        const std::map<std::string, bufferlist>& attrs,
                         const std::string& tenant);
 
 inline int get_system_versioning_params(req_state *s,
index f81eda7fe97a09a9e1e51bfae62f5ef34b65b18a..1bc98cabc89c07472e9f2d690fd78e6018630b68 100644 (file)
@@ -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<bool>("rgw_policy_reject_invalid_principals"));
   } catch (const rgw::IAM::PolicyParseException& e) {
     std::cerr << fname << ": " << e.what() << std::endl;
index ec0b2ff6e86562a0d037cfb2a05b9ba47e575295..9f02c5e73a82038cbc95aa038e6a389c181b1380 100644 (file)
@@ -86,11 +86,10 @@ bool topics_has_endpoint_secret(const rgw_pubsub_topics& topics) {
 }
 
 std::optional<rgw::IAM::Policy> 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<bool>("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);
index 0e338bc63a019ff38f61e31309ca1fa90684597f..f9580b206ffcc8b7159ffe58388a1a02752ebe15 100644 (file)
@@ -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<bool>("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<bool>("rgw_policy_reject_invalid_principals"));
   }
   catch (rgw::IAM::PolicyParseException& e) {
index 4ec0d211687929a93030f6028225d379fb4a3b7d..84284a587f980b0b3abf556565935d689d9de448 100644 (file)
@@ -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<bool>("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<bool>("rgw_policy_reject_invalid_principals"));
     }
     catch (rgw::IAM::PolicyParseException& e) {
index e452369ccb7a2d5b10b9b37d41dc19bb6649c88f..b390bfb5b82a5abd04b61beec8e5228cfa654cfa 100644 (file)
@@ -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<bool>("rgw_policy_reject_invalid_principals"));
     std::map<std::string, std::string> policies;
     if (auto it = user->get_attrs().find(RGW_ATTR_USER_POLICY); it != user->get_attrs().end()) {
index 156b341ca70d34becf9ce43ab39e386ae1107afa..1a308c0f68a02b1c643dded58b54cb24cc036eeb 100644 (file)
@@ -178,9 +178,7 @@ public:
 TEST_F(PolicyTest, Parse1) {
   boost::optional<Policy> 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<Policy> 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<Policy> 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<Policy> 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<Policy> 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<Policy> 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<Policy> 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<Policy> 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");