Fix up the PQconnectionUsedPassword mess: create a separate
authorTom Lane <tgl@sss.pgh.pa.us>
Sun, 9 Dec 2007 19:01:40 +0000 (19:01 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Sun, 9 Dec 2007 19:01:40 +0000 (19:01 +0000)
PQconnectionNeedsPassword function that tells the right thing for whether to
prompt for a password, and improve PQconnectionUsedPassword so that it checks
whether the password used by the connection was actually supplied as a
connection argument, instead of coming from environment or a password file.
Per bug report from Mark Cave-Ayland and subsequent discussion.

13 files changed:
doc/src/sgml/libpq.sgml
doc/src/sgml/release.sgml
src/bin/pg_ctl/pg_ctl.c
src/bin/pg_dump/pg_backup_db.c
src/bin/pg_dump/pg_dumpall.c
src/bin/psql/command.c
src/bin/psql/startup.c
src/bin/scripts/common.c
src/interfaces/libpq/exports.txt
src/interfaces/libpq/fe-auth.c
src/interfaces/libpq/fe-connect.c
src/interfaces/libpq/libpq-fe.h
src/interfaces/libpq/libpq-int.h

index 3acbe2c3d30eb32781993f7284fc44881c169a54..fec498c19a8536cdaff7c8a770dd6d0c46d0cd10 100644 (file)
@@ -1,4 +1,4 @@
-<!-- $PostgreSQL: pgsql/doc/src/sgml/libpq.sgml,v 1.247 2007/11/28 15:42:31 petere Exp $ -->
+<!-- $PostgreSQL: pgsql/doc/src/sgml/libpq.sgml,v 1.248 2007/12/09 19:01:40 tgl Exp $ -->
 
 <chapter id="libpq">
  <title><application>libpq</application> - C Library</title>
@@ -1145,12 +1145,33 @@ typedef struct
      </listitem>
     </varlistentry>
 
+    <varlistentry>
+     <term><function>PQconnectionNeedsPassword</function><indexterm><primary>PQconnectionNeedsPassword</></></term>
+     <listitem>
+      <para>
+       Returns true (1) if the connection authentication method
+       required a password, but none was available.
+       Returns false (0) if not.
+
+       <synopsis>
+        int PQconnectionNeedsPassword(const PGconn *conn);
+       </synopsis>
+
+      </para>
+
+      <para>
+       This function can be applied after a failed connection attempt
+       to decide whether to prompt the user for a password.
+      </para>
+     </listitem>
+    </varlistentry>
+
     <varlistentry>
      <term><function>PQconnectionUsedPassword</function><indexterm><primary>PQconnectionUsedPassword</></></term>
      <listitem>
       <para>
        Returns true (1) if the connection authentication method
-       required a password to be supplied. Returns false (0) if not.
+       used a caller-supplied password. Returns false (0) if not.
 
        <synopsis>
         int PQconnectionUsedPassword(const PGconn *conn);
@@ -1159,9 +1180,10 @@ typedef struct
       </para>
 
       <para>
-       This function can be applied after either successful or failed
-       connection attempts.  In the case of failure, it can for example
-       be used to decide whether to prompt the user for a password.
+       This function detects whether a password supplied to the connection
+       function was actually used.  Passwords obtained from other
+       sources (such as the <filename>.pgpass</> file) are not considered
+       caller-supplied.
       </para>
      </listitem>
     </varlistentry>
index ef4373fe6d646665674a1e538e67547091bda51d..4bb10cbf61102cf43b17e1ffbad07981912f39ef 100644 (file)
@@ -1,4 +1,4 @@
-<!-- $PostgreSQL: pgsql/doc/src/sgml/release.sgml,v 1.563 2007/12/08 17:24:03 momjian Exp $ -->
+<!-- $PostgreSQL: pgsql/doc/src/sgml/release.sgml,v 1.564 2007/12/09 19:01:40 tgl Exp $ -->
 <!--
 
 Typical markup:
@@ -2184,8 +2184,9 @@ current_date &lt; 2017-11-17
 
      <listitem>
       <para>
-       Add <function>PQconnectionUsedPassword()</function> that returns
-       true if the server required a password (Joe Conway)
+       Add <function>PQconnectionNeedsPassword()</function> that returns
+       true if the server required a password but none was supplied
+       (Joe Conway, Tom)
       </para>
 
       <para>
@@ -2197,6 +2198,19 @@ current_date &lt; 2017-11-17
       </para>
      </listitem>
 
+     <listitem>
+      <para>
+       Add <function>PQconnectionUsedPassword()</function> that returns
+       true if the supplied password was actually used
+       (Joe Conway, Tom)
+      </para>
+
+      <para>
+       This is useful in some security contexts where it is important
+       to know whether a user-supplied password is actually valid.
+      </para>
+     </listitem>
+
     </itemizedlist>
 
    </sect3>
index 849287fca7631abe3a5594d492e471652a67f964..b83e52a87bab27ad18faab50e326ec71703fbea4 100644 (file)
@@ -4,7 +4,7 @@
  *
  * Portions Copyright (c) 1996-2007, PostgreSQL Global Development Group
  *
- * $PostgreSQL: pgsql/src/bin/pg_ctl/pg_ctl.c,v 1.90 2007/11/20 19:24:26 momjian Exp $
+ * $PostgreSQL: pgsql/src/bin/pg_ctl/pg_ctl.c,v 1.91 2007/12/09 19:01:40 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -492,7 +492,7 @@ test_postmaster_connection(bool do_checkpoint)
    {
        if ((conn = PQconnectdb(connstr)) != NULL &&
            (PQstatus(conn) == CONNECTION_OK ||
-            PQconnectionUsedPassword(conn)))
+            PQconnectionNeedsPassword(conn)))
        {
            PQfinish(conn);
            success = true;
index 5f8039e98271fcb83f69459d75815929ddb5c74e..d3e7959d8175e5771645c9b6545d0ced6bd55702 100644 (file)
@@ -5,7 +5,7 @@
  * Implements the basic DB functions used by the archiver.
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/bin/pg_dump/pg_backup_db.c,v 1.76 2007/07/08 19:07:38 tgl Exp $
+ *   $PostgreSQL: pgsql/src/bin/pg_dump/pg_backup_db.c,v 1.77 2007/12/09 19:01:40 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -159,7 +159,7 @@ _connectDB(ArchiveHandle *AH, const char *reqdb, const char *requser)
 
        if (PQstatus(newConn) == CONNECTION_BAD)
        {
-           if (!PQconnectionUsedPassword(newConn))
+           if (!PQconnectionNeedsPassword(newConn))
                die_horribly(AH, modulename, "could not reconnect to database: %s",
                             PQerrorMessage(newConn));
            PQfinish(newConn);
@@ -234,7 +234,7 @@ ConnectDatabase(Archive *AHX,
            die_horribly(AH, modulename, "failed to connect to database\n");
 
        if (PQstatus(AH->connection) == CONNECTION_BAD &&
-           PQconnectionUsedPassword(AH->connection) &&
+           PQconnectionNeedsPassword(AH->connection) &&
            password == NULL &&
            !feof(stdin))
        {
index bb01994ebf8c97ba2895fb4c3b2caf6cb22910ba..7ed48d110e58087ccae1d2ce11605ec9a26c79be 100644 (file)
@@ -6,7 +6,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  *
- * $PostgreSQL: pgsql/src/bin/pg_dump/pg_dumpall.c,v 1.98 2007/11/15 21:14:42 momjian Exp $
+ * $PostgreSQL: pgsql/src/bin/pg_dump/pg_dumpall.c,v 1.99 2007/12/09 19:01:40 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1336,7 +1336,7 @@ connectDatabase(const char *dbname, const char *pghost, const char *pgport,
        }
 
        if (PQstatus(conn) == CONNECTION_BAD &&
-           PQconnectionUsedPassword(conn) &&
+           PQconnectionNeedsPassword(conn) &&
            password == NULL &&
            !feof(stdin))
        {
index 2fa68972ff301d38beea8f3f58e180b2c159938f..19d9d5141e76ac00a7e90780cb9de6778bc5548c 100644 (file)
@@ -3,7 +3,7 @@
  *
  * Copyright (c) 2000-2007, PostgreSQL Global Development Group
  *
- * $PostgreSQL: pgsql/src/bin/psql/command.c,v 1.183 2007/11/15 21:14:42 momjian Exp $
+ * $PostgreSQL: pgsql/src/bin/psql/command.c,v 1.184 2007/12/09 19:01:40 tgl Exp $
  */
 #include "postgres_fe.h"
 #include "command.h"
@@ -1164,7 +1164,7 @@ do_connect(char *dbname, char *user, char *host, char *port)
         * Connection attempt failed; either retry the connection attempt with
         * a new password, or give up.
         */
-       if (!password && PQconnectionUsedPassword(n_conn))
+       if (!password && PQconnectionNeedsPassword(n_conn))
        {
            PQfinish(n_conn);
            password = prompt_for_password(user);
index 65c2e1d906d2f7761660afa3280525ce90a98e0e..2305d4bdb11838db9156b969f9300fe8ff4bd811 100644 (file)
@@ -3,7 +3,7 @@
  *
  * Copyright (c) 2000-2007, PostgreSQL Global Development Group
  *
- * $PostgreSQL: pgsql/src/bin/psql/startup.c,v 1.141 2007/07/08 19:07:38 tgl Exp $
+ * $PostgreSQL: pgsql/src/bin/psql/startup.c,v 1.142 2007/12/09 19:01:40 tgl Exp $
  */
 #include "postgres_fe.h"
 
@@ -211,7 +211,7 @@ main(int argc, char *argv[])
                               username, password);
 
        if (PQstatus(pset.db) == CONNECTION_BAD &&
-           PQconnectionUsedPassword(pset.db) &&
+           PQconnectionNeedsPassword(pset.db) &&
            password == NULL &&
            !feof(stdin))
        {
index 5e815e57f4bd23347dbccf53ea62cef9551c2d87..6e20f659a399ad18a1ee31c383de127c17a8ec06 100644 (file)
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2007, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/bin/scripts/common.c,v 1.29 2007/11/15 21:14:42 momjian Exp $
+ * $PostgreSQL: pgsql/src/bin/scripts/common.c,v 1.30 2007/12/09 19:01:40 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -123,7 +123,7 @@ connectDatabase(const char *dbname, const char *pghost, const char *pgport,
        }
 
        if (PQstatus(conn) == CONNECTION_BAD &&
-           PQconnectionUsedPassword(conn) &&
+           PQconnectionNeedsPassword(conn) &&
            password == NULL &&
            !feof(stdin))
        {
index 222a6595b43971fadca5d2d4f5c5e1ee31d5980f..2b7b8fe94b7407876c6968b2a42830601017eb01 100644 (file)
@@ -1,4 +1,4 @@
-# $PostgreSQL: pgsql/src/interfaces/libpq/exports.txt,v 1.17 2007/10/13 20:18:41 tgl Exp $
+# $PostgreSQL: pgsql/src/interfaces/libpq/exports.txt,v 1.18 2007/12/09 19:01:40 tgl Exp $
 # Functions to be exported by libpq DLLs
 PQconnectdb               1
 PQsetdbLogin              2
@@ -139,3 +139,4 @@ PQsendDescribePortal      136
 lo_truncate               137
 PQconnectionUsedPassword  138
 pg_valid_server_encoding_id 139
+PQconnectionNeedsPassword 140
index f3a177c985eea01820a8b8e82ed5202698f48c55..0c0fc1c4908fa33dfcc6eed0819a02aae5d097e4 100644 (file)
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/interfaces/libpq/fe-auth.c,v 1.134 2007/12/04 13:02:53 mha Exp $
+ *   $PostgreSQL: pgsql/src/interfaces/libpq/fe-auth.c,v 1.135 2007/12/09 19:01:40 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -954,7 +954,8 @@ pg_fe_sendauth(AuthRequest areq, PGconn *conn)
        case AUTH_REQ_MD5:
        case AUTH_REQ_CRYPT:
        case AUTH_REQ_PASSWORD:
-           if (conn->pgpass == NULL || *conn->pgpass == '\0')
+           conn->password_needed = true;
+           if (conn->pgpass == NULL || conn->pgpass[0] == '\0')
            {
                printfPQExpBuffer(&conn->errorMessage,
                                  PQnoPasswordSupplied);
index 65033b5af552b5036a8d6a91959e4fd2f50861af..dda32c5158cd8cb25694e8e3edcb236043ab602d 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/interfaces/libpq/fe-connect.c,v 1.353 2007/11/15 21:14:46 momjian Exp $
+ *   $PostgreSQL: pgsql/src/interfaces/libpq/fe-connect.c,v 1.354 2007/12/09 19:01:40 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -232,7 +232,7 @@ static PGconn *makeEmptyPGconn(void);
 static void freePGconn(PGconn *conn);
 static void closePGconn(PGconn *conn);
 static PQconninfoOption *conninfo_parse(const char *conninfo,
-              PQExpBuffer errorMessage);
+              PQExpBuffer errorMessage, bool *password_from_string);
 static char *conninfo_getval(PQconninfoOption *connOptions,
                const char *keyword);
 static void defaultNoticeReceiver(void *arg, const PGresult *res);
@@ -376,7 +376,8 @@ connectOptions1(PGconn *conn, const char *conninfo)
    /*
     * Parse the conninfo string
     */
-   connOptions = conninfo_parse(conninfo, &conn->errorMessage);
+   connOptions = conninfo_parse(conninfo, &conn->errorMessage,
+                                &conn->pgpass_from_client);
    if (connOptions == NULL)
    {
        conn->status = CONNECTION_BAD;
@@ -472,6 +473,7 @@ connectOptions2(PGconn *conn)
                                        conn->dbName, conn->pguser);
        if (conn->pgpass == NULL)
            conn->pgpass = strdup(DefaultPassword);
+       conn->pgpass_from_client = false;
    }
 
    /*
@@ -555,10 +557,11 @@ PQconninfoOption *
 PQconndefaults(void)
 {
    PQExpBufferData errorBuf;
+   bool        password_from_string;
    PQconninfoOption *connOptions;
 
    initPQExpBuffer(&errorBuf);
-   connOptions = conninfo_parse("", &errorBuf);
+   connOptions = conninfo_parse("", &errorBuf, &password_from_string);
    termPQExpBuffer(&errorBuf);
    return connOptions;
 }
@@ -659,6 +662,7 @@ PQsetdbLogin(const char *pghost, const char *pgport, const char *pgoptions,
        if (conn->pgpass)
            free(conn->pgpass);
        conn->pgpass = strdup(pwd);
+       conn->pgpass_from_client = true;
    }
 
    /*
@@ -1718,10 +1722,6 @@ keep_going:                      /* We will come back to here until there is
                 */
                conn->inStart = conn->inCursor;
 
-               /* Save the authentication request type, if first one. */
-               if (conn->areq == AUTH_REQ_OK)
-                   conn->areq = areq;
-
                /* Respond to the request if necessary. */
 
                /*
@@ -1924,7 +1924,7 @@ makeEmptyPGconn(void)
    conn->std_strings = false;  /* unless server says differently */
    conn->verbosity = PQERRORS_DEFAULT;
    conn->sock = -1;
-   conn->areq = AUTH_REQ_OK;   /* until we receive something else */
+   conn->password_needed = false;
 #ifdef USE_SSL
    conn->allow_ssl_try = true;
    conn->wait_ssl_try = false;
@@ -3064,9 +3064,12 @@ parseServiceInfo(PQconninfoOption *options, PQExpBuffer errorMessage)
  * If successful, a malloc'd PQconninfoOption array is returned.
  * If not successful, NULL is returned and an error message is
  * left in errorMessage.
+ * *password_from_string is set TRUE if we got a password from the
+ * conninfo string, otherwise FALSE.
  */
 static PQconninfoOption *
-conninfo_parse(const char *conninfo, PQExpBuffer errorMessage)
+conninfo_parse(const char *conninfo, PQExpBuffer errorMessage,
+              bool *password_from_string)
 {
    char       *pname;
    char       *pval;
@@ -3077,6 +3080,8 @@ conninfo_parse(const char *conninfo, PQExpBuffer errorMessage)
    PQconninfoOption *options;
    PQconninfoOption *option;
 
+   *password_from_string = false;          /* default result */
+
    /* Make a working copy of PQconninfoOptions */
    options = malloc(sizeof(PQconninfoOptions));
    if (options == NULL)
@@ -3233,6 +3238,12 @@ conninfo_parse(const char *conninfo, PQExpBuffer errorMessage)
            free(buf);
            return NULL;
        }
+
+       /*
+        * Special handling for password
+        */
+       if (strcmp(option->keyword, "password") == 0)
+           *password_from_string = (option->val[0] != '\0');
    }
 
    /* Done with the modifiable input string */
@@ -3475,14 +3486,24 @@ PQbackendPID(const PGconn *conn)
    return conn->be_pid;
 }
 
+int
+PQconnectionNeedsPassword(const PGconn *conn)
+{
+   if (!conn)
+       return false;
+   if (conn->password_needed &&
+       (conn->pgpass == NULL || conn->pgpass[0] == '\0'))
+       return true;
+   else
+       return false;
+}
+
 int
 PQconnectionUsedPassword(const PGconn *conn)
 {
    if (!conn)
        return false;
-   if (conn->areq == AUTH_REQ_MD5 ||
-       conn->areq == AUTH_REQ_CRYPT ||
-       conn->areq == AUTH_REQ_PASSWORD)
+   if (conn->password_needed && conn->pgpass_from_client)
        return true;
    else
        return false;
index 9c51c532dc677c14af73999a0281d0d2aab60ced..143cf17adab119092bb42565adf235cbec2becaa 100644 (file)
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2007, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/interfaces/libpq/libpq-fe.h,v 1.139 2007/10/13 20:18:42 tgl Exp $
+ * $PostgreSQL: pgsql/src/interfaces/libpq/libpq-fe.h,v 1.140 2007/12/09 19:01:40 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -263,6 +263,7 @@ extern int  PQserverVersion(const PGconn *conn);
 extern char *PQerrorMessage(const PGconn *conn);
 extern int PQsocket(const PGconn *conn);
 extern int PQbackendPID(const PGconn *conn);
+extern int PQconnectionNeedsPassword(const PGconn *conn);
 extern int PQconnectionUsedPassword(const PGconn *conn);
 extern int PQclientEncoding(const PGconn *conn);
 extern int PQsetClientEncoding(PGconn *conn, const char *encoding);
@@ -426,7 +427,7 @@ extern void PQfreemem(void *ptr);
 #define PQfreeNotify(ptr) PQfreemem(ptr)
 
 /* Error when no password was given. */
-/* Note: depending on this is deprecated; use PQconnectionUsedPassword(). */
+/* Note: depending on this is deprecated; use PQconnectionNeedsPassword(). */
 #define PQnoPasswordSupplied   "fe_sendauth: no password supplied\n"
 
 /*
index 5a9709b073e93bfd9f6d94a7c0a61f3d4b2e4c5a..26671d17cfe5ecf44e579716e93e0f5a1aee3f15 100644 (file)
@@ -12,7 +12,7 @@
  * Portions Copyright (c) 1996-2007, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/interfaces/libpq/libpq-int.h,v 1.127 2007/11/15 21:14:46 momjian Exp $
+ * $PostgreSQL: pgsql/src/interfaces/libpq/libpq-int.h,v 1.128 2007/12/09 19:01:40 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -291,6 +291,7 @@ struct pg_conn
    char       *dbName;         /* database name */
    char       *pguser;         /* Postgres username and password, if any */
    char       *pgpass;
+   bool        pgpass_from_client; /* did password come from connect args? */
    char       *sslmode;        /* SSL mode (require,prefer,allow,disable) */
 #if defined(KRB5) || defined(ENABLE_GSS) || defined(ENABLE_SSPI)
    char       *krbsrvname;     /* Kerberos service name */
@@ -323,7 +324,7 @@ struct pg_conn
    SockAddr    raddr;          /* Remote address */
    ProtocolVersion pversion;   /* FE/BE protocol version in use */
    int         sversion;       /* server version, e.g. 70401 for 7.4.1 */
-   AuthRequest areq;           /* auth type demanded by server */
+   bool        password_needed;    /* true if server demanded a password */
 
    /* Transient state needed while establishing connection */
    struct addrinfo *addrlist;  /* list of possible backend addresses */