GNOME Bugzilla – Bug 648875
Gnome Screensaver should handle expired password tokens
Last modified: 2014-08-21 14:16:58 UTC
Created attachment 186822 [details] [review] Unified diff to add password changing for expired password tokens. Gnome Screensaver should handle expired password tokens. Currently it does not. The attached patch works for me.
I don't know if this patch works, but it would be realy nice to implement this feature. Gnome-screensaver from ubuntu 11.10 doesn't handle expired passwords, it just unlocks srcreen, so my kerberos credentials stays expired. $ gnome-screensaver --version gnome-screensaver 3.2.0
Review of attachment 186822 [details] [review]: ::: gnome-screensaver-2.30.2.orig/src/gs-auth-pam.c @@ +540,3 @@ + status2, + PAM_STRERROR (pam_handle, status2)); + status = status2; we don't actually want to fail unlock if updating the auth token fails. The user is already logged in, we're just trying to prove they are who they say they are, not that they're allowed to login.
(In reply to comment #2) > Review of attachment 186822 [details] [review]: > > ::: gnome-screensaver-2.30.2.orig/src/gs-auth-pam.c > @@ +540,3 @@ > + status2, > + PAM_STRERROR (pam_handle, status2)); > + status = status2; > > we don't actually want to fail unlock if updating the auth token fails. The > user is already logged in, we're just trying to prove they are who they say > they are, not that they're allowed to login. I think the best way is to make it tunable via dconf: * do not try to update token (like now) * try to update token and unlock screen if updating fails (default?) * try to update token and do not unlock screen on failure So, if administrator doesn't want to allow expired unlocks, he can mandatory lock this gnome-screensaver setting. I would be one of these administrators :)
Ray, With this patch applied, the unlock doesn't fail; gnome-screensaver (maybe due to pam) triggers a dialog to change the password. At least that's how it worked when I first worked on this. -b
Ok - I just realized what you meant. Yes - if the auth token update fails, the way this is written the unlock will fail. I agree with urusha - tunable via dconf would allow for easy modification of behavior.
nah, there's no reason to make this a dconf setting. The thing is, if we want to add some administrator boot people off feature it should be instant, not the next time they happen to let their screen lock. Anyway, overall the patch makes sense. Mind updating it to ignore the return value?
Perhaps my goal here isn't clear. It's not to boot people off - it's to force a password change when using a centralized authentication system (such as LikeWise Open - now called BeyondTrust). In this case - if they choose a new password that is incompatible with our policies (complexity), I *do* want it to fail. So....not sure how to handle this in a generic way that works for everybody.
The user certainly won't be able to succeed at setting an unapproved password. When the pam module does its pam_sm_chauthtok function, if the new password doesn't meet the requirements, it can ask the user to retry an adminstrator configured number of times. If it fails beyond that, the password won't be updated, and the next time they try to login they will get another shot at it. Of course if the administrator wants to make it go on forever, they configure their pam stack that way. For instance, pam_cracklib has a retry= argument. So I think ignoring the status is right.
Side effect of ignoring status: if users don't read error message that is shown for one second after failed try (and most of users do not, I'm sure) thay will think password is changed. And you know what will happen after... locking account, calling to support. But this way is better than not trying to update token at all, so I think patch should be applied with or without ignoring status. Another thing is: If we use centralized password policies (no cracklib or other pam_modules) then only using pam_cracklib with retry= to make gnome-screensaver work as expected looks like a crutch for me. So, if not dconf, then may be default policy should be configured via pam the way like this: 1. Do not ignore pam status. 2. Default /etc/pam.d/gnome-screensaver config should contain: password required pam_permit.so So, if token updating is required, then just unlock screen (like now). 3. If administrator wants to disallow unlocking on not updated tokens, he replace this string with the chain he needs: @include common-password The main idea is to _not_ ignore pam status. And pam does what it should - controls the behavior of authentication.
I'm experiencing this issue on Ubuntu 10.04.3 LTS. We're planning an upgrade to 12.04 anyway, but it seems from the disinterest in this patch that it will not be fixed. Until then, is there anyway we can work around this issue until the patch is either applied or rejected?
What does the screenlock do in Gnome Shell now? I think because it does the authentication in GDM it will now require the auth token to be changed before unlocking. So I think it would make sense to have the same behaviour in gnome-screensaver.
we ignore failures in GDM when reauthenticating (for unlock): http://git.gnome.org/browse/gdm/tree/daemon/gdm-session-worker.c#n1227
Reassigning to gdm, as this still might be a problem there. gnome-screensaver is obsolete, and isn't used in newer versions of GNOME or in Ubuntu.
nope, see comment 12
(In reply to comment #14) > nope, see comment 12 Well, it might be a problem in that the original request was for them not to be ignored ;)