[libvirt] [PATCH v2] polkit: Allow password-less access for 'libvirtadm' group

[Cole Robinson]

On 04/29/2015 11:28 AM, Daniel P. Berrange wrote:
> On Wed, Apr 29, 2015 at 11:04:42AM -0400, Cole Robinson wrote:
>> Many users, who admin their own machines, want to be able to access
>> system libvirtd via tools like virt-manager without having to enter
>> a root password. Just google 'virt-manager without password' and
>> you'll find many hits. I've read at least 5 blog posts over the years
>> describing slightly different ways of achieving this goal.
>>
>> Let's finally add official support for this.
>>
>> Install a polkit-1 rules file granting password-less auth for any user
>> in the new 'libvirtadm' group. Create the group on RPM install
>>
>> https://bugzilla.redhat.com/show_bug.cgi?id=957300
>> ---
>> v2:
>> - Name the group libvirtadm (danpb)
>> - Name the source file libvirt.rules and rename on install (eblake)
>>
>>  daemon/Makefile.am   | 13 +++++++++++++
>>  daemon/libvirt.rules |  9 +++++++++
>>  libvirt.spec.in      | 15 +++++++++++++--
>>  3 files changed, 35 insertions(+), 2 deletions(-)
>>  create mode 100644 daemon/libvirt.rules
>>
>> diff --git a/daemon/Makefile.am b/daemon/Makefile.am
>> index 300b9a5..974feed 100644
>> --- a/daemon/Makefile.am
>> +++ b/daemon/Makefile.am
>> @@ -53,6 +53,7 @@ EXTRA_DIST =						\
>>  	libvirtd.init.in				\
>>  	libvirtd.upstart				\
>>  	libvirtd.policy.in				\
>> +	libvirt.rules					\
>>  	libvirtd.sasl					\
>>  	libvirtd.service.in				\
>>  	libvirtd.socket.in				\
>> @@ -233,6 +234,8 @@ policyauth = auth_admin_keep_session
>>  else ! WITH_POLKIT0
>>  policydir = $(datadir)/polkit-1/actions
>>  policyauth = auth_admin_keep
>> +rulesdir = $(datadir)/polkit-1/rules.d
>> +rulesfile = libvirt.rules
>>  endif ! WITH_POLKIT0
>>  endif WITH_POLKIT
>>  
>> @@ -263,9 +266,19 @@ if WITH_POLKIT
>>  install-data-polkit::
>>  	$(MKDIR_P) $(DESTDIR)$(policydir)
>>  	$(INSTALL_DATA) libvirtd.policy $(DESTDIR)$(policydir)/org.libvirt.unix.policy
>> +if ! WITH_POLKIT0
>> +	$(MKDIR_P) $(DESTDIR)$(rulesdir)
>> +	$(INSTALL_DATA) $(srcdir)/$(rulesfile) $(DESTDIR)$(rulesdir)/50-libvirt.rules
>> +endif ! WITH_POLKIT0
>> +
>>  uninstall-data-polkit::
>>  	rm -f $(DESTDIR)$(policydir)/org.libvirt.unix.policy
>>  	rmdir $(DESTDIR)$(policydir) || :
>> +if ! WITH_POLKIT0
>> +	rm -f $(DESTDIR)$(rulesdir)/50-libvirt.rules
>> +	rmdir $(DESTDIR)$(rulesdir) || :
>> +endif ! WITH_POLKIT0
>> +
>>  else ! WITH_POLKIT
>>  install-data-polkit::
>>  uninstall-data-polkit::
>> diff --git a/daemon/libvirt.rules b/daemon/libvirt.rules
>> new file mode 100644
>> index 0000000..e70c09b
>> --- /dev/null
>> +++ b/daemon/libvirt.rules
>> @@ -0,0 +1,9 @@
>> +// Allow any user in the 'libvirtadm' group to connect to system libvirtd
>> +// without entering a password.
>> +
>> +polkit.addRule(function(action, subject) {
>> +    if (action.id == "org.libvirt.unix.manage" &&
>> +        subject.isInGroup("libvirtadm")) {
>> +        return polkit.Result.YES;
>> +    }
>> +});
>> diff --git a/libvirt.spec.in b/libvirt.spec.in
>> index 20af502..10a28a2 100644
>> --- a/libvirt.spec.in
>> +++ b/libvirt.spec.in
>> @@ -1645,9 +1645,9 @@ then
>>  fi
>>  
>>  %if %{with_libvirtd}
>> +%pre daemon
>>      %if ! %{with_driver_modules}
>>          %if %{with_qemu}
>> -%pre daemon
>>              %if 0%{?fedora} || 0%{?rhel} >= 6
>>  # We want soft static allocation of well-known ids, as disk images
>>  # are commonly shared across NFS mounts by id rather than name; see
>> @@ -1661,11 +1661,21 @@ if ! getent passwd qemu >/dev/null; then
>>      useradd -r -g qemu -G kvm -d / -s /sbin/nologin -c "qemu user" qemu
>>    fi
>>  fi
>> -exit 0
>>              %endif
>>          %endif
>>      %endif
>>  
>> +    %if %{with_polkit}
>> +        %if 0%{?fedora} || 0%{?rhel} >= 6
>> +# 'libvirtadm' group is just to allow password-less polkit access to
>> +# libvirtd. The uid number is irrelevant, so we use dynamic allocation
>> +# described at the above link.
>> +getent group libvirtadm >/dev/null || groupadd -r libvirtadm
> 
> Hmm, you know I think we should probably file a bug against the
> 'setup' RPM in Fedora to request allocation of a group ID value
> for this, so we can default to using a fixed group ID, as we do
> for other users/groups we create
> 

The recommendations don't seem to suggest that:

https://fedoraproject.org/wiki/Packaging:UsersAndGroups?rd=Packaging/UsersAndGroups#Allocation_Strategies

Quote:  Soft static allocation is only appropriate for packages where the UID
or GID values are shared between computers

I can't think of a good case when we would need that for libvirtadm...
cetainly no files need to be owned by it

- Cole