From ef7b2767098203394992468882f86f49aa709c38 Mon Sep 17 00:00:00 2001 From: Zhefeng Chen Date: Thu, 5 Sep 2024 10:43:34 +0800 Subject: [PATCH] bugfix: `setkeepalive` failure on TLSv1.3 When TLSv1.3 is used, the server may send a NewSessionTicket message after the handshake. While this message is ssl-layer data, `tcpsock:sslhandshake` does not consume it. In the implementation of `setkeepalive`, `recv` is used to confirm the connection is still open and there is no unread data in the buffer. But it treats the NewSessionTicket message as application layer data and then `setkeepalive` fails with this error `connection in dubious state`. In fact we don't need to peek here, because if the application data is read successfully then the connection is going to be closed anyway. Therefore, `c->recv` can be used instead which will consume the ssl-layer data implicitly. --- src/ngx_http_lua_socket_tcp.c | 19 +++------- t/058-tcp-socket.t | 65 +++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 15 deletions(-) diff --git a/src/ngx_http_lua_socket_tcp.c b/src/ngx_http_lua_socket_tcp.c index a0e041b017..998881d1ca 100644 --- a/src/ngx_http_lua_socket_tcp.c +++ b/src/ngx_http_lua_socket_tcp.c @@ -5725,8 +5725,7 @@ ngx_http_lua_socket_keepalive_close_handler(ngx_event_t *ev) ngx_http_lua_socket_pool_t *spool; int n; - int err; - char buf[1]; + unsigned char buf[1]; ngx_connection_t *c; c = ev->data; @@ -5747,20 +5746,10 @@ ngx_http_lua_socket_keepalive_close_handler(ngx_event_t *ev) ngx_log_debug0(NGX_LOG_DEBUG_HTTP, ev->log, 0, "lua tcp socket keepalive close handler check stale events"); - n = recv(c->fd, buf, 1, MSG_PEEK); - err = ngx_socket_errno; -#if (NGX_HTTP_SSL) - /* ignore ssl protocol data like change cipher spec */ - if (n == 1 && c->ssl != NULL) { - n = c->recv(c, (unsigned char *) buf, 1); - if (n == NGX_AGAIN) { - n = -1; - err = NGX_EAGAIN; - } - } -#endif + /* consume the possible ssl-layer data implicitly */ + n = c->recv(c, buf, 1); - if (n == -1 && err == NGX_EAGAIN) { + if (n == NGX_AGAIN) { /* stale event */ if (ngx_handle_read_event(c->read, 0) != NGX_OK) { diff --git a/t/058-tcp-socket.t b/t/058-tcp-socket.t index ef2b05f0d0..2934f85404 100644 --- a/t/058-tcp-socket.t +++ b/t/058-tcp-socket.t @@ -10,6 +10,7 @@ our $HtmlDir = html_dir; $ENV{TEST_NGINX_MEMCACHED_PORT} ||= 11211; $ENV{TEST_NGINX_RESOLVER} ||= '8.8.8.8'; +$ENV{TEST_NGINX_HTML_DIR} ||= html_dir(); #log_level 'warn'; log_level 'debug'; @@ -4497,3 +4498,67 @@ reused times: 3, setkeepalive err: closed --- no_error_log [error] --- skip_eval: 3: $ENV{TEST_NGINX_EVENT_TYPE} && $ENV{TEST_NGINX_EVENT_TYPE} ne 'epoll' + + + +=== TEST 74: setkeepalive with TLSv1.3 +--- skip_openssl: 3: < 1.1.1 +--- stream_server_config + listen unix:$TEST_NGINX_HTML_DIR/nginx.sock ssl; + ssl_certificate ../../cert/test.crt; + ssl_certificate_key ../../cert/test.key; + ssl_protocols TLSv1.3; + + content_by_lua_block { + local sock = assert(ngx.req.socket(true)) + local data + while true do + local data = assert(sock:receive()) + assert(data == "hello") + end + } +--- config + location /test { + lua_ssl_protocols TLSv1.3; + content_by_lua_block { + local sock = ngx.socket.tcp() + sock:settimeout(2000) + + local ok, err = sock:connect("unix:$TEST_NGINX_HTML_DIR/nginx.sock") + if not ok then + ngx.say("failed to connect: ", err) + return + end + + ngx.say("connected: ", ok) + + local ok, err = sock:sslhandshake(false, nil, false) + if not ok then + ngx.say("failed to sslhandshake: ", err) + return + end + + local ok, err = sock:send("hello\n") + if not ok then + ngx.say("failed to send: ", err) + return + end + + -- sleep a while to make sure the NewSessionTicket message has arrived + ngx.sleep(1) + + local ok, err = sock:setkeepalive() + if not ok then + ngx.say("failed to setkeepalive: ", err) + else + ngx.say("setkeepalive: ", ok) + end + } + } +--- request +GET /test +--- response_body +connected: 1 +setkeepalive: 1 +--- no_error_log +[error]