bugfix: skipped 'ssl_session_store_by_lua*' and 'ssl_session_fetch_by_lua*' when using TLS 1.3.

spacewander · thibaultcha · commit d3dbc0c8102a · 2019-07-17T13:35:13.000-07:00
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 &lt;thibaultcha@me.com&gt;
diff --git a/src/ngx_http_lua_ssl_session_fetchby.c b/src/ngx_http_lua_ssl_session_fetchby.c
@@ -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;
@@ -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);
 
diff --git a/src/ngx_http_lua_ssl_session_storeby.c b/src/ngx_http_lua_ssl_session_storeby.c
@@ -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;
@@ -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);
 
diff --git a/t/142-ssl-session-store.t b/t/142-ssl-session-store.t
@@ -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]
diff --git a/t/143-ssl-session-fetch.t b/t/143-ssl-session-fetch.t
@@ -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]
