Re: elog() patch - Mailing list pgsql-hackers

Here is a better patch I am inclined to apply.  I fixes the debug
messages during authentication problem in a cleaner way, and removes
password echo to server logs and client.

---------------------------------------------------------------------------

Bruce Momjian wrote:
> Tom Lane wrote:
> > Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > > Is this what you were looking for?  I set client_min_messages to the max
> > > of debug5 and the output is attached.
> >
> > If the DBA wants to do that, I don't have a problem with it.  I'm
> > wondering what happens if an unprivileged user tries to do it,
> > via either PGOPTIONS or Peter's new user/database-local options.
> >
> > Please note also that I'm wondering about the messages emitted during
> > an authorization *failure*, not a successful connection.
>
> You ask a very good question here.  I never tested authentication with
> debug sent to the client.  The answer is that it doesn't work without
> the attached patch.  Now, I am not about to apply this because it does
> change getNotice() to an extern and moves its prototype to libpq-int.h.
> This is necessary because I now use getNotice() in fe-connect.c.
>
> The second issue is that this isn't going to work for pre-7.2 clients
> because the protocol doesn't expect 'N' messages during the
> authentication phase.  I think we can live with a client_min_messages
> level of debug* not working on old clients, though we should make a
> mention of it in the release notes.
>
> And finally, here is the output from a failed password login with the
> patch applied:
>
>     $ psql test
>     Password:
>     DEBUG:  received password packet with len=12, pw=lkjasdf
>
>     DEBUG:  received password packet with len=12, pw=lkjasdf
>
>     psql: FATAL:  Password authentication failed for user "postgres"
>
> Basically it echoes the failed password back to the user.  Again, this
> is only with client_min_messages set to debug1-5.  I don't know how to
> fix this because we specifically set things up so the client could see
> everything the server logs see.  I wonder if echoing the failed password
> into the logs is a good idea either.  I don't think so.
>
> Someone please advise on patch application.  Are there other places that
> don't expect a NOTICE in the middle of a protocol handshake?

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
Index: src/backend/libpq/auth.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/libpq/auth.c,v
retrieving revision 1.76
diff -c -r1.76 auth.c
*** src/backend/libpq/auth.c    2 Mar 2002 21:39:25 -0000    1.76
--- src/backend/libpq/auth.c    3 Mar 2002 21:39:40 -0000
***************
*** 854,861 ****
          return STATUS_EOF;
      }

!     elog(DEBUG5, "received password packet with len=%d, pw=%s\n",
!         len, buf.data);

      result = checkPassword(port, port->user, buf.data);
      pfree(buf.data);
--- 854,861 ----
          return STATUS_EOF;
      }

!     /* For security reasons, do not output contents of password packet */
!     elog(DEBUG5, "received password packet");

      result = checkPassword(port, port->user, buf.data);
      pfree(buf.data);
Index: src/interfaces/libpq/fe-connect.c
===================================================================
RCS file: /cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.182
diff -c -r1.182 fe-connect.c
*** src/interfaces/libpq/fe-connect.c    2 Mar 2002 00:49:22 -0000    1.182
--- src/interfaces/libpq/fe-connect.c    3 Mar 2002 21:39:42 -0000
***************
*** 1296,1301 ****
--- 1296,1310 ----
                      return PGRES_POLLING_READING;
                  }

+                 /* Grab NOTICE/INFO/DEBUG and discard them. */
+                 while (beresp == 'N')
+                 {
+                     if (getNotice(conn))
+                         return PGRES_POLLING_READING;
+                     if (pqGetc(&beresp, conn))
+                         return PGRES_POLLING_READING;
+                 }
+
                  /* Handle errors. */
                  if (beresp == 'E')
                  {
Index: src/interfaces/libpq/fe-exec.c
===================================================================
RCS file: /cvsroot/pgsql/src/interfaces/libpq/fe-exec.c,v
retrieving revision 1.113
diff -c -r1.113 fe-exec.c
*** src/interfaces/libpq/fe-exec.c    25 Oct 2001 05:50:13 -0000    1.113
--- src/interfaces/libpq/fe-exec.c    3 Mar 2002 21:39:44 -0000
***************
*** 54,60 ****
  static int    getRowDescriptions(PGconn *conn);
  static int    getAnotherTuple(PGconn *conn, int binary);
  static int    getNotify(PGconn *conn);
- static int    getNotice(PGconn *conn);

  /* ---------------
   * Escaping arbitrary strings to get valid SQL strings/identifiers.
--- 54,59 ----
***************
*** 1379,1385 ****
   * Exit: returns 0 if successfully consumed Notice message.
   *         returns EOF if not enough data.
   */
! static int
  getNotice(PGconn *conn)
  {
      /*
--- 1378,1384 ----
   * Exit: returns 0 if successfully consumed Notice message.
   *         returns EOF if not enough data.
   */
! int
  getNotice(PGconn *conn)
  {
      /*
Index: src/interfaces/libpq/libpq-int.h
===================================================================
RCS file: /cvsroot/pgsql/src/interfaces/libpq/libpq-int.h,v
retrieving revision 1.44
diff -c -r1.44 libpq-int.h
*** src/interfaces/libpq/libpq-int.h    5 Nov 2001 17:46:38 -0000    1.44
--- src/interfaces/libpq/libpq-int.h    3 Mar 2002 21:39:44 -0000
***************
*** 305,310 ****
--- 305,311 ----
  extern void *pqResultAlloc(PGresult *res, size_t nBytes, bool isBinary);
  extern char *pqResultStrdup(PGresult *res, const char *str);
  extern void pqClearAsyncResult(PGconn *conn);
+ extern int    getNotice(PGconn *conn);

  /* === in fe-misc.c === */