]> granicus.if.org Git - postgresql/commitdiff
Lock down regression testing temporary clusters on Windows.
authorNoah Misch <noah@leadboat.com>
Thu, 18 Dec 2014 03:48:40 +0000 (22:48 -0500)
committerNoah Misch <noah@leadboat.com>
Thu, 18 Dec 2014 03:48:47 +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/test/regress/pg_regress.c
src/tools/msvc/vcregress.pl

index 5f01ec45fbc9940594cf6cb6da2178998df41d79..721ca091962fd356b6124db6a778ad701c473384 100644 (file)
@@ -10,7 +10,8 @@ EXTENSION = dblink
 DATA = dblink--1.0.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 9650fa13afd922d3455a18ea317816c87c6fd747..d3dae39ab539c49af2fff7bfb82fdaaa25e1a30c 100644 (file)
@@ -811,7 +811,6 @@ SELECT dblink_disconnect('dtest1');
 (1 row)
 
 -- test foreign data wrapper functionality
-CREATE USER dblink_regression_test;
 CREATE FOREIGN DATA WRAPPER postgresql;
 CREATE SERVER fdtest FOREIGN DATA WRAPPER postgresql OPTIONS (dbname 'contrib_regression');
 CREATE USER MAPPING FOR public SERVER fdtest;
@@ -849,7 +848,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;
 DROP FOREIGN DATA WRAPPER postgresql;
index e89e016df7303bf1d4298a029e6e1cf3b01ebe1b..d0ac578b0c72959360034d5e7d8dfe739b9b9ebe 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 FOREIGN DATA WRAPPER postgresql;
 CREATE SERVER fdtest FOREIGN DATA WRAPPER postgresql OPTIONS (dbname 'contrib_regression');
@@ -406,7 +405,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;
 DROP FOREIGN DATA WRAPPER postgresql;
index 8fa2ccc094e52b27d17b27983be71c90895095ef..d44c599a8ee9618f7de571046989bf903373eb1c 100644 (file)
@@ -15,13 +15,20 @@ set -e
 : ${PGPORT=50432}
 export PGPORT
 
+# Run a given "initdb" binary and overlay the regression testing
+# authentication configuration.
+standard_initdb() {
+       "$1"
+       ../../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=""
@@ -47,11 +54,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
 
@@ -117,7 +124,7 @@ mkdir "$logdir"
 # enable echo so the user can see what is being executed
 set -x
 
-$oldbindir/initdb
+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=$?
@@ -157,7 +164,7 @@ fi
 
 PGDATA=$BASE_PGDATA
 
-initdb
+standard_initdb 'initdb'
 
 pg_upgrade -d "${PGDATA}.old" -D "${PGDATA}" -b "$oldbindir" -B "$bindir"
 
index 86549e239a4745e05fb982248ff3d10cc44e429c..f4143dcdebd6167ee80deab68bad764661290fbb 100644 (file)
@@ -56,19 +56,6 @@ gmake check
    <quote>failure</> represents a serious problem.
   </para>
 
-  <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.
-   </para>
-  </warning>
-
    <para>
     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 7f6bef9fd73a02ccdc8e9f26c182aeae91073260..0b3f645579c65e0f146ca1e43be8e7ca7d0c9c43 100644 (file)
@@ -105,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;
@@ -970,6 +971,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;
+       char            username[128];
+       DWORD           sz = sizeof(username) - 1;
+       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);
+       if (!GetUserName(username, &sz))
+       {
+               fprintf(stderr, _("%s: could not get current user name: %s\n"),
+                               progname, strerror(errno));
+               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
  *
@@ -1969,6 +2114,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"));
@@ -2042,6 +2188,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}
        };
 
@@ -2146,6 +2293,14 @@ 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 = strdup(optarg);
+                               if (!config_auth_datadir)
+                               {
+                                       fprintf(stderr, _("out of memory\n"));
+                                       exit(EXIT_FAILURE);
+                               }
+                               break;
                        default:
                                /* getopt_long already emitted a complaint */
                                fprintf(stderr, _("\nTry \"%s -h\" for more information.\n"),
@@ -2163,6 +2318,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)
 
                /*
@@ -2303,6 +2466,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 d6ab91782c6360fcea13d5dd48d4f92a33fef899..6eb7cfc8b82beef36990e330a5092090a6fef1ab 100644 (file)
@@ -241,6 +241,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;
@@ -252,6 +261,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;
@@ -268,7 +278,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";
@@ -281,7 +291,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;