Skip to content

Avoid race condition authenticating GDBusServer with libdbus client (#1831)

Simon McVittie requested to merge smcv/glib:issue1831 into master
  • gcredentialsprivate: Document the various private macros

  • 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.

  • 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, #1831 (closed) 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.

  • Add a test for GDBusServer authentication

    In particular, if libbdus is available, we test interoperability with a libdbus client: see #1831 (closed). Because that issue describes a race condition, we do each test repeatedly to try to hit the failing case.

Fixes #1831 (closed)


Note that Gitlab-CI cannot actually run the tests for this until !1177 (merged) (or something similar) lands.

Edited by Simon McVittie

Merge request reports