Skip to content

Commit

Permalink
bugfix: setkeepalive failure on TLSv1.3
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
catbro666 committed Sep 5, 2024
1 parent 8ec4f0b commit cdc6025
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 15 deletions.
19 changes: 4 additions & 15 deletions src/ngx_http_lua_socket_tcp.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) {
Expand Down
65 changes: 65 additions & 0 deletions t/058-tcp-socket.t
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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]

0 comments on commit cdc6025

Please sign in to comment.