From: Oguzhan Ozmen <oozmen@bloomberg.net>
Date: Sat, 7 Oct 2023 01:41:58 +0000 (-0400)
Subject: RGW/Roles: use the target/new max-session-duration value when validating it
X-Git-Tag: v19.0.0~154^2~2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=ebbc50c90794f6d8eef6dbf9b55fbc68e01c7b15;p=ceph.git

RGW/Roles: use the target/new max-session-duration value when validating it

If we validate before updating the role's max-session-duration, the
validator function wrongly uses the on-disk (existing/old) value for
validation. Note that the "role" object being updated is in-memory and
only after validation passes, it's persisted on-disk. So, calling role
object's update_max_session_duration API function is OK before the
role->validate_max_session_duration call.

validate_max_session_duration is used by both "role creation" and "role
update". The latter wrongly uses existing role's max_session_duration
value for validation instead of the new/target duration:

$ radosgw-admin ... role create --role-name=myrole ...
$ radosgw-admin ... role get --role-name=myrole | jq
'.MaxSessionDuration'
3600

where 3600 seconds is the default value.

$ radosgw-admin ... role update --role-name=myrole
--max_session_duration=100000
Max session duration updated successfully for role: myrole

Although above update call should have failed since 100K is higher than
43200 (the default max), it succeeded.

$ radosgw-admin ... role get --role-name=myrole | jq
'.MaxSessionDuration'
100000

Fixes: https://tracker.ceph.com/issues/63109

Signed-off-by: Oguzhan Ozmen <oozmen@bloomberg.net>
---

diff --git a/src/rgw/rgw_admin.cc b/src/rgw/rgw_admin.cc
index cc7f5811c9ef..25955a52213c 100644
--- a/src/rgw/rgw_admin.cc
+++ b/src/rgw/rgw_admin.cc
@@ -6922,11 +6922,11 @@ int main(int argc, const char **argv)
       if (ret < 0) {
         return -ret;
       }
+      role->update_max_session_duration(max_session_duration);
       if (!role->validate_max_session_duration(dpp())) {
         ret = -EINVAL;
         return ret;
       }
-      role->update_max_session_duration(max_session_duration);
       ret = role->update(dpp(), null_yield);
       if (ret < 0) {
         return -ret;
diff --git a/src/rgw/rgw_rest_role.cc b/src/rgw/rgw_rest_role.cc
index e71dff5708ff..14e164553665 100644
--- a/src/rgw/rgw_rest_role.cc
+++ b/src/rgw/rgw_rest_role.cc
@@ -1005,12 +1005,12 @@ void RGWUpdateRole::execute(optional_yield y)
     }
   }
 
+  _role->update_max_session_duration(max_session_duration);
   if (!_role->validate_max_session_duration(this)) {
     op_ret = -EINVAL;
     return;
   }
 
-  _role->update_max_session_duration(max_session_duration);
   op_ret = _role->update(this, y);
 
   s->formatter->open_object_section("UpdateRoleResponse");