pg.ddx.io pgsql-hackers@postgresql.org mailing list archivehelp / color / mirror / Atom feed
src/test/ssl/t/001_ssltests.pl should not tromp on file permissions 3+ messages / 2 participants [nested] [flat]
Commits from this thread: 2017-05-16 91102dab44 21c2c3d75e ce80240a7d In SSL tests, don't scribble on permissions of a repo file.
* src/test/ssl/t/001_ssltests.pl should not tromp on file permissions @ 2017-05-15 22:05 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 1 reply; 3+ messages in thread From: Tom Lane @ 2017-05-15 22:05 UTC (permalink / raw) To: Heikki Linnakangas <hlinnaka@iki.fi>; +Cc: pgsql-hackers I got tripped up while building the 10beta1 tarballs by the fact that src/test/ssl/ssl/client.key had permissions 0600 in my git checkout. After a fair amount of head-scratching, I figured out that this must have been a side-effect of having run the SSL regression tests at some point in the past. I do not like test scripts that scribble on non-transient files, not even (or perhaps especially not) if they're "just" changing the permissions. We could maybe make 001_ssltests.pl save and restore the file's permissions, but I think probably a cleaner answer is to have it make a temporary copy and set the permissions on that. Thoughts? 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 ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: src/test/ssl/t/001_ssltests.pl should not tromp on file permissions @ 2017-05-16 02:18 Michael Paquier <michael.paquier@gmail.com> parent: Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 1 reply; 3+ messages in thread From: Michael Paquier @ 2017-05-16 02:18 UTC (permalink / raw) To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Heikki Linnakangas <hlinnaka@iki.fi>; pgsql-hackers On Tue, May 16, 2017 at 7:05 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > I got tripped up while building the 10beta1 tarballs by the fact > that src/test/ssl/ssl/client.key had permissions 0600 in my git > checkout. After a fair amount of head-scratching, I figured out > that this must have been a side-effect of having run the SSL regression > tests at some point in the past. I do not like test scripts that > scribble on non-transient files, not even (or perhaps especially not) > if they're "just" changing the permissions. > > We could maybe make 001_ssltests.pl save and restore the file's > permissions, but I think probably a cleaner answer is to have it > make a temporary copy and set the permissions on that. Ah, you are talking about ss/client.key here... Using a temporary copy makes the most sense to me, as there is no need to think about putting back the old permissions on test failure, and the original tree remains unmodified all the time. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers Attachments:^ permalink raw reply [nested|flat] 3+ messages in threadssl-test-perms.patch (application/octet-stream, 2.0K) download
diff --git a/src/test/ssl/ssl/.gitignore b/src/test/ssl/ssl/.gitignore index 8feb8643ff..e2a36f73a2 100644 --- a/src/test/ssl/ssl/.gitignore +++ b/src/test/ssl/ssl/.gitignore @@ -1,2 +1,3 @@ *.old new_certs_dir +client_tmp.key diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl index 0c69bf08a5..c0e563e48a 100644 --- a/src/test/ssl/t/001_ssltests.pl +++ b/src/test/ssl/t/001_ssltests.pl @@ -66,10 +66,10 @@ sub test_connect_fails ok(!$result, "$connstr (should fail)"); } -# The client's private key must not be world-readable. Git doesn't track -# permissions (except for the executable bit), so they might be wrong after -# a checkout. -chmod 0600, "ssl/client.key"; +# The client's private key must not be world-readable, so take a copy +# of the key stored in the code tree and update its permissions. +copy("ssl/client.key", "ssl/client_tmp.key"); +chmod 0600, "ssl/client_tmp.key"; #### Part 0. Set up the server. @@ -229,11 +229,11 @@ test_connect_fails("user=ssltestuser sslcert=invalid"); # correct client cert test_connect_ok( - "user=ssltestuser sslcert=ssl/client.crt sslkey=ssl/client.key"); + "user=ssltestuser sslcert=ssl/client.crt sslkey=ssl/client_tmp.key"); # client cert belonging to another user test_connect_fails( - "user=anotheruser sslcert=ssl/client.crt sslkey=ssl/client.key"); + "user=anotheruser sslcert=ssl/client.crt sslkey=ssl/client_tmp.key"); # revoked client cert test_connect_fails( @@ -243,7 +243,7 @@ test_connect_fails( # intermediate client_ca.crt is provided by client, and isn't in server's ssl_ca_file switch_server_cert($node, 'server-cn-only', 'root_ca'); $common_connstr = -"user=ssltestuser dbname=certdb sslkey=ssl/client.key sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR"; +"user=ssltestuser dbname=certdb sslkey=ssl/client_tmp.key sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR"; test_connect_ok("sslmode=require sslcert=ssl/client+client_ca.crt"); test_connect_fails("sslmode=require sslcert=ssl/client.crt");
* Re: src/test/ssl/t/001_ssltests.pl should not tromp on file permissions @ 2017-05-16 03:29 Tom Lane <tgl@sss.pgh.pa.us> parent: Michael Paquier <michael.paquier@gmail.com> 0 siblings, 0 replies; 3+ messages in thread From: Tom Lane @ 2017-05-16 03:29 UTC (permalink / raw) To: Michael Paquier <michael.paquier@gmail.com>; +Cc: Heikki Linnakangas <hlinnaka@iki.fi>; pgsql-hackers Michael Paquier <michael.paquier@gmail.com> writes: > On Tue, May 16, 2017 at 7:05 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> We could maybe make 001_ssltests.pl save and restore the file's >> permissions, but I think probably a cleaner answer is to have it >> make a temporary copy and set the permissions on that. > Ah, you are talking about ss/client.key here... Using a temporary copy > makes the most sense to me, as there is no need to think about putting > back the old permissions on test failure, and the original tree > remains unmodified all the time. Pushed with minor adjustments --- I made the test script unlink the temp file at the end, and tightened the .gitignore patterns a bit. 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 ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2017-05-16 03:29 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2017-05-15 22:05 src/test/ssl/t/001_ssltests.pl should not tromp on file permissions Tom Lane <tgl@sss.pgh.pa.us> 2017-05-16 02:18 ` Michael Paquier <michael.paquier@gmail.com> 2017-05-16 03:29 ` Tom Lane <tgl@sss.pgh.pa.us>
This inbox is served by DDX for PostgreSQL; see mirroring instructions for how to clone and mirror all data and code used for this inbox