Thread: check error messages in SSL tests
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. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachment
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
Attachment
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
Attachment
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
Attachment
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
Attachment
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