[virt-tools-list] [virt-manager PATCH 2/2] cli: stop forking into the background

[Cole Robinson]

On 05/01/2018 09:23 AM, Daniel P. Berrangé wrote:
> On Tue, May 01, 2018 at 09:05:49AM -0400, Cole Robinson wrote:
>> On 05/01/2018 04:01 AM, Daniel P. Berrangé wrote:
>>> On Mon, Apr 30, 2018 at 12:47:38PM -0400, Cole Robinson wrote:
>>>> On 04/30/2018 12:19 PM, Daniel P. Berrangé wrote:
>>>>> On Mon, Apr 30, 2018 at 12:12:08PM -0400, Cole Robinson wrote:
>>>>>> On 04/30/2018 08:33 AM, Daniel P. Berrangé wrote:
>>>>>>> The behaviour whereby virt-manager forks into the background was added
>>>>>>> way back in:
>>>>>>>
>>>>>>>   commit 99c92b9471a6a55859307071aa4a0e712991f158
>>>>>>>   Author: Daniel P. Berrange <berrange at redhat.com>
>>>>>>>   Date:   Mon Sep 10 20:10:20 2007 -0400
>>>>>>>
>>>>>>>     Refactor startup to drop controlling TTY, avoiding annoying SSH prompts
>>>>>>>
>>>>>>> While it achieves its stated goal, this is quite a big hammer to use
>>>>>>> with unpleasant side effects. Most end users will launch virt-manager
>>>>>>> from the desktop which will fork the app into the background already.
>>>>>>> Even when running from the command line, modern desktop environments
>>>>>>> will have things setup up so that all SSH prompts are intercepted and
>>>>>>> presented via a graphical window. Forking into the background causes
>>>>>>> extra pain for developers as warnings that would otherwise appear on
>>>>>>> stderr get lost e.g.
>>>>>>>
>>>>>>>   commit 24a8b66b35c92bed919a4a6beb7c7fb80e85b3b2
>>>>>>>   Author: Daniel P. Berrangé <berrange at redhat.com>
>>>>>>>   Date:   Wed Apr 4 14:35:40 2018 +0100
>>>>>>>
>>>>>>>     avoid referencing ConnectError if it is None
>>>>>>>
>>>>>>>     Currently it throws an exception at startup which is hidden unless you
>>>>>>>     run with --no-fork
>>>>>>>
>>>>>>> The limited benefit of forking is not worth the pain it causes, so
>>>>>>> just start "normally" as any other GTK app would.
>>>>>>
>>>>>> I'd love to be able to drop this, but consider this case: install
>>>>>> virt-manager to /usr/share with this patch, then run it from gnome-shell
>>>>>> and try to connect to an ssh host that requires a password. ssh will
>>>>>> print the password prompt to stdout which the user doesn't see, and the
>>>>>> connection attempt just hangs until whenever ssh times out.
>>>>>>
>>>>>> This is the crux of the problem and I don't know any way around it.
>>>>>> There's no way to force ssh to launch askpass without forking+setsid. if
>>>>>> we wanted to drop passwordauth entirely for ssh and mandate keys or
>>>>>> other auth, we can extend libvirt to allow passing -o
>>>>>> PasswordAuthentication=no to ssh, but then it'd still be years before we
>>>>>> could drop the --no-fork behavior.
>>>>>
>>>>> You can add the 'no_tty=1'  URI parameter to any libvirt remote URI.
>>>>>
>>>>> This adds  '-T -o BatchMode=yes -e none':
>>>>>
>>>>>      -T      Disable pseudo-terminal allocation.
>>>>>      -e escape_char
>>>>>              Sets the escape character for sessions with a
>>>>>              pty (default: ‘~’).  The escape character is
>>>>>              only recognized at the beginning of a line.
>>>>>              The escape character followed by a dot (‘.’)
>>>>>              closes the connection; followed by control-Z
>>>>>              suspends the connection; and followed by
>>>>>              itself sends the escape character once.  Set‐
>>>>>              ting the character to “none” disables any
>>>>>              escapes and makes the session fully transpar‐
>>>>>              ent.
>>>>>
>>>>>      BatchMode
>>>>>              If set to yes, passphrase/password querying
>>>>>              will be disabled.  This option is useful in
>>>>>              scripts and other batch jobs where no user is
>>>>>              present to supply the password.  The argument
>>>>>              must be yes or no (the default).
>>>>>
>>>>> Even in BatchMode, the graphical agent prompt will still be used
>>>>> for passphrases to unlock keys.
>>>>>
>>>>
>>>> Ahh yes I definitely knew about that at one point, thanks for the
>>>> reminder. Though this will kill keyless ssh access with virt-manager,
>>>> unclear to me if it's worth the tradeoff
>>>
>>> How does keyless ssh access currently work though - it can't prompt on
>>> the terminal if we've forked into background, and it doesn't appear to
>>> ask for passwords in the graphical ssh agent dialog ? So I'm unclear
>>> what we'd be using by adding no_tty=1.  Amuzingly, I actually added
>>> this no_tty=1 feature to libvirt just after doing this fork hack in
>>> virt-manager, and then forgot to ever make virt-manager use no_tty=1 :-)
>>
>> If openssh-askpass is installed, ssh will launch that and ask for the
>> password. Last I checked it can only be convinced to launch it if we
>> fork though
> 
> I question whether that is a genuinely useful solution for virt-manager
> though. I just tested it and to open virt-manager and display a single
> guest console I am prompted to enter the same password 5 times. For
> each additional guest I look at I need to enter another 4 passwords.
> If SPICE features like file sharing, USB sharing and so on are used,
> it prompts for the password even more times, because of the number of
> sockets SPICE needs.

Yeah for spice it's not realistically usable IMO, though I know some
people have tried due to bug reports over the years. VNC isn't so bad.
Since we control the console ssh side of things we potentially have more
options though, like maybe using paramiko, though I don't know its full
capabilities here so maybe it doesn't give us anything.

askpass+virt-manager definitely gets some users, at least people who are
first using the app or trying to connect to a one off server, see a
google search for 'virt-manager askpass'. It's not a killer feature but
removing it to facilitate dropping the fork behavior isn't a clear win IMO.

Thanks,
Cole