Ralf Habacker [Wed, 4 Feb 2015 12:50:03 +0000 (13:50 +0100)]
Provide appropriate DBUS_USER and DBUS_TEST_USER under CMake
[separated out from a larger commit -smcv]
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=88964 Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Ralf Habacker [Wed, 4 Feb 2015 12:23:34 +0000 (13:23 +0100)]
Link tests to test-utils-glib.c under CMake too
[Separated out from a larger commit -smcv]
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=88964 Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Simon McVittie [Tue, 3 Feb 2015 19:35:39 +0000 (19:35 +0000)]
Treat root as a valid candidate for TEST_USER_ME
If spawn_dbus_daemon() can fail for TEST_USER_ME, then we'd have to
go through all the tests adding the ability to skip tests after
it fails, which is a fairly extensive change.
The tests have historically all run as whatever uid is supplied, and
if the tests are being run as root for some reason - perhaps in a CI
framework for an embedded platform that doesn't have non-root users,
or in an environment where you can be root or non-root but not both -
there is no particular reason to skip them.
Simon McVittie [Mon, 26 Jan 2015 19:12:01 +0000 (19:12 +0000)]
bus driver: factor out bus_driver_check_caller_is_privileged, and allow root
Unlike the initial mitigation for CVE-2014-8148, we now allow
uid 0 to call UpdateActivationEnvironment. There's no point in root
doing that, but there's also no reason why it's particularly bad -
if an attacker is uid 0 we've already lost - and it simplifies
use of this function for future things that do want to be callable
by root, like BecomeMonitor for #46787.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=88810 Reviewed-by: Philip Withnall
Simon McVittie [Mon, 26 Jan 2015 15:47:22 +0000 (15:47 +0000)]
test: implement GLib-style "installed tests"
We run each test twice:
* once with the system's session.conf, as an integration test
(test-cases that need a special configuration are automatically
skipped)
* once with our special test configuration files, which provide better
coverage
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=88810 Reviewed-by: Philip Withnall
Simon McVittie [Mon, 2 Feb 2015 16:04:52 +0000 (16:04 +0000)]
Bump required GLib version to 2.36
This is for g_close(), which the next commit will use. It also lets us
rely on g_type_init() being a no-op (since 2.32 the type system is
always initialized by a global constructor).
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=88810 Reviewed-by: Philip Withnall
Simon McVittie [Thu, 6 Nov 2014 14:24:14 +0000 (14:24 +0000)]
Use pygi instead of pygobject 2
pygobject 2 is obsolete and unmaintained, and anyway this is for
optional functionality (full regression test coverage) rather than
anything that will be needed in production builds.
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=85969
Simon McVittie [Fri, 19 Dec 2014 18:51:04 +0000 (18:51 +0000)]
Hardening: only accept Stats function calls at the canonical object path
These function calls are not a privilege escalation risk like
UpdateActivationEnvironment, but they might provide sensitive
information or be enhanced to provide sensitive information
in future, so the default system.conf locks them down to root-only.
Apply the same canonical-object-path hardening as for
UpdateActivationEnvironment.
We do not apply the uid check here because they are less dangerous
than UpdateActivationEnvironment, and because the ability to unlock
these function calls for specific uids is a documented configuration
for developers.
Simon McVittie [Fri, 19 Dec 2014 19:19:00 +0000 (19:19 +0000)]
Hardening: only allow the uid of the dbus-daemon to call UpdateActivationEnvironment
As with the previous commit, this is probably not actually privilege
escalation due to the use of an activation helper that cleans up its
environment, but let's be extra-careful here.
Simon McVittie [Fri, 19 Dec 2014 18:49:33 +0000 (18:49 +0000)]
Hardening: reject UpdateActivationEnvironment on non-canonical path
UpdateActivationEnvironment is the one dbus-daemon API call that is
obviously dangerous (it is intended for the session bus),
so the default system.conf does not allow anyone to call it.
It has recently come to the D-Bus maintainers' attention that some
system services incorrectly install D-Bus policy rules that allow
arbitrary method calls to any destination as long as they have a
"safe" object path. This is not actually safe: some system services
that use low-level D-Bus bindings like libdbus, including dbus-daemon
itself, provide the same API on all object paths.
Unauthorized calls to UpdateActivationEnvironment are probably just
resource consumption rather than privilege escalation, because on
the system bus, the modified environment is only used to execute
a setuid wrapper that avoids LD_PRELOAD etc. via normal setuid
handling, and sanitizes its own environment before executing
the real service. However, it's safest to assume the worst and
treat it as a potential privilege escalation.
Accordingly, as a hardening measure to avoid privilege escalation on
systems with these faulty services, stop allowing calls to
("/com/example/Whatever",
"org.freedesktop.DBus.UpdateActivationEnvironment")
and only allow ("/org/freedesktop/DBus",
"org.freedesktop.DBus.UpdateActivationEnvironment").
We deliberately continue to provide read-only APIs like
GetConnectionUnixUser at all object paths, for backwards compatibility.
Reviewed-by: Thiago Macieira <thiago@kde.org>
[adjusted commit message to note that this is probably only DoS -smcv]
It appears this change may cause intermittent slow or failed boot,
more commonly on slower/older machines, in at least Mageia and
possibly also Debian. This would indicate that while the system
is under load, system services are not completing authentication
within 5 seconds.
This change was not the main part of fixing CVE-2014-3639, but does
help to mitigate that attack. As such, increasing this timeout makes
the denial of service attack described by CVE-2014-3639 somewhat
more effective: a local user connecting to the system bus repeatedly
from many parallel processes can cause other users' attempts to
connect to take longer.
If your machine boots reliably with the shorter timeout, and
resilience against local denial of service attacks is important
to you, putting this in /etc/dbus-1/system-local.conf
or a file matching /etc/dbus-1/system.d/*.conf can restore
the lower limit:
Jacek Bukarewicz [Fri, 14 Nov 2014 18:39:38 +0000 (18:39 +0000)]
Set error when message delivery is denied due to receive rule
This makes bus_context_check_security_policy follow convention of
setting errors if function indicates failure and has error parameter.
Notable implication is that AccessDenied error will be sent if sending message
to addressed recipient is denied due to receive rule. Previously, message
was silently dropped.
This also fixes assertion failure when message is denied at addressed recipient
while sending pending auto activation messages.
Simon McVittie [Tue, 4 Nov 2014 14:41:54 +0000 (14:41 +0000)]
CVE-2014-7824: set fd rlimit to 64k for the system dbus-daemon
This ensures that our rlimit is actually high enough to avoid the
denial of service described in CVE-2014-3636 part A.
CVE-2014-7824 has been allocated for this incomplete fix.
Restore the original rlimit for activated services, to avoid
them getting undesired higher limits.
(Thanks to Alban Crequy for various adjustments which have been
included in this commit.)
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=85105 Reviewed-by: Alban Crequy <alban.crequy@collabora.co.uk>
Simon McVittie [Mon, 15 Sep 2014 17:37:59 +0000 (18:37 +0100)]
Add NetBSD to the list of platforms where credentials-passing a pid should work
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=69702 Reviewed-by: Patrick Welche <prlw1@cam.ac.uk> Reviewed-by: Alban Crequy <alban.crequy@collabora.co.uk> Tested-by: Patrick Welche <prlw1@cam.ac.uk>
Simon McVittie [Mon, 15 Sep 2014 17:38:49 +0000 (18:38 +0100)]
test_processid: only assert that it works if we expect it to work
Otherwise, this would fail on, for instance, QNX.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=69702 Reviewed-by: Patrick Welche <prlw1@cam.ac.uk> Reviewed-by: Alban Crequy <alban.crequy@collabora.co.uk> Tested-by: Patrick Welche <prlw1@cam.ac.uk>
Patrick Welche [Thu, 30 Oct 2014 12:39:26 +0000 (12:39 +0000)]
whitespace/comment fixes
[originally part of the previous commit -smcv]
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=69702 Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk> Reviewed-by: Alban Crequy <alban.crequy@collabora.co.uk>
Patrick Welche [Tue, 29 Jul 2014 13:08:20 +0000 (14:08 +0100)]
Implement NetBSD credentials-passing with LOCAL_PEEREID
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=69702 Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk> Reviewed-by: Alban Crequy <alban.crequy@collabora.co.uk> Tested-by: Patrick Welche <prlw1@cam.ac.uk>
Simon McVittie [Mon, 15 Sep 2014 17:04:36 +0000 (18:04 +0100)]
dbus-daemon test: don't assert we pass uid/pid on unknown Unix platforms
We know that Linux, FreeBSD and OpenBSD are "first class citizens"
for credentials-passing, with NetBSD not far behind: people have
turned up on the bug tracking system and told us that tests passed.
On other Unixes, we can't really assert that it works, until someone
who runs them tells us that it worked for them.
Additions to these lists are welcome.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=69702 Reviewed-by: Patrick Welche <prlw1@cam.ac.uk> Reviewed-by: Alban Crequy <alban.crequy@collabora.co.uk> Tested-by: Patrick Welche <prlw1@cam.ac.uk>
Simon McVittie [Thu, 11 Sep 2014 09:59:32 +0000 (10:59 +0100)]
dbus-spawn: do not forget the exec() errno when the grandchild exits
As is already noted in a comment in
_dbus_babysitter_set_child_exit_error(), if the grandchild fails
to exec() the desired process, we get both CHILD_EXEC_FAILED (with
an errno) and CHILD_EXITED (with a status), and we want to report
the former, since it is more informative. However, clearing
sitter->errnum meant we lose the errno value.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=24821 Reviewed-by: Ross Lagerwall
Simon McVittie [Wed, 8 May 2013 15:58:08 +0000 (16:58 +0100)]
Stop asserting that we're not using the dummy lock implementation
That implementation no longer exists, so neither 0xABCDEF nor 0xABCDEF2
has any special meaning any more.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=54972 Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk> Reviewed-by: Alban Crequy <alban.crequy@collabora.co.uk>
Simon McVittie [Wed, 29 Oct 2014 14:10:48 +0000 (14:10 +0000)]
Use a better NoReply message for disconnection with reply pending
As an implementation detail, dbus-daemon handles this situation by
artificially triggering a timeout (even if its configured timeout for
method calls is in fact infinite). However, using the same debug message
for both is misleading, and can lead people who are debugging a service
crash to blame dbus-daemon instead, wasting their time.
Simon McVittie [Mon, 8 Sep 2014 19:18:22 +0000 (20:18 +0100)]
Consistently save and restore errno
Some functions in dbus-transport-socket.c make a (wrapped)
socket syscall, then call other APIs, then test the result and
errno of the socket syscall.
This would break horribly if those "other APIs" overwrote errno with
their own value (... and this is part of why errno is an awful API).
Notably, if running under DBUS_VERBOSE, _dbus_verbose() is basically
fprintf(), which sets errno; and our Unix fd-passing support
makes calls of the form _dbus_verbose ("Read/wrote %i unix fds\n", n)
between the syscall and the result processing.
Maybe one day we'll convert all of dbus' syscall wrappers to either
raise a DBusError, or use the "negative errno" convention that systemd
borrowed from the Linux kernel, and in particular, we would need to
do that if we ever ported it to a platform where socket error reporting
was not basically errno. However, in practice everyone uses something
derived from BSD sockets, so "this sets errno, you know what errno is"
is a good enough internal API if we make sure to use it correctly.
Nothing calls _dbus_get_is_errno_nonzero(), so I just removed it instead
of converting it to the new calling convention.
Simon McVittie [Thu, 25 Sep 2014 12:19:24 +0000 (13:19 +0100)]
dbus-spec, dbus-protocol: add ALLOW_INTERACTIVE_AUTHORIZATION flag
Heavily based on a patch from Lennart Poettering.
This is useful for authentication frameworks such as polkit, but this
flag is supposed to be generic, and not be bound to any implementation
of such a framework.
The dbus specification already clarifies that unknown flags must be
ignored, the reference implementation and the other implementations we
checked indeed ignore any new flags, hence we should be fine with
compatibility here.
Simon McVittie [Fri, 24 Oct 2014 12:41:13 +0000 (13:41 +0100)]
test-bus, test-dbus: close any inherited fds from caller
It is probably a bug for them to pass us any fds without close-on-exec;
but apparently CMake has this bug, and so does at least some NetBSD GUI
environment. Cope.
Simon McVittie [Fri, 24 Oct 2014 12:42:57 +0000 (13:42 +0100)]
cmake: only copy session.conf and system.conf into test data dir
Historically, CMake used the glob *.conf.in whereas Autotools listed
the files explicitly. This used to be equivalent, but broke down
when we added example-*.conf.in which are just snippets rather than
complete configuration files (they're intended to go in session.d
or system.d, or otherwise get included by the main config file).