Fix up the PQconnectionUsedPassword mess: create a separate

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.

diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 1ba79113eaeed1ca5b1ebc334711f8040ac4253d..4298001a51000053ca331cd042876f8b0500d18c 100644 (file)
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -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>

diff --git a/doc/src/sgml/release.sgml b/doc/src/sgml/release.sgml
index 16b6061306f85c99206721286014f05ff593fdf0..7a688aeb86cc84277927797a03adc89fd1099526 100644 (file)
--- a/doc/src/sgml/release.sgml
+++ b/doc/src/sgml/release.sgml
@@ -2184,8 +2184,9 @@ current_date < 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>

diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index b8bf1488a3eaffaf0d264f0ef5b6a9ffb5bf73f4..8d3bc7fadf6f85bec1a340fb994f2d0362c71485 100644 (file)
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -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;

diff --git a/src/bin/pg_dump/pg_backup_db.c b/src/bin/pg_dump/pg_backup_db.c
index 4ec0ca7737cf066773fe146e29d2ed90bcba0c0e..d61d7dd7b685c38635ff2bffccc312b96f4fa1a4 100644 (file)
--- a/src/bin/pg_dump/pg_backup_db.c
+++ b/src/bin/pg_dump/pg_backup_db.c
@@ -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))
                {

diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 75787c90db84147b0e0ddf605f65a445cc538f98..e279281f506ccef33014d13077110d3c453ddeae 100644 (file)
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -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))
                {

diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 09931bf94162214c815e45ca390dd1a92762e5f5..c63f70e1a69575e7e2cd3cc5dcf3aa3268707c2d 100644 (file)
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -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);

diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index b3e11e298359a84d2042dca6f4d25aa98c728a4f..1395a6895fe4f2c23b9b031e358423730ca68f08 100644 (file)
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -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))
                {

diff --git a/src/bin/scripts/common.c b/src/bin/scripts/common.c
index af515ebb4360584ef9b7220a96ff8413141fa7a4..268447ef436dfceb1f0758b20c8ec7f1d3a4e5ce 100644 (file)
--- a/src/bin/scripts/common.c
+++ b/src/bin/scripts/common.c
@@ -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))
                {

diff --git a/src/interfaces/libpq/exports.txt b/src/interfaces/libpq/exports.txt
index fa82432e16e4a0e74126cb9696528d3094d56285..d9951b2650be005b034dc164b5c3f7fabab534c2 100644 (file)
--- a/src/interfaces/libpq/exports.txt
+++ b/src/interfaces/libpq/exports.txt
@@ -139,3 +139,4 @@ PQsendDescribePortal      136
 lo_truncate               137
 PQconnectionUsedPassword  138
 pg_valid_server_encoding_id 139
+PQconnectionNeedsPassword 140

diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c
index fd6684bae75b1e75b02bb71f8ee6cb09e80c45d0..148101d25f523889b1df489d2b36f71084e57251 100644 (file)
--- a/src/interfaces/libpq/fe-auth.c
+++ b/src/interfaces/libpq/fe-auth.c
@@ -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);

diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 8bb6bce70acfa7f3d6f86876b2ab450a8f6b3d09..0a16fb12c475109d5ab1b75edfe1cad60e4ecf86 100644 (file)
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -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;

diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h
index ecd7de9797787481d31895f15b6b1ad61a4f75bc..75061e9a31778767ea40952c29972976b7f93dde 100644 (file)
--- a/src/interfaces/libpq/libpq-fe.h
+++ b/src/interfaces/libpq/libpq-fe.h
@@ -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"
 
 /*

diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index e1fce8bb6ee76e41fd1daa3282bf3a37c184ff6d..0614fc5e786d3930f3d0a27775fab4e75f74a406 100644 (file)
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -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 */