From 6fd713ca0f4f0a6deee3a0fa7e1051096a97ac81 Mon Sep 17 00:00:00 2001 From: Zhefeng Chen Date: Fri, 6 Sep 2024 20:39:33 +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_stream_lua_socket_tcp.c | 19 +++-------- t/058-tcp-socket.t | 58 ++++++++++++++++++++++++++++++++- 2 files changed, 61 insertions(+), 16 deletions(-) diff --git a/src/ngx_stream_lua_socket_tcp.c b/src/ngx_stream_lua_socket_tcp.c index 4d076e97..c5b33df5 100644 --- a/src/ngx_stream_lua_socket_tcp.c +++ b/src/ngx_stream_lua_socket_tcp.c @@ -5595,8 +5595,7 @@ ngx_stream_lua_socket_keepalive_close_handler(ngx_event_t *ev) ngx_stream_lua_socket_pool_t *spool; int n; - int err; - char buf[1]; + unsigned char buf[1]; ngx_connection_t *c; c = ev->data; @@ -5618,20 +5617,10 @@ ngx_stream_lua_socket_keepalive_close_handler(ngx_event_t *ev) "stream lua tcp socket keepalive close handler " "check stale events"); - n = recv(c->fd, buf, 1, MSG_PEEK); - err = ngx_socket_errno; -#if (NGX_STREAM_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 /* NGX_STREAM_SSL */ + /* 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 d0593fce..adc09b11 100644 --- a/t/058-tcp-socket.t +++ b/t/058-tcp-socket.t @@ -4,12 +4,13 @@ use Test::Nginx::Socket::Lua::Stream; repeat_each(2); -plan tests => repeat_each() * 221; +plan tests => repeat_each() * 224; 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'; @@ -3545,3 +3546,58 @@ lua tcp socket calling receiveany() method to read at most 7 bytes --- error_log shutdown on a not connected socket: closed + + + +=== TEST 68: setkeepalive with TLSv1.3 +--- skip_openssl: 3: < 1.1.1 +--- stream_config + server { + listen unix:$TEST_NGINX_HTML_DIR/nginx.sock ssl; + ssl_certificate ../../cert/test_ecdsa.crt; + ssl_certificate_key ../../cert/test_ecdsa.key; + ssl_protocols TLSv1.3; + content_by_lua_block { + local sock = assert(ngx.req.socket(true)) + local data + while true do + data = assert(sock:receive()) + assert(data == "hello") + end + } + } +--- stream_server_config + 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 + } +--- stream_response +connected: 1 +setkeepalive: 1 +--- no_error_log +[error]