Skip to content

Commit

Permalink
bugfix: skipped 'ssl_session_store_by_lua*' and 'ssl_session_fetch_by…
Browse files Browse the repository at this point in the history
…_lua*' when using TLS 1.3.

Previously, we used the OpenSSL 1.1.1 ClientHello callback to do ssl
session fetching non-blockingly. However, this way cannot handle an edge
case: the ssl session resumption via session ticket might fail, and the
client fallbacks to session ID resumption. The ClientHello callback is
run too early to know if the client will fallback to use session ID
resumption.

Therefore, we have to take back the OpenSSL sess_set_get_cb_yield patch
and upgrade it to adapt OpenSSL 1.1.1, which was done in our
openresty/openresty repository.

Now, this means that for the time being, we must skip
`ssl_session_(fetch|store)_by_lua*` for TLS 1.3. It is possible to
support PSK with session ID in TLS 1.3., but we need to modify a number
of functions to pass the result up, which will make the patch too
complex to maintain. Since PSK with session ticket is supported,
supporting PSK with session ID is not so profitable. If someone needs
this feature, they can contribute it themselves.

Thanks Yongjian Xu and crasyangel for their help.

Signed-off-by: Thibault Charbonnier <thibaultcha@me.com>
  • Loading branch information
spacewander authored and thibaultcha committed Jul 17, 2019
1 parent 2014dd8 commit d3dbc0c
Show file tree
Hide file tree
Showing 4 changed files with 226 additions and 0 deletions.
15 changes: 15 additions & 0 deletions src/ngx_http_lua_ssl_session_fetchby.c
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,9 @@ ngx_http_lua_ssl_sess_fetch_handler(ngx_ssl_conn_t *ssl_conn,
#endif
u_char *id, int len, int *copy)
{
#if defined(NGX_SSL_TLSv1_3) && defined(TLS1_3_VERSION)
int tls_version;
#endif
lua_State *L;
ngx_int_t rc;
ngx_connection_t *c, *fc = NULL;
Expand All @@ -198,6 +201,18 @@ ngx_http_lua_ssl_sess_fetch_handler(ngx_ssl_conn_t *ssl_conn,

c = ngx_ssl_get_connection(ssl_conn);

#if defined(NGX_SSL_TLSv1_3) && defined(TLS1_3_VERSION)
tls_version = SSL_version(ssl_conn);

if (tls_version >= TLS1_3_VERSION) {
ngx_log_debug1(NGX_LOG_DEBUG_HTTP, c->log, 0,
"ssl_session_fetch_by_lua*: skipped since "
"TLS version >= 1.3 (%xd)", tls_version);

return 0;
}
#endif

ngx_log_debug1(NGX_LOG_DEBUG_HTTP, c->log, 0,
"ssl session fetch: connection reusable: %ud", c->reusable);

Expand Down
15 changes: 15 additions & 0 deletions src/ngx_http_lua_ssl_session_storeby.c
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,9 @@ int
ngx_http_lua_ssl_sess_store_handler(ngx_ssl_conn_t *ssl_conn,
ngx_ssl_session_t *sess)
{
#if defined(NGX_SSL_TLSv1_3) && defined(TLS1_3_VERSION)
int tls_version;
#endif
const u_char *sess_id;
unsigned int sess_id_len;
lua_State *L;
Expand All @@ -189,6 +192,18 @@ ngx_http_lua_ssl_sess_store_handler(ngx_ssl_conn_t *ssl_conn,

c = ngx_ssl_get_connection(ssl_conn);

#if defined(NGX_SSL_TLSv1_3) && defined(TLS1_3_VERSION)
tls_version = SSL_version(ssl_conn);

if (tls_version >= TLS1_3_VERSION) {
ngx_log_debug1(NGX_LOG_DEBUG_HTTP, c->log, 0,
"ssl_session_store_by_lua*: skipped since "
"TLS version >= 1.3 (%xd)", tls_version);

return 0;
}
#endif

ngx_log_debug1(NGX_LOG_DEBUG_HTTP, c->log, 0,
"ssl session store: connection reusable: %ud", c->reusable);

Expand Down
62 changes: 62 additions & 0 deletions t/142-ssl-session-store.t
Original file line number Diff line number Diff line change
Expand Up @@ -901,3 +901,65 @@ ssl_session_store_by_lua_block:1: ssl session store by lua is running!
--- no_error_log
[error]
[alert]



=== TEST 13: ssl_session_store_by_lua* is skipped when using TLSv1.3
--- skip_openssl: 6: < 1.1.1
--- http_config
ssl_session_store_by_lua_block { ngx.log(ngx.ERR, "ssl_session_store_by_lua* is running!") }
server {
listen unix:$TEST_NGINX_HTML_DIR/nginx.sock ssl;
server_name test.com;
ssl_certificate $TEST_NGINX_CERT_DIR/cert/test.crt;
ssl_certificate_key $TEST_NGINX_CERT_DIR/cert/test.key;
ssl_session_tickets off;
ssl_protocols TLSv1.3;
server_tokens off;
}
--- config
server_tokens off;
lua_ssl_trusted_certificate $TEST_NGINX_CERT_DIR/cert/test.crt;
lua_ssl_protocols TLSv1.3;

location /t {
content_by_lua_block {
do
local sock = ngx.socket.tcp()

sock:settimeout(5000)

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 sess, err = sock:sslhandshake(nil, "test.com", true)
if not sess then
ngx.say("failed to do SSL handshake: ", err)
return
end

ngx.say("ssl handshake: ", type(sess))

local ok, err = sock:close()
ngx.say("close: ", ok, " ", err)
end -- do
-- collectgarbage()
}
}
--- request
GET /t
--- response_body
connected: 1
ssl handshake: userdata
close: 1 nil
--- error_log eval
qr/ssl_session_store_by_lua\*: skipped since TLS version >= 1\.3 \(\d+\)/
--- no_error_log
[error]
[alert]
[emerg]
134 changes: 134 additions & 0 deletions t/143-ssl-session-fetch.t
Original file line number Diff line number Diff line change
Expand Up @@ -1219,3 +1219,137 @@ GET /t
[error]
[alert]
[emerg]



=== TEST 15: ssl_session_fetch_by_lua* is skipped when session ticket is provided
--- http_config
ssl_session_fetch_by_lua_block { ngx.log(ngx.ERR, "ssl_session_fetch_by_lua* is running!") }
server {
listen unix:$TEST_NGINX_HTML_DIR/nginx.sock ssl;
server_name test.com;
ssl_certificate $TEST_NGINX_CERT_DIR/cert/test.crt;
ssl_certificate_key $TEST_NGINX_CERT_DIR/cert/test.key;
server_tokens off;
}
--- config
server_tokens off;
lua_ssl_trusted_certificate $TEST_NGINX_CERT_DIR/cert/test.crt;

location /t {
content_by_lua_block {
do
local sock = ngx.socket.tcp()

sock:settimeout(5000)

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 sess, err = sock:sslhandshake(package.loaded.session, "test.com", true)
if not sess then
ngx.say("failed to do SSL handshake: ", err)
return
end

ngx.say("ssl handshake: ", type(sess))

package.loaded.session = sess

local ok, err = sock:close()
ngx.say("close: ", ok, " ", err)
end -- do
-- collectgarbage()
}
}
--- request
GET /t
--- response_body
connected: 1
ssl handshake: userdata
close: 1 nil
--- no_error_log
[warn]
[error]
[alert]
[emerg]



=== TEST 16: ssl_session_fetch_by_lua* always runs when using SSLv3 (SSLv3 does not support session tickets)
--- http_config
ssl_session_fetch_by_lua_block { print("ssl_session_fetch_by_lua* is running!") }
server {
listen unix:$TEST_NGINX_HTML_DIR/nginx.sock ssl;
server_name test.com;
ssl_certificate $TEST_NGINX_CERT_DIR/cert/test.crt;
ssl_certificate_key $TEST_NGINX_CERT_DIR/cert/test.key;
ssl_protocols SSLv3;
server_tokens off;
}
--- config
server_tokens off;
lua_ssl_trusted_certificate $TEST_NGINX_CERT_DIR/cert/test.crt;
lua_ssl_protocols SSLv3;

location /t {
content_by_lua_block {
do
local sock = ngx.socket.tcp()

sock:settimeout(5000)

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 sess, err = sock:sslhandshake(package.loaded.session, "test.com", true)
if not sess then
ngx.say("failed to do SSL handshake: ", err)
return
end

ngx.say("ssl handshake: ", type(sess))

package.loaded.session = sess

local ok, err = sock:close()
ngx.say("close: ", ok, " ", err)
end -- do
-- collectgarbage()
}
}
--- request
GET /t
--- response_body
connected: 1
ssl handshake: userdata
close: 1 nil
--- grep_error_log eval: qr/ssl_session_fetch_by_lua_block:.*?,|\bssl session fetch: connection reusable: \d+|\breusable connection: \d+/
--- grep_error_log_out eval
[
qr/\A(?:reusable connection: [01]\n)+\z/s,
qr/^reusable connection: 1
ssl session fetch: connection reusable: 1
reusable connection: 0
ssl_session_fetch_by_lua_block:1: ssl_session_fetch_by_lua\* is running!,
/m,
qr/^reusable connection: 1
ssl session fetch: connection reusable: 1
reusable connection: 0
ssl_session_fetch_by_lua_block:1: ssl_session_fetch_by_lua\* is running!,
/m,
]
--- no_error_log
[error]
[alert]
[emerg]

0 comments on commit d3dbc0c

Please sign in to comment.