src/test/ssl broken on HEAD

Started by Michael Paquierover 10 years ago11 messageshackers
Jump to latest
#1Michael Paquier
michael@paquier.xyz

Hi all,

The following commit has broken the SSL test suite (embarrassing and
lonely moment):
commit 13d856e177e69083f543d6383eeda9e12ce3c55c
Author: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date: Wed Jul 29 19:17:02 2015 +0300

Make TAP tests work on Windows.
[...]
*Michael Paquier*, reviewed by Noah Misch, some additional tweaking by me.

What actually happens is that the SSL suite needs to use a host IP for
the tests, but listen_addresses has been reshuffled in the commit
above. Only HEAD is impacted, and attached is a patch to fix the
problem. I have refactored as well the code so as 127.0.0.1 is not
hardcoded around anymore, and the IP used for the tests is
centralized. The tests also used a log file, but this does not make
sense anymore as all the output from stderr and stdout is captured in
a log file.
Regards,
--
Michael

Attachments:

20150826_ssl_tap_fix.patchtext/x-patch; charset=US-ASCII; name=20150826_ssl_tap_fix.patchDownload+11-19
#2Michael Paquier
michael@paquier.xyz
In reply to: Michael Paquier (#1)
Re: src/test/ssl broken on HEAD

On Wed, Aug 26, 2015 at 4:35 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

Only HEAD is impacted, and attached is a patch to fix the problem.

Actually this version is better, I forgot to update a comment.
--
Michael

Attachments:

20150826_ssl_tap_fix.patchtext/x-patch; charset=US-ASCII; name=20150826_ssl_tap_fix.patchDownload+9-19
#3Robert Haas
robertmhaas@gmail.com
In reply to: Michael Paquier (#2)
Re: src/test/ssl broken on HEAD

On Wed, Aug 26, 2015 at 3:37 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:

On Wed, Aug 26, 2015 at 4:35 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

Only HEAD is impacted, and attached is a patch to fix the problem.

Actually this version is better, I forgot to update a comment.

For so long as this test suite is not run by 'make check-world' or by
the buildfarm, it's likely to keep getting broken, and we're likely to
keep not noticing. I realize that the decision to exclude this from
'make check-world' was deliberately made for security reasons, but
that doesn't take anything away from the maintenance headache.

Still, that's not a reason not commit this, so done.

--
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

#4Andrew Dunstan
andrew@dunslane.net
In reply to: Robert Haas (#3)
Re: src/test/ssl broken on HEAD

On 09/02/2015 04:22 PM, Robert Haas wrote:

On Wed, Aug 26, 2015 at 3:37 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:

On Wed, Aug 26, 2015 at 4:35 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

Only HEAD is impacted, and attached is a patch to fix the problem.

Actually this version is better, I forgot to update a comment.

For so long as this test suite is not run by 'make check-world' or by
the buildfarm, it's likely to keep getting broken, and we're likely to
keep not noticing. I realize that the decision to exclude this from
'make check-world' was deliberately made for security reasons, but
that doesn't take anything away from the maintenance headache.

Still, that's not a reason not commit this, so done.

Tell me what's needed and I'll look at creating a buildfarm test module
for it.

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

#5Robert Haas
robertmhaas@gmail.com
In reply to: Andrew Dunstan (#4)
Re: src/test/ssl broken on HEAD

On Wed, Sep 2, 2015 at 4:50 PM, Andrew Dunstan <andrew@dunslane.net> wrote:

Tell me what's needed and I'll look at creating a buildfarm test module for
it.

You run the tests via: make -C src/test/ssl check

But nota bene security caveats:

commit e39250c644ea7cd3904e4e24570db21a209cf97f
Author: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date: Tue Dec 9 17:21:18 2014 +0200

Add a regression test suite for SSL support.

It's not run by the global "check" or "installcheck" targets, because the
temporary installation it creates accepts TCP connections from any user
the same host, which is insecure.

--
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

#6Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Andrew Dunstan (#4)
Re: src/test/ssl broken on HEAD

Andrew Dunstan wrote:

On 09/02/2015 04:22 PM, Robert Haas wrote:

For so long as this test suite is not run by 'make check-world' or by
the buildfarm, it's likely to keep getting broken, and we're likely to
keep not noticing. I realize that the decision to exclude this from
'make check-world' was deliberately made for security reasons, but
that doesn't take anything away from the maintenance headache.

Tell me what's needed and I'll look at creating a buildfarm test module for
it.

AFAICS it just requires running "make check" in src/test/ssl. Maybe
this could be part of a generic module that runs "make check" in
specified subdirectories; see
/messages/by-id/20150813181107.GC5232@alvherre.pgsql
for previous discussion about that.

--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, 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

#7Andres Freund
andres@anarazel.de
In reply to: Robert Haas (#5)
Re: src/test/ssl broken on HEAD

On 2015-09-02 17:03:46 -0400, Robert Haas wrote:

On Wed, Sep 2, 2015 at 4:50 PM, Andrew Dunstan <andrew@dunslane.net> wrote:

Tell me what's needed and I'll look at creating a buildfarm test module for
it.

You run the tests via: make -C src/test/ssl check

But nota bene security caveats:

commit e39250c644ea7cd3904e4e24570db21a209cf97f
Author: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date: Tue Dec 9 17:21:18 2014 +0200

Add a regression test suite for SSL support.

It's not run by the global "check" or "installcheck" targets, because the
temporary installation it creates accepts TCP connections from any user
the same host, which is insecure.

We could just implement SSL over unix sockets. Obviously the
connection-encryption aspect isn't actually useful, but e.g. client
certs still make sense. Besides, it allows to avoid concerns like the
above...

Greetings,

Andres Freund

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#8Michael Paquier
michael@paquier.xyz
In reply to: Robert Haas (#3)
Re: src/test/ssl broken on HEAD

On Thu, Sep 3, 2015 at 5:22 AM, Robert Haas wrote:

Still, that's not a reason not commit this, so done.

Thanks.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#9Noah Misch
noah@leadboat.com
In reply to: Andres Freund (#7)
Re: src/test/ssl broken on HEAD

On Thu, Sep 03, 2015 at 01:15:47AM +0200, Andres Freund wrote:

On 2015-09-02 17:03:46 -0400, Robert Haas wrote:

On Wed, Sep 2, 2015 at 4:50 PM, Andrew Dunstan <andrew@dunslane.net> wrote:

Tell me what's needed and I'll look at creating a buildfarm test module for
it.

It's not run by the global "check" or "installcheck" targets, because the
temporary installation it creates accepts TCP connections from any user
the same host, which is insecure.

The buildfarm client may as well ignore that security hazard, because today's
buildfarm client starts likewise-exposed postmasters directly.

We could just implement SSL over unix sockets. Obviously the
connection-encryption aspect isn't actually useful, but e.g. client
certs still make sense. Besides, it allows to avoid concerns like the
above...

Agreed.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#10Peter Eisentraut
peter_e@gmx.net
In reply to: Andres Freund (#7)
Re: src/test/ssl broken on HEAD

On 9/2/15 7:15 PM, Andres Freund wrote:

Add a regression test suite for SSL support.

It's not run by the global "check" or "installcheck" targets, because the
temporary installation it creates accepts TCP connections from any user
the same host, which is insecure.

We could just implement SSL over unix sockets. Obviously the
connection-encryption aspect isn't actually useful, but e.g. client
certs still make sense. Besides, it allows to avoid concerns like the
above...

See old discussion here:
/messages/by-id/49CA2524.5010809@gmx.net

At the time, we didn't have this test suite, obviously, so the utility
would be have been limited, but now it looks quite interesting.

The only trick, as I remember, was that clients tend to prefer SSL
automatically, which we probably don't want for Unix-domain sockets, so
we'd need to tweak those settings a bit.

The "old patch" referred to in that old thread wasn't actually attached,
so here it is, for amusement.

Attachments:

ssl-unix-sockets.patchapplication/x-patch; name=ssl-unix-sockets.patchDownload+2-7
#11Michael Paquier
michael@paquier.xyz
In reply to: Peter Eisentraut (#10)
Re: src/test/ssl broken on HEAD

On Tue, Sep 15, 2015 at 5:23 PM, Peter Eisentraut wrote:

The only trick, as I remember, was that clients tend to prefer SSL
automatically, which we probably don't want for Unix-domain sockets, so
we'd need to tweak those settings a bit.

The "old patch" referred to in that old thread wasn't actually attached,
so here it is, for amusement.

Wouldn't this need as well a separate configuration value in
pg_hba.conf? Let's say localssl.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers