[openssh-unix-dev] auth*.c

[mandar at webchat.chatsystems.com]

Dave - great to hear someone already working on this ;) Re: #1, how do you
increment unsuccessful_login_count for non-password method failures?

Re: /etc/nologin, why not stat() the file before calling
loginrestrictions()?

- Mandar

On Fri, 28 Dec 2001, David Bronder wrote:

> Date: Fri, 28 Dec 2001 11:26:23 -0600 (CST)
> From: David Bronder <david-bronder at uiowa.edu>
> To: openssh-unix-dev at mindrot.org
> Cc: mandar at webchat.chatsystems.com
> Subject: Re: [openssh-unix-dev] auth*.c
>
> I've been working on some patches to address these same issues
> with OpenSSH and AIX.  In the process, though, I've uncovered
> some further complications.
>
> The current incarnation of my patch does the following things:
>
>   * Moves the AIX loginfailed() call into the auth_log() call as
>     in your point (2) below.  This effectively addresses your point
>     (1) as well; your fix for that point doesn't cover it since the
>     client decides how many or few authentication attempts to make
>     (the client may try fewer methods or fewer retries).
>
>   * Moves the AIX loginsuccess() call down in session.c to after the
>     record_login() call, which made more sense to me.
>
> I haven't submitted the patch yet because of new problems that it
> uncovered.  The problems have to do with how /etc/nologin is handled
> under AIX in particular, and "invalid" users in general.  If these
> problems can be resolved (or maybe even if they can't), I'll post
> the patch after I've finished testing.
>
> The first problem is that the AIX loginrestrictions() call returns
> failure if /etc/nologin exists and the user is not root.  So in
> OpenSSH, a non-root user will be marked as invalid, and will never
> reach the normal nologin handling.  There is no way to tell from
> loginrestrictions() what condition(s) caused the failure.  So the
> connection attempt will fail but the client will get no indication
> of why.
>
> The other problem is that OpenSSH allows an invalid user to continue
> retrying authentication, even though all the authentication methods
> immediately fail when authctxt->valid is false or authctxt->pw is
> NULL.  In the case of AIX and /etc/nologin, the user inflates the
> unsuccessful login counter, but gets no feedback as to what's going
> on.
>
> My question to the developers is this:  Should login attempts by an
> invalid user behave this way?  Or should the invalid user check be
> made after a successful authentication instead of before, and then
> cause the disconnect?  The latter seems more correct to me.
>
> Also, I'll take any advice offered on how to handle the /etc/nologin
> feedback issue under AIX...
>
> =Dave
>
> mandar at webchat.chatsystems.com wrote:
> >
> > During testing, we found a couple of issues with openssh3.0.2p1:
> >
> > 1. In userauth_finish() in auth2.c (as well as in do_authloop in auth1.c),
> > the foll. check:
> >
> >   if (authctxt->failures++ > AUTH_FAIL_MAX)
> >
> > is never satisfied and thus packet_disconnect() never gets called. I
> > suspect the code just drops out of the dispatch_run function list instead.
> > This should be an == instead of >. While looking at the debug output
> > when deliberately entering wrong passwords, I noticed one try for none,
> > three for password, and then three for keyboard-interactive, at which point
> > authctxt->failures is 6, and then the loop completes.
> >
> > 2. I'd like to move loginfailed() within the #ifdef WITH_AIXAUTHENTICATE
> > of auth1.c and auth2.c to auth_log() instead, and call it on every
> > password method failure, as well as an overall authctxt->failures ==
> > AUTH_FAIL_MAX check for the other methods. This should clean up the code a
> > bit, and should fix the issue of the unsuccessful login counter not being
> > incremented on each unsucessful try.
>
>
> --
> Hello World.                                    David Bronder - Systems Admin
> Segmentation Fault                                     ITS-SPA, Univ. of Iowa
> Core dumped, disk trashed, quota filled, soda warm.   david-bronder at uiowa.edu
>