From 6d45ee572d0f698c91495e458b52be35c2b3cae5 Mon Sep 17 00:00:00 2001 From: Noah Misch Date: Wed, 17 Dec 2014 22:48:40 -0500 Subject: [PATCH] Lock down regression testing temporary clusters on Windows. 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 | 3 +- contrib/dblink/expected/dblink.out | 2 - contrib/dblink/sql/dblink.sql | 2 - doc/src/sgml/regress.sgml | 13 --- src/test/regress/pg_regress.c | 175 +++++++++++++++++++++++++++++ 5 files changed, 177 insertions(+), 18 deletions(-) diff --git a/contrib/dblink/Makefile b/contrib/dblink/Makefile index 61763a019e5..9a0a11c1a6a 100644 --- a/contrib/dblink/Makefile +++ b/contrib/dblink/Makefile @@ -8,7 +8,8 @@ SHLIB_LINK = $(libpq) DATA_built = dblink.sql DATA = uninstall_dblink.sql REGRESS = paths dblink -REGRESS_OPTS = --dbname=$(CONTRIB_TESTDB) --dlpath=$(top_builddir)/src/test/regress +REGRESS_OPTS = --dbname=$(CONTRIB_TESTDB) --dlpath=$(top_builddir)/src/test/regress \ + --create-role=dblink_regression_test EXTRA_CLEAN = sql/paths.sql expected/paths.out diff --git a/contrib/dblink/expected/dblink.out b/contrib/dblink/expected/dblink.out index 66413a3add7..c1a2be9d727 100644 --- a/contrib/dblink/expected/dblink.out +++ b/contrib/dblink/expected/dblink.out @@ -817,7 +817,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; @@ -855,7 +854,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; diff --git a/contrib/dblink/sql/dblink.sql b/contrib/dblink/sql/dblink.sql index 4adef490e75..a3df862cd54 100644 --- a/contrib/dblink/sql/dblink.sql +++ b/contrib/dblink/sql/dblink.sql @@ -396,7 +396,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'); @@ -415,7 +414,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; diff --git a/doc/src/sgml/regress.sgml b/doc/src/sgml/regress.sgml index 333b2159b9d..ac30ba293c0 100644 --- a/doc/src/sgml/regress.sgml +++ b/doc/src/sgml/regress.sgml @@ -55,19 +55,6 @@ gmake 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 gmake 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 diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c index fcf9fbf6db4..5380dd21118 100644 --- a/src/test/regress/pg_regress.c +++ b/src/test/regress/pg_regress.c @@ -102,6 +102,7 @@ static bool port_specified_by_user = false; static char *dlpath = PKGLIBDIR; static char *user = NULL; static _stringlist *extraroles = NULL; +static char *config_auth_datadir = NULL; /* internal variables */ static const char *progname; @@ -974,6 +975,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 * @@ -1963,6 +2108,7 @@ help(void) printf(_("Usage: %s [options...] [extra tests...]\n"), progname); printf(_("\n")); printf(_("Options:\n")); + printf(_(" --config-auth=DATADIR update authentication settings for DATADIR\n")); printf(_(" --dbname=DB use database DB (default \"regression\")\n")); printf(_(" --debug turn on debug mode in programs that are run\n")); printf(_(" --inputdir=DIR take input files from DIR (default \".\")\n")); @@ -2029,6 +2175,7 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc {"create-role", required_argument, NULL, 18}, {"temp-config", required_argument, NULL, 19}, {"use-existing", no_argument, NULL, 20}, + {"config-auth", required_argument, NULL, 24}, {NULL, 0, NULL, 0} }; @@ -2122,6 +2269,14 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc case 20: use_existing = true; 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"), @@ -2139,6 +2294,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) /* @@ -2260,6 +2423,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. */