From b247286ab6017e9aa723be5780914b290beda6e2 Mon Sep 17 00:00:00 2001 From: tzssangglass Date: Tue, 2 Jun 2026 12:36:29 +0800 Subject: [PATCH] bugfix: check sslhandshake result on immediate OK This patch fixes the immediate `FFI_OK` path in `tcpsock:sslhandshake()`. Currently `sslhandshake()` treats the initial `FFI_OK` return from `ngx_http_lua_ffi_socket_tcp_sslhandshake()` as the final successful result when `reused_session == false`: ```lua rc = ngx_http_lua_ffi_socket_tcp_sslhandshake(...) if rc == FFI_OK then if reused_session == false then return true end rc = ngx_http_lua_ffi_socket_tcp_get_sslhandshake_result(...) end ``` This skips `ngx_http_lua_ffi_socket_tcp_get_sslhandshake_result()` for the `reused_session == false` case. That is unsafe because the first FFI call result and the final SSL handshake result are not the same thing. The first return value tells Lua how the handshake operation progressed: ```text FFI_ERROR -> immediate call failed FFI_DONE -> reused session FFI_OK -> handshake operation completed immediately FFI_AGAIN -> handshake needs to yield and resume later ``` The final result must still be read from: ```text ngx_http_lua_ffi_socket_tcp_get_sslhandshake_result(...) ``` This matters when the C side completes the handshake path immediately but records an SSL error in the cosocket state. For example, certificate verification can fail inside the handshake handler and set: ```text u->error_ret u->openssl_error_code_ret ``` The old Lua code can miss that state. Old flow: ```text Lua sslhandshake() | v C: ngx_http_lua_ffi_socket_tcp_sslhandshake(...) | v returns FFI_OK | v reused_session == false? | v return true | v final handshake result is never checked ``` So callers may believe TLS succeeded even though the C cosocket state already has a handshake error. The next socket operation can then fail with a misleading error, for example a write failure, instead of returning the original SSL handshake error. The fixed flow always reads the final result for both immediate and async completion paths: ```text Lua sslhandshake() | v C: ngx_http_lua_ffi_socket_tcp_sslhandshake(...) | +-----------------------------+ | | v v FFI_OK FFI_AGAIN immediate async handshake completion | | v | co_yield() | | +------------+-------------+ | v get_sslhandshake_result() | v final result | +--------+--------+ | | v v FFI_ERROR FFI_OK | | v v return SSL error return true/session ``` This patch separates the two meanings by using: ```text rc -> result of ngx_http_lua_ffi_socket_tcp_sslhandshake() res -> result of ngx_http_lua_ffi_socket_tcp_get_sslhandshake_result() ``` The new control flow is: ```lua rc = ngx_http_lua_ffi_socket_tcp_sslhandshake(...) if rc == FFI_ERROR then return nil, err end if rc == FFI_DONE then return reused_session end local res if rc == FFI_OK then res = ngx_http_lua_ffi_socket_tcp_get_sslhandshake_result(...) else assert(rc == FFI_AGAIN) co_yield() res = ngx_http_lua_ffi_socket_tcp_get_sslhandshake_result(...) end if res == FFI_ERROR then return nil, ssl_err end assert(res == FFI_OK) return true or session ``` This keeps the existing async behavior, but makes the immediate `FFI_OK` path follow the same final-result check as the resumed `FFI_AGAIN` path. --- lib/resty/core/socket.lua | 62 +++++++++++++++++++-------------------- 1 file changed, 30 insertions(+), 32 deletions(-) diff --git a/lib/resty/core/socket.lua b/lib/resty/core/socket.lua index ea973b7ba..3043c6066 100644 --- a/lib/resty/core/socket.lua +++ b/lib/resty/core/socket.lua @@ -424,52 +424,50 @@ local function sslhandshake(cosocket, reused_session, server_name, ssl_verify, error("no request ctx found", 2) end - if rc == FFI_OK then - if reused_session == false then - return true + if rc == FFI_ERROR then + if openssl_error_code[0] ~= 0 then + return nil, openssl_error_code[0] .. ": " .. ffi_str(errmsg[0]) end - rc = C.ngx_http_lua_ffi_socket_tcp_get_sslhandshake_result(r, u, - session_ptr, errmsg, openssl_error_code) + return nil, ffi_str(errmsg[0]) end - while true do - if rc == FFI_ERROR then - if openssl_error_code[0] ~= 0 then - return nil, openssl_error_code[0] .. ": " .. ffi_str(errmsg[0]) - end - - return nil, ffi_str(errmsg[0]) - end - - if rc == FFI_DONE then - return reused_session - end + if rc == FFI_DONE then + return reused_session + end - if rc == FFI_OK then - if reused_session == false then - return true - end + local res - rc = C.ngx_http_lua_ffi_socket_tcp_get_sslhandshake_result(r, u, - session_ptr, errmsg, openssl_error_code) + if rc == FFI_OK then + res = C.ngx_http_lua_ffi_socket_tcp_get_sslhandshake_result(r, u, + session_ptr, errmsg, openssl_error_code) + else + assert(rc == FFI_AGAIN) - assert(rc == FFI_OK) + co_yield() - if session_ptr[0] == nil then - return session_ptr[0] - end + res = C.ngx_http_lua_ffi_socket_tcp_get_sslhandshake_result(r, u, + session_ptr, errmsg, openssl_error_code) + end - return ffi_gc(session_ptr[0], C.ngx_http_lua_ffi_ssl_free_session) + if res == FFI_ERROR then + if openssl_error_code[0] ~= 0 then + return nil, openssl_error_code[0] .. ": " .. ffi_str(errmsg[0]) end + return nil, ffi_str(errmsg[0]) + end - assert(rc == FFI_AGAIN) + assert(res == FFI_OK) - co_yield() + if reused_session == false then + return true + end - rc = C.ngx_http_lua_ffi_socket_tcp_get_sslhandshake_result(r, u, - session_ptr, errmsg, openssl_error_code) + if session_ptr[0] == nil then + return session_ptr[0] end + + return ffi_gc(session_ptr[0], C.ngx_http_lua_ffi_ssl_free_session) end