Improve handling of password reuse in src/bin/scripts programs.

This reverts most of commit 83dec5a71 in favor of having connectDatabase()
store the possibly-reusable password in a static variable, similar to the
coding we've had for a long time in pg_dump's version of that function.
To avoid possible problems with unwanted password reuse, make callers
specify whether it's reasonable to attempt to re-use the password.
This is a wash for cases where re-use isn't needed, but it is far simpler
for callers that do want that.  Functionally there should be no difference.

Even though we're past RC1, it seems like a good idea to back-patch this
into 9.5, like the prior commit.  Otherwise, if there are any third-party
users of connectDatabase(), they'll have to deal with an API change in
9.5 and then another one in 9.6.

Michael Paquier

diff --git a/src/bin/scripts/clusterdb.c b/src/bin/scripts/clusterdb.c
index 2f15c82273f8c4fab4fc3090b4f331cfa20ace73..f87a9cee295e41540ed51e506c5d6805aec6ed85 100644 (file)
--- a/src/bin/scripts/clusterdb.c
+++ b/src/bin/scripts/clusterdb.c
@@ -203,8 +203,8 @@ cluster_one_database(const char *dbname, bool verbose, const char *table,
        appendPQExpBuffer(&sql, " %s", table);
    appendPQExpBufferChar(&sql, ';');
 
-   conn = connectDatabase(dbname, host, port, username, NULL, prompt_password,
-                          progname, false);
+   conn = connectDatabase(dbname, host, port, username, prompt_password,
+                          progname, false, false);
    if (!executeMaintenanceCommand(conn, sql.data, echo))
    {
        if (table)

diff --git a/src/bin/scripts/common.c b/src/bin/scripts/common.c
index d26a4edbb68e6305aa86013c2edbe09f1a2ce474..362e748b9a70f3d00e0187f7db6f2b9fc726b752 100644 (file)
--- a/src/bin/scripts/common.c
+++ b/src/bin/scripts/common.c
@@ -54,22 +54,31 @@ handle_help_version_opts(int argc, char *argv[],
 /*
  * Make a database connection with the given parameters.
  *
- * A password can be given, but if not (or if user forces us to) we prompt
- * interactively for one, unless caller prohibited us from doing so.
+ * An interactive password prompt is automatically issued if needed and
+ * allowed by prompt_password.
+ *
+ * If allow_password_reuse is true, we will try to re-use any password
+ * given during previous calls to this routine.  (Callers should not pass
+ * allow_password_reuse=true unless reconnecting to the same database+user
+ * as before, else we might create password exposure hazards.)
  */
 PGconn *
 connectDatabase(const char *dbname, const char *pghost, const char *pgport,
-               const char *pguser, const char *pgpassword,
-               enum trivalue prompt_password, const char *progname,
-               bool fail_ok)
+               const char *pguser, enum trivalue prompt_password,
+               const char *progname, bool fail_ok, bool allow_password_reuse)
 {
    PGconn     *conn;
-   char       *password;
+   static char *password = NULL;
    bool        new_pass;
 
-   password = pgpassword ? strdup(pgpassword) : NULL;
+   if (!allow_password_reuse)
+   {
+       if (password)
+           free(password);
+       password = NULL;
+   }
 
-   if (prompt_password == TRI_YES && !pgpassword)
+   if (password == NULL && prompt_password == TRI_YES)
        password = simple_prompt("Password: ", 100, false);
 
    /*
@@ -78,9 +87,8 @@ connectDatabase(const char *dbname, const char *pghost, const char *pgport,
     */
    do
    {
-#define PARAMS_ARRAY_SIZE  7
-       const char **keywords = pg_malloc(PARAMS_ARRAY_SIZE * sizeof(*keywords));
-       const char **values = pg_malloc(PARAMS_ARRAY_SIZE * sizeof(*values));
+       const char *keywords[7];
+       const char *values[7];
 
        keywords[0] = "host";
        values[0] = pghost;
@@ -107,9 +115,6 @@ connectDatabase(const char *dbname, const char *pghost, const char *pgport,
            exit(1);
        }
 
-       pg_free(keywords);
-       pg_free(values);
-
        /*
         * No luck?  Trying asking (again) for a password.
         */
@@ -125,9 +130,6 @@ connectDatabase(const char *dbname, const char *pghost, const char *pgport,
        }
    } while (new_pass);
 
-   if (password)
-       free(password);
-
    /* check to see that the backend connection was successfully made */
    if (PQstatus(conn) == CONNECTION_BAD)
    {
@@ -157,15 +159,15 @@ connectMaintenanceDatabase(const char *maintenance_db, const char *pghost,
 
    /* If a maintenance database name was specified, just connect to it. */
    if (maintenance_db)
-       return connectDatabase(maintenance_db, pghost, pgport, pguser, NULL,
-                              prompt_password, progname, false);
+       return connectDatabase(maintenance_db, pghost, pgport, pguser,
+                              prompt_password, progname, false, false);
 
    /* Otherwise, try postgres first and then template1. */
-   conn = connectDatabase("postgres", pghost, pgport, pguser, NULL,
-                          prompt_password, progname, true);
+   conn = connectDatabase("postgres", pghost, pgport, pguser, prompt_password,
+                          progname, true, false);
    if (!conn)
-       conn = connectDatabase("template1", pghost, pgport, pguser, NULL,
-                              prompt_password, progname, false);
+       conn = connectDatabase("template1", pghost, pgport, pguser,
+                              prompt_password, progname, false, false);
 
    return conn;
 }

diff --git a/src/bin/scripts/common.h b/src/bin/scripts/common.h
index dafd58f42255c271bad23acd983207eeb5dbe720..9038295918471aceb5c1082409eae44ee01dc3a6 100644 (file)
--- a/src/bin/scripts/common.h
+++ b/src/bin/scripts/common.h
@@ -31,8 +31,8 @@ extern void handle_help_version_opts(int argc, char *argv[],
 
 extern PGconn *connectDatabase(const char *dbname, const char *pghost,
                const char *pgport, const char *pguser,
-               const char *pgpassword, enum trivalue prompt_password,
-               const char *progname, bool fail_ok);
+               enum trivalue prompt_password, const char *progname,
+               bool fail_ok, bool allow_password_reuse);
 
 extern PGconn *connectMaintenanceDatabase(const char *maintenance_db,
                  const char *pghost, const char *pgport, const char *pguser,

diff --git a/src/bin/scripts/createlang.c b/src/bin/scripts/createlang.c
index 87cd623d2512b23a3249b6c8d2c1319ced5c4ed0..f349624092246206fc2afbaee55769abd1c084fe 100644 (file)
--- a/src/bin/scripts/createlang.c
+++ b/src/bin/scripts/createlang.c
@@ -140,8 +140,8 @@ main(int argc, char *argv[])
        printQueryOpt popt;
        static const bool translate_columns[] = {false, true};
 
-       conn = connectDatabase(dbname, host, port, username, NULL,
-                              prompt_password, progname, false);
+       conn = connectDatabase(dbname, host, port, username, prompt_password,
+                              progname, false, false);
 
        printfPQExpBuffer(&sql, "SELECT lanname as \"%s\", "
                "(CASE WHEN lanpltrusted THEN '%s' ELSE '%s' END) as \"%s\" "
@@ -180,8 +180,8 @@ main(int argc, char *argv[])
        if (*p >= 'A' && *p <= 'Z')
            *p += ('a' - 'A');
 
-   conn = connectDatabase(dbname, host, port, username, NULL,
-                          prompt_password, progname, false);
+   conn = connectDatabase(dbname, host, port, username, prompt_password,
+                          progname, false, false);
 
    /*
     * Make sure the language isn't already installed

diff --git a/src/bin/scripts/createuser.c b/src/bin/scripts/createuser.c
index 9e7f84d9d543fe221c95add51dbc32a91af0ad10..dc358e40ce37e9b05165e54776b6ebf95f53636d 100644 (file)
--- a/src/bin/scripts/createuser.c
+++ b/src/bin/scripts/createuser.c
@@ -250,8 +250,8 @@ main(int argc, char *argv[])
    if (login == 0)
        login = TRI_YES;
 
-   conn = connectDatabase("postgres", host, port, username, NULL,
-                          prompt_password, progname, false);
+   conn = connectDatabase("postgres", host, port, username, prompt_password,
+                          progname, false, false);
 
    initPQExpBuffer(&sql);
 

diff --git a/src/bin/scripts/droplang.c b/src/bin/scripts/droplang.c
index fb4025ddf75338edd32cb7fbb36b30e36e82bfa2..e35fdc0092cdc953863a2aa0ffece3dd3d9ae73d 100644 (file)
--- a/src/bin/scripts/droplang.c
+++ b/src/bin/scripts/droplang.c
@@ -139,8 +139,8 @@ main(int argc, char *argv[])
        printQueryOpt popt;
        static const bool translate_columns[] = {false, true};
 
-       conn = connectDatabase(dbname, host, port, username, NULL,
-                              prompt_password, progname, false);
+       conn = connectDatabase(dbname, host, port, username, prompt_password,
+                              progname, false, false);
 
        printfPQExpBuffer(&sql, "SELECT lanname as \"%s\", "
                "(CASE WHEN lanpltrusted THEN '%s' ELSE '%s' END) as \"%s\" "
@@ -181,8 +181,8 @@ main(int argc, char *argv[])
        if (*p >= 'A' && *p <= 'Z')
            *p += ('a' - 'A');
 
-   conn = connectDatabase(dbname, host, port, username, NULL,
-                          prompt_password, progname, false);
+   conn = connectDatabase(dbname, host, port, username, prompt_password,
+                          progname, false, false);
 
    /*
     * Force schema search path to be just pg_catalog, so that we don't have

diff --git a/src/bin/scripts/dropuser.c b/src/bin/scripts/dropuser.c
index 10d0691c6772656b5b209f05f3b5b6378de83e48..1b0cf78f2a47e3b5514c2da7975924c6cc064299 100644 (file)
--- a/src/bin/scripts/dropuser.c
+++ b/src/bin/scripts/dropuser.c
@@ -128,8 +128,8 @@ main(int argc, char *argv[])
    appendPQExpBuffer(&sql, "DROP ROLE %s%s;",
                      (if_exists ? "IF EXISTS " : ""), fmtId(dropuser));
 
-   conn = connectDatabase("postgres", host, port, username, NULL,
-                          prompt_password, progname, false);
+   conn = connectDatabase("postgres", host, port, username, prompt_password,
+                          progname, false, false);
 
    if (echo)
        printf("%s\n", sql.data);

diff --git a/src/bin/scripts/reindexdb.c b/src/bin/scripts/reindexdb.c
index fbf436c6d5973683cfa932cc21f7f3e4161ef8f8..b6f3dcbca04bfe47a5e4d8309716d94e39a8d2c3 100644 (file)
--- a/src/bin/scripts/reindexdb.c
+++ b/src/bin/scripts/reindexdb.c
@@ -297,8 +297,8 @@ reindex_one_database(const char *name, const char *dbname, const char *type,
        appendPQExpBuffer(&sql, " DATABASE %s", fmtId(name));
    appendPQExpBufferChar(&sql, ';');
 
-   conn = connectDatabase(dbname, host, port, username, NULL,
-                          prompt_password, progname, false);
+   conn = connectDatabase(dbname, host, port, username, prompt_password,
+                          progname, false, false);
 
    if (!executeMaintenanceCommand(conn, sql.data, echo))
    {
@@ -372,8 +372,8 @@ reindex_system_catalogs(const char *dbname, const char *host, const char *port,
 
    appendPQExpBuffer(&sql, " SYSTEM %s;", dbname);
 
-   conn = connectDatabase(dbname, host, port, username, NULL,
-                          prompt_password, progname, false);
+   conn = connectDatabase(dbname, host, port, username, prompt_password,
+                          progname, false, false);
    if (!executeMaintenanceCommand(conn, sql.data, echo))
    {
        fprintf(stderr, _("%s: reindexing of system catalogs failed: %s"),

diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c
index 8cbf0392a8dfbd721f1812f0919a3fe0f7788247..0863c64cbc5d42768d8df3320ffb39ae156a0ba5 100644 (file)
--- a/src/bin/scripts/vacuumdb.c
+++ b/src/bin/scripts/vacuumdb.c
@@ -43,8 +43,7 @@ static void vacuum_one_database(const char *dbname, vacuumingOptions *vacopts,
                    const char *host, const char *port,
                    const char *username, enum trivalue prompt_password,
                    int concurrentCons,
-                   const char *progname, bool echo, bool quiet,
-                   char **password);
+                   const char *progname, bool echo, bool quiet);
 
 static void vacuum_all_databases(vacuumingOptions *vacopts,
                     bool analyze_in_stages,
@@ -276,8 +275,6 @@ main(int argc, char *argv[])
    }
    else
    {
-       char *password = NULL;
-
        if (dbname == NULL)
        {
            if (getenv("PGDATABASE"))
@@ -299,8 +296,7 @@ main(int argc, char *argv[])
                                    &tables,
                                    host, port, username, prompt_password,
                                    concurrentCons,
-                                   progname, echo, quiet,
-                                   &password);
+                                   progname, echo, quiet);
            }
        }
        else
@@ -309,10 +305,7 @@ main(int argc, char *argv[])
                                &tables,
                                host, port, username, prompt_password,
                                concurrentCons,
-                               progname, echo, quiet,
-                               &password);
-
-       pg_free(password);
+                               progname, echo, quiet);
    }
 
    exit(0);
@@ -330,21 +323,15 @@ main(int argc, char *argv[])
  * If concurrentCons is > 1, multiple connections are used to vacuum tables
  * in parallel.  In this case and if the table list is empty, we first obtain
  * a list of tables from the database.
- *
- * 'password' is both an input and output parameter.  If one is not passed,
- * then whatever is used in a connection is returned so that caller can
- * reuse it in future connections.
  */
 static void
 vacuum_one_database(const char *dbname, vacuumingOptions *vacopts,
                    int stage,
                    SimpleStringList *tables,
                    const char *host, const char *port,
-                   const char *username,
-                   enum trivalue prompt_password,
+                   const char *username, enum trivalue prompt_password,
                    int concurrentCons,
-                   const char *progname, bool echo, bool quiet,
-                   char **password)
+                   const char *progname, bool echo, bool quiet)
 {
    PQExpBufferData sql;
    PGconn     *conn;
@@ -378,15 +365,8 @@ vacuum_one_database(const char *dbname, vacuumingOptions *vacopts,
        fflush(stdout);
    }
 
-   conn = connectDatabase(dbname, host, port, username, *password,
-                          prompt_password, progname, false);
-
-   /*
-    * If no password was not specified by caller and the connection required
-    * one, remember it; this suppresses further password prompts.
-    */
-   if (PQconnectionUsedPassword(conn) && *password == NULL)
-       *password = pg_strdup(PQpass(conn));
+   conn = connectDatabase(dbname, host, port, username, prompt_password,
+                          progname, false, true);
 
    initPQExpBuffer(&sql);
 
@@ -444,20 +424,10 @@ vacuum_one_database(const char *dbname, vacuumingOptions *vacopts,
    init_slot(slots, conn);
    if (parallel)
    {
-       const char *pqpass;
-
-       /*
-        * If a password was supplied for the initial connection, use it for
-        * subsequent ones too.  (Note that since we're connecting to the same
-        * database with the same user, there's no need to update the stored
-        * password any further.)
-        */
-       pqpass = PQpass(conn);
-
        for (i = 1; i < concurrentCons; i++)
        {
-           conn = connectDatabase(dbname, host, port, username, pqpass,
-                                  prompt_password, progname, false);
+           conn = connectDatabase(dbname, host, port, username, prompt_password,
+                                  progname, false, true);
            init_slot(slots + i, conn);
        }
    }
@@ -572,23 +542,12 @@ vacuum_all_databases(vacuumingOptions *vacopts,
    PGresult   *result;
    int         stage;
    int         i;
-   char       *password = NULL;
 
    conn = connectMaintenanceDatabase(maintenance_db, host, port,
                                      username, prompt_password, progname);
-
    result = executeQuery(conn,
            "SELECT datname FROM pg_database WHERE datallowconn ORDER BY 1;",
                          progname, echo);
-
-   /*
-    * Remember the password for further connections.  If no password was
-    * required for the maintenance db connection, this gets updated for the
-    * first connection that does.
-    */
-   if (PQconnectionUsedPassword(conn))
-       password = pg_strdup(PQpass(conn));
-
    PQfinish(conn);
 
    if (analyze_in_stages)
@@ -613,8 +572,7 @@ vacuum_all_databases(vacuumingOptions *vacopts,
                                    NULL,
                                    host, port, username, prompt_password,
                                    concurrentCons,
-                                   progname, echo, quiet,
-                                   &password);
+                                   progname, echo, quiet);
            }
        }
    }
@@ -630,13 +588,11 @@ vacuum_all_databases(vacuumingOptions *vacopts,
                                NULL,
                                host, port, username, prompt_password,
                                concurrentCons,
-                               progname, echo, quiet,
-                               &password);
+                               progname, echo, quiet);
        }
    }
 
    PQclear(result);
-   pg_free(password);
 }
 
 /*