Skip to content

Commit 453a5d9

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 a919937 commit 453a5d9

File tree

5 files changed

+151
-29
lines changed

5 files changed

+151
-29
lines changed

‎contrib/pg_upgrade/test.sh

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,43 @@ set -e
1515
: ${PGPORT=50432}
1616
export PGPORT
1717

18+
# Establish how the server will listen for connections
1819
testhost=`uname -s`
1920

2021
case $testhost in
21-
MINGW*) LISTEN_ADDRESSES="localhost" ;;
22-
*) LISTEN_ADDRESSES="" ;;
22+
MINGW*)
23+
LISTEN_ADDRESSES="localhost"
24+
PGHOST=""; unset PGHOST
25+
;;
26+
*)
27+
LISTEN_ADDRESSES=""
28+
# Select a socket directory. The algorithm is from the "configure"
29+
# script; the outcome mimics pg_regress.c:make_temp_sockdir().
30+
PGHOST=$PG_REGRESS_SOCK_DIR
31+
if [ "x$PGHOST" = x ]; then
32+
{
33+
dir=`(umask 077 &&
34+
mktemp -d /tmp/pg_upgrade_check-XXXXXX) 2>/dev/null` &&
35+
[ -d "$dir" ]
36+
} ||
37+
{
38+
dir=/tmp/pg_upgrade_check-$$-$RANDOM
39+
(umask 077 && mkdir "$dir")
40+
} ||
41+
{
42+
echo "could not create socket temporary directory in \"/tmp\""
43+
exit 1
44+
}
45+
46+
PGHOST=$dir
47+
trap 'rm -rf "$PGHOST"' 0
48+
trap 'exit 3' 1 2 13 15
49+
fi
50+
export PGHOST
51+
;;
2352
esac
2453

25-
POSTMASTER_OPTS="-F -c listen_addresses=$LISTEN_ADDRESSES"
54+
POSTMASTER_OPTS="-F -c listen_addresses=$LISTEN_ADDRESSES -k \"$PGHOST\""
2655

2756
temp_root=$PWD/tmp_check
2857

@@ -79,7 +108,6 @@ PGSERVICE=""; unset PGSERVICE
79108
PGSSLMODE=""; unset PGSSLMODE
80109
PGREQUIRESSL=""; unset PGREQUIRESSL
81110
PGCONNECT_TIMEOUT=""; unset PGCONNECT_TIMEOUT
82-
PGHOST=""; unset PGHOST
83111
PGHOSTADDR=""; unset PGHOSTADDR
84112

85113
logdir=$PWD/log

‎doc/src/sgml/regress.sgml

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

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

‎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: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,13 +43,18 @@ EXTRADEFS = '-DHOST_TUPLE="$(host_tuple)"' \
4343

4444
all: pg_regress$(X)
4545

46-
pg_regress$(X): pg_regress.o pg_regress_main.o | submake-libpgport
46+
pg_regress$(X): pg_regress.o pg_regress_main.o pqsignal.o | submake-libpgport
4747
$(CC) $(CFLAGS) $^ $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@
4848

4949
# dependencies ensure that path changes propagate
5050
pg_regress.o: pg_regress.c $(top_builddir)/src/port/pg_config_paths.h
5151
pg_regress.o: override CPPFLAGS += -I$(top_builddir)/src/port $(EXTRADEFS)
5252

53+
# Pull in pqsignal like initdb does.
54+
pqsignal.c: % : $(top_srcdir)/src/interfaces/libpq/%
55+
rm -f $@ && $(LN_S) $< .
56+
pqsignal.o: override CPPFLAGS += -I$(libpq_srcdir)
57+
5358
$(top_builddir)/src/port/pg_config_paths.h: $(top_builddir)/src/Makefile.global
5459
$(MAKE) -C $(top_builddir)/src/port pg_config_paths.h
5560

@@ -166,7 +171,7 @@ bigcheck: all tablespace-setup
166171
clean distclean maintainer-clean: clean-lib
167172
# things built by `all' target
168173
rm -f $(OBJS) refint$(DLSUFFIX) autoinc$(DLSUFFIX) dummy_seclabel$(DLSUFFIX)
169-
rm -f pg_regress_main.o pg_regress.o pg_regress$(X)
174+
rm -f pqsignal.c pg_regress_main.o pg_regress.o pqsignal.o pg_regress$(X)
170175
# things created by various check targets
171176
rm -f $(output_files) $(input_files)
172177
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 */
@@ -109,6 +111,12 @@ static const char *progname;
109111
static char *logfilename;
110112
static FILE *logfile;
111113
static char *difffilename;
114+
static const char *sockdir;
115+
#ifdef HAVE_UNIX_SOCKETS
116+
static const char *temp_sockdir;
117+
static char sockself[MAXPGPATH];
118+
static char socklock[MAXPGPATH];
119+
#endif
112120

113121
static _resultmap *resultmap = NULL;
114122

@@ -307,6 +315,81 @@ stop_postmaster(void)
307315
}
308316
}
309317

318+
#ifdef HAVE_UNIX_SOCKETS
319+
/*
320+
* Remove the socket temporary directory. pg_regress never waits for a
321+
* postmaster exit, so it is indeterminate whether the postmaster has yet to
322+
* unlink the socket and lock file. Unlink them here so we can proceed to
323+
* remove the directory. Ignore errors; leaking a temporary directory is
324+
* unimportant. This can run from a signal handler. The code is not
325+
* acceptable in a Windows signal handler (see initdb.c:trapsig()), but
326+
* Windows is not a HAVE_UNIX_SOCKETS platform.
327+
*/
328+
static void
329+
remove_temp(void)
330+
{
331+
unlink(sockself);
332+
unlink(socklock);
333+
rmdir(temp_sockdir);
334+
}
335+
336+
/*
337+
* Signal handler that calls remove_temp() and reraises the signal.
338+
*/
339+
static void
340+
signal_remove_temp(int signum)
341+
{
342+
remove_temp();
343+
344+
pqsignal(signum, SIG_DFL);
345+
raise(signum);
346+
}
347+
348+
/*
349+
* Create a temporary directory suitable for the server's Unix-domain socket.
350+
* The directory will have mode 0700 or stricter, so no other OS user can open
351+
* our socket to exploit our use of trust authentication. Most systems
352+
* constrain the length of socket paths well below _POSIX_PATH_MAX, so we
353+
* place the directory under /tmp rather than relative to the possibly-deep
354+
* current working directory.
355+
*
356+
* Compared to using the compiled-in DEFAULT_PGSOCKET_DIR, this also permits
357+
* testing to work in builds that relocate it to a directory not writable to
358+
* the build/test user.
359+
*/
360+
static const char *
361+
make_temp_sockdir(void)
362+
{
363+
char *template = strdup("/tmp/pg_regress-XXXXXX");
364+
365+
temp_sockdir = mkdtemp(template);
366+
if (temp_sockdir == NULL)
367+
{
368+
fprintf(stderr, _("%s: could not create directory \"%s\": %s\n"),
369+
progname, template, strerror(errno));
370+
exit(2);
371+
}
372+
373+
/* Stage file names for remove_temp(). Unsafe in a signal handler. */
374+
UNIXSOCK_PATH(sockself, port, temp_sockdir);
375+
snprintf(socklock, sizeof(socklock), "%s.lock", sockself);
376+
377+
/* Remove the directory during clean exit. */
378+
atexit(remove_temp);
379+
380+
/*
381+
* Remove the directory before dying to the usual signals. Omit SIGQUIT,
382+
* preserving it as a quick, untidy exit.
383+
*/
384+
pqsignal(SIGHUP, signal_remove_temp);
385+
pqsignal(SIGINT, signal_remove_temp);
386+
pqsignal(SIGPIPE, signal_remove_temp);
387+
pqsignal(SIGTERM, signal_remove_temp);
388+
389+
return temp_sockdir;
390+
}
391+
#endif /* HAVE_UNIX_SOCKETS */
392+
310393
/*
311394
* Check whether string matches pattern
312395
*
@@ -759,8 +842,7 @@ initialize_environment(void)
759842
* the wrong postmaster, or otherwise behave in nondefault ways. (Note
760843
* we also use psql's -X switch consistently, so that ~/.psqlrc files
761844
* won't mess things up.) Also, set PGPORT to the temp port, and set
762-
* or unset PGHOST depending on whether we are using TCP or Unix
763-
* sockets.
845+
* PGHOST depending on whether we are using TCP or Unix sockets.
764846
*/
765847
unsetenv("PGDATABASE");
766848
unsetenv("PGUSER");
@@ -769,10 +851,19 @@ initialize_environment(void)
769851
unsetenv("PGREQUIRESSL");
770852
unsetenv("PGCONNECT_TIMEOUT");
771853
unsetenv("PGDATA");
854+
#ifdef HAVE_UNIX_SOCKETS
772855
if (hostname != NULL)
773856
doputenv("PGHOST", hostname);
774857
else
775-
unsetenv("PGHOST");
858+
{
859+
sockdir = getenv("PG_REGRESS_SOCK_DIR");
860+
if (!sockdir)
861+
sockdir = make_temp_sockdir();
862+
doputenv("PGHOST", sockdir);
863+
}
864+
#else
865+
doputenv("PGHOST", hostname);
866+
#endif
776867
unsetenv("PGHOSTADDR");
777868
if (port != -1)
778869
{
@@ -2077,7 +2168,9 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc
20772168
/*
20782169
* To reduce chances of interference with parallel installations, use
20792170
* a port number starting in the private range (49152-65535)
2080-
* calculated from the version number.
2171+
* calculated from the version number. This aids !HAVE_UNIX_SOCKETS
2172+
* systems; elsewhere, the use of a private socket directory already
2173+
* prevents interference.
20812174
*/
20822175
port = 0xC000 | (PG_VERSION_NUM & 0x3FFF);
20832176

@@ -2246,10 +2339,11 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc
22462339
*/
22472340
header(_("starting postmaster"));
22482341
snprintf(buf, sizeof(buf),
2249-
SYSTEMQUOTE "\"%s/postgres\" -D \"%s/data\" -F%s -c \"listen_addresses=%s\" > \"%s/log/postmaster.log\" 2>&1" SYSTEMQUOTE,
2250-
bindir, temp_install,
2251-
debug ? " -d 5" : "",
2252-
hostname ? hostname : "",
2342+
SYSTEMQUOTE "\"%s/postgres\" -D \"%s/data\" -F%s "
2343+
"-c \"listen_addresses=%s\" -k \"%s\" "
2344+
"> \"%s/log/postmaster.log\" 2>&1" SYSTEMQUOTE,
2345+
bindir, temp_install, debug ? " -d 5" : "",
2346+
hostname ? hostname : "", sockdir ? sockdir : "",
22532347
outputdir);
22542348
postmaster_pid = spawn_process(buf);
22552349
if (postmaster_pid == INVALID_PID)

0 commit comments

Comments
 (0)