Hi, the following series adds a --key option in the majority of tools: this makes it possible to pass LUKS credentials programmatically, avoid the need to manually input them, or unsafely pass them via stdin. Thanks, Pino Toscano (2): mltools: create a cmdline_options struct Introduce a --key option in tools that accept keys builder/cmdline.ml | 2 +- builder/repository_main.ml | 2 +- cat/cat.c | 6 + cat/log.c | 6 + cat/ls.c | 6 + cat/tail.c | 12 +- cat/virt-cat.pod | 17 +++ cat/virt-log.pod | 17 +++ cat/virt-ls.pod | 17 +++ cat/virt-tail.pod | 17 +++ common/mltools/getopt_tests.ml | 2 +- common/mltools/machine_readable_tests.ml | 2 +- common/mltools/tools_utils-c.c | 44 ++++++- common/mltools/tools_utils.ml | 43 ++++++- common/mltools/tools_utils.mli | 20 ++- common/options/decrypt.c | 4 +- common/options/inspect.c | 4 +- common/options/keys.c | 155 +++++++++++++++++++++++ common/options/options.h | 40 +++++- customize/customize_main.ml | 4 +- customize/virt-customize.pod | 17 +++ dib/cmdline.ml | 2 +- diff/diff.c | 9 +- diff/virt-diff.pod | 17 +++ edit/edit.c | 6 + edit/virt-edit.pod | 17 +++ fish/fish.c | 6 + fish/guestfish.pod | 17 +++ fuse/guestmount.c | 6 + fuse/guestmount.pod | 17 +++ get-kernel/get_kernel.ml | 8 +- get-kernel/virt-get-kernel.pod | 17 +++ inspector/inspector.c | 9 +- inspector/virt-inspector.pod | 17 +++ rescue/rescue.c | 1 + resize/resize.ml | 2 +- sparsify/cmdline.ml | 4 +- sparsify/cmdline.mli | 1 + sparsify/copying.ml | 4 +- sparsify/copying.mli | 2 +- sparsify/in_place.ml | 4 +- sparsify/in_place.mli | 2 +- sparsify/sparsify.ml | 2 + sparsify/virt-sparsify.pod | 17 +++ sysprep/main.ml | 8 +- sysprep/virt-sysprep.pod | 17 +++ v2v/cmdline.ml | 4 +- v2v/cmdline.mli | 1 + v2v/copy_to_local.ml | 2 +- v2v/v2v.ml | 2 +- v2v/virt-v2v.pod | 17 +++ 51 files changed, 626 insertions(+), 49 deletions(-) -- 2.17.1
Pino Toscano
2018-Sep-19 10:37 UTC
[Libguestfs] [PATCH 1/2] mltools: create a cmdline_options struct
Instead of returning directly a Getopt.t handle, now Tools_utils.create_standard_options returns a struct, which at the moment contains only the Getopt.t handle. This way, it will be easy to add more data needed for handling standard command line options. This is mostly refactoring, with no functional changes. --- builder/cmdline.ml | 2 +- builder/repository_main.ml | 2 +- common/mltools/getopt_tests.ml | 2 +- common/mltools/machine_readable_tests.ml | 2 +- common/mltools/tools_utils.ml | 9 ++++++++- common/mltools/tools_utils.mli | 10 ++++++++-- customize/customize_main.ml | 2 +- dib/cmdline.ml | 2 +- get-kernel/get_kernel.ml | 2 +- resize/resize.ml | 2 +- sparsify/cmdline.ml | 2 +- sysprep/main.ml | 2 +- v2v/cmdline.ml | 2 +- v2v/copy_to_local.ml | 2 +- 14 files changed, 28 insertions(+), 15 deletions(-) diff --git a/builder/cmdline.ml b/builder/cmdline.ml index f05aecc76..bd099e218 100644 --- a/builder/cmdline.ml +++ b/builder/cmdline.ml @@ -191,7 +191,7 @@ read the man page virt-builder(1). ") prog in let opthandle = create_standard_options argspec ~anon_fun ~machine_readable:true usage_msg in - Getopt.parse opthandle; + Getopt.parse opthandle.getopt; (* Dereference options. *) let args = List.rev !args in diff --git a/builder/repository_main.ml b/builder/repository_main.ml index 554715a73..60ee96299 100644 --- a/builder/repository_main.ml +++ b/builder/repository_main.ml @@ -69,7 +69,7 @@ read the man page virt-builder-repository(1). ") prog in let opthandle = create_standard_options argspec ~anon_fun ~machine_readable:true usage_msg in - Getopt.parse opthandle; + Getopt.parse opthandle.getopt; (* Machine-readable mode? Print out some facts about what * this binary supports. diff --git a/common/mltools/getopt_tests.ml b/common/mltools/getopt_tests.ml index 1617b3056..ca6ce5d48 100644 --- a/common/mltools/getopt_tests.ml +++ b/common/mltools/getopt_tests.ml @@ -68,7 +68,7 @@ let print_optstring_value = function let opthandle = create_standard_options argspec ~anon_fun usage_msg let () - Getopt.parse opthandle; + Getopt.parse opthandle.getopt; (* Implicit settings. *) printf "trace = %b\n" (trace ()); diff --git a/common/mltools/machine_readable_tests.ml b/common/mltools/machine_readable_tests.ml index 907f05207..809eff01d 100644 --- a/common/mltools/machine_readable_tests.ml +++ b/common/mltools/machine_readable_tests.ml @@ -30,7 +30,7 @@ let usage_msg = sprintf "%s: test the --machine-readable functionality" prog let opthandle = create_standard_options [] ~machine_readable:true usage_msg let () - Getopt.parse opthandle; + Getopt.parse opthandle.getopt; print_endline "on-stdout"; prerr_endline "on-stderr"; diff --git a/common/mltools/tools_utils.ml b/common/mltools/tools_utils.ml index c4f230275..2b2d43db9 100644 --- a/common/mltools/tools_utils.ml +++ b/common/mltools/tools_utils.ml @@ -259,6 +259,10 @@ let machine_readable () in Some { pr } +type cmdline_options = { + getopt : Getopt.t; +} + let create_standard_options argspec ?anon_fun ?(key_opts = false) ?(machine_readable = false) usage_msg (** Install an exit hook to check gc consistency for --debug-gc *) let set_debug_gc () @@ -306,7 +310,10 @@ let create_standard_options argspec ?anon_fun ?(key_opts = false) ?(machine_read [ L"machine-readable" ], Getopt.OptString ("format", parse_machine_readable), s_"Make output machine readable"; ] else []) in - Getopt.create argspec ?anon_fun usage_msg + let getopt = Getopt.create argspec ?anon_fun usage_msg in + { + getopt; + } (* Run an external command, slurp up the output as a list of lines. *) let external_command ?(echo_cmd = true) cmd diff --git a/common/mltools/tools_utils.mli b/common/mltools/tools_utils.mli index 2b8c2b78a..99984bfa1 100644 --- a/common/mltools/tools_utils.mli +++ b/common/mltools/tools_utils.mli @@ -74,7 +74,13 @@ val machine_readable : unit -> machine_readable_fn option readable output to, in case it was enabled via [--machine-readable]. *) -val create_standard_options : Getopt.speclist -> ?anon_fun:Getopt.anon_fun -> ?key_opts:bool -> ?machine_readable:bool -> Getopt.usage_msg -> Getopt.t +type cmdline_options = { + getopt : Getopt.t; (** The actual Getopt handle. *) +} +(** Structure representing all the data needed for handling command + line options. *) + +val create_standard_options : Getopt.speclist -> ?anon_fun:Getopt.anon_fun -> ?key_opts:bool -> ?machine_readable:bool -> Getopt.usage_msg -> cmdline_options (** Adds the standard libguestfs command line options to the specified ones, sorting them, and setting [long_options] to them. @@ -84,7 +90,7 @@ val create_standard_options : Getopt.speclist -> ?anon_fun:Getopt.anon_fun -> ?k [machine_readable] specifies whether add the [--machine-readable] option. - Returns a new [Getopt.t] handle. *) + Returns a new [cmdline_options] structure. *) val external_command : ?echo_cmd:bool -> string -> string list (** Run an external command, slurp up the output as a list of lines. diff --git a/customize/customize_main.ml b/customize/customize_main.ml index 8ba4f5ce7..b4da87368 100644 --- a/customize/customize_main.ml +++ b/customize/customize_main.ml @@ -103,7 +103,7 @@ read the man page virt-customize(1). ") prog in let opthandle = create_standard_options argspec ~key_opts:true usage_msg in - Getopt.parse opthandle; + Getopt.parse opthandle.getopt; if not !format_consumed then error (f_"--format parameter must appear before -a parameter"); diff --git a/dib/cmdline.ml b/dib/cmdline.ml index 5f0cb6dca..220350d9d 100644 --- a/dib/cmdline.ml +++ b/dib/cmdline.ml @@ -196,7 +196,7 @@ read the man page virt-dib(1). let argspec = argspec @ Output_format.extra_args () in let opthandle = create_standard_options argspec ~anon_fun:append_element ~machine_readable:true usage_msg in - Getopt.parse opthandle; + Getopt.parse opthandle.getopt; let debug = !debug in let basepath = !basepath in diff --git a/get-kernel/get_kernel.ml b/get-kernel/get_kernel.ml index c11136adb..cbc617bb8 100644 --- a/get-kernel/get_kernel.ml +++ b/get-kernel/get_kernel.ml @@ -70,7 +70,7 @@ read the man page virt-get-kernel(1). ") prog in let opthandle = create_standard_options argspec ~key_opts:true ~machine_readable:true usage_msg in - Getopt.parse opthandle; + Getopt.parse opthandle.getopt; (* Machine-readable mode? Print out some facts about what * this binary supports. diff --git a/resize/resize.ml b/resize/resize.ml index fe1389b6e..63f911d2c 100644 --- a/resize/resize.ml +++ b/resize/resize.ml @@ -224,7 +224,7 @@ read the man page virt-resize(1). ") prog in let opthandle = create_standard_options argspec ~anon_fun ~machine_readable:true usage_msg in - Getopt.parse opthandle; + Getopt.parse opthandle.getopt; if verbose () then ( printf "command line:"; diff --git a/sparsify/cmdline.ml b/sparsify/cmdline.ml index 4ef43a505..ca509b97f 100644 --- a/sparsify/cmdline.ml +++ b/sparsify/cmdline.ml @@ -89,7 +89,7 @@ read the man page virt-sparsify(1). ") prog in let opthandle = create_standard_options argspec ~anon_fun ~key_opts:true ~machine_readable:true usage_msg in - Getopt.parse opthandle; + Getopt.parse opthandle.getopt; (* Dereference the rest of the args. *) let check_tmpdir = !check_tmpdir in diff --git a/sysprep/main.ml b/sysprep/main.ml index 980b62a35..37fe6be01 100644 --- a/sysprep/main.ml +++ b/sysprep/main.ml @@ -149,7 +149,7 @@ read the man page virt-sysprep(1). ") prog in let opthandle = create_standard_options args ~key_opts:true usage_msg in - Getopt.parse opthandle; + Getopt.parse opthandle.getopt; if not !format_consumed then error (f_"--format parameter must appear before -a parameter"); diff --git a/v2v/cmdline.ml b/v2v/cmdline.ml index f051738b2..29c3e5d4f 100644 --- a/v2v/cmdline.ml +++ b/v2v/cmdline.ml @@ -300,7 +300,7 @@ read the man page virt-v2v(1). ") prog in let opthandle = create_standard_options argspec ~anon_fun ~key_opts:true ~machine_readable:true usage_msg in - Getopt.parse opthandle; + Getopt.parse opthandle.getopt; (* Dereference the arguments. *) let args = List.rev !args in diff --git a/v2v/copy_to_local.ml b/v2v/copy_to_local.ml index c626b6368..86ff72b3a 100644 --- a/v2v/copy_to_local.ml +++ b/v2v/copy_to_local.ml @@ -76,7 +76,7 @@ read the man page virt-v2v-copy-to-local(1). ") prog in let opthandle = create_standard_options argspec ~anon_fun usage_msg in - Getopt.parse opthandle; + Getopt.parse opthandle.getopt; let args = !args in let input_conn = !input_conn in -- 2.17.1
Pino Toscano
2018-Sep-19 10:37 UTC
[Libguestfs] [PATCH 2/2] Introduce a --key option in tools that accept keys
The majority of the tools have already options (--echo-keys & --keys-from-stdin) to deal with LUKS credentials, although there is no way to automatically provide credentials. --keys-from-stdin is suboptimal, because it is an usable solution only when there is just one device to open, and no other input passed via stdin to the tool (like the commands for guestfish). To overcome this limitation, introduce a new --key option in tools: * --key /dev/device:file:/filename/with/key * --key /dev/device:string:the-actual-key this way it is possible to pass all the credentials needed for the specific devices to open, with no risk of conflict with stdin, and also in a secure way (when using the "file" way). On the technical side: this adds a new "key_store" API for the C tools, making sure it is used only when needed. Partially mirror it also for the OCaml tools, although there will be a conversion to the C API because the decryption helpers used are in the common C parts. --- cat/cat.c | 6 ++ cat/log.c | 6 ++ cat/ls.c | 6 ++ cat/tail.c | 12 ++- cat/virt-cat.pod | 17 ++++ cat/virt-log.pod | 17 ++++ cat/virt-ls.pod | 17 ++++ cat/virt-tail.pod | 17 ++++ common/mltools/tools_utils-c.c | 44 +++++++++- common/mltools/tools_utils.ml | 36 +++++++- common/mltools/tools_utils.mli | 10 ++- common/options/decrypt.c | 4 +- common/options/inspect.c | 4 +- common/options/keys.c | 155 +++++++++++++++++++++++++++++++++ common/options/options.h | 40 ++++++++- customize/customize_main.ml | 2 +- customize/virt-customize.pod | 17 ++++ diff/diff.c | 9 +- diff/virt-diff.pod | 17 ++++ edit/edit.c | 6 ++ edit/virt-edit.pod | 17 ++++ fish/fish.c | 6 ++ fish/guestfish.pod | 17 ++++ fuse/guestmount.c | 6 ++ fuse/guestmount.pod | 17 ++++ get-kernel/get_kernel.ml | 6 +- get-kernel/virt-get-kernel.pod | 17 ++++ inspector/inspector.c | 9 +- inspector/virt-inspector.pod | 17 ++++ rescue/rescue.c | 1 + sparsify/cmdline.ml | 2 + sparsify/cmdline.mli | 1 + sparsify/copying.ml | 4 +- sparsify/copying.mli | 2 +- sparsify/in_place.ml | 4 +- sparsify/in_place.mli | 2 +- sparsify/sparsify.ml | 2 + sparsify/virt-sparsify.pod | 17 ++++ sysprep/main.ml | 6 +- sysprep/virt-sysprep.pod | 17 ++++ v2v/cmdline.ml | 2 + v2v/cmdline.mli | 1 + v2v/v2v.ml | 2 +- v2v/virt-v2v.pod | 17 ++++ 44 files changed, 599 insertions(+), 35 deletions(-) diff --git a/cat/cat.c b/cat/cat.c index 3706172c8..4f0d7035e 100644 --- a/cat/cat.c +++ b/cat/cat.c @@ -71,6 +71,7 @@ usage (int status) " --echo-keys Don't turn off echo for passphrases\n" " --format[=raw|..] Force disk format for -a option\n" " --help Display brief help\n" + " --key selector Specify a LUKS key\n" " --keys-from-stdin Read passphrases from stdin\n" " -m|--mount dev[:mnt[:opts[:fstype]]]\n" " Mount dev on mnt (if omitted, /)\n" @@ -101,6 +102,7 @@ main (int argc, char *argv[]) { "echo-keys", 0, 0, 0 }, { "format", 2, 0, 0 }, { "help", 0, 0, HELP_OPTION }, + { "key", 1, 0, 0 }, { "keys-from-stdin", 0, 0, 0 }, { "long-options", 0, 0, 0 }, { "mount", 1, 0, 'm' }, @@ -119,6 +121,7 @@ main (int argc, char *argv[]) int c; int r; int option_index; + struct key_store *ks = NULL; g = guestfs_create (); if (g == NULL) @@ -140,6 +143,8 @@ main (int argc, char *argv[]) echo_keys = 1; } else if (STREQ (long_options[option_index].name, "format")) { OPTION_format; + } else if (STREQ (long_options[option_index].name, "key")) { + OPTION_key; } else error (EXIT_FAILURE, 0, _("unknown long option: %s (%d)"), @@ -249,6 +254,7 @@ main (int argc, char *argv[]) /* Free up data structures, no longer needed after this point. */ free_drives (drvs); free_mps (mps); + free_key_store (ks); r = do_cat (argc - optind, &argv[optind]); diff --git a/cat/log.c b/cat/log.c index d6e98cb4c..afba541ca 100644 --- a/cat/log.c +++ b/cat/log.c @@ -81,6 +81,7 @@ usage (int status) " --echo-keys Don't turn off echo for passphrases\n" " --format[=raw|..] Force disk format for -a option\n" " --help Display brief help\n" + " --key selector Specify a LUKS key\n" " --keys-from-stdin Read passphrases from stdin\n" " -v|--verbose Verbose messages\n" " -V|--version Display version and exit\n" @@ -109,6 +110,7 @@ main (int argc, char *argv[]) { "echo-keys", 0, 0, 0 }, { "format", 2, 0, 0 }, { "help", 0, 0, HELP_OPTION }, + { "key", 1, 0, 0 }, { "keys-from-stdin", 0, 0, 0 }, { "long-options", 0, 0, 0 }, { "short-options", 0, 0, 0 }, @@ -122,6 +124,7 @@ main (int argc, char *argv[]) int c; int r; int option_index; + struct key_store *ks = NULL; g = guestfs_create (); if (g == NULL) @@ -143,6 +146,8 @@ main (int argc, char *argv[]) echo_keys = 1; } else if (STREQ (long_options[option_index].name, "format")) { OPTION_format; + } else if (STREQ (long_options[option_index].name, "key")) { + OPTION_key; } else error (EXIT_FAILURE, 0, _("unknown long option: %s (%d)"), @@ -219,6 +224,7 @@ main (int argc, char *argv[]) /* Free up data structures, no longer needed after this point. */ free_drives (drvs); + free_key_store (ks); r = do_log (); diff --git a/cat/ls.c b/cat/ls.c index 9f0641892..055939a80 100644 --- a/cat/ls.c +++ b/cat/ls.c @@ -113,6 +113,7 @@ usage (int status) " --format[=raw|..] Force disk format for -a option\n" " --help Display brief help\n" " -h|--human-readable Human-readable sizes in output\n" + " --key selector Specify a LUKS key\n" " --keys-from-stdin Read passphrases from stdin\n" " -l|--long Long listing\n" " -m|--mount dev[:mnt[:opts[:fstype]]]\n" @@ -159,6 +160,7 @@ main (int argc, char *argv[]) { "format", 2, 0, 0 }, { "help", 0, 0, HELP_OPTION }, { "human-readable", 0, 0, 'h' }, + { "key", 1, 0, 0 }, { "keys-from-stdin", 0, 0, 0 }, { "long", 0, 0, 'l' }, { "long-options", 0, 0, 0 }, @@ -189,6 +191,7 @@ main (int argc, char *argv[]) #define MODE_LS_R 2 #define MODE_LS_LR (MODE_LS_L|MODE_LS_R) int mode = 0; + struct key_store *ks = NULL; g = guestfs_create (); if (g == NULL) @@ -238,6 +241,8 @@ main (int argc, char *argv[]) } else if (STREQ (long_options[option_index].name, "uid") || STREQ (long_options[option_index].name, "uids")) { enable_uids = 1; + } else if (STREQ (long_options[option_index].name, "key")) { + OPTION_key; } else error (EXIT_FAILURE, 0, _("unknown long option: %s (%d)"), @@ -373,6 +378,7 @@ main (int argc, char *argv[]) /* Free up data structures, no longer needed after this point. */ free_drives (drvs); free_mps (mps); + free_key_store (ks); unsigned errors = 0; diff --git a/cat/tail.c b/cat/tail.c index e932820e6..9e3af7c7d 100644 --- a/cat/tail.c +++ b/cat/tail.c @@ -55,7 +55,7 @@ int inspector = 1; int in_guestfish = 0; int in_virt_rescue = 0; -static int do_tail (int argc, char *argv[], struct drv *drvs, struct mp *mps); +static int do_tail (int argc, char *argv[], struct drv *drvs, struct mp *mps, struct key_store *ks); static time_t disk_mtime (struct drv *drvs); static int reopen_handle (void); @@ -79,6 +79,7 @@ usage (int status) " -f|--follow Ignored for compatibility with tail\n" " --format[=raw|..] Force disk format for -a option\n" " --help Display brief help\n" + " --key selector Specify a LUKS key\n" " --keys-from-stdin Read passphrases from stdin\n" " -m|--mount dev[:mnt[:opts[:fstype]]]\n" " Mount dev on mnt (if omitted, /)\n" @@ -110,6 +111,7 @@ main (int argc, char *argv[]) { "follow", 0, 0, 'f' }, { "format", 2, 0, 0 }, { "help", 0, 0, HELP_OPTION }, + { "key", 1, 0, 0 }, { "keys-from-stdin", 0, 0, 0 }, { "long-options", 0, 0, 0 }, { "mount", 1, 0, 'm' }, @@ -127,6 +129,7 @@ main (int argc, char *argv[]) int c; int r; int option_index; + struct key_store *ks = NULL; g = guestfs_create (); if (g == NULL) @@ -148,6 +151,8 @@ main (int argc, char *argv[]) echo_keys = 1; } else if (STREQ (long_options[option_index].name, "format")) { OPTION_format; + } else if (STREQ (long_options[option_index].name, "key")) { + OPTION_key; } else error (EXIT_FAILURE, 0, _("unknown long option: %s (%d)"), @@ -220,10 +225,11 @@ main (int argc, char *argv[]) usage (EXIT_FAILURE); } - r = do_tail (argc - optind, &argv[optind], drvs, mps); + r = do_tail (argc - optind, &argv[optind], drvs, mps, ks); free_drives (drvs); free_mps (mps); + free_key_store (ks); guestfs_close (g); @@ -246,7 +252,7 @@ user_cancel (int sig) static int do_tail (int argc, char *argv[], /* list of files in the guest */ - struct drv *drvs, struct mp *mps) + struct drv *drvs, struct mp *mps, struct key_store *ks) { struct sigaction sa; time_t drvt; diff --git a/cat/virt-cat.pod b/cat/virt-cat.pod index 327ef255e..745d4a4b6 100644 --- a/cat/virt-cat.pod +++ b/cat/virt-cat.pod @@ -121,6 +121,23 @@ If you have untrusted raw-format guest disk images, you should use this option to specify the disk format. This avoids a possible security problem with malicious guests (CVE-2010-3851). +=item B<--key> SELECTOR + +Specify a key for LUKS, to automatically open a LUKS device when using +the inspection. C<SELECTOR> can be in one of the following formats: + +=over 4 + +=item B<--key> C<DEVICE>:key:KEY_STRING + +Use the specified C<KEY_STRING> as passphrase. + +=item B<--key> C<DEVICE>:file:FILENAME + +Read the passphrase from F<FILENAME>. + +=back + =item B<--keys-from-stdin> Read key or passphrase parameters from stdin. The default is diff --git a/cat/virt-log.pod b/cat/virt-log.pod index 812b7a220..5841824c3 100644 --- a/cat/virt-log.pod +++ b/cat/virt-log.pod @@ -105,6 +105,23 @@ If you have untrusted raw-format guest disk images, you should use this option to specify the disk format. This avoids a possible security problem with malicious guests (CVE-2010-3851). +=item B<--key> SELECTOR + +Specify a key for LUKS, to automatically open a LUKS device when using +the inspection. C<SELECTOR> can be in one of the following formats: + +=over 4 + +=item B<--key> C<DEVICE>:key:KEY_STRING + +Use the specified C<KEY_STRING> as passphrase. + +=item B<--key> C<DEVICE>:file:FILENAME + +Read the passphrase from F<FILENAME>. + +=back + =item B<--keys-from-stdin> Read key or passphrase parameters from stdin. The default is diff --git a/cat/virt-ls.pod b/cat/virt-ls.pod index 80e1cf496..f00000b12 100644 --- a/cat/virt-ls.pod +++ b/cat/virt-ls.pod @@ -352,6 +352,23 @@ Display file sizes in human-readable format. This option only has effect in I<-lR> output mode. See L</RECURSIVE LONG LISTING> above. +=item B<--key> SELECTOR + +Specify a key for LUKS, to automatically open a LUKS device when using +the inspection. C<SELECTOR> can be in one of the following formats: + +=over 4 + +=item B<--key> C<DEVICE>:key:KEY_STRING + +Use the specified C<KEY_STRING> as passphrase. + +=item B<--key> C<DEVICE>:file:FILENAME + +Read the passphrase from F<FILENAME>. + +=back + =item B<--keys-from-stdin> Read key or passphrase parameters from stdin. The default is diff --git a/cat/virt-tail.pod b/cat/virt-tail.pod index 2278ad034..cf8700d1a 100644 --- a/cat/virt-tail.pod +++ b/cat/virt-tail.pod @@ -123,6 +123,23 @@ If you have untrusted raw-format guest disk images, you should use this option to specify the disk format. This avoids a possible security problem with malicious guests (CVE-2010-3851). +=item B<--key> SELECTOR + +Specify a key for LUKS, to automatically open a LUKS device when using +the inspection. C<SELECTOR> can be in one of the following formats: + +=over 4 + +=item B<--key> C<DEVICE>:key:KEY_STRING + +Use the specified C<KEY_STRING> as passphrase. + +=item B<--key> C<DEVICE>:file:FILENAME + +Read the passphrase from F<FILENAME>. + +=back + =item B<--keys-from-stdin> Read key or passphrase parameters from stdin. The default is diff --git a/common/mltools/tools_utils-c.c b/common/mltools/tools_utils-c.c index 94ecebead..c88c95082 100644 --- a/common/mltools/tools_utils-c.c +++ b/common/mltools/tools_utils-c.c @@ -22,6 +22,7 @@ #include <stdlib.h> #include <unistd.h> #include <errno.h> +#include <error.h> #include <caml/alloc.h> #include <caml/fail.h> @@ -33,7 +34,7 @@ #include "options.h" -extern value guestfs_int_mllib_inspect_decrypt (value gv, value gpv); +extern value guestfs_int_mllib_inspect_decrypt (value gv, value gpv, value keysv); extern value guestfs_int_mllib_set_echo_keys (value unitv); extern value guestfs_int_mllib_set_keys_from_stdin (value unitv); @@ -42,12 +43,47 @@ int echo_keys = 0; int keys_from_stdin = 0; value -guestfs_int_mllib_inspect_decrypt (value gv, value gpv) +guestfs_int_mllib_inspect_decrypt (value gv, value gpv, value keysv) { - CAMLparam2 (gv, gpv); + CAMLparam3 (gv, gpv, keysv); + CAMLlocal2 (elemv, v); guestfs_h *g = (guestfs_h *) (intptr_t) Int64_val (gpv); + struct key_store *ks = NULL; - inspect_do_decrypt (g); + while (keysv != Val_emptylist) { + struct key_store_key key; + + elemv = Field (keysv, 0); + key.device = strdup (String_val (Field (elemv, 0))); + if (!key.device) + 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)); + } + + ks = key_store_import_key (ks, &key); + + keysv = Field (keysv, 1); + } + + inspect_do_decrypt (g, ks); CAMLreturn (Val_unit); } diff --git a/common/mltools/tools_utils.ml b/common/mltools/tools_utils.ml index 2b2d43db9..ad08d05eb 100644 --- a/common/mltools/tools_utils.ml +++ b/common/mltools/tools_utils.ml @@ -22,7 +22,14 @@ open Std_utils open Common_gettext.Gettext open Getopt.OptionName -external c_inspect_decrypt : Guestfs.t -> int64 -> unit = "guestfs_int_mllib_inspect_decrypt" +type key_store = { + keys : (string, key_store_key) Hashtbl.t; +} +and key_store_key + | KeyString of string + | KeyFileName of string + +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" @@ -261,6 +268,7 @@ let machine_readable () type cmdline_options = { getopt : Getopt.t; + ks : key_store; } let create_standard_options argspec ?anon_fun ?(key_opts = false) ?(machine_readable = false) usage_msg @@ -288,6 +296,9 @@ let create_standard_options argspec ?anon_fun ?(key_opts = false) ?(machine_read error (f_"invalid output for --machine-readable: %s") fmt ) in + let ks = { + keys = Hashtbl.create 13; + } in let argspec = [ [ S 'V'; L"version" ], Getopt.Unit print_version_and_exit, s_"Display version and exit"; [ S 'v'; L"verbose" ], Getopt.Unit set_verbose, s_"Enable libguestfs debugging messages"; @@ -300,9 +311,20 @@ let create_standard_options argspec ?anon_fun ?(key_opts = false) ?(machine_read let argspec argspec @ (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) + | [ device; "file"; file ] -> + Hashtbl.replace ks.keys device (KeyFileName file) + | _ -> + error (f_"invalid selector string for --key: %s") arg + in [ [ L"echo-keys" ], Getopt.Unit c_set_echo_keys, s_"Don’t turn off echo for passphrases"; [ L"keys-from-stdin" ], Getopt.Unit c_set_keys_from_stdin, s_"Read passphrases from stdin"; + [ L"key" ], Getopt.String (s_"SELECTOR", parse_key_selector), s_"Specify a LUKS key"; ] else []) @ (if machine_readable then @@ -312,7 +334,7 @@ let create_standard_options argspec ?anon_fun ?(key_opts = false) ?(machine_read else []) in let getopt = Getopt.create argspec ?anon_fun usage_msg in { - getopt; + getopt; ks; } (* Run an external command, slurp up the output as a list of lines. *) @@ -599,13 +621,21 @@ let is_btrfs_subvolume g fs if g#last_errno () = Guestfs.Errno.errno_EINVAL then false else raise exn -let inspect_decrypt g +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 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 99984bfa1..01c5e51bb 100644 --- a/common/mltools/tools_utils.mli +++ b/common/mltools/tools_utils.mli @@ -74,8 +74,11 @@ val machine_readable : unit -> machine_readable_fn option readable output to, in case it was enabled via [--machine-readable]. *) +type key_store + type cmdline_options = { getopt : Getopt.t; (** The actual Getopt handle. *) + ks : key_store; (** Container for keys read via [--key]. *) } (** Structure representing all the data needed for handling command line options. *) @@ -85,7 +88,10 @@ val create_standard_options : Getopt.speclist -> ?anon_fun:Getopt.anon_fun -> ?k sorting them, and setting [long_options] to them. [key_opts] specifies whether add the standard options related to - keys management, i.e. [--echo-keys] and [--keys-from-stdin]. + keys management, i.e. [--echo-keys], [--key], and [--keys-from-stdin]. + In case [key_opts] is specified, {!recfield:cmdline_options.ks} will + contain the keys specified via [--key], so it ought to be passed around + where needed. [machine_readable] specifies whether add the [--machine-readable] option. @@ -188,7 +194,7 @@ 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 inspect_decrypt : Guestfs.guestfs -> unit +val inspect_decrypt : Guestfs.guestfs -> key_store -> unit (** Simple implementation of decryption: look for any [crypto_LUKS] partitions and decrypt them, then rescan for VGs. This only works for Fedora whole-disk encryption. *) diff --git a/common/options/decrypt.c b/common/options/decrypt.c index b9113073c..234163d8c 100644 --- a/common/options/decrypt.c +++ b/common/options/decrypt.c @@ -68,7 +68,7 @@ make_mapname (const char *device, char *mapname, size_t len) * encryption schemes. */ void -inspect_do_decrypt (guestfs_h *g) +inspect_do_decrypt (guestfs_h *g, struct key_store *ks) { CLEANUP_FREE_STRING_LIST char **partitions = guestfs_list_partitions (g); if (partitions == NULL) @@ -82,7 +82,7 @@ inspect_do_decrypt (guestfs_h *g) char mapname[32]; make_mapname (partitions[i], mapname, sizeof mapname); - CLEANUP_FREE char *key = read_key (partitions[i]); + CLEANUP_FREE char *key = get_key (ks, partitions[i]); /* XXX Should we call guestfs_luks_open_ro if readonly flag * is set? This might break 'mount_ro'. */ diff --git a/common/options/inspect.c b/common/options/inspect.c index e0deae2df..3de6d70f1 100644 --- a/common/options/inspect.c +++ b/common/options/inspect.c @@ -65,12 +65,12 @@ compare_keys (const void *p1, const void *p2) * This function implements the I<-i> option. */ void -inspect_mount_handle (guestfs_h *g) +inspect_mount_handle (guestfs_h *g, struct key_store *ks) { if (live) error (EXIT_FAILURE, 0, _("don’t use --live and -i options together")); - inspect_do_decrypt (g); + inspect_do_decrypt (g, ks); char **roots = guestfs_inspect_os (g); if (roots == NULL) diff --git a/common/options/keys.c b/common/options/keys.c index 1c0769725..7f689866b 100644 --- a/common/options/keys.c +++ b/common/options/keys.c @@ -24,6 +24,9 @@ #include <termios.h> #include <string.h> #include <libintl.h> +#include <errno.h> +#include <error.h> +#include <assert.h> #include "guestfs.h" @@ -94,3 +97,155 @@ read_key (const char *param) return ret; } + +static char * +read_first_line_from_file (const char *filename) +{ + CLEANUP_FCLOSE FILE *fp = NULL; + char *ret = NULL; + size_t allocsize = 0; + ssize_t len; + + fp = fopen (filename, "r"); + if (!fp) + error (EXIT_FAILURE, errno, "fopen: %s", filename); + + len = getline (&ret, &allocsize, fp); + if (len == -1) + error (EXIT_FAILURE, errno, "getline: %s", filename); + + /* Remove the terminating \n if there is one. */ + if (len > 0 && ret[len-1] == '\n') + ret[len-1] = '\0'; + + return ret; +} + +char * +get_key (struct key_store *ks, const char *device) +{ + size_t i; + + if (ks) { + for (i = 0; i < ks->nr_keys; ++i) { + struct key_store_key *key = &ks->keys[i]; + char *s; + + if (STRNEQ (key->device, device)) + continue; + + switch (key->type) { + case key_string: + s = strdup (key->string.s); + if (!s) + error (EXIT_FAILURE, errno, "strdup"); + return s; + case key_file: + return read_first_line_from_file (key->file.name); + } + + /* Key not found in the key store, ask the user for it. */ + break; + } + } + + return read_key (device); +} + +struct key_store * +key_store_add_from_selector (struct key_store *ks, const char *selector_orig) +{ + CLEANUP_FREE char *selector = strdup (selector_orig); + const char *elem; + char *saveptr; + struct key_store_key key; + + if (!selector) + error (EXIT_FAILURE, errno, "strdup"); + + /* 1: device */ + elem = strtok_r (selector, ":", &saveptr); + if (!elem) { + invalid_selector: + error (EXIT_FAILURE, 0, "invalid selector for --key: %s", selector_orig); + } + key.device = strdup (elem); + if (!key.device) + error (EXIT_FAILURE, errno, "strdup"); + + /* 2: key type */ + elem = strtok_r (NULL, ":", &saveptr); + if (!elem) + goto invalid_selector; + else if (STREQ (elem, "key")) + key.type = key_string; + else if (STREQ (elem, "file")) + key.type = key_file; + else + goto invalid_selector; + + /* 3: actual key */ + elem = strtok_r (NULL, ":", &saveptr); + if (!elem) + goto invalid_selector; + switch (key.type) { + case key_string: + key.string.s = strdup (elem); + if (!key.string.s) + error (EXIT_FAILURE, errno, "strdup"); + break; + case key_file: + key.file.name = strdup (elem); + if (!key.file.name) + error (EXIT_FAILURE, errno, "strdup"); + break; + } + + return key_store_import_key (ks, &key); +} + +struct key_store * +key_store_import_key (struct key_store *ks, const struct key_store_key *key) +{ + struct key_store_key *new_keys; + + if (!ks) { + ks = calloc (1, sizeof (*ks)); + if (!ks) + error (EXIT_FAILURE, errno, "strdup"); + } + assert (ks != NULL); + + new_keys = realloc (ks->keys, sizeof (*ks->keys) + 1); + if (!new_keys) + error (EXIT_FAILURE, errno, "realloc"); + + ks->keys = new_keys; + ks->keys[ks->nr_keys] = *key; + ++ks->nr_keys; + + return ks; +} + +void +free_key_store (struct key_store *ks) +{ + size_t i; + + if (!ks) + return; + + for (i = 0; i < ks->nr_keys; ++i) { + struct key_store_key *key = &ks->keys[i]; + + switch (key->type) { + case key_string: + free (key->string.s); + break; + case key_file: + free (key->file.name); + break; + } + free (key->device); + } +} diff --git a/common/options/options.h b/common/options/options.h index f4482e478..6fadf1e76 100644 --- a/common/options/options.h +++ b/common/options/options.h @@ -102,23 +102,54 @@ struct mp { char *fstype; }; +/* A key in the key store. */ +struct key_store_key { + /* The device this key refers to. */ + char *device; + + enum { + key_string, /* key specified as string */ + key_file, /* key stored in a file */ + } type; + union { + struct { + char *s; /* string of the key */ + } string; + struct { + char *name; /* filename with the key */ + } file; + }; +}; + +/* Container for keys, usually collected via the '--key' command line option + * in tools. + */ +struct key_store { + struct key_store_key *keys; + size_t nr_keys; +}; + /* in config.c */ extern void parse_config (void); /* in decrypt.c */ -extern void inspect_do_decrypt (guestfs_h *g); +extern void inspect_do_decrypt (guestfs_h *g, struct key_store *ks); /* in domain.c */ extern int add_libvirt_drives (guestfs_h *g, const char *guest); /* in inspect.c */ -extern void inspect_mount_handle (guestfs_h *g); +extern void inspect_mount_handle (guestfs_h *g, struct key_store *ks); extern void inspect_mount_root (guestfs_h *g, const char *root); -#define inspect_mount() inspect_mount_handle (g) +#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_key (struct key_store *ks, const char *device); +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 */ extern void option_a (const char *arg, const char *format, struct drv **drvsp); @@ -219,6 +250,9 @@ extern void free_mps (struct mp *mp); #define OPTION_x \ guestfs_set_trace (g, 1) +#define OPTION_key \ + ks = key_store_add_from_selector (ks, optarg) + #define CHECK_OPTION_format_consumed \ do { \ if (!format_consumed) { \ diff --git a/customize/customize_main.ml b/customize/customize_main.ml index b4da87368..8a022342f 100644 --- a/customize/customize_main.ml +++ b/customize/customize_main.ml @@ -174,7 +174,7 @@ read the man page virt-customize(1). g in (* Decrypt the disks. *) - inspect_decrypt g; + inspect_decrypt g opthandle.ks; (* Inspection. *) (match Array.to_list (g#inspect_os ()) with diff --git a/customize/virt-customize.pod b/customize/virt-customize.pod index 2af29b93b..997e8bb3f 100644 --- a/customize/virt-customize.pod +++ b/customize/virt-customize.pod @@ -138,6 +138,23 @@ If you have untrusted raw-format guest disk images, you should use this option to specify the disk format. This avoids a possible security problem with malicious guests (CVE-2010-3851). +=item B<--key> SELECTOR + +Specify a key for LUKS, to automatically open a LUKS device when using +the inspection. C<SELECTOR> can be in one of the following formats: + +=over 4 + +=item B<--key> C<DEVICE>:key:KEY_STRING + +Use the specified C<KEY_STRING> as passphrase. + +=item B<--key> C<DEVICE>:file:FILENAME + +Read the passphrase from F<FILENAME>. + +=back + =item B<--keys-from-stdin> Read key or passphrase parameters from stdin. The default is diff --git a/diff/diff.c b/diff/diff.c index ebbce8212..4c801b4f7 100644 --- a/diff/diff.c +++ b/diff/diff.c @@ -128,6 +128,7 @@ usage (int status) " --format[=raw|..] Force disk format for -a or -A option\n" " --help Display brief help\n" " -h|--human-readable Human-readable sizes in output\n" + " --key selector Specify a LUKS key\n" " --keys-from-stdin Read passphrases from stdin\n" " --times Display file times\n" " --time-days Display file times as days before now\n" @@ -180,6 +181,7 @@ main (int argc, char *argv[]) { "help", 0, 0, HELP_OPTION }, { "human-readable", 0, 0, 'h' }, { "long-options", 0, 0, 0 }, + { "key", 1, 0, 0 }, { "keys-from-stdin", 0, 0, 0 }, { "short-options", 0, 0, 0 }, { "time", 0, 0, 0 }, @@ -202,6 +204,7 @@ main (int argc, char *argv[]) int c; int option_index; struct tree *tree1, *tree2; + struct key_store *ks = NULL; g = guestfs_create (); if (g == NULL) @@ -272,6 +275,8 @@ main (int argc, char *argv[]) } else if (STREQ (long_options[option_index].name, "xattr") || STREQ (long_options[option_index].name, "xattrs")) { enable_xattrs = 1; + } else if (STREQ (long_options[option_index].name, "key")) { + OPTION_key; } else error (EXIT_FAILURE, 0, _("unknown long option: %s (%d)"), @@ -381,7 +386,7 @@ main (int argc, char *argv[]) if (guestfs_launch (g2) == -1) exit (EXIT_FAILURE); - inspect_mount_handle (g2); + inspect_mount_handle (g2, ks); if ((tree2 = visit_guest (g2)) == NULL) errors++; @@ -397,6 +402,8 @@ main (int argc, char *argv[]) free_drives (drvs); free_drives (drvs2); + free_key_store (ks); + guestfs_close (g); guestfs_close (g2); diff --git a/diff/virt-diff.pod b/diff/virt-diff.pod index 192237697..f8c0bceb4 100644 --- a/diff/virt-diff.pod +++ b/diff/virt-diff.pod @@ -166,6 +166,23 @@ security problem with malicious guests (CVE-2010-3851). Display file sizes in human-readable format. +=item B<--key> SELECTOR + +Specify a key for LUKS, to automatically open a LUKS device when using +the inspection. C<SELECTOR> can be in one of the following formats: + +=over 4 + +=item B<--key> C<DEVICE>:key:KEY_STRING + +Use the specified C<KEY_STRING> as passphrase. + +=item B<--key> C<DEVICE>:file:FILENAME + +Read the passphrase from F<FILENAME>. + +=back + =item B<--keys-from-stdin> Read key or passphrase parameters from stdin. The default is diff --git a/edit/edit.c b/edit/edit.c index 053088ee5..eb1f1fd5f 100644 --- a/edit/edit.c +++ b/edit/edit.c @@ -82,6 +82,7 @@ usage (int status) " -e|--edit|--expr expr Non-interactive editing using Perl expr\n" " --format[=raw|..] Force disk format for -a option\n" " --help Display brief help\n" + " --key selector Specify a LUKS key\n" " --keys-from-stdin Read passphrases from stdin\n" " -m|--mount dev[:mnt[:opts[:fstype]]]\n" " Mount dev on mnt (if omitted, /)\n" @@ -115,6 +116,7 @@ main (int argc, char *argv[]) { "expr", 1, 0, 'e' }, { "format", 2, 0, 0 }, { "help", 0, 0, HELP_OPTION }, + { "key", 1, 0, 0 }, { "keys-from-stdin", 0, 0, 0 }, { "long-options", 0, 0, 0 }, { "mount", 1, 0, 'm' }, @@ -132,6 +134,7 @@ main (int argc, char *argv[]) bool format_consumed = true; int c; int option_index; + struct key_store *ks = NULL; g = guestfs_create (); if (g == NULL) @@ -153,6 +156,8 @@ main (int argc, char *argv[]) echo_keys = 1; } else if (STREQ (long_options[option_index].name, "format")) { OPTION_format; + } else if (STREQ (long_options[option_index].name, "key")) { + OPTION_key; } else error (EXIT_FAILURE, 0, _("unknown long option: %s (%d)"), @@ -274,6 +279,7 @@ main (int argc, char *argv[]) /* Free up data structures, no longer needed after this point. */ free_drives (drvs); free_mps (mps); + free_key_store (ks); edit_files (argc - optind, &argv[optind]); diff --git a/edit/virt-edit.pod b/edit/virt-edit.pod index 509473a0f..8f222d379 100644 --- a/edit/virt-edit.pod +++ b/edit/virt-edit.pod @@ -153,6 +153,23 @@ If you have untrusted raw-format guest disk images, you should use this option to specify the disk format. This avoids a possible security problem with malicious guests (CVE-2010-3851). +=item B<--key> SELECTOR + +Specify a key for LUKS, to automatically open a LUKS device when using +the inspection. C<SELECTOR> can be in one of the following formats: + +=over 4 + +=item B<--key> C<DEVICE>:key:KEY_STRING + +Use the specified C<KEY_STRING> as passphrase. + +=item B<--key> C<DEVICE>:file:FILENAME + +Read the passphrase from F<FILENAME>. + +=back + =item B<--keys-from-stdin> Read key or passphrase parameters from stdin. The default is diff --git a/fish/fish.c b/fish/fish.c index ad85c3fae..3851bfed3 100644 --- a/fish/fish.c +++ b/fish/fish.c @@ -132,6 +132,7 @@ usage (int status) " --format[=raw|..] Force disk format for -a option\n" " --help Display brief help\n" " -i|--inspector Automatically mount filesystems\n" + " --key selector Specify a LUKS key\n" " --keys-from-stdin Read passphrases from stdin\n" " --listen Listen for remote commands\n" " --live Connect to a live virtual machine\n" @@ -198,6 +199,7 @@ main (int argc, char *argv[]) { "format", 2, 0, 0 }, { "help", 0, 0, HELP_OPTION }, { "inspector", 0, 0, 'i' }, + { "key", 1, 0, 0 }, { "keys-from-stdin", 0, 0, 0 }, { "listen", 0, 0, 0 }, { "live", 0, 0, 0 }, @@ -230,6 +232,7 @@ main (int argc, char *argv[]) int option_index; struct sigaction sa; int next_prepared_drive = 1; + struct key_store *ks = NULL; initialize_readline (); init_event_handlers (); @@ -293,6 +296,8 @@ main (int argc, char *argv[]) exit (EXIT_FAILURE); } else if (STREQ (long_options[option_index].name, "no-dest-paths")) { complete_dest_paths = 0; + } else if (STREQ (long_options[option_index].name, "key")) { + OPTION_key; } else error (EXIT_FAILURE, 0, _("unknown long option: %s (%d)"), @@ -496,6 +501,7 @@ main (int argc, char *argv[]) /* Free up data structures, no longer needed after this point. */ free_drives (drvs); free_mps (mps); + free_key_store (ks); /* Remote control? */ if (remote_control_listen && remote_control) diff --git a/fish/guestfish.pod b/fish/guestfish.pod index 4f24006b8..9df88845c 100644 --- a/fish/guestfish.pod +++ b/fish/guestfish.pod @@ -280,6 +280,23 @@ Using this flag is mostly equivalent to using the C<inspect-os> command and then using other commands to mount the filesystems that were found. +=item B<--key> SELECTOR + +Specify a key for LUKS, to automatically open a LUKS device when using +the inspection. C<SELECTOR> can be in one of the following formats: + +=over 4 + +=item B<--key> C<DEVICE>:key:KEY_STRING + +Use the specified C<KEY_STRING> as passphrase. + +=item B<--key> C<DEVICE>:file:FILENAME + +Read the passphrase from F<FILENAME>. + +=back + =item B<--keys-from-stdin> Read key or passphrase parameters from stdin. The default is diff --git a/fuse/guestmount.c b/fuse/guestmount.c index a7ebae4e3..7f323f1fc 100644 --- a/fuse/guestmount.c +++ b/fuse/guestmount.c @@ -119,6 +119,7 @@ usage (int status) " --fuse-help Display extra FUSE options\n" " -i|--inspector Automatically mount filesystems\n" " --help Display help message and exit\n" + " --key selector Specify a LUKS key\n" " --keys-from-stdin Read passphrases from stdin\n" " --live Connect to a live virtual machine\n" " -m|--mount dev[:mnt[:opts[:fstype]] Mount dev on mnt (if omitted, /)\n" @@ -165,6 +166,7 @@ main (int argc, char *argv[]) { "fuse-help", 0, 0, 0 }, { "help", 0, 0, HELP_OPTION }, { "inspector", 0, 0, 'i' }, + { "key", 1, 0, 0 }, { "keys-from-stdin", 0, 0, 0 }, { "live", 0, 0, 0 }, { "long-options", 0, 0, 0 }, @@ -192,6 +194,7 @@ main (int argc, char *argv[]) int c, r; int option_index; struct sigaction sa; + struct key_store *ks = NULL; int debug_calls = 0; int dir_cache_timeout = -1; @@ -246,6 +249,8 @@ main (int argc, char *argv[]) if (sscanf (optarg, "%d", &pipe_fd) != 1 || pipe_fd < 0) error (EXIT_FAILURE, 0, _("unable to parse --fd option value: %s"), optarg); + } else if (STREQ (long_options[option_index].name, "key")) { + OPTION_key; } else error (EXIT_FAILURE, 0, _("unknown long option: %s (%d)"), @@ -369,6 +374,7 @@ main (int argc, char *argv[]) free_drives (drvs); free_mps (mps); + free_key_store (ks); /* FUSE example does this, not clear if it's necessary, but ... */ if (guestfs_umask (g, 0) == -1) diff --git a/fuse/guestmount.pod b/fuse/guestmount.pod index e9fab6622..bd4d6bf6f 100644 --- a/fuse/guestmount.pod +++ b/fuse/guestmount.pod @@ -246,6 +246,23 @@ Using L<virt-inspector(1)> code, inspect the disks looking for an operating system and mount filesystems as they would be mounted on the real virtual machine. +=item B<--key> SELECTOR + +Specify a key for LUKS, to automatically open a LUKS device when using +the inspection. C<SELECTOR> can be in one of the following formats: + +=over 4 + +=item B<--key> C<DEVICE>:key:KEY_STRING + +Use the specified C<KEY_STRING> as passphrase. + +=item B<--key> C<DEVICE>:file:FILENAME + +Read the passphrase from F<FILENAME>. + +=back + =item B<--keys-from-stdin> Read key or passphrase parameters from stdin. The default is diff --git a/get-kernel/get_kernel.ml b/get-kernel/get_kernel.ml index cbc617bb8..94849702f 100644 --- a/get-kernel/get_kernel.ml +++ b/get-kernel/get_kernel.ml @@ -112,7 +112,7 @@ read the man page virt-get-kernel(1). let unversioned = !unversioned in let prefix = !prefix in - add, output, unversioned, prefix + add, output, unversioned, prefix, opthandle.ks let rec do_fetch ~transform_fn ~outputdir g root (* Mount up the disks. *) @@ -166,7 +166,7 @@ and pick_kernel_files_linux (g : Guestfs.guestfs) root (* Main program. *) let main () - let add, output, unversioned, prefix = parse_cmdline () in + let add, output, unversioned, prefix, ks = parse_cmdline () in (* Connect to libguestfs. *) let g = open_guestfs () in @@ -174,7 +174,7 @@ let main () g#launch (); (* Decrypt the disks. *) - inspect_decrypt g; + inspect_decrypt g ks; let roots = g#inspect_os () in if Array.length roots = 0 then diff --git a/get-kernel/virt-get-kernel.pod b/get-kernel/virt-get-kernel.pod index 9aa0b0b1c..30704aee4 100644 --- a/get-kernel/virt-get-kernel.pod +++ b/get-kernel/virt-get-kernel.pod @@ -89,6 +89,23 @@ If you have untrusted raw-format guest disk images, you should use this option to specify the disk format. This avoids a possible security problem with malicious guests (CVE-2010-3851). +=item B<--key> SELECTOR + +Specify a key for LUKS, to automatically open a LUKS device when using +the inspection. C<SELECTOR> can be in one of the following formats: + +=over 4 + +=item B<--key> C<DEVICE>:key:KEY_STRING + +Use the specified C<KEY_STRING> as passphrase. + +=item B<--key> C<DEVICE>:file:FILENAME + +Read the passphrase from F<FILENAME>. + +=back + =item B<--keys-from-stdin> Read key or passphrase parameters from stdin. The default is diff --git a/inspector/inspector.c b/inspector/inspector.c index 5075a8f04..9be8f5110 100644 --- a/inspector/inspector.c +++ b/inspector/inspector.c @@ -88,6 +88,7 @@ usage (int status) " --echo-keys Don't turn off echo for passphrases\n" " --format[=raw|..] Force disk format for -a option\n" " --help Display brief help\n" + " --key selector Specify a LUKS key\n" " --keys-from-stdin Read passphrases from stdin\n" " --no-applications Do not output the installed applications\n" " --no-icon Do not output the guest icon\n" @@ -119,6 +120,7 @@ main (int argc, char *argv[]) { "echo-keys", 0, 0, 0 }, { "format", 2, 0, 0 }, { "help", 0, 0, HELP_OPTION }, + { "key", 1, 0, 0 }, { "keys-from-stdin", 0, 0, 0 }, { "long-options", 0, 0, 0 }, { "no-applications", 0, 0, 0 }, @@ -135,6 +137,7 @@ main (int argc, char *argv[]) bool format_consumed = true; int c; int option_index; + struct key_store *ks = NULL; g = guestfs_create (); if (g == NULL) @@ -162,6 +165,8 @@ main (int argc, char *argv[]) inspect_apps = 0; } else if (STREQ (long_options[option_index].name, "no-icon")) { inspect_icon = 0; + } else if (STREQ (long_options[option_index].name, "key")) { + OPTION_key; } else error (EXIT_FAILURE, 0, _("unknown long option: %s (%d)"), @@ -284,7 +289,9 @@ main (int argc, char *argv[]) * the -i option) because it can only handle a single root. So we * use low-level APIs. */ - inspect_do_decrypt (g); + inspect_do_decrypt (g, ks); + + free_key_store (ks); { CLEANUP_FREE_STRING_LIST char **roots = guestfs_inspect_os (g); diff --git a/inspector/virt-inspector.pod b/inspector/virt-inspector.pod index 1cea542c7..98b278f26 100644 --- a/inspector/virt-inspector.pod +++ b/inspector/virt-inspector.pod @@ -114,6 +114,23 @@ parameter is ignored. If working with untrusted raw-format guest disk images, you should ensure the format is always specified. +=item B<--key> SELECTOR + +Specify a key for LUKS, to automatically open a LUKS device when using +the inspection. C<SELECTOR> can be in one of the following formats: + +=over 4 + +=item B<--key> C<DEVICE>:key:KEY_STRING + +Use the specified C<KEY_STRING> as passphrase. + +=item B<--key> C<DEVICE>:file:FILENAME + +Read the passphrase from F<FILENAME>. + +=back + =item B<--keys-from-stdin> Read key or passphrase parameters from stdin. The default is diff --git a/rescue/rescue.c b/rescue/rescue.c index 80e0e5a93..929c881f0 100644 --- a/rescue/rescue.c +++ b/rescue/rescue.c @@ -163,6 +163,7 @@ main (int argc, char *argv[]) int suggest = 0; char *append_full; int sock; + struct key_store *ks = NULL; g = guestfs_create (); if (g == NULL) diff --git a/sparsify/cmdline.ml b/sparsify/cmdline.ml index ca509b97f..903b7e3c1 100644 --- a/sparsify/cmdline.ml +++ b/sparsify/cmdline.ml @@ -33,6 +33,7 @@ type cmdline = { ignores : string list; zeroes : string list; mode : mode_t; + ks : key_store; } and mode_t @@ -180,4 +181,5 @@ read the man page virt-sparsify(1). ignores = ignores; zeroes = zeroes; mode = mode; + ks = opthandle.ks; } diff --git a/sparsify/cmdline.mli b/sparsify/cmdline.mli index 89848df8b..09a95887a 100644 --- a/sparsify/cmdline.mli +++ b/sparsify/cmdline.mli @@ -24,6 +24,7 @@ type cmdline = { ignores : string list; zeroes : string list; mode : mode_t; + ks : Tools_utils.key_store; } and mode_t diff --git a/sparsify/copying.ml b/sparsify/copying.ml index a33b91e69..43fb15e77 100644 --- a/sparsify/copying.ml +++ b/sparsify/copying.ml @@ -37,7 +37,7 @@ type tmp_place | Directory of string | Block_device of string | Prebuilt_file of string let run indisk outdisk check_tmpdir compress convert - format ignores option tmp_param zeroes + format ignores option tmp_param zeroes ks (* Once we have got past argument parsing and start to create * temporary files (including the potentially massive overlay file), we @@ -188,7 +188,7 @@ You can ignore this warning or change it to a hard failure using the g in (* Decrypt the disks. *) - inspect_decrypt g; + inspect_decrypt g ks; (* Modify SIGINT handler (set first above) to cancel the handle. *) let do_sigint _ diff --git a/sparsify/copying.mli b/sparsify/copying.mli index 50605cc71..d3a561275 100644 --- a/sparsify/copying.mli +++ b/sparsify/copying.mli @@ -22,4 +22,4 @@ type tmp_place | Directory of string | Block_device of string | Prebuilt_file of string -val run : string -> string -> Cmdline.check_t -> bool -> string option -> string option -> string list -> string option -> string option -> string list -> unit +val run : string -> string -> Cmdline.check_t -> bool -> string option -> string option -> string list -> string option -> string option -> string list -> Tools_utils.key_store -> unit diff --git a/sparsify/in_place.ml b/sparsify/in_place.ml index 1eaca7024..50d94e88d 100644 --- a/sparsify/in_place.ml +++ b/sparsify/in_place.ml @@ -30,7 +30,7 @@ open Cmdline module G = Guestfs -let run disk format ignores zeroes +let run disk format ignores zeroes ks (* Connect to libguestfs. *) let g = open_guestfs () in @@ -62,7 +62,7 @@ let run disk format ignores zeroes error ~exit_code:3 (f_"discard/trim is not supported"); (* Decrypt the disks. *) - inspect_decrypt g; + inspect_decrypt g ks; (* Discard non-ignored filesystems that we are able to mount, and * selected swap partitions. diff --git a/sparsify/in_place.mli b/sparsify/in_place.mli index 8f59ea1be..8e6a035fb 100644 --- a/sparsify/in_place.mli +++ b/sparsify/in_place.mli @@ -18,4 +18,4 @@ (** This is the virt-sparsify --in-place mode. *) -val run : string -> string option -> string list -> string list -> unit +val run : string -> string option -> string list -> string list -> Tools_utils.key_store -> unit diff --git a/sparsify/sparsify.ml b/sparsify/sparsify.ml index 9658b4175..6499139fa 100644 --- a/sparsify/sparsify.ml +++ b/sparsify/sparsify.ml @@ -36,8 +36,10 @@ let rec main () | Mode_copying (outdisk, check_tmpdir, compress, convert, option, tmp) -> Copying.run cmdline.indisk outdisk check_tmpdir compress convert cmdline.format cmdline.ignores option tmp cmdline.zeroes + cmdline.ks | Mode_in_place -> In_place.run cmdline.indisk cmdline.format cmdline.ignores cmdline.zeroes + cmdline.ks ) let () = run_main_and_handle_errors main diff --git a/sparsify/virt-sparsify.pod b/sparsify/virt-sparsify.pod index f5e5d2395..8a074ec61 100644 --- a/sparsify/virt-sparsify.pod +++ b/sparsify/virt-sparsify.pod @@ -230,6 +230,23 @@ You can give this option multiple times. Do in-place sparsification instead of copying sparsification. See L</IN-PLACE SPARSIFICATION> below. +=item B<--key> SELECTOR + +Specify a key for LUKS, to automatically open a LUKS device when using +the inspection. C<SELECTOR> can be in one of the following formats: + +=over 4 + +=item B<--key> C<DEVICE>:key:KEY_STRING + +Use the specified C<KEY_STRING> as passphrase. + +=item B<--key> C<DEVICE>:file:FILENAME + +Read the passphrase from F<FILENAME>. + +=back + =item B<--keys-from-stdin> Read key or passphrase parameters from stdin. The default is diff --git a/sysprep/main.ml b/sysprep/main.ml index 37fe6be01..047c38cd3 100644 --- a/sysprep/main.ml +++ b/sysprep/main.ml @@ -36,7 +36,7 @@ let () = Sysprep_operation.bake () let () = Random.self_init () let main () - let operations, g, mount_opts + let operations, g, mount_opts, ks let domain = ref None in let dryrun = ref false in let files = ref [] in @@ -213,10 +213,10 @@ read the man page virt-sysprep(1). add g dryrun; g#launch (); - operations, g, mount_opts in + operations, g, mount_opts, opthandle.ks in (* Decrypt the disks. *) - inspect_decrypt g; + inspect_decrypt g ks; (* Inspection. *) (match Array.to_list (g#inspect_os ()) with diff --git a/sysprep/virt-sysprep.pod b/sysprep/virt-sysprep.pod index d52adf8fe..4df33b1db 100644 --- a/sysprep/virt-sysprep.pod +++ b/sysprep/virt-sysprep.pod @@ -186,6 +186,23 @@ If you have untrusted raw-format guest disk images, you should use this option to specify the disk format. This avoids a possible security problem with malicious guests (CVE-2010-3851). +=item B<--key> SELECTOR + +Specify a key for LUKS, to automatically open a LUKS device when using +the inspection. C<SELECTOR> can be in one of the following formats: + +=over 4 + +=item B<--key> C<DEVICE>:key:KEY_STRING + +Use the specified C<KEY_STRING> as passphrase. + +=item B<--key> C<DEVICE>:file:FILENAME + +Read the passphrase from F<FILENAME>. + +=back + =item B<--keys-from-stdin> Read key or passphrase parameters from stdin. The default is diff --git a/v2v/cmdline.ml b/v2v/cmdline.ml index 29c3e5d4f..cd2d279e9 100644 --- a/v2v/cmdline.ml +++ b/v2v/cmdline.ml @@ -40,6 +40,7 @@ type cmdline = { print_estimate : bool; print_source : bool; root_choice : root_choice; + ks : Tools_utils.key_store; } (* Matches --mac command line parameters. *) @@ -658,5 +659,6 @@ read the man page virt-v2v(1). compressed; debug_overlays; do_copy; in_place; network_map; output_alloc; output_format; output_name; print_estimate; print_source; root_choice; + ks = opthandle.ks; }, input, output diff --git a/v2v/cmdline.mli b/v2v/cmdline.mli index de6281bab..875ddbb9e 100644 --- a/v2v/cmdline.mli +++ b/v2v/cmdline.mli @@ -30,6 +30,7 @@ type cmdline = { print_estimate : bool; print_source : bool; root_choice : Types.root_choice; + ks : Tools_utils.key_store; } val parse_cmdline : unit -> cmdline * Types.input * Types.output diff --git a/v2v/v2v.ml b/v2v/v2v.ml index 1d6b7e5eb..2102560a5 100644 --- a/v2v/v2v.ml +++ b/v2v/v2v.ml @@ -104,7 +104,7 @@ let rec main () g#launch (); (* Decrypt the disks. *) - inspect_decrypt g; + inspect_decrypt g cmdline.ks; (* Inspection - this also mounts up the filesystems. *) (match conversion_mode with diff --git a/v2v/virt-v2v.pod b/v2v/virt-v2v.pod index ae032ed3f..cea9822ef 100644 --- a/v2v/virt-v2v.pod +++ b/v2v/virt-v2v.pod @@ -463,6 +463,23 @@ L</INPUT FROM VDDK> below. If you use this parameter then you may need to use other I<-io vddk*> options to specify how to connect through VDDK. +=item B<--key> SELECTOR + +Specify a key for LUKS, to automatically open a LUKS device when using +the inspection. C<SELECTOR> can be in one of the following formats: + +=over 4 + +=item B<--key> C<DEVICE>:key:KEY_STRING + +Use the specified C<KEY_STRING> as passphrase. + +=item B<--key> C<DEVICE>:file:FILENAME + +Read the passphrase from F<FILENAME>. + +=back + =item B<--keys-from-stdin> Read key or passphrase parameters from stdin. The default is -- 2.17.1
Eric Blake
2018-Sep-19 14:44 UTC
Re: [Libguestfs] [PATCH 2/2] Introduce a --key option in tools that accept keys
On 9/19/18 5:37 AM, Pino Toscano wrote:> The majority of the tools have already options (--echo-keys & > --keys-from-stdin) to deal with LUKS credentials, although there is no > way to automatically provide credentials. --keys-from-stdin is > suboptimal, because it is an usable solution only when there is just ones/an/a/ (English is weird, the choice of 'a' or 'an' before a word beginning with 'u' depends on whether the pronunciation resembles soft 'uh' [an umbrella] or hard 'yoo' [a unicorn]).> device to open, and no other input passed via stdin to the tool (like > the commands for guestfish). > > To overcome this limitation, introduce a new --key option in tools: > * --key /dev/device:file:/filename/with/keyPerhaps safe, if /filename/with/key cannot be read by an attacker.> * --key /dev/device:string:the-actual-keyRather dangerous, as an attacker reading /proc/NNN/cmdline can get at the actual key. But useful for testing. Qemu's solution was more complex - in addition to allowing plaintext keys (for testing), and filenames containing plaintext keys (whether in UTF-8 or base64 form), it also includes ways to pass keys that are themselves encrypted by a shared key, thus making command line passing safe. Qemu's filename solution also has a nice hack: anywhere that /path/to/filename works for passing something in the filesystem, /dev/fdset/NNN can also be used to pass in something via a file descriptor (inherited from the parent process or previously passed via SCM_RIGHTS). So when libvirt wants to pass multiple secrets to qemu, it pre-creates a single shared key stored in a temporary file, passes that file in by fd, and then uses that key to encrypt everything else passed on the command line, so that it only needs one file/fd rather than one per key. https://git.qemu.org/?p=qemu.git;a=blob;f=include/crypto/secret.h;h=edd0e13236#l34 I won't say that you have to repeat qemu's complexity on secret management, but it is food for thought on whether your design is flexible enough.> this way it is possible to pass all the credentials needed for the > specific devices to open, with no risk of conflict with stdin, and also > in a secure way (when using the "file" way). > > On the technical side: this adds a new "key_store" API for the C tools, > making sure it is used only when needed. Partially mirror it also for > the OCaml tools, although there will be a conversion to the C API > because the decryption helpers used are in the common C parts. > ----- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Richard W.M. Jones
2018-Sep-20 10:07 UTC
Re: [Libguestfs] [PATCH 1/2] mltools: create a cmdline_options struct
On Wed, Sep 19, 2018 at 12:37:00PM +0200, Pino Toscano wrote:> diff --git a/common/mltools/tools_utils.mli b/common/mltools/tools_utils.mli > index 2b8c2b78a..99984bfa1 100644 > --- a/common/mltools/tools_utils.mli > +++ b/common/mltools/tools_utils.mli > @@ -74,7 +74,13 @@ val machine_readable : unit -> machine_readable_fn option > readable output to, in case it was enabled via > [--machine-readable]. *) > > -val create_standard_options : Getopt.speclist -> ?anon_fun:Getopt.anon_fun -> ?key_opts:bool -> ?machine_readable:bool -> Getopt.usage_msg -> Getopt.t > +type cmdline_options = { > + getopt : Getopt.t; (** The actual Getopt handle. *) > +} > +(** Structure representing all the data needed for handling command > + line options. *) > + > +val create_standard_options : Getopt.speclist -> ?anon_fun:Getopt.anon_fun -> ?key_opts:bool -> ?machine_readable:bool -> Getopt.usage_msg -> cmdline_options > (** Adds the standard libguestfs command line options to the specified ones, > sorting them, and setting [long_options] to them. > > @@ -84,7 +90,7 @@ val create_standard_options : Getopt.speclist -> ?anon_fun:Getopt.anon_fun -> ?k > [machine_readable] specifies whether add the [--machine-readable] > option. > > - Returns a new [Getopt.t] handle. *) > + Returns a new [cmdline_options] structure. *)There's actually a bug in the old documentation here which should be fixed at the same time. It should use {!...} to link the reference, ie: Returns a new {!cmdline_options} structure. *) This patch is straightforward refactoring, so ACK. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org
Richard W.M. Jones
2018-Sep-20 10:15 UTC
Re: [Libguestfs] [PATCH 2/2] Introduce a --key option in tools that accept keys
This would have been a bit easier to review if the keystore changes had been broken out from the tools changes. On Wed, Sep 19, 2018 at 12:37:01PM +0200, Pino Toscano wrote:> @@ -599,13 +621,21 @@ let is_btrfs_subvolume g fs > if g#last_errno () = Guestfs.Errno.errno_EINVAL then false > else raise exn > > -let inspect_decrypt g > +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_listAn array would be even easier, but I guess you've written the list code now :-) - - - I think Eric's / qemu's shared key stuff sounds very complex, and I wonder who uses it. But in any case what you've proposed is extensible enough that we would be able to add other methods to pass the key in future. It all looks good to me, so ACK. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org
Possibly Parallel Threads
- [PATCH 1/2] mlstdutils/mltools: factorize the machine-readable option
- [PATCH 0/2] RFC: add output selection for --machine-readable
- [PATCH v2 0/2] add output selection for --machine-readable
- [PATCH 0/3] add crypto/LUKS support in some OCaml-based tools
- Re: [PATCH 1/2] mltools: create a cmdline_options struct