Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace the shelled out openssl pem to der conversion, in favor of l… #873

Closed
wants to merge 1 commit into from

Conversation

cleblanc87
Copy link

…ua-resty-core conversion method cert_pem_to_der, this will now preserve the full certificate chain

…a-resty-core conversion method cert_pem_to_der, this will now preserve the full certificate chain
@cleblanc87
Copy link
Author

Fixes #872.

Started in on adding tests, but importing "ngx.ssl" breaks the tests with
/usr/local/share/lua/5.1/ngx/ssl.lua:5: module 'resty.core.base' not found:No LuaRocks module found for resty.core.base

I am not too familiar with this ecosystem, so a nudge in the right direction would be very welcome :)

@thibaultcha
Copy link
Member

This error occurs because in spec/plugins/ssl/access_spec.lua, the kong.plugins.ssl.ssl_util module is being unit tested, so it is required here. Because this does not run inside Nginx, resty.core.base is not in the LUA_PATH.

iirc, this is the reason why the module is required inside the functions, such as here, so they can be required while unit testing. I am not a fan of this solution, I think a patch that properly requires ngx.ssl at module-level for this plugin (and removing the unit tests in access_spec.lua) would be an improvement. Do you want some time to update yours?

@thibaultcha thibaultcha added pr/changes requested Changes were requested to this PR by a maintainer. Please address them and ping back once done. and removed pr/status/needs review labels Jan 13, 2016
@subnetmarco
Copy link
Member

Looking forward to see a test for this, so we can close the issue.

@subnetmarco
Copy link
Member

By the way it seems like the new version of OpenResty is also going to ship with priv_key_pem_to_der which will allow us to never use OpenSSL for both conversions (key/cert to DER).

Also, https://github.com/openresty/lua-resty-core/blob/master/lib/ngx/ssl.md#priv_key_pem_to_der

@subnetmarco
Copy link
Member

Fixed with #968.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/changes requested Changes were requested to this PR by a maintainer. Please address them and ping back once done.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants