Bug 151914 - xscreensaver won't unlock when using pam_ccreds
xscreensaver won't unlock when using pam_ccreds
Status: CLOSED RAWHIDE
Product: Red Hat Enterprise Linux 4
Classification: Red Hat
Component: pam_ccreds (Show other bugs)
4.0
i386 Linux
medium Severity medium
: ---
: ---
Assigned To: Tomas Mraz
:
Depends On:
Blocks: FC5Target
  Show dependency treegraph
 
Reported: 2005-03-23 10:24 EST by Kathy Whyte
Modified: 2007-11-30 17:07 EST (History)
2 users (show)

See Also:
Fixed In Version: pam_ccreds-3-2
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-01-04 10:56:22 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
Patch to add ccreds_chkpwd to pam_ccreds (like unix_chkpwd) (13.48 KB, patch)
2005-06-11 22:39 EDT, W. Michael Petullo
no flags Details | Diff
Patch to add ccreds_chkpwd to pam_ccreds (like unix_chkpwd) (13.48 KB, patch)
2005-06-11 22:39 EDT, W. Michael Petullo
no flags Details | Diff
Patch to add ccreds_chkpwd to pam_ccreds (like unix_chkpwd) (13.48 KB, patch)
2005-06-11 22:40 EDT, W. Michael Petullo
no flags Details | Diff
Patch to add ccreds_chkpwd to pam_ccreds (like unix_chkpwd) (14.48 KB, patch)
2005-06-26 17:40 EDT, W. Michael Petullo
no flags Details | Diff
Patch to add ccreds_chkpwd to pam_ccreds (like unix_chkpwd) (13.71 KB, patch)
2005-09-27 23:51 EDT, W. Michael Petullo
no flags Details | Diff
Patch to add ccreds_* to pam_ccreds (like unix_chkpwd) (32.77 KB, patch)
2005-11-11 23:56 EST, W. Michael Petullo
no flags Details | Diff
Patch to add ccreds_chkpwd to pam_ccreds (like unix_chkpwd) (16.81 KB, patch)
2005-12-08 22:43 EST, W. Michael Petullo
no flags Details | Diff

  None (edit)
Description Kathy Whyte 2005-03-23 10:24:55 EST
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.
Comment 1 W. Michael Petullo 2005-06-03 11:06:30 EDT
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.
Comment 2 Ray Strode [halfline] 2005-06-03 11:08:25 EDT
Yea, that's the correct fix.  I vaguely remember talking about this before.
Comment 3 W. Michael Petullo 2005-06-03 12:15:44 EDT
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.
Comment 4 Kathy Whyte 2005-06-03 12:43:58 EDT
Sure!
That'd be great!
Thanks!

Kathy Whyte
Comment 5 W. Michael Petullo 2005-06-11 22:39:23 EDT
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.
Comment 6 W. Michael Petullo 2005-06-11 22:39:46 EDT
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.
Comment 7 W. Michael Petullo 2005-06-11 22:40:28 EDT
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.
Comment 8 W. Michael Petullo 2005-06-26 17:40:12 EDT
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().
Comment 9 W. Michael Petullo 2005-09-14 12:14:02 EDT
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.
Comment 10 Kathy Whyte 2005-09-14 16:57:12 EDT
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
Comment 11 Tomas Mraz 2005-09-27 09:53:05 EDT
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?
Comment 12 W. Michael Petullo 2005-09-27 23:51:47 EDT
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.
Comment 13 Panu Matilainen 2005-10-14 08:42:38 EDT
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
Comment 14 W. Michael Petullo 2005-10-14 09:38:31 EDT
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.
Comment 15 W. Michael Petullo 2005-10-14 09:42:16 EDT
In the above comment, unix_passwd is a mistake.  It should be unix_chkpwd.
Comment 16 Kathy Whyte 2005-10-14 09:47:24 EDT
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
Comment 17 Panu Matilainen 2005-10-14 09:52:16 EDT
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.
Comment 18 W. Michael Petullo 2005-11-11 23:56:42 EST
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.
Comment 19 Tomas Mraz 2005-12-06 05:05:38 EST
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.
Comment 20 W. Michael Petullo 2005-12-08 22:43:24 EST
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.
Comment 21 Tomas Mraz 2005-12-09 05:19:58 EST
Yes, it should be OK and I agree that the store and update operations in this
case don't have to be implemented.
Comment 22 Tomas Mraz 2006-01-04 10:56:22 EST
I've added your patch with other modifications to rawhide package. Please test it.
Comment 23 W. Michael Petullo 2006-01-05 18:38:32 EST
Confirmed fixed.
Comment 24 W. Michael Petullo 2006-01-05 18:48:05 EST
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.
Comment 25 Luke Howard 2007-02-06 20:07:56 EST
If you are going to submit a patch, please ensure that it conforms with the existing coding style.
Comment 26 W. Michael Petullo 2007-10-28 13:11:08 EDT
It looks like this patch is now upstream, as of pam_ccreds-5.

Note You need to log in before you can comment on or make changes to this bug.