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
>From cdcd1a6d4ed835610c7577f21955268ee90cc9a9 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Fri, 23 Feb 2018 13:54:45 -0500
Subject: [PATCH v2] Check error messages in SSL tests
In tests that check whether a connection fails, also check the error
message. That makes sure that the connection was rejected for the right
reason.
This discovered that two tests had their connection failing for the
wrong reason. One test failed because pg_hba.conf was not set up to
allow that user, one test failed because the client key file did not
have the right permissions. Fix those tests and add a new one that is
really supposed to check the file permission issue.
---
src/test/ssl/ServerSetup.pm | 42 ++++++++++++++++-------------------------
src/test/ssl/ssl/.gitignore | 2 +-
src/test/ssl/t/001_ssltests.pl | 43 ++++++++++++++++++++++++++++++++++++++----
src/test/ssl/t/002_scram.pl | 4 +++-
4 files changed, 59 insertions(+), 32 deletions(-)
diff --git a/src/test/ssl/ServerSetup.pm b/src/test/ssl/ServerSetup.pm
index 45991d61a2..27a676b65c 100644
--- a/src/test/ssl/ServerSetup.pm
+++ b/src/test/ssl/ServerSetup.pm
@@ -27,7 +27,6 @@ use Test::More;
use Exporter 'import';
our @EXPORT = qw(
configure_test_server_for_ssl
- run_test_psql
switch_server_cert
test_connect_fails
test_connect_ok
@@ -35,37 +34,28 @@ our @EXPORT = qw(
# Define a couple of helper functions to test connecting to the server.
-# Attempt connection to server with given connection string.
-sub run_test_psql
-{
- my $connstr = $_[0];
-
- my $cmd = [
- 'psql', '-X', '-A', '-t', '-c', "SELECT \$\$connected with
$connstr\$\$",
- '-d', "$connstr" ];
-
- my $result = run_log($cmd);
- return $result;
-}
-
# The first argument is a base connection string to use for connection.
# The second argument is a complementary connection string.
sub test_connect_ok
{
- my $common_connstr = $_[0];
- my $connstr = $_[1];
- my $test_name = $_[2];
+ my ($common_connstr, $connstr, $test_name) = @_;
- ok(run_test_psql("$common_connstr $connstr"), $test_name);
+ my $cmd = [
+ 'psql', '-X', '-A', '-t', '-c', "SELECT \$\$connected with
$connstr\$\$",
+ '-d', "$common_connstr $connstr" ];
+
+ command_ok($cmd, $test_name);
}
sub test_connect_fails
{
- my $common_connstr = $_[0];
- my $connstr = $_[1];
- my $test_name = $_[2];
+ my ($common_connstr, $connstr, $expected_stderr, $test_name) = @_;
+
+ my $cmd = [
+ 'psql', '-X', '-A', '-t', '-c', "SELECT \$\$connected with
$connstr\$\$",
+ '-d', "$common_connstr $connstr" ];
- ok(!run_test_psql("$common_connstr $connstr"), $test_name);
+ command_fails_like($cmd, $expected_stderr, $test_name);
}
# Copy a set of files, taking into account wildcards
@@ -169,12 +159,12 @@ sub configure_hba_for_ssl
print $hba
"# TYPE DATABASE USER ADDRESS METHOD\n";
print $hba
-"hostssl trustdb ssltestuser $serverhost/32
$authmethod\n";
+"hostssl trustdb all $serverhost/32
$authmethod\n";
print $hba
-"hostssl trustdb ssltestuser ::1/128
$authmethod\n";
+"hostssl trustdb all ::1/128
$authmethod\n";
print $hba
-"hostssl certdb ssltestuser $serverhost/32 cert\n";
+"hostssl certdb all $serverhost/32 cert\n";
print $hba
-"hostssl certdb ssltestuser ::1/128 cert\n";
+"hostssl certdb all ::1/128 cert\n";
close $hba;
}
diff --git a/src/test/ssl/ssl/.gitignore b/src/test/ssl/ssl/.gitignore
index 10b74f0848..af753d4c7d 100644
--- a/src/test/ssl/ssl/.gitignore
+++ b/src/test/ssl/ssl/.gitignore
@@ -1,3 +1,3 @@
/*.old
/new_certs_dir/
-/client_tmp.key
+/client*_tmp.key
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index e53bd12ae9..4b097a69bf 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -2,7 +2,7 @@
use warnings;
use PostgresNode;
use TestLib;
-use Test::More tests => 40;
+use Test::More tests => 62;
use ServerSetup;
use File::Copy;
@@ -20,6 +20,14 @@
# 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";
+copy("ssl/client-revoked.key", "ssl/client-revoked_tmp.key");
+chmod 0600, "ssl/client-revoked_tmp.key";
+
+# Also make a copy of that explicitly world-readable. We can't
+# necessarily rely on the file in the source tree having those
+# permissions.
+copy("ssl/client.key", "ssl/client_wrongperms_tmp.key");
+chmod 0644, "ssl/client_wrongperms_tmp.key";
#### Part 0. Set up the server.
@@ -48,6 +56,7 @@
# The server should not accept non-SSL connections.
test_connect_fails($common_connstr, "sslmode=disable",
+ qr/\Qno pg_hba.conf entry\E/,
"server doesn't accept non-SSL connections");
# Try without a root cert. In sslmode=require, this should work. In verify-ca
@@ -55,26 +64,32 @@
test_connect_ok($common_connstr, "sslrootcert=invalid sslmode=require",
"connect without server root cert
sslmode=require");
test_connect_fails($common_connstr, "sslrootcert=invalid sslmode=verify-ca",
+ qr/root certificate file "invalid" does not
exist/,
"connect without server root cert
sslmode=verify-ca");
test_connect_fails($common_connstr, "sslrootcert=invalid sslmode=verify-full",
+ qr/root certificate file "invalid" does not
exist/,
"connect without server root cert
sslmode=verify-full");
# Try with wrong root cert, should fail. (We're using the client CA as the
# root, but the server's key is signed by the server CA.)
test_connect_fails($common_connstr,
"sslrootcert=ssl/client_ca.crt
sslmode=require",
+ qr/SSL error/,
"connect with wrong server root cert
sslmode=require");
test_connect_fails($common_connstr,
"sslrootcert=ssl/client_ca.crt
sslmode=verify-ca",
+ qr/SSL error/,
"connect with wrong server root cert
sslmode=verify-ca");
test_connect_fails($common_connstr,
"sslrootcert=ssl/client_ca.crt
sslmode=verify-full",
+ qr/SSL error/,
"connect with wrong server root cert
sslmode=verify-full");
# Try with just the server CA's cert. This fails because the root file
# must contain the whole chain up to the root CA.
test_connect_fails($common_connstr,
"sslrootcert=ssl/server_ca.crt
sslmode=verify-ca",
+ qr/SSL error/,
"connect with server CA cert, without root
CA");
# And finally, with the correct root cert.
@@ -107,6 +122,7 @@
# A CRL belonging to a different CA is not accepted, fails
test_connect_fails($common_connstr,
"sslrootcert=ssl/root+server_ca.crt
sslmode=verify-ca sslcrl=ssl/client.crl",
+ qr/SSL error/,
"CRL belonging to a different CA");
# With the correct CRL, succeeds (this cert is not revoked)
@@ -124,9 +140,9 @@
test_connect_ok($common_connstr, "sslmode=verify-ca host=wronghost.test",
"mismatch between host name and server
certificate sslmode=verify-ca");
test_connect_fails($common_connstr, "sslmode=verify-full host=wronghost.test",
+ qr/\Qserver certificate for
"common-name.pg-ssltest.test" does not match host name "wronghost.test"\E/,
"mismatch between host name and server
certificate sslmode=verify-full");
-
# Test Subject Alternative Names.
switch_server_cert($node, 'server-multiple-alt-names');
@@ -141,9 +157,11 @@
"host name matching with X.509 Subject
Alternative Names wildcard");
test_connect_fails($common_connstr, "host=wronghost.alt-name.pg-ssltest.test",
+ qr/\Qserver certificate for
"dns1.alt-name.pg-ssltest.test" (and 2 other names) does not match host name
"wronghost.alt-name.pg-ssltest.test"\E/,
"host name not matching with X.509 Subject
Alternative Names");
test_connect_fails($common_connstr,
"host=deep.subdomain.wildcard.pg-ssltest.test",
+ qr/\Qserver certificate for
"dns1.alt-name.pg-ssltest.test" (and 2 other names) does not match host name
"deep.subdomain.wildcard.pg-ssltest.test"\E/,
"host name not matching with X.509 Subject
Alternative Names wildcard");
# Test certificate with a single Subject Alternative Name. (this gives a
@@ -157,9 +175,11 @@
"host name matching with a single X.509 Subject
Alternative Name");
test_connect_fails($common_connstr, "host=wronghost.alt-name.pg-ssltest.test",
+ qr/\Qserver certificate for
"single.alt-name.pg-ssltest.test" does not match host name
"wronghost.alt-name.pg-ssltest.test"\E/,
"host name not matching with a single X.509
Subject Alternative Name");
test_connect_fails($common_connstr,
"host=deep.subdomain.wildcard.pg-ssltest.test",
+ qr/\Qserver certificate for
"single.alt-name.pg-ssltest.test" does not match host name
"deep.subdomain.wildcard.pg-ssltest.test"\E/,
"host name not matching with a single X.509
Subject Alternative Name wildcard");
# Test server certificate with a CN and SANs. Per RFCs 2818 and 6125, the CN
@@ -174,6 +194,7 @@
test_connect_ok($common_connstr, "host=dns2.alt-name.pg-ssltest.test",
"certificate with both a CN and SANs 2");
test_connect_fails($common_connstr, "host=common-name.pg-ssltest.test",
+ qr/\Qserver certificate for
"dns1.alt-name.pg-ssltest.test" (and 1 other name) does not match host name
"common-name.pg-ssltest.test"\E/,
"certificate with both a CN and SANs ignores
CN");
# Finally, test a server certificate that has no CN or SANs. Of course, that's
@@ -187,6 +208,7 @@
"server certificate without CN or SANs
sslmode=verify-ca");
test_connect_fails($common_connstr,
"sslmode=verify-full
host=common-name.pg-ssltest.test",
+ qr/could not get server's host name from
server certificate/,
"server certificate without CN or SANs
sslmode=verify-full");
# Test that the CRL works
@@ -201,6 +223,7 @@
"connects without client-side CRL");
test_connect_fails($common_connstr,
"sslrootcert=ssl/root+server_ca.crt
sslmode=verify-ca sslcrl=ssl/root+server.crl",
+ qr/SSL error/,
"does not connect with client-side CRL");
### Part 2. Server-side tests.
@@ -215,6 +238,7 @@
# no client cert
test_connect_fails($common_connstr,
"user=ssltestuser sslcert=invalid",
+ qr/connection requires a valid client
certificate/,
"certificate authorization fails without
client cert");
# correct client cert
@@ -222,14 +246,22 @@
"user=ssltestuser sslcert=ssl/client.crt
sslkey=ssl/client_tmp.key",
"certificate authorization succeeds with
correct client cert");
+# client key with wrong permissions
+test_connect_fails($common_connstr,
+ "user=ssltestuser sslcert=ssl/client.crt
sslkey=ssl/client_wrongperms_tmp.key",
+ qr!\Qprivate key file
"ssl/client_wrongperms_tmp.key" has group or world access\E!,
+ "certificate authorization fails because of
file permissions");
+
# client cert belonging to another user
test_connect_fails($common_connstr,
"user=anotheruser sslcert=ssl/client.crt
sslkey=ssl/client_tmp.key",
+ qr/certificate authentication failed for
user "anotheruser"/,
"certificate authorization fails with client
cert belonging to another user");
# revoked client cert
test_connect_fails($common_connstr,
- "user=ssltestuser
sslcert=ssl/client-revoked.crt sslkey=ssl/client-revoked.key",
+ "user=ssltestuser
sslcert=ssl/client-revoked.crt sslkey=ssl/client-revoked_tmp.key",
+ qr/SSL error/,
"certificate authorization fails with
revoked client cert");
# intermediate client_ca.crt is provided by client, and isn't in server's
ssl_ca_file
@@ -241,7 +273,10 @@
"sslmode=require
sslcert=ssl/client+client_ca.crt",
"intermediate client certificate is provided by
client");
test_connect_fails($common_connstr, "sslmode=require sslcert=ssl/client.crt",
+ qr/SSL error/,
"intermediate client certificate is
missing");
# clean up
-unlink "ssl/client_tmp.key";
+unlink("ssl/client_tmp.key",
+ "ssl/client_wrongperms_tmp.key",
+ "ssl/client-revoked_tmp.key");
diff --git a/src/test/ssl/t/002_scram.pl b/src/test/ssl/t/002_scram.pl
index 67c1409a6e..9460763a65 100644
--- a/src/test/ssl/t/002_scram.pl
+++ b/src/test/ssl/t/002_scram.pl
@@ -4,7 +4,7 @@
use warnings;
use PostgresNode;
use TestLib;
-use Test::More tests => 5;
+use Test::More tests => 6;
use ServerSetup;
use File::Copy;
@@ -59,8 +59,10 @@
{
test_connect_fails($common_connstr,
"scram_channel_binding=tls-server-end-point",
+ qr/unsupported SCRAM channel-binding
type/,
"SCRAM authentication with
tls-server-end-point as channel binding");
}
test_connect_fails($common_connstr,
"scram_channel_binding=not-exists",
+ qr/unsupported SCRAM channel-binding type/,
"SCRAM authentication with invalid channel binding");
base-commit: f724022d0ae04e687c309f99df27b7ce64d19761
--
2.16.2
