Warning about invalid .pgpass passwords
in [PgSql]
|
Prev: Access violation from palloc, Visual Studio 2005, C-language function
Next: [HACKERS] walreceiver is uninterruptible on win32
From: Andrew Dunstan on 11 Mar 2010 22:39 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. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Bruce Momjian on 12 Mar 2010 11:54 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(a)momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com PG East: http://www.enterprisedb.com/community/nav-pg-east-2010.do
From: Bruce Momjian on 13 Mar 2010 09:56
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(a)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(a)postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers -- Bruce Momjian <bruce(a)momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com PG East: http://www.enterprisedb.com/community/nav-pg-east-2010.do -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers |