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
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.
Signed-off-by: Simon McVittie <smcv@collabora.com>
The compiler can’t work out from the combination of other conditions
that it’s not possible for (m2 == NULL) to hold true when memcmp() is
called, so add an explicit condition.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Fixes: #1897
Otherwise we’ll end up using the host’s `objcopy`, which will output
object files in the wrong format.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Fixes: #1916
Since we have the type of the GValue we're going to initialize, we can
allow passing an empty (but valid) GValue when retrieving the default
value of a GParamSpec.
This will eliminate additional checks and an unnecessary reset.
Previously, these GTimeZone objects were being cached in the `time_zones` cache, but dropped from it when their final ref was dropped (which was frequently). That meant additional reads of `/etc/localtime` next time they were created, which was noticeable on profiles. Keep a permanent ref to the UTC and local timezones.
This removes the limitation of select() that only FDs with values lower
than FD_SETSIZE can be used. Previously, if the out/err pipe FDs had
high values (which could happen if a large process, like Firefox, was
spawning subprocesses while having a lot of FDs open), GLib would abort
due to an assertion failure in libc.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Fixes: #954
glibc declares memcpy() with the first two arguments (the pointers)
annotated as non-null via an attribute, which results in the undefined
behaviour sanitizer considering it to be UB to pass a null pointer
in the second argument, even if we are copying 0 bytes (and hence not
actually dereferencing the pointer).
This shows up in array-test when run with the undefined behaviour
sanitizer.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Note that I deliberately haven't used g_autoptr here, because while we
encourage GLib users to use g_autoptr in their own code, GLib itself
still supports being compiled in environments like MSVC that can't
support g_autoptr.
Signed-off-by: Simon McVittie <smcv@collabora.com>
The user_data for g_ptr_array_sort_with_data is passed directly, not
with an extra layer of pointer like the data pointers.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Fixes: 52c130f8
Let's not encourage library users to sprinkle casts through their code
when they don't need to.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Fixes: 52c130f8
This is like `GMutexLocker`, in that if you are able to use
`g_autoptr()`, it makes popping a `GMainContext` off the thread-default
main context stack easier when exiting a function.
A few uses of `G_GNUC_{BEGIN,END}_IGNORE_DEPRECATIONS` are needed to
avoid warnings when building apps against GLib with
`GLIB_VERSION_MAX_ALLOWED < GLIB_VERSION_2_64`.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
We may need to declare autocleanups for new types, which will be marked
as ‘deprecated’ if the code which includes GLib doesn’t declare a high
enough `GLIB_VERSION_MAX_ALLOWED`. Despite that, we still need to
declare the autocleanups.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
The token parsing done by g_variant_parse() uses recursive function
calls, so at some point it will hit the stack limit. As with previous
changes to `GVariantType` parsing (commit 7c4e6e9fbe), limit the level
of nesting of containers parsed by g_variant_parse() to something
reasonable. We guarantee 64 levels of nesting, which should be enough
for anyone, and is the same as what we guarantee for types.
oss-fuzz#10286
Signed-off-by: Philip Withnall <withnall@endlessm.com>
This reverts commit 4aba03562b, preserving
the new tests but adjusting them to assert that the old behaviour is
restored.
As expected, there were a few projects which broke because of this.
Unfortunately, in one case the breakage crosses a project boundary:
sysprof ships D-Bus introspection XML, which is consumed by mutter and
passed through gdbus-codegen.
Since sysprof cannot add this annotation without breaking its existing
users, a warning is also not appropriate.
https://gitlab.gnome.org/GNOME/jhbuild/issues/41https://gitlab.gnome.org/GNOME/sysprof/issues/17https://gitlab.gnome.org/GNOME/glib/issues/1726
On closer reading of `man 3 timezone`, it’s actually permissible for
`TZ` to contain an absolute path which points to a tzfile file outside
the system time zone database. This is indeed what happens when building
GLib under Fedora’s toolbox, so relax that check in the tests.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
There are a lot of Unix-like systems which have not implemented the
os-release spec. On such system, we can use POSIX uname function as a
fallback to get basic information of the system.
/etc/os-release is a spec designed for Linux. While other OSes can
implement it, it doesn't make sense to use Linux as the default value
on systems which don't use Linux.
