Created attachment 4496 Patch for xflock4 to add custom screensavers support Currently the entry point for locking the screen for ``xfce4-session'' and ``xfce4-power-manager'' is ``xflock4'' script which is shipped with ``xfce4-session'' package. The script checks the list of "known" screensavers and when the first available is found it will be launched. By default its knowledge is limited by the following applications: xscreensaver, gnome-screensaver, xlock, slock. No other are supported. So the only way to add support of user-preferred screensaver is to hack ``xflock4'' contents which could be overwritten during the next update. Please consider the following patch to be reviewed/added. The patch allows to specify a custom screensaver application in XfConf registry by the following path: xfce4-session/screenlock/command Please find the patch below and on Pastebin: http://pastebin.ca/2158914 === xflock4-add-custom-screensaver-support.patch === diff --git scripts/xflock4 scripts/xflock4 index ec4d05d..f9846ce 100644 --- scripts/xflock4 +++ scripts/xflock4 @@ -24,6 +24,12 @@ PATH=/bin:/usr/bin export PATH +# Lock by user specified screen lock application +lock_cmd=$(xfconf-query -c xfce4-session -p /screenlock/command 2>/dev/null) +if [ $? -a ! -z $lock_cmd ]; then + $lock_cmd >/dev/null 2>&1 && exit +fi + # Lock by xscreensaver or gnome-screensaver, if a respective daemon is running for lock_cmd in \ "xscreensaver-command -lock" \ === xflock4-add-custom-screensaver-support.patch ===
(In reply to comment #0) > +# Lock by user specified screen lock application > +lock_cmd=$(xfconf-query -c xfce4-session -p /screenlock/command 2>/dev/null) > +if [ $? -a ! -z $lock_cmd ]; then This is broken, $lock_cmd must be quoted since it undergoes word splitting, also there is no need to use the non-portable -a switch and check $?, finally ! -z is better expressed as -n: if lock_cmd=$(xfconf-query -c xfce4-session -p /screenlock/command 2>/dev/null) && [ -n "${lock_cmd}" ]; then > + $lock_cmd >/dev/null 2>&1 && exit The problem with that is that there will be no consistent behavior with regard to whether xflock4 exits upon locking the screen, tools such as xlock or slock remain in the foreground.
(In reply to comment #1) > (In reply to comment #0) > This is broken, Guido, thanks for the correction. > The problem with that is that there will be no consistent behavior with > regard to whether xflock4 exits upon locking the screen, tools such as xlock > or slock remain in the foreground. Is not it possible to make a contract that any custom screensaver application specified in that way should run on the background? So for some applications which remain in the foreground after start a user will be able to create a simple wrapping script which does so.
(In reply to comment #2) > > The problem with that is that there will be no consistent behavior with > > regard to whether xflock4 exits upon locking the screen, tools such as xlock > > or slock remain in the foreground. > > Is not it possible to make a contract that any custom screensaver > application specified in that way should run on the background? So for some > applications which remain in the foreground after start a user will be able > to create a simple wrapping script which does so. Yes, you would have to make that a requirement that the custom command exits after trying to lock the screen. IMHO that's not very elegant but I'm not a Xfce developer who gets to decide on that.
Couldn't the property value have & in its value, if the preferred command needs manual backgrounding? I am using Xfce 4.10.1, and I can not create a property named /screenlock/command for xfce4-session using the xfce-settings-editor. Anyway, I have proposed xflock4 that supports more alternatives at https://bugzilla.xfce.org/show_bug.cgi?id=3770 One of them, named xautolock, adds support for custom lockers, but it does not have Xfce specific configuration. It needs to be started as a daemon, instead.
See also bug #11324.
*** Bug 11324 has been marked as a duplicate of this bug. ***
What's the progress here? It would be nice to have this patch in the official 4.12 release.
I am against this patch being applied. Xflock4 already needs fixing in that it allows people to launch arbitrary screensavers (e.g. ~/.local/bin/xscreensaver) by tweaking their session environment, which is insecure. The locker could of course have a form of xfconf key to help users choose their screensaver, but all screensavers should be looked up exclusively in /usr, and we should check the actual binaries are owned by root. I'll make a separate report with more details about that. Wrt. the patch itself, why not add the content of the xfconf key to the existing locker lists? And in fact if there are two very specific behaviours, two separate xfconf keys could be used. In any case the script would need to be rewritten to ensure it picks the list with the user-chosen locker first.
(In reply to Steve Dodier-Lazaro from comment #8) > I am against this patch being applied. Xflock4 already needs fixing in that > it allows people to launch arbitrary screensavers (e.g. > ~/.local/bin/xscreensaver) by tweaking their session environment, which is > insecure. > > The locker could of course have a form of xfconf key to help users choose > their screensaver, but all screensavers should be looked up exclusively in > /usr, and we should check the actual binaries are owned by root. I'll make a > separate report with more details about that. Steve, please do so on bug #10217 so that it is all in one place and have a look at comments #20, #21, and #22 (you can ignore the rest) about a sane redesign and reliable locking as proposed by Eric, me and Simon. I'm afraid this is a messy topic with lots of different bug reports where people try to add ever more layers upon a fundamentally flawed design we currently have. > Wrt. the patch itself, why not add the content of the xfconf key to the > existing locker lists? And in fact if there are two very specific > behaviours, two separate xfconf keys could be used. In any case the script > would need to be rewritten to ensure it picks the list with the user-chosen > locker first. No need to, the script just needs to go away, see the proposal in bug #10217.
I just wanted to open a bug about lightdm lock support and was redirected to this bug here (due duplication). lightdm has an own locking mechanism which can be fired up with "dm-tool lock" (provided with lightdm). My xflock4.sh file only contains a oneliner: #!/bin/bash --login # Begin xflock4.sh dm-tool lock exit # End xflock4.sh I would be thankful if dm-took lock could be added bevor or behind "slock" within the original xflock4.sh file before 4.12. Thanks.
(In reply to Steve Dodier-Lazaro from comment #8) > I am against this patch being applied. Xflock4 already needs fixing in that > it allows people to launch arbitrary screensavers (e.g. > ~/.local/bin/xscreensaver) by tweaking their session environment, which is > insecure. True, PATH setting is not relevant there, when you can give an absolute path in the xfconf variable. > The locker could of course have a form of xfconf key to help users choose > their screensaver, but all screensavers should be looked up exclusively in > /usr, and we should check the actual binaries are owned by root. I'll make a > separate report with more details about that. In bug 12282 it requested that locker should be accepted outside /usr, too. How would you check, if the locker command in arbitrary string is owned by root? Note, that the string could be e.g. 'time dash -c "mylocker --myoption"' > Wrt. the patch itself, why not add the content of the xfconf key to the > existing locker lists? And in fact if there are two very specific > behaviours, two separate xfconf keys could be used. In any case the script > would need to be rewritten to ensure it picks the list with the user-chosen > locker first. What do you mean by using two separate xfconf keys?
I think the xfconf variable could be used to select a locker from a list that is maintained by administrator; that way (with additional information given by administrator) system would know, if the selected locker command forks, and could rely on the command being reasonable. Also the possible respective daemon could be started on demand. On the other hand, as for Andrew Sichevoi's concern about custom xflock4 being overwritten by update, at least in Xubuntu Linux I have used the following strategy: I store custom xflock4 in /usr/local/bin which is set in PATH before /usr/bin where the supplied xflock4 is located. (That does not work, if I call xflock4 securely by `command -p xflock4`.)
> I am against this patch being applied. Xflock4 already needs fixing in that it allows people to launch arbitrary screensavers (e.g. ~/.local/bin/xscreensaver) by tweaking their session environment, which is insecure. But as I know, third-party cannot modify anything in user's home directory. To modify files in home directory one must have corresponding authority. Is it really insecure? By the way, is there any progress concerning this enhancement?
It is in current Git, but using a different xfconf variable: http://git.xfce.org/xfce/xfce4-session/tree/scripts/xflock4
Steve Dodier-Lazaro, are you still there? Where is the report you referred to?
So, does it mean that I can put any command string to /general/LockCommand, and this command will be invoked on xflock4 invocation? By the way, do we have something like that for "Switch user"? As I know, it currently looks for gdmflexiserver, but it is only provided by gdm, though other display managers may have their own mechanisms: for example, light-locker has "dm-tool switch-to-greeter" And one final question (sorry for something like offtopic): Do we have something to automatically enable screensaver/screenlocker on session idle/suspend/hibernate? I mean, some screensavers (simple ones like i3lock, slock, etc.) do not have such mechanisms and are only invoked directly by command. It would be great if xfce user could open xfce4-settings-manager and find settings like "Run command when session id idle for N minutes" and "Run command when going/returning from hibernate/suspend"
(In reply to socexema from comment #16) > So, does it mean that I can put any command string to /general/LockCommand, > and this command will be invoked on xflock4 invocation? Yes. > By the way, do we have something like that for "Switch user"? As I know, it > currently looks for gdmflexiserver, but it is only provided by gdm, though > other display managers may have their own mechanisms: for example, > light-locker has "dm-tool switch-to-greeter" Switch User may not work, but you can make a script to do the job and setup a panel launcher to run it. xflock4 should be run successfully before running dm-tool. > And one final question (sorry for something like offtopic): Do we have > something to automatically enable screensaver/screenlocker on session > idle/suspend/hibernate? I mean, some screensavers (simple ones like i3lock, > slock, etc.) do not have such mechanisms and are only invoked directly by > command. It would be great if xfce user could open xfce4-settings-manager > and find settings like "Run command when session id idle for N minutes" and > "Run command when going/returning from hibernate/suspend" Yes we do. It is in the session settings or power manager settings. You can choose, if xflock4 is called or not. IIRC the power manager may call some locker even if xflock4 is not found. Also screensavers may have their own settings.
In power management I found an option to lock screen when system is going for sleep. And what about locker activation on session idle, i.e. when I do not do anything for N minutes?
(In reply to socexema from comment #18) > In power management I found an option to lock screen when system is going > for sleep. And what about locker activation on session idle, i.e. when I do > not do anything for N minutes? Why don't you inspect the settings of your screensaver like I hinted before and continue discussion in a support channel of the operating system you are using?
(In reply to socexema from comment #18) > In power management I found an option to lock screen when system is going > for sleep. And what about locker activation on session idle, i.e. when I do > not do anything for N minutes? Or use Xfce's discussion channels: http://www.xfce.org/community
Of course, sorry for offtopic. Thank you.
https://git.xfce.org/xfce/xfce4-session/commit/?id=e940818853582290af21bf38d73ee26143d500ad https://git.xfce.org/xfce/xfce4-session/commit/?id=495aac78058cd78e2d34505af204e72a1b4f19ac Marking as fixed.