Lock down regression testing temporary clusters on Windows.
authorNoah Misch
Thu, 18 Dec 2014 03:48:40 +0000 (22:48 -0500)
committerNoah Misch
Thu, 18 Dec 2014 03:48:45 +0000 (22:48 -0500)
Use SSPI authentication to allow connections exclusively from the OS
user that launched the test suite.  This closes on Windows the
vulnerability that commit be76a6d39e2832d4b88c0e1cc381aa44a7f86881
closed on other platforms.  Users of "make installcheck" or custom test
harnesses can run "pg_regress --config-auth=DATADIR" to activate the
same authentication configuration that "make check" would use.
Back-patch to 9.0 (all supported versions).

Security: CVE-2014-0067

contrib/dblink/Makefile
contrib/dblink/expected/dblink.out
contrib/dblink/sql/dblink.sql
contrib/pg_upgrade/test.sh
doc/src/sgml/regress.sgml
src/Makefile.global.in
src/bin/pg_ctl/t/001_start_stop.pl
src/bin/pg_ctl/t/002_status.pl
src/test/perl/TestLib.pm
src/test/regress/pg_regress.c
src/tools/msvc/vcregress.pl

index 7cb023705ad500dedb5a4b7f9dfcc2d9e2f7ce2f..e833b9203aca3cc7be079827be86c642033de3ea 100644 (file)
@@ -9,7 +9,8 @@ EXTENSION = dblink
 DATA = dblink--1.1.sql dblink--1.0--1.1.sql dblink--unpackaged--1.0.sql
 
 REGRESS = paths dblink
-REGRESS_OPTS = --dlpath=$(top_builddir)/src/test/regress
+REGRESS_OPTS = --dlpath=$(top_builddir)/src/test/regress \
+   --create-role=dblink_regression_test
 EXTRA_CLEAN = sql/paths.sql expected/paths.out
 
 # the db name is hard-coded in the tests
index f503691bf218a50fa15f71500c1be9c20af6bddc..87eb142bd30b329d48f0ca55cf30678fb9ec5052 100644 (file)
@@ -809,7 +809,6 @@ SELECT dblink_disconnect('dtest1');
 (1 row)
 
 -- test foreign data wrapper functionality
-CREATE USER dblink_regression_test;
 CREATE SERVER fdtest FOREIGN DATA WRAPPER dblink_fdw
   OPTIONS (dbname 'contrib_regression');
 CREATE USER MAPPING FOR public SERVER fdtest
@@ -851,7 +850,6 @@ SELECT * FROM dblink('myconn','SELECT * FROM foo') AS t(a int, b text, c text[])
 \c - :ORIGINAL_USER
 REVOKE USAGE ON FOREIGN SERVER fdtest FROM dblink_regression_test;
 REVOKE EXECUTE ON FUNCTION dblink_connect_u(text, text) FROM dblink_regression_test;
-DROP USER dblink_regression_test;
 DROP USER MAPPING FOR public SERVER fdtest;
 DROP SERVER fdtest;
 -- test asynchronous notifications
index d8d248260c02d9d6ce10ee258606e3e07ba9b914..5305d5a8f5b0b2a7e8375798a058420ef0b1130e 100644 (file)
@@ -387,7 +387,6 @@ SELECT dblink_error_message('dtest1');
 SELECT dblink_disconnect('dtest1');
 
 -- test foreign data wrapper functionality
-CREATE USER dblink_regression_test;
 CREATE SERVER fdtest FOREIGN DATA WRAPPER dblink_fdw
   OPTIONS (dbname 'contrib_regression');
 CREATE USER MAPPING FOR public SERVER fdtest
@@ -408,7 +407,6 @@ SELECT * FROM dblink('myconn','SELECT * FROM foo') AS t(a int, b text, c text[])
 \c - :ORIGINAL_USER
 REVOKE USAGE ON FOREIGN SERVER fdtest FROM dblink_regression_test;
 REVOKE EXECUTE ON FUNCTION dblink_connect_u(text, text) FROM dblink_regression_test;
-DROP USER dblink_regression_test;
 DROP USER MAPPING FOR public SERVER fdtest;
 DROP SERVER fdtest;
 
index 9d31f9a6e45911332295650aac589dc83e3903c5..b7c6fc3b5b9672f956a6d71d0caffd9b6a037a01 100644 (file)
@@ -17,13 +17,20 @@ set -e
 unset MAKEFLAGS
 unset MAKELEVEL
 
+# Run a given "initdb" binary and overlay the regression testing
+# authentication configuration.
+standard_initdb() {
+   "$1" -N
+   ../../src/test/regress/pg_regress --config-auth "$PGDATA"
+}
+
 # Establish how the server will listen for connections
 testhost=`uname -s`
 
 case $testhost in
    MINGW*)
        LISTEN_ADDRESSES="localhost"
-       PGHOST=""; unset PGHOST
+       PGHOST=localhost
        ;;
    *)
        LISTEN_ADDRESSES=""
@@ -49,11 +56,11 @@ case $testhost in
            trap 'rm -rf "$PGHOST"' 0
            trap 'exit 3' 1 2 13 15
        fi
-       export PGHOST
        ;;
 esac
 
 POSTMASTER_OPTS="-F -c listen_addresses=$LISTEN_ADDRESSES -k \"$PGHOST\""
+export PGHOST
 
 temp_root=$PWD/tmp_check
 
@@ -141,7 +148,7 @@ export EXTRA_REGRESS_OPTS
 # enable echo so the user can see what is being executed
 set -x
 
-$oldbindir/initdb -N
+standard_initdb "$oldbindir"/initdb
 $oldbindir/pg_ctl start -l "$logdir/postmaster1.log" -o "$POSTMASTER_OPTS" -w
 if "$MAKE" -C "$oldsrc" installcheck; then
    pg_dumpall -f "$temp_root"/dump1.sql || pg_dumpall1_status=$?
@@ -181,7 +188,7 @@ fi
 
 PGDATA=$BASE_PGDATA
 
-initdb -N
+standard_initdb 'initdb'
 
 pg_upgrade $PG_UPGRADE_OPTS -d "${PGDATA}.old" -D "${PGDATA}" -b "$oldbindir" -B "$bindir" -p "$PGPORT" -P "$PGPORT"
 
index 71196a1aca310161f15a50dfef236d53f679d37c..504d8daa71be02c87b21694974553778375e8890 100644 (file)
@@ -56,19 +56,6 @@ make check
    failure represents a serious problem.
   
 
-  
-   
-    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 make check on an affected
-    system shared with untrusted users.  Instead, run the tests after
-    completing the installation, as described in the next section.
-   
-  
-
    
     Because this test method runs a temporary server, it will not work
     if you did the build as the root user, since the server will not start as
index 377812ba1ae2ceb42a80c947664e5faac6738cdb..f9ce7e4dcc20bc2975828ca8003a0b53aa1cad59 100644 (file)
@@ -323,7 +323,7 @@ endef
 define prove_check
 $(MKDIR_P) tmp_check/log
 $(MAKE) -C $(top_builddir) DESTDIR='$(CURDIR)'/tmp_check/install install >'$(CURDIR)'/tmp_check/log/install.log 2>&1
-cd $(srcdir) && TESTDIR='$(CURDIR)' PATH="$(CURDIR)/tmp_check/install$(bindir):$$PATH" $(call add_to_path,$(ld_library_path_var),$(CURDIR)/tmp_check/install$(libdir)) PGPORT='6$(DEF_PGPORT)' $(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) t/*.pl
+cd $(srcdir) && TESTDIR='$(CURDIR)' PATH="$(CURDIR)/tmp_check/install$(bindir):$$PATH" $(call add_to_path,$(ld_library_path_var),$(CURDIR)/tmp_check/install$(libdir)) top_srcdir='$(top_srcdir)' PGPORT='6$(DEF_PGPORT)' $(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) t/*.pl
 endef
 
 else
index 61dfab77eb3ecfb81e3fd1985054ea06458aebdc..be76f314b0c7bca38dd52fcaa0b736d82dad1b18 100644 (file)
@@ -1,7 +1,7 @@
 use strict;
 use warnings;
 use TestLib;
-use Test::More tests => 15;
+use Test::More tests => 16;
 
 my $tempdir = TestLib::tempdir;
 my $tempdir_short = TestLib::tempdir_short;
@@ -11,6 +11,10 @@ program_version_ok('pg_ctl');
 program_options_handling_ok('pg_ctl');
 
 command_ok([ 'pg_ctl', 'initdb', '-D', "$tempdir/data" ], 'pg_ctl initdb');
+command_ok(
+   [   "$ENV{top_srcdir}/src/test/regress/pg_regress", '--config-auth',
+       "$tempdir/data" ],
+   'configure authentication');
 open CONF, ">>$tempdir/data/postgresql.conf";
 print CONF "listen_addresses = ''\n";
 print CONF "unix_socket_directories = '$tempdir_short'\n";
index e10fd1b3e390daa28e85f5493971c0265441390d..9f84f3849c05e608676d9fb7607f14ccc0003108 100644 (file)
@@ -6,7 +6,7 @@ use Test::More tests => 2;
 my $tempdir = TestLib::tempdir;
 my $tempdir_short = TestLib::tempdir_short;
 
-system_or_bail "initdb -D '$tempdir'/data -A trust >/dev/null";
+standard_initdb "$tempdir/data";
 open CONF, ">>$tempdir/data/postgresql.conf";
 print CONF "listen_addresses = ''\n";
 print CONF "unix_socket_directories = '$tempdir_short'\n";
index 46a8bece1e50c9f1f561f87b54a139853aa506db..57abdb92bf47442ada3a2ebe3a688ee77bd64eaf 100644 (file)
@@ -7,6 +7,7 @@ use Exporter 'import';
 our @EXPORT = qw(
   tempdir
   tempdir_short
+  standard_initdb
   start_test_server
   restart_test_server
   psql
@@ -69,6 +70,14 @@ sub tempdir_short
    return File::Temp::tempdir(CLEANUP => 1);
 }
 
+sub standard_initdb
+{
+   my $pgdata = shift;
+   system_or_bail("initdb -D '$pgdata' -A trust -N >/dev/null");
+   system_or_bail("$ENV{top_srcdir}/src/test/regress/pg_regress",
+                  '--config-auth', $pgdata);
+}
+
 my ($test_server_datadir, $test_server_logfile);
 
 sub start_test_server
@@ -78,7 +87,7 @@ sub start_test_server
 
    my $tempdir_short = tempdir_short;
 
-   system "initdb -D '$tempdir'/pgdata -A trust -N >/dev/null";
+   standard_initdb "$tempdir/pgdata";
    $ret = system 'pg_ctl', '-D', "$tempdir/pgdata", '-s', '-w', '-l',
      "$tempdir/logfile", '-o',
      "--fsync=off -k $tempdir_short --listen-addresses='' --log-statement=all",
index 27c46abc96af1375488b43e882b030c19d10dcbf..cb092f9d82112c84391d0a7c6ac5b48aaa6973b0 100644 (file)
@@ -29,6 +29,7 @@
 #include 
 #endif
 
+#include "common/username.h"
 #include "getopt_long.h"
 #include "libpq/pqcomm.h"      /* needed for UNIXSOCK_PATH() */
 #include "pg_config_paths.h"
@@ -104,6 +105,7 @@ static char *dlpath = PKGLIBDIR;
 static char *user = NULL;
 static _stringlist *extraroles = NULL;
 static _stringlist *extra_install = NULL;
+static char *config_auth_datadir = NULL;
 
 /* internal variables */
 static const char *progname;
@@ -965,6 +967,150 @@ initialize_environment(void)
    load_resultmap();
 }
 
+#ifdef ENABLE_SSPI
+/*
+ * Get account and domain/realm names for the current user.  This is based on
+ * pg_SSPI_recvauth().  The returned strings use static storage.
+ */
+static void
+current_windows_user(const char **acct, const char **dom)
+{
+   static char accountname[MAXPGPATH];
+   static char domainname[MAXPGPATH];
+   HANDLE      token;
+   TOKEN_USER *tokenuser;
+   DWORD       retlen;
+   DWORD       accountnamesize = sizeof(accountname);
+   DWORD       domainnamesize = sizeof(domainname);
+   SID_NAME_USE accountnameuse;
+
+   if (!OpenProcessToken(GetCurrentProcess(), TOKEN_READ, &token))
+   {
+       fprintf(stderr,
+               _("%s: could not open process token: error code %lu\n"),
+               progname, GetLastError());
+       exit(2);
+   }
+
+   if (!GetTokenInformation(token, TokenUser, NULL, 0, &retlen) && GetLastError() != 122)
+   {
+       fprintf(stderr,
+               _("%s: could not get token user size: error code %lu\n"),
+               progname, GetLastError());
+       exit(2);
+   }
+   tokenuser = malloc(retlen);
+   if (!GetTokenInformation(token, TokenUser, tokenuser, retlen, &retlen))
+   {
+       fprintf(stderr,
+               _("%s: could not get token user: error code %lu\n"),
+               progname, GetLastError());
+       exit(2);
+   }
+
+   if (!LookupAccountSid(NULL, tokenuser->User.Sid, accountname, &accountnamesize,
+                         domainname, &domainnamesize, &accountnameuse))
+   {
+       fprintf(stderr,
+               _("%s: could not look up account SID: error code %lu\n"),
+               progname, GetLastError());
+       exit(2);
+   }
+
+   free(tokenuser);
+
+   *acct = accountname;
+   *dom = domainname;
+}
+
+/*
+ * Rewrite pg_hba.conf and pg_ident.conf to use SSPI authentication.  Permit
+ * the current OS user to authenticate as the bootstrap superuser and as any
+ * user named in a --create-role option.
+ */
+static void
+config_sspi_auth(const char *pgdata)
+{
+   const char *accountname,
+              *domainname;
+   const char *username;
+   char       *errstr;
+   char        fname[MAXPGPATH];
+   int         res;
+   FILE       *hba,
+              *ident;
+   _stringlist *sl;
+
+   /*
+    * "username", the initdb-chosen bootstrap superuser name, may always
+    * match "accountname", the value SSPI authentication discovers.  The
+    * underlying system functions do not clearly guarantee that.
+    */
+   current_windows_user(&accountname, &domainname);
+   username = get_user_name(&errstr);
+   if (username == NULL)
+   {
+       fprintf(stderr, "%s: %s\n", progname, errstr);
+       exit(2);
+   }
+
+   /* Check a Write outcome and report any error. */
+#define CW(cond)   \
+   do { \
+       if (!(cond)) \
+       { \
+           fprintf(stderr, _("%s: could not write to file \"%s\": %s\n"), \
+                   progname, fname, strerror(errno)); \
+           exit(2); \
+       } \
+   } while (0)
+
+   res = snprintf(fname, sizeof(fname), "%s/pg_hba.conf", pgdata);
+   if (res < 0 || res >= sizeof(fname) - 1)
+   {
+       /*
+        * Truncating this name is a fatal error, because we must not fail to
+        * overwrite an original trust-authentication pg_hba.conf.
+        */
+       fprintf(stderr, _("%s: directory name too long\n"), progname);
+       exit(2);
+   }
+   hba = fopen(fname, "w");
+   if (hba == NULL)
+   {
+       fprintf(stderr, _("%s: could not open file \"%s\" for writing: %s\n"),
+               progname, fname, strerror(errno));
+       exit(2);
+   }
+   CW(fputs("# Configuration written by config_sspi_auth()\n", hba) >= 0);
+   CW(fputs("host all all 127.0.0.1/32  sspi include_realm=1 map=regress\n",
+            hba) >= 0);
+   CW(fclose(hba) == 0);
+
+   snprintf(fname, sizeof(fname), "%s/pg_ident.conf", pgdata);
+   ident = fopen(fname, "w");
+   if (ident == NULL)
+   {
+       fprintf(stderr, _("%s: could not open file \"%s\" for writing: %s\n"),
+               progname, fname, strerror(errno));
+       exit(2);
+   }
+   CW(fputs("# Configuration written by config_sspi_auth()\n", ident) >= 0);
+
+   /*
+    * Double-quote for the benefit of account names containing whitespace or
+    * '#'.  Windows forbids the double-quote character itself, so don't
+    * bother escaping embedded double-quote characters.
+    */
+   CW(fprintf(ident, "regress  \"%s@%s\"  \"%s\"\n",
+              accountname, domainname, username) >= 0);
+   for (sl = extraroles; sl; sl = sl->next)
+       CW(fprintf(ident, "regress  \"%s@%s\"  \"%s\"\n",
+                  accountname, domainname, sl->str) >= 0);
+   CW(fclose(ident) == 0);
+}
+#endif
+
 /*
  * Issue a command via psql, connecting to the specified database
  *
@@ -1957,6 +2103,7 @@ help(void)
    printf(_("Usage:\n  %s [OPTION]... [EXTRA-TEST]...\n"), progname);
    printf(_("\n"));
    printf(_("Options:\n"));
+   printf(_("  --config-auth=DATADIR     update authentication settings for DATADIR\n"));
    printf(_("  --create-role=ROLE        create the specified role before testing\n"));
    printf(_("  --dbname=DB               use database DB (default \"regression\")\n"));
    printf(_("  --debug                   turn on debug mode in programs that are run\n"));
@@ -2023,6 +2170,7 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc
        {"launcher", required_argument, NULL, 21},
        {"load-extension", required_argument, NULL, 22},
        {"extra-install", required_argument, NULL, 23},
+       {"config-auth", required_argument, NULL, 24},
        {NULL, 0, NULL, 0}
    };
 
@@ -2137,6 +2285,9 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc
            case 23:
                add_stringlist_item(&extra_install, optarg);
                break;
+           case 24:
+               config_auth_datadir = pstrdup(optarg);
+               break;
            default:
                /* getopt_long already emitted a complaint */
                fprintf(stderr, _("\nTry \"%s -h\" for more information.\n"),
@@ -2154,6 +2305,14 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc
        optind++;
    }
 
+   if (config_auth_datadir)
+   {
+#ifdef ENABLE_SSPI
+       config_sspi_auth(config_auth_datadir);
+#endif
+       exit(0);
+   }
+
    if (temp_install && !port_specified_by_user)
 
        /*
@@ -2298,6 +2457,18 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc
 
        fclose(pg_conf);
 
+#ifdef ENABLE_SSPI
+
+       /*
+        * Since we successfully used the same buffer for the much-longer
+        * "initdb" command, this can't truncate.
+        */
+       snprintf(buf, sizeof(buf), "%s/data", temp_install);
+       config_sspi_auth(buf);
+#elif !defined(HAVE_UNIX_SOCKETS)
+#error Platform has no means to secure the test installation.
+#endif
+
        /*
         * Check if there is a postmaster running already.
         */
index 30a322eecfcf21560eb4100bc75121a434e8f49b..d5d398c995a6c91bb612a85a159328e0d0c766c2 100644 (file)
@@ -247,6 +247,15 @@ sub contribcheck
    exit $mstat if $mstat;
 }
 
+# Run "initdb", then reconfigure authentication.
+sub standard_initdb
+{
+   return (
+       system('initdb', '-N') == 0 and system(
+           "$topdir/$Config/pg_regress/pg_regress", '--config-auth',
+           $ENV{PGDATA}) == 0);
+}
+
 sub upgradecheck
 {
    my $status;
@@ -258,6 +267,7 @@ sub upgradecheck
    # i.e. only this version to this version check. That's
    # what pg_upgrade's "make check" does.
 
+   $ENV{PGHOST} = 'localhost';
    $ENV{PGPORT} ||= 50432;
    my $tmp_root = "$topdir/contrib/pg_upgrade/tmp_check";
    (mkdir $tmp_root || die $!) unless -d $tmp_root;
@@ -275,7 +285,7 @@ sub upgradecheck
    my $logdir = "$topdir/contrib/pg_upgrade/log";
    (mkdir $logdir || die $!) unless -d $logdir;
    print "\nRunning initdb on old cluster\n\n";
-   system("initdb") == 0 or exit 1;
+   standard_initdb() or exit 1;
    print "\nStarting old cluster\n\n";
    system("pg_ctl start -l $logdir/postmaster1.log -w") == 0 or exit 1;
    print "\nSetting up data for upgrading\n\n";
@@ -289,7 +299,7 @@ sub upgradecheck
    system("pg_ctl -m fast stop") == 0 or exit 1;
    $ENV{PGDATA} = "$data";
    print "\nSetting up new cluster\n\n";
-   system("initdb") == 0 or exit 1;
+   standard_initdb() or exit 1;
    print "\nRunning pg_upgrade\n\n";
    system("pg_upgrade -d $data.old -D $data -b $bindir -B $bindir") == 0
      or exit 1;