]> granicus.if.org Git - postgresql/commitdiff
Secure Unix-domain sockets of "make check" temporary clusters.
authorNoah Misch <noah@leadboat.com>
Sat, 14 Jun 2014 13:41:13 +0000 (09:41 -0400)
committerNoah Misch <noah@leadboat.com>
Sat, 14 Jun 2014 13:41:17 +0000 (09:41 -0400)
Any OS user able to access the socket can connect as the bootstrap
superuser and proceed to execute arbitrary code as the OS user running
the test.  Protect against that by placing the socket in a temporary,
mode-0700 subdirectory of /tmp.  The pg_regress-based test suites and
the pg_upgrade test suite were vulnerable; the $(prove_check)-based test
suites were already secure.  Back-patch to 8.4 (all supported versions).
The hazard remains wherever the temporary cluster accepts TCP
connections, notably on Windows.

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
doc/src/sgml/regress.sgml
src/test/regress/.gitignore
src/test/regress/GNUmakefile
src/test/regress/pg_regress.c

index 28130f16010720104aae2b73c55334905a73b86d..8fa2ccc094e52b27d17b27983be71c90895095ef 100644 (file)
@@ -15,14 +15,43 @@ set -e
 : ${PGPORT=50432}
 export PGPORT
 
+# Establish how the server will listen for connections
 testhost=`uname -s`
 
 case $testhost in
-       MINGW*) LISTEN_ADDRESSES="localhost" ;;
-       *)              LISTEN_ADDRESSES="" ;;
+       MINGW*)
+               LISTEN_ADDRESSES="localhost"
+               PGHOST=""; unset PGHOST
+               ;;
+       *)
+               LISTEN_ADDRESSES=""
+               # Select a socket directory.  The algorithm is from the "configure"
+               # script; the outcome mimics pg_regress.c:make_temp_sockdir().
+               PGHOST=$PG_REGRESS_SOCK_DIR
+               if [ "x$PGHOST" = x ]; then
+                       {
+                               dir=`(umask 077 &&
+                                         mktemp -d /tmp/pg_upgrade_check-XXXXXX) 2>/dev/null` &&
+                               [ -d "$dir" ]
+                       } ||
+                       {
+                               dir=/tmp/pg_upgrade_check-$$-$RANDOM
+                               (umask 077 && mkdir "$dir")
+                       } ||
+                       {
+                               echo "could not create socket temporary directory in \"/tmp\""
+                               exit 1
+                       }
+
+                       PGHOST=$dir
+                       trap 'rm -rf "$PGHOST"' 0
+                       trap 'exit 3' 1 2 13 15
+               fi
+               export PGHOST
+               ;;
 esac
 
-POSTMASTER_OPTS="-F -c listen_addresses=$LISTEN_ADDRESSES"
+POSTMASTER_OPTS="-F -c listen_addresses=$LISTEN_ADDRESSES -k \"$PGHOST\""
 
 temp_root=$PWD/tmp_check
 
@@ -79,7 +108,6 @@ PGSERVICE="";         unset PGSERVICE
 PGSSLMODE="";         unset PGSSLMODE
 PGREQUIRESSL="";      unset PGREQUIRESSL
 PGCONNECT_TIMEOUT=""; unset PGCONNECT_TIMEOUT
-PGHOST="";            unset PGHOST
 PGHOSTADDR="";        unset PGHOSTADDR
 
 logdir=$PWD/log
index 466b492d25e19f23918987fe784bc009548e2481..86549e239a4745e05fb982248ff3d10cc44e429c 100644 (file)
@@ -58,21 +58,14 @@ gmake check
 
   <warning>
    <para>
-    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.
+    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.
    </para>
   </warning>
 
index 7573addc94dbecf4b2b2233c55463b0dc747b251..86a85f7eb7326edd9ece0b7ab22a57791cef578b 100644 (file)
@@ -1,3 +1,5 @@
+/pqsignal.c
+
 # Local binaries
 /pg_regress
 
index 0239b6ff70bbb2fb7344f7ed3bddd46264fabd48..3bfaf2580d1ff3175a036fe2cc6d71de1fc8f815 100644 (file)
@@ -43,13 +43,18 @@ EXTRADEFS = '-DHOST_TUPLE="$(host_tuple)"' \
 
 all: pg_regress$(X)
 
-pg_regress$(X): pg_regress.o pg_regress_main.o | submake-libpgport
+pg_regress$(X): pg_regress.o pg_regress_main.o pqsignal.o | submake-libpgport
        $(CC) $(CFLAGS) $^ $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@
 
 # dependencies ensure that path changes propagate
 pg_regress.o: pg_regress.c $(top_builddir)/src/port/pg_config_paths.h
 pg_regress.o: override CPPFLAGS += -I$(top_builddir)/src/port $(EXTRADEFS)
 
+# Pull in pqsignal like initdb does.
+pqsignal.c: % : $(top_srcdir)/src/interfaces/libpq/%
+       rm -f $@ && $(LN_S) $< .
+pqsignal.o: override CPPFLAGS += -I$(libpq_srcdir)
+
 $(top_builddir)/src/port/pg_config_paths.h: $(top_builddir)/src/Makefile.global
        $(MAKE) -C $(top_builddir)/src/port pg_config_paths.h
 
@@ -166,7 +171,7 @@ bigcheck: all tablespace-setup
 clean distclean maintainer-clean: clean-lib
 # things built by `all' target
        rm -f $(OBJS) refint$(DLSUFFIX) autoinc$(DLSUFFIX) dummy_seclabel$(DLSUFFIX)
-       rm -f pg_regress_main.o pg_regress.o pg_regress$(X)
+       rm -f pqsignal.c pg_regress_main.o pg_regress.o pqsignal.o pg_regress$(X)
 # things created by various check targets
        rm -f $(output_files) $(input_files)
        rm -rf testtablespace
index 0ea14ac57d746f31d5dfdbd1b774c6b396773173..7f6bef9fd73a02ccdc8e9f26c182aeae91073260 100644 (file)
@@ -30,6 +30,8 @@
 #endif
 
 #include "getopt_long.h"
+#include "libpq/pqcomm.h"              /* needed for UNIXSOCK_PATH() */
+#include "libpq/pqsignal.h"
 #include "pg_config_paths.h"
 
 /* for resultmap we need a list of pairs of strings */
@@ -109,6 +111,12 @@ static const char *progname;
 static char *logfilename;
 static FILE *logfile;
 static char *difffilename;
+static const char *sockdir;
+#ifdef HAVE_UNIX_SOCKETS
+static const char *temp_sockdir;
+static char sockself[MAXPGPATH];
+static char socklock[MAXPGPATH];
+#endif
 
 static _resultmap *resultmap = NULL;
 
@@ -307,6 +315,81 @@ stop_postmaster(void)
        }
 }
 
+#ifdef HAVE_UNIX_SOCKETS
+/*
+ * Remove the socket temporary directory.  pg_regress never waits for a
+ * postmaster exit, so it is indeterminate whether the postmaster has yet to
+ * unlink the socket and lock file.  Unlink them here so we can proceed to
+ * remove the directory.  Ignore errors; leaking a temporary directory is
+ * unimportant.  This can run from a signal handler.  The code is not
+ * acceptable in a Windows signal handler (see initdb.c:trapsig()), but
+ * Windows is not a HAVE_UNIX_SOCKETS platform.
+ */
+static void
+remove_temp(void)
+{
+       unlink(sockself);
+       unlink(socklock);
+       rmdir(temp_sockdir);
+}
+
+/*
+ * Signal handler that calls remove_temp() and reraises the signal.
+ */
+static void
+signal_remove_temp(int signum)
+{
+       remove_temp();
+
+       pqsignal(signum, SIG_DFL);
+       raise(signum);
+}
+
+/*
+ * Create a temporary directory suitable for the server's Unix-domain socket.
+ * The directory will have mode 0700 or stricter, so no other OS user can open
+ * our socket to exploit our use of trust authentication.  Most systems
+ * constrain the length of socket paths well below _POSIX_PATH_MAX, so we
+ * place the directory under /tmp rather than relative to the possibly-deep
+ * current working directory.
+ *
+ * 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.
+ */
+static const char *
+make_temp_sockdir(void)
+{
+       char       *template = strdup("/tmp/pg_regress-XXXXXX");
+
+       temp_sockdir = mkdtemp(template);
+       if (temp_sockdir == NULL)
+       {
+               fprintf(stderr, _("%s: could not create directory \"%s\": %s\n"),
+                               progname, template, strerror(errno));
+               exit(2);
+       }
+
+       /* Stage file names for remove_temp().  Unsafe in a signal handler. */
+       UNIXSOCK_PATH(sockself, port, temp_sockdir);
+       snprintf(socklock, sizeof(socklock), "%s.lock", sockself);
+
+       /* Remove the directory during clean exit. */
+       atexit(remove_temp);
+
+       /*
+        * Remove the directory before dying to the usual signals.  Omit SIGQUIT,
+        * preserving it as a quick, untidy exit.
+        */
+       pqsignal(SIGHUP, signal_remove_temp);
+       pqsignal(SIGINT, signal_remove_temp);
+       pqsignal(SIGPIPE, signal_remove_temp);
+       pqsignal(SIGTERM, signal_remove_temp);
+
+       return temp_sockdir;
+}
+#endif   /* HAVE_UNIX_SOCKETS */
+
 /*
  * Check whether string matches pattern
  *
@@ -759,8 +842,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");
@@ -769,10 +851,19 @@ initialize_environment(void)
                unsetenv("PGREQUIRESSL");
                unsetenv("PGCONNECT_TIMEOUT");
                unsetenv("PGDATA");
+#ifdef HAVE_UNIX_SOCKETS
                if (hostname != NULL)
                        doputenv("PGHOST", hostname);
                else
-                       unsetenv("PGHOST");
+               {
+                       sockdir = getenv("PG_REGRESS_SOCK_DIR");
+                       if (!sockdir)
+                               sockdir = make_temp_sockdir();
+                       doputenv("PGHOST", sockdir);
+               }
+#else
+               doputenv("PGHOST", hostname);
+#endif
                unsetenv("PGHOSTADDR");
                if (port != -1)
                {
@@ -2077,7 +2168,9 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc
                /*
                 * To reduce chances of interference with parallel installations, use
                 * a port number starting in the private range (49152-65535)
-                * calculated from the version number.
+                * calculated from the version number.  This aids !HAVE_UNIX_SOCKETS
+                * systems; elsewhere, the use of a private socket directory already
+                * prevents interference.
                 */
                port = 0xC000 | (PG_VERSION_NUM & 0x3FFF);
 
@@ -2246,10 +2339,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)