From a2e04f9811d39f65b1d1294823a7f56455a49769 Mon Sep 17 00:00:00 2001 From: Ming Lu Date: Tue, 17 Dec 2024 18:00:44 +0800 Subject: [PATCH 1/2] CA-403620: Drop the usage of fuser in stunnel client proxy The drawback of fuser is that it gets too many things involved. E.g. it is observed that it got stuck on cifs kernel module. This change uses a cleaner way to remember the stunnel client proxy. Even when the xapi restarted unexpectedly, it can stop the remnant stunnel proxy and start a new one. Signed-off-by: Ming Lu --- ocaml/libs/stunnel/stunnel.ml | 58 ++++++++++++-------------------- ocaml/libs/stunnel/stunnel.mli | 3 +- ocaml/xapi/repository_helpers.ml | 7 ++-- 3 files changed, 28 insertions(+), 40 deletions(-) diff --git a/ocaml/libs/stunnel/stunnel.ml b/ocaml/libs/stunnel/stunnel.ml index 6b7d42608e7..0f5c74564c8 100644 --- a/ocaml/libs/stunnel/stunnel.ml +++ b/ocaml/libs/stunnel/stunnel.ml @@ -448,44 +448,30 @@ let with_connect ?unique_id ?use_fork_exec_helper ?write_to_log ~verify_cert ) 5 -let with_client_proxy ~verify_cert ~remote_host ~remote_port ~local_host - ~local_port f = - ( try - D.debug "Clean up running stunnel client proxy if there is any ..." ; - let out, _ = - Forkhelpers.execute_command_get_output "/usr/sbin/fuser" - ["-4k"; string_of_int local_port ^ "/tcp"] - in - D.debug "Killed running stunnel client proxy:%s" out - with - | Forkhelpers.Spawn_internal_error (stderr, stdout, process_status) -> ( - match process_status with - | Unix.WEXITED 1 -> - D.debug "No running stunnel client proxy" - | _ -> - D.warn - "Cleaning up running stunnel client proxy returned unexpectedly: \ - stdout=(%s); stderr=(%s)" - stdout stderr - ) - ) ; - - retry +let with_client_proxy_systemd_service ~verify_cert ~remote_host ~remote_port + ~local_host ~local_port ~service f = + let cmd_path = stunnel_path () in + let config = + config_file + ~accept:(Some (local_host, local_port)) + verify_cert remote_host remote_port + in + let stop () = ignore (Fe_systemctl.stop ~service) in + (* Try stopping anyway before starting it. *) + ignore_exn stop () ; + let conf_path, out = Filename.open_temp_file service ".conf" in + let finally = Xapi_stdext_pervasives.Pervasiveext.finally in + finally (fun () -> - let pid, _ = - attempt_one_connect - (`Local_host_port (local_host, local_port)) - verify_cert remote_host remote_port - in - D.debug "Started a client proxy (pid:%s): %s:%s -> %s:%s" - (string_of_int (getpid pid)) - local_host (string_of_int local_port) remote_host - (string_of_int remote_port) ; - Xapi_stdext_pervasives.Pervasiveext.finally - (fun () -> f ()) - (fun () -> disconnect_with_pid ~wait:false ~force:true pid) + finally (fun () -> output_string out config) (fun () -> close_out out) ; + finally + (fun () -> + Fe_systemctl.start_transient ~service cmd_path [conf_path] ; + f () + ) + (fun () -> ignore_exn stop ()) ) - 5 + (fun () -> Unixext.unlink_safe conf_path) let check_verify_error line = let sub_after i s = diff --git a/ocaml/libs/stunnel/stunnel.mli b/ocaml/libs/stunnel/stunnel.mli index eba084a9ef2..99fcba608ce 100644 --- a/ocaml/libs/stunnel/stunnel.mli +++ b/ocaml/libs/stunnel/stunnel.mli @@ -88,11 +88,12 @@ val with_moved_exn : t -> (t -> 'd) -> 'd val safe_release : t -> unit -val with_client_proxy : +val with_client_proxy_systemd_service : verify_cert:verification_config option -> remote_host:string -> remote_port:int -> local_host:string -> local_port:int + -> service:string -> (unit -> 'a) -> 'a diff --git a/ocaml/xapi/repository_helpers.ml b/ocaml/xapi/repository_helpers.ml index 51699612739..38a46edc3bb 100644 --- a/ocaml/xapi/repository_helpers.ml +++ b/ocaml/xapi/repository_helpers.ml @@ -398,10 +398,11 @@ let with_local_repositories ~__context f = with Pool_role.This_host_is_a_master -> Option.get (Helpers.get_management_ip_addr ~__context) in - Stunnel.with_client_proxy ~verify_cert:(Stunnel_client.pool ()) - ~remote_host:master_addr ~remote_port:Constants.default_ssl_port - ~local_host:"127.0.0.1" + Stunnel.with_client_proxy_systemd_service + ~verify_cert:(Stunnel_client.pool ()) ~remote_host:master_addr + ~remote_port:Constants.default_ssl_port ~local_host:"127.0.0.1" ~local_port:!Xapi_globs.local_yum_repo_port + ~service:"stunnel_proxy_for_update_client" @@ fun () -> let enabled = get_enabled_repositories ~__context in Xapi_stdext_pervasives.Pervasiveext.finally From e7f2b70dfa210418c08b2bb16432f6943c370c16 Mon Sep 17 00:00:00 2001 From: Ming Lu Date: Thu, 26 Dec 2024 14:41:31 +0800 Subject: [PATCH 2/2] CA-403620: Make the stunnel proxy local port configurable Making it configurable can avoid the situation when the port conflicts with others, e.g. an external program from users. Signed-off-by: Ming Lu --- ocaml/xapi/xapi_globs.ml | 1 + 1 file changed, 1 insertion(+) diff --git a/ocaml/xapi/xapi_globs.ml b/ocaml/xapi/xapi_globs.ml index efdcabfbdb6..a62ba9cdb43 100644 --- a/ocaml/xapi/xapi_globs.ml +++ b/ocaml/xapi/xapi_globs.ml @@ -1143,6 +1143,7 @@ let xapi_globs_spec = ; ("max_traces", Int max_traces) ; ("max_observer_file_size", Int max_observer_file_size) ; ("test-open", Int test_open) (* for consistency with xenopsd *) + ; ("local_yum_repo_port", Int local_yum_repo_port) ] let xapi_globs_spec_with_descriptions = []