8000 Secure Unix-domain sockets of "make check" temporary clusters. · lansz/postgres@95cefd3 · GitHub
[go: up one dir, main page]
Skip to content

Commit 95cefd3

Browse files
nmischnmisch
committed
Secure Unix-domain sockets of "make check" temporary clusters.
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
1 parent 8b0d1c8 commit 95cefd3

File tree

4 files changed

+120
-25
lines changed

4 files changed

+120
-25
lines changed

‎doc/src/sgml/regress.sgml

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -60,21 +60,14 @@ gmake check
6060

6161
<warning>
6262
<para>
63-
This test method starts a temporary server, which is configured to accept
64-
any connection originating on the local machine. Any local user can gain
65-
database superuser privileges when connecting to this server, and could
66-
in principle exploit all privileges of the operating-system user running
67-
the tests. Therefore, it is not recommended that you use <literal>gmake
68-
check</> on machines shared with untrusted users. Instead, run the tests
69-
after completing the installation, as described in the next section.
70-
</para>
71-
72-
<para>
73-
On Unix-like machines, this danger can be avoided if the temporary
74-
server's socket file is made inaccessible to other users, for example
75-
by running the tests in a protected chroot. On Windows, the temporary
76-
server opens a locally-accessible TCP socket, so filesystem protections
77-
cannot help.
63+
On systems lacking Unix-domain sockets, notably Windows, this test method
64+
starts a temporary server configured to accept any connection originating
65+
on the local machine. Any local user can gain database superuser
66+
privileges when connecting to this server, and could in principle exploit
67+
all privileges of the operating-system user running the tests. Therefore,
68+
it is not recommended that you use <literal>gmake check</> on an affected
69+
system shared with untrusted users. Instead, run the tests after
70+
completing the installation, as described in the next section.
7871
</para>
7972
</warning>
8073

‎src/test/regress/.gitignore

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
/pqsignal.c
2+
13
# Local binaries
24
/pg_regress
35

‎src/test/regress/GNUmakefile

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,13 +52,18 @@ EXTRADEFS = '-DHOST_TUPLE="$(host_tuple)"' \
5252

5353
all: submake-libpgport pg_regress$(X)
5454

55-
pg_regress$(X): pg_regress.o pg_regress_main.o
55+
pg_regress$(X): pg_regress.o pg_regress_main.o pqsignal.o
5656
$(CC) $(CFLAGS) $^ $(LDFLAGS) $(LIBS) -o $@
5757

5858
# dependencies ensure that path changes propagate
5959
pg_regress.o: pg_regress.c $(top_builddir)/src/port/pg_config_paths.h
6060
$(CC) $(CFLAGS) $(CPPFLAGS) -I$(top_builddir)/src/port $(EXTRADEFS) -c -o $@ $<
6161

62+
# Pull in pqsignal like initdb does.
63+
pqsignal.c: % : $(top_srcdir)/src/interfaces/libpq/%
64+
rm -f $@ && $(LN_S) $< .
65+
pqsignal.o: override CPPFLAGS += -I$(libpq_srcdir)
66+
6267
$(top_builddir)/src/port/pg_config_paths.h: $(top_builddir)/src/Makefile.global
6368
$(MAKE) -C $(top_builddir)/src/port pg_config_paths.h
6469

@@ -169,7 +174,8 @@ bigcheck: all
169174

170175
clean distclean maintainer-clean: clean-lib
171176
# things built by `all' target
172-
rm -f $(OBJS) refint$(DLSUFFIX) autoinc$(DLSUFFIX) pg_regress_main.o pg_regress.o pg_regress$(X)
177+
rm -f $(OBJS) refint$(DLSUFFIX) autoinc$(DLSUFFIX)
178+
rm -f pqsignal.c pg_regress_main.o pg_regress.o pqsignal.o pg_regress$(X)
173179
# things created by various check targets
174180
rm -f $(output_files) $(input_files)
175181
rm -rf testtablespace

‎src/test/regress/pg_regress.c

Lines changed: 102 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@
3030
#endif
3131

3232
#include "getopt_long.h"
33+
#include "libpq/pqcomm.h" /* needed for UNIXSOCK_PATH() */
34+
#include "libpq/pqsignal.h"
3335
#include "pg_config_paths.h"
3436

3537
/* for resultmap we need a list of pairs of strings */
@@ -96,6 +98,12 @@ static const char *progname;
9698
static char *logfilename;
9799
static FILE *logfile;
98100
static char *difffilename;
101+
static const char *sockdir;
102+
#ifdef HAVE_UNIX_SOCKETS
103+
static const char *temp_sockdir;
104+
static char sockself[MAXPGPATH];
105+
static char socklock[MAXPGPATH];
106+
#endif
99107

100108
static _resultmap *resultmap = NULL;
101109

@@ -292,6 +300,81 @@ stop_postmaster(void)
292300
}
293301
}
294302

303+
#ifdef HAVE_UNIX_SOCKETS
304+
/*
305+
* Remove the socket temporary directory. pg_regress never waits for a
306+
* postmaster exit, so it is indeterminate whether the postmaster has yet to
307+
* unlink the socket and lock file. Unlink them here so we can proceed to
308+
* remove the directory. Ignore errors; leaking a temporary directory is
309+
* unimportant. This can run from a signal handler. The code is not
310+
* acceptable in a Windows signal handler (see initdb.c:trapsig()), but
311+
* Windows is not a HAVE_UNIX_SOCKETS platform.
312+
*/
313+
static void
314+
remove_temp(void)
315+
{
316+
unlink(sockself);
317+
unlink(socklock);
318+
rmdir(temp_sockdir);
319+
}
320+
321+
/*
322+
* Signal handler that calls remove_temp() and reraises the signal.
323+
*/
324+
static void
325+
signal_remove_temp(int signum)
326+
{
327+
remove_temp();
328+
329+
pqsignal(signum, SIG_DFL);
330+
raise(signum);
331+
}
332+
333+
/*
334+
* Create a temporary directory suitable for the server's Unix-domain socket.
335+
* The directory will have mode 0700 or stricter, so no other OS user can open
336+
* our socket to exploit our use of trust authentication. Most systems
337+
* constrain the length of socket paths well below _POSIX_PATH_MAX, so we
338+
* place the directory under /tmp rather than relative to the possibly-deep
339+
* current working directory.
340+
*
341+
* Compared to using the compiled-in DEFAULT_PGSOCKET_DIR, this also permits
342+
* testing to work in builds that relocate it to a directory not writable to
343+
* the build/test user.
344+
*/
345+
static const char *
346+
make_temp_sockdir(void)
347+
{
348+
char *template = strdup("/tmp/pg_regress-XXXXXX");
349+
350+
temp_sockdir = mkdtemp(template);
351+
if (temp_sockdir == NULL)
352+
{
353+
fprintf(stderr, _("%s: could not create directory \"%s\": %s\n"),
354+
progname, template, strerror(errno));
355+
exit(2);
356+
}
357+
358+
/* Stage file names for remove_temp(). Unsafe in a signal handler. */
359+
UNIXSOCK_PATH(sockself, port, temp_sockdir);
360+
snprintf(socklock, sizeof(socklock), "%s.lock", sockself);
361+
362+
/* Remove the directory during clean exit. */
363+
atexit(remove_temp);
364+
365+
/*
366+
* Remove the directory before dying to the usual signals. Omit SIGQUIT,
367+
* preserving it as a quick, untidy exit.
368+
*/
369+
pqsignal(SIGHUP, signal_remove_temp);
370+
pqsignal(SIGINT, signal_remove_temp);
371+
pqsignal(SIGPIPE, signal_remove_temp);
372+
pqsignal(SIGTERM, signal_remove_temp);
373+
374+
return temp_sockdir;
375+
}
376+
#endif /* HAVE_UNIX_SOCKETS */
377+
295378
/*
296379
* Always exit through here, not through plain exit(), to ensure we make
297380
* an effort to shut down a temp postmaster
@@ -753,8 +836,7 @@ initialize_environment(void)
753836
* the wrong postmaster, or otherwise behave in nondefault ways. (Note
754837
* we also use psql's -X switch consistently, so that ~/.psqlrc files
755838
* won't mess things up.) Also, set PGPORT to the temp port, and set
756-
* or unset PGHOST depending on whether we are using TCP or Unix
757-
* sockets.
839+
* PGHOST depending on whether we are using TCP or Unix sockets.
758840
*/
759841
unsetenv("PGDATABASE");
760842
unsetenv("PGUSER");
@@ -763,10 +845,19 @@ initialize_environment(void)
763845
unsetenv("PGREQUIRESSL");
764846
unsetenv("PGCONNECT_TIMEOUT");
765847
unsetenv("PGDATA");
848+
#ifdef HAVE_UNIX_SOCKETS
766849
if (hostname != NULL)
767850
doputenv("PGHOST", hostname);
768851
else
769-
unsetenv("PGHOST");
852+
{
853+
sockdir = getenv("PG_REGRESS_SOCK_DIR");
854+
if (!sockdir)
855+
sockdir = make_temp_sockdir();
856+
doputenv("PGHOST", sockdir);
857+
}
858+
#else
859+
doputenv("PGHOST", hostname);
860+
#endif
770861
unsetenv("PGHOSTADDR");
771862
if (port != -1)
772863
{
@@ -2035,7 +2126,9 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc
20352126
/*
20362127
* To reduce chances of interference with parallel installations, use
20372128
* a port number starting in the private range (49152-65535)
2038-
* calculated from the version number.
2129+
* calculated from the version number. This aids !HAVE_UNIX_SOCKETS
2130+
* systems; elsewhere, the use of a private socket directory already
2131+
* prevents interference.
20392132
*/
20402133
port = 0xC000 | (PG_VERSION_NUM & 0x3FFF);
20412134

@@ -2185,10 +2278,11 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc
21852278
*/
21862279
header(_("starting postmaster"));
21872280
snprintf(buf, sizeof(buf),
2188-
SYSTEMQUOTE "\"%s/postgres\" -D \"%s/data\" -F%s -c \"listen_addresses=%s\" > \"%s/log/postmaster.log\" 2>&1" SYSTEMQUOTE,
2189-
bindir, temp_install,
2190-
debug ? " -d 5" : "",
2191-
hostname ? hostname : "",
2281+
SYSTEMQUOTE "\"%s/postgres\" -D \"%s/data\" -F%s "
2282+
"-c \"listen_addresses=%s\" -k \"%s\" "
2283+
"> \"%s/log/postmaster.log\" 2>&1" SYSTEMQUOTE,
2284+
bindir, temp_install, debug ? " -d 5" : "",
2285+
hostname ? hostname : "", sockdir ? sockdir : "",
21922286
outputdir);
21932287
postmaster_pid = spawn_process(buf);
21942288
if (postmaster_pid == INVALID_PID)

0 commit comments

Comments
 (0)
0