[PATCH v2v 0/2] v2v: nbdkit: Don't use password=- parameter.
Part 2 fix for: https://bugzilla.redhat.com/show_bug.cgi?id=1842440 Actually this fix on its own should be sufficient, but probably we want the nbdkit fixes too. Note this uses actual OCaml 4.05 features! ("let open" and the Unix.tcgetattr functions). I checked that both features are available on RHEL 7's OCaml. Rich.
Just refactoring. --- v2v/nbdkit_sources.ml | 42 +++++++++++++++++++----------------------- 1 file changed, 19 insertions(+), 23 deletions(-) diff --git a/v2v/nbdkit_sources.ml b/v2v/nbdkit_sources.ml index bfda91a79..47832011d 100644 --- a/v2v/nbdkit_sources.ml +++ b/v2v/nbdkit_sources.ml @@ -58,7 +58,8 @@ let error_unless_nbdkit_compiled_with_selinux config = error (f_"nbdkit was compiled without SELinux support. You will have to recompile nbdkit with libselinux-devel installed, or else set SELinux to Permissive mode while doing the conversion.") ) -let common_create ?bandwidth ?extra_debug ?extra_env plugin_name plugin_args = +let common_create ?bandwidth ?extra_debug ?extra_env password + plugin_name plugin_args = error_unless_nbdkit_working (); let config = Nbdkit.config () in error_unless_nbdkit_min_version config; @@ -136,6 +137,15 @@ let common_create ?bandwidth ?extra_debug ?extra_env plugin_name plugin_args = List.fold_left (fun cmd (k, v) -> Nbdkit.add_arg cmd k v) cmd (plugin_args @ rate_args) in + (* Handle the password parameter specially. *) + let cmd = + match password with + | NoPassword -> cmd + | AskForPassword -> + Nbdkit.add_arg cmd "password" "-" + | PasswordFile password_file -> + Nbdkit.add_arg cmd "password" ("+" ^ password_file) in + cmd (* VDDK libraries are located under lib32/ or lib64/ relative to the @@ -223,20 +233,16 @@ See also the virt-v2v-input-vmware(1) manual.") libNN let get_args () = List.rev !args in add_arg, get_args in - let password_param = - match password_file with - | None -> - (* nbdkit asks for the password interactively *) - "password", "-" - | Some password_file -> - (* nbdkit reads the password from the file *) - "password", "+" ^ password_file in add_arg ("server", server); add_arg ("user", user); - add_arg password_param; add_arg ("vm", sprintf "moref=%s" moref); add_arg ("file", path); + let password = + match password_file with + | None -> AskForPassword + | Some password_file -> PasswordFile password_file in + (* The passthrough parameters. *) Option.may (fun s -> add_arg ("config", s)) config; Option.may (fun s -> add_arg ("cookie", s)) cookie; @@ -251,7 +257,7 @@ See also the virt-v2v-input-vmware(1) manual.") libNN let debug_flag = if version >= (1, 17, 10) then Some ("vddk.datapath", "0") else None in - common_create ?bandwidth ?extra_debug:debug_flag ?extra_env:env + common_create ?bandwidth ?extra_debug:debug_flag ?extra_env:env password "vddk" (get_args ()) (* Create an nbdkit module specialized for reading from SSH sources. *) @@ -267,14 +273,9 @@ let create_ssh ?bandwidth ~password ?port ~server ?user path = add_arg ("host", server); Option.may (fun s -> add_arg ("port", s)) port; Option.may (fun s -> add_arg ("user", s)) user; - (match password with - | NoPassword -> () - | AskForPassword -> add_arg ("password", "-") - | PasswordFile password_file -> add_arg ("password", "+" ^ password_file) - ); add_arg ("path", path); - common_create ?bandwidth "ssh" (get_args ()) + common_create ?bandwidth password "ssh" (get_args ()) (* Create an nbdkit module specialized for reading from Curl sources. *) let create_curl ?bandwidth ?cookie ~password ?(sslverify=true) ?user url = @@ -287,18 +288,13 @@ let create_curl ?bandwidth ?cookie ~password ?(sslverify=true) ?user url = add_arg, get_args in Option.may (fun s -> add_arg ("user", s)) user; - (match password with - | NoPassword -> () - | AskForPassword -> add_arg ("password", "-") - | PasswordFile password_file -> add_arg ("password", "+" ^ password_file) - ); (* https://bugzilla.redhat.com/show_bug.cgi?id=1146007#c10 *) add_arg ("timeout", "2000"); Option.may (fun s -> add_arg ("cookie", s)) cookie; if not sslverify then add_arg ("sslverify", "false"); add_arg ("url", url); - common_create ?bandwidth "curl" (get_args ()) + common_create ?bandwidth password "curl" (get_args ()) let run cmd = let sock, _ = Nbdkit.run_unix cmd in -- 2.25.0
This was broken with all nbdkit plugins, some in more ways than others. Because we start nbdkit in the background and wait 30 seconds for it to start running, the user had only 30 seconds to type in a password before we timed out the process. In addition with the VDDK plugin password=- had been broken ever since we changed the plugin to use a reexec (https://www.redhat.com/archives/libguestfs/2020-June/msg00012.html). The solution is to read the password ourselves and pass it to nbdkit as a private file. --- v2v/nbdkit_sources.ml | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/v2v/nbdkit_sources.ml b/v2v/nbdkit_sources.ml index 47832011d..f5e919116 100644 --- a/v2v/nbdkit_sources.ml +++ b/v2v/nbdkit_sources.ml @@ -142,7 +142,26 @@ let common_create ?bandwidth ?extra_debug ?extra_env password match password with | NoPassword -> cmd | AskForPassword -> - Nbdkit.add_arg cmd "password" "-" + (* Because we will start nbdkit in the background and then wait + * for 30 seconds for it to start up, we cannot use the + * password=- feature of nbdkit to read the password + * interactively (since in the words of the movie the user has + * only "30 seconds to comply"). In any case this feature broke + * in the VDDK plugin in nbdkit 1.18 and 1.20. So in the + * AskForPassword case we read the password here. + *) + printf "password: "; + let open Unix in + let orig = tcgetattr stdin in + let tios = { orig with c_echo = false } in + tcsetattr stdin TCSAFLUSH tios; (* Disable echo. *) + let password = read_line () in + tcsetattr stdin TCSAFLUSH orig; (* Restore echo. *) + printf "\n"; + let password_file = Filename.temp_file "v2vnbdkit" ".txt" in + unlink_on_exit password_file; + with_open_out password_file (fun chan -> output_string chan password); + Nbdkit.add_arg cmd "password" ("+" ^ password_file) | PasswordFile password_file -> Nbdkit.add_arg cmd "password" ("+" ^ password_file) in -- 2.25.0
On Monday, 1 June 2020 18:43:36 CEST Richard W.M. Jones wrote:
Part 2 fix for: https://bugzilla.redhat.com/show_bug.cgi?id=1842440
Actually this fix on its own should be sufficient, but probably we want the nbdkit fixes too.
Note this uses actual OCaml 4.05 features! ("let open" and the Unix.tcgetattr functions).
BTW Unix.tcgetattr is very old, even old 3.11 had it (found in some old tests of mine). LGTM; IMHO printing user/host in the password prompt would make it more clear as to for which host the password is asked. -- Pino Toscano
participants (2)
-
Pino Toscano -
Richard W.M. Jones