Commit 8bb14cdd authored by Heikki Linnakangas's avatar Heikki Linnakangas

Don't share SSL_CTX between libpq connections.

There were several issues with the old coding:

1. There was a race condition, if two threads opened a connection at the
   same time. We used a mutex around SSL_CTX_* calls, but that was not
   enough, e.g. if one thread SSL_CTX_load_verify_locations() with one
   path, and another thread set it with a different path, before the first
   thread got to establish the connection.

2. Opening two different connections, with different sslrootcert settings,
   seemed to fail outright with "SSL error: block type is not 01". Not sure
   why.

3. We created the SSL object, before calling SSL_CTX_load_verify_locations
   and SSL_CTX_use_certificate_chain_file on the SSL context. That was
   wrong, because the options set on the SSL context are propagated to the
   SSL object, when the SSL object is created. If they are set after the
   SSL object has already been created, they won't take effect until the
   next connection. (This is bug #14329)

At least some of these could've been fixed while still using a shared
context, but it would've been more complicated and error-prone. To keep
things simple, let's just use a separate SSL context for each connection,
and accept the overhead.

Backpatch to all supported versions.

Report, analysis and test case by Kacper Zuk.

Discussion: <20160920101051.1355.79453@wrigleys.postgresql.org>
parent d7eb76b9
This diff is collapsed.
......@@ -23,7 +23,8 @@ SSLFILES := $(CERTIFICATES:%=ssl/%.key) $(CERTIFICATES:%=ssl/%.crt) \
ssl/client.crl ssl/server.crl ssl/root.crl \
ssl/both-cas-1.crt ssl/both-cas-2.crt \
ssl/root+server_ca.crt ssl/root+server.crl \
ssl/root+client_ca.crt ssl/root+client.crl
ssl/root+client_ca.crt ssl/root+client.crl \
ssl/client+client_ca.crt
# This target generates all the key and certificate files.
sslfiles: $(SSLFILES)
......@@ -99,6 +100,9 @@ ssl/root+server_ca.crt: ssl/root_ca.crt ssl/server_ca.crt
ssl/root+client_ca.crt: ssl/root_ca.crt ssl/client_ca.crt
cat $^ > $@
ssl/client+client_ca.crt: ssl/client.crt ssl/client_ca.crt
cat $^ > $@
#### CRLs
ssl/client.crl: ssl/client-revoked.crt
......
......@@ -65,6 +65,10 @@ root+server_ca
root+client_ca
Contains root_crt and client_ca.crt. For use as server's "ssl_ca_file".
client+client_ca
Contains client.crt and client_ca.crt in that order. For use as client's
certificate chain.
There are also CRLs for each of the CAs: root.crl, server.crl and client.crl.
For convenience, all of these keypairs and certificates are included in the
......
......@@ -75,6 +75,7 @@ sub configure_test_server_for_ssl
copy_files("ssl/server-*.key", $pgdata);
chmod(0600, glob "$pgdata/server-*.key") or die $!;
copy_files("ssl/root+client_ca.crt", $pgdata);
copy_files("ssl/root_ca.crt", $pgdata);
copy_files("ssl/root+client.crl", $pgdata);
# Only accept SSL connections from localhost. Our tests don't depend on this
......@@ -101,13 +102,14 @@ sub switch_server_cert
{
my $node = $_[0];
my $certfile = $_[1];
my $cafile = $_[2] || "root+client_ca";
my $pgdata = $node->data_dir;
diag "Restarting server with certfile \"$certfile\"...";
diag "Restarting server with certfile \"$certfile\" and cafile \"$cafile\"...";
open SSLCONF, ">$pgdata/sslconfig.conf";
print SSLCONF "ssl=on\n";
print SSLCONF "ssl_ca_file='root+client_ca.crt'\n";
print SSLCONF "ssl_ca_file='$cafile.crt'\n";
print SSLCONF "ssl_cert_file='$certfile.crt'\n";
print SSLCONF "ssl_key_file='$certfile.key'\n";
print SSLCONF "ssl_crl_file='root+client.crl'\n";
......
-----BEGIN CERTIFICATE-----
MIIBxzCCATACAQEwDQYJKoZIhvcNAQEFBQAwQjFAMD4GA1UEAww3VGVzdCBDQSBm
b3IgUG9zdGdyZVNRTCBTU0wgcmVncmVzc2lvbiB0ZXN0IGNsaWVudCBjZXJ0czAe
Fw0xNjA5MTIxNjMwMDFaFw00NDAxMjkxNjMwMDFaMBYxFDASBgNVBAMMC3NzbHRl
c3R1c2VyMIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDN3RFl8VWMEBN1Qas0
w1CFcXdDEbKVNSPsqWHzHIEPoGJv+eUIBK2lQ/Ce8nRCdelO50RsmlbcXBIrjVl6
BN0RmEeEVclgCdiamYN53LBdc5KWKpKCKn45lCtlZodWt0hNNx1pAmh85jDKpoO9
ErbCnSU1wODPqnOzdkLU7jBu5QIDAQABMA0GCSqGSIb3DQEBBQUAA4GBABUz+vnu
dD1Q1N/Ezs5DzJeQDtiJb9PNzBHAUPQoXeLvuITcDdyYWc18Yi4fX7gwyD42q2iu
1I0hmm2bNJfujsGbvGYFLuQ4hC2ucAAj2Gm681GhhaNYtfsfHYm9R8GRZFvp40oj
qXpkDkYsPdyVxUyoxJ+M0Ub5VC/k1pQNtIaq
-----END CERTIFICATE-----
-----BEGIN CERTIFICATE-----
MIICCDCCAXGgAwIBAgIBAjANBgkqhkiG9w0BAQUFADBAMT4wPAYDVQQDDDVUZXN0
IHJvb3QgQ0EgZm9yIFBvc3RncmVTUUwgU1NMIHJlZ3Jlc3Npb24gdGVzdCBzdWl0
ZTAeFw0xNjA5MTIxNjMwMDFaFw00NDAxMjkxNjMwMDFaMEIxQDA+BgNVBAMMN1Rl
c3QgQ0EgZm9yIFBvc3RncmVTUUwgU1NMIHJlZ3Jlc3Npb24gdGVzdCBjbGllbnQg
Y2VydHMwgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGBAMI2MXWSb8TZnCLVNYJ+
19b4noxRmaR1W2zUxl4aTMfiPt9cK06lNY39EPBfjmb7hjxD76w8fLoV/aZ0gOgd
JXFRZvIg7SyM7QVFma0AJAIZayes+ba1odEmBEi378g0mLrjCLqZtBVHfvJxL/6x
6/flSTAn/+09vtELvvLWBePZAgMBAAGjEDAOMAwGA1UdEwQFMAMBAf8wDQYJKoZI
hvcNAQEFBQADgYEAlGC24V2TsiSlo9RIboBZTZqd0raUpKkmVbkwKyqcmecoFfCI
TCmoyJLYyUL5/e3dtn/cGDcaqxaO3qxnstxVEMSrlCGfZdZJ2oouXZMpDy9CkeOM
ypCCx9pc4EmP3mvu64f21+dNCXlhM36pZ1IokeS5jk2FIHUda+m5jlk5o6I=
-----END CERTIFICATE-----
......@@ -2,7 +2,7 @@ use strict;
use warnings;
use PostgresNode;
use TestLib;
use Test::More tests => 38;
use Test::More tests => 40;
use ServerSetup;
use File::Copy;
......@@ -239,3 +239,11 @@ test_connect_fails(
test_connect_fails(
"user=ssltestuser sslcert=ssl/client-revoked.crt sslkey=ssl/client-revoked.key"
);
# 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=ssl/client.key sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR";
test_connect_ok("sslmode=require sslcert=ssl/client+client_ca.crt");
test_connect_fails("sslmode=require sslcert=ssl/client.crt");
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment