Re: [HACKERS] Securing make check (CVE-2014-0067)
On 25 December 2014 at 18:27, Noah Misch n...@leadboat.com wrote: On Thu, Dec 25, 2014 at 03:55:02PM +1300, David Rowley wrote: f6dc6dd seems to have broken vcregress check for me: FATAL: no pg_hba.conf entry for host ::1, user David, database postgres ... FATAL: no pg_hba.conf entry for host ::1, user David, database postgres Thanks. I bet this is the reason buildfarm members hamerkop, jacana and bowerbird have not been reporting in. @@ -1085,6 +1085,8 @@ config_sspi_auth(const char *pgdata) 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(fputs(host all all ::1/128 sspi include_realm=1 map=regress\n, + hba) = 0); This needs to be conditional on whether the platform supports IPv6, like we do in setup_config(). The attached patch works on these configurations: 64-bit Windows Server 2003, 32-bit VS2010 64-bit Windows Server 2003, MinGW (always 32-bit) 64-bit Windows Server 2008, 64-bit VS2012 64-bit Windows Server 2008, 64-bit MinGW-w64 If the patch looks reasonable, I will commit it. I'm just looking at initdb.c I see that there's this: #ifdef HAVE_IPV6 /* * Probe to see if there is really any platform support for IPv6, and * comment out the relevant pg_hba line if not. This avoids runtime * warnings if getaddrinfo doesn't actually cope with IPv6. Particularly * useful on Windows, where executables built on a machine with IPv6 may * have to run on a machine without. */ The comment does seem to indicate that getaddrinfo might give a warning on an IPv4 only machine when given an IPv6 address to resolve. I think likely we want that here too. Though I don't have an IPv4 only machine to test on. I'll test the patch with IPv4 disabled and see if I get a warning... Ok, it seems to still write the Ipv6 entry into the pg_hba.conf with IPv6 disabled, so perhaps disabling IPv6 is not sufficient, maybe it needs to be tested on a machine that does not support IPv6 at all. Regards David Rowley
Re: [HACKERS] Securing make check (CVE-2014-0067)
On Thu, Dec 25, 2014 at 11:35:31PM +1300, David Rowley wrote: On 25 December 2014 at 18:27, Noah Misch n...@leadboat.com wrote: This needs to be conditional on whether the platform supports IPv6, like we do in setup_config(). The attached patch works on these configurations: 64-bit Windows Server 2003, 32-bit VS2010 64-bit Windows Server 2003, MinGW (always 32-bit) 64-bit Windows Server 2008, 64-bit VS2012 64-bit Windows Server 2008, 64-bit MinGW-w64 If the patch looks reasonable, I will commit it. I'm just looking at initdb.c I see that there's this: #ifdef HAVE_IPV6 /* * Probe to see if there is really any platform support for IPv6, and * comment out the relevant pg_hba line if not. This avoids runtime * warnings if getaddrinfo doesn't actually cope with IPv6. Particularly * useful on Windows, where executables built on a machine with IPv6 may * have to run on a machine without. */ The comment does seem to indicate that getaddrinfo might give a warning on an IPv4 only machine when given an IPv6 address to resolve. I think likely we want that here too. Though I don't have an IPv4 only machine to test on. A default installation of Windows Server 2003 is IPv4-only. Putting a ::1/128 line in pg_hba.conf makes the postmaster fail to start there, reporting error specifying both host name and CIDR mask is invalid. I'll test the patch with IPv4 disabled and see if I get a warning... Ok, it seems to still write the Ipv6 entry into the pg_hba.conf with IPv6 disabled, so perhaps disabling IPv6 is not sufficient, maybe it needs to be tested on a machine that does not support IPv6 at all. It is fine to emit the IPv6 entry on any system where it does not impede postmaster start, even systems that won't actually use IPv6 to connect. I went ahead and committed this. Andrew, would you unstick buildfarm members jacana and bowerbird on all branches? SRA, would you do the same for hamerkop? Thanks. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
On 30 November 2014 at 15:02, Noah Misch n...@leadboat.com wrote: On Sun, Sep 21, 2014 at 02:31:15AM -0400, Noah Misch wrote: It then dawned on me that every Windows build of PostgreSQL already has a way to limit connections to a particular OS user. SSPI authentication is essentially the Windows equivalent of peer authentication. A brief trial thereof looked promising. Regression runs will need a pg_ident.conf listing each role used in the regression tests. That's not ideal, but the buildfarm will quickly reveal any omissions. Unless someone sees a problem here, I will look at fleshing this out into a complete patch. I bet it will even turn out to be back-patchable. That worked out nicely. pg_regress --temp-install rewrites pg_ident.conf and pg_hba.conf such that the current OS user may authenticate as the bootstrap superuser and as any user named in --create-role. Suites not using --temp-install (pg_upgrade, TAP) call pg_regress --config-auth=DATADIR to pick up those same configuration changes. My hope is that out-of-tree test harnesses wanting this hardening can do likewise. On non-Windows systems, pg_regress --config-auth does nothing. f6dc6dd seems to have broken vcregress check for me: D:\Postgres\a\src\tools\msvcvcregress check == removing existing temp installation== == creating temporary installation== == initializing database system == == starting postmaster== pg_regress: postmaster did not respond within 60 seconds Examine D:/Postgres/a/src/test/regress/log/postmaster.log for the reason The postmaster.log reads: LOG: database system was shut down at 2014-12-25 15:26:33 NZDT LOG: database system is ready to accept connections LOG: autovacuum launcher started FATAL: no pg_hba.conf entry for host ::1, user David, database postgres ... FATAL: no pg_hba.conf entry for host ::1, user David, database postgres Having a look at the pg_hba.conf that's been generated by pgregress, it looks like it only adds a line for IPv4 addresses. I'll admit that I don't have a great understanding of what the SSPI stuff is about, but at least the attached patch seems to fix the problem for me. Regards David Rowley diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c index cb092f9..e2adaca 100644 --- a/src/test/regress/pg_regress.c +++ b/src/test/regress/pg_regress.c @@ -1085,6 +1085,8 @@ config_sspi_auth(const char *pgdata) 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(fputs(host all all ::1/128 sspi include_realm=1 map=regress\n, +hba) = 0); CW(fclose(hba) == 0); snprintf(fname, sizeof(fname), %s/pg_ident.conf, pgdata); -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
On Thu, Dec 25, 2014 at 11:55 AM, David Rowley dgrowle...@gmail.com wrote: f6dc6dd seems to have broken vcregress check for me: Having a look at the pg_hba.conf that's been generated by pgregress, it looks like it only adds a line for IPv4 addresses. Indeed. I can see this problem as well on my win7 box, and your patch fixes it. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
On Thu, Dec 25, 2014 at 03:55:02PM +1300, David Rowley wrote:
f6dc6dd seems to have broken vcregress check for me:
FATAL: no pg_hba.conf entry for host ::1, user David, database
postgres
...
FATAL: no pg_hba.conf entry for host ::1, user David, database
postgres
Thanks. I bet this is the reason buildfarm members hamerkop, jacana and
bowerbird have not been reporting in.
@@ -1085,6 +1085,8 @@ config_sspi_auth(const char *pgdata)
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(fputs(host all all ::1/128 sspi include_realm=1 map=regress\n,
+ hba) = 0);
This needs to be conditional on whether the platform supports IPv6, like we do
in setup_config(). The attached patch works on these configurations:
64-bit Windows Server 2003, 32-bit VS2010
64-bit Windows Server 2003, MinGW (always 32-bit)
64-bit Windows Server 2008, 64-bit VS2012
64-bit Windows Server 2008, 64-bit MinGW-w64
If the patch looks reasonable, I will commit it.
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index cb092f9..e8c644b 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -1035,6 +1035,7 @@ config_sspi_auth(const char *pgdata)
*domainname;
const char *username;
char *errstr;
+ boolhave_ipv6;
charfname[MAXPGPATH];
int res;
FILE *hba,
@@ -1054,6 +1055,28 @@ config_sspi_auth(const char *pgdata)
exit(2);
}
+ /*
+* Like initdb.c:setup_config(), determine whether the platform
recognizes
+* ::1 (IPv6 loopback) as a numeric host address string.
+*/
+ {
+ struct addrinfo *gai_result;
+ struct addrinfo hints;
+ WSADATA wsaData;
+
+ hints.ai_flags = AI_NUMERICHOST;
+ hints.ai_family = AF_UNSPEC;
+ hints.ai_socktype = 0;
+ hints.ai_protocol = 0;
+ hints.ai_addrlen = 0;
+ hints.ai_canonname = NULL;
+ hints.ai_addr = NULL;
+ hints.ai_next = NULL;
+
+ have_ipv6 = (WSAStartup(MAKEWORD(2, 2), wsaData) == 0
+getaddrinfo(::1, NULL, hints,
gai_result) == 0);
+ }
+
/* Check a Write outcome and report any error. */
#define CW(cond) \
do { \
@@ -1085,6 +1108,9 @@ config_sspi_auth(const char *pgdata)
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);
+ if (have_ipv6)
+ CW(fputs(host all all ::1/128 sspi include_realm=1
map=regress\n,
+hba) = 0);
CW(fclose(hba) == 0);
snprintf(fname, sizeof(fname), %s/pg_ident.conf, pgdata);
diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm
index 004942c..4506739 100644
--- a/src/tools/msvc/Mkvcbuild.pm
+++ b/src/tools/msvc/Mkvcbuild.pm
@@ -345,6 +345,7 @@ sub mkvcbuild
$pgregress_ecpg-AddIncludeDir('src\test\regress');
$pgregress_ecpg-AddDefine('HOST_TUPLE=i686-pc-win32vc');
$pgregress_ecpg-AddDefine('FRONTEND');
+ $pgregress_ecpg-AddLibrary('ws2_32.lib');
$pgregress_ecpg-AddDirResourceFile('src\interfaces\ecpg\test');
$pgregress_ecpg-AddReference($libpgcommon, $libpgport);
@@ -372,6 +373,7 @@ sub mkvcbuild
$pgregress_isolation-AddIncludeDir('src\test\regress');
$pgregress_isolation-AddDefine('HOST_TUPLE=i686-pc-win32vc');
$pgregress_isolation-AddDefine('FRONTEND');
+ $pgregress_isolation-AddLibrary('ws2_32.lib');
$pgregress_isolation-AddDirResourceFile('src\test\isolation');
$pgregress_isolation-AddReference($libpgcommon, $libpgport);
@@ -605,6 +607,8 @@ sub mkvcbuild
$pgregress-AddFile('src\test\regress\pg_regress_main.c');
$pgregress-AddIncludeDir('src\port');
$pgregress-AddDefine('HOST_TUPLE=i686-pc-win32vc');
+ $pgregress-AddDefine('FRONTEND');
+ $pgregress-AddLibrary('ws2_32.lib');
$pgregress-AddDirResourceFile('src\test\regress');
$pgregress-AddReference($libpgcommon, $libpgport);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
On Sun, Sep 21, 2014 at 02:31:15AM -0400, Noah Misch wrote:
It then dawned on me that every Windows build of PostgreSQL already has a way
to limit connections to a particular OS user. SSPI authentication is
essentially the Windows equivalent of peer authentication. A brief trial
thereof looked promising. Regression runs will need a pg_ident.conf listing
each role used in the regression tests. That's not ideal, but the buildfarm
will quickly reveal any omissions. Unless someone sees a problem here, I will
look at fleshing this out into a complete patch. I bet it will even turn out
to be back-patchable.
That worked out nicely. pg_regress --temp-install rewrites pg_ident.conf
and pg_hba.conf such that the current OS user may authenticate as the
bootstrap superuser and as any user named in --create-role. Suites not using
--temp-install (pg_upgrade, TAP) call pg_regress --config-auth=DATADIR to
pick up those same configuration changes. My hope is that out-of-tree test
harnesses wanting this hardening can do likewise. On non-Windows systems,
pg_regress --config-auth does nothing.
The TAP suite did not and does not succeed on Windows. I have good confidence
in my changes to make it use SSPI, but I tested them fully on GNU/Linux only.
Adding the explicit PGHOST=localhost to the pg_upgrade test suite is necessary
to avoid the host name must be specified error under SSPI authentication. I
tentatively view that as a bug in libpq, but it's orthogonal to this patch.
pg_regress.c already sets PGHOST explicitly.
Since I was rewriting various test suite initdb calls anyway, I made a few
use -N that weren't using it previously.
Thanks,
nm
diff --git a/contrib/pg_upgrade/test.sh b/contrib/pg_upgrade/test.sh
index 7bbd2c7..3bda790 100644
--- a/contrib/pg_upgrade/test.sh
+++ b/contrib/pg_upgrade/test.sh
@@ -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
diff --git a/doc/src/sgml/regress.sgml b/doc/src/sgml/regress.sgml
index 71196a1..504d8da 100644
--- a/doc/src/sgml/regress.sgml
+++ b/doc/src/sgml/regress.sgml
@@ -56,19 +56,6 @@ make check
quotefailure/ 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 literalmake 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
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index 63ff50b..2743c8f 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -320,7 +320,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 21
-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
Re: [HACKERS] Securing make check (CVE-2014-0067)
On Sun, Mar 02, 2014 at 11:36:41PM +0100, Magnus Hagander wrote: On Sun, Mar 2, 2014 at 7:27 PM, Tom Lane t...@sss.pgh.pa.us wrote: Noah Misch n...@leadboat.com writes: One option that would simplify things is to fix only non-Windows in the back branches, via socket protection, and fix Windows in HEAD only. We could even do so by extending HAVE_UNIX_SOCKETS support to Windows through named pipes. +1 for that solution, if it's not an unreasonable amount of work to add named-pipe sockets in Windows. That would offer a feature to Windows users that they didn't have before, ie the ability to restrict connections based on filesystem permissions; so it seems useful quite aside from any make check considerations. I think it might be a bigger piece of work than we'd like - and IIRC that's one of the reasons we didn't do it from the start. Named pipes on windows do act as files on Windows, but they do *not* act as sockets. As in, they return HANDLEs, not SOCKETs, and you can't recv() and send() on them. I have experimented with Windows named pipes. PQsocket() creates thorny problems on the client side. That function is central to asynchronous use of libpq, in which you call select(), poll() or similar on the returned socket. To expand that to cover Windows named pipes, we might provide two new libpq functions. The first would return an opaque connection handle. The second would resemble select() or poll(), accept the opaque handles, and use relevant OS primitives internally. The need to make such a prominent user-visible libpq change dims my affection for this strategy. (Challenges on the server side were simple matters of programming thus far.) This is similar to the problem PQgetssl() poses for new SSL implementations. It then dawned on me that every Windows build of PostgreSQL already has a way to limit connections to a particular OS user. SSPI authentication is essentially the Windows equivalent of peer authentication. A brief trial thereof looked promising. Regression runs will need a pg_ident.conf listing each role used in the regression tests. That's not ideal, but the buildfarm will quickly reveal any omissions. Unless someone sees a problem here, I will look at fleshing this out into a complete patch. I bet it will even turn out to be back-patchable. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
Re: Noah Misch 2014-07-12 20140712170151.ga1985...@tornado.leadboat.com Thanks. Preliminary questions: +#ifdef HAVE_UNIX_SOCKETS +/* make_temp_sockdir() is invoked at most twice from pg_upgrade.c via get_sock_dir() */ +#define MAX_TEMPDIRS 2 +static int n_tempdirs = 0; /* actual number of directories created */ +static const char *temp_sockdir[MAX_TEMPDIRS]; +#endif get_sock_dir() currently returns the same directory, the CWD, for both calls; can't it continue to do so? We already have good reason not to start two postmasters simultaneously during pg_upgrade. +/* + * Remove the socket temporary directories. pg_ctl waits for postmaster + * shutdown, so we expect the directory to be empty, unless we are interrupted + * by a signal, in which case the postmaster will clean up the sockets, but + * there's a race condition with us removing the directory. What's the reason for addressing that race condition in pg_regress and not addressing it in pg_upgrade? I didn't want to have too many arrays for additionally storing the socket and lockfile names, and unlike pg_regress, pg_upgrade usually doesn't need to delete the files by itself, so it seemed like a good choice to rely on the postmaster removing them. Now, if get_sock_dir() should only return a single directory instead of many (well, two), that makes the original code from pg_regress more attractive to use. (Possibly it will even be a candidate for moving to pgcommon.a, though the static/global variables might defeat that.) I'll send an updated patch soonish. Christoph -- c...@df7cb.de | http://www.df7cb.de/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
On Fri, Jul 11, 2014 at 12:40:09PM +0300, Christoph Berg wrote: I believe pg_upgrade itself still needs a fix. While it's not a security problem to put the socket in $CWD while upgrading (it is using -c unix_socket_permissions=0700), this behavior is pretty unexpected, and does fail if your $CWD is 107 bytes. Here's the patch. Proposed commit message: Create pg_upgrade sockets in temp directories pg_upgrade used to use the current directory for UNIX sockets to access the old/new cluster. This fails when the current path is 107 bytes. Fix by reusing the tempdir code from pg_regress introduced in be76a6d39e2832d4b88c0e1cc381aa44a7f86881. For cleanup, we need to remember up to two directories. Thanks. Preliminary questions: +#ifdef HAVE_UNIX_SOCKETS +/* make_temp_sockdir() is invoked at most twice from pg_upgrade.c via get_sock_dir() */ +#define MAX_TEMPDIRS 2 +static int n_tempdirs = 0; /* actual number of directories created */ +static const char *temp_sockdir[MAX_TEMPDIRS]; +#endif get_sock_dir() currently returns the same directory, the CWD, for both calls; can't it continue to do so? We already have good reason not to start two postmasters simultaneously during pg_upgrade. +/* + * Remove the socket temporary directories. pg_ctl waits for postmaster + * shutdown, so we expect the directory to be empty, unless we are interrupted + * by a signal, in which case the postmaster will clean up the sockets, but + * there's a race condition with us removing the directory. What's the reason for addressing that race condition in pg_regress and not addressing it in pg_upgrade? -- Noah Misch EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
Re: Bruce Momjian 2014-07-08 20140708202114.gd9...@momjian.us I believe pg_upgrade itself still needs a fix. While it's not a security problem to put the socket in $CWD while upgrading (it is using -c unix_socket_permissions=0700), this behavior is pretty unexpected, and does fail if your $CWD is 107 bytes. In f545d233ebce6971b6f9847680e48b679e707d22 Peter fixed the pg_ctl perl tests to avoid that problem, so imho it would make even more sense to fix pg_upgrade which could also fail in production. +1. Does writing that patch interest you? I'll give it a try once I've finished this CF review. OK. Let me know if you need help. Here's the patch. Proposed commit message: Create pg_upgrade sockets in temp directories pg_upgrade used to use the current directory for UNIX sockets to access the old/new cluster. This fails when the current path is 107 bytes. Fix by reusing the tempdir code from pg_regress introduced in be76a6d39e2832d4b88c0e1cc381aa44a7f86881. For cleanup, we need to remember up to two directories. Christoph -- c...@df7cb.de | http://www.df7cb.de/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
Re: To Bruce Momjian 2014-07-11 20140711093923.ga3...@msg.df7cb.de
Re: Bruce Momjian 2014-07-08 20140708202114.gd9...@momjian.us
I believe pg_upgrade itself still needs a fix. While it's not a
security problem to put the socket in $CWD while upgrading (it is
using -c unix_socket_permissions=0700), this behavior is pretty
unexpected, and does fail if your $CWD is 107 bytes.
In f545d233ebce6971b6f9847680e48b679e707d22 Peter fixed the pg_ctl
perl tests to avoid that problem, so imho it would make even more
sense to fix pg_upgrade which could also fail in production.
+1. Does writing that patch interest you?
I'll give it a try once I've finished this CF review.
OK. Let me know if you need help.
Here's the patch. Proposed commit message:
Create pg_upgrade sockets in temp directories
pg_upgrade used to use the current directory for UNIX sockets to
access the old/new cluster. This fails when the current path is
107 bytes. Fix by reusing the tempdir code from pg_regress
introduced in be76a6d39e2832d4b88c0e1cc381aa44a7f86881. For cleanup,
we need to remember up to two directories.
Uh... now really.
Christoph
--
c...@df7cb.de | http://www.df7cb.de/
diff --git a/contrib/pg_upgrade/option.c b/contrib/pg_upgrade/option.c
index b81010a..121a3d4 100644
--- a/contrib/pg_upgrade/option.c
+++ b/contrib/pg_upgrade/option.c
@@ -14,6 +14,7 @@
#include pg_upgrade.h
+#include signal.h
#include time.h
#include sys/types.h
#ifdef WIN32
@@ -26,6 +27,13 @@ static void check_required_directory(char **dirpath, char **configpath,
char *envVarName, char *cmdLineOption, char *description);
#define FIX_DEFAULT_READ_ONLY -c default_transaction_read_only=false
+#ifdef HAVE_UNIX_SOCKETS
+/* make_temp_sockdir() is invoked at most twice from pg_upgrade.c via get_sock_dir() */
+#define MAX_TEMPDIRS 2
+static int n_tempdirs = 0; /* actual number of directories created */
+static const char *temp_sockdir[MAX_TEMPDIRS];
+#endif
+
UserOpts user_opts;
@@ -396,6 +404,86 @@ adjust_data_dir(ClusterInfo *cluster)
check_ok();
}
+#ifdef HAVE_UNIX_SOCKETS
+/*
+ * Remove the socket temporary directories. pg_ctl waits for postmaster
+ * shutdown, so we expect the directory to be empty, unless we are interrupted
+ * by a signal, in which case the postmaster will clean up the sockets, but
+ * there's a race condition with us removing the directory. Ignore errors;
+ * leaking a (usually empty) temporary directory is unimportant. This can run
+ * from a signal handler. The code is not acceptable in a Windows signal
+ * handler (see initdb.c:trapsig()), but Windows is not a HAVE_UNIX_SOCKETS
+ * platform.
+ */
+static void remove_temp(void)
+{
+ int i;
+
+ for (i = 0; i n_tempdirs; i++)
+ {
+ Assert(temp_sockdir[i]);
+ rmdir(temp_sockdir[i]);
+ }
+}
+
+/*
+ * Signal handler that calls remove_temp() and reraises the signal.
+ */
+static void
+signal_remove_temp(int signum)
+{
+ remove_temp();
+
+ pqsignal(signum, SIG_DFL);
+ raise(signum);
+}
+
+/*
+ * Create a temporary directory suitable for the server's Unix-domain socket.
+ * The directory will have mode 0700 or stricter, so no other OS user can open
+ * our socket to exploit it independently from the auth method used. Most
+ * systems constrain the length of socket paths well below _POSIX_PATH_MAX, so
+ * we place the directory under /tmp rather than relative to the possibly-deep
+ * current working directory.
+ *
+ * Using a temporary directory so no connections arrive other than what
+ * pg_upgrade initiate itself. Compared to using the compiled-in
+ * DEFAULT_PGSOCKET_DIR, this also permits pg_upgrade to work in builds that
+ * relocate it to a directory not writable to the cluster owner.
+ */
+static const char *
+make_temp_sockdir(void)
+{
+ char *template = strdup(/tmp/pg_upgrade-XX);
+
+ Assert(n_tempdirs MAX_TEMPDIRS);
+ temp_sockdir[n_tempdirs] = mkdtemp(template);
+ if (temp_sockdir[n_tempdirs] == NULL)
+ {
+ fprintf(stderr, _(%s: could not create directory \%s\: %s\n),
+os_info.progname, template, strerror(errno));
+ exit(2);
+ }
+
+ /*
+ * Remove the directories during clean exit. Register the handler only
+ * once, though.
+ */
+ if (n_tempdirs == 0)
+ atexit(remove_temp);
+
+ /*
+ * Remove the directory before dying to the usual signals. Omit SIGQUIT,
+ * preserving it as a quick, untidy exit.
+ */
+ pqsignal(SIGHUP, signal_remove_temp);
+ pqsignal(SIGINT, signal_remove_temp);
+ pqsignal(SIGPIPE, signal_remove_temp);
+ pqsignal(SIGTERM, signal_remove_temp);
+
+ return temp_sockdir[n_tempdirs++];
+}
+#endif /* HAVE_UNIX_SOCKETS */
/*
* get_sock_dir
@@ -418,10 +506,8 @@ get_sock_dir(ClusterInfo *cluster, bool live_check)
{
if (!live_check)
{
- /* Use the current directory for the socket */
- cluster-sockdir = pg_malloc(MAXPGPATH);
- if (!getcwd(cluster-sockdir, MAXPGPATH))
-pg_fatal(cannot find current directory\n);
+ /*
Re: [HACKERS] Securing make check (CVE-2014-0067)
Re: Noah Misch 2014-06-08 20140608135713.ga525...@tornado.leadboat.com Here's an update that places the socket in a temporary subdirectory of /tmp. The first attached patch adds NetBSD mkdtemp() to libpgport. The second, principal, patch uses mkdtemp() to implement this design in pg_regress. The corresponding change to contrib/pg_upgrade/test.sh is based on the configure script's arrangements for its temporary directory. Hi, I believe pg_upgrade itself still needs a fix. While it's not a security problem to put the socket in $CWD while upgrading (it is using -c unix_socket_permissions=0700), this behavior is pretty unexpected, and does fail if your $CWD is 107 bytes. In f545d233ebce6971b6f9847680e48b679e707d22 Peter fixed the pg_ctl perl tests to avoid that problem, so imho it would make even more sense to fix pg_upgrade which could also fail in production. This has been discussed here and elsewhere [1] before, but was rejected as not being in line what the other utilities do, but now pg_upgrade is the lone outlier. Noah's changes let Debian drop 4 out of 5 pg_regress-sockdir patches, having this fixed would also let us get rid of the last one [2]. [1] http://lists.debian.org/debian-wb-team/2013/05/msg00015.html [2] https://alioth.debian.org/scm/loggerhead/pkg-postgresql/postgresql-9.5/trunk/view/head:/debian/patches/64-pg_upgrade-sockdir Christoph -- c...@df7cb.de | http://www.df7cb.de/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
On Tue, Jul 08, 2014 at 07:02:04PM +0200, Christoph Berg wrote: Re: Noah Misch 2014-06-08 20140608135713.ga525...@tornado.leadboat.com Here's an update that places the socket in a temporary subdirectory of /tmp. The first attached patch adds NetBSD mkdtemp() to libpgport. The second, principal, patch uses mkdtemp() to implement this design in pg_regress. The corresponding change to contrib/pg_upgrade/test.sh is based on the configure script's arrangements for its temporary directory. Hi, I believe pg_upgrade itself still needs a fix. While it's not a security problem to put the socket in $CWD while upgrading (it is using -c unix_socket_permissions=0700), this behavior is pretty unexpected, and does fail if your $CWD is 107 bytes. In f545d233ebce6971b6f9847680e48b679e707d22 Peter fixed the pg_ctl perl tests to avoid that problem, so imho it would make even more sense to fix pg_upgrade which could also fail in production. +1. Does writing that patch interest you? -- Noah Misch EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
Re: Noah Misch 2014-07-08 20140708174125.ga1884...@tornado.leadboat.com On Tue, Jul 08, 2014 at 07:02:04PM +0200, Christoph Berg wrote: Re: Noah Misch 2014-06-08 20140608135713.ga525...@tornado.leadboat.com Here's an update that places the socket in a temporary subdirectory of /tmp. The first attached patch adds NetBSD mkdtemp() to libpgport. The second, principal, patch uses mkdtemp() to implement this design in pg_regress. The corresponding change to contrib/pg_upgrade/test.sh is based on the configure script's arrangements for its temporary directory. Hi, I believe pg_upgrade itself still needs a fix. While it's not a security problem to put the socket in $CWD while upgrading (it is using -c unix_socket_permissions=0700), this behavior is pretty unexpected, and does fail if your $CWD is 107 bytes. In f545d233ebce6971b6f9847680e48b679e707d22 Peter fixed the pg_ctl perl tests to avoid that problem, so imho it would make even more sense to fix pg_upgrade which could also fail in production. +1. Does writing that patch interest you? I'll give it a try once I've finished this CF review. Christoph -- c...@df7cb.de | http://www.df7cb.de/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
On Tue, Jul 8, 2014 at 08:21:48PM +0200, Christoph Berg wrote: Re: Noah Misch 2014-07-08 20140708174125.ga1884...@tornado.leadboat.com On Tue, Jul 08, 2014 at 07:02:04PM +0200, Christoph Berg wrote: Re: Noah Misch 2014-06-08 20140608135713.ga525...@tornado.leadboat.com Here's an update that places the socket in a temporary subdirectory of /tmp. The first attached patch adds NetBSD mkdtemp() to libpgport. The second, principal, patch uses mkdtemp() to implement this design in pg_regress. The corresponding change to contrib/pg_upgrade/test.sh is based on the configure script's arrangements for its temporary directory. Hi, I believe pg_upgrade itself still needs a fix. While it's not a security problem to put the socket in $CWD while upgrading (it is using -c unix_socket_permissions=0700), this behavior is pretty unexpected, and does fail if your $CWD is 107 bytes. In f545d233ebce6971b6f9847680e48b679e707d22 Peter fixed the pg_ctl perl tests to avoid that problem, so imho it would make even more sense to fix pg_upgrade which could also fail in production. +1. Does writing that patch interest you? I'll give it a try once I've finished this CF review. OK. Let me know if you need help. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
On Sun, Mar 23, 2014 at 07:04:20PM -0400, Noah Misch wrote: On Thu, Mar 06, 2014 at 11:52:22PM -0500, Noah Misch wrote: On Thu, Mar 06, 2014 at 12:44:34PM -0500, Tom Lane wrote: I'm inclined to suggest that we should put the socket under $CWD by default, but provide some way for the user to override that choice. If they want to put it in /tmp, it's on their head as to how secure that is. On most modern platforms it'd be fine. I am skeptical about the value of protecting systems with non-sticky /tmp, but long $CWD isn't of great importance, either. I'm fine with your suggestion. Though the $CWD or one of its parents could be world-writable, that would typically mean an attacker could just replace the test cases directly. Here's the patch. The temporary data directory makes for a convenient socket directory; initdb already gives it mode 0700, and we have existing arrangements to purge it when finished. One can override the socket directory by defining PG_REGRESS_SOCK_DIR in the environment. Socket path length limitations thwarted that patch: http://www.postgresql.org/message-id/flat/e1wtnv2-00047s...@gemulon.postgresql.org Here's an update that places the socket in a temporary subdirectory of /tmp. The first attached patch adds NetBSD mkdtemp() to libpgport. The second, principal, patch uses mkdtemp() to implement this design in pg_regress. The corresponding change to contrib/pg_upgrade/test.sh is based on the configure script's arrangements for its temporary directory. NetBSD's mkdtemp() has assertions, and I initially mapped its assertion macro to our Assert(). However, a bug in our MinGW build process causes build failures if an Assert() call appears in libpgport. I will post about that in a new thread. The affected assertions were uncompelling, so I dropped them. -- Noah Misch EnterpriseDB http://www.enterprisedb.com diff --git a/configure b/configure index ed1ff0a..f8232db 100755 --- a/configure +++ b/configure @@ -11650,6 +11650,19 @@ esac fi +ac_fn_c_check_func $LINENO mkdtemp ac_cv_func_mkdtemp +if test x$ac_cv_func_mkdtemp = xyes; then : + $as_echo #define HAVE_MKDTEMP 1 confdefs.h + +else + case $LIBOBJS in + * mkdtemp.$ac_objext * ) ;; + *) LIBOBJS=$LIBOBJS mkdtemp.$ac_objext + ;; +esac + +fi + ac_fn_c_check_func $LINENO random ac_cv_func_random if test x$ac_cv_func_random = xyes; then : $as_echo #define HAVE_RANDOM 1 confdefs.h diff --git a/configure.in b/configure.in index 80df1d7..c95e2cd 100644 --- a/configure.in +++ b/configure.in @@ -1357,7 +1357,7 @@ else AC_CHECK_FUNCS([fpclass fp_class fp_class_d class], [break]) fi -AC_REPLACE_FUNCS([crypt fls getopt getrusage inet_aton random rint srandom strerror strlcat strlcpy]) +AC_REPLACE_FUNCS([crypt fls getopt getrusage inet_aton mkdtemp random rint srandom strerror strlcat strlcpy]) case $host_os in diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in index 5ff9e41..4fb7288 100644 --- a/src/include/pg_config.h.in +++ b/src/include/pg_config.h.in @@ -330,6 +330,9 @@ /* Define to 1 if the system has the type `MINIDUMP_TYPE'. */ #undef HAVE_MINIDUMP_TYPE +/* Define to 1 if you have the `mkdtemp' function. */ +#undef HAVE_MKDTEMP + /* Define to 1 if you have the netinet/in.h header file. */ #undef HAVE_NETINET_IN_H diff --git a/src/include/pg_config.h.win32 b/src/include/pg_config.h.win32 index e6e3c8d..58777ca 100644 --- a/src/include/pg_config.h.win32 +++ b/src/include/pg_config.h.win32 @@ -249,6 +249,9 @@ /* Define to 1 if the system has the type `MINIDUMP_TYPE'. */ #define HAVE_MINIDUMP_TYPE 1 +/* Define to 1 if you have the `mkdtemp' function. */ +/* #undef HAVE_MKDTEMP */ + /* Define to 1 if you have the netinet/in.h header file. */ #define HAVE_NETINET_IN_H 1 diff --git a/src/include/port.h b/src/include/port.h index c9226f3..3d97481 100644 --- a/src/include/port.h +++ b/src/include/port.h @@ -462,6 +462,9 @@ extern int pg_check_dir(const char *dir); /* port/pgmkdirp.c */ extern int pg_mkdir_p(char *path, int omode); +/* port/mkdtemp.c */ +extern char *mkdtemp(char *path); + /* port/pqsignal.c */ typedef void (*pqsigfunc) (int signo); extern pqsigfunc pqsignal(int signo, pqsigfunc func); diff --git a/src/port/mkdtemp.c b/src/port/mkdtemp.c new file mode 100644 index 000..a5e991f --- /dev/null +++ b/src/port/mkdtemp.c @@ -0,0 +1,293 @@ +/*- + * + * mkdtemp.c + * create a mode-0700 temporary directory + * + * Portions Copyright (c) 1996-2014, PostgreSQL Global Development Group + * + * + * IDENTIFICATION + * src/port/mkdtemp.c + * + * This code was taken from NetBSD to provide an implementation for platforms + * that lack it. (Among compatibly-licensed implementations, the OpenBSD + * version better resists denial-of-service attacks. However, it has a + * cryptographic dependency.) The NetBSD copyright
Re: [HACKERS] Securing make check (CVE-2014-0067)
On Fri, Apr 04, 2014 at 02:36:05AM +, YAMAMOTO Takashi wrote: Thanks. To avoid socket path length limitations, I lean toward placing the socket temporary directory under /tmp rather than placing under the CWD: http://www.postgresql.org/message-id/flat/20121129223632.ga15...@tornado.leadboat.com openvswitch has some tricks to overcome the socket path length limitation using symlink. (or procfs where available) iirc these were introduced for debian builds which use deep CWD. That's another reasonable approach. Does it have a notable advantage over placing the socket in a subdirectory of /tmp? Offhand, the security and compatibility consequences look similar. an advantage is that the socket can be placed under CWD and thus automatically obeys its directory permissions etc. YAMAMOTO Takashi -- Noah Misch EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
y...@netbsd.org (YAMAMOTO Takashi) writes: On Fri, Apr 04, 2014 at 02:36:05AM +, YAMAMOTO Takashi wrote: openvswitch has some tricks to overcome the socket path length limitation using symlink. (or procfs where available) iirc these were introduced for debian builds which use deep CWD. That's another reasonable approach. Does it have a notable advantage over placing the socket in a subdirectory of /tmp? Offhand, the security and compatibility consequences look similar. an advantage is that the socket can be placed under CWD and thus automatically obeys its directory permissions etc. I'm confused. The proposed alternative is to make a symlink in /tmp or someplace like that, pointing to a socket that might be deeply buried? How is that any better from a security standpoint from putting the socket right in /tmp? If /tmp is not sticky then an attacker can replace the symlink, no? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
Thanks. To avoid socket path length limitations, I lean toward placing the socket temporary directory under /tmp rather than placing under the CWD: http://www.postgresql.org/message-id/flat/20121129223632.ga15...@tornado.leadboat.com openvswitch has some tricks to overcome the socket path length limitation using symlink. (or procfs where available) iirc these were introduced for debian builds which use deep CWD. http://git.openvswitch.org/cgi-bin/gitweb.cgi?p=openvswitch;a=blob;f=lib/socket- util.c;h=aa0c7196da9926de38b7388b8e28ead12e12913e;hb=HEAD look at shorten_name_via_symlink and shorten_name_via_proc. YAMAMOTO Takashi -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
On Fri, Apr 04, 2014 at 02:36:05AM +, YAMAMOTO Takashi wrote: Thanks. To avoid socket path length limitations, I lean toward placing the socket temporary directory under /tmp rather than placing under the CWD: http://www.postgresql.org/message-id/flat/20121129223632.ga15...@tornado.leadboat.com openvswitch has some tricks to overcome the socket path length limitation using symlink. (or procfs where available) iirc these were introduced for debian builds which use deep CWD. That's another reasonable approach. Does it have a notable advantage over placing the socket in a subdirectory of /tmp? Offhand, the security and compatibility consequences look similar. -- Noah Misch EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
On Sun, Mar 30, 2014 at 3:52 PM, Christoph Berg c...@df7cb.de wrote: Re: Noah Misch 2014-03-30 20140330014531.ge170...@tornado.leadboat.com On Sat, Mar 29, 2014 at 10:04:55AM +0100, Christoph Berg wrote: Fwiw, to relocate the pg_regress socket dir, there is already the possibility to run make check EXTRA_REGRESS_OPTS=--host=/tmp. (With the pending fix I sent yesterday to extend this to contrib/test_decoding.) That doesn't work for make check, because the postmaster ends up with listen_addresses=/tmp. Oh, right. There's this other patch which apparently works so well that I already forgot it's there: Enable pg_regress --host=/path/to/socket: https://alioth.debian.org/scm/loggerhead/pkg-postgresql/postgresql-9.4/trunk/view/head:/debian/patches/60-pg_regress_socketdir.patch Wasn't this patch submitted for inclusion in PostgreSQL at some point? Did we have some good reason for not accepting it? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
Robert Haas robertmh...@gmail.com writes: On Sun, Mar 30, 2014 at 3:52 PM, Christoph Berg c...@df7cb.de wrote: Oh, right. There's this other patch which apparently works so well that I already forgot it's there: Enable pg_regress --host=/path/to/socket: https://alioth.debian.org/scm/loggerhead/pkg-postgresql/postgresql-9.4/trunk/view/head:/debian/patches/60-pg_regress_socketdir.patch Wasn't this patch submitted for inclusion in PostgreSQL at some point? Did we have some good reason for not accepting it? Well, other than very bad coding style (casual disregard of the message localizability guidelines, and the dubious practice of two different format strings in one printf call) it doesn't seem like a bad idea on its face to allow pg_regress to set a socket path. But do we want pg_regress to *not* specify a listen_addresses string? I think we are currently setting that to empty intentionally on non-Windows. If it defaults to not-empty, which is what I think will happen with this patch, isn't that opening a different security hole? I think we need a somewhat larger understanding of what cases we're trying to support, in any case ... regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
Re: Tom Lane 2014-03-31 22183.1396293...@sss.pgh.pa.us Enable pg_regress --host=/path/to/socket: https://alioth.debian.org/scm/loggerhead/pkg-postgresql/postgresql-9.4/trunk/view/head:/debian/patches/60-pg_regress_socketdir.patch Wasn't this patch submitted for inclusion in PostgreSQL at some point? Did we have some good reason for not accepting it? Well, other than very bad coding style (casual disregard of the message localizability guidelines, and the dubious practice of two different format strings in one printf call) it doesn't seem like a bad idea on I had posted it here before, but I've got around to formally put it into a CF, so sorry for not cleaning up. The double-formatstr thing was done to avoid the need for twice as much almost-identical formatstrs. There's probably smarter ways to do that. its face to allow pg_regress to set a socket path. But do we want pg_regress to *not* specify a listen_addresses string? I think we are currently setting that to empty intentionally on non-Windows. The patch tries to reuse the existing switches; --host=/tmp is just the equivalent of the host=/tmp connection parameter. Of course it could as well introduce a new parameter --socket-dir=/tmp. If it defaults to not-empty, which is what I think will happen with this patch, isn't that opening a different security hole? I think we need a somewhat larger understanding of what cases we're trying to support, in any case ... The patch solves a usability problem, security wasn't a concern at the time of writing. I'll rethink that bit and come up with a better solution. Christoph -- c...@df7cb.de | http://www.df7cb.de/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
On Mon, Mar 31, 2014 at 3:19 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Sun, Mar 30, 2014 at 3:52 PM, Christoph Berg c...@df7cb.de wrote: Oh, right. There's this other patch which apparently works so well that I already forgot it's there: Enable pg_regress --host=/path/to/socket: https://alioth.debian.org/scm/loggerhead/pkg-postgresql/postgresql-9.4/trunk/view/head:/debian/patches/60-pg_regress_socketdir.patch Wasn't this patch submitted for inclusion in PostgreSQL at some point? Did we have some good reason for not accepting it? Well, other than very bad coding style (casual disregard of the message localizability guidelines, and the dubious practice of two different format strings in one printf call) it doesn't seem like a bad idea on its face to allow pg_regress to set a socket path. But do we want pg_regress to *not* specify a listen_addresses string? I think we are currently setting that to empty intentionally on non-Windows. If it defaults to not-empty, which is what I think will happen with this patch, isn't that opening a different security hole? Good points... I think we need a somewhat larger understanding of what cases we're trying to support, in any case ... My impression is that some people just want the postmaster that gets started for the pg_regress runs to listen on a socket rather than a port. Obviously that won't work on Windows, but it seems like a pretty reasonable thing to want to do everywhere else, especially in light of the security issues around make check we've been discussing. There's not going to be any convenient, cross-platform, unprivileged way of preventing other applications on the same system from connecting to a local TCP socket, whereas with a UNIX socket you can nail that door shut pretty tight using filesystem permissions. That isn't to say that you can't secure the TCP socket, but it's gotta be done through the user auth methods and is only as good as those methods are. I think an adequate solution can be achieved that way, but with the UNIX socket method the attack surface area is a whole lot more restricted, which seems appealing from where I sit. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
Re: Noah Misch 2014-03-30 20140330014531.ge170...@tornado.leadboat.com On Sat, Mar 29, 2014 at 10:04:55AM +0100, Christoph Berg wrote: Fwiw, to relocate the pg_regress socket dir, there is already the possibility to run make check EXTRA_REGRESS_OPTS=--host=/tmp. (With the pending fix I sent yesterday to extend this to contrib/test_decoding.) That doesn't work for make check, because the postmaster ends up with listen_addresses=/tmp. Oh, right. There's this other patch which apparently works so well that I already forgot it's there: Enable pg_regress --host=/path/to/socket: https://alioth.debian.org/scm/loggerhead/pkg-postgresql/postgresql-9.4/trunk/view/head:/debian/patches/60-pg_regress_socketdir.patch This, along with 61-extra_regress_opts and 64-pg_upgrade-sockdir (at the same location, both also recently posted here) should be safe for general use, i.e. inclusion in git. (I didn't check how much this overlaps with what Noah tried, I'm just mentioning the patches here because they work for Debian.) There's two other patches: 62-pg_upgrade-test-in-tmp hardcodes /tmp for the pg_upgrade test which should obviously be done smarter, and 63-pg_upgrade-test-bindir which forwards PSQLDIR through contrib/pg_upgrade/test.sh. The latter is probably also safe for general use, but I'd be more confident if someone rechecked that. We've been putting a small patch into pg_upgrade in Debian to work around too long socket paths generated by pg_upgrade during running the testsuite (and effectively on end user systems, but I don't think anyone is using such long paths there). A similar code bit could be put into pg_regress itself. Thanks for reminding me about Debian's troubles here. Once the dust settles on pg_regress, it will probably make sense to do likewise to pg_upgrade. Nod, it'd be nice if we could get rid of some patches in Debian. Christoph -- c...@df7cb.de | http://www.df7cb.de/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
Re: Noah Misch 2014-03-24 20140323230420.ga4139...@tornado.leadboat.com
On Thu, Mar 06, 2014 at 11:52:22PM -0500, Noah Misch wrote:
On Thu, Mar 06, 2014 at 12:44:34PM -0500, Tom Lane wrote:
I'm inclined to suggest that we should put the socket under $CWD by
default, but provide some way for the user to override that choice.
If they want to put it in /tmp, it's on their head as to how secure
that is. On most modern platforms it'd be fine.
Fwiw, to relocate the pg_regress socket dir, there is already the
possibility to run make check EXTRA_REGRESS_OPTS=--host=/tmp. (With
the pending fix I sent yesterday to extend this to contrib/test_decoding.)
I am skeptical about the value of protecting systems with non-sticky /tmp,
but
long $CWD isn't of great importance, either. I'm fine with your suggestion.
Though the $CWD or one of its parents could be world-writable, that would
typically mean an attacker could just replace the test cases directly.
Here's the patch. The temporary data directory makes for a convenient socket
directory; initdb already gives it mode 0700, and we have existing
arrangements to purge it when finished. One can override the socket directory
by defining PG_REGRESS_SOCK_DIR in the environment.
We've been putting a small patch into pg_upgrade in Debian to work
around too long socket paths generated by pg_upgrade during running
the testsuite (and effectively on end user systems, but I don't think
anyone is using such long paths there).
A similar code bit could be put into pg_regress itself.
Fix for: connection to database failed: Unix-domain socket path
/build/buildd-postgresql-9.3_9.3~beta1-1-i386-mHjRUH/postgresql-9.3-9.3~beta1/build/contrib/pg_upgrade/.s.PGSQL.50432
is too long (maximum 107 bytes)
See also: http://lists.debian.org/debian-wb-team/2013/05/msg00015.html
--- a/contrib/pg_upgrade/option.c
+++ b/contrib/pg_upgrade/option.c
@@ -422,6 +422,11 @@ get_sock_dir(ClusterInfo *cluster, bool
cluster-sockdir = pg_malloc(MAXPGPATH);
if (!getcwd(cluster-sockdir, MAXPGPATH))
pg_fatal(cannot find current directory\n);
+#ifndef UNIX_PATH_MAX
+#define UNIX_PATH_MAX 108
+#endif
+ if (strlen(cluster-sockdir) UNIX_PATH_MAX -
sizeof(.s.PGSQL.50432))
+ strcpy(cluster-sockdir, /tmp); /* fall back
to tmp */
}
else
{
I had proposed that patch here before, but iirc it was rejected on
grounds of not a PostgreSQL problem.
Christoph
--
c...@df7cb.de | http://www.df7cb.de/
signature.asc
Description: Digital signature
Re: [HACKERS] Securing make check (CVE-2014-0067)
On Sat, Mar 29, 2014 at 10:04:55AM +0100, Christoph Berg wrote: Fwiw, to relocate the pg_regress socket dir, there is already the possibility to run make check EXTRA_REGRESS_OPTS=--host=/tmp. (With the pending fix I sent yesterday to extend this to contrib/test_decoding.) That doesn't work for make check, because the postmaster ends up with listen_addresses=/tmp. We've been putting a small patch into pg_upgrade in Debian to work around too long socket paths generated by pg_upgrade during running the testsuite (and effectively on end user systems, but I don't think anyone is using such long paths there). A similar code bit could be put into pg_regress itself. Thanks for reminding me about Debian's troubles here. Once the dust settles on pg_regress, it will probably make sense to do likewise to pg_upgrade. -- Noah Misch EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
On Thu, Mar 06, 2014 at 11:52:22PM -0500, Noah Misch wrote:
On Thu, Mar 06, 2014 at 12:44:34PM -0500, Tom Lane wrote:
I'm inclined to suggest that we should put the socket under $CWD by
default, but provide some way for the user to override that choice.
If they want to put it in /tmp, it's on their head as to how secure
that is. On most modern platforms it'd be fine.
I am skeptical about the value of protecting systems with non-sticky /tmp, but
long $CWD isn't of great importance, either. I'm fine with your suggestion.
Though the $CWD or one of its parents could be world-writable, that would
typically mean an attacker could just replace the test cases directly.
Here's the patch. The temporary data directory makes for a convenient socket
directory; initdb already gives it mode 0700, and we have existing
arrangements to purge it when finished. One can override the socket directory
by defining PG_REGRESS_SOCK_DIR in the environment.
nm
--
Noah Misch
EnterpriseDB http://www.enterprisedb.com
diff --git a/contrib/pg_upgrade/test.sh b/contrib/pg_upgrade/test.sh
index baa7d47..c04398b 100644
--- a/contrib/pg_upgrade/test.sh
+++ b/contrib/pg_upgrade/test.sh
@@ -25,8 +25,6 @@ case $testhost in
*) LISTEN_ADDRESSES= ;;
esac
-POSTMASTER_OPTS=-F -c listen_addresses=$LISTEN_ADDRESSES
-
temp_root=$PWD/tmp_check
if [ $1 = '--install' ]; then
@@ -86,13 +84,16 @@ PGSERVICE=; unset PGSERVICE
PGSSLMODE=; unset PGSSLMODE
PGREQUIRESSL=; unset PGREQUIRESSL
PGCONNECT_TIMEOUT=; unset PGCONNECT_TIMEOUT
-PGHOST=;unset PGHOST
PGHOSTADDR=;unset PGHOSTADDR
-# Select a non-conflicting port number, similarly to pg_regress.c
+# Select a port number and socket directory, similarly to pg_regress.c
PG_VERSION_NUM=`grep '#define PG_VERSION_NUM' $newsrc/src/include/pg_config.h
| awk '{print $3}'`
PGPORT=`expr $PG_VERSION_NUM % 16384 + 49152`
export PGPORT
+PGHOST=${PG_REGRESS_SOCK_DIR-$PGDATA}
+export PGHOST
+
+POSTMASTER_OPTS=-F -c listen_addresses=$LISTEN_ADDRESSES -k \$PGHOST\
i=0
while psql -X postgres /dev/null 2/dev/null
diff --git a/doc/src/sgml/regress.sgml b/doc/src/sgml/regress.sgml
index 16b3621..f931963 100644
--- a/doc/src/sgml/regress.sgml
+++ b/doc/src/sgml/regress.sgml
@@ -58,21 +58,14 @@ make check
warning
para
-This test method starts a temporary server, which is 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 literalmake
-check/ on machines shared with untrusted users. Instead, run the tests
-after completing the installation, as described in the next section.
- /para
-
- para
-On Unix-like machines, this danger can be avoided if the temporary
-server's socket file is made inaccessible to other users, for example
-by running the tests in a protected chroot. On Windows, the temporary
-server opens a locally-accessible TCP socket, so filesystem protections
-cannot help.
+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 literalmake 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
@@ -111,6 +104,17 @@ make MAX_CONNECTIONS=10 check
/screen
runs no more than ten tests concurrently.
/para
+
+ para
+To protect your operating system user account, the test driver places the
+server's socket in a relative subdirectory inaccessible to other users.
+Since most systems constrain the length of socket paths well
+below literal_POSIX_PATH_MAX/, testing may fail to start from a
+directory with a long name. Work around this problem by pointing
+the envarPG_REGRESS_SOCK_DIR/ environment variable to a substitute
+socket directory having a shorter path. On a multi-user system, give that
+directory mode literal0700/.
+ /para
/sect2
sect2
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index abde5b4..14bf222 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -109,6 +109,7 @@ static const char *progname;
static char *logfilename;
static FILE *logfile;
static char *difffilename;
+static char *sockdir;
static _resultmap *resultmap = NULL;
@@ -758,8 +759,7 @@
Re: [HACKERS] Securing make check (CVE-2014-0067)
On Sun, Mar 23, 2014 at 07:04:20PM -0400, Noah Misch wrote: On Thu, Mar 06, 2014 at 11:52:22PM -0500, Noah Misch wrote: On Thu, Mar 06, 2014 at 12:44:34PM -0500, Tom Lane wrote: I'm inclined to suggest that we should put the socket under $CWD by default, but provide some way for the user to override that choice. If they want to put it in /tmp, it's on their head as to how secure that is. On most modern platforms it'd be fine. I am skeptical about the value of protecting systems with non-sticky /tmp, but long $CWD isn't of great importance, either. I'm fine with your suggestion. Though the $CWD or one of its parents could be world-writable, that would typically mean an attacker could just replace the test cases directly. Here's the patch. The temporary data directory makes for a convenient socket directory; initdb already gives it mode 0700, and we have existing arrangements to purge it when finished. One can override the socket directory by defining PG_REGRESS_SOCK_DIR in the environment. And here's a documentation patch warning about the limited applicability of unix_socket_permissions. diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 4eff91e..1c25ded 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -640,7 +640,11 @@ include 'filename' /para para -This parameter is irrelevant on Windows, which does not have +This parameter is irrelevant on systems, notably Solaris as of Solaris +10, that ignore socket permissions entirely. There, one can achieve a +similar effect by pointing varnameunix_socket_directories/ to a +directory having search permission limited to the desired audience. +This parameter is also irrelevant on Windows, which does not have Unix-domain sockets. /para /listitem -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
Noah Misch n...@leadboat.com writes: Thanks. To avoid socket path length limitations, I lean toward placing the socket temporary directory under /tmp rather than placing under the CWD: I'm not thrilled with that; it's totally insecure on platforms where /tmp isn't sticky, so it doesn't seem like an appropriate solution given that this discussion is now being driven by security concerns. http://www.postgresql.org/message-id/flat/20121129223632.ga15...@tornado.leadboat.com I re-read that thread. While we did fix the reporting end of it, ie the postmaster will now give you a clear failure message if your socket path is too long, that's going to be cold comfort to anyone who has to build in an environment they don't have much control over (such as my still-hypothetical-I-hope scenario about Red Hat package updates). I'm inclined to suggest that we should put the socket under $CWD by default, but provide some way for the user to override that choice. If they want to put it in /tmp, it's on their head as to how secure that is. On most modern platforms it'd be fine. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
On Thu, Mar 06, 2014 at 12:44:34PM -0500, Tom Lane wrote: Noah Misch n...@leadboat.com writes: Thanks. To avoid socket path length limitations, I lean toward placing the socket temporary directory under /tmp rather than placing under the CWD: I'm not thrilled with that; it's totally insecure on platforms where /tmp isn't sticky, so it doesn't seem like an appropriate solution given that this discussion is now being driven by security concerns. http://www.postgresql.org/message-id/flat/20121129223632.ga15...@tornado.leadboat.com I re-read that thread. While we did fix the reporting end of it, ie the postmaster will now give you a clear failure message if your socket path is too long, that's going to be cold comfort to anyone who has to build in an environment they don't have much control over (such as my still-hypothetical-I-hope scenario about Red Hat package updates). I'm inclined to suggest that we should put the socket under $CWD by default, but provide some way for the user to override that choice. If they want to put it in /tmp, it's on their head as to how secure that is. On most modern platforms it'd be fine. I am skeptical about the value of protecting systems with non-sticky /tmp, but long $CWD isn't of great importance, either. I'm fine with your suggestion. Though the $CWD or one of its parents could be world-writable, that would typically mean an attacker could just replace the test cases directly. -- Noah Misch EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
Noah Misch n...@leadboat.com writes: On Thu, Mar 06, 2014 at 12:44:34PM -0500, Tom Lane wrote: I'm inclined to suggest that we should put the socket under $CWD by default, but provide some way for the user to override that choice. If they want to put it in /tmp, it's on their head as to how secure that is. On most modern platforms it'd be fine. I am skeptical about the value of protecting systems with non-sticky /tmp, but long $CWD isn't of great importance, either. I'm fine with your suggestion. Though the $CWD or one of its parents could be world-writable, that would typically mean an attacker could just replace the test cases directly. If the build tree is world-writable, that is clearly Not Our Fault. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
On Tue, Mar 04, 2014 at 07:10:27PM -0500, Bruce Momjian wrote: On Sat, Mar 1, 2014 at 01:35:45PM -0500, Noah Misch wrote: Having that said, I can appreciate the value of tightening the socket mode for a bit of *extra* safety: --- a/src/test/regress/pg_regress.c +++ b/src/test/regress/pg_regress.c @@ -2299,4 +2299,5 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc fputs(\n# Configuration added by pg_regress\n\n, pg_conf); fputs(max_prepared_transactions = 2\n, pg_conf); + fputs(unix_socket_permissions = 0700\n, pg_conf); Pg_upgrade has this exact same problem, and take the same approach. You might want to look in pg_upgrade/server.c. Thanks. To avoid socket path length limitations, I lean toward placing the socket temporary directory under /tmp rather than placing under the CWD: http://www.postgresql.org/message-id/flat/20121129223632.ga15...@tornado.leadboat.com -- Noah Misch EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
On Sun, Mar 02, 2014 at 05:38:38PM -0500, Noah Misch wrote: Concerning the immediate fix for non-Windows systems, does any modern system ignore modes of Unix domain sockets? It appears to be a long-fixed problem: http://web.nvd.nist.gov/view/vuln/detail?vulnId=CVE-1999-1402 http://unix.stackexchange.com/questions/83032/which-systems-do-not-honor-socket-read-write-permissions Nonetheless, it would be helpful for folks to test any rare platforms they have at hand. Start a postmaster with --unix-socket-permissions= and attempt to connect via local socket. If psql gives something other than psql: could not connect to server: Permission denied, please report it. Some results are in. Both Solaris 10 and omnios-6de5e81 (OmniOS v11 r151008) ignore socket modes. That justifies wrapping the socket in a directory. -- Noah Misch EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
On Mon, Mar 03, 2014 at 08:15:41PM -0500, Tom Lane wrote: Noah Misch n...@leadboat.com writes: On Mon, Mar 03, 2014 at 01:29:00AM -0500, Tom Lane wrote: What I was envisioning was that we'd be relying on the permissions of the containing directory to keep out bad guys. Permissions on the socket itself might be sufficient, but what does it save us to assume that? My first preference is to use the simplest code that POSIX requires to have the behavior we desire. POSIX specifies as implementation-defined whether connect() checks filesystem permissions. That's true of both directory search permissions and permissions on the socket itself. Surely you are misinterpreting that. If it worked as you suggest, connect() would provide a trivial method of bypassing directory permissions, at least to the extent of being able to probe for the existence of files within supposedly-unreadable directories. I can believe that there are implementations that don't examine the permissions on the socket file itself, but not that there are any that disregard directory permissions during the file lookup. Wherever POSIX makes something implementation-defined, it is possible to design a conforming system with detestable properties. That does not shake the fact that this behavior is indeed implementation-defined. I found no evidence either way concerning the prevalence of systems that ignore directory search permissions above sockets. That's because the question is ridiculous on its face, so nobody ever bothered to address it. I think the burden is on you to show that there has ever been any system that read the spec the way you propose. I doubt any exist. I don't care for interposing a directory based solely on the fact that some ancient systems needed that. Changing unix_socket_permissions is a one-liner in each test driver. Placing the socket in a directory entails setting PGHOST in the psql and postmaster environments and cleaning up the directory on exit. Placing the socket anywhere besides the default location will require setting PGHOST anyway, so I don't see that this argument holds much water. If we have moved the socket anyway, then the costs of moving the socket vanish? Yes, yes they do... Your responses have not added to this thread. -- Noah Misch EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
On Sat, Mar 1, 2014 at 01:35:45PM -0500, Noah Misch wrote: Having that said, I can appreciate the value of tightening the socket mode for a bit of *extra* safety: --- a/src/test/regress/pg_regress.c +++ b/src/test/regress/pg_regress.c @@ -2299,4 +2299,5 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc fputs(\n# Configuration added by pg_regress\n\n, pg_conf); fputs(max_prepared_transactions = 2\n, pg_conf); + fputs(unix_socket_permissions = 0700\n, pg_conf); Pg_upgrade has this exact same problem, and take the same approach. You might want to look in pg_upgrade/server.c. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
On 03/03/2014 02:00 AM, Tom Lane wrote: Josh Berkus j...@agliodbs.com writes: The only way I can see this being of real use to an attacker is if they could use this exploit to create a wormed version of PostgresQL on the target build system. Is that possible? It's theoretically possible, since having broken into the build user's account they could modify the already-built-but-not-yet-packaged PG executables. Having said that, though, I concur with the feeling that this probably isn't a useful exploit in practice. On Red Hat's build systems, for example, different packages are built in different chroots. So even if a malicious package is being built concurrently, it could not reach the postmaster's socket. A breakin would only be possible for somebody who had outside-the-chroots control of the build machine ... in which case they can hack pretty much any built package pretty much any way they want, without need for anything as fiddly as this. Other vendors might do things differently, but it still seems likely that there would be easier exploits available to anyone who's managed to get control on a machine used for package building. I'm less worried about vendor build systems and more about roll your own systems like Gentoo, FreeBSD ports, and Homebrew. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
On Mon, Mar 03, 2014 at 01:29:00AM -0500, Tom Lane wrote: Noah Misch n...@leadboat.com writes: Concerning the immediate fix for non-Windows systems, does any modern system ignore modes of Unix domain sockets? It appears to be a long-fixed problem: What I was envisioning was that we'd be relying on the permissions of the containing directory to keep out bad guys. Permissions on the socket itself might be sufficient, but what does it save us to assume that? My first preference is to use the simplest code that POSIX requires to have the behavior we desire. POSIX specifies as implementation-defined whether connect() checks filesystem permissions. That's true of both directory search permissions and permissions on the socket itself. POSIX alone can't help us here. My second preference is to use the simplest code known to be portable to all credible PostgreSQL target systems. Brief research was inconclusive, but it turned up no solid evidence of a modern target ignoring socket permissions. (It did turn up solid evidence of 15-year-old targets having that problem.) I found no evidence either way concerning the prevalence of systems that ignore directory search permissions above sockets. I don't care for interposing a directory based solely on the fact that some ancient systems needed that. Changing unix_socket_permissions is a one-liner in each test driver. Placing the socket in a directory entails setting PGHOST in the psql and postmaster environments and cleaning up the directory on exit. That would be fine if restricted to pg_regress, but it would also show up in contrib/pg_upgrade/test.sh, perhaps eventually in vcregress.pl:upgradecheck(), perhaps in the buildfarm code, in the DBD::Pg test suite, and in any other test suite that creates a temporary cluster. We should not lead all those test drivers into using a temporary socket directory based on long-gone bugs or cargo cult programming. If there are notable systems today where it helps, that's a different matter. Also, test drivers should not be the sole place where we express doubt about the reliability of socket permissions. If they are unreliable on a noteworthy target, then the unix_socket_permissions documentation ought to say so. nm -- Noah Misch EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
Noah Misch n...@leadboat.com writes: On Mon, Mar 03, 2014 at 01:29:00AM -0500, Tom Lane wrote: What I was envisioning was that we'd be relying on the permissions of the containing directory to keep out bad guys. Permissions on the socket itself might be sufficient, but what does it save us to assume that? My first preference is to use the simplest code that POSIX requires to have the behavior we desire. POSIX specifies as implementation-defined whether connect() checks filesystem permissions. That's true of both directory search permissions and permissions on the socket itself. Surely you are misinterpreting that. If it worked as you suggest, connect() would provide a trivial method of bypassing directory permissions, at least to the extent of being able to probe for the existence of files within supposedly-unreadable directories. I can believe that there are implementations that don't examine the permissions on the socket file itself, but not that there are any that disregard directory permissions during the file lookup. I found no evidence either way concerning the prevalence of systems that ignore directory search permissions above sockets. That's because the question is ridiculous on its face, so nobody ever bothered to address it. I think the burden is on you to show that there has ever been any system that read the spec the way you propose. I don't care for interposing a directory based solely on the fact that some ancient systems needed that. Changing unix_socket_permissions is a one-liner in each test driver. Placing the socket in a directory entails setting PGHOST in the psql and postmaster environments and cleaning up the directory on exit. Placing the socket anywhere besides the default location will require setting PGHOST anyway, so I don't see that this argument holds much water. The cleanup aspect is likewise not that exciting; pg_regress creates a lot of stuff it doesn't remove. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
I wrote: Placing the socket anywhere besides the default location will require setting PGHOST anyway, so I don't see that this argument holds much water. The cleanup aspect is likewise not that exciting; pg_regress creates a lot of stuff it doesn't remove. There's another point here, if you think back to the discussion as it stood before we noticed that there was a security problem. What was originally under discussion was making life easier for packagers who want to build with a default socket location like /var/run/postgresql/. In a build environment, that directory may not exist, and even if it does, there is no way that the build user should have write permission on it. So it is already the case that some packagers have to override the socket location if they want to do make check while packaging, and what we were originally on about was making that part of the normal pg_regress procedures instead of being something requiring patching. So, whether or not unix_socket_permissions would be a bulletproof security fix by itself, I'd still want to see some provisions made for putting the socket into a local directory during make check. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
On Sun, Mar 2, 2014 at 6:20 AM, Noah Misch n...@leadboat.com wrote: On Sat, Mar 01, 2014 at 05:51:46PM -0500, Andrew Dunstan wrote: On 03/01/2014 05:10 PM, Tom Lane wrote: One other thought here: is it actually reasonable to expend a lot of effort on the Windows case? I'm not aware that people normally expect a Windows box to have multiple users at all, let alone non-mutually-trusting users. As Stephen said, it's fairly unusual. There are usually quite a few roles, but it's rare to have more than one human type role connected to the machine at a given time. I, too, agree it's rare. Rare enough to justify leaving the vulnerability open on Windows, indefinitely? I'd say not. Windows itself has been pushing steadily toward better multi-user support over the past 15 years or so. Releasing software for Windows as though it were a single-user platform is backwards-looking. We should be a model in this area, not a straggler. Terminal Services have definitely become more common over time, but with faster and cheaper virtualization, a lot of people have switched to that instead, which would remove the problem of course. I wonder how common it actually is, though, to *build postgres* on a terminal services machine with other users on it... Not saying we can't ignore it, and I gree that we should not be a straggler on this, so doing a proper fix wwould definitely be the better. I'd be happy doing nothing in this case, or not very much. e.g. provide a password but not with great cryptographic strength. One option that would simplify things is to fix only non-Windows in the back branches, via socket protection, and fix Windows in HEAD only. We could even do so by extending HAVE_UNIX_SOCKETS support to Windows through named pipes. That could certainly be a useful feature of it's own. But as you say, non-backpatchable. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
Re: [HACKERS] Securing make check (CVE-2014-0067)
* Dave Page (dp...@pgadmin.org) wrote: It's not that rare in my experience - certainly there are far more single user installations, but Terminal Server configurations are common for deploying apps Citrix-style or VDI. The one and only Windows server maintained by the EDB infrastructure team is a terminal server for example. Sure- but do you have a full build environment there for building PG? That's really what I'm referring to as being relatively rare. I'm very familiar with terminal servers, but those are almost always used for getting access to IE or other corporate dependencies, or for coming in from remote, or running Windows-only applications. We've got a terminal server at my current job, and I ran a whole slew of them at my last job and in neither case did we have development tools installed. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Securing make check (CVE-2014-0067)
Noah Misch n...@leadboat.com writes: One option that would simplify things is to fix only non-Windows in the back branches, via socket protection, and fix Windows in HEAD only. We could even do so by extending HAVE_UNIX_SOCKETS support to Windows through named pipes. +1 for that solution, if it's not an unreasonable amount of work to add named-pipe sockets in Windows. That would offer a feature to Windows users that they didn't have before, ie the ability to restrict connections based on filesystem permissions; so it seems useful quite aside from any make check considerations. There's an independent question of whether the regression tests will work for make installcheck against a server that's not set up for trust auth. I'm inclined to think that we can leave it to the user to generate appropriate passwords if he's using password auth, but don't we still need some test procedure adjustments? Also, to what extent does any of this affect buildfarm animals? Whatever we do for make check will presumably make those tests safe for them, but how are the postmasters they test under make installcheck set up? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
On 02/03/2014 15:30, Magnus Hagander wrote: Terminal Services have definitely become more common over time, but with faster and cheaper virtualization, a lot of people have switched to that instead, which would remove the problem of course. I wonder how common it actually is, though, to *build postgres* on a terminal services machine with other users on it... Well, the banks I've contracted at recently are all rather keen on virtual desktops for developers, and some of those are terminal services. We're a headache, and packaging up all the things we need is a pain, so there is some mileage in buying grunty servers and doing specific installs that are then shared, rather than making an MSI generally available. Also I have experience of being given accounts for jenkins etc that are essentially terminal services logins, and having these things unable to maintain a software stack can effectively disqualify tech we would otherwise use. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
On 03/02/2014 01:27 PM, Tom Lane wrote: Also, to what extent does any of this affect buildfarm animals? Whatever we do for make check will presumably make those tests safe for them, but how are the postmasters they test under make installcheck set up? Nothing special. bin/initdb -U buildfarm --locale=$locale data-$locale ... bin/pg_ctl -D data-$locale -l logfile -w start We have wide control over what's done, just let me know what's wanted. For example, it would be pretty simple to make it use a non-standard socket directory and turn tcp connections off on Unix, or to set up password auth for that matter, assuming we already have a strong password. I generally assume that people aren't running buildfarm animals on general purpose multi-user machines, but it might be as well to take precautions. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
* james (ja...@mansionfamily.plus.com) wrote: Well, the banks I've contracted at recently are all rather keen on virtual desktops for developers, and some of those are terminal services. We're a headache, and packaging up all the things we need is a pain, so there is some mileage in buying grunty servers and doing specific installs that are then shared, rather than making an MSI generally available. Also I have experience of being given accounts for jenkins etc that are essentially terminal services logins, and having these things unable to maintain a software stack can effectively disqualify tech we would otherwise use. And what are the feelings security on these multi-user development environments? Is everyone on them trusted users, or are there untrusted / general accounts? The issue here is about how much effort to go to in order to secure the PostgreSQL system that is started up to do the regression tests. It's already set up to only listen on localhost and will run with only the privileges of the user running the tests. The concern is that another user on the same system could gain access to the account which is running the 'make check' by connecting over localhost to the PostgreSQL instance and being superuser there, which would allow executing commands, etc, as that other user (eg: with COPY PIPE). THanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Securing make check (CVE-2014-0067)
On 03/02/2014 12:17 PM, Stephen Frost wrote: The issue here is about how much effort to go to in order to secure the PostgreSQL system that is started up to do the regression tests. It's already set up to only listen on localhost and will run with only the privileges of the user running the tests. The concern is that another user on the same system could gain access to the account which is running the 'make check' by connecting over localhost to the PostgreSQL instance and being superuser there, which would allow executing commands, etc, as that other user (eg: with COPY PIPE). My $0.02: Not a lot of effort. A) Few users run the regression tests at all, because they use packages. B) Of the users who do self-builds, most do so on secure systems deep inside the corporate firewall. C) A related attack requires not only access to the host but good timing as well, or the ability to leave a booby-trap program on the system. D) If the host is compromised, the user gains access to the build user ... which should be a regular, unprivilged, shell user. The only way I can see this being of real use to an attacker is if they could use this exploit to create a wormed version of PostgresQL on the target build system. Is that possible? -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
* Josh Berkus (j...@agliodbs.com) wrote: The only way I can see this being of real use to an attacker is if they could use this exploit to create a wormed version of PostgresQL on the target build system. Is that possible? I don't see why it wouldn't be- once the attacker is on the box as any user, they could gain access to the account doing the builds and then build whatever they want. Of course, if they've been able to compromise an account on the host it's entirely likely they've already been able to gain admin access (probably more easily than going through PG to get at the build user) and then it's a moot point. All that said- if we can use named pipes on Windows, ala what we do on Unix, I'm all for it.. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Securing make check (CVE-2014-0067)
On Sun, Mar 2, 2014 at 7:27 PM, Tom Lane t...@sss.pgh.pa.us wrote: Noah Misch n...@leadboat.com writes: One option that would simplify things is to fix only non-Windows in the back branches, via socket protection, and fix Windows in HEAD only. We could even do so by extending HAVE_UNIX_SOCKETS support to Windows through named pipes. +1 for that solution, if it's not an unreasonable amount of work to add named-pipe sockets in Windows. That would offer a feature to Windows users that they didn't have before, ie the ability to restrict connections based on filesystem permissions; so it seems useful quite aside from any make check considerations. I think it might be a bigger piece of work than we'd like - and IIRC that's one of the reasons we didn't do it from the start. Named pipes on windows do act as files on Windows, but they do *not* act as sockets. As in, they return HANDLEs, not SOCKETs, and you can't recv() and send() on them. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
Re: [HACKERS] Securing make check (CVE-2014-0067)
On Sun, Mar 02, 2014 at 01:27:18PM -0500, Tom Lane wrote: Noah Misch n...@leadboat.com writes: One option that would simplify things is to fix only non-Windows in the back branches, via socket protection, and fix Windows in HEAD only. We could even do so by extending HAVE_UNIX_SOCKETS support to Windows through named pipes. +1 for that solution, if it's not an unreasonable amount of work to add named-pipe sockets in Windows. That would offer a feature to Windows users that they didn't have before, ie the ability to restrict connections based on filesystem permissions; so it seems useful quite aside from any make check considerations. Agreed. Windows named pipes do not go through the winsock API, so it might take a good amount of muddle to achieve this. If it doesn't work out, we'll revisit use of MD5 authentication for regression tests. Also, I'd be just as happy for someone else to do the primary development on such a project. Concerning the immediate fix for non-Windows systems, does any modern system ignore modes of Unix domain sockets? It appears to be a long-fixed problem: http://web.nvd.nist.gov/view/vuln/detail?vulnId=CVE-1999-1402 http://unix.stackexchange.com/questions/83032/which-systems-do-not-honor-socket-read-write-permissions Nonetheless, it would be helpful for folks to test any rare platforms they have at hand. Start a postmaster with --unix-socket-permissions= and attempt to connect via local socket. If psql gives something other than psql: could not connect to server: Permission denied, please report it. There's an independent question of whether the regression tests will work for make installcheck against a server that's not set up for trust auth. I'm inclined to think that we can leave it to the user to generate appropriate passwords if he's using password auth, but don't we still need some test procedure adjustments? Right. To have make installcheck-world work against a cluster requiring md5 authentication, I would use the makecheck-secure-v3.patch test suite changes. I suppose that's a good thing to nail down, even if testing against md5 does not become the norm. -- Noah Misch EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
Noah Misch n...@leadboat.com writes: Concerning the immediate fix for non-Windows systems, does any modern system ignore modes of Unix domain sockets? It appears to be a long-fixed problem: What I was envisioning was that we'd be relying on the permissions of the containing directory to keep out bad guys. Permissions on the socket itself might be sufficient, but what does it save us to assume that? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
Josh Berkus j...@agliodbs.com writes: The only way I can see this being of real use to an attacker is if they could use this exploit to create a wormed version of PostgresQL on the target build system. Is that possible? It's theoretically possible, since having broken into the build user's account they could modify the already-built-but-not-yet-packaged PG executables. Having said that, though, I concur with the feeling that this probably isn't a useful exploit in practice. On Red Hat's build systems, for example, different packages are built in different chroots. So even if a malicious package is being built concurrently, it could not reach the postmaster's socket. A breakin would only be possible for somebody who had outside-the-chroots control of the build machine ... in which case they can hack pretty much any built package pretty much any way they want, without need for anything as fiddly as this. Other vendors might do things differently, but it still seems likely that there would be easier exploits available to anyone who's managed to get control on a machine used for package building. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
* Tom Lane (t...@sss.pgh.pa.us) wrote: Noah Misch n...@leadboat.com writes: Concerning the immediate fix for non-Windows systems, does any modern system ignore modes of Unix domain sockets? It appears to be a long-fixed problem: What I was envisioning was that we'd be relying on the permissions of the containing directory to keep out bad guys. Permissions on the socket itself might be sufficient, but what does it save us to assume that? Agreed- the general approach to this, from what I've seen, is to handle it with the directory. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Securing make check (CVE-2014-0067)
I didn't check the patch in detail, but it seems to me that both the encode stuff as well as pgrand belong in src/common rather than src/port. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
On Sat, Mar 01, 2014 at 12:48:08PM -0300, Alvaro Herrera wrote: I didn't check the patch in detail, but it seems to me that both the encode stuff as well as pgrand belong in src/common rather than src/port. Since src/common exists only in 9.3 and up, that would mean putting them in different libraries depending on the branch. I did not think that worthwhile. -- Noah Misch EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
Noah Misch n...@leadboat.com writes: As announced with last week's releases, use of trust authentication in the make check temporary database cluster makes it straightforward to hijack the OS user account involved. The prerequisite is another user account on the same system. The solution we discussed on secur...@postgresql.org was to switch to md5 authentication with a generated, strong password. Noah is leaving the impression that there was consensus on that approach; there was not, which is one reason that this patch didn't simply get committed last week. There are two big problems with the lets-generate-a-random-password approach. Noah acknowledged the portability issue of possibly not having a strong entropy source available. The other issue though is whether doing this doesn't introduce enough crypto dependency into the core code to create an export-restrictions hazard. We've kept contrib/pgcrypto out of core all these years in order to give people a relatively straightforward solution if they are worried about export laws: just omit contrib/pgcrypto. But just omit regression testing isn't palatable. In the case of Unix systems, there is a *far* simpler and more portable solution technique, which is to tell the test postmaster to put its socket in some non-world-accessible directory created by the test scaffolding. Of course that doesn't work for Windows, which is why we looked at the random-password solution. But I wonder whether we shouldn't use the nonstandard-socket-location approach everywhere else, and only use random passwords on Windows. That would greatly reduce the number of cases to worry about for portability of the password-generation code; and perhaps we could also push the crypto issue into reliance on some Windows-supplied functionality (though I'm just speculating about that part). regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
On 03/01/2014 12:29 PM, Tom Lane wrote: In the case of Unix systems, there is a *far* simpler and more portable solution technique, which is to tell the test postmaster to put its socket in some non-world-accessible directory created by the test scaffolding. +1 - I'm all for KISS. Of course that doesn't work for Windows, which is why we looked at the random-password solution. But I wonder whether we shouldn't use the nonstandard-socket-location approach everywhere else, and only use random passwords on Windows. That would greatly reduce the number of cases to worry about for portability of the password-generation code; and perhaps we could also push the crypto issue into reliance on some Windows-supplied functionality (though I'm just speculating about that part). See for example http://msdn.microsoft.com/en-us/library/windows/desktop/aa379942%28v=vs.85%29.aspx cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
On Sat, Mar 1, 2014 at 7:09 PM, Andrew Dunstan and...@dunslane.net wrote: On 03/01/2014 12:29 PM, Tom Lane wrote: In the case of Unix systems, there is a *far* simpler and more portable solution technique, which is to tell the test postmaster to put its socket in some non-world-accessible directory created by the test scaffolding. +1 - I'm all for KISS. Of course that doesn't work for Windows, which is why we looked at the random-password solution. But I wonder whether we shouldn't use the nonstandard-socket-location approach everywhere else, and only use random passwords on Windows. That would greatly reduce the number of cases to worry about for portability of the password-generation code; and perhaps we could also push the crypto issue into reliance on some Windows-supplied functionality (though I'm just speculating about that part). See for example http://msdn.microsoft.com/en-us/library/windows/desktop/ aa379942%28v=vs.85%29.aspx For a one-off password used locally only, we could also consider just using a guid, and generate it using http://msdn.microsoft.com/en-us/library/windows/desktop/aa379205(v=vs.85).aspx. Obviously windows only though - do we have *any* Unix platforms that can't do unix sockets? -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
Re: [HACKERS] Securing make check (CVE-2014-0067)
On Sat, Mar 01, 2014 at 12:29:38PM -0500, Tom Lane wrote: There are two big problems with the lets-generate-a-random-password approach. Noah acknowledged the portability issue of possibly not having a strong entropy source available. The other issue though is whether doing this doesn't introduce enough crypto dependency into the core code to create an export-restrictions hazard. We've kept contrib/pgcrypto out of core all these years in order to give people a relatively straightforward solution if they are worried about export laws: just omit contrib/pgcrypto. But just omit regression testing isn't palatable. If pgrand.c poses an export control problem, then be-secure.c poses a greater one. pgrand.c is just glue to an OS-provided CSPRNG. In the case of Unix systems, there is a *far* simpler and more portable solution technique, which is to tell the test postmaster to put its socket in some non-world-accessible directory created by the test scaffolding. Of course that doesn't work for Windows, which is why we looked at the random-password solution. But I wonder whether we shouldn't use the nonstandard-socket-location approach everywhere else, and only use random passwords on Windows. That would greatly reduce the number of cases to worry about for portability of the password-generation code; Further worrying about systems lacking /dev/urandom is a waste of time. They will only get less common, and they are already rare enough that we have zero of them on the buildfarm. I provided them with a straightforward workaround: point the PGTESTPWFILE environment variable to a file containing a password. Having that said, I can appreciate the value of tightening the socket mode for a bit of *extra* safety: --- a/src/test/regress/pg_regress.c +++ b/src/test/regress/pg_regress.c @@ -2299,4 +2299,5 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc fputs(\n# Configuration added by pg_regress\n\n, pg_conf); fputs(max_prepared_transactions = 2\n, pg_conf); + fputs(unix_socket_permissions = 0700\n, pg_conf); Taking the further step of retaining trust authentication on Unix would make it easier to commit tests guaranteed to fail on Windows. I see no benefit cancelling that out. and perhaps we could also push the crypto issue into reliance on some Windows-supplied functionality (though I'm just speculating about that part). Every version of the patch has done it that way. It has used OS-supplied cryptography on every platform. nm -- Noah Misch EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
* Tom Lane (t...@sss.pgh.pa.us) wrote: In the case of Unix systems, there is a *far* simpler and more portable solution technique, which is to tell the test postmaster to put its socket in some non-world-accessible directory created by the test scaffolding. Yes, yes, yes. Of course that doesn't work for Windows, which is why we looked at the random-password solution. But I wonder whether we shouldn't use the nonstandard-socket-location approach everywhere else, and only use random passwords on Windows. That would greatly reduce the number of cases to worry about for portability of the password-generation code; and perhaps we could also push the crypto issue into reliance on some Windows-supplied functionality (though I'm just speculating about that part). Multi-user Windows build systems are *far* more rare than unix equivilants (though even those are semi-rare in these days w/ all the VMs running around, but still, you may have University common unix systems with students building PG- the same just doesn't exist in my experience on the Windows side). Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Securing make check (CVE-2014-0067)
Magnus Hagander mag...@hagander.net writes: For a one-off password used locally only, we could also consider just using a guid, and generate it using http://msdn.microsoft.com/en-us/library/windows/desktop/aa379205(v=vs.85).aspx. Not sure if that API is intended to create an unpredictable UUID, rather than just a unique one. Obviously windows only though - do we have *any* Unix platforms that can't do unix sockets? I'm not aware of any. A look into the git history of pg_config_manual.h shows that QNX and BEOS used to be marked as not having Unix sockets, but of course we dropped support for those in 2006. I'd rather bet on all non-Windows platforms have Unix sockets than all non-Windows platforms have /dev/urandom; the former standard is far older than the latter. One other thought here: is it actually reasonable to expend a lot of effort on the Windows case? I'm not aware that people normally expect a Windows box to have multiple users at all, let alone non-mutually-trusting users. BTW, a different problem with the proposed patch is that it changes some test cases in ecpg and contrib/dblink, apparently to avoid session reconnections. That seems likely to me to be losing test coverage. Perhaps there is no alternative, but I'd like to have some discussion around that point as well. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
On 03/01/2014 05:10 PM, Tom Lane wrote: One other thought here: is it actually reasonable to expend a lot of effort on the Windows case? I'm not aware that people normally expect a Windows box to have multiple users at all, let alone non-mutually-trusting users. As Stephen said, it's fairly unusual. There are usually quite a few roles, but it's rare to have more than one human type role connected to the machine at a given time. I'd be happy doing nothing in this case, or not very much. e.g. provide a password but not with great cryptographic strength. BTW, a different problem with the proposed patch is that it changes some test cases in ecpg and contrib/dblink, apparently to avoid session reconnections. That seems likely to me to be losing test coverage. Perhaps there is no alternative, but I'd like to have some discussion around that point as well. Yeah. Assuming we make the changes you're suggesting that should no longer be necessary, right? cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
Andrew Dunstan and...@dunslane.net writes: On 03/01/2014 05:10 PM, Tom Lane wrote: BTW, a different problem with the proposed patch is that it changes some test cases in ecpg and contrib/dblink, apparently to avoid session reconnections. That seems likely to me to be losing test coverage. Perhaps there is no alternative, but I'd like to have some discussion around that point as well. Yeah. Assuming we make the changes you're suggesting that should no longer be necessary, right? Not sure. I believe the point of those changes is that the scaffolding only sets up a password for the original superuser, so that connecting as anybody else will fail if the test postmaster is configured for password auth. If so, the only way to avoid doing any work would be if we don't implement any fix at all for Windows. Whether or not you're worried about the security of make check on Windows, there are definitely some things to be unhappy about here. One being that the core regression tests are evidently not testing connecting as anybody but superuser; and the proposed changes would remove such testing from contrib and ecpg as well, which is surely not better from a coverage standpoint. (It's not terribly hard to imagine permissions bugs that would cause postinit.c to fail for non-superusers.) Another issue is that (I presume, haven't checked) make installcheck on contrib or ecpg will currently fail against a server that's not configured for trust auth. Ideally you should be able to do make installcheck against a reasonably-configured server. I'm not real sure what to do about either of those points, but I am sure that the proposed patch isn't moving the ball downfield. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
On Sat, Mar 01, 2014 at 05:51:46PM -0500, Andrew Dunstan wrote: On 03/01/2014 05:10 PM, Tom Lane wrote: One other thought here: is it actually reasonable to expend a lot of effort on the Windows case? I'm not aware that people normally expect a Windows box to have multiple users at all, let alone non-mutually-trusting users. As Stephen said, it's fairly unusual. There are usually quite a few roles, but it's rare to have more than one human type role connected to the machine at a given time. I, too, agree it's rare. Rare enough to justify leaving the vulnerability open on Windows, indefinitely? I'd say not. Windows itself has been pushing steadily toward better multi-user support over the past 15 years or so. Releasing software for Windows as though it were a single-user platform is backwards-looking. We should be a model in this area, not a straggler. I'd be happy doing nothing in this case, or not very much. e.g. provide a password but not with great cryptographic strength. One option that would simplify things is to fix only non-Windows in the back branches, via socket protection, and fix Windows in HEAD only. We could even do so by extending HAVE_UNIX_SOCKETS support to Windows through named pipes. Using weak passwords on Windows alone would not simplify the effort. -- Noah Misch EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
On Sat, Mar 01, 2014 at 09:43:19PM -0500, Tom Lane wrote: Andrew Dunstan and...@dunslane.net writes: On 03/01/2014 05:10 PM, Tom Lane wrote: BTW, a different problem with the proposed patch is that it changes some test cases in ecpg and contrib/dblink, apparently to avoid session reconnections. That seems likely to me to be losing test coverage. Perhaps there is no alternative, but I'd like to have some discussion around that point as well. connect/test5.pgc has the same number of session reconnections before and after the patch. The change is to assign passwords to the connection test accounts and use those passwords during the tests. Test coverage actually increased there; before, it did not matter whether ecpg conveyed each password in good order. The dblink change does replace a non-superuser reconnection with a SET SESSION AUTHORIZATION. I believe the point of those changes is that the scaffolding only sets up a password for the original superuser, so that connecting as anybody else will fail if the test postmaster is configured for password auth. Essentially, yes. You can connect as another user if you assign a password; the ecpg suite does so. (That user had better be unprivileged.) The dblink change has a second point. Since the dblink_regression_test role has use of the dblink_connect_u() function, it needs to be treated as a privileged account. (I'll add a comment about that.) Another issue is that (I presume, haven't checked) make installcheck on contrib or ecpg will currently fail against a server that's not configured for trust auth. Ideally you should be able to do make installcheck against a reasonably-configured server. No, I had verified make installcheck-world under md5 auth. The setup I recommend, which I mentioned in the initial message of this thread, is to put the same PGPASSFILE in the postmaster environment as you put in the make installcheck environment. (It's contrib/dblink and contrib/postgres_fdw that would otherwise fail.) nm -- Noah Misch EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Securing make check (CVE-2014-0067)
On 2 Mar 2014, at 05:20, Noah Misch n...@leadboat.com wrote: On Sat, Mar 01, 2014 at 05:51:46PM -0500, Andrew Dunstan wrote: On 03/01/2014 05:10 PM, Tom Lane wrote: One other thought here: is it actually reasonable to expend a lot of effort on the Windows case? I'm not aware that people normally expect a Windows box to have multiple users at all, let alone non-mutually-trusting users. As Stephen said, it's fairly unusual. There are usually quite a few roles, but it's rare to have more than one human type role connected to the machine at a given time. I, too, agree it's rare. Rare enough to justify leaving the vulnerability open on Windows, indefinitely? It's not that rare in my experience - certainly there are far more single user installations, but Terminal Server configurations are common for deploying apps Citrix-style or VDI. The one and only Windows server maintained by the EDB infrastructure team is a terminal server for example. I'd say not. Windows itself has been pushing steadily toward better multi-user support over the past 15 years or so. Releasing software for Windows as though it were a single-user platform is backwards-looking. We should be a model in this area, not a straggler. Definitely. I'd be happy doing nothing in this case, or not very much. e.g. provide a password but not with great cryptographic strength. One option that would simplify things is to fix only non-Windows in the back branches, via socket protection, and fix Windows in HEAD only. We could even do so by extending HAVE_UNIX_SOCKETS support to Windows through named pipes. Using weak passwords on Windows alone would not simplify the effort. -- Noah Misch EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
