From Bugzilla Helper: User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.5) Gecko/20041215 Firefox/1.0 Red Hat/1.0-12.EL4 Description of problem: I have pam_ccreds enabled in the authentication for xscreensaver as so: auth sufficient /lib/security/pam_ccreds.so action=validate use_first_pass However, this does not allow the user to unlock the screen with his cached credential. The reason is that the /var/cache/.security.db file is by default readable only by root. If I modify the read permission on the /var/cache/.security.db to be readable by all the screen unlocks as expected. Version-Release number of selected component (if applicable): xscreensaver-4.18 pam_ccreds-1-3 How reproducible: Always Steps to Reproduce: 1. set up your /etc/pam.d/system-auth to use pam_ccreds 2. login once while online such that the cached credential gets stored in to /var/cache/.security.db 3.set up your /etc/pam.d/xscreensaver file to use: auth sufficient /lib/security/pam_ccreds.so action=validate use_first_pass 4. blank and lock your screen 5. try unlocking the screen with the cached password. Actual Results: xscreensaver - verbose shows something similar to: pam_cc_start failed: Error in service module syslog shows something like: host xscreensaver[14622]: pam_ccreds: failed to open cached credentials "/var/cache/.security.db": Permission denied screen is not unlocked. Expected Results: User should be allowed to unlock the screen if he has provided the proper password. Additional info: By default pam_ccreds creates the /var/cache/.security.db file as: -r-------- root root It needs it to be: -r--r--r-- root root in order for xscreensaver to unlock the screen.
The pam_ccreds module could fork/exec a setuid program that could read and write .security.db. This would be similar to unix_chkpwd. SELinux could be used to further protect the use of this program.
Yea, that's the correct fix. I vaguely remember talking about this before.
Okay, I just looked at the code for unix_ckpwd and pam_ccreds. I think I could implement this. Will you accept a decent patch to pam_ccreds? Eventually, it should get upstream.
Sure! That'd be great! Thanks! Kathy Whyte
Created attachment 115327 [details] Patch to add ccreds_chkpwd to pam_ccreds (like unix_chkpwd) This patch adds a helper program, ccreds_chkpwd, to pam_ccreds. This program may be setuid root so that xscreensaver, which does not run as root, may authenticate users using pam_ccreds. This is the same idea that unix_chkpwd/pam_unix uses.
Created attachment 115328 [details] Patch to add ccreds_chkpwd to pam_ccreds (like unix_chkpwd) This patch adds a helper program, ccreds_chkpwd, to pam_ccreds. This program may be setuid root so that xscreensaver, which does not run as root, may authenticate users using pam_ccreds. This is the same idea that unix_chkpwd/pam_unix uses.
Created attachment 115329 [details] Patch to add ccreds_chkpwd to pam_ccreds (like unix_chkpwd) This patch adds a helper program, ccreds_chkpwd, to pam_ccreds. This program may be setuid root so that xscreensaver, which does not run as root, may authenticate users using pam_ccreds. This is the same idea that unix_chkpwd/pam_unix uses.
Created attachment 115993 [details] Patch to add ccreds_chkpwd to pam_ccreds (like unix_chkpwd) This version of the patch fixes a segfault that sometimes happened because of an incorrect call to pam_cc_end().
Any comments on this patch? This is a proven solution from pam_unix. This seems like a no-brainer, once the patch has been reviewed.
Sorry I dropped the ball on providing feedback. I just tested it and it works great! Would love to get it into an official pam_ccreds release (and also into RHEL 4) Kathy Whyte
There is missing check for p == NULL before strlen(p) in the cc_chkpwd.c in _ccreds_verify_password(). Or probably even more correctly the _ccreds_verify_password() should not be called if no data is read from stdin? Otherwise the patch seems OK to me and I'll probably include it in next pam_ccreds rebuild. Will you send it upstream?
Created attachment 119338 [details] Patch to add ccreds_chkpwd to pam_ccreds (like unix_chkpwd) Okay, _ccreds_verify_password not receives "" if the password is blank. This is consistent with how pam_ccreds works before the patch. I have tried to propose this patch using one of PADL's mailing lists. I have not yet received a response. More recently, I tried to submit the patch using PADL's Bugzilla. Unfortunately, pam_ccreds is not in their Bugzilla. I submitted a request that the project be added to Bugzilla and am waiting for that.
Have you folks gotten xscreensaver + pam_ccreds to unlock when connected to network? With this patch and system-auth similar to the one in bug 145044 (except using ldap auth, not kerberos) everything else is working nicely, but xscreensaver wont unlock when connected to network. I had to basically make a copy of the system-auth to xscreensaver pam config and change the pam_ldap auth line in the new xscreensaver pam config to this to make it work in both connected and offline mode: auth sufficient /lib/security/$ISA/pam_ldap.so use_first_pass
This patch fixes the authenticate mode of the pam_ccreds module. A similar technique would need to be used to fix the other two modes. Right now, only unlocking when the network authentication daemon is NOT available is fixed. I have tried to approach PADL with this. I am waiting for them to add the pam_ccreds project to their Bugzilla so I may start communicating with them and providing patches upstream. See http://bugzilla.padl.com/show_bug.cgi? id=227. I think this is a critical problem with pam_ccreds (if it wasn't then unix_passwd would not exist.) However, I have received no interest yet in fixing it from PADL. I know what has to be done, but need feedback from the maintainer before I will proceed.
In the above comment, unix_passwd is a mistake. It should be unix_chkpwd.
The patch works fine. Yes, mods to /etc/pam.d/xscreensaver are required, but works as it should. Mike, We understand and respect your decision/situation. We appreciate your work to date. Kathy Whyte
Ok, thanks for explaining the situation, I was just wondering if I'm missing something and indeed I was :) Thanks Mike for the work on this.
Created attachment 120974 [details] Patch to add ccreds_* to pam_ccreds (like unix_chkpwd) This patch replaces ccreds_chkpwd with ccreds_validate and adds ccreds_store and ccreds_update. All three features are now supported. This patch needs testing and a review of its code. Is it safe to implement ccreds_store and ccreds_update as suid programs? Note that they have the same checks as unix_chkpwd. Could someone help me convince PADL to open up their Bugzilla to pam_ccreds? See http://bugzilla.padl.com/show_bug.cgi?id=227. I'd like to get this work upstream or at least get some feedback from the developers.
I'm afraid that the store and update actions can't be implemented securely through setuid helpers. The setuid helpers would allow storing any password to the ccreds database without knowing the old password and regardless of the real password set in the remote (LDAP or KRB5) server. So the only acceptable patch allows only verification of the stored password.
Created attachment 122060 [details] Patch to add ccreds_chkpwd to pam_ccreds (like unix_chkpwd) This patch no longer attempts to implement support for storing/updating when EUID != 0. Instead, when the EUID != 0, store and updating operation simply return PAM_SUCCESS. No operation is performed. Is this safer? I don't think the store and update operations are critical, especially when invoked by a program running with user privileges.
Yes, it should be OK and I agree that the store and update operations in this case don't have to be implemented.
I've added your patch with other modifications to rawhide package. Please test it.
Confirmed fixed.
Could anyone at Red Hat encourage padl.org to open up a pam_ccreds project in their Bugzilla? See comment #18 and http://bugzilla.padl.com/show_bug.cgi?id=227. I'd like to get this upstream.
If you are going to submit a patch, please ensure that it conforms with the existing coding style.
It looks like this patch is now upstream, as of pam_ccreds-5.