From 310429c6ed2204af660cfc525e49d019b7295f7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edwin=20T=C3=B6r=C3=B6k?= Date: Fri, 11 Oct 2024 17:33:39 +0100 Subject: [PATCH 1/2] Revert "CP-48676: Don't check resuable pool session validity by default" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit c27b1d45b9a209ae922250a54b2a0a076af7a531. Signed-off-by: Edwin Török --- ocaml/xapi/xapi_globs.ml | 7 ------- ocaml/xapi/xapi_session.ml | 4 +--- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/ocaml/xapi/xapi_globs.ml b/ocaml/xapi/xapi_globs.ml index c675e036451..38c9f88ac74 100644 --- a/ocaml/xapi/xapi_globs.ml +++ b/ocaml/xapi/xapi_globs.ml @@ -1055,8 +1055,6 @@ let disable_webserver = ref false let reuse_pool_sessions = ref true -let validate_reusable_pool_session = ref false - let test_open = ref 0 let xapi_globs_spec = @@ -1627,11 +1625,6 @@ let other_options = , (fun () -> string_of_bool !reuse_pool_sessions) , "Enable the reuse of pool sessions" ) - ; ( "validate-reusable-pool-session" - , Arg.Set validate_reusable_pool_session - , (fun () -> string_of_bool !validate_reusable_pool_session) - , "Enable the reuse of pool sessions" - ) ] (* The options can be set with the variable xapiflags in /etc/sysconfig/xapi. diff --git a/ocaml/xapi/xapi_session.ml b/ocaml/xapi/xapi_session.ml index bd981cb3692..abced81ca42 100644 --- a/ocaml/xapi/xapi_session.ml +++ b/ocaml/xapi/xapi_session.ml @@ -701,9 +701,7 @@ let login_no_password_common ~__context ~uname ~originator ~host ~pool with_lock reusable_pool_session_lock (fun () -> if !reusable_pool_session <> Ref.null - && ((not !Xapi_globs.validate_reusable_pool_session) - || is_valid_session !reusable_pool_session - ) + && is_valid_session !reusable_pool_session then !reusable_pool_session else From 76008ce63fff0a6c0c2558aa00423488ed5bd872 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edwin=20T=C3=B6r=C3=B6k?= Date: Fri, 11 Oct 2024 17:33:44 +0100 Subject: [PATCH 2/2] Revert "CP-48676: Reuse pool sessions on slave logins." MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit af68185ba81b9817741992410b48f9e28e118e06. Signed-off-by: Edwin Török --- ocaml/xapi/xapi_globs.ml | 7 ---- ocaml/xapi/xapi_session.ml | 77 +++++++------------------------------- 2 files changed, 13 insertions(+), 71 deletions(-) diff --git a/ocaml/xapi/xapi_globs.ml b/ocaml/xapi/xapi_globs.ml index 38c9f88ac74..5407faf3bf4 100644 --- a/ocaml/xapi/xapi_globs.ml +++ b/ocaml/xapi/xapi_globs.ml @@ -1053,8 +1053,6 @@ let pool_recommendations_dir = ref "/etc/xapi.pool-recommendations.d" let disable_webserver = ref false -let reuse_pool_sessions = ref true - let test_open = ref 0 let xapi_globs_spec = @@ -1620,11 +1618,6 @@ let other_options = , (fun () -> !Uuidx.make_default == Uuidx.make_uuid_fast |> string_of_bool) , "Use PRNG based UUID generator instead of CSPRNG" ) - ; ( "reuse-pool-sessions" - , Arg.Set reuse_pool_sessions - , (fun () -> string_of_bool !reuse_pool_sessions) - , "Enable the reuse of pool sessions" - ) ] (* The options can be set with the variable xapiflags in /etc/sysconfig/xapi. diff --git a/ocaml/xapi/xapi_session.ml b/ocaml/xapi/xapi_session.ml index abced81ca42..7e77def1f43 100644 --- a/ocaml/xapi/xapi_session.ml +++ b/ocaml/xapi/xapi_session.ml @@ -398,29 +398,19 @@ let is_subject_suspended ~__context ~cache subject_identifier = debug "Subject identifier %s is suspended" subject_identifier ; (is_suspended, subject_name) -let reusable_pool_session = ref Ref.null - -let reusable_pool_session_lock = Mutex.create () - let destroy_db_session ~__context ~self = Context.with_tracing ~__context __FUNCTION__ @@ fun __context -> - with_lock reusable_pool_session_lock (fun () -> - if self <> !reusable_pool_session then ( - Xapi_event.on_session_deleted self ; - (* unregister from the event system *) - (* This info line is important for tracking, auditability and client accountability purposes on XenServer *) - (* Never print the session id nor uuid: they are secret values that should be known only to the user that *) - (* logged in. Instead, we print a non-invertible hash as the tracking id for the session id *) - (* see also task creation in context.ml *) - (* CP-982: create tracking id in log files to link username to actions *) - info "Session.destroy %s" (trackid self) ; - Rbac_audit.session_destroy ~__context ~session_id:self ; - (try Db.Session.destroy ~__context ~self with _ -> ()) ; - Rbac.destroy_session_permissions_tbl ~session_id:self - ) else - info "Skipping Session.destroy for reusable pool session %s" - (trackid self) - ) + Xapi_event.on_session_deleted self ; + (* unregister from the event system *) + (* This info line is important for tracking, auditability and client accountability purposes on XenServer *) + (* Never print the session id nor uuid: they are secret values that should be known only to the user that *) + (* logged in. Instead, we print a non-invertible hash as the tracking id for the session id *) + (* see also task creation in context.ml *) + (* CP-982: create tracking id in log files to link username to actions *) + info "Session.destroy %s" (trackid self) ; + Rbac_audit.session_destroy ~__context ~session_id:self ; + (try Db.Session.destroy ~__context ~self with _ -> ()) ; + Rbac.destroy_session_permissions_tbl ~session_id:self (* CP-703: ensure that activate sessions are invalidated in a bounded time *) (* in response to external authentication/directory services updates, such as *) @@ -620,8 +610,8 @@ let revalidate_all_sessions ~__context = debug "Unexpected exception while revalidating external sessions: %s" (ExnHelper.string_of_exn e) -let login_no_password_common_create_session ~__context ~uname ~originator ~host - ~pool ~is_local_superuser ~subject ~auth_user_sid ~auth_user_name +let login_no_password_common ~__context ~uname ~originator ~host ~pool + ~is_local_superuser ~subject ~auth_user_sid ~auth_user_name ~rbac_permissions ~db_ref ~client_certificate = Context.with_tracing ~originator ~__context __FUNCTION__ @@ fun __context -> let create_session () = @@ -671,47 +661,6 @@ let login_no_password_common_create_session ~__context ~uname ~originator ~host ignore (Client.Pool.get_all ~rpc ~session_id) ; session_id -let login_no_password_common ~__context ~uname ~originator ~host ~pool - ~is_local_superuser ~subject ~auth_user_sid ~auth_user_name - ~rbac_permissions ~db_ref ~client_certificate = - Context.with_tracing ~originator ~__context __FUNCTION__ @@ fun __context -> - let is_valid_session session_id = - try - (* Call an API function to check the session is still valid *) - let rpc = Helpers.make_rpc ~__context in - ignore (Client.Pool.get_all ~rpc ~session_id) ; - true - with Api_errors.Server_error (err, _) -> - info "Invalid session: %s" err ; - false - in - let create_session () = - let new_session_id = - login_no_password_common_create_session ~__context ~uname ~originator - ~host ~pool ~is_local_superuser ~subject ~auth_user_sid ~auth_user_name - ~rbac_permissions ~db_ref ~client_certificate - in - new_session_id - in - if - (originator, pool, is_local_superuser, uname) - = (xapi_internal_originator, true, true, None) - && !Xapi_globs.reuse_pool_sessions - then - with_lock reusable_pool_session_lock (fun () -> - if - !reusable_pool_session <> Ref.null - && is_valid_session !reusable_pool_session - then - !reusable_pool_session - else - let new_session_id = create_session () in - reusable_pool_session := new_session_id ; - new_session_id - ) - else - create_session () - (* XXX: only used internally by the code which grants the guest access to the API. Needs to be protected by a proper access control system *) let login_no_password ~__context ~uname ~host ~pool ~is_local_superuser ~subject