| Summary: | pam_end() is not called if authentication fails, which breaks pam-abl | ||
|---|---|---|---|
| Product: | Portable OpenSSH | Reporter: | Christoffer Hammarström <kreiger> |
| Component: | PAM support | Assignee: | Assigned to nobody <unassigned-bugs> |
| Status: | CLOSED FIXED | ||
| Severity: | normal | CC: | djm, dtucker, feldt, sandro.wefel |
| Priority: | P2 | ||
| Version: | 4.6p1 | ||
| Hardware: | Other | ||
| OS: | Linux | ||
| URL: | http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=405041 | ||
| Bug Depends on: | |||
| Bug Blocks: | 1289, 1305 | ||
| Attachments: | |||
|
Description
Christoffer Hammarström
2007-06-18 05:40:49 AEST
Created attachment 1307 [details]
Changeset that introduced the change in question.
This is the changeset that introduced the change for reference.
DO NOT apply the patch in the Debian bug. It will expose your system to the signal handler vulnerability fixed in openssh-4.4 This is the "difficult to fix" SIGALRM handler. We could make sshpam_cleanup() fire if do_cleanup was not called in signal context, but that would just open a different workaround for password guessers: make max_auth_tries-1 guesses and keep the connection open until it times out. Created attachment 1308 [details]
Patch by Sandro Wefel from Debian bug #405041 (don't use as per djm's comments)
Proposed patch from the Debian bug.
Target next release Comment on attachment 1308 [details] Patch by Sandro Wefel from Debian bug #405041 (don't use as per djm's comments) as per comment #2 Created attachment 1325 [details]
Patch for do_cleanup with respect to the signal handler vulnerability fixed in openssh-4.4
Please have a look at the attached patch (id=1325). The idea is to call sshpam_cleanup() if authctxt->authenticated is not set before the KRB5 and GSSAPI blocks. After the pam-call we just return from the function do_cleanup(). This means that krb5_cleanup_proc(authctxt) is not called with an invalid parameter but the sshpam_cleanup() is done which leads to the pam_end call. IMHO this should avoid the signal handler race condition CVE-2006-5051 in krb5_cleanup_proc but calls pam_end() if the user authentication fails. Created attachment 1339 [details]
Patch #1325 with dead code removed.
Damien points out that this makes the existing PAM cleanup code unnecessary.
*** Bug 1308 has been marked as a duplicate of this bug. *** Comment on attachment 1339 [details]
Patch #1325 with dead code removed.
Oops, the patch is wrong; it won't clean up after authenticated connections.
Created attachment 1342 [details]
Allow PAM cleanup for unathenticated connections based on previous
I think this is the simplest patch that does what is required.
The last patch works for me like my patch on all tested architectures and machines in combination with pam_abl. Good work. Comment on attachment 1342 [details]
Allow PAM cleanup for unathenticated connections based on previous
ok for 4.7
Applied, thanks to all. Close resolved bugs after release. |