Re: Change authentication error message (patch)

On Fri, Jan 24, 2014 at 10:10:00AM -0500, Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)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 required both 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(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +