diff --git a/configure.ml b/configure.ml index 91eee0f0ed7..607d3f58dae 100644 --- a/configure.ml +++ b/configure.ml @@ -58,7 +58,7 @@ let args = ; flag "yumpluginconfdir" ~doc:"DIR YUM plugins conf dir" ~default:"/etc/yum/pluginconf.d" ; flag "xapi_api_version_major" ~doc:"xapi api major version" ~default:"2" - ; flag "xapi_api_version_minor" ~doc:"xapi api minor version" ~default:"21" + ; flag "xapi_api_version_minor" ~doc:"xapi api minor version" ~default:"23" ] |> Arg.align diff --git a/dune-project b/dune-project index 548b8b5efad..7fb1ad9fc89 100644 --- a/dune-project +++ b/dune-project @@ -50,6 +50,7 @@ (package (name tgroup) + (synopsis "Thread group management library") (depends xapi-log xapi-stdext-unix)) (package diff --git a/ocaml/idl/api_version.ml b/ocaml/idl/api_version.ml index 23028c50796..b73173fbd1f 100644 --- a/ocaml/idl/api_version.ml +++ b/ocaml/idl/api_version.ml @@ -18,4 +18,4 @@ let api_version_major = 2L -let api_version_minor = 21L +let api_version_minor = 23L diff --git a/ocaml/idl/datamodel_common.ml b/ocaml/idl/datamodel_common.ml index 25e689f4fa4..fd3ed22710a 100644 --- a/ocaml/idl/datamodel_common.ml +++ b/ocaml/idl/datamodel_common.ml @@ -10,7 +10,7 @@ open Datamodel_roles to leave a gap for potential hotfixes needing to increment the schema version.*) let schema_major_vsn = 5 -let schema_minor_vsn = 902 +let schema_minor_vsn = 903 (* Historical schema versions just in case this is useful later *) let rio_schema_major_vsn = 5 diff --git a/ocaml/idl/datamodel_lifecycle.ml b/ocaml/idl/datamodel_lifecycle.ml index 738146e9cc4..ac61b896a5f 100644 --- a/ocaml/idl/datamodel_lifecycle.ml +++ b/ocaml/idl/datamodel_lifecycle.ml @@ -145,6 +145,8 @@ let prototyped_of_field = function Some "26.2.0" | "VM_metrics", "numa_optimised" -> Some "26.2.0" + | "VM", "secureboot_certificates_state" -> + Some "26.14.0" | "VM", "groups" -> Some "24.19.1" | "VM", "pending_guidances_full" -> @@ -289,6 +291,8 @@ let prototyped_of_message = function Some "24.0.0" | "VM", "sysprep" -> Some "25.24.0" + | "VM", "update_secureboot_certificates_on_boot" -> + Some "26.14.0" | "VM", "get_secureboot_readiness" -> Some "24.17.0" | "VM", "set_uefi_mode" -> diff --git a/ocaml/idl/datamodel_types.ml b/ocaml/idl/datamodel_types.ml index 1be90528856..72b977b18b8 100644 --- a/ocaml/idl/datamodel_types.ml +++ b/ocaml/idl/datamodel_types.ml @@ -101,6 +101,8 @@ let rel_nile_preview = "nile-preview" let rel_nile = "nile" +let rel_orinoco = "orinoco" + type api_release = { code_name: string option ; version_major: int @@ -352,6 +354,13 @@ let release_order_full = ; branding= "XenServer 8" ; release_date= None } + ; { + code_name= Some rel_orinoco + ; version_major= 2 + ; version_minor= 23 + ; branding= "XenServer 9" + ; release_date= None + } ] (* When you add a new release, use the version number of the latest release, "Unreleased" for the branding, and Some "" for the release date, until the actual values are finalised. *) diff --git a/ocaml/idl/datamodel_vm.ml b/ocaml/idl/datamodel_vm.ml index 018433498d0..fcd93c02aea 100644 --- a/ocaml/idl/datamodel_vm.ml +++ b/ocaml/idl/datamodel_vm.ml @@ -2353,7 +2353,45 @@ let set_HVM_boot_policy = let set_NVRAM_EFI_variables = call ~flags:[`Session] ~name:"set_NVRAM_EFI_variables" ~lifecycle:[(Published, rel_naples, "")] - ~params:[(Ref _vm, "self", "The VM"); (String, "value", "The value")] + ~versioned_params: + [ + { + param_type= Ref _vm + ; param_name= "self" + ; param_doc= "The VM" + ; param_release= naples_release + ; param_default= None + } + ; { + param_type= String + ; param_name= "value" + ; param_doc= "The EFI-variables value" + ; param_release= naples_release + ; param_default= None + } + ; { + param_type= + Enum + ( "update_status" + , [ + ("yes", "Set secureboot_certificates_state to ok") + ; ("no", "Leave secureboot_certificates_state unchanged") + ; ( "unspecified" + , "Check certificates and update \ + secureboot_certificates_state accordingly" + ) + ] + ) + ; param_name= "update" + ; param_doc= + "If 'yes', set secureboot_certificates_state to ok. If 'no', keep \ + the current secureboot_certificates_state unchanged. If omitted \ + (defaults to 'unspecified'), run certificate check to determine \ + the state." + ; param_release= numbered_release "26.12.0" + ; param_default= Some (VEnum "unspecified") + } + ] ~hide_from_docs:true ~allowed_roles:_R_LOCAL_ROOT_ONLY () let restart_device_models = @@ -2508,6 +2546,40 @@ let set_uefi_mode = ~result:(String, "Result from the varstore-sb-state call") ~doc:"Set the UEFI mode of a VM" ~allowed_roles:_R_POOL_ADMIN () +let vm_secureboot_certificates_state = + Enum + ( "vm_secureboot_certificates_state" + , [ + ( "ok" + , "The VM's certificates do not need to be updated (including the case \ + where Secure Boot does not apply to this VM, e.g. BIOS VM)." + ) + ; ( "update_available" + , "The Secure Boot certificates are due to expire or have already \ + expired." + ) + ; ( "update_on_boot" + , "An update of the certificates will be triggered whenever the VM \ + boots. This includes VM.start, VM.reboot and a guest-triggered \ + reboot." + ) + ] + ) + +let update_secureboot_certificates_on_boot = + call ~name:"update_secureboot_certificates_on_boot" ~lifecycle:[] + ~params: + [ + (Ref _vm, "self", "The VM") + ; ( Bool + , "mark" + , "If true: mark certificates for update on next boot. If false: \ + remove the mark" + ) + ] + ~doc:"Mark or unmark secure boot certificate update on VM boot" + ~allowed_roles:_R_VM_ADMIN () + let vm_secureboot_readiness = Enum ( "vm_secureboot_readiness" @@ -2682,6 +2754,7 @@ let t = ; restart_device_models ; set_uefi_mode ; get_secureboot_readiness + ; update_secureboot_certificates_on_boot ; set_blocked_operations ; add_to_blocked_operations ; remove_from_blocked_operations @@ -3293,6 +3366,11 @@ let t = 'average user' doesn't need to" ; field ~qualifier:DynamicRO ~lifecycle:[] ~ty:(Set (Ref _vm_group)) "groups" "VM groups associated with the VM" + ; field ~qualifier:DynamicRO ~lifecycle:[] + ~ty:vm_secureboot_certificates_state + ~default_value:(Some (VEnum "ok")) "secureboot_certificates_state" + "The state of the Secure Boot certificates, showing whether an \ + update is available, already scheduled, or not needed." ] ) () diff --git a/ocaml/idl/schematest.ml b/ocaml/idl/schematest.ml index cb48f9b54aa..889a8637498 100644 --- a/ocaml/idl/schematest.ml +++ b/ocaml/idl/schematest.ml @@ -3,7 +3,7 @@ let hash x = Digest.string x |> Digest.to_hex (* BEWARE: if this changes, check that schema has been bumped accordingly in ocaml/idl/datamodel_common.ml, usually schema_minor_vsn *) -let last_known_schema_hash = "2d8501063ef6b243facc24a3dbdc2a5d" +let last_known_schema_hash = "fe75fa03d7ce97f9e51426bfb9b078fe" let current_schema_hash : string = let open Datamodel_types in diff --git a/ocaml/libs/stunnel/dune b/ocaml/libs/stunnel/dune index 776f4a8c7ed..db700ed807f 100644 --- a/ocaml/libs/stunnel/dune +++ b/ocaml/libs/stunnel/dune @@ -1,21 +1,19 @@ (library - (name stunnel) - (public_name stunnel) - (wrapped false) - (libraries - astring - forkexec - safe-resources - threads.posix - unix - uuid - xapi-consts - xapi-inventory - xapi-log - xapi-stdext-pervasives - xapi-stdext-std - xapi-stdext-threads - xapi-stdext-unix - ) -) - + (name stunnel) + (public_name stunnel) + (wrapped false) + (libraries + astring + forkexec + mtime + safe-resources + threads.posix + unix + uuid + xapi-consts + xapi-inventory + xapi-log + xapi-stdext-pervasives + xapi-stdext-std + xapi-stdext-threads + xapi-stdext-unix)) diff --git a/ocaml/libs/stunnel/stunnel.ml b/ocaml/libs/stunnel/stunnel.ml index 70e0b40f37a..6c3c2748e01 100644 --- a/ocaml/libs/stunnel/stunnel.ml +++ b/ocaml/libs/stunnel/stunnel.ml @@ -639,6 +639,7 @@ end This is useful for TOFU (Trust-On-First-Use) scenarios. *) let fetch_server_cert ~remote_host ~remote_port = try + let timeout = Mtime.Span.(30 * s) in let openssl = !Constants.openssl_path in (* First get the certificate with s_client *) let s_client_args = @@ -650,13 +651,14 @@ let fetch_server_cert ~remote_host ~remote_port = ] in let cert_output, _ = - Forkhelpers.execute_command_get_output_send_stdin openssl s_client_args "" + Forkhelpers.execute_command_get_output_send_stdin ~timeout openssl + s_client_args "" in (* Then parse it with x509 to get PEM format *) let x509_args = ["x509"; "-outform"; "PEM"] in let pem_output, _ = - Forkhelpers.execute_command_get_output_send_stdin openssl x509_args - cert_output + Forkhelpers.execute_command_get_output_send_stdin ~timeout openssl + x509_args cert_output in if String.length pem_output > 0 diff --git a/ocaml/tests/test_vm.ml b/ocaml/tests/test_vm.ml index 4bceac90ee8..cd4c3ca4621 100644 --- a/ocaml/tests/test_vm.ml +++ b/ocaml/tests/test_vm.ml @@ -191,4 +191,128 @@ module VMSetBiosStrings = Generic.MakeStateful (struct `QuickAndAutoDocumented tests end) -let tests = [("test_vm_set_bios_strings", VMSetBiosStrings.tests)] +module VMSecurebootCertificatesStateField = Generic.MakeStateful (struct + module Io = struct + type input_t = [`ok | `update_available | `update_on_boot] + + type output_t = [`ok | `update_available | `update_on_boot] + + let string_of_state = function + | `ok -> + "ok" + | `update_available -> + "update_available" + | `update_on_boot -> + "update_on_boot" + + let string_of_input_t = string_of_state + + let string_of_output_t = string_of_state + end + + module State = Test_state.XapiDb + + let name_label = "secureboot-certs-state" + + let load_input __context state = + let self = Test_common.make_vm ~__context ~name_label () in + Db.VM.set_secureboot_certificates_state ~__context ~self ~value:state + + let extract_output __context _ = + let self = + List.nth (Db.VM.get_by_name_label ~__context ~label:name_label) 0 + in + Db.VM.get_secureboot_certificates_state ~__context ~self + + let tests = + `QuickAndAutoDocumented + [ + (`ok, `ok) + ; (`update_available, `update_available) + ; (`update_on_boot, `update_on_boot) + ] +end) + +module VMUpdateSecurebootCertificatesOnBoot = Generic.MakeStateful (struct + module Io = struct + type input_t = [`ok | `update_available | `update_on_boot] * bool + + type output_t = + ([`ok | `update_available | `update_on_boot], string * string list) result + + let string_of_state = function + | `ok -> + "ok" + | `update_available -> + "update_available" + | `update_on_boot -> + "update_on_boot" + + let string_of_input_t (state, mark) = + Printf.sprintf "(%s, mark=%b)" (string_of_state state) mark + + let string_of_output_t = function + | Ok state -> + Printf.sprintf "Ok %s" (string_of_state state) + | Error (code, args) -> + Printf.sprintf "Error %s(%s)" code (String.concat ", " args) + end + + module State = Test_state.XapiDb + + let name_label = "update-secureboot-certs-on-boot" + + let load_input __context (state, _) = + let self = Test_common.make_vm ~__context ~name_label () in + Db.VM.set_secureboot_certificates_state ~__context ~self ~value:state + + let extract_output __context (_, mark) = + let self = + List.nth (Db.VM.get_by_name_label ~__context ~label:name_label) 0 + in + try + Xapi_vm.update_secureboot_certificates_on_boot ~__context ~self ~mark ; + Ok (Db.VM.get_secureboot_certificates_state ~__context ~self) + with Api_errors.Server_error (code, args) -> Error (code, args) + + let tests = + `QuickAndAutoDocumented + [ + (* transitions *) + ((`update_available, true), Ok `update_on_boot) + ; ((`update_on_boot, false), Ok `update_available) + (* idempotent: already in desired state *) + ; ((`update_on_boot, true), Ok `update_on_boot) + ; ((`update_available, false), Ok `update_available) + (* irrelevant state: raises *) + ; ( (`ok, true) + , Error + ( Api_errors.operation_not_allowed + , [ + "Cannot set update_on_boot: VM.secureboot_certificates_state \ + is not in a valid state" + ] + ) + ) + ; ( (`ok, false) + , Error + ( Api_errors.operation_not_allowed + , [ + "Cannot clear update_on_boot: VM.secureboot_certificates_state \ + is not in a valid state" + ] + ) + ) + ] +end) + +let tests = + [ + ("test_vm_set_bios_strings", VMSetBiosStrings.tests) + ; ( "test_vm_secureboot_certificates_state_field" + , VMSecurebootCertificatesStateField.tests + ) + ; ( "test_vm_update_secureboot_certificates_on_boot" + , VMUpdateSecurebootCertificatesOnBoot.tests + ) + ] diff --git a/ocaml/tests/test_vm_check_operation_error.ml b/ocaml/tests/test_vm_check_operation_error.ml index 40d37c4beb5..460ded911e2 100644 --- a/ocaml/tests/test_vm_check_operation_error.ml +++ b/ocaml/tests/test_vm_check_operation_error.ml @@ -52,7 +52,7 @@ let test_vm_set_nvram_running () = let new_vars = "CCCC" in let new_nvram = [("EFI-variables", new_vars)] in Api_server_common.Forwarder.VM.set_NVRAM_EFI_variables ~__context - ~self:vm_ref ~value:new_vars ; + ~self:vm_ref ~value:new_vars ~update:`no ; let read_nvram = Db.VM.get_NVRAM ~__context ~self:vm_ref in Alcotest.(check (list (pair string string))) "NVRAM updated" new_nvram read_nvram diff --git a/ocaml/tests/test_xapi_db_upgrade.ml b/ocaml/tests/test_xapi_db_upgrade.ml index 99f30140a41..fb95e13b4e9 100644 --- a/ocaml/tests/test_xapi_db_upgrade.ml +++ b/ocaml/tests/test_xapi_db_upgrade.ml @@ -88,9 +88,35 @@ let remove_restricted_pbd_keys () = ) other_keys +let upgrade_secureboot_certificates_state_for_uefi_vm_without_secureboot () = + let __context = T.make_test_database () in + let vm = + T.make_vm ~__context ~name_label:"uefi-no-secureboot" + ~hVM_boot_params:[("firmware", "uefi")] + ~platform:[("secureboot", "false")] + () + in + Db.VM.set_secureboot_certificates_state ~__context ~self:vm + ~value:`update_available ; + Alcotest.(check string) + "precondition: state starts as update_available" "update_available" + (Record_util.vm_secureboot_certificates_state_to_string + (Db.VM.get_secureboot_certificates_state ~__context ~self:vm) + ) ; + X.upgrade_secureboot_certificates_state.fn ~__context ; + Alcotest.(check string) + "UEFI VM without secureboot still gets checked" "ok" + (Record_util.vm_secureboot_certificates_state_to_string + (Db.VM.get_secureboot_certificates_state ~__context ~self:vm) + ) + let test = [ ("upgrade_bios", `Quick, upgrade_bios) ; ("update_snapshots", `Quick, update_snapshots) ; ("remove_restricted_pbd_keys", `Quick, remove_restricted_pbd_keys) + ; ( "upgrade_secureboot_certificates_state_for_uefi_vm_without_secureboot" + , `Quick + , upgrade_secureboot_certificates_state_for_uefi_vm_without_secureboot + ) ] diff --git a/ocaml/xapi-cli-server/cli_frontend.ml b/ocaml/xapi-cli-server/cli_frontend.ml index 323834902cf..92e5ff51045 100644 --- a/ocaml/xapi-cli-server/cli_frontend.ml +++ b/ocaml/xapi-cli-server/cli_frontend.ml @@ -2779,6 +2779,16 @@ let rec cmdtable_data : (string * cmd_spec) list = ; flags= [] } ) + ; ( "vm-update-secureboot-certificates-on-boot" + , { + reqd= ["mark"] + ; optn= [] + ; help= "Mark/unmark secure boot certificate update for next VM boot." + ; implementation= + No_fd Cli_operations.vm_update_secureboot_certificates_on_boot + ; flags= [Vm_selectors] + } + ) ; ( "vm-group-create" , { reqd= ["name-label"; "placement"] diff --git a/ocaml/xapi-cli-server/cli_operations.ml b/ocaml/xapi-cli-server/cli_operations.ml index 9ea78f48b23..8726ae9356b 100644 --- a/ocaml/xapi-cli-server/cli_operations.ml +++ b/ocaml/xapi-cli-server/cli_operations.ml @@ -6307,6 +6307,17 @@ let vm_get_secureboot_readiness printer rpc session_id params = printer (Cli_printer.PMsg (Record_util.vm_secureboot_readiness_to_string result)) +let vm_update_secureboot_certificates_on_boot printer rpc session_id params = + let mark = get_bool_param params "mark" in + ignore + (do_vm_op printer rpc session_id + (fun vm -> + Client.VM.update_secureboot_certificates_on_boot ~rpc ~session_id + ~self:(vm.getref ()) ~mark + ) + params ["mark"] + ) + let cd_list printer rpc session_id params = let srs = Client.SR.get_all_records_where ~rpc ~session_id ~expr:"true" in let cd_srs = diff --git a/ocaml/xapi-cli-server/records.ml b/ocaml/xapi-cli-server/records.ml index c72f2ffe9e2..19d45572e73 100644 --- a/ocaml/xapi-cli-server/records.ml +++ b/ocaml/xapi-cli-server/records.ml @@ -2676,6 +2676,12 @@ let vm_record rpc session_id vm = Client.VM.set_groups ~rpc ~session_id ~self:vm ~value ) () + ; make_field ~name:"secureboot-certificates-state" + ~get:(fun () -> + Record_util.vm_secureboot_certificates_state_to_string + (x ()).API.vM_secureboot_certificates_state + ) + () ; make_field ~name:"snapshot-schedule" ~get:(fun () -> get_uuid_from_ref (x ()).API.vM_snapshot_schedule) ~set:(fun x -> diff --git a/ocaml/xapi-guard/lib/server_interface.ml b/ocaml/xapi-guard/lib/server_interface.ml index 6e81ce77f2d..574400e5351 100644 --- a/ocaml/xapi-guard/lib/server_interface.ml +++ b/ocaml/xapi-guard/lib/server_interface.ml @@ -188,7 +188,35 @@ let make_server_varstored _persist ~cache path vm_uuid = in let set_nvram _ _ nvram = (let* self = get_vm_ref () in - with_xapi ~cache @@ VM.set_NVRAM_EFI_variables ~self ~value:nvram + with_xapi ~cache + @@ VM.set_NVRAM_EFI_variables ~self ~value:nvram ~update:`unspecified + ) + |> ret + in + let set_nvram_v2 _ _ nvram update = + (let* self = get_vm_ref () in + let update = + match update with "yes" -> `yes | "no" -> `no | _ -> `unspecified + in + with_xapi ~cache @@ VM.set_NVRAM_EFI_variables ~self ~value:nvram ~update + ) + |> ret + in + let get_secureboot_certificates_state _ _ = + (let* self = get_vm_ref () in + let* state = + with_xapi ~cache @@ VM.get_secureboot_certificates_state ~self + in + let state_str = + match state with + | `ok -> + "ok" + | `update_available -> + "update_available" + | `update_on_boot -> + "update_on_boot" + in + Lwt.return state_str ) |> ret in @@ -207,6 +235,8 @@ let make_server_varstored _persist ~cache path vm_uuid = let dummy_logout _ = ret @@ Lwt.return_unit in Server.get_NVRAM get_nvram ; Server.set_NVRAM set_nvram ; + Server.set_NVRAM_v2 set_nvram_v2 ; + Server.get_secureboot_certificates_state get_secureboot_certificates_state ; Server.message_create message_create ; Server.session_login dummy_login ; Server.session_logout dummy_logout ; diff --git a/ocaml/xapi-guard/test/xapi_guard_test.ml b/ocaml/xapi-guard/test/xapi_guard_test.ml index 2f048fe6660..e945376f316 100644 --- a/ocaml/xapi-guard/test/xapi_guard_test.ml +++ b/ocaml/xapi-guard/test/xapi_guard_test.ml @@ -57,6 +57,16 @@ let xapi_rpc call = expect_vm vm_rpc ; nvram_contents := [("EFI-variables", API.string_of_rpc contents)] ; ret_ok "" + | "VM.set_NVRAM_EFI_variables_v2", [session_id_rpc; vm_rpc; contents; _update] + -> + expect_session_id session_id_rpc ; + expect_vm vm_rpc ; + nvram_contents := [("EFI-variables", API.string_of_rpc contents)] ; + ret_ok "" + | "VM.get_secureboot_certificates_state", [session_id_rpc; vm_rpc] -> + expect_session_id session_id_rpc ; + expect_vm vm_rpc ; + ret_ok "ok" | _ -> Fmt.failwith "XAPI RPC call %s not expected in test" call.Rpc.name @@ -108,7 +118,10 @@ let test_change_nvram ~rpc ~session_id () = let* nvram0 = VM.get_NVRAM ~rpc ~session_id ~self in Alcotest.(check' dict) ~msg:"nvram initial" ~expected:[] ~actual:nvram0 ; let contents = "nvramnew" in - let* () = VM.set_NVRAM_EFI_variables ~rpc ~session_id ~self ~value:contents in + let* () = + VM.set_NVRAM_EFI_variables ~rpc ~session_id ~self ~value:contents + ~update:`no + in let* nvram1 = VM.get_NVRAM ~rpc ~session_id ~self in Alcotest.(check' dict) ~msg:"nvram changed" @@ -133,6 +146,7 @@ let test_bad_get_nvram ~rpc ~session_id () = let test_bad_set_nvram ~rpc ~session_id () = let* () = VM.set_NVRAM_EFI_variables ~rpc ~session_id ~self:vm_bad ~value:"bad" + ~update:`no in let* vm_ref = VM.get_by_uuid ~rpc ~session_id ~uuid:vm_uuid_str in let* nvram = VM.get_NVRAM ~rpc ~session_id ~self:vm_ref in diff --git a/ocaml/xapi-idl/guard/varstored/interface.ml b/ocaml/xapi-idl/guard/varstored/interface.ml index e418dc4d094..20b529b672f 100644 --- a/ocaml/xapi-idl/guard/varstored/interface.ml +++ b/ocaml/xapi-idl/guard/varstored/interface.ml @@ -80,6 +80,16 @@ module RPC_API (R : RPC) = struct ["Set the current VM's NVRAM contents"] (string_p @-> string_p @-> string_p @-> returning unit_p err) + let set_NVRAM_v2 = + declare "VM.set_NVRAM_EFI_variables_v2" + ["Set the current VM's NVRAM contents, with update flag"] + (string_p @-> string_p @-> string_p @-> string_p @-> returning unit_p err) + + let get_secureboot_certificates_state = + declare "VM.get_secureboot_certificates_state" + ["Get the current VM's Secure Boot certificates state"] + (string_p @-> string_p @-> returning string_p err) + let message_create = declare "message.create" ["Send an alert when booting a UEFI guest fails"] diff --git a/ocaml/xapi-idl/lib_test/test_data/guard/varstored/requests/VM.get_secureboot_certificates_state.request.0 b/ocaml/xapi-idl/lib_test/test_data/guard/varstored/requests/VM.get_secureboot_certificates_state.request.0 new file mode 100644 index 00000000000..dbdf9d24d47 --- /dev/null +++ b/ocaml/xapi-idl/lib_test/test_data/guard/varstored/requests/VM.get_secureboot_certificates_state.request.0 @@ -0,0 +1 @@ +VM.get_secureboot_certificates_statestringstring \ No newline at end of file diff --git a/ocaml/xapi-idl/lib_test/test_data/guard/varstored/requests/VM.set_NVRAM_EFI_variables_v2.request.0 b/ocaml/xapi-idl/lib_test/test_data/guard/varstored/requests/VM.set_NVRAM_EFI_variables_v2.request.0 new file mode 100644 index 00000000000..8ba486eb48f --- /dev/null +++ b/ocaml/xapi-idl/lib_test/test_data/guard/varstored/requests/VM.set_NVRAM_EFI_variables_v2.request.0 @@ -0,0 +1 @@ +VM.set_NVRAM_EFI_variables_v2stringstringstringstring \ No newline at end of file diff --git a/ocaml/xapi-idl/lib_test/test_data/guard/varstored/responses/VM.get_secureboot_certificates_state.response.0 b/ocaml/xapi-idl/lib_test/test_data/guard/varstored/responses/VM.get_secureboot_certificates_state.response.0 new file mode 100644 index 00000000000..18ed8315ea9 --- /dev/null +++ b/ocaml/xapi-idl/lib_test/test_data/guard/varstored/responses/VM.get_secureboot_certificates_state.response.0 @@ -0,0 +1 @@ +StatusSuccessValuestring \ No newline at end of file diff --git a/ocaml/xapi-idl/lib_test/test_data/guard/varstored/responses/VM.set_NVRAM_EFI_variables_v2.response.0 b/ocaml/xapi-idl/lib_test/test_data/guard/varstored/responses/VM.set_NVRAM_EFI_variables_v2.response.0 new file mode 100644 index 00000000000..4fa6e28fc1f --- /dev/null +++ b/ocaml/xapi-idl/lib_test/test_data/guard/varstored/responses/VM.set_NVRAM_EFI_variables_v2.response.0 @@ -0,0 +1 @@ +StatusSuccessValue \ No newline at end of file diff --git a/ocaml/xapi/create_misc.ml b/ocaml/xapi/create_misc.ml index 3c2d680d328..e85c205d54f 100644 --- a/ocaml/xapi/create_misc.ml +++ b/ocaml/xapi/create_misc.ml @@ -317,7 +317,8 @@ and create_domain_zero_record ~__context ~domain_zero_ref (host_info : host_info ~has_vendor_device:false ~requires_reboot:false ~reference_label:"" ~domain_type:Xapi_globs.domain_zero_domain_type ~nVRAM:[] ~pending_guidances:[] ~recommended_guidances:[] - ~pending_guidances_recommended:[] ~pending_guidances_full:[] ; + ~pending_guidances_recommended:[] ~pending_guidances_full:[] + ~secureboot_certificates_state:`ok ; ensure_domain_zero_metrics_record ~__context ~domain_zero_ref host_info ; Db.Host.set_control_domain ~__context ~self:localhost ~value:domain_zero_ref ; Xapi_vm_helpers.update_memory_overhead ~__context ~vm:domain_zero_ref diff --git a/ocaml/xapi/import.ml b/ocaml/xapi/import.ml index fbf5b53791f..eb3e71d7b2b 100644 --- a/ocaml/xapi/import.ml +++ b/ocaml/xapi/import.ml @@ -619,6 +619,12 @@ module VM : HandlerTools = struct else {vm_record with API.vM_has_vendor_device= false} in + (* Always default secureboot_certificates_state to ok in the record + passed to create_from_record -- the actual state will be recomputed + below against the importing pool's certificates. *) + let vm_record = + {vm_record with API.vM_secureboot_certificates_state= `ok} + in let vm_record = { vm_record with @@ -775,6 +781,19 @@ module VM : HandlerTools = struct ) ; Db.VM.set_bios_strings ~__context ~self:vm ~value:vm_record.API.vM_bios_strings ; + (* Always recompute secureboot_certificates_state against the + importing pool's certificates, since the exporting pool's state + is not meaningful here. *) + ( if not vm_record.API.vM_is_default_template then + let state = + ( Xapi_vm_helpers.check_secureboot_certificates_state ~__context + ~self:vm + :> API.vm_secureboot_certificates_state + ) + in + Db.VM.set_secureboot_certificates_state ~__context ~self:vm + ~value:state + ) ; debug "Created VM: %s (was %s)" (Ref.string_of vm) x.id ; (* Although someone could sneak in here and attempt to power on the VM, it doesn't really matter since no VBDs have been created yet. diff --git a/ocaml/xapi/message_forwarding.ml b/ocaml/xapi/message_forwarding.ml index 890e38704dc..dc324bf1cdb 100644 --- a/ocaml/xapi/message_forwarding.ml +++ b/ocaml/xapi/message_forwarding.ml @@ -3206,10 +3206,10 @@ functor (vm_uuid ~__context self) value ; Local.VM.set_HVM_boot_policy ~__context ~self ~value - let set_NVRAM_EFI_variables ~__context ~self ~value = + let set_NVRAM_EFI_variables ~__context ~self ~value ~update = (* called by varstored, bypasses VM powerstate check *) info "VM.set_NVRAM_EFI_variables: self = '%s'" (vm_uuid ~__context self) ; - Local.VM.set_NVRAM_EFI_variables ~__context ~self ~value + Local.VM.set_NVRAM_EFI_variables ~__context ~self ~value ~update let restart_device_models ~__context ~self = info "VM.restart_device_models: self = '%s'" (vm_uuid ~__context self) ; @@ -3228,6 +3228,12 @@ functor info "VM.get_secureboot_readiness: self = '%s'" (vm_uuid ~__context self) ; Local.VM.get_secureboot_readiness ~__context ~self + let update_secureboot_certificates_on_boot ~__context ~self ~mark = + info + "VM.update_secureboot_certificates_on_boot: self = '%s'; mark = '%b'" + (vm_uuid ~__context self) mark ; + Local.VM.update_secureboot_certificates_on_boot ~__context ~self ~mark + let set_blocked_operations ~__context ~self ~value = info "VM.set_blocked_operations: self = '%s'" (vm_uuid ~__context self) ; Local.VM.set_blocked_operations ~__context ~self ~value ; diff --git a/ocaml/xapi/repository_helpers.ml b/ocaml/xapi/repository_helpers.ml index 3bce3becf47..d3c789127a8 100644 --- a/ocaml/xapi/repository_helpers.ml +++ b/ocaml/xapi/repository_helpers.ml @@ -446,8 +446,8 @@ let with_local_repositories ~__context f = s in remove_repo_conf_file repo_name ; - write_yum_config ~proxy_config:[] ~source_url:None ~binary_url - ~repo_gpgcheck:false ~gpgkey_path ~repo_name ; + write_yum_config ~proxy_config:["proxy="] ~source_url:None + ~binary_url ~repo_gpgcheck:false ~gpgkey_path ~repo_name ; clean_yum_cache repo_name ; let Pkg_mgr.{cmd; params} = [ diff --git a/ocaml/xapi/xapi_db_upgrade.ml b/ocaml/xapi/xapi_db_upgrade.ml index aba1cf15444..d1641dd6781 100644 --- a/ocaml/xapi/xapi_db_upgrade.ml +++ b/ocaml/xapi/xapi_db_upgrade.ml @@ -965,6 +965,38 @@ let upgrade_ca_fingerprints = ) } +let upgrade_secureboot_certificates_state = + { + description= "Set secureboot_certificates_state for existing VMs" + ; version= (fun x -> x < (5, 903)) + ; fn= + (fun ~__context -> + List.iter + (fun self -> + if + Db.VM.get_is_control_domain ~__context ~self + || Db.VM.get_is_default_template ~__context ~self + then + () + else + let boot_params = Db.VM.get_HVM_boot_params ~__context ~self in + let is_uefi = + List.assoc_opt "firmware" boot_params = Some "uefi" + in + if is_uefi then + let state = + ( Xapi_vm_helpers.check_secureboot_certificates_state + ~__context ~self + :> API.vm_secureboot_certificates_state + ) + in + Db.VM.set_secureboot_certificates_state ~__context ~self + ~value:state + ) + (Db.VM.get_all ~__context) + ) + } + let rules = [ upgrade_domain_type @@ -995,6 +1027,7 @@ let rules = ; empty_pool_uefi_certificates ; upgrade_update_guidance ; upgrade_ca_fingerprints + ; upgrade_secureboot_certificates_state ] (* Maybe upgrade most recent db *) diff --git a/ocaml/xapi/xapi_globs.ml b/ocaml/xapi/xapi_globs.ml index 8b59b254b00..d9b263bdfc5 100644 --- a/ocaml/xapi/xapi_globs.ml +++ b/ocaml/xapi/xapi_globs.ml @@ -910,6 +910,8 @@ let varstore_ls = ref "/usr/bin/varstore-ls" let varstore_dir = ref "/var/lib/varstored" +let varstore_nvram_certcheck = ref "/usr/bin/varstore-nvram-certcheck" + let default_auth_dir = ref "/usr/share/varstored" let allow_custom_uefi_certs = ref false @@ -2116,6 +2118,10 @@ module Resources = struct ) ; ("varstore-ls", varstore_ls, "Executed to list the UEFI variables of a VM") ; ("varstore_dir", varstore_dir, "Path to local varstored directory") + ; ( "varstore-nvram-certcheck" + , varstore_nvram_certcheck + , "Executed to check VM NVRAM Secure Boot certificate status" + ) ; ( "nvidia-sriov-manage" , nvidia_sriov_manage_script , "Path to NVIDIA sriov-manage script" diff --git a/ocaml/xapi/xapi_vm.ml b/ocaml/xapi/xapi_vm.ml index bd80666c5b8..09cfc811345 100644 --- a/ocaml/xapi/xapi_vm.ml +++ b/ocaml/xapi/xapi_vm.ml @@ -688,7 +688,8 @@ let create ~__context ~name_label ~name_description ~power_state ~user_version ~suspend_SR ~version ~generation_id ~hardware_platform_version ~has_vendor_device ~requires_reboot:false ~reference_label ~domain_type ~pending_guidances:[] ~recommended_guidances:[] - ~pending_guidances_recommended:[] ~pending_guidances_full:[] ; + ~pending_guidances_recommended:[] ~pending_guidances_full:[] + ~secureboot_certificates_state:`ok ; Xapi_vm_lifecycle.update_allowed_operations ~__context ~self:vm_ref ; update_memory_overhead ~__context ~vm:vm_ref ; update_vm_virtual_hardware_platform_version ~__context ~vm:vm_ref ; @@ -1650,7 +1651,7 @@ let set_HVM_boot_policy ~__context ~self ~value = let nvram = Mutex.create () -let set_NVRAM_EFI_variables ~__context ~self ~value = +let set_NVRAM_EFI_variables ~__context ~self ~value ~update = with_lock nvram (fun () -> (* do not use remove_from_NVRAM: we do not want to * temporarily end up with an empty NVRAM in HA *) @@ -1658,7 +1659,20 @@ let set_NVRAM_EFI_variables ~__context ~self ~value = let nvram = Db.VM.get_NVRAM ~__context ~self in let value = (key, value) :: List.remove_assoc key nvram in Db.VM.set_NVRAM ~__context ~self ~value - ) + ) ; + (* Update secureboot_certificates_state after NVRAM is written *) + match update with + | `yes -> + Db.VM.set_secureboot_certificates_state ~__context ~self ~value:`ok + | `no -> + () (* keep current state unchanged *) + | `unspecified -> + let new_state = + ( Xapi_vm_helpers.check_secureboot_certificates_state ~__context ~self + :> API.vm_secureboot_certificates_state + ) + in + Db.VM.set_secureboot_certificates_state ~__context ~self ~value:new_state let restart_device_models ~__context ~self = let power_state = Db.VM.get_power_state ~__context ~self in @@ -1740,6 +1754,36 @@ let get_secureboot_readiness ~__context ~self = ) ) +let update_secureboot_certificates_on_boot ~__context ~self ~mark = + let current = Db.VM.get_secureboot_certificates_state ~__context ~self in + match (mark, current) with + | true, `update_available -> + Db.VM.set_secureboot_certificates_state ~__context ~self + ~value:`update_on_boot + | false, `update_on_boot -> + Db.VM.set_secureboot_certificates_state ~__context ~self + ~value:`update_available + | true, `update_on_boot -> + () (* already marked for update on boot — idempotent *) + | false, `update_available -> + () (* already cleared — idempotent *) + | _, _ -> + raise + (Api_errors.Server_error + ( Api_errors.operation_not_allowed + , [ + Printf.sprintf + "Cannot %s update_on_boot: VM.secureboot_certificates_state \ + is not in a valid state" + ( if mark then + "set" + else + "clear" + ) + ] + ) + ) + let sysprep ~__context ~self ~unattend ~timeout = let uuid = Db.VM.get_uuid ~__context ~self in debug "%s %S (timeout %f)" __FUNCTION__ uuid timeout ; diff --git a/ocaml/xapi/xapi_vm.mli b/ocaml/xapi/xapi_vm.mli index 5c3392e4e14..04dc6f1b020 100644 --- a/ocaml/xapi/xapi_vm.mli +++ b/ocaml/xapi/xapi_vm.mli @@ -425,7 +425,11 @@ val set_HVM_boot_policy : __context:Context.t -> self:API.ref_VM -> value:string -> unit val set_NVRAM_EFI_variables : - __context:Context.t -> self:API.ref_VM -> value:string -> unit + __context:Context.t + -> self:API.ref_VM + -> value:string + -> update:[`yes | `no | `unspecified] + -> unit val restart_device_models : __context:Context.t -> self:API.ref_VM -> unit @@ -435,6 +439,9 @@ val set_uefi_mode : val get_secureboot_readiness : __context:Context.t -> self:API.ref_VM -> API.vm_secureboot_readiness +val update_secureboot_certificates_on_boot : + __context:Context.t -> self:API.ref_VM -> mark:bool -> unit + val set_blocked_operations : __context:Context.t -> self:API.ref_VM diff --git a/ocaml/xapi/xapi_vm_clone.ml b/ocaml/xapi/xapi_vm_clone.ml index 4bc09fee111..25a30eac278 100644 --- a/ocaml/xapi/xapi_vm_clone.ml +++ b/ocaml/xapi/xapi_vm_clone.ml @@ -402,7 +402,9 @@ let copy_vm_record ?snapshot_info_record ~__context ~vm ~disk_op ~new_name ~requires_reboot:false ~reference_label:all.Db_actions.vM_reference_label ~domain_type:all.Db_actions.vM_domain_type ~nVRAM:all.Db_actions.vM_NVRAM ~pending_guidances:[] ~recommended_guidances:[] - ~pending_guidances_recommended:[] ~pending_guidances_full:[] ; + ~pending_guidances_recommended:[] ~pending_guidances_full:[] + ~secureboot_certificates_state: + all.Db_actions.vM_secureboot_certificates_state ; (* update the VM's parent field in case of snapshot. Note this must be done after "ref" has been created, so that its "children" field can be updated by the database layer *) ( match disk_op with diff --git a/ocaml/xapi/xapi_vm_helpers.ml b/ocaml/xapi/xapi_vm_helpers.ml index 748e2afb20e..8d39dc9b339 100644 --- a/ocaml/xapi/xapi_vm_helpers.ml +++ b/ocaml/xapi/xapi_vm_helpers.ml @@ -1705,3 +1705,43 @@ let ensure_device_model_profile_present ~__context ~domain_type ~is_a_template else (* only add device-model to an HVM VM platform if it is not already there *) default :: platform + +let check_secureboot_certificates_state ~__context ~self : + [`ok | `update_available] = + let vm_uuid = Db.VM.get_uuid ~__context ~self in + let nvram = Db.VM.get_NVRAM ~__context ~self in + match List.assoc_opt "EFI-variables" nvram with + | None -> + D.info "VM %s has no EFI-variables in NVRAM, defaulting to ok" vm_uuid ; + `ok + | Some efi_vars -> ( + try + let tmp_path = Filename.temp_file ("nvram-" ^ vm_uuid ^ "-") ".dat" in + let result = + finally + (fun () -> + Xapi_stdext_unix.Unixext.write_string_to_file tmp_path efi_vars ; + let output, _ = + Forkhelpers.execute_command_get_output + !Xapi_globs.varstore_nvram_certcheck + [tmp_path] + in + String.trim output + ) + (fun () -> Xapi_stdext_unix.Unixext.unlink_safe tmp_path) + in + match result with + | "update_required" -> + `update_available + | "update_ok" -> + `ok + | other -> + D.warn + "varstore-nvram-certcheck returned unexpected output for VM %s: %s" + vm_uuid other ; + `ok + with e -> + D.warn "Failed to check secureboot certificate state for VM %s: %s" + vm_uuid (Printexc.to_string e) ; + `ok + ) diff --git a/ocaml/xenopsd/lib/xenops_server.ml b/ocaml/xenopsd/lib/xenops_server.ml index 3db369d0d57..92fd8d55c16 100644 --- a/ocaml/xenopsd/lib/xenops_server.ml +++ b/ocaml/xenopsd/lib/xenops_server.ml @@ -303,11 +303,17 @@ let rec name_of_atomic = function | Best_effort atomic -> Printf.sprintf "Best_effort (%s)" (name_of_atomic atomic) +let max_parallel_atoms = 10 + let rec atomic_expires_after = function | Serial (_, _, ops) -> List.map atomic_expires_after ops |> List.fold_left ( +. ) 0. | Parallel (_, _, ops) | Nested_parallel (_, _, ops) -> - List.map atomic_expires_after ops |> List.fold_left Float.max 0. + let expires_of = List.map atomic_expires_after in + let max_of = List.fold_left Float.max 0. in + Xenops_utils.chunks max_parallel_atoms ops + |> List.map (fun ops -> ops |> expires_of |> max_of) + |> List.fold_left ( +. ) 0. | _ -> (* 20 minutes, in seconds *) 1200. @@ -2010,8 +2016,8 @@ and parallel_atomic ~progress_callback ~description ~nested atoms t = let with_tracing = id_with_tracing parallel_id t in debug "begin_%s" parallel_id ; let task_list = - queue_atomics_and_wait ~progress_callback ~max_parallel_atoms:10 - with_tracing parallel_id atoms redirector + queue_atomics_and_wait ~progress_callback with_tracing parallel_id atoms + redirector in debug "end_%s" parallel_id ; (* make sure that we destroy all the parallel tasks that finished *) @@ -2067,8 +2073,7 @@ and queue_atomic_int ~progress_callback dbg id op redirector = Redirector.push redirector id item ; task -and queue_atomics_and_wait ~progress_callback ~max_parallel_atoms dbg id ops - redirector = +and queue_atomics_and_wait ~progress_callback dbg id ops redirector = let from = Updates.last_id dbg updates in Xenops_utils.chunks max_parallel_atoms ops |> List.mapi (fun chunk_idx ops -> @@ -2089,6 +2094,7 @@ and queue_atomics_and_wait ~progress_callback ~max_parallel_atoms dbg id ops (fun (task, op) -> let task_id = Xenops_task.id_of_handle task in let expiration = atomic_expires_after op in + debug "Task %s expires_after=%f" task_id expiration ; let completion = event_wait updates task ~from ~timeout_start expiration (is_task task_id) task_ended diff --git a/opam/tgroup.opam b/opam/tgroup.opam index 6080d67b802..6a0a2535f3f 100644 --- a/opam/tgroup.opam +++ b/opam/tgroup.opam @@ -1,5 +1,6 @@ # This file is generated by dune, edit dune-project instead opam-version: "2.0" +synopsis: "Thread group management library" maintainer: ["Xapi project maintainers"] authors: ["xen-api@lists.xen.org"] license: "LGPL-2.1-only WITH OCaml-LGPL-linking-exception" diff --git a/python3/extensions/pool_update.apply b/python3/extensions/pool_update.apply index 96d00adcf32..9f353aaf5b2 100644 --- a/python3/extensions/pool_update.apply +++ b/python3/extensions/pool_update.apply @@ -4,7 +4,6 @@ import errno import logging import os -import re import shutil import subprocess import sys @@ -14,17 +13,6 @@ import fasteners import xcp.logger import XenAPI -TMP_DIR = "/tmp/" -UPDATE_ALREADY_APPLIED = "UPDATE_ALREADY_APPLIED" -UPDATE_APPLY_FAILED = "UPDATE_APPLY_FAILED" -OTHER_OPERATION_IN_PROGRESS = "OTHER_OPERATION_IN_PROGRESS" -UPDATE_PRECHECK_FAILED_UNKNOWN_ERROR = "UPDATE_PRECHECK_FAILED_UNKNOWN_ERROR" -CANNOT_FIND_UPDATE = "CANNOT_FIND_UPDATE" -INVALID_UPDATE = "INVALID_UPDATE" -ERROR_MESSAGE_DOWNLOAD_PACKAGE = "Error downloading packages:\n" -ERROR_MESSAGE_START = "Error: " -ERROR_MESSAGE_END = "You could try " - TMP_DIR = '/tmp/' UPDATE_ALREADY_APPLIED = 'UPDATE_ALREADY_APPLIED' UPDATE_APPLY_FAILED = 'UPDATE_APPLY_FAILED' @@ -32,12 +20,9 @@ OTHER_OPERATION_IN_PROGRESS = 'OTHER_OPERATION_IN_PROGRESS' UPDATE_PRECHECK_FAILED_UNKNOWN_ERROR = 'UPDATE_PRECHECK_FAILED_UNKNOWN_ERROR' CANNOT_FIND_UPDATE = 'CANNOT_FIND_UPDATE' INVALID_UPDATE = 'INVALID_UPDATE' -ERROR_MESSAGE_DOWNLOAD_PACKAGE = 'Error downloading packages:\n' -ERROR_MESSAGE_START = 'Error: ' -ERROR_MESSAGE_END = 'You could try ' -YUM_CMD = '/usr/bin/yum' +ERROR_MESSAGE_DOWNLOAD_PACKAGE = 'Failed to download packages' DNF_CMD = '/usr/bin/dnf' -PKG_MGR = DNF_CMD if os.path.exists(DNF_CMD) else YUM_CMD +PKG_MGR = DNF_CMD class EnvironmentFailure(Exception): """Failure due to running environment""" @@ -81,10 +66,8 @@ def execute_apply(update_package, yum_conf_file): if p.returncode != 0: raise EnvironmentFailure("Error cleaning yum cache") - # dnf reject to upgrade group if it is not installed, - # `dnf install` upgrade the group if it is already installed - sub_cmd = 'upgrade' if PKG_MGR == YUM_CMD else 'install' - cmd = [PKG_MGR, sub_cmd, '-y', '--noplugins', '-c', yum_conf_file, update_package] + # `dnf install` upgrades the group if it is already installed. + cmd = [PKG_MGR, 'install', '-y', '--noplugins', '-c', yum_conf_file, update_package] p = subprocess.Popen(cmd, shell=False, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, close_fds=True, env=yum_env, universal_newlines=True) output, _ = p.communicate() @@ -94,12 +77,6 @@ def execute_apply(update_package, yum_conf_file): if p.returncode != 0: if ERROR_MESSAGE_DOWNLOAD_PACKAGE in output: raise InvalidUpdate("Missing package(s) in the update.") - - m = re.search("(?<=" + ERROR_MESSAGE_START + ").+$", output, flags=re.DOTALL) - if m: - errmsg = m.group() - errmsg = re.sub(ERROR_MESSAGE_END + ".+", "", errmsg, flags=re.DOTALL) - raise ApplyFailure(errmsg) raise ApplyFailure(output) diff --git a/python3/extensions/pool_update.precheck b/python3/extensions/pool_update.precheck index 8e5f1858668..792128f18be 100755 --- a/python3/extensions/pool_update.precheck +++ b/python3/extensions/pool_update.precheck @@ -31,17 +31,19 @@ PATCH_PRECHECK_FAILED_ISO_MOUNTED = 'PATCH_PRECHECK_FAILED_ISO_MOUNTED' PATCH_PRECHECK_FAILED_VM_RUNNING = 'PATCH_PRECHECK_FAILED_VM_RUNNING' INVALID_UPDATE = 'INVALID_UPDATE' CANNOT_FIND_UPDATE = 'CANNOT_FIND_UPDATE' -ERROR_MESSAGE_START = 'Error: ' -ERROR_MESSAGE_END = 'You could try ' +ERROR_MESSAGE_FAILED_TO_RESOLVE = 'Failed to resolve the transaction:\n' +ERROR_MESSAGE_END = 'You can try to add to command line:' ERROR_MESSAGE_CONFLICTS_WITH = ' conflicts with ' -ERROR_MESSAGE_CONFLICTS = 'conflicts ' -ERROR_MESSAGE_PROCESSING_CONFLICT = '--> Processing Conflict:' -ERROR_MESSAGE_PREREQUISITE = 'Requires: ' -ERROR_MESSAGE_VERSION_REQUIRED = 'Requires: ' -ERROR_MESSAGE_VERSION_INSTALLED = 'Installed: ' -ERROR_MESSAGE_VERSION_UPDATED_BY = 'Updated By: ' -ERROR_MESSAGE_DOWNLOAD_PACKAGE = 'Error downloading packages:\n' -ERROR_MESSAGE_GPGKEY_NOT_IMPORTED = 'Gpg Keys not imported' +ERROR_MESSAGE_NOTHING_PROVIDES = 'nothing provides ' +ERROR_MESSAGE_REQUIRES = ' requires ' +ERROR_MESSAGE_DOWNLOAD_PACKAGE = 'Failed to download packages' +ERROR_MESSAGE_GPGKEY_NOT_IMPORTED_PATTERNS = ( + 'Signature verification failed.', + 'Public key is not installed.', + 'Public key import failed.', + 'The package is not signed.', + 'GPG signature verification error', +) ERROR_XML_START = '' ERRORCODE = 'errorcode' @@ -132,6 +134,70 @@ def parse_precheck_failure(xmldoc): raise PrecheckError(code, *params) raise PrecheckFailure(xmldoc.toxml()) + +def classify_dnf_failure(output): + """Classify a failed `dnf install` output into a precheck exception. + + Always raises; the caller is responsible for the fallback when none + of the known patterns match (in which case this function simply + returns and the caller raises PrecheckFailure(output)). + """ + if ERROR_MESSAGE_DOWNLOAD_PACKAGE in output: + raise InvalidUpdate('Missing package(s) in the update') + + # GPG patterns must only be matched when dnf5 never reached the solver. + # Librepo prints transient messages like + # ">>> Librepo error: repomd.xml GPG signature verification error: ..." + # before auto-importing keys; those appear even on successful runs and + # also precede genuine resolution failures. If a "Failed to resolve" + # block is present, the real error is there, not in the GPG noise. + if (ERROR_MESSAGE_FAILED_TO_RESOLVE not in output + and any(pat in output + for pat in ERROR_MESSAGE_GPGKEY_NOT_IMPORTED_PATTERNS)): + raise GpgkeyNotImported() + + if PATCH_PRECHECK_FAILED_ISO_MOUNTED in output: + raise IsoMounted + + if PATCH_PRECHECK_FAILED_VM_RUNNING in output: + raise VmRunning + + m = re.search(ERROR_MESSAGE_FAILED_TO_RESOLVE + '(.+)$', + output, flags=re.DOTALL) + if m: + errmsg = m.group(1) + errmsg = re.sub(ERROR_MESSAGE_END + '.+', '', errmsg, flags=re.DOTALL) + if ERROR_MESSAGE_CONFLICTS_WITH in errmsg: + conflicts = re.findall( + ERROR_MESSAGE_CONFLICTS_WITH + r'(\S+)', errmsg) + if conflicts: + raise ConflictPresent(' '.join(conflicts)) + raise PrecheckFailure(errmsg) + wrong_version = re.search( + ERROR_MESSAGE_NOTHING_PROVIDES + + r'(\S+\s*[<>=]+\s*\S+) needed by (\S+)', errmsg) + if wrong_version: + required = wrong_version.group(1).strip() + installed = wrong_version.group(2).strip() + raise WrongServerVersion(required, installed) + prerequisites = re.findall( + ERROR_MESSAGE_NOTHING_PROVIDES + r'(.+?)(?: needed by | from |\n)', + errmsg) + prerequisites += re.findall( + ERROR_MESSAGE_REQUIRES + r'(.+?), but none of', errmsg) + if prerequisites: + raise PrerequisiteMissing(' '.join(prerequisites)) + raise PrecheckFailure(errmsg) + + regex = ERROR_XML_START + '.+' + ERROR_XML_END + m = re.search(regex, output, flags=re.DOTALL) + if m: + try: + xmldoc = xml.dom.minidom.parseString(m.group(0)) + except Exception as e: + raise PrecheckFailure(output) from e + parse_precheck_failure(xmldoc) + def execute_precheck(control_package, yum_conf_file, update_precheck_file): #pylint: disable=too-many-locals #pylint: disable=too-many-branches @@ -162,56 +228,9 @@ def execute_precheck(control_package, yum_conf_file, update_precheck_file): for line in output.split('\n'): xcp.logger.info(line) if p.returncode != 0: - if ERROR_MESSAGE_DOWNLOAD_PACKAGE in output: - raise InvalidUpdate('Missing package(s) in the update') - - if ERROR_MESSAGE_GPGKEY_NOT_IMPORTED in output: - raise GpgkeyNotImported() - - if PATCH_PRECHECK_FAILED_ISO_MOUNTED in output: - raise IsoMounted - - if PATCH_PRECHECK_FAILED_VM_RUNNING in output: - raise VmRunning - - m = re.search('(?<=' + ERROR_MESSAGE_START + ').+$', output, flags=re.DOTALL) - if m: - errmsg = m.group() - errmsg = re.sub(ERROR_MESSAGE_END + '.+', '', errmsg, flags=re.DOTALL) - if (ERROR_MESSAGE_CONFLICTS_WITH in errmsg and - ERROR_MESSAGE_PROCESSING_CONFLICT in output): - regex = (ERROR_MESSAGE_PROCESSING_CONFLICT + '(.*)' - + ERROR_MESSAGE_CONFLICTS + '(.+?)\n') - conflict_tuples = re.findall(regex, output) - if conflict_tuples: - raise ConflictPresent(' '.join([tup[1] for tup in conflict_tuples])) - raise PrecheckFailure(errmsg) - if (ERROR_MESSAGE_VERSION_REQUIRED in errmsg and - (ERROR_MESSAGE_VERSION_INSTALLED in errmsg - or ERROR_MESSAGE_VERSION_UPDATED_BY in errmsg)): - regex = ERROR_MESSAGE_VERSION_REQUIRED + '(.+?)\n.+ {2,2}(.+)$' - match = re.search(regex, errmsg, flags=re.DOTALL) - if match: - required_version = match.group(1).rstrip() - installed_version = match.group(2).rstrip() - raise WrongServerVersion(required_version, installed_version) - raise PrecheckFailure(errmsg) - if ERROR_MESSAGE_PREREQUISITE in errmsg: - regex = ERROR_MESSAGE_PREREQUISITE + '(.+?)\n' - prerequisite_list = re.findall(regex, errmsg) - if prerequisite_list: - raise PrerequisiteMissing(' '.join(prerequisite_list)) - raise PrecheckFailure(errmsg) - raise PrecheckFailure(errmsg) - - regex = ERROR_XML_START + '.+' + ERROR_XML_END - m = re.search(regex, output, flags=re.DOTALL) - if m: - try: - xmldoc = xml.dom.minidom.parseString(m.group(0)) - except Exception as e: - raise PrecheckFailure(output) from e - parse_precheck_failure(xmldoc) + classify_dnf_failure(output) + # classify_dnf_failure raises in every case it understands; if we get + # here it didn't match any pattern, so surface the raw dnf output. raise PrecheckFailure(output) if os.path.isfile(update_precheck_file): diff --git a/python3/tests/test_pool_update_precheck.py b/python3/tests/test_pool_update_precheck.py new file mode 100644 index 00000000000..72b9aaa3bbe --- /dev/null +++ b/python3/tests/test_pool_update_precheck.py @@ -0,0 +1,181 @@ +#!/usr/bin/env python3 +""" +Unit tests for the dnf5 output parser in pool_update.precheck. +""" + +import unittest + +from python3.tests.import_helper import import_file_as_module, mocked_modules + +with mocked_modules("XenAPI", "xcp", "xcp.logger"): + precheck = import_file_as_module("python3/extensions/pool_update.precheck") + + +# --------------------------------------------------------------------------- +# Captured dnf5 outputs (real, not synthesised) +# --------------------------------------------------------------------------- + +CONFLICT = """\ +Updating and loading repositories: +Repositories loaded. +Failed to resolve the transaction: +Problem: installed package xs-host-1.0-1.noarch conflicts with xs-host provided by xs-conflict-1.0-1.noarch from testrepo + - problem with installed package + - conflicting requests + - nothing provides xs-platform >= 2.0 needed by xs-host-2.0-1.noarch from testrepo +You can try to add to command line: + --skip-broken to skip uninstallable packages +""" + +WRONG_VERSION = """\ +Updating and loading repositories: + testrepo 100% | 75.3 KiB/s | 3.4 KiB | 00m00s +Repositories loaded. +Failed to resolve the transaction: +Package "xs-host-1.0-1.noarch" is already installed. +Problem: cannot install the best candidate for the job + - nothing provides xs-platform >= 2.0 needed by xs-host-2.0-1.noarch from testrepo +You can try to add to command line: + --no-best to not limit the transaction to the best candidates + --skip-broken to skip uninstallable packages +""" + +PREREQ_MISSING = """\ +Updating and loading repositories: +Repositories loaded. +Failed to resolve the transaction: +Problem: conflicting requests + - nothing provides missing-lib needed by xs-prereq-1.0-1.noarch from testrepo +You can try to add to command line: + --skip-broken to skip uninstallable packages +""" + +NONEXISTENT = """\ +Updating and loading repositories: +Repositories loaded. +Failed to resolve the transaction: +No match for argument: no-such-pkg-xyz +You can try to add to command line: + --skip-unavailable to skip unavailable packages +""" + +# Librepo emits "GPG signature verification error" on first access to a repo +# whose key is not yet trusted, before dnf5 auto-imports the key. The same +# line therefore also appears in front of unrelated solver failures, so the +# precheck plugin must only raise GpgkeyNotImported when no "Failed to +# resolve" block follows. The two fixtures below are condensed XenRT +# captures (jobs 31109885 / 31109895) used to exercise that gate. + +LIBREPO_GPG_THEN_PREREQ = """\ +Updating and loading repositories: +>>> Librepo error: repomd.xml GPG signature verification error: Bad GPG signature +Importing PGP key 0xDEADBEEF: + UserID : "XenServer Updates " +Key imported. +Repositories loaded. +Failed to resolve the transaction: +Problem: conflicting requests + - nothing provides update-test-hotfix-basic-1 needed by test-hotfix-depend-1-1.0-1.noarch from updates +You can try to add to command line: + --skip-broken to skip uninstallable packages +""" + +LIBREPO_GPG_THEN_WRONG_VERSION = """\ +Updating and loading repositories: +>>> Librepo error: repomd.xml GPG signature verification error: Bad GPG signature +Importing PGP key 0xDEADBEEF: +Key imported. +Repositories loaded. +Failed to resolve the transaction: +Problem: cannot install the best candidate for the job + - nothing provides platform-version = 0.73.2 needed by control-test-hotfix-basic-4-1.0-1.noarch from updates +You can try to add to command line: + --no-best to not limit the transaction to the best candidates +""" + + +class TestDnf5OutputParser(unittest.TestCase): + """Parser tests against captured dnf5 5.2.12 output.""" + + def test_conflict(self): + with self.assertRaises(precheck.ConflictPresent) as ctx: + precheck.classify_dnf_failure(CONFLICT) + # First "conflicts with" token in the captured output. + self.assertIn("xs-host", ctx.exception.args[0]) + + def test_wrong_server_version(self): + with self.assertRaises(precheck.WrongServerVersion) as ctx: + precheck.classify_dnf_failure(WRONG_VERSION) + self.assertEqual( + ctx.exception.args, + ("xs-platform >= 2.0", "xs-host-2.0-1.noarch"), + ) + + def test_prerequisite_missing(self): + with self.assertRaises(precheck.PrerequisiteMissing) as ctx: + precheck.classify_dnf_failure(PREREQ_MISSING) + self.assertEqual(ctx.exception.args, ("missing-lib",)) + + def test_nonexistent_package_falls_through(self): + # No conflict / nothing-provides / requires pattern: classifier + # raises PrecheckFailure with the cleaned errmsg. + with self.assertRaises(precheck.PrecheckFailure) as ctx: + precheck.classify_dnf_failure(NONEXISTENT) + self.assertIn("No match for argument: no-such-pkg-xyz", + ctx.exception.args[0]) + + def test_download_failure_short_circuits(self): + output = "Some preamble\nFailed to download packages\nmore noise\n" + with self.assertRaises(precheck.InvalidUpdate): + precheck.classify_dnf_failure(output) + + def test_gpgkey_patterns_short_circuit(self): + # Each GPG marker alone (no "Failed to resolve" block) must classify + # as GpgkeyNotImported. + for pattern in precheck.ERROR_MESSAGE_GPGKEY_NOT_IMPORTED_PATTERNS: + with self.subTest(pattern=pattern): + with self.assertRaises(precheck.GpgkeyNotImported): + precheck.classify_dnf_failure("preamble\n" + pattern + "\n") + + def test_librepo_gpg_noise_does_not_mask_prereq(self): + """Regression for XenRT 31109885 / 31109895. + + Librepo prints a transient GPG verification error before dnf5 + auto-imports the repo key; the run then fails at the solver stage + for an unrelated reason. The plugin must surface the real reason + (PrerequisiteMissing here) rather than GpgkeyNotImported. + """ + with self.assertRaises(precheck.PrerequisiteMissing) as ctx: + precheck.classify_dnf_failure(LIBREPO_GPG_THEN_PREREQ) + self.assertEqual( + ctx.exception.args, ("update-test-hotfix-basic-1",), + ) + + def test_librepo_gpg_noise_does_not_mask_wrong_version(self): + """Same regression, wrong-version variant.""" + with self.assertRaises(precheck.WrongServerVersion) as ctx: + precheck.classify_dnf_failure(LIBREPO_GPG_THEN_WRONG_VERSION) + self.assertEqual( + ctx.exception.args, + ("platform-version = 0.73.2", + "control-test-hotfix-basic-4-1.0-1.noarch"), + ) + + def test_iso_mounted(self): + output = "noise\n" + precheck.PATCH_PRECHECK_FAILED_ISO_MOUNTED + "\n" + with self.assertRaises(precheck.IsoMounted): + precheck.classify_dnf_failure(output) + + def test_vm_running(self): + output = "noise\n" + precheck.PATCH_PRECHECK_FAILED_VM_RUNNING + "\n" + with self.assertRaises(precheck.VmRunning): + precheck.classify_dnf_failure(output) + + def test_unrecognised_output_returns_silently(self): + # When the output matches none of the patterns the classifier just + # returns; execute_precheck then raises PrecheckFailure(output). + self.assertIsNone(precheck.classify_dnf_failure("totally unrelated\n")) + + +if __name__ == "__main__": + unittest.main()