From 3efffddd95e564fe10f59e1de45afc2b551a5cba Mon Sep 17 00:00:00 2001 From: Andrey Suvorov Date: Wed, 26 May 2021 11:11:58 -0700 Subject: [PATCH] Fixing crash during TLS connection shutdown. A crash was caused by an incorrect timer handler nxt_h1p_idle_timeout() if SSL_shutdown() returned SSL_ERROR_WANT_READ/SSL_ERROR_WANT_WRITE. The flag SSL_RECEIVED_SHUTDOWN is used to avoid getting SSL_ERROR_WANT_READ, so the server won't wait for a close notification from a client. For SSL_ERROR_WANT_WRITE, a correct timer handler is set up. --- docs/changes.xml | 6 ++++++ src/nxt_h1proto.c | 0 src/nxt_openssl.c | 32 ++++++++++++++++++++++++++------ src/nxt_router.c | 1 + src/nxt_tls.h | 2 ++ 5 files changed, 35 insertions(+), 6 deletions(-) mode change 100644 => 100755 src/nxt_h1proto.c diff --git a/docs/changes.xml b/docs/changes.xml index 5717f1bd..cbe6269a 100644 --- a/docs/changes.xml +++ b/docs/changes.xml @@ -68,6 +68,12 @@ compatibility with Ruby 3.0. + + +the router process could crash while closing TLS connection. + + + a segmentation fault might have occurred in the PHP module if diff --git a/src/nxt_h1proto.c b/src/nxt_h1proto.c old mode 100644 new mode 100755 diff --git a/src/nxt_openssl.c b/src/nxt_openssl.c index 9b86150f..af4dc24a 100644 --- a/src/nxt_openssl.c +++ b/src/nxt_openssl.c @@ -66,6 +66,8 @@ static void nxt_openssl_conn_io_shutdown(nxt_task_t *task, void *obj, void *data); static nxt_int_t nxt_openssl_conn_test_error(nxt_task_t *task, nxt_conn_t *c, int ret, nxt_err_t sys_err, nxt_openssl_io_t io); +static void nxt_openssl_conn_io_shutdown_timeout(nxt_task_t *task, void *obj, + void *data); static void nxt_cdecl nxt_openssl_conn_error(nxt_task_t *task, nxt_err_t err, const char *fmt, ...); static nxt_uint_t nxt_openssl_log_error_level(nxt_err_t err); @@ -839,11 +841,7 @@ nxt_openssl_conn_init(nxt_task_t *task, nxt_tls_conf_t *conf, nxt_conn_t *c) c->sendfile = NXT_CONN_SENDFILE_OFF; nxt_openssl_conn_handshake(task, c, c->socket.data); - /* - * TLS configuration might be destroyed after the TLS connection - * is established. - */ - tls->conf = NULL; + return; fail: @@ -1099,6 +1097,10 @@ nxt_openssl_conn_io_shutdown(nxt_task_t *task, void *obj, void *data) SSL_set_quiet_shutdown(s, quiet); + if (tls->conf->no_wait_shutdown) { + mode |= SSL_RECEIVED_SHUTDOWN; + } + once = 1; for ( ;; ) { @@ -1153,7 +1155,8 @@ nxt_openssl_conn_io_shutdown(nxt_task_t *task, void *obj, void *data) break; case NXT_AGAIN: - nxt_timer_add(task->thread->engine, &c->read_timer, 5000); + c->write_timer.handler = nxt_openssl_conn_io_shutdown_timeout; + nxt_timer_add(task->thread->engine, &c->write_timer, 5000); return; default: @@ -1237,6 +1240,23 @@ nxt_openssl_conn_test_error(nxt_task_t *task, nxt_conn_t *c, int ret, } +static void +nxt_openssl_conn_io_shutdown_timeout(nxt_task_t *task, void *obj, void *data) +{ + nxt_conn_t *c; + nxt_timer_t *timer; + + timer = obj; + + nxt_debug(task, "openssl conn shutdown timeout"); + + c = nxt_write_timer_conn(timer); + + c->socket.timedout = 1; + nxt_openssl_conn_io_shutdown(task, c, NULL); +} + + static void nxt_cdecl nxt_openssl_conn_error(nxt_task_t *task, nxt_err_t err, const char *fmt, ...) { diff --git a/src/nxt_router.c b/src/nxt_router.c index c597863e..122fd523 100644 --- a/src/nxt_router.c +++ b/src/nxt_router.c @@ -2477,6 +2477,7 @@ nxt_router_tls_rpc_handler(nxt_task_t *task, nxt_port_recv_msg_t *msg, goto fail; } + tlscf->no_wait_shutdown = 1; rpc->socket_conf->tls = tlscf; } else { diff --git a/src/nxt_tls.h b/src/nxt_tls.h index c44bfe56..2a29f3ca 100644 --- a/src/nxt_tls.h +++ b/src/nxt_tls.h @@ -69,6 +69,8 @@ struct nxt_tls_conf_s { char *ca_certificate; size_t buffer_size; + + uint8_t no_wait_shutdown; /* 1 bit */ };