Refine SSL tests test name reporting

Instead of using the psql/libpq connection string as the displayed test
name and relying on "notes" and source code comments to explain the
tests, give the tests self-explanatory names, like we do elsewhere.

Reviewed-by: Michael Paquier <michael.paquier@gmail.com>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>

2 files changed, 89 insertions, 68 deletions

diff --git a/src/test/ssl/ServerSetup.pm b/src/test/ssl/ServerSetup.pm
index b4d5746e208..45991d61a2a 100644
--- a/src/test/ssl/ServerSetup.pm
+++ b/src/test/ssl/ServerSetup.pm
@@ -39,7 +39,6 @@ our @EXPORT = qw(
 sub run_test_psql
 {
 	my $connstr   = $_[0];
-	my $logstring = $_[1];
 
 	my $cmd = [
 		'psql', '-X', '-A', '-t', '-c', "SELECT \$\$connected with $connstr\$\$",
@@ -49,19 +48,15 @@ sub run_test_psql
 	return $result;
 }
 
-#
 # The first argument is a base connection string to use for connection.
-# The second argument is a complementary connection string, and it's also
-# printed out as the test case name.
+# The second argument is a complementary connection string.
 sub test_connect_ok
 {
 	my $common_connstr = $_[0];
 	my $connstr = $_[1];
 	my $test_name = $_[2];
 
-	my $result =
-	  run_test_psql("$common_connstr $connstr", "(should succeed)");
-	ok($result, $test_name || $connstr);
+	ok(run_test_psql("$common_connstr $connstr"), $test_name);
 }
 
 sub test_connect_fails
@@ -70,8 +65,7 @@ sub test_connect_fails
 	my $connstr = $_[1];
 	my $test_name = $_[2];
 
-	my $result = run_test_psql("$common_connstr $connstr", "(should fail)");
-	ok(!$result, $test_name || "$connstr (should fail)");
+	ok(!run_test_psql("$common_connstr $connstr"), $test_name);
 }
 
 # Copy a set of files, taking into account wildcards
@@ -151,9 +145,6 @@ sub switch_server_cert
 	my $cafile   = $_[2] || "root+client_ca";
 	my $pgdata   = $node->data_dir;
 
-	note
-	  "reloading server with certfile \"$certfile\" and cafile \"$cafile\"";
-
 	open my $sslconf, '>', "$pgdata/sslconfig.conf";
 	print $sslconf "ssl=on\n";
 	print $sslconf "ssl_ca_file='$cafile.crt'\n";
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index 28837a13914..e53bd12ae9f 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -47,113 +47,134 @@ $common_connstr =
 "user=ssltestuser dbname=trustdb sslcert=invalid hostaddr=$SERVERHOSTADDR host=common-name.pg-ssltest.test";
 
 # The server should not accept non-SSL connections.
-note "test that the server doesn't accept non-SSL connections";
-test_connect_fails($common_connstr, "sslmode=disable");
+test_connect_fails($common_connstr, "sslmode=disable",
+				   "server doesn't accept non-SSL connections");
 
 # Try without a root cert. In sslmode=require, this should work. In verify-ca
 # or verify-full mode it should fail.
-note "connect without server root cert";
-test_connect_ok($common_connstr, "sslrootcert=invalid sslmode=require");
-test_connect_fails($common_connstr, "sslrootcert=invalid sslmode=verify-ca");
-test_connect_fails($common_connstr, "sslrootcert=invalid sslmode=verify-full");
+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",
+				"connect without server root cert sslmode=verify-ca");
+test_connect_fails($common_connstr, "sslrootcert=invalid sslmode=verify-full",
+				   "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.)
-note "connect with wrong server root cert";
 test_connect_fails($common_connstr,
-	"sslrootcert=ssl/client_ca.crt sslmode=require");
+				   "sslrootcert=ssl/client_ca.crt sslmode=require",
+				   "connect with wrong server root cert sslmode=require");
 test_connect_fails($common_connstr,
-	"sslrootcert=ssl/client_ca.crt sslmode=verify-ca");
+				   "sslrootcert=ssl/client_ca.crt sslmode=verify-ca",
+				   "connect with wrong server root cert sslmode=verify-ca");
 test_connect_fails($common_connstr,
-	"sslrootcert=ssl/client_ca.crt sslmode=verify-full");
+				   "sslrootcert=ssl/client_ca.crt sslmode=verify-full",
+				   "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.
-note "connect with server CA cert, without root CA";
 test_connect_fails($common_connstr,
-	"sslrootcert=ssl/server_ca.crt sslmode=verify-ca");
+				   "sslrootcert=ssl/server_ca.crt sslmode=verify-ca",
+				   "connect with server CA cert, without root CA");
 
 # And finally, with the correct root cert.
-note "connect with correct server CA cert file";
 test_connect_ok($common_connstr,
-	"sslrootcert=ssl/root+server_ca.crt sslmode=require");
+				"sslrootcert=ssl/root+server_ca.crt sslmode=require",
+				"connect with correct server CA cert file sslmode=require");
 test_connect_ok($common_connstr,
-	"sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca");
+				"sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca",
+				"connect with correct server CA cert file sslmode=verify-ca");
 test_connect_ok($common_connstr,
-	"sslrootcert=ssl/root+server_ca.crt sslmode=verify-full");
+				"sslrootcert=ssl/root+server_ca.crt sslmode=verify-full",
+				"connect with correct server CA cert file sslmode=verify-full");
 
 # Test with cert root file that contains two certificates. The client should
 # be able to pick the right one, regardless of the order in the file.
 test_connect_ok($common_connstr,
-	"sslrootcert=ssl/both-cas-1.crt sslmode=verify-ca");
+				"sslrootcert=ssl/both-cas-1.crt sslmode=verify-ca",
+				"cert root file that contains two certificates, order 1");
 test_connect_ok($common_connstr,
-	"sslrootcert=ssl/both-cas-2.crt sslmode=verify-ca");
+				"sslrootcert=ssl/both-cas-2.crt sslmode=verify-ca",
+				"cert root file that contains two certificates, order 2");
 
-note "testing sslcrl option with a non-revoked cert";
+# CRL tests
 
 # Invalid CRL filename is the same as no CRL, succeeds
 test_connect_ok($common_connstr,
-	"sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca sslcrl=invalid");
+				"sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca sslcrl=invalid",
+				"sslcrl option with invalid file name");
 
 # 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");
+				   "sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca sslcrl=ssl/client.crl",
+				   "CRL belonging to a different CA");
 
 # With the correct CRL, succeeds (this cert is not revoked)
 test_connect_ok($common_connstr,
-"sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca sslcrl=ssl/root+server.crl"
-);
+				"sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca sslcrl=ssl/root+server.crl",
+				"CRL with a non-revoked cert");
 
 # Check that connecting with verify-full fails, when the hostname doesn't
 # match the hostname in the server's certificate.
-note "test mismatch between hostname and server certificate";
 $common_connstr =
-"user=ssltestuser dbname=trustdb sslcert=invalid sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR sslmode=verify-full";
+"user=ssltestuser dbname=trustdb sslcert=invalid sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR";
+
+test_connect_ok($common_connstr, "sslmode=require host=wronghost.test",
+				"mismatch between host name and server certificate sslmode=require");
+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",
+				   "mismatch between host name and server certificate sslmode=verify-full");
 
-test_connect_ok($common_connstr, "sslmode=require host=wronghost.test");
-test_connect_ok($common_connstr, "sslmode=verify-ca host=wronghost.test");
-test_connect_fails($common_connstr, "sslmode=verify-full host=wronghost.test");
 
 # Test Subject Alternative Names.
 switch_server_cert($node, 'server-multiple-alt-names');
 
-note "test hostname matching with X.509 Subject Alternative Names";
 $common_connstr =
 "user=ssltestuser dbname=trustdb sslcert=invalid sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR sslmode=verify-full";
 
-test_connect_ok($common_connstr, "host=dns1.alt-name.pg-ssltest.test");
-test_connect_ok($common_connstr, "host=dns2.alt-name.pg-ssltest.test");
-test_connect_ok($common_connstr, "host=foo.wildcard.pg-ssltest.test");
+test_connect_ok($common_connstr, "host=dns1.alt-name.pg-ssltest.test",
+				"host name matching with X.509 Subject Alternative Names 1");
+test_connect_ok($common_connstr, "host=dns2.alt-name.pg-ssltest.test",
+				"host name matching with X.509 Subject Alternative Names 2");
+test_connect_ok($common_connstr, "host=foo.wildcard.pg-ssltest.test",
+				"host name matching with X.509 Subject Alternative Names wildcard");
 
-test_connect_fails($common_connstr, "host=wronghost.alt-name.pg-ssltest.test");
+test_connect_fails($common_connstr, "host=wronghost.alt-name.pg-ssltest.test",
+				   "host name not matching with X.509 Subject Alternative Names");
 test_connect_fails($common_connstr,
-	"host=deep.subdomain.wildcard.pg-ssltest.test");
+				   "host=deep.subdomain.wildcard.pg-ssltest.test",
+				   "host name not matching with X.509 Subject Alternative Names wildcard");
 
 # Test certificate with a single Subject Alternative Name. (this gives a
 # slightly different error message, that's all)
 switch_server_cert($node, 'server-single-alt-name');
 
-note "test hostname matching with a single X.509 Subject Alternative Name";
 $common_connstr =
 "user=ssltestuser dbname=trustdb sslcert=invalid sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR sslmode=verify-full";
 
-test_connect_ok($common_connstr, "host=single.alt-name.pg-ssltest.test");
+test_connect_ok($common_connstr, "host=single.alt-name.pg-ssltest.test",
+				"host name matching with a single X.509 Subject Alternative Name");
 
-test_connect_fails($common_connstr, "host=wronghost.alt-name.pg-ssltest.test");
+test_connect_fails($common_connstr, "host=wronghost.alt-name.pg-ssltest.test",
+				   "host name not matching with a single X.509 Subject Alternative Name");
 test_connect_fails($common_connstr,
-	"host=deep.subdomain.wildcard.pg-ssltest.test");
+				   "host=deep.subdomain.wildcard.pg-ssltest.test",
+				   "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
 # should be ignored when the certificate has both.
 switch_server_cert($node, 'server-cn-and-alt-names');
 
-note "test certificate with both a CN and SANs";
 $common_connstr =
 "user=ssltestuser dbname=trustdb sslcert=invalid sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR sslmode=verify-full";
 
-test_connect_ok($common_connstr, "host=dns1.alt-name.pg-ssltest.test");
-test_connect_ok($common_connstr, "host=dns2.alt-name.pg-ssltest.test");
-test_connect_fails($common_connstr, "host=common-name.pg-ssltest.test");
+test_connect_ok($common_connstr, "host=dns1.alt-name.pg-ssltest.test",
+				"certificate with both a CN and SANs 1");
+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",
+				   "certificate with both a CN and SANs ignores CN");
 
 # Finally, test a server certificate that has no CN or SANs. Of course, that's
 # not a very sensible certificate, but libpq should handle it gracefully.
@@ -162,12 +183,13 @@ $common_connstr =
 "user=ssltestuser dbname=trustdb sslcert=invalid sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR";
 
 test_connect_ok($common_connstr,
-	"sslmode=verify-ca host=common-name.pg-ssltest.test");
+				"sslmode=verify-ca host=common-name.pg-ssltest.test",
+				"server certificate without CN or SANs sslmode=verify-ca");
 test_connect_fails($common_connstr,
-	"sslmode=verify-full host=common-name.pg-ssltest.test");
+				   "sslmode=verify-full host=common-name.pg-ssltest.test",
+				   "server certificate without CN or SANs sslmode=verify-full");
 
 # Test that the CRL works
-note "testing client-side CRL";
 switch_server_cert($node, 'server-revoked');
 
 $common_connstr =
@@ -175,34 +197,40 @@ $common_connstr =
 
 # Without the CRL, succeeds. With it, fails.
 test_connect_ok($common_connstr,
-	"sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca");
+				"sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca",
+				"connects without client-side CRL");
 test_connect_fails($common_connstr,
-"sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca sslcrl=ssl/root+server.crl"
-);
+				   "sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca sslcrl=ssl/root+server.crl",
+				   "does not connect with client-side CRL");
 
 ### Part 2. Server-side tests.
 ###
 ### Test certificate authorization.
 
-note "testing certificate authorization";
+note "running server tests";
+
 $common_connstr =
 "sslrootcert=ssl/root+server_ca.crt sslmode=require dbname=certdb hostaddr=$SERVERHOSTADDR";
 
 # no client cert
-test_connect_fails($common_connstr, "user=ssltestuser sslcert=invalid");
+test_connect_fails($common_connstr,
+				   "user=ssltestuser sslcert=invalid",
+				   "certificate authorization fails without client cert");
 
 # correct client cert
 test_connect_ok($common_connstr,
-	"user=ssltestuser sslcert=ssl/client.crt sslkey=ssl/client_tmp.key");
+				"user=ssltestuser sslcert=ssl/client.crt sslkey=ssl/client_tmp.key",
+				"certificate authorization succeeds with correct client cert");
 
 # client cert belonging to another user
 test_connect_fails($common_connstr,
-	"user=anotheruser sslcert=ssl/client.crt sslkey=ssl/client_tmp.key");
+				   "user=anotheruser sslcert=ssl/client.crt sslkey=ssl/client_tmp.key",
+				   "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.key",
+				   "certificate authorization fails with revoked client cert");
 
 # 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');
@@ -210,8 +238,10 @@ $common_connstr =
 "user=ssltestuser dbname=certdb sslkey=ssl/client_tmp.key sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR";
 
 test_connect_ok($common_connstr,
-	"sslmode=require sslcert=ssl/client+client_ca.crt");
-test_connect_fails($common_connstr, "sslmode=require sslcert=ssl/client.crt");
+				"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",
+				   "intermediate client certificate is missing");
 
 # clean up
 unlink "ssl/client_tmp.key";