Screensaver config #808
Comments
|
Thats because xscreensaver is the only screensaver Lumina can use right now. |
|
If you remove xscreensaver (using pkg remove -f on FreeBSD, because it's currently a dependency of the port), then lock the screen via the "Leave" menu, the alternate screensaver starts and functions perfectly. It also starts on its own after sufficient idle time. Even if xscreensaver is installed and running, the other screensaver still starts first. There are no other screensaver apps installed as far as I can tell, so I thought this was part of Lumina. Maybe part of Xorg? |
|
xscreensaver was technically never a "hard requirement" by lumina... It was a soft requirement because virtually no one wants a system without a screensaver. |
|
libXScrnSaver is a dependency of the x11/xorg-libraries port, so it's basic to every FreeBSD desktop installation. |
|
Here is what I see if I ssh into a machine with the screensaver running: |
|
The planned removal of xscreensaver was due to Kens plans for the Native Lumina Window Manager. We were going to have our own screensaver that was baked right into the desktop session, to eliminate the security issues of it being a separate process that's running. Keep in mind that the move from 1.6.0 to 1.6.1 included about a year and a half of small changes in the master branch after Jan 31, 2020 when 1.6.0 was released. I think the xscreensaver one was something that happened in that time. During those 20 months, Ken and I were mostly the only ones using the updated code in master, since all the distros were operating off the 1.6.0 release tag. |
|
No worries, just trying to help home in on the cause. I'm very happy to see development resumed and will do my best to provide detailed information on the issues I encounter. |
|
I see there's some screensaver code in the lumina sources: Might there be a simple way to disable the new screensaver until it's fully integrated? We could just patch the FreeBSD port and keep the xscreensaver dependency for now if a new release is not coming soon. |
|
I see a bunch of commits by Ken in April 2020, after the 1.6.0 release. The first couple of them: The question now is how to configure it. There does not appear to be an interface in Settings. |
|
That is the screensaver for the unified desktop (aka Lumina 2.0 Window
Manager).
I spoke with Ken about it the other day and and he mentioned that
screensaver should not be active right now, not only because the Window
Manager is not complete, but also because it's not exposed to lumina-config.
So I'm going to make sure its properly disabled in the next version,
because it shouldn't be enabled right now.
…On Tue, Feb 22, 2022 at 8:14 PM Jason Bacon ***@***.***> wrote:
I see a bunch of commits by Ken in April 2020, after the 1.6.0 release.
The first couple of them:
commit bf545b9
Author: Ken Moore ***@***.***>
Date: Fri Apr 24 17:53:28 2020 -0400
Initial merge of the screensaver into the main lumina-desktop builds
commit 297bf0a
Author: Ken Moore ***@***.***>
Date: Fri Apr 24 17:50:05 2020 -0400
Disable xscreensaver from auto-start.
Enable lumina-checkpass
The question now is how to configure it. There does not appear to be an
interface in Settings.
The default screensaver uses 400% CPU and I would really like to change it
to blank screen only and put the monitor to sleep if possible.
Or just disable it and use xscreensaver for now.
—
Reply to this email directly, view it on GitHub
<#808 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABDQJJ447X5H66RFXTFXL3TU4QYGZANCNFSM5N53SWLQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you commented.Message ID:
***@***.***>
|
|
Sounds good. Can you provide some guidance on how we might patch it out in the current version? We can just hack the FreeBSD port until the next release. |
|
Ken mentioned to me that he thought he had it disabled behind the lumina2
build flag. And from what I can tell in the repo... it *IS* disabled.
The source for that screensaver is within the unified desktop
folder: https://github.com/lumina-desktop/lumina/tree/master/src-qt5/core/lumina-desktop-unified/src-screensaver and that
folder is disabled in https://github.com/lumina-desktop/lumina/blob/master/src-qt5/core/core.pro#L15
…On Tue, Feb 22, 2022 at 8:25 PM Jason Bacon ***@***.***> wrote:
Sounds good. Can you provide some guidance on how we might patch it out in
the current version? We can just hack the FreeBSD port until the next
release.
—
Reply to this email directly, view it on GitHub
<#808 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABDQJJZSXS444Y6XSVX5WULU4QZP5ANCNFSM5N53SWLQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you commented.Message ID:
***@***.***>
|
|
So, there appears to be an identical copy under lumina-desktop: I'll first try commenting out the "include(src-screensaver/screensaver.pri)" in src-qt5/core/lumina-desktop/lumina-desktop.pro and if that works, give it a good testing period. |
|
Not so simple... |
|
LScreenSaver needs lumina-checkpass to work, and it doesn't. You can unlock the desktop by simply hitting enter... which doesn't make for a very good screensaver lock. |
|
Oh yeah, I see that it ignores the password now. ;-) If rolling back to xscreensaver is easier, that would be fine as well. I see some of xscreensaver code commented out in 1.6.2, so maybe that just needs to be restored along with disabling LScreensaver. |
|
FYI, I took a crack at rolling back to xscreensaver and failed. It appears that the xscreensaver code was commented out in some places and completely removed in others. I think we would have to review past commits to see what was removed and there could still be other issues. So based on my limited understanding of the code, I'd be inclined to move forward with LScreensaver. Be reminded that I don't know the code well at all, so my impression that moving forward will be easier could be wrong. Even so, I think it would be a better use of precious man-hours. |
|
I think the only urgent problem in terms of moving forward with LScreensaver is password validation. If we have a working screensaver that actually requires a password to unlock the console, then at least it's usable and secure. Then there would be little reason to consider rolling back to xscreensaver. |
|
The code for validating passwords to unlock the screen was already there, but disabled. It just needed a few fixes to get it working. I also cleaned up a bit to improve readability and eliminate a dangerous atoi() call. I commented out the chmod 4555 for lumina-checkpass since it wasn't working, and this needs to be done via pkg-plist in FreeBSD ports anyway. |
|
If memory serves, the problem was with the PAM implementation. I think
that was the last bit of code that Ken worked on. And while I dont
remember the specifics I do remember him saying that it wasn't working
properly.
I was hoping to get some time this past weekend to work on things, but a
family emergency came up. Hopefully this weekend I can get some time. I'll
try your patch and see what results I get.
…On Thu, Mar 3, 2022 at 11:22 AM Jason Bacon ***@***.***> wrote:
The code for validating passwords to unlock the screen was already there,
but disabled. It just needed a few fixes to get it working. I also cleaned
up a bit to improve readability and eliminate a dangerous atoi() call. I
commented out the chmod 4555 for lumina-checkpass since it wasn't working,
and this needs to be done via pkg-plist in FreeBSD ports anyway.
--- core.pro.orig 2022-03-03 00:56:25 UTC
+++ core.pro
@@ -10,8 +10,8 @@ SUBDIRS+= lumina-desktop \
lumina-info \
lumina-pingcursor \
$${PWD}/../../icon-theme \
- lumina-theme-engine
-# lumina-checkpass
+ lumina-theme-engine \
+ lumina-checkpass
# lumina-desktop-unified
#Also install any special menu scripts
--- lumina-checkpass/lumina-checkpass.pro.orig 2021-12-26 02:33:45 UTC
+++ lumina-checkpass/lumina-checkpass.pro
@@ -13,6 +13,7 @@ LIBS += -lpam
SOURCES += main.c
perms.path = $$DESTDIR$${PREFIX}/sbin
-perms.extra = "chmod 4555 $$DESTDIR$${PREFIX}/sbin/lumina-checkpass"
+# FIXME: This does not work: DESTDIR is blank
+# perms.extra = "chmod 4555 $$DESTDIR$${PREFIX}/sbin/lumina-checkpass"
INSTALLS += target perms
--- lumina-checkpass/main.c.orig 2022-03-03 15:51:53 UTC
+++ lumina-checkpass/main.c
@@ -30,7 +30,7 @@ static int PAM_conv(
struct pam_response **resp,
void *ctx)
{
- return 0;
+ return PAM_SUCCESS;
}
//-----
@@ -42,6 +42,7 @@ void showUsage(){
puts(" lumina-checkpass -fd <file descriptor>");
puts(" lumina-checkpass -f <file path>");
puts("Returns: 0 for a valid password, 1 for invalid");
+ exit(1); // FIXME: Switch to EX_USAGE, or do callers depend on 1?
}
int main(int argc, char** argv){
@@ -49,13 +50,20 @@ int main(int argc, char** argv){
if(argc!=3){
//Invalid inputs - show the help text
showUsage();
- return 1;
}
- char*pass = 0;
+ char *pass = NULL;
if(argc==3 && 0==strcmp(argv[1],"-fd") ){
+ // This replaces dangerous atoi(), which does no validation
+ char *end;
+ int fd = strtol(argv[2], &end, 10);
+ if ( *end != '\0' )
+ {
+ fprintf(stderr, "Invalid file descriptor: %s\n", argv[2]);
+ showUsage();
+ }
FILE *fp = fdopen(atoi(argv[2]), "r");
size_t len;
- if(fp!=0){
+ if(fp!=NULL){
ssize_t slen = getline(&pass, &len, fp);
if(pass[slen-1]=='\n'){ pass[slen-1] = '\0'; }
}
@@ -63,25 +71,24 @@ int main(int argc, char** argv){
}else if(argc==3 && 0==strcmp(argv[1],"-f") ){
FILE *fp = fopen(argv[2], "r");
size_t len;
- if(fp!=0){
+ if(fp!=NULL){
ssize_t slen = getline(&pass, &len, fp);
if(pass[slen-1]=='\n'){ pass[slen-1] = '\0'; }
}else{
puts("[ERROR] Unknown option provided");
puts("----------------");
showUsage();
- return 1;
}
fclose(fp);
}
- if(pass == 0){ puts("Could not read password!!"); return 1; } //error in reading password
+ if(pass == NULL){ puts("Could not read password!!"); return 1; } //error in reading password
//puts("Read Password:");
//puts(pass);
//Validate current user (make sure current UID matches the logged-in user,
char* cUser = getlogin();
- struct passwd *pwd = 0;
+ struct passwd *pwd = NULL;
pwd = getpwnam(cUser);
- if(pwd==0){ return 1; } //Login user could not be found in the database? (should never happen)
+ if(pwd==NULL){ return 1; } //Login user could not be found in the database? (should never happen)
if( getuid() != pwd->pw_uid ){ return 1; } //Current UID does not match currently logged-in user UID
//Create the non-interactive PAM structures
pam_handle_t *pamh;
@@ -102,5 +109,5 @@ int main(int argc, char** argv){
//Stop the PAM instance
pam_end(pamh,ret);
//return verification result
- return ((ret==PAM_SUCCESS) ? 0 : 1);
+ return ret == PAM_SUCCESS ? 0 : 1; // 0 exit status = success
}
--- lumina-desktop/src-screensaver/LLockScreen.cpp.orig 2021-12-26 02:33:45 UTC
+++ lumina-desktop/src-screensaver/LLockScreen.cpp
@@ -79,7 +79,9 @@ void LLockScreen::TryUnlock(){
this->setEnabled(false);
QString pass = ui->line_password->text();
ui->line_password->clear();
- //Create a temporary file for the password, then pass that file descriptor to lumina-checkpass
+ // Create a temporary file for the password, then pass that file descriptor
+ // to lumina-checkpass. Might seem wacky to shell this out, but we need
+ // SUID to authenticate the pw with PAM.
QTemporaryFile *TF = new QTemporaryFile(".XXXXXXXXXX");
TF->setAutoRemove(true);
bool ok = false;
@@ -90,7 +92,7 @@ void LLockScreen::TryUnlock(){
if(DEBUG){ qDebug() << "Trying to unlock session:" << getlogin(); }
LUtils::runCommand(ok, "lumina-checkpass",QStringList() << "-f" << TF->fileName() );
if(DEBUG){ qDebug() << " - Success:" << ok; }
- ok = true; //bypass for the moment
+ // ok = true; //bypass if lumina-checkpass is not working
}
delete TF; //ensure the temporary file is removed **right now** for security purposes
if(ok){
—
Reply to this email directly, view it on GitHub
<#808 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABDQJJZH55LJGRRXIII4J3TU6DRKRANCNFSM5N53SWLQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you commented.Message ID:
***@***.***>
|
|
Seems to work fine for me since I worked around the lumina-checkpass.pro issue and set the SUID via pkg-plist. The main problem was that $$DESTDIR came out blank, so the chmod didn't work. I'll report back if I encounter any problems. Also, it seems that the screensaver starts out in active mode, but drops to blank screen at some point later, so energy usage is not much of an issue. Given that, integrating LScreensaver into settings is even less urgent than I originally thought. |
|
I ran into a problem with this patch working, but that may have been a result of something on that system. I've been needing to do a clean nuke&pave of it anyway. So once I get that system re-installed I'll try your patch again. |
|
Certainly you'll want to test it thoroughly before merging. I haven't had any problems with it, but if there's one thing I've learned from decades of coding, it's that testing on one platform is never sufficient, no matter how small the change. I test all my own code on FreeBSD, Mac, Linux, and NetBSD, though I don't have the means to do that with Lumina patches. Maybe also figure out why DESTDIR is blank, though solving that won't be useful for FreeBSD, since the ports system doesn't allow upstream build systems to set SUID anyway. |
|
Already suggested throwing out the screensaver and screen blocker from lumina? I'm not sure that they are needed, especially without the ability to configure and disable. |
|
The Lumina screensaver actually works better than xscreensaver. The only issue is not having a configuration interface, which I don't think will be difficult to fix. |
|
Starting this summer I should have more time to dig into Lumina issues and learn what's necessary to patch up some of the issues. One thing that would be valuable in the short term would be a blank-screen only option. Since the existing screensavers use a lot of CPU time, I would like to patch the Lumina FreeBSD port to just blank the screen instead. I know very little about QT programming at the moment, though. Anyone know how to create a blank screen option in the JSON/QML files? Here's an example of one of the existing screensavers: |
With 1.6.2, xscreensaver is no longer needed.
However:
The text was updated successfully, but these errors were encountered: