Bug 1595 - Server option PrintLastLog does not work on AIX
Summary: Server option PrintLastLog does not work on AIX
Status: CLOSED FIXED
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: sshd (show other bugs)
Version: 5.2p1
Hardware: PPC AIX
: P2 normal
Assignee: Assigned to nobody
URL:
Keywords:
Depends on:
Blocks: V_5_3
  Show dependency treegraph
 
Reported: 2009-05-04 02:09 AEST by Miguel Sanders
Modified: 2009-10-06 15:02 AEDT (History)
3 users (show)

See Also:


Attachments
auth.c patch (600 bytes, patch)
2009-05-04 02:09 AEST, Miguel Sanders
no flags Details | Diff
port-aix.c patch (768 bytes, patch)
2009-05-04 02:11 AEST, Miguel Sanders
no flags Details | Diff
port-aix.h patch (599 bytes, patch)
2009-05-04 02:17 AEST, Miguel Sanders
no flags Details | Diff
Updated patch: loginrec.c plus minor style nits (2.79 KB, patch)
2009-05-04 13:06 AEST, Darren Tucker
no flags Details | Diff
add missing argument in port-aix.c (3.04 KB, patch)
2009-05-04 13:13 AEST, Darren Tucker
no flags Details | Diff
option 2: leave print_lastlog logic all in sshlogin.c (2.91 KB, patch)
2009-05-04 13:55 AEST, Darren Tucker
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Miguel Sanders 2009-05-04 02:09:39 AEST
Created attachment 1631 [details]
auth.c patch

Hi

Apparently, the server option "PrintLastLog" does not work on AIX.
The last login time is always displayed, disregarding the option.
When browsing the code, I found out there are several functions in loginrec.c which solely handle the processing of the last login info (login_get_lastlog, getlast_entry).
Since AIX does not provide such a function natively, the configure script sets the DISABLE_LASTLOG define. 
A small code snippet from getlast_entry in loginrec.c shows this

#if defined(DISABLE_LASTLOG)
        /* On some systems we shouldn't even try to obtain last login
         * time, e.g. AIX */
        return (0);

On the other hand, when issuing the AIX loginsuccess() call (which writes a new login record), the last login record can be retrieved by that very same call.
If we look at port-aix.c, we can see the following:

if (loginsuccess((char *)user, (char *)host, (char *)ttynm, &msg) == 0) {
	success = 1;
            if (msg != NULL && loginmsg != NULL && !msg_done) {
                     debug("AIX/loginsuccess: msg %s", msg);
                     buffer_append(loginmsg, msg, strlen(msg));
                     xfree(msg);
                     msg_done = 1;
            }
 }


The pointer "msg" points to the new last login info for the user and it always appended to the loginmsg buffer. 
The buffer_append call should only be called if options.print_lastlog is set.

Proposed solution:

At first I thought it would be sufficient to embed the buffer_append call 
if (options.print_lastlog)
	buffer_append(loginmsg, msg, strlen(msg));

And to add the following to port-aix.c
#include "servconf.h"
extern ServerOptions options;

But then compiling other modules (f.e. ssh-keyscan) will fail because of the missing "options" in the openbsd-compat library .
        cc -qlanglvl=extc89 -o ssh-keyscan ssh-keyscan.o -L. -Lopenbsd-compat/ -L/usr/lib -L/usr/lib -q64 -L/usr/lib -blibpath:/usr/lib:/lib:/usr/lib -lssh -lopenbsd-compat -lssh -lcrypto -lz -lksvc -lgssapi_krb5 -lkrb5
ld: 0711-224 WARNING: Duplicate symbol: .bcopy
ld: 0711-345 Use the -bloadmap or -bnoquiet option to obtain more information.
ld: 0711-317 ERROR: Undefined symbol: options

The only solution I currently see for this is to add an additional parameter (value of options.print_lastlog) to the sys_auth_record_login function in port-aix.c, port-aix.h and auth.c.
auth.c
# ifdef WITH_AIXAUTHENTICATE
        if (authenticated)
                sys_auth_record_login(authctxt->user,
                    get_canonical_hostname(options.use_dns), "ssh", &loginmsg, options.print_lastlog);
# endif

port-aix.c
int
sys_auth_record_login(const char *user, const char *host, const char *ttynm,
    Buffer *loginmsg, int print_lastlog)
{
...
	if(print_lastlog == 1)
		buffer_append(loginmsg, msg, strlen(msg));
	xfree(msg);
	msg_done = 1;
...
}

I uploaded some patches.
Comment 1 Miguel Sanders 2009-05-04 02:11:05 AEST
Created attachment 1632 [details]
port-aix.c patch
Comment 2 Miguel Sanders 2009-05-04 02:17:05 AEST
Created attachment 1633 [details]
port-aix.h patch
Comment 3 Darren Tucker 2009-05-04 13:06:26 AEST
Created attachment 1634 [details]
Updated patch: loginrec.c plus minor style nits

This looks mostly reasonable.  There's a call in loginrec.c that you didn't patch (I suspect you just didn't attach it, it would cause a compile error) and a few minor style nits (line length, spacing, nothing major).

I did consider having a function in port-aix.c return the login message instead and using the !print_lastlog logic in store_lastlog_message() but that's shared code.

BTW these are easier to read if you append related diffs together and attach them as a unit.
Comment 4 Darren Tucker 2009-05-04 13:13:37 AEST
Created attachment 1635 [details]
add missing argument in port-aix.c

oops, left off the argument in port-aix.c while I was playing with the diff.
Comment 5 Darren Tucker 2009-05-04 13:55:28 AEST
Created attachment 1636 [details]
option 2: leave print_lastlog logic all in sshlogin.c

This is an implementation of the other option which has less layering violations but more code.  I'm not sure which I prefer.  djm?

Note: both patches are currently untested (my AIX box is currently offline).
Comment 6 Miguel Sanders 2009-05-04 15:59:54 AEST
Strangely enough, the login_write call from loginrec.c never gets called. That's also the reason why I didn't have to alter it.
The way I see it:
auth_log (auth.c) calls sys_auth_record_login (port-aix.c) directly without using the generic login recording functions of loginrec.c. As a result I only had to modify auth.c,port-aix.h and port-aix.c (tested it and works well).
Comment 7 Damien Miller 2009-07-31 12:12:55 AEST
I prefer option 2
Comment 8 Darren Tucker 2009-08-17 09:40:59 AEST
option 2 applied, it will be in the 5.3p1 release.  Thanks for the report.
Comment 9 Damien Miller 2009-10-06 15:02:46 AEDT
Mass move of RESOLVED bugs to CLOSED now that 5.3 is out.