GNOME Bugzilla – Bug 726196
pam_gnome_keyring should update it's in memory cache of the users password from pam_sm_chauthtok
Last modified: 2014-03-14 20:00:02 UTC
The pam_sm_chauthtok doesn't currently update the gkr_system_authtok pam item to the "new" password. This means if the keyring gets created for the first time at initial login, that a stale password will get used.
Created attachment 271849 [details] [review] pam: Fix issue with changed password not unlocking keyring If a user (needs to) change their password while authenticating (via GDM for example), and pam_gnome_keyring is configured to start the daemon from the session PAM stage, then we were failing to pass the changed password to our session handler. Fix this issue so that this workflow works.
Created attachment 271850 [details] [review] backport gnome-3-10: pam: Fix issue with changed password not unlocking keyring This is a backport of fix on master with the same subject. There's a bit of strange code in the are of this fix, but lets keep it as minimal as possible. If a user (needs to) change their password while authenticating (via GDM for example), and pam_gnome_keyring is configured to start the daemon from the session PAM stage, then we were failing to pass the changed password to our session handler. Fix this issue so that this workflow works.
Created attachment 271853 [details] [review] pam: Pass XDG_RUNTIME_DIR to new process If XDG_RUNTIME_DIR is not in the PAM envlist, but *is* in the process environment, then steal it from there similar to how we handle DISPLAY.
The *testing* parts of the git master patch depend on bug #726245 to apply, and the XDG_RUNTIME_DIR patch for the tests to pass. The gnome-3-10 patch depends on nothing else.
Review of attachment 271849 [details] [review]: Hey thanks for the quick turnaround this! ::: pam/gkr-pam-module.c @@ +989,3 @@ + syslog (GKR_LOG_ERR, "gkr-pam: error storing authtok"); + } + } looks right at a glance. I see at the top of pam_chauthtok_update that the function returns early if there was no "old" password. Couldn't we run into the same situation as comment 0 where the original password is unset and "set on first login" ?
Comment on attachment 271853 [details] [review] pam: Pass XDG_RUNTIME_DIR to new process Attachment 271853 [details] pushed as deae9b5 - pam: Pass XDG_RUNTIME_DIR to new process
(In reply to comment #5) > looks right at a glance. I see at the top of pam_chauthtok_update that the > function returns early if there was no "old" password. Couldn't we run into > the same situation as comment 0 where the original password is unset and "set > on first login" ? Good point, will push new patches.
Created attachment 271928 [details] [review] pam: Fix issue with changed password not unlocking keyring If a user (needs to) change their password while authenticating (via GDM for example), and pam_gnome_keyring is configured to start the daemon from the session PAM stage, then we were failing to pass the changed password to our session handler. Fix this issue so that this workflow works.
Created attachment 271929 [details] [review] backport gnome-3-10: pam: Fix issue with changed password not unlocking keyring Updated both patches to account for the issue Ray brought up: When no old password is present, we still want to stash the new password for the session handler to use to bring up the daemon.
Review of attachment 271928 [details] [review]: cool, looks good to me. ::: pam/gkr-pam-module.c @@ +945,1 @@ return PAM_IGNORE; You could potentially make this: ret = PAM_IGNORE goto out; and then add an out on line 990, though then you'd have to check for NULL password down there. Though, actually, now that i've said it, i'm not even really sure it would be better, just different.
Comment on attachment 271928 [details] [review] pam: Fix issue with changed password not unlocking keyring Attachment 271928 [details] pushed as 808d189 - pam: Fix issue with changed password not unlocking keyring