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

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(a)redhat.com&gt; > --- > 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.