Re: Warning about invalid .pgpass passwords

Applied.

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

Bruce Momjian wrote:
> 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

[ text/x-diff is unsupported, treating like TEXT/PLAIN ]

> 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
>   },
>   

> 
> -- 
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com
 PG East:  http://www.enterprisedb.com/community/nav-pg-east-2010.do