[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#944233: stretch-pu: package glib2.0/2.50.3-2+deb9u1



Package: release.debian.org
Severity: normal
Tags: stretch
User: release.debian.org@packages.debian.org
Usertags: pu

A recent security fix to ibus (CVE-2019-14822, #940267, DSA-4525-1)
exposed an interoperability bug between GLib's implementation of D-Bus
and the reference implementation libdbus (#941018). The practical impact
is that Qt clients cannot use the updated ibus input method until GLib
is fixed.

This is the same as #944133, but for the older GLib in stretch. I've
tested both this and the buster update in corresponding GNOME virtual
machines; in both cases I can reproduce #941018 with the updated ibus,
and installing this version of GLib appears to fix it.

I haven't included the new unit test in this version. It usually passes
when run manually (confirming that the fix does work), but intermittently
hangs, and seems to hang more frequently when run in sbuild. I suspect
this is an unrelated multi-threading issue in GDBus message processing
(the GDBus tests tend to exercise this more aggressively than normal
applications). If so, it's unlikely to be a regression.

    smcv
diffstat for glib2.0-2.50.3 glib2.0-2.50.3

 changelog                                                               |   11 
 control                                                                 |    4 
 control.in                                                              |    4 
 patches/GDBus-prefer-getsockopt-style-credentials-passing-APIs.patch    |  167 ++++++++++
 patches/credentials-Invalid-Linux-struct-ucred-means-no-informati.patch |  117 +++++++
 patches/series                                                          |    2 
 6 files changed, 301 insertions(+), 4 deletions(-)

diff -Nru glib2.0-2.50.3/debian/changelog glib2.0-2.50.3/debian/changelog
--- glib2.0-2.50.3/debian/changelog	2019-08-13 10:46:20.000000000 +0100
+++ glib2.0-2.50.3/debian/changelog	2019-11-06 08:29:30.000000000 +0000
@@ -1,3 +1,14 @@
+glib2.0 (2.50.3-2+deb9u2) stretch; urgency=medium
+
+  * Team upload
+  * d/p/credentials-Invalid-Linux-struct-ucred-means-no-informati.patch,
+    d/p/GDBus-prefer-getsockopt-style-credentials-passing-APIs.patch:
+    Ensure libdbus clients can authenticate with a GDBusServer like the
+    one in ibus, backported from upstream 2.62.x branch (Closes: #941018)
+  * d/control.in: Update Vcs-Git, Vcs-Browser
+
+ -- Simon McVittie <smcv@debian.org>  Wed, 06 Nov 2019 08:29:30 +0000
+
 glib2.0 (2.50.3-2+deb9u1) stretch; urgency=medium
 
   * Team upload
diff -Nru glib2.0-2.50.3/debian/control glib2.0-2.50.3/debian/control
--- glib2.0-2.50.3/debian/control	2019-08-13 10:46:20.000000000 +0100
+++ glib2.0-2.50.3/debian/control	2019-11-06 08:29:30.000000000 +0000
@@ -35,8 +35,8 @@
                libffi-dev (>= 3.0.0)
 Standards-Version: 3.9.8
 Homepage: http://www.gtk.org/
-Vcs-Browser: https://anonscm.debian.org/viewvc/pkg-gnome/desktop/unstable/glib2.0/
-Vcs-Svn: svn://anonscm.debian.org/pkg-gnome/desktop/unstable/glib2.0/
+Vcs-Browser: https://salsa.debian.org/gnome-team/glib
+Vcs-Git: https://salsa.debian.org/gnome-team/glib.git -b debian/stretch
 
 Package: libglib2.0-0
 Architecture: any
diff -Nru glib2.0-2.50.3/debian/control.in glib2.0-2.50.3/debian/control.in
--- glib2.0-2.50.3/debian/control.in	2019-08-13 10:46:20.000000000 +0100
+++ glib2.0-2.50.3/debian/control.in	2019-11-06 08:29:30.000000000 +0000
@@ -35,8 +35,8 @@
                libffi-dev (>= 3.0.0)
 Standards-Version: 3.9.8
 Homepage: http://www.gtk.org/
-Vcs-Browser: https://anonscm.debian.org/viewvc/pkg-gnome/desktop/unstable/glib2.0/
-Vcs-Svn: svn://anonscm.debian.org/pkg-gnome/desktop/unstable/glib2.0/
+Vcs-Browser: https://salsa.debian.org/gnome-team/glib
+Vcs-Git: https://salsa.debian.org/gnome-team/glib.git -b debian/stretch
 
 Package: @SHARED_PKG@
 Architecture: any
diff -Nru glib2.0-2.50.3/debian/patches/credentials-Invalid-Linux-struct-ucred-means-no-informati.patch glib2.0-2.50.3/debian/patches/credentials-Invalid-Linux-struct-ucred-means-no-informati.patch
--- glib2.0-2.50.3/debian/patches/credentials-Invalid-Linux-struct-ucred-means-no-informati.patch	1970-01-01 01:00:00.000000000 +0100
+++ glib2.0-2.50.3/debian/patches/credentials-Invalid-Linux-struct-ucred-means-no-informati.patch	2019-11-06 08:29:30.000000000 +0000
@@ -0,0 +1,117 @@
+From: Simon McVittie <smcv@collabora.com>
+Date: Fri, 18 Oct 2019 10:55:09 +0100
+Subject: credentials: Invalid Linux struct ucred means "no information"
+
+On Linux, if getsockopt SO_PEERCRED is used on a TCP socket, one
+might expect it to fail with an appropriate error like ENOTSUP or
+EPROTONOSUPPORT. However, it appears that in fact it succeeds, but
+yields a credentials structure with pid 0, uid -1 and gid -1. These
+are not real process, user and group IDs that can be allocated to a
+real process (pid 0 needs to be reserved to give kill(0) its documented
+special semantics, and similarly uid and gid -1 need to be reserved for
+setresuid() and setresgid()) so it is not meaningful to signal them to
+high-level API users.
+
+An API user with Linux-specific knowledge can still inspect these fields
+via g_credentials_get_native() if desired.
+
+Similarly, if SO_PASSCRED is used to receive a SCM_CREDENTIALS message
+on a receiving Unix socket, but the sending socket had not enabled
+SO_PASSCRED at the time that the message was sent, it is possible
+for it to succeed but yield a credentials structure with pid 0, uid
+/proc/sys/kernel/overflowuid and gid /proc/sys/kernel/overflowgid. Even
+if we were to read those pseudo-files, we cannot distinguish between
+the overflow IDs and a real process that legitimately has the same IDs
+(typically they are set to 'nobody' and 'nogroup', which can be used
+by a real process), so we detect this situation by noticing that
+pid == 0, and to save syscalls we do not read the overflow IDs from
+/proc at all.
+
+This results in a small API change: g_credentials_is_same_user() now
+returns FALSE if we compare two credentials structures that are both
+invalid. This seems like reasonable, conservative behaviour: if we cannot
+prove that they are the same user, we should assume they are not.
+
+[Philip Withnall: Dropped new translatable string when backporting to
+`glib-2-62`.]
+
+Signed-off-by: Simon McVittie <smcv@collabora.com>
+Origin: upstream, 2.62.3, commit:5f9318af8f19756685c1b79cf8b76f3e66614d84
+---
+ gio/gcredentials.c | 42 +++++++++++++++++++++++++++++++++++++++---
+ 1 file changed, 39 insertions(+), 3 deletions(-)
+
+diff --git a/gio/gcredentials.c b/gio/gcredentials.c
+index 6d6b92a..6be69ed 100644
+--- a/gio/gcredentials.c
++++ b/gio/gcredentials.c
+@@ -262,6 +262,35 @@ g_credentials_to_string (GCredentials *credentials)
+ 
+ /* ---------------------------------------------------------------------------------------------------- */
+ 
++#if G_CREDENTIALS_USE_LINUX_UCRED
++/*
++ * Check whether @native contains invalid data. If getsockopt SO_PEERCRED
++ * is used on a TCP socket, it succeeds but yields a credentials structure
++ * with pid 0, uid -1 and gid -1. Similarly, if SO_PASSCRED is used on a
++ * receiving Unix socket when the sending socket did not also enable
++ * SO_PASSCRED, it can succeed but yield a credentials structure with
++ * pid 0, uid /proc/sys/kernel/overflowuid and gid
++ * /proc/sys/kernel/overflowgid.
++ */
++static gboolean
++linux_ucred_check_valid (struct ucred  *native,
++                         GError       **error)
++{
++  if (native->pid == 0
++      || native->uid == -1
++      || native->gid == -1)
++    {
++      g_set_error_literal (error,
++                           G_IO_ERROR,
++                           G_IO_ERROR_INVALID_DATA,
++                           "GCredentials contains invalid data");
++      return FALSE;
++    }
++
++  return TRUE;
++}
++#endif
++
+ /**
+  * g_credentials_is_same_user:
+  * @credentials: A #GCredentials.
+@@ -291,7 +320,8 @@ g_credentials_is_same_user (GCredentials  *credentials,
+ 
+   ret = FALSE;
+ #if G_CREDENTIALS_USE_LINUX_UCRED
+-  if (credentials->native.uid == other_credentials->native.uid)
++  if (linux_ucred_check_valid (&credentials->native, NULL)
++      && credentials->native.uid == other_credentials->native.uid)
+     ret = TRUE;
+ #elif G_CREDENTIALS_USE_FREEBSD_CMSGCRED
+   if (credentials->native.cmcred_euid == other_credentials->native.cmcred_euid)
+@@ -450,7 +480,10 @@ g_credentials_get_unix_user (GCredentials    *credentials,
+   g_return_val_if_fail (error == NULL || *error == NULL, -1);
+ 
+ #if G_CREDENTIALS_USE_LINUX_UCRED
+-  ret = credentials->native.uid;
++  if (linux_ucred_check_valid (&credentials->native, error))
++    ret = credentials->native.uid;
++  else
++    ret = -1;
+ #elif G_CREDENTIALS_USE_FREEBSD_CMSGCRED
+   ret = credentials->native.cmcred_euid;
+ #elif G_CREDENTIALS_USE_NETBSD_UNPCBID
+@@ -496,7 +529,10 @@ g_credentials_get_unix_pid (GCredentials    *credentials,
+   g_return_val_if_fail (error == NULL || *error == NULL, -1);
+ 
+ #if G_CREDENTIALS_USE_LINUX_UCRED
+-  ret = credentials->native.pid;
++  if (linux_ucred_check_valid (&credentials->native, error))
++    ret = credentials->native.pid;
++  else
++    ret = -1;
+ #elif G_CREDENTIALS_USE_FREEBSD_CMSGCRED
+   ret = credentials->native.cmcred_pid;
+ #elif G_CREDENTIALS_USE_NETBSD_UNPCBID
diff -Nru glib2.0-2.50.3/debian/patches/GDBus-prefer-getsockopt-style-credentials-passing-APIs.patch glib2.0-2.50.3/debian/patches/GDBus-prefer-getsockopt-style-credentials-passing-APIs.patch
--- glib2.0-2.50.3/debian/patches/GDBus-prefer-getsockopt-style-credentials-passing-APIs.patch	1970-01-01 01:00:00.000000000 +0100
+++ glib2.0-2.50.3/debian/patches/GDBus-prefer-getsockopt-style-credentials-passing-APIs.patch	2019-11-06 08:29:30.000000000 +0000
@@ -0,0 +1,167 @@
+From: Simon McVittie <smcv@collabora.com>
+Date: Mon, 14 Oct 2019 08:47:39 +0100
+Subject: 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>
+Bug: https://gitlab.gnome.org/GNOME/glib/issues/1831
+Origin: upstream, 2.62.3, commit:c7618cce3752e1f3681f75d0a26c7e07c15bd6a2
+---
+ 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 e156276..7a90342 100644
+--- a/gio/gcredentialsprivate.h
++++ b/gio/gcredentialsprivate.h
+@@ -22,6 +22,18 @@
+ #include "gio/gcredentials.h"
+ #include "gio/gnetworking.h"
+ 
++/*
++ * 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
+@@ -41,6 +53,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 a870e86..2ef19af 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"
+@@ -997,9 +998,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 (TODO: read credentials if using a unix domain socket) */
++  /* 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),
diff -Nru glib2.0-2.50.3/debian/patches/series glib2.0-2.50.3/debian/patches/series
--- glib2.0-2.50.3/debian/patches/series	2019-08-13 10:46:20.000000000 +0100
+++ glib2.0-2.50.3/debian/patches/series	2019-11-06 08:29:30.000000000 +0000
@@ -18,3 +18,5 @@
 gmarkup-Fix-unvalidated-UTF-8-read-in-markup-parsing-erro.patch
 gmarkup-Avoid-reading-off-the-end-of-a-buffer-when-non-nu.patch
 gmarkup-Fix-crash-in-error-handling-path-for-closing-elem.patch
+credentials-Invalid-Linux-struct-ucred-means-no-informati.patch
+GDBus-prefer-getsockopt-style-credentials-passing-APIs.patch

Reply to: