[libvirt] [PATCH 2/2] virsh: Don't attempt polkit processing for non local authn/authz

[Michal Privoznik]

On 05/11/2017 05:04 PM, John Ferlan wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1374126
> 
> Due to how the processing for authentication using polkit works, the
> virshConnect code must first "attempt" an virConnectOpenAuth and then
> check for a "special" return error code VIR_ERR_AUTH_UNAVAILABLE in
> order to attempt to "retry" the authentication after performing a creation
> of a pkttyagent to handle the challenge/response for the client.
> 
> However, attempting to use a remote connection, (such as perhaps
> "qemu+ssh://someuser@localhost/system"), will cause a never ending
> loop since attempting to generate a pkttyagent would fail for the
> network client connection resulting in a never ending loop since the
> return code is always VIR_ERR_AUTH_UNAVAILABLE from virPolkitCheckAuth.
> The only way out of the loop is a forced quit (e.g. ctrl-c) as the
> @authfail wouldn't be incremented as a result of a failed authn from
> pkttyagent.
> 
> So rather than take any extra step for which the only result will be
> a failure, let's check if there is a URI and if it's not using ":///",
> then just fail.
> 
> This resolves the never ending loop and will generate an error:
> 
> error: failed to connect to the hypervisor
> error: authentication unavailable: no polkit agent available to authenticate action 'org.libvirt.unix.manage'
> 
> NB: If the authentication was for a sufficiently privileged client, such as
> qemu+ssh://root@localhost/system, then the remoteDispatchAuthList "allows"
> the authentication to use libvirt since @callerUid would be 0.
> 
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  tools/virsh.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/tools/virsh.c b/tools/virsh.c
> index 1f5c2b1..be368ba 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -166,6 +166,11 @@ virshConnect(vshControl *ctl, const char *uri, bool readonly)
>          if (readonly)
>              goto cleanup;
>  
> +        /* No URI or indication of a requesting a remote connection, then
> +         * polkit will not work for the authentication/authorization */
> +        if (!uri || !(strstr(uri, ":///")))
> +            goto cleanup;

But we can get here even without polkit, right? Therefore it'd be much
more safer to check err && err->domain == VIR_FROM_POLKIT.

Michal