In Debian bug <https://bugs.debian.org/829348>, lightdm appears to
have been starting dbus-launch with at least one of the three
standard fds 0, 1, 2 (stdin, stdout, stderr) closed. This resulted
in the dbus-daemon's epoll_create1() returning a fd less than 3.
Later, _dbus_become_daemon() replaces fds 0-2 with /dev/null. As a
result, a subsequent call to _dbus_loop_add_watch() for the reload
pipe resulted in calling epoll_ctl on the non-epoll fd pointing to
/dev/null, which fails with EINVAL, resulting in the dbus-daemon
exiting unsuccessfully.
Unix programs are not normally expected to behave correctly when
launched with the standard fds not already open; but at the same time,
X11 autolaunching means that dbus-launch (and hence the dbus-daemon)
can get started from an arbitrarily precarious situation.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=97008
Signed-off-by: Simon McVittie <smcv@debian.org>
Reviewed-by: Thiago Macieira <thiago@kde.org>
(cherry picked from commit c8f73a2a3a)
With --systemd-activation we special-case the name
org.freedesktop.systemd1 by assuming that it will eventually connect
to the bus. With that in mind, we can ignore whether it has a
.service file, and let it be "activated" regardless.
This fixes a regression test failure on non-systemd systems such
as the Ubuntu 14.04 OS on travis-ci.org: UpdateActivationEnvironment
failed, because it tried to update the (fake) systemd environment,
but because systemd was not actually installed, there was no
service file for it in the system's search paths. We could address this
by placing a dummy service file with Exec=/bin/false in our search path
like the real systemd does, but it seems cleaner to not require this;
this would eventually enable the real systemd to stop installing
that dummy service file.
This would not happen outside the regression tests, because there is
no sense in using --systemd-activation without systemd installed.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=93194
Reviewed-by: Ralf Habacker <ralf.habacker@freenet.de>
This makes them semi-command-line-compatible with a way we can
invoke the GLib-based tests to get more useful debug logs.
These tests still do not actually produce TAP output yet; I tried
implementing that, but it requires changing a lot of noise on stdout
to come out of stderr, and there was something weird going on with
subprocesses restarting the test numbering which will need further
investigation before making that change.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=93194
Reviewed-by: Ralf Habacker <ralf.habacker@freenet.de>
Attempting to call SetEnvironment on systemd causes it to inquire
about the caller's connection UID and PID. If this check fails,
the call is rejected.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=92857
Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
[smcv: go back to DBUS_ERROR_UNIX_PROCESS_ID_UNKNOWN as the error code
for failure to determine the pid]
If we use systemd activation, forward all UpdateActivationEnvironment
requests to org.freedesktop.systemd1.Manager.SetEnvironment, in order
to ensure variables needed by D-Bus services are available when these
services are launched by systemd.
Since UpdateActivationEnvironment is not available on the system bus,
this only applies to user buses.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=92857
Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
The default policy already disallows calls on system buses. Since any
bus with a service helper cleans the environment anyway, there's no
point in allowing this to be called.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=92857
Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Because GetConnectionUnixUser is not supported on windows it fails with
DBUS_ERROR_FAIL.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=92721
Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
The system bus is unsupported there and Windows does not
have any concept of setuid binaries, so it can't ever
actually work.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=92721
Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Windows returns unhandled exceptions from a running child
by specific exit codes and not by signals as on UNIX.
Therefore we use DBUS_ERROR_SPAWN_CHILD_EXITED for propagating
unhandled exceptions to the parent too.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=92721
Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
DBus test cases running the server *and* client loop in the same
process assumed that all messages send from the server has to be
received in one client dispatch, which is not the case in all
environments.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=92721
Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Previously, we didn't consistently test parsing of every file in
valid-config-files-system/ everywhere that we tested valid-config-files/.
We now test it on Unix.
The system bus is not supported on Windows, so we do not test
valid-config-files-system/ there.
valid-config-files/many-rules.conf contains <user> and <group> rules
which are not applicable to Windows. Copy the original many-rules.conf
to valid-config-files-system/ so that it will be tested on Unix, and
remove the non-portable rules from valid-config-files/many-rules.conf.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=92721
Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
[rh:base patch came from Simon]
If D-Bus was configured for /usr/local and built in Z:/build,
the previous code would use
Z:/build/dbus/.libs/usr/local/share/dbus-1/services
whereas the intention was to replace the configured prefix /usr/local
with the detected location, more like
Z:/build/dbus/.libs/share/dbus-1/services
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=92721
Reviewed-by: Ralf Habacker <ralf.habacker@freenet.de>
If the user gave us a syntactically invalid error name, we'd
overwrite the MatchRuleInvalid error with NoMemory, causing an
assertion failure (crash) in the dbus-daemon.
This is not a denial-of-service vulnerability on the system bus,
because monitoring is a privileged action, and root privilege
is checked before this code is reached. However, it's an annoying
bug on the session bus.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=92298
Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Reviewed-by: Ralf Habacker <ralf.habacker@freenet.de>
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
This makes an installed tree with
/some-prefix/
etc/
dbus-1/
session-local.conf
share/
dbus-1/
session.conf
relocatable to any location.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=92028
Reviewed-by: Ralf Habacker <ralf.habacker@freenet.de>
Tested-by: Ralf Habacker <ralf.habacker@freenet.de>
This fixes the error reporting if you make two attempts
to activate a service that cannot be activated due to an
error that is reported synchronously, such as a system
service with no User= line in its .service file.
This is easy to reproduce with the gdbus(1) tool, which
sends an Introspect call in addition to the one you asked
it to. If you try to activate a service using
gdbus call --session -d com.example.FailToActivate \
-o / -m org.freedesktop.DBus.Peer.Ping
then gdbus will actually send two method calls: one
Introspect, and one Ping. The Introspect gets the correct
error reply, but when dbus-daemon enters
bus_activation_activate_service() for the Ping call, it
sees that there is a pending activation and does an
early-return. The pending activation does not finish
until the timeout is reached.
A couple of error cases handled this correctly, but the
majority did not; make them all go into the same code path.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=92200
Reviewed-by: Thiago Macieira <thiago@kde.org>
libcap-ng < 0.7.7 leaks one non-close-on-exec fd during initialization.
test-bus asserts that all fds beyond 2 passed to an executed subprocess
have the close-on-exec flag set, which will fail at that leaked fd.
This was unnoticed until commit 517c4685, because libaudit was
previously only initialized if we were configured to switch uid,
which the regression tests do not do; the system bus is normally
the only place that happens, but the system bus is not normally
run with the "embedded tests" enabled (since they are bad
for performance and security).
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=91684
Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
This will effectively print a warning when failing to open the audit
socket running as a session bus.
The call to audit_open() should succeed even if the dbus-daemon doesn't
have the CAP_AUDIT_WRITE capability.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=83856
Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
If SELinux is enabled on the system, dbus will check the permissions but
no audit trails will be generated in case of denial as the audit
subsystem is not initialized. Same should apply for apparmor.
[smcv: without audit, the equivalent of the audit trail goes to stderr
where it can be picked up by systemd-journald]
A unprivileged user should be able to open the audit socket
(audit_open()) but should not have the permissions to log an audit
trail. The CAP_AUDIT_WRITE file capability could be set on the
dbus-daemon executable in order to allow the session bus to log an AVC
denial.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=83856
[smcv: s/should/could/ in commit message to reflect lack of consensus that
"setcap cap_audit_write+ep dbus-daemon" is desirable in general]
Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
This fixes various duplicated libaudit interactions in both
SELinux and AppArmor code paths, including opening two audit sockets
if both SELinux and AppArmor were enabled at compile time.
In particular, audit.c is now the only user of libcap-ng.
This commit is not intended to introduce any functional changes,
except for the de-duplication.
The actual audit_log_user_avc_message() call is still duplicated,
because the SELinux and AppArmor code paths use different mechanisms
to compose the audit message: the SELinux path uses a statically-sized
buffer on the stack which might be subject to truncation, whereas
the AppArmor path uses malloc() (via DBusString) and falls back to
using syslog on a memory allocation failure.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=89225
Reviewed-by: Colin Walters <walters@verbum.org>
[smcv: minor issues raised during review are subsequently fixed]
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Because the context parameter is dereferenced several times in related code without a null check,
we need to make sure to have a valid context.
Reported by Coverity: CID 54764: Dereference after null check (FORWARD_NULL)
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=90021
Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Reply message was not unreferenced when GetConnectionCredentials
handler was successful.
Signed-off-by: Jacek Bukarewicz <j.bukarewicz@samsung.com>
[smcv: changed bus_message_unref() to dbus_message_unref()]
Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=91008
This is more robust against broken setups where we run out
of memory or cannot read /dev/urandom.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=90414
Reviewed-by: Ralf Habacker <ralf.habacker@freenet.de>
[smcv: document @error]
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Previously, this would always succeed, but might use
weak random numbers in rare failure cases. I don't think
these UUIDs are security-sensitive, but if they're generated
by a PRNG as weak as rand() (<= 32 bits of entropy), we
certainly can't claim that they're universally unique.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=90414
Reviewed-by: Ralf Habacker <ralf.habacker@freenet.de>
[smcv: document @error]
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
DBUS_COOKIE_SHA1 is dependent on unguessable strings, i.e.
indirectly dependent on high-quality pseudo-random numbers
whereas EXTERNAL authentication (credentials-passing)
is mediated by the kernel and cannot be faked.
On Windows, EXTERNAL authentication is not available,
so we continue to use the hard-coded default (all
authentication mechanisms are tried).
Users of tcp: or nonce-tcp: on Unix will have to comment
this out, but they would have had to use a special
configuration anyway (to set the listening address),
and the tcp: and nonce-tcp: transports are inherently
insecure unless special steps are taken to have them
restricted to a VPN or SSH tunnelling.
Users of obscure Unix platforms (those that trigger
the warning "Socket credentials not supported on this Unix OS"
when compiling dbus-sysdeps-unix.c) might also have to
comment this out, or preferably provide a tested patch
to enable credentials-passing on that OS.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=90414
Reviewed-by: Ralf Habacker <ralf.habacker@freenet.de>
This requires generic support for keying hash tables by DBusPollable:
there are already implementations for int and uintptr_t keys, but not
for "int or uintptr_t depending on platform", which is what
DBusPollable now means.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=89444
DBUS_COOKIE_SHA1 is dependent on unguessable strings, i.e.
indirectly dependent on high-quality pseudo-random numbers
whereas EXTERNAL authentication (credentials-passing)
is mediated by the kernel and cannot be faked.
On Windows, EXTERNAL authentication is not available,
so we continue to use the hard-coded default (all
authentication mechanisms are tried).
Users of tcp: or nonce-tcp: on Unix will have to comment
this out, but they would have had to use a special
configuration anyway (to set the listening address),
and the tcp: and nonce-tcp: transports are inherently
insecure unless special steps are taken to have them
restricted to a VPN or SSH tunnelling.
Users of obscure Unix platforms (those that trigger
the warning "Socket credentials not supported on this Unix OS"
when compiling dbus-sysdeps-unix.c) might also have to
comment this out, or preferably provide a tested patch
to enable credentials-passing on that OS.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=90414
Every DBusConnection in the dbus-daemon should have been through
bus_connections_setup_connection(), so we can assert that the
BusConnectionData has been attached to it. Having this assertion
is enough to hint to Coverity that it does not need to worry about
whether this pointer might be NULL.
In regression tests, we do work with a few fake client-side
DBusConnection instances in the same process; but it would be a
serious bug if we mixed those up with the ones processed by
dbus-daemon's real code, so the assertion is still valid.
This patch has been inspired by (and fixes) the following coverity scan issues:
CID 54846: Dereference null return value (NULL_RETURNS).
CID 54854: Dereference null return value (NULL_RETURNS).
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=90021
Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
[smcv: fixed -Wdeclaration-after-statement; more informative commit message]
Reviewed-by: Ralf Habacker <ralf.habacker@freenet.de>
We already skipped processing for DBUS_ERROR_FILE_NOT_FOUND;
but if the error was something else, we would pass the NULL
pointer dir to _dbus_directory_get_next_file(), which dereferences it.
Reported by Coverity: CID 54744: Dereference after null check (FORWARD_NULL)
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=90021
[smcv: re-worded commit message]
Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
