Bug#762609: lintian: new checks: deprecated D-Bus policies
On Wed, 01 Oct 2014 at 21:45:02 +0200, Niels Thykier wrote:
> Thanks for amending your patch. Unfortunately, I have a couple of new
> minor remarks. Furthermore, I have updated our internal API in some
> cases that will be useful in this check as well, which I would like to
> promote as well. :)
I can't keep redoing this patch forever, but OK, here's another round.
I've also added some trivial checks for .service files being named correctly,
as a separate patch. If you think the policy part is OK but the .service bit
isn't, please apply the policy part anyway (I'd really like to be able
to see statistics for this check on lintian.d.o so I can use them as a
data-point in upstream D-Bus development), and I'll fix the service part
later.
Thanks,
S
>From ced780c0791a5edb00b87b42d511acea8a6a7232 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@debian.org>
Date: Fri, 10 Oct 2014 16:51:07 +0100
Subject: [PATCH 1/2] Add checks for deprecated D-Bus policies, and a
regression test
Bug: https://bugs.debian.org/762609
---
checks/dbus.desc | 56 +++++++++++++++
checks/dbus.pm | 83 ++++++++++++++++++++++
profiles/debian/main.profile | 2 +-
t/tests/dbus-policy/debian/debian/install | 1 +
.../debian/etc/dbus-1/system.d/at-console.conf | 13 ++++
.../etc/dbus-1/system.d/send-destination.conf | 8 +++
t/tests/dbus-policy/desc | 7 ++
t/tests/dbus-policy/tags | 4 ++
8 files changed, 173 insertions(+), 1 deletion(-)
create mode 100644 checks/dbus.desc
create mode 100644 checks/dbus.pm
create mode 100644 t/tests/dbus-policy/debian/debian/install
create mode 100644 t/tests/dbus-policy/debian/etc/dbus-1/system.d/at-console.conf
create mode 100644 t/tests/dbus-policy/debian/etc/dbus-1/system.d/send-destination.conf
create mode 100644 t/tests/dbus-policy/desc
create mode 100644 t/tests/dbus-policy/tags
diff --git a/checks/dbus.desc b/checks/dbus.desc
new file mode 100644
index 0000000..9f8ca36
--- /dev/null
+++ b/checks/dbus.desc
@@ -0,0 +1,56 @@
+Check-Script: dbus
+Author: Simon McVittie <simon.mcvittie@collabora.co.uk>
+Type: binary
+Info: Checks for deprecated or harmful D-Bus configuration
+Needs-Info: unpacked
+
+Tag: dbus-policy-at-console
+Severity: normal
+Certainty: certain
+Info: The package contains D-Bus policy configuration that uses the
+ deprecated <tt>at_console</tt> condition to impose a different policy
+ for users who are "logged in at the console" according to
+ systemd-logind, ConsoleKit or similar APIs, such as:
+ .
+ <policy context="default">
+ <deny send_destination="com.example.PowerManagementDaemon"/>
+ </policy>
+ <policy at_console="true">
+ <allow send_destination="com.example.PowerManagementDaemon"/>
+ </policy>
+ .
+ The maintainers of D-Bus recommend that services should allow or deny
+ method calls according to broad categories that are not typically altered
+ by the system administrator (usually either "all users", or only root
+ and/or a specified system user). If finer-grained authorization
+ is required, the service should accept the method call message, then call
+ out to PolicyKit to decide whether to honor the request. PolicyKit can
+ use system-administrator-configurable policies to make that decision,
+ including distinguishing between users who are "at the console" and
+ those who are not.
+Ref: https://bugs.freedesktop.org/show_bug.cgi?id=39611
+Experimental: yes
+
+Tag: dbus-policy-without-send-destination
+Severity: normal
+Certainty: possible
+Info: The package contains D-Bus policy configuration that uses
+ one of the <tt>send_*</tt> conditions but does not specify a
+ <tt>send_destination</tt>.
+ .
+ Rules of the form
+ .
+ <allow send_interface="com.example.MyInterface"/>
+ .
+ allow messages with the given interface to be sent to <i>any</i>
+ service, not just the one installing the rule, which is rarely
+ what was intended.
+ .
+ Similarly, on the system bus, rules of the form
+ .
+ <deny send_interface="com.example.MyInterface"/>
+ .
+ are redundant with the system bus' default-deny policy, and have
+ unintended effects on other services.
+Ref: https://bugs.freedesktop.org/show_bug.cgi?id=18961,http://lists.freedesktop.org/archives/dbus/2008-February/009401.html
+Experimental: yes
diff --git a/checks/dbus.pm b/checks/dbus.pm
new file mode 100644
index 0000000..cc73289
--- /dev/null
+++ b/checks/dbus.pm
@@ -0,0 +1,83 @@
+# dbus -- lintian check script, vaguely based on apache2 -*- perl -*-
+#
+# Copyright © 2012 Arno Töll
+# Copyright © 2014 Collabora Ltd.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, you can find it on the World Wide
+# Web at http://www.gnu.org/copyleft/gpl.html, or write to the Free
+# Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
+# MA 02110-1301, USA.
+
+package Lintian::dbus;
+
+use strict;
+use warnings;
+use autodie;
+
+use Lintian::Tags qw(tag);
+use Lintian::Util qw(slurp_entire_file);
+
+sub run {
+ my ($pkg, $type, $info) = @_;
+
+ my @files;
+ foreach my $dirname (qw(etc/dbus-1/session.d/ etc/dbus-1/system.d)) {
+ if (my $dir = $info->index_resolved_path($dirname)) {
+ push @files, $dir->children;
+ }
+ }
+
+ foreach my $file (@files) {
+ next unless $file->is_open_ok;
+ _check_policy($file);
+ }
+
+ return;
+}
+
+sub _check_policy {
+ my ($file) = @_;
+
+ my $xml = $file->file_contents;
+
+ # Parsing XML via regexes is evil, but good enough here...
+ # note that we are parsing the entire file as one big string,
+ # so that we catch <policy\nat_console="true"\n> or whatever.
+
+ if ($xml =~ m{<policy[^>]+at_console=(["'])true\1.*?</policy>}s) {
+ tag('dbus-policy-at-console', $file);
+ }
+
+ my @rules;
+ while ($xml =~ m{(<(?:allow|deny)[^>]+send_\w+=[^>]+>)}sg) {
+ push(@rules, $1);
+ }
+ foreach my $rule (@rules) {
+ if ($rule !~ m{send_destination=}) {
+ # normalize whitespace a bit
+ $rule =~ s{\s+}{ }g;
+ tag('dbus-policy-without-send-destination', $file, $rule);
+ }
+ }
+
+ return;
+}
+
+1;
+
+# Local Variables:
+# indent-tabs-mode: nil
+# cperl-indent-level: 4
+# End:
+# vim: syntax=perl sw=4 sts=4 sr et
diff --git a/profiles/debian/main.profile b/profiles/debian/main.profile
index 336d1f0..7c27ff3 100644
--- a/profiles/debian/main.profile
+++ b/profiles/debian/main.profile
@@ -2,7 +2,7 @@
Profile: debian/main
Extends: debian/ftp-master-auto-reject
Enable-Tags-From-Check: apache2, automake, binaries, changelog-file, changes-file,
- conffiles, control-file, control-files, copyright-file, cruft, deb-format,
+ conffiles, control-file, control-files, copyright-file, cruft, dbus, deb-format,
debconf, debhelper, debian-readme, debian-source-dir, description,
duplicate-files, fields, filename-length, files, group-checks, huge-usr-share,
infofiles, init.d, java, lintian, manpages, md5sums, menu-format, menus, nmu,
diff --git a/t/tests/dbus-policy/debian/debian/install b/t/tests/dbus-policy/debian/debian/install
new file mode 100644
index 0000000..ee19d5d
--- /dev/null
+++ b/t/tests/dbus-policy/debian/debian/install
@@ -0,0 +1 @@
+etc
diff --git a/t/tests/dbus-policy/debian/etc/dbus-1/system.d/at-console.conf b/t/tests/dbus-policy/debian/etc/dbus-1/system.d/at-console.conf
new file mode 100644
index 0000000..06d96c8
--- /dev/null
+++ b/t/tests/dbus-policy/debian/etc/dbus-1/system.d/at-console.conf
@@ -0,0 +1,13 @@
+<!DOCTYPE busconfig PUBLIC "-//freedesktop//DTD D-BUS Bus Configuration 1.0//EN" "http://www.freedesktop.org/standards/dbus/1.0/busconfig.dtd";>
+<busconfig>
+
+ <!-- this is OK, at least for now -->
+ <policy group="bluetooth">
+ <allow send_destination="com.example.Service"/>
+ </policy>
+
+ <!-- this is deprecated -->
+ <policy at_console="true">
+ <allow send_destination="com.example.Service"/>
+ </policy>
+</busconfig>
diff --git a/t/tests/dbus-policy/debian/etc/dbus-1/system.d/send-destination.conf b/t/tests/dbus-policy/debian/etc/dbus-1/system.d/send-destination.conf
new file mode 100644
index 0000000..6bb7150
--- /dev/null
+++ b/t/tests/dbus-policy/debian/etc/dbus-1/system.d/send-destination.conf
@@ -0,0 +1,8 @@
+<!DOCTYPE busconfig PUBLIC "-//freedesktop//DTD D-BUS Bus Configuration 1.0//EN" "http://www.freedesktop.org/standards/dbus/1.0/busconfig.dtd";>
+<busconfig>
+ <policy context="default">
+ <allow send_interface="org.freedesktop.DBus.ObjectManager"/>
+ <allow send_member="AreYouReallySureThisMethodIsAlwaysOK"/>
+ <allow send_path="/com/example/Here"/>
+ </policy>
+</busconfig>
diff --git a/t/tests/dbus-policy/desc b/t/tests/dbus-policy/desc
new file mode 100644
index 0000000..a569815
--- /dev/null
+++ b/t/tests/dbus-policy/desc
@@ -0,0 +1,7 @@
+Testname: dbus-policy
+Sequence: 6000
+Version: 1.0
+Description: test deprecated D-Bus policies
+Test-For:
+ dbus-policy-at-console
+ dbus-policy-without-send-destination
diff --git a/t/tests/dbus-policy/tags b/t/tests/dbus-policy/tags
new file mode 100644
index 0000000..0705661
--- /dev/null
+++ b/t/tests/dbus-policy/tags
@@ -0,0 +1,4 @@
+X: dbus-policy: dbus-policy-at-console etc/dbus-1/system.d/at-console.conf
+X: dbus-policy: dbus-policy-without-send-destination etc/dbus-1/system.d/send-destination.conf <allow send_interface="org.freedesktop.DBus.ObjectManager"/>
+X: dbus-policy: dbus-policy-without-send-destination etc/dbus-1/system.d/send-destination.conf <allow send_member="AreYouReallySureThisMethodIsAlwaysOK"/>
+X: dbus-policy: dbus-policy-without-send-destination etc/dbus-1/system.d/send-destination.conf <allow send_path="/com/example/Here"/>
--
2.1.1
>From 9249c2ff72d490c299ef48e4e01e035ccbb19b00 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@debian.org>
Date: Fri, 10 Oct 2014 17:14:21 +0100
Subject: [PATCH 2/2] Add checks for mis-named D-Bus .service files, with
regression tests
Bug: https://bugs.debian.org/762609
---
checks/dbus.desc | 31 +++++++++++++++++
checks/dbus.pm | 40 ++++++++++++++++++++--
t/tests/dbus-services/debian/debian/install | 1 +
.../usr/share/dbus-1/services/gvfs-daemon.service | 3 ++
.../services/org.mpris.MediaPlayer2.mpd.service | 3 ++
.../org.freedesktop.PolicyKit1.service | 5 +++
.../system-services/this-name-cannot-work.service | 4 +++
t/tests/dbus-services/desc | 7 ++++
t/tests/dbus-services/tags | 2 ++
9 files changed, 94 insertions(+), 2 deletions(-)
create mode 100644 t/tests/dbus-services/debian/debian/install
create mode 100644 t/tests/dbus-services/debian/usr/share/dbus-1/services/gvfs-daemon.service
create mode 100644 t/tests/dbus-services/debian/usr/share/dbus-1/services/org.mpris.MediaPlayer2.mpd.service
create mode 100644 t/tests/dbus-services/debian/usr/share/dbus-1/system-services/org.freedesktop.PolicyKit1.service
create mode 100644 t/tests/dbus-services/debian/usr/share/dbus-1/system-services/this-name-cannot-work.service
create mode 100644 t/tests/dbus-services/desc
create mode 100644 t/tests/dbus-services/tags
diff --git a/checks/dbus.desc b/checks/dbus.desc
index 9f8ca36..42b9665 100644
--- a/checks/dbus.desc
+++ b/checks/dbus.desc
@@ -54,3 +54,34 @@ Info: The package contains D-Bus policy configuration that uses
unintended effects on other services.
Ref: https://bugs.freedesktop.org/show_bug.cgi?id=18961,http://lists.freedesktop.org/archives/dbus/2008-February/009401.html
Experimental: yes
+
+Tag: dbus-session-service-wrong-name
+Severity: wishlist
+Certainty: certain
+Info: The package contains a D-Bus session service whose filename
+ does not match the <tt>Name</tt> field found in the file.
+ This makes it possible that two non-conflicting packages could
+ provide the same service name with the same search-path priority
+ (i.e. in the same directory). dbus-daemon will arbitrarily choose
+ one of them, which is unlikely to be the desired result.
+ .
+ Best-practice is that if you implement a session service whose well-known
+ name is <tt>com.example.MyService1</tt>, and it should be
+ service-activatable, you should achieve that by packaging
+ <tt>/usr/share/dbus-1/services/com.example.MyService1.service</tt>.
+Experimental: yes
+
+Tag: dbus-system-service-wrong-name
+Severity: serious
+Certainty: certain
+Info: The package contains a D-Bus system service whose filename
+ does not match the <tt>Name</tt> field found in the file.
+ This will not work, because dbus-daemon-launch-helper specifically
+ looks for that filename, in order to keep system-level activation
+ secure and predictable.
+ .
+ If you implement a session service whose well-known name is
+ <tt>com.example.MyService1</tt>, and it should be service-activatable,
+ you must provide
+ <tt>/usr/share/dbus-1/system-services/com.example.MyService1.service</tt>.
+Experimental: yes
diff --git a/checks/dbus.pm b/checks/dbus.pm
index cc73289..23ea0cd 100644
--- a/checks/dbus.pm
+++ b/checks/dbus.pm
@@ -32,8 +32,8 @@ sub run {
my ($pkg, $type, $info) = @_;
my @files;
- foreach my $dirname (qw(etc/dbus-1/session.d/ etc/dbus-1/system.d)) {
- if (my $dir = $info->index_resolved_path($dirname)) {
+ foreach my $dirname (qw(session system)) {
+ if (my $dir = $info->index_resolved_path("etc/dbus-1/${dirname}.d")) {
push @files, $dir->children;
}
}
@@ -43,6 +43,21 @@ sub run {
_check_policy($file);
}
+ if (my $dir = $info->index_resolved_path('usr/share/dbus-1/services')) {
+ foreach my $file ($dir->children) {
+ next unless $file->is_open_ok;
+ _check_service($file, session => 1);
+ }
+ }
+
+ if (my $dir
+ = $info->index_resolved_path('usr/share/dbus-1/system-services')) {
+ foreach my $file ($dir->children) {
+ next unless $file->is_open_ok;
+ _check_service($file);
+ }
+ }
+
return;
}
@@ -74,6 +89,27 @@ sub _check_policy {
return;
}
+sub _check_service {
+ my ($file, %kwargs) = @_;
+
+ my $basename = $file->basename;
+ my $text = $file->file_contents;
+
+ while ($text =~ m{^Name=(.*)$}gm) {
+ my $name = $1;
+ if ($basename ne "${name}.service") {
+ if ($kwargs{session}) {
+ tag('dbus-session-service-wrong-name',
+ "${name}.service", $file);
+ } else {
+ tag('dbus-system-service-wrong-name',"${name}.service", $file);
+ }
+ }
+ }
+
+ return;
+}
+
1;
# Local Variables:
diff --git a/t/tests/dbus-services/debian/debian/install b/t/tests/dbus-services/debian/debian/install
new file mode 100644
index 0000000..73752c9
--- /dev/null
+++ b/t/tests/dbus-services/debian/debian/install
@@ -0,0 +1 @@
+usr
diff --git a/t/tests/dbus-services/debian/usr/share/dbus-1/services/gvfs-daemon.service b/t/tests/dbus-services/debian/usr/share/dbus-1/services/gvfs-daemon.service
new file mode 100644
index 0000000..1a8607d
--- /dev/null
+++ b/t/tests/dbus-services/debian/usr/share/dbus-1/services/gvfs-daemon.service
@@ -0,0 +1,3 @@
+[D-BUS Service]
+Name=org.gtk.vfs.Daemon
+Exec=/usr/lib/gvfs/gvfsd
diff --git a/t/tests/dbus-services/debian/usr/share/dbus-1/services/org.mpris.MediaPlayer2.mpd.service b/t/tests/dbus-services/debian/usr/share/dbus-1/services/org.mpris.MediaPlayer2.mpd.service
new file mode 100644
index 0000000..3f14f4a
--- /dev/null
+++ b/t/tests/dbus-services/debian/usr/share/dbus-1/services/org.mpris.MediaPlayer2.mpd.service
@@ -0,0 +1,3 @@
+[D-BUS Service]
+Name=org.mpris.MediaPlayer2.mpd
+Exec=/usr/bin/mpDris2
diff --git a/t/tests/dbus-services/debian/usr/share/dbus-1/system-services/org.freedesktop.PolicyKit1.service b/t/tests/dbus-services/debian/usr/share/dbus-1/system-services/org.freedesktop.PolicyKit1.service
new file mode 100644
index 0000000..51d1f94
--- /dev/null
+++ b/t/tests/dbus-services/debian/usr/share/dbus-1/system-services/org.freedesktop.PolicyKit1.service
@@ -0,0 +1,5 @@
+[D-BUS Service]
+Name=org.freedesktop.PolicyKit1
+Exec=/usr/lib/policykit-1/polkitd --no-debug
+User=root
+SystemdService=polkitd.service
diff --git a/t/tests/dbus-services/debian/usr/share/dbus-1/system-services/this-name-cannot-work.service b/t/tests/dbus-services/debian/usr/share/dbus-1/system-services/this-name-cannot-work.service
new file mode 100644
index 0000000..e87a5bb
--- /dev/null
+++ b/t/tests/dbus-services/debian/usr/share/dbus-1/system-services/this-name-cannot-work.service
@@ -0,0 +1,4 @@
+[D-BUS Service]
+Name=com.example.SystemDaemon1
+Exec=/usr/sbin/example-system-daemon
+User=nobody
diff --git a/t/tests/dbus-services/desc b/t/tests/dbus-services/desc
new file mode 100644
index 0000000..ffb0e71
--- /dev/null
+++ b/t/tests/dbus-services/desc
@@ -0,0 +1,7 @@
+Testname: dbus-services
+Sequence: 6000
+Version: 1.0
+Description: test D-Bus .service files
+Test-For:
+ dbus-session-service-wrong-name
+ dbus-system-service-wrong-name
diff --git a/t/tests/dbus-services/tags b/t/tests/dbus-services/tags
new file mode 100644
index 0000000..217b153
--- /dev/null
+++ b/t/tests/dbus-services/tags
@@ -0,0 +1,2 @@
+X: dbus-services: dbus-session-service-wrong-name org.gtk.vfs.Daemon.service usr/share/dbus-1/services/gvfs-daemon.service
+X: dbus-services: dbus-system-service-wrong-name com.example.SystemDaemon1.service usr/share/dbus-1/system-services/this-name-cannot-work.service
--
2.1.1
Reply to: