The attached patch provides a workaround for changing expired passwords on login with sshd running in privsep mode. It does so by delegating the the change dialog to a suid helper program. (Yes, yet another one :) The patch incorporates the HP-UX trusted system patch by Dan Wanek, submitted with [BUG 419]. I have tested this patch successfully on Linux (Debian with libpam0g 0.72-32) HP-UX 11.00 and 11.11, both trusted and non-trusted mode Solaris 2.7 It seems to be even a bit more robust than the builtin change routine for non-privsep mode, which crashes on trusted systems when using the dialog options for random generated passwords. (No idea why, unfortunately) The ssh-chauthtok-helper passed them flawlessly.
Created attachment 162 [details] Patch: Workaround for pw change in privsep mode (3.5.p1)
*** Bug 419 has been marked as a duplicate of this bug. ***
Created attachment 163 [details] Non-privsep pw change failure on HP-UX trusted systems Found why the password change dialog in non-privsep mode on a HP-UX 11.x trusted system fails when the user selects random password options. It's because OpenSSH's log function interferes with a log stub in /usr/lib/libsec.2: Program received signal SIGSEGV, Segmentation fault. 0xc00fadf0 in _doprnt+0x188 () from /usr/lib/libc.2 (gdb) bt #0 0xc00fadf0 in _doprnt+0x188 () from /usr/lib/libc.2 #1 0xc010b80c in vsnprintf+0x54 () from /usr/lib/libc.2 #2 0x4f94c in do_log (level=SYSLOG_LEVEL_INFO, fmt=0xcb000000 <Error reading address 0xcb000000: Bad address>, args=0x7f7f417c) at log.c:387 #3 0x4f20c in log (fmt=0xcb000000 <Error reading address 0xcb000000: Bad address>) at log.c:135 #4 0xc0164360 in passlen+0xa0 () from /usr/lib/libsec.2 #5 0xc1e8b2a4 in pwd_get_random+0x69c () from /usr/lib/security/libpam_unix.1 #6 0xc1e8ca28 in chauthtok_get_pwd+0x78 () from /usr/lib/security/libpam_unix.1 #7 0xc1e8139c in get_newpasswd+0x4a4 () from /usr/lib/security/libpam_unix.1 #8 0xc1e7f5fc in __update_authtok+0x30c () from /usr/lib/security/libpam_unix.1 #9 0xc1e7e6d0 in pam_sm_chauthtok+0xf8 () from /usr/lib/security/libpam_unix.1 #10 0xc06571b0 in pam_chauthtok+0x100 () from /usr/lib/libpam.1 #11 0x22660 in do_pam_chauthtok () at auth-pam.c:422 #12 0x2ea44 in do_login (s=0x40015fd0, command=0x0) at session.c:755 #13 0x2e594 in do_exec_pty (s=0x40015fd0, command=0x0) at session.c:616 #14 0x2e898 in do_exec (s=0x40015fd0, command=0x0) at session.c:709 #15 0x2df68 in do_authenticated1 (authctxt=0x4001cb90) at session.c:402 #16 0x2daec in do_authenticated (authctxt=0x4001cb90) at session.c:221 #17 0x16704 in main (ac=1, av=0x7f7e07f4) at sshd.c:1528 Renaming function log() to sshlog() in log.h resolves this conflict. See attached patch.
regarding log() clash, shouldn't the HP libsec log() be static or renamed or ?
*** Bug 129 has been marked as a duplicate of this bug. ***
Created attachment 198 [details] Add privsep'ed do_pam_chauthtok() Implements do_pam_chauthtok() in the monitor via a passed file descriptor. Resets forwarding flags in slave by trapping a SIGUSR from the child forked to run the shell. This should be safe because to spoof a reset of the forwarding flags you would already need to be logged in anyway and could install your own forwarders. This is the same patch I posted to the openssh-devel list earlier but it belongs to this bug.
*** Bug 381 has been marked as a duplicate of this bug. ***
*** Bug 473 has been marked as a duplicate of this bug. ***
The patch looks good, but the only thing that makes me wary is the use of signals for IPC. Would it not be possible to do the chauthtok call earlier? E.g. after the call to do_pam_session() in do_exec_pty()?
Are stdin/out/err even connected to the client at that point? I didn't think they were. If so then yes, that should eliminate the signal stuff and the monitor call from the shell child (which is potentially racy and *bad* according to Markus and Niels, see http://marc.theaimsgroup.com/?l=openssh-unix-dev&m=104384154418133).
Created attachment 246 [details] INCOMPLETE example patch Actually, what I suggested won't work. The chauthtok needs to happen after the tty magic in do_exec_pty. I suppose we could do something horrid like the incomplete attached patch. I.e run a separate subshell for the chauthtok before forking the real child.
Hmm, that should work. With a bit of care the same function could run /bin/passwd for non-pam password changes too (although there is less need for that since passwd is setuid, it would reduce duplicated code). I'll make some changes to my patch to do it that way and see how well it works.
That approach isn't as easy as it seemed at first... as soon as you wait() for the child the ssh protocol processing stops. If you let it run, you then start the session without the password being changed. I guess you could use a SIGCHLD handler for successful change, but then you need to some way to synchronise the shell child. I can't see any easy way to make this work. How badly do you hate using SIGUSR :-?
Well, nothing suggested so far avoids the shell child communicating with the monitor. I agree with Markus and Niels that this is bad. If we accept, for now, the smaller bug of chauthtok-requiring sessions not being able to use port,X11 or agent forwardings, can we make it work with privsep under the above constraint?
Actually, the first proposal (ssh-chauthtok-helper) by Michael Steffens does not require monitor calls by the shell child. It wasn't very popular, though. The current architecture requires a change after the tty is established in the shell child, which has already setuid to the user. Assuming that it must be done that way (to support protocol 1), changing the password may require privs, and the shell child can't *call* the monitor, some possible options are: 1) A setuid helper (ssh-chauthtok-helper, /bin/passwd), eg, if (use_privsep) exec /bin/passwd else do_pam_chauthtok() 2) Have the monitor fork a child *that does not exit immediately* and return a descriptor connected to it. Have that child wait for a write on the descriptor. When the shell child runs, it writes to the descriptor to wake up the privileged child which runs do_pam_chauthok(), using a little protocol to indicate success or failure. This is fiddly and complex (== bad) but might work. Since there's always *someone* who hates a given solution to this problem, I vote for 1) with /bin/passwd. It's simple and I've already written most of it :-) The PAM purists will hate it but they can turn off privsep if they need the real do_pam_chauthtok, or re-define PATH_PASSWD_PROGRAM and make it run whatever they like.
Regarding the name space collision with the log function, the collision is not in libsec as comment #3 asserts, but is in lib_pamunix. While in trusted mode and attempting to generate passwords for the user, lib_pamunix calls the log(3M) function from the math library. (see also Bug #484). Although you could say that libraries should be statically linked, no one has any real control over dynamically loaded libraries (like all pam libraries). Comapnies can write their own and link them however they wish. For that reason, it is safer to avoid re-using function names that are found in common system libraries. This is why HP-UX trusted mode has failed during do_pam_chauthtok() when the system is generating a password for the user. As far the case where HP-UX trusted mode fails when the user picks a password, see bug #473 (which is marked as a duplicate of this bug) for an explanation of why do_pam_chauthtok() fails.
*** Bug 521 has been marked as a duplicate of this bug. ***
The patch id-162 has fixed my problem of changing passwords running in privileged seperation mode and in a trusted system on HP-UX 11. Thanks for all the help. Todd
*** Bug 562 has been marked as a duplicate of this bug. ***
Has the new PAM code fixed this? I haven't tried it for a long time.
I just tried with CVS version as of today on HP-UX 11.00. No, unfortunately it doesn't even get that far. The issue from [BUG 419] is still unresolved, meaning that PAM session setup breaks with trusted mode and privsep: fatal: PAM: pam_open_session(): General Commercial Security error This can be observed with PAMAuthenticationViaKbdInt enabled, while "traditional" password authentication got broken entirely with trusted mode and privsep. Correct passwords are being rejected: Failed password for ... When turning off trusted mode the same passwords get accepted again (and session setup succeeds).
Created attachment 375 [details] Ported version of Dan's original session setup patch With the attached version of Dan Wanek's original session setup patch I could at least get login to work again with trusted mode and pivsep. (I also had to unwrap the struct initialization of sshpam_conv in auth-pam.c, as the compiler didn't accept the current construct.) However, it still looks a bit strange. Password authentication only works with ChallengeResponseAuthentication(?) enabled. Otherwise correct passwords are being rejected. With proto 2 I'm getting the usual password prompt from PAM. With proto 1 it looks like challenge response (but both prompts popping up immediately) Password: Response: where authentication succeeds after typing the regular user password as "Response".
Will that patch (attachment #375 [details]) cause other PAM-using platforms to cough up a hairball?
Created attachment 376 [details] Corrected port of Dan's original session setup patch Sorry, with (attachment #375 [details]) I introduced a stupid bug, making the initialization of sshpam_conv conditional (which didn't show up because USE_POSIX_THREADS was not defined). The attached version has this one corrected. When I last tested Dan's proposed modifications on Solaris and Linux, these didn't hickup. Would prefer someone more familiar with these platforms to verify, though.
Comment on attachment 376 [details] Corrected port of Dan's original session setup patch >--- auth-pam.c-orig Tue Aug 26 03:58:16 2003 >+++ auth-pam.c Mon Sep 1 08:04:13 2003 >@@ -199,10 +199,15 @@ > { > struct pam_ctxt *ctxt = ctxtp; > Buffer buffer; >- struct pam_conv sshpam_conv = { sshpam_thread_conv, ctxt }; >+ struct pam_conv sshpam_conv; > #ifndef USE_POSIX_THREADS > const char *pam_user; >+#endif >+ >+ sshpam_conv.conv = sshpam_thread_conv; >+ sshpam_conv.appdata_ptr = ctxt; I don't understand this part of the patch. Why does break the initialisation from the declaration? >+#ifndef USE_POSIX_THREADS I'm not sure which version you are diffing against, but CVS HEAD already has this test. > #if defined(USE_PAM) > if (options.use_pam) { >- do_pam_session(s->pw->pw_name, NULL); > do_pam_setcred(1); > if (is_pam_password_change_required()) > packet_disconnect("Password change required but no " >@@ -561,7 +560,7 @@ > > #if defined(USE_PAM) > if (options.use_pam) { >- do_pam_session(s->pw->pw_name, s->tty); >+ do_pam_set_tty(s->tty); > do_pam_setcred(1); > } > #endif >@@ -1235,6 +1234,7 @@ > */ > if (options.use_pam) > do_pam_setcred(0); >+ do_pam_session(pw->pw_name,NULL); This is missing braces after the "if" statement. I.e if (options.use_pam) { do_pam_setcred(0); do_pam_session(pw->pw_name,NULL); } I agree that do_pam_session makes more sense is setusercontext, but if we split the PAM_TTY setting, then we should remove do_pam_session's second argument entirely.
> ------- Additional Comments From djm@mindrot.org 2003-09-01 16:51 ------- > (From update of attachment 376 [details]) > >>--- auth-pam.c-orig Tue Aug 26 03:58:16 2003 >>+++ auth-pam.c Mon Sep 1 08:04:13 2003 >>@@ -199,10 +199,15 @@ >>{ >> struct pam_ctxt *ctxt = ctxtp; >> Buffer buffer; >>- struct pam_conv sshpam_conv = { sshpam_thread_conv, ctxt }; >>+ struct pam_conv sshpam_conv; >>#ifndef USE_POSIX_THREADS >> const char *pam_user; >>+#endif >>+ >>+ sshpam_conv.conv = sshpam_thread_conv; >>+ sshpam_conv.appdata_ptr = ctxt; > > > I don't understand this part of the patch. Why does break the initialisation > from the declaration? To be honest, I don't know. Compiler refused it with error 1521: Incorrect initialization. > > >>+#ifndef USE_POSIX_THREADS > > > I'm not sure which version you are diffing against, but CVS HEAD already has > this test. Yes, but I erranously moved initialization inside the #ifndef block when wanting to get it past the declaration of pam_user. Correction was to split the #ifndef, and put the initialization in between. > > > >>#if defined(USE_PAM) >> if (options.use_pam) { >>- do_pam_session(s->pw->pw_name, NULL); >> do_pam_setcred(1); >> if (is_pam_password_change_required()) >> packet_disconnect("Password change required but no " >>@@ -561,7 +560,7 @@ >> >>#if defined(USE_PAM) >> if (options.use_pam) { >>- do_pam_session(s->pw->pw_name, s->tty); >>+ do_pam_set_tty(s->tty); >> do_pam_setcred(1); >> } >>#endif >>@@ -1235,6 +1234,7 @@ >> */ >> if (options.use_pam) >> do_pam_setcred(0); >>+ do_pam_session(pw->pw_name,NULL); > > > This is missing braces after the "if" statement. I.e > > if (options.use_pam) { > do_pam_setcred(0); > do_pam_session(pw->pw_name,NULL); > } Yep, another stupid bug of mine :(. Thanks for catching. (This also didn't show up when testing, because options.use_pam was true, of course.) > > I agree that do_pam_session makes more sense is setusercontext, but if we split > the PAM_TTY setting, then we should remove do_pam_session's second argument > entirely. Also agreed. (I used Dan's original modifications as they were, and these were presumably meant to modify at little as possible. But there is only one invokation of do_pam_session left, not using the second argument.)
Created attachment 377 [details] 3rd version port of Dan's original session setup patch Now a version taking djm's correction and suggestion into account.
Tried to find where the strange password authentication behaviour mentioned in comment #22 comes from, and why plain old password authentication fails in HP-UX trusted mode. Maybe this got me a bit closer to how the new code works. :) Apparently it's falling back to non-PAM authentication for plain old password authentication (the one where the client prompts "user@host's password:"). This fails in trusted mode, because DISABLE_SHADOW is defined for all versions of HP-UX, thus getspnam is not being used to retrieve the real pw hash. Instead the '*' from /etc/passwd is being used. By enabling shadow this can be fixed. I have tried it and it worked. On the other hand, with proto 1, TIS authentication has precedence over password authentication, and it actually works using sshpam_device. PAM happens to generate the challenge "Password:", and succeeds when getting the correct password on the prompt "Response:". In case password via TIS fails (for example because the user was confused by the prompts), traditional password authentication, bypassing PAM, is being tried. Is this correct and the intended order?
very similar patch applied, thanks. Please file a separate bug for the HAVE_SHADOW issue
Hi Damien, thanks for applying! However, this modification only solves the problem of getting session set up at all with privsep in trusted mode, but not yet changing expired passwords. (The one being prerequisite to perform the other.) This fix would rather apply to BUG #419, which is marked resolved/duplicate of BUG #83. However, I'm having no idea whether they are related... Michael
Didnt Patch 162 fix all this? I have OpenSSH 3.5p1 working on a HP-UX 11 trusted system in privelage seperation mode with password aging and it works like a charm.
It did, besides the additional suid ssh-chauthtok-helper did not get accepted. The more elegant approach should be enabled by new PAM code, but is apparently not active yet. (unless, beat me, I missed something. It didn't work when I tried yesterday.)
Hmm, this bug probably should have been reopened, but anyway... the following change was just committed which should solve this for SSH2 logins and should work fine with privsep. I would appreciate if anyone interested in this could try a snapshot (20031219 or later). - (dtucker) [auth-pam.c] Do PAM chauthtok during SSH2 keyboard-interactive authentication. Partially fixes bug #423. Feedback & ok djm@
Mass change of RESOLVED bugs to CLOSED