From 95a3ca3be9f66d7222e4f8188318125992b381d9 Mon Sep 17 00:00:00 2001 From: Noah Misch Date: Sat, 29 Mar 2014 00:52:56 -0400 Subject: [PATCH] Secure Unix-domain sockets of "make check" temporary clusters. Any OS user able to access the socket can connect as the bootstrap superuser and in turn execute arbitrary code as the OS user running the test. Protect against that by placing the socket in the temporary data directory, which has mode 0700 thanks to initdb. Back-patch to 8.4 (all supported versions). The hazard remains wherever the temporary cluster accepts TCP connections, notably on Windows. Attempts to run "make check" from a directory with a long name will now fail. An alternative not sharing that problem was to place the socket in a subdirectory of /tmp, but that is only secure if /tmp is sticky. The PG_REGRESS_SOCK_DIR environment variable is available as a workaround when testing from long directory paths. As a convenient side effect, this lets testing proceed smoothly in builds that override DEFAULT_PGSOCKET_DIR. Popular non-default values like /var/run/postgresql are often unwritable to the build user. Security: CVE-2014-0067 --- contrib/pg_upgrade/test.sh | 9 +++++---- doc/src/sgml/regress.sgml | 34 +++++++++++++++++++--------------- src/test/regress/pg_regress.c | 32 +++++++++++++++++++++++++------- 3 files changed, 49 insertions(+), 26 deletions(-) diff --git a/contrib/pg_upgrade/test.sh b/contrib/pg_upgrade/test.sh index 21a44b4b44..9fa0397790 100644 --- a/contrib/pg_upgrade/test.sh +++ b/contrib/pg_upgrade/test.sh @@ -25,8 +25,6 @@ case $testhost in *) LISTEN_ADDRESSES="" ;; esac -POSTMASTER_OPTS="-F -c listen_addresses=$LISTEN_ADDRESSES" - temp_root=$PWD/tmp_check if [ "$1" = '--install' ]; then @@ -86,13 +84,16 @@ PGSERVICE=""; unset PGSERVICE PGSSLMODE=""; unset PGSSLMODE PGREQUIRESSL=""; unset PGREQUIRESSL PGCONNECT_TIMEOUT=""; unset PGCONNECT_TIMEOUT -PGHOST=""; unset PGHOST PGHOSTADDR=""; unset PGHOSTADDR -# Select a non-conflicting port number, similarly to pg_regress.c +# Select a port number and socket directory, similarly to pg_regress.c PG_VERSION_NUM=`grep '#define PG_VERSION_NUM' $newsrc/src/include/pg_config.h | awk '{print $3}'` PGPORT=`expr $PG_VERSION_NUM % 16384 + 49152` export PGPORT +PGHOST=${PG_REGRESS_SOCK_DIR-$PGDATA} +export PGHOST + +POSTMASTER_OPTS="-F -c listen_addresses=$LISTEN_ADDRESSES -k \"$PGHOST\"" i=0 while psql -X postgres /dev/null diff --git a/doc/src/sgml/regress.sgml b/doc/src/sgml/regress.sgml index 0849c77810..d2367c086d 100644 --- a/doc/src/sgml/regress.sgml +++ b/doc/src/sgml/regress.sgml @@ -58,21 +58,14 @@ gmake check - This test method starts a temporary server, which is configured to accept - any connection originating on the local machine. Any local user can gain - database superuser privileges when connecting to this server, and could - in principle exploit all privileges of the operating-system user running - the tests. Therefore, it is not recommended that you use gmake - check on machines shared with untrusted users. Instead, run the tests - after completing the installation, as described in the next section. - - - - On Unix-like machines, this danger can be avoided if the temporary - server's socket file is made inaccessible to other users, for example - by running the tests in a protected chroot. On Windows, the temporary - server opens a locally-accessible TCP socket, so filesystem protections - cannot help. + On systems lacking Unix-domain sockets, notably Windows, this test method + starts a temporary server configured to accept any connection originating + on the local machine. Any local user can gain database superuser + privileges when connecting to this server, and could in principle exploit + all privileges of the operating-system user running the tests. Therefore, + it is not recommended that you use gmake check on an affected + system shared with untrusted users. Instead, run the tests after + completing the installation, as described in the next section. @@ -111,6 +104,17 @@ gmake MAX_CONNECTIONS=10 check runs no more than ten tests concurrently. + + + To protect your operating system user account, the test driver places the + server's socket in a relative subdirectory inaccessible to other users. + Since most systems constrain the length of socket paths well + below _POSIX_PATH_MAX, testing may fail to start from a + directory with a long name. Work around this problem by pointing + the PG_REGRESS_SOCK_DIR environment variable to a substitute + socket directory having a shorter path. On a multi-user system, give that + directory mode 0700. + diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c index 5eddb36e9e..ae96916aa6 100644 --- a/src/test/regress/pg_regress.c +++ b/src/test/regress/pg_regress.c @@ -109,6 +109,7 @@ static const char *progname; static char *logfilename; static FILE *logfile; static char *difffilename; +static char *sockdir; static _resultmap *resultmap = NULL; @@ -759,8 +760,7 @@ initialize_environment(void) * the wrong postmaster, or otherwise behave in nondefault ways. (Note * we also use psql's -X switch consistently, so that ~/.psqlrc files * won't mess things up.) Also, set PGPORT to the temp port, and set - * or unset PGHOST depending on whether we are using TCP or Unix - * sockets. + * PGHOST depending on whether we are using TCP or Unix sockets. */ unsetenv("PGDATABASE"); unsetenv("PGUSER"); @@ -772,7 +772,24 @@ initialize_environment(void) if (hostname != NULL) doputenv("PGHOST", hostname); else - unsetenv("PGHOST"); + { + sockdir = getenv("PG_REGRESS_SOCK_DIR"); + if (!sockdir) + { + /* + * Since initdb creates the data directory with secure + * permissions, we place the socket there. This ensures no + * other OS user can open our socket to exploit our use of + * trust authentication. Compared to using the compiled-in + * DEFAULT_PGSOCKET_DIR, this also permits testing to work in + * builds that relocate it to a directory not writable to the + * build/test user. + */ + sockdir = malloc(strlen(temp_install) + sizeof("/data")); + sprintf(sockdir, "%s/data", temp_install); + } + doputenv("PGHOST", sockdir); + } unsetenv("PGHOSTADDR"); if (port != -1) { @@ -2249,10 +2266,11 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc */ header(_("starting postmaster")); snprintf(buf, sizeof(buf), - SYSTEMQUOTE "\"%s/postgres\" -D \"%s/data\" -F%s -c \"listen_addresses=%s\" > \"%s/log/postmaster.log\" 2>&1" SYSTEMQUOTE, - bindir, temp_install, - debug ? " -d 5" : "", - hostname ? hostname : "", + SYSTEMQUOTE "\"%s/postgres\" -D \"%s/data\" -F%s " + "-c \"listen_addresses=%s\" -k \"%s\" " + "> \"%s/log/postmaster.log\" 2>&1" SYSTEMQUOTE, + bindir, temp_install, debug ? " -d 5" : "", + hostname ? hostname : "", sockdir ? sockdir : "", outputdir); postmaster_pid = spawn_process(buf); if (postmaster_pid == INVALID_PID) -- 2.40.0