Simon McVittie [Fri, 30 Sep 2022 12:46:31 +0000 (13:46 +0100)]
dbus-marshal-byteswap: Byte-swap Unix fd indexes if needed
When a D-Bus message includes attached file descriptors, the body of the
message contains unsigned 32-bit indexes pointing into an out-of-band
array of file descriptors. Some D-Bus APIs like GLib's GDBus refer to
these indexes as "handles" for the associated fds (not to be confused
with a Windows HANDLE, which is a kernel object).
The assertion message removed by this commit is arguably correct up to
a point: fd-passing is only reasonable on a local machine, and no known
operating system allows processes of differing endianness even on a
multi-endian ARM or PowerPC CPU, so it makes little sense for the sender
to specify a byte-order that differs from the byte-order of the recipient.
However, this doesn't account for the fact that a malicious sender
doesn't have to restrict itself to only doing things that make sense.
On a system with untrusted local users, a message sender could crash
the system dbus-daemon (a denial of service) by sending a message in
the opposite endianness that contains handles to file descriptors.
Before this commit, if assertions are enabled, attempting to byteswap
a fd index would cleanly crash the message recipient with an assertion
failure. If assertions are disabled, attempting to byteswap a fd index
would silently do nothing without advancing the pointer p, causing the
message's type and the pointer into its contents to go out of sync, which
can result in a subsequent crash (the crash demonstrated by fuzzing was
a use-after-free, but other failure modes might be possible).
In principle we could resolve this by rejecting wrong-endianness messages
from a local sender, but it's actually simpler and less code to treat
wrong-endianness messages as valid and byteswap them.
Simon McVittie [Mon, 12 Sep 2022 12:28:47 +0000 (13:28 +0100)]
test: Add targeted tests for dbus/dbus#413, dbus/dbus#418
Unlike the message-internals test, these do not rely on extra debug
instrumentation in libdbus, and so can be used for "as-installed"
testing. (However, they do require GLib.)
Reproduces: https://gitlab.freedesktop.org/dbus/dbus/-/issues/413
Reproduces: https://gitlab.freedesktop.org/dbus/dbus/-/issues/418 Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Mon, 12 Sep 2022 12:24:28 +0000 (13:24 +0100)]
test: Export G_TEST_BUILDDIR, G_TEST_SRCDIR
These environment variables are used by GLib's g_test_build_filename()
and related convenience functions, which make it easier for unit tests
to find data files in a way that works for both build-time tests and
"as-installed" tests. During "as-installed" testing, both variables
will normally be unset, and GLib uses the directory containing the
executable. In most cases that results in the right thing happening, and
this will also be true for dbus, since we install the test executables
in ${libexecdir}/installed-tests, helper executables in the same place,
and test data in ${libexecdir}/installed-tests/data.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Mon, 12 Sep 2022 12:28:29 +0000 (13:28 +0100)]
test/data: Add messages that reproduce dbus/dbus#413, dbus/dbus#418
In debug builds with "embedded tests" enabled, these will automatically
be used as input for the message-internals test.
Some of the messages themselves are output from a fuzzer, others are
simplifications to include only one reason for lack of validity per
message.
I've included an annotated hex-dump for each message here, but the dbus
test suite doesn't currently know how to convert hex to binary, so I've
also committed the corresponding binary. See the comment at the top of
each hex-dump for how to create the binary version (which requires the
xxd tool shipped with vim).
It would be nice for the dbus test suite to be able to convert the
annotated hex-dump to binary, either at build-time with a Python script
or at runtime by loading the text file and decoding the hex, but I don't
want to block on that for dbus/dbus#413 and dbus/dbus#418.
Reproduces: https://gitlab.freedesktop.org/dbus/dbus/-/issues/413
Reproduces: https://gitlab.freedesktop.org/dbus/dbus/-/issues/418 Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Mon, 12 Sep 2022 12:14:18 +0000 (13:14 +0100)]
dbus-marshal-validate: Validate length of arrays of fixed-length items
This fast-path previously did not check that the array was made up
of an integer number of items. This could lead to assertion failures
and out-of-bounds accesses during subsequent message processing (which
assumes that the message has already been validated), particularly after
the addition of _dbus_header_remove_unknown_fields(), which makes it
more likely that dbus-daemon will apply non-trivial edits to messages.
Thanks: Evgeny Vereshchagin Fixes: e61f13cf "Bug 18064 - more efficient validation for fixed-size type arrays"
Resolves: https://gitlab.freedesktop.org/dbus/dbus/-/issues/413
Resolves: CVE-2022-42011 Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Tue, 13 Sep 2022 14:10:22 +0000 (15:10 +0100)]
dbus-marshal-validate: Check brackets in signature nest correctly
In debug builds with assertions enabled, a signature with incorrectly
nested `()` and `{}`, for example `a{i(u}` or `(a{ii)}`, could result
in an assertion failure.
In production builds without assertions enabled, a signature with
incorrectly nested `()` and `{}` could potentially result in a crash
or incorrect message parsing, although we do not have a concrete example
of either of these failure modes.
Simon McVittie [Thu, 29 Sep 2022 15:50:07 +0000 (16:50 +0100)]
dbus-server-socket: Make unix:tmpdir equivalent to unix:dir
On Linux, there are two classes of AF_UNIX socket, which D-Bus refers
to as unix:path=... (portable to non-Linux systems) and unix:abstract=...
(not portable).
Back in 2003 when dbus gained support for abstract Unix sockets on Linux,
everyone thought they were better in every way than path-based Unix
sockets: if a DBusServer crashes or is terminated abnormally, there's
no detritus left in the filesystem. What's not to like? As a result,
since commit a70b042f (2003-06-04), when a DBusServer listens on a
unix:tmpdir=... address on Linux, the default is for the result to be
a unix:abstract=... address, with unix:path=... addresses only used on
non-Linux platforms.
However, the world has changed in the last 19 years, and namespace-based
Linux containers (which didn't exist in 2003) are now very popular. This
makes abstract sockets problematic.
Abstract sockets are tied to the network namespace, which is
all-or-nothing: if a container is to access the Internet without using
some sort of proxy or intermediary (like slirp4netns) then it needs to
share the network namespace with the host system, and that implies
sharing all abstract sockets with the host system. If the well-known
session bus is listening on an abstract socket, then it's a sandbox
escape route for any sandboxed or containerized app running under the
same uid. Conversely, if a container is *not* sharing the network
namespace with the host system, then it cannot access a session bus that
is listening on an abstract socket without using some sort of proxy
(like xdg-dbus-proxy), even if it isn't intended to impose a security
boundary and giving it direct access to the session bus would have been
more desirable.
Path-based sockets do not have this problem because they exist in the
filesystem (part of the "everything is a file" Unix philosophy),
allowing mount namespaces and bind-mounts to be used to share or
unshare them selectively.
On systems with `systemd --user` where dbus has been configured with
`--enable-user-session`, in general the session bus will already be
using a path-based socket for the "user bus", disregarding the listening
address specified in /usr/share/dbus-1/session.conf. The default in many
recent Linux distributions is either to use dbus-daemon in this way, or
to use dbus-broker, a reimplementation of the message bus service which
has similar "user bus" behaviour.
However, the <listen> address in session.conf is used when dbus-launch(1)
or dbus-run-session(1) is used to start a session bus, either manually,
via autolaunching, or via system integration glue in operating systems
that are not using `systemd --user`. This will occur particularly often
in operating systems that boot using a non-systemd init system.
Making unix:tmpdir=/tmp equivalent to unix:dir=/tmp ensures that the
well-known session bus listens on a path-based socket, allowing container
and sandboxing frameworks to mediate access to it in the same way they
would for the user bus. The D-Bus Specification already allows (but does
not require) this behaviour, because it is the only thing that was
implementable on non-Linux systems such as *BSD.
This change has the potential to cause regressions. If a container
framework enters a chroot or unshares the mount namespace but does not
unshare the network namespace, and is relying on the ability for a
process inside a container to access the session bus outside the
container via its abstract socket, then that assumption will be broken
by this change. Some use cases of schroot(1) are likely to suffer from
this. However, container frameworks with that assumption would already
have found that it does not hold when using the user bus, and it is
necessary to break that assumption if we want it to be possible to apply
application-level sandboxing in a secure way.
Another potential regression from this change is that if a dbus-daemon
is terminated abnormally, it will leave a socket in /tmp. Distributors
of operating systems where heavy use of dbus-launch(1) is expected might
wish to run dbus-cleanup-sockets(1) periodically.
Alex Richardson [Sun, 14 Aug 2022 16:31:10 +0000 (16:31 +0000)]
DBusMessageIter: ensure contiguous layout with 128-bit pointers
I am building DBus targeting the Arm Morello board and the "no padding"
layout assertion fails here since pointers require 16-byte alignment, and
therefore we have to add two additional ints to the DBusMessageIter struct.
As this is a new architecture, where DBus previously failed to compiled
we do not have any layout backwards compatibility requirements, so we can
simplify the DBusMessageIter structure to allocate space for 16 pointers
(which should give us a lot of space for any further changes).
Alex Richardson [Sun, 14 Aug 2022 16:28:56 +0000 (16:28 +0000)]
dbus-message.c: skip 1.10 layout compatibility test on new architectures
These static assertions fail on CHERI-enabled architectures such as Arm
Morello, where pointers are 128 bits. Architectures with 128-bit pointers
were not supported in DBus 1.10, so we can skip the checks for DBus 1.10
structure layout compatibility for architectures with pointer size > 64 bit.
Alex Richardson [Fri, 12 Aug 2022 10:34:06 +0000 (10:34 +0000)]
dbus-marshal-recursive.h: reduce padding in DBusType{Reader,Writer}
When building for Arm Morello (where pointers are 16 bytes), I hit the
static assertion that sizeof (DBusMessageRealIter) <= sizeof (DBusMessageIter)
inside _dbus_message_iter_init_common() otherwise. This can be fixed by
moving the pointers to the beginning of the struct to remove padding.
Alex Richardson [Thu, 15 Sep 2022 18:53:30 +0000 (18:53 +0000)]
dbus-mempool.c: ensure that all alignments are aligned to max_align_t
This is required e.g. for CHERI-enabled targets such as Arm Morello where
aligning to sizeof(long) is not sufficient to load/store pointers (which
need 16 byte alignment instead of 8 bytes).
As we can't depend on C11 yet, this commit adds a max_align_t emulation
to dbus-internals.h.
Alex Richardson [Fri, 12 Aug 2022 10:18:05 +0000 (10:18 +0000)]
dbus-internals: use size_t in _DBUS_ALIGN_VALUE()
When targeting CHERI-enabled architectures such as Arm Morello, performing
a bitwise and with uintptr_t values can result in an ambiguous operation
compiler warning. Fix this warning by telling compiler which operand is
(potentially) a pointer and which one is an integer by changing the
boundary type to size_t. This change has no functional effect on other
architectures but is required to build with -Werror for Morello.
Example warning message:
```
warning: binary expression on capability types 'unsigned __intcap' and 'unsigned __intcap'; it is not clear which should be used as the source of provenance; currently provenance is inherited from the left-hand side [-Wcheri-provenance]
_dbus_assert (_DBUS_ALIGN_VALUE (insert_at, 8) == (unsigned) insert_at);
```
Alex Richardson [Wed, 21 Sep 2022 08:33:33 +0000 (08:33 +0000)]
gitlab-ci: update FreeBSD CI image to 13.1
FreeBSD 13.0 has reached EOL and it appears packages
are built against a newer baseline now, so we end up
with missing symbol errors:
ld-elf.so.1: /lib/libc.so.7: version FBSD_1.7 required by /usr/local/lib/libpython3.9.so.1.0 not found
It also appears that the fdpass test still fails on 13.1, so update the
condition to less than 14.0
Alex Richardson [Fri, 12 Aug 2022 10:15:14 +0000 (10:15 +0000)]
test: Use DBUS_TEST_SOCKET_DIR to create the test socket
I am trying to run cross-compiled tests in QEMU with the build directory
mounted via smbfs, and therefore creating the sockets in the CWD does not
work. Using DBUS_TEST_SOCKET_DIR (/tmp by default) allows me to run the
tests successfully.
Simon McVittie [Wed, 14 Sep 2022 15:23:23 +0000 (16:23 +0100)]
dbus-mempool: Use flexible or zero-length arrays if possible
If the elements field has a fixed nonzero size, accessing elements
beyond that size is technically undefined behaviour, which is caught
by some options of the undefined behaviour sanitizer. Try to use a C99
flexible array, or failing that, a zero-length array (which is a popular
non-standard syntax to achieve the same thing).
dbus 1.15.x has C99 as a requirement, but this commit avoids assuming
C99 in order to make this change backportable to 1.14.x if it becomes
necessary to do so (for example to be able to run tests or fuzzers
against 1.14.x, or if compilers' defaults become more strict).
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Mon, 12 Sep 2022 17:13:48 +0000 (18:13 +0100)]
CI: Don't pin msys2 packages to a specific version at all
Similar to dbus/dbus!286, but more so: just use the package names,
ignoring their version numbers completely.
pcre2 is not strictly needed at the moment, but it'll be a dependency
for GLib >= 2.73.x (older versions used pcre). For a bit of
future-proofing, download both pcre and pcre2.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Mon, 12 Sep 2022 18:36:16 +0000 (19:36 +0100)]
test: Don't use strict TAP parsing on Windows
Debug messages in a background thread can corrupt the machine-readable
TAP output, and in particular GWin32AppInfo emits debug messages from
a background thread when we link to libgio.
Resolves: https://gitlab.freedesktop.org/dbus/dbus/-/issues/414 Signed-off-by: Simon McVittie <smcv@collabora.com>
With internal DBus checks disabled, but with assertions enabled, the
function would be ifdef'ed out. This is problematic, since the function
is called from within an assertion statement in _dbus_variant_write().
Fixes #412.
Co-authored-by: Simon McVittie <smcv@collabora.com>
Alex Richardson [Tue, 9 Aug 2022 10:56:18 +0000 (10:56 +0000)]
test/fdpass.c: skip test that fails on FreeBSD 13.0
The /odd-limit/at test passes on 13.1 and 14.0 images, but fails on 13.1.
Debugging has not given me any useful hints why this may be the case, so
disable this test on 13.0 for now.
This allows us to drop the ci_test_fatal: "no" override which will ensure
that any FreeBSD regressions are caught.
Alex Richardson [Tue, 9 Aug 2022 08:31:03 +0000 (08:31 +0000)]
gitlab-ci: Use GNU make on FreeBSD
The Makefile.am files contain % pattern rules that are not supported by
`make` (bmake) on FreeBSD. Since the replacing the patterns is non-trivial,
this commit updates the CI script to use GNU make when building on FreeBSD.
Alex Richardson [Mon, 8 Aug 2022 21:21:18 +0000 (21:21 +0000)]
gitlab-ci: Install pkg-config on FreeBSD
Without this change the autotools build system fails to find glib and
reports an error. The CMake build worked prior to this change since CMake
has fallback logic to find glib even without pkg-config.
Alex Richardson [Wed, 25 May 2022 20:51:13 +0000 (20:51 +0000)]
gitlab-ci: Add a FreeBSD CMake CI job
This uses https://gitlab.freedesktop.org/freedesktop/ci-templates,
and runs the FreeBSD build+test inside a QEMU VM.
This depends on the recently merged ci-templates MR
https://gitlab.freedesktop.org/freedesktop/ci-templates/-/merge_requests/142,
so we bump the required ci-templates commit hash.
There is one undiagnosed failure in test-fdpass that needs investigation,
so for now failures are permitted. Autotools and meson jobs will be added
in follow-up commits.
Alex Richardson [Tue, 9 Aug 2022 10:17:57 +0000 (10:17 +0000)]
dbus-sysdeps-util-unix.c: Avoid including sys/syslimits.h
This header is GCC specific header that on my system just contains
`#include_next <limits.h>`. FreeBSD also provides this header but it
contains a `#warning` that it should not be used. Replace the one use
with `#include <limit.h>` and drop the configure checks.
Alex Richardson [Sun, 29 May 2022 12:58:49 +0000 (12:58 +0000)]
cmake: Always try to find pkg-config
Commit 97bdefd4e2598e6ea72337acb4230928594bda81 move the
include(FindPkgConfig) call into a Linux-specific codepath, so pkg-config
was not being detected on FreeBSD. This mean that the check for
PKG_CONFIG_FOUND to determine whether to install .pc files later on
would always fail and .pc files were not installed on FreeBSD.
Alex Richardson [Mon, 8 Aug 2022 19:17:12 +0000 (19:17 +0000)]
dbus-sysdeps-unix.c: Fix unused function warning on FreeBSD
The function close_ignore_error() is only used in some cases. To avoid
duplicating the #ifdef condition, this patch moves the check just before
the definition of _dbus_close_all().
Alex Richardson [Mon, 8 Aug 2022 15:29:56 +0000 (15:29 +0000)]
tools/ci-build.sh: Ensure that $builddir is set
When adding the new FreeBSD CI, this was not implicitly forwarded to QEMU,
so the build script failed with confusing errors. Add an explicit check
that the variable is set to make those cases easier to debug.
Alex Richardson [Wed, 25 May 2022 21:33:17 +0000 (21:33 +0000)]
tools/ci-{build,install}.sh: Use a portable interpreter argument
FreeBSD has bash installed as /usr/local/bin/bash, so hardcoding /bin/bash
does not work. Instead use the portable replacement using env which will
find bash in $PATH.
Simon McVittie [Mon, 25 Jul 2022 13:30:02 +0000 (14:30 +0100)]
meson: Only require C++ compiler when building for Windows
dbus is generally a C-only project, but the Windows side has a tiny
amount of C++ to initialize global locks (because Windows doesn't have
a direct equivalent of PTHREAD_MUTEX_INITIALIZER). We don't need a C++
compiler when building for a non-Windows OS, so there's no need to
find it or check which options it supports.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Jordan Williams [Sat, 23 Jul 2022 11:21:44 +0000 (06:21 -0500)]
Use project relative paths in CMake files
This makes it possible for projects to incorporate D-Bus as a CMake sub-project in a larger CMake project.
Before this PR, doing so would result in many errors.
This is because CMAKE_SOURCE_DIR and CMAKE_BINARY_DIR would point to directories above the D-Bus project.
Using paths relative to the project directory, PROJECT_SOURCE_DIR and PROJECT_BINARY_DIR, corrects for this.
Jordan Williams [Fri, 22 Jul 2022 19:27:14 +0000 (14:27 -0500)]
Bump CMake version to 3.4
With the minimum version set to 3.4, none of the policies need to be set explicitly to the `NEW` behavior.
Each of the policies removed here was introduced before CMake version 3.4.
By default then, each of them will be set to NEW automatically.
This is part of the behavior of cmake_minimum_required.
The cmake_policy commands are therefore redundant and have been removed.
Simon McVittie [Tue, 19 Jul 2022 19:30:57 +0000 (20:30 +0100)]
NEWS, README.win: Officially drop support for non-C99 compilers
We're considering MSVC 2015 to be sufficiently close to C99 for our
purposes, and we now have CI for it, so we can easily check whether any
desired C99 feature works. Other pre-C99 compilers are obsolete.
Resolves: dbus#404 Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Fri, 15 Jul 2022 14:41:14 +0000 (15:41 +0100)]
test/bus: Break up dispatch test into three separate tests
This is really three separate test-cases: one for traditional
activation as a direct child process of the dbus-daemon, and two for
traditional activation (successful and failing) via the setuid
dbus-daemon-launch-helper on Unix.
The ones where activation succeeds extremely slow, as a result of the
instrumentation for simulating malloc() failures combined with a large
number of memory operations, particularly when using AddressSanitizer.
Splitting up "OOM" tests like these has a disproportionately good impact
on the time they take, because the simulated malloc() failure
instrumentation repeats the entire test making the first malloc() fail,
then making the second malloc() fail, and so on. For allocation failures
in the second half of the test, this means we repeat the first half of
the test with no malloc() failures a very large number of times, which
is not a good use of time, because we already tested it successfully.
Even when not using the "OOM" instrumentation, splitting up these tests
lets them run in parallel, which is also a major time saving.
Needless to say, this speeds up testing considerably. On my modern but
unexceptional x86 laptop, in a typical debug build with Meson, the old
dispatch test took just over 21 minutes, which drops to about 40 seconds
each for the new normal-activation and helper-activation tests (and for
most of that time, they're running in parallel, so the wall-clock time
taken for the whole test suite is somewhere around a minute).
In a debug build with Meson, gcc and AddressSanitizer, the old dispatch
test takes longer than my patience will allow, and the new separate
tests take about 5-6 minutes each. Reduce their timeout accordingly, but
not as far as the default for slow tests (5 minutes) to allow some
headroom for AddressSanitizer or slower systems.
The failed-helper-activation test is almost instantaneous, and no longer
needs to be marked as slow.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Mon, 18 Jul 2022 10:14:08 +0000 (11:14 +0100)]
test: Skip tests that involve switching uid if unable to do so
In a Linux user namespace, it is possible that we are uid 0 but are
unable to switch to some other uid like DBUS_USER or DBUS_TEST_USER,
because the other uid is not "mapped" in the user namespace, resulting
in setuid() or setresuid() failing with EINVAL "Invalid argument".
For example, it's easy for this to happen when running under the
bubblewrap tool.
Try to drop privileges in a child process, and skip the test if we
are unable to do so.
Resolves: dbus#407 Signed-off-by: Simon McVittie <smcv@collabora.com>
projects / thirdparty / dbus.git / log
