[Bug 423] Workaround for pw change in privsep mode (3.5.p1)

[bugzilla-daemon at mindrot.org]

http://bugzilla.mindrot.org/show_bug.cgi?id=423





------- Additional Comments From michael_steffens at hp.com  2003-09-01 17:24 -------
> ------- Additional Comments From djm at mindrot.org  2003-09-01 16:51 -------
> (From update of attachment 376)
> 
>>--- 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.)




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.