Skip to content

Commit cdc6025

Browse files
committed
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.
1 parent 8ec4f0b commit cdc6025

File tree

2 files changed

+69
-15
lines changed

2 files changed

+69
-15
lines changed

src/ngx_http_lua_socket_tcp.c

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5725,8 +5725,7 @@ ngx_http_lua_socket_keepalive_close_handler(ngx_event_t *ev)
57255725
ngx_http_lua_socket_pool_t *spool;
57265726

57275727
int n;
5728-
int err;
5729-
char buf[1];
5728+
unsigned char buf[1];
57305729
ngx_connection_t *c;
57315730

57325731
c = ev->data;
@@ -5747,20 +5746,10 @@ ngx_http_lua_socket_keepalive_close_handler(ngx_event_t *ev)
57475746
ngx_log_debug0(NGX_LOG_DEBUG_HTTP, ev->log, 0,
57485747
"lua tcp socket keepalive close handler check stale events");
57495748

5750-
n = recv(c->fd, buf, 1, MSG_PEEK);
5751-
err = ngx_socket_errno;
5752-
#if (NGX_HTTP_SSL)
5753-
/* ignore ssl protocol data like change cipher spec */
5754-
if (n == 1 && c->ssl != NULL) {
5755-
n = c->recv(c, (unsigned char *) buf, 1);
5756-
if (n == NGX_AGAIN) {
5757-
n = -1;
5758-
err = NGX_EAGAIN;
5759-
}
5760-
}
5761-
#endif
5749+
/* consume the possible ssl-layer data implicitly */
5750+
n = c->recv(c, buf, 1);
57625751

5763-
if (n == -1 && err == NGX_EAGAIN) {
5752+
if (n == NGX_AGAIN) {
57645753
/* stale event */
57655754

57665755
if (ngx_handle_read_event(c->read, 0) != NGX_OK) {

t/058-tcp-socket.t

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ our $HtmlDir = html_dir;
1010

1111
$ENV{TEST_NGINX_MEMCACHED_PORT} ||= 11211;
1212
$ENV{TEST_NGINX_RESOLVER} ||= '8.8.8.8';
13+
$ENV{TEST_NGINX_HTML_DIR} ||= html_dir();
1314

1415
#log_level 'warn';
1516
log_level 'debug';
@@ -4497,3 +4498,67 @@ reused times: 3, setkeepalive err: closed
44974498
--- no_error_log
44984499
[error]
44994500
--- skip_eval: 3: $ENV{TEST_NGINX_EVENT_TYPE} && $ENV{TEST_NGINX_EVENT_TYPE} ne 'epoll'
4501+
4502+
4503+
4504+
=== TEST 74: setkeepalive with TLSv1.3
4505+
--- skip_openssl: 3: < 1.1.1
4506+
--- stream_server_config
4507+
listen unix:$TEST_NGINX_HTML_DIR/nginx.sock ssl;
4508+
ssl_certificate ../../cert/test.crt;
4509+
ssl_certificate_key ../../cert/test.key;
4510+
ssl_protocols TLSv1.3;
4511+
4512+
content_by_lua_block {
4513+
local sock = assert(ngx.req.socket(true))
4514+
local data
4515+
while true do
4516+
data = assert(sock:receive())
4517+
assert(data == "hello")
4518+
end
4519+
}
4520+
--- config
4521+
location /test {
4522+
lua_ssl_protocols TLSv1.3;
4523+
content_by_lua_block {
4524+
local sock = ngx.socket.tcp()
4525+
sock:settimeout(2000)
4526+
4527+
local ok, err = sock:connect("unix:$TEST_NGINX_HTML_DIR/nginx.sock")
4528+
if not ok then
4529+
ngx.say("failed to connect: ", err)
4530+
return
4531+
end
4532+
4533+
ngx.say("connected: ", ok)
4534+
4535+
local ok, err = sock:sslhandshake(false, nil, false)
4536+
if not ok then
4537+
ngx.say("failed to sslhandshake: ", err)
4538+
return
4539+
end
4540+
4541+
local ok, err = sock:send("hello\n")
4542+
if not ok then
4543+
ngx.say("failed to send: ", err)
4544+
return
4545+
end
4546+
4547+
-- sleep a while to make sure the NewSessionTicket message has arrived
4548+
ngx.sleep(1)
4549+
4550+
local ok, err = sock:setkeepalive()
4551+
if not ok then
4552+
ngx.say("failed to setkeepalive: ", err)
4553+
else
4554+
ngx.say("setkeepalive: ", ok)
4555+
end
4556+
}
4557+
}
4558+
--- request
4559+
GET /test
4560+
--- response_body
4561+
connected: 1
4562+
setkeepalive: 1
4563+
--- no_error_log
4564+
[error]

0 commit comments

Comments
 (0)