Each connection that is an active monitor holds a pointer to its own
link in this list, via BusConnectionData.link_in_monitors. We can't
validly free the list while these pointers exist: that would be a
use-after-free, when each connection gets disconnected and tries to
remove itself from the list.
Instead, let each connection remove itself from the list, then assert
that the list has become empty.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Resolves: https://gitlab.freedesktop.org/dbus/dbus/issues/291
Ralf Habacker [Thu, 20 Feb 2020 08:50:00 +0000 (08:50 +0000)]
In _dbus_verbose_real() avoid possible stack overflows on output to the Windows debug port
Instead of creating a fixed memory area on the stack that can lead to
a stack overflow if exceeded, this configuration now uses a DBusString
instance that dynamically manages memory.
Ralf Habacker [Thu, 12 Dec 2019 08:34:58 +0000 (09:34 +0100)]
Add a trivial sanity-check for the atomic primitives
This doesn't verify that they're atomic, but does verify that they
return the right things.
This commit adds a new test function _dbus_test_check (a) to make
writing tests easier. It checks the given boolean expression and
generates a "not ok" test result if the expression is false.
Due to the current design of the test api, the test is only compiled
if embedded tests were enabled at the time of configuration.
It was also necessary to move the test_atomic target definitions in
test/Makefile.am to the --enable-embedded-tests section to avoid a
make distcheck build error.
The test case itself has been authored by smcv.
Co-authored-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Fri, 6 Dec 2019 01:54:18 +0000 (02:54 +0100)]
_dbus_modify_sigpipe: be thread-safe
This needs new atomic primitives: we don't have "set to a value",
and in fact that's a bit annoying to implement in terms of gcc
intrinsics. "Set to 0" and "set to nonzero" are easy, though.
Currently, if the "dbus" security class or the associated AV doesn't
exist, dbus-daemon fails to initialize and exits immediately. Also the
security classes or access vector cannot be reordered in the policy.
This can be a problem for people developing their own policy or trying
to access a machine where, for some reasons, there is not policy defined
at all.
The code here copy the behaviour of the selinux_check_access() function.
We cannot use this function here as it doesn't allow us to define the
AVC entry reference.
See the discussion at https://marc.info/?l=selinux&m=152163374332372&w=2
Clients listening for a signal can match against the 'sender', expecting
it to come from a connection with a specific name. With this change,
dbus-send can send signals to them.
Simon McVittie [Wed, 3 Jul 2019 08:58:04 +0000 (09:58 +0100)]
tests: Move _dbus_sha_test outside libdbus
Instead of exposing _dbus_sha_test() as a private exported symbol,
we can expose _dbus_sha_compute(), which is the only thing called by
the test that isn't already exported.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Sat, 29 Jun 2019 15:45:25 +0000 (16:45 +0100)]
bus tests: Shut down audit socket
Some CI environments run build-time tests as root with CAP_AUDIT_WRITE.
In this case we need to close the audit socket so that it will not be
reported as leaked.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Sat, 29 Jun 2019 16:36:36 +0000 (17:36 +0100)]
bus: Make audit initialization idempotent
The audit module is initialized every time a new BusContext is created,
which is only once in the real dbus-daemon, but can happen several times
in some unit tests.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Sat, 29 Jun 2019 14:16:03 +0000 (15:16 +0100)]
tests: Skip if unable to launch uninstalled dbus-daemon as other uid
Some CI systems do the entire build as uid 0 in a throwaway container.
If this is done in a build directory for which the messagebus user
does not have search (+x) permission, then they will be unable to
execute the just-built dbus-daemon binary.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Tue, 2 Jul 2019 19:42:54 +0000 (20:42 +0100)]
tests: Skip system bus test if we are root but messagebus does not exist
Some CI systems do the build as root in a disposable container, and
run tests without ever having installed dbus. This means we can't
expect to be able to drop privileges from root to the DBUS_USER (usually
named messagebus or dbus) unless we have checked that the
DBUS_USER exists.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Thu, 30 May 2019 11:58:28 +0000 (12:58 +0100)]
test: Add basic test coverage for DBUS_COOKIE_SHA1
We don't actually complete successful authentication, because that
would require us to generate a cookie and compute the correct SHA1,
which is difficult to do in a deterministic authentication script.
However, we do assert that dbus#269 (CVE-2019-12749) has been fixed.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Thu, 30 May 2019 11:53:03 +0000 (12:53 +0100)]
auth: Reject DBUS_COOKIE_SHA1 for users other than the server owner
The DBUS_COOKIE_SHA1 authentication mechanism aims to prove ownership
of a shared home directory by having the server write a secret "cookie"
into a .dbus-keyrings subdirectory of the desired identity's home
directory with 0700 permissions, and having the client prove that it can
read the cookie. This never actually worked for non-malicious clients in
the case where server uid != client uid (unless the server and client
both have privileges, such as Linux CAP_DAC_OVERRIDE or traditional
Unix uid 0) because an unprivileged server would fail to write out the
cookie, and an unprivileged client would be unable to read the resulting
file owned by the server.
Additionally, since dbus 1.7.10 we have checked that ~/.dbus-keyrings
is owned by the uid of the server (a side-effect of a check added to
harden our use of XDG_RUNTIME_DIR), further ruling out successful use
by a non-malicious client with a uid differing from the server's.
Joe Vennix of Apple Information Security discovered that the
implementation of DBUS_COOKIE_SHA1 was susceptible to a symbolic link
attack: a malicious client with write access to its own home directory
could manipulate a ~/.dbus-keyrings symlink to cause the DBusServer to
read and write in unintended locations. In the worst case this could
result in the DBusServer reusing a cookie that is known to the
malicious client, and treating that cookie as evidence that a subsequent
client connection came from an attacker-chosen uid, allowing
authentication bypass.
This is mitigated by the fact that by default, the well-known system
dbus-daemon (since 2003) and the well-known session dbus-daemon (in
stable releases since dbus 1.10.0 in 2015) only accept the EXTERNAL
authentication mechanism, and as a result will reject DBUS_COOKIE_SHA1
at an early stage, before manipulating cookies. As a result, this
vulnerability only applies to:
* system or session dbus-daemons with non-standard configuration
* third-party dbus-daemon invocations such as at-spi2-core (although
in practice at-spi2-core also only accepts EXTERNAL by default)
* third-party uses of DBusServer such as the one in Upstart
Avoiding symlink attacks in a portable way is difficult, because APIs
like openat() and Linux /proc/self/fd are not universally available.
However, because DBUS_COOKIE_SHA1 already doesn't work in practice for
a non-matching uid, we can solve this vulnerability in an easier way
without regressions, by rejecting it early (before looking at
~/.dbus-keyrings) whenever the requested identity doesn't match the
identity of the process hosting the DBusServer.
Signed-off-by: Simon McVittie <smcv@collabora.com> Closes: https://gitlab.freedesktop.org/dbus/dbus/issues/269 Closes: CVE-2019-12749
Simon McVittie [Thu, 30 May 2019 14:38:49 +0000 (15:38 +0100)]
bus: Clarify names of methods that query owned names
It wasn't immediately clear from the names of these method whether they
should return TRUE or FALSE for queued owners other than the primary
owner. Renaming them makes it obvious that the answer should be TRUE.
While I'm there, make the corresponding _dbus_verbose() messages more
precise.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Resolves: https://gitlab.freedesktop.org/dbus/dbus/issues/270
Adrian Szyndela [Thu, 25 Apr 2019 07:35:59 +0000 (09:35 +0200)]
test: send_destination(_prefix) tests
This adds tests for mostly "send_destination_prefix" cases
and some "send_destination" cases.
The general test case is:
- addressed recipient is running and owns a name;
- a message is sent to the name owner;
- the response is checked for allow/deny (method return/error).
Each test case is executed both for primary and queued ownership.
The tests include:
- checking send allow/deny for names and namespaces, including nesting;
- checking send allow/deny for neighbour names;
- checking send allow/deny for names/namespaces+interface+member.
Signed-off-by: Adrian Szyndela <adrian.s@samsung.com>
Change-Id: If5fcada01601355e7aadefadad79c0b24f8c397f
Adrian Szyndela [Wed, 28 Nov 2018 14:15:16 +0000 (15:15 +0100)]
dbus-daemon: add send_destination_prefix support
This extends dbus-daemon with support for send_destination_prefix
attribute in XML policies.
It allows having policy rules for sending to bus names generated
within namespaces defined by a prefix. The similar behaviour can be
emulated by owning an additional name, not used for addressing messages,
as described in
https://lists.freedesktop.org/archives/dbus/2017-May/017188.html
However, introducing send_destination_prefix creates possibility
of communicating intentions in a more direct way, which is easier
to understand.
Signed-off-by: Adrian Szyndela <adrian.s@samsung.com>
Change-Id: I0016ad93f1c16b7742fef5f45ebaf01b55694d3c
Adrian Szyndela [Wed, 16 Jan 2019 11:30:22 +0000 (12:30 +0100)]
DBusString: extend with checking for starting with words
This extracts a few lines of code and adds it as a DBusString function
that checks if a DBusString starts with words given with a C string
and a word separator. In other words, it checks if:
- a DBusString is a given C string, or
- a DBusString starts with a given C string and the next character is
a given word separator.
It is used for matching names to prefixes when checking the policy.
Signed-off-by: Adrian Szyndela <adrian.s@samsung.com>
Change-Id: Ie39d33916863d950dde38d3b8b20c8a539217302
Simon McVittie [Mon, 11 Mar 2019 09:03:39 +0000 (09:03 +0000)]
bus: Try to raise soft fd limit to match hard limit
Linux systems have traditionally set the soft limit to 1024 and the hard
limit to 4096. Recent versions of systemd keep the soft fd limit at
1024 to avoid breaking programs that still use select(), but raise the
hard limit to 512*1024, while in recent Debian versions a complicated
interaction between components gives a soft limit of 1024 and a hard
limit of 1024*1024. If we can, we might as well elevate our soft limit
to match the hard limit, minimizing the chance that we will run out of
file descriptor slots.
Unlike the previous code to raise the hard and soft limits to at least
65536, we do this even if we don't have privileges: privileges are
unnecessary to raise the soft limit up to the hard limit.
If we *do* have privileges, we also continue to raise the hard and soft
limits to at least 65536 if they weren't already that high, making
it harder to carry out a denial of service attack on the system bus on
systems that use the traditional limit (CVE-2014-7824).
As was previously the case on the system bus, we'll drop the limits back
to our initial limits before we execute a subprocess for traditional
(non-systemd) activation, if enabled.
systemd activation doesn't involve us starting subprocesses at all,
so in both cases activated services will still inherit the same limits
they did previously.
Reviewed-by: Lennart Poettering <lennart@poettering.net>
[smcv: Correct a comment based on Lennart's review, reword commit message] Signed-off-by: Simon McVittie <smcv@collabora.com>
Topi Miettinen [Sat, 23 Mar 2019 20:18:11 +0000 (22:18 +0200)]
Configure option to disable traditional activation
Traditional activation could be disabled if all services use
SystemdService activation instead. Provide an example of a hardened
DBus systemd service drop-in file for such a setup.
Clemens Lang [Fri, 8 Feb 2019 14:48:15 +0000 (15:48 +0100)]
cmake: Avoid overwriting PKG_CONFIG_PATH env var
The CMake config file installed by DBus will run in the context of other
projects. Consequently, changing the value of the PKG_CONFIG_DIR,
PKG_CONFIG_PATH or PKG_CONFIG_LIBDIR environment variables will affect
any further calls to pkg-config made by such projects, which can cause
problems.
A common case of this happening are pkg-config files installed in
usr/share/pkgconfig for .pc files that are architecture-independent, as
for example systemd does.
Avoid clobbering the environment variables by saving and restoring their
values. Note that for some of the variables, setting them to an empty
string is different from not setting them at all.
Signed-off-by: Clemens Lang <clemens.lang@bmw-carit.de>