You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2020/01/04 14:40:27 UTC

[GitHub] [incubator-apisix] xiaobiaozhao opened a new pull request #1021: Modify the SSL module from ffi to ngx.ssl

xiaobiaozhao opened a new pull request #1021: Modify the SSL module from ffi to ngx.ssl
URL: https://github.com/apache/incubator-apisix/pull/1021
 
 
   
   
   ### Summary
   
   ssl module ffi too old, ngx.ssl has new function parse pem & key .
   
   ### Full changelog
   
   Modify the SSL module from ffi to ngx.ssl
   
   ### Issues resolved
   
   optimize
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] membphis commented on a change in pull request #1021: Modify the SSL module from ffi to ngx.ssl

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1021: Modify the SSL module from ffi to ngx.ssl
URL: https://github.com/apache/incubator-apisix/pull/1021#discussion_r363150019
 
 

 ##########
 File path: lua/apisix/http/router/radixtree_sni.lua
 ##########
 @@ -84,31 +72,24 @@ local function set_pem_ssl_key(cert, pkey)
 
     ngx_ssl.clear_certs()
 
-    local out = ffi.new("char [?]", #cert)
-    local rc = C.ngx_http_lua_ffi_cert_pem_to_der(cert, #cert, out, errmsg)
-    if rc < 1 then
-        return false, "failed to parse PEM cert: " .. ffi_string(errmsg[0])
-    end
-
-    local cert_der = ffi_string(out, rc)
-    local rc = C.ngx_http_lua_ffi_ssl_set_der_certificate(r, cert_der,
-                    #cert_der, errmsg)
-    if rc ~= 0 then
-        return false, "failed to set DER cert: " .. ffi_string(errmsg[0])
-    end
-
-    out = ffi.new("char [?]", #pkey)
-    local rc = C.ngx_http_lua_ffi_priv_key_pem_to_der(pkey, #pkey, out, errmsg)
-    if rc < 1 then
-        return false, "failed to parse PEM priv key: " .. ffi_string(errmsg[0])
+    local parse_cert, err = ngx_ssl.parse_pem_cert(cert)
+    if parse_cert then
+        local ok, err = ngx_ssl.set_cert(parse_cert)
+        if not ok then
+            return false, "failed to set PEM cert: " .. err
+        end
+    else
+        return false, "failed to parse PEM cert: " .. cert .. " err:" .. err
 
 Review comment:
   We cannot return the `cert` information in an error message, that is dangerous.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] membphis commented on issue #1021: Modify the SSL module from ffi to ngx.ssl

Posted by GitBox <gi...@apache.org>.
membphis commented on issue #1021: Modify the SSL module from ffi to ngx.ssl
URL: https://github.com/apache/incubator-apisix/pull/1021#issuecomment-571164892
 
 
   @xiaobiaozhao merged, many thx.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] membphis commented on a change in pull request #1021: Modify the SSL module from ffi to ngx.ssl

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1021: Modify the SSL module from ffi to ngx.ssl
URL: https://github.com/apache/incubator-apisix/pull/1021#discussion_r363150041
 
 

 ##########
 File path: lua/apisix/http/router/radixtree_sni.lua
 ##########
 @@ -84,31 +72,24 @@ local function set_pem_ssl_key(cert, pkey)
 
     ngx_ssl.clear_certs()
 
-    local out = ffi.new("char [?]", #cert)
-    local rc = C.ngx_http_lua_ffi_cert_pem_to_der(cert, #cert, out, errmsg)
-    if rc < 1 then
-        return false, "failed to parse PEM cert: " .. ffi_string(errmsg[0])
-    end
-
-    local cert_der = ffi_string(out, rc)
-    local rc = C.ngx_http_lua_ffi_ssl_set_der_certificate(r, cert_der,
-                    #cert_der, errmsg)
-    if rc ~= 0 then
-        return false, "failed to set DER cert: " .. ffi_string(errmsg[0])
-    end
-
-    out = ffi.new("char [?]", #pkey)
-    local rc = C.ngx_http_lua_ffi_priv_key_pem_to_der(pkey, #pkey, out, errmsg)
-    if rc < 1 then
-        return false, "failed to parse PEM priv key: " .. ffi_string(errmsg[0])
+    local parse_cert, err = ngx_ssl.parse_pem_cert(cert)
+    if parse_cert then
+        local ok, err = ngx_ssl.set_cert(parse_cert)
+        if not ok then
+            return false, "failed to set PEM cert: " .. err
+        end
+    else
+        return false, "failed to parse PEM cert: " .. cert .. " err:" .. err
     end
 
-    local pkey_der = ffi_string(out, rc)
-
-    local rc = C.ngx_http_lua_ffi_ssl_set_der_private_key(r, pkey_der,
-                    #pkey_der, errmsg)
-    if rc ~= 0 then
-        return false, "failed to set DER priv key: " .. ffi_string(errmsg[0])
+    local parse_pkey, err = ngx_ssl.parse_pem_priv_key(pkey)
+    if parse_pkey then
+        local ok, err = ngx_ssl.set_priv_key(parse_pkey)
+        if not ok then
+            return false, "failed to set PEM priv key: " .. err
+        end
+    else
+        return false, "failed to parse PEM priv key: " .. pkey .. " err:" .. err
 
 Review comment:
   ditto

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] membphis commented on issue #1021: Modify the SSL module from ffi to ngx.ssl

Posted by GitBox <gi...@apache.org>.
membphis commented on issue #1021: Modify the SSL module from ffi to ngx.ssl
URL: https://github.com/apache/incubator-apisix/pull/1021#issuecomment-570833818
 
 
   @xiaobiaozhao Please fix the code style.
   
   https://travis-ci.org/apache/incubator-apisix/jobs/632655357#L698
   
   ```shell
   +./bin/apisix stop
   +sleep 1
   +make lint
   luacheck -q lua
   Checking lua/apisix/http/router/radixtree_sni.lua 2 warnings
       lua/apisix/http/router/radixtree_sni.lua:75:23: unused variable err
       lua/apisix/http/router/radixtree_sni.lua:85:23: unused variable err
   Total: 2 warnings / 0 errors in 70 files
   Makefile:71: recipe for target 'lint' failed
   make: *** [lint] Error 1
   ```

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] membphis merged pull request #1021: Modify the SSL module from ffi to ngx.ssl

Posted by GitBox <gi...@apache.org>.
membphis merged pull request #1021: Modify the SSL module from ffi to ngx.ssl
URL: https://github.com/apache/incubator-apisix/pull/1021
 
 
   

----------------------------------------------------------------
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


With regards,
Apache Git Services