Don't just exercise _dbus_get_standard_session_servicedirs(), but
also its integration into the BusConfigParser.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99825
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
We can rely on the Autotools build system to pass in some safe values
for XDG_DATA_HOME and XDG_DATA_DIRS that match DBUS_TEST_BUILDDIR.
This test will now be skipped when running test-bus manually,
or under the CMake build system. Under CMake it could be reinstated
by setting the right environment variables.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99825
Reviewed-by: Philip Withnall <withnall@endlessm.com>
[smcv: add missing newline as requested]
[smcv: align DBUS_TEST_BUILDDIR with G_TEST_BUILDDIR]
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
There's little point in asserting that the defaults (without
setting XDG_DATA_HOME, etc.) end with share/dbus-1/services,
because we are about to re-test with known values for XDG_DATA_HOME
etc., at which point we can check exact values which is more strict.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99825
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
progs was never used, because it was originally only used on
Windows, where this test makes no sense and so is no longer run.
It is unnecessary to check that the system service directories end
with dbus-1/system-services, because we are going to check their
exact values a short time later anyway.
It is also unnecessary to set XDG_DATA_HOME and XDG_DATA_DIRS,
because those variables are no longer respected for system service
directories, only for session service directories.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99825
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
There are two circumstances in which we load .service files. The first
is bus_activation_reload(), which is given an ordered list of directory
paths, and reads each one in its correct order, highest-precedence
first (normally ~/.local/share > /usr/local/share > /usr/share). This
seems correct.
However, if we are asked to activate a service for which we do not know
of a .service file, we opportunistically reload the search path and
try again, in the hope that it was recently-installed and not yet
discovered by inotify. Prior to this commit, this would iterate through
the hash table in arbitrary hash order, so we might load a service
from /usr/share even though it was meant to be masked by a
higher-priority service file in ~/.local/share or /usr/local/share.
Before I add more elements to the search path, we should make sure
it is always searched in the expected order.
We do not actually make use of the hash table's faster-than-O(n)
lookup by directory path anywhere, so there is no point in using a
hash table, and we can safely replace it with an ordered data structure.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99825
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99825
Reviewed-by: Philip Withnall <withnall@endlessm.com>
[smcv: also correct the same thing for system service directories]
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
We had two ways to append a path to the list of service directories.
Collapse them into one.
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99825
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Creating a directory is atomic, stat'ing it to see whether to remove
it is very much not.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99828
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
check_parse() can return NULL on OOM, which we might as well handle
gracefully in the tests with an assertion, rather than an explosion. At
least it will shut Coverity up.
Coverity ID: 54724
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99643
Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
context is definitely non-NULL at this point, and has been dereferenced
already on all paths leading to it.
Coverity ID: 141062
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99642
Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
This adds a "sysusers.d" snippet for creating the system user "dbus" at
boot, if it is missing, in order to support stateless systems that boot
up with an empty /etc and need static information for determining which
system users to create.
This is only installed on systemd-based systems.
Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99162
With recent code starting dbus-daemon with an unsupported auth mechanism
let dbus-daemon silently ignore this issue. Clients connecting to this
server fails to connect without any descriptive explanation of the
root cause, only the message 'Rejected client connection due to lack
of memory' error is reported in dbus-daemon verbose log, which is disabled
in production environments.
With this patch dbus-daemon checks the supported auth mechanisms on startup
and shuts down with a descriptive error message, which gives admins an
immediate feedback on service startup/restart.
Signed-off-by: Ralf Habacker <ralf.habacker@freenet.de>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99622
This is almost certainly not going to make a difference, as it’s on the
OOM handling path; but the fewer leaks the better.
Coverity ID: 141058
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99612
Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
The code counting pending fds relied on restart of timeouts when they are
enabled. This patch adds function that ensures that such enabled timeouts
have their timekeeping data reset (and not only when timeout is
registered into event loop processing).
When timeouts weren't reset, they'd fire at rather random and mainly
incorrect moments leading to interruption of connections of dbus-daemon.
Every time we reset the interval, we also need to re-enable the timeout
and mark its end time to be recalculated by the event loop, so combine
the old set_enabled(TRUE) with set_interval() as a new restart() method.
This leaves all the set_enabled() calls having a FALSE parameter, so
remove the parameter and rename the method to disable().
[smcv: fix minor coding style issues]
[smcv: replace set_reenabled()/set_interval() pair with restart()]
[smcv: replace set_enabled(FALSE) with disable()]
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=95619
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
This quiets -Wswitch-enum warnings. The trivial config parser
is used by the setuid activation helper, and only handles the
elements whose contents influence the operation of that helper:
system service directories, the setuid activation helper itself,
the bus uid, and the bus type.
[smcv: split out from a larger commit; add justification; move
ELEMENT_SERVICEDIR start handler to a functionally equivalent list
of elements whose content we are going to process later]
Reviewed-by: Simon McVittie <smcv@debian.org>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=98191
This is clearly equivalent, and quiets -Wswitch-default.
Based on part of a patch by Thomas Zimmermann.
Signed-off-by: Simon McVittie <smcv@debian.org>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=98191
This is a workaround for
<https://bugs.freedesktop.org/show_bug.cgi?id=95263>. If a service
sends a file descriptor sufficiently frequently that its queue of
messages never goes down to 0 fds pending, then it will eventually be
disconnected. logind is one such service.
We do not currently have a good solution for this: the proposed
patches either don't work, or reintroduce a denial of service
security vulnerability (CVE-2014-3637). Neither seems desirable.
However, we can avoid the worst symptoms by trusting uid 0 not to be
malicious.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=95263
Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/systemd/+bug/1591411
Reviewed-by: Łukasz Zemczak
Tested-by: Ivan Kozik
Tested-by: Finn Herpich
Tested-by: autostatic
Tested-by: Ben Parafina
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
(cherry picked from commit d5fae1db78)
[smcv: omit the test/dbus-daemon.c part, which does not apply unless
a363822f5f is also applied]
This is either a denial-of-service attempt, a pathological performance
problem or a dbus-daemon bug. Sysadmins should be told about any of
these.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=86442
[smcv: add units to timeout: it is in milliseconds]
Signed-off-by: Simon McVittie <smcv@debian.org>
(cherry picked from commit 05cb619f0a)
Because the recipient process is not yet available, we have to make some
assumption about its AppArmor profile. Parsing the first word of
the Exec value and then chasing symlinks seems like too much magic,
so I've gone for something more explicit. If the .service file contains
AssumedAppArmorLabel=/foo/bar
then we will do the AppArmor query on the assumption that the recipient
AppArmor label will be as stated. Otherwise, we will do a query
with an unspecified label, which means that AppArmor rules that do
specify a peer label will never match it.
Regardless of the result of this query, we will do an independent
AppArmor query when the activation has actually happened, this time
with the correct peer label; that second query will still be used
to decide whether to deliver the message. As a result, if this change
has any effect, it is to make the bus more restrictive; it does not
allow anything that would previously have been denied.
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=98666
We specifically do not check recipient policies, because
the recipient policy is based on properties of the
recipient process (in particular, its uid), which we do
not necessarily know until we have already started it.
In this initial implementation we do not check LSMs either,
because we cannot know what LSM context the recipient process
is going to have. However, LSM support will need to be added
to make this feature useful, because StartServiceByName is
normally allowed in non-LSM environments, and is more
powerful than auto-activation anyway.
The StartServiceByName method does not go through this check,
because if access to that method has been granted, then
it's somewhat obvious that you can start arbitrary services.
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=98666
This is a workaround for
<https://bugs.freedesktop.org/show_bug.cgi?id=95263>. If a service
sends a file descriptor sufficiently frequently that its queue of
messages never goes down to 0 fds pending, then it will eventually be
disconnected. logind is one such service.
We do not currently have a good solution for this: the proposed
patches either don't work, or reintroduce a denial of service
security vulnerability (CVE-2014-3637). Neither seems desirable.
However, we can avoid the worst symptoms by trusting uid 0 not to be
malicious.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=95263
Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/systemd/+bug/1591411
Reviewed-by: Łukasz Zemczak
Tested-by: Ivan Kozik
Tested-by: Finn Herpich
Tested-by: autostatic
Tested-by: Ben Parafina
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
As a general design principle, strings that we aren't going to modify
should usually be const. When compiling with -Wwrite-strings, quoted
string constants are of type "const char *", causing compiler warnings
when they are assigned to char * variables.
Unfortunately, we need to add casts in a few places:
* _dbus_list_append(), _dbus_test_oom_handling() and similar generic
"user-data" APIs take a void *, not a const void *, so we have
to cast
* For historical reasons the execve() family of functions take a
(char * const *), i.e. a constant pointer to an array of mutable
strings, so again we have to cast
* _dbus_spawn_async_with_babysitter similarly takes a char **,
although we can make it a little more const-correct by making it
take (char * const *) like execve() does
This also incorporates a subsequent patch by Thomas Zimmermann to
put various string constants in static storage, which is a little
more efficient.
Signed-off-by: Simon McVittie <smcv@debian.org>
Reviewed-by: Thomas Zimmermann <tdz@users.sourceforge.net>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=97357
Several internal functions are not used on Windows. This patch
hides them behind DBUS_WIN.
Signed-off-by: Thomas Zimmermann <tdz@users.sourceforge.net>
Reviewed-by: Simon McVittie <smcv@debian.org>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=97357
This isn't security-related, just defensive programming: if
dbus-daemon wasn't run with --systemd-activation, then there is no
reason why systemd would legitimately send us this signal, and if it
does we should just ignore it.
Signed-off-by: Simon McVittie <smcv@debian.org>
Reviewed-by: Colin Walters <walters@verbum.org>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=98157
Specifically, this will allow ActivationFailure messages from our
own uid or from root, but reject them otherwise, even if the bus
configuration for who can own org.freedesktop.systemd1 is entirely
wrong due to something like CVE-2014-8148.
Signed-off-by: Simon McVittie <smcv@debian.org>
Reviewed-by: Colin Walters <walters@verbum.org>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=98157
In principle this could lead to arbitrary memory overwrite via
a format string attack in the message received from systemd,
resulting in arbitrary code execution.
This is not believed to be an exploitable security vulnerability on the
system bus in practice: it can only be exploited by the owner of the
org.freedesktop.systemd1 bus name, which is restricted to uid 0, so
if systemd is attacker-controlled then the system is already doomed.
Similarly, if a systemd system unit mentioned in the activation failure
message has an attacker-controlled name, then the attacker likely already
has sufficient access to execute arbitrary code as root in any case.
However, prior to dbus 1.8.16 and 1.9.10, due to a missing check for
systemd's identity, unprivileged processes could forge activation
failure messages which would have gone through this code path.
We thought at the time that this was a denial of service vulnerability
(CVE-2015-0245); this bug means that it was in fact potentially an
arbitrary code execution vulnerability.
Bug found using -Wsuggest-attribute=format and -Wformat-security.
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Reviewed-by: Colin Walters <walters@verbum.org>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=98157
This isn't security-related, just defensive programming: if
dbus-daemon wasn't run with --systemd-activation, then there is no
reason why systemd would legitimately send us this signal, and if it
does we should just ignore it.
Signed-off-by: Simon McVittie <smcv@debian.org>
Reviewed-by: Colin Walters <walters@verbum.org>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=98157
Specifically, this will allow ActivationFailure messages from our
own uid or from root, but reject them otherwise, even if the bus
configuration for who can own org.freedesktop.systemd1 is entirely
wrong due to something like CVE-2014-8148.
Signed-off-by: Simon McVittie <smcv@debian.org>
Reviewed-by: Colin Walters <walters@verbum.org>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=98157
In principle this could lead to arbitrary memory overwrite via
a format string attack in the message received from systemd,
resulting in arbitrary code execution.
This is not believed to be an exploitable security vulnerability on the
system bus in practice: it can only be exploited by the owner of the
org.freedesktop.systemd1 bus name, which is restricted to uid 0, so
if systemd is attacker-controlled then the system is already doomed.
Similarly, if a systemd system unit mentioned in the activation failure
message has an attacker-controlled name, then the attacker likely already
has sufficient access to execute arbitrary code as root in any case.
However, prior to dbus 1.8.16 and 1.9.10, due to a missing check for
systemd's identity, unprivileged processes could forge activation
failure messages which would have gone through this code path.
We thought at the time that this was a denial of service vulnerability
(CVE-2015-0245); this bug means that it was in fact potentially an
arbitrary code execution vulnerability.
Bug found using -Wsuggest-attribute=format and -Wformat-security.
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Reviewed-by: Colin Walters <walters@verbum.org>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=98157
If it is set (i.e. if an LSM is in use) this will make it appear in
various places in log output.
With SELinux, for example, this appends something like:
label="system_u:object_r:unlabeled_t:s0"
This commit partially rearranges the code which sets the loginfo string,
so that it consistently puts a space between fields, and not one at the
end.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=68212
Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
This clarifies
Activating via systemd: service name='com.example.Example'
unit='example.service'
to
Activating via systemd: service name='com.example.Example'
unit='example.service' requested by ':1.23' (uid 1000 pid 123
comm "whatever-activat")
Similarly for the non-systemd code paths.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=68212
Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
They used to be needed, but are not needed any more, and we were
never completely consistent about including them in any case.
Signed-off-by: Simon McVittie <smcv@debian.org>
In particular this means the test suite won't spam the Journal
any more.
Signed-off-by: Simon McVittie <smcv@debian.org>
Reviewed-by: Ralf Habacker <ralf.habacker@freenet.de>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=97009
Like --fork and --nofork, these override what the configuration says.
Use --syslog-only to force the systemd services to log to the Journal
(via syslog, which means we see the severity metadata) instead of
testing sd_booted() in the configuration implementation.
Signed-off-by: Simon McVittie <smcv@debian.org>
_dbus_log() and _dbus_logv() are always the right functions to call now.
Signed-off-by: Simon McVittie <smcv@debian.org>
Reviewed-by: Ralf Habacker <ralf.habacker@freenet.de>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=97009
This changes the behaviour of _dbus_logv() if _dbus_init_system_log() was
not called. Previously, _dbus_logv() would always log to syslog;
additionally, it would log to stderr, unless the process is dbus-daemon
and it was started by systemd. Now, it will log to stderr only,
unless _dbus_init_system_log() was called first.
This is the desired behaviour because when we hook up
_dbus_warn_check_failed() to _dbus_logv() in the next commit, we don't
want typical users of libdbus to start logging their check failures to
syslog - we only want the dbus-daemon to do that.
In practice this is not usually a behaviour change, because there was
only one situation in which we called _dbus_logv() without first calling
_dbus_init_system_log(), namely an error while parsing configuration
files. Initialize the system log "just in time" in that situation
to preserve existing behaviour.
Signed-off-by: Simon McVittie <smcv@debian.org>
This is either a denial-of-service attempt, a pathological performance
problem or a dbus-daemon bug. Sysadmins should be told about any of
these.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=86442
[smcv: add units to timeout: it is in milliseconds]
Signed-off-by: Simon McVittie <smcv@debian.org>
