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 6, 2024
1 parent 69f0cd7 commit f0fabd5
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 15 deletions.
18 changes: 4 additions & 14 deletions src/ngx_stream_lua_socket_tcp.c
Original file line number Diff line number Diff line change
Expand Up @@ -5596,7 +5596,7 @@ ngx_stream_lua_socket_keepalive_close_handler(ngx_event_t *ev)

int n;
int err;
char buf[1];
unsigned char buf[1];
ngx_connection_t *c;

c = ev->data;
Expand All @@ -5618,20 +5618,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) {
Expand Down
58 changes: 57 additions & 1 deletion t/058-tcp-socket.t
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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]

0 comments on commit f0fabd5

Please sign in to comment.