From ba005f193d88a8404e81db3df223cf689d64d75e Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 24 Jan 2017 17:06:21 -0500 Subject: [PATCH] Allow password file name to be specified as a libpq connection parameter. Formerly an alternate password file could only be selected via the environment variable PGPASSFILE; now it can also be selected via a new connection parameter "passfile", corresponding to the conventions for most other connection parameters. There was some concern about this creating a security weakness, but it was agreed that that argument was pretty thin, and there are clear use-cases for handling password files this way. Julian Markwort, reviewed by Fabien Coelho, some adjustments by me Discussion: https://postgr.es/m/a4b4f4f1-7b58-a0e8-5268-5f7db8e8ccaa@uni-muenster.de --- doc/src/sgml/libpq.sgml | 35 ++++++++--- src/interfaces/libpq/fe-auth.c | 5 +- src/interfaces/libpq/fe-connect.c | 100 +++++++++++++++--------------- src/interfaces/libpq/libpq-int.h | 3 +- 4 files changed, 81 insertions(+), 62 deletions(-) diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index 2620eec033..ea7e7da9d4 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -943,7 +943,7 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname Note that authentication is likely to fail if host is not the name of the server at network address hostaddr. Also, note that host rather than hostaddr - is used to identify the connection in ~/.pgpass (see + is used to identify the connection in a password file (see ). @@ -1002,6 +1002,19 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname + + passfile + + + Specifies the name of the file used to store passwords + (see ). + Defaults to ~/.pgpass, or + %APPDATA%\postgresql\pgpass.conf on Microsoft Windows. + (No error is reported if this file does not exist.) + + + + connect_timeout @@ -6893,8 +6906,8 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough) Use of this environment variable is not recommended for security reasons, as some operating systems allow non-root users to see process environment variables via - ps; instead consider using the - ~/.pgpass file (see ). + ps; instead consider using a password file + (see ). @@ -6903,9 +6916,8 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough) PGPASSFILE - PGPASSFILE specifies the name of the password file to - use for lookups. If not set, it defaults to ~/.pgpass - (see ). + PGPASSFILE behaves the same as the connection parameter. @@ -7187,13 +7199,16 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough) - The file .pgpass in a user's home directory or the - file referenced by PGPASSFILE can contain passwords to + The file .pgpass in a user's home directory can + contain passwords to be used if the connection requires a password (and no password has been specified otherwise). On Microsoft Windows the file is named %APPDATA%\postgresql\pgpass.conf (where %APPDATA% refers to the Application Data subdirectory in the user's profile). + Alternatively, a password file can be specified + using the connection parameter + or the environment variable PGPASSFILE. @@ -7219,8 +7234,8 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough) - On Unix systems, the permissions on .pgpass must - disallow any access to world or group; achieve this by the command + On Unix systems, the permissions on a password file must + disallow any access to world or group; achieve this by a command such as chmod 0600 ~/.pgpass. If the permissions are less strict than this, the file will be ignored. On Microsoft Windows, it is assumed that the file is stored in a directory that is secure, so diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c index fe1e276f56..2845d3b9d2 100644 --- a/src/interfaces/libpq/fe-auth.c +++ b/src/interfaces/libpq/fe-auth.c @@ -686,11 +686,12 @@ pg_fe_sendauth(AuthRequest areq, PGconn *conn) case AUTH_REQ_MD5: case AUTH_REQ_PASSWORD: { - char *password = conn->connhost[conn->whichhost].password; + char *password; + conn->password_needed = true; + password = conn->connhost[conn->whichhost].password; if (password == NULL) password = conn->pgpass; - conn->password_needed = true; if (password == NULL || password[0] == '\0') { printfPQExpBuffer(&conn->errorMessage, diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index e13fc98249..0dda1804a5 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -107,7 +107,6 @@ static int ldapServiceLookup(const char *purl, PQconninfoOption *options, #define DefaultTty "" #define DefaultOption "" #define DefaultAuthtype "" -#define DefaultPassword "" #define DefaultTargetSessionAttrs "any" #ifdef USE_SSL #define DefaultSSLMode "prefer" @@ -185,6 +184,10 @@ static const internalPQconninfoOption PQconninfoOptions[] = { "Database-Password", "*", 20, offsetof(struct pg_conn, pgpass)}, + {"passfile", "PGPASSFILE", NULL, NULL, + "Database-Password-File", "", 64, + offsetof(struct pg_conn, pgpassfile)}, + {"connect_timeout", "PGCONNECT_TIMEOUT", NULL, NULL, "Connect-timeout", "", 10, /* strlen(INT32_MAX) == 10 */ offsetof(struct pg_conn, connect_timeout)}, @@ -382,10 +385,9 @@ static int parseServiceFile(const char *serviceFile, PQExpBuffer errorMessage, bool *group_found); static char *pwdfMatchesString(char *buf, char *token); -static char *PasswordFromFile(char *hostname, char *port, char *dbname, - char *username); -static bool getPgPassFilename(char *pgpassfile); -static void dot_pg_pass_warning(PGconn *conn); +static char *passwordFromFile(char *hostname, char *port, char *dbname, + char *username, char *pgpassfile); +static void pgpassfileWarning(PGconn *conn); static void default_threadlock(int acquire); @@ -957,19 +959,40 @@ connectOptions2(PGconn *conn) { int i; - if (conn->pgpass) - free(conn->pgpass); - conn->pgpass = strdup(DefaultPassword); - if (!conn->pgpass) - goto oom_error; - for (i = 0; i < conn->nconnhost; ++i) + if (conn->pgpassfile == NULL || conn->pgpassfile[0] == '\0') { + /* Identify password file to use; fail if we can't */ + char homedir[MAXPGPATH]; + + if (!pqGetHomeDirectory(homedir, sizeof(homedir))) + { + conn->status = CONNECTION_BAD; + printfPQExpBuffer(&conn->errorMessage, + libpq_gettext("could not get home directory to locate password file\n")); + return false; + } + + if (conn->pgpassfile) + free(conn->pgpassfile); + conn->pgpassfile = malloc(MAXPGPATH); + if (!conn->pgpassfile) + goto oom_error; + + snprintf(conn->pgpassfile, MAXPGPATH, "%s/%s", homedir, PGPASSFILE); + } + + for (i = 0; i < conn->nconnhost; i++) + { + /* Try to get a password for this host from pgpassfile */ conn->connhost[i].password = - PasswordFromFile(conn->connhost[i].host, + passwordFromFile(conn->connhost[i].host, conn->connhost[i].port, - conn->dbName, conn->pguser); + conn->dbName, + conn->pguser, + conn->pgpassfile); + /* If we got one, set pgpassfile_used */ if (conn->connhost[i].password != NULL) - conn->dot_pgpass_used = true; + conn->pgpassfile_used = true; } } @@ -3016,7 +3039,7 @@ keep_going: /* We will come back to here until there is error_return: - dot_pg_pass_warning(conn); + pgpassfileWarning(conn); /* * We used to close the socket at this point, but that makes it awkward @@ -3147,7 +3170,7 @@ makeEmptyPGconn(void) conn->sock = PGINVALID_SOCKET; conn->auth_req_received = false; conn->password_needed = false; - conn->dot_pgpass_used = false; + conn->pgpassfile_used = false; #ifdef USE_SSL conn->allow_ssl_try = true; conn->wait_ssl_try = false; @@ -3256,6 +3279,8 @@ freePGconn(PGconn *conn) free(conn->pguser); if (conn->pgpass) free(conn->pgpass); + if (conn->pgpassfile) + free(conn->pgpassfile); if (conn->keepalives) free(conn->keepalives); if (conn->keepalives_idle) @@ -5794,6 +5819,9 @@ PQpass(const PGconn *conn) password = conn->connhost[conn->whichhost].password; if (password == NULL) password = conn->pgpass; + /* Historically we've returned "" not NULL for no password specified */ + if (password == NULL) + password = ""; return password; } @@ -6160,10 +6188,10 @@ pwdfMatchesString(char *buf, char *token) /* Get a password from the password file. Return value is malloc'd. */ static char * -PasswordFromFile(char *hostname, char *port, char *dbname, char *username) +passwordFromFile(char *hostname, char *port, char *dbname, + char *username, char *pgpassfile) { FILE *fp; - char pgpassfile[MAXPGPATH]; struct stat stat_buf; #define LINELEN NAMEDATALEN*5 @@ -6190,9 +6218,6 @@ PasswordFromFile(char *hostname, char *port, char *dbname, char *username) if (port == NULL) port = DEF_PGPORT_STR; - if (!getPgPassFilename(pgpassfile)) - return NULL; - /* If password file cannot be opened, ignore it. */ if (stat(pgpassfile, &stat_buf) != 0) return NULL; @@ -6286,46 +6311,23 @@ PasswordFromFile(char *hostname, char *port, char *dbname, char *username) } -static bool -getPgPassFilename(char *pgpassfile) -{ - char *passfile_env; - - if ((passfile_env = getenv("PGPASSFILE")) != NULL) - /* use the literal path from the environment, if set */ - strlcpy(pgpassfile, passfile_env, MAXPGPATH); - else - { - char homedir[MAXPGPATH]; - - if (!pqGetHomeDirectory(homedir, sizeof(homedir))) - return false; - snprintf(pgpassfile, MAXPGPATH, "%s/%s", homedir, PGPASSFILE); - } - return true; -} - /* * If the connection failed, we should mention if - * we got the password from .pgpass in case that + * we got the password from the pgpassfile in case that * password is wrong. */ static void -dot_pg_pass_warning(PGconn *conn) +pgpassfileWarning(PGconn *conn) { - /* If it was 'invalid authorization', add .pgpass mention */ + /* If it was 'invalid authorization', add pgpassfile mention */ /* only works with >= 9.0 servers */ - if (conn->dot_pgpass_used && conn->password_needed && conn->result && + if (conn->pgpassfile_used && conn->password_needed && conn->result && strcmp(PQresultErrorField(conn->result, PG_DIAG_SQLSTATE), ERRCODE_INVALID_PASSWORD) == 0) { - char pgpassfile[MAXPGPATH]; - - if (!getPgPassFilename(pgpassfile)) - return; appendPQExpBuffer(&conn->errorMessage, libpq_gettext("password retrieved from file \"%s\"\n"), - pgpassfile); + conn->pgpassfile); } } diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h index 7289bd15c0..c655388864 100644 --- a/src/interfaces/libpq/libpq-int.h +++ b/src/interfaces/libpq/libpq-int.h @@ -343,6 +343,7 @@ struct pg_conn char *replication; /* connect as the replication standby? */ char *pguser; /* Postgres username and password, if any */ char *pgpass; + char *pgpassfile; /* path to a file containing password(s) */ char *keepalives; /* use TCP keepalives? */ char *keepalives_idle; /* time between TCP keepalives */ char *keepalives_interval; /* time between TCP keepalive @@ -407,7 +408,7 @@ struct pg_conn bool auth_req_received; /* true if any type of auth req * received */ bool password_needed; /* true if server demanded a password */ - bool dot_pgpass_used; /* true if used .pgpass */ + bool pgpassfile_used; /* true if password is from pgpassfile */ bool sigpipe_so; /* have we masked SIGPIPE via SO_NOSIGPIPE? */ bool sigpipe_flag; /* can we mask SIGPIPE via MSG_NOSIGNAL? */