Age | Commit message (Collapse) | Author | Files | Lines |
|
1.12.x never supports SO_PEERGROUPS.
Signed-off-by: Simon McVittie <smcv@collabora.com>
|
|
Signed-off-by: Simon McVittie <smcv@collabora.com>
|
|
[1.12] Fix error behaviour on reload if a connection has an unknown uid
See merge request dbus/dbus!440
|
|
If we have a large number of connections to the bus, and we fail to
reload the policy for one of them (perhaps because its uid no longer
exists in the system user database), previously we would crash, which
is obviously unintended. After the previous commit, we would stop
iteration through the list of client connections, which doesn't seem
great either: one bad connection shouldn't prevent us from reloading
the rest of our state.
Instead, let's distinguish between new connections (where we want
failure to establish a security policy to be fatal), and pre-existing
connections (where the current security policy is presumably good
enough to keep using if we have nothing better). If we're unable to
reload the policy for a pre-existing connection, log a warning and
carry on iterating.
Helps: https://gitlab.freedesktop.org/dbus/dbus/-/issues/343
Signed-off-by: Simon McVittie <smcv@collabora.com>
|
|
Helps: https://gitlab.freedesktop.org/dbus/dbus/-/issues/343
Signed-off-by: Simon McVittie <smcv@collabora.com>
|
|
Previously, if dbus_connection_get_unix_user() succeeded but
_dbus_unix_groups_from_uid() failed, then bus_connection_get_unix_groups()
would incorrectly fail without setting the error indicator, resulting
in "(null)" being logged, which is rather unhelpful.
This also lets us distinguish between ENOMEM and other errors, such as
the uid not existing in the system's user database.
Fixes: 145fb99b (untitled refactoring commit, 2006-12-12)
Helps: https://gitlab.freedesktop.org/dbus/dbus/-/issues/343
Signed-off-by: Simon McVittie <smcv@collabora.com>
|
|
[1.12] bus: Don't crash if bus_context_create_client_policy() fails
See merge request dbus/dbus!438
|
|
If policy creation fails, we can't usefully leave a NULL policy in the
BusConnectionData. If we did, the next attempt to reload policy would
crash with a NULL dereference when we tried to unref it, or with
an assertion failure.
One situation in which we can legitimately fail to create a client policy
is an out-of-memory condition. Another is if we are unable to look up a
connection's supplementary groups with SO_PEERGROUPS, and also unable to
look up the connection's uid's groups in the system user database, for
example because it belongs to a user account that has been deleted (which
is sysadmin error, but can happen, particularly in automated test systems)
or because a service required by a Name Service Switch plugin has failed.
Keeping the last known policy is consistent with what happens to all
the connections that are after this one in iteration order: after we
early-return, all of those connections retain their previous policies
(which doesn't seem ideal either, but that's how this has always worked).
[smcv: Add commit message]
Co-authored-by: Simon McVittie <smcv@collabora.com>
Resolves: https://gitlab.freedesktop.org/dbus/dbus/-/issues/343
(cherry picked from commit 63522f2887878e6b9e40c9bb6742484679631ea9)
|
|
Signed-off-by: Simon McVittie <smcv@collabora.com>
|
|
Signed-off-by: Simon McVittie <smcv@collabora.com>
|
|
Signed-off-by: Simon McVittie <smcv@collabora.com>
|
|
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit 05367daa101247b1b5f7648b635cbe47eb220b39)
|
|
The exact failure mode reported in dbus/dbus#457 is quite difficult
to achieve in a reliable way in a unit test, because we'd have to send
enough messages to a client to fill up its queue, then stop that client
from draining its queue, while still triggering a message that gets a
reply from the bus driver. However, we can trigger the same crash in a
slightly different way by not allowing the client to receive a
particular message. I chose NameAcquired.
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit 986611ad0f7f67a3693e5672cd66bc608c00b228)
|
|
Normally, it's enough to rely on a message being given a serial number
by the DBusConnection just before it is actually sent. However, in the
rare case where the policy blocks the driver from sending a message
(due to a deny rule or the outgoing message quota being full), we need
to get a valid serial number sooner, so that we can copy it into the
DBUS_HEADER_FIELD_REPLY_SERIAL field (which is mandatory) in the error
message sent to monitors. Otherwise, the dbus-daemon will crash with
an assertion failure if at least one Monitoring client is attached,
because zero is not a valid serial number to copy.
This fixes a denial-of-service vulnerability: if a privileged user is
monitoring the well-known system bus using a Monitoring client like
dbus-monitor or `busctl monitor`, then an unprivileged user can cause
denial-of-service by triggering this crash. A mitigation for this
vulnerability is to avoid attaching Monitoring clients to the system
bus when they are not needed. If there are no Monitoring clients, then
the vulnerable code is not reached.
Co-authored-by: Simon McVittie <smcv@collabora.com>
Resolves: dbus/dbus#457
(cherry picked from commit b159849e031000d1dbc1ab876b5fc78a3ce9b534)
|
|
This is helpful while debugging test failures.
Helps: dbus/dbus#457
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit 8ee5d3e04420975107c27073b50f8758871a998b)
|
|
It appears some freedesktop CI runners still have /builds/dbus/dbus
owned by a different user ID.
This reverts commit 3f6400b8b929d902b7317e90440bd369c8f5407a.
Signed-off-by: Simon McVittie <smcv@collabora.com>
|
|
Signed-off-by: Simon McVittie <smcv@collabora.com>
|
|
In practice the pipeline is going to fail for namespaces other than
dbus, so don't waste time on trying to run it there; only run the
detached pipeline for the MR.
Signed-off-by: Simon McVittie <smcv@collabora.com>
|
|
After abuses of fdo infrastructure were mitigated in
freedesktop/freedesktop#540, contributors cannot usually run pipelines
in their own forks of dbus.
Signed-off-by: Simon McVittie <smcv@collabora.com>
|
|
The original code of 'ProgressNotification' had a wrong closing tag,
which should use '</signal>' instead of '</method>'
Signed-off-by: Yen-Chin, Lee <coldnew.tw@gmail.com>
|
|
Signed-off-by: Simon McVittie <smcv@collabora.com>
|
|
Signed-off-by: Simon McVittie <smcv@collabora.com>
|
|
Signed-off-by: Simon McVittie <smcv@collabora.com>
|
|
Signed-off-by: Simon McVittie <smcv@collabora.com>
|
|
Bugzilla is dead for long time now.
(cherry picked from commit dc59aa5df2c0c9a8fce5f5f0c33b69f21258c1ac)
(cherry picked from commit b5638c9882a9ce8d26b828abe1f9dffe19400aac)
|
|
After 6e48c317 the test-apparmor-activation test fails as it can no
longer access the dbus socket in /tmp. This commit updates the apparmor
profile used within the test
(cherry picked from commit 9950464761f0c44e08fb93155ca6e7f4b1d8a31f)
(cherry picked from commit 849382fe6daaf06b99269410091aa96f718f346a)
|
|
Signed-off-by: Simon McVittie <smcv@collabora.com>
|
|
This appears to have been a copy/paste mistake. If only blanks (defined as
spaces or tabs) were removed, then it cannot be right to check for white
space (defined as spaces, tabs, carriage return or linefeed) afterwards.
If libdbus was compiled with assertions enabled, then this is a
denial-of-service issue for dbus-daemon or other users of DBusServer:
an unauthenticated user with access to the server's socket can send
whitespace that triggers this assertion failure. We recommend that
production versions of dbus, for example in OS distributions, should be
compiled with checks but without assertions.
[smcv: expanded commit message]
Thanks: Evgeny Vereshchagin
Resolves: https://gitlab.freedesktop.org/dbus/dbus/-/issues/421
|
|
Signed-off-by: Simon McVittie <smcv@collabora.com>
|
|
Signed-off-by: Simon McVittie <smcv@collabora.com>
|
|
Signed-off-by: Simon McVittie <smcv@collabora.com>
|
|
Reproduces: https://gitlab.freedesktop.org/dbus/dbus/-/issues/417
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit bef693f442d854505e7013fd31efe41747d7493c)
[backport to 1.14.x: discard Meson build system updates]
(cherry picked from commit 71dd3ad20cf8aca3310fa8d533801fb1d8bdaf1a)
[backport to 1.12.x: resolve conflicts in Autotools build system]
|
|
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit 7a2c13d21be702c7b5b7288fb82a60adc5bd7378)
[backport to 1.14.x: discard Meson build system updates]
(cherry picked from commit 7a8f3c2af94063e4994a9780d00d700aa0ea8aef)
[backport to 1.12.x: resolve conflicts in build system; adjust signature
of test function for absence of 39ef65d0 in this branch]
|
|
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.
Thanks: Evgeny Vereshchagin
Fixes: ba7daa60 "unix-fd: add basic marshalling code for unix fds"
Resolves: https://gitlab.freedesktop.org/dbus/dbus/-/issues/417
Resolves: CVE-2022-42012
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit 236f16e444e88a984cf12b09225e0f8efa6c5b44)
(cherry picked from commit 3fb065b0752db1e298e4ada52cf4adc414f5e946)
|
|
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>
(cherry picked from commit 3ef342410a1cefe3d0bfaf46279c6517f4b44a26)
(cherry picked from commit 6b88e768d887470573bc746bf9f22a24f2c00dbf)
[backport to 1.12.x: resolve conflicts; call _dbus_header_delete_field
directly because _dbus_message_remove_unknown_fields didn't yet exist
in this branch]
|
|
Backported from 1.13.0. Previously this was fixed in the dbus-1.14 branch
but unfixed in the dbus-1.12 branch, but we need it fixed in dbus-1.12
if we want the additional test coverage for dbus#413 to pass.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=100317
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit ecbcffae19ff1b811fc7d0d602458c0f00dd6771)
|
|
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>
(cherry picked from commit 37e01259435a28b11a087867ea199cd09f0a5cff)
[backport to 1.14.x: discard Meson build system updates]
(cherry picked from commit bbf5ef008a629af62f817771adee6ae4adcf3bcd)
[backport to 1.12.x: resolve conflicts in CMake build system]
|
|
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>
(cherry picked from commit d633016f7340f83142f19f4bcac08c57e1f2fd2f)
[backport to 1.14.x: discard Meson build system updates]
(cherry picked from commit c0bfcc097896fb57a96f55818fc5fe97db679cbf)
[backport to 1.12.x: resolve conflicts vs. Autotools before 09076dfe]
|
|
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>
(cherry picked from commit 079bbf16186e87fb0157adf8951f19864bc2ed69)
(cherry picked from commit b9e6a7523085a2cfceaffca7ba1ab4251f12a984)
|
|
Reproduces: https://gitlab.freedesktop.org/dbus/dbus/-/issues/418
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit 67800ac5febc9f15d6c4f113c758797472842ff3)
(cherry picked from commit fd73d1ef1ba6ba4e55c07f5e65300bf40a94917f)
|
|
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.
Thanks: Evgeny Vereshchagin
Resolves: https://gitlab.freedesktop.org/dbus/dbus/-/issues/418
Resolves: CVE-2022-42010
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit 9d07424e9011e3bbe535e83043d335f3093d2916)
(cherry picked from commit 3e53a785dee8d1432156188a2c4260e4cbc78c4d)
|
|
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit 1aa0d897c25b22ed9f23ce5bb164b4076687bf9c)
|
|
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.
This partially reverts commit a70b042f.
Resolves: https://gitlab.freedesktop.org/dbus/dbus/-/issues/416
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit f01382ae310c7d63790c07ed280f575d91ea57b8)
[backport to 1.14.x: adjust to absence of d98c98d1 in this branch]
(cherry picked from commit b5a09fb11c05b3b1922e99d18720f586fc91cd0b)
|
|
Signed-off-by: Simon McVittie <smcv@collabora.com>
|
|
The Windows code in dbus is careful to use Windows-specific equivalents
of the Standard C features that are not implemented by msvcrt.dll, so
we don't need to substitute a Standard C printf implementation.
This avoids compiler warnings/errors when gcc expects us to be using
Microsoft printf syntax (`ms_printf` attribute), but newer versions of
mingw-w64 expect us to be using GNU or Standard C printf syntax
(`gnu_printf` attribute) as a result of `__USE_MINGW_ANSI_STDIO` being
enabled by default if not otherwise specified.
Resolves: https://gitlab.freedesktop.org/dbus/dbus/-/issues/380
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit daf362ee61170727be0ca5dec1cf39435485ad0c)
|
|
Without this running, dbus-daemon with long XDG_DATA_DIRS
will crash on out-of-bounds write:
$ XDG_DATA_DIRS=$(seq -f "/foo/%g" -s ':' 129) dbus-daemon --session
*** stack smashing detected ***: terminated
(cherry picked from commit b551b3e9737958216a1a9d359150a4110a9d0549)
Backported-from: dbus!302
(cherry picked from commit 9e8fe0718c1abf93e92fcf98c3fe962fc524bfa8)
|
|
Fix CI for 1.12.x
See merge request dbus/dbus!342
|
|
dbus 1.12.x was included in both Debian 10 'buster', which is now EOL,
and Debian 11 'bullseye', which continues to be supported. I need this
branch to work on Debian 11 for security and bugfix backports, but I'm
no longer supporting Debian 10.
Leave one build variant, the "legacy" build, on Debian 10 to check that
we can still build there, to be nice to the Debian LTS subproject.
Also continue to use Debian 10 for mingw builds, since dbus#380 has
not been fixed in this branch (which is now the security-fix-only
old-stable branch, so the fix will not be backported).
Signed-off-by: Simon McVittie <smcv@collabora.com>
|
|
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>
(cherry picked from commit 8aa34131dd3665df1c0d6a2c92600e04593d09c0)
|
|
the downloaded mingw packages
This is necessary to have a clean initial state when running the script
locally.
Signed-off-by: Ralf Habacker <ralf.habacker@freenet.de>
(cherry picked from commit 067733d4b0e7c9c03ee25cb85eecb80980f01f68)
|