Page MenuHomeLubuntu Development
Create Task
Maniphest T90

Bionic locker should be light-locker
Closed, ResolvedPublic
Actions

Description

See upstream bug, but this should be self-explanatory and easy.

Details

Differential Revisions
D23: Set default lock to light-locker-command

Related Objects
Search...

Event Timeline

wxl triaged this task as Normal priority.Jul 13 2019, 8:17 PM
wxl created this task.
apt-ghetto added a subscriber: apt-ghetto.Jul 14 2019, 5:02 AM

I have started playing around with this and uploaded a package to https://launchpad.net/~apt-ghetto/+archive/ubuntu/ppa-test

Now I have to find out, how to do a SRU.

wxl added a comment.Jul 14 2019, 1:17 PM

See the description of the parent task. There's a link there.

wxl added a comment.Jul 14 2019, 1:23 PM

Also you've got some Travis stuff in the diff you should probably get rid of.

apt-ghetto added a subscriber: tsimonq2.Jul 16 2019, 2:53 PM

I started with the paperwork. So far I have this:

[Summary]

Instead to light-locker, the keyboard shortcut to lock the screen is bound to lxlock. Unfortunately, lxlock is not included in 18.04.0, whereas light-locker is included in every 18.04 point release.
The ability to lock the screen might be a basic security requirement for some users (e.g. at work) and this fix make it easier.

[Test Case]

1. Boot Lubuntu 18.04 (from the older iso lubuntu-18.04-desktop-amd64.iso)
2. Login
3. When the desktop environment is ready, press CTRL-ALT-L
4. The desktop is not locked, although this is expected
5. Install the package with the fix (0.54.3).
6. Press CTRL-ALT-L
7. The desktop session is locked

[Regression Potential]

If the command in the Exec line does not exist, nothing is executed, so the behaviour does not change. Users with lxlock installed, won't notice any change in the behaviour.

[Other Info]

Lubuntu uses another desktop environment from version 18.10 onwards. The only supported version, which contains the patched file is 18.04.

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

[Previous Description]

Lubuntu 18.04 lxsession defines `lxlock` as the default for "lock_manager/command" which lxhotkey calls via `lxsession-default lock` when Ctrl-Alt-L is pressed. This should be changed in /etc/xdg/lxsession/Lubuntu/desktop.conf to `light-locker`.

Of course I was testing it with a new vm and the 18.04.2 iso. And surprisingly everything worked.
I downloaded also the 18.04(.0) iso and set up another vm, where it did not work, which is less surprising. Can anyone tell me, why the isos differ?

Tomorrow I will start with the packaging:

  1. I work on ubuntu/bionic?
  2. Then I play locally with debuild/pbuilder/debdiff etc.
  3. Then I create an arc diff and arc patch and upload it, and after some phab magic, it will be uploaded to launchpad?
  4. And I subscribe ubuntu-sponsors

@wxl @tsimonq2 et al: Please review my work mercilessly

wxl added a comment.Jul 16 2019, 4:54 PM

I would say this is a basic security feature and should function as expected. I wouldn't say that this bug is a security issue since it is still possible to lock the screen, but it just makes it harder. I guess it's a security issue if a user hits the button expecting it to work and walks away without paying attention.

Also: I think it was around this time that we started using light-locker, so clearly, we didn't clean up all the places. It should be assumed that no Lubuntu user would have lxlock.

I would emphasize alternative methods to trigger the lock, i.e. lxsession-default lock. This should eliminate any likelihood of lxhotkey causing problems, though it might open up new possibilities 😆

As I understand it in the documentation, the Test Case section should be for how to trigger the bug. I usually separate that with a new paragraph and describe what should happen when it's fixed. So there's kind of two test cases.

I see little regression potential. I think it's safe to say none.

It may be fair to say in your Other Info section that the Lubuntu Team does not support Xenial, even though it is still supported by Ubuntu.

Answers:

  1. Yes, ubuntu/bionic
  2. Yes, arc diff. After approval, you can arc land, but you'll have to bug one of us to dput to Launchpad.
  3. Only subscribe ~ubuntu-sponsors after the upload happens.

Oh and regarding the difference between the ISOs, you can do

diff -y \
    <(wget --quiet -O - http://cdimage.ubuntu.com/lubuntu/releases/18.04.2/release/lubuntu-18.04-desktop-amd64.manifest) \
    <(wget --quiet -O - http://cdimage.ubuntu.com/lubuntu/releases/18.04.2/release/lubuntu-18.04.2-desktop-amd64.manifest)

Results are below, but do note that strangely lxlock was added. It's possible we might also have a bug in lxsession-default-apps.

{P15}

apt-ghetto added a comment.Jul 20 2019, 10:50 AM

After testing:

[Summary]

Instead to light-locker-command, the keyboard shortcut to lock the screen is bound to lxlock. The problem is, that lxlock is not included in the iso of the first two point releases (18.04 and 18.04.1) and therefore, the shortcut does not work.
The ability to lock the screen is a basic security feature and should work as expected, even with the provided shortcut.


[Test Case]

Steps to reproduce
------------------

1. Boot Lubuntu 18.04 (installed from an older iso, not 18.04.2)
2. Login
3. Delete ~/.config/lxsession/Lubuntu/desktop.conf
4. Logout
5. Login
6. Press CTRL-ALT-L

Expected behaviour
------------------

The desktop session is locked and requires the password to unlock.

Current behaviour
-----------------

Nothing happens.


[Regression Potential]

If the command in the Exec line does not exist, nothing is executed, so the behaviour does not change. Users with lxlock installed, won't notice any change in the behaviour.
lxlock is a shell script, which calls light-locker-command (see: https://github.com/lxde/lxsession/blob/master/lxlock/lxlock#L27), so the regression potential is very, very small.


[Other Info]

Lubuntu uses another desktop environment from version 18.10 onwards. The only version, which contains the patched file is 18.04. The Lubuntu Team does not support Xenial anymore (16.04 has reached its end of life after 3 years of community support).


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

[Previous Description]

Lubuntu 18.04 lxsession defines `lxlock` as the default for "lock_manager/command" which lxhotkey calls via `lxsession-default lock` when Ctrl-Alt-L is pressed. This should be changed in /etc/xdg/lxsession/Lubuntu/desktop.conf to `light-locker`.

Is this ok?

Is there a way to change/delete the user setting?

wxl added a comment.Jul 20 2019, 5:54 PM

Can you share the debdiff for the change just to be perfectly clear?

Is there a way to change/delete the user setting?

Do you mean the definition in ~/.config/lxsession/Lubuntu/desktop.conf?

apt-ghetto added a comment.Jul 21 2019, 4:08 AM
In T90#1459, @wxl wrote:

Can you share the debdiff for the change just to be perfectly clear?

Here it is:

diff -Nru lubuntu-default-settings-0.54.2/debian/changelog lubuntu-default-settings-0.54.3/debian/changelog
--- lubuntu-default-settings-0.54.2/debian/changelog	2019-02-04 06:56:24.000000000 +0100
+++ lubuntu-default-settings-0.54.3/debian/changelog	2019-07-21 10:42:29.000000000 +0200
@@ -1,3 +1,10 @@
+lubuntu-default-settings (0.54.3) bionic; urgency=medium
+
+  * Set the default lock to light-locker-command instead of lxlock
+    (LP: #1812594)
+
+ -- apt-ghetto <apt-ghetto@lubuntu.me>  Sun, 21 Jul 2019 10:42:29 +0200
+
 lubuntu-default-settings (0.54.2) bionic; urgency=medium
 
   * Remove /usr/share/xsessions/QLubuntu.desktop from
diff -Nru lubuntu-default-settings-0.54.2/src/etc/xdg/lxsession/Lubuntu/desktop.conf lubuntu-default-settings-0.54.3/src/etc/xdg/lxsession/Lubuntu/desktop.conf
--- lubuntu-default-settings-0.54.2/src/etc/xdg/lxsession/Lubuntu/desktop.conf	2019-02-04 06:30:55.000000000 +0100
+++ lubuntu-default-settings-0.54.3/src/etc/xdg/lxsession/Lubuntu/desktop.conf	2019-07-21 10:42:29.000000000 +0200
@@ -55,6 +55,8 @@
 
 webcam/command=guvcview
 
+lock_manager/command=light-locker-command -l
+
 [State]
 laptop_mode=unknown
 
diff -Nru lubuntu-default-settings-0.54.2/src/usr/share/applications/lubuntu-screenlock.desktop.in lubuntu-default-settings-0.54.3/src/usr/share/applications/lubuntu-screenlock.desktop.in
--- lubuntu-default-settings-0.54.2/src/usr/share/applications/lubuntu-screenlock.desktop.in	2019-02-04 06:30:55.000000000 +0100
+++ lubuntu-default-settings-0.54.3/src/usr/share/applications/lubuntu-screenlock.desktop.in	2019-07-21 10:42:29.000000000 +0200
@@ -2,6 +2,6 @@
 _Name=ScreenLock
 _Comment=Lock your screen
 Icon=system-lock-screen
-Exec=lxlock
+Exec=light-locker-command -l
 NoDisplay=true
 Type=Application

Is there a way to change/delete the user setting?

Do you mean the definition in ~/.config/lxsession/Lubuntu/desktop.conf?

Yes. After installing the package, the user settings are not changed.

wxl added a comment.Jul 21 2019, 5:21 PM

LGTM.

Re: user settings, yeah I don't have a good solution except perhaps a post-inst script that checks for the lxlock definition and queries the user to change it to light-locker? @tsimonq2 do you think that makes sense?

tsimonq2 added a comment.Jul 24 2019, 9:43 PM

This change is to our default settings, not to the generic package in the archive. We have the right to say that we don't support lxlock, and as long as light-locker is installed (hint: make it a dep of settings), we should just proceed as-is.

tsimonq2 added a comment.Jul 24 2019, 9:44 PM

To clarify about default settings, I don't know if we should modify existing user config.

wxl added a comment.Jul 24 2019, 9:48 PM

The problem is that as it stands, the released configuration (and thus the user configuration) necessarily specifies lxlock, so none of this fixes anything. Until we do that, the fix does nothing.

tsimonq2 added a comment.Jul 25 2019, 12:01 AM

It does something for users installing from scratch.

apt-ghetto added a comment.Jul 25 2019, 3:29 AM

Newest debdiff:

diff -Nru lubuntu-default-settings-0.54.2/debian/changelog lubuntu-default-settings-0.54.3/debian/changelog
--- lubuntu-default-settings-0.54.2/debian/changelog	2019-02-04 06:56:24.000000000 +0100
+++ lubuntu-default-settings-0.54.3/debian/changelog	2019-07-25 10:21:00.000000000 +0200
@@ -1,3 +1,11 @@
+lubuntu-default-settings (0.54.3) bionic; urgency=medium
+
+  * Set the default lock to light-locker-command instead of lxlock
+    (LP: #1812594)
+  * Add light-locker as dependency
+
+ -- apt-ghetto <apt-ghetto@lubuntu.me>  Thu, 25 Jul 2019 10:21:00 +0200
+
 lubuntu-default-settings (0.54.2) bionic; urgency=medium
 
   * Remove /usr/share/xsessions/QLubuntu.desktop from
diff -Nru lubuntu-default-settings-0.54.2/debian/control lubuntu-default-settings-0.54.3/debian/control
--- lubuntu-default-settings-0.54.2/debian/control	2019-02-04 06:30:55.000000000 +0100
+++ lubuntu-default-settings-0.54.3/debian/control	2019-07-25 10:19:34.000000000 +0200
@@ -15,6 +15,7 @@
          lubuntu-icon-theme,
          policykit-desktop-privileges,
          ttf-ubuntu-font-family,
+         light-locker,
          ${misc:Depends}
 Suggests: amixer,
           dmz-cursor-theme,
diff -Nru lubuntu-default-settings-0.54.2/src/etc/xdg/lxsession/Lubuntu/desktop.conf lubuntu-default-settings-0.54.3/src/etc/xdg/lxsession/Lubuntu/desktop.conf
--- lubuntu-default-settings-0.54.2/src/etc/xdg/lxsession/Lubuntu/desktop.conf	2019-02-04 06:30:55.000000000 +0100
+++ lubuntu-default-settings-0.54.3/src/etc/xdg/lxsession/Lubuntu/desktop.conf	2019-07-21 10:57:11.000000000 +0200
@@ -55,6 +55,8 @@
 
 webcam/command=guvcview
 
+lock_manager/command=light-locker-command -l
+
 [State]
 laptop_mode=unknown
 
diff -Nru lubuntu-default-settings-0.54.2/src/usr/share/applications/lubuntu-screenlock.desktop.in lubuntu-default-settings-0.54.3/src/usr/share/applications/lubuntu-screenlock.desktop.in
--- lubuntu-default-settings-0.54.2/src/usr/share/applications/lubuntu-screenlock.desktop.in	2019-02-04 06:30:55.000000000 +0100
+++ lubuntu-default-settings-0.54.3/src/usr/share/applications/lubuntu-screenlock.desktop.in	2019-07-21 10:57:11.000000000 +0200
@@ -2,6 +2,6 @@
 _Name=ScreenLock
 _Comment=Lock your screen
 Icon=system-lock-screen
-Exec=lxlock
+Exec=light-locker-command -l
 NoDisplay=true
 Type=Application
wxl added a comment.Jul 25 2019, 10:08 AM

@tsimonq2 true, but that's kind of a halfway solution. Perhaps just a postinst script with a notice about the change? Gives the user the ability to do what they want.

@apt-ghetto we should add the depend to rLUBUNTUMETAPACKAGING and rSEED not to settings where it isn't technically a dependency.

apt-ghetto added a comment.Jul 28 2019, 3:56 AM

I have updated the Launchpad bug report with the SRU description.

According to the Debian Maintainer Guide, it is discouraged to use maintainer scripts such as postinst (don't bother the user, testing overhead, etc.).

Maybe we should revive the "Lubuntu Development Newsletter"? We could provide shell snippets and explain other important things (such as UMASK=0077 for the initramfs config).

What about lubuntu-meta? rLUBUNTUMETAPACKAGING does not contain the bionic branch. Do I have to create a new SRU Launchpad bug?

wxl added a comment.Jul 28 2019, 8:39 PM

Actually, the the seed already has it. Nevermind that.

I'd follow the rest of the SRU instructions, i.e. subscribe ~ubuntu-sponsors. Normally I'd say attach a debdiff (won't hurt if you want the practice) but @tsimonq2 will upload (I hope!).

Meanwhile, yeah, I guess we can just use the blog. I do think the development newsletter would be a welcome change.

tsimonq2 added a comment.Jul 28 2019, 9:00 PM

I have uploaded the tip of ubuntu/bionic to Ubuntu. It is awaiting review.

wxl added a comment.Jul 30 2019, 11:36 PM

Verified.

wxl closed this task as Resolved.Aug 5 2019, 6:42 PM
This bug was fixed in the package lubuntu-default-settings - 0.54.3

---------------
lubuntu-default-settings (0.54.3) bionic; urgency=medium

  * Set the default lock to light-locker-command instead of lxlock
    (LP: #1812594)

 -- apt-ghetto <apt-ghetto@lubuntu.me> Thu, 25 Jul 2019 10:21:00 +0200