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
(cherry picked from commit b034b83b59)
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.
This change also fixes a bug when the hard limit is very large but
the soft limit is not, for example seen as a regression when upgrading
to systemd >= 240 (Debian #928877). In such environments, dbus-daemon
would previously have changed its fd limit to 64K soft/64K hard. Because
this hard limit is less than its original hard limit, it was unable to
restore its original hard limit as intended when carrying out traditional
activation, leaving activated subprocesses with unintended limits (while
logging a warning).
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>
(cherry picked from commit 7eacbfece7)
[smcv: Mention that this also fixes Debian #928877]
Group names in desktop files may contain all ASCII characters, except
control characters and '[' and ']'. Rather than accepting all values,
thanks to a logical operator confusion found by GCC warning
-Wlogical-op, instead explicitly reject the invalid values.
Signed-off-by: David King <dking@redhat.com>
Fixes: https://gitlab.freedesktop.org/dbus/dbus/issues/208
(cherry picked from commit 3ef9e789c1)
AX_CODE_COVERAGE recently changed the way it embedded its Makefile rules
in the output file: instead of using @CODE_COVERAGE_RULES@, users
are now meant to include aminclude_static.am.
The new AX_CODE_COVERAGE is only in the latest autoconf-archive release,
version 2019.01.06, which is inconveniently new, so bundle everything
we need for the moment.
This requires us to stop using the deprecated CODE_COVERAGE_LDFLAGS
(which we still used to support older versions of autoconf-archive)
and replace them with CODE_COVERAGE_LIBS.
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit 2938c2125e)
Closes: dbus#265
This is technically a denial of service because the dbus-daemon will
run out of memory eventually, but it's a very slow and noisy one,
because all the rejected messages are also very likely to have
been logged to the system log.
Detected by AddressSanitizer.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Resolves: https://gitlab.freedesktop.org/dbus/dbus/issues/234
Reviewed-by: pwithnall
Startup ordering was changed in #92832 to ensure that SELinux audit
messages could be sent. As a side effect, the raising of file descriptor
limits was moved to after the dropping of root privileges, resulting in
the limit change always failing.
Move the raise_file_descriptor_limit() call to ensure that it is called
before dropping root privileges.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=105165
Bug-RedHat: https://bugzilla.redhat.com/show_bug.cgi?id=1529044
[smcv: Call raise_file_descriptor_limit() even if !context->user]
Reviewed-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit 6e42964f5f)
This snippet was already attempting to create /var/lib/dbus/machine-id,
but would fail on volatile or stateless systems where /var/lib/dbus/
did not already exist. systemd-tmpfiles automatically creates parent
directories for tmpfiles of type 'd', 'D', etc., but not for files
or symlinks (https://github.com/systemd/systemd/issues/7853).
Signed-off-by: Chris Lesiak <chris.lesiak@licor.com>
[smcv: Extended commit message to clarify why we need this]
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=104577
Reviewed-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit aeebf801f1)
While reviewing fd.o#101354, Philip Withnall pointed out that if we
rejected a connection in the new code there, we didn't log why. It
turns out we didn't log that in the more normal code path either.
Redo the error handling so that failure to set up a connection
is logged.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=103592
(cherry picked from commit 56847ae818)
If an async signal interrupts some function, we can have this
anti-pattern:
/* in normal code */
result = some_syscall (); /* fails, e.g. errno = EINVAL */
/* interrupted by async signal handler */
write (...); /* fails, e.g. errno = ENOBUFS */
/* back to normal code */
if (errno == EINVAL) /* problem! it should be but it isn't */
The solution is for signal handlers to save and restore errno.
This is unnecessary for signal handlers that can't touch errno (like
the one in dbus-launch that just sets a flag), and for signal handlers
that never return (like the one in test-utils-glib for timeouts).
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=103010
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
LSB-style (SysV-style) init scripts have not historically been
portable between distributions, as evidenced by the presence of both
"Red Hat" and "Slackware" init scripts in dbus. Many distributors
prefer to maintain them downstream, as is done in Debian (and its
derivatives) and in Slackware, so that the init script can follow
OS conventions (for example regarding boot messages) and make use
of OS-provided facilities (for example, the Debian init script uses
dpkg's start-stop-daemon utility).
The Slackware and Red Hat init scripts removed by this commit are not
tested or maintained in practice, and so are likely to have bugs. The
Slackware init-script provided here is not used on actual Slackware
systems, which provide a different implementation of rc.messagebus in
their packaging, while the Red Hat init script has been superseded by
the systemd unit in current Fedora, CentOS and RHEL versions.
The Cgywin messagebus-config provided here does appear to be used in
production in cygwin-ports, but it's full of Cygwin-specifics with which
the dbus maintainers are not familiar, so it is probably more appropriate
for it to be tracked downstream as part of the Cygwin packaging.
The systemd unit is not removed, since it is used on multiple Linux
distributions with little or no modification, and receives regular
testing and maintenance; this makes it appropriate to maintain upstream.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Bug: https://bugs.freedesktop.org/101706
Reviewed-by: Philip Withnall <withnall@endlessm.com>
If we add a rule like
<allow send_destination="com.example" send_broadcast="true"/>
then it cannot possibly match anything, because to be a broadcast, the
message would have to have no destination. The only value of
send_destination that can be combined with send_broadcast="true" is
the wildcard "*", but by this point in the function we already
replaced "*" with NULL.
Adapted from an earlier implementation of send_broadcast by
Alban Crequy.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Bug: https://bugs.freedesktop.org/92853
Reviewed-by: Philip Withnall <withnall@endlessm.com>
<allow send_broadcast="true" ...> only matches broadcasts,
which are signals with a NULL destination. There was previously
no way for the policy language to express "NULL destination",
only "any destination".
<allow send_broadcast="false" ...> only matches non-broadcasts,
which are non-signals or signals with a non-NULL destination.
There was previously no way for the policy language to express
"any non-NULL destination", only "any destination".
Reviewed-by: Philip Withnall <withnall@endlessm.com>
[smcv: improved documentation as per Philip's review]
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Thiago Macieira <thiago@kde.org>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=92853
The giant conditionals used to check policy attributes are increasingly
unwieldy, so let's try something else. Bundle together the send_
attributes, the receive_ attributes, the eavesdrop attribute
(which can go on either send or receive rules) and the other attributes
into equivalence classes, and write the conditionals in terms of those
equivalence classes.
In particular, this correctly forbids
<allow receive_type="..." send_destination="..."/>
which was previously allowed but nonsensical (the send part took
precedence and the receive part was ignored).
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Reviewed-by: Thiago Macieira <thiago@kde.org>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=92853
By default, Expat uses cryptographic-quality random numbers as a salt for
its hash algorithm, and since 2.2.1 it gets them from the getrandom
syscall on Linux. That syscall refuses to return any entropy until the
kernel's CSPRNG (random pool) has been initialized. Unfortunately, this
can take as long as 40 seconds on embedded devices with few entropy
sources, which is too long: if the system dbus-daemon blocks for that
length of time, important D-Bus clients like systemd and systemd-logind
time out and fail to connect to it.
We're parsing small configuration files here, and we trust them
completely, so we don't need to defend against hash collisions: nobody
is going to be crafting them to cause pathological performance.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101858
Tested-by: Christopher Hewitt <hewitt@ieee.org>
[smcv: Adjust build-system changes for 1.11.x]
Signed-off-by: Simon McVittie <smcv@debian.org>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
This lets _dbus_warn() and _dbus_warn_check_failed() fall through
to flushing stderr and calling _dbus_abort(), meaning that failed
checks and warnings can result in a core dump as intended.
By renaming the FATAL severity to ERROR, we ensure that any code
contributions that assumed the old semantics will fail to compile.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101568
Now that we're starting to implement methods in more places, it makes
sense to share this code. The Stats interface can already benefit.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101567
Eavesdropping on unicast messages to other processes is not something
that should be done by processes in containers, or on the system bus
by users other than root or the bus owner. bus/system.conf.in
does not enable eavesdropping, but adding inadvisable configuration
could. This brings it into line with Monitoring.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101567
On systemd systems, /etc/machine-id is guaranteed to exist and has
the same format as the D-Bus machine ID. The major D-Bus implementations
read /etc/machine-id if it exists, but some less up-to-date
implementations still only read /var/lib/dbus/machine-id. We can be
nice to those implementations by ensuring /var/lib/dbus/machine-id
is a symlink; this way, the two files can never get out of sync.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101570
Currently when asked the SELinux context of the owner of
org.freedesktop.DBus, the dbus-daemon is returning an error.
In the same situation when asked about the Unix user or the PID, the
daemon would return its own user or pid. Do the same for the SELinux
context by returning the daemon one.
In particular this avoids an issue seen with systemd --user, where
dbus-daemon responds to UpdateActivationEnvironment() by passing on the
new environment to systemd with o.fd.systemd1.Manager.SetEnvironment(),
but systemd cannot get the caller's SELinux context and so rejects the
SetEnvironment() call.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101315
[smcv: Extend commit message to describe the symptom this fixes]
Reviewed-by: Simon McVittie <smcv@collabora.com>
At the moment there is a hack in the implementation of GetMachineId()
to stop tests from failing during "make check" on a system where
dbus has never been installed, by silently generating a new unique
fake "machine ID" for each process. I'm about to change that
behaviour to report errors properly; skip affected test-cases if we
can't read the real machine ID.
The shell scripts to test dbus-launch are run both as "make check"
tests (for which it is valid for dbus to be not correctly installed)
and as installed-tests (for which that is not valid), so make them
pass during "make check" but fail during installed testing.
The tests in bus/ and test/name-test/ are only run during "make check"
so they only have the code path where they are skipped.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=13194
We recommend using Properties for this sort of thing when designing
D-Bus APIs, so it's a bit hypocritical that the reference message bus
didn't. The Features and Interfaces properties can be used for
feature-discovery as we add new larger features, while the Properties
support can be used for finer-grained properties, for example in the
interface planned for #100344.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101257
This makes the /org/freedesktop/DBus path discoverable.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101257
The default for the future should be that new functionality should
only be available at /org/freedesktop/DBus, which is the canonical
path of the "bus driver" object that represents the message bus.
The disallowed methods are still in Introspect() output, and
raise AccessDenied instead of UnknownMethod, to preserve the invariant
that the o.fd.DBus interface has constant contents.
test/dbus-daemon.c already asserts that UpdateActivationEnvironment()
still raises AccessDenied when invoked on a non-canonical path;
this has been in place since 1.8.14.
Reviewed-by: Philip Withnall <withnall@endlessm.com>
[smcv: Adjust comments, enum order, variable naming as per Philip's review]
Signed-off-by: Simon McVittie <smcv@collabora.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101256
The o.fd.DBus interface needs to remain available on arbitrary object
paths for backwards compatibility, and the Introspectable interface
is genuinely useful, but everything else can be skipped.
This is arguably an incompatible change for the undocumented Verbose
interface, and for the GetAllMatchRules method on the undocumented
Stats interface: previously those were available at all object paths.
Reviewed-by: Philip Withnall <withnall@endlessm.com>
[smcv: Adjust comments, enum order, variable naming as per Philip's review]
Signed-off-by: Simon McVittie <smcv@collabora.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101256
sidget and sidput functions are noop and deprecated since libselinux 2.0.86.
Also use pkg-config to detect libselinux and force version >= 2.0.86
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=100912
Reviewed-by: Simon McVittie <smcv@collabora.com>
We haven't supported XML libraries other than Expat since 2013.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=69801
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
The version of expat that added the .pc file was released in 2012.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=69801
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
This silences -Wswitch-default.
Based on part of a patch from Thomas Zimmermann.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=98191
For these types, the tagged union in the Element struct does not store
anything we could usefuly compare.
Based on part of a patch from Thomas Zimmermann.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=98191
This silences -Wswitch-default.
Based on part of a patch from Thomas Zimmermann.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=98191
This silences -Wswitch-default.
Based on part of a patch from Thomas Zimmermann.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=98191
There should be no way signal_handler() can be called for a signal
we didn't ask for. If it somehow happens, ignore it.
Based on a patch from Thomas Zimmermann.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=98191
check_got_service_info() can't actually return an invalid
GotServiceInfo, but if it somehow does, we want to fail the test.
GOT_SOMETHING_ELSE already has that effect, and a similar meaning.
Based on a patch from Thomas Zimmermann.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=98191
For configuration purposes these are treated as part of the standard
session service directories, to avoid having to add new configuration
syntax which would prevent an old dbus-daemon from reloading
successfully. From an API perspective, they're separate, though.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99825
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
This is done on a per-directory basis.
The use of the BusContext here means we have to make the activation
test a little more realistic, by providing a non-NULL BusContext.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99825
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
This lets us give them a flags word, which we immediately use to
track whether this directory should be watched with inotify or
equivalent.
The struct name is unfortunately a bit odd, because I had aimed to
use BusServiceDir, but activation.c already has BusServiceDirectory
so that would have been too confusing.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99825
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
I'm about to change the version in the full-fat parser to return
BusServiceDir structs. Name this one with "paths" instead, to avoid
confusion.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99825
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
This means we can test it more easily. At the moment it just
contains service directories, because this config file is so
cut-down that it doesn't have any config.d directories.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99825
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Don't just exercise _dbus_get_standard_session_servicedirs(), but
also its integration into the BusConfigParser.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99825
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
