There is no reason for the libvirt-dbus daemon to require root privileges. All
it actually needs is ability to connect to libvirtd, which can be achieved by
dropping in a polkit configuration file

Now a libvirt connection to the system bus gives you privileges equivalent to
root, so this doesn't really improve security on its own. It relies on there
being a dbus policy that prevents users from issuing elevated APIs.

For example, a DBus policy could allow non-root users to list VMs on the
system bus and get their status (aka virsh list equiv). In this case, the
security isolation does give some benefit.

Security can be further improved if the admin uses the libvirt polkit file to
restrict what libvirt-dbus is permitted to do.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 configure.ac                       |  5 +++++
 data/Makefile.am                   | 33 +++++++++++++++++++++++++++------
 data/system/libvirt-dbus.rules.in  |  8 ++++++++
 data/system/org.libvirt.conf       |  2 +-
 data/system/org.libvirt.conf.in    | 15 +++++++++++++++
 data/system/org.libvirt.service.in |  2 +-
 libvirt-dbus.spec.in               |  9 +++++++++
 src/main.c                         |  8 ++++++++
 8 files changed, 74 insertions(+), 8 deletions(-)
 create mode 100644 data/system/libvirt-dbus.rules.in
 create mode 100644 data/system/org.libvirt.conf.in

diff --git a/configure.ac b/configure.ac
index 228ea11..aef3d37 100644
--- a/configure.ac
+++ b/configure.ac
@@ -70,6 +70,11 @@ else
 fi
 AC_SUBST(DBUS_SYSTEM_POLICIES_DIR)
 
+LIBVIRT_ARG_WITH([SYSTEM_USER], [username to run system instance as],
+                 ['libvirtdbus'])
+SYSTEM_USER=$with_system_user
+AC_SUBST([SYSTEM_USER])
+
 AC_OUTPUT(Makefile
           data/Makefile
           src/Makefile
diff --git a/data/Makefile.am b/data/Makefile.am
index 58e855f..3f27b02 100644
--- a/data/Makefile.am
+++ b/data/Makefile.am
@@ -9,18 +9,28 @@ system_servicedir = $(DBUS_SYSTEM_SERVICES_DIR)
 system_service_DATA = $(system_service_in_files:.service.in=.service)
 
 system_policy_files = \
-	system/org.libvirt.conf
+	system/org.libvirt.conf.in
 system_policydir = $(DBUS_SYSTEM_POLICIES_DIR)
-system_policy_DATA = $(system_policy_files)
+system_policy_DATA = $(system_policy_files:.conf.in=.conf)
+
+polkit_files = \
+	system/libvirt-dbus.rules.in
+polkit_policydir = $(sysconfdir)/polkit-1/rules.d
+polkit_policy_DATA = $(polkit_files:.rules.in=.rules)
 
 EXTRA_DIST = \
 	$(service_in_files) \
 	$(system_service_in_files) \
-	$(system_policy_files)
+	$(system_policy_files) \
+	$(polkit_files) \
+	$(NULL)
 
 CLEANFILES = \
 	$(service_DATA) \
-	$(system_service_DATA)
+	$(system_service_DATA) \
+	$(system_policy_DATA) \
+	$(polkit_DATA) \
+	$(NULL)
 
 session/org.libvirt.service: session/org.libvirt.service.in
 	$(AM_V_GEN)$(MKDIR_P) session && \
@@ -29,5 +39,16 @@ session/org.libvirt.service: session/org.libvirt.service.in
 
 system/org.libvirt.service: system/org.libvirt.service.in
 	$(AM_V_GEN)$(MKDIR_P) system && \
-		sed -e 's|[@]bindir[@]|$(bindir)|g' < $< > $@-t && \
-			mv $@-t $@
+		sed -e 's|[@]bindir[@]|$(bindir)|g' \
+		    -e 's|[@]SYSTEM_USER[@]|$(SYSTEM_USER)|' \
+			< $< > $@-t && mv $@-t $@
+
+system/org.libvirt.conf: system/org.libvirt.conf.in
+	$(AM_V_GEN)$(MKDIR_P) system && \
+		sed -e 's|[@]SYSTEM_USER[@]|$(SYSTEM_USER)|' \
+			< $< > $@-t && mv $@-t $@
+
+system/libvirt-dbus.rules: system/libvirt-dbus.rules.in
+	$(AM_V_GEN)$(MKDIR_P) system && \
+		sed -e 's|[@]SYSTEM_USER[@]|$(SYSTEM_USER)|' \
+			< $< > $@-t && mv $@-t $@
diff --git a/data/system/libvirt-dbus.rules.in b/data/system/libvirt-dbus.rules.in
new file mode 100644
index 0000000..4eb4ee1
--- /dev/null
+++ b/data/system/libvirt-dbus.rules.in
@@ -0,0 +1,8 @@
+// Allow libvirt-dbus running in dedicated account to use libvirt
+
+polkit.addRule(function(action, subject) {
+    if (action.id == "org.libvirt.unix.manage" &&
+        subject.user == "@SYSTEM_USER@") {
+        return polkit.Result.YES;
+    }
+});
diff --git a/data/system/org.libvirt.conf b/data/system/org.libvirt.conf
index 5cbc732..2b11717 100644
--- a/data/system/org.libvirt.conf
+++ b/data/system/org.libvirt.conf
@@ -4,7 +4,7 @@
 
 <busconfig>
 
-  <policy user="root">
+  <policy user="libvirtdbus">
     <allow own="org.libvirt"/>
     <allow send_destination="org.libvirt"/>
   </policy>
diff --git a/data/system/org.libvirt.conf.in b/data/system/org.libvirt.conf.in
new file mode 100644
index 0000000..fe61b70
--- /dev/null
+++ b/data/system/org.libvirt.conf.in
@@ -0,0 +1,15 @@
+<?xml version="1.0"?>
+<!DOCTYPE busconfig PUBLIC "-//freedesktop//DTD D-BUS Bus Configuration 1.0//EN"
+ "http://www.freedesktop.org/standards/dbus/1.0/busconfig.dtd">
+
+<busconfig>
+
+  <policy user="@SYSTEM_USER@">
+    <allow own="org.libvirt"/>
+  </policy>
+
+  <policy user="root">
+    <allow send_destination="org.libvirt"/>
+  </policy>
+
+</busconfig>
diff --git a/data/system/org.libvirt.service.in b/data/system/org.libvirt.service.in
index 08d32a2..0d3abdd 100644
--- a/data/system/org.libvirt.service.in
+++ b/data/system/org.libvirt.service.in
@@ -1,4 +1,4 @@
 [D-BUS Service]
 Name=org.libvirt
 Exec=@bindir@/libvirt-dbus --system
-User=root
+User=@SYSTEM_USER@
diff --git a/libvirt-dbus.spec.in b/libvirt-dbus.spec.in
index 5be4c22..572300f 100644
--- a/libvirt-dbus.spec.in
+++ b/libvirt-dbus.spec.in
@@ -19,6 +19,7 @@ BuildRequires: systemd-devel >= %{systemd_version}
 Requires: libvirt-libs >= %{libvirt_version}
 Requires: systemd-libs >= %{systemd_version}
 
+Requires(pre): shadow-utils
 
 %description
 This package provides integration between libvirt and the DBus
@@ -37,9 +38,17 @@ rm -rf $RPM_BUILD_ROOT
 %clean
 rm -rf $RPM_BUILD_ROOT
 
+%pre
+getent group libvirtdbus >/dev/null || groupadd -r libvirtdbus
+getent passwd libvirtdbus >/dev/null || \
+    useradd -r -g libvirtdbus -d / -s /sbin/nologin \
+    -c "Libvirt DBus bridge" libvirtdbus
+exit 0
+
 %files
 %defattr(-,root,root,-)
 %doc README COPYING AUTHORS NEWS
+%{_sysconfdir}/polkit-1/rules.d/libvirt-dbus.rules
 %{_bindir}/libvirt-dbus
 %{_datadir}/dbus-1/services/org.libvirt.service
 %{_datadir}/dbus-1/system-services/org.libvirt.service
diff --git a/src/main.c b/src/main.c
index a6a0212..225fb46 100644
--- a/src/main.c
+++ b/src/main.c
@@ -143,6 +143,14 @@ main(int argc, char *argv[])
         }
     }
 
+    if (uri == NULL) {
+        if (system_bus) {
+            uri = "qemu:///system";
+        } else {
+            uri = "qemu:///session";
+        }
+    }
+
     sigemptyset(&mask);
     sigaddset(&mask, SIGTERM);
     sigaddset(&mask, SIGINT);
-- 
2.13.6

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

On Friday, 27 October 2017 16:18:42 CEST Daniel P. Berrange wrote:
> There is no reason for the libvirt-dbus daemon to require root privileges. All
> it actually needs is ability to connect to libvirtd, which can be achieved by
> dropping in a polkit configuration file
> 
> Now a libvirt connection to the system bus gives you privileges equivalent to
> root, so this doesn't really improve security on its own. It relies on there
> being a dbus policy that prevents users from issuing elevated APIs.
> 
> For example, a DBus policy could allow non-root users to list VMs on the
> system bus and get their status (aka virsh list equiv). In this case, the
> security isolation does give some benefit.
> 
> Security can be further improved if the admin uses the libvirt polkit file to
> restrict what libvirt-dbus is permitted to do.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
> [...]
> diff --git a/data/system/org.libvirt.conf b/data/system/org.libvirt.conf
> index 5cbc732..2b11717 100644
> --- a/data/system/org.libvirt.conf
> +++ b/data/system/org.libvirt.conf
> @@ -4,7 +4,7 @@
>  
>  <busconfig>
>  
> -  <policy user="root">
> +  <policy user="libvirtdbus">
>      <allow own="org.libvirt"/>
>      <allow send_destination="org.libvirt"/>
>    </policy>

Most probably this file should be git rm'ed, and added to the
.gitignore.

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

On Fri, Oct 27, 2017 at 04:35:39PM +0200, Pino Toscano wrote:
> On Friday, 27 October 2017 16:18:42 CEST Daniel P. Berrange wrote:
> > There is no reason for the libvirt-dbus daemon to require root privileges. All
> > it actually needs is ability to connect to libvirtd, which can be achieved by
> > dropping in a polkit configuration file
> > 
> > Now a libvirt connection to the system bus gives you privileges equivalent to
> > root, so this doesn't really improve security on its own. It relies on there
> > being a dbus policy that prevents users from issuing elevated APIs.
> > 
> > For example, a DBus policy could allow non-root users to list VMs on the
> > system bus and get their status (aka virsh list equiv). In this case, the
> > security isolation does give some benefit.
> > 
> > Security can be further improved if the admin uses the libvirt polkit file to
> > restrict what libvirt-dbus is permitted to do.
> > 
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > ---
> > [...]
> > diff --git a/data/system/org.libvirt.conf b/data/system/org.libvirt.conf
> > index 5cbc732..2b11717 100644
> > --- a/data/system/org.libvirt.conf
> > +++ b/data/system/org.libvirt.conf
> > @@ -4,7 +4,7 @@
> >  
> >  <busconfig>
> >  
> > -  <policy user="root">
> > +  <policy user="libvirtdbus">
> >      <allow own="org.libvirt"/>
> >      <allow send_destination="org.libvirt"/>
> >    </policy>
> 
> Most probably this file should be git rm'ed, and added to the
> .gitignore.

Urgh yes. It seems the deletion got lost when I did a  git stash and then
unstashed.

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 :|

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