From cdc602581fda0649fb1204c8b1bb53818303b889 Mon Sep 17 00:00:00 2001
From: Zhefeng Chen <chzf_zju@163.com>
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..87a4f03ff5 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
+                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]