From dc468f4c5edb28e59d39e7d5a11244d8765667f6 Mon Sep 17 00:00:00 2001
From: Zhefeng Chen <chzf_zju@163.com>
Date: Mon, 8 Jan 2024 14:54:47 +0800
Subject: [PATCH 01/21] fix poolname to include the digest of the cert for mTLS

---
 lib/resty/http_connect.lua | 55 ++++++++++++++++++++++++++++++++------
 1 file changed, 47 insertions(+), 8 deletions(-)

diff --git a/lib/resty/http_connect.lua b/lib/resty/http_connect.lua
index d89452a..052d20e 100644
--- a/lib/resty/http_connect.lua
+++ b/lib/resty/http_connect.lua
@@ -1,8 +1,12 @@
+local ffi = require "ffi"
 local ngx_re_gmatch = ngx.re.gmatch
 local ngx_re_sub = ngx.re.sub
 local ngx_re_find = ngx.re.find
 local ngx_log = ngx.log
 local ngx_WARN = ngx.WARN
+local to_hex = require("resty.string").to_hex
+local ffi_gc = ffi.gc
+local string_format = string.format
 
 --[[
 A connection function that incorporates:
@@ -160,16 +164,51 @@ local function connect(self, options)
         proxy_port = proxy_uri_t[3]
     end
 
+    local cert_hash
+    if ssl and ssl_client_cert and ssl_client_priv_key then
+        local status, res = xpcall(function()
+            return require("resty.openssl.x509")
+        end, debug.traceback)
+
+        if status then
+            local x509 = res
+            local cert, err = x509.new(ssl_client_cert)
+            if not cert then
+              return nil, err
+            end
+            -- should not free the cdata passed in
+            ffi_gc(cert.ctx, nil)
+
+            cert_hash, err = cert:digest("sha256")
+            if cert_hash then
+              cert_hash = to_hex(cert_hash) -- convert to hex so that it's printable
+
+            else
+              return nil, err
+            end
+
+        else
+            if type(res) == "string" and ngx_re_find(res, "module 'resty\\.openssl\\.x509)' not found") then
+                ngx_log(ngx_WARN, "can't use mTLS without module `lua-resty-openssl`, falling back to non-mTLS.")
+
+            else
+                return nil, "failed to load module 'resty.openssl.x509':\n" .. res
+            end
+        end
+    end
+
     -- construct a poolname unique within proxy and ssl info
     if not poolname then
-        poolname = (request_scheme or "")
-                   .. ":" .. request_host
-                   .. ":" .. tostring(request_port)
-                   .. ":" .. tostring(ssl)
-                   .. ":" .. (ssl_server_name or "")
-                   .. ":" .. tostring(ssl_verify)
-                   .. ":" .. (proxy_uri or "")
-                   .. ":" .. (request_scheme == "https" and proxy_authorization or "")
+        poolname = string_format("%s:%s:%s:%s:%s:%s:%s:%s:%s",
+                    request_scheme or "",
+                    request_host,
+                    tostring(request_port),
+                    tostring(ssl),
+                    ssl_server_name or "",
+                    tostring(ssl_verify),
+                    proxy_uri or "",
+                    request_scheme == "https" and proxy_authorization or "",
+                    cert_hash or "")
         -- in the above we only add the 'proxy_authorization' as part of the poolname
         -- when the request is https. Because in that case the CONNECT request (which
         -- carries the authorization header) is part of the connect procedure, whereas

From 67485a144ef5a91cbcc59f9f7954bc930bc2d3d2 Mon Sep 17 00:00:00 2001
From: Zhefeng Chen <chzf_zju@163.com>
Date: Mon, 8 Jan 2024 14:56:57 +0800
Subject: [PATCH 02/21] check the private key in order to make sure the caller
 is indeed the holder of the cert

---
 lib/resty/http_connect.lua | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/lib/resty/http_connect.lua b/lib/resty/http_connect.lua
index 052d20e..91d16ba 100644
--- a/lib/resty/http_connect.lua
+++ b/lib/resty/http_connect.lua
@@ -167,11 +167,14 @@ local function connect(self, options)
     local cert_hash
     if ssl and ssl_client_cert and ssl_client_priv_key then
         local status, res = xpcall(function()
-            return require("resty.openssl.x509")
+            local x509 = require("resty.openssl.x509")
+            local pkey = require("resty.openssl.pkey")
+            return { x509, pkey }
         end, debug.traceback)
 
         if status then
-            local x509 = res
+            local x509 = res[1]
+            local pkey = res[2]
             local cert, err = x509.new(ssl_client_cert)
             if not cert then
               return nil, err
@@ -179,6 +182,19 @@ local function connect(self, options)
             -- should not free the cdata passed in
             ffi_gc(cert.ctx, nil)
 
+            local key, err = pkey.new(ssl_client_priv_key)
+            if not key then
+              return nil, err
+            end
+            -- should not free the cdata passed in
+            ffi_gc(key.ctx, nil)
+
+            -- check the private key in order to make sure the caller is indeed the holder of the cert
+            ok, err = cert:check_private_key(key)
+            if not ok then
+              return nil, "failed to match the private key with the certificate: " .. err
+            end
+
             cert_hash, err = cert:digest("sha256")
             if cert_hash then
               cert_hash = to_hex(cert_hash) -- convert to hex so that it's printable
@@ -188,11 +204,11 @@ local function connect(self, options)
             end
 
         else
-            if type(res) == "string" and ngx_re_find(res, "module 'resty\\.openssl\\.x509)' not found") then
+            if type(res) == "string" and ngx_re_find(res, "module 'resty\\.openssl\\.(x509|pkey)' not found") then
                 ngx_log(ngx_WARN, "can't use mTLS without module `lua-resty-openssl`, falling back to non-mTLS.")
 
             else
-                return nil, "failed to load module 'resty.openssl.x509':\n" .. res
+                return nil, "failed to load module 'resty.openssl.*':\n" .. res
             end
         end
     end

From 3ff72f433db6fc014a0a5189cd518cb0f24a6679 Mon Sep 17 00:00:00 2001
From: Zhefeng Chen <chzf_zju@163.com>
Date: Mon, 8 Jan 2024 16:17:40 +0800
Subject: [PATCH 03/21] add test

---
 t/20-mtls.t | 98 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 98 insertions(+)

diff --git a/t/20-mtls.t b/t/20-mtls.t
index e6395bd..f48c70e 100644
--- a/t/20-mtls.t
+++ b/t/20-mtls.t
@@ -209,3 +209,101 @@ GET /t
 --- response_body
 hello, CN=foo@example.com,O=OpenResty,ST=California,C=US
 
+
+=== TEST 4: users with different client certs should not share the same pool.
+--- SKIP
+--- http_config eval: $::mtls_http_config
+--- config eval
+"
+lua_ssl_trusted_certificate $::HtmlDir/test.crt;
+
+location /t {
+    content_by_lua_block {
+        local f = assert(io.open('$::HtmlDir/mtls_client.crt'))
+        local cert_data = f:read('*a')
+        f:close()
+
+        f = assert(io.open('$::HtmlDir/mtls_client.key'))
+        local key_data = f:read('*a')
+        f:close()
+
+        local ssl = require('ngx.ssl')
+
+        local cert = assert(ssl.parse_pem_cert(cert_data))
+        local key = assert(ssl.parse_pem_priv_key(key_data))
+
+        f = assert(io.open('$::HtmlDir/test.crt'))
+        local invalid_cert_data = f:read('*a')
+        f:close()
+
+        f = assert(io.open('$::HtmlDir/test.key'))
+        local invalid_key_data = f:read('*a')
+        f:close()
+
+        local invalid_cert = assert(ssl.parse_pem_cert(invalid_cert_data))
+        local invalid_key = assert(ssl.parse_pem_priv_key(invalid_key_data))
+
+        local httpc = assert(require('resty.http').new())
+
+        local ok, err = httpc:connect {
+          scheme = 'https',
+          host = 'unix:$::HtmlDir/mtls.sock',
+          ssl_client_cert = cert,
+          ssl_client_priv_key = key,
+        }
+
+        if ok and not err then
+          local res, err = assert(httpc:request {
+            method = 'GET',
+            path = '/',
+            headers = {
+              ['Host'] = 'example.com',
+            },
+          })
+
+          ngx.say(res:read_body())
+        end
+
+        httpc:set_keepalive()
+
+        local httpc = assert(require('resty.http').new())
+
+        local ok, err = httpc:connect {
+          scheme = 'https',
+          host = 'unix:$::HtmlDir/mtls.sock',
+          ssl_client_cert = invalid_cert,
+          ssl_client_priv_key = invalid_key,
+        }
+
+        ngx.say(httpc:get_reused_times())
+        ngx.say(ok)
+        ngx.say(err)
+
+        if ok and not err then
+          local res, err = assert(httpc:request {
+            method = 'GET',
+            path = '/',
+            headers = {
+              ['Host'] = 'example.com',
+            },
+          })
+
+          ngx.say(res.status)   -- expect 400
+        end
+
+        httpc:close()
+    }
+}
+"
+--- user_files eval: $::mtls_user_files
+--- request
+GET /t
+--- no_error_log
+[error]
+[warn]
+--- response_body
+hello, CN=foo@example.com,O=OpenResty,ST=California,C=US
+0
+true
+nil
+400

From 6eced0291be902dd81bbd738fdfef787fc75a19f Mon Sep 17 00:00:00 2001
From: Zhefeng Chen <chzf_zju@163.com>
Date: Mon, 8 Jan 2024 17:13:29 +0800
Subject: [PATCH 04/21] fix: ssl_client_cert is a chain of x509 instead of a
 x509

---
 lib/resty/http_connect.lua | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/lib/resty/http_connect.lua b/lib/resty/http_connect.lua
index 91d16ba..91eb18b 100644
--- a/lib/resty/http_connect.lua
+++ b/lib/resty/http_connect.lua
@@ -167,20 +167,30 @@ local function connect(self, options)
     local cert_hash
     if ssl and ssl_client_cert and ssl_client_priv_key then
         local status, res = xpcall(function()
+            local chain = require("resty.openssl.chain")
             local x509 = require("resty.openssl.x509")
             local pkey = require("resty.openssl.pkey")
-            return { x509, pkey }
+            return { chain, x509, pkey }
         end, debug.traceback)
 
         if status then
-            local x509 = res[1]
-            local pkey = res[2]
-            local cert, err = x509.new(ssl_client_cert)
+            local chain = res[1]
+            local x509 = res[2]
+            local pkey = res[3]
+
+            local cert_chain, err = chain.dup(ssl_client_cert)
+            if not cert_chain then
+              return nil, err
+            end
+
+            if #cert_chain < 1 then
+              return nil, "no cert in the chain"
+            end
+
+            local cert, err = x509.dup(cert_chain[1].ctx)
             if not cert then
               return nil, err
             end
-            -- should not free the cdata passed in
-            ffi_gc(cert.ctx, nil)
 
             local key, err = pkey.new(ssl_client_priv_key)
             if not key then
@@ -204,8 +214,8 @@ local function connect(self, options)
             end
 
         else
-            if type(res) == "string" and ngx_re_find(res, "module 'resty\\.openssl\\.(x509|pkey)' not found") then
-                ngx_log(ngx_WARN, "can't use mTLS without module `lua-resty-openssl`, falling back to non-mTLS.")
+            if type(res) == "string" and ngx_re_find(res, "module 'resty\\.openssl\\.(chain|x509|pkey)' not found") then
+                ngx_log(ngx_WARN, "can't use mTLS without module `lua-resty-openssl`, falling back to non-mTLS." .. res)
 
             else
                 return nil, "failed to load module 'resty.openssl.*':\n" .. res

From 3ff2af2d9032b1ce34435773d48337b7f4d8f8d9 Mon Sep 17 00:00:00 2001
From: Zhefeng Chen <chzf_zju@163.com>
Date: Wed, 10 Jan 2024 11:09:40 +0800
Subject: [PATCH 05/21] fix: use the correct chain path and add `ffi.cast`

---
 lib/resty/http_connect.lua | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/lib/resty/http_connect.lua b/lib/resty/http_connect.lua
index 91eb18b..560534d 100644
--- a/lib/resty/http_connect.lua
+++ b/lib/resty/http_connect.lua
@@ -6,6 +6,7 @@ local ngx_log = ngx.log
 local ngx_WARN = ngx.WARN
 local to_hex = require("resty.string").to_hex
 local ffi_gc = ffi.gc
+local ffi_cast = ffi.cast
 local string_format = string.format
 
 --[[
@@ -167,7 +168,7 @@ local function connect(self, options)
     local cert_hash
     if ssl and ssl_client_cert and ssl_client_priv_key then
         local status, res = xpcall(function()
-            local chain = require("resty.openssl.chain")
+            local chain = require("resty.openssl.x509.chain")
             local x509 = require("resty.openssl.x509")
             local pkey = require("resty.openssl.pkey")
             return { chain, x509, pkey }
@@ -178,7 +179,9 @@ local function connect(self, options)
             local x509 = res[2]
             local pkey = res[3]
 
-            local cert_chain, err = chain.dup(ssl_client_cert)
+
+            -- convert from `void*` to `OPENSSL_STACK*`
+            local cert_chain, err = chain.dup(ffi_cast("OPENSSL_STACK*", ssl_client_cert))
             if not cert_chain then
               return nil, err
             end
@@ -192,7 +195,8 @@ local function connect(self, options)
               return nil, err
             end
 
-            local key, err = pkey.new(ssl_client_priv_key)
+            -- convert from `void*` to `EVP_PKEY*`
+            local key, err = pkey.new(ffi_cast("EVP_PKEY*", ssl_client_priv_key))
             if not key then
               return nil, err
             end
@@ -214,7 +218,7 @@ local function connect(self, options)
             end
 
         else
-            if type(res) == "string" and ngx_re_find(res, "module 'resty\\.openssl\\.(chain|x509|pkey)' not found") then
+            if type(res) == "string" and ngx_re_find(res, "module 'resty\\.openssl\\..+' not found") then
                 ngx_log(ngx_WARN, "can't use mTLS without module `lua-resty-openssl`, falling back to non-mTLS." .. res)
 
             else

From 67a6d8a623b7b2ff6f1e53a8505e65f6af079455 Mon Sep 17 00:00:00 2001
From: Zhefeng Chen <chzf_zju@163.com>
Date: Wed, 10 Jan 2024 11:42:14 +0800
Subject: [PATCH 06/21] add the latest openresty versions and skip mtls tests
 only when nginx version < 1.21.4

---
 .github/workflows/test.yml |  6 +++++-
 t/20-mtls.t                | 11 ++++++-----
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index f78eb83..1fe8a0b 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -20,6 +20,8 @@ jobs:
         openresty_version:
           - 1.17.8.2
           - 1.19.9.1
+          - 1.21.4.3
+          - 1.25.3.1
 
     runs-on: ubuntu-latest
     container:
@@ -49,7 +51,9 @@ jobs:
       run: cpanm -q -n Test::Nginx
 
     - name: Install Luacov
-      run: /usr/local/openresty/luajit/bin/luarocks install luacov
+      run: |
+        /usr/local/openresty/luajit/bin/luarocks install luacov
+        /usr/local/openresty/luajit/bin/luarocks install lua-resty-openssl
 
     - uses: actions/checkout@v2
 
diff --git a/t/20-mtls.t b/t/20-mtls.t
index f48c70e..9384dc6 100644
--- a/t/20-mtls.t
+++ b/t/20-mtls.t
@@ -105,7 +105,6 @@ GET /t
 
 === TEST 2: Connection fails during handshake with not priv_key
 --- http_config eval: $::mtls_http_config
---- SKIP
 --- config eval
 "
 lua_ssl_trusted_certificate $::HtmlDir/test.crt;
@@ -151,10 +150,10 @@ GET /t
 --- error_log
 could not set client certificate: bad client pkey type
 --- response_body_unlike: hello, CN=foo@example.com,O=OpenResty,ST=California,C=US
-
+--- skip_nginx
+4: < 1.21.4
 
 === TEST 3: Connection succeeds with client cert and key. SKIP'd for CI until feature is merged.
---- SKIP
 --- http_config eval: $::mtls_http_config
 --- config eval
 "
@@ -208,10 +207,10 @@ GET /t
 [warn]
 --- response_body
 hello, CN=foo@example.com,O=OpenResty,ST=California,C=US
-
+--- skip_nginx
+4: < 1.21.4
 
 === TEST 4: users with different client certs should not share the same pool.
---- SKIP
 --- http_config eval: $::mtls_http_config
 --- config eval
 "
@@ -307,3 +306,5 @@ hello, CN=foo@example.com,O=OpenResty,ST=California,C=US
 true
 nil
 400
+--- skip_nginx
+4: < 1.21.4

From aa3c82a4e2d5cf7a5cb4f22762de3c15cb1c1608 Mon Sep 17 00:00:00 2001
From: Zhefeng Chen <chzf_zju@163.com>
Date: Wed, 10 Jan 2024 14:06:09 +0800
Subject: [PATCH 07/21] add type check for client cert and key

---
 lib/resty/http_connect.lua | 7 +++++++
 t/20-mtls.t                | 2 +-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/lib/resty/http_connect.lua b/lib/resty/http_connect.lua
index 560534d..04fa5bc 100644
--- a/lib/resty/http_connect.lua
+++ b/lib/resty/http_connect.lua
@@ -179,6 +179,13 @@ local function connect(self, options)
             local x509 = res[2]
             local pkey = res[3]
 
+            if type(ssl_client_cert) ~= "cdata" then
+              return nil, "bad ssl_client_cert: cdata expected, got " .. type(ssl_client_cert)
+            end
+
+            if type(ssl_client_priv_key) ~= "cdata" then
+              return nil, "bad ssl_client_priv_key: cdata expected, got " .. type(ssl_client_priv_key)
+            end
 
             -- convert from `void*` to `OPENSSL_STACK*`
             local cert_chain, err = chain.dup(ffi_cast("OPENSSL_STACK*", ssl_client_cert))
diff --git a/t/20-mtls.t b/t/20-mtls.t
index 9384dc6..761185a 100644
--- a/t/20-mtls.t
+++ b/t/20-mtls.t
@@ -148,7 +148,7 @@ location /t {
 GET /t
 --- error_code: 200
 --- error_log
-could not set client certificate: bad client pkey type
+bad ssl_client_priv_key: cdata expected, got string
 --- response_body_unlike: hello, CN=foo@example.com,O=OpenResty,ST=California,C=US
 --- skip_nginx
 4: < 1.21.4

From bc289e687ea475b673e51203b6464ac12bc43eba Mon Sep 17 00:00:00 2001
From: Zhefeng Chen <chzf_zju@163.com>
Date: Wed, 10 Jan 2024 14:42:56 +0800
Subject: [PATCH 08/21] fallback to non-mTLS when any cert/key error and log a
 warning

---
 lib/resty/http_connect.lua | 124 +++++++++++++++++++++----------------
 1 file changed, 72 insertions(+), 52 deletions(-)

diff --git a/lib/resty/http_connect.lua b/lib/resty/http_connect.lua
index 04fa5bc..afe00ad 100644
--- a/lib/resty/http_connect.lua
+++ b/lib/resty/http_connect.lua
@@ -167,71 +167,91 @@ local function connect(self, options)
 
     local cert_hash
     if ssl and ssl_client_cert and ssl_client_priv_key then
-        local status, res = xpcall(function()
-            local chain = require("resty.openssl.x509.chain")
-            local x509 = require("resty.openssl.x509")
-            local pkey = require("resty.openssl.pkey")
-            return { chain, x509, pkey }
-        end, debug.traceback)
-
-        if status then
-            local chain = res[1]
-            local x509 = res[2]
-            local pkey = res[3]
-
-            if type(ssl_client_cert) ~= "cdata" then
-              return nil, "bad ssl_client_cert: cdata expected, got " .. type(ssl_client_cert)
+        -- fallback to non-mTLS when any error
+        repeat
+            local cert_type = type(ssl_client_cert)
+            local key_type = type(ssl_client_priv_key)
+
+            if cert_type ~= "cdata" then
+                ngx_log(ngx_WARN, "bad ssl_client_cert: cdata expected, got " .. cert_type)
+                break
             end
 
-            if type(ssl_client_priv_key) ~= "cdata" then
-              return nil, "bad ssl_client_priv_key: cdata expected, got " .. type(ssl_client_priv_key)
+            if key_type ~= "cdata" then
+                ngx_log(ngx_WARN, "bad ssl_client_priv_key: cdata expected, got " .. key_type)
+                break
             end
 
-            -- convert from `void*` to `OPENSSL_STACK*`
-            local cert_chain, err = chain.dup(ffi_cast("OPENSSL_STACK*", ssl_client_cert))
-            if not cert_chain then
-              return nil, err
-            end
-
-            if #cert_chain < 1 then
-              return nil, "no cert in the chain"
-            end
+            local status, res = xpcall(function()
+                local chain = require("resty.openssl.x509.chain")
+                local x509 = require("resty.openssl.x509")
+                local pkey = require("resty.openssl.pkey")
+                return { chain, x509, pkey }
+            end, debug.traceback)
+
+            if status then
+                local chain = res[1]
+                local x509 = res[2]
+                local pkey = res[3]
+
+                -- convert from `void*` to `OPENSSL_STACK*`
+                local cert_chain, err = chain.dup(ffi_cast("OPENSSL_STACK*", ssl_client_cert))
+                if not cert_chain then
+                    ngx_log(ngx_WARN, "failed to dup the ssl_client_cert, falling back to non-mTLS: " .. err)
+                    break
+                end
 
-            local cert, err = x509.dup(cert_chain[1].ctx)
-            if not cert then
-              return nil, err
-            end
+                if #cert_chain < 1 then
+                    ngx_log(ngx_WARN, "no cert in ssl_client_cert, falling back to non-mTLS: " .. err)
+                    break
+                end
 
-            -- convert from `void*` to `EVP_PKEY*`
-            local key, err = pkey.new(ffi_cast("EVP_PKEY*", ssl_client_priv_key))
-            if not key then
-              return nil, err
-            end
-            -- should not free the cdata passed in
-            ffi_gc(key.ctx, nil)
+                local cert, err = x509.dup(cert_chain[1].ctx)
+                if not cert then
+                    ngx_log(ngx_WARN, "failed to dup the x509, falling back to non-mTLS: " .. err)
+                    break
+                end
 
-            -- check the private key in order to make sure the caller is indeed the holder of the cert
-            ok, err = cert:check_private_key(key)
-            if not ok then
-              return nil, "failed to match the private key with the certificate: " .. err
-            end
+                -- convert from `void*` to `EVP_PKEY*`
+                local key, err = pkey.new(ffi_cast("EVP_PKEY*", ssl_client_priv_key))
+                if not key then
+                    ngx_log(ngx_WARN, "failed to new the pkey, falling back to non-mTLS: " .. err)
+                    break
+                end
+                -- should not free the cdata passed in
+                ffi_gc(key.ctx, nil)
 
-            cert_hash, err = cert:digest("sha256")
-            if cert_hash then
-              cert_hash = to_hex(cert_hash) -- convert to hex so that it's printable
+                -- check the private key in order to make sure the caller is indeed the holder of the cert
+                ok, err = cert:check_private_key(key)
+                if not ok then
+                    ngx_log(ngx_WARN, "the private key doesn't match the cert, falling back to non-mTLS: " .. err)
+                    break
+                end
 
-            else
-              return nil, err
-            end
+                cert_hash, err = cert:digest("sha256")
+                if cert_hash then
+                    cert_hash = to_hex(cert_hash) -- convert to hex so that it's printable
 
-        else
-            if type(res) == "string" and ngx_re_find(res, "module 'resty\\.openssl\\..+' not found") then
-                ngx_log(ngx_WARN, "can't use mTLS without module `lua-resty-openssl`, falling back to non-mTLS." .. res)
+                else
+                    ngx_log(ngx_WARN, "failed to calculate the digest of the cert, falling back to non-mTLS: " .. err)
+                    break
+                end
 
             else
-                return nil, "failed to load module 'resty.openssl.*':\n" .. res
+                if type(res) == "string" and ngx_re_find(res, "module 'resty\\.openssl\\..+' not found") then
+                    ngx_log(ngx_WARN, "can't use mTLS without module `lua-resty-openssl`, falling back to non-mTLS:\n "
+                                      .. res)
+
+                else
+                    ngx_log(ngx_WARN, "failed to load module `resty.openssl.*`, falling back to non-mTLS:\n" .. res)
+                end
             end
-        end
+        until true
+    end
+
+    if not cert_hash then
+        ssl_client_cert = nil
+        ssl_client_priv_key = nil
     end
 
     -- construct a poolname unique within proxy and ssl info

From 8832e33afb4c0bd3e0e1aa5d85a67d8f00217102 Mon Sep 17 00:00:00 2001
From: Zhefeng Chen <chzf_zju@163.com>
Date: Mon, 15 Jan 2024 16:53:28 +0800
Subject: [PATCH 09/21] add debug log to print poolname

---
 lib/resty/http_connect.lua | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/resty/http_connect.lua b/lib/resty/http_connect.lua
index afe00ad..10add14 100644
--- a/lib/resty/http_connect.lua
+++ b/lib/resty/http_connect.lua
@@ -4,6 +4,7 @@ local ngx_re_sub = ngx.re.sub
 local ngx_re_find = ngx.re.find
 local ngx_log = ngx.log
 local ngx_WARN = ngx.WARN
+local ngx_DEBUG = ngx.DEBUG
 local to_hex = require("resty.string").to_hex
 local ffi_gc = ffi.gc
 local ffi_cast = ffi.cast
@@ -272,6 +273,8 @@ local function connect(self, options)
         -- with a plain http request the authorization is part of the actual request.
     end
 
+    ngx_log(ngx_DEBUG, "poolname: " .. poolname)
+
     -- do TCP level connection
     local tcp_opts = { pool = poolname, pool_size = pool_size, backlog = backlog }
     if proxy then

From 4e858093a8dfa7e8edfe2031b2fafc3536959025 Mon Sep 17 00:00:00 2001
From: Zhefeng Chen <chzf_zju@163.com>
Date: Tue, 27 Feb 2024 14:10:22 +0800
Subject: [PATCH 10/21] apply the comments

---
 lib/resty/http_connect.lua | 119 +++++++++++++++++++------------------
 1 file changed, 62 insertions(+), 57 deletions(-)

diff --git a/lib/resty/http_connect.lua b/lib/resty/http_connect.lua
index 10add14..f1df20c 100644
--- a/lib/resty/http_connect.lua
+++ b/lib/resty/http_connect.lua
@@ -9,6 +9,14 @@ local to_hex = require("resty.string").to_hex
 local ffi_gc = ffi.gc
 local ffi_cast = ffi.cast
 local string_format = string.format
+local type = type
+
+local function require_openssl_libs()
+    local chain = require("resty.openssl.x509.chain")
+    local x509 = require("resty.openssl.x509")
+    local pkey = require("resty.openssl.pkey")
+    return { chain, x509, pkey }
+end
 
 --[[
 A connection function that incorporates:
@@ -174,79 +182,76 @@ local function connect(self, options)
             local key_type = type(ssl_client_priv_key)
 
             if cert_type ~= "cdata" then
-                ngx_log(ngx_WARN, "bad ssl_client_cert: cdata expected, got " .. cert_type)
+                ngx_log(ngx_WARN, "bad ssl_client_cert: cdata expected, got ", cert_type)
                 break
             end
 
             if key_type ~= "cdata" then
-                ngx_log(ngx_WARN, "bad ssl_client_priv_key: cdata expected, got " .. key_type)
+                ngx_log(ngx_WARN, "bad ssl_client_priv_key: cdata expected, got ", key_type)
                 break
             end
 
-            local status, res = xpcall(function()
-                local chain = require("resty.openssl.x509.chain")
-                local x509 = require("resty.openssl.x509")
-                local pkey = require("resty.openssl.pkey")
-                return { chain, x509, pkey }
-            end, debug.traceback)
-
-            if status then
-                local chain = res[1]
-                local x509 = res[2]
-                local pkey = res[3]
-
-                -- convert from `void*` to `OPENSSL_STACK*`
-                local cert_chain, err = chain.dup(ffi_cast("OPENSSL_STACK*", ssl_client_cert))
-                if not cert_chain then
-                    ngx_log(ngx_WARN, "failed to dup the ssl_client_cert, falling back to non-mTLS: " .. err)
-                    break
-                end
+            local status, res = xpcall(require_openssl_libs, debug.traceback)
 
-                if #cert_chain < 1 then
-                    ngx_log(ngx_WARN, "no cert in ssl_client_cert, falling back to non-mTLS: " .. err)
-                    break
-                end
+            if not status then
+                if type(res) == "string" and ngx_re_find(res, "module 'resty\\.openssl\\..+' not found") then
+                    ngx_log(ngx_WARN, "can't use mTLS without module `lua-resty-openssl`, falling back to non-mTLS:\n "
+                                    , res)
 
-                local cert, err = x509.dup(cert_chain[1].ctx)
-                if not cert then
-                    ngx_log(ngx_WARN, "failed to dup the x509, falling back to non-mTLS: " .. err)
-                    break
+                else
+                    ngx_log(ngx_WARN, "failed to load module `resty.openssl.*`, falling back to non-mTLS:\n", res)
                 end
 
-                -- convert from `void*` to `EVP_PKEY*`
-                local key, err = pkey.new(ffi_cast("EVP_PKEY*", ssl_client_priv_key))
-                if not key then
-                    ngx_log(ngx_WARN, "failed to new the pkey, falling back to non-mTLS: " .. err)
-                    break
-                end
-                -- should not free the cdata passed in
-                ffi_gc(key.ctx, nil)
+                break
+            end
 
-                -- check the private key in order to make sure the caller is indeed the holder of the cert
-                ok, err = cert:check_private_key(key)
-                if not ok then
-                    ngx_log(ngx_WARN, "the private key doesn't match the cert, falling back to non-mTLS: " .. err)
-                    break
-                end
+            local chain = res[1]
+            local x509 = res[2]
+            local pkey = res[3]
 
-                cert_hash, err = cert:digest("sha256")
-                if cert_hash then
-                    cert_hash = to_hex(cert_hash) -- convert to hex so that it's printable
+            -- convert from `void*` to `OPENSSL_STACK*`
+            local cert_chain, err = chain.dup(ffi_cast("OPENSSL_STACK*", ssl_client_cert))
+            if not cert_chain then
+                ngx_log(ngx_WARN, "failed to dup the ssl_client_cert, falling back to non-mTLS: ", err)
+                break
+            end
 
-                else
-                    ngx_log(ngx_WARN, "failed to calculate the digest of the cert, falling back to non-mTLS: " .. err)
-                    break
-                end
+            if #cert_chain < 1 then
+                ngx_log(ngx_WARN, "no cert in ssl_client_cert, falling back to non-mTLS: ", err)
+                break
+            end
 
-            else
-                if type(res) == "string" and ngx_re_find(res, "module 'resty\\.openssl\\..+' not found") then
-                    ngx_log(ngx_WARN, "can't use mTLS without module `lua-resty-openssl`, falling back to non-mTLS:\n "
-                                      .. res)
+            local cert, err = x509.dup(cert_chain[1].ctx)
+            if not cert then
+                ngx_log(ngx_WARN, "failed to dup the x509, falling back to non-mTLS: ", err)
+                break
+            end
 
-                else
-                    ngx_log(ngx_WARN, "failed to load module `resty.openssl.*`, falling back to non-mTLS:\n" .. res)
-                end
+            -- convert from `void*` to `EVP_PKEY*`
+            local key, err = pkey.new(ffi_cast("EVP_PKEY*", ssl_client_priv_key))
+            if not key then
+                ngx_log(ngx_WARN, "failed to new the pkey, falling back to non-mTLS: ", err)
+                break
             end
+            -- should not free the cdata passed in
+            ffi_gc(key.ctx, nil)
+
+            -- check the private key in order to make sure the caller is indeed the holder of the cert
+            ok, err = cert:check_private_key(key)
+            if not ok then
+                ngx_log(ngx_WARN, "the private key doesn't match the cert, falling back to non-mTLS: ", err)
+                break
+            end
+
+            cert_hash, err = cert:digest("sha256")
+            if cert_hash then
+                cert_hash = to_hex(cert_hash) -- convert to hex so that it's printable
+
+            else
+                ngx_log(ngx_WARN, "failed to calculate the digest of the cert, falling back to non-mTLS: ", err)
+                break
+            end
+
         until true
     end
 
@@ -273,7 +278,7 @@ local function connect(self, options)
         -- with a plain http request the authorization is part of the actual request.
     end
 
-    ngx_log(ngx_DEBUG, "poolname: " .. poolname)
+    ngx_log(ngx_DEBUG, "poolname: ", poolname)
 
     -- do TCP level connection
     local tcp_opts = { pool = poolname, pool_size = pool_size, backlog = backlog }

From a20212ed3c2ef9cdd8c1e50920bcf7bca473402d Mon Sep 17 00:00:00 2001
From: Zhefeng Chen <chzf_zju@163.com>
Date: Tue, 27 Feb 2024 19:32:26 +0800
Subject: [PATCH 11/21] apply comments

---
 lib/resty/http_connect.lua | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/lib/resty/http_connect.lua b/lib/resty/http_connect.lua
index f1df20c..e7594ee 100644
--- a/lib/resty/http_connect.lua
+++ b/lib/resty/http_connect.lua
@@ -244,14 +244,13 @@ local function connect(self, options)
             end
 
             cert_hash, err = cert:digest("sha256")
-            if cert_hash then
-                cert_hash = to_hex(cert_hash) -- convert to hex so that it's printable
-
-            else
+            if not cert_hash then
                 ngx_log(ngx_WARN, "failed to calculate the digest of the cert, falling back to non-mTLS: ", err)
                 break
             end
 
+            cert_hash = to_hex(cert_hash) -- convert to hex so that it's printable
+
         until true
     end
 
@@ -265,10 +264,10 @@ local function connect(self, options)
         poolname = string_format("%s:%s:%s:%s:%s:%s:%s:%s:%s",
                     request_scheme or "",
                     request_host,
-                    tostring(request_port),
-                    tostring(ssl),
+                    request_port,
+                    ssl,
                     ssl_server_name or "",
-                    tostring(ssl_verify),
+                    ssl_verify,
                     proxy_uri or "",
                     request_scheme == "https" and proxy_authorization or "",
                     cert_hash or "")

From baf011a7e390ac6b082823d6d0b4fe17dc52fc8a Mon Sep 17 00:00:00 2001
From: Zhefeng Chen <chzf_zju@163.com>
Date: Tue, 27 Feb 2024 20:16:07 +0800
Subject: [PATCH 12/21] apply the comment

---
 lib/resty/http_connect.lua | 128 +++++++++++++++++++------------------
 1 file changed, 65 insertions(+), 63 deletions(-)

diff --git a/lib/resty/http_connect.lua b/lib/resty/http_connect.lua
index e7594ee..abae712 100644
--- a/lib/resty/http_connect.lua
+++ b/lib/resty/http_connect.lua
@@ -175,84 +175,86 @@ local function connect(self, options)
     end
 
     local cert_hash
-    if ssl and ssl_client_cert and ssl_client_priv_key then
-        -- fallback to non-mTLS when any error
-        repeat
-            local cert_type = type(ssl_client_cert)
-            local key_type = type(ssl_client_priv_key)
-
-            if cert_type ~= "cdata" then
-                ngx_log(ngx_WARN, "bad ssl_client_cert: cdata expected, got ", cert_type)
-                break
-            end
+    -- fallback to non-mTLS when any error
+    repeat
+        if not ssl or not ssl_client_cert or not ssl_client_priv_key then
+            break
+        end
 
-            if key_type ~= "cdata" then
-                ngx_log(ngx_WARN, "bad ssl_client_priv_key: cdata expected, got ", key_type)
-                break
-            end
+        local cert_type = type(ssl_client_cert)
+        local key_type = type(ssl_client_priv_key)
 
-            local status, res = xpcall(require_openssl_libs, debug.traceback)
+        if cert_type ~= "cdata" then
+            ngx_log(ngx_WARN, "bad ssl_client_cert: cdata expected, got ", cert_type)
+            break
+        end
 
-            if not status then
-                if type(res) == "string" and ngx_re_find(res, "module 'resty\\.openssl\\..+' not found") then
-                    ngx_log(ngx_WARN, "can't use mTLS without module `lua-resty-openssl`, falling back to non-mTLS:\n "
-                                    , res)
+        if key_type ~= "cdata" then
+            ngx_log(ngx_WARN, "bad ssl_client_priv_key: cdata expected, got ", key_type)
+            break
+        end
 
-                else
-                    ngx_log(ngx_WARN, "failed to load module `resty.openssl.*`, falling back to non-mTLS:\n", res)
-                end
+        local status, res = xpcall(require_openssl_libs, debug.traceback)
 
-                break
+        if not status then
+            if type(res) == "string" and ngx_re_find(res, "module 'resty\\.openssl\\..+' not found") then
+                ngx_log(ngx_WARN, "can't use mTLS without module `lua-resty-openssl`, falling back to non-mTLS:\n "
+                                , res)
+
+            else
+                ngx_log(ngx_WARN, "failed to load module `resty.openssl.*`, falling back to non-mTLS:\n", res)
             end
 
-            local chain = res[1]
-            local x509 = res[2]
-            local pkey = res[3]
+            break
+        end
 
-            -- convert from `void*` to `OPENSSL_STACK*`
-            local cert_chain, err = chain.dup(ffi_cast("OPENSSL_STACK*", ssl_client_cert))
-            if not cert_chain then
-                ngx_log(ngx_WARN, "failed to dup the ssl_client_cert, falling back to non-mTLS: ", err)
-                break
-            end
+        local chain = res[1]
+        local x509 = res[2]
+        local pkey = res[3]
 
-            if #cert_chain < 1 then
-                ngx_log(ngx_WARN, "no cert in ssl_client_cert, falling back to non-mTLS: ", err)
-                break
-            end
+        -- convert from `void*` to `OPENSSL_STACK*`
+        local cert_chain, err = chain.dup(ffi_cast("OPENSSL_STACK*", ssl_client_cert))
+        if not cert_chain then
+            ngx_log(ngx_WARN, "failed to dup the ssl_client_cert, falling back to non-mTLS: ", err)
+            break
+        end
 
-            local cert, err = x509.dup(cert_chain[1].ctx)
-            if not cert then
-                ngx_log(ngx_WARN, "failed to dup the x509, falling back to non-mTLS: ", err)
-                break
-            end
+        if #cert_chain < 1 then
+            ngx_log(ngx_WARN, "no cert in ssl_client_cert, falling back to non-mTLS: ", err)
+            break
+        end
 
-            -- convert from `void*` to `EVP_PKEY*`
-            local key, err = pkey.new(ffi_cast("EVP_PKEY*", ssl_client_priv_key))
-            if not key then
-                ngx_log(ngx_WARN, "failed to new the pkey, falling back to non-mTLS: ", err)
-                break
-            end
-            -- should not free the cdata passed in
-            ffi_gc(key.ctx, nil)
+        local cert, err = x509.dup(cert_chain[1].ctx)
+        if not cert then
+            ngx_log(ngx_WARN, "failed to dup the x509, falling back to non-mTLS: ", err)
+            break
+        end
 
-            -- check the private key in order to make sure the caller is indeed the holder of the cert
-            ok, err = cert:check_private_key(key)
-            if not ok then
-                ngx_log(ngx_WARN, "the private key doesn't match the cert, falling back to non-mTLS: ", err)
-                break
-            end
+        -- convert from `void*` to `EVP_PKEY*`
+        local key, err = pkey.new(ffi_cast("EVP_PKEY*", ssl_client_priv_key))
+        if not key then
+            ngx_log(ngx_WARN, "failed to new the pkey, falling back to non-mTLS: ", err)
+            break
+        end
+        -- should not free the cdata passed in
+        ffi_gc(key.ctx, nil)
 
-            cert_hash, err = cert:digest("sha256")
-            if not cert_hash then
-                ngx_log(ngx_WARN, "failed to calculate the digest of the cert, falling back to non-mTLS: ", err)
-                break
-            end
+        -- check the private key in order to make sure the caller is indeed the holder of the cert
+        ok, err = cert:check_private_key(key)
+        if not ok then
+            ngx_log(ngx_WARN, "the private key doesn't match the cert, falling back to non-mTLS: ", err)
+            break
+        end
 
-            cert_hash = to_hex(cert_hash) -- convert to hex so that it's printable
+        cert_hash, err = cert:digest("sha256")
+        if not cert_hash then
+            ngx_log(ngx_WARN, "failed to calculate the digest of the cert, falling back to non-mTLS: ", err)
+            break
+        end
 
-        until true
-    end
+        cert_hash = to_hex(cert_hash) -- convert to hex so that it's printable
+
+    until true
 
     if not cert_hash then
         ssl_client_cert = nil

From 273bf56baa81c48a678fc596ca3a17af75ae7929 Mon Sep 17 00:00:00 2001
From: Zhefeng Chen <chzf_zju@163.com>
Date: Tue, 27 Feb 2024 22:07:02 +0800
Subject: [PATCH 13/21] load resty.openssl.* at the module level and error out
 if the client cert set isn't valid

---
 lib/resty/http_connect.lua | 51 ++++++++++++++------------------------
 1 file changed, 19 insertions(+), 32 deletions(-)

diff --git a/lib/resty/http_connect.lua b/lib/resty/http_connect.lua
index abae712..dfed31e 100644
--- a/lib/resty/http_connect.lua
+++ b/lib/resty/http_connect.lua
@@ -11,11 +11,15 @@ local ffi_cast = ffi.cast
 local string_format = string.format
 local type = type
 
-local function require_openssl_libs()
-    local chain = require("resty.openssl.x509.chain")
-    local x509 = require("resty.openssl.x509")
-    local pkey = require("resty.openssl.pkey")
-    return { chain, x509, pkey }
+local lib_chain, lib_x509, lib_pkey
+local openssl_available, res = xpcall(function()
+    lib_chain = require("resty.openssl.x509.chain")
+    lib_x509 = require("resty.openssl.x509")
+    lib_pkey = require("resty.openssl.pkey")
+end, debug.traceback)
+
+if not openssl_available then
+  ngx_log(ngx_WARN, "failed to load module `resty.openssl.*`, mTLS isn't supported without lua-resty-openssl :\n", res)
 end
 
 --[[
@@ -175,7 +179,7 @@ local function connect(self, options)
     end
 
     local cert_hash
-    -- fallback to non-mTLS when any error
+    -- fallback to non-mTLS if it's not an error due to the caller
     repeat
         if not ssl or not ssl_client_cert or not ssl_client_priv_key then
             break
@@ -185,53 +189,37 @@ local function connect(self, options)
         local key_type = type(ssl_client_priv_key)
 
         if cert_type ~= "cdata" then
-            ngx_log(ngx_WARN, "bad ssl_client_cert: cdata expected, got ", cert_type)
-            break
+            return nil, string_format("bad ssl_client_cert: cdata expected, got %s", cert_type)
         end
 
         if key_type ~= "cdata" then
-            ngx_log(ngx_WARN, "bad ssl_client_priv_key: cdata expected, got ", key_type)
-            break
+            return nil, string_format("bad ssl_client_priv_key: cdata expected, got %s", key_type)
         end
 
-        local status, res = xpcall(require_openssl_libs, debug.traceback)
-
-        if not status then
-            if type(res) == "string" and ngx_re_find(res, "module 'resty\\.openssl\\..+' not found") then
-                ngx_log(ngx_WARN, "can't use mTLS without module `lua-resty-openssl`, falling back to non-mTLS:\n "
-                                , res)
-
-            else
-                ngx_log(ngx_WARN, "failed to load module `resty.openssl.*`, falling back to non-mTLS:\n", res)
-            end
-
+        if not openssl_available then
+            ngx_log(ngx_WARN, "module `resty.openssl.*` not available, falling back to non-mTLS:\n")
             break
         end
 
-        local chain = res[1]
-        local x509 = res[2]
-        local pkey = res[3]
-
         -- convert from `void*` to `OPENSSL_STACK*`
-        local cert_chain, err = chain.dup(ffi_cast("OPENSSL_STACK*", ssl_client_cert))
+        local cert_chain, err = lib_chain.dup(ffi_cast("OPENSSL_STACK*", ssl_client_cert))
         if not cert_chain then
             ngx_log(ngx_WARN, "failed to dup the ssl_client_cert, falling back to non-mTLS: ", err)
             break
         end
 
         if #cert_chain < 1 then
-            ngx_log(ngx_WARN, "no cert in ssl_client_cert, falling back to non-mTLS: ", err)
-            break
+            return nil, "no cert in ssl_client_cert"
         end
 
-        local cert, err = x509.dup(cert_chain[1].ctx)
+        local cert, err = lib_x509.dup(cert_chain[1].ctx)
         if not cert then
             ngx_log(ngx_WARN, "failed to dup the x509, falling back to non-mTLS: ", err)
             break
         end
 
         -- convert from `void*` to `EVP_PKEY*`
-        local key, err = pkey.new(ffi_cast("EVP_PKEY*", ssl_client_priv_key))
+        local key, err = lib_pkey.new(ffi_cast("EVP_PKEY*", ssl_client_priv_key))
         if not key then
             ngx_log(ngx_WARN, "failed to new the pkey, falling back to non-mTLS: ", err)
             break
@@ -242,8 +230,7 @@ local function connect(self, options)
         -- check the private key in order to make sure the caller is indeed the holder of the cert
         ok, err = cert:check_private_key(key)
         if not ok then
-            ngx_log(ngx_WARN, "the private key doesn't match the cert, falling back to non-mTLS: ", err)
-            break
+            return nil, string_format("the private key doesn't match the cert: %s", err)
         end
 
         cert_hash, err = cert:digest("sha256")

From 705f57d1526f34ab7a7d4908a691baff173b9d50 Mon Sep 17 00:00:00 2001
From: Zhefeng Chen <chzf_zju@163.com>
Date: Tue, 27 Feb 2024 22:29:38 +0800
Subject: [PATCH 14/21] error out directly for other cases as well and fix test

---
 lib/resty/http_connect.lua | 38 +++++++++++---------------------------
 t/20-mtls.t                | 13 +++++++++----
 2 files changed, 20 insertions(+), 31 deletions(-)

diff --git a/lib/resty/http_connect.lua b/lib/resty/http_connect.lua
index dfed31e..43996ef 100644
--- a/lib/resty/http_connect.lua
+++ b/lib/resty/http_connect.lua
@@ -179,12 +179,7 @@ local function connect(self, options)
     end
 
     local cert_hash
-    -- fallback to non-mTLS if it's not an error due to the caller
-    repeat
-        if not ssl or not ssl_client_cert or not ssl_client_priv_key then
-            break
-        end
-
+    if ssl and ssl_client_cert and ssl_client_priv_key then
         local cert_type = type(ssl_client_cert)
         local key_type = type(ssl_client_priv_key)
 
@@ -197,15 +192,13 @@ local function connect(self, options)
         end
 
         if not openssl_available then
-            ngx_log(ngx_WARN, "module `resty.openssl.*` not available, falling back to non-mTLS:\n")
-            break
+            return nil, "module `resty.openssl.*` not available, mTLS isn't supported with lua-resty-openssl"
         end
 
         -- convert from `void*` to `OPENSSL_STACK*`
         local cert_chain, err = lib_chain.dup(ffi_cast("OPENSSL_STACK*", ssl_client_cert))
         if not cert_chain then
-            ngx_log(ngx_WARN, "failed to dup the ssl_client_cert, falling back to non-mTLS: ", err)
-            break
+            return nil, string_format("failed to dup the ssl_client_cert: %s", err)
         end
 
         if #cert_chain < 1 then
@@ -214,15 +207,13 @@ local function connect(self, options)
 
         local cert, err = lib_x509.dup(cert_chain[1].ctx)
         if not cert then
-            ngx_log(ngx_WARN, "failed to dup the x509, falling back to non-mTLS: ", err)
-            break
+            return nil, string_format("failed to dup the x509: %s", err)
         end
 
         -- convert from `void*` to `EVP_PKEY*`
         local key, err = lib_pkey.new(ffi_cast("EVP_PKEY*", ssl_client_priv_key))
         if not key then
-            ngx_log(ngx_WARN, "failed to new the pkey, falling back to non-mTLS: ", err)
-            break
+            return nil, string_format("failed to new the pkey: %s": err)
         end
         -- should not free the cdata passed in
         ffi_gc(key.ctx, nil)
@@ -235,17 +226,10 @@ local function connect(self, options)
 
         cert_hash, err = cert:digest("sha256")
         if not cert_hash then
-            ngx_log(ngx_WARN, "failed to calculate the digest of the cert, falling back to non-mTLS: ", err)
-            break
+            return nil, string_format("failed to calculate the digest of the cert: %s", err)
         end
 
         cert_hash = to_hex(cert_hash) -- convert to hex so that it's printable
-
-    until true
-
-    if not cert_hash then
-        ssl_client_cert = nil
-        ssl_client_priv_key = nil
     end
 
     -- construct a poolname unique within proxy and ssl info
@@ -326,13 +310,13 @@ local function connect(self, options)
         -- Experimental mTLS support
         if ssl_client_cert and ssl_client_priv_key then
           if type(sock.setclientcert) ~= "function" then
-            ngx_log(ngx_WARN, "cannot use SSL client cert and key without mTLS support")
+              return nil, "cannot use SSL client cert and key without mTLS support"
 
           else
-            ok, err = sock:setclientcert(ssl_client_cert, ssl_client_priv_key)
-            if not ok then
-              ngx_log(ngx_WARN, "could not set client certificate: ", err)
-            end
+              ok, err = sock:setclientcert(ssl_client_cert, ssl_client_priv_key)
+              if not ok then
+                  return nil, string_format("could not set client certificate: %s", err)
+              end
           end
         end
 
diff --git a/t/20-mtls.t b/t/20-mtls.t
index 761185a..7508dbb 100644
--- a/t/20-mtls.t
+++ b/t/20-mtls.t
@@ -137,6 +137,9 @@ location /t {
           })
 
           ngx.say(res:read_body())
+
+        else
+          ngx.say("failed to connect: " .. err or "")
         end
 
         httpc:close()
@@ -147,13 +150,15 @@ location /t {
 --- request
 GET /t
 --- error_code: 200
---- error_log
-bad ssl_client_priv_key: cdata expected, got string
---- response_body_unlike: hello, CN=foo@example.com,O=OpenResty,ST=California,C=US
+--- no_error_log
+[error]
+[warn]
+--- response_body
+failed to connect: bad ssl_client_priv_key: cdata expected, got string
 --- skip_nginx
 4: < 1.21.4
 
-=== TEST 3: Connection succeeds with client cert and key. SKIP'd for CI until feature is merged.
+=== TEST 3: Connection succeeds with client cert and key.
 --- http_config eval: $::mtls_http_config
 --- config eval
 "

From 9d731c8c0272a209444d41302ab27545ce1ed8aa Mon Sep 17 00:00:00 2001
From: Zhefeng Chen <chzf_zju@163.com>
Date: Tue, 27 Feb 2024 22:31:36 +0800
Subject: [PATCH 15/21] fix typo

---
 lib/resty/http_connect.lua | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/resty/http_connect.lua b/lib/resty/http_connect.lua
index 43996ef..049d4ec 100644
--- a/lib/resty/http_connect.lua
+++ b/lib/resty/http_connect.lua
@@ -213,7 +213,7 @@ local function connect(self, options)
         -- convert from `void*` to `EVP_PKEY*`
         local key, err = lib_pkey.new(ffi_cast("EVP_PKEY*", ssl_client_priv_key))
         if not key then
-            return nil, string_format("failed to new the pkey: %s": err)
+            return nil, string_format("failed to new the pkey: %s", err)
         end
         -- should not free the cdata passed in
         ffi_gc(key.ctx, nil)

From b91ddec55000758a1c33036efc0fcb338c5fa89d Mon Sep 17 00:00:00 2001
From: Zhefeng Chen <chzf_zju@163.com>
Date: Tue, 27 Feb 2024 22:44:57 +0800
Subject: [PATCH 16/21] fix test

---
 t/20-mtls.t | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/t/20-mtls.t b/t/20-mtls.t
index 7508dbb..e5b6df6 100644
--- a/t/20-mtls.t
+++ b/t/20-mtls.t
@@ -139,7 +139,7 @@ location /t {
           ngx.say(res:read_body())
 
         else
-          ngx.say("failed to connect: " .. err or "")
+          ngx.say('failed to connect: ' .. (err or ''))
         end
 
         httpc:close()
@@ -158,6 +158,7 @@ failed to connect: bad ssl_client_priv_key: cdata expected, got string
 --- skip_nginx
 4: < 1.21.4
 
+
 === TEST 3: Connection succeeds with client cert and key.
 --- http_config eval: $::mtls_http_config
 --- config eval

From 62a7e1de87e397b8c669cd9e4365718fa937b260 Mon Sep 17 00:00:00 2001
From: Zhefeng Chen <chzf_zju@163.com>
Date: Tue, 27 Feb 2024 22:49:37 +0800
Subject: [PATCH 17/21] fix error message

---
 lib/resty/http_connect.lua | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/resty/http_connect.lua b/lib/resty/http_connect.lua
index 049d4ec..4ea7999 100644
--- a/lib/resty/http_connect.lua
+++ b/lib/resty/http_connect.lua
@@ -192,7 +192,7 @@ local function connect(self, options)
         end
 
         if not openssl_available then
-            return nil, "module `resty.openssl.*` not available, mTLS isn't supported with lua-resty-openssl"
+            return nil, "module `resty.openssl.*` not available, mTLS isn't supported without lua-resty-openssl"
         end
 
         -- convert from `void*` to `OPENSSL_STACK*`

From efa2b651593ebd3ecf77a325033069833a45a898 Mon Sep 17 00:00:00 2001
From: Zhefeng Chen <chzf_zju@163.com>
Date: Tue, 27 Feb 2024 22:52:47 +0800
Subject: [PATCH 18/21] fix error message

---
 lib/resty/http_connect.lua | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/resty/http_connect.lua b/lib/resty/http_connect.lua
index 4ea7999..e05cda0 100644
--- a/lib/resty/http_connect.lua
+++ b/lib/resty/http_connect.lua
@@ -19,7 +19,7 @@ local openssl_available, res = xpcall(function()
 end, debug.traceback)
 
 if not openssl_available then
-  ngx_log(ngx_WARN, "failed to load module `resty.openssl.*`, mTLS isn't supported without lua-resty-openssl :\n", res)
+  ngx_log(ngx_WARN, "failed to load module `resty.openssl.*`, mTLS isn't supported without lua-resty-openssl:\n", res)
 end
 
 --[[

From 863be745db7ab384302b99f89be20a09609528a7 Mon Sep 17 00:00:00 2001
From: Zhefeng Chen <chzf_zju@163.com>
Date: Wed, 28 Feb 2024 11:03:18 +0800
Subject: [PATCH 19/21] use `..` concat operation to keep the original style

---
 lib/resty/http_connect.lua | 39 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 20 deletions(-)

diff --git a/lib/resty/http_connect.lua b/lib/resty/http_connect.lua
index e05cda0..5da3524 100644
--- a/lib/resty/http_connect.lua
+++ b/lib/resty/http_connect.lua
@@ -8,7 +8,6 @@ local ngx_DEBUG = ngx.DEBUG
 local to_hex = require("resty.string").to_hex
 local ffi_gc = ffi.gc
 local ffi_cast = ffi.cast
-local string_format = string.format
 local type = type
 
 local lib_chain, lib_x509, lib_pkey
@@ -19,7 +18,8 @@ local openssl_available, res = xpcall(function()
 end, debug.traceback)
 
 if not openssl_available then
-  ngx_log(ngx_WARN, "failed to load module `resty.openssl.*`, mTLS isn't supported without lua-resty-openssl:\n", res)
+  ngx_log(ngx_WARN, "failed to load module `resty.openssl.*`, \z
+                     mTLS isn't supported without lua-resty-openssl:\n", res)
 end
 
 --[[
@@ -184,11 +184,11 @@ local function connect(self, options)
         local key_type = type(ssl_client_priv_key)
 
         if cert_type ~= "cdata" then
-            return nil, string_format("bad ssl_client_cert: cdata expected, got %s", cert_type)
+            return nil, "bad ssl_client_cert: cdata expected, got " .. cert_type
         end
 
         if key_type ~= "cdata" then
-            return nil, string_format("bad ssl_client_priv_key: cdata expected, got %s", key_type)
+            return nil, "bad ssl_client_priv_key: cdata expected, got " .. key_type
         end
 
         if not openssl_available then
@@ -198,7 +198,7 @@ local function connect(self, options)
         -- convert from `void*` to `OPENSSL_STACK*`
         local cert_chain, err = lib_chain.dup(ffi_cast("OPENSSL_STACK*", ssl_client_cert))
         if not cert_chain then
-            return nil, string_format("failed to dup the ssl_client_cert: %s", err)
+            return nil, "failed to dup the ssl_client_cert: " .. err
         end
 
         if #cert_chain < 1 then
@@ -207,26 +207,27 @@ local function connect(self, options)
 
         local cert, err = lib_x509.dup(cert_chain[1].ctx)
         if not cert then
-            return nil, string_format("failed to dup the x509: %s", err)
+            return nil, "failed to dup the x509: " .. err
         end
 
         -- convert from `void*` to `EVP_PKEY*`
         local key, err = lib_pkey.new(ffi_cast("EVP_PKEY*", ssl_client_priv_key))
         if not key then
-            return nil, string_format("failed to new the pkey: %s", err)
+            return nil, "failed to new the pkey: " .. err
         end
+
         -- should not free the cdata passed in
         ffi_gc(key.ctx, nil)
 
         -- check the private key in order to make sure the caller is indeed the holder of the cert
         ok, err = cert:check_private_key(key)
         if not ok then
-            return nil, string_format("the private key doesn't match the cert: %s", err)
+            return nil, "the private key doesn't match the cert: " .. err
         end
 
         cert_hash, err = cert:digest("sha256")
         if not cert_hash then
-            return nil, string_format("failed to calculate the digest of the cert: %s", err)
+            return nil, "failed to calculate the digest of the cert: " .. err
         end
 
         cert_hash = to_hex(cert_hash) -- convert to hex so that it's printable
@@ -234,16 +235,14 @@ local function connect(self, options)
 
     -- construct a poolname unique within proxy and ssl info
     if not poolname then
-        poolname = string_format("%s:%s:%s:%s:%s:%s:%s:%s:%s",
-                    request_scheme or "",
-                    request_host,
-                    request_port,
-                    ssl,
-                    ssl_server_name or "",
-                    ssl_verify,
-                    proxy_uri or "",
-                    request_scheme == "https" and proxy_authorization or "",
-                    cert_hash or "")
+        poolname = (request_scheme or "")
+                   .. ":" .. request_host
+                   .. ":" .. tostring(request_port)
+                   .. ":" .. tostring(ssl)
+                   .. ":" .. (ssl_server_name or "")
+                   .. ":" .. tostring(ssl_verify)
+                   .. ":" .. (proxy_uri or "")
+                   .. ":" .. (request_scheme == "https" and proxy_authorization or "")
         -- in the above we only add the 'proxy_authorization' as part of the poolname
         -- when the request is https. Because in that case the CONNECT request (which
         -- carries the authorization header) is part of the connect procedure, whereas
@@ -315,7 +314,7 @@ local function connect(self, options)
           else
               ok, err = sock:setclientcert(ssl_client_cert, ssl_client_priv_key)
               if not ok then
-                  return nil, string_format("could not set client certificate: %s", err)
+                  return nil, "could not set client certificate: " .. err
               end
           end
         end

From 75274fd8aecc5451df801cdcabecd5c26fafc2f7 Mon Sep 17 00:00:00 2001
From: Zhefeng Chen <chzf_zju@163.com>
Date: Wed, 28 Feb 2024 11:07:51 +0800
Subject: [PATCH 20/21] fixup

---
 lib/resty/http_connect.lua | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/resty/http_connect.lua b/lib/resty/http_connect.lua
index 5da3524..6503985 100644
--- a/lib/resty/http_connect.lua
+++ b/lib/resty/http_connect.lua
@@ -243,6 +243,7 @@ local function connect(self, options)
                    .. ":" .. tostring(ssl_verify)
                    .. ":" .. (proxy_uri or "")
                    .. ":" .. (request_scheme == "https" and proxy_authorization or "")
+                   .. ":" .. (cert_hash or "")
         -- in the above we only add the 'proxy_authorization' as part of the poolname
         -- when the request is https. Because in that case the CONNECT request (which
         -- carries the authorization header) is part of the connect procedure, whereas

From 43f9d215ff46d107117b5d9c5d089922b59e2f14 Mon Sep 17 00:00:00 2001
From: Zhefeng Chen <chzf_zju@163.com>
Date: Wed, 28 Feb 2024 11:50:15 +0800
Subject: [PATCH 21/21] fix some return value errors

---
 lib/resty/http_connect.lua | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/resty/http_connect.lua b/lib/resty/http_connect.lua
index 6503985..83b6d2f 100644
--- a/lib/resty/http_connect.lua
+++ b/lib/resty/http_connect.lua
@@ -166,7 +166,7 @@ local function connect(self, options)
         local proxy_uri_t
         proxy_uri_t, err = self:parse_uri(proxy_uri)
         if not proxy_uri_t then
-            return nil, "uri parse error: ", err
+            return nil, "uri parse error: " .. err
         end
 
         local proxy_scheme = proxy_uri_t[1]
@@ -260,7 +260,7 @@ local function connect(self, options)
         if not ok then
             return nil, "failed to connect to: " .. (proxy_host or "") ..
                         ":" .. (proxy_port or "") ..
-                        ": ", err
+                        ": " .. err
         end
 
         if ssl and sock:getreusedtimes() == 0 then
@@ -280,7 +280,7 @@ local function connect(self, options)
             })
 
             if not res then
-                return nil, "failed to issue CONNECT to proxy:", err
+                return nil, "failed to issue CONNECT to proxy: " .. err
             end
 
             if res.status < 200 or res.status > 299 then