Bug 87 - Last logon that gets reported upon login is the current login time
Summary: Last logon that gets reported upon login is the current login time
Status: CLOSED FIXED
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: sshd (show other bugs)
Version: -current
Hardware: UltraSPARC Solaris
: P2 normal
Assignee: Kevin Steves
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2002-01-31 09:24 AEDT by William Knox
Modified: 2021-04-23 15:10 AEST (History)
3 users (show)

See Also:


Attachments
Patch as an attachment for use of use (2.84 KB, patch)
2002-02-14 01:35 AEDT, William Knox
no flags Details | Diff
There was an error in the previous patch - use this one (2.62 KB, patch)
2002-02-26 07:51 AEDT, William Knox
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description William Knox 2002-01-31 09:24:33 AEDT
The last login time that gets reported with a Solaris PAM enabled build of
OpenSSH 3.0.2p1 is the login time of the current session. Examining the mail
archive, this was reported by Benn Oshrin on 10/12/2001, and he sent in a patch
that moved the do_pam_session call into the do_login function in session.c. I
can't find any discussion about why that patch did not get applied, but it
hasn't, so I propose, instead of moving the do_pam_session call, to move the
call to get_last_login_time into do_exec_pty and pass the resultant information
to do_login. This has the unfortunate result of getting information that you may
never use, but it seems that the portable release does have a few things in it
explicit for one system or another, so this may be viable. The definition of
do_login could be changed and the call could be made in do_exec_pty only on
Solaris using PAM (using a #ifdef PAM_SUN_CODEBASE), but that seemed overly
complicated to me. Any comments, anyone? Patch is below.

--- session.c~  Sat Dec  1 18:37:08 2001
+++ session.c   Tue Jan 29 16:14:25 2002
@@ -128,7 +128,7 @@
 void   do_exec_pty(Session *, const char *);
 void   do_exec_no_pty(Session *, const char *);
 void   do_exec(Session *, const char *);
-void   do_login(Session *, const char *);
+void   do_login(Session *, const char *, const time_t, const char *);
 #ifdef LOGIN_NEEDS_UTMPX
 static void    do_pre_login(Session *s);
 #endif
@@ -548,11 +548,17 @@
 {
        int fdout, ptyfd, ttyfd, ptymaster;
        pid_t pid;
+       char hostname[MAXHOSTNAMELEN];
+       time_t last_login_time;
 
        if (s == NULL)
                fatal("do_exec_pty: no session");
        ptyfd = s->ptyfd;
        ttyfd = s->ttyfd;
+       /* Get the time and hostname when the user last logged in. */
+       hostname[0] = '\0';
+       last_login_time = get_last_login_time(s->pw->pw_uid, s->pw->pw_name,
+           hostname, sizeof(hostname));
 
 #if defined(USE_PAM)
        do_pam_session(s->pw->pw_name, s->tty);
@@ -584,7 +590,7 @@
                /* record login, etc. similar to login(1) */
 #ifndef HAVE_OSF_SIA
                if (!(options.use_login && command == NULL))
-                       do_login(s, command);
+                       do_login(s, command, last_login_time, hostname);
 # ifdef LOGIN_NEEDS_UTMPX
                else
                        do_pre_login(s);
@@ -682,13 +688,11 @@
 
 /* administrative, login(1)-like work */
 void
-do_login(Session *s, const char *command)
+do_login(Session *s, const char *command, const time_t found_last_login_time,
const char *last_host)
 {
        char *time_string;
-       char hostname[MAXHOSTNAMELEN];
        socklen_t fromlen;
        struct sockaddr_storage from;
-       time_t last_login_time;
        struct passwd * pw = s->pw;
        pid_t pid = getpid();
 
@@ -706,13 +710,6 @@
                }
        }
 
-       /* Get the time and hostname when the user last logged in. */
-       if (options.print_lastlog) {
-               hostname[0] = '\0';
-               last_login_time = get_last_login_time(pw->pw_uid, pw->pw_name,
-                   hostname, sizeof(hostname));
-       }
-
        /* Record that there was a login on that tty from the remote host. */
        record_login(pid, s->tty, pw->pw_name, pw->pw_uid,
            get_remote_name_or_ip(utmp_len, options.reverse_mapping_check),
@@ -741,14 +738,14 @@
                printf("%s\n", aixloginmsg);
 #endif /* WITH_AIXAUTHENTICATE */
 
-       if (options.print_lastlog && last_login_time != 0) {
-               time_string = ctime(&last_login_time);
+       if (options.print_lastlog && found_last_login_time != 0) {
+               time_string = ctime(&found_last_login_time);
                if (strchr(time_string, '\n'))
                        *strchr(time_string, '\n') = 0;
-               if (strcmp(hostname, "") == 0)
+               if (strcmp(last_host, "") == 0)
                        printf("Last login: %s\r\n", time_string);
                else
-                       printf("Last login: %s from %s\r\n", time_string,
hostname);
+                       printf("Last login: %s from %s\r\n", time_string,
last_host);
        }
 
        do_motd();
@@ -1866,7 +1863,7 @@
 
        /* Record that the user has logged out. */
        if (s->pid != 0)
-               record_logout(s->pid, s->tty);
+               record_logout(s->pid, s->tty, s->pw->pw_name);
 
        /* Release the pseudo-tty. */
        pty_release(s->tty);
Comment 1 Damien Miller 2002-02-13 23:14:53 AEDT
so a PAM session module is setting the last login time before we retrieve it?
Why not just disable the module in the PAM config?

BTW you should attach your patch using the "create a new attachment" link (under
the keywords entry). Pasting them into the comments makes them near impossible
to apply.
Comment 2 William Knox 2002-02-14 01:35:54 AEDT
Created attachment 25 [details]
Patch as an attachment for use of use
Comment 3 William Knox 2002-02-14 02:08:38 AEDT
The update is happening (I believe) due to the call to pam_open_session in the
do_pam_session function. Can you change the behavior of pam_open_session by
modifying your pam configuration (this is a serious question, as I am by no
means a PAM expert)? And even if you can, wouldn't it be nice if users didn't
have to? After all, I just let ssh use the default entries in the pam
configuration file, and it would be a pain to have to set up separate entries
for ssh just so I could keep the session entry as the default.
Comment 4 William Knox 2002-02-26 07:51:10 AEDT
Created attachment 29 [details]
There was an error in the previous patch - use this one
Comment 5 Kevin Steves 2002-04-05 19:12:09 AEST
i see this too. will investigate.
PAM is fragile.
Comment 6 William Knox 2002-04-06 00:36:47 AEST
This appears to be fixed in the latest SNAP (according to the ChangeLog, this
was a fix by markus on 2002/03/29 18:59:32. His fix is to stuff the last login
time into the Session structure prior to any pty allocation), which I have
tested, so this bug can likely be closed.

As a side note, a little credit in the ChangeLog (like Tim gave for my patch for
bug 84) for finding both this bug and bug 158 (the change in behavior of
ssh-add), as well as submitting patches (which, while they weren't used,
certainly provided direction as to the exact nature of the problem, I imagine)
would be nice. One can just fix problems locally - the incentive to turn the
patch back over to the developers is partially to get credit for having done
some very small amount to help an excellent product be even better.

Well, pardon the whining, and thanks for the fix.
Comment 7 Markus Friedl 2002-04-06 02:03:50 AEST
i usually reference the bug in the commit message and mentionthe reporter. however, when fixing this particualar bugis was not aware of this bug report. your bug was fixed accidentally.
Comment 8 Damien Miller 2004-04-14 12:24:17 AEST
Mass change of RESOLVED bugs to CLOSED
Comment 9 Yann Rouillard 2010-12-20 01:21:01 AEDT
I am reopening this bug because I am still able to reproduce it on Solaris 10 with openssh 5.6p1.

It seems to be caused by the same reason: do_pam_session updates the /var/adm/lastlog before record_login / store_lastlog_message is called.

I don't understand why the fix doesn't work but do_exec_pty doesn't seem to be called before do_pam_session.
Comment 10 Damien Miller 2020-07-31 13:36:54 AEST
Closing because of staleness and lack of further details. If this bug is still happening then please provide a debug trace from the server ("sshd -ddd")
Comment 11 Damien Miller 2021-04-23 15:10:04 AEST
closing resolved bugs as of 8.6p1 release