lastlog on Solaris with PAM (patch included)

[Mark Mentovai]

On Solaris, the pam_unix module includes a pam_session which updates the
lastlog file.  Since OpenSSH calls pam_session before reading the lastlog
file, SSH logins to systems with this configuration (as well as similar
ones, I'd imagine) report the last login time and remote host as the values
from the current session.

My solution to this problem is to call pam_open_session in the child, after
do_login has been called and the last login information has been retrieved
and printed.  This can result in the lastlog file being updated twice, but
we'll need to live with that, since there's no way short of reading it
directly to find out whether or not it has been updated, and even then,
there's a mostly harmless race condition.

A separate but slightly related issue that I'm nailing at the same time (as
well as an explanation for the change in do_exec_no_pty): It's possible for
pam_setcred to print messages that should be considered part of the session,
and that would need to be done after the stdio and pty plumbing.

Finally, I'm not too thrilled about the use of strlcpy to get the hostname
out of lastlog.  The ll_host field of lastlog is a fixed-width field and is
not null-terminated.  Since strlcpy copies n-1 bytes and then sets byte n-1
to 0, it would be more appropriate to use:

	strlcpy(li->hostname, last->ll_host,
		MIN_SIZEOF(li->hostname, last->ll_host + 1));

Think about what happens if sizeof(last->ll_host) = n (n < LINFO_HOSTSIZE)
and a specific hostname is exactly n bytes long without any trailing
0-bytes.

Also, watch out for other null-termination issues in the same region of code
- I mention this because if LINFO_HOSTSIZE was set to coincide with
sizeof(last->ll_host) of some particular system, it really should be bumped
up by one to handle the trailing 0.

Mark

--
OpenSSH 2.9p2 Patch, 2001 September 6, Mark Mentovai

Fix for the ordering of some of the PAM calls, including the problem with
incorrect last login times on Solaris with PAM.  See comments for details.

diff -ur openssh-2.9p2.dist/session.c openssh-2.9p2/session.c
Index: openssh-2.9p2/session.c
--- openssh-2.9p2.dist/session.c	Sat Jun 16 23:40:51 2001
+++ openssh-2.9p2/session.c	Thu Sep  6 11:48:04 2001
@@ -456,10 +456,6 @@

 	session_proctitle(s);

-#if defined(USE_PAM)
-	do_pam_setcred(1);
-#endif /* USE_PAM */
-
 	/* Fork the child. */
 	if ((pid = fork()) == 0) {
 		/* Child.  Reinitialize the log since the pid has changed. */
@@ -509,6 +505,13 @@
 			perror("dup2 stderr");
 #endif /* USE_PIPES */

+#if defined(USE_PAM)
+		/* pam_setcred might print things that belong to the
+		   session, put it after the stdio plumbing.
+		   Mark Mentovai, 20010906 */
+		do_pam_setcred(1);
+#endif /* USE_PAM */
+
 		/* Do processing for the child (exec command etc). */
 		do_child(s, command);
 		/* NOTREACHED */
@@ -570,11 +573,6 @@
 	ptyfd = s->ptyfd;
 	ttyfd = s->ttyfd;

-#if defined(USE_PAM)
-	do_pam_session(s->pw->pw_name, s->tty);
-	do_pam_setcred(1);
-#endif
-
 	/* Fork the child. */
 	if ((pid = fork()) == 0) {
 		/* Child.  Reinitialize the log because the pid has changed. */
@@ -609,6 +607,23 @@
 		else
 			do_pre_login(s);
 # endif
+#endif
+
+#if defined(USE_PAM)
+		/* The PAM session code must be after do_login because the
+		   pam_open_session handler in the pam_unix module in
+		   Solaris updates the lastlog file, which causes do_login
+		   to report the current login as the last one.  A lastlog
+		   entry will be written again by lastlog_perform_login,
+		   but we'll live with that for now.  Can't just disable
+		   lastlog writing in lastlog_perform_login if USE_PAM is
+		   defined, because there are systems and configurations
+		   with which pam_open_session does not touch lastlog.
+		   Also, pam_setcred might print things that belong to the
+		   session, so it should be called after the pty is set up.
+		   Mark Mentovai, 20010906 */
+		do_pam_session(s->pw->pw_name, s->tty);
+		do_pam_setcred(1);
 #endif

 		/* Do common processing for the child, such as execing the command. */