After explaining the patch to a college we identified potentially execution
of another user when it is defined in as a command parameter. To protect
agains it I've removed the possibility to pass the 'passcommand'. With the
result libpq only allows the PGPASSCOMMAND environment variable, which can
only be defined by the executing user, and will be executed by the same
user. It only reduces the need of unencrypted password's in a file.
I think this solution is secure enough, shall we solve this feature-request?
Regards, Marco
On Tue, Jul 24, 2018 at 4:00 PM Tom Lane <t...@sss.pgh.pa.us> wrote:
> Marco van Eck <marco.van...@gmail.com> writes:
> > Indeed having unencrypted password lying (.pgpass or PGPASSWORD or -W)
> > around is making my auditors unhappy, and forcing me to enter the
> password
> > over and over again. With a simple test it seems the password entered by
> > the user also stays in memory, since it is able to reset a broken
> > connection. Finding the password in memory is not trivial, but prevention
> > is always preferred.
>
> > It might be an idea to wipe the password after the login, and
> decrypt/read
> > it again if it needs to reconnect. Would this make the solution more
> > secure? I had a quick look at the code and the patch would stay compact.
> > Please let me know of doing this would make sense.
>
> We're basically not going to accept any added complication that's designed
> to prevent memory-inspection attacks, because in general that's a waste
> of effort. All you're doing is (slightly) reducing the attack window.
>
> regards, tom lane
>
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index d67212b831..ae526a35c8 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -1099,6 +1099,16 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
</listitem>
</varlistentry>
+ <varlistentry id="libpq-connect-passcommand" xreflabel="passcommand">
+ <term><literal>passcommand</literal></term>
+ <listitem>
+ <para>
+ Specifies the command to run to get the contents of store passwords
+ (see <xref linkend="libpq-pgpass"/>).
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="libpq-connect-connect-timeout" xreflabel="connect_timeout">
<term><literal>connect_timeout</literal></term>
<listitem>
@@ -7231,6 +7241,16 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough)
</para>
</listitem>
+ <listitem>
+ <para>
+ <indexterm>
+ <primary><envar>PGPASSCOMMAND</envar></primary>
+ </indexterm>
+ <envar>PGPASSCOMMAND</envar> behaves the same as the <xref
+ linkend="libpq-connect-passcommand"/> connection parameter.
+ </para>
+ </listitem>
+
<listitem>
<para>
<indexterm>
@@ -7486,7 +7506,7 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough)
<sect1 id="libpq-pgpass">
- <title>The Password File</title>
+ <title>The Password File / Password Command</title>
<indexterm zone="libpq-pgpass">
<primary>password file</primary>
@@ -7494,6 +7514,9 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough)
<indexterm zone="libpq-pgpass">
<primary>.pgpass</primary>
</indexterm>
+ <indexterm zone="libpq-pgpass">
+ <primary>password command</primary>
+ </indexterm>
<para>
The file <filename>.pgpass</filename> in a user's home directory can
@@ -7509,7 +7532,25 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough)
</para>
<para>
- This file should contain lines of the following format:
+ The command which produces passwords to connect if the connection needs a
+ password (and no password has been specified otherwise).
+ Alternatively, a password command can be specifiedin the environment
+ variable <envar>PGPASSCOMMAND</envar>. It is not possible to specify the
+ command as a connect parameter since it could be executed by another user.
+
+ examples:
+ <programlisting>
+ PGPASSCOMMAND = "gpg -q -d pgpass.gpg"
+ PGPASSCOMMAND = "curl http://passwords/really-unsecure-pgpass";
+ PGPASSCOMMAND = "my-own-secure-pgpass-script"
+ </programlisting>
+
+ NOTE: if a <filename>.pgpass</filename> is present the command will not be
+ issued
+ </para>
+
+ <para>
+ This file/command-output should contain lines of the following format:
<synopsis>
<replaceable>hostname</replaceable>:<replaceable>port</replaceable>:<replaceable>database</replaceable>:<replaceable>username</replaceable>:<replaceable>password</replaceable>
</synopsis>
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index bd7dac120d..db0d5e6d81 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -204,6 +204,13 @@ static const internalPQconninfoOption PQconninfoOptions[] = {
"Database-Password-File", "", 64,
offsetof(struct pg_conn, pgpassfile)},
+ /* PGPASSCOMMAND may only be passed as an environment variable to
+ since adding it to the connect_string could potentially allow the command
+ to run with the priviliges of the database-server */
+ {"", "PGPASSCOMMAND", NULL, NULL,
+ "Database-Password-Command", "", 64,
+ offsetof(struct pg_conn, pgpasscommand)},
+
{"connect_timeout", "PGCONNECT_TIMEOUT", NULL, NULL,
"Connect-timeout", "", 10, /* strlen(INT32_MAX) == 10 */
offsetof(struct pg_conn, connect_timeout)},
@@ -406,8 +413,8 @@ static int parseServiceFile(const char *serviceFile,
PQExpBuffer errorMessage,
bool *group_found);
static char *pwdfMatchesString(char *buf, const char *token);
-static char *passwordFromFile(const char *hostname, const char *port, const char *dbname,
- const char *username, const char *pgpassfile);
+static char *passwordFromFileOrCommand(const char *hostname, const char *port, const char *dbname,
+ const char *username, const char *pgpassfile, const char *pgpasscommand);
static void pgpassfileWarning(PGconn *conn);
static void default_threadlock(int acquire);
@@ -1087,7 +1094,25 @@ connectOptions2(PGconn *conn)
}
}
- if (conn->pgpassfile != NULL && conn->pgpassfile[0] != '\0')
+ // if (conn->pgpasscommand == NULL)
+ // {
+ // char *tmp;
+ // if ((tmp = getenv("PGPASSCOMMAND")) != NULL)
+ // {
+ // conn->pgpasscommand = strdup(tmp);
+ // if (!conn->pgpasscommand)
+ // {
+ // conn->status = CONNECTION_BAD;
+ // if (errorMessage)
+ // printfPQExpBuffer(errorMessage,
+ // libpq_gettext("out of memory\n"));
+ // return false;
+ // }
+ // }
+ // }
+
+ if ( (conn->pgpassfile != NULL && conn->pgpassfile[0] != '\0')
+ || (conn->pgpasscommand != NULL && conn->pgpasscommand[0] != '\0') )
{
int i;
@@ -1106,11 +1131,12 @@ connectOptions2(PGconn *conn)
pwhost = conn->connhost[i].hostaddr;
conn->connhost[i].password =
- passwordFromFile(pwhost,
+ passwordFromFileOrCommand(pwhost,
conn->connhost[i].port,
conn->dbName,
conn->pguser,
- conn->pgpassfile);
+ conn->pgpassfile,
+ conn->pgpasscommand);
/* If we got one, set pgpassfile_used */
if (conn->connhost[i].password != NULL)
conn->pgpassfile_used = true;
@@ -1751,7 +1777,7 @@ connectDBStart(PGconn *conn)
if (ret || !ch->addrlist)
appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not parse network address \"%s\": %s\n"),
- ch->hostaddr, gai_strerror(ret));
+ ch->host, gai_strerror(ret));
break;
case CHT_UNIX_SOCKET:
@@ -3468,6 +3494,8 @@ freePGconn(PGconn *conn)
free(conn->pgpass);
if (conn->pgpassfile)
free(conn->pgpassfile);
+ if (conn->pgpasscommand)
+ free(conn->pgpasscommand);
if (conn->keepalives)
free(conn->keepalives);
if (conn->keepalives_idle)
@@ -6376,11 +6404,12 @@ pwdfMatchesString(char *buf, const char *token)
/* Get a password from the password file. Return value is malloc'd. */
static char *
-passwordFromFile(const char *hostname, const char *port, const char *dbname,
- const char *username, const char *pgpassfile)
+passwordFromFileOrCommand(const char *hostname, const char *port, const char *dbname,
+ const char *username, const char *pgpassfile, const char *pgpasscommand)
{
FILE *fp;
struct stat stat_buf;
+ char has_pgpassfile=0;
#define LINELEN NAMEDATALEN*5
char buf[LINELEN];
@@ -6407,11 +6436,12 @@ passwordFromFile(const char *hostname, const char *port, const char *dbname,
port = DEF_PGPORT_STR;
/* If password file cannot be opened, ignore it. */
- if (stat(pgpassfile, &stat_buf) != 0)
- return NULL;
+ if (stat(pgpassfile, &stat_buf) == 0)
+ has_pgpassfile=1;
+
#ifndef WIN32
- if (!S_ISREG(stat_buf.st_mode))
+ if (has_pgpassfile && !S_ISREG(stat_buf.st_mode))
{
fprintf(stderr,
libpq_gettext("WARNING: password file \"%s\" is not a plain file\n"),
@@ -6420,7 +6450,7 @@ passwordFromFile(const char *hostname, const char *port, const char *dbname,
}
/* If password file is insecure, alert the user and ignore it. */
- if (stat_buf.st_mode & (S_IRWXG | S_IRWXO))
+ if (has_pgpassfile && stat_buf.st_mode & (S_IRWXG | S_IRWXO))
{
fprintf(stderr,
libpq_gettext("WARNING: password file \"%s\" has group or world access; permissions should be u=rw (0600) or less\n"),
@@ -6434,8 +6464,19 @@ passwordFromFile(const char *hostname, const char *port, const char *dbname,
* file.
*/
#endif
+ if (has_pgpassfile)
+ {
+ fp = fopen(pgpassfile, "r");
+ }
+ else if (pgpasscommand != NULL && pgpasscommand[0] != '\0')
+ {
+ fp = popen(pgpasscommand, "r");
+ }
+ else
+ {
+ return NULL;
+ }
- fp = fopen(pgpassfile, "r");
if (fp == NULL)
return NULL;
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 9a586ff25a..77af2ab550 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -343,6 +343,7 @@ struct pg_conn
char *pguser; /* Postgres username and password, if any */
char *pgpass;
char *pgpassfile; /* path to a file containing password(s) */
+ char *pgpasscommand; /* path to command producing the password(s) */
char *keepalives; /* use TCP keepalives? */
char *keepalives_idle; /* time between TCP keepalives */
char *keepalives_interval; /* time between TCP keepalive
