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: | pam | Assignee: | Tomas Mraz <tmraz> | ||||||||
| Status: | CLOSED ERRATA | QA Contact: | |||||||||
| Severity: | low | Docs Contact: | |||||||||
| Priority: | medium | ||||||||||
| Version: | 4 | CC: | 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: | |||||||||
| Embargoed: | |||||||||||
| Attachments: |
|
||||||||||
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.
Conclusion:
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?
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. Dan 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. 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. Dan This would be another possibility but at least the failure should be also logged to the system log. 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.
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.
Looks good. As soon as you have a build I will test it. 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. 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! 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. The current plan is to make this public on 2005-10-26 1400UTC. 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 Yes, the patch was not quite complete as the unix_chkpwd binary is called from two more places. Created attachment 120035 [details]
Better patch
If there is any desire to extend the embargo date, it's not an issue. I was under the impression the patch was complete. Lifting embargo, this issue is now public. 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. Fixed also in FC-3 and devel. |
Description of problem: Version-Release number of selected component (if applicable): all pam with the SELinux patch included (FC3,4 RHEL4) How reproducible: Try: $ 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.