thr3ads.net - Libguestfs - [Libguestfs] [libguestfs-common PATCH 02/12] options: fix UUID comparison logic bug in get

If this information is useful, please help other people find it:
Share via:

Laszlo Ersek

2022-Jun-28 11:49 UTC

[Libguestfs] [libguestfs-common PATCH 00/12] LUKS decryption with Clevis+Tang | CVE-2022-2211

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1809453
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2100862

Please refer to the parent cover letter
<https://listman.redhat.com/archives/libguestfs/2022-June/029274.html>
regarding the relationship between the CVE fix and the larger series.

The first four patches are bugfixes (of varying importance).

The rest are refactorings and feature-let additions, intermixed as
needed.

Thanks,
Laszlo

Laszlo Ersek (12):
  options: fix buffer overflow in get_keys() [CVE-2022-2211]
  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 tpe "key_clevis"
  options: generalize "--key" selector parsing for C-language
utilities
  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

 mltools/tools_utils-c.c |  47 ++++---
 mltools/tools_utils.ml  |  51 ++++----
 mltools/tools_utils.mli |  12 +-
 options/decrypt.c       |  24 ++--
 options/key-option.pod  |   9 ++
 options/keys.c          | 130 ++++++++++++++------
 options/options.h       |  19 ++-
 7 files changed, 195 insertions(+), 97 deletions(-)

-- 
2.19.1.3.g30247aa5d201

Laszlo Ersek

2022-Jun-28 11:49 UTC

head link

[Libguestfs] [libguestfs-common PATCH 02/12] options: fix UUID comparison logic bug in get_keys()

When we iterate over the keys in the keystore, we intend to skip a key
(i.e., we do not append it to the result array) if:

  (key_id differs from device name) && (key_id differs from device UUID)

The problem is how we currently express the second condition:

  uuid && STRNEQ (key->id, uuid)

This is wrong: when the LUKS UUID is missing (e.g. because
guestfs_luks_uuid() failed or we're looking up the keys for a BitLocker
device), this expression evaluates to 0. Which states that the key_id does
*not* differ from the device UUID -- in other words, that we have a match.

Invert the result for when "uuid" is NULL:

  !uuid || STRNEQ (key->id, uuid)

(

- Equivalently: what we currently have for "differs" is:

    uuid ? STRNEQ (key->id, uuid) : false

  but we need:

    uuid ? STRNEQ (key->id, uuid) : true

- Yet another way to express

    key_id differs from device UUID

  correctly is with the "implication operator":

    uuid ? STRNEQ (key->id, uuid)

  The crucial feature is that, from a false premise, everything follows;
  therefore, if the UUID is missing, we get "true" (which we want for
  "differs"). In C, we don't have an "implication
operator", but

    A ? B

  is equivalent to

    ?A ? B

  (compare the truth tables!), which we *can* express in C, as

    !A || B

)

In practice the bug has been masked for the following reasons:

- It's pretty rare that "uuid" is NULL.

- Even when "uuid" is NULL, and therefore we incorrectly include a key
in
  the result list, the consequence is only bad performance. Namely, we
  attempt unlocking a LUKS or BitLocker volume with a key that the user
  never intended for that -- so the attempt will fail, and
  decrypt_mountables() [options/decrypt.c] will just advance to the next
  key in the result list. Put differently, we cannot "mistakenly"
unlock
  an encrypted device.

However, because unlocking disk encryption requires plenty of computation
by design (in order to resist brute forcing), this waste may not be
trivial.

Fixes: bb4a2dc17a78b53437896d4215ae82df8e11b788
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1809453
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 options/keys.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/options/keys.c b/options/keys.c
index d27a7123e67e..8713372a305e 100644
--- a/options/keys.c
+++ b/options/keys.c
@@ -152,11 +152,11 @@ get_keys (struct key_store *ks, const char *device, const
char *uuid)
 
   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) {
       case key_string:
         s = strdup (key->string.s);
-- 
2.19.1.3.g30247aa5d201

Laszlo Ersek

2022-Jun-28 11:49 UTC

head link

[Libguestfs] [libguestfs-common PATCH 03/12] mltools/tools_utils: remove unused function "key_store_to_cli"

Function "key_store_to_cli" was introduced in commit 310ffe7e507e
("mltools: Add key_store_to_cli", 2021-09-21), and put to use in
virt-v2v
commit cff4514927b3 ("v2v: Pass --key parameters through to the input
helper", 2021-09-21).

However, the function has not been in use since virt-v2v commit
724ecb5e887e ("input: Turn helper into an OCaml module", 2021-12-02);
remove it now.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1809453
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 mltools/tools_utils.mli |  8 --------
 mltools/tools_utils.ml  | 10 ----------
 2 files changed, 18 deletions(-)

diff --git a/mltools/tools_utils.mli b/mltools/tools_utils.mli
index 501830065038..8d9af7a0a661 100644
--- a/mltools/tools_utils.mli
+++ b/mltools/tools_utils.mli
@@ -101,18 +101,10 @@ val create_standard_options : Getopt.speclist ->
?anon_fun:Getopt.anon_fun -> ?k
     which allows another tool to run this tool and change the program
     name used in error messages.
 
     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.
 
     [echo_cmd] specifies whether to output the full command on verbose
     mode, and it's on by default. *)
diff --git a/mltools/tools_utils.ml b/mltools/tools_utils.ml
index 695fda7e548c..8508534e16ee 100644
--- a/mltools/tools_utils.ml
+++ b/mltools/tools_utils.ml
@@ -418,20 +418,10 @@ let create_standard_options argspec ?anon_fun ?(key_opts =
false)
   let argspec = !argspec in
 
   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
     debug "%s" cmd;
   let chan = Unix.open_process_in cmd in
-- 
2.19.1.3.g30247aa5d201

Laszlo Ersek

2022-Jun-28 11:49 UTC

head link

[Libguestfs] [libguestfs-common PATCH 04/12] mltools/tools_utils: allow multiple "--key" options for OCaml tools too

Commit c10c8baedb88 ("options: Allow multiple --key parameters.",
2019-11-28) enabled multi-key support for the C-language tools only.
C-language tools parse the "--key" option as follows:

  OPTION_key                      [options/options.h]
    key_store_add_from_selector() [options/keys.c]
      key_store_import_key()      [options/keys.c]

And the last function above simply appends the imported key to the
keystore.

However, commit c10c8baedb88 was ineffective (already at the time of
writing) for OCaml-language tools. From commit f84d95474ccf ("Introduce a
--key option in tools that accept keys", 2018-09-21), OCaml tools first
de-duplicate the "--key" options (using the device identifier as a
hash
table key), and only (distinct) elements of the flattened hash table are
passed to key_store_import_key():

  parse_key_selector                    [mltools/tools_utils.ml]
    Hashtbl.replace

  inspect_decrypt                       [mltools/tools_utils.ml]
    Hashtbl.fold
    c_inspect_decrypt                   [mltools/tools_utils.ml]
      guestfs_int_mllib_inspect_decrypt [mltools/tools_utils-c.c]
        key_store_import_key()          [options/keys.c]

This problem can be demonstrated by passing two keys, a good one and a
wrong one, for the same device identifier, to a C-language guestfs tool
(such as virt-cat), and to an OCaml-language one (such as
virt-get-kernel). The latter is sensitive to the order of "--key"
options:

$ virt-cat \
    --key /dev/sda2:key:good-key \
    --key /dev/sda2:key:wrong-key \
    -d DOMAIN \
    /no-such-file> libguestfs: error: download: /no-such-file: No such file or directory
Here the wrong key (passed as the 2nd "--key" option) does not
invalidate
the first (good) key.

$ virt-get-kernel \
    --key /dev/sda2:key:good-key \
    --key /dev/sda2:key:wrong-key \
    -d DOMAIN \
    -o /tmp> virt-get-kernel: could not find key to open LUKS encrypted /dev/sda2.
>
> Try using --key on the command line.
>
> Original error: cryptsetup_open: cryptsetup exited with status 2: No key
> available with this passphrase. (0)
Here the wrong key replaces the good key.

$ virt-get-kernel \
    --key /dev/sda2:key:wrong-key \
    --key /dev/sda2:key:good-key \
    -d DOMAIN \
    -o /tmp> download: /boot/vmlinuz-[...] -> /tmp/vmlinuz-[...]
> download: /boot/initramfs-[...].img -> /tmp/initramfs-[...].img
Reversing the order of "--key" options for the OCaml-language tool
allows
the good key to prevail (and the wrong to be overwritten).

Fix this discrepancy by replacing the hash table with a plain list
(reference).

(Side comment: the hash table-based deduplication didn't do its job
entirely, either. We could still pass two keys for the same LUKS block
device: once by pathname, and another time by LUKS UUID.)

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1809453
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 mltools/tools_utils.ml | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/mltools/tools_utils.ml b/mltools/tools_utils.ml
index 8508534e16ee..6006ab7e4f6c 100644
--- a/mltools/tools_utils.ml
+++ b/mltools/tools_utils.ml
@@ -27,11 +27,11 @@ open Getopt.OptionName
  * messages.
  *)
 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
 
@@ -374,11 +374,11 @@ let create_standard_options argspec ?anon_fun ?(key_opts =
false)
       | n ->
         error (f_"invalid output for --machine-readable: %s") fmt
       )
   in
   let ks = {
-    keys = Hashtbl.create 13;
+    keys = ref [];
   } in
   let argspec = ref argspec in
   let add_argspec = List.push_back argspec in
 
   add_argspec ([ S 'V'; L"version" ], Getopt.Unit
print_version_and_exit, s_"Display version and exit");
@@ -393,13 +393,13 @@ 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
       match parts with
       | [ device; "key"; key ] ->
-         Hashtbl.replace ks.keys device (KeyString key)
+         List.push_back ks.keys (device, KeyString key)
       | [ device; "file"; file ] ->
-         Hashtbl.replace ks.keys device (KeyFileName file)
+         List.push_back ks.keys (device, KeyFileName file)
       | _ ->
          error (f_"invalid selector string for --key: %s") arg
     in
 
     add_argspec ([ L"echo-keys" ],       Getopt.Unit c_set_echo_keys,
s_"Don?t turn off echo for passphrases");
@@ -680,24 +680,17 @@ let is_btrfs_subvolume g fs    with Guestfs.Error msg as
exn ->
     if g#last_errno () = Guestfs.Errno.errno_EINVAL then false
     else raise exn
 
 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
   let rec loop ()      if Unix.gettimeofday () -. start_t > float_of_int
timeout then
-- 
2.19.1.3.g30247aa5d201

Laszlo Ersek

2022-Jun-28 11:49 UTC

head link

[Libguestfs] [libguestfs-common PATCH 05/12] options: replace NULL-termination with number-of-elements in get_keys()

Currently, get_keys() returns a NULL-terminated array of (char*) elements.
In a later patch in this series, we'll want to change the element type to
a structure type, at which point the NULL-termination would become
awkward. Replace the NULL-termination scheme with the number-of-elements
scheme. No behavioral changes.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1809453
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 options/options.h |  4 +++-
 options/decrypt.c | 17 +++++++-------
 options/keys.c    | 24 ++++++++++++++------
 3 files changed, 29 insertions(+), 16 deletions(-)

diff --git a/options/options.h b/options/options.h
index 80df91a85f98..d7a3aeff6f41 100644
--- a/options/options.h
+++ b/options/options.h
@@ -149,11 +149,13 @@ extern void inspect_mount_root (guestfs_h *g, const char
*root);
 #define inspect_mount() inspect_mount_handle (g, ks)
 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 char **get_keys (struct key_store *ks, const char *device, const char
*uuid,
+                        size_t *nr_matches);
+extern void free_keys (char **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 void free_key_store (struct key_store *ks);
 
 /* in options.c */
diff --git a/options/decrypt.c b/options/decrypt.c
index 1cd7b627e264..c25b5888d8b4 100644
--- a/options/decrypt.c
+++ b/options/decrypt.c
@@ -122,14 +122,14 @@ decrypt_mountables (guestfs_h *g, const char * const
*mountables,
   const char *mountable;
 
   while ((mountable = *mnt_scan++) != NULL) {
     CLEANUP_FREE char *type = NULL;
     CLEANUP_FREE char *uuid = NULL;
-    CLEANUP_FREE_STRING_LIST char **keys = NULL;
+    char **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)
       continue;
 
@@ -142,37 +142,38 @@ decrypt_mountables (guestfs_h *g, const char * const
*mountables,
       continue;
 
     /* 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) {
+      const char *key = keys[scan];
       int r;
 
       guestfs_push_error_handler (g, NULL, NULL);
       r = guestfs_cryptsetup_open (g, mountable, key, 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;
   }
 
   return decrypted_some;
 }
diff --git a/options/keys.c b/options/keys.c
index 8713372a305e..1d97d980a460 100644
--- a/options/keys.c
+++ b/options/keys.c
@@ -124,11 +124,12 @@ read_first_line_from_file (const char *filename)
 /* Return the key(s) matching this particular device from the
  * 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)
+get_keys (struct key_store *ks, const char *device, const char *uuid,
+          size_t *nr_matches)
 {
   size_t i, j, nmemb;
   char **r;
   char *s;
 
@@ -137,18 +138,16 @@ get_keys (struct key_store *ks, const char *device, const
char *uuid)
    */
   nmemb = 1;
   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;
 
   if (ks) {
     for (i = 0; i < ks->nr_keys; ++i) {
@@ -175,16 +174,27 @@ get_keys (struct key_store *ks, const char *device, const
char *uuid)
   if (j == 0) {
     /* 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;
+    r[j++] = s;
   }
 
+  *nr_matches = j;
   return r;
 }
 
+void
+free_keys (char **keys, size_t nr_matches)
+{
+  size_t i;
+
+  for (i = 0; i < nr_matches; ++i)
+    free (keys[i]);
+  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);
-- 
2.19.1.3.g30247aa5d201

Laszlo Ersek

2022-Jun-28 11:49 UTC

head link

[Libguestfs] [libguestfs-common PATCH 06/12] options: wrap each passphrase from get_keys() into a struct

For adding Clevis support later in this patch set, replace the (char *)
element type of the get_keys() result array with a new structure: "struct
matching_key". No behavioral changes.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1809453
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 options/options.h | 13 +++++++--
 options/decrypt.c |  6 ++--
 options/keys.c    | 30 ++++++++++++--------
 3 files changed, 31 insertions(+), 18 deletions(-)

diff --git a/options/options.h b/options/options.h
index d7a3aeff6f41..9bd812525d8a 100644
--- a/options/options.h
+++ b/options/options.h
@@ -132,10 +132,17 @@ struct key_store_key {
 struct key_store {
   struct key_store_key *keys;
   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 {
+  char *passphrase;
+};
+
 /* in config.c */
 extern void parse_config (void);
 
 /* in decrypt.c */
 extern void inspect_do_decrypt (guestfs_h *g, struct key_store *ks);
@@ -149,13 +156,13 @@ extern void inspect_mount_root (guestfs_h *g, const char
*root);
 #define inspect_mount() inspect_mount_handle (g, ks)
 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,
-                        size_t *nr_matches);
-extern void free_keys (char **keys, size_t nr_matches);
+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 void free_key_store (struct key_store *ks);
 
 /* in options.c */
diff --git a/options/decrypt.c b/options/decrypt.c
index c25b5888d8b4..421a38c2a11f 100644
--- a/options/decrypt.c
+++ b/options/decrypt.c
@@ -122,11 +122,11 @@ decrypt_mountables (guestfs_h *g, const char * const
*mountables,
   const char *mountable;
 
   while ((mountable = *mnt_scan++) != NULL) {
     CLEANUP_FREE char *type = NULL;
     CLEANUP_FREE char *uuid = NULL;
-    char **keys;
+    struct matching_key *keys;
     size_t nr_matches;
     CLEANUP_FREE char *mapname = NULL;
     size_t scan;
 
     type = guestfs_vfs_type (g, mountable);
@@ -151,15 +151,15 @@ decrypt_mountables (guestfs_h *g, const char * const
*mountables,
     if (uuid == NULL || asprintf (&mapname, "luks-%s", uuid) ==
-1)
       mapname = make_mapname (mountable);
 
     /* Try each key in turn. */
     for (scan = 0; scan < nr_matches; ++scan) {
-      const char *key = keys[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);
+      r = guestfs_cryptsetup_open (g, mountable, key->passphrase, mapname,
-1);
       guestfs_pop_error_handler (g);
 
       if (r == 0)
         break;
     }
diff --git a/options/keys.c b/options/keys.c
index 1d97d980a460..56fca17a94b5 100644
--- a/options/keys.c
+++ b/options/keys.c
@@ -123,16 +123,16 @@ read_first_line_from_file (const char *filename)
 
 /* Return the key(s) matching this particular device from the
  * keystore.  There may be multiple.  If none are read from the
  * keystore, ask the user.
  */
-char **
+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
    * more than ks->nr_keys.
    */
@@ -145,11 +145,11 @@ get_keys (struct key_store *ks, const char *device, const
char *uuid,
 
   r = malloc (nmemb * sizeof *r);
   if (r == NULL)
     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];
 
@@ -159,39 +159,45 @@ get_keys (struct key_store *ks, const char *device, const
char *uuid,
       switch (key->type) {
       case key_string:
         s = strdup (key->string.s);
         if (!s)
           error (EXIT_FAILURE, errno, "strdup");
-        r[j++] = s;
+        match->passphrase = s;
+        ++match;
         break;
       case key_file:
         s = read_first_line_from_file (key->file.name);
-        r[j++] = s;
+        match->passphrase = s;
+        ++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[j++] = s;
+    match->passphrase = s;
+    ++match;
   }
 
-  *nr_matches = j;
+  *nr_matches = (size_t)(match - r);
   return r;
 }
 
 void
-free_keys (char **keys, size_t nr_matches)
+free_keys (struct matching_key *keys, size_t nr_matches)
 {
   size_t i;
 
-  for (i = 0; i < nr_matches; ++i)
-    free (keys[i]);
+  for (i = 0; i < nr_matches; ++i) {
+    struct matching_key *key = keys + i;
+
+    free (key->passphrase);
+  }
   free (keys);
 }
 
 struct key_store *
 key_store_add_from_selector (struct key_store *ks, const char *selector)
-- 
2.19.1.3.g30247aa5d201

Laszlo Ersek

2022-Jun-28 11:49 UTC

head link

[Libguestfs] [libguestfs-common PATCH 07/12] options: add back-end for LUKS decryption with Clevis+Tang

Extend the "matching_key" structure with a new boolean field,
"clevis".
"clevis" is mutually exclusive with a non-NULL "passphrase"
field. If
"clevis" is set, open the LUKS device with
guestfs_clevis_luks_unlock() --
which requires no explicit passphrase --; otherwise, continue calling
guestfs_cryptsetup_open().

This patch introduces no change in observable behavior; there is no user
interface yet for setting the "clevis" field to "true".

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1809453
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---

Notes:
    This patch introduces a call to guestfs_clevis_luks_unlock(), which is a
    new libguestfs API (introduced in the sibling libguestfs series).
    
    Assuming we still care about building guestfs-tools and virt-v2v against
    an independent libguestfs (library and appliance), we need to do one of
    the following things:
    
    (1) Make the call dependent on the GUESTFS_HAVE_CLEVIS_LUKS_UNLOCK
    feature test macro. If it is missing, the library is too old, just set
    "r = -1", and (possibly) print a warning to stderr.
    
    (2) Cut a new libguestfs release, and bump the minimum version in
    "m4/guestfs-libraries.m4" (in both guestfs-tools and virt-v2v) to
the
    new version.
    
    Both of these have drawbacks.
    
    Disadvantages of (1):
    
    - Printing a raw warning to stderr is OK for the C-language tools, but
      the code is used by OCaml tools as well, which have pretty-printed
      warnings.
    
    - Simply skipping the guestfs_clevis_luks_unlock() call -- i.e.,
      pretending it fails -- is not user-friendly. In particular, once we
      add the new selector type for "--key" later in this series, and
      document it in "options/key-option.pod", all the tools' docs
will pick
      it up at once, even if the library is too old to provide the
      interface.
    
    Disadvantages of (2):
    
    - We may not want to leap over a large version distance in
      "m4/guestfs-libraries.m4" (in both guestfs-tools and virt-v2v),
for
      whatever reason.
    
    - Even if we make the guestfs_clevis_luks_unlock() API mandatory in the
      library, the daemon may not implement it -- it's ultimately appliance
      (host distro) dependent, which is why the API belongs to a new option
      group called "clevisluks". That is, the actual behavior of
      guestfs_clevis_luks_unlock() may still differ from what the user
      expects based on "options/key-option.pod".
    
      This drawback is mitigated however: the documentation of the new
      selector in "options/key-option.pod" references "ENCRYPTED
DISKS" in
      guestfs(3), which in turn references
"guestfs_clevis_luks_unlock",
      which does highlight the "clevisluks" option group.
    
    I vote (2) -- cut a new libguestfs release, then bump the
    "m4/guestfs-libraries.m4" requirements. I'm not doing that
just yet in
    those projects (in the sibling series here): if I did that, those series
    would not compile; the libguestfs version number would have to be
    increased first! And I don't know if we want *something else* in the new
    libguestfs release, additionally.

 options/options.h | 6 ++++++
 options/decrypt.c | 7 ++++++-
 options/keys.c    | 7 ++++++-
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/options/options.h b/options/options.h
index 9bd812525d8a..61a385da13ae 100644
--- a/options/options.h
+++ b/options/options.h
@@ -136,10 +136,16 @@ struct key_store {
 
 /* 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);
diff --git a/options/decrypt.c b/options/decrypt.c
index 421a38c2a11f..820fbc5629cd 100644
--- a/options/decrypt.c
+++ b/options/decrypt.c
@@ -155,11 +155,16 @@ decrypt_mountables (guestfs_h *g, const char * const
*mountables,
     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->passphrase, mapname,
-1);
+      assert (key->clevis == (key->passphrase == NULL));
+      if (key->clevis)
+        r = guestfs_clevis_luks_unlock (g, mountable, mapname);
+      else
+        r = guestfs_cryptsetup_open (g, mountable, key->passphrase, mapname,
+                                     -1);
       guestfs_pop_error_handler (g);
 
       if (r == 0)
         break;
     }
diff --git a/options/keys.c b/options/keys.c
index 56fca17a94b5..75c659561c52 100644
--- a/options/keys.c
+++ b/options/keys.c
@@ -159,15 +159,17 @@ get_keys (struct key_store *ks, const char *device, const
char *uuid,
       switch (key->type) {
       case key_string:
         s = strdup (key->string.s);
         if (!s)
           error (EXIT_FAILURE, errno, "strdup");
+        match->clevis = false;
         match->passphrase = s;
         ++match;
         break;
       case key_file:
         s = read_first_line_from_file (key->file.name);
+        match->clevis = false;
         match->passphrase = s;
         ++match;
         break;
       }
     }
@@ -176,10 +178,11 @@ get_keys (struct key_store *ks, const char *device, const
char *uuid,
   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"));
+    match->clevis = false;
     match->passphrase = s;
     ++match;
   }
 
   *nr_matches = (size_t)(match - r);
@@ -192,11 +195,13 @@ 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;
 
-    free (key->passphrase);
+    assert (key->clevis == (key->passphrase == NULL));
+    if (!key->clevis)
+      free (key->passphrase);
   }
   free (keys);
 }
 
 struct key_store *
-- 
2.19.1.3.g30247aa5d201

Laszlo Ersek

2022-Jun-28 11:49 UTC

head link

[Libguestfs] [libguestfs-common PATCH 08/12] options: introduce selector tpe "key_clevis"

From an earlier patch in this series, we can now represent LUKS decryption
with Clevis+Tang in those matching keys that we distill for a particular
guestfs device or LUKS UUID. Now extend the keystore (composed from
command line options), i.e. the store that is filtered into matching keys,
with a selector type that stands for Clevis+Tang.

Again, this patch introduces no change in observable behavior; there is
still no user interface for placing a selector of the new type into the
keystore.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1809453
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 options/options.h | 1 +
 options/keys.c    | 8 ++++++++
 2 files changed, 9 insertions(+)

diff --git a/options/options.h b/options/options.h
index 61a385da13ae..e7a0364cc926 100644
--- a/options/options.h
+++ b/options/options.h
@@ -113,10 +113,11 @@ struct key_store_key {
   char *id;
 
   enum {
     key_string,             /* key specified as string */
     key_file,               /* key stored in a file */
+    key_clevis,             /* key reconstructed with Clevis+Tang */
   } type;
   union {
     struct {
       char *s;              /* string of the key */
     } string;
diff --git a/options/keys.c b/options/keys.c
index 75c659561c52..7729fe79c99b 100644
--- a/options/keys.c
+++ b/options/keys.c
@@ -169,10 +169,15 @@ get_keys (struct key_store *ks, const char *device, const
char *uuid,
         s = read_first_line_from_file (key->file.name);
         match->clevis = false;
         match->passphrase = s;
         ++match;
         break;
+      case key_clevis:
+        match->clevis = true;
+        match->passphrase = NULL;
+        ++match;
+        break;
       }
     }
   }
 
   if (match == r) {
@@ -289,9 +294,12 @@ free_key_store (struct key_store *ks)
       free (key->string.s);
       break;
     case key_file:
       free (key->file.name);
       break;
+    case key_clevis:
+      /* nothing */
+      break;
     }
     free (key->id);
   }
 }
-- 
2.19.1.3.g30247aa5d201

Laszlo Ersek

2022-Jun-28 11:49 UTC

head link

[Libguestfs] [libguestfs-common PATCH 09/12] options: generalize "--key" selector parsing for C-language utilities

The key_store_add_from_selector() function parses the argument of the
"--key" option in C-language tools:

  OPTION_key                      [options/options.h]
    key_store_add_from_selector() [options/keys.c]
      key_store_import_key()      [options/keys.c]

The function currently (a) uses a horrible "goto", (b) is not flexible
enough for selector types that do not take precisely 1 type-specific
parameter. Rewrite the function with more informative error messages and
an easier to follow structure, plus make the type-specific parameter count
a function of the type.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1809453
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 options/keys.c | 46 ++++++++++----------
 1 file changed, 24 insertions(+), 22 deletions(-)

diff --git a/options/keys.c b/options/keys.c
index 7729fe79c99b..a6ef2d78b589 100644
--- a/options/keys.c
+++ b/options/keys.c
@@ -210,48 +210,50 @@ free_keys (struct matching_key *keys, size_t nr_matches)
 }
 
 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;
+  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
+    error (EXIT_FAILURE, 0, _("selector '%s': invalid TYPE"),
selector);
 
   return key_store_import_key (ks, &key);
 }
 
 struct key_store *
-- 
2.19.1.3.g30247aa5d201

Laszlo Ersek

2022-Jun-28 11:49 UTC

head link

[Libguestfs] [libguestfs-common PATCH 10/12] mltools/tools_utils: generalize "--key" selector parsing for OCaml utils

In another patch in this series, we generalize the "--key" selector
parsing for C-language utilities. Adapt the OCaml-langauge "--key"
parser:

- Incorporate the new (more informative) error messages for consistency.

- Prepare for selector types that do not take any type-specific
  parameters. These will be represented with constant constructors of the
  "key_store_key" type, and such values are not blocks, but unboxed
  integers:
  <https://v2.ocaml.org/manual/intfc.html#ss:c-concrete-datatypes>.

(This patch is best shown with "git show -b" for review.)

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1809453
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 mltools/tools_utils.ml  | 14 ++++++-
 mltools/tools_utils-c.c | 44 ++++++++++++--------
 2 files changed, 38 insertions(+), 20 deletions(-)

diff --git a/mltools/tools_utils.ml b/mltools/tools_utils.ml
index 6006ab7e4f6c..e534cbead47a 100644
--- a/mltools/tools_utils.ml
+++ b/mltools/tools_utils.ml
@@ -390,18 +390,28 @@ let create_standard_options argspec ?anon_fun ?(key_opts =
false)
                  L"colour"; L"colours" ], Getopt.Unit
set_colours, s_"Use ANSI colour sequences even if not tty");
   add_argspec ([ L"wrap" ],           Getopt.Unit set_wrap,    
s_"Wrap log messages even if not tty");
 
   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 ] ->
          List.push_back ks.keys (device, KeyString key)
+      | [ _; "file" ]
+      |  _ :: "file" :: _ :: _ :: _ ->
+        error (f_"selector '%s': missing FILENAME, or too many
fields") arg
       | [ device; "file"; file ] ->
          List.push_back ks.keys (device, KeyFileName file)
       | _ ->
-         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");
     add_argspec ([ L"keys-from-stdin" ], Getopt.Unit
c_set_keys_from_stdin, s_"Read passphrases from stdin");
     add_argspec ([ L"key" ], Getopt.String (s_"SELECTOR",
parse_key_selector), s_"Specify a LUKS key");
diff --git a/mltools/tools_utils-c.c b/mltools/tools_utils-c.c
index 081466776666..e9f273ec857f 100644
--- a/mltools/tools_utils-c.c
+++ b/mltools/tools_utils-c.c
@@ -60,28 +60,36 @@ guestfs_int_mllib_inspect_decrypt (value gv, value gpv,
value keysv)
     key.id = strdup (String_val (Field (elemv, 0)));
     if (!key.id)
       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:
+        error (EXIT_FAILURE, 0,
+               "internal error: unhandled Tag_val (v) = %d",
+               Tag_val (v));
+      }
+    else
+      switch (Int_val (v)) {
+      default:
+        error (EXIT_FAILURE, 0,
+               "internal error: unhandled Int_val (v) = %d",
+               Int_val (v));
+      }
 
     ks = key_store_import_key (ks, &key);
 
     keysv = Field (keysv, 1);
   }
-- 
2.19.1.3.g30247aa5d201

Laszlo Ersek

2022-Jun-28 11:49 UTC

head link

[Libguestfs] [libguestfs-common PATCH 11/12] options, mltools/tools_utils: parse "--key ID:clevis" options

Provide the user interface (in both the C and the OCaml tools) for
selecting network-based, passphrase-less decryption. This is the front-end
exposing the previously added back-end.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1809453
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 mltools/tools_utils.ml  | 5 +++++
 mltools/tools_utils-c.c | 3 +++
 options/keys.c          | 4 ++++
 options/key-option.pod  | 6 ++++++
 4 files changed, 18 insertions(+)

diff --git a/mltools/tools_utils.ml b/mltools/tools_utils.ml
index e534cbead47a..1da5850340d4 100644
--- a/mltools/tools_utils.ml
+++ b/mltools/tools_utils.ml
@@ -32,10 +32,11 @@ type key_store = {
   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]
 external c_set_keys_from_stdin : unit -> unit =
"guestfs_int_mllib_set_keys_from_stdin" [@@noalloc]
 external c_rfc3339_date_time_string : unit -> string =
"guestfs_int_mllib_rfc3339_date_time_string"
@@ -406,10 +407,14 @@ let create_standard_options argspec ?anon_fun ?(key_opts =
false)
       | [ _; "file" ]
       |  _ :: "file" :: _ :: _ :: _ ->
         error (f_"selector '%s': missing FILENAME, or too many
fields") arg
       | [ device; "file"; 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_"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");
diff --git a/mltools/tools_utils-c.c b/mltools/tools_utils-c.c
index e9f273ec857f..f429d7708772 100644
--- a/mltools/tools_utils-c.c
+++ b/mltools/tools_utils-c.c
@@ -81,10 +81,13 @@ guestfs_int_mllib_inspect_decrypt (value gv, value gpv,
value keysv)
                "internal error: unhandled Tag_val (v) = %d",
                Tag_val (v));
       }
     else
       switch (Int_val (v)) {
+      case 0:  /* KeyClevis */
+        key.type = key_clevis;
+        break;
       default:
         error (EXIT_FAILURE, 0,
                "internal error: unhandled Int_val (v) = %d",
                Int_val (v));
       }
diff --git a/options/keys.c b/options/keys.c
index a6ef2d78b589..d53e3e774a9b 100644
--- a/options/keys.c
+++ b/options/keys.c
@@ -248,10 +248,14 @@ key_store_add_from_selector (struct key_store *ks, const
char *selector)
              _("selector '%s': missing FILENAME, or too many
fields"),
              selector);
     key.file.name = strdup (fields[2]);
     if (!key.file.name)
       error (EXIT_FAILURE, errno, "strdup");
+  } 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);
 }
diff --git a/options/key-option.pod b/options/key-option.pod
index 90a3b15c57a2..34229ce9cbb2 100644
--- a/options/key-option.pod
+++ b/options/key-option.pod
@@ -12,6 +12,12 @@ Use the specified C<KEY_STRING> as passphrase.
 
 =item B<--key> C<ID>:file:FILENAME
 
 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).
+
 =back
-- 
2.19.1.3.g30247aa5d201

Laszlo Ersek

2022-Jun-28 11:49 UTC

head link

[Libguestfs] [libguestfs-common PATCH 12/12] options, mltools/tools_utils: add helper for network dependency

For "--key ID:clevis" to actually work, the appliance needs network
connectivity. Some tools allow the user to control that (for example,
guestfish takes "--network"); those tools can already use "--key
ID:clevis", provided they enable inspection and networking.

However, other tools have had no use for networking thus far. Introduce a
helper function for the OCaml utils, and another for the C utils, so that
the utils can query if there's at least one clevis selector, and if so,
enable networking for the appliance, before launching the appliance.

Note that we need to implement both helpers separately from each other;
that is, the OCaml one is not based on the C one. The reason is that
"struct key_store" (from "options/options.h"), upon which we
could
logically base a common helper, exists in the OCaml tools only
ephemerally, in guestfs_int_mllib_inspect_decrypt():

  inspect_decrypt                       [mltools/tools_utils.ml]
    c_inspect_decrypt                   [mltools/tools_utils.ml]
      guestfs_int_mllib_inspect_decrypt [mltools/tools_utils-c.c]
        key_store_import_key()          [options/keys.c]
        inspect_do_decrypt()            [options/decrypt.c]
        free_key_store()                [options/keys.c]

At that time, it's too late for enabling networking for the appliance.
Therefore, in the OCaml tools, search the "earlier" data structure
called
"cmdline_options.ks". *That* "key_store" type comes from
"mltools/tools_utils.ml".

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1809453
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 options/options.h       |  1 +
 mltools/tools_utils.mli |  4 ++++
 mltools/tools_utils.ml  |  5 +++++
 options/keys.c          | 15 +++++++++++++++
 options/key-option.pod  |  3 +++
 5 files changed, 28 insertions(+)

diff --git a/options/options.h b/options/options.h
index e7a0364cc926..60d5d8064113 100644
--- a/options/options.h
+++ b/options/options.h
@@ -168,10 +168,11 @@ extern char *read_key (const char *param);
 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 */
 extern void option_a (const char *arg, const char *format, int blocksize,
struct drv **drvsp);
 extern void option_d (const char *arg, struct drv **drvsp);
diff --git a/mltools/tools_utils.mli b/mltools/tools_utils.mli
index 8d9af7a0a661..ec900e6389bc 100644
--- a/mltools/tools_utils.mli
+++ b/mltools/tools_utils.mli
@@ -194,10 +194,14 @@ val inspect_mount_root_ro : Guestfs.guestfs -> string
-> unit
     read-only. *)
 
 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. *)
 
 val with_timeout : string -> int -> ?sleep:int -> (unit -> 'a
option) -> 'a
diff --git a/mltools/tools_utils.ml b/mltools/tools_utils.ml
index 1da5850340d4..562bfadc18a8 100644
--- a/mltools/tools_utils.ml
+++ b/mltools/tools_utils.ml
@@ -694,10 +694,15 @@ let is_btrfs_subvolume g fs      ignore
(g#mountable_subvolume fs); true
   with Guestfs.Error msg as exn ->
     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    (* 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.
diff --git a/options/keys.c b/options/keys.c
index d53e3e774a9b..97da6df93ee6 100644
--- a/options/keys.c
+++ b/options/keys.c
@@ -282,10 +282,25 @@ key_store_import_key (struct key_store *ks, const struct
key_store_key *key)
   ++ks->nr_keys;
 
   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)
 {
   size_t i;
 
diff --git a/options/key-option.pod b/options/key-option.pod
index 34229ce9cbb2..6bc04df177b1 100644
--- a/options/key-option.pod
+++ b/options/key-option.pod
@@ -18,6 +18,9 @@ Read the passphrase from F<FILENAME>.
 
 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
-- 
2.19.1.3.g30247aa5d201

Libguestfs - Jun 2022 - [libguestfs-common PATCH 02/12] options: fix UUID comparison logic bug in get_keys()

[Libguestfs] [libguestfs-common PATCH 00/12] LUKS decryption with Clevis+Tang | CVE-2022-2211

[Libguestfs] [libguestfs-common PATCH 02/12] options: fix UUID comparison logic bug in get_keys()

[Libguestfs] [libguestfs-common PATCH 03/12] mltools/tools_utils: remove unused function "key_store_to_cli"

[Libguestfs] [libguestfs-common PATCH 04/12] mltools/tools_utils: allow multiple "--key" options for OCaml tools too

[Libguestfs] [libguestfs-common PATCH 05/12] options: replace NULL-termination with number-of-elements in get_keys()

[Libguestfs] [libguestfs-common PATCH 06/12] options: wrap each passphrase from get_keys() into a struct

[Libguestfs] [libguestfs-common PATCH 07/12] options: add back-end for LUKS decryption with Clevis+Tang

[Libguestfs] [libguestfs-common PATCH 08/12] options: introduce selector tpe "key_clevis"

[Libguestfs] [libguestfs-common PATCH 09/12] options: generalize "--key" selector parsing for C-language utilities

[Libguestfs] [libguestfs-common PATCH 10/12] mltools/tools_utils: generalize "--key" selector parsing for OCaml utils

[Libguestfs] [libguestfs-common PATCH 11/12] options, mltools/tools_utils: parse "--key ID:clevis" options

[Libguestfs] [libguestfs-common PATCH 12/12] options, mltools/tools_utils: add helper for network dependency