Skip to contentSkip to conversation

D-Bus policy fixes

PATCH Superseded Jussi Kukkonen 1 participant 5 messages Sep 30, 2015 – Oct 1, 2015

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

cover D-Bus policy fixes

The major patch in the series is the bluez one: Bluez D-Bus policy was incorrectly written so it actually allowed access to system services _other than bluetoothd_ overriding the default deny policy on the system bus. Fixing this may naturally affect other system services too.

The patches I'm sending are for master but I believe both fido and dizzy behave similarly. I can send a patch for those as well but am not sure what to include there: I'm guessing people now have services running that are expecting an open-by-default system bus -- closing it now will require good release notes at the very least.

So RFC on fido and dizzy: The best I can think of is taking the bluez patch, patching in an xuser allow policy for bluez, and making the (practical) policy change very clear in the release notes.

- Jussi

The following changes since commit 4bc3f0994e68b3302a0523a3156dd0dca0cac7a0:

bitbake: toaster: move clones into subdirectory (2015-09-29 14:11:39 +0100)

are available in the git repository at:

git://git.yoctoproject.org/poky-contrib jku/dbus-policy
http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=jku/dbus-policy

Jussi Kukkonen (3):
bluez5: Use upstream D-Bus policy
dbus: Use the xuser policy file
xuser-account: Take over xuser specific D-Bus policy

 meta/recipes-connectivity/bluez5/bluez5.inc        |  5 +--
 .../bluez5/bluez5/bluetooth.conf                   | 17 ---------
 meta/recipes-connectivity/connman/connman.inc      |  1 -
 .../connman/add_xuser_dbus_permission.patch        | 43 ----------------------
 meta/recipes-connectivity/connman/connman_1.30.bb  |  1 -
 meta/recipes-core/dbus/dbus.inc                    |  1 +
 ...-Apply-xuser-specific-policies-if-present.patch | 33 +++++++++++++++++
 .../user-creation/files/system-xuser.conf          | 15 ++++++++
 .../user-creation/xuser-account_0.1.bb             |  6 ++-
 9 files changed, 55 insertions(+), 67 deletions(-)
 delete mode 100644 meta/recipes-connectivity/bluez5/bluez5/bluetooth.conf
 delete mode 100644 meta/recipes-connectivity/connman/connman/add_xuser_dbus_permission.patch
 create mode 100644 meta/recipes-core/dbus/dbus/0001-Apply-xuser-specific-policies-if-present.patch
 create mode 100644 meta/recipes-support/user-creation/files/system-xuser.conf
Signature · 1 line
2.1.4
#1 lore raw

PATCH 1/3 bluez5: Use upstream D-Bus policy

The Bluez D-Bus policy is much too open and affects not just bluez but all system services: Use upstream policy configuration instead.

This change has a chance of affecting other D-Bus services: the bug that is fixed here may have hidden problems in other policies.

[YOCTO #8414]

---
 meta/recipes-connectivity/bluez5/bluez5.inc            |  5 ++---
 meta/recipes-connectivity/bluez5/bluez5/bluetooth.conf | 17 -----------------
 2 files changed, 2 insertions(+), 20 deletions(-)
 delete mode 100644 meta/recipes-connectivity/bluez5/bluez5/bluetooth.conf
meta/recipes-connectivity/bluez5/bluez5.inc +2 -3
index 039c443..df42c88 100644
@@ -18,7 +18,6 @@ PACKAGECONFIG[experimental] = "--enable-experimental,--disable-experimental,"
 
 SRC_URI = "\
     ${KERNELORG_MIRROR}/linux/bluetooth/bluez-${PV}.tar.xz \
-    file://bluetooth.conf \
 "
 S = "${WORKDIR}/bluez-${PV}"
 
@@ -53,8 +52,8 @@ do_install_append() {
 	if [ -f ${S}/profiles/input/input.conf ]; then
 	    install -m 0644 ${S}/profiles/input/input.conf ${D}/${sysconfdir}/bluetooth/
 	fi
-	# at_console doesn't really work with the current state of OE, so punch some more holes so people can actually use BT
-	install -m 0644 ${WORKDIR}/bluetooth.conf ${D}/${sysconfdir}/dbus-1/system.d/
+
+	install -m 0644 ${S}/src/bluetooth.conf ${D}/${sysconfdir}/dbus-1/system.d/
 
 	# Install desired tools that upstream leaves in build area
         for f in ${NOINST_TOOLS} ; do
meta/recipes-connectivity/bluez5/bluez5/bluetooth.conf deleted +0 -17
deleted file mode 100644
index 26845bb..0000000
@@ -1,17 +0,0 @@
-<!-- This configuration file specifies the required security policies
-     for Bluetooth core daemon to work. -->
-
-<!DOCTYPE busconfig PUBLIC "-//freedesktop//DTD D-BUS Bus Configuration 1.0//EN"
- "http://www.freedesktop.org/standards/dbus/1.0/busconfig.dtd">
-<busconfig>
-
-  <!-- ../system.conf have denied everything, so we just punch some holes -->
-
-  <policy context="default">
-    <allow own="org.bluez"/>
-    <allow send_destination="org.bluez"/>
-    <allow send_interface="org.bluez.Agent1"/>
-    <allow send_type="method_call"/>
-  </policy>
-
-</busconfig>
Signature · 1 line
2.1.4
#2 lore raw

PATCH 2/3 dbus: Use the xuser policy file

Apply the xuser-related policies (if they have been installed by xuser-account) after the service-specific policies are applied.

---
 meta/recipes-core/dbus/dbus.inc                    |  1 +
 ...-Apply-xuser-specific-policies-if-present.patch | 33 ++++++++++++++++++++++
 2 files changed, 34 insertions(+)
 create mode 100644 meta/recipes-core/dbus/dbus/0001-Apply-xuser-specific-policies-if-present.patch
meta/recipes-core/dbus/dbus.inc +1 -0
index 3971081..59e3afe 100644
@@ -17,6 +17,7 @@ SRC_URI = "http://dbus.freedesktop.org/releases/dbus/dbus-${PV}.tar.gz \
            file://dbus-1.init \
            file://os-test.patch \
            file://clear-guid_from_server-if-send_negotiate_unix_f.patch \
+           file://0001-Apply-xuser-specific-policies-if-present.patch \
 "
 
 inherit useradd autotools pkgconfig gettext update-rc.d
meta/recipes-core/dbus/dbus/0001-Apply-xuser-specific-policies-if-present.patch new file +33 -0
new file mode 100644
index 0000000..01a4870
@@ -0,0 +1,33 @@
+From 3a37ec47ffc9e4d34ac726d649a822cdead1b38f Mon Sep 17 00:00:00 2001
+From: Jussi Kukkonen <[email protected]>
+Date: Wed, 30 Sep 2015 11:25:08 +0300
+Subject: [PATCH] Apply xuser-specific policies if present
+
+system-xuser.conf is installed by xuser-account and contains
+policies that override the default service policies (allowing
+xuser to send messages to the services).
+
+Upstream-Status: Inappropriate [configuration]
+Signed-off-by: Jussi Kukkonen <[email protected]>
+---
+ bus/system.conf.in | 4 ++++
+ 1 file changed, 4 insertions(+)
+
+diff --git a/bus/system.conf.in b/bus/system.conf.in
+index 851b9e6..1822011 100644
+--- a/bus/system.conf.in
++++ b/bus/system.conf.in
+@@ -82,6 +82,10 @@
+        holes in the above policy for specific services. -->
+   <includedir>system.d</includedir>
+ 
++  <!-- Apply xuser policies (if present) after the service
++       policies so the xuser ones don't get overridden. -->
++  <include ignore_missing="yes">system-xuser.conf</include>
++
+   <!-- This is included last so local configuration can override what's 
+        in this standard file -->
+   <include ignore_missing="yes">system-local.conf</include>
+-- 
+2.1.4
+
Signature · 1 line
2.1.4
#3 lore raw

PATCH 3/3 xuser-account: Take over xuser specific D-Bus policy

In reply to Jussi Kukkonen

Move connmans xuser-related D-Bus policy to a separate file that xuser-account installs: This way connman does not need to depend on xuser-account. Add policies for bluez and ofono in the same file.

The new policy file still needs to be used by dbus-daemon.

---
 meta/recipes-connectivity/connman/connman.inc      |  1 -
 .../connman/add_xuser_dbus_permission.patch        | 43 ----------------------
 meta/recipes-connectivity/connman/connman_1.30.bb  |  1 -
 .../user-creation/files/system-xuser.conf          | 15 ++++++++
 .../user-creation/xuser-account_0.1.bb             |  6 ++-
 5 files changed, 19 insertions(+), 47 deletions(-)
 delete mode 100644 meta/recipes-connectivity/connman/connman/add_xuser_dbus_permission.patch
 create mode 100644 meta/recipes-support/user-creation/files/system-xuser.conf
meta/recipes-connectivity/connman/connman.inc +0 -1
index 1712af3..ab7f86d 100644
@@ -107,7 +107,6 @@ RPROVIDES_${PN} = "\
 
 RDEPENDS_${PN} = "\
 	dbus \
-	xuser-account \
 	"
 
 PACKAGES_DYNAMIC += "^${PN}-plugin-.*"
meta/recipes-connectivity/connman/connman/add_xuser_dbus_permission.patch deleted +0 -43

43 lines not on this page · raw message

meta/recipes-connectivity/connman/connman_1.30.bb +0 -1
index 9b512c5..7d65ac9 100644
@@ -2,7 +2,6 @@ require connman.inc
 
 SRC_URI  = "${KERNELORG_MIRROR}/linux/network/${BPN}/${BP}.tar.xz \
             file://0001-plugin.h-Change-visibility-to-default-for-debug-symb.patch \
-            file://add_xuser_dbus_permission.patch \
             file://0001-Detect-backtrace-API-availability-before-using-it.patch \
             file://0002-resolve-musl-does-not-implement-res_ninit.patch \
             file://0003-Fix-header-inclusions-for-musl.patch \
meta/recipes-support/user-creation/files/system-xuser.conf new file +15 -0
new file mode 100644
index 0000000..7a8e786
@@ -0,0 +1,15 @@
+<!DOCTYPE busconfig PUBLIC "-//freedesktop//DTD D-BUS Bus Configuration 1.0//EN"
+ "http://www.freedesktop.org/standards/dbus/1.0/busconfig.dtd">
+<busconfig>
+
+    <!-- This policy allows xuser to send messages to various services.
+         It should be applied after the service policies. -->
+
+    <policy user="xuser">
+        <allow send_destination="net.connman"/>
+        <allow send_destination="net.connman.vpn"/>
+        <allow send_destination="org.ofono"/>
+        <allow send_destination="org.bluez"/>
+    </policy>
+</busconfig>
+
meta/recipes-support/user-creation/xuser-account_0.1.bb +4 -2
index 77ba97d..f7830fb 100644
@@ -2,7 +2,7 @@ SUMMARY = "Creates an 'xuser' account used for running X11"
 LICENSE = "MIT"
 LIC_FILES_CHKSUM = "file://${COREBASE}/meta/COPYING.MIT;md5=3da9cfbcb788c80a0384361b4de20420"
 
-SRC_URI = ""
+SRC_URI = "file://system-xuser.conf"
 
 inherit allarch useradd
 
@@ -15,9 +15,11 @@ do_compile() {
 }
 
 do_install() {
-    :
+    install -D -m 0644 ${WORKDIR}/system-xuser.conf ${D}${sysconfdir}/dbus-1/system-xuser.conf
 }
 
+FILES_${PN} = "${sysconfdir}/dbus-1/system-xuser.conf"
+
 USERADD_PACKAGES = "${PN}"
 GROUPADD_PARAM_${PN} = "--system shutdown"
 USERADD_PARAM_${PN} = "--create-home \
Signature · 1 line
2.1.4
#4 lore raw
In reply to Jussi Kukkonen
Quoted 4 lines · Jussi Kukkonen

bluez5: Use upstream D-Bus policy
dbus: Use the xuser policy file
xuser-account: Take over xuser specific D-Bus policy

Please don't take the last two patches yet: I believe dbus itself does not actually have to be modified and the xuser policy file can just be a normal file in /etc/dbus-1/system.d/. I originally thought the default context policies in the services files could override the xuser user policy but this seems to not be the case: user policy always overrides default context policy.

Quoted 23 lines
 meta/recipes-connectivity/bluez5/bluez5.inc        |  5 +--
 .../bluez5/bluez5/bluetooth.conf                   | 17 ---------
 meta/recipes-connectivity/connman/connman.inc      |  1 -
 .../connman/add_xuser_dbus_permission.patch        | 43
----------------------
 meta/recipes-connectivity/connman/connman_1.30.bb  |  1 -
 meta/recipes-core/dbus/dbus.inc                    |  1 +
 ...-Apply-xuser-specific-policies-if-present.patch | 33 +++++++++++++++++
 .../user-creation/files/system-xuser.conf          | 15 ++++++++
 .../user-creation/xuser-account_0.1.bb             |  6 ++-
 9 files changed, 55 insertions(+), 67 deletions(-)
 delete mode 100644 meta/recipes-connectivity/bluez5/bluez5/bluetooth.conf
 delete mode 100644
meta/recipes-connectivity/connman/connman/add_xuser_dbus_permission.patch
 create mode 100644
meta/recipes-core/dbus/dbus/0001-Apply-xuser-specific-policies-if-present.patch
 create mode 100644

meta/recipes-support/user-creation/files/system-xuser.conf

--
2.1.4