After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 648875 - Gnome Screensaver should handle expired password tokens
Gnome Screensaver should handle expired password tokens
Status: RESOLVED FIXED
Product: gdm
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: GDM maintainers
GDM maintainers
Depends on:
Blocks:
 
 
Reported: 2011-04-28 14:30 UTC by Brian C. Huffman
Modified: 2014-08-21 14:16 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Unified diff to add password changing for expired password tokens. (852 bytes, patch)
2011-04-28 14:30 UTC, Brian C. Huffman
none Details | Review

Description Brian C. Huffman 2011-04-28 14:30:44 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.
Comment 1 urusha 2012-01-25 09:46:38 UTC
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
Comment 2 Ray Strode [halfline] 2012-01-26 23:05:25 UTC
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.
Comment 3 urusha 2012-01-27 11:49:12 UTC
(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 :)
Comment 4 Brian C. Huffman 2012-01-27 11:58:08 UTC
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
Comment 5 Brian C. Huffman 2012-01-27 15:01:44 UTC
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.
Comment 6 Ray Strode [halfline] 2012-01-27 19:31:03 UTC
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?
Comment 7 Brian C. Huffman 2012-01-27 19:45:16 UTC
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.
Comment 8 Ray Strode [halfline] 2012-01-27 21:04:17 UTC
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.
Comment 9 urusha 2012-01-30 11:02:34 UTC
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.
Comment 10 graffatcolmingov 2012-06-28 13:17:30 UTC
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?
Comment 11 Robert Ancell 2012-11-30 03:27:17 UTC
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.
Comment 12 Ray Strode [halfline] 2012-11-30 17:20:34 UTC
we ignore failures in GDM when reauthenticating (for unlock):

http://git.gnome.org/browse/gdm/tree/daemon/gdm-session-worker.c#n1227
Comment 13 Bastien Nocera 2014-08-20 20:18:01 UTC
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.
Comment 14 Ray Strode [halfline] 2014-08-21 14:11:41 UTC
nope, see comment 12
Comment 15 Bastien Nocera 2014-08-21 14:16:58 UTC
(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 ;)