Date: Thu, 17 Mar 2022 16:22:14 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Daniel Gustafsson <daniel@yesql.se>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>,
	pgsql-hackers@lists.postgresql.org
Subject: Re: Out-of-tree certificate interferes ssltest
Message-ID: <YjLhpog7Q0kRQq1K@paquier.xyz>
References: <20220316.163658.1122740600489097632.horikyota.ntt@gmail.com>
 <99384E03-E59D-4CA3-8D5E-E3F90B11D335@yesql.se>
 <YjLOPibZ/ooGlZw6@paquier.xyz>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="73Ap0Y9kvmxUoSbu"
Content-Disposition: inline
In-Reply-To: <YjLOPibZ/ooGlZw6@paquier.xyz>
Archived-At: <https://www.postgresql.org/message-id/YjLhpog7Q0kRQq1K%40paquier.xyz>
Precedence: bulk


--73Ap0Y9kvmxUoSbu
Content-Type: multipart/mixed; boundary="i6Umhnoayhq3lMZJ"
Content-Disposition: inline


--i6Umhnoayhq3lMZJ
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Thu, Mar 17, 2022 at 02:59:26PM +0900, Michael Paquier wrote:
> 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.

Actually, there is a trick I have recalled here: we can enforce sslcrl
to an empty value in the connection string after the default.  This
still ensures that the test won't pick up any SSL data from the local
environment and avoids any interferences of OpenSSL's
X509_STORE_load_locations().  This gives a much simpler and cleaner
patch.

Thoughts?
--
Michael

--i6Umhnoayhq3lMZJ
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="ssltest-tap-2.patch"
Content-Transfer-Encoding: quoted-printable

diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index 5c5b16fbe7..45bd962f40 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";
=20
 switch_server_cert($node, 'server-cn-only');
=20
+# 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 =3D "sslkey=3Dinvalid sslcert=3Dinvalid sslrootcer=
t=3Dinvalid sslcrl=3Dinvalid sslcrldir=3Dinvalid";
+
 $common_connstr =3D
-  "user=3Dssltestuser dbname=3Dtrustdb sslcert=3Dinvalid hostaddr=3D$SERVE=
RHOSTADDR host=3Dcommon-name.pg-ssltest.test";
+  "$default_ssl_connstr user=3Dssltestuser dbname=3Dtrustdb hostaddr=3D$SE=
RVERHOSTADDR host=3Dcommon-name.pg-ssltest.test";
=20
 # The server should not accept non-SSL connections.
 $node->connect_fails(
@@ -150,14 +155,14 @@ $node->connect_fails(
 # Try without a root cert. In sslmode=3Drequire, this should work. In veri=
fy-ca
 # or verify-full mode it should fail.
 $node->connect_ok(
-	"$common_connstr sslrootcert=3Dinvalid sslmode=3Drequire",
+	"$common_connstr sslmode=3Drequire",
 	"connect without server root cert sslmode=3Drequire");
 $node->connect_fails(
-	"$common_connstr sslrootcert=3Dinvalid sslmode=3Dverify-ca",
+	"$common_connstr sslmode=3Dverify-ca",
 	"connect without server root cert sslmode=3Dverify-ca",
 	expected_stderr =3D> qr/root certificate file "invalid" does not exist/);
 $node->connect_fails(
-	"$common_connstr sslrootcert=3Dinvalid sslmode=3Dverify-full",
+	"$common_connstr sslmode=3Dverify-full",
 	"connect without server root cert sslmode=3Dverify-full",
 	expected_stderr =3D> qr/root certificate file "invalid" does not exist/);
=20
@@ -216,9 +221,10 @@ $node->connect_fails(
 	"CRL belonging to a different CA",
 	expected_stderr =3D> qr/SSL error: certificate verify failed/);
=20
-# The same for CRL directory
+# The same for CRL directory.  sslcrl=3D'' is added here to override the
+# invalid default, so as this does not interfere with this case.
 $node->connect_fails(
-	"$common_connstr sslrootcert=3Dssl/root+server_ca.crt sslmode=3Dverify-ca=
 sslcrldir=3Dssl/client-crldir",
+	"$common_connstr sslcrl=3D'' sslrootcert=3Dssl/root+server_ca.crt sslmode=
=3Dverify-ca sslcrldir=3Dssl/client-crldir",
 	"directory CRL belonging to a different CA",
 	expected_stderr =3D> qr/SSL error: certificate verify failed/);
=20
@@ -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 =3D
-  "user=3Dssltestuser dbname=3Dtrustdb sslcert=3Dinvalid sslrootcert=3Dssl=
/root+server_ca.crt hostaddr=3D$SERVERHOSTADDR";
+  "$default_ssl_connstr user=3Dssltestuser dbname=3Dtrustdb sslrootcert=3D=
ssl/root+server_ca.crt hostaddr=3D$SERVERHOSTADDR";
=20
 $node->connect_ok("$common_connstr sslmode=3Drequire host=3Dwronghost.test=
",
 	"mismatch between host name and server certificate sslmode=3Drequire");
@@ -253,7 +259,7 @@ $node->connect_fails(
 switch_server_cert($node, 'server-multiple-alt-names');
=20
 $common_connstr =3D
-  "user=3Dssltestuser dbname=3Dtrustdb sslcert=3Dinvalid sslrootcert=3Dssl=
/root+server_ca.crt hostaddr=3D$SERVERHOSTADDR sslmode=3Dverify-full";
+  "$default_ssl_connstr user=3Dssltestuser dbname=3Dtrustdb sslrootcert=3D=
ssl/root+server_ca.crt hostaddr=3D$SERVERHOSTADDR sslmode=3Dverify-full";
=20
 $node->connect_ok(
 	"$common_connstr host=3Ddns1.alt-name.pg-ssltest.test",
@@ -282,7 +288,7 @@ $node->connect_fails(
 switch_server_cert($node, 'server-single-alt-name');
=20
 $common_connstr =3D
-  "user=3Dssltestuser dbname=3Dtrustdb sslcert=3Dinvalid sslrootcert=3Dssl=
/root+server_ca.crt hostaddr=3D$SERVERHOSTADDR sslmode=3Dverify-full";
+  "$default_ssl_connstr user=3Dssltestuser dbname=3Dtrustdb sslrootcert=3D=
ssl/root+server_ca.crt hostaddr=3D$SERVERHOSTADDR sslmode=3Dverify-full";
=20
 $node->connect_ok(
 	"$common_connstr host=3Dsingle.alt-name.pg-ssltest.test",
@@ -306,7 +312,7 @@ $node->connect_fails(
 switch_server_cert($node, 'server-cn-and-alt-names');
=20
 $common_connstr =3D
-  "user=3Dssltestuser dbname=3Dtrustdb sslcert=3Dinvalid sslrootcert=3Dssl=
/root+server_ca.crt hostaddr=3D$SERVERHOSTADDR sslmode=3Dverify-full";
+  "$default_ssl_connstr user=3Dssltestuser dbname=3Dtrustdb sslrootcert=3D=
ssl/root+server_ca.crt hostaddr=3D$SERVERHOSTADDR sslmode=3Dverify-full";
=20
 $node->connect_ok("$common_connstr host=3Ddns1.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 =3D
-  "user=3Dssltestuser dbname=3Dtrustdb sslcert=3Dinvalid sslrootcert=3Dssl=
/root+server_ca.crt hostaddr=3D$SERVERHOSTADDR";
+  "$default_ssl_connstr user=3Dssltestuser dbname=3Dtrustdb sslrootcert=3D=
ssl/root+server_ca.crt hostaddr=3D$SERVERHOSTADDR";
=20
 $node->connect_ok(
 	"$common_connstr sslmode=3Dverify-ca host=3Dcommon-name.pg-ssltest.test",
@@ -339,7 +345,7 @@ $node->connect_fails(
 switch_server_cert($node, 'server-revoked');
=20
 $common_connstr =3D
-  "user=3Dssltestuser dbname=3Dtrustdb sslcert=3Dinvalid hostaddr=3D$SERVE=
RHOSTADDR host=3Dcommon-name.pg-ssltest.test";
+  "$default_ssl_connstr user=3Dssltestuser dbname=3Dtrustdb hostaddr=3D$SE=
RVERHOSTADDR host=3Dcommon-name.pg-ssltest.test";
=20
 # Without the CRL, succeeds. With it, fails.
 $node->connect_ok(
@@ -349,8 +355,10 @@ $node->connect_fails(
 	"$common_connstr sslrootcert=3Dssl/root+server_ca.crt sslmode=3Dverify-ca=
 sslcrl=3Dssl/root+server.crl",
 	"does not connect with client-side CRL file",
 	expected_stderr =3D> qr/SSL error: certificate verify failed/);
+# sslcrl=3D'' is added here to override the invalid default, so as this
+# does not interfere with this case.
 $node->connect_fails(
-	"$common_connstr sslrootcert=3Dssl/root+server_ca.crt sslmode=3Dverify-ca=
 sslcrldir=3Dssl/root+server-crldir",
+	"$common_connstr sslcrl=3D'' sslrootcert=3Dssl/root+server_ca.crt sslmode=
=3Dverify-ca sslcrldir=3Dssl/root+server-crldir",
 	"does not connect with client-side CRL directory",
 	expected_stderr =3D> qr/SSL error: certificate verify failed/);
=20
@@ -392,7 +400,7 @@ $node->connect_fails(
 note "running server tests";
=20
 $common_connstr =3D
-  "sslrootcert=3Dssl/root+server_ca.crt sslmode=3Drequire dbname=3Dcertdb =
hostaddr=3D$SERVERHOSTADDR host=3Dlocalhost";
+  "$default_ssl_connstr sslrootcert=3Dssl/root+server_ca.crt sslmode=3Dreq=
uire dbname=3Dcertdb hostaddr=3D$SERVERHOSTADDR host=3Dlocalhost";
=20
 # 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 =3D
-  "sslrootcert=3Dssl/root+server_ca.crt sslmode=3Drequire dbname=3Dverifyd=
b hostaddr=3D$SERVERHOSTADDR host=3Dlocalhost";
+  "$default_ssl_connstr sslrootcert=3Dssl/root+server_ca.crt sslmode=3Dreq=
uire dbname=3Dverifydb hostaddr=3D$SERVERHOSTADDR host=3Dlocalhost";
=20
 $node->connect_ok(
 	"$common_connstr user=3Dssltestuser sslcert=3Dssl/client.crt sslkey=3D$ke=
y{'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 =3D
-  "user=3Dssltestuser dbname=3Dcertdb sslkey=3D$key{'client.key'} sslrootc=
ert=3Dssl/root+server_ca.crt hostaddr=3D$SERVERHOSTADDR host=3Dlocalhost";
+  "$default_ssl_connstr user=3Dssltestuser dbname=3Dcertdb sslkey=3D$key{'=
client.key'} sslrootcert=3Dssl/root+server_ca.crt hostaddr=3D$SERVERHOSTADD=
R host=3Dlocalhost";
=20
 $node->connect_ok(
 	"$common_connstr sslmode=3Drequire sslcert=3Dssl/client+client_ca.crt",

--i6Umhnoayhq3lMZJ--

--73Ap0Y9kvmxUoSbu
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmIy4aYACgkQnvQgOdby
QH0tcQ/6AzsOVY0v/0h3tRnW90X8VDbQhOKOQ7wr6e8s6d2Lv+fosxUzuy3saAHg
+Z0ZYY7GvcF5hVymMkZTEALVQTwFSUqsaDzCAQFuegsy36K3aVG1bTIUHN+yV6v5
oBW3DL2d7GSBa22NuVRM9S3j3RBiXuETVeoKfxvVhxXeWMFDm+DfIw3RxQtKizcE
W0lESe2e24YoMlrEz2GSLA4eu2dSabXvgTKzu27y8LQUhZaEPNe71Fo74yC0ZD3X
X0hEKcXO9I13JEI1KGTCSXPEVxnF8wSnavjTiqFDAgpolMWRtEC+LbZcWiNkgtFu
sUDDiVXK0Pu6RDIn/ATO0/OSZke48pb7dsF6BzNCx6eF87H25Eyoxg8EacJh8fdE
LUT5ZfdQMa6fZQvHwYLEpljhSXBODID1BRoR4hgr9zxV8NkEAVhHvPb7hUMsf63R
iBhGitXnewB8ZGVdb9g1bkg6JEF2dVStOjTXJHI5vNnzdSOL5iHSZZYkXgbEglY/
F8FctW99jzD4kfp86ZY9a8FFqLmd7JTwr9jXrJhujo5PEk7X2RPFBhHLdFJfqcU/
n+sd19zxnMw7+sN7tdp9eWphQ0iZ4gCFWx2Ili4QPSsf8fhcoUshzN5rt5k2B4fi
wsnAFBRG5JDbmOdMxWFoML/FUhIqaWJM+n0z+0VzZa4O//M8NZ8=
=RgZq
-----END PGP SIGNATURE-----

--73Ap0Y9kvmxUoSbu--