openssh unix dev - Feb 2002 - openssh + pam errors (fwd)

heres a fix for pam support im openssh, inline and attached..  openssh
calls do_pam_session early, before a fork().  it does this on the proc
still running as root, so it checks the users limits, against what root
has running, and depending on limits can fail at the fork() (and almost
always does).  this patch moves it past the fork.  ive been running it for
a couple of weeks and everything seems good.  i used to have to use
uselogin yes and set the limits with login because openssh was broken,
but this takes care of it.

---------- Forwarded message ----------
Date: Mon, 11 Feb 2002 23:05:54 -0500
From: Dug Song <dugsong at monkey.org>
To: Rob Mosher <nyt at countercultured.net>
Subject: Re: openssh + pam errors

send this patch to openssh-unix-dev at mindrot.org

Damien Miller maintains the OpenSSH-portable tree, he'll know what to
do with it...

-d.

---
http://www.monkey.org/~dugsong/




--- old/session.c	Sat Dec  1 18:37:08 2001
+++ session.c	Tue Feb 12 00:03:37 2002
@@ -437,7 +437,6 @@
 	session_proctitle(s);

 #if defined(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 "
@@ -494,6 +493,9 @@
 #endif /* USE_PIPES */

 		/* Do processing for the child (exec command etc). */
+#if defined(USE_PAM)
+		do_pam_session(s->pw->pw_name, NULL);
+#endif /* USE_PAM */
 		do_child(s, command);
 		/* NOTREACHED */
 	}
@@ -555,7 +557,6 @@
 	ttyfd = s->ttyfd;

 #if defined(USE_PAM)
-	do_pam_session(s->pw->pw_name, s->tty);
 	do_pam_setcred(1);
 #endif

@@ -580,6 +581,9 @@

 		/* Close the extra descriptor for the pseudo tty. */
 		close(ttyfd);
+#if defined(USE_PAM)
+		do_pam_session(s->pw->pw_name, s->tty);
+#endif /* USE_PAM */

 		/* record login, etc. similar to login(1) */
 #ifndef HAVE_OSF_SIA



Rob Mosher
Lead Programmer / Sr Systems Engineer
Lightning Internet Services, LLC


-------------- next part --------------
--- old/session.c	Sat Dec  1 18:37:08 2001
+++ session.c	Tue Feb 12 00:03:37 2002
@@ -437,7 +437,6 @@
 	session_proctitle(s);
 
 #if defined(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 "
@@ -494,6 +493,9 @@
 #endif /* USE_PIPES */
 
 		/* Do processing for the child (exec command etc). */
+#if defined(USE_PAM)
+		do_pam_session(s->pw->pw_name, NULL);
+#endif /* USE_PAM */
 		do_child(s, command);
 		/* NOTREACHED */
 	}
@@ -555,7 +557,6 @@
 	ttyfd = s->ttyfd;
 
 #if defined(USE_PAM)
-	do_pam_session(s->pw->pw_name, s->tty);
 	do_pam_setcred(1);
 #endif
 
@@ -580,6 +581,9 @@
 
 		/* Close the extra descriptor for the pseudo tty. */
 		close(ttyfd);
+#if defined(USE_PAM)
+		do_pam_session(s->pw->pw_name, s->tty);
+#endif /* USE_PAM */
 
 		/* record login, etc. similar to login(1) */
 #ifndef HAVE_OSF_SIA

On Tue, 12 Feb 2002, Rob Mosher wrote:
> heres a fix for pam support im openssh, inline and attached..  openssh
> calls do_pam_session early, before a fork().  it does this on the proc
> still running as root, so it checks the users limits, against what root
> has running, and depending on limits can fail at the fork() (and almost
> always does).  this patch moves it past the fork.  ive been running it for
> a couple of weeks and everything seems good.  i used to have to use
> uselogin yes and set the limits with login because openssh was broken,
> but this takes care of it.
BTW this is bug #83[1], here is my commentry on the bug itself. Please 
post followups through bugzilla.
> The problem is that we call pam_session as root, before we fork the 
> child. Therefore the server picks up the limits, rather than the child. 
>
> I recall that we tried moving the pam_session call to the child a while
> (~18 months) ago to avoid this problem, but other stuff broke much 
> worse. IIRC the breakage was because we did pam_session stuff in one 
> process (as non-root)  and then did cleanup in another process (as 
> root).
>
> A possible way around this is with a gratuitous fork() before we call
> pam_session, but that is pretty ugly.
-d

[1] http://bugzilla.mindrot.org/show_bug.cgi?id=83