Re: BUG #14329: libpq doesn't send complete client certificate chain on first SSL connection

On 10/04/2016 02:08 PM, Zuk, Kacper wrote:
> On 9/21/16, 10:06 AM, "Heikki Linnakangas" <hlinnaka(at)gmail(dot)com on behalf of hlinnaka(at)iki(dot)fi> wrote:
>> Setting up a test environment with the required certificates and CAs is
>> a bit tedious. Would you be interested in adding a test case for this in
>> the SSL test suite, in src/test/ssl, and posting a patch for that? I can
>> take a stab at fixing this, but having a test case ready would give me a
>> head start.
>
> I’ve attached the patch that adds a test case. Please let me know if I should send it to pgsql-hackers instead. Remember to run make sslfiles, as I didn’t include generated certificate in patch.

Thanks!

Now that I look closer into this, I can see more trouble:

1. Since the SSL context is shared between all connections and all
threads, there's a race condition if two threads try to open a
connection at the same time. The SSL_CTX_use_certificate_chain_file()
call, and SSL_CTX_load_verify_locations() later in the function, are
protected by the global mutex, but we only hold the mutex for the
duration of those calls. It doesn't protect from the fact that while one
thread is opening a connection with one set of options, another thread
opens a connection with different options. They might get mixed up.
That's a problem if the connections use different sslcert or sslrootcert
settings.

2. Even with a single thread, using different sslrootcert options in
different PQconnectdb() calls fails outright, with error:

SSL error: block type is not 01

I got that with a test program that simply calls:

PQconnectdb(<connection string 1>);
PQfinish();
PQconnectdb(<connection string 2>);
PQfinish();

when the two connection strings point to different servers, with
different certificates signed by different root CA, and the connection
strings have different (correct) sslrootcert options for the servers.
Either PQconnectdb() call works separately, but put them in the same
program, you get that error.

I don't understand why that second error happens. I don't think we're
doing anything illegal by reusing the same SSL_CTX object for two
different connections.

I'm starting to feel that using the same SSL_CTX object for multiple
connections is just too fragile. Perhaps we could share one SSL_CTX
object for all the connections with no sslcert and no sslrootcert, but
I'm not sure if even that is worth it.

In quick testing, calling SSL_CTX_new() for each connection adds about
3% of overhead to establishing a new connection, with the default
OpenSSL settings (seems to use ECDHE-RSA-AES256-GCM-SHA384 cipher here).
I also tested memory usage with a program that opens 10000 connections,
and it used about 15% more memory, when SSL_CTX_new() is called for each
connection. I think that's acceptable. Barring objections, I'm going to
write a patch to use a separate SSL context for every connection.

- Heikki