[Libguestfs] [p2v PATCH 1/4] ssh: shield virt-v2v from bash RC files

[Richard W.M. Jones]

On Thu, Jan 19, 2023 at 01:14:45PM +0100, Laszlo Ersek wrote:
> I don't have virt-v2v installed system-wide on my RHEL9 laptop, I rely on
> the various "./run" scripts to put everything that virt-p2v's "make check"
> requires on the PATH environment variable.
> 
> However, "test-virt-p2v-nbdkit.sh" still breaks for me; it complains that
> "virt-v2v --version" cannot be executed by test_connection(), due to
> "virt-v2v" not being found.  Prefixing the "virt-v2v --version" invocation
> in test_connection() with "echo \"$PATH\"; ", I've determined from
> "test-virt-p2v-nbdkit.sh.log" that the *non-appending* PATH=... variable
> assignment from my $HOME/.bashrc takes effect.  It wipes out the PATH
> changes from the "./run" scripts, hiding virt-v2v.
> 
> I've added the following snippet to my $HOME/.bashrc file, for debugging:
> 
> > XXX=mess-$(date --rfc-3339=ns)--$$.log
> > /bin/pstree -a -A -l -n -p $$ >| "$HOME/tmp/$XXX"
> 
> so that whichever shell read the RC file create a log file, named with a
> nanosecond-resolution timestamp and the shell's PID, and record the "path"
> in the process tree that lead to the shell.
> 
> The snippet created the following two files:
> 
> > mess-2023-01-18 09:33:49.896065330+01:00--36312.log
> > mess-2023-01-18 09:33:49.937365639+01:00--36312.log
> 
> containing, respectively:
> 
> > bash,36312
> >   `-pstree,36315 -a -A -l -n -p 36312
> 
> and
> 
> > bash,36312 --noediting --noprofile
> >   `-pstree,36320 -a -A -l -n -p 36312
> 
> Note that the PID of the shell is unchanged, but the pstree PID changes.
> This means that the same PID (same process) reads the bash RC file twice
> -- which can only be explained by the *image* of the process being
> replaced, from bash, to bash.
> 
> So the problem happens in two places:
> 
> - First, when we "exec" the interactive shell in "test-virt-p2v-ssh.sh",
>   that is, our ssh "shim".  Interactive *non-login* shells read the RC
>   file, unless the "--norc" option is passed.
> 
> - Second, when we "exec" bash from start_ssh(), on the remote machine.
> 
>   This invocation already passes the "--noprofile" option, but that has no
>   effect.  "--noprofile" prevents the shell from reading
>   "$HOME/.bash_profile" when the shell is a *login* shell, regardless of
>   whether it is interactive or not.  Because the existent "--noprofile"
>   option does not prevent the symptom, we can determine that the remote
>   shell started by start_ssh() is a *non-login* shell, and that it's also
>   interactive (otherwise it wouldn't read the RC file).  Thus, we need to
>   pass "--norc" here as well.
> 
>   (While I believe, based on the above, that "--noprofile" is superfluous,
>   I'd like to avoid any potential regressions here, so I'm keeping
>   "--noprofile" too.)
> 
> Append "--norc" to both command lines.
> 
> Signed-off-by: Laszlo Ersek <lersek at redhat.com>
> ---
>  ssh.c                | 2 +-
>  test-virt-p2v-ssh.sh | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/ssh.c b/ssh.c
> index aeb57584cf55..513a20318359 100644
> --- a/ssh.c
> +++ b/ssh.c
> @@ -469,7 +469,7 @@ start_ssh (unsigned spawn_flags, struct config *config,
>     * We don't know how command line editing is set up
>     * (https://bugzilla.redhat.com/1314244#c9).
>     */
> -  if (mexp_printf (h, "exec bash --noediting --noprofile\n") == -1) {
> +  if (mexp_printf (h, "exec bash --noediting --noprofile --norc\n") == -1) {
>      set_ssh_mexp_error ("mexp_printf");
>      mexp_close (h);
>      return NULL;
> diff --git a/test-virt-p2v-ssh.sh b/test-virt-p2v-ssh.sh
> index 8a14b71fbd4b..f8b86b539ffe 100755
> --- a/test-virt-p2v-ssh.sh
> +++ b/test-virt-p2v-ssh.sh
> @@ -57,4 +57,4 @@ while true ; do
>  done
>  
>  # Now run the interactive shell.
> -exec bash
> +exec bash --norc

Yes, this seems reasonable given that we were already using --noprofile.

ACK.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top