Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.

Bug 168180

Summary: CAN-2005-2977 unix_chkpwd helper doesn't verify requesting user if SELinux is enabled
Product: [Fedora] Fedora Reporter: Tomas Mraz <tmraz>
Component: pamAssignee: Tomas Mraz <tmraz>
Status: CLOSED ERRATA QA Contact:
Severity: low Docs Contact:
Priority: medium    
Version: 4CC: cpebenito, dwalsh, mpitt, nalin, security-response-team, sgrubb
Target Milestone: ---Keywords: Security
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard: impact=low,source=redhat,embargo=20051026,reported=20050913
Fixed In Version: pam-0.79-9.6 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2005-10-28 07:55:11 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Description Flags
Proposed patch with the setuid call
Proposed patch with the setuid call with added logging
Better patch none

Description Tomas Mraz 2005-09-13 09:54:17 UTC
Description of problem:

Version-Release number of selected component (if applicable):
all pam with the SELinux patch included (FC3,4 RHEL4)

How reproducible:

$ echo -n <root password> | /sbin/unix_chkpwd root nonull && echo OK

OK should not be printed.

So the problem is in this part of SELinux patch:
-        * determine the current user's name is
+        * determine the current user's name is.
+        * On a SELinux enabled system, policy will prevent third
parties from using
+        * unix_chkpwd as a password guesser.  Leaving the existing
check prevents
+        * su from working,  Since the current uid is the users and the
password is
+        * for root.
-       user = getuidname(getuid());
-       if (argc == 2) {
-           /* if the caller specifies the username, verify that user
-              matches it */
-           if (strcmp(user, argv[1])) {
-               force_failure = 1;
-           }
+       if (SELINUX_ENABLED) {
+         user=argv[1];
+       }
+       else {
+         user = getuidname(getuid());
+         /* if the caller specifies the username, verify that user
+            matches it */
+         if (strcmp(user, argv[1])) {
+           return PAM_AUTH_ERR;
+         }
+       }

Obviously when you set SELinux to permissive mode it's possible to run
unix_chkpwd without problem. But even when I set it to enforcing I was
able to run it. (Probably it's disabled only by strict policy?)

So unix_chkpwd allows brute-forcing of any passwords in /etc/shadow.
Also note that there is no delay if the password is incorrect - it exits
immediately, and there is no logging of failed attempts.

Comment 1 Tomas Mraz 2005-09-14 08:18:27 UTC
Dan Walsh:
From the testing I have been doing,  It looks like this part of SELinux patch 
can be removed. And we can go back to the original code.

My comment to this:
Without selinux unix_chkpwd is invoked only when the pam_unix module cannot read
/etc/shadow which can happen only when PAM is invoked by regular user from
non-setuid program, currently this is the case probably only for xscreensaver.

With selinux however the access to /etc/shadow can be restricted even for root
and in case of the setuid binaries invoking PAM (such as su, sudo, passwd) the
unix_chkpwd will fail in the strcmp(user, argv[1]) check even if policy would
allow access to /etc/shadow by unix_chkpwd. 

Probably current targeted and strict policies don't have such restriction on
/etc/shadow so it isn't a problem now but theoretically it would be a good idea
to restrict access to /etc/shadow only to the unix_chkpwd helper.

But even then the check for uid should stay in unix_chkpwd unaffected and to
make it not fail the PAM library should probably check if euid==0 and euid!=uid
and call setuid before executing the unix_chkpwd helper in such case.

Remove the 
+       if (SELINUX_ENABLED) {
+         user=argv[1];
+       }
part from the unix_chkpwd patch.

For devel branch also add the proposed setuid in pam_unix module.

Any comments?

Comment 3 Daniel Walsh 2005-09-14 13:27:20 UTC
From reading the strict policy, all login apps are not allowed to read the
shadow file.  So removing this check should cause them to fail if they have not
done a seteuid before running auth_chkpwd.


Comment 4 Tomas Mraz 2005-09-14 13:37:27 UTC
This only affects su, sudo, passwd, userhelper which have setuid bit set and
most probably don't call setuid before PAM calls.

Is strict policy supported for RHEL4? I am not sure of consequences of adding
the setuid call in pam_unix, there most probably shouldn't be any but I'd want
to be really sure before adding it.

Comment 5 Daniel Walsh 2005-09-14 14:00:50 UTC
What is we just add a 2 second delay when running with SELInux and a failure
happens.  This is what pam does.  So in strict policy you could have up to 4
second delay on login failure.


Comment 6 Tomas Mraz 2005-09-14 14:16:19 UTC
This would be another possibility but at least the failure should be also logged
to the system log.

Comment 15 Tomas Mraz 2005-09-15 15:10:52 UTC
Created attachment 118857 [details]
Proposed patch with the setuid call

Here is the proposed patch with the setuid call.

Another possibility - probably much more clear and safe from the security point
of view would be to split the functionality to two executables - the old
unix_chkpwd helper before it was patched for SELinux purposes (with setuid bit
set) and a new helper binary which wouldn't have setuid bit but would be
allowed to access /etc/shadow by SELinux policy.

Comment 21 Tomas Mraz 2005-10-07 14:51:37 UTC
Created attachment 119709 [details]
Proposed patch with the setuid call with added logging

If we agree that this patch is the right way to go I can start building the
packages immediately.

Comment 22 Daniel Walsh 2005-10-07 15:00:35 UTC
Looks good.  As soon as you have a build I will test it.

Comment 23 Mark J. Cox 2005-10-10 10:25:26 UTC
Adding Ubuntu security team menber Martin Pitt to this errata so he can
determine if this issue affects Ubuntu and if we therefore need to do a
co-ordinated release.

Comment 24 Martin Pitt 2005-10-10 12:18:09 UTC
Thank you for giving me the chance to check this. Our versions are not affected
by this issue (we have some SELinux patches, but not yet the PAM one).

I also checked Debian: Sarge is not affected either, just unstable and testing.
But for these it is fine to fix this bug after the embargo ends.

So there is no need for a coordinated release from my side. Thanks!

Comment 25 Chris PeBenito 2005-10-16 16:51:16 UTC
Gentoo does have the PAM SELinux patch, so I guess there will have to be some
coordination on the release.  I'll start testing on our side.

Comment 26 Josh Bressers 2005-10-16 17:31:25 UTC
The current plan is to make this public on 2005-10-26 1400UTC.

Comment 27 Chris PeBenito 2005-10-16 21:16:43 UTC
Adding the above patch breaks su'ing to root.  I get:

Oct 16 17:14:07 [su(pam_unix)] ERROR 0:Permission denied _
Oct 16 17:14:07 [su] pam_acct_mgmt: Authentication service cannot retrieve
authentication info.
Oct 16 17:14:07 [su] - pts/1 pebenito:root

Comment 28 Tomas Mraz 2005-10-16 22:29:19 UTC
Yes, the patch was not quite complete as the unix_chkpwd binary is called from
two more places.

Comment 29 Tomas Mraz 2005-10-16 22:31:09 UTC
Created attachment 120035 [details]
Better patch

Comment 31 Josh Bressers 2005-10-17 01:13:36 UTC
If there is any desire to extend the embargo date, it's not an issue.  I was
under the impression the patch was complete.

Comment 32 Josh Bressers 2005-10-26 15:46:20 UTC
Lifting embargo, this issue is now public.

Comment 33 Fedora Update System 2005-10-27 17:07:44 UTC
From User-Agent: XML-RPC

pam-0.79-9.6 has been pushed for FC4, which should resolve this issue.  If these problems are still present in this version, then please make note of it in this bug report.

Comment 34 Tomas Mraz 2005-10-28 07:55:11 UTC
Fixed also in FC-3 and devel.