Bug 1222159 (CVE-2024-5148)

We already have a pending proactive audit in bug 1219093 for this, since I
noticed it during the course of bug 1218922.

There is currently some backlog in the team regarding reviews and other tasks
so it can take a bit longer than usual.

*** Bug 1219093 has been marked as a duplicate of this bug. ***

Is there any rdesktop client implementation running on Linux that actually
works against this implementation in gnome-remote-desktop?

`rdesktop` fails, because g-r-d only supports "FreeRDP_NlaSecurity", which
isn't supported by `rdesktop` except via Kerberos tickets.

`krdc` and `vinagre` fail with

    [RDP] Client did not advertise support for the Graphics Pipeline, closing connection

And remmina gets past this point but still fails with an abstract

    gnome-remote-de[2209]: Unable to check file descriptor, closing connection

Note that I tried the versions present in Leap 15.5 for testing.

(In reply to Matthias Gerstner from comment #7)
> Is there any rdesktop client implementation running on Linux that actually
> works against this implementation in gnome-remote-desktop?
> 

I'd expect 'gnome-connections' to work here

Freerdp clients should work. Either xfreerdp from version 2 and version 3. And the sdl-client from version 3 too. I think on Leap 15.5 there's version 2 and should work.
Remmina should work too. Maybe if it fails there's some build config missing to support the Redirection or Auth, in the RDP plugin (Remmina uses the freerdp library).
The rdp clients from Microsoft should work too.

Doesn't look too good with the client support. New tests show the following
results:

- gnome-connections on Leap15.6 -> alternating between a segmentation fault
  in the client and a connection error.
- gnome-connections on Tumbleweed -> actually gets the farthest of all, but no
  image shows, the transport gets disconnected for some reason.
- remmina on Tumbleweed: Says something about "you requested a h.264 gfx mode
  for the server '%s', but your libfreerdp does not support ..."
- xfreerdp on Tumbleweed tries to authenticate via Kerberos (seems no support
  for ntauth, or it needs to be explicitly configured?)

Anyway, the gnome-connections on Tumbleweed case at least keeps a session
alife in the g-r-d systems daemon for me to test its D-Bus interface. So that
is good enough for the moment.

Hmm, that's weird.

Maybe there's something failing on the server side. 
You can check it with:
sudo grdctl --system status --show-credentials

I've tested the clients again on plain Tumbleweed and I don't get those errors.

Regards the h264 error, maybe on your Tumbleweed there isn't the Open H.264 Codec repo? I think some clients need libopenh264.

With xfreerdp and remmina, only giving as connection parameters: username, password, host; makes the connection work.

Getting no image might mean still that no session has been created, so the dbus process won't work.

When using xfreerdp version 2 I think +gfx-progressive param might be needed.

True I didn't have h264 codecs installed, since I always use pristine
Tumbleweed VMs for testing. After install them Remmina shows the same behaviour
as gnome-connections. Connection seems to succeed, but no screen is visible.
This is the server side log:

gnome-remote-de[1796]: [RDP] Sending server redirection
gnome-remote-desktop-daemon[1796]: [14:35:03:955] [1796:00000b6a] [ERROR][com.freerdp.core.transport] - [transport_read_layer]: BIO_read retries exceeded
gnome-remote-desktop-daemon[1796]: [14:35:03:955] [1796:00000b6a] [ERROR][com.freerdp.core.peer] - [transport_read_layer]: ERRCONNECT_CONNECT_TRANSPORT_FAILED [0x0002000D]
gnome-remote-de[1796]: Unable to check file descriptor, closing connection

> Getting no image might mean still that no session has been created, so the dbus process won't work.

Maybe I still don't fully understand what this system daemon is trying to do.
My understanding is that it should show the display manager for logging in
remotely. Why shouldn't that work?

To help debugging you could add the env G_MESSAGES_DEBUG=all in the systemd unit system services and in the handover desktop file.

What should happen:
1. The system daemon gets a new RDP client.
2. It is started a new headless (GDM) login session with a handover daemon.
3. The system daemon receives "StartHandover" dbus method from that handover daemon.
4. A server redirection happens.
5. The RDP client connects to this new handover daemon and displays the login session.

From your log, it looks like the new session is started, the handover daemon on that session too. It is sent the "StartHandover" to the system daemon which makes a server redirection of the RDP client, but... maybe the RDP client isn't connecting again.

Here I attach a reference log.

Created attachment 874306 [details]
gnome-remote-desktop.logs

The reference log.

Created attachment 874353 [details]
log from connection with gnome-connections resulting in black screen

I still didn't manage to get an actual image. The attached log is from Tumbleweed using gnome-connections. The result is still a black screen with some random pixels mixed in. It's not really important to me to get this working, it does enough for testing the handover parts.

I'm through with the review for now. I could likely spend more weeks on this
but I need to make a cut somewhere. I identified a number of problems. I believe a couple of them are severe enough to justify offering a coordinated disclosure process.

For this reason I'm making this bug private. I will share the full report by tomorrow I hope. Upstream's security contact needs to be involved. I will do that by email when everything is ready.

I just created a private Gitlab issue for GNOME security here:

https://gitlab.gnome.org/Teams/Releng/security/-/issues/129

Please treat all information about this as private until we know whether
upstream wants to establish coordinated disclosure.

Following is the full report that I posted upstream:

------------------------------------------------------------------------------

Hello GNOME security,

I am writing to you 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.

Unauthorized D-Bus Interfaces (#1)
==================================

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.

Local Private Key Leak (#1.a)
-----------------------------

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.

System Credentials Leak (#1.b)
------------------------------

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.

The Socket Connection can be Obtained via TakeClient() (#1.c)
-------------------------------------------------------------

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.

Single Polkit Action for Everything (2#)
========================================

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.

ImportCertificate D-Bus Method DoS Weaknesses and Overly Complex API (3#)
=========================================================================

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.

credentials.ini is Created World-Readable (#4)
==============================================

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.

The system service can be started by anybody in the system (#5)
===============================================================

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.

VNC by Default does not Authenticate, but Only Prompts for Accept/Deny (#10)
============================================================================

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 <some-ip> 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
   buffer `nt_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 by `create_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
==========

[1]: https://gitlab.gnome.org/GNOME/gnome-remote-desktop
[2]: https://en.opensuse.org/openSUSE:Security_disclosure_policy
[3]: https://www.openwall.com/lists/oss-security

@Joan: Discussion in the upstream Gitlab has started. It's a bit difficult
with regards to sharing information. They told me one can only view a private
issue in their Gitlab if they are the _creator_ of the issue. This means they
cannot really CC people to them.

We are supposed to communicate separately, or maybe we can setup a private
email thread, if further people need to be involved?

I've already got the required permissions to watch that issue so not a problem.

I would prefer to follow the discussion on the upstream issues.

Now it's the SUSE Labs conference so it's a bit hard for me to follow all the discussion. Though I'll try.

BTW thank you very much for your full detailed security review.

(In reply to joan.torres@suse.com from comment #20)
> I've already got the required permissions to watch that issue so not a problem.
> 
> I would prefer to follow the discussion on the upstream issues.

Sure, that's the best approach.

> Now it's the SUSE Labs conference so it's a bit hard for me to follow all the discussion. Though I'll try.

Take your time.

> BTW thank you very much for your full detailed security review.

Upstream is pretty active about the report already. The one major problem is
issue #1.a and related, which stays private for now. The fix might take a bit
longer.

One outcome of the discussion already is that VNC should no longer be enabled
by default, and they already published a commit to this end:

    https://gitlab.gnome.org/GNOME/gnome-remote-desktop/-/commit/55ce55afa1d

They recommed to integrate this change into our packaging.

There is no full agreement yet about the sensitivity of the shared credentials
for accessing the g-r-d system daemon to get to the display manager screen.
One of the devs suggests that these are near public anyway. This seems a bit
of a strange notion to me.

Also remaining is a question about how private the private SSL key in the
system daemon context actually is. A developer suggests that it is by protocol
design, that it is shared between all users logging in via the g-r-d system
daemon. That would be an "interesting" design IMHO, as it means that all
users that are allowed to access a system can basically intercept and
manipulate each others communication.

Mitre is not responsive currently and so I tried to retract my request for a
CVE and Gnome upstream pulled a CVE from RedHat instead. They assigned
CVE-2024-5148 for the lack of authentication for parts of the D-Bus interface
(issues #1.a through #1.c). For item #9 upstream does not want to assign a
CVE, although I believe it would be justified.

public in rh bugtzilla
        rh#2282003: https://bugzilla.redhat.com/show_bug.cgi?id=2282003

I asked upstream about the time frame for the reamining private issue, and they quickly published everything by now. A new release has been made. Can you package that so I can re-check the current state?

Hi Matthias.

The only missing fixes are #5 and #9, which will be merged for version 47.

The latest 46.2 is at https://build.opensuse.org/package/show/GNOME:Next/gnome-remote-desktop.

I checked version 46.2 and the fixes seem okay so far. The remaining issues #5
and #9 don't relate to the D-Bus interface, so whitelisting the D-Bus API is
okay now.

Issue #9 is rather unfortunate though, since upstream doesn't want to remove
the insecure CLI invocation style. Depending upon how this ends up in release
47, we could consider adding a patch in our packaging at least, that warns the
user when this insecure invocation style is used.

Whitelisting is in progress.

The whitelisting is through, closing as fixed.