fix poolname to include the digest of the cert for mTLS

.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
 

lib/resty/http_connect.lua

@@ -1,8 +1,26 @@
+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 ngx_DEBUG = ngx.DEBUG
+local to_hex = require("resty.string").to_hex
+local ffi_gc = ffi.gc
+local ffi_cast = ffi.cast
+local type = type
+
+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.*`, \z
+                     mTLS isn't supported without lua-resty-openssl:\n", res)
+end
 
 --[[
 A connection function that incorporates:
@@ -160,6 +178,61 @@ 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 cert_type = type(ssl_client_cert)
+        local key_type = type(ssl_client_priv_key)
+
+        if cert_type ~= "cdata" then
+            return nil, "bad ssl_client_cert: cdata expected, got " .. cert_type
+        end
+
+        if key_type ~= "cdata" then
+            return nil, "bad ssl_client_priv_key: cdata expected, got " .. key_type
+        end
+
+        if not openssl_available then
+            return nil, "module `resty.openssl.*` not available, mTLS isn't supported without 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
+            return nil, "failed to dup the ssl_client_cert: " .. err
+        end
+
+        if #cert_chain < 1 then
+            return nil, "no cert in ssl_client_cert"
+        end
+
+        local cert, err = lib_x509.dup(cert_chain[1].ctx)
+        if not cert then
+            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, "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, "the private key doesn't match the cert: " .. err
+        end
+
+        cert_hash, err = cert:digest("sha256")
+        if not cert_hash then
+            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
+    end
+
     -- construct a poolname unique within proxy and ssl info
     if not poolname then
         poolname = (request_scheme or "")
@@ -170,12 +243,15 @@ 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
         -- 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
@@ -234,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, "could not set client certificate: " .. err
+              end
           end
         end
 

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;
@@ -138,6 +137,9 @@ location /t {
           })
 
           ngx.say(res:read_body())
+
+        else
+          ngx.say('failed to connect: ' .. (err or ''))
         end
 
         httpc:close()
@@ -148,13 +150,16 @@ location /t {
 --- request
 GET /t
 --- error_code: 200
---- error_log
-could not set client certificate: bad client pkey type
---- response_body_unlike: hello, [email protected],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.
---- SKIP
+=== TEST 3: Connection succeeds with client cert and key.
 --- http_config eval: $::mtls_http_config
 --- config eval
 "
@@ -208,4 +213,104 @@ GET /t
 [warn]
 --- response_body
 hello, [email protected],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.
+--- 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, [email protected],O=OpenResty,ST=California,C=US
+0
+true
+nil
+400
+--- skip_nginx
+4: < 1.21.4