pgsql: Set libcrypto callbacks for all connection threads in libpq

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: pgsql-committers(at)lists(dot)postgresql(dot)org
Subject: pgsql: Set libcrypto callbacks for all connection threads in libpq
Date: 2021-03-11 08:16:37
Message-ID: E1lKGUn-00005M-PS@gemulon.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

Set libcrypto callbacks for all connection threads in libpq

Based on an analysis of the OpenSSL code with Jacob, moving to EVP for
the cryptohash computations makes necessary the setup of the libcrypto
callbacks that were getting set only for SSL connections, but not for
connections without SSL. Not setting the callbacks makes the use of
threads potentially unsafe for connections calling cryptohashes during
authentication, like MD5 or SCRAM, if a failure happens during a
cryptohash computation. The logic setting the libssl and libcrypto
states is then split into two parts, both using the same locking, with
libcrypto being set up for SSL and non-SSL connections, while SSL
connections set any libssl state afterwards as needed.

Prior to this commit, only SSL connections would have set libcrypto
callbacks that are necessary to ensure a proper thread locking when
using multiple concurrent threads in libpq (ENABLE_THREAD_SAFETY). Note
that this is only required for OpenSSL 1.0.2 and 1.0.1 (oldest version
supported on HEAD), as 1.1.0 has its own internal locking and it has
dropped support for CRYPTO_set_locking_callback().

Tests with up to 300 threads with OpenSSL 1.0.1 and 1.0.2, mixing SSL
and non-SSL connection threads did not show any performance impact after
some micro-benchmarking. pgbench can be used here with -C and a
mostly-empty script (with one \set meta-command for example) to stress
authentication requests, and we have mixed that with some custom
programs for testing.

Reported-by: Jacob Champion
Author: Michael Paquier
Reviewed-by: Jacob Champion
Discussion: https://postgr.es/m/fd3ba610085f1ff54623478cf2f7adf5af193cbb.camel@vmware.com

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/2c0cefcd18161549e9e8b103f46c0f65fca84d99

Modified Files
--------------
src/interfaces/libpq/fe-connect.c | 20 +++++-
src/interfaces/libpq/fe-secure-openssl.c | 107 +++++++++++++++++++------------
src/interfaces/libpq/fe-secure.c | 7 +-
src/interfaces/libpq/libpq-int.h | 13 +++-
4 files changed, 96 insertions(+), 51 deletions(-)

Browse pgsql-committers by date

  From Date Subject
Next Message Laurenz Albe 2021-03-11 09:36:37 Re: pgsql: Drop index behind pg_upgrade test issue.
Previous Message Peter Geoghegan 2021-03-11 07:33:42 pgsql: Drop other index behind pg_upgrade test issue.