Unset libpq environment variables

By default libpq uses environment variables as a fallback way
to specify connection options, potentially they could be in
a conflict with internal implementation and introduce
security risks.

Introduce new option `timescaledb.passfile` which specifies
the name of the file used to store passwords used for a data node
connection. It is intended to be used instead of PGPASSFILE
variable, which is no longer accessible.
This commit is contained in:
Dmitry Simonenko 2019-08-30 14:07:58 +03:00 committed by Erik Nordström
parent 33923548c7
commit bcb8352be2
10 changed files with 80 additions and 18 deletions

View File

@ -59,6 +59,7 @@ TSDLLEXPORT int ts_guc_max_insert_batch_size = 1000;
TSDLLEXPORT bool ts_guc_enable_connection_binary_data;
TSDLLEXPORT bool ts_guc_enable_client_ddl_on_data_nodes = false;
TSDLLEXPORT char *ts_guc_ssl_dir = NULL;
TSDLLEXPORT char *ts_guc_passfile = NULL;
#ifdef TS_DEBUG
bool ts_shutdown_bgw = false;
@ -272,6 +273,18 @@ _guc_init(void)
NULL,
NULL);
DefineCustomStringVariable("timescaledb.passfile",
"TimescaleDB password file path",
"Specifies the name of the file used to store passwords used for "
"data node connections",
&ts_guc_passfile,
NULL,
PGC_SIGHUP,
GUC_SUPERUSER_ONLY,
NULL,
NULL,
NULL);
DefineCustomIntVariable("timescaledb.max_open_chunks_per_insert",
"Maximum open chunks per insert",
"Maximum number of open chunk tables per insert",

View File

@ -35,6 +35,7 @@ extern TSDLLEXPORT int ts_guc_max_insert_batch_size;
extern TSDLLEXPORT bool ts_guc_enable_connection_binary_data;
extern TSDLLEXPORT bool ts_guc_enable_client_ddl_on_data_nodes;
extern TSDLLEXPORT char *ts_guc_ssl_dir;
extern TSDLLEXPORT char *ts_guc_passfile;
#ifdef TS_DEBUG
extern bool ts_shutdown_bgw;

View File

@ -15,4 +15,5 @@ log_line_prefix='%u [%p] %d '
# numbers. Setting extra_float_digits=0 retains the old behavior which
# is needed to make our tests work for multiple PostgreSQL versions.
extra_float_digits=0
timescaledb.passfile='@TEST_PASSFILE@'
hba_file='@TEST_PG_HBA_FILE@'

View File

@ -21,7 +21,7 @@ set(TEST_PGPORT_TEMP_INSTANCE 15432 CACHE STRING "The port to run a temporary te
set(TEST_SCHEDULE ${CMAKE_CURRENT_BINARY_DIR}/test_schedule)
set(TEST_SCHEDULE_SHARED ${CMAKE_CURRENT_BINARY_DIR}/shared/test_schedule_shared)
set(ISOLATION_TEST_SCHEDULE ${CMAKE_CURRENT_BINARY_DIR}/isolation_test_schedule)
set(TEST_PGPASSFILE ${TEST_OUTPUT_DIR}/pgpass.conf)
set(TEST_PASSFILE ${TEST_OUTPUT_DIR}/pgpass.conf)
# Add clustering users with enabled SSL for PG version >= 11.
#
@ -109,8 +109,7 @@ if(PG_REGRESS)
TEST_OUTPUT_DIR=${TEST_OUTPUT_DIR}
TEST_SCHEDULE=${TEST_SCHEDULE}
PG_BINDIR=${PG_BINDIR}
PG_REGRESS=${PG_REGRESS}
PGPASSFILE=${TEST_PGPASSFILE})
PG_REGRESS=${PG_REGRESS})
endif()
if(PG_ISOLATION_REGRESS)
@ -128,8 +127,7 @@ if(PG_ISOLATION_REGRESS)
TEST_INPUT_DIR=${TEST_INPUT_DIR}
TEST_OUTPUT_DIR=${TEST_OUTPUT_DIR}
ISOLATION_TEST_SCHEDULE=${ISOLATION_TEST_SCHEDULE}
PG_ISOLATION_REGRESS=${PG_ISOLATION_REGRESS}
PGPASSFILE=${TEST_PGPASSFILE})
PG_ISOLATION_REGRESS=${PG_ISOLATION_REGRESS})
endif()
if (${PG_VERSION_MAJOR} GREATER "9")

View File

@ -333,6 +333,30 @@ get_libpq_options()
return libpq_options;
}
static void
unset_libpq_envvar(void)
{
PQconninfoOption *lopt;
PQconninfoOption *options = PQconndefaults();
Assert(options != NULL);
/* Explicitly unset all libpq environment variables.
*
* By default libpq uses environment variables as a fallback
* to specify connection options, potentially they could be in
* a conflict with PostgreSQL variables and introduce
* security risks.
*/
for (lopt = options; lopt->keyword; lopt++)
{
if (lopt->envvar)
unsetenv(lopt->envvar);
}
PQconninfoFree(options);
}
static bool
is_libpq_option(const char *keyword, char **display_option)
{
@ -819,9 +843,33 @@ remote_connection_set_peer_dist_id(TSConnection *conn)
/* fallback_application_name, client_encoding, end marker */
#define REMOTE_CONNECTION_SESSION_OPTIONS_N 3
/* passfile */
#define REMOTE_CONNECTION_PASSWORD_OPTIONS_N 1
/* sslmode, sslrootcert, sslcert, sslkey */
#define REMOTE_CONNECTION_SSL_OPTIONS_N 4
/* default password file basename */
#define DEFAULT_PASSFILE_NAME "passfile"
static void
set_password_options(const char **keywords, const char **values, int *option_start)
{
int option_pos = *option_start;
/* Set user specified password file path using timescaledb.passfile or
* use default path assuming that the file is stored in the
* data directory */
keywords[option_pos] = "passfile";
if (ts_guc_passfile)
values[option_pos] = ts_guc_passfile;
else
values[option_pos] = psprintf("%s/" DEFAULT_PASSFILE_NAME, DataDir);
option_pos++;
*option_start = option_pos;
}
static void
set_ssl_options(const char *user_name, const char **keywords, const char **values,
int *option_start)
@ -884,10 +932,10 @@ remote_connection_open_internal(const char *node_name, List *connection_options)
* and user. (Some of them might not be libpq options, in
* which case we'll just waste a few array slots.) Add 3 extra slots
* for fallback_application_name, client_encoding, end marker.
* Add additional 4 slots for ssl options.
* One additional slot to set passfile and 4 slots for ssl options.
*/
option_count = list_length(connection_options) + REMOTE_CONNECTION_SESSION_OPTIONS_N +
REMOTE_CONNECTION_SSL_OPTIONS_N;
REMOTE_CONNECTION_PASSWORD_OPTIONS_N + REMOTE_CONNECTION_SSL_OPTIONS_N;
keywords = (const char **) palloc(option_count * sizeof(char *));
values = (const char **) palloc(option_count * sizeof(char *));
@ -903,6 +951,9 @@ remote_connection_open_internal(const char *node_name, List *connection_options)
values[option_pos] = GetDatabaseEncodingName();
option_pos++;
/* Set passfile options */
set_password_options(keywords, values, &option_pos);
/* Set client specific SSL connection options */
set_ssl_options(user_name, keywords, values, &option_pos);
@ -927,9 +978,6 @@ remote_connection_open_internal(const char *node_name, List *connection_options)
return ts_conn;
}
#undef REMOTE_CONNECTION_SSL_OPTIONS
#undef REMOTE_CONNECTION_ADDITIONAL_OPTIONS
/*
* Opens a connection.
*
@ -1389,6 +1437,8 @@ _remote_connection_init(void)
{
RegisterXactCallback(remote_connection_xact_end, NULL);
RegisterSubXactCallback(remote_connection_subxact_end, NULL);
unset_libpq_envvar();
}
void

View File

@ -10,16 +10,16 @@ DO $d$
BEGIN
EXECUTE $$CREATE SERVER loopback_1 FOREIGN DATA WRAPPER timescaledb_fdw
OPTIONS (dbname '$$||current_database()||$$',
host 'localhost',
port '$$||current_setting('port')||$$'
)$$;
EXECUTE $$CREATE SERVER loopback_2 FOREIGN DATA WRAPPER timescaledb_fdw
OPTIONS (dbname '$$||current_database()||$$',
host 'localhost',
port '$$||current_setting('port')||$$'
)$$;
END;
$d$;
CREATE USER MAPPING FOR CURRENT_USER SERVER loopback_1;
CREATE USER MAPPING FOR CURRENT_USER SERVER loopback_2;
SELECT _timescaledb_internal.test_remote_connection_cache();
test_remote_connection_cache
------------------------------

View File

@ -24,3 +24,4 @@ ssl_ca_file='@TEST_OUTPUT_DIR@/ts_root.crt'
ssl_cert_file='@TEST_OUTPUT_DIR@/ts_data_node.crt'
ssl_key_file='@TEST_OUTPUT_DIR@/ts_data_node.key'
timescaledb.ssl_dir='@TEST_OUTPUT_DIR@'
timescaledb.passfile='@TEST_PASSFILE@'

View File

@ -13,16 +13,15 @@ DO $d$
BEGIN
EXECUTE $$CREATE SERVER loopback_1 FOREIGN DATA WRAPPER timescaledb_fdw
OPTIONS (dbname '$$||current_database()||$$',
host 'localhost',
port '$$||current_setting('port')||$$'
)$$;
EXECUTE $$CREATE SERVER loopback_2 FOREIGN DATA WRAPPER timescaledb_fdw
OPTIONS (dbname '$$||current_database()||$$',
host 'localhost',
port '$$||current_setting('port')||$$'
)$$;
END;
$d$;
CREATE USER MAPPING FOR CURRENT_USER SERVER loopback_1;
CREATE USER MAPPING FOR CURRENT_USER SERVER loopback_2;
SELECT _timescaledb_internal.test_remote_connection_cache();

View File

@ -34,9 +34,10 @@ get_connection()
{
return remote_connection_open_with_options(
"testdb",
list_make3(makeDefElem("user",
list_make4(makeDefElem("user",
(Node *) makeString(GetUserNameFromId(GetUserId(), false)),
-1),
makeDefElem("host", (Node *) makeString("localhost"), -1),
makeDefElem("dbname", (Node *) makeString(get_database_name(MyDatabaseId)), -1),
makeDefElem("port",
(Node *) makeString(pstrdup(GetConfigOption("port", false, false))),

View File

@ -28,8 +28,6 @@
#include "test_utils.h"
#include "connection.h"
#define NEW_APPLICATION_NAME "app2"
TS_FUNCTION_INFO_V1(tsl_test_remote_connection_cache);
static void