[PATCH 0/3] virsh: Don't expect keyboard input from completers
See 3/3 for in depth explanation. After this, there's still one problem - the ssh transport (tcp+ssh://...) because virNetSocketNewConnectSSH() does not really ask for virConnectAuthCallbackPtr. There should be a way to fix it though: from bash script parse URI and append ?no_tty=1 at the end of URI. This directs virNetSocketNewConnectSSH() to execute SSH binary with BatchMode=yes which is documented to suppress keyboard interaction. If we find this ^^ needed I can try to write a patch. But for now, let's fix other transports. Michal Prívozník (3): virnetsshsession: Don't check for auth callbacks in virNetSSHAuthenticatePassword() virnetlibsshsession: Check later for auth callback in virNetLibsshAuthenticatePassword() virsh: Provide no auth callbacks for bash completer src/rpc/virnetlibsshsession.c | 14 +++++++------- src/rpc/virnetsshsession.c | 7 ------- tools/virsh.c | 10 +++++++++- 3 files changed, 16 insertions(+), 15 deletions(-) -- 2.53.0
From: Michal Privoznik <mprivozn@redhat.com> For the VIR_NET_SSH_AUTH_PASSWORD authentication mechanism the virNetSSHAuthenticatePassword() is called. Inside it, virAuthGetPasswordPath() is called to obtain password. Firstly reading from our auth.conf file is attempted and if that fails then corresponding callback from virConnectAuthCallbackPtr is called. But virAuthGetPasswordPath() checks whether the callback is NULL or not. There is no need for virNetSSHAuthenticatePassword() to check it too. Drop the duplicate check. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/rpc/virnetsshsession.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/rpc/virnetsshsession.c b/src/rpc/virnetsshsession.c index 83c8630480..3f60315822 100644 --- a/src/rpc/virnetsshsession.c +++ b/src/rpc/virnetsshsession.c @@ -661,13 +661,6 @@ virNetSSHAuthenticatePassword(virNetSSHSession *sess) VIR_DEBUG("sess=%p", sess); - /* password authentication with interactive password request */ - if (!sess->cred || !sess->cred->cb) { - virReportError(VIR_ERR_SSH, "%s", - _("Can't perform authentication: Authentication callback not provided")); - goto cleanup; - } - /* Try the authenticating the set amount of times. The server breaks the * connection if maximum number of bad auth tries is exceeded */ while (true) { -- 2.53.0
On a Friday in 2026, Michal Privoznik via Devel wrote:
From: Michal Privoznik <mprivozn@redhat.com>
For the VIR_NET_SSH_AUTH_PASSWORD authentication mechanism the virNetSSHAuthenticatePassword() is called. Inside it, virAuthGetPasswordPath() is called to obtain password. Firstly reading from our auth.conf file is attempted and if that fails then corresponding callback from virConnectAuthCallbackPtr is called. But virAuthGetPasswordPath() checks whether the callback is NULL or not. There is no need for
I don't see a check for auth->cb in virAuthGetPasswordPath. In comparison, virAuthGetUsernamePath checks for both auth and auth->cb before calling the callback. Jano
virNetSSHAuthenticatePassword() to check it too. Drop the duplicate check.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/rpc/virnetsshsession.c | 7 ------- 1 file changed, 7 deletions(-)
diff --git a/src/rpc/virnetsshsession.c b/src/rpc/virnetsshsession.c index 83c8630480..3f60315822 100644 --- a/src/rpc/virnetsshsession.c +++ b/src/rpc/virnetsshsession.c @@ -661,13 +661,6 @@ virNetSSHAuthenticatePassword(virNetSSHSession *sess)
VIR_DEBUG("sess=%p", sess);
- /* password authentication with interactive password request */ - if (!sess->cred || !sess->cred->cb) { - virReportError(VIR_ERR_SSH, "%s", - _("Can't perform authentication: Authentication callback not provided")); - goto cleanup; - } - /* Try the authenticating the set amount of times. The server breaks the * connection if maximum number of bad auth tries is exceeded */ while (true) { -- 2.53.0
On 5/15/26 13:31, Ján Tomko wrote:
On a Friday in 2026, Michal Privoznik via Devel wrote:
From: Michal Privoznik <mprivozn@redhat.com>
For the VIR_NET_SSH_AUTH_PASSWORD authentication mechanism the virNetSSHAuthenticatePassword() is called. Inside it, virAuthGetPasswordPath() is called to obtain password. Firstly reading from our auth.conf file is attempted and if that fails then corresponding callback from virConnectAuthCallbackPtr is called. But virAuthGetPasswordPath() checks whether the callback is NULL or not. There is no need for
I don't see a check for auth->cb in virAuthGetPasswordPath.
In comparison, virAuthGetUsernamePath checks for both auth and auth->cb before calling the callback.
Good point. I'll post a follow up patch for that. Michal
From: Michal Privoznik <mprivozn@redhat.com> The first thing that virNetLibsshAuthenticatePassword() does is read password from config file. For this it does not need auth callback. If that password fails to authenticate then corresponding callback from the auth callback is called. This is actual place where auth callback should be checked for. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/rpc/virnetlibsshsession.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/rpc/virnetlibsshsession.c b/src/rpc/virnetlibsshsession.c index 2f590ec1c4..bbcfb19e5c 100644 --- a/src/rpc/virnetlibsshsession.c +++ b/src/rpc/virnetlibsshsession.c @@ -517,13 +517,6 @@ virNetLibsshAuthenticatePassword(virNetLibsshSession *sess) VIR_DEBUG("sess=%p", sess); - /* password authentication with interactive password request */ - if (!sess->cred || !sess->cred->cb) { - virReportError(VIR_ERR_LIBSSH, "%s", - _("Can't perform authentication: Authentication callback not provided")); - return SSH_AUTH_ERROR; - } - /* first try to get password from config */ if (virAuthGetCredential("ssh", sess->hostname, "password", sess->authPath, &password) < 0) @@ -539,6 +532,13 @@ virNetLibsshAuthenticatePassword(virNetLibsshSession *sess) goto error; } + /* password authentication with interactive password request */ + if (!sess->cred || !sess->cred->cb) { + virReportError(VIR_ERR_LIBSSH, "%s", + _("Can't perform authentication: Authentication callback not provided")); + return SSH_AUTH_ERROR; + } + /* Try the authenticating the set amount of times. The server breaks the * connection if maximum number of bad auth tries is exceeded */ while (true) { -- 2.53.0
From: Michal Privoznik <mprivozn@redhat.com> Our bash completion script parses (partially incomplete) command line and looks for two arguments: --readonly and --connect because in the next step it executes virsh with those two arguments like this: virsh --readonly --connect $URI complete -- "text to complete" Now, whenever virsh sees connection URI specified on its cmd line it connects to it right away (before executing any command). This happens inside virshConnect(). Here, virConnectOpenAuth() is called with the default auth callback (virConnectAuthPtrDefault). In majority of the cases this is desirable, as it might ask user for credentials (password for example). But in case of bash completion this is not desired because bash completion script must not expect users to input anything (that's why we even close stdin in cmdComplete()). Therefore, when connecting from virsh that's executed by the bash completion script provide no auth callbacks to prevent virsh from asking for credentials. Resolves: https://gitlab.com/libvirt/libvirt/-/work_items/879 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tools/virsh.c b/tools/virsh.c index fdce3220b3..72d233f98d 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -124,8 +124,16 @@ virshConnect(vshControl *ctl, const char *uri, bool readonly) do { virErrorPtr err; + virConnectAuthPtr auth = virConnectAuthPtrDefault; - if ((c = virConnectOpenAuth(uri, virConnectAuthPtrDefault, + if (ctl->cmd->def->handler == cmdComplete) { + /* When running from a bash completer we need to avoid any kind of + * keyboard input (e.g. ssh asking for a password). To achieve + * this, provide no authentication callbacks. */ + auth = NULL; + } + + if ((c = virConnectOpenAuth(uri, auth, readonly ? VIR_CONNECT_RO : 0))) break; -- 2.53.0
On a Friday in 2026, Michal Privoznik via Devel wrote:
See 3/3 for in depth explanation. After this, there's still one problem - the ssh transport (tcp+ssh://...) because virNetSocketNewConnectSSH() does not really ask for virConnectAuthCallbackPtr. There should be a way to fix it though: from bash script parse URI and append ?no_tty=1 at the end of URI. This directs virNetSocketNewConnectSSH() to execute SSH binary with BatchMode=yes which is documented to suppress keyboard interaction.
If we find this ^^ needed I can try to write a patch. But for now, let's fix other transports.
Michal Prívozník (3): virnetsshsession: Don't check for auth callbacks in virNetSSHAuthenticatePassword() virnetlibsshsession: Check later for auth callback in virNetLibsshAuthenticatePassword() virsh: Provide no auth callbacks for bash completer
src/rpc/virnetlibsshsession.c | 14 +++++++------- src/rpc/virnetsshsession.c | 7 ------- tools/virsh.c | 10 +++++++++- 3 files changed, 16 insertions(+), 15 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
On Fri, May 15, 2026 at 01:05:30PM +0200, Michal Privoznik via Devel wrote:
See 3/3 for in depth explanation. After this, there's still one problem - the ssh transport (tcp+ssh://...) because virNetSocketNewConnectSSH() does not really ask for virConnectAuthCallbackPtr. There should be a way to fix it though: from bash script parse URI and append ?no_tty=1 at the end of URI. This directs virNetSocketNewConnectSSH() to execute SSH binary with BatchMode=yes which is documented to suppress keyboard interaction.
If we find this ^^ needed I can try to write a patch. But for now, let's fix other transports.
Michal Prívozník (3): virnetsshsession: Don't check for auth callbacks in virNetSSHAuthenticatePassword() virnetlibsshsession: Check later for auth callback in virNetLibsshAuthenticatePassword() virsh: Provide no auth callbacks for bash completer
src/rpc/virnetlibsshsession.c | 14 +++++++------- src/rpc/virnetsshsession.c | 7 ------- tools/virsh.c | 10 +++++++++- 3 files changed, 16 insertions(+), 15 deletions(-)
I tested the series. I had to use 'build/run bash' to set shell variables so that the virsh subcommand would use the patched version. I also strace'd that bash so I could be doubly sure it was running the 'virsh complete' subcommand: 2600499 execve("/home/rjones/d/libvirt/build/tools/virsh", ["virsh", "-q", "-c", "esx://root@[redacted]"..., "complete", "--", ""], 0x561fdfcd7000 /* 80 vars */) = 0 Anyhow it fixes the issue I was having, so: Tested-by: Richrad W.M. Jones <rjones@redhat.com> Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW
On 5/15/26 14:19, Richard W.M. Jones wrote:
On Fri, May 15, 2026 at 01:05:30PM +0200, Michal Privoznik via Devel wrote:
See 3/3 for in depth explanation. After this, there's still one problem - the ssh transport (tcp+ssh://...) because virNetSocketNewConnectSSH() does not really ask for virConnectAuthCallbackPtr. There should be a way to fix it though: from bash script parse URI and append ?no_tty=1 at the end of URI. This directs virNetSocketNewConnectSSH() to execute SSH binary with BatchMode=yes which is documented to suppress keyboard interaction.
If we find this ^^ needed I can try to write a patch. But for now, let's fix other transports.
Michal Prívozník (3): virnetsshsession: Don't check for auth callbacks in virNetSSHAuthenticatePassword() virnetlibsshsession: Check later for auth callback in virNetLibsshAuthenticatePassword() virsh: Provide no auth callbacks for bash completer
src/rpc/virnetlibsshsession.c | 14 +++++++------- src/rpc/virnetsshsession.c | 7 ------- tools/virsh.c | 10 +++++++++- 3 files changed, 16 insertions(+), 15 deletions(-)
I tested the series. I had to use 'build/run bash' to set shell variables so that the virsh subcommand would use the patched version.
The bash completion script is written in such way that it should execute ARGV[0]. So in fact all that's needed is: cd build; ./tools/virsh -c esx://... <TAB><TAB>
I also strace'd that bash so I could be doubly sure it was running the 'virsh complete' subcommand:
2600499 execve("/home/rjones/d/libvirt/build/tools/virsh", ["virsh", "-q", "-c", "esx://root@[redacted]"..., "complete", "--", ""], 0x561fdfcd7000 /* 80 vars */) = 0
Anyhow it fixes the issue I was having, so:
Tested-by: Richrad W.M. Jones <rjones@redhat.com>
Thanks for testing! Michal
participants (4)
-
Ján Tomko -
Michal Privoznik -
Michal Prívozník
-
Richard W.M. Jones