[GitHub] [apisix] membphis commented on a change in pull request #2163: improve: cache parsed SSL certs and pkeys

[GitBox <gi...@apache.org>]

membphis commented on a change in pull request #2163:
URL: https://github.com/apache/apisix/pull/2163#discussion_r486031725



##########
File path: apisix/http/router/radixtree_sni.lua
##########
@@ -30,14 +30,53 @@ local ngx_decode_base64 = ngx.decode_base64
 local ssl_certificates
 local radixtree_router
 local radixtree_router_ver
-
+local parsed_certs
+local parsed_pkeys
 
 local _M = {
     version = 0.1,
     server_name = ngx_ssl.server_name,
 }
 
 
+local function parse_pem_cert(sni, cert)
+    local parsed = parsed_certs[cert]
+    if parsed then
+        return parsed
+    end
+
+    core.log.debug("parsing cert for sni: ", sni)
+
+    local parsed, err = ngx_ssl.parse_pem_cert(cert)
+    if not parsed then
+        return nil, err
+    end
+
+    parsed_certs[cert] = parsed
+
+    return parsed
+end
+
+
+local function parse_pem_priv_key(sni, pkey)
+    local parsed = parsed_pkeys[pkey]
+    if parsed then
+        return parsed
+    end
+
+    core.log.debug("parsing priv key for sni: ", sni)
+
+    local parsed, err = ngx_ssl.parse_pem_priv_key(pkey)
+    if not parsed then
+        return nil, err
+    end
+
+    parsed_pkeys[pkey] = parsed

Review comment:
       ditto

##########
File path: apisix/http/router/radixtree_sni.lua
##########
@@ -95,6 +134,9 @@ local function create_router(ssl_items)
         end
     end
 
+    parsed_certs = core.table.new(0, idx)
+    parsed_pkeys = core.table.new(0, idx)

Review comment:
       The current way is wrong. We cannot store them directly in the hash table. This method will cause memory overflow.
   
   We need to cache the parsed data in lrucache, and we need to control the number of cached certificates.

##########
File path: apisix/http/router/radixtree_sni.lua
##########
@@ -30,14 +30,53 @@ local ngx_decode_base64 = ngx.decode_base64
 local ssl_certificates
 local radixtree_router
 local radixtree_router_ver
-
+local parsed_certs
+local parsed_pkeys
 
 local _M = {
     version = 0.1,
     server_name = ngx_ssl.server_name,
 }
 
 
+local function parse_pem_cert(sni, cert)
+    local parsed = parsed_certs[cert]
+    if parsed then
+        return parsed
+    end
+
+    core.log.debug("parsing cert for sni: ", sni)
+
+    local parsed, err = ngx_ssl.parse_pem_cert(cert)
+    if not parsed then
+        return nil, err
+    end
+
+    parsed_certs[cert] = parsed

Review comment:
       bad way, need to use `lrucache`




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org