Package: gnome-screensaver
Version: 3.6.1-2
Severity: grave
Tags: security patch

Dear maintainer,

After upgrading my desktop from wheezy to jessie (w/ GNOME Flashback
mode), I was surprised to find that closing the lid of my laptop
suspended the system, but upon resume the screen was not locked and no
password prompt was needed to actually resume working on my screen.

Suffice to say, I think that's a security issue and thus, release
critical.

I investigated this quite a bit; it looks like with jessie's version,
GNOME doesn't use ConsoleKit anymore, but the alternative codepath for
this, namely handling systemd-login events, has been turned off by
passing --without-systemd to configure, over two years ago, with no
justification in the changelog.

Even with systemd support, though, it seems that in the (very old)
upstream version only Lock events are being processed, not suspend
(PrepareForSleep) ones (like gnome-shell does).  gnome-screensaver is
abandoned upstream, so I assume the API plans changed along the way over
the past two and a half years.

Fortunately, Ubuntu has prepared a patch for this and a) is trivial
enough, b) has been released with several Ubuntu versions and hence is
tested in the wild. While at it, I also ported another couple of Ubuntu
patches that while not strictly needed, help considerably in this use
case (namely, a) adding support for non-systemd Linux systems and b) not
leaking screen contents on resume).

Attached you will find a patch for the package to address this. The
total debdiff is:
  configure.ac           |    2 +-
  src/gs-listener-dbus.c |   33 +++++++++++++++++++++++++++++++--
  src/gs-listener-dbus.h |    1 +
  src/gs-manager.c       |    2 +-
  src/gs-monitor.c       |   16 ++++++++++++++++
  5 files changed, 50 insertions(+), 4 deletions(-)
...and is easily readable and understood, as well as widely tested. I
would definitely recommend including this in jessie.

Best,
Faidon
diff -Nurp gnome-screensaver-3.6.1/debian/changelog gnome-screensaver-3.6.1-suspendlock/debian/changelog
--- gnome-screensaver-3.6.1/debian/changelog	2014-09-11 23:26:14.000000000 +0300
+++ gnome-screensaver-3.6.1-suspendlock/debian/changelog	2014-12-13 13:03:22.112670213 +0200
@@ -1,3 +1,20 @@
+gnome-screensaver (3.6.1-2.1) UNRELEASED; urgency=medium
+
+  * Non-maintainer upload.
+  * Reenable support for locking the screen on suspend.
+    - Build with systemd support by passing --with-systemd=auto to configure
+      and build-depending on libsystemd-login-dev. Use "auto" and a
+      [linux-any] dependency to keep compatibility with non-Linux systems.
+    - 00git_logind_check.patch from Ubuntu/upstream, to make this dependent on
+      just logind, not systemd-as-pid1, as recommended by systemd upstream &
+      Debian systemd maintainers. Drops libsystemd-daemon-dev build-dep.
+    - 31_lock_screen_on_suspend.patch from Ubuntu, to listen for logind's
+      PrepareForSleep signal, similarly to gnome-shell's behavior.
+    - 14_no_fade_on_user_switch.patch from Ubuntu, as to not fade on screen
+      lock. Prevents leaking of the screen contents on resume from suspend.
+
+ -- Faidon Liambotis <parav...@debian.org>  Sat, 13 Dec 2014 11:32:25 +0200
+
 gnome-screensaver (3.6.1-2) unstable; urgency=medium
 
   * Team upload
diff -Nurp gnome-screensaver-3.6.1/debian/control gnome-screensaver-3.6.1-suspendlock/debian/control
--- gnome-screensaver-3.6.1/debian/control	2014-12-13 12:36:01.941262458 +0200
+++ gnome-screensaver-3.6.1-suspendlock/debian/control	2014-12-13 13:02:25.484828745 +0200
@@ -19,8 +19,7 @@ Build-Depends: cdbs,
                libgtk-3-dev (>= 3.0.0),
                libgnome-desktop-3-dev (>= 3.1.91),
                libgnomekbd-dev (>= 2.91.91),
-#               libsystemd-login-dev [linux-any],
-#               libsystemd-daemon-dev [linux-any],
+               libsystemd-login-dev [linux-any],
                libxklavier-dev,
                libx11-dev,
                libxt-dev,
diff -Nurp gnome-screensaver-3.6.1/debian/control.in gnome-screensaver-3.6.1-suspendlock/debian/control.in
--- gnome-screensaver-3.6.1/debian/control.in	2014-09-11 23:21:50.000000000 +0300
+++ gnome-screensaver-3.6.1-suspendlock/debian/control.in	2014-12-13 13:02:17.124852278 +0200
@@ -15,8 +15,7 @@ Build-Depends: cdbs,
                libgtk-3-dev (>= 3.0.0),
                libgnome-desktop-3-dev (>= 3.1.91),
                libgnomekbd-dev (>= 2.91.91),
-#               libsystemd-login-dev [linux-any],
-#               libsystemd-daemon-dev [linux-any],
+               libsystemd-login-dev [linux-any],
                libxklavier-dev,
                libx11-dev,
                libxt-dev,
diff -Nurp gnome-screensaver-3.6.1/debian/patches/00git_logind_check.patch gnome-screensaver-3.6.1-suspendlock/debian/patches/00git_logind_check.patch
--- gnome-screensaver-3.6.1/debian/patches/00git_logind_check.patch	1970-01-01 02:00:00.000000000 +0200
+++ gnome-screensaver-3.6.1-suspendlock/debian/patches/00git_logind_check.patch	2014-12-13 12:34:57.925441694 +0200
@@ -0,0 +1,55 @@
+From fd353244020cf53ce58f4abf69a5ec7893ca1fd1 Mon Sep 17 00:00:00 2001
+From: Martin Pitt <martinp...@gnome.org>
+Date: Thu, 21 Mar 2013 10:24:20 +0000
+Subject: Check for logind, not for systemd
+
+It is possible to build systemd without logind, in which case
+/sys/fs/cgroup/systemd would still exist. Check for /run/systemd/seats instead,
+as recommended by systemd upstream.
+
+For details, see:
+<https://mail.gnome.org/archives/desktop-devel-list/2013-March/msg00092.html>
+
+Drop the now unnecessary linking against libsystemd-daemon.
+
+https://bugzilla.gnome.org/show_bug.cgi?id=696264
+---
+--- a/configure.ac
++++ b/configure.ac
+@@ -601,7 +601,7 @@ AC_ARG_WITH(systemd,
+             [with_systemd=$withval], [with_systemd=auto])
+ 
+ PKG_CHECK_MODULES(SYSTEMD,
+-                  [libsystemd-login libsystemd-daemon],
++                  [libsystemd-login],
+                   [have_systemd=yes], [have_systemd=no])
+ 
+ if test "x$with_systemd" = "xauto" ; then
+--- a/src/gs-listener-dbus.c
++++ b/src/gs-listener-dbus.c
+@@ -25,6 +25,7 @@
+ #include <stdio.h>
+ #include <time.h>
+ #include <string.h>
++#include <unistd.h>
+ 
+ #include <glib/gi18n.h>
+ 
+@@ -33,7 +34,6 @@
+ #include <dbus/dbus-glib-lowlevel.h>
+ 
+ #ifdef WITH_SYSTEMD
+-#include <systemd/sd-daemon.h>
+ #include <systemd/sd-login.h>
+ #endif
+ 
+@@ -1479,7 +1479,8 @@ gs_listener_init (GSListener *listener)
+         listener->priv = GS_LISTENER_GET_PRIVATE (listener);
+ 
+ #ifdef WITH_SYSTEMD
+-        listener->priv->have_systemd = sd_booted () > 0;
++        /* check if logind is running */
++        listener->priv->have_systemd = (access("/run/systemd/seats/", F_OK) >= 0);
+ #endif
+ 
+         gs_listener_dbus_init (listener);
diff -Nurp gnome-screensaver-3.6.1/debian/patches/14_no_fade_on_user_switch.patch gnome-screensaver-3.6.1-suspendlock/debian/patches/14_no_fade_on_user_switch.patch
--- gnome-screensaver-3.6.1/debian/patches/14_no_fade_on_user_switch.patch	1970-01-01 02:00:00.000000000 +0200
+++ gnome-screensaver-3.6.1-suspendlock/debian/patches/14_no_fade_on_user_switch.patch	2014-12-13 12:35:02.573428680 +0200
@@ -0,0 +1,16 @@
+Description: Work around LP: #546578 by disabling the fade effect on screen locking
+Bug-Ubuntu: https://launchpad.net/bugs/546578
+Forwarded: not-needed
+Author: Chris Coulson <chris.coul...@canonical.com>
+
+--- a/src/gs-manager.c
++++ b/src/gs-manager.c
+@@ -1268,7 +1268,7 @@ gs_manager_activate (GSManager *manager)
+         manager->priv->active = TRUE;
+ 
+         /* fade to black and show windows */
+-        do_fade = TRUE;
++        do_fade = FALSE;
+         if (do_fade) {
+                 manager->priv->fading = TRUE;
+                 gs_debug ("fading out");
diff -Nurp gnome-screensaver-3.6.1/debian/patches/31_lock_screen_on_suspend.patch gnome-screensaver-3.6.1-suspendlock/debian/patches/31_lock_screen_on_suspend.patch
--- gnome-screensaver-3.6.1/debian/patches/31_lock_screen_on_suspend.patch	1970-01-01 02:00:00.000000000 +0200
+++ gnome-screensaver-3.6.1-suspendlock/debian/patches/31_lock_screen_on_suspend.patch	2014-12-13 11:59:03.735473141 +0200
@@ -0,0 +1,120 @@
+From f8f9beb6a3bf81240d36bfec43e5db9b102ea91e Mon Sep 17 00:00:00 2001
+From: Martin Pitt <martinp...@gnome.org>
+Date: Wed, 1 May 2013 10:55:49 -0700
+Subject: [PATCH] Lock screen on suspend
+
+Listen for logind's PrepareForSleep signal, and lock the screen (if configured
+to do so). This mirrors what gnome-shell's screensaver does.
+---
+ src/gs-listener-dbus.c | 28 ++++++++++++++++++++++++++++
+ src/gs-listener-dbus.h |  1 +
+ src/gs-monitor.c       | 20 ++++++++++++++++++++
+ 3 files changed, 49 insertions(+)
+
+--- a/src/gs-listener-dbus.c
++++ b/src/gs-listener-dbus.c
+@@ -83,6 +83,7 @@ struct GSListenerPrivate
+ 
+ enum {
+         LOCK,
++        CONFIG_LOCK,
+         QUIT,
+         SIMULATE_USER_ACTIVITY,
+         ACTIVE_CHANGED,
+@@ -872,6 +873,17 @@ listener_dbus_handle_system_message (DBu
+                         }
+ 
+                         return DBUS_HANDLER_RESULT_HANDLED;
++                } else if (dbus_message_is_signal (message, SYSTEMD_LOGIND_INTERFACE, "PrepareForSleep")) {
++                        gboolean active;
++                        if (dbus_message_get_args (message, NULL,
++                                    DBUS_TYPE_BOOLEAN, &active,
++                                    DBUS_TYPE_INVALID) && active) {
++                                gs_debug ("systemd notified that system is about to sleep");
++                                g_signal_emit (listener, signals [CONFIG_LOCK], 0);
++                        } else {
++                                gs_debug ("cannot parse PrepareForSleep");
++                        }
++                        return DBUS_HANDLER_RESULT_HANDLED;
+                 } else if (dbus_message_is_signal (message, DBUS_INTERFACE_PROPERTIES, "PropertiesChanged")) {
+ 
+                         if (_listener_message_path_is_our_session (listener, message)) {
+@@ -1186,6 +1198,16 @@ gs_listener_class_init (GSListenerClass
+                               g_cclosure_marshal_VOID__VOID,
+                               G_TYPE_NONE,
+                               0);
++        signals [CONFIG_LOCK] =
++                g_signal_new ("config-lock",
++                              G_TYPE_FROM_CLASS (object_class),
++                              G_SIGNAL_RUN_LAST,
++                              G_STRUCT_OFFSET (GSListenerClass, config_lock),
++                              NULL,
++                              NULL,
++                              g_cclosure_marshal_VOID__VOID,
++                              G_TYPE_NONE,
++                              0);
+         signals [QUIT] =
+                 g_signal_new ("quit",
+                               G_TYPE_FROM_CLASS (object_class),
+@@ -1371,6 +1393,12 @@ gs_listener_acquire (GSListener *listene
+                                             ",interface='"DBUS_INTERFACE_PROPERTIES"'"
+                                             ",member='PropertiesChanged'",
+                                             NULL);
++                        dbus_bus_add_match (listener->priv->system_connection,
++                                            "type='signal'"
++                                            ",sender='"SYSTEMD_LOGIND_SERVICE"'"
++                                            ",interface='"SYSTEMD_LOGIND_INTERFACE"'"
++                                            ",member='PrepareForSleep'",
++                                            NULL);
+ 
+                         return (res != -1);
+                 }
+--- a/src/gs-listener-dbus.h
++++ b/src/gs-listener-dbus.h
+@@ -45,6 +45,7 @@ typedef struct
+         GObjectClass       parent_class;
+ 
+         void            (* lock)                     (GSListener *listener);
++        void            (* config_lock)              (GSListener *listener);
+         void            (* quit)                     (GSListener *listener);
+         void            (* simulate_user_activity)   (GSListener *listener);
+         gboolean        (* active_changed)           (GSListener *listener,
+--- a/src/gs-monitor.c
++++ b/src/gs-monitor.c
+@@ -210,6 +210,19 @@ listener_lock_cb (GSListener *listener,
+ }
+ 
+ static void
++listener_config_lock_cb (GSListener *listener,
++                         GSMonitor  *monitor)
++{
++        if (! monitor->priv->prefs->lock_disabled) {
++		gs_debug ("Locking the screen suspend");
++		gs_monitor_lock_screen (monitor);
++        } else {
++                gs_debug ("Locking disabled by the administrator");
++        }
++
++}
++
++static void
+ listener_quit_cb (GSListener *listener,
+                   GSMonitor  *monitor)
+ {
+@@ -328,6 +341,7 @@ static void
+ disconnect_listener_signals (GSMonitor *monitor)
+ {
+         g_signal_handlers_disconnect_by_func (monitor->priv->listener, listener_lock_cb, monitor);
++        g_signal_handlers_disconnect_by_func (monitor->priv->listener, listener_config_lock_cb, monitor);
+         g_signal_handlers_disconnect_by_func (monitor->priv->listener, listener_quit_cb, monitor);
+         g_signal_handlers_disconnect_by_func (monitor->priv->listener, listener_active_changed_cb, monitor);
+         g_signal_handlers_disconnect_by_func (monitor->priv->listener, listener_simulate_user_activity_cb, monitor);
+@@ -339,6 +353,8 @@ connect_listener_signals (GSMonitor *mon
+ {
+         g_signal_connect (monitor->priv->listener, "lock",
+                           G_CALLBACK (listener_lock_cb), monitor);
++        g_signal_connect (monitor->priv->listener, "config-lock",
++                          G_CALLBACK (listener_config_lock_cb), monitor);
+         g_signal_connect (monitor->priv->listener, "quit",
+                           G_CALLBACK (listener_quit_cb), monitor);
+         g_signal_connect (monitor->priv->listener, "active-changed",
diff -Nurp gnome-screensaver-3.6.1/debian/patches/series gnome-screensaver-3.6.1-suspendlock/debian/patches/series
--- gnome-screensaver-3.6.1/debian/patches/series	2014-09-11 22:47:40.000000000 +0300
+++ gnome-screensaver-3.6.1-suspendlock/debian/patches/series	2014-12-13 12:34:42.445484818 +0200
@@ -1 +1,4 @@
+00git_logind_check.patch
 01_no_autostart.patch
+14_no_fade_on_user_switch.patch
+31_lock_screen_on_suspend.patch
diff -Nurp gnome-screensaver-3.6.1/debian/rules gnome-screensaver-3.6.1-suspendlock/debian/rules
--- gnome-screensaver-3.6.1/debian/rules	2014-09-11 22:39:16.000000000 +0300
+++ gnome-screensaver-3.6.1-suspendlock/debian/rules	2014-12-13 12:58:23.081507411 +0200
@@ -10,7 +10,7 @@ include /usr/share/gnome-pkg-tools/1/rul
 DEB_CONFIGURE_EXTRA_FLAGS += \
 			--enable-locking \
 			--enable-docbook-docs \
-			--without-systemd
+			--with-systemd=auto
 
 LDFLAGS += -Wl,-z,defs -Wl,-O1 -Wl,--as-needed
 

Reply via email to