John Johansen [Thu, 13 Feb 2014 19:07:32 +0000 (13:07 -0600)]
Mediation of processes sending and receiving messages
When an AppArmor confined process wants to send or receive a message, a
check is performed to see if the action should be allowed.
When a message is going through dbus-daemon, there are two checks
performed at once. One for the sending process and one for the receiving
process.
The checks are based on the process's label, the bus type, destination,
path, interface, and member, as well as the peer's label and/or
destination name.
This allows for the traditional connection-based enforcement, as well as
any fine-grained filtering desired by the system administrator.
It is important to note that error and method_return messages are
allowed to cut down on the amount of rules needed. If a process was
allowed to send a message, it can receive error and method_return
messages.
An example AppArmor rule that would be needed to allow a process to call
the UpdateActivationEnvironment method of the session bus itself would be:
To let a process acquire any name on any bus, the rule would be:
dbus bind,
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=75113 Signed-off-by: John Johansen <john.johansen@canonical.com>
[tyhicks: Use BusAppArmorConfinement, bug fixes, cleanup, commit msg]
[tyhicks: initialize reserved area at the start of the query string]
[tyhicks: Use empty string for NULL bustypes when building queries] Signed-off-by: Tyler Hicks <tyhicks@canonical.com> Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Tyler Hicks [Thu, 13 Feb 2014 15:59:53 +0000 (09:59 -0600)]
Store AppArmor label of connecting processes
When processes connect the bus, the AppArmor confinement context should
be stored for later use when checks are to be done during message
sending/receiving, acquire a name, and eavesdropping.
Code outside of apparmor.c will need to initialize and unreference the
confinement context, so bus_apparmor_confinement_unref() can no longer
be a static function.
[Move bus_apparmor_confinement_unref back to its old location for
a more reasonable diff -smcv]
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=75113 Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Tyler Hicks [Wed, 12 Feb 2014 23:28:13 +0000 (17:28 -0600)]
Store AppArmor label of bus during initialization
During dbus-daemon initialization, the AppArmor confinement context
should be stored for later use when checks are to be done on messages
to/from the bus itself.
AppArmor confinement contexts are documented in aa_getcon(2). They
contain a confinement string and a mode string. The confinement string
is typically the name of the AppArmor profile confining a given process.
The mode string gives the current enforcement mode of the process
confinement. For example, it may indicate that the confinement should be
enforced or it may indicate that the confinement should allow all
actions with the caveat that actions which would be denied should be
audited.
It is important to note that libapparmor mallocs a single buffer to
store the con and mode strings and separates them with a NUL terminator.
Because of this, only con should be freed.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=75113 Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
[smcv: use BUS_SET_OOM]
[smcv: dbus_set_error doesn't need extra newlines] Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk> Reviewed-by: Tyler Hicks <tyhicks@canonical.com>
John Johansen [Wed, 12 Feb 2014 18:37:41 +0000 (12:37 -0600)]
Initialize AppArmor mediation
When starting dbus-daemon, autodetect AppArmor kernel support and use
the results from parsing the busconfig to determine if mediation should
be enabled.
In the busconfig, "enabled" means that kernel support is autodetected
and, if available, AppArmor mediation occurs in dbus-daemon. In
"enabled" mode, if kernel support is not detected, mediation is
disabled. "disabled" means that mediation does not occur. "required"
means that kernel support must be detected for dbus-daemon to start.
Additionally, when libaudit support is built into dbus-daemon, the
AppArmor initialization routines set up the audit connection.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=75113 Signed-off-by: John Johansen <john.johansen@canonical.com>
[tyhicks: Honor enforcement modes and detect AppArmor dbus rule support]
[tyhicks: fix unreachable return when AppArmor support is built]
[tyhicks: make bus_apparmor_full_init() able to raise a DBusError] Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
[smcv: _bus_apparmor_aa_supports_dbus: document necessary kernel API guarantee]
[smcv: bus_apparmor_pre_init: distinguish between OOM and AppArmor not enabled]
[smcv: document why we open() and not just stat()] Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk> Reviewed-by: Tyler Hicks <tyhicks@canonical.com>
Tyler Hicks [Tue, 11 Feb 2014 01:02:04 +0000 (19:02 -0600)]
Add apparmor element support to bus config parsing
The <apparmor> element can contain a single mode attribute that has one
of three values:
"enabled"
"disabled"
"required"
"enabled" means that kernel support is autodetected and, if available,
AppArmor mediation occurs in dbus-daemon. If kernel support is not
detected, mediation is disabled. "disabled" means that mediation does
not occur. "required" means that kernel support must be detected for
dbus-daemon to start.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=75113 Signed-off-by: Tyler Hicks <tyhicks@canonical.com> Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Tyler Hicks [Mon, 10 Feb 2014 23:40:03 +0000 (17:40 -0600)]
Update autoconf file to build against libapparmor
AppArmor support can be configured at build time with --enable-apparmor
and --disable-apparmor. By default, the build time decision is
automatically decided by checking if a sufficient libapparmor is
available.
A minimum required libapparmor is version 2.8.95.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=75113 Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
[smcv: avoid potential non-portability from "test EXPR -a EXPR"] Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk> Reviewed-by: Tyler Hicks <tyhicks@canonical.com>
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