svn commit: r313965 - head/crypto/openssh

[Ian Lepore]

On Sun, 2017-02-19 at 18:06 -0500, Kurt Lidl wrote:
> On 2/19/17 4:42 PM, Oliver Pinter wrote:
> > 
> > Hello!
> > 
> > On 2/19/17, Kurt Lidl <lidl at freebsd.org> wrote:
> > > 
> > > Author: lidl
> > > Date: Sun Feb 19 20:35:39 2017
> > > New Revision: 313965
> > > URL: https://svnweb.freebsd.org/changeset/base/313965
> > > 
> > > Log:
> > >   Only notify blacklistd for successful logins in auth.c
> > What's the rationale behind this change?
> Without this change, every pass through auth.c results in a
> call to blacklist_notify().
> 
> So, in a normal remote login, you'd get a failed
> login flagged for the printing of the "xxx login:" prompt,
> before the remote user could enter a password.
> 
> If the user successfully entered a good password,
> you'd get a good login flagged, and everything would be OK.
> 
> If the user entered an incorrect password, you'd get
> another failed login in auth1.c (or auth2.c), and finally,
> when sshd got around to issuing the second "xxx login:"
> prompt, you'd have yet another failed login notice sent
> to blacklistd.
> 
> So, if you had 3 bad logins set to the limit, you'd actually
> be blocking the address after the first bad login attempt.
> 
> -Kurt
> 

I would contend that this explanation, exactly as written, should have
been part of the commit message.  It's a perfect example of explaining
*why* a change was made, instead of just saying what was changed.

-- Ian

> > 
> > 
> > > 
> > > 
> > >   Reported by:	Rick Adams
> > >   Reviewed by:	des
> > >   MFC after:	3 days
> > >   Sponsored by:	The FreeBSD Foundation
> > > 
> > > Modified:
> > >   head/crypto/openssh/auth.c
> > > 
> > > Modified: head/crypto/openssh/auth.c
> > > =================================================================
> > > =============
> > > --- head/crypto/openssh/auth.c	Sun Feb 19 19:56:12 2017	
> > > (r313964)
> > > +++ head/crypto/openssh/auth.c	Sun Feb 19 20:35:39 2017	
> > > (r313965)
> > > @@ -295,8 +295,8 @@ auth_log(Authctxt *authctxt, int authent
> > >  		authmsg = "Partial";
> > >  	else {
> > >  		authmsg = authenticated ? "Accepted" : "Failed";
> > > -		BLACKLIST_NOTIFY(authenticated ?
> > > -		    BLACKLIST_AUTH_OK : BLACKLIST_AUTH_FAIL);
> > > +		if (authenticated)
> > > +			BLACKLIST_NOTIFY(BLACKLIST_AUTH_OK);
> > >  	}
> > > 
> > >  	authlog("%s %s%s%s for %s%.100s from %.200s port %d
> > > %s%s%s",
> > > _______________________________________________
> > > svn-src-head at freebsd.org mailing list
> > > https://lists.freebsd.org/mailman/listinfo/svn-src-head
> > > To unsubscribe, send any mail to "svn-src-head-unsubscribe at freebs
> > > d.org"
> > > 
>