Re: Warning about invalid .pgpass passwords - Mailing list pgsql-hackers

Andrew Dunstan wrote:
>
>
> Bruce Momjian wrote:
> > +     /* If it was 'invalid authorization', add .pgpass mention */
> > +     if (conn->dot_pgpass_used && conn->password_needed && conn->result &&
> > +         /* only works with >= 9.0 servers */
> > +         strcmp(PQresultErrorField(conn->result, PG_DIAG_SQLSTATE),
> > +             ERRCODE_INVALID_PASSWORD_SPECIFICATION) == 0)
> > +         appendPQExpBufferStr(&conn->errorMessage,
> > +             libpq_gettext("password retrieved from .pgpass\n"));
> >
>
> Surely we should use the name of the actual file from which the password
> was retrieved here, which could be quite different from ".pgpass" (see
> PGPASSFILE environment setting) and is different by default on Windows
> anyway. Using a hardcoded ".pgpass" in those situations could be quite
> confusing.

Agreed, very good idea.  Update patch attached.

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  PG East:  http://www.enterprisedb.com/community/nav-pg-east-2010.do
Index: doc/src/sgml/errcodes.sgml
===================================================================
RCS file: /cvsroot/pgsql/doc/src/sgml/errcodes.sgml,v
retrieving revision 1.28
diff -c -c -r1.28 errcodes.sgml
*** doc/src/sgml/errcodes.sgml    7 Dec 2009 05:22:21 -0000    1.28
--- doc/src/sgml/errcodes.sgml    12 Mar 2010 16:53:22 -0000
***************
*** 761,766 ****
--- 761,772 ----
  <entry>invalid_authorization_specification</entry>
  </row>

+ <row>
+ <entry><literal>28P01</literal></entry>
+ <entry>INVALID PASSWORD</entry>
+ <entry>invalid_password</entry>
+ </row>
+

  <row>
  <entry spanname="span13"><emphasis role="bold">Class 2B — Dependent Privilege Descriptors Still
Exist</></entry>
Index: src/backend/libpq/auth.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/libpq/auth.c,v
retrieving revision 1.195
diff -c -c -r1.195 auth.c
*** src/backend/libpq/auth.c    26 Feb 2010 02:00:42 -0000    1.195
--- src/backend/libpq/auth.c    12 Mar 2010 16:53:24 -0000
***************
*** 232,238 ****
  auth_failed(Port *port, int status)
  {
      const char *errstr;
!
      /*
       * If we failed due to EOF from client, just quit; there's no point in
       * trying to send a message to the client, and not much point in logging
--- 232,239 ----
  auth_failed(Port *port, int status)
  {
      const char *errstr;
!     int        errcode_return = ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION;
!
      /*
       * If we failed due to EOF from client, just quit; there's no point in
       * trying to send a message to the client, and not much point in logging
***************
*** 269,274 ****
--- 270,277 ----
          case uaMD5:
          case uaPassword:
              errstr = gettext_noop("password authentication failed for user \"%s\"");
+             /* We use it to indicate if a .pgpass password failed. */
+             errcode_return = ERRCODE_INVALID_PASSWORD;
              break;
          case uaPAM:
              errstr = gettext_noop("PAM authentication failed for user \"%s\"");
***************
*** 285,291 ****
      }

      ereport(FATAL,
!             (errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
               errmsg(errstr, port->user_name)));
      /* doesn't return */
  }
--- 288,294 ----
      }

      ereport(FATAL,
!             (errcode(errcode_return),
               errmsg(errstr, port->user_name)));
      /* doesn't return */
  }
Index: src/include/utils/errcodes.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/utils/errcodes.h,v
retrieving revision 1.31
diff -c -c -r1.31 errcodes.h
*** src/include/utils/errcodes.h    2 Jan 2010 16:58:10 -0000    1.31
--- src/include/utils/errcodes.h    12 Mar 2010 16:53:24 -0000
***************
*** 194,199 ****
--- 194,200 ----

  /* Class 28 - Invalid Authorization Specification */
  #define ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION MAKE_SQLSTATE('2','8', '0','0','0')
+ #define ERRCODE_INVALID_PASSWORD MAKE_SQLSTATE('2','8', 'P','0','1')

  /* Class 2B - Dependent Privilege Descriptors Still Exist */
  #define ERRCODE_DEPENDENT_PRIVILEGE_DESCRIPTORS_STILL_EXIST        MAKE_SQLSTATE('2','B', '0','0','0')
Index: src/interfaces/libpq/fe-connect.c
===================================================================
RCS file: /cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.389
diff -c -c -r1.389 fe-connect.c
*** src/interfaces/libpq/fe-connect.c    3 Mar 2010 20:31:09 -0000    1.389
--- src/interfaces/libpq/fe-connect.c    12 Mar 2010 16:53:25 -0000
***************
*** 91,96 ****
--- 91,99 ----
   */
  #define ERRCODE_APPNAME_UNKNOWN "42704"

+ /* This is part of the protocol so just define it */
+ #define ERRCODE_INVALID_PASSWORD "28P01"
+
  /*
   * fall back options if they are not specified by arguments or defined
   * by environment variables
***************
*** 284,289 ****
--- 287,294 ----
  static char *pwdfMatchesString(char *buf, char *token);
  static char *PasswordFromFile(char *hostname, char *port, char *dbname,
                   char *username);
+ static bool getPgPassFilename(char *pgpassfile);
+ static void dot_pg_pass_warning(PGconn *conn);
  static void default_threadlock(int acquire);


***************
*** 652,657 ****
--- 657,664 ----
                                          conn->dbName, conn->pguser);
          if (conn->pgpass == NULL)
              conn->pgpass = strdup(DefaultPassword);
+         else
+             conn->dot_pgpass_used = true;
      }

      /*
***************
*** 2133,2138 ****
--- 2140,2147 ----

  error_return:

+     dot_pg_pass_warning(conn);
+
      /*
       * We used to close the socket at this point, but that makes it awkward
       * for those above us if they wish to remove this socket from their own
***************
*** 2191,2196 ****
--- 2200,2206 ----
      conn->verbosity = PQERRORS_DEFAULT;
      conn->sock = -1;
      conn->password_needed = false;
+     conn->dot_pgpass_used = false;
  #ifdef USE_SSL
      conn->allow_ssl_try = true;
      conn->wait_ssl_try = false;
***************
*** 4323,4329 ****
      FILE       *fp;
      char        pgpassfile[MAXPGPATH];
      struct stat stat_buf;
-     char       *passfile_env;

  #define LINELEN NAMEDATALEN*5
      char        buf[LINELEN];
--- 4333,4338 ----
***************
*** 4349,4365 ****
      if (port == NULL)
          port = DEF_PGPORT_STR;

!     if ((passfile_env = getenv("PGPASSFILE")) != NULL)
!         /* use the literal path from the environment, if set */
!         strlcpy(pgpassfile, passfile_env, sizeof(pgpassfile));
!     else
!     {
!         char        homedir[MAXPGPATH];
!
!         if (!pqGetHomeDirectory(homedir, sizeof(homedir)))
!             return NULL;
!         snprintf(pgpassfile, MAXPGPATH, "%s/%s", homedir, PGPASSFILE);
!     }

      /* If password file cannot be opened, ignore it. */
      if (stat(pgpassfile, &stat_buf) != 0)
--- 4358,4365 ----
      if (port == NULL)
          port = DEF_PGPORT_STR;

!     if (!getPgPassFilename(pgpassfile))
!         return NULL;

      /* If password file cannot be opened, ignore it. */
      if (stat(pgpassfile, &stat_buf) != 0)
***************
*** 4426,4431 ****
--- 4426,4476 ----
  #undef LINELEN
  }

+
+ static bool getPgPassFilename(char *pgpassfile)
+ {
+     char       *passfile_env;
+
+     if ((passfile_env = getenv("PGPASSFILE")) != NULL)
+         /* use the literal path from the environment, if set */
+         strlcpy(pgpassfile, passfile_env, MAXPGPATH);
+     else
+     {
+         char        homedir[MAXPGPATH];
+
+         if (!pqGetHomeDirectory(homedir, sizeof(homedir)))
+             return false;
+         snprintf(pgpassfile, MAXPGPATH, "%s/%s", homedir, PGPASSFILE);
+     }
+     return true;
+ }
+
+ /*
+  *    If the connection failed, we should mention if
+  *    we got the password from .pgpass in case that
+  *    password is wrong.
+  */
+ static void
+ dot_pg_pass_warning(PGconn *conn)
+ {
+     /* If it was 'invalid authorization', add .pgpass mention */
+     if (conn->dot_pgpass_used && conn->password_needed && conn->result &&
+         /* only works with >= 9.0 servers */
+         strcmp(PQresultErrorField(conn->result, PG_DIAG_SQLSTATE),
+             ERRCODE_INVALID_PASSWORD) == 0)
+     {
+         char        pgpassfile[MAXPGPATH];
+
+         if (!getPgPassFilename(pgpassfile))
+             return;
+         appendPQExpBufferStr(&conn->errorMessage,
+             libpq_gettext("password retrieved from "));
+         appendPQExpBufferStr(&conn->errorMessage, pgpassfile);
+         appendPQExpBufferChar(&conn->errorMessage, '\n');
+     }
+ }
+
+
  /*
   * Obtain user's home directory, return in given buffer
   *
Index: src/interfaces/libpq/libpq-int.h
===================================================================
RCS file: /cvsroot/pgsql/src/interfaces/libpq/libpq-int.h,v
retrieving revision 1.149
diff -c -c -r1.149 libpq-int.h
*** src/interfaces/libpq/libpq-int.h    26 Feb 2010 02:01:33 -0000    1.149
--- src/interfaces/libpq/libpq-int.h    12 Mar 2010 16:53:25 -0000
***************
*** 343,348 ****
--- 343,349 ----
      ProtocolVersion pversion;    /* FE/BE protocol version in use */
      int            sversion;        /* server version, e.g. 70401 for 7.4.1 */
      bool        password_needed;    /* true if server demanded a password */
+     bool        dot_pgpass_used;    /* true if used .pgpass */
      bool        sigpipe_so;        /* have we masked SIGPIPE via SO_NOSIGPIPE? */
      bool        sigpipe_flag;    /* can we mask SIGPIPE via MSG_NOSIGNAL? */

Index: src/pl/plpgsql/src/plerrcodes.h
===================================================================
RCS file: /cvsroot/pgsql/src/pl/plpgsql/src/plerrcodes.h,v
retrieving revision 1.20
diff -c -c -r1.20 plerrcodes.h
*** src/pl/plpgsql/src/plerrcodes.h    2 Jan 2010 16:58:13 -0000    1.20
--- src/pl/plpgsql/src/plerrcodes.h    12 Mar 2010 16:53:25 -0000
***************
*** 368,373 ****
--- 368,377 ----
  },

  {
+     "invalid_password", ERRCODE_INVALID_PASSWORD
+ },
+
+ {
      "dependent_privilege_descriptors_still_exist", ERRCODE_DEPENDENT_PRIVILEGE_DESCRIPTORS_STILL_EXIST
  },