On Thu, Mar 17, 2022 at 02:28:49PM +0100, Daniel Gustafsson wrote: > One small concern though. This hunk: > > +my $default_ssl_connstr = "sslkey=invalid sslcert=invalid > sslrootcert=invalid sslcrl=invalid sslcrldir=invalid"; > + > $common_connstr = > - "user=ssltestuser dbname=trustdb sslcert=invalid hostaddr=$SERVERHOSTADDR > host=common-name.pg-ssltest.test"; > + "$default_ssl_connstr user=ssltestuser dbname=trustdb > hostaddr=$SERVERHOSTADDR host=common-name.pg-ssltest.test"; > > ..together with the following changes along the lines of: > > - "$common_connstr sslrootcert=invalid sslmode=require", > + "$common_connstr sslmode=require", > > ..is making it fairly hard to read the test and visualize what the connection > string is and how the test should behave. I don't have a better idea off the > top of my head right now, but I think this is an area to revisit and improve > on.
I agree that this makes this set of three tests harder to follow, as we expect a root cert to *not* be set locally. Keeping the behavior documented in each individual string would be better, even if that duplicates more the keys in those final strings. Another thing that Horiguchi-san has pointed out upthread (?) is 003, where it is also possible to trigger failures once the environment is hijacked. The attached allows the full test suite to pass without issues on my side. -- Michael
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index 5c5b16fbe7..605e405de3 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -138,8 +138,13 @@ note "running client tests";
switch_server_cert($node, 'server-cn-only');
+# Set of default settings for SSL parameters in connection string. This
+# makes the tests protected against any defaults the environment may have
+# in ~/.postgresql/.
+my $default_ssl_connstr = "sslkey=invalid sslcert=invalid sslrootcert=invalid sslcrl=invalid sslcrldir=invalid";
+
$common_connstr =
- "user=ssltestuser dbname=trustdb sslcert=invalid hostaddr=$SERVERHOSTADDR host=common-name.pg-ssltest.test";
+ "$default_ssl_connstr user=ssltestuser dbname=trustdb hostaddr=$SERVERHOSTADDR host=common-name.pg-ssltest.test";
# The server should not accept non-SSL connections.
$node->connect_fails(
@@ -216,9 +221,10 @@ $node->connect_fails(
"CRL belonging to a different CA",
expected_stderr => qr/SSL error: certificate verify failed/);
-# The same for CRL directory
+# The same for CRL directory. sslcrl='' is added here to override the
+# invalid default, so as this does not interfere with this case.
$node->connect_fails(
- "$common_connstr sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca sslcrldir=ssl/client-crldir",
+ "$common_connstr sslcrl='' sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca sslcrldir=ssl/client-crldir",
"directory CRL belonging to a different CA",
expected_stderr => qr/SSL error: certificate verify failed/);
@@ -235,7 +241,7 @@ $node->connect_ok(
# Check that connecting with verify-full fails, when the hostname doesn't
# match the hostname in the server's certificate.
$common_connstr =
- "user=ssltestuser dbname=trustdb sslcert=invalid sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR";
+ "$default_ssl_connstr user=ssltestuser dbname=trustdb sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR";
$node->connect_ok("$common_connstr sslmode=require host=wronghost.test",
"mismatch between host name and server certificate sslmode=require");
@@ -253,7 +259,7 @@ $node->connect_fails(
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";
+ "$default_ssl_connstr user=ssltestuser dbname=trustdb sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR sslmode=verify-full";
$node->connect_ok(
"$common_connstr host=dns1.alt-name.pg-ssltest.test",
@@ -282,7 +288,7 @@ $node->connect_fails(
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";
+ "$default_ssl_connstr user=ssltestuser dbname=trustdb sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR sslmode=verify-full";
$node->connect_ok(
"$common_connstr host=single.alt-name.pg-ssltest.test",
@@ -306,7 +312,7 @@ $node->connect_fails(
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";
+ "$default_ssl_connstr user=ssltestuser dbname=trustdb sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR sslmode=verify-full";
$node->connect_ok("$common_connstr host=dns1.alt-name.pg-ssltest.test",
"certificate with both a CN and SANs 1");
@@ -323,7 +329,7 @@ $node->connect_fails(
# not a very sensible certificate, but libpq should handle it gracefully.
switch_server_cert($node, 'server-no-names');
$common_connstr =
- "user=ssltestuser dbname=trustdb sslcert=invalid sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR";
+ "$default_ssl_connstr user=ssltestuser dbname=trustdb sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR";
$node->connect_ok(
"$common_connstr sslmode=verify-ca host=common-name.pg-ssltest.test",
@@ -339,7 +345,7 @@ $node->connect_fails(
switch_server_cert($node, 'server-revoked');
$common_connstr =
- "user=ssltestuser dbname=trustdb sslcert=invalid hostaddr=$SERVERHOSTADDR host=common-name.pg-ssltest.test";
+ "$default_ssl_connstr user=ssltestuser dbname=trustdb hostaddr=$SERVERHOSTADDR host=common-name.pg-ssltest.test";
# Without the CRL, succeeds. With it, fails.
$node->connect_ok(
@@ -349,8 +355,10 @@ $node->connect_fails(
"$common_connstr sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca sslcrl=ssl/root+server.crl",
"does not connect with client-side CRL file",
expected_stderr => qr/SSL error: certificate verify failed/);
+# sslcrl='' is added here to override the invalid default, so as this
+# does not interfere with this case.
$node->connect_fails(
- "$common_connstr sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca sslcrldir=ssl/root+server-crldir",
+ "$common_connstr sslcrl='' sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca sslcrldir=ssl/root+server-crldir",
"does not connect with client-side CRL directory",
expected_stderr => qr/SSL error: certificate verify failed/);
@@ -392,7 +400,7 @@ $node->connect_fails(
note "running server tests";
$common_connstr =
- "sslrootcert=ssl/root+server_ca.crt sslmode=require dbname=certdb hostaddr=$SERVERHOSTADDR host=localhost";
+ "$default_ssl_connstr sslrootcert=ssl/root+server_ca.crt sslmode=require dbname=certdb hostaddr=$SERVERHOSTADDR host=localhost";
# no client cert
$node->connect_fails(
@@ -569,7 +577,7 @@ $node->connect_fails(
# works, iff username matches Common Name
# fails, iff username doesn't match Common Name.
$common_connstr =
- "sslrootcert=ssl/root+server_ca.crt sslmode=require dbname=verifydb hostaddr=$SERVERHOSTADDR host=localhost";
+ "$default_ssl_connstr sslrootcert=ssl/root+server_ca.crt sslmode=require dbname=verifydb hostaddr=$SERVERHOSTADDR host=localhost";
$node->connect_ok(
"$common_connstr user=ssltestuser sslcert=ssl/client.crt sslkey=$key{'client.key'}",
@@ -596,7 +604,7 @@ $node->connect_ok(
# 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=$key{'client.key'} sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR host=localhost";
+ "$default_ssl_connstr user=ssltestuser dbname=certdb sslkey=$key{'client.key'} sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR host=localhost";
$node->connect_ok(
"$common_connstr sslmode=require sslcert=ssl/client+client_ca.crt",
diff --git a/src/test/ssl/t/003_sslinfo.pl b/src/test/ssl/t/003_sslinfo.pl
index 95742081f3..72eb6b860c 100644
--- a/src/test/ssl/t/003_sslinfo.pl
+++ b/src/test/ssl/t/003_sslinfo.pl
@@ -62,8 +62,13 @@ configure_test_server_for_ssl($node, $SERVERHOSTADDR, $SERVERHOSTCIDR,
# 001 test does.
switch_server_cert($node, 'server-revoked');
+# Set of default settings for SSL parameters in connection string. This
+# makes the tests protected against any defaults the environment may have
+# in ~/.postgresql/.
+my $default_ssl_connstr = "sslkey=invalid sslcert=invalid sslrootcert=invalid sslcrl=invalid sslcrldir=invalid";
+
$common_connstr =
- "sslrootcert=ssl/root+server_ca.crt sslmode=require dbname=certdb hostaddr=$SERVERHOSTADDR host=localhost " .
+ "$default_ssl_connstr sslrootcert=ssl/root+server_ca.crt sslmode=require dbname=certdb hostaddr=$SERVERHOSTADDR host=localhost " .
"user=ssltestuser sslcert=ssl/client_ext.crt sslkey=$client_tmp_key";
# Make sure we can connect even though previous test suites have established this
@@ -93,7 +98,7 @@ $result = $node->safe_psql("certdb", "SELECT ssl_client_cert_present();",
is($result, 't', "ssl_client_cert_present() for connection with cert");
$result = $node->safe_psql("trustdb", "SELECT ssl_client_cert_present();",
- connstr => "sslrootcert=ssl/root+server_ca.crt sslmode=require " .
+ connstr => "$default_ssl_connstr sslrootcert=ssl/root+server_ca.crt sslmode=require " .
"dbname=trustdb hostaddr=$SERVERHOSTADDR user=ssltestuser host=localhost");
is($result, 'f', "ssl_client_cert_present() for connection without cert");
@@ -108,7 +113,7 @@ $result = $node->psql("certdb", "SELECT ssl_client_dn_field('invalid');",
is($result, '3', "ssl_client_dn_field() for an invalid field");
$result = $node->safe_psql("trustdb", "SELECT ssl_client_dn_field('commonName');",
- connstr => "sslrootcert=ssl/root+server_ca.crt sslmode=require " .
+ connstr => "$default_ssl_connstr sslrootcert=ssl/root+server_ca.crt sslmode=require " .
"dbname=trustdb hostaddr=$SERVERHOSTADDR user=ssltestuser host=localhost");
is($result, '', "ssl_client_dn_field() for connection without cert");
signature.asc
Description: PGP signature
