Skip to content

Commit fdb1be4

Browse files
michaelpqmichaelpq
committed
Fix failures in SSL tests caused by out-of-tree keys and certificates
This issue is environment-sensitive, where the SSL tests could fail in various way by feeding on defaults provided by sslcert, sslkey, sslrootkey, sslrootcert, sslcrl and sslcrldir coming from a local setup, as of ~/.postgresql/ by default. Horiguchi-san has reported two failures, but more advanced testing from me (aka inclusion of garbage SSL configuration in ~/.postgresql/ for all the configuration parameters) has showed dozens of failures that can be triggered in the whole test suite. History has showed that we are not good when it comes to address such issues, fixing them locally like in dd87799, and such problems keep appearing. This commit strengthens the entire test suite to put an end to this set of problems by embedding invalid default values in all the connection strings used in the tests. The invalid values are prefixed in each connection string, relying on the follow-up values passed in the connection string to enforce any invalid value previously set. Note that two tests related to CRLs are required to fail with certain pre-set configurations, but we can rely on enforcing an empty value instead after the invalid set of values. Reported-by: Kyotaro Horiguchi Reviewed-by: Andrew Dunstan, Daniel Gustafsson, Kyotaro Horiguchi Discussion: https://postgr.es/m/20220316.163658.1122740600489097632.horikyota.ntt@gmail.com backpatch-through: 10
1 parent 48b6035 commit fdb1be4

File tree

1 file changed

+21
-13
lines changed

1 file changed

+21
-13
lines changed

‎src/test/ssl/t/001_ssltests.pl

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,13 @@
134134

135135
switch_server_cert($node, 'server-cn-only');
136136

137+
# Set of default settings for SSL parameters in connection string. This
138+
# makes the tests protected against any defaults the environment may have
139+
# in ~/.postgresql/.
140+
my $default_ssl_connstr = "sslkey=invalid sslcert=invalid sslrootcert=invalid sslcrl=invalid sslcrldir=invalid";
141+
137142
$common_connstr =
138-
"user=ssltestuser dbname=trustdb sslcert=invalid hostaddr=$SERVERHOSTADDR host=common-name.pg-ssltest.test";
143+
"$default_ssl_connstr user=ssltestuser dbname=trustdb hostaddr=$SERVERHOSTADDR host=common-name.pg-ssltest.test";
139144

140145
# The server should not accept non-SSL connections.
141146
$node->connect_fails(
@@ -212,9 +217,10 @@
212217
"CRL belonging to a different CA",
213218
expected_stderr => qr/SSL error: certificate verify failed/);
214219

215-
# The same for CRL directory
220+
# The same for CRL directory. sslcrl='' is added here to override the
221+
# invalid default, so as this does not interfere with this case.
216222
$node->connect_fails(
217-
"$common_connstr sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca sslcrldir=ssl/client-crldir",
223+
"$common_connstr sslcrl='' sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca sslcrldir=ssl/client-crldir",
218224
"directory CRL belonging to a different CA",
219225
expected_stderr => qr/SSL error: certificate verify failed/);
220226

@@ -231,7 +237,7 @@
231237
# Check that connecting with verify-full fails, when the hostname doesn't
232238
# match the hostname in the server's certificate.
233239
$common_connstr =
234-
"user=ssltestuser dbname=trustdb sslcert=invalid sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR";
240+
"$default_ssl_connstr user=ssltestuser dbname=trustdb sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR";
235241

236242
$node->connect_ok("$common_connstr sslmode=require host=wronghost.test",
237243
"mismatch between host name and server certificate sslmode=require");
@@ -249,7 +255,7 @@
249255
switch_server_cert($node, 'server-multiple-alt-names');
250256

251257
$common_connstr =
252-
"user=ssltestuser dbname=trustdb sslcert=invalid sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR sslmode=verify-full";
258+
"$default_ssl_connstr user=ssltestuser dbname=trustdb sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR sslmode=verify-full";
253259

254260
$node->connect_ok(
255261
"$common_connstr host=dns1.alt-name.pg-ssltest.test",
@@ -278,7 +284,7 @@
278284
switch_server_cert($node, 'server-single-alt-name');
279285

280286
$common_connstr =
281-
"user=ssltestuser dbname=trustdb sslcert=invalid sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR sslmode=verify-full";
287+
"$default_ssl_connstr user=ssltestuser dbname=trustdb sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR sslmode=verify-full";
282288

283289
$node->connect_ok(
284290
"$common_connstr host=single.alt-name.pg-ssltest.test",
@@ -302,7 +308,7 @@
302308
switch_server_cert($node, 'server-cn-and-alt-names');
303309

304310
$common_connstr =
305-
"user=ssltestuser dbname=trustdb sslcert=invalid sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR sslmode=verify-full";
311+
"$default_ssl_connstr user=ssltestuser dbname=trustdb sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR sslmode=verify-full";
306312

307313
$node->connect_ok("$common_connstr host=dns1.alt-name.pg-ssltest.test",
308314
"certificate with both a CN and SANs 1");
@@ -319,7 +325,7 @@
319325
# not a very sensible certificate, but libpq should handle it gracefully.
320326
switch_server_cert($node, 'server-no-names');
321327
$common_connstr =
322-
"user=ssltestuser dbname=trustdb sslcert=invalid sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR";
328+
"$default_ssl_connstr user=ssltestuser dbname=trustdb sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR";
323329

324330
$node->connect_ok(
325331
"$common_connstr sslmode=verify-ca host=common-name.pg-ssltest.test",
@@ -335,7 +341,7 @@
335341
switch_server_cert($node, 'server-revoked');
336342

337343
$common_connstr =
338-
"user=ssltestuser dbname=trustdb sslcert=invalid hostaddr=$SERVERHOSTADDR host=common-name.pg-ssltest.test";
344+
"$default_ssl_connstr user=ssltestuser dbname=trustdb hostaddr=$SERVERHOSTADDR host=common-name.pg-ssltest.test";
339345

340346
# Without the CRL, succeeds. With it, fails.
341347
$node->connect_ok(
@@ -345,8 +351,10 @@
345351
"$common_connstr sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca sslcrl=ssl/root+server.crl",
346352
"does not connect with client-side CRL file",
347353
expected_stderr => qr/SSL error: certificate verify failed/);
354+
# sslcrl='' is added here to override the invalid default, so as this
355+
# does not interfere with this case.
348356
$node->connect_fails(
349-
"$common_connstr sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca sslcrldir=ssl/root+server-crldir",
357+
"$common_connstr sslcrl='' sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca sslcrldir=ssl/root+server-crldir",
350358
"does not connect with client-side CRL directory",
351359
expected_stderr => qr/SSL error: certificate verify failed/);
352360

@@ -388,7 +396,7 @@
388396
note "running server tests";
389397

390398
$common_connstr =
391-
"sslrootcert=ssl/root+server_ca.crt sslmode=require dbname=certdb hostaddr=$SERVERHOSTADDR host=localhost";
399+
"$default_ssl_connstr sslrootcert=ssl/root+server_ca.crt sslmode=require dbname=certdb hostaddr=$SERVERHOSTADDR host=localhost";
392400

393401
# no client cert
394402
$node->connect_fails(
@@ -538,7 +546,7 @@
538546
# works, iff username matches Common Name
539547
# fails, iff username doesn't match Common Name.
540548
$common_connstr =
541-
"sslrootcert=ssl/root+server_ca.crt sslmode=require dbname=verifydb hostaddr=$SERVERHOSTADDR host=localhost";
549+
"$default_ssl_connstr sslrootcert=ssl/root+server_ca.crt sslmode=require dbname=verifydb hostaddr=$SERVERHOSTADDR host=localhost";
542550

543551
$node->connect_ok(
544552
"$common_connstr user=ssltestuser sslcert=ssl/client.crt sslkey=ssl/client_tmp.key",
@@ -565,7 +573,7 @@
565573
# intermediate client_ca.crt is provided by client, and isn't in server's ssl_ca_file
566574
switch_server_cert($node, 'server-cn-only', 'root_ca');
567575
$common_connstr =
568-
"user=ssltestuser dbname=certdb sslkey=ssl/client_tmp.key sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR host=localhost";
576+
"$default_ssl_connstr user=ssltestuser dbname=certdb sslkey=ssl/client_tmp.key sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR host=localhost";
569577

570578
$node->connect_ok(
571579
"$common_connstr sslmode=require sslcert=ssl/client+client_ca.crt",

0 commit comments

Comments
 (0)