From bc47a1b78c419bb93df85e5dda3a1bd0e7f3ef63 Mon Sep 17 00:00:00 2001 From: Vincent Balat Date: Fri, 26 Apr 2024 16:47:02 +0100 Subject: [PATCH] Remove the ability to switch to another user/group when launched as root This feature is not supported by cohttp. In https://github.com/mirage/ocaml-cohttp/issues/943, Anil suggest to do that instead: setcap 'cap_net_bind_service=+ep' to bind a low port to a non-priviledged user I keep OCSIGENUSER in Makefile for install --- src/Makefile | 4 -- src/baselib/Makefile | 2 - src/baselib/ocsigen_config_static.ml.in | 2 - src/files/ocsigenserver.conf.in | 2 - src/server/ocsigen_config.ml | 8 ---- src/server/ocsigen_config.mli | 6 --- src/server/ocsigen_messages.ml | 34 +--------------- src/server/ocsigen_messages.mli | 7 +--- src/server/ocsigen_parseconfig.ml | 52 ++++++++----------------- src/server/ocsigen_server.ml | 33 +--------------- 10 files changed, 20 insertions(+), 130 deletions(-) diff --git a/src/Makefile b/src/Makefile index ed2cd35f4..f28b91bd9 100644 --- a/src/Makefile +++ b/src/Makefile @@ -12,8 +12,6 @@ confs: ../ocsigenserver.conf.sample ../local/etc/ocsigenserver.conf cat $< \ | sed s%_LOGDIR_%$(LOGDIR)%g \ | sed s%_DATADIR_%$(DATADIR)%g \ - | sed s%_OCSIGENUSER_%$(OCSIGENUSER)%g \ - | sed s%_OCSIGENGROUP_%"$(OCSIGENGROUP)"%g \ | sed s%_COMMANDPIPE_%$(COMMANDPIPE)%g \ | sed s%_MIMEFILE_%$(CONFIGDIR)/mime.types%g \ | sed s%_METADIR_%$(LIBDIR)%g \ @@ -30,8 +28,6 @@ confs: ../ocsigenserver.conf.sample ../local/etc/ocsigenserver.conf | sed s%80\%8080\%g \ | sed s%_LOGDIR_%$(SRC)/local/var/log%g \ | sed s%_DATADIR_%$(SRC)/local/var/lib%g \ - | sed s%\_OCSIGENUSER_\%%g \ - | sed s%\_OCSIGENGROUP_\%%g \ | sed s%_COMMANDPIPE_%$(SRC)/local/var/run/ocsigenserver_command%g \ | sed s%_MIMEFILE_%$(SRC)/src/files/mime.types%g \ | sed s%_METADIR_%${LIBDIR}\"/\>\\_LOGDIR_ _DATADIR_ - _OCSIGENUSER_ - _OCSIGENGROUP_ diff --git a/src/server/ocsigen_config.ml b/src/server/ocsigen_config.ml index db0994240..e4364e288 100644 --- a/src/server/ocsigen_config.ml +++ b/src/server/ocsigen_config.ml @@ -66,8 +66,6 @@ let maxrequestbodysize = ref (Some (Int64.of_int 8000000)) let maxrequestbodysizeinmemory = ref 8192 let maxuploadfilesize = ref (Some (Int64.of_int 2000000)) let defaultcharset = ref (None : string option) -let user = ref (Some !default_user) -let group = ref (Some !default_group) let debugmode = ref false let disablepartialrequests = ref false let usedefaulthostname = ref false @@ -130,8 +128,6 @@ let set_default_charset o = defaultcharset := o let set_datadir o = datadir := o let set_bindir o = bindir := o let set_extdir o = extdir := o -let set_user o = user := o -let set_group o = group := o let set_command_pipe s = command_pipe := s let set_debugmode s = debugmode := s let set_disablepartialrequests s = disablepartialrequests := s @@ -158,8 +154,6 @@ let get_silent () = !silent let get_daemon () = !daemon let get_veryverbose () = !veryverbose let get_debug () = !debug -let get_default_user () = !default_user -let get_default_group () = !default_group let get_minthreads () = !minthreads let get_maxthreads () = !maxthreads @@ -177,8 +171,6 @@ let get_default_charset () = !defaultcharset let get_datadir () = !datadir let get_bindir () = !bindir let get_extdir () = !extdir -let get_user () = !user -let get_group () = !group let get_command_pipe () = !command_pipe let get_debugmode () = !debugmode let get_disablepartialrequests () = !disablepartialrequests diff --git a/src/server/ocsigen_config.mli b/src/server/ocsigen_config.mli index a0ca20f2a..1970a1be2 100644 --- a/src/server/ocsigen_config.mli +++ b/src/server/ocsigen_config.mli @@ -70,8 +70,6 @@ val set_default_charset : string option -> unit val set_datadir : string -> unit val set_bindir : string -> unit val set_extdir : string -> unit -val set_user : string option -> unit -val set_group : string option -> unit val set_command_pipe : string -> unit val set_debugmode : bool -> unit val set_disablepartialrequests : bool -> unit @@ -92,8 +90,6 @@ val get_silent : unit -> bool val get_daemon : unit -> bool val get_veryverbose : unit -> bool val get_debug : unit -> bool -val get_default_user : unit -> string -val get_default_group : unit -> string val get_minthreads : unit -> int val get_maxthreads : unit -> int val get_max_number_of_threads_queued : unit -> int @@ -110,8 +106,6 @@ val get_default_charset : unit -> string option val get_datadir : unit -> string val get_bindir : unit -> string val get_extdir : unit -> string -val get_user : unit -> string option -val get_group : unit -> string option val get_command_pipe : unit -> string val get_debugmode : unit -> bool val get_disablepartialrequests : unit -> bool diff --git a/src/server/ocsigen_messages.ml b/src/server/ocsigen_messages.ml index d1bee2fb4..a94030df3 100644 --- a/src/server/ocsigen_messages.ml +++ b/src/server/ocsigen_messages.ml @@ -30,9 +30,7 @@ let stdout = Lwt_log.channel ~close_mode:`Keep ~channel:Lwt_io.stdout () let loggers = ref [] let access_logger = ref Lwt_log_core.null -let open_files ?(user = Ocsigen_config.get_user ()) - ?(group = Ocsigen_config.get_group ()) () - = +let open_files () = (* CHECK: we are closing asynchronously! That should be ok, though. *) List.iter (fun l -> ignore (Lwt_log.close l : unit Lwt.t)) !loggers; match Ocsigen_config.get_syslog_facility () with @@ -75,35 +73,7 @@ let open_files ?(user = Ocsigen_config.get_user ()) match lev with | Lwt_log.Warning | Lwt_log.Error | Lwt_log.Fatal -> stderr | _ -> stdout) ]; - let gid = - match group with - | None -> Unix.getgid () - | Some group -> ( - try (Unix.getgrnam group).Unix.gr_gid - with Not_found as e -> - ignore (Lwt_log.error "Error: Wrong group"); - raise e) - in - let uid = - match user with - | None -> Unix.getuid () - | Some user -> ( - try (Unix.getpwnam user).Unix.pw_uid - with Not_found as e -> - ignore (Lwt_log.error "Error: Wrong user"); - raise e) - in - Lwt.catch - (fun () -> - Lwt_unix.chown (full_path access_file) uid gid >>= fun () -> - Lwt_unix.chown (full_path warning_file) uid gid >>= fun () -> - Lwt_unix.chown (full_path error_file) uid gid) - (fun e -> - match e with - | Unix.Unix_error (Unix.EPERM, _, _) -> - (* to allow for symlinks to /dev/null *) - Lwt.return_unit - | _ -> Lwt.fail e) + Lwt.return () (****) diff --git a/src/server/ocsigen_messages.mli b/src/server/ocsigen_messages.mli index 9a88baaf2..3aee6b748 100644 --- a/src/server/ocsigen_messages.mli +++ b/src/server/ocsigen_messages.mli @@ -46,10 +46,5 @@ val error_log_path : unit -> string (**/**) -val open_files : - ?user:string option - -> ?group:string option - -> unit - -> unit Lwt.t - +val open_files : unit -> unit Lwt.t val command_f : exn -> string -> string list -> unit Lwt.t diff --git a/src/server/ocsigen_parseconfig.ml b/src/server/ocsigen_parseconfig.ml index a6f4de5cb..ec1dbb97e 100644 --- a/src/server/ocsigen_parseconfig.ml +++ b/src/server/ocsigen_parseconfig.ml @@ -599,15 +599,15 @@ let rec parse_ssl l ~certificate ~privatekey ~ciphers ~dhfile ~curve = | _ -> raise (Config_file_error "Unexpected content inside ") let first_pass c = - let rec aux user group ssl ports sslports = function - | [] -> (user, group), (ssl, ports, sslports) + let rec aux ssl ports sslports = function + | [] -> ssl, ports, sslports | Element (("logdir" as st), [], p) :: ll -> set_logdir (parse_string_tag st p); - aux user group ssl ports sslports ll + aux ssl ports sslports ll | Element (("syslog" as st), [], p) :: ll -> let str = String.lowercase_ascii (parse_string_tag st p) in set_syslog_facility (Some (parse_facility str)); - aux user group ssl ports sslports ll + aux ssl ports sslports ll | Element (("port" as st), atts, p) :: ll -> ( match atts with | [] | [("protocol", "HTTP")] -> @@ -616,21 +616,21 @@ let first_pass c = with Failure _ -> raise (Config_file_error "Wrong value for tag") in - aux user group ssl (po :: ports) sslports ll + aux ssl (po :: ports) sslports ll | [("protocol", "HTTPS")] -> let po = try parse_port (parse_string_tag st p) with Failure _ -> raise (Config_file_error "Wrong value for tag") in - aux user group ssl ports (po :: sslports) ll + aux ssl ports (po :: sslports) ll | _ -> raise (Config_file_error "Wrong attribute for ")) | Element (("minthreads" as st), [], p) :: ll -> set_minthreads (int_of_string st (parse_string_tag st p)); - aux user group ssl ports sslports ll + aux ssl ports sslports ll | Element (("maxthreads" as st), [], p) :: ll -> set_maxthreads (int_of_string st (parse_string_tag st p)); - aux user group ssl ports sslports ll + aux ssl ports sslports ll | Element ("ssl", [], p) :: ll -> ( match ssl with | None -> @@ -642,42 +642,22 @@ let first_pass c = and curve = None in parse_ssl ~certificate ~privatekey ~ciphers ~dhfile ~curve p in - aux user group ssl ports sslports ll + aux ssl ports sslports ll | _ -> raise (Config_file_error "Only one ssl certificate for each server supported for now")) - | Element (("user" as st), [], p) :: ll -> ( - match user with - | None -> aux (Some (parse_string_tag st p)) group ssl ports sslports ll - | _ -> - raise - (Config_file_error "Only one tag for each server allowed")) - | Element (("group" as st), [], p) :: ll -> ( - match group with - | None -> aux user (Some (parse_string_tag st p)) ssl ports sslports ll - | _ -> - raise - (Config_file_error "Only one tag for each server allowed")) + | Element ("user", [], _) :: ll | Element ("group", [], _) :: ll -> + Lwt_log.ign_warning ~section + "Config file: and deprecated. Please do not launch as root."; + aux ssl ports sslports ll | Element (("commandpipe" as st), [], p) :: ll -> set_command_pipe (parse_string_tag st p); - aux user group ssl ports sslports ll - | Element _ :: ll -> aux user group ssl ports sslports ll + aux ssl ports sslports ll + | Element _ :: ll -> aux ssl ports sslports ll | _ -> raise (Config_file_error "Syntax error") in - let (user, group), (si, ports, ssl_ports) = aux None None None [] [] c in - let user = - match user with - | None -> None (* Some (get_default_user ()) *) - | Some s -> if s = "" then None else Some s - in - let group = - match group with - | None -> None (* Some (get_default_group ()) *) - | Some s -> if s = "" then None else Some s - in - Ocsigen_config.set_user user; - Ocsigen_config.set_group group; + let si, ports, ssl_ports = aux None [] [] c in Ocsigen_config.set_ssl_info si; Ocsigen_config.set_ports ports; Ocsigen_config.set_ssl_ports ssl_ports; diff --git a/src/server/ocsigen_server.ml b/src/server/ocsigen_server.ml index 59f5c3391..e4970d136 100644 --- a/src/server/ocsigen_server.ml +++ b/src/server/ocsigen_server.ml @@ -295,9 +295,7 @@ let start ?config () = in let extensions_connector = Ocsigen_extensions.compute_result in let run s = - let user = Ocsigen_config.get_user () - and group = Ocsigen_config.get_group () in - Lwt_main.run (Ocsigen_messages.open_files ~user ~group ()); + Lwt_main.run (Ocsigen_messages.open_files ()); let ports = Ocsigen_config.get_ports () and ssl_ports = Ocsigen_config.get_ssl_ports () in let connection = match ports with [] -> [`All, 80] | l -> l in @@ -326,25 +324,6 @@ let start ?config () = | l, Some (crt, key) -> List.map (fun (a, p) -> a, p, (crt, key)) l | _ -> [] in - let current_uid = Unix.getuid () in - let gid = - match group with - | None -> Unix.getgid () - | Some group -> ( - try (Unix.getgrnam group).Unix.gr_gid - with Not_found as e -> - Ocsigen_messages.errlog "Error: Wrong group"; - raise e) - in - let uid = - match user with - | None -> current_uid - | Some user -> ( - try (Unix.getpwnam user).Unix.pw_uid - with Not_found as e -> - Ocsigen_messages.errlog "Error: Wrong user"; - raise e) - in (* A pipe to communicate with the server *) let commandpipe = Ocsigen_config.get_command_pipe () in (try ignore (Unix.stat commandpipe) @@ -352,20 +331,10 @@ let start ?config () = try let umask = Unix.umask 0 in Unix.mkfifo commandpipe 0o660; - Unix.chown commandpipe uid gid; ignore (Unix.umask umask); Lwt_log.ign_warning ~section "Command pipe created" with e -> Lwt_log.ign_error ~section ~exn:e "Cannot create the command pipe")); - (* I change the user for the process *) - (try - (if current_uid = 0 - then - match user with None -> () | Some user -> Unix.initgroups user gid); - Unix.setgid gid; Unix.setuid uid - with (Unix.Unix_error _ | Failure _) as e -> - Lwt_log.ign_error ~section "Error: Wrong user or group"; - raise e); let minthreads = Ocsigen_config.get_minthreads () and maxthreads = Ocsigen_config.get_maxthreads () in if minthreads > maxthreads