From e5e1bda219c3d64112d4969f02622d02906deedf Mon Sep 17 00:00:00 2001 From: Daniel Gustafsson Date: Fri, 27 Jun 2025 19:16:11 +0200 Subject: [PATCH] libpq: Fix multithreading issues in the OpenSSL support In order to make the OpenSSL code in libpq be threadsafe the global variable used by the certificate verification callback need to be replaced with passing private data, and the call to strerror need to be replaced with strerror_r. * The callback use a new member in the Port struct for passing the error detail string, and the Port struct is in turn passed as private data in the SSL object * The strerror call is replaced with a strerror_r Author: Daniel Gustafsson Reviewed-by: Discussion: https://postgr.es/m/xxx --- src/backend/libpq/be-secure-openssl.c | 35 +++++++++++++++++++++------ src/include/libpq/libpq-be.h | 6 +++++ 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c index 64ff3ce3d6a..df6502e3e64 100644 --- a/src/backend/libpq/be-secure-openssl.c +++ b/src/backend/libpq/be-secure-openssl.c @@ -87,9 +87,6 @@ static bool ssl_is_server_start; static int ssl_protocol_version_to_openssl(int v); static const char *ssl_protocol_version_to_string(int v); -/* for passing data back from verify_cb() */ -static const char *cert_errdetail; - /* ------------------------------------------------------------ */ /* Public interface */ /* ------------------------------------------------------------ */ @@ -477,6 +474,14 @@ be_tls_open_server(Port *port) SSLerrmessage(ERR_get_error())))); return -1; } + + /* + * This is a bit of a circle motion, but we want to be able to access the + * Port object in callbacks for passing data from callback to the main + * process. + */ + SSL_set_ex_data(port->ssl, 0, port); + port->ssl_in_use = true; aloop: @@ -576,7 +581,7 @@ aloop: (errcode(ERRCODE_PROTOCOL_VIOLATION), errmsg("could not accept SSL connection: %s", SSLerrmessage(ecode)), - cert_errdetail ? errdetail_internal("%s", cert_errdetail) : 0, + port->cert_errdetail ? errdetail_internal("%s", port->cert_errdetail) : 0, give_proto_hint ? errhint("This may indicate that the client does not support any SSL protocol version between %s and %s.", ssl_min_protocol_version ? @@ -585,7 +590,11 @@ aloop: ssl_max_protocol_version ? ssl_protocol_version_to_string(ssl_max_protocol_version) : MAX_OPENSSL_TLS_VERSION) : 0)); - cert_errdetail = NULL; + if (port->cert_errdetail) + { + pfree(port->cert_errdetail); + port->cert_errdetail = NULL; + } break; case SSL_ERROR_ZERO_RETURN: ereport(COMMERROR, @@ -1209,6 +1218,8 @@ verify_cb(int ok, X509_STORE_CTX *ctx) const char *errstring; StringInfoData str; X509 *cert; + SSL *ssl; + Port *port; if (ok) { @@ -1221,6 +1232,13 @@ verify_cb(int ok, X509_STORE_CTX *ctx) errcode = X509_STORE_CTX_get_error(ctx); errstring = X509_verify_cert_error_string(errcode); + /* + * Extract the current SSL and Port object to use for passing error detail + * back from the callback. + */ + ssl = X509_STORE_CTX_get_ex_data(ctx, SSL_get_ex_data_X509_STORE_CTX_idx()); + port = (Port *) SSL_get_ex_data(ssl, 0); + initStringInfo(&str); appendStringInfo(&str, _("Client certificate verification failed at depth %d: %s."), @@ -1271,7 +1289,7 @@ verify_cb(int ok, X509_STORE_CTX *ctx) } /* Store our detail message to be logged later. */ - cert_errdetail = str.data; + port->cert_errdetail = str.data; return ok; } @@ -1499,7 +1517,10 @@ SSLerrmessage(unsigned long ecode) */ #ifdef ERR_SYSTEM_ERROR if (ERR_SYSTEM_ERROR(ecode)) - return strerror(ERR_GET_REASON(ecode)); + { + strerror_r(ERR_GET_REASON(ecode), errbuf, sizeof(errbuf)); + return errbuf; + } #endif /* No choice but to report the numeric ecode */ diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h index d6e671a6382..2d25cd3aaa5 100644 --- a/src/include/libpq/libpq-be.h +++ b/src/include/libpq/libpq-be.h @@ -159,6 +159,12 @@ typedef struct Port */ char *application_name; + /* + * Storage for passing certificate verification error logging from the + * callback. + */ + char *cert_errdetail; + /* * Information that needs to be held during the authentication cycle. */ -- 2.39.3 (Apple Git-146)