Bug#704197: Please review: systemd checks
Hi Niels,
Niels Thykier <niels@thykier.net> writes:
> I thought this was safe, but it does have an issue as well. Consider
> symlink chaining:
>
> safe-symlink -> unsafe-symlink
> unsafe-symlink -> ../../../../etc/passwd
>
> $path->link_resolved will approve "safe-symlink" because it can be
> resolved safely. However, it does not check that the target is also a
> safe symlink - so a loop/recursion is needed. That said, using the new
> "is_ancestor_of" (from L::Util) is probably a lot easier to use
> correctly. Basically:
Thanks for the explanation and the example. I have updated my code and
the tests still work. Find the updated patches attached (rebased against
current master).
--
Best regards,
Michael
>From ceb4afecf02c6c1a1277ad69bb2d3430baed6fa9 Mon Sep 17 00:00:00 2001
From: Michael Stapelberg <michael@stapelberg.de>
Date: Sat, 13 Apr 2013 23:14:31 +0200
Subject: [PATCH 1/3] add systemd checks
---
checks/systemd | 252 +++++++++++++++++++++++++++++++++++++++++++++++++++
checks/systemd.desc | 76 ++++++++++++++++
2 files changed, 328 insertions(+)
create mode 100644 checks/systemd
create mode 100644 checks/systemd.desc
diff --git a/checks/systemd b/checks/systemd
new file mode 100644
index 0000000..866110c
--- /dev/null
+++ b/checks/systemd
@@ -0,0 +1,252 @@
+# systemd -- lintian check script -*- perl -*-
+#
+# Copyright © 2013 Michael Stapelberg
+#
+# based on the apache2 checks file by:
+# Copyright © 2012 Arno Töll
+#
+# 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::systemd;
+
+use strict;
+use warnings;
+
+use File::Basename;
+use Text::ParseWords qw(shellwords);
+
+use Lintian::Tags qw(tag);
+use Lintian::Util qw(fail);
+
+sub run {
+ my ($pkg, $type, $info) = @_;
+
+ # Figure out whether the maintainer of this package did any effort to
+ # make the package work with systemd. If not, we will not warn in case
+ # of an init script that has no systemd equivalent, for example.
+ my $ships_systemd_file = (scalar ( grep { m,/systemd/, } $info->sorted_index ) > 0);
+
+ # An array of names which are provided by the service files.
+ # This includes Alias= directives, so after parsing
+ # NetworkManager.service, it will contain NetworkManager and
+ # network-manager.
+ my @systemd_targets;
+
+ for my $file ($info->sorted_index) {
+ if ($file =~ m,^etc/tmpfiles\.d/.*\.conf$,) {
+ tag 'systemd-tmpfiles.d-outside-usr-lib', $file;
+ }
+ if ($file =~ m,^etc/systemd/system/.*\.service$,) {
+ tag 'systemd-service-file-outside-lib', $file;
+ }
+ if ($file =~ m,/systemd/system/.*\.service$,) {
+ check_systemd_service_file ($pkg, $info, $file);
+ for my $name (extract_service_file_names ($pkg, $info, $file)) {
+ push @systemd_targets, $name;
+ }
+ }
+ }
+
+ my @init_scripts = grep { m,^etc/init\.d/.+, } $info->sorted_index;
+
+ # Verify that each init script includes /lib/lsb/init-functions,
+ # because that is where the systemd diversion happens.
+ for my $init_script (@init_scripts) {
+ check_init_script ($pkg, $info, $init_script);
+ }
+
+ @init_scripts = map { basename($_) } @init_scripts;
+
+ if ($ships_systemd_file) {
+ for my $init_script (@init_scripts) {
+ tag 'systemd-no-service-for-init-script', $init_script
+ unless grep /\Q$init_script\E\.service/, @systemd_targets;
+ }
+ }
+
+ check_maintainer_scripts ($info);
+}
+
+sub check_init_script {
+ my ($pkg, $info, $file) = @_;
+
+ my $lsb_source_seen;
+ my $path = $info->index ($file);
+ unless ($path->is_regular_file ||
+ ($path->is_symlink && defined($path->link_resolved))) {
+ tag 'init-script-is-not-a-file', $file;
+ return;
+ }
+ open(my $fh, '<', $info->unpacked ($file))
+ or fail "cannot open $file: $!";
+ while (<$fh>) {
+ s/^\s+//;
+ next if /^#/;
+ if (m,^(?:\.|source)\s+/lib/lsb/init-functions,) {
+ $lsb_source_seen = 1;
+ last;
+ }
+ }
+ close($fh);
+
+ if (!$lsb_source_seen) {
+ tag 'init.d-script-does-not-source-init-functions', $file;
+ }
+}
+
+sub check_systemd_service_file {
+ my ($pkg, $info, $file) = @_;
+
+ my @values = extract_service_file_values ($pkg, $info, $file, 'Unit', 'After');
+ my @obsolete = grep { /^(?:syslog|dbus)\.target$/ } @values;
+ tag 'systemd-service-file-refers-to-obsolete-target', $file, $_ for @obsolete;
+}
+
+sub service_file_lines {
+ my ($file, $path) = @_;
+ open(my $fh, '<', $path)
+ or fail "cannot open $file: $!";
+ my @lines;
+ my $continuation;
+ while (<$fh>) {
+ chomp;
+
+ if (defined($continuation)) {
+ $_ = $continuation . $_;
+ $continuation = undef;
+ }
+
+ if (/\\$/) {
+ $continuation = $_;
+ $continuation =~ s/\\$/ /;
+ next;
+ }
+
+ # equivalent of strstrip(l)
+ $_ =~ s,[ \t\n\r]+$,,g;
+
+ next if $_ eq '';
+
+ next if /^[#;\n]/;
+
+ push @lines, $_;
+ }
+ close($fh);
+
+ return @lines;
+}
+
+# Extracts the values of a specific Key from a .service file
+sub extract_service_file_values {
+ my ($pkg, $info, $file, $extract_section, $extract_key) = @_;
+
+ my @values;
+ my $section;
+ my $path = $info->index ($file);
+ unless ($path->is_regular_file ||
+ ($path->is_symlink && defined($path->link_resolved))) {
+ tag 'service-file-is-not-a-file', $file;
+ return;
+ }
+ my @lines = service_file_lines($file, $info->unpacked ($file));
+ while (grep { /^\.include / } @lines) {
+ @lines = map {
+ if (/^\.include (.+)$/) {
+# XXX: edge case: what should we do when a service file .includes a file in another package? lintian will not have access and therefore cannot properly check the file.
+ my $path = $1;
+ $path =~ s,^/,,;
+ service_file_lines(basename($path), $info->unpacked ($path));
+ } else {
+ $_;
+ }
+ } @lines;
+ }
+ for (@lines) {
+ # section header
+ if (/^\[([^\]]+)\]$/) {
+ $section = $1;
+ next;
+ }
+
+ if (!defined($section)) {
+ # Assignment outside of section. Ignoring.
+ next;
+ }
+
+ my ($key, $value) = ($_ =~ m,^(.*)=(.*)$,);
+ if ($section eq $extract_section &&
+ $key eq $extract_key) {
+ if ($value eq '') {
+ # Empty assignment resets the list
+ @values = ();
+ }
+
+ @values = (@values, shellwords ($value));
+ }
+ }
+
+ return @values;
+}
+
+sub extract_service_file_names {
+ my ($pkg, $info, $file) = @_;
+
+ my @aliases = extract_service_file_values ($pkg, $info, $file, 'Install', 'Alias');
+ return (basename ($file), @aliases);
+}
+
+sub check_maintainer_scripts {
+ my ($info) = @_;
+
+ open my $fd, '<', $info->lab_data_path('/control-scripts')
+ or fail "cannot open lintian control-scripts file: $!";
+
+ while (<$fd>) {
+ m/^(\S*) (.*)$/ or fail("bad line in control-scripts file: $_");
+ my $interpreter = $1;
+ my $file = $2;
+ my $filename = $info->control ($file);
+
+ # Don't follow links
+ next if -l $filename;
+ # Don't try to parse the file if it does not appear to be a shell script
+ next if $interpreter !~ m/sh\b/;
+
+ open my $sfd, '<', $filename or fail "cannot open maintainer script $filename: $!";
+ while (<$sfd>) {
+ # skip comments
+ next if substr ($_, 0, $-[0]) =~ /#/;
+
+ # XXX: this is temporarily disabled until we are sure which systemctl actions are allowed, if any.
+ ## systemctl should not be called in maintainer scripts at all.
+ #if (m/\bsystemctl\b/) {
+ # tag 'maintainer-script-calls-systemctl', $file;
+ #}
+ }
+ close $sfd;
+ }
+
+ close $fd;
+}
+
+1;
+
+# Local Variables:
+# indent-tabs-mode: nil
+# cperl-indent-level: 4
+# End:
+# vim: syntax=perl sw=4 sts=4 sr et
diff --git a/checks/systemd.desc b/checks/systemd.desc
new file mode 100644
index 0000000..cc014f9
--- /dev/null
+++ b/checks/systemd.desc
@@ -0,0 +1,76 @@
+Check-Script: systemd
+Author: Michael Stapelberg <stapelberg@debian.org>
+Type: binary
+Info: Checks various systemd policy things
+Needs-Info: scripts, index, unpacked, file-info
+
+Tag: systemd-service-file-outside-lib
+Severity: serious
+Certainty: certain
+Info: The package ships a systemd service file outside
+ <tt>/lib/systemd/system/</tt>
+ .
+ System administrators should have the possibility to overwrite a service file
+ (or parts of it, in newer systemd versions) by placing a file in
+ <tt>/etc/systemd/system</tt>, so the canonical location we use for service
+ files is <tt>/lib/systemd/system/</tt>.
+
+Tag: systemd-tmpfiles.d-outside-usr-lib
+Severity: serious
+Certainty: certain
+Info: The package ships a systemd tmpfiles.d(5) conf file outside
+ <tt>/usr/lib/tmpfiles.d/</tt>
+
+Tag: systemd-service-file-refers-to-obsolete-target
+Severity: normal
+Certainty: certain
+Info: The systemd service file refers to an obsolete target.
+ .
+ Some targets are obsolete by now, e.g. syslog.target or dbus.target. For
+ example, declaring <tt>After=syslog.target</tt> is unnecessary by now because
+ syslog is socket-activated and will therefore be started when needed.
+
+Tag: systemd-no-service-for-init-script
+Severity: serious
+Certainty: certain
+Info: The listed init.d script has no systemd equivalent.
+ .
+ Systemd has a SysV init.d script compatibility mode. It provides access to
+ each SysV init.d script as long as there is no native service file with the
+ same name (e.g. <tt>/lib/systemd/system/rsyslog.service</tt> corresponds to
+ <tt>/etc/init.d/rsyslog</tt>).
+ .
+ Your package ships a service file, but for the listed init.d script, there is
+ no corresponding systemd service file.
+
+Tag: init.d-script-does-not-source-init-functions
+Severity: normal
+Certainty: certain
+Info: The <tt>/etc/init.d</tt> script does not source
+ <tt>/lib/lsb/init-functions</tt>. The <tt>systemd</tt> package provides
+ <tt>/lib/lsb/init-functions.d/40-systemd</tt> to redirect
+ <tt>/etc/init.d/$script</tt> calls to systemctl.
+ .
+ Please add a line like this to your <tt>/etc/init.d</tt> script:
+ .
+ . /lib/lsb/init-functions
+
+Tag: maintainer-script-calls-systemctl
+Severity: normal
+Certainty: certain
+Info: The maintainer script calls systemctl directly. Actions such as enabling
+ or starting a service have to be done via <tt>update-rc.d</tt> or
+ <tt>invoke-rc.d</tt>, respectively, which both do the right thing when systemd
+ is installed/running.
+
+Tag: init-script-is-not-a-file
+Severity: serious
+Certainty: certain
+Info: The package contains an init script that is not a regular file or
+ resolvable symlink.
+
+Tag: service-file-is-not-a-file
+Severity: serious
+Certainty: certain
+Info: The package contains a service file that is not a regular file or
+ resolvable symlink.
--
1.7.10.4
>From 785c9985ec5986dda6d17337ced9e508c4f4ab67 Mon Sep 17 00:00:00 2001
From: Michael Stapelberg <michael@stapelberg.de>
Date: Sat, 13 Apr 2013 23:14:40 +0200
Subject: [PATCH 2/3] add systemd tests
---
.../debian/debian/install | 2 +
.../debian/debian/test.service | 8 +
.../debian/debian/test2.service | 5 +
t/tests/systemd-complex-service-file/desc | 7 +
t/tests/systemd-complex-service-file/tags | 2 +
t/tests/systemd-general/debian/debian/init | 158 ++++++++++++++++++++
t/tests/systemd-general/debian/debian/install | 2 +
t/tests/systemd-general/debian/debian/test.conf | 2 +
t/tests/systemd-general/debian/debian/test.service | 9 ++
t/tests/systemd-general/desc | 10 ++
t/tests/systemd-general/tags | 5 +
11 files changed, 210 insertions(+)
create mode 100644 t/tests/systemd-complex-service-file/debian/debian/install
create mode 100644 t/tests/systemd-complex-service-file/debian/debian/test.service
create mode 100644 t/tests/systemd-complex-service-file/debian/debian/test2.service
create mode 100644 t/tests/systemd-complex-service-file/desc
create mode 100644 t/tests/systemd-complex-service-file/tags
create mode 100644 t/tests/systemd-general/debian/debian/init
create mode 100644 t/tests/systemd-general/debian/debian/install
create mode 100644 t/tests/systemd-general/debian/debian/test.conf
create mode 100644 t/tests/systemd-general/debian/debian/test.service
create mode 100644 t/tests/systemd-general/desc
create mode 100644 t/tests/systemd-general/tags
diff --git a/t/tests/systemd-complex-service-file/debian/debian/install b/t/tests/systemd-complex-service-file/debian/debian/install
new file mode 100644
index 0000000..5d4e053
--- /dev/null
+++ b/t/tests/systemd-complex-service-file/debian/debian/install
@@ -0,0 +1,2 @@
+debian/test.service lib/systemd/system/
+debian/test2.service lib/systemd/system/
diff --git a/t/tests/systemd-complex-service-file/debian/debian/test.service b/t/tests/systemd-complex-service-file/debian/debian/test.service
new file mode 100644
index 0000000..820b731
--- /dev/null
+++ b/t/tests/systemd-complex-service-file/debian/debian/test.service
@@ -0,0 +1,8 @@
+[Unit]
+After=dbus.target
+
+[Service]
+ExecStart=/usr/bin/test
+
+[Install]
+WantedBy=multi-user.target
diff --git a/t/tests/systemd-complex-service-file/debian/debian/test2.service b/t/tests/systemd-complex-service-file/debian/debian/test2.service
new file mode 100644
index 0000000..71c1297
--- /dev/null
+++ b/t/tests/systemd-complex-service-file/debian/debian/test2.service
@@ -0,0 +1,5 @@
+.include /lib/systemd/system/test.service
+
+[Unit]
+After=
+After=syslog.target
diff --git a/t/tests/systemd-complex-service-file/desc b/t/tests/systemd-complex-service-file/desc
new file mode 100644
index 0000000..b42f559
--- /dev/null
+++ b/t/tests/systemd-complex-service-file/desc
@@ -0,0 +1,7 @@
+Testname: systemd-complex-service-file
+Sequence: 6000
+Version: 1.0
+Description: Verifies the .service file parser properly handles .include within
+ service files.
+Test-For:
+ systemd-service-file-refers-to-obsolete-target
diff --git a/t/tests/systemd-complex-service-file/tags b/t/tests/systemd-complex-service-file/tags
new file mode 100644
index 0000000..c4715b9
--- /dev/null
+++ b/t/tests/systemd-complex-service-file/tags
@@ -0,0 +1,2 @@
+W: systemd-complex-service-file: systemd-service-file-refers-to-obsolete-target lib/systemd/system/test.service dbus.target
+W: systemd-complex-service-file: systemd-service-file-refers-to-obsolete-target lib/systemd/system/test2.service syslog.target
diff --git a/t/tests/systemd-general/debian/debian/init b/t/tests/systemd-general/debian/debian/init
new file mode 100644
index 0000000..afffa18
--- /dev/null
+++ b/t/tests/systemd-general/debian/debian/init
@@ -0,0 +1,158 @@
+#! /bin/sh
+### BEGIN INIT INFO
+# Provides: systemd-general
+# Required-Start: $remote_fs $syslog
+# Required-Stop: $remote_fs $syslog
+# Default-Start: 2 3 4 5
+# Default-Stop: 0 1 6
+# Short-Description: Example initscript
+# Description: This file should be used to construct scripts to be
+# placed in /etc/init.d.
+### END INIT INFO
+
+# Author: Foo Bar <foobar@baz.org>
+#
+# Please remove the "Author" lines above and replace them
+# with your own name if you copy and modify this script.
+
+# Do NOT "set -e"
+
+# PATH should only include /usr/* if it runs after the mountnfs.sh script
+PATH=/sbin:/usr/sbin:/bin:/usr/bin
+DESC="Description of the service"
+NAME=daemonexecutablename
+DAEMON=/usr/sbin/$NAME
+DAEMON_ARGS="--options args"
+PIDFILE=/var/run/$NAME.pid
+SCRIPTNAME=/etc/init.d/$NAME
+
+# Exit if the package is not installed
+[ -x "$DAEMON" ] || exit 0
+
+# Read configuration variable file if it is present
+[ -r /etc/default/$NAME ] && . /etc/default/$NAME
+
+# Load the VERBOSE setting and other rcS variables
+. /lib/init/vars.sh
+
+# Define LSB log_* functions.
+# Depend on lsb-base (>= 3.2-14) to ensure that this file is present
+# and status_of_proc is working.
+
+#
+# Function that starts the daemon/service
+#
+do_start()
+{
+ # Return
+ # 0 if daemon has been started
+ # 1 if daemon was already running
+ # 2 if daemon could not be started
+ start-stop-daemon --start --quiet --pidfile $PIDFILE --exec $DAEMON --test > /dev/null \
+ || return 1
+ start-stop-daemon --start --quiet --pidfile $PIDFILE --exec $DAEMON -- \
+ $DAEMON_ARGS \
+ || return 2
+ # Add code here, if necessary, that waits for the process to be ready
+ # to handle requests from services started subsequently which depend
+ # on this one. As a last resort, sleep for some time.
+}
+
+#
+# Function that stops the daemon/service
+#
+do_stop()
+{
+ # Return
+ # 0 if daemon has been stopped
+ # 1 if daemon was already stopped
+ # 2 if daemon could not be stopped
+ # other if a failure occurred
+ start-stop-daemon --stop --quiet --retry=TERM/30/KILL/5 --pidfile $PIDFILE --name $NAME
+ RETVAL="$?"
+ [ "$RETVAL" = 2 ] && return 2
+ # Wait for children to finish too if this is a daemon that forks
+ # and if the daemon is only ever run from this initscript.
+ # If the above conditions are not satisfied then add some other code
+ # that waits for the process to drop all resources that could be
+ # needed by services started subsequently. A last resort is to
+ # sleep for some time.
+ start-stop-daemon --stop --quiet --oknodo --retry=0/30/KILL/5 --exec $DAEMON
+ [ "$?" = 2 ] && return 2
+ # Many daemons don't delete their pidfiles when they exit.
+ rm -f $PIDFILE
+ return "$RETVAL"
+}
+
+#
+# Function that sends a SIGHUP to the daemon/service
+#
+do_reload() {
+ #
+ # If the daemon can reload its configuration without
+ # restarting (for example, when it is sent a SIGHUP),
+ # then implement that here.
+ #
+ start-stop-daemon --stop --signal 1 --quiet --pidfile $PIDFILE --name $NAME
+ return 0
+}
+
+case "$1" in
+ start)
+ [ "$VERBOSE" != no ] && log_daemon_msg "Starting $DESC" "$NAME"
+ do_start
+ case "$?" in
+ 0|1) [ "$VERBOSE" != no ] && log_end_msg 0 ;;
+ 2) [ "$VERBOSE" != no ] && log_end_msg 1 ;;
+ esac
+ ;;
+ stop)
+ [ "$VERBOSE" != no ] && log_daemon_msg "Stopping $DESC" "$NAME"
+ do_stop
+ case "$?" in
+ 0|1) [ "$VERBOSE" != no ] && log_end_msg 0 ;;
+ 2) [ "$VERBOSE" != no ] && log_end_msg 1 ;;
+ esac
+ ;;
+ status)
+ status_of_proc "$DAEMON" "$NAME" && exit 0 || exit $?
+ ;;
+ #reload|force-reload)
+ #
+ # If do_reload() is not implemented then leave this commented out
+ # and leave 'force-reload' as an alias for 'restart'.
+ #
+ #log_daemon_msg "Reloading $DESC" "$NAME"
+ #do_reload
+ #log_end_msg $?
+ #;;
+ restart|force-reload)
+ #
+ # If the "reload" option is implemented then remove the
+ # 'force-reload' alias
+ #
+ log_daemon_msg "Restarting $DESC" "$NAME"
+ do_stop
+ case "$?" in
+ 0|1)
+ do_start
+ case "$?" in
+ 0) log_end_msg 0 ;;
+ 1) log_end_msg 1 ;; # Old process is still running
+ *) log_end_msg 1 ;; # Failed to start
+ esac
+ ;;
+ *)
+ # Failed to stop
+ log_end_msg 1
+ ;;
+ esac
+ ;;
+ *)
+ #echo "Usage: $SCRIPTNAME {start|stop|restart|reload|force-reload}" >&2
+ echo "Usage: $SCRIPTNAME {start|stop|status|restart|force-reload}" >&2
+ exit 3
+ ;;
+esac
+
+:
diff --git a/t/tests/systemd-general/debian/debian/install b/t/tests/systemd-general/debian/debian/install
new file mode 100644
index 0000000..363d12a
--- /dev/null
+++ b/t/tests/systemd-general/debian/debian/install
@@ -0,0 +1,2 @@
+debian/test.service etc/systemd/system/
+debian/test.conf etc/tmpfiles.d/
diff --git a/t/tests/systemd-general/debian/debian/test.conf b/t/tests/systemd-general/debian/debian/test.conf
new file mode 100644
index 0000000..b0c4604
--- /dev/null
+++ b/t/tests/systemd-general/debian/debian/test.conf
@@ -0,0 +1,2 @@
+# See tmpfiles.d(5) for details
+d /var/run/bacula 2775 bacula bacula -
diff --git a/t/tests/systemd-general/debian/debian/test.service b/t/tests/systemd-general/debian/debian/test.service
new file mode 100644
index 0000000..9d73aca
--- /dev/null
+++ b/t/tests/systemd-general/debian/debian/test.service
@@ -0,0 +1,9 @@
+[Unit]
+After=network.target \
+syslog.target
+
+[Service]
+ExecStart=/usr/bin/test
+
+[Install]
+WantedBy=multi-user.target
diff --git a/t/tests/systemd-general/desc b/t/tests/systemd-general/desc
new file mode 100644
index 0000000..5de48b4
--- /dev/null
+++ b/t/tests/systemd-general/desc
@@ -0,0 +1,10 @@
+Testname: systemd-general
+Sequence: 6000
+Version: 1.0
+Description: General systemd tests
+Test-For:
+ systemd-service-file-outside-lib
+ systemd-tmpfiles.d-outside-usr-lib
+ systemd-service-file-refers-to-obsolete-target
+ systemd-no-service-for-init-script
+ init.d-script-does-not-source-init-functions
diff --git a/t/tests/systemd-general/tags b/t/tests/systemd-general/tags
new file mode 100644
index 0000000..e01a0ec
--- /dev/null
+++ b/t/tests/systemd-general/tags
@@ -0,0 +1,5 @@
+E: systemd-general: systemd-no-service-for-init-script systemd-general
+E: systemd-general: systemd-service-file-outside-lib etc/systemd/system/test.service
+E: systemd-general: systemd-tmpfiles.d-outside-usr-lib etc/tmpfiles.d/test.conf
+W: systemd-general: init.d-script-does-not-source-init-functions etc/init.d/systemd-general
+W: systemd-general: systemd-service-file-refers-to-obsolete-target etc/systemd/system/test.service syslog.target
--
1.7.10.4
>From 5d2f28433e3dbf5d2fef10be36d8f774742aea17 Mon Sep 17 00:00:00 2001
From: Michael Stapelberg <michael@stapelberg.de>
Date: Wed, 17 Apr 2013 22:49:37 +0200
Subject: [PATCH 3/3] systemd: use is_ancestor_of instead of just
link_resolved
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Quote from Niels’ email:
I thought this was safe, but it does have an issue as well. Consider
symlink chaining:
safe-symlink -> unsafe-symlink
unsafe-symlink -> ../../../../etc/passwd
$path->link_resolved will approve "safe-symlink" because it can be
resolved safely. However, it does not check that the target is also a
safe symlink - so a loop/recursion is needed. That said, using the new
"is_ancestor_of" (from L::Util) is probably a lot easier to use
correctly.
---
checks/systemd | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/checks/systemd b/checks/systemd
index 866110c..88a5377 100644
--- a/checks/systemd
+++ b/checks/systemd
@@ -30,7 +30,7 @@ use File::Basename;
use Text::ParseWords qw(shellwords);
use Lintian::Tags qw(tag);
-use Lintian::Util qw(fail);
+use Lintian::Util qw(fail is_ancestor_of);
sub run {
my ($pkg, $type, $info) = @_;
@@ -85,9 +85,9 @@ sub check_init_script {
my ($pkg, $info, $file) = @_;
my $lsb_source_seen;
- my $path = $info->index ($file);
- unless ($path->is_regular_file ||
- ($path->is_symlink && defined($path->link_resolved))) {
+ my $unpacked_file = $info->unpacked ($file);
+ unless (-f $unpacked_file &&
+ is_ancestor_of ($info->unpacked, $unpacked_file)) {
tag 'init-script-is-not-a-file', $file;
return;
}
@@ -156,9 +156,10 @@ sub extract_service_file_values {
my @values;
my $section;
- my $path = $info->index ($file);
- unless ($path->is_regular_file ||
- ($path->is_symlink && defined($path->link_resolved))) {
+
+ my $unpacked_file = $info->unpacked ($file);
+ unless (-f $unpacked_file &&
+ is_ancestor_of ($info->unpacked, $unpacked_file)) {
tag 'service-file-is-not-a-file', $file;
return;
}
--
1.7.10.4
Reply to: