Philip Withnall [Thu, 5 Feb 2015 12:08:03 +0000 (12:08 +0000)]
doc: Add a guide to designing D-Bus APIs
This guide gives some pointers on how to write D-Bus APIs which are nice
to use.
It adds an optional dependency on Ducktype and yelp-build from
yelp-tools. These are used when available, but are not required unless
--enable-ducktype-docs is passed to configure. They are required for
uploading the docs, however.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=88994 Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
dbus-send could already pretty-print bytestrings that do not have
\0 termination, but those are awkward to work with (they need copying),
so they are now discouraged. Teach it to print bytestrings that
do have \0 termination as well.
In the process, rewrite this part of the message parser
to use dbus_message_iter_get_fixed_array(), which is the Right way
to get arrays of numbers out of a message.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=89109 Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Simon McVittie [Wed, 11 Feb 2015 11:47:15 +0000 (11:47 +0000)]
Reduce the number of fds the fdpass test uses
It was relying on a higher-than-default fd limit; cut it down to
more than 256 but rather less than 1024, since the default Linux
limit is 1024 fds per user.
Also automatically skip this test if our rlimit is too small.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=88998 Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Without this code change, non-systemd processes can make dbus-daemon
think systemd failed to activate a system service, resulting in an
error reply back to the requester. In practice we can address this in
system.conf by only allowing root to forge these messages, but this
check is the real solution, particularly on systems where root is
not all-powerful.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=88811 Reviewed-by: Alban Crequy Reviewed-by: David King Reviewed-by: Philip Withnall
Simon McVittie [Thu, 5 Feb 2015 12:47:32 +0000 (12:47 +0000)]
tests: make sure to specify CPPFLAGS where needed
test-marshal and test-syntax need the
$(testutils_shared_if_possible_cppflags), so that they will get the
$(static_cflags) when we are not linking to dbus-glib.
Simon McVittie [Mon, 2 Feb 2015 20:08:07 +0000 (20:08 +0000)]
dbus-monitor: add support for using BecomeMonitor to be a read-only monitor
Move the dbus_connection_add_filter() call further up as a precaution,
because it isn't safe for a monitor to not have a filter that
swallows all messages.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=46787 Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Simon McVittie [Mon, 2 Feb 2015 20:02:56 +0000 (20:02 +0000)]
Add a regression test for being a new-style monitor
This includes most of the situations I could think of:
* method call on dbus-daemon and response
* NameOwnerChanged
* NameAcquired, NameLost (although I'm not 100% sure these should
get captured, since they're redundant with NameOwnerChanged)
* unicast message is allowed through
* unicast message is rejected by no-sending or no-receiving policy
* broadcast is allowed through
* broadcast is rejected by no-sending policy (the error reply
is also captured)
* broadcast is rejected by no-receiving policy (there is no error
reply)
* message causing service activation, and the message telling systemd
to do the actual activation
* systemd reporting that activation failed
It does not cover:
* sending a message to dbus-daemon, then provoking a reply, then
dbus-daemon does not allow itself to send the reply due to its
own security policy
This is such an obscure corner case that I'm not even convinced it's
testable without dropping down into lower-level socket manipulation:
dbus-daemon's replies are always assumed to be requested replies,
and replies contain so little other metadata that I think we can
only forbid them by forbidding all method replies. If we do that,
the reply to Hello() won't arrive and the client-side connection will
not become active.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=46787 Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Simon McVittie [Fri, 23 Jan 2015 19:11:31 +0000 (19:11 +0000)]
Add support for morphing a D-Bus connection into a "monitor"
This is a special connection that is not allowed to send anything,
and loses all its well-known names.
In future commits, it will get a new set of match rules and the
ability to eavesdrop on messages before the rest of the bus daemon
has had a chance to process them.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=46787 Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Simon McVittie [Mon, 26 Jan 2015 20:09:56 +0000 (20:09 +0000)]
CVE-2015-0245: prevent forged ActivationFailure from non-root processes
Without either this rule or better checking in dbus-daemon, non-systemd
processes can make dbus-daemon think systemd failed to activate a system
service, resulting in an error reply back to the requester.
This is redundant with the fix in the C code (which I consider to be
the real solution), but is likely to be easier to backport.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=88811 Reviewed-by: Alban Crequy Reviewed-by: David King Reviewed-by: Philip Withnall
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]