On Wed, Mar 16, 2022 at 11:45:39AM +0100, Daniel Gustafsson wrote:
> On 16 Mar 2022, at 08:36, Kyotaro Horiguchi <horikyota....@gmail.com> wrote:
>> The attached fixes that and make-world successfully finished even if I
>> have a cert file in my home direcotory.
> 
> Seems correct to me, thanks!

The ultimate test I can think about to stress the robustness of this
test suite is to generate various certs and keys using "make
sslfiles", save them into a ~/.postgresql/ (postgresql.crt,
postgresql.key, root.crl and root.crt), and then run the tests to see
how much junk data the SSL scripts would feed on.  With this method, I
have caught a total of 71 failures, much more than reported upthread.

We should really put more attention to set invalid default values for
sslcert, sslkey, sslcrl, sslcrldir and sslrootcert, rather than
hardcoding a couple of them in only a few places, opening ourselves to
the same problem, again, each time a new test is added.  The best way
I can think about here is to use a string that includes all the
default SSL settings, appending that at the beginning of each
$common_connstr.  This takes care of most the failures, except two
cases related to expected failures for sslcrldir:
- directory CRL belonging to a different CA
- does not connect with client-side CRL directory

In both cases, enforcing sslcrl to a value of "invalid" interferes
with the failure scenario we expect from sslcrldir.  It is possible to
bypass that with something like the attached, but that's a kind of
ugly hack.  Another alternative would be to drop those two tests, and
I am not sure how much we care about these two negative scenarios.

Thoughts?
--
Michael
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index 5c5b16fbe7..55f2a52dee 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -138,8 +138,17 @@ 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/.  no_crl is a flavor that does not have a default
+# value set for the connection parameter sslcrl.
+my $default_ssl_connstr = "sslkey=invalid sslcert=invalid sslrootcert=invalid sslcrl=invalid sslcrldir=invalid";
+my $default_ssl_connstr_no_crl = "sslkey=invalid sslcert=invalid sslrootcert=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";
+my $common_connstr_no_crl =
+  "$default_ssl_connstr_no_crl user=ssltestuser dbname=trustdb hostaddr=$SERVERHOSTADDR host=common-name.pg-ssltest.test";
 
 # The server should not accept non-SSL connections.
 $node->connect_fails(
@@ -150,14 +159,14 @@ $node->connect_fails(
 # Try without a root cert. In sslmode=require, this should work. In verify-ca
 # or verify-full mode it should fail.
 $node->connect_ok(
-	"$common_connstr sslrootcert=invalid sslmode=require",
+	"$common_connstr sslmode=require",
 	"connect without server root cert sslmode=require");
 $node->connect_fails(
-	"$common_connstr sslrootcert=invalid sslmode=verify-ca",
+	"$common_connstr sslmode=verify-ca",
 	"connect without server root cert sslmode=verify-ca",
 	expected_stderr => qr/root certificate file "invalid" does not exist/);
 $node->connect_fails(
-	"$common_connstr sslrootcert=invalid sslmode=verify-full",
+	"$common_connstr sslmode=verify-full",
 	"connect without server root cert sslmode=verify-full",
 	expected_stderr => qr/root certificate file "invalid" does not exist/);
 
@@ -217,8 +226,10 @@ $node->connect_fails(
 	expected_stderr => qr/SSL error: certificate verify failed/);
 
 # The same for CRL directory
+# The default connection string should not include a default value for
+# sslcrl, or this interferes with the result of this test.
 $node->connect_fails(
-	"$common_connstr sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca sslcrldir=ssl/client-crldir",
+	"$common_connstr_no_crl 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 +246,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 +264,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 +293,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 +317,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 +334,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 +350,9 @@ $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";
+$common_connstr_no_crl =
+  "$default_ssl_connstr_no_crl user=ssltestuser dbname=trustdb hostaddr=$SERVERHOSTADDR host=common-name.pg-ssltest.test";
 
 # Without the CRL, succeeds. With it, fails.
 $node->connect_ok(
@@ -349,8 +362,11 @@ $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/);
+# Special case for the connection string here.  Using sslcrl=invalid
+# interferes with the negative test of sslcrldir as it gets taken into
+# account, causing an incorrect failure.
 $node->connect_fails(
-	"$common_connstr sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca sslcrldir=ssl/root+server-crldir",
+	"$common_connstr_no_crl 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 +408,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 +585,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 +612,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",

Attachment: signature.asc
Description: PGP signature

Reply via email to