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