Libguestfs - Aug 2016 - [PATCH] v2v: Use OVMF secure boot file (RHBZ#1367615).

This is only lightly tested.  In particularly I only tested that the
non-secure-boot path still works.  I didn't test it on RHEL 7.3 yet
because I haven't got enough free disk space for these giant source
*.ova files :-(  Will try to give that a go later.

Rich.

>From RHEL 7.3, Red Hat have decided to only compile the secure boot
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.
---
 src/appliance.c                 |  6 ++++--
 src/guestfs-internal-frontend.h |  2 ++
 src/guestfs-internal.h          |  2 +-
 src/launch-direct.c             | 15 ++++++++++++++-
 src/launch-libvirt.c            | 16 +++++++++++++++-
 src/uefi.c                      | 37 +++++++++++++++++++++++++++++--------
 v2v/output_libvirt.ml           | 40 +++++++++++++++++++++++++++++-----------
 v2v/output_qemu.ml              | 13 ++++++++++---
 v2v/utils-c.c                   | 20 ++++++++++++++++++--
 v2v/utils.ml                    |  3 +++
 v2v/utils.mli                   |  3 +++
 v2v/v2v_unit_tests.ml           | 35 +++++++++++++++++++++++++----------
 v2v/virt-v2v.pod                |  2 ++
 13 files changed, 155 insertions(+), 39 deletions(-)

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 8689009..a41ee9c 100644
--- a/src/guestfs-internal-frontend.h
+++ b/src/guestfs-internal-frontend.h
@@ -100,6 +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;                    /* 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/src/uefi.c b/src/uefi.c
index 44340fe..90d5e63 100644
--- a/src/uefi.c
+++ b/src/uefi.c
@@ -18,6 +18,10 @@
 
 /**
  * Locations of UEFI files.
+ *
+ * danpb is proposing that libvirt supports E<lt>loader
type="efi"/E<gt>
+ * (L<https://bugzilla.redhat.com/1217444#c6>).  If that happens we can
+ * simplify or even remove this code.
  */
 
 #include <config.h>
@@ -33,7 +37,8 @@ guestfs_int_ovmf_i386_firmware[] = {
   /* kraxel's old repository, these will be removed by end of 2016. */
   { "/usr/share/edk2.git/ovmf-ia32/OVMF_CODE-pure-efi.fd",
     NULL,
-    "/usr/share/edk2.git/ovmf-ia32/OVMF_VARS-pure-efi.fd" },
+    "/usr/share/edk2.git/ovmf-ia32/OVMF_VARS-pure-efi.fd",
+    0 },
 
   { NULL }
 };
@@ -42,20 +47,33 @@ struct uefi_firmware
 guestfs_int_ovmf_x86_64_firmware[] = {
   { "/usr/share/OVMF/OVMF_CODE.fd",
     NULL,
-    "/usr/share/OVMF/OVMF_VARS.fd" },
+    "/usr/share/OVMF/OVMF_VARS.fd",
+    0 },
+
+  /* From RHEL 7.3, only secure boot variants of UEFI are shipped.
+   * This requires additional qemu options, see RHBZ#1367615 for
+   * details.
+   */
+  { "/usr/share/OVMF/OVMF_CODE.secboot.fd",
+    NULL,
+    "/usr/share/OVMF/OVMF_VARS.fd",
+    UEFI_FLAG_SECURE_BOOT_REQUIRED },
 
   { "/usr/share/edk2/ovmf/OVMF_CODE.fd",
     NULL,
-    "/usr/share/edk2/ovmf/OVMF_VARS.fd" },
+    "/usr/share/edk2/ovmf/OVMF_VARS.fd",
+    0 },
 
   /* kraxel's old repository, these will be removed by end of 2016. */
   { "/usr/share/edk2.git/ovmf-x64/OVMF_CODE-pure-efi.fd",
     NULL,
-    "/usr/share/edk2.git/ovmf-x64/OVMF_VARS-pure-efi.fd" },
+    "/usr/share/edk2.git/ovmf-x64/OVMF_VARS-pure-efi.fd",
+    0 },
 
   { "/usr/share/qemu/ovmf-x86_64-code.bin",
     NULL,
-    "/usr/share/qemu/ovmf-x86_64-vars.bin" },
+    "/usr/share/qemu/ovmf-x86_64-vars.bin",
+    0 },
 
   { NULL }
 };
@@ -64,16 +82,19 @@ struct uefi_firmware
 guestfs_int_aavmf_firmware[] = {
   { "/usr/share/AAVMF/AAVMF_CODE.fd",
     "/usr/share/AAVMF/AAVMF_CODE.verbose.fd",
-    "/usr/share/AAVMF/AAVMF_VARS.fd" },
+    "/usr/share/AAVMF/AAVMF_VARS.fd",
+    0 },
 
   { "/usr/share/edk2/aarch64/QEMU_EFI-pflash.raw",
     NULL,
-    "/usr/share/edk2/aarch64/vars-template-pflash.raw" },
+    "/usr/share/edk2/aarch64/vars-template-pflash.raw",
+    0 },
 
   /* kraxel's old repository, these will be removed by end of 2016. */
   { "/usr/share/edk2.git/aarch64/QEMU_EFI-pflash.raw",
     NULL,
-    "/usr/share/edk2.git/aarch64/vars-template-pflash.raw" },
+    "/usr/share/edk2.git/aarch64/vars-template-pflash.raw",
+    0 },
 
   { NULL }
 };
diff --git a/v2v/output_libvirt.ml b/v2v/output_libvirt.ml
index 1f66d6a..e637595 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 { flags = flags }
+         when List.mem 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 { 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 { 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 b6093be..37ede41 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 { flags = flags }
+           when List.mem UEFI_FLAG_SECURE_BOOT_REQUIRED flags -> true
+      | _ -> false in
 
     let chan = open_out file in
 
@@ -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 { 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/utils-c.c b/v2v/utils-c.c
index 2812a3d..ccc405e 100644
--- a/v2v/utils-c.c
+++ b/v2v/utils-c.c
@@ -71,6 +71,7 @@ get_firmware (struct uefi_firmware *firmware)
 {
   CAMLparam0 ();
   CAMLlocal5 (rv, v, v1, v2, cons);
+  CAMLlocal3 (flag, flags, flagcons);
   size_t i, len;
 
   rv = Val_int (0);
@@ -83,13 +84,28 @@ get_firmware (struct uefi_firmware *firmware)
   for (i = len; i > 0; --i) {
     v1 = caml_copy_string (firmware[i-1].code);
     v2 = caml_copy_string (firmware[i-1].vars);
-    v = caml_alloc (2, 0);
+
+    flags = Val_int (0);
+    if (firmware[i-1].flags & UEFI_FLAG_SECURE_BOOT_REQUIRED) {
+      flag = Val_int (0); /* position of UEFI_FLAG_SECURE_BOOT_REQUIRED
+                             in uefi_flag type */
+      flagcons = caml_alloc (2, 0);
+      Store_field (flagcons, 0, flag);
+      Store_field (flagcons, 1, flags);
+      flags = flagcons;
+    }
+
+    /* Construct uefi_firmware node. */
+    v = caml_alloc (3, 0);
     Store_field (v, 0, v1);
     Store_field (v, 1, v2);
+    Store_field (v, 2, flags);
+
+    /* Prepend it to the uefi_firmware list returned. */
     cons = caml_alloc (2, 0);
+    Store_field (cons, 0, v);
     Store_field (cons, 1, rv);
     rv = cons;
-    Store_field (cons, 0, v);
   }
 
   CAMLreturn (rv);
diff --git a/v2v/utils.ml b/v2v/utils.ml
index 2dff6aa..edc2a8c 100644
--- a/v2v/utils.ml
+++ b/v2v/utils.ml
@@ -84,7 +84,10 @@ let qemu_supports_sound_card = function
 type uefi_firmware = {
   code : string;       (* code file *)
   vars : string;       (* vars template file *)
+  flags : uefi_flags;  (** flags *)
 }
+and uefi_flags = uefi_flag list
+and uefi_flag = UEFI_FLAG_SECURE_BOOT_REQUIRED
 
 external ovmf_i386_firmware : unit -> uefi_firmware list =
"v2v_utils_ovmf_i386_firmware"
 external ovmf_x86_64_firmware : unit -> uefi_firmware list =
"v2v_utils_ovmf_x86_64_firmware"
diff --git a/v2v/utils.mli b/v2v/utils.mli
index a00e60b..ff12bd8 100644
--- a/v2v/utils.mli
+++ b/v2v/utils.mli
@@ -46,7 +46,10 @@ val qemu_supports_sound_card : Types.source_sound_model ->
bool
 type uefi_firmware = {
   code : string;       (** code file *)
   vars : string;       (** vars template file *)
+  flags : uefi_flags;  (** flags *)
 }
+and uefi_flags = uefi_flag list
+and uefi_flag = UEFI_FLAG_SECURE_BOOT_REQUIRED
 
 val find_uefi_firmware : string -> uefi_firmware
 (** Find the UEFI firmware for the guest architecture.
diff --git a/v2v/v2v_unit_tests.ml b/v2v/v2v_unit_tests.ml
index 6064448..fbb71eb 100644
--- a/v2v/v2v_unit_tests.ml
+++ b/v2v/v2v_unit_tests.ml
@@ -782,30 +782,45 @@ let test_shell_unquote ctx  let test_find_uefi_firmware
ctx    let rec printer = function
   | [] -> ""
-  | { Utils.code = code; vars = vars } :: xs ->
-    sprintf "code=%s vars=%s\n%s" code vars (printer xs)
+  | { Utils.code = code; vars = vars; flags = flags } :: xs ->
+    sprintf "code=%s vars=%s flags=[%s]\n%s"
+            code vars (String.concat ";" (List.map string_of_flag
flags))
+            (printer xs)
+  and string_of_flag = function
+  | Utils.UEFI_FLAG_SECURE_BOOT_REQUIRED -> "secure_boot_required"
   in
   assert_equal ~printer
     [ { Utils.code =
"/usr/share/edk2.git/ovmf-ia32/OVMF_CODE-pure-efi.fd";
-              vars =
"/usr/share/edk2.git/ovmf-ia32/OVMF_VARS-pure-efi.fd" } ]
+              vars =
"/usr/share/edk2.git/ovmf-ia32/OVMF_VARS-pure-efi.fd";
+             flags = [] } ]
     (Utils.UNIT_TESTS.ovmf_i386_firmware ());
   assert_equal ~printer
     [ { Utils.code = "/usr/share/OVMF/OVMF_CODE.fd";
-              vars = "/usr/share/OVMF/OVMF_VARS.fd" };
+              vars = "/usr/share/OVMF/OVMF_VARS.fd";
+             flags = [] };
+      { Utils.code = "/usr/share/OVMF/OVMF_CODE.secboot.fd";
+              vars = "/usr/share/OVMF/OVMF_VARS.fd";
+             flags = [ Utils.UEFI_FLAG_SECURE_BOOT_REQUIRED ] };
       { Utils.code = "/usr/share/edk2/ovmf/OVMF_CODE.fd";
-              vars = "/usr/share/edk2/ovmf/OVMF_VARS.fd" };
+              vars = "/usr/share/edk2/ovmf/OVMF_VARS.fd";
+             flags = [] };
       { Utils.code =
"/usr/share/edk2.git/ovmf-x64/OVMF_CODE-pure-efi.fd";
-              vars =
"/usr/share/edk2.git/ovmf-x64/OVMF_VARS-pure-efi.fd" };
+              vars =
"/usr/share/edk2.git/ovmf-x64/OVMF_VARS-pure-efi.fd";
+             flags = [] };
       { Utils.code = "/usr/share/qemu/ovmf-x86_64-code.bin";
-              vars = "/usr/share/qemu/ovmf-x86_64-vars.bin" } ]
+              vars = "/usr/share/qemu/ovmf-x86_64-vars.bin";
+             flags = [] } ]
     (Utils.UNIT_TESTS.ovmf_x86_64_firmware ());
   assert_equal ~printer
     [ { Utils.code = "/usr/share/AAVMF/AAVMF_CODE.fd";
-              vars = "/usr/share/AAVMF/AAVMF_VARS.fd" };
+              vars = "/usr/share/AAVMF/AAVMF_VARS.fd";
+             flags = [] };
       { Utils.code = "/usr/share/edk2/aarch64/QEMU_EFI-pflash.raw";
-              vars =
"/usr/share/edk2/aarch64/vars-template-pflash.raw" };
+              vars =
"/usr/share/edk2/aarch64/vars-template-pflash.raw";
+             flags = [] };
       { Utils.code =
"/usr/share/edk2.git/aarch64/QEMU_EFI-pflash.raw";
-              vars =
"/usr/share/edk2.git/aarch64/vars-template-pflash.raw" } ]
+              vars =
"/usr/share/edk2.git/aarch64/vars-template-pflash.raw";
+             flags = [] } ]
     (Utils.UNIT_TESTS.aavmf_firmware ())
 
 (* Suites declaration. *)
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