Commit 54d26df5 "config: change default auth_timeout to 5 seconds"
reduced the hard-coded default from 30 to 5 seconds, and the
commented-out informational copy of this information in system.conf
reflected that.
Commit 02e1ddf9 'Revert "config: change default auth_timeout to 5 seconds"'
subsequently increased hard-coded default back to 30 seconds, but did
not update the commented-out version in this file.
See also CVE-2014-3639, fd.o #80919, fd.o #86431.
Fixes: 1a36f983 "Document default limits in system.conf.in"
Fixes: 02e1ddf9 'Revert "config: change default auth_timeout to 5 seconds"'
Signed-off-by: Simon McVittie <smcv@collabora.com>
libselinux 3.8 includes an API break in which the `refcnt` field of
struct security_id_t (originally intended to be a reference count, but
in practice always initialized to 1 and never modified) was renamed and
repurposed as an `id` field. This caused a build failure if dbus was
compiled with both SELinux support and verbose mode, for example in the
instrumented debug build that Debian includes in the `dbus-tests` package.
This particular piece of debug logging has little value, so just
remove it.
Reference: e5fd7b078f
Bug-Debian: https://bugs.debian.org/1096212
Signed-off-by: Simon McVittie <smcv@collabora.com>
We no longer have an Autotools build system, but Meson sets the same
environment variables that Autotools used to set. CMake does not, yet,
but ideally should.
Resolves: https://gitlab.freedesktop.org/dbus/dbus/-/issues/538
Signed-off-by: Simon McVittie <smcv@collabora.com>
The `Containers1` interface is not stable or production-ready, and is
compile-time-optional behind a build option, which in fact currently
cannot be enabled (it intentionally provokes a compiler `#error` if
enabled). If it isn't enabled, this directory won't be useful, so
there's no reason to create it.
As discussed in <https://github.com/systemd/mkosi/issues/3189>, if we
use the `@DBUS_USER@` here, it makes `tmpfiles.d/dbus.conf` dependent
on having created the `@DBUS_USER@` via `sysusers.d` or some
appropriate distro-specific mechanism. This is problematic in distros
that split up the functionality of dbus into several layers, such as
Debian: the `@DBUS_USER@` conceptually belongs to the same layer as
the well-known system bus, but `/etc/machine-id` and
`/var/lib/dbus/machine-id` are also used by the well-known session bus,
which is orthogonal to the system bus; so we want `dbus.conf` to be in a
lower layer than the `@DBUS_USER@`.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Tthis is useful when an asset manager wants to install a system
service while /usr/ is read-only (e.g.: portable
services running on a different namespaced image). Unlike other
directories, enforce strict naming and do not set up an inotify,
as the directories might not even exist until much later. Also
search in /run/ for ephemeral services that will disappear after
a reboot.
Signed-off-by: Luca Boccassi <bluca@debian.org>
It's a wrapper around snprintf(), so we are not gaining any efficiency
versus _dbus_string_append_printf(), and might as well use the more
general function instead. Doing it this way might even be a little *more*
efficient, since it reduces reallocations; it's certainly more concise.
Signed-off-by: Simon McVittie <smcv@collabora.com>
It's a wrapper around snprintf(), so we are not gaining any efficiency
versus _dbus_string_append_printf(), and might as well use the more
general function instead.
Signed-off-by: Simon McVittie <smcv@collabora.com>
When `dbus-daemon` sets more than 128 directories for `XDG_DATA_DIRS`,
none of the elements in `new_dirs` will be `NULL`, which resulted in
these loops reading out-of-bounds (undefined behaviour). In practice
this led to a crash.
To avoid this, make sure to stop iteration at the end of the array.
[smcv: Expanded commit message]
Resolves: dbus/dbus#481
On 32 bit systems long will overflow in 2038, causing complete breakage.
This is confirmed by running dbus's test suite on a 32 bit system
with system time set to 2040 (and configured to use 64 bit time_t of course).
Note that both timespec and timeval are specified with time_t for the
seconds component. This should propagate everywhere where that data is
passed and stored, but previously _dbus_get_monotonic_time() and
_dbus_get_monotonic_time() would truncate it to long.
Also add a function for parsing dbus_int64_t from
files, as existing functions can only handle long.
Signed-off-by: Alexander Kanavin <alex@linutronix.de>
When running tests, we use DBUS_FATAL_WARNINGS=1 to make the tests fail
on internal errors. Failing to set up inotify is not really an internal
error: it's more like an environmental error, which can occur for
reasons outside our control.
Instead, log using bus_context_log(), which never crashes the process
and always just logs a warning.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Resolves: https://gitlab.freedesktop.org/dbus/dbus/-/issues/473
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>
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>
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
Allows to track a process by pinning to a file descriptor,
which unlike a PID cannot be reused.
root@image:~# busctl call org.freedesktop.DBus /org/freedesktop/DBus org.freedesktop.DBus GetConnectionCredentials "s" org.freedesktop.systemd1
a{sv} 3 "ProcessID" u 1 "UnixUserID" u 0 "ProcessFD" h 4
Signed-off-by: Luca Boccassi <bluca@debian.org>
When starting as root files in /proc/self/fdinfo/ will be owned as root
and set to 400, so we cannot read them. Nowadays it is not necessary to
start as root when running under systemd, so just add User/Group with
the configured user to the system unit.
If libaudit support is enabled, add AmbientCapabilities=CAP_AUDIT_WRITE
so that we can still write to the audit log.
Signed-off-by: Luca Boccassi <bluca@debian.org>
The man page and --help imply that
dbus-daemon --print-address --print-pid
is a valid/useful thing to do, but because --print-address takes an
optional argument, it is ambiguous whether --print-pid is meant to
be the argument for --print-address (same as --print-address=--print-pid)
or a new option (same as --print-address=1 --print-pid). In fact,
before this commit, the dbus-daemon would interpret --print-pid as
the optional argument to --print-address, and then fail to parse it
because it isn't an integer.
Because none of our options are syntactically valid as arguments for
any option that takes an optional argument, we can avoid the ambiguity
by delaying parsing of optional arguments until all known options
have been tried.
Resolves: dbus/dbus#467
Signed-off-by: Xin Shi <shixin21@huawei.com>
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
In log_callback() the same va_list is reused for a call to vsnprintf and
vsyslog. A va_list can't be reused in this manner, such use is undefined
behavior that changes depending on glibc version.
In current glibc versions a segfault can be observed from the callsite at
bus/selinux.c:412. When trying to log a non-auditable event, the segfault
happens in strlen inside vsyslog.
Moving the call to vsnprintf closer to audit_log_user_avc_message (which is
followed by a 'goto out') avoids the reuse and segfault.
Signed-off-by: Jeremi Piotrowski <jpiotrowski@microsoft.com>
The full license texts are not added because they were already
added in a previous commit.
Signed-off-by: Ralf Habacker <ralf.habacker@freenet.de>
see #394
In some more complicated loops, we do need to use 'goto' to exit from
an inner loop, or to jump to cleanup or an increment of an iterator
immediately before the next loop iteration. However, in these simple
cases, jumping to a label immediately before the 'while' keyword is
unnecessary: we can use an equivalent 'continue' statement for flow
control.
This makes it easier for maintainers to notice the loops where we are
doing something more complicated, which still use 'goto', and know
that they need to pay more attention in those cases.
Signed-off-by: Simon McVittie <smcv@collabora.com>
This lets OS distributors configure --runstatedir=/run if they want to,
although for interoperability, they should only do this if they can
guarantee that their /run and /var/run are equivalent.
A previous commit adds a warning if we are using the default path on a
system where /run and /var/run are not synoymous, mitigating the
compatibility impact of this change.
For CMake, this requires version 3.9, released in 2017.
For Meson, this is currently controlled by the runtime_dir option,
which defaults to /run if the prefix is /usr. The rationale for this
is that /run is correct for modern Unix systems, and distributors who
switch from Autotools or CMake to Meson need to review all their build
options at that time, which is an ideal opportunity to check that they
are doing the right thing around /run.
Helps: https://gitlab.freedesktop.org/dbus/dbus/-/issues/180
Co-authored-by: Simon McVittie <smcv@collabora.com>
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.
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>
_dbus_warn() normally only logs a warning, but can be made fatal by
environment variables. In particular, we do that during unit testing,
which can result in a build-time test failure if dbus is built in a
sandbox environment that prevents write access.
_dbus_log() does only the logging part of _dbus_warn(), which seems
more appropriate here.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Use install_emptydir() in Meson versions that support it, or a script
with similar invocation in versions that do not. This will make it
straightforward to migrate to install_emptydir() when we drop support
for Meson versions older than 0.60.0.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Use install_symlink() in Meson versions that support it, or a script
with similar invocation in versions that do not. This will make it
straightforward to migrate to install_symlink() when we drop support
for Meson versions older than 0.61.0.
Based on an implementation in the game-data-packager package, which used
a shell script.
Signed-off-by: Simon McVittie <smcv@collabora.com>
As long as we are treating Autotools as a first-class citizen, what we
release will be `make distcheck` output.
Signed-off-by: Simon McVittie <smcv@collabora.com>
gcc 10.3 warns that link->data is a possible NULL dereference.
However, that can't actually happen without an earlier programming
error, because bus_service_remove_owner() is only valid to call for
a connection that is currently in the queue to own the service,
in which case we know _bus_service_find_owner_link() will succeed.
Part-of: https://gitlab.freedesktop.org/dbus/dbus/-/merge_requests/275
Reviewed-by: Simon McVittie <smcv@collabora.com>
gcc 10.3 warns that link->data might be NULL, which would make
new_owner->conn a null pointer dereference. However, we know that
we only add valid, non-null BusOwner objects to the list, so that
can't happen in reality.
Part-of: https://gitlab.freedesktop.org/dbus/dbus/-/merge_requests/275
Reviewed-by: Simon McVittie <smcv@collabora.com>
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
Using PRId64, etc. to print dbus_int64_t or dbus_uint64_t is not 100%
portable. On platforms where both long and long long are 64-bit (such as
Linux and macOS), we will prefer to define dbus_int64_t as long.
If the operating system has chosen to define int64_t as long long,
which is apparently the case on macOS, then the compiler can warn that
we are passing a long argument to PRId64, which is "lld" and therefore
expects a long long argument (even though that ends up with the same
bit-pattern being used).
We can't necessarily just use int64_t and uint64_t directly, even if all
our supported platforms have them available now, because swapping
dbus_int64_t between long and long long might change C++ name mangling,
causing ABI breaks in third-party libraries if they define C++ functions
that take a dbus_int64_t argument.
Signed-off-by: Simon McVittie <smcv@collabora.com>
There should be no need to include the directory above the DBus sources,
if that is actually required users can always pass -I flags to CMake.
I noticed this because CLion started indexing all my cloned projects when
I opened DBus due to this include path.
fd 0 is a valid fd - although if we are using stdin as our inotify fd,
something is weird somewhere.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Previously, we were relying on the system bus being able to reset
its OOM score adjustment after it forks, but before it execs the
dbus-daemon-launch-helper. However, it can't actually do that (leading
to dbus#378), because the system bus typically starts as root, uses its
root privileges to adjust resource limits, and then drops privileges
to the `@DBUS_USER@`, typically `dbus` or `messagebus`. This leaves the
pseudo-files in /proc for its process parameters owned by root, and the
`@DBUS_USER@` is not allowed to open them for writing.
The dbus-daemon-launch-helper is setuid root, so it can certainly
alter its OOM score adjustment before exec'ing the actual activated
service. We need to do this before dropping privileges, because after
dropping privileges we would be unable to write to this process
parameter.
This is a non-async-signal-safe context, so we can safely log errors
here, unlike the fork-and-exec code paths.
Resolves: https://gitlab.freedesktop.org/dbus/dbus/-/issues/378
Signed-off-by: Simon McVittie <smcv@collabora.com>
After any reload of the activatable service files the mentioned signal is
emitted to the current bus to inform clients.
The calls to signal emmission have not been implemented in the platform
specific functions _dbus_daemon_report_reloaded() to avoid duplicate
implementations.
Fixes#376
Signed-off-by: Ralf Habacker <ralf.habacker@freenet.de>
We've had a request for a 1.14.x stable-branch, but the Containers
interface is only partially implemented, not yet described in the
D-Bus Specification, and not ready to be part of our API guarantees.
Signed-off-by: Simon McVittie <smcv@collabora.com>
In the mentioned function a local DBusError instance is now used to
fulfill the requirement of dbus_error_has_name() that the parameter
'error' must not be null.
See #360
dbus-run-session starts a dbus-daemon before the client application.
We must avoid letting the application try to connect before the
dbus-daemon's DBusServer is listening for connections.
In the Unix implementation, we already achieved this via the
--print-address option. If the client tried to connect too soon,
the server would not yet be listening and the client would fail.
In the Windows implementation, we communicate the bus address to
the client application as an autolaunch: address, so if the client
tried to connect too soon, it would autolaunch a new dbus-daemon
instead of using the one that it was intended to use.
We can avoid this by using a new option to pass in a Windows event
object, which will be set when the server has started and is ready
to process connections.
Fixes#297
