171 lines
6.7 KiB
Diff
171 lines
6.7 KiB
Diff
From ee502dbbe89a5976c32eb8863c9a9d274ddb60e1 Mon Sep 17 00:00:00 2001
|
|
From: Simon McVittie <smcv@collabora.com>
|
|
Date: Mon, 14 Oct 2019 08:47:39 +0100
|
|
Subject: [PATCH] GDBus: prefer getsockopt()-style credentials-passing APIs
|
|
|
|
Conceptually, a D-Bus server is really trying to determine the credentials
|
|
of (the process that initiated) a connection, not the credentials that
|
|
the process had when it sent a particular message. Ideally, it does
|
|
this with a getsockopt()-style API that queries the credentials of the
|
|
connection's initiator without requiring any particular cooperation from
|
|
that process, avoiding a class of possible failures.
|
|
|
|
The leading '\0' in the D-Bus protocol is primarily a workaround
|
|
for platforms where the message-based credentials-passing API is
|
|
strictly better than the getsockopt()-style API (for example, on
|
|
FreeBSD, SCM_CREDS includes a process ID but getpeereid() does not),
|
|
or where the getsockopt()-style API does not exist at all. As a result
|
|
libdbus, the reference implementation of D-Bus, does not implement
|
|
Linux SCM_CREDENTIALS at all - it has no reason to do so, because the
|
|
SO_PEERCRED socket option is equally informative.
|
|
|
|
This change makes GDBusServer on Linux more closely match the behaviour
|
|
of libdbus.
|
|
|
|
In particular, GNOME/glib#1831 indicates that when a libdbus client
|
|
connects to a GDBus server, recvmsg() sometimes yields a SCM_CREDENTIALS
|
|
message with cmsg_data={pid=0, uid=65534, gid=65534}. I think this is
|
|
most likely a race condition in the early steps to connect:
|
|
|
|
client server
|
|
connect
|
|
accept
|
|
send '\0' <- race -> set SO_PASSCRED = 1
|
|
receive '\0'
|
|
|
|
If the server wins the race:
|
|
|
|
client server
|
|
connect
|
|
accept
|
|
set SO_PASSCRED = 1
|
|
send '\0'
|
|
receive '\0'
|
|
|
|
then everything is fine. However, if the client wins the race:
|
|
|
|
client server
|
|
connect
|
|
accept
|
|
send '\0'
|
|
set SO_PASSCRED = 1
|
|
receive '\0'
|
|
|
|
then the kernel does not record credentials for the message containing
|
|
'\0' (because SO_PASSCRED was 0 at the time). However, by the time the
|
|
server receives the message, the kernel knows that credentials are
|
|
desired. I would have expected the kernel to omit the credentials header
|
|
in this case, but it seems that instead, it synthesizes a credentials
|
|
structure with a dummy process ID 0, a dummy uid derived from
|
|
/proc/sys/kernel/overflowuid and a dummy gid derived from
|
|
/proc/sys/kernel/overflowgid.
|
|
|
|
In an unconfigured GDBusServer, hitting this race condition results in
|
|
falling back to DBUS_COOKIE_SHA1 authentication, which in practice usually
|
|
succeeds in authenticating the peer's uid. However, we encourage AF_UNIX
|
|
servers on Unix platforms to allow only EXTERNAL authentication as a
|
|
security-hardening measure, because DBUS_COOKIE_SHA1 relies on a series
|
|
of assumptions including a cryptographically strong PRNG and a shared
|
|
home directory with no write access by others, which are not necessarily
|
|
true for all operating systems and users. EXTERNAL authentication will
|
|
fail if the server cannot determine the client's credentials.
|
|
|
|
In particular, this caused a regression when CVE-2019-14822 was fixed
|
|
in ibus, which appears to be resolved by this commit. Qt clients
|
|
(which use libdbus) intermittently fail to connect to an ibus server
|
|
(which uses GDBusServer), because ibus no longer allows DBUS_COOKIE_SHA1
|
|
authentication or non-matching uids.
|
|
|
|
Signed-off-by: Simon McVittie <smcv@collabora.com>
|
|
Closes: https://gitlab.gnome.org/GNOME/glib/issues/1831
|
|
---
|
|
gio/gcredentialsprivate.h | 18 ++++++++++++++++++
|
|
gio/gdbusauth.c | 27 +++++++++++++++++++++++++--
|
|
2 files changed, 43 insertions(+), 2 deletions(-)
|
|
|
|
diff --git a/gio/gcredentialsprivate.h b/gio/gcredentialsprivate.h
|
|
index 06f0aed19..e9ec09b9f 100644
|
|
--- a/gio/gcredentialsprivate.h
|
|
+++ b/gio/gcredentialsprivate.h
|
|
@@ -81,6 +81,18 @@
|
|
*/
|
|
#undef G_CREDENTIALS_SPOOFING_SUPPORTED
|
|
|
|
+/*
|
|
+ * G_CREDENTIALS_PREFER_MESSAGE_PASSING:
|
|
+ *
|
|
+ * Defined to 1 if the data structure transferred by the message-passing
|
|
+ * API is strictly more informative than the one transferred by the
|
|
+ * `getsockopt()`-style API, and hence should be preferred, even for
|
|
+ * protocols like D-Bus that are defined in terms of the credentials of
|
|
+ * the (process that opened the) socket, as opposed to the credentials
|
|
+ * of an individual message.
|
|
+ */
|
|
+#undef G_CREDENTIALS_PREFER_MESSAGE_PASSING
|
|
+
|
|
#ifdef __linux__
|
|
#define G_CREDENTIALS_SUPPORTED 1
|
|
#define G_CREDENTIALS_USE_LINUX_UCRED 1
|
|
@@ -100,6 +112,12 @@
|
|
#define G_CREDENTIALS_NATIVE_SIZE (sizeof (struct cmsgcred))
|
|
#define G_CREDENTIALS_UNIX_CREDENTIALS_MESSAGE_SUPPORTED 1
|
|
#define G_CREDENTIALS_SPOOFING_SUPPORTED 1
|
|
+/* GLib doesn't implement it yet, but FreeBSD's getsockopt()-style API
|
|
+ * is getpeereid(), which is not as informative as struct cmsgcred -
|
|
+ * it does not tell us the PID. As a result, libdbus prefers to use
|
|
+ * SCM_CREDS, and if we implement getpeereid() in future, we should
|
|
+ * do the same. */
|
|
+#define G_CREDENTIALS_PREFER_MESSAGE_PASSING 1
|
|
|
|
#elif defined(__NetBSD__)
|
|
#define G_CREDENTIALS_SUPPORTED 1
|
|
diff --git a/gio/gdbusauth.c b/gio/gdbusauth.c
|
|
index 752ec23fc..14cc5d70e 100644
|
|
--- a/gio/gdbusauth.c
|
|
+++ b/gio/gdbusauth.c
|
|
@@ -31,6 +31,7 @@
|
|
#include "gdbusutils.h"
|
|
#include "gioenumtypes.h"
|
|
#include "gcredentials.h"
|
|
+#include "gcredentialsprivate.h"
|
|
#include "gdbusprivate.h"
|
|
#include "giostream.h"
|
|
#include "gdatainputstream.h"
|
|
@@ -969,9 +970,31 @@ _g_dbus_auth_run_server (GDBusAuth *auth,
|
|
|
|
g_data_input_stream_set_newline_type (dis, G_DATA_STREAM_NEWLINE_TYPE_CR_LF);
|
|
|
|
- /* first read the NUL-byte */
|
|
+ /* read the NUL-byte, possibly with credentials attached */
|
|
#ifdef G_OS_UNIX
|
|
- if (G_IS_UNIX_CONNECTION (auth->priv->stream))
|
|
+#ifndef G_CREDENTIALS_PREFER_MESSAGE_PASSING
|
|
+ if (G_IS_SOCKET_CONNECTION (auth->priv->stream))
|
|
+ {
|
|
+ GSocket *sock = g_socket_connection_get_socket (G_SOCKET_CONNECTION (auth->priv->stream));
|
|
+
|
|
+ local_error = NULL;
|
|
+ credentials = g_socket_get_credentials (sock, &local_error);
|
|
+
|
|
+ if (credentials == NULL && !g_error_matches (local_error, G_IO_ERROR, G_IO_ERROR_NOT_SUPPORTED))
|
|
+ {
|
|
+ g_propagate_error (error, local_error);
|
|
+ goto out;
|
|
+ }
|
|
+ else
|
|
+ {
|
|
+ /* Clear the error indicator, so we can retry with
|
|
+ * g_unix_connection_receive_credentials() if necessary */
|
|
+ g_clear_error (&local_error);
|
|
+ }
|
|
+ }
|
|
+#endif
|
|
+
|
|
+ if (credentials == NULL && G_IS_UNIX_CONNECTION (auth->priv->stream))
|
|
{
|
|
local_error = NULL;
|
|
credentials = g_unix_connection_receive_credentials (G_UNIX_CONNECTION (auth->priv->stream),
|
|
--
|
|
2.23.0
|
|
|