gnome-control-center incorrectly claims remote login is off

[Zygmunt Krynicki]

GNOME control center offers a way to disable or enable remote shell (ssh) connections. This functionality is outsourced to /usr/libexec/cc-remote-login-helper which starts and stops the systemd service ssh.service using the code:

      if (!cc_disable_service (SSHD_SERVICE, G_BUS_TYPE_SYSTEM, &error))
      ...

      if (!cc_enable_service (SSHD_SERVICE, G_BUS_TYPE_SYSTEM, &error))

The irony is that ssh.service is socket activated:

zyga@x240:~$ systemctl status ssh.service
● ssh.service - OpenBSD Secure Shell server
     Loaded: loaded (/lib/systemd/system/ssh.service; enabled; preset: enabled)
    Drop-In: /etc/systemd/system/ssh.service.d
             └─00-socket.conf
     Active: active (running) since Tue 2023-10-17 16:40:04 CEST; 21s ago
TriggeredBy: ● ssh.socket
       Docs: man:sshd(8)
             man:sshd_config(5)
    Process: 7055 ExecStartPre=/usr/sbin/sshd -t (code=exited, status=0/SUCCESS)
   Main PID: 7056 (sshd)
      Tasks: 1 (limit: 9305)
     Memory: 1.4M
        CPU: 21ms
     CGroup: /system.slice/ssh.service
             └─7056 "sshd: /usr/sbin/sshd -D [listener] 0 of 10-100 startups"

paź 17 16:40:04 x240 systemd[1]: Starting ssh.service - OpenBSD Secure Shell server...
paź 17 16:40:04 x240 sshd[7056]: Server listening on :: port 22.
paź 17 16:40:04 x240 systemd[1]: Started ssh.service - OpenBSD Secure Shell server.

In effect, it will always activate again whenever someone attempts to connect.

This bug is a security vulnerability, as users may be prone to attacks while thinking remote shell is disabled.

I would suggest to *mask* the service, so that it cannot be socket activated.

ProblemType: Bug
DistroRelease: Ubuntu 23.10
Package: gnome-control-center 1:45.0-1ubuntu2
ProcVersionSignature: Ubuntu 6.5.0-9.9-generic 6.5.3
Uname: Linux 6.5.0-9-generic x86_64
ApportVersion: 2.27.0-0ubuntu5
Architecture: amd64
CasperMD5CheckResult: pass
CurrentDesktop: ubuntu:GNOME
Date: Tue Oct 17 16:36:23 2023
InstallationDate: Installed on 2023-10-17 (0 days ago)
InstallationMedia: Ubuntu 23.10.1 "Mantic Minotaur" - Release amd64 (20231016.1)
ProcEnviron:
 LANG=pl_PL.UTF-8
 PATH=(custom, no user)
 SHELL=/bin/bash
 TERM=xterm-256color
 XDG_RUNTIME_DIR=<set>
SourcePackage: gnome-control-center
UpgradeStatus: No upgrade log present (probably fresh install)

[Marc Deslauriers (mdeslaur)]

Subscribing Sebastien and Jeremy from the Desktop team so they can take a look at the issue.

[Marc Deslauriers (mdeslaur)]

It looks like we switched to openssh using socket-based activation by default in Lunar. Though gnome-control-center on Lunar just hangs when I attempt to access the Sharing tab, so I'm not sure what the status is there.

For Focal, we did ship ssh.socket with openssh and we do document in the README.Debian file how to switch to socket-based activation, so we probably need to fix gnome-control-center there also even if it's not a default configuration.

[Zygmunt Krynicki (zyga)]

I've prepared a crude patch that disables the socket and the service together and tested that locally.

[Zygmunt Krynicki (zyga)]

A follow-up to the previous patch, with adjustments to existing copy-pasted error messages.

[Mark Esler (eslerm)]

Please refer to this issue as CVE-2023-5616.

Thank you for bringing this to our attention @zyga. Would you like to be credited as the discoverer?

[Zygmunt Krynicki (zyga)]

Yes, please. That's always nice.

[Robert Ancell (robert-ancell)]

I've reviewed the two patches and they look like an appropriate solution, thanks!

[Marc Deslauriers (mdeslaur)]

Thanks for the patches, and for the patch review. I will test them next week and will assign a date we can make this CVE public and release updates. Thanks!

[Marc Deslauriers (mdeslaur)]

So I took a look and tested the patch and it doesn't actually fix the issue. In a default installation of Lucid+, the ssh.socket unit is enabled and active, and the ssh.service unit is disabled and inactive.

The code in cc-remote-login.c only checks ssh.service to determine the status of the daemon, so it is still showing ssh as being disabled.

Also, the patch will attempt to enable and start both the socket and the daemon, when only one of them should be enabled at the same time.

The difficulty here is that while by default the socket is the one that is active on Lunar+, and the service is what is enabled by default on jammy and older, the README.Debian has documentation that allows an admin to change which of the two activation methods can be the default.

While we could modify the patch to hardcode socket on lunar+ and service on jammy-, I wonder if there's a better way to handle this.

[Marc Deslauriers (mdeslaur)]

So I think the patch should do the following:

1- To update the switch status, it should check both the socket and the service, if either one of them is both enabled and active, the switch should display as "on"
2- If the switch is turned off, both the socket and the service should be stopped and disabled
3- If the switch is turned on, the service should be stopped and disabled and the socket should be enabled and started.

For Jammy and lower, #3 should do the opposite.

[Alex Murray (alexmurray)]

I agree with mdeslaur so I went ahead and implemented a version of this for gnome-control-center in noble - any reviews would be great. Once we are happy with the approach in noble I will backport to mantic, lunar and jammy, AND will also backport to focal, bionic but will change the behaviour to enable the service not the socket on these releases.

[Robert Ancell (robert-ancell)]

There's a typo in an error message:

      if (!cc_disable_service (SSHD_SERVICE, G_BUS_TYPE_SYSTEM, &error))
        {
          g_critical ("Failed to enable %s: %s", SSHD_SERVICE, error->message);
          return EXIT_FAILURE;
        }

[Robert Ancell (robert-ancell)]

Or the wrong function call?

[Robert Ancell (robert-ancell)]

Some general comments on the patch:

There's some code duplication with two implementations of org.freedesktop.systemd1.Manager.GetUnitFileState - this could be shared in a function.

The two services are checked in series, which makes the callback logic somewhat complicated. You should probably check both services in parallel, and only call set_switch_state when the second result is returned.

[Robert Ancell (robert-ancell)]

Is there the possiblity the change could be on a private branch and reviewed there?

[Alex Murray (alexmurray)]

Thanks for the review Robert - the problem with trying to do both checks in parallel is it makes the management of the callback data more complicated since both will want to share the same structure but each needs to know which systemd unit it is checking, but I will see what I can do. Also I was hesitant to refactor the existing code too much to keep this easier to review but I agree it will probably make the whole thing a bit nicer so will also have a look at that. Thanks.

[Marc Deslauriers (mdeslaur)]

There are two different places in the code where the status is checked, one for the main "sharing" window and one for the small "remote login" window.

The small "remote login" window does the check in cc-remote-login.c by doing all the dbus calls asynchronously with callbacks. This code patch has been addressed by the patch in comment #12.

The main "sharing" window check is done in is_remote_desktop_enabled() in cc-sharing-panel.c by calling cc_is_service_active() in cc-systemd-service.c which does all the dbus calls synchronously. I don't believe that check has been addressed by the patch in comment #12.

I wonder if it's worth having all that code duplicated, and being done asynchronously...it would certainly be easier if we could just have a single synchronous function for both checks, but I wonder if that would have an impact on the widget in the dialog.

In any case, the main "sharing" window code needs to be fixed.

[Robert Ancell (robert-ancell)]

The pattern I tend to use for async glib code in parallel is:

```
struct {
   bool have_result1 // Or an object if can instead check != NULL
   bool have_result2
}

check_result() {
  if (!have_result1 || !have_result2){
     return;
  }

  do_the_thing().
}

result1_cb {
   have_result1 = true
   check_result()
}

result2_cb {
   have_result2 = true
   check_result()
}

do_check() {
   check1(have_result1)
   check2(have_result2)
}
```

[Alex Murray (alexmurray)]

@mdeslaur - I am a bit confused by your comment#18 above - the code in cc-sharing-panel.c is checking for the REMOTE_DESKTOP_SERVICE which is #define'd as "gnome-remote-desktop.service" so I don't think this is relevant since as far as I understand this bug is about the ssh.service vs ssh.socket.

I have also updated the patch to do the checks in parallel as suggested by Robert - this makes things a bit more complicated in handling the lifetime of the callback data and the nature of embedding the context for each within it but hopefully this isn't too ugly.

Let me know your thoughts.

[Marc Deslauriers (mdeslaur)]

@alexmurray you're totally right, I got them confused, apologies for the noise.

[Robert Ancell (robert-ancell)]

Thanks for updating the patch. The parallel code looks like it will function well.

I would think you could reduce the complexity a little by replacing state_ready_callback with sshd_service_state_ready_callback and sshd_socket_state_ready_callback - that way you can remove some of the branches required to handle multiple cases. You could also just pass CallbackData in all cases and reduce the amount of casting used.

You don't really need the CC_/Cc namespacing prefixes for the new code - it's not used outside this module and existing code generally doesn't put these on.

Neither of the above are blockers, just feedback.

[Alex Murray (alexmurray)]

Thanks Robert - so I chose to avoid duplicating the existing callbacks otherwise we would have almost identical versions of each callback for both the socket and service cases which I felt was more ugly than going with the container_of() type approach and reusing the callbacks between both services.

Will remove the CC prefixes though - thanks.

[Alex Murray (alexmurray)]

I've finished preparing backports for the other Ubuntu releases (M,L,J,F) and used the following test procedure to validate each - packages are building in the private ~ubuntu-security PPA - since this is an Ubuntu internal issue I plan to just publish these along with an associated USN tomorrow unless anyone feels differently - let me know.

# get current status
systemctl is-enabled ssh.{service,socket} && systemctl is-active ssh.{service,socket} && echo "g-c-c should show active" || echo "g-c-c should show inactive"
# disable in GUI if enabled then check CLI has right output
gnome-control-center sharing
! systemctl is-enabled ssh.service && ! systemctl is-enabled ssh.socket && echo pass || echo fail
# check can't ssh locally
ssh localhost echo fail || echo pass

# now enable and activate just ssh.service
sudo systemctl disable ssh.{service,socket}
sudo systemctl enable ssh.service
sudo systemctl start ssh.service

ssh localhost echo pass || echo fail

# check is active in GUI
G_MESSAGES_DEBUG=all gnome-control-center sharing | grep 'Setting switch state for ssh.service: 1' && echo pass || echo fail

# then disable in GUI and check status on CLI
gnome-control-center sharing
! systemctl is-enabled ssh.service && ! systemctl is-enabled ssh.socket && echo pass || echo fail
# check can't ssh locally
ssh localhost echo fail || echo pass

# now enable ssh.socket
sudo systemctl disable ssh.{service,socket}
sudo systemctl enable ssh.socket
sudo systemctl start ssh.socket

ssh localhost echo pass || echo fail

# check is active in GUI
G_MESSAGES_DEBUG=all gnome-control-center sharing | grep 'Setting switch state for ssh.socket: 1' && echo pass || echo fail
# then disable in GUI and check status on CLI
gnome-control-center sharing
! systemctl is-enabled ssh.service && ! systemctl is-enabled ssh.socket && echo pass || echo fail
# check can't ssh locally
ssh localhost echo fail || echo pass

# finally enable in the gui and check the correct service/socket is active
gnome-control-center sharing

# reboot so we can check the correct service/socket is active
sudo reboot

series=$(lsb_release -cs)
enabled=socket
disabled=service
case $series in
  xenial|bionic|focal|jammy)
    enabled=service
    disabled=socket
    ;;
esac

systemctl is-enabled ssh.$enabled && systemctl is-active ssh.$enabled && ! systemctl is-enabled ssh.$disabled && ! systemctl is-active ssh.$disabled && echo "pass" || echo "fail"

# finally check can ssh locally
ssh localhost echo pass || echo fail

[Launchpad Janitor (janitor)]

This bug was fixed in the package gnome-control-center - 1:45.0-1ubuntu3.1

---------------
gnome-control-center (1:45.0-1ubuntu3.1) mantic-security; urgency=medium

  * SECURITY UPDATE: SSH remote access status not displayed correctly
    (LP: #2039577)
    - d/p/u/CVE-2023-5616.patch: Check the status of both the sshd service and
      socket since either could be in use. Also when enabling remote access,
      enable this via the socket rather than the service.
    - debian/rules: set SSHD_SOCKET=ssh.socket via CPPFLAGS in debian/rules.
    - CVE-2023-5616

 -- Alex Murray <email address hidden> Fri, 08 Dec 2023 18:43:18 +1030