polkit: Allow libvirt group access to libvirtd ro socket

[PATCH] polkit: Allow libvirt group access to libvirtd ro socket

Posted by Jim Fehlig 3 years, 5 months ago

As a normal user, 'virsh connect qemu:///system' and
'virsh connect --readonly qemu:///system' will prompt for root password.
If the user is added to the libvirt group, only
'virsh connect --readonly qemu:///system' will prompt for root password.

The libvirt polkit rules already allow libvirt group members access to
the rw socket. Add a rule allowing to access the ro socket.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---
 src/remote/libvirtd.rules | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/remote/libvirtd.rules b/src/remote/libvirtd.rules
index 01a15fac2e..d9be94fcc4 100644
--- a/src/remote/libvirtd.rules
+++ b/src/remote/libvirtd.rules
@@ -1,5 +1,12 @@
-// Allow any user in the 'libvirt' group to connect to system libvirtd
-// without entering a password.
+// Allow any user in the 'libvirt' group to connect to the system libvirtd
+// ro and rw sockets without entering a password.
+
+polkit.addRule(function(action, subject) {
+    if (action.id == "org.libvirt.unix.monitor" &&
+        subject.isInGroup("libvirt")) {
+        return polkit.Result.YES;
+    }
+});
 
 polkit.addRule(function(action, subject) {
     if (action.id == "org.libvirt.unix.manage" &&
-- 
2.29.2

Re: [PATCH] polkit: Allow libvirt group access to libvirtd ro socket

Posted by Daniel P. Berrangé 3 years, 5 months ago

On Mon, Nov 30, 2020 at 05:28:16PM -0700, Jim Fehlig wrote:
> As a normal user, 'virsh connect qemu:///system' and
> 'virsh connect --readonly qemu:///system' will prompt for root password.
> If the user is added to the libvirt group, only
> 'virsh connect --readonly qemu:///system' will prompt for root password.

This doesn't make sense - the readonly case should never prompt for
a password, since libvirtd.polkit.in grants that permission out of
the box. The libvirtd.rules file should just be extending what is
defined in the main libvirtd.polkit file.

> 
> The libvirt polkit rules already allow libvirt group members access to
> the rw socket. Add a rule allowing to access the ro socket.
> 
> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
> ---
>  src/remote/libvirtd.rules | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/src/remote/libvirtd.rules b/src/remote/libvirtd.rules
> index 01a15fac2e..d9be94fcc4 100644
> --- a/src/remote/libvirtd.rules
> +++ b/src/remote/libvirtd.rules
> @@ -1,5 +1,12 @@
> -// Allow any user in the 'libvirt' group to connect to system libvirtd
> -// without entering a password.
> +// Allow any user in the 'libvirt' group to connect to the system libvirtd
> +// ro and rw sockets without entering a password.
> +
> +polkit.addRule(function(action, subject) {
> +    if (action.id == "org.libvirt.unix.monitor" &&
> +        subject.isInGroup("libvirt")) {
> +        return polkit.Result.YES;
> +    }
> +});
>  
>  polkit.addRule(function(action, subject) {
>      if (action.id == "org.libvirt.unix.manage" &&
> -- 
> 2.29.2
> 
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH] polkit: Allow libvirt group access to libvirtd ro socket

Posted by Jim Fehlig 3 years, 4 months ago

On 12/1/20 2:17 AM, Daniel P. Berrangé wrote:
> On Mon, Nov 30, 2020 at 05:28:16PM -0700, Jim Fehlig wrote:
>> As a normal user, 'virsh connect qemu:///system' and
>> 'virsh connect --readonly qemu:///system' will prompt for root password.
>> If the user is added to the libvirt group, only
>> 'virsh connect --readonly qemu:///system' will prompt for root password.
> 
> This doesn't make sense - the readonly case should never prompt for
> a password, since libvirtd.polkit.in grants that permission out of
> the box.

I thought something smelled a bit fishy. I meant to annotate the patch with "It 
is possible I have a broader polkit config issue", but forgot before sending it 
last night.

And indeed after looking again today with fresh eyes I see the problem is in our 
polkit-default-privs package -> downstream bug. Ignore this patch.

Regards,
Jim

Re: [PATCH] polkit: Allow libvirt group access to libvirtd ro socket

Posted by Neal Gompa 3 years, 4 months ago

On Tue, Dec 1, 2020 at 4:23 PM Jim Fehlig <jfehlig@suse.com> wrote:
>
> On 12/1/20 2:17 AM, Daniel P. Berrangé wrote:
> > On Mon, Nov 30, 2020 at 05:28:16PM -0700, Jim Fehlig wrote:
> >> As a normal user, 'virsh connect qemu:///system' and
> >> 'virsh connect --readonly qemu:///system' will prompt for root password.
> >> If the user is added to the libvirt group, only
> >> 'virsh connect --readonly qemu:///system' will prompt for root password.
> >
> > This doesn't make sense - the readonly case should never prompt for
> > a password, since libvirtd.polkit.in grants that permission out of
> > the box.
>
> I thought something smelled a bit fishy. I meant to annotate the patch with "It
> is possible I have a broader polkit config issue", but forgot before sending it
> last night.
>
> And indeed after looking again today with fresh eyes I see the problem is in our
> polkit-default-privs package -> downstream bug. Ignore this patch.
>

Hah, and I didn't catch this because I rip out the default openSUSE
stuff that ruins usability by restricting polkit too much. :)

Shame on me for not double checking my environment. :)


-- 
真実はいつも一つ！/ Always, there's only one truth!

Re: [PATCH] polkit: Allow libvirt group access to libvirtd ro socket

Posted by Jim Fehlig 3 years, 4 months ago

On 12/1/20 5:15 PM, Neal Gompa wrote:
> On Tue, Dec 1, 2020 at 4:23 PM Jim Fehlig <jfehlig@suse.com> wrote:
>>
>> On 12/1/20 2:17 AM, Daniel P. Berrangé wrote:
>>> On Mon, Nov 30, 2020 at 05:28:16PM -0700, Jim Fehlig wrote:
>>>> As a normal user, 'virsh connect qemu:///system' and
>>>> 'virsh connect --readonly qemu:///system' will prompt for root password.
>>>> If the user is added to the libvirt group, only
>>>> 'virsh connect --readonly qemu:///system' will prompt for root password.
>>>
>>> This doesn't make sense - the readonly case should never prompt for
>>> a password, since libvirtd.polkit.in grants that permission out of
>>> the box.
>>
>> I thought something smelled a bit fishy. I meant to annotate the patch with "It
>> is possible I have a broader polkit config issue", but forgot before sending it
>> last night.
>>
>> And indeed after looking again today with fresh eyes I see the problem is in our
>> polkit-default-privs package -> downstream bug. Ignore this patch.
>>
> 
> Hah, and I didn't catch this because I rip out the default openSUSE
> stuff that ruins usability by restricting polkit too much. :)

It has been a long time, but I've tripped over default-privs in the past. This 
time it was the difference between "restricted" (default in SLES) and "standard" 
(default in openSUSE) rules that got me. See /etc/sysconfig/security.

Regards,
Jim

Re: [PATCH] polkit: Allow libvirt group access to libvirtd ro socket

Posted by Erik Skultety 3 years, 5 months ago

On Tue, Dec 01, 2020 at 09:17:01AM +0000, Daniel P. Berrangé wrote:
> On Mon, Nov 30, 2020 at 05:28:16PM -0700, Jim Fehlig wrote:
> > As a normal user, 'virsh connect qemu:///system' and
> > 'virsh connect --readonly qemu:///system' will prompt for root password.
> > If the user is added to the libvirt group, only
> > 'virsh connect --readonly qemu:///system' will prompt for root password.
> 
> This doesn't make sense - the readonly case should never prompt for
> a password, since libvirtd.polkit.in grants that permission out of
> the box. The libvirtd.rules file should just be extending what is
> defined in the main libvirtd.polkit file.

In fact, this caught my eye and it works as expected on Fedora, can you share a
bit more on what setup this fails for you?

Erik

Re: [PATCH] polkit: Allow libvirt group access to libvirtd ro socket

Posted by Neal Gompa 3 years, 5 months ago

On Mon, Nov 30, 2020 at 7:29 PM Jim Fehlig <jfehlig@suse.com> wrote:
>
> As a normal user, 'virsh connect qemu:///system' and
> 'virsh connect --readonly qemu:///system' will prompt for root password.
> If the user is added to the libvirt group, only
> 'virsh connect --readonly qemu:///system' will prompt for root password.
>
> The libvirt polkit rules already allow libvirt group members access to
> the rw socket. Add a rule allowing to access the ro socket.
>
> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
> ---
>  src/remote/libvirtd.rules | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/src/remote/libvirtd.rules b/src/remote/libvirtd.rules
> index 01a15fac2e..d9be94fcc4 100644
> --- a/src/remote/libvirtd.rules
> +++ b/src/remote/libvirtd.rules
> @@ -1,5 +1,12 @@
> -// Allow any user in the 'libvirt' group to connect to system libvirtd
> -// without entering a password.
> +// Allow any user in the 'libvirt' group to connect to the system libvirtd
> +// ro and rw sockets without entering a password.
> +
> +polkit.addRule(function(action, subject) {
> +    if (action.id == "org.libvirt.unix.monitor" &&
> +        subject.isInGroup("libvirt")) {
> +        return polkit.Result.YES;
> +    }
> +});
>
>  polkit.addRule(function(action, subject) {
>      if (action.id == "org.libvirt.unix.manage" &&
> --
> 2.29.2
>
>

LGTM.

Reviewed-by: Neal Gompa <ngompa13@gmail.com>


-- 
真実はいつも一つ！/ Always, there's only one truth!