[glib/glib-2-62: 7/8] GDBus: prefer getsockopt()-style credentials-passing APIs

[Simon McVittie <smcv src gnome org>]

commit c7618cce3752e1f3681f75d0a26c7e07c15bd6a2
Author: Simon McVittie <smcv collabora com>
Date:   Mon Oct 14 08:47:39 2019 +0100

    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),