Bruce Momjian wrote:
> Tom Lane wrote:
> > Bruce Momjian <br...@momjian.us> writes:
> > > Tom Lane wrote:
> > >> I'm inclined to think that maybe we should make the server return a
> > >> distinct SQLSTATE for "bad password", and have libpq check for that
> > >> rather than just assuming that the failure must be bad password.
> >
> > > Modifying the backend to issue this hint seems like overkill, unless we
> > > have some other use for it.
> >
> > I wouldn't suggest it if I thought it were only helpful for this
> > particular message. It seems to me that we've spent a lot of time
> > kluging around the lack of certainty about whether a connection failure
> > is a password issue. Admittedly a lot of that was between libpq and its
> > client, but the state of affairs on the wire isn't great either.
>
> Yes, I have seen that myself in psql.
>
> > I'm not convinced we have to do it that way, but now is definitely
> > the time to think about it before we implement yet another
> > sort-of-good-enough kluge. Which is what this is.
>
> True. Should we just hold this all for 9.1 or should I code it and
> let's look at the size of the patch?
With no one replying, I decide to code up a patch that adds a new
SQLSTATE (28001) to report invalid/missing passwords. With this code,
the warning will only appear when connecting to 9.0 servers. The output
still looks the same, but will only appear for a password failure:
$ sql -h localhost test
psql: FATAL: password authentication failed for user "postgres"
password retrieved from .pgpass
--
Bruce Momjian <br...@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 11 Mar 2010 21:07:09 -0000
***************
*** 761,766 ****
--- 761,772 ----
<entry>invalid_authorization_specification</entry>
</row>
+ <row>
+ <entry><literal>28001</literal></entry>
+ <entry>INVALID PASSWORD SPECIFICATION</entry>
+ <entry>invalid_password_specification</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 11 Mar 2010 21:07:16 -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,280 ----
case uaMD5:
case uaPassword:
errstr = gettext_noop("password authentication failed for user \"%s\"");
+ /*
+ * This might require the client to prompt for a password, so we use
+ * a distinct error code.
+ */
+ errcode_return = ERRCODE_INVALID_PASSWORD_SPECIFICATION;
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 */
}
--- 291,297 ----
}
ereport(FATAL,
! (errcode(errcode_return),
errmsg(errstr, port->user_name)));
/* doesn't return */
}
Index: src/include/utils/elog.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/utils/elog.h,v
retrieving revision 1.102
diff -c -c -r1.102 elog.h
*** src/include/utils/elog.h 2 Jan 2010 16:58:10 -0000 1.102
--- src/include/utils/elog.h 11 Mar 2010 21:07:16 -0000
***************
*** 61,66 ****
--- 61,73 ----
(PGSIXBIT(ch1) + (PGSIXBIT(ch2) << 6) + (PGSIXBIT(ch3) << 12) + \
(PGSIXBIT(ch4) << 18) + (PGSIXBIT(ch5) << 24))
+ #define SQLSTATE_TO_BASE10(state) \
+ (((state) & 0x3F) * 10000 + \
+ (((state) >> 6) & 0x3F) * 1000 + \
+ (((state) >> 12) & 0x3F) * 100 + \
+ (((state) >> 18) & 0x3F) * 10 + \
+ (((state) >> 24) & 0x3F))
+
/* These macros depend on the fact that '0' becomes a zero in SIXBIT */
#define ERRCODE_TO_CATEGORY(ec) ((ec) & ((1 << 12) - 1))
#define ERRCODE_IS_CATEGORY(ec) (((ec) & ~((1 << 12) - 1)) == 0)
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 11 Mar 2010 21:07:16 -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_SPECIFICATION MAKE_SQLSTATE('2','8', '0','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 11 Mar 2010 21:07:17 -0000
***************
*** 71,76 ****
--- 71,78 ----
#include "libpq/ip.h"
#include "mb/pg_wchar.h"
+ #include "utils/elog.h"
+ #include "utils/errcodes.h"
#ifndef FD_CLOEXEC
#define FD_CLOEXEC 1
***************
*** 284,289 ****
--- 286,292 ----
static char *pwdfMatchesString(char *buf, char *token);
static char *PasswordFromFile(char *hostname, char *port, char *dbname,
char *username);
+ static void dot_pg_pass_warning(PGconn *conn);
static void default_threadlock(int acquire);
***************
*** 652,657 ****
--- 655,662 ----
conn->dbName, conn->pguser);
if (conn->pgpass == NULL)
conn->pgpass = strdup(DefaultPassword);
+ else
+ conn->dot_pgpass_used = true;
}
/*
***************
*** 2133,2138 ****
--- 2138,2145 ----
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 ****
--- 2198,2204 ----
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;
***************
*** 4426,4431 ****
--- 4434,4458 ----
#undef LINELEN
}
+
+ /*
+ * 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 */
+ atoi(PQresultErrorField(conn->result, PG_DIAG_SQLSTATE)) ==
+ SQLSTATE_TO_BASE10(ERRCODE_INVALID_PASSWORD_SPECIFICATION))
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("password retrieved from .pgpass\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 11 Mar 2010 21:07:19 -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 11 Mar 2010 21:07:19 -0000
***************
*** 368,373 ****
--- 368,377 ----
},
{
+ "invalid_password_specification", ERRCODE_INVALID_PASSWORD_SPECIFICATION
+ },
+
+ {
"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
