From 38a701d88b14f0747003c4e893d9fb13f51639ca Mon Sep 17 00:00:00 2001 From: Sergey Kandaurov Date: Thu, 6 Nov 2025 17:30:41 +0400 Subject: [PATCH] SSL: ngx_ssl_set_client_hello_callback() error handling. The function interface is changed to follow a common approach to other functions used to setup SSL_CTX, with an exception of "ngx_conf_t *cf" since it is not bound to nginx configuration. This is required to report and propagate SSL_CTX_set_ex_data() errors, as reminded by Coverity (CID 1668589). --- src/event/ngx_event_openssl.c | 25 +++++++++++++++++++------ src/event/ngx_event_openssl.h | 2 +- src/http/modules/ngx_http_ssl_module.c | 4 +++- src/stream/ngx_stream_ssl_module.c | 4 +++- 4 files changed, 26 insertions(+), 9 deletions(-) diff --git a/src/event/ngx_event_openssl.c b/src/event/ngx_event_openssl.c index 5175d7a7e..4f07894ff 100644 --- a/src/event/ngx_event_openssl.c +++ b/src/event/ngx_event_openssl.c @@ -1872,21 +1872,34 @@ ngx_ssl_new_client_session(ngx_ssl_conn_t *ssl_conn, ngx_ssl_session_t *sess) } -void -ngx_ssl_set_client_hello_callback(SSL_CTX *ssl_ctx, - ngx_ssl_client_hello_arg *cb) +ngx_int_t +ngx_ssl_set_client_hello_callback(ngx_ssl_t *ssl, ngx_ssl_client_hello_arg *cb) { #ifdef SSL_CLIENT_HELLO_SUCCESS - SSL_CTX_set_client_hello_cb(ssl_ctx, ngx_ssl_client_hello_callback, NULL); - SSL_CTX_set_ex_data(ssl_ctx, ngx_ssl_client_hello_arg_index, cb); + SSL_CTX_set_client_hello_cb(ssl->ctx, ngx_ssl_client_hello_callback, NULL); + + if (SSL_CTX_set_ex_data(ssl->ctx, ngx_ssl_client_hello_arg_index, cb) == 0) + { + ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0, + "SSL_CTX_set_ex_data() failed"); + return NGX_ERROR; + } #elif defined OPENSSL_IS_BORINGSSL SSL_CTX_set_select_certificate_cb(ssl_ctx, ngx_ssl_select_certificate); - SSL_CTX_set_ex_data(ssl_ctx, ngx_ssl_client_hello_arg_index, cb); + + if (SSL_CTX_set_ex_data(ssl->ctx, ngx_ssl_client_hello_arg_index, cb) == 0) + { + ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0, + "SSL_CTX_set_ex_data() failed"); + return NGX_ERROR; + } #endif + + return NGX_OK; } diff --git a/src/event/ngx_event_openssl.h b/src/event/ngx_event_openssl.h index ae0e173de..a156c4bb9 100644 --- a/src/event/ngx_event_openssl.h +++ b/src/event/ngx_event_openssl.h @@ -286,7 +286,7 @@ ngx_int_t ngx_ssl_session_ticket_keys(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_array_t *paths); ngx_int_t ngx_ssl_session_cache_init(ngx_shm_zone_t *shm_zone, void *data); -void ngx_ssl_set_client_hello_callback(SSL_CTX *ssl_ctx, +ngx_int_t ngx_ssl_set_client_hello_callback(ngx_ssl_t *ssl, ngx_ssl_client_hello_arg *cb); #ifdef SSL_CLIENT_HELLO_SUCCESS int ngx_ssl_client_hello_callback(ngx_ssl_conn_t *ssl_conn, int *ad, void *arg); diff --git a/src/http/modules/ngx_http_ssl_module.c b/src/http/modules/ngx_http_ssl_module.c index dc82472d3..c71a5de08 100644 --- a/src/http/modules/ngx_http_ssl_module.c +++ b/src/http/modules/ngx_http_ssl_module.c @@ -758,7 +758,9 @@ ngx_http_ssl_merge_srv_conf(ngx_conf_t *cf, void *parent, void *child) { static ngx_ssl_client_hello_arg cb = { ngx_http_ssl_servername }; - ngx_ssl_set_client_hello_callback(conf->ssl.ctx, &cb); + if (ngx_ssl_set_client_hello_callback(&conf->ssl, &cb) != NGX_OK) { + return NGX_CONF_ERROR; + } if (SSL_CTX_set_tlsext_servername_callback(conf->ssl.ctx, ngx_http_ssl_servername) diff --git a/src/stream/ngx_stream_ssl_module.c b/src/stream/ngx_stream_ssl_module.c index b4a722a68..6a5160f27 100644 --- a/src/stream/ngx_stream_ssl_module.c +++ b/src/stream/ngx_stream_ssl_module.c @@ -1008,7 +1008,9 @@ ngx_stream_ssl_merge_srv_conf(ngx_conf_t *cf, void *parent, void *child) { static ngx_ssl_client_hello_arg cb = { ngx_stream_ssl_servername }; - ngx_ssl_set_client_hello_callback(conf->ssl.ctx, &cb); + if (ngx_ssl_set_client_hello_callback(&conf->ssl, &cb) != NGX_OK) { + return NGX_CONF_ERROR; + } SSL_CTX_set_tlsext_servername_callback(conf->ssl.ctx, ngx_stream_ssl_servername);