Thanks for reviewing; I hope I've fixed all your concerns.
On Tue, 23 Sep 2014 at 21:58:33 +0200, Niels Thykier wrote:
> Could you be persuaded to write some build-time
> test cases for lintian as well?
Done, see attached.
> debian/rules runtests onlyrun=suite:scripts
> - It checks for various minor issues like code-style
Done, it does not complain about dbus.pm now
> Abbrev is optional and is (mostly) useless if it is not shorter than
> Check-Script
Removed
> > + if ($type eq 'binary') {
>
> >From my remark earlier, this condition is always true.
Removed
> > + if ($file =~ m{^etc/dbus-1/(?:system|session).d/}) {
>
> The dot after ")" and before "d" is not escaped.
Fixed
> I appreciate the effort, but this is technically vulnerable to a DoS by
> creating including a named pipe. But at least you caught the symlink
> attack, which must people forget. :)
I will admit that this was cargo-culted from apache2.pm. Now fixed properly.
> In Lintian git, we (very) recently devised a new set of methods to avoid
> this kind of problem. You may want to use those instead.
Done
> Arguments should use "my ($file, $filename) = @_;"
Done
> > + my $callback = shift;
>
> This appear to be unused.
Removed (it was a relic of dbus.pm being usable as a standalone command-line
tool while I was getting the actual checks right)
> This entire block can be replaced by:
>
> my $xml = slurp_entire_file($filename);
Done
> Style: We are converting to: push(@rules, $1); (i.e. with parentheses).
Fixed
Regards,
S
>From 64e856a319330d3dfb4d888028258527688977f7 Mon Sep 17 00:00:00 2001
From: Simon McVittie <s...@debian.org>
Date: Tue, 30 Sep 2014 13:56:38 +0100
Subject: [PATCH] Add checks for deprecated D-Bus policies, and a regression
test
Bug: https://bugs.debian.org/762609
---
checks/dbus.desc | 56 +++++++++++++++
checks/dbus.pm | 81 ++++++++++++++++++++++
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, 171 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.mcvit...@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..f2db47e
--- /dev/null
+++ b/checks/dbus.pm
@@ -0,0 +1,81 @@
+# 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) = @_;
+
+ foreach my $file ($info->sorted_index) {
+ next if $file->is_dir;
+
+ if ($file =~ m{^etc/dbus-1/(?:system|session)\.d/}) {
+ my $path = $info->index_resolved_path($file);
+ next unless $file->is_open_ok;
+ _check_policy($file, $path);
+ }
+ }
+
+ return;
+}
+
+sub _check_policy {
+ my ($file, $path) = @_;
+
+ my $xml = slurp_entire_file($path->open);
+
+ # 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
