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,22 @@
+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 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:
@@ -160,22 +174,112 @@ 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
+        -- 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 key_type ~= "cdata" then
+                ngx_log(ngx_WARN, "bad ssl_client_priv_key: cdata expected, got ", key_type)
+                break
+            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
+
+                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))
+            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
+            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
+
+            -- 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
+
+    if not cert_hash then
+        ssl_client_cert = nil
+        ssl_client_priv_key = nil
+    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
         -- 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

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;
@@ -149,12 +148,12 @@ 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, [email protected],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,4 +207,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