Initialize audit subsystem even for the session bus
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>
Simon McVittie [Fri, 3 Jul 2015 15:57:28 +0000 (16:57 +0100)]
audit: only check for CAP_AUDIT_WRITE once, during initialization
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=89225 Reviewed-by: Colin Walters <walters@verbum.org> Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Simon McVittie [Thu, 19 Feb 2015 12:04:26 +0000 (12:04 +0000)]
bus: move shared libaudit code to a new audit.[ch]
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>
Simon McVittie [Fri, 3 Jul 2015 12:50:04 +0000 (13:50 +0100)]
dbus-monitor: disable automatic handling of o.fd.Peer messages
A normal DBusConnection will automatically reply to o.fd.Peer
messages such as Ping. We don't want this: we'll
confuse everyone else by replying to messages that weren't
intended for us.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=90952 Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk> Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
(cherry picked from commit d9ee040d0bff2b421bca80c2339dcd9347d906db,
commit message adjusted to describe the impact in versions < 1.9)
Conflicts:
tools/dbus-monitor.c
Simon McVittie [Fri, 3 Jul 2015 12:59:44 +0000 (13:59 +0100)]
Add test-case for the same situation as fd.o #90952
This does not directly test the code in the previous commit, but it does
confirm that calling dbus_connection_set_route_peer_messages() is enough
to fix the observed bug.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=90952 Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk> Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
[smcv: re-worded commit message in response to review]
Simon McVittie [Fri, 3 Jul 2015 12:50:04 +0000 (13:50 +0100)]
dbus-monitor: disable automatic handling of o.fd.Peer messages
A normal DBusConnection will automatically reply to o.fd.Peer
messages such as Ping. We don't want this: if we are using
traditional eavesdropping with an older dbus-daemon, we'll
confuse everyone else by replying to messages that weren't
intended for us. If we are using the new Monitoring
interface (since 1.9.12), the same still applies, but in
addition, the dbus-daemon will disconnect us for not being
a well-behaved monitor.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=90952 Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk> Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Jacek Bukarewicz [Wed, 17 Jun 2015 17:53:41 +0000 (18:53 +0100)]
Fix memleak in GetConnectionCredentials handler
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
Simon McVittie [Tue, 12 May 2015 10:35:04 +0000 (11:35 +0100)]
Make UUID generation failable
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.
Simon McVittie [Thu, 14 May 2015 11:17:10 +0000 (12:17 +0100)]
_dbus_server_new_for_socket: raise a DBusError
This can currently only fail due to OOM, but I'm about to
make it possible to fail for other reasons.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=90414 Reviewed-by: Ralf Habacker <ralf.habacker@freenet.de>
[smcv: correct failure to set error in one case; document @error] Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Simon McVittie [Tue, 12 May 2015 10:10:58 +0000 (11:10 +0100)]
Security hardening: force EXTERNAL auth in session.conf on Unix
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.
Simon McVittie [Tue, 27 Aug 2013 18:16:42 +0000 (19:16 +0100)]
Avoid reading beyond the length of a variable
Appending &some as DBUS_TYPE_INT64, DBUS_TYPE_UINT64 or DBUS_TYPE_DOUBLE,
where "some" is an int, reads beyond the bounds of that variable.
Use a zero-filled DBusBasicValue instead.
Simon McVittie [Thu, 12 Mar 2015 15:21:40 +0000 (15:21 +0000)]
DBusMainLoop, DBusSocketSet: work in terms of DBusPollable
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.
Ralf Habacker [Tue, 31 Mar 2015 19:53:22 +0000 (21:53 +0200)]
Use typedef DBusSocket for sockets fd's to avoid conversion warnings.
[smcv: remove unneeded and invalid dbus-sysdeps.h from public header;
make prototype of _dbus_socketpair() consistent; undo conversion
of getaddrinfo result from int to SOCKET; don't call
_dbus_return_val_if_fail() from internal function]
Simon McVittie [Tue, 12 May 2015 10:54:50 +0000 (11:54 +0100)]
Security hardening: force EXTERNAL auth in session.conf on Unix
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.
This appears to cause a segfault, presumably resulting from something
assuming that reader_init() would not reinitialize all fields:
#0 0x00007ffff7b74777 in _dbus_type_reader_get_current_type (reader=reader@entry=0x7fffffffda50) at .../dbus/dbus-marshal-recursive.c:791
#1 0x00007ffff7b719d0 in _dbus_header_cache_check (header=<optimized out>)
at .../dbus/dbus-marshal-header.c:209
#2 0x00007ffff7b719d0 in _dbus_header_cache_check (header=header@entry=0x624658, field=field@entry=6) at .../dbus/dbus-marshal-header.c:250
#3 0x00007ffff7b72884 in _dbus_header_get_field_basic (header=header@entry=0x624658, field=field@entry=6, type=type@entry=115, value=value@entry=0x7fffffffdbd8) at .../dbus/dbus-marshal-header.c:1365
#4 0x00007ffff7b7d8c2 in dbus_message_get_destination (message=message@entry=0x624650) at .../dbus/dbus-message.c:3457
#5 0x00007ffff7b67be6 in _dbus_connection_send_preallocated_unlocked_no_update (connection=connection@entry=0x6236d0, preallocated=0x0,
preallocated@entry=0x6234c0, message=message@entry=0x624650, client_serial=client_serial@entry=0x7fffffffdcbc)
at .../dbus/dbus-connection.c:2017
Adrian Szyndela [Tue, 5 May 2015 11:30:30 +0000 (12:30 +0100)]
DBusCounter: add a mutex to protect the refcount and notify function
The overall problem here is that DBusCounter is indirectly linked
to a DBusConnection, but is not actually guaranteed to be protected by
that connection's mutex; and a DBusMessage can carry a reference to the
DBusCounter, resulting in freeing that DBusMessage having an effect on
the DBusCounter.
Making the refcount atomic would not be a sufficient fix, since it would
not protect the notify function: _dbus_counter_notify() could be called
indirectly by dbus_message_unref(), in an arbitrary thread that does not
hold the DBusConnection's lock, at the same time that the holder
of the DBusConnection lock calls _dbus_transport_set_max_message_size().
[smcv: added commit message]
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=89297 Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Adrian Szyndela [Tue, 5 May 2015 11:27:15 +0000 (12:27 +0100)]
extend lock's range in live_messages_notify()
The other code paths that ref or unref a transport are protected by
the DBusConnection's lock. This function already used that lock,
but for a narrower scope than the refcount manipulation.
live_messages_notify() could be triggered by unreffing messages
that originated from the same connection in a different thread.
[smcv: added commit message]
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=90312 Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
unistd.h and sleep() are normally Unix-specific, although mingw also provides them.
The Windows C runtime documents _environ as its equivalent of Unix environ.
https://bugs.freedesktop.org/show_bug.cgi?id=90089 Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Always assert that BUS_CONNECTION_DATA() returns non-NULL
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>
Simon McVittie [Wed, 1 Apr 2015 12:07:19 +0000 (13:07 +0100)]
Change syntax of AM_TESTS_ENVIRONMENT to what the Automake docs prefer
On closer inspection of Automake docs, this is how AM_TESTS_ENVIRONMENT
is actually meant to work; the parallel test driver is even less
compatible with the old serial test driver than I'd realised :-(
Also, according to <http://www.unix.com/man-page/POSIX/1posix/export>,
"export FOO=bar" is actually required functionality for POSIX shells,
and is not a bashism. The Autoconf documentation mentions Solaris 10
as an example of somewhere this doesn't work... but at this point
I'd prefer to say "compiling dbus requires a POSIX shell".
Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=89846
Simon McVittie [Mon, 2 Mar 2015 11:46:13 +0000 (11:46 +0000)]
Depend on Automake 1.13 so we can use the correct AM_TESTS_ENVIRONMENT
Since Automake 1.13 (released December 2012) the correct way for a
maintainer to specify environment variables has been
AM_TESTS_ENVIRONMENT, with TESTS_ENVIRONMENT reserved for the user.
That doesn't work in older Automake, so drop support for such old
versions.
Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=89846
Simon McVittie [Thu, 26 Feb 2015 17:55:45 +0000 (17:55 +0000)]
installed-tests: don't set DBUS_TEST_HOME which is misleading
It doesn't do anything - the variable I was thinking of is called
DBUS_TEST_HOMEDIR. Also, if I had spelled it correctly, the tests
would have failed, because libdbus (quite reasonably) won't create
a nonexistent $HOME to write out cookie_sha1 files in ~/.dbus_keyrings.
Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=89846
Simon McVittie [Thu, 26 Feb 2015 17:53:46 +0000 (17:53 +0000)]
installed-tests: declare that the output is in TAP format
For the ones written using GLib, the output is in TAP format if we
say --tap. For the one not written using GLib, the output is in TAP
and the command-line is ignored.
Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=89846
Simon McVittie [Thu, 26 Feb 2015 17:11:19 +0000 (17:11 +0000)]
tests: provide g_test_skip() emulation for older GLib
We don't hard-depend on a new enough GLib to have g_test_skip();
if our GLib is older, fake it using g_test_message() and degrade to
reporting it as a pass rather than a skip.
Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=89846
projects / thirdparty / dbus.git / log
