gnome-remote-desktop: multiple security issues and recommendations
Hello GNOME security,
this issue is about the gnome-remote-desktop project 1. I am a member of the SUSE security team and I have conducted a source code review of gnome-remote-desktop, because a D-Bus system service has been added to it with GNOME version 46. In the course of this review I found security issues and collected recommendations that I am reporting to you privately this way. Please find the details below.
Coordinated Disclosure
We offer coordinated disclosure based on our disclosure policy 2. Please answer the following questions in this regard:
-
Do you want to perform coordinated disclosure? If yes, then please communicate your desired coordinated release date (CRD) for publishing the security issues and any patches/updates. We offer to keep the issues private for up to 90 days (resulting in publication latest on 2024-07-18), but prefer to maintain shorter time frames of two to four weeks, if possible.
If you don't want to perform coordinated disclosure, then we will publish all information we have on the oss-security mailing list 3 and in our bug tracker right away.
-
Do you acknowledge the finding? If so, will you obtain CVE identifiers for the relevant issues? If yes, then please share the CVEs with us once they are assigned. Alternatively we offer to request CVEs from the Mitre organization for you.
Design Overview
gnome-remote-desktop offers access to the graphics system either via the VNC or the RDP (Microsoft remote desktop) network protocol. Before version 46, gnome-remote-desktop was only used in the context of existing graphical sessions. Starting with version 46, one can also configure a system daemon, that allows to connect to the GDM display manager, allowing to create new graphical sessions remotely.
The system daemon runs as a dedicated "gnome-remote-desktop" user. It provides a D-Bus interface on the D-Bus system bus. The daemon also interacts with a newly introduced D-Bus interface provided by GDM, to create remote displays.
Review Motivation and Scope
D-Bus system services require a review by the SUSE security team, before they can be added to openSUSE distributions and derived products. With the addition of the system daemon, a review of gnome-remote-desktop has become necessary, before adding it to openSUSE Tumbleweed in the context of the larger GNOME 46 release.
The review was mainly concerned with the newly introduced system level gnome-remote-desktop daemon. The focus was furthermore on the RDP protocol, which is the default and preferred over the VNC protocol.
Since the codebase of gnome-remote-desktop is rather large, I further tried to limit the review to the security of the D-Bus methods, the Polkit authentication and parts of the network processing. I did not look closely into the freerdp library, which is used by gnome-remote-desktop for processing the majority of the RDP protocol.
#1
)
Unauthorized D-Bus Interfaces (Only the "/org/gnome/RemoteDesktop/Rdp/Server" D-Bus interface is
protected by Polkit. auth_admin
authorization is required on this
interface for all methods. The other two interfaces "Dispatcher" and
"Handover" are not authorized and are accessible to all local users in
the system. This leads to a number of local security issues described in
the following sub sections.
#1.a
)
Local Private Key Leak (The system daemon keeps public SSL certificates and their corresponding private keys in "/var/lib/gnome-remote-desktop/.local/share/gnome-remote-desktop/certificates". Access to the service's home directory in "/var/lib/gnome-remote-desktop" is restricted to the service user "gnome-remote-desktop", mode 0700.
Through the "/org/gnome/RemoteDesktop/Rdp/Handovers" D-Bus interface any
local user can intercept the private SSL key, though. The private key is
returned from the StartHandover
D-Bus function. When a remote desktop
client connects to the system daemon, then there is a rather long time
window, during which a local user (even nobody
) can call this method on
the created session object. This is an example call to achieve this:
gdbus call -y -d org.gnome.RemoteDesktop -o /org/gnome/RemoteDesktop/Rdp/Handovers/sessionc11 \
-m org.gnome.RemoteDesktop.Rdp.Handover.StartHandover someuser somepass
The username and password parameters are not important here, they will only be forwarded to the connecting client. Doing this, as another effect, also serves as a DoS, because the proper connection handover will be prevented.
A local attacker does not necessarily have to wait for somebody to connect to the system daemon, it can connect on its own via localhost, to achieve the same result. Valid credentials for RDP authentication are necessary to get to the handover stage, however.
The impact of this problem is a local information leak and local DoS. The information leak means that the integrity and privacy of RDP connections on the system are compromised. Attached to this issue is also a simple Python script that allows to reproduce the issue.
#1.b
)
System Credentials Leak (If an RDP connection uses system credentials (see struct member
GrdRemoteClient.use_system_credentials
), then a local attacker with
low privileges can obtain these credentials in cleartext in a similar
fashion to issue #1.a
, by calling the unauthenticated
GetSystemCredentials()
D-Bus method of the Handover interface.
Using these system credentials, the attacker will be able to connect to the display manager via RDP. This should not directly grant access to a session, since a login on display manager level still has to happen. An exception would be if things like automatic login are enabled, which I don't know if they apply to remote connections.
#1.c
)
The Socket Connection can be Obtained via TakeClient() (The equally unauthenticated D-Bus method Handover.TakeClient()
allows
any local user in the system to obtain the file descriptor pertaining to
the RDP client that is in handover state. This could allow a local user
to DoS the RDP connection or to setup a crafted RDP session.
Obtaining the socket via this call only works in certain system daemon
states, most notably it seems the StartHandover()
needs to have been
performed for this to succeed. I did not fully investigate what the
exact preconditions are. If the preconditions are not met then this
call triggers a series of (non-fatal) assertions, which should also not
be reachable this way, I believe:
gnome-remote-de[31380]: g_socket_connection_get_socket: assertion 'G_IS_SOCKET_CONNECTION (connection)' failed
gnome-remote-de[31380]: g_socket_get_fd: assertion 'G_IS_SOCKET (socket)' failed
gnome-remote-de[31380]: g_unix_fd_list_append: assertion 'fd >= 0' failed
Dispatcher Interface seems Safe
The only method existing in the Dispatcher interface, RequestHandover
,
only succeeds if the caller's session has a connection waiting for
handover. It should be okay that this interface is not authorized in its
current form, because the sessions are managed by the privileged systemd
daemon and cannot be crafted.
Suggested Fix
The Handover D-Bus interface needs to be protected either by Polkit or via D-Bus configuration. If only specific static users need to call these D-Bus methods, then the D-Bus configuration is the correct spot.
The unauthenticated handover methods seem to be designed to be called by
the --handover
personality of the gnome-remote-desktop daemon, and
this instance likely should run as the dynamic session user that
authenticates in GDM. If this is correct, then neither D-Bus nor Polkit
offer the necessary features to restrict access to the D-Bus API to
exactly this user. A possibility to fix this could be to use the same
approach as in the Dispatcher interface, to only grant access to a
client that originates from the session that has the respective handover
pending.
Even if only a legitimate user has access to the Handover D-Bus API, there are still problems with the API design. Nobody except the gnome-remote-desktop system daemon service user should ever have access to the private server key and the system credentials. Why these are passed into the handover context is not fully clear to me. A larger redesign might be necessary to address these overarching issues.
2
)
Single Polkit Action for Everything (The bits of the D-Bus interface that are authenticated all use the same
Polkit action org.gnome.remotedesktop.configure-system-daemon
. Most of
the D-Bus methods are indeed for configuring things. The
GetCredentials()
method could be considered different, though.
Furthermore the grdctl
utility also uses this action, if it is
supposed to act on the system daemon (by passing the --system
switch).
The action also implies the right to modify systemd units. This is only
needed by grdctl
to enable or disable the system daemon systemd unit,
though. For the D-Bus interface these implied permissions are never
needed and are therefore excess privileges most of the time.
Suggested Fix
A more fine grained approach with multiple Polkit actions would allow to avoid these excess privileges and also would allow Admins to better control the authentication settings for individual actions. For end users, more fine grained actions would result in better authentication messages, that explain what exactly is about to happen.
3#
)
ImportCertificate D-Bus Method DoS Weaknesses and Overly Complex API (This D-Bus method requires auth_admin
Polkit authentication, so it is
not generally accessible by default. Polkit allows to lower
authentication requirements, though. Apart from this, any D-Bus method
should be implemented in a robust way. This is not the case for the
"ImportCertificate()" method.
The method expects two pairs of (filename, file descriptor) both for the public SSL certificate and the private key. Each file descriptor can refer to anything and the D-Bus service simply copies the data from the file descriptor to a file beneath "/var/lib/gnome-remote-desktop". If a caller passes a file descriptor for "/dev/zero", then the service will fill up the file system.
Since the call allows to specify a custom filename, the service needs to worry about problematic paths containing path components like "/" and "..". It succeeds mostly with this, but there is still an error condition that can be triggered by passing "." as a path. The service will then try to open the path "/var/lib/gnome-remote-desktop/.local/share/gnome-remote-desktop/certificates" as a file, which results in an EISDIR error. This has no security relevance, but is still kind of unexpected behaviour.
Supporting custom filenames also means that an infinite amount of files can be placed in the certificates directory, including hidden files or files containing unexpected characters like wildcards, binary data, files starting with "--" (possibly injecting command line options) etc. This approach also add the burden of managing the lifetime of multiple installed certificates, but this doesn't even happen. Files are never removed again from the certificates directory. Even more so, if an existing filename is to be overwritten, then backups of the old files are created. But I don't think any admin will be aware of that fact, even if a certificate would be accidentally overwritten.
Furthermore, if the function cannot fully complete the request (e.g. writing the certificate succeeded, but writing the private key failed), then the partially imported data is not removed again. The function can also be tricked into generating an invalid configuration by specifying the same filename for both certificate and key. Only the private key will then remain under this name, but gnome-remote-desktop will try to use the same path also for the certificate.
Suggested Fix
Allowing custom filenames in this API just adds complexity that seems not to offer much in return. If admins want to manage multiple sets of certificates, then they can do this in their own context. Only one set of certificate and key will ever be active in the system daemon anyway. So I would simply drop the filename support and use a fixed name for both certificate and private key.
The file descriptor passed here should be checked for its type (via
fstat()
) and be rejected if it is not a regular file. Then a maximum
file size should be enforced to prevent the file system space being
depleted. No certificate / key combination should be larger than a few
kilobytes. Once the file type is restricted to regular files, there
should also be no big risk of reads on the file descriptor to be
blocking. For full security coverage one could also add a timeout
handling, to make sure the call completes withing a reasonable amount of
time.
#4
)
credentials.ini is Created World-Readable (The file
"/var/lib/gnome-remote-desktop/.local/share/gnome-remote-desktop/credentials.ini",
which contains the cleartext authentication credentials, is saved with
the world readable bit set. This happens in function
grd_credentials_file_new()
. This is not a problem in the context of
the system daemon, since its home directory, "/var/lib/gnome-remote-desktop",
has mode 0700. Still the file itself should get proper protection bits.
This is also inconsistent with respect to the certificate and private
key, which do have mode 0700, in contrast.
Suggested Fix
Fixing this should be straight forward simply by applying the proper 0600 permissions to the file when creating it.
#5
)
The system service can be started by anybody in the system (Independently of the enable/disable and start/stop status of the gnome-remote-desktop systemd service, the system daemon can be started by anybody in the system via the D-Bus autostart logic configured in "/usr/share/dbus-1/system-services/org.gnome.RemoteDesktop.service". Introspecting the service on D-Bus level is enough to trigger the start of the service.
If the service has already been configured once by an admin, then this means that the system service will even go live, listening on the network. Only the default firewall settings might prevent it becoming actually accessible on the network.
Suggested Fix
For a sensitive network service such as this I would consider not to configure the D-Bus autostart, but to require an explicit admin decision to start or enable the service on systemd level.
find_cr_lf()
Suffers from a one Byte Overread (#6
)
This function processes untrusted pre-authentication RDP protocol network data (the routing token) and looks for a terminating \r\n sequence. The size calculation in the function's for loop is wrong: If the final byte of the buffer is 0x0D, then the logic will access the next byte out of bounds. This buffer is not non-null terminated.
The impact should be negligible in most cases, but the issue should still be addressed. This is the output of Valgrind I obtained after sending a crafted packet to the daemon:
==31119== Invalid read of size 1
==31119== at 0x15A1EF: UnknownInlinedFun (grd-rdp-routing-token.c:65)
==31119== by 0x15A1EF: UnknownInlinedFun (grd-rdp-routing-token.c:159)
==31119== by 0x15A1EF: UnknownInlinedFun (grd-rdp-routing-token.c:239)
==31119== by 0x15A1EF: peek_routing_token_in_thread
(grd-rdp-routing-token.c:281)
<snip>
Suggested Fix
The fix should be straight forward by correcting the maximum search index for the terminator.
grd_rdp_sam_create_sam_file()
Uses a Racy System Call Sequence to Remove the O_CLOEXEC
flag (#7
)
This function creates a temporary file using mkstemp()
. In a second
step the O_CLOEXEC
flag is removed from the resulting file descriptor
via fcntl()
. If another thread calls execve()
during this time
window, then this file descriptor will be inherited to other processes
unintentionally.
Suggested Fix
On a number of systems, including Linux with glibc, mkostemp()
can be
used to get a file descriptor with O_CLOEXEC
set right away, to avoid
this race condition.
on_incoming_redirected_connection()
Outputs Untrusted String via g_debug()
(#8
)
The routing_token_str
is data obtained from an untrusted RDP client
before authentication is performed. This is the same data that is
processed by find_cr_lf()
mentioned in item #7
. Luckily the network
protocol parsing code in peek_routing_token()
at least ensures that
this string is always null terminated, otherwise this would allow for an
arbitrary unauthenticated remote DoS against the RDP daemon. g_debug()
generates the string to be logged, even if the log level DEBUG is not
active. Thus formatting errors will always trigger, independently of the
logging configuration.
If the DEBUG log level is active, then the output of the untrusted data allows an unauthenticated remote attacker to perform log spoofing. The string can be roughly 200 bytes long (limited by the protocol headers) and can contain arbitrary data (except for the "\r\n" terminator sequence). Thus it can also contain terminal control characters that allow to overwrite previous log lines, or to create a larger number of seemingly authentic log lines.
The function later on converts the untrusted data into an unsigned long integer. The resulting integer is only used for looking up an existing RDP session. Therefore no further attack surface via crafted data should exist.
Suggested Fix
The output of the untrusted data should either be removed completely, or the routing token should be verified both in content and length, to prevent unexpected things happening on output.
grdctl
Utility Accepts Cleartext Password on the Command Line (#9
)
The text based grdctl
configuration utility which is used for both,
system and session context RDP setups, accepts cleartext passwords in
the following invocation styles:
grdctl [--system] rdp set-credentials <username> <password>
grdctl [--system] vnc set-password <username> <password>
This means that the cleartext password will leak via the /proc file
system and will be visible in the process task list via ps
, when
configured this way. Other users can thus get access to the
authentication data.
Suggested Fix
Passwords should either be queried from stdin, passed in files or via environment variables to prevent such kinds of local information leaks.
#10
)
VNC by Default does not Authenticate, but Only Prompts for Accept/Deny (VNC is hidden away from the graphical user interface by now, but by
using grdctl
it can still be enabled. I tested this only in session
context, not in the system daemon context, since I did not manage to
enable VNC for the system daemon using grdctl
.
The default for the VNC protocol is to prompt only, when a client connects, and to limit access to view-only (no control over input devices). The resulting process is that, on the server side, the user is presented with a popup dialog like "Host wants to access your screen. Allow/Deny?". This workflow offers no client verification at all, in particular a man-in-the-middle attack or source IP spoofing can happen.
The VNC protocol is not a secure protocol by default, but this prompt-only approach in gnome-remote-desktop removes all chances of security, by design.
Suggested Fix
If proper trust cannot be established via the VNC protocol, then a prompt-only mode should not be offered, and password authentication made mandatory.
Miscellaneous
Following are various bits that I observed while digging through the codebase that I find worth mentioning.
-
The D-Bus method "Rdp.Server.GetCredentials()" allows to get the cleartext credentials for the system daemon (after Polkit admin authentication). Passing cleartext passwords around this way, even after authorization, should be well considered and it is unclear what the purpose of this is.
-
create_sam_string()
contains a questionable array size calculation:sam_string = g_malloc0 ((username_length + 3 + 32 + 3 + 1 + 1) * sizeof (char));
There is no documentation what this is about. The freerdp library function
NTOWFv1A()
that is called in this function, receives a buffernt_hash
, without knowing about the available buffer space. It seems this is "by contract", but pretty dangerous. Furthermore the return value of the function is not checked bycreate_sam_string()
. This and a few other looks into the freerdp library raise questions about its code and API quality.I suggest to add comments about the purpose of this and to evaluate the return code of the library function.
-
The
start_rdp_server_when_ready()
function has a very strange semantic:start_rdp_server_when_ready (GrdDaemon *daemon, gboolean should_start_when_ready)
Basically this says
start_when_ready(or_dont)
. I believe this is the result of some unfortunate naming of the function that has a different purpose than what it sounds at first.I suggest to rename this to something more fitting.
-
The
peek_routing_token_in_thread()
function parses the RDP protocol outside of the freerdp library, to extract the routing token. This is pretty complex and dangerous code. Especially bad is that the extracted untrusted routing token is passed through the codebase via various indirections (callbacks, signals). The boundary where trusted and untrusted data is processed is not well established.The untrusted data should be clearly marked and ideally all code that deals with it should be grouped in a dedicated compilation unit that only deals with untrusted data like this.
-
A renderer thread with an optional NVidia CUDA context is setup for every connecting client, without even having authorized the client. Also a layout manager thread is created. Overall this accounts for some five threads for each unauthenticated session. Luckily the daemon does not perform parallel incoming connection processing, otherwise this would pose a simple remote DoS amplification vector. Still I wonder if it is a good idea to start so many threads, a renderer and hardware acceleration before a client is even authorized for anything.
I suggest to only acquire heavy resources after a client successfully authenticated.
CVE Assignments
Items #1.a
through #1.c
are CVE relevant. A single CVE could be issued
grouping these as "unauthenticated D-Bus API" issues. Item #9
also
justifies a CVE since this is a very tangible local information leak.
While they formally would qualify for a CVE, I would not assign any for
items #6
and #8
, because their impact is negligible in default
configurations.
Regarding the fixes for the reported issues we are available to review them, before publication, if you intend to perform coordinated disclosure.
Review Summary
Even considering the concrete findings from this report fixed, my trust into the gnome-remote-desktop codebase is low. The current design for the handover feels too complex. Implementation wise I don't see why a sensitive network service is still developed with C89 level code these days anymore. The codebase is really hard to follow and has nearly no comments or documentation. Function names like these don't help either:
grd_rdp_network_autodetection_get_bw_measure_stop_event_handle()
The only thing that reduces the burden a bit are the automatic memory
management helpers like g_autofree
. gnome-remote-desktop consists of
40,000 lines of C code though, and this doesn't even include the actual
parsing of the protocol. The gobject model with its many indirections and
the specific coding style found in this project represents a high bar
for a third party to comprehend what is going on.
As already mentioned above, there is no clear separation of trusted and untrusted data. In the system daemon context there are at least four different security domains: The GDM domain, the gnome-remote-desktop domain, untrusted remote data from unauthenticated clients and finally trusted remote data from authenticated clients. Without documentation and a clear program structure that clearly separate these security domains, errors can easily slip in.
From what I have seen of the freerdp library I don't have the best impression of it either. It seems remote desktop applications and their protocols are generally a problematic area that justify looking more closely into them.
Given all this there might well linger further issues in the design and implementation of gnome-remote-desktop. Should you decide to improve this codebase in the future, then we would be happy to do a follow-up review, or discuss possible options for improvement.
End User Experience
The end user experience of gnome-remote-desktop matches the complexity found in the codebase. Only a few clients proved to be compatible with it, like Remmina and gnome-connections. But even with them I never managed to actually get a visible screen, only black with some random colored pixels sprinkled in, for reasons that I did not investigate fully. What I got was enough to create a connection on network level, for testing the D-Bus security.
An older version of Remmina even crashed with a segmentation fault
trying to connect. On openSUSE the H.264 patent encumbered codecs first
have to be installed, for being able to connect. Other RDP clients like
rdesktop
and xfreerdp
do not support the NTLM authentication used in
gnome-remote-desktop and fall back to kerberos authentication against a
domain controller instead. The NTLM password authentication method,
which is based on MD digests, is a security worry on its own, by the
way.
References
- gnome remote desktop project
- SUSE disclosure policy
- oss-security mailing list[wait_for_grd_priv_key.py]
- wait_for_grd_priv_key.py