RGW: Getting an RGW service Segfault when assigning an attribute to an IAM role

The current implementation of RGWTagRole, which inherits RGWRestRole::verify_permission() from its base class, encounters a critical issue when loading RGWRole from storage and initializing the RGWRestRole::_role member variable.To address this issue and ensure that errors in initialization are appropriately handled, it is proposed to separate the initialization logic from the permission-checking logic.

Fixes: https://tracker.ceph.com/issues/64232
Signed-off-by: Shreyansh Sancheti <ssanchet@redhat.com>

diff --git a/src/rgw/rgw_rest_role.cc b/src/rgw/rgw_rest_role.cc
index fd537c0c994692113c393d2aee9c741032c36b51..6132b111178706492feefcca6fb7937c081dfecb 100644 (file)
--- a/src/rgw/rgw_rest_role.cc
+++ b/src/rgw/rgw_rest_role.cc
@@ -80,23 +80,13 @@ int RGWRestRole::verify_permission(optional_yield y)
   if (s->auth.identity->is_anonymous()) {
     return -EACCES;
   }
-
+  
   string role_name = s->info.args.get("RoleName");
-  std::unique_ptr<rgw::sal::RGWRole> role = driver->get_role(role_name,
-                                                           s->user->get_tenant());
-  if (op_ret = role->get(s, y); op_ret < 0) {
-    if (op_ret == -ENOENT) {
-      op_ret = -ERR_NO_ROLE_FOUND;
-    }
-    return op_ret;
-  }
-
   if (int ret = check_caps(s->user->get_caps()); ret == 0) {
-    _role = std::move(role);
     return ret;
   }
 
-  string resource_name = role->get_path() + role_name;
+  string resource_name = _role->get_path() + role_name;
   uint64_t op = get_op();
   if (!verify_user_permission(this,
                               s,
@@ -107,8 +97,21 @@ int RGWRestRole::verify_permission(optional_yield y)
     return -EACCES;
   }
 
-  _role = std::move(role);
+  return 0;
+}
 
+int RGWRestRole::init_processing(optional_yield y)
+{
+  string role_name = s->info.args.get("RoleName");
+  std::unique_ptr<rgw::sal::RGWRole> role = driver->get_role(role_name,
+                                                             s->user->get_tenant());
+  if (int ret = role->get(s, y); ret < 0) {
+    if (ret == -ENOENT) {
+      return -ERR_NO_ROLE_FOUND;
+    }
+    return ret;
+  }
+  _role = std::move(role);
   return 0;
 }
 
@@ -202,6 +205,11 @@ int RGWCreateRole::verify_permission(optional_yield y)
   return 0;
 }
 
+int RGWCreateRole::init_processing(optional_yield y)
+{
+  return 0; // avoid calling RGWRestRole::init_processing()
+}
+
 int RGWCreateRole::get_params()
 {
   role_name = s->info.args.get("RoleName");
@@ -437,6 +445,11 @@ int RGWGetRole::_verify_permission(const rgw::sal::RGWRole* role)
   return 0;
 }
 
+int RGWGetRole::init_processing(optional_yield y)
+{
+  return 0; // avoid calling RGWRestRole::init_processing()
+}
+
 int RGWGetRole::get_params()
 {
   role_name = s->info.args.get("RoleName");
@@ -558,6 +571,11 @@ int RGWListRoles::verify_permission(optional_yield y)
   return 0;
 }
 
+int RGWListRoles::init_processing(optional_yield y)
+{
+  return 0; // avoid calling RGWRestRole::init_processing()
+}
+
 int RGWListRoles::get_params()
 {
   path_prefix = s->info.args.get("PathPrefix");

diff --git a/src/rgw/rgw_rest_role.h b/src/rgw/rgw_rest_role.h
index 98a08833bf72bdac77090f81d943aff06d6c49fb..a93c418001f98924658707ab9c6e0a6438662eb0 100644 (file)
--- a/src/rgw/rgw_rest_role.h
+++ b/src/rgw/rgw_rest_role.h
@@ -21,6 +21,7 @@ protected:
   std::vector<std::string> tagKeys;
   std::unique_ptr<rgw::sal::RGWRole> _role;
   int verify_permission(optional_yield y) override;
+  int init_processing(optional_yield y) override;
   void send_response() override;
   virtual uint64_t get_op() = 0;
   int parse_tags();
@@ -43,6 +44,7 @@ class RGWCreateRole : public RGWRoleWrite {
 public:
   RGWCreateRole(const bufferlist& bl_post_body) : bl_post_body(bl_post_body) {};
   int verify_permission(optional_yield y) override;
+  int init_processing(optional_yield y) override;
   void execute(optional_yield y) override;
   int get_params();
   const char* name() const override { return "create_role"; }
@@ -66,6 +68,7 @@ class RGWGetRole : public RGWRoleRead {
 public:
   RGWGetRole() = default;
   int verify_permission(optional_yield y) override;
+  int init_processing(optional_yield y) override; 
   void execute(optional_yield y) override;
   int get_params();
   const char* name() const override { return "get_role"; }
@@ -88,6 +91,7 @@ class RGWListRoles : public RGWRoleRead {
 public:
   RGWListRoles() = default;
   int verify_permission(optional_yield y) override;
+  int init_processing(optional_yield y) override;
   void execute(optional_yield y) override;
   int get_params();
   const char* name() const override { return "list_roles"; }