check error messages in SSL tests

On Thu, Feb 22, 2018 at 08:34:30AM -0500, Peter Eisentraut wrote:

I noticed that a couple of test cases in the SSL tests fail to connect
not for the reason that the tests think they should. Here is a patch to
augment the test setup so that a test for connection rejection also
checks that we get the expected error message.

+1 for the idea. And good catches.

One of the tests is failing:
t/001_ssltests.pl .. 1/62
# Failed test 'certificate authorization fails with revoked client cert: matches'
# at /home/XXXX/git/postgres/src/test/ssl/../../../src/test/perl/TestLib.pm line 354.
# 'psql: private key file "ssl/client-revoked.key" has
group or world access; permissions should be u=rw (0600) or less
# '
# doesn't match '(?^:SSL error)'
# Looks like you failed 1 test of 62.
t/001_ssltests.pl .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/62 subtests

This comes from libpq itself, and the tree uses 0644 on this file. You
just need to update this test so as ssl/client-revoked_tmp.key is used
instead of ssl/client-revoked.key, and then the tests pass.
--
Michael

On 2/22/18 23:58, Michael Paquier wrote:

One of the tests is failing:
t/001_ssltests.pl .. 1/62
# Failed test 'certificate authorization fails with revoked client cert: matches'
# at /home/XXXX/git/postgres/src/test/ssl/../../../src/test/perl/TestLib.pm line 354.
# 'psql: private key file "ssl/client-revoked.key" has
group or world access; permissions should be u=rw (0600) or less
# '
# doesn't match '(?^:SSL error)'
# Looks like you failed 1 test of 62.
t/001_ssltests.pl .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/62 subtests

This comes from libpq itself, and the tree uses 0644 on this file. You
just need to update this test so as ssl/client-revoked_tmp.key is used
instead of ssl/client-revoked.key, and then the tests pass.

Oh. I actually had that file as 0600 in my checked-out tree, probably
from earlier experiments. Fixed that. And I also changed it to make
another temp file that is explicitly 0644, because we can't rely on that
being the default either.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Fri, Feb 23, 2018 at 01:57:44PM -0500, Peter Eisentraut wrote:

Oh. I actually had that file as 0600 in my checked-out tree, probably
from earlier experiments. Fixed that. And I also changed it to make
another temp file that is explicitly 0644, because we can't rely on that
being the default either.

Thanks for fixing the first one. I also like the second change. This
patch looks fine to me.
--
Michael

On 2/24/18 07:37, Michael Paquier wrote:

On Fri, Feb 23, 2018 at 01:57:44PM -0500, Peter Eisentraut wrote:

Oh. I actually had that file as 0600 in my checked-out tree, probably
from earlier experiments. Fixed that. And I also changed it to make
another temp file that is explicitly 0644, because we can't rely on that
being the default either.

Thanks for fixing the first one. I also like the second change. This
patch looks fine to me.

committed

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 2/24/18 10:12, Peter Eisentraut wrote:

On 2/24/18 07:37, Michael Paquier wrote:

On Fri, Feb 23, 2018 at 01:57:44PM -0500, Peter Eisentraut wrote:

Oh. I actually had that file as 0600 in my checked-out tree, probably
from earlier experiments. Fixed that. And I also changed it to make
another temp file that is explicitly 0644, because we can't rely on that
being the default either.

Thanks for fixing the first one. I also like the second change. This
patch looks fine to me.

committed

This contains a slight problem: The tests contain two different
branches, depending on whether tls-server-end-point is supported. But
these branches run a different number of tests, so the test count of the
top of the test file might be wrong. Here is a patch that restructures
this to count the tests more dynamically.

Thoughts?

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Tue, Mar 06, 2018 at 02:54:16PM -0500, Peter Eisentraut wrote:

This contains a slight problem: The tests contain two different
branches, depending on whether tls-server-end-point is supported. But
these branches run a different number of tests, so the test count of the
top of the test file might be wrong. Here is a patch that restructures
this to count the tests more dynamically.

Not sure how I missed that. The error is obvious as command_ok triggers
one test and command_fails_like does two of them. Test::More also
documents what you are using here, so the patch looks fine to me.
--
Michael