Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 9ca234b

Browse files
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 208c5d6 commit 9ca234b

File tree

2 files changed

+29
-16
lines changed

2 files changed

+29
-16
lines changed

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

+21-13
Original file line numberDiff line numberDiff line change
@@ -138,8 +138,13 @@
138138

139139
switch_server_cert($node, 'server-cn-only');
140140

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

144149
# The server should not accept non-SSL connections.
145150
$node->connect_fails(
@@ -216,9 +221,10 @@
216221
"CRL belonging to a different CA",
217222
expected_stderr => qr/SSL error: certificate verify failed/);
218223

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

@@ -235,7 +241,7 @@
235241
# Check that connecting with verify-full fails, when the hostname doesn't
236242
# match the hostname in the server's certificate.
237243
$common_connstr =
238-
"user=ssltestuser dbname=trustdb sslcert=invalid sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR";
244+
"$default_ssl_connstr user=ssltestuser dbname=trustdb sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR";
239245

240246
$node->connect_ok("$common_connstr sslmode=require host=wronghost.test",
241247
"mismatch between host name and server certificate sslmode=require");
@@ -253,7 +259,7 @@
253259
switch_server_cert($node, 'server-multiple-alt-names');
254260

255261
$common_connstr =
256-
"user=ssltestuser dbname=trustdb sslcert=invalid sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR sslmode=verify-full";
262+
"$default_ssl_connstr user=ssltestuser dbname=trustdb sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR sslmode=verify-full";
257263

258264
$node->connect_ok(
259265
"$common_connstr host=dns1.alt-name.pg-ssltest.test",
@@ -282,7 +288,7 @@
282288
switch_server_cert($node, 'server-single-alt-name');
283289

284290
$common_connstr =
285-
"user=ssltestuser dbname=trustdb sslcert=invalid sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR sslmode=verify-full";
291+
"$default_ssl_connstr user=ssltestuser dbname=trustdb sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR sslmode=verify-full";
286292

287293
$node->connect_ok(
288294
"$common_connstr host=single.alt-name.pg-ssltest.test",
@@ -306,7 +312,7 @@
306312
switch_server_cert($node, 'server-cn-and-alt-names');
307313

308314
$common_connstr =
309-
"user=ssltestuser dbname=trustdb sslcert=invalid sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR sslmode=verify-full";
315+
"$default_ssl_connstr user=ssltestuser dbname=trustdb sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR sslmode=verify-full";
310316

311317
$node->connect_ok("$common_connstr host=dns1.alt-name.pg-ssltest.test",
312318
"certificate with both a CN and SANs 1");
@@ -323,7 +329,7 @@
323329
# not a very sensible certificate, but libpq should handle it gracefully.
324330
switch_server_cert($node, 'server-no-names');
325331
$common_connstr =
326-
"user=ssltestuser dbname=trustdb sslcert=invalid sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR";
332+
"$default_ssl_connstr user=ssltestuser dbname=trustdb sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR";
327333

328334
$node->connect_ok(
329335
"$common_connstr sslmode=verify-ca host=common-name.pg-ssltest.test",
@@ -339,7 +345,7 @@
339345
switch_server_cert($node, 'server-revoked');
340346

341347
$common_connstr =
342-
"user=ssltestuser dbname=trustdb sslcert=invalid hostaddr=$SERVERHOSTADDR host=common-name.pg-ssltest.test";
348+
"$default_ssl_connstr user=ssltestuser dbname=trustdb hostaddr=$SERVERHOSTADDR host=common-name.pg-ssltest.test";
343349

344350
# Without the CRL, succeeds. With it, fails.
345351
$node->connect_ok(
@@ -349,8 +355,10 @@
349355
"$common_connstr sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca sslcrl=ssl/root+server.crl",
350356
"does not connect with client-side CRL file",
351357
expected_stderr => qr/SSL error: certificate verify failed/);
358+
# sslcrl='' is added here to override the invalid default, so as this
359+
# does not interfere with this case.
352360
$node->connect_fails(
353-
"$common_connstr sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca sslcrldir=ssl/root+server-crldir",
361+
"$common_connstr sslcrl='' sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca sslcrldir=ssl/root+server-crldir",
354362
"does not connect with client-side CRL directory",
355363
expected_stderr => qr/SSL error: certificate verify failed/);
356364

@@ -392,7 +400,7 @@
392400
note "running server tests";
393401

394402
$common_connstr =
395-
"sslrootcert=ssl/root+server_ca.crt sslmode=require dbname=certdb hostaddr=$SERVERHOSTADDR host=localhost";
403+
"$default_ssl_connstr sslrootcert=ssl/root+server_ca.crt sslmode=require dbname=certdb hostaddr=$SERVERHOSTADDR host=localhost";
396404

397405
# no client cert
398406
$node->connect_fails(
@@ -569,7 +577,7 @@
569577
# works, iff username matches Common Name
570578
# fails, iff username doesn't match Common Name.
571579
$common_connstr =
572-
"sslrootcert=ssl/root+server_ca.crt sslmode=require dbname=verifydb hostaddr=$SERVERHOSTADDR host=localhost";
580+
"$default_ssl_connstr sslrootcert=ssl/root+server_ca.crt sslmode=require dbname=verifydb hostaddr=$SERVERHOSTADDR host=localhost";
573581

574582
$node->connect_ok(
575583
"$common_connstr user=ssltestuser sslcert=ssl/client.crt sslkey=$key{'client.key'}",
@@ -596,7 +604,7 @@
596604
# intermediate client_ca.crt is provided by client, and isn't in server's ssl_ca_file
597605
switch_server_cert($node, 'server-cn-only', 'root_ca');
598606
$common_connstr =
599-
"user=ssltestuser dbname=certdb sslkey=$key{'client.key'} sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR host=localhost";
607+
"$default_ssl_connstr user=ssltestuser dbname=certdb sslkey=$key{'client.key'} sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR host=localhost";
600608

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

src/test/ssl/t/003_sslinfo.pl

+8-3
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,13 @@
6262
# 001 test does.
6363
switch_server_cert($node, 'server-revoked');
6464

65+
# Set of default settings for SSL parameters in connection string. This
66+
# makes the tests protected against any defaults the environment may have
67+
# in ~/.postgresql/.
68+
my $default_ssl_connstr = "sslkey=invalid sslcert=invalid sslrootcert=invalid sslcrl=invalid sslcrldir=invalid";
69+
6570
$common_connstr =
66-
"sslrootcert=ssl/root+server_ca.crt sslmode=require dbname=certdb hostaddr=$SERVERHOSTADDR host=localhost " .
71+
"$default_ssl_connstr sslrootcert=ssl/root+server_ca.crt sslmode=require dbname=certdb hostaddr=$SERVERHOSTADDR host=localhost " .
6772
"user=ssltestuser sslcert=ssl/client_ext.crt sslkey=$client_tmp_key";
6873

6974
# Make sure we can connect even though previous test suites have established this
@@ -93,7 +98,7 @@
9398
is($result, 't', "ssl_client_cert_present() for connection with cert");
9499

95100
$result = $node->safe_psql("trustdb", "SELECT ssl_client_cert_present();",
96-
connstr => "sslrootcert=ssl/root+server_ca.crt sslmode=require " .
101+
connstr => "$default_ssl_connstr sslrootcert=ssl/root+server_ca.crt sslmode=require " .
97102
"dbname=trustdb hostaddr=$SERVERHOSTADDR user=ssltestuser host=localhost");
98103
is($result, 'f', "ssl_client_cert_present() for connection without cert");
99104

@@ -108,7 +113,7 @@
108113
is($result, '3', "ssl_client_dn_field() for an invalid field");
109114

110115
$result = $node->safe_psql("trustdb", "SELECT ssl_client_dn_field('commonName');",
111-
connstr => "sslrootcert=ssl/root+server_ca.crt sslmode=require " .
116+
connstr => "$default_ssl_connstr sslrootcert=ssl/root+server_ca.crt sslmode=require " .
112117
"dbname=trustdb hostaddr=$SERVERHOSTADDR user=ssltestuser host=localhost");
113118
is($result, '', "ssl_client_dn_field() for connection without cert");
114119

0 commit comments

Comments
 (0)