virt-v2v/0026-convert-document-networking-dependency-of-key-ID-cle.patch

From 94b57f647d67d4713284af1f0580d1afedd6ebaa Mon Sep 17 00:00:00 2001
From: Laszlo Ersek <lersek@redhat.com>
Date: Fri, 1 Jul 2022 15:30:42 +0200
Subject: [PATCH] convert: document networking dependency of "--key ID:clevis"

Virt-v2v enables appliance networking already, for the sake of
"unconfigure_vmware". We now have a second use case for networking: "--key
ID:clevis". Update the comment in the code.

(Short log for libguestfs-common commit range 35467027f657..af6cb55bc58a:

Laszlo Ersek (12):
      options: fix UUID comparison logic bug in get_keys()
      mltools/tools_utils: remove unused function "key_store_to_cli"
      mltools/tools_utils: allow multiple "--key" options for OCaml tools too
      options: replace NULL-termination with number-of-elements in get_keys()
      options: wrap each passphrase from get_keys() into a struct
      options: add back-end for LUKS decryption with Clevis+Tang
      options: introduce selector type "key_clevis"
      options: generalize "--key" selector parsing for C-language utilities
      mltools/tools_utils-c: handle internal type error with abort()
      mltools/tools_utils: generalize "--key" selector parsing for OCaml utils
      options, mltools/tools_utils: parse "--key ID:clevis" options
      options, mltools/tools_utils: add helper for network dependency
).

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1809453
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20220628115856.5820-1-lersek@redhat.com>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
(cherry picked from commit 98fa5ab2685371c681282ce5de704877af27be74)
---
 common             | 2 +-
 convert/convert.ml | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

Submodule common 35467027..af6cb55b:
diff --git a/common/mltools/tools_utils-c.c b/common/mltools/tools_utils-c.c
index 08146677..4ff42e5d 100644
--- a/common/mltools/tools_utils-c.c
+++ b/common/mltools/tools_utils-c.c
@@ -62,24 +62,31 @@ guestfs_int_mllib_inspect_decrypt (value gv, value gpv, value keysv)
       caml_raise_out_of_memory ();
 
     v = Field (elemv, 1);
-    switch (Tag_val (v)) {
-    case 0:  /* KeyString of string */
-      key.type = key_string;
-      key.string.s = strdup (String_val (Field (v, 0)));
-      if (!key.string.s)
-        caml_raise_out_of_memory ();
-      break;
-    case 1:  /* KeyFileName of string */
-      key.type = key_file;
-      key.file.name = strdup (String_val (Field (v, 0)));
-      if (!key.file.name)
-        caml_raise_out_of_memory ();
-      break;
-    default:
-      error (EXIT_FAILURE, 0,
-             "internal error: unhandled Tag_val (v) = %d",
-             Tag_val (v));
-    }
+    if (Is_block (v))
+      switch (Tag_val (v)) {
+      case 0:  /* KeyString of string */
+        key.type = key_string;
+        key.string.s = strdup (String_val (Field (v, 0)));
+        if (!key.string.s)
+          caml_raise_out_of_memory ();
+        break;
+      case 1:  /* KeyFileName of string */
+        key.type = key_file;
+        key.file.name = strdup (String_val (Field (v, 0)));
+        if (!key.file.name)
+          caml_raise_out_of_memory ();
+        break;
+      default:
+        abort ();
+      }
+    else
+      switch (Int_val (v)) {
+      case 0:  /* KeyClevis */
+        key.type = key_clevis;
+        break;
+      default:
+        abort ();
+      }
 
     ks = key_store_import_key (ks, &key);
 
diff --git a/common/mltools/tools_utils.ml b/common/mltools/tools_utils.ml
index 695fda7e..562bfadc 100644
--- a/common/mltools/tools_utils.ml
+++ b/common/mltools/tools_utils.ml
@@ -29,11 +29,12 @@ open Getopt.OptionName
 let prog = ref prog
 
 type key_store = {
-  keys : (string, key_store_key) Hashtbl.t;
+  keys : (string * key_store_key) list ref;
 }
 and key_store_key =
   | KeyString of string
   | KeyFileName of string
+  | KeyClevis
 
 external c_inspect_decrypt : Guestfs.t -> int64 -> (string * key_store_key) list -> unit = "guestfs_int_mllib_inspect_decrypt"
 external c_set_echo_keys : unit -> unit = "guestfs_int_mllib_set_echo_keys" [@@noalloc]
@@ -376,7 +377,7 @@ let create_standard_options argspec ?anon_fun ?(key_opts = false)
       )
   in
   let ks = {
-    keys = Hashtbl.create 13;
+    keys = ref [];
   } in
   let argspec = ref argspec in
   let add_argspec = List.push_back argspec in
@@ -392,14 +393,28 @@ let create_standard_options argspec ?anon_fun ?(key_opts = false)
 
   if key_opts then (
     let parse_key_selector arg =
-      let parts = String.nsplit ~max:3 ":" arg in
+      let parts = String.nsplit ":" arg in
       match parts with
+      | [] ->
+        error (f_"selector '%s': missing ID") arg
+      | [ _ ] ->
+        error (f_"selector '%s': missing TYPE") arg
+      | [ _; "key" ]
+      |  _ :: "key" :: _ :: _ :: _ ->
+        error (f_"selector '%s': missing KEY_STRING, or too many fields") arg
       | [ device; "key"; key ] ->
-         Hashtbl.replace ks.keys device (KeyString key)
+         List.push_back ks.keys (device, KeyString key)
+      | [ _; "file" ]
+      |  _ :: "file" :: _ :: _ :: _ ->
+        error (f_"selector '%s': missing FILENAME, or too many fields") arg
       | [ device; "file"; file ] ->
-         Hashtbl.replace ks.keys device (KeyFileName file)
+         List.push_back ks.keys (device, KeyFileName file)
+      |  _ :: "clevis" :: _ :: _ ->
+        error (f_"selector '%s': too many fields") arg
+      | [ device; "clevis" ] ->
+         List.push_back ks.keys (device, KeyClevis)
       | _ ->
-         error (f_"invalid selector string for --key: %s") arg
+         error (f_"selector '%s': invalid TYPE") arg
     in
 
     add_argspec ([ L"echo-keys" ],       Getopt.Unit c_set_echo_keys,       s_"Don’t turn off echo for passphrases");
@@ -420,16 +435,6 @@ let create_standard_options argspec ?anon_fun ?(key_opts = false)
   let getopt = Getopt.create argspec ?anon_fun usage_msg in
   { getopt; ks; debug_gc }
 
-let key_store_to_cli { keys } =
-  Hashtbl.fold (
-    fun k v acc ->
-      let arg =
-        match v with
-        | KeyString s -> sprintf "%s:key:%s" k s
-        | KeyFileName f -> sprintf "%s:file:%s" k f in
-      "--key" :: arg :: acc
-  ) keys []
-
 (* Run an external command, slurp up the output as a list of lines. *)
 let external_command ?(echo_cmd = true) cmd =
   if echo_cmd then
@@ -691,21 +696,19 @@ let is_btrfs_subvolume g fs =
     if g#last_errno () = Guestfs.Errno.errno_EINVAL then false
     else raise exn
 
+let key_store_requires_network ks =
+  List.exists (function
+               | _, KeyClevis -> true
+               | _ -> false) !(ks.keys)
+
 let inspect_decrypt g ks =
-  (* Turn the keys in the key_store into a simpler struct, so it is possible
-   * to read it using the C API.
-   *)
-  let keys_as_list = Hashtbl.fold (
-    fun k v acc ->
-      (k, v) :: acc
-  ) ks.keys [] in
   (* Note we pass original 'g' even though it is not used by the
    * callee.  This is so that 'g' is kept as a root on the stack, and
    * so cannot be garbage collected while we are in the c_inspect_decrypt
    * function.
    *)
   c_inspect_decrypt g#ocaml_handle (Guestfs.c_pointer g#ocaml_handle)
-    keys_as_list
+    !(ks.keys)
 
 let with_timeout op timeout ?(sleep = 2) fn =
   let start_t = Unix.gettimeofday () in
diff --git a/common/mltools/tools_utils.mli b/common/mltools/tools_utils.mli
index 50183006..ec900e63 100644
--- a/common/mltools/tools_utils.mli
+++ b/common/mltools/tools_utils.mli
@@ -103,14 +103,6 @@ val create_standard_options : Getopt.speclist -> ?anon_fun:Getopt.anon_fun -> ?k
 
     Returns a new {!cmdline_options} structure. *)
 
-val key_store_to_cli : key_store -> string list
-(** Convert a {!key_store} object back to a list of command line
-    options, essentially undoing the effect of Getopt parsing.
-    This is used in virt-v2v to pass the keystore to helpers.
-    It is not particularly secure, especially if you use the
-    [:key:] selector, although not any less secure than passing
-    them via the command line in the first place. *)
-
 val external_command : ?echo_cmd:bool -> string -> string list
 (** Run an external command, slurp up the output as a list of lines.
 
@@ -204,6 +196,10 @@ val inspect_mount_root_ro : Guestfs.guestfs -> string -> unit
 val is_btrfs_subvolume : Guestfs.guestfs -> string -> bool
 (** Checks if a filesystem is a btrfs subvolume. *)
 
+val key_store_requires_network : key_store -> bool
+(** [key_store_requires_network ks] returns [true] iff [ks] contains at least
+    one "ID:clevis" selector. *)
+
 val inspect_decrypt : Guestfs.guestfs -> key_store -> unit
 (** Simple implementation of decryption: look for any encrypted
     partitions and decrypt them, then rescan for VGs. *)
diff --git a/common/options/decrypt.c b/common/options/decrypt.c
index 1cd7b627..97c8b88d 100644
--- a/common/options/decrypt.c
+++ b/common/options/decrypt.c
@@ -124,10 +124,10 @@ decrypt_mountables (guestfs_h *g, const char * const *mountables,
   while ((mountable = *mnt_scan++) != NULL) {
     CLEANUP_FREE char *type = NULL;
     CLEANUP_FREE char *uuid = NULL;
-    CLEANUP_FREE_STRING_LIST char **keys = NULL;
+    struct matching_key *keys;
+    size_t nr_matches;
     CLEANUP_FREE char *mapname = NULL;
-    const char * const *key_scan;
-    const char *key;
+    size_t scan;
 
     type = guestfs_vfs_type (g, mountable);
     if (type == NULL)
@@ -144,33 +144,45 @@ decrypt_mountables (guestfs_h *g, const char * const *mountables,
     /* Grab the keys that we should try with this device, based on device name,
      * or UUID (if any).
      */
-    keys = get_keys (ks, mountable, uuid);
-    assert (keys[0] != NULL);
+    keys = get_keys (ks, mountable, uuid, &nr_matches);
+    assert (nr_matches > 0);
 
     /* Generate a node name for the plaintext (decrypted) device node. */
     if (uuid == NULL || asprintf (&mapname, "luks-%s", uuid) == -1)
       mapname = make_mapname (mountable);
 
     /* Try each key in turn. */
-    key_scan = (const char * const *)keys;
-    while ((key = *key_scan++) != NULL) {
+    for (scan = 0; scan < nr_matches; ++scan) {
+      struct matching_key *key = keys + scan;
       int r;
 
       guestfs_push_error_handler (g, NULL, NULL);
-      r = guestfs_cryptsetup_open (g, mountable, key, mapname, -1);
+      assert (key->clevis == (key->passphrase == NULL));
+      if (key->clevis)
+#ifdef GUESTFS_HAVE_CLEVIS_LUKS_UNLOCK
+        r = guestfs_clevis_luks_unlock (g, mountable, mapname);
+#else
+        error (EXIT_FAILURE, 0,
+               _("'clevis_luks_unlock', needed for decrypting %s, is "
+                 "unavailable in this libguestfs version"), mountable);
+#endif
+      else
+        r = guestfs_cryptsetup_open (g, mountable, key->passphrase, mapname,
+                                     -1);
       guestfs_pop_error_handler (g);
 
       if (r == 0)
         break;
     }
 
-    if (key == NULL)
+    if (scan == nr_matches)
       error (EXIT_FAILURE, 0,
              _("could not find key to open LUKS encrypted %s.\n\n"
                "Try using --key on the command line.\n\n"
                "Original error: %s (%d)"),
              mountable, guestfs_last_error (g), guestfs_last_errno (g));
 
+    free_keys (keys, nr_matches);
     decrypted_some = true;
   }
 
diff --git a/common/options/key-option.pod b/common/options/key-option.pod
index 90a3b15c..6bc04df1 100644
--- a/common/options/key-option.pod
+++ b/common/options/key-option.pod
@@ -14,4 +14,13 @@ Use the specified C<KEY_STRING> as passphrase.
 
 Read the passphrase from F<FILENAME>.
 
+=item B<--key> C<ID>:clevis
+
+Attempt passphrase-less unlocking for C<ID> with Clevis, over the
+network.  Please refer to L<guestfs(3)/ENCRYPTED DISKS> for more
+information on network-bound disk encryption (NBDE).
+
+Note that if any such option is present on the command line, QEMU user
+networking will be automatically enabled for the libguestfs appliance.
+
 =back
diff --git a/common/options/keys.c b/common/options/keys.c
index d27a7123..d987ae56 100644
--- a/common/options/keys.c
+++ b/common/options/keys.c
@@ -125,11 +125,12 @@ read_first_line_from_file (const char *filename)
  * keystore.  There may be multiple.  If none are read from the
  * keystore, ask the user.
  */
-char **
-get_keys (struct key_store *ks, const char *device, const char *uuid)
+struct matching_key *
+get_keys (struct key_store *ks, const char *device, const char *uuid,
+          size_t *nr_matches)
 {
-  size_t i, j, nmemb;
-  char **r;
+  size_t i, nmemb;
+  struct matching_key *r, *match;
   char *s;
 
   /* We know the returned list must have at least one element and not
@@ -139,22 +140,20 @@ get_keys (struct key_store *ks, const char *device, const char *uuid)
   if (ks && ks->nr_keys > nmemb)
     nmemb = ks->nr_keys;
 
-  /* make room for the terminating NULL */
-  if (nmemb == (size_t)-1)
+  if (nmemb > (size_t)-1 / sizeof *r)
     error (EXIT_FAILURE, 0, _("size_t overflow"));
-  nmemb++;
 
-  r = calloc (nmemb, sizeof (char *));
+  r = malloc (nmemb * sizeof *r);
   if (r == NULL)
-    error (EXIT_FAILURE, errno, "calloc");
+    error (EXIT_FAILURE, errno, "malloc");
 
-  j = 0;
+  match = r;
 
   if (ks) {
     for (i = 0; i < ks->nr_keys; ++i) {
       struct key_store_key *key = &ks->keys[i];
 
-      if (STRNEQ (key->id, device) && (uuid && STRNEQ (key->id, uuid)))
+      if (STRNEQ (key->id, device) && (!uuid || STRNEQ (key->id, uuid)))
         continue;
 
       switch (key->type) {
@@ -162,68 +161,101 @@ get_keys (struct key_store *ks, const char *device, const char *uuid)
         s = strdup (key->string.s);
         if (!s)
           error (EXIT_FAILURE, errno, "strdup");
-        r[j++] = s;
+        match->clevis = false;
+        match->passphrase = s;
+        ++match;
         break;
       case key_file:
         s = read_first_line_from_file (key->file.name);
-        r[j++] = s;
+        match->clevis = false;
+        match->passphrase = s;
+        ++match;
+        break;
+      case key_clevis:
+        match->clevis = true;
+        match->passphrase = NULL;
+        ++match;
         break;
       }
     }
   }
 
-  if (j == 0) {
+  if (match == r) {
     /* Key not found in the key store, ask the user for it. */
     s = read_key (device);
     if (!s)
       error (EXIT_FAILURE, 0, _("could not read key from user"));
-    r[0] = s;
+    match->clevis = false;
+    match->passphrase = s;
+    ++match;
   }
 
+  *nr_matches = (size_t)(match - r);
   return r;
 }
 
+void
+free_keys (struct matching_key *keys, size_t nr_matches)
+{
+  size_t i;
+
+  for (i = 0; i < nr_matches; ++i) {
+    struct matching_key *key = keys + i;
+
+    assert (key->clevis == (key->passphrase == NULL));
+    if (!key->clevis)
+      free (key->passphrase);
+  }
+  free (keys);
+}
+
 struct key_store *
 key_store_add_from_selector (struct key_store *ks, const char *selector)
 {
-  CLEANUP_FREE_STRING_LIST char **fields =
-    guestfs_int_split_string (':', selector);
+  CLEANUP_FREE_STRING_LIST char **fields = NULL;
+  size_t field_count;
   struct key_store_key key;
 
+  fields = guestfs_int_split_string (':', selector);
   if (!fields)
     error (EXIT_FAILURE, errno, "guestfs_int_split_string");
+  field_count = guestfs_int_count_strings (fields);
 
-  if (guestfs_int_count_strings (fields) != 3) {
-   invalid_selector:
-    error (EXIT_FAILURE, 0, "invalid selector for --key: %s", selector);
-  }
-
-  /* 1: device */
+  /* field#0: ID */
+  if (field_count < 1)
+    error (EXIT_FAILURE, 0, _("selector '%s': missing ID"), selector);
   key.id = strdup (fields[0]);
   if (!key.id)
     error (EXIT_FAILURE, errno, "strdup");
 
-  /* 2: key type */
-  if (STREQ (fields[1], "key"))
+  /* field#1...: TYPE, and TYPE-specific properties */
+  if (field_count < 2)
+    error (EXIT_FAILURE, 0, _("selector '%s': missing TYPE"), selector);
+
+  if (STREQ (fields[1], "key")) {
     key.type = key_string;
-  else if (STREQ (fields[1], "file"))
-    key.type = key_file;
-  else
-    goto invalid_selector;
-
-  /* 3: actual key */
-  switch (key.type) {
-  case key_string:
+    if (field_count != 3)
+      error (EXIT_FAILURE, 0,
+             _("selector '%s': missing KEY_STRING, or too many fields"),
+             selector);
     key.string.s = strdup (fields[2]);
     if (!key.string.s)
       error (EXIT_FAILURE, errno, "strdup");
-    break;
-  case key_file:
+  } else if (STREQ (fields[1], "file")) {
+    key.type = key_file;
+    if (field_count != 3)
+      error (EXIT_FAILURE, 0,
+             _("selector '%s': missing FILENAME, or too many fields"),
+             selector);
     key.file.name = strdup (fields[2]);
     if (!key.file.name)
       error (EXIT_FAILURE, errno, "strdup");
-    break;
-  }
+  } else if (STREQ (fields[1], "clevis")) {
+    key.type = key_clevis;
+    if (field_count != 2)
+      error (EXIT_FAILURE, 0, _("selector '%s': too many fields"), selector);
+  } else
+    error (EXIT_FAILURE, 0, _("selector '%s': invalid TYPE"), selector);
 
   return key_store_import_key (ks, &key);
 }
@@ -252,6 +284,21 @@ key_store_import_key (struct key_store *ks, const struct key_store_key *key)
   return ks;
 }
 
+bool
+key_store_requires_network (const struct key_store *ks)
+{
+  size_t i;
+
+  if (ks == NULL)
+    return false;
+
+  for (i = 0; i < ks->nr_keys; ++i)
+    if (ks->keys[i].type == key_clevis)
+      return true;
+
+  return false;
+}
+
 void
 free_key_store (struct key_store *ks)
 {
@@ -270,6 +317,9 @@ free_key_store (struct key_store *ks)
     case key_file:
       free (key->file.name);
       break;
+    case key_clevis:
+      /* nothing */
+      break;
     }
     free (key->id);
   }
diff --git a/common/options/options.h b/common/options/options.h
index 80df91a8..60d5d806 100644
--- a/common/options/options.h
+++ b/common/options/options.h
@@ -115,6 +115,7 @@ struct key_store_key {
   enum {
     key_string,             /* key specified as string */
     key_file,               /* key stored in a file */
+    key_clevis,             /* key reconstructed with Clevis+Tang */
   } type;
   union {
     struct {
@@ -134,6 +135,19 @@ struct key_store {
   size_t nr_keys;
 };
 
+/* A key matching a particular ID (pathname of the libguestfs device node that
+ * stands for the encrypted block device, or LUKS UUID).
+ */
+struct matching_key {
+  /* True iff the passphrase should be reconstructed using Clevis, talking to
+   * Tang servers over the network.
+   */
+  bool clevis;
+
+  /* Explicit passphrase, otherwise. */
+  char *passphrase;
+};
+
 /* in config.c */
 extern void parse_config (void);
 
@@ -151,9 +165,12 @@ extern void print_inspect_prompt (void);
 
 /* in key.c */
 extern char *read_key (const char *param);
-extern char **get_keys (struct key_store *ks, const char *device, const char *uuid);
+extern struct matching_key *get_keys (struct key_store *ks, const char *device,
+                                      const char *uuid, size_t *nr_matches);
+extern void free_keys (struct matching_key *keys, size_t nr_matches);
 extern struct key_store *key_store_add_from_selector (struct key_store *ks, const char *selector);
 extern struct key_store *key_store_import_key (struct key_store *ks, const struct key_store_key *key);
+extern bool key_store_requires_network (const struct key_store *ks);
 extern void free_key_store (struct key_store *ks);
 
 /* in options.c */
diff --git a/convert/convert.ml b/convert/convert.ml
index 5e0e6c2b..b678dc92 100644
--- a/convert/convert.ml
+++ b/convert/convert.ml
@@ -57,7 +57,8 @@ let rec convert dir options source =
    * sure this is not too large because each vCPU consumes guest RAM.
    *)
   g#set_smp (min 8 (Sysconf.nr_processors_online ()));
-  (* The network is only used by the unconfigure_vmware () function. *)
+  (* The network is used by the unconfigure_vmware () function, and the "--key
+   * ID:clevis" command line options (if any). *)
   g#set_network true;
   List.iter (
     fun { s_disk_id = i } ->
-- 
2.31.1