[Libguestfs] [PATCH v2 2/2] v2v: Use OVMF secure boot file (RHBZ#1367615).

version of OVMF on x86-64, with flags -D SECURE_BOOT_ENABLE -D SMM_REQUIRE. The filename has also changed to reflect this - it is now /usr/share/OVMF/OVMF_CODE.secboot.fd. The old file /usr/share/OVMF/OVMF_CODE.fd is no longer shipped. However switching to using this variant of OVMF for UEFI guests is not just a matter of changing the filename. The new OVMF variant won't run unless we also change: - The qemu machine model, from the default ("pc" == "pc-i440fx-rhel7.3.0" or later) to Q35 ("q35" == "pc-q35-rhel7.3.0" or later). - Add <smm> under <features>. - Set <loader ... secure="yes">. NB: On RHEL the changes requires qemu-kvm-rhev. It is no longer possible to convert UEFI guests using the basic qemu-kvm. Thanks: Laszlo Ersek, Ming Xie. --- generator/uefi.ml | 24 ++++++++++++++++++------ src/appliance.c | 6 ++++-- src/guestfs-internal-frontend.h | 3 ++- src/guestfs-internal.h | 2 +- src/launch-direct.c | 15 ++++++++++++++- src/launch-libvirt.c | 16 +++++++++++++++- v2v/output_libvirt.ml | 40 +++++++++++++++++++++++++++++----------- v2v/output_qemu.ml | 15 +++++++++++---- v2v/virt-v2v.pod | 2 ++ 9 files changed, 96 insertions(+), 27 deletions(-) diff --git a/generator/uefi.ml b/generator/uefi.ml index 1f78a6d..88e54b8 100644 --- a/generator/uefi.ml +++ b/generator/uefi.ml @@ -41,6 +41,16 @@ let firmware = [ "/usr/share/OVMF/OVMF_VARS.fd", []; + (* From RHEL 7.3, only secure boot variants of UEFI are shipped. + * This requires additional qemu options, see RHBZ#1367615 for + * details. + *) + "x86_64", + "/usr/share/OVMF/OVMF_CODE.secboot.fd", + None, + "/usr/share/OVMF/OVMF_VARS.fd", + [ "UEFI_FLAG_SECURE_BOOT_REQUIRED" ]; + "x86_64", "/usr/share/edk2/ovmf/OVMF_CODE.fd", None, @@ -100,14 +110,13 @@ let generate_uefi_c () = pr "guestfs_int_uefi_%s_firmware[] = {\n" arch; List.iter ( fun (_, code, code_debug, vars, flags) -> - assert (flags = []); pr " { \"%s\",\n" (c_quote code); (match code_debug with | None -> pr " NULL,\n" | Some code_debug -> pr " \"%s\",\n" (c_quote code_debug) ); pr " \"%s\",\n" (c_quote vars); - pr " 0\n"; + pr " %s\n" (if flags <> [] then String.concat "|" flags else "0"); pr " },\n"; ) firmware; pr "};\n"; @@ -121,8 +130,10 @@ type uefi_firmware = { code : string; code_debug : string option; vars : string; - flags : unit list; + flags : uefi_flags; } +and uefi_flags = uefi_flag list +and uefi_flag = UEFI_FLAG_SECURE_BOOT_REQUIRED "; List.iter ( fun arch -> @@ -132,14 +143,13 @@ type uefi_firmware = { pr "let uefi_%s_firmware = [\n" arch; List.iter ( fun (_, code, code_debug, vars, flags) -> - assert (flags = []); pr " { code = %S;\n" code; (match code_debug with | None -> pr " code_debug = None;\n" | Some code_debug -> pr " code_debug = Some %S;\n" code_debug ); pr " vars = %S;\n" vars; - pr " flags = [];\n"; + pr " flags = [%s];\n" (String.concat "; " flags); pr " };\n"; ) firmware; pr "]\n"; @@ -155,8 +165,10 @@ type uefi_firmware = { code : string; (** code file *) code_debug : string option; (** code debug file *) vars : string; (** vars template file *) - flags : unit list; (** flags *) + flags : uefi_flags; (** flags *) } +and uefi_flags = uefi_flag list +and uefi_flag = UEFI_FLAG_SECURE_BOOT_REQUIRED "; diff --git a/src/appliance.c b/src/appliance.c index 942ecae..025d55d 100644 --- a/src/appliance.c +++ b/src/appliance.c @@ -423,10 +423,10 @@ dir_contains_files (guestfs_h *g, const char *dir, ...) * should cause appliance building to fail (no UEFI firmware is not an * error). * - * XXX See also F<v2v/utils.ml>:find_uefi_firmware + * See also F<v2v/utils.ml>:find_uefi_firmware */ int -guestfs_int_get_uefi (guestfs_h *g, char **code, char **vars) +guestfs_int_get_uefi (guestfs_h *g, char **code, char **vars, int *flags) { #ifdef __aarch64__ size_t i; @@ -464,6 +464,7 @@ guestfs_int_get_uefi (guestfs_h *g, char **code, char **vars) /* Caller frees. */ *code = safe_strdup (g, codefile); *vars = varst; + *flags = guestfs_int_aavmf_firmware[i].flags; return 0; } } @@ -471,5 +472,6 @@ guestfs_int_get_uefi (guestfs_h *g, char **code, char **vars) /* Not found. */ *code = *vars = NULL; + *flags = 0; return 0; } diff --git a/src/guestfs-internal-frontend.h b/src/guestfs-internal-frontend.h index f40d2c6..a41ee9c 100644 --- a/src/guestfs-internal-frontend.h +++ b/src/guestfs-internal-frontend.h @@ -100,7 +100,8 @@ struct uefi_firmware { const char *code; /* code file (NULL = end of list) */ const char *code_debug; /* code file with debugging msgs (may be NULL)*/ const char *vars; /* vars template file */ - int flags; /* flags */ + int flags; /* various flags, see below */ +#define UEFI_FLAG_SECURE_BOOT_REQUIRED 1 /* secure boot (see RHBZ#1367615) */ }; extern struct uefi_firmware guestfs_int_ovmf_i386_firmware[]; extern struct uefi_firmware guestfs_int_ovmf_x86_64_firmware[]; diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h index 2b49011..d437b9a 100644 --- a/src/guestfs-internal.h +++ b/src/guestfs-internal.h @@ -783,7 +783,7 @@ extern const char *guestfs_int_drive_protocol_to_string (enum drive_protocol pro /* appliance.c */ extern int guestfs_int_build_appliance (guestfs_h *g, char **kernel, char **initrd, char **appliance); -extern int guestfs_int_get_uefi (guestfs_h *g, char **code, char **vars); +extern int guestfs_int_get_uefi (guestfs_h *g, char **code, char **vars, int *flags); /* launch.c */ extern int64_t guestfs_int_timeval_diff (const struct timeval *x, const struct timeval *y); diff --git a/src/launch-direct.c b/src/launch-direct.c index 6450d7e..64e52e8 100644 --- a/src/launch-direct.c +++ b/src/launch-direct.c @@ -226,6 +226,7 @@ launch_direct (guestfs_h *g, void *datav, const char *arg) int sv[2]; struct sockaddr_un addr; CLEANUP_FREE char *uefi_code = NULL, *uefi_vars = NULL; + int uefi_flags; CLEANUP_FREE char *kernel = NULL, *initrd = NULL, *appliance = NULL; int has_appliance_drive; CLEANUP_FREE char *appliance_dev = NULL; @@ -422,8 +423,20 @@ launch_direct (guestfs_h *g, void *datav, const char *arg) } /* UEFI (firmware) if required. */ - if (guestfs_int_get_uefi (g, &uefi_code, &uefi_vars) == -1) + if (guestfs_int_get_uefi (g, &uefi_code, &uefi_vars, &uefi_flags) == -1) goto cleanup0; + if (uefi_flags & UEFI_FLAG_SECURE_BOOT_REQUIRED) { + /* Implementing this requires changes to the qemu command line. + * See RHBZ#1367615 for details. As the guestfs_int_get_uefi + * function is only implemented for aarch64, and UEFI secure boot + * is some way off on aarch64 (2017/2018), we only need to worry + * about this later. + */ + error (g, "internal error: direct backend " + "does not implement UEFI secure boot, " + "see comments in the code"); + goto cleanup0; + } if (uefi_code) { ADD_CMDLINE ("-drive"); ADD_CMDLINE_PRINTF ("if=pflash,format=raw,file=%s,readonly", uefi_code); diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c index 6249494..d8479dc 100644 --- a/src/launch-libvirt.c +++ b/src/launch-libvirt.c @@ -313,6 +313,7 @@ launch_libvirt (guestfs_h *g, void *datav, const char *libvirt_uri) uint32_t size; CLEANUP_FREE void *buf = NULL; unsigned long version_number; + int uefi_flags; params.current_proc_is_root = geteuid () == 0; @@ -409,8 +410,21 @@ launch_libvirt (guestfs_h *g, void *datav, const char *libvirt_uri) goto cleanup; /* UEFI code and variables, on architectures where that is required. */ - if (guestfs_int_get_uefi (g, &data->uefi_code, &data->uefi_vars) == -1) + if (guestfs_int_get_uefi (g, &data->uefi_code, &data->uefi_vars, + &uefi_flags) == -1) goto cleanup; + if (uefi_flags & UEFI_FLAG_SECURE_BOOT_REQUIRED) { + /* Implementing this requires changes to the libvirt XML. See + * RHBZ#1367615 for details. As the guestfs_int_get_uefi function + * is only implemented for aarch64, and UEFI secure boot is some + * way off on aarch64 (2017/2018), we only need to worry about + * this later. + */ + error (g, "internal error: libvirt backend " + "does not implement UEFI secure boot, " + "see comments in the code"); + goto cleanup; + } /* Misc backend settings. */ guestfs_push_error_handler (g, NULL, NULL); diff --git a/v2v/output_libvirt.ml b/v2v/output_libvirt.ml index df0963e..ed430a2 100644 --- a/v2v/output_libvirt.ml +++ b/v2v/output_libvirt.ml @@ -71,6 +71,16 @@ let target_features_of_capabilities_doc doc arch = let create_libvirt_xml ?pool source target_buses guestcaps target_features target_firmware = + let uefi_firmware = + match target_firmware with + | TargetBIOS -> None + | TargetUEFI -> Some (find_uefi_firmware guestcaps.gcaps_arch) in + let secure_boot_required = + match uefi_firmware with + | Some { Uefi.flags = flags } + when List.mem Uefi.UEFI_FLAG_SECURE_BOOT_REQUIRED flags -> true + | _ -> false in + let memory_k = source.s_memory /^ 1024L in (* We have the machine features of the guest when it was on the @@ -106,24 +116,32 @@ let create_libvirt_xml ?pool source target_buses guestcaps StringSet.inter(*section*) force_features target_features in let features = StringSet.union features force_features in + (* Add <smm> feature if UEFI requires it. Note that libvirt + * capabilities doesn't list this feature even if it is supported + * by qemu, so we have to blindly add it, which might cause libvirt + * to fail. (XXX) + *) + let features = + if secure_boot_required then StringSet.add "smm" features else features in + let features = List.sort compare (StringSet.elements features) in (* The <os> section subelements. *) let os_section = + let machine = if secure_boot_required then [ "machine", "q35" ] else [] in + let loader = - match target_firmware with - | TargetBIOS -> [] - | TargetUEFI -> - (* danpb is proposing that libvirt supports <loader type="efi"/>, - * (https://bugzilla.redhat.com/show_bug.cgi?id=1217444#c6) but - * until that day we have to use a bunch of heuristics. XXX - *) - let { Uefi.code = code; vars = vars_template } = - find_uefi_firmware guestcaps.gcaps_arch in - [ e "loader" ["readonly", "yes"; "type", "pflash"] [ PCData code ]; + match uefi_firmware with + | None -> [] + | Some { Uefi.code = code; vars = vars_template } -> + let secure = + if secure_boot_required then [ "secure", "yes" ] else [] in + [ e "loader" (["readonly", "yes"; "type", "pflash"] @ secure) + [ PCData code ]; e "nvram" ["template", vars_template] [] ] in - (e "type" ["arch", guestcaps.gcaps_arch] [PCData "hvm"]) :: loader in + (e "type" (["arch", guestcaps.gcaps_arch] @ machine) [PCData "hvm"]) + :: loader in (* The devices. *) let devices = ref [] in diff --git a/v2v/output_qemu.ml b/v2v/output_qemu.ml index 93694b5..0788e6c 100644 --- a/v2v/output_qemu.ml +++ b/v2v/output_qemu.ml @@ -57,8 +57,12 @@ object let uefi_firmware = match target_firmware with | TargetBIOS -> None - | TargetUEFI -> - Some (find_uefi_firmware guestcaps.gcaps_arch) in + | TargetUEFI -> Some (find_uefi_firmware guestcaps.gcaps_arch) in + let secure_boot_required = + match uefi_firmware with + | Some { Uefi.flags = flags } + when List.mem Uefi.UEFI_FLAG_SECURE_BOOT_REQUIRED flags -> true + | _ -> false in let chan = open_out file in @@ -69,7 +73,7 @@ object (match uefi_firmware with | None -> () - | Some { vars = vars_template } -> + | Some { Uefi.vars = vars_template } -> fpf "# Make a copy of the UEFI variables template\n"; fpf "uefi_vars=\"$(mktemp)\"\n"; fpf "cp %s \"$uefi_vars\"\n" (quote vars_template); @@ -79,11 +83,14 @@ object fpf "qemu-system-%s" guestcaps.gcaps_arch; fpf "%s-no-user-config -nodefaults" nl; fpf "%s-name %s" nl (quote source.s_name); - fpf "%s-machine accel=kvm:tcg" nl; + fpf "%s-machine %saccel=kvm:tcg" nl + (if secure_boot_required then "q35,smm=on," else ""); (match uefi_firmware with | None -> () | Some { Uefi.code = code } -> + if secure_boot_required then + fpf "%s-global driver=cfi.pflash01,property=secure,value=on" nl; fpf "%s-drive if=pflash,format=raw,file=%s,readonly" nl (quote code); fpf "%s-drive if=pflash,format=raw,file=\"$uefi_vars\"" nl ); diff --git a/v2v/virt-v2v.pod b/v2v/virt-v2v.pod index ed0d286..220dd45 100644 --- a/v2v/virt-v2v.pod +++ b/v2v/virt-v2v.pod @@ -881,6 +881,8 @@ automatically, but note that the same version of OVMF must be installed on the conversion host as is installed on the target hypervisor, else you will have to adjust paths in the metadata. +On RHEL E<ge> 7.3, only qemu-kvm-rhev (not qemu-kvm) is supported. + =item UEFI on OpenStack Not supported. -- 2.7.4