From a44017d75981f7e69643891fa9ecb84a19ddbdc1 Mon Sep 17 00:00:00 2001 From: Marcus Granado Date: Fri, 20 Jan 2023 15:38:44 +0000 Subject: [PATCH 1/7] CP-41675: add new field override_uefi_certs to xapi.conf This new field will control how the UEFI certificates are managed in two directories: - varstore_dir: by default at "/var/lib/varstored" - default_auth_dir: by default at "/usr/share/varstored" If override_uefi_certs is false: (default) - then use default_auth_dir above, which is the previous behaviour If override_uefi_certs is true: (forced by xapi.conf) - then use varstore_dir above Signed-off-by: Marcus Granado --- ocaml/xapi/xapi_globs.ml | 12 +++++++++++- scripts/xapi.conf | 4 ++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/ocaml/xapi/xapi_globs.ml b/ocaml/xapi/xapi_globs.ml index 42b1fd72b21..b269a6c6a05 100644 --- a/ocaml/xapi/xapi_globs.ml +++ b/ocaml/xapi/xapi_globs.ml @@ -844,7 +844,11 @@ let nbd_client_manager_script = let varstore_rm = ref "/usr/bin/varstore-rm" -let varstore_dir = ref "/usr/share/varstored" +let varstore_dir = ref "/var/lib/varstored" + +let default_auth_dir = ref "/usr/share/varstored" + +let override_uefi_certs = ref false let disable_logging_for = ref [] @@ -1388,6 +1392,12 @@ let other_options = , (fun () -> string_of_bool !ignore_vtpm_unimplemented) , "Do not raise errors on use-cases where VTPM codepaths are not finished." ) + ; ( "override-uefi-certs" + , Arg.Set override_uefi_certs + , (fun () -> string_of_bool !override_uefi_certs) + , "Enable (true) or Disable (false) overriding location for varstored UEFI \ + certificates" + ) ] (* The options can be set with the variable xapiflags in /etc/sysconfig/xapi. diff --git a/scripts/xapi.conf b/scripts/xapi.conf index 18523ba428f..520d2d54771 100644 --- a/scripts/xapi.conf +++ b/scripts/xapi.conf @@ -77,6 +77,10 @@ igd-passthru-vendor-whitelist = 8086 # Allowlist of domain name pattern in binary-url and source-url in repository # repository-domain-name-allowlist = +# Override the default location of RPM-provided certificates in default_auth_dir (/usr/share/varstored) +# to force use of customised UEFI certificates in varstore_dir (/var/lib/varstored) +# override-uefi-certs = true + # Paths to utilities: ############################################ search-path = @LIBEXECDIR@:@OPTDIR@/bin From 3adaf315134cb7f4007121894a35d8a5f5cd70cf Mon Sep 17 00:00:00 2001 From: Marcus Granado Date: Tue, 28 Feb 2023 15:37:59 +0000 Subject: [PATCH 2/7] CP-41675: xapi-start behaves according to field override-uefi-certs in xapi.conf Before: one behaviour: 1) xapi only uses xapi.conf:varstore_dir (default is /usr/share/varstored) After: three possible behaviours: 1) override-uefi-certs=false (default if not in xapi.conf) - create symlink: .from xapi.conf:varstored_dir (default is /var/lib/varstored) .to xapi.conf:default_auth_dir (default is /usr/share/varstored) - contents of xapi.conf:default_auth_dir should be populated and controlled by RPMs. 2) override-uefi-certs=true (must be explicitly stated in xapi.conf) - create new empty directory in xapi.conf:varstored_dir - contents of xapi.conf:default_auth_dir should be populated and controlled by RPMs. - contents of xapi.conf:varstored_dir are supposed to be populated and controlled by user. 2a) pool.uefi-certificates="" - copy all files from xapi.conf:default_auth_dir to xapi.conf:varstored_dir 2b) pool.uefi-certificates contain a valid base64-encoded tar file - untar all files in this field to xapi.conf:varstored_dir Signed-off-by: Marcus Granado --- ocaml/xapi/helpers.ml | 4 ++ ocaml/xapi/xapi_host.ml | 107 ++++++++++++++++++++++++++++++---------- 2 files changed, 86 insertions(+), 25 deletions(-) diff --git a/ocaml/xapi/helpers.ml b/ocaml/xapi/helpers.ml index a5fdd042ca7..af7aa531ee3 100644 --- a/ocaml/xapi/helpers.ml +++ b/ocaml/xapi/helpers.ml @@ -1959,6 +1959,8 @@ module FileSys : sig (* bash-like interface for manipulating files *) type path = string + val realpathm : path -> path + val rmrf : ?rm_top:bool -> path -> unit val mv : src:path -> dest:path -> unit @@ -1969,6 +1971,8 @@ module FileSys : sig end = struct type path = string + let realpathm path = try Unix.readlink path with _ -> path + let rmrf ?(rm_top = true) path = let ( // ) = Filename.concat in let rec rm rm_top path = diff --git a/ocaml/xapi/xapi_host.ml b/ocaml/xapi/xapi_host.ml index c83886f9d78..744340f5603 100644 --- a/ocaml/xapi/xapi_host.ml +++ b/ocaml/xapi/xapi_host.ml @@ -2671,26 +2671,31 @@ let with_temp_file_contents ~contents f = ) (fun () -> Sys.remove filename) -let write_uefi_certificates_to_disk ~__context ~host:_ = - if - Sys.file_exists !Xapi_globs.varstore_dir - && Sys.is_directory !Xapi_globs.varstore_dir - then - match - Base64.decode - (Db.Pool.get_uefi_certificates ~__context - ~self:(Helpers.get_pool ~__context) - ) - with +let ( let@ ) f x = f x + +let ( // ) = Filename.concat + +let really_write_uefi_certificates_to_disk ~__context ~host:_ ~value = + match value with + | "" -> + (* from an existing directory *) + Sys.readdir !Xapi_globs.default_auth_dir + |> Array.iter (fun file -> + let src = !Xapi_globs.default_auth_dir // file in + let dst = !Xapi_globs.varstore_dir // file in + let@ src_fd = Unixext.with_file src [Unix.O_RDONLY] 0o400 in + let@ dst_fd = + Unixext.with_file dst + [Unix.O_WRONLY; Unix.O_CREAT; Unix.O_TRUNC] + 0o644 + in + debug "override_uefi_certs: copy_file %s->%s" src dst ; + ignore (Unixext.copy_file src_fd dst_fd) + ) + | base64_value -> ( + (* from an existing base64 tar file *) + match Base64.decode base64_value with | Ok contents -> - (* Remove existing certs before extracting xapi ones - * to avoid a extract override issue. *) - List.iter - (fun name -> - let path = Filename.concat !Xapi_globs.varstore_dir name in - Unixext.unlink_safe path - ) - ["KEK.auth"; "db.auth"] ; (* No uefi certificates, nothing to do. *) if contents <> "" then ( with_temp_file_contents ~contents @@ -2701,14 +2706,66 @@ let write_uefi_certificates_to_disk ~__context ~host:_ = | Error _ -> debug "UEFI tar file was not extracted: it was not base64-encoded correctly" + ) + +let write_uefi_certificates_to_disk ~__context ~host = + let with_valid_symlink ~from_path ~to_path fn = + debug "override_uefi_certs: with_valid_symlink %s->%s" from_path to_path ; + if Helpers.FileSys.realpathm from_path <> to_path then ( + Helpers.FileSys.rmrf ~rm_top:true from_path ; + Unix.symlink to_path from_path + ) ; + fn from_path + in + let with_empty_dir path fn = + debug "override_uefi_certs: with_empty_dir %s" path ; + Helpers.FileSys.rmrf ~rm_top:false path ; + Unixext.mkdir_rec path 0o755 ; + fn path + in + let check_valid_uefi_certs_in path = + let uefi_certs_in_disk = path |> Helpers.FileSys.realpathm |> Sys.readdir in + (* check expected uefi certificates are present *) + ["KEK.auth"; "db.auth"] + |> List.iter (fun cert -> + let log_of found = + (if found then info else error) + "check_valid_uefi_certs: %s %s in %s" + (if found then "found" else "missing") + cert path + in + uefi_certs_in_disk |> Array.mem cert |> log_of + ) + in + match !Xapi_globs.override_uefi_certs with + | false -> + let@ path = + with_valid_symlink ~from_path:!Xapi_globs.varstore_dir + ~to_path:!Xapi_globs.default_auth_dir + in + check_valid_uefi_certs_in path + | true -> + let@ path = with_empty_dir !Xapi_globs.varstore_dir in + (* get from pool for consistent results across hosts *) + let pool_uefi_certs = + Db.Pool.get_uefi_certificates ~__context + ~self:(Helpers.get_pool ~__context) + in + really_write_uefi_certificates_to_disk ~__context ~host + ~value:pool_uefi_certs ; + check_valid_uefi_certs_in path let set_uefi_certificates ~__context ~host ~value = - Db.Host.set_uefi_certificates ~__context ~self:host ~value ; - Helpers.call_api_functions ~__context (fun rpc session_id -> - Client.Client.Pool.set_uefi_certificates ~rpc ~session_id - ~self:(Helpers.get_pool ~__context) - ~value - ) + match !Xapi_globs.override_uefi_certs with + | false -> + raise Api_errors.(Server_error (Api_errors.operation_not_allowed, [""])) + | true -> + Db.Host.set_uefi_certificates ~__context ~self:host ~value ; + Helpers.call_api_functions ~__context (fun rpc session_id -> + Client.Client.Pool.set_uefi_certificates ~rpc ~session_id + ~self:(Helpers.get_pool ~__context) + ~value + ) let set_iscsi_iqn ~__context ~host ~value = if value = "" then From 656204498a6d3e0faf5472944962c913cd51c725 Mon Sep 17 00:00:00 2001 From: Marcus Granado Date: Fri, 3 Feb 2023 14:29:10 +0000 Subject: [PATCH 3/7] CP-41672: wipe the contents of the pool.uefi_certificates during upgrade as the UEFI certificates will be provided in RPMs, see 'Handling upgrades' in https://github.com/xapi-project/xen-api/issues/4822 Signed-off-by: Marcus Granado --- ocaml/xapi/xapi_db_upgrade.ml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/ocaml/xapi/xapi_db_upgrade.ml b/ocaml/xapi/xapi_db_upgrade.ml index 7315495aa5f..7544046c997 100644 --- a/ocaml/xapi/xapi_db_upgrade.ml +++ b/ocaml/xapi/xapi_db_upgrade.ml @@ -868,6 +868,19 @@ let remove_legacy_ssl_support = ) } +let empty_pool_uefi_certificates = + { + description= + "empty contents of pool.uefi_certificates, as they are now provided in \ + RPMs" + ; version= (fun _ -> true) + ; fn= + (fun ~__context -> + let pool = Helpers.get_pool ~__context in + Db.Pool.set_uefi_certificates ~__context ~self:pool ~value:"" + ) + } + let rules = [ upgrade_domain_type @@ -896,6 +909,7 @@ let rules = ; upgrade_cluster_timeouts ; upgrade_secrets ; remove_legacy_ssl_support + ; empty_pool_uefi_certificates ] (* Maybe upgrade most recent db *) From cde18c0bc10d05b91ac222b297982ea7731c1449 Mon Sep 17 00:00:00 2001 From: Marcus Granado Date: Fri, 17 Feb 2023 17:12:53 +0000 Subject: [PATCH 4/7] CP-40847: CP-42007: make pool.uefi-certificates field read-only when xapi.conf:override_uefi_certs=false, by blocking the access to the api method Pool.set_uefi_certificates in this situation. Signed-off-by: Marcus Granado --- ocaml/xapi/xapi_pool.ml | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/ocaml/xapi/xapi_pool.ml b/ocaml/xapi/xapi_pool.ml index 05ac3d03240..0467b6557b6 100644 --- a/ocaml/xapi/xapi_pool.ml +++ b/ocaml/xapi/xapi_pool.ml @@ -3588,14 +3588,27 @@ let disable_repository_proxy ~__context ~self = ) let set_uefi_certificates ~__context ~self ~value = - Db.Pool.set_uefi_certificates ~__context ~self ~value ; - Helpers.call_api_functions ~__context (fun rpc session_id -> - List.iter - (fun host -> - Client.Host.write_uefi_certificates_to_disk ~rpc ~session_id ~host + match !Xapi_globs.override_uefi_certs with + | false -> + raise + Api_errors.( + Server_error + ( Api_errors.operation_not_allowed + , [ + "Setting UEFI certificates is not possible when \ + override_uefi_certs is false" + ] + ) ) - (Db.Host.get_all ~__context) - ) + | true -> + Db.Pool.set_uefi_certificates ~__context ~self ~value ; + Helpers.call_api_functions ~__context (fun rpc session_id -> + List.iter + (fun host -> + Client.Host.write_uefi_certificates_to_disk ~rpc ~session_id ~host + ) + (Db.Host.get_all ~__context) + ) let set_https_only ~__context ~self:_ ~value = Helpers.call_api_functions ~__context (fun rpc session_id -> From c622a7e0a76e1f83ee7d6613fe912180155dea72 Mon Sep 17 00:00:00 2001 From: Marcus Granado Date: Tue, 21 Feb 2023 17:26:55 +0000 Subject: [PATCH 5/7] CP-41675: fix idempotent behaviour of Helpers.FileSys.rmrf If a file has already been deleted using rmrf, using it again currently raises a ENOENT exception. This is not the expected behaviour of rm -rf. This patch ensures that for rmrf, ENOENT is a noop as expected. The few modules in xapi using rmrf expect no file after calling rmrf, so this change does not change their expectation for the rmrf behaviour. Signed-off-by: Marcus Granado --- ocaml/xapi/helpers.ml | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/ocaml/xapi/helpers.ml b/ocaml/xapi/helpers.ml index af7aa531ee3..6abd2fb8ad3 100644 --- a/ocaml/xapi/helpers.ml +++ b/ocaml/xapi/helpers.ml @@ -1976,13 +1976,19 @@ end = struct let rmrf ?(rm_top = true) path = let ( // ) = Filename.concat in let rec rm rm_top path = - let st = Unix.lstat path in - match st.Unix.st_kind with - | Unix.S_DIR -> - Sys.readdir path |> Array.iter (fun file -> rm true (path // file)) ; - if rm_top then Unix.rmdir path - | _ -> - Unix.unlink path + match Unix.lstat path with + | exception Unix.Unix_error (Unix.ENOENT, _, _) -> + () (*noop*) + | exception e -> + raise e + | st -> ( + match st.Unix.st_kind with + | Unix.S_DIR -> + Sys.readdir path |> Array.iter (fun file -> rm true (path // file)) ; + if rm_top then Unix.rmdir path + | _ -> + Unix.unlink path + ) in try rm rm_top path with e -> From 65713477a51aca57f81afc3911a606600596fcd9 Mon Sep 17 00:00:00 2001 From: Marcus Granado Date: Wed, 22 Feb 2023 17:05:57 +0000 Subject: [PATCH 6/7] CP-42007: platform:secureboot=auto means platform:secureboot=true always When the uefi certificates are present in the pool.uefi-certificates field of the xapi db: 1) During vm-start, platform:secureboot=auto means platform:secureboot=true 2) During vm-create in XenCenter, the UEFI Secure Boot checkbox is enabled 3) other clients may rely on this field being populated to decide if/how they should set the VM platform:secureboot field. To maintain compatibility with this existing behaviour of these components, in case override_uefi_certs=false (which is default in xenserver), then during xapi-start do populate pool.uefi-certificates with the uefi certificates from /var/lib/varstored, which links to the uefi certificates statically installed by RPMs. Signed-off-by: Marcus Granado --- ocaml/xapi/helpers.ml | 12 ++++++++++++ ocaml/xapi/xapi_host.ml | 27 ++++++++++++++++++++++++++- 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/ocaml/xapi/helpers.ml b/ocaml/xapi/helpers.ml index 6abd2fb8ad3..ff1ec47dcb7 100644 --- a/ocaml/xapi/helpers.ml +++ b/ocaml/xapi/helpers.ml @@ -1955,6 +1955,18 @@ end = struct Xapi_psr_util.load_psr_pool_secrets () end +let ( let@ ) f x = f x + +let with_temp_out_ch ch f = finally (fun () -> f ch) (fun () -> close_out ch) + +let with_temp_file ?mode prefix suffix f = + let path, channel = Filename.open_temp_file ?mode prefix suffix in + finally (fun () -> f (path, channel)) (fun () -> Unix.unlink path) + +let with_temp_out_ch_of_temp_file ?mode prefix suffix f = + let@ path, channel = with_temp_file ?mode prefix suffix in + f (path, channel |> with_temp_out_ch) + module FileSys : sig (* bash-like interface for manipulating files *) type path = string diff --git a/ocaml/xapi/xapi_host.ml b/ocaml/xapi/xapi_host.ml index 744340f5603..44efc37cc26 100644 --- a/ocaml/xapi/xapi_host.ml +++ b/ocaml/xapi/xapi_host.ml @@ -2675,6 +2675,22 @@ let ( let@ ) f x = f x let ( // ) = Filename.concat +let really_read_uefi_certificates_from_disk ~__context ~host:_ from_path = + let certs_files = Sys.readdir from_path |> Array.map (( // ) from_path) in + let@ temp_file, with_temp_out_ch = + Helpers.with_temp_out_ch_of_temp_file ~mode:[Open_binary] + "pool-uefi-certificates" "tar" + in + if Array.length certs_files > 0 then ( + let@ temp_out_ch = with_temp_out_ch in + Tar_unix.Archive.create + (certs_files |> Array.to_list) + (temp_out_ch |> Unix.descr_of_out_channel) ; + debug "UEFI tar file %s populated from directory %s" temp_file from_path + ) else + debug "UEFI tar file %s empty from directory %s" temp_file from_path ; + temp_file |> Unixext.string_of_file |> Base64.encode_string + let really_write_uefi_certificates_to_disk ~__context ~host:_ ~value = match value with | "" -> @@ -2743,7 +2759,16 @@ let write_uefi_certificates_to_disk ~__context ~host = with_valid_symlink ~from_path:!Xapi_globs.varstore_dir ~to_path:!Xapi_globs.default_auth_dir in - check_valid_uefi_certs_in path + check_valid_uefi_certs_in path ; + if Pool_role.is_master () then + let disk_uefi_certs_tar = + really_read_uefi_certificates_from_disk ~__context ~host + !Xapi_globs.varstore_dir + in + (* synchronize read-only field with contents in disk *) + Db.Pool.set_uefi_certificates ~__context + ~self:(Helpers.get_pool ~__context) + ~value:disk_uefi_certs_tar | true -> let@ path = with_empty_dir !Xapi_globs.varstore_dir in (* get from pool for consistent results across hosts *) From 097dd5af39d3e70457f3c3db6f7e120683272903 Mon Sep 17 00:00:00 2001 From: Marcus Granado <590521+mg12@users.noreply.github.com> Date: Tue, 28 Feb 2023 15:06:55 +0000 Subject: [PATCH 7/7] CP-42007: separate error msg from exception generation Co-authored-by: Pau Ruiz Safont Signed-off-by: Marcus Granado <590521+mg12@users.noreply.github.com> --- ocaml/xapi/xapi_pool.ml | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/ocaml/xapi/xapi_pool.ml b/ocaml/xapi/xapi_pool.ml index 0467b6557b6..8faa89926fb 100644 --- a/ocaml/xapi/xapi_pool.ml +++ b/ocaml/xapi/xapi_pool.ml @@ -3590,16 +3590,11 @@ let disable_repository_proxy ~__context ~self = let set_uefi_certificates ~__context ~self ~value = match !Xapi_globs.override_uefi_certs with | false -> - raise - Api_errors.( - Server_error - ( Api_errors.operation_not_allowed - , [ - "Setting UEFI certificates is not possible when \ - override_uefi_certs is false" - ] - ) - ) + let msg = + "Setting UEFI certificates is not possible when override_uefi_certs is \ + false" + in + raise Api_errors.(Server_error (operation_not_allowed, [msg])) | true -> Db.Pool.set_uefi_certificates ~__context ~self ~value ; Helpers.call_api_functions ~__context (fun rpc session_id ->