Revert "Secure Unix-domain sockets of "make check" temporary clusters."

nmisch · nmisch · commit 563e82073ac5 · 2014-03-29T03:14:50.000-04:00
About half of the buildfarm members use too-long directory names,
strongly suggesting that this approach is a dead end.
diff --git a/doc/src/sgml/regress.sgml b/doc/src/sgml/regress.sgml
@@ -60,14 +60,21 @@ gmake check
 
   <warning>
    <para>
-    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 <literal>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 <literal>gmake
+    check</> on machines shared with untrusted users.  Instead, run the tests
+    after completing the installation, as described in the next section.
+   </para>
+
+   <para>
+    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.
    </para>
   </warning>
 
@@ -107,17 +114,6 @@ gmake MAX_CONNECTIONS=10 check
     runs no more than ten tests concurrently.
    </para>
 
-   <para>
-    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 <literal>_POSIX_PATH_MAX</>, testing may fail to start from a
-    directory with a long name.  Work around this problem by pointing
-    the <envar>PG_REGRESS_SOCK_DIR</> environment variable to a substitute
-    socket directory having a shorter path.  On a multi-user system, give that
-    directory mode <literal>0700</>.
-   </para>
-
   <para>
    To run the tests after installation<![%standalone-ignore;[ (see <xref linkend="installation">)]]>,
    initialize a data area and start the
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
@@ -96,7 +96,6 @@ static const char *progname;
 static char *logfilename;
 static FILE *logfile;
 static char *difffilename;
-static char *sockdir;
 
 static _resultmap *resultmap = NULL;
 
@@ -754,7 +753,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");
@@ -766,24 +766,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)
 		{
@@ -2202,11 +2185,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)
