From 244590ebfb0c2856a5b349fb50836d28722c893d Mon Sep 17 00:00:00 2001 From: Noah Misch Date: Sat, 29 Mar 2014 03:12:00 -0400 Subject: [PATCH] Revert "Secure Unix-domain sockets of "make check" temporary clusters." About half of the buildfarm members use too-long directory names, strongly suggesting that this approach is a dead end. --- contrib/pg_upgrade/test.sh | 9 ++++----- doc/src/sgml/regress.sgml | 34 +++++++++++++++------------------- src/test/regress/pg_regress.c | 32 +++++++------------------------- 3 files changed, 26 insertions(+), 49 deletions(-) diff --git a/contrib/pg_upgrade/test.sh b/contrib/pg_upgrade/test.sh index 9fa0397790..21a44b4b44 100644 --- a/contrib/pg_upgrade/test.sh +++ b/contrib/pg_upgrade/test.sh @@ -25,6 +25,8 @@ case $testhost in *) LISTEN_ADDRESSES="" ;; esac +POSTMASTER_OPTS="-F -c listen_addresses=$LISTEN_ADDRESSES" + temp_root=$PWD/tmp_check if [ "$1" = '--install' ]; then @@ -84,16 +86,13 @@ PGSERVICE=""; unset PGSERVICE PGSSLMODE=""; unset PGSSLMODE PGREQUIRESSL=""; unset PGREQUIRESSL PGCONNECT_TIMEOUT=""; unset PGCONNECT_TIMEOUT +PGHOST=""; unset PGHOST PGHOSTADDR=""; unset PGHOSTADDR -# Select a port number and socket directory, similarly to pg_regress.c +# Select a non-conflicting port number, 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 d2367c086d..0849c77810 100644 --- a/doc/src/sgml/regress.sgml +++ b/doc/src/sgml/regress.sgml @@ -58,14 +58,21 @@ gmake check - 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. + 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. @@ -104,17 +111,6 @@ 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 ae96916aa6..5eddb36e9e 100644 --- a/src/test/regress/pg_regress.c +++ b/src/test/regress/pg_regress.c @@ -109,7 +109,6 @@ static const char *progname; static char *logfilename; static FILE *logfile; static char *difffilename; -static char *sockdir; static _resultmap *resultmap = NULL; @@ -760,7 +759,8 @@ 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 - * PGHOST depending on whether we are using TCP or Unix sockets. + * or unset PGHOST depending on whether we are using TCP or Unix + * sockets. */ unsetenv("PGDATABASE"); unsetenv("PGUSER"); @@ -772,24 +772,7 @@ initialize_environment(void) if (hostname != NULL) doputenv("PGHOST", hostname); else - { - 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("PGHOST"); unsetenv("PGHOSTADDR"); if (port != -1) { @@ -2266,11 +2249,10 @@ 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\" -k \"%s\" " - "> \"%s/log/postmaster.log\" 2>&1" SYSTEMQUOTE, - bindir, temp_install, debug ? " -d 5" : "", - hostname ? hostname : "", sockdir ? sockdir : "", + 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 : "", outputdir); postmaster_pid = spawn_process(buf); if (postmaster_pid == INVALID_PID) -- 2.40.0