1. Patchew
  2. Libvirt
  3. [libvirt] [PATCH 0/2] Fix issue with attempting to use polkit
  4. View patch

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

John Ferlan posted 2 patches 8 years, 9 months ago
[libvirt] [PATCH 2/2] virsh: Don't attempt polkit processing for non local authn/authz
Posted by John Ferlan 8 years, 9 months ago 
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@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;
+
         err = virGetLastError();
         if (!agentCreated &&
             err && err->domain == VIR_FROM_POLKIT &&
-- 
2.9.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] virsh: Don't attempt polkit processing for non local authn/authz
Posted by Michal Privoznik 8 years, 8 months ago 
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@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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] virsh: Don't attempt polkit processing for non local authn/authz
Posted by John Ferlan 8 years, 8 months ago 

On 05/24/2017 10:38 AM, Michal Privoznik wrote:
> 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@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.
> 

True - that's simple enough to adjust... 

So instead of:

    err = virGetLastError();
    if (err && err->domain == VIR_FROM_POLKIT &&
        err->code == VIR_ERR_AUTH_UNAVAILABLE) {
        if (!pkagent && !(pkagent = virPolkitAgentCreate()))
            goto cleanup;
    } else if (err && err->domain == VIR_FROM_POLKIT &&
               err->code == VIR_ERR_AUTH_FAILED) {
        authfail++;
    } else {
        goto cleanup;
    }

I could have

    if (err && err->domain == VIR_FROM_POLKIT) {
        /* 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;
        if (err->code == VIR_ERR_AUTH_UNAVAILABLE) {
            if (!pkagent && !(pkagent = virPolkitAgentCreate()))
                goto cleanup;
        } else if (err->code == VIR_ERR_AUTH_FAILED) {
            authfail++;
        } else {
            goto cleanup;
        }
    } else {
        goto cleanup;
    }


So is there a preferred approach?  See the cover letter...

Tks -

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] virsh: Don't attempt polkit processing for non local authn/authz
Posted by Michal Privoznik 8 years, 8 months ago 
On 05/24/2017 05:45 PM, John Ferlan wrote:
> 
> 
> On 05/24/2017 10:38 AM, Michal Privoznik wrote:
>> 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@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.
>>
> 
> True - that's simple enough to adjust... 
> 
> So instead of:
> 
>     err = virGetLastError();
>     if (err && err->domain == VIR_FROM_POLKIT &&
>         err->code == VIR_ERR_AUTH_UNAVAILABLE) {
>         if (!pkagent && !(pkagent = virPolkitAgentCreate()))
>             goto cleanup;
>     } else if (err && err->domain == VIR_FROM_POLKIT &&
>                err->code == VIR_ERR_AUTH_FAILED) {
>         authfail++;
>     } else {
>         goto cleanup;
>     }
> 
> I could have
> 
>     if (err && err->domain == VIR_FROM_POLKIT) {
>         /* 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;
>         if (err->code == VIR_ERR_AUTH_UNAVAILABLE) {
>             if (!pkagent && !(pkagent = virPolkitAgentCreate()))
>                 goto cleanup;
>         } else if (err->code == VIR_ERR_AUTH_FAILED) {
>             authfail++;
>         } else {
>             goto cleanup;
>         }
>     } else {
>         goto cleanup;
>     }
> 
> 
> So is there a preferred approach?  See the cover letter...

Aha. So these two patches are mutually exclusive? I haven't read it
until now O:-)
Well, I like the first one better because it's more simple. It fixes
just what is broken and is not introducing any bug.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.