Better detail logging for password auth failures - Mailing list pgsql-hackers

We often tell people to look in the postmaster log for more information
about authentication problems; but an off-list question prompted me to
notice that many of the common authentication failure cases don't actually
get any useful commentary in the log.  The attached proposed patch
remedies this by adding specific log detail messages for all the
non-out-of-memory cases processed by md5_crypt_verify().  Essentially,
this is just covering cases that I omitted to cover in commit 64e43c59,
for reasons that no longer seem very good to me.

I did not bother with going through the other auth methods in similar
detail.  It seems like only password authentication is in use by people
who are in need of this kind of help.  (But if someone else wants to do
something similar for other auth methods, feel free.)

In passing, the patch gets rid of a vestigial CHECK_FOR_INTERRUPTS()
call; it was added by e710b65c and IMO should have been removed again
by 6647248e.  There's certainly no very good reason to have one right
at that spot anymore.

Any objections?

            regards, tom lane

diff --git a/src/backend/libpq/crypt.c b/src/backend/libpq/crypt.c
index 97be944..64c25e8 100644
*** a/src/backend/libpq/crypt.c
--- b/src/backend/libpq/crypt.c
*************** md5_crypt_verify(const Port *port, const
*** 50,56 ****
--- 50,60 ----
      /* Get role info from pg_authid */
      roleTup = SearchSysCache1(AUTHNAME, PointerGetDatum(role));
      if (!HeapTupleIsValid(roleTup))
+     {
+         *logdetail = psprintf(_("Role \"%s\" does not exist."),
+                               role);
          return STATUS_ERROR;    /* no such user */
+     }

      datum = SysCacheGetAttr(AUTHNAME, roleTup,
                              Anum_pg_authid_rolpassword, &isnull);
*************** md5_crypt_verify(const Port *port, const
*** 71,83 ****
      ReleaseSysCache(roleTup);

      if (*shadow_pass == '\0')
          return STATUS_ERROR;    /* empty password */
!
!     CHECK_FOR_INTERRUPTS();

      /*
       * Compare with the encrypted or plain password depending on the
!      * authentication method being used for this connection.
       */
      switch (port->hba->auth_method)
      {
--- 75,92 ----
      ReleaseSysCache(roleTup);

      if (*shadow_pass == '\0')
+     {
+         *logdetail = psprintf(_("User \"%s\" has an empty password."),
+                               role);
          return STATUS_ERROR;    /* empty password */
!     }

      /*
       * Compare with the encrypted or plain password depending on the
!      * authentication method being used for this connection.  (We do not
!      * bother setting logdetail for pg_md5_encrypt failure: the only possible
!      * error is out-of-memory, which is unlikely, and if it did happen adding
!      * a psprintf call would only make things worse.)
       */
      switch (port->hba->auth_method)
      {
*************** md5_crypt_verify(const Port *port, const
*** 154,159 ****
--- 163,171 ----
          else
              retval = STATUS_OK;
      }
+     else
+         *logdetail = psprintf(_("Password does not match for user \"%s\"."),
+                               role);

      if (port->hba->auth_method == uaMD5)
          pfree(crypt_pwd);