Re: Change authentication error message (patch)

On Fri, Jan 24, 2014 at 10:10:00AM -0500, Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > On Thu, Jan 23, 2014 at 10:39:34PM -0500, Tom Lane wrote:
> >> I'm not convinced that this improves anything.  The problem might not in
> >> fact be either of the things you mention, in which case the new message 
> >> is outright misleading.  Also, what of the policy stated in the header
> >> comment for the function you're hacking, ie we intentionally don't reveal
> >> the precise cause of the failure to the client?
> 
> > Well, the only solution then would be to add some weasel words like
> > "perhaps expired password", but that seems so rare that I doubt it would
> > apply very often and seems like an odd suggestion.   We could go with:
> 
> >     password authentication failed for user \"%s\": perhaps invalid or expired password
> 
> > We did have two threads on this issue in the past 12 months so I figured
> > we should try to do something.
> 
> I agree with doing *something*, but this particular thing seems to violate
> our very long-standing policy on how to deal with authentication failures,
> as well as being too vague to be really useful.
> 
> What would be well within that policy is to log additional information
> into the postmaster log.  I see that md5_crypt_verify knows perfectly
> well whether the problem is no password set, wrong password, or expired
> password.  I don't see anything wrong with having it emit a log entry
> --- maybe not in the second case for fear of log-spam complaints, but
> certainly the third case and maybe the first one.  Or possibly cleaner,
> have it return additional status so that auth_failed() can include the
> info in the main ereport using errdetail_log().

I was afraid that PGOPTIONS='-c client_min_messages=log' would allow
clients to see the log messages, but in testing I found we don't show
them during authentication, and I found this C comment:
        * client_min_messages is honored only after we complete the        * authentication handshake.  This is
requiredboth for security        * reasons and because many clients can't handle NOTICE messages        * during
authentication.

I like the 'LOG' idea very much, and liked your patch too.  :-)

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com
 + Everyone has their own god. +