On Tue, Mar 30, 2021 at 12:15:07PM -0300, Alvaro Herrera wrote:
> The only complain I have is that "the given node" is nonsensical in
> PostgresNode.  I suggest to delete the word "given".  Also "This is
> expected to fail with a message that matches the regular expression
> $expected_stderr".

Your suggestions are better, indeed.

> The POD doc for connect_fails uses order: ($connstr, $testname, 
> $expected_stderr)
> but the routine has:
>   +   my ($self, $connstr, $expected_stderr, $testname) = @_;
> 
> these should match.

Fixed.

> (There's quite an inconsistency in the existing test code about
> expected_stderr being a string or a regex; and some regexes are quite
> generic: just qr/SSL error/.  Not this patch responsibility to fix that.)

Jacob has just raised this as an issue for an integration with NLS,
because it may be possible that things fail with "SSL error" but a
different error pattern, causing false positives:
https://www.postgresql.org/message-id/e0f0484a1815b26bb99ef9ddc7a110dfd6425931.ca...@vmware.com

I agree that those matches should be much more picky.  We may need to
be careful across all versions of OpenSSL supported though :/

> As I understand, our perlcriticrc no longer requires 'return' at the end
> of routines (commit 0516f94d18c5), so you can omit that.

Fixed.  Thanks.

With all the comments addressed, with updates to use a single scalar
for all the connection strings and with a proper indentation, I finish
with the attached.  Does that look fine?
--
Michael
diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
index 1e96357d7e..d6e10544bb 100644
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
@@ -1511,6 +1511,11 @@ the B<timed_out> parameter is also given.
 If B<timeout> is set and this parameter is given, the scalar it references
 is set to true if the psql call times out.
 
+=item connstr => B<value>
+
+If set, use this as the connection string for the connection to the
+backend.
+
 =item replication => B<value>
 
 If set, add B<replication=value> to the conninfo string.
@@ -1550,14 +1555,20 @@ sub psql
 	my $replication       = $params{replication};
 	my $timeout           = undef;
 	my $timeout_exception = 'psql timed out';
-	my @psql_params       = (
-		'psql',
-		'-XAtq',
-		'-d',
-		$self->connstr($dbname)
-		  . (defined $replication ? " replication=$replication" : ""),
-		'-f',
-		'-');
+
+	# Build the connection string.
+	my $psql_connstr;
+	if (defined $params{connstr})
+	{
+		$psql_connstr = $params{connstr};
+	}
+	else
+	{
+		$psql_connstr = $self->connstr($dbname);
+	}
+	$psql_connstr .= defined $replication ? " replication=$replication" : "";
+
+	my @psql_params = ('psql', '-XAtq', '-d', $psql_connstr, '-f', '-');
 
 	# If the caller wants an array and hasn't passed stdout/stderr
 	# references, allocate temporary ones to capture them so we
@@ -1849,6 +1860,51 @@ sub interactive_psql
 
 =pod
 
+=item $node->connect_ok($connstr, $test_name)
+
+Attempt a connection with a custom connection string.  This is expected
+to succeed.
+
+=cut
+
+sub connect_ok
+{
+	local $Test::Builder::Level = $Test::Builder::Level + 1;
+	my ($self, $connstr, $test_name) = @_;
+	my ($ret,  $stdout,  $stderr)    = $self->psql(
+		'postgres',
+		"SELECT \$\$connected with $connstr\$\$",
+		connstr       => "$connstr",
+		on_error_stop => 0);
+
+	ok($ret == 0, $test_name);
+}
+
+=pod
+
+=item $node->connect_fails($connstr, $expected_stderr, $test_name)
+
+Attempt a connection with a custom connection string.  This is expected
+to fail with a message that matches the regular expression
+$expected_stderr.
+
+=cut
+
+sub connect_fails
+{
+	local $Test::Builder::Level = $Test::Builder::Level + 1;
+	my ($self, $connstr, $expected_stderr, $test_name) = @_;
+	my ($ret, $stdout, $stderr) = $self->psql(
+		'postgres',
+		"SELECT \$\$connected with $connstr\$\$",
+		connstr => "$connstr");
+
+	ok($ret != 0, $test_name);
+	like($stderr, $expected_stderr, "$test_name: matches");
+}
+
+=pod
+
 =item $node->poll_query_until($dbname, $query [, $expected ])
 
 Run B<$query> repeatedly, until it returns the B<$expected> result
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index adaa1b4e9b..b1a63f279c 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -135,103 +135,94 @@ $common_connstr =
   "user=ssltestuser dbname=trustdb sslcert=invalid hostaddr=$SERVERHOSTADDR host=common-name.pg-ssltest.test";
 
 # The server should not accept non-SSL connections.
-test_connect_fails(
-	$common_connstr, "sslmode=disable",
+$node->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
 # or verify-full mode it should fail.
-test_connect_ok(
-	$common_connstr,
-	"sslrootcert=invalid sslmode=require",
+$node->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",
+$node->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",
+$node->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");
+$node->connect_fails(
+	"$common_connstr sslrootcert=ssl/client_ca.crt sslmode=require",
+	qr/SSL error/,
+	"connect with wrong server root cert sslmode=require");
+$node->connect_fails(
+	"$common_connstr sslrootcert=ssl/client_ca.crt sslmode=verify-ca",
+	qr/SSL error/,
+	"connect with wrong server root cert sslmode=verify-ca");
+$node->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");
+$node->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.
-test_connect_ok(
-	$common_connstr,
-	"sslrootcert=ssl/root+server_ca.crt sslmode=require",
+$node->connect_ok(
+	"$common_connstr 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",
+$node->connect_ok(
+	"$common_connstr 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",
+$node->connect_ok(
+	"$common_connstr 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",
+$node->connect_ok(
+	"$common_connstr 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",
+$node->connect_ok(
+	"$common_connstr sslrootcert=ssl/both-cas-2.crt sslmode=verify-ca",
 	"cert root file that contains two certificates, order 2");
 
 # 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",
+$node->connect_ok(
+	"$common_connstr 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",
+$node->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");
 
 # The same for CRL directory
-test_connect_fails(
-	$common_connstr,
-	"sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca sslcrldir=ssl/client-crldir",
+$node->connect_fails(
+	"$common_connstr sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca sslcrldir=ssl/client-crldir",
 	qr/SSL error/,
 	"directory 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",
+$node->connect_ok(
+	"$common_connstr sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca sslcrl=ssl/root+server.crl",
 	"CRL with a non-revoked cert");
 
 # The same for CRL directory
-test_connect_ok(
-	$common_connstr,
-	"sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca sslcrldir=ssl/root+server-crldir",
+$node->connect_ok(
+	"$common_connstr sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca sslcrldir=ssl/root+server-crldir",
 	"directory CRL with a non-revoked cert");
 
 # Check that connecting with verify-full fails, when the hostname doesn't
@@ -239,17 +230,13 @@ test_connect_ok(
 $common_connstr =
   "user=ssltestuser dbname=trustdb sslcert=invalid sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR";
 
-test_connect_ok(
-	$common_connstr,
-	"sslmode=require host=wronghost.test",
+$node->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",
+$node->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",
+$node->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");
 
@@ -259,27 +246,21 @@ switch_server_cert($node, 'server-multiple-alt-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",
+$node->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",
+$node->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",
+$node->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",
+$node->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",
+$node->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");
 
@@ -290,19 +271,16 @@ switch_server_cert($node, 'server-single-alt-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",
+$node->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",
+$node->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",
+$node->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"
 );
@@ -314,17 +292,12 @@ switch_server_cert($node, 'server-cn-and-alt-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",
+$node->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",
+$node->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",
+$node->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");
 
@@ -334,13 +307,12 @@ switch_server_cert($node, 'server-no-names');
 $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",
+$node->connect_ok(
+	"$common_connstr 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",
+$node->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");
 
@@ -351,18 +323,15 @@ $common_connstr =
   "user=ssltestuser dbname=trustdb sslcert=invalid hostaddr=$SERVERHOSTADDR host=common-name.pg-ssltest.test";
 
 # Without the CRL, succeeds. With it, fails.
-test_connect_ok(
-	$common_connstr,
-	"sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca",
+$node->connect_ok(
+	"$common_connstr 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",
+$node->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 file");
-test_connect_fails(
-	$common_connstr,
-	"sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca sslcrldir=ssl/root+server-crldir",
+$node->connect_fails(
+	"$common_connstr sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca sslcrldir=ssl/root+server-crldir",
 	qr/SSL error/,
 	"does not connect with client-side CRL directory");
 
@@ -381,23 +350,19 @@ command_like(
 	'pg_stat_ssl view without client certificate');
 
 # Test min/max SSL protocol versions.
-test_connect_ok(
-	$common_connstr,
-	"sslrootcert=ssl/root+server_ca.crt sslmode=require ssl_min_protocol_version=TLSv1.2 ssl_max_protocol_version=TLSv1.2",
+$node->connect_ok(
+	"$common_connstr sslrootcert=ssl/root+server_ca.crt sslmode=require ssl_min_protocol_version=TLSv1.2 ssl_max_protocol_version=TLSv1.2",
 	"connection success with correct range of TLS protocol versions");
-test_connect_fails(
-	$common_connstr,
-	"sslrootcert=ssl/root+server_ca.crt sslmode=require ssl_min_protocol_version=TLSv1.2 ssl_max_protocol_version=TLSv1.1",
+$node->connect_fails(
+	"$common_connstr sslrootcert=ssl/root+server_ca.crt sslmode=require ssl_min_protocol_version=TLSv1.2 ssl_max_protocol_version=TLSv1.1",
 	qr/invalid SSL protocol version range/,
 	"connection failure with incorrect range of TLS protocol versions");
-test_connect_fails(
-	$common_connstr,
-	"sslrootcert=ssl/root+server_ca.crt sslmode=require ssl_min_protocol_version=incorrect_tls",
+$node->connect_fails(
+	"$common_connstr sslrootcert=ssl/root+server_ca.crt sslmode=require ssl_min_protocol_version=incorrect_tls",
 	qr/invalid ssl_min_protocol_version value/,
 	"connection failure with an incorrect SSL protocol minimum bound");
-test_connect_fails(
-	$common_connstr,
-	"sslrootcert=ssl/root+server_ca.crt sslmode=require ssl_max_protocol_version=incorrect_tls",
+$node->connect_fails(
+	"$common_connstr sslrootcert=ssl/root+server_ca.crt sslmode=require ssl_max_protocol_version=incorrect_tls",
 	qr/invalid ssl_max_protocol_version value/,
 	"connection failure with an incorrect SSL protocol maximum bound");
 
@@ -411,44 +376,38 @@ $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",
+$node->connect_fails(
+	"$common_connstr user=ssltestuser sslcert=invalid",
 	qr/connection requires a valid client certificate/,
 	"certificate authorization fails without client cert");
 
 # correct client cert in unencrypted PEM
-test_connect_ok(
-	$common_connstr,
-	"user=ssltestuser sslcert=ssl/client.crt sslkey=ssl/client_tmp.key",
+$node->connect_ok(
+	"$common_connstr user=ssltestuser sslcert=ssl/client.crt sslkey=ssl/client_tmp.key",
 	"certificate authorization succeeds with correct client cert in PEM format"
 );
 
 # correct client cert in unencrypted DER
-test_connect_ok(
-	$common_connstr,
-	"user=ssltestuser sslcert=ssl/client.crt sslkey=ssl/client-der_tmp.key",
+$node->connect_ok(
+	"$common_connstr user=ssltestuser sslcert=ssl/client.crt sslkey=ssl/client-der_tmp.key",
 	"certificate authorization succeeds with correct client cert in DER format"
 );
 
 # correct client cert in encrypted PEM
-test_connect_ok(
-	$common_connstr,
-	"user=ssltestuser sslcert=ssl/client.crt sslkey=ssl/client-encrypted-pem_tmp.key sslpassword='dUmmyP^#+'",
+$node->connect_ok(
+	"$common_connstr user=ssltestuser sslcert=ssl/client.crt sslkey=ssl/client-encrypted-pem_tmp.key sslpassword='dUmmyP^#+'",
 	"certificate authorization succeeds with correct client cert in encrypted PEM format"
 );
 
 # correct client cert in encrypted DER
-test_connect_ok(
-	$common_connstr,
-	"user=ssltestuser sslcert=ssl/client.crt sslkey=ssl/client-encrypted-der_tmp.key sslpassword='dUmmyP^#+'",
+$node->connect_ok(
+	"$common_connstr user=ssltestuser sslcert=ssl/client.crt sslkey=ssl/client-encrypted-der_tmp.key sslpassword='dUmmyP^#+'",
 	"certificate authorization succeeds with correct client cert in encrypted DER format"
 );
 
 # correct client cert in encrypted PEM with wrong password
-test_connect_fails(
-	$common_connstr,
-	"user=ssltestuser sslcert=ssl/client.crt sslkey=ssl/client-encrypted-pem_tmp.key sslpassword='wrong'",
+$node->connect_fails(
+	"$common_connstr user=ssltestuser sslcert=ssl/client.crt sslkey=ssl/client-encrypted-pem_tmp.key sslpassword='wrong'",
 	qr!\Qprivate key file "ssl/client-encrypted-pem_tmp.key": bad decrypt\E!,
 	"certificate authorization fails with correct client cert and wrong password in encrypted PEM format"
 );
@@ -457,29 +416,23 @@ test_connect_fails(
 # correct client cert using whole DN
 my $dn_connstr = "$common_connstr dbname=certdb_dn";
 
-test_connect_ok(
-	$dn_connstr,
-	"user=ssltestuser sslcert=ssl/client-dn.crt sslkey=ssl/client-dn_tmp.key",
-	"certificate authorization succeeds with DN mapping"
-);
+$node->connect_ok(
+	"$dn_connstr user=ssltestuser sslcert=ssl/client-dn.crt sslkey=ssl/client-dn_tmp.key",
+	"certificate authorization succeeds with DN mapping");
 
 # same thing but with a regex
 $dn_connstr = "$common_connstr dbname=certdb_dn_re";
 
-test_connect_ok(
-	$dn_connstr,
-	"user=ssltestuser sslcert=ssl/client-dn.crt sslkey=ssl/client-dn_tmp.key",
-	"certificate authorization succeeds with DN regex mapping"
-);
+$node->connect_ok(
+	"$dn_connstr user=ssltestuser sslcert=ssl/client-dn.crt sslkey=ssl/client-dn_tmp.key",
+	"certificate authorization succeeds with DN regex mapping");
 
 # same thing but using explicit CN
 $dn_connstr = "$common_connstr dbname=certdb_cn";
 
-test_connect_ok(
-	$dn_connstr,
-	"user=ssltestuser sslcert=ssl/client-dn.crt sslkey=ssl/client-dn_tmp.key",
-	"certificate authorization succeeds with CN mapping"
-);
+$node->connect_ok(
+	"$dn_connstr user=ssltestuser sslcert=ssl/client-dn.crt sslkey=ssl/client-dn_tmp.key",
+	"certificate authorization succeeds with CN mapping");
 
 
 
@@ -491,17 +444,15 @@ TODO:
 	todo_skip "Need Pty support", 4;
 
 	# correct client cert in encrypted PEM with empty password
-	test_connect_fails(
-		$common_connstr,
-		"user=ssltestuser sslcert=ssl/client.crt sslkey=ssl/client-encrypted-pem_tmp.key sslpassword=''",
+	$node->connect_fails(
+		"$common_connstr user=ssltestuser sslcert=ssl/client.crt sslkey=ssl/client-encrypted-pem_tmp.key sslpassword=''",
 		qr!\Qprivate key file "ssl/client-encrypted-pem_tmp.key": processing error\E!,
 		"certificate authorization fails with correct client cert and empty password in encrypted PEM format"
 	);
 
 	# correct client cert in encrypted PEM with no password
-	test_connect_fails(
-		$common_connstr,
-		"user=ssltestuser sslcert=ssl/client.crt sslkey=ssl/client-encrypted-pem_tmp.key",
+	$node->connect_fails(
+		"$common_connstr user=ssltestuser sslcert=ssl/client.crt sslkey=ssl/client-encrypted-pem_tmp.key",
 		qr!\Qprivate key file "ssl/client-encrypted-pem_tmp.key": processing error\E!,
 		"certificate authorization fails with correct client cert and no password in encrypted PEM format"
 	);
@@ -532,25 +483,22 @@ SKIP:
 {
 	skip "Permissions check not enforced on Windows", 2 if ($windows_os);
 
-	test_connect_fails(
-		$common_connstr,
-		"user=ssltestuser sslcert=ssl/client.crt sslkey=ssl/client_wrongperms_tmp.key",
+	$node->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",
+$node->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_tmp.key",
+$node->connect_fails(
+	"$common_connstr user=ssltestuser sslcert=ssl/client-revoked.crt sslkey=ssl/client-revoked_tmp.key",
 	qr/SSL error/,
 	"certificate authorization fails with revoked client cert");
 
@@ -560,24 +508,21 @@ test_connect_fails(
 $common_connstr =
   "sslrootcert=ssl/root+server_ca.crt sslmode=require dbname=verifydb hostaddr=$SERVERHOSTADDR";
 
-test_connect_ok(
-	$common_connstr,
-	"user=ssltestuser sslcert=ssl/client.crt sslkey=ssl/client_tmp.key",
+$node->connect_ok(
+	"$common_connstr user=ssltestuser sslcert=ssl/client.crt sslkey=ssl/client_tmp.key",
 	"auth_option clientcert=verify-full succeeds with matching username and Common Name"
 );
 
-test_connect_fails(
-	$common_connstr,
-	"user=anotheruser sslcert=ssl/client.crt sslkey=ssl/client_tmp.key",
+$node->connect_fails(
+	"$common_connstr user=anotheruser sslcert=ssl/client.crt sslkey=ssl/client_tmp.key",
 	qr/FATAL/,
 	"auth_option clientcert=verify-full fails with mismatching username and Common Name"
 );
 
 # Check that connecting with auth-optionverify-ca in pg_hba :
 # works, when username doesn't match Common Name
-test_connect_ok(
-	$common_connstr,
-	"user=yetanotheruser sslcert=ssl/client.crt sslkey=ssl/client_tmp.key",
+$node->connect_ok(
+	"$common_connstr user=yetanotheruser sslcert=ssl/client.crt sslkey=ssl/client_tmp.key",
 	"auth_option clientcert=verify-ca succeeds with mismatching username and Common Name"
 );
 
@@ -586,20 +531,19 @@ switch_server_cert($node, 'server-cn-only', 'root_ca');
 $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",
+$node->connect_ok(
+	"$common_connstr 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",
+$node->connect_fails(
+	$common_connstr . " " . "sslmode=require sslcert=ssl/client.crt",
 	qr/SSL error/, "intermediate client certificate is missing");
 
 # test server-side CRL directory
 switch_server_cert($node, 'server-cn-only', undef, undef, 'root+client-crldir');
 
 # revoked client cert
-test_connect_fails(
-	$common_connstr,
-	"user=ssltestuser sslcert=ssl/client-revoked.crt sslkey=ssl/client-revoked_tmp.key",
+$node->connect_fails(
+	"$common_connstr user=ssltestuser sslcert=ssl/client-revoked.crt sslkey=ssl/client-revoked_tmp.key",
 	qr/SSL error/,
 	"certificate authorization fails with revoked client cert with server-side CRL directory");
 
diff --git a/src/test/ssl/t/002_scram.pl b/src/test/ssl/t/002_scram.pl
index 410b9e910d..e31650b931 100644
--- a/src/test/ssl/t/002_scram.pl
+++ b/src/test/ssl/t/002_scram.pl
@@ -53,39 +53,34 @@ $common_connstr =
   "dbname=trustdb sslmode=require sslcert=invalid sslrootcert=invalid hostaddr=$SERVERHOSTADDR";
 
 # Default settings
-test_connect_ok($common_connstr, "user=ssltestuser",
+$node->connect_ok(
+	"$common_connstr user=ssltestuser",
 	"Basic SCRAM authentication with SSL");
 
 # Test channel_binding
-test_connect_fails(
-	$common_connstr,
-	"user=ssltestuser channel_binding=invalid_value",
+$node->connect_fails(
+	"$common_connstr user=ssltestuser channel_binding=invalid_value",
 	qr/invalid channel_binding value: "invalid_value"/,
 	"SCRAM with SSL and channel_binding=invalid_value");
-test_connect_ok(
-	$common_connstr,
-	"user=ssltestuser channel_binding=disable",
+$node->connect_ok("$common_connstr user=ssltestuser channel_binding=disable",
 	"SCRAM with SSL and channel_binding=disable");
 if ($supports_tls_server_end_point)
 {
-	test_connect_ok(
-		$common_connstr,
-		"user=ssltestuser channel_binding=require",
+	$node->connect_ok(
+		"$common_connstr user=ssltestuser channel_binding=require",
 		"SCRAM with SSL and channel_binding=require");
 }
 else
 {
-	test_connect_fails(
-		$common_connstr,
-		"user=ssltestuser channel_binding=require",
+	$node->connect_fails(
+		"$common_connstr user=ssltestuser channel_binding=require",
 		qr/channel binding is required, but server did not offer an authentication method that supports channel binding/,
 		"SCRAM with SSL and channel_binding=require");
 }
 
 # Now test when the user has an MD5-encrypted password; should fail
-test_connect_fails(
-	$common_connstr,
-	"user=md5testuser channel_binding=require",
+$node->connect_fails(
+	"$common_connstr user=md5testuser channel_binding=require",
 	qr/channel binding required but not supported by server's authentication request/,
 	"MD5 with SSL and channel_binding=require");
 
@@ -96,9 +91,8 @@ test_connect_fails(
 my $client_tmp_key = "ssl/client_scram_tmp.key";
 copy("ssl/client.key", $client_tmp_key);
 chmod 0600, $client_tmp_key;
-test_connect_fails(
-	"sslcert=ssl/client.crt sslkey=$client_tmp_key sslrootcert=invalid hostaddr=$SERVERHOSTADDR",
-	"dbname=certdb user=ssltestuser channel_binding=require",
+$node->connect_fails(
+	"sslcert=ssl/client.crt sslkey=$client_tmp_key sslrootcert=invalid hostaddr=$SERVERHOSTADDR dbname=certdb user=ssltestuser channel_binding=require",
 	qr/channel binding required, but server authenticated client without channel binding/,
 	"Cert authentication and channel_binding=require");
 
diff --git a/src/test/ssl/t/SSLServer.pm b/src/test/ssl/t/SSLServer.pm
index c494c1ad1c..129a7154a9 100644
--- a/src/test/ssl/t/SSLServer.pm
+++ b/src/test/ssl/t/SSLServer.pm
@@ -37,46 +37,8 @@ use Exporter 'import';
 our @EXPORT = qw(
   configure_test_server_for_ssl
   switch_server_cert
-  test_connect_fails
-  test_connect_ok
 );
 
-# Define a couple of helper functions to test connecting to the server.
-
-# The first argument is a base connection string to use for connection.
-# The second argument is a complementary connection string.
-sub test_connect_ok
-{
-	local $Test::Builder::Level = $Test::Builder::Level + 1;
-
-	my ($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);
-	return;
-}
-
-sub test_connect_fails
-{
-	local $Test::Builder::Level = $Test::Builder::Level + 1;
-
-	my ($common_connstr, $connstr, $expected_stderr, $test_name) = @_;
-
-	my $cmd = [
-		'psql', '-X', '-A', '-t', '-c',
-		"SELECT \$\$connected with $connstr\$\$",
-		'-d', "$common_connstr $connstr"
-	];
-
-	command_fails_like($cmd, $expected_stderr, $test_name);
-	return;
-}
-
 # Copy a set of files, taking into account wildcards
 sub copy_files
 {

Attachment: signature.asc
Description: PGP signature

Reply via email to