From d95394da96af41b03c9347721a177a4ad9b7f1b0 Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Fri, 1 Jul 2022 15:20:39 +0200 Subject: [PATCH] cat, log, ls, tail, diff, edit, insp.: set networking for "--key ID:clevis" Call the C-language helper key_store_requires_network() in those C utilities that understand "OPTION_key". (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 Message-Id: <20220628115702.5584-2-lersek@redhat.com> Reviewed-by: Richard W.M. Jones (cherry picked from commit 14bf833e21cd89f1273e09f4952999b8da86b6ff) --- cat/cat.c | 3 +++ cat/log.c | 3 +++ cat/ls.c | 3 +++ cat/tail.c | 3 +++ common | 2 +- diff/diff.c | 8 ++++++++ edit/edit.c | 3 +++ inspector/inspector.c | 3 +++ 8 files changed, 27 insertions(+), 1 deletion(-) diff --git a/cat/cat.c b/cat/cat.c index 5b51b7df8..ea2021140 100644 --- a/cat/cat.c +++ b/cat/cat.c @@ -250,6 +250,9 @@ main (int argc, char *argv[]) /* Add drives, inspect and mount. */ add_drives (drvs); + if (key_store_requires_network (ks) && guestfs_set_network (g, 1) == -1) + exit (EXIT_FAILURE); + if (guestfs_launch (g) == -1) exit (EXIT_FAILURE); diff --git a/cat/log.c b/cat/log.c index df7e2be92..0fe486c05 100644 --- a/cat/log.c +++ b/cat/log.c @@ -224,6 +224,9 @@ main (int argc, char *argv[]) */ add_drives (drvs); + if (key_store_requires_network (ks) && guestfs_set_network (g, 1) == -1) + exit (EXIT_FAILURE); + if (guestfs_launch (g) == -1) exit (EXIT_FAILURE); diff --git a/cat/ls.c b/cat/ls.c index e062823b8..1b8e87225 100644 --- a/cat/ls.c +++ b/cat/ls.c @@ -374,6 +374,9 @@ main (int argc, char *argv[]) /* Add drives, inspect and mount. */ add_drives (drvs); + if (key_store_requires_network (ks) && guestfs_set_network (g, 1) == -1) + exit (EXIT_FAILURE); + if (guestfs_launch (g) == -1) exit (EXIT_FAILURE); diff --git a/cat/tail.c b/cat/tail.c index 1cf1d6e0e..2a06e0ebd 100644 --- a/cat/tail.c +++ b/cat/tail.c @@ -296,6 +296,9 @@ do_tail (int argc, char *argv[], /* list of files in the guest */ /* Add drives, inspect and mount. */ add_drives (drvs); + if (key_store_requires_network (ks) && guestfs_set_network (g, 1) == -1) + exit (EXIT_FAILURE); + if (guestfs_launch (g) == -1) return -1; Submodule common 35467027f..af6cb55bc: diff --git a/common/mltools/tools_utils-c.c b/common/mltools/tools_utils-c.c index 0814667..4ff42e5 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 695fda7..562bfad 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 5018300..ec900e6 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 1cd7b62..97c8b88 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 90a3b15..6bc04df 100644 --- a/common/options/key-option.pod +++ b/common/options/key-option.pod @@ -14,4 +14,13 @@ Use the specified C as passphrase. Read the passphrase from F. +=item B<--key> C:clevis + +Attempt passphrase-less unlocking for C with Clevis, over the +network. Please refer to L 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 d27a712..d987ae5 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 80df91a..60d5d80 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/diff/diff.c b/diff/diff.c index 6aae88e6a..c73129c82 100644 --- a/diff/diff.c +++ b/diff/diff.c @@ -209,6 +209,7 @@ main (int argc, char *argv[]) int option_index; struct tree *tree1, *tree2; struct key_store *ks = NULL; + bool network; g = guestfs_create (); if (g == NULL) @@ -378,6 +379,10 @@ main (int argc, char *argv[]) /* Mount up first guest. */ add_drives (drvs); + network = key_store_requires_network (ks); + if (guestfs_set_network (g, network) == -1) + exit (EXIT_FAILURE); + if (guestfs_launch (g) == -1) exit (EXIT_FAILURE); @@ -389,6 +394,9 @@ main (int argc, char *argv[]) /* Mount up second guest. */ add_drives_handle (g2, drvs2, 0); + if (guestfs_set_network (g2, network) == -1) + exit (EXIT_FAILURE); + if (guestfs_launch (g2) == -1) exit (EXIT_FAILURE); diff --git a/edit/edit.c b/edit/edit.c index 7f06bce7f..90c6b85d5 100644 --- a/edit/edit.c +++ b/edit/edit.c @@ -274,6 +274,9 @@ main (int argc, char *argv[]) /* Add drives. */ add_drives (drvs); + if (key_store_requires_network (ks) && guestfs_set_network (g, 1) == -1) + exit (EXIT_FAILURE); + if (guestfs_launch (g) == -1) exit (EXIT_FAILURE); diff --git a/inspector/inspector.c b/inspector/inspector.c index 25ee40f3f..2702e3310 100644 --- a/inspector/inspector.c +++ b/inspector/inspector.c @@ -294,6 +294,9 @@ main (int argc, char *argv[]) */ add_drives (drvs); + if (key_store_requires_network (ks) && guestfs_set_network (g, 1) == -1) + exit (EXIT_FAILURE); + if (guestfs_launch (g) == -1) exit (EXIT_FAILURE); -- 2.31.1