POSIX.1-2001 and POSIX.1-2008 specifies include <poll.h> so use that
rather than the non-standard/legacy include <sys/poll.h>.

This fixes the following warnings when building with musl libc:

    1 | #warning redirecting incorrect #include <sys/poll.h> to <poll.h>
      |  ^~~~~~~
Signed-off-by: Natanael Copa <ncopa@alpinelinux.org>

Use getrandom(2) and fall back to /dev/urandom if it is missing or if it
fails some any reason.

This solves problem where dbus-uuidgen is called from a chroot which
lacks /dev/urandom.
Signed-off-by: Natanael Copa <ncopa@alpinelinux.org>

If we ran out of memory while handling connect() errors, we didn't
free the linked list of struct addrinfo. Move their cleanup to the
"out" phase of the function so that we always do it.

While I'm there, change the iterator variable tmp to be const, to make
it more obvious that we aren't meant to free it.

This is similar to commit 00badeba (!143

) in the corresponding Windows
code path, but with some refactoring.
Signed-off-by: Simon McVittie <smcv@collabora.com>

Simon McVittie authored Dec 06, 2019 and Ralf Habacker committed Dec 12, 2019

This needs new atomic primitives: we don't have "set to a value",
and in fact that's a bit annoying to implement in terms of gcc
intrinsics. "Set to 0" and "set to nonzero" are easy, though.

Signed-off-by: Simon McVittie <smcv@collabora.com>

This should avoid test failures under CMake in which the
dbus-daemon inherits an unwanted fd from CMake's test framework, causing
the close-on-exec check before executing activated services to fail.

The dbus-daemon now marks all fds that it inherits, except for its
stdin, stdout and stderr, to be closed on exec. For completeness, the
dbus-daemons run by dbus-run-session and dbus-launch also now inherit
stdin, stdout, stderr and the pipes used to communicate with their
callers, but nothing else.
Signed-off-by: Simon McVittie <smcv@collabora.com>

As suggested by Philip Withnall in dbus!43.
Signed-off-by: Simon McVittie <smcv@collabora.com>

Solaris 2.3 and 2.4 took their getpwnam_r() signature from draft 6
of the POSIX threads standard. Since Solaris 2.5 (1995), defining
_POSIX_PTHREAD_SEMANTICS opts-in to the non-draft version of
getpwnam_r(), and since Solaris 11.4 (2018), the non-draft version is
the default.

We already use AC_USE_SYSTEM_EXTENSIONS, which defines
_POSIX_PTHREAD_SEMANTICS, among other useful macros.

Thanks to Alan Coopersmith for assistance with Solaris history.
Signed-off-by: Simon McVittie <smcv@collabora.com>

David King authored Oct 12, 2018 and Simon McVittie committed Oct 18, 2018

EAGAIN and EWOULDBLOCK are documented to possibly be numerically equal,
for instance in errno(3), and a simple logical OR check will trigger the
-Wlogical-op warning of GCC. The GCC developers consider the warning to
work as-designed in this case:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69602

Avoid such a warning by explicitly checking if the values are identical.

Fixes: #225

Signed-off-by: David King <dking@redhat.com>
Reviewed-by: Simon McVittie <smcv@collabora.com>

Using strncpy (buffer, str, strlen (str)) is a "code smell" that
might indicate a serious bug (it effectively turns strncpy into
strcpy), and gcc 8 now warns about it. In fact we avoided the bug
here, but it wasn't at all obvious.

We already checked that path_len is less than or equal to
_DBUS_MAX_SUN_PATH_LENGTH, which is 99, chosen to be strictly less
than the POSIX minimum sizeof(sun_path) >= 100, so we couldn't
actually be overflowing the available buffer.

The new static assertion in this commit matches a comment above the
definition of _DBUS_MAX_SUN_PATH_LENGTH: we define
_DBUS_MAX_SUN_PATH_LENGTH to 99, because POSIX says struct
sockaddr_un's sun_path member is at least 100 bytes (including space
for a \0 terminator). dbus will now fail to compile on
platforms that are non-POSIX-compliant in this way, except for Windows.

We zeroed the struct sockaddr_un before writing into it, so stopping
one byte short of the end of sun_path ensures that we get \0
termination.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107350

Reviewed-by: Thiago Macieira <thiago@kde.org>
Reviewed-by: Philip Withnall <withnall@endlessm.com>

getaddrinfo and getnameinfo have their own error-handling convention
in which the library call returns either 0 or an EAI_* error code
unrelated to errno. If the error code is not EAI_SYSTEM, then
the value of errno is undefined (in particular it might be carried
over from a previous system call or library call). Introduce a
new helper function _dbus_error_from_gai() to handle this.

The equivalent code paths in Windows appear to be OK: the Windows
implementation of getaddrinfo() is documented to return a Winsock
error code, which we seem to be handling correctly.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=106395

Specifying a dbus tcp address without a family let dbus-daemon the choice
for listen on ipv4 or ipv6, but did not return the real used ip family,
which is fixed with this commit.

Bug:https://bugs.freedesktop.org/show_bug.cgi?id=105489

Reviewed-by: Simon McVittie <smcv@collabora.com>

A DBusString that was initialized with a constant doesn't hold any
allocated memory, so it doesn't need to be freed.
Reviewed-by: Simon McVittie <smcv@collabora.com>
https://bugs.freedesktop.org/show_bug.cgi?id=61922

Simon McVittie authored Mar 08, 2018 and Ralf Habacker committed Mar 10, 2018

Previously, we took the errno from the most recent connect() error,
and used a more generic diagnostic message.
Reviewed-by: Ralf Habacker <ralf.habacker@freenet.de>
Signed-off-by: Simon McVittie <smcv@collabora.com>
https://bugs.freedesktop.org/show_bug.cgi?id=61922

Simon McVittie authored Mar 08, 2018 and Ralf Habacker committed Mar 10, 2018

This will make it easier to do single-exit-path cleanup.
Reviewed-by: Ralf Habacker <ralf.habacker@freenet.de>
Signed-off-by: Simon McVittie <smcv@collabora.com>
https://bugs.freedesktop.org/show_bug.cgi?id=61922

When we use AF_UNSPEC, we are likely to get multiple addresses back
from getaddrinfo(), and perhaps we won't be able to use them all.
Give that failure mode, or any other bind() failure, the same treatment
as EADDRINUSE failures here and all connect() failures in
_dbus_connect_tcp_socket_with_nonce(): if any address succeeds, then
the overall operation succeeds, but if all of them fail, then the
overall operation fails.

I've made _dbus_combine_tcp_errors() generic enough that
_dbus_connect_tcp_socket_with_nonce() could use it too, although that
isn't implemented here.
Reviewed-by: Ralf Habacker <ralf.habacker@freenet.de>
[smcv: Adjust commit message]
Signed-off-by: Simon McVittie <smcv@collabora.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=61922

When we have resolved a hostname/port pair to a list of IPv4 or IPv6
addresses, if we are unable to listen on a a specific one of those
addresses, we should report which one.

When IPv6 is disabled for the loopback interface, this changes the
diagnostic from:

Failed to bind socket "localhost:1234": Cannot assign requested address

to the more informative

Failed to bind socket "::1" port 1234: Cannot assign requested address
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Ralf Habacker <ralf.habacker@freenet.de>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=61922

Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Ralf Habacker <ralf.habacker@freenet.de>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=61922

Some libdbus functions have as their API "this function sets errno",
but that just leads to strangeness on Windows, where socket (Winsock)
functions don't actually set errno themselves, leading to those
functions having to copy the Winsock error into errno to fake the
Unix-like behaviour. With hindsight, that was probably a bad idea.

This function should be portable enough to be used with functions that
are essentially the same on Unix and Windows, like inet_ntop().
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Ralf Habacker <ralf.habacker@freenet.de>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=61922

If what actually failed was reading the address from the socket,
we might as well say so.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Ralf Habacker <ralf.habacker@freenet.de>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=61922

In the current implementation, we can reach the "err" label either
because we ran out of memory, or because we got a socket error from
getsockname() or inet_ntop(), and we blindly assume that running out
of memory will set ENOMEM.

However, that isn't actually true: when we are simulating OOM in
dbus_malloc(), the fake OOM doesn't set ENOMEM. Handle the OOM
condition explicitly instead.

In the AF_UNIX case, the break statement is no longer reached at all,
so leave a comment there. In the AF_INET and AF_INET6 cases, if
inet_ntop() fails, explicitly goto err instead of using break
(this has the same practical effect) to make it clearer that we are
going to the "socket error" code path.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Ralf Habacker <ralf.habacker@freenet.de>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=61922

It only has the most important credentials, not the full set.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=103737

Signed-off-by: Simon McVittie <smcv@collabora.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=103737

Reviewed-by: Philip Withnall <withnall@endlessm.com>

dbus_realloc() doesn't guarantee to set errno (if it did, the
only reasonable thing it could set it to would be ENOMEM). In
particular, faking OOM conditions doesn't set it. This can cause an
assertion failure when OOM tests assert that the only error that can
validly occur is DBUS_ERROR_NO_MEMORY.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=89104

stdout and stderr are close-on-exec and buffered, so we can't rely on
their buffers being empty. If we continue to execute application code
after forking (as opposed to immediately exec()ing), then the child
process might later flush the libc stdio buffers, resulting in
output that is printed by the parent also being printed by the child.

In particular, test-bus.log sometimes grows extremely large for
this reason, because this test repeatedly attempts to carry out
legacy activation.
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Signed-off-by: Simon McVittie <smcv@collabora.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=103601

This is nicer for cross-compiling, because AC_RUN_IFELSE can't work
there. In practice abstract sockets are supported on Linux since
2.2 (so, all relevant versions), and on no other platform; so it
seems futile to keep this complexity.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=34905

Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>

This is a better match for the way we use it in practice.
Signed-off-by: Simon McVittie <smcv@debian.org>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=102686

Reviewed-by: Philip Withnall <withnall@endlessm.com>

dbus-sysdeps-unix.c: In function ‘_dbus_read_credentials_socket’:
dbus-sysdeps-unix.c:2061:9: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
         adt_session_data_t *adth = NULL;
         ^
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=102145

Reviewed-by: Philip Withnall <withnall@endlessm.com>
Reviewed-by: Simon McVittie <smcv@collabora.com>

Previously, the listen() backlog was set to an arbitrary 30. This means
that if dbus-daemon is overloaded only 30 more connections may be queued
by the kernel, before connect() fails with EAGAIN. (Note that EAGAIN !=
EINPROGRESS -- the latter is what is returned if a connection is queued
and being processed for asynchronous sockets; EAGAIN in this case is
really an error, that cannot be recovered from).

Most software simply sets SOMAXCONN as backlog for AF_UNIX sockets, to
allow queuing of as many connections as the kernel allows. SOMAXCONN is
128 on Linux, which is not particularly high, but at least higher than
30.

This patch changes dbus-daemon to do the same.

I noticed this when flooding dbus-daemon with a lot of connections,
where it pretty quickly ceased to respond, much earlier than it really
should.

Note that the backlog has nothing to do with the number of concurrent
connections allowed, it simply controls how many queued, but not
accept()ed connections there may be on the listening socket.

(cherry picked from commit 12bd6e89)
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=95264
Bug-Debian: https://bugs.debian.org/872144

Reviewed-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Thiago Macieira <thiago@kde.org>

This lets cooperating processes with the same value of $HOME
interoperate for DBUS_COOKIE_SHA1 by reading and writing $HOME, even
if their $HOME differs from the uid's "official" home directory
according to getpwuid(). Out of paranoia, we only do this if the uid
and the euid are equal, since if they were unequal the correct thing
to do would be ambiguous.

In particular, Debian autobuilders run as a user whose "official"
home directory in /etc/passwd is "/nonexistent", as a mechanism to
detect non-deterministic build processes that rely on the contents of
the home directory. Until now, this meant we couldn't run dbus'
build-time tests, because every test that used DBUS_COOKIE_SHA1 would
fail in this environment.

In the tests, set HOME as well as DBUS_TEST_HOMEDIR. We keep
DBUS_TEST_HOMEDIR too, because Windows doesn't use HOME, only HOMEDRIVE
and HOMEPATH.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101960
Bug-Debian: https://bugs.debian.org/630152

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

This function already raised an error, and all callers handled that
error as gracefully as they could (because _dbus_generate_uuid() is
failable, since 2015). Given that, it seems unnecessarily hostile
to do a _dbus_warn_check_failed() unless we have no better alternative:
yes, it indicates that dbus has not been installed correctly, but
during build-time tests it's entirely reasonable that dbus has not
yet been installed.

Callers are:

* DBusConnection, to implement Peer.GetMachineId()
* The bus driver, to implement Peer.GetMachineId()
* X11 autolaunching
* dbus_get_local_machine_id()

Of those, only the last one is not in a position to return an error
gracefully, so move the _dbus_warn_check_failed() to there.

Migrate the text about the D-Bus library being incorrectly set up
into the error emitted by the Unix implementation, and to make it
less misleading, include separate error messages for both the
files we try to read:

$ bwrap --ro-bind / / --dev /dev --tmpfs /etc --tmpfs /var \
  ./tools/dbus-uuidgen --get
D-Bus library appears to be incorrectly set up: see the manual
page for dbus-uuidgen to correct this issue. (Failed to open
"/var/lib/dbus/machine-id": No such file or directory; Failed to open
"/etc/machine-id": No such file or directory)
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

System integration scripts use dbus-uuidgen --ensure, so they are
unaffected by this, and in particular the solution to Bug #77941
is still valid.

The shared library is typically loaded by unprivileged users, so
trying to write out the machine-id file is not going to work anyway.
However, if we *can* write to our ${sysconfdir} - notably during
`make distcheck` - then it is unexpected that merely reading the
machine ID has the side-effect of writing to ${sysconfdir},
and in particular it will make the check for a complete uninstall
fail. We definitely must not delete the machine ID during
`make uninstall`.
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

Simon McVittie authored Jan 17, 2017 and Simon McVittie committed Apr 07, 2017

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

If we don't trap EEXIST and its Windows equivalent, we are unable to
detect the situation where we create an ostensibly unique
subdirectory in a shared /tmp, but an attacker has already created it.
This affects dbus-nonce (the nonce-tcp transport) and the activation
reload test.

Add a new _dbus_ensure_directory() for the one case where we want it to
succeed even on EEXIST: the DBUS_COOKIE_SHA1 keyring, which we know
we are creating in our own trusted "official" $HOME. In the new
transient service support on Bug #99825, ensure_owned_directory()
would need the same treatment.

We are not treating this as a serious security problem, because the
nonce-tcp transport is rarely enabled on Unix and there are multiple
mitigations.

The nonce-tcp transport creates a new unique file with O_EXCL and 0600
(private to user) permissions, then overwrites the requested filename
via atomic-overwrite, so the worst that could happen there is that an
attacker could place a symbolic link matching the name of a directory
we are going to create, causing a dbus-daemon configured for nonce-tcp
to traverse the symlink and atomically overwrite a file named "nonce"
in a directory of the attacker's choice, with new random contents that
are not known to the attacker. This seems unlikely to be exploitable
for anything worse than denial of service in practice. In mainline
Linux since 3.6, this attack is also defeated by the
fs.protected_symlinks sysctl, which many distributions enable by default.

The activation reload test suffers from a classic symlink attack
due to time-of-check/time-of-use errors in its implementation, but as
part of the developer-only "embedded tests" that are only intended
to be run on a trusted machine, it is not treated as security-sensitive.
That code path will be fixed in a subsequent commit.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99828

Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Reviewed-by: Philip Withnall <withnall@endlessm.com>

res is an integer, not a string.

Bug found by adding more _DBUS_GNUC_PRINTF attributes.
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>

glibc >= 2.24 marks readdir_r() as deprecated. It is meant to be a
thread-safe version of readdir(), but modern implementations of readdir()
are thread-safe anyway (when called with a distinct DIR * argument),
and readdir_r() has some design issues involving PATH_MAX.

This code path is in Linux-specific code, so we can safely assume a
high-quality implementation of readdir().
Signed-off-by: Simon McVittie <smcv@debian.org>
Reviewed-by: Thomas Zimmermann <tdz@users.sourceforge.net>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=97357

As a general design principle, strings that we aren't going to modify
should usually be const. When compiling with -Wwrite-strings, quoted
string constants are of type "const char *", causing compiler warnings
when they are assigned to char * variables.

Unfortunately, we need to add casts in a few places:

* _dbus_list_append(), _dbus_test_oom_handling() and similar generic
  "user-data" APIs take a void *, not a const void *, so we have
  to cast
* For historical reasons the execve() family of functions take a
  (char * const *), i.e. a constant pointer to an array of mutable
  strings, so again we have to cast
* _dbus_spawn_async_with_babysitter similarly takes a char **,
  although we can make it a little more const-correct by making it
  take (char * const *) like execve() does

This also incorporates a subsequent patch by Thomas Zimmermann to
put various string constants in static storage, which is a little
more efficient.
Signed-off-by: Simon McVittie <smcv@debian.org>
Review...

They used to be needed, but are not needed any more, and we were
never completely consistent about including them in any case.
Signed-off-by: Simon McVittie <smcv@debian.org>

This changes the behaviour of _dbus_logv() if _dbus_init_system_log() was
not called. Previously, _dbus_logv() would always log to syslog;
additionally, it would log to stderr, unless the process is dbus-daemon
and it was started by systemd. Now, it will log to stderr only,
unless _dbus_init_system_log() was called first.

This is the desired behaviour because when we hook up
_dbus_warn_check_failed() to _dbus_logv() in the next commit, we don't
want typical users of libdbus to start logging their check failures to
syslog - we only want the dbus-daemon to do that.

In practice this is not usually a behaviour change, because there was
only one situation in which we called _dbus_logv() without first calling
_dbus_init_system_log(), namely an error while parsing configuration
files. Initialize the system log "just in time" in that situation
to preserve existing behaviour.
Signed-off-by: Simon McVittie <smcv@debian.org>