-
Notifications
You must be signed in to change notification settings - Fork 2.1k
feature: proxy_ssl_verify_by_lua directives #2436
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
base: master
Are you sure you want to change the base?
Conversation
working after receiving server certificates, allowing us to control upstream ssl handshake dynamically with Lua
to control whether to skip openssl's default verify function
…ify server certificate
Should this pr be prepared for grpc and uwsgi module as well? They also support upstream SSL related configurations. grpc_ssl_verify_by_lua |
Let's first focus on proxy_ssl_verify_by_lua, since grpc/uwsgi are not that commonly used |
@agentzh @zhuizhuhaomeng ping for review...sorry to bother, but please take some time to review the PR, thanks! |
|
||
ssl = cln->data; | ||
|
||
plcf = ngx_http_conf_get_module_loc_conf(cf, ngx_http_proxy_module); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ssl = plcf->upstream.ssl
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.


Here the comments have described why we do this! Nginx doesn't export ngx_http_proxy_loc_conf_t
, it's an internal structure inside ngx_http_proxy_module.c
file, so we can't using plcf->upstream.ssl to get ssl context unless we add a header file to Nginx to export it outside, which would break Nginx's code organization, since there's no header files in src/http/modules
directory, and also we don't want to change that much.
According to openresty-PR-1066, we take advantage of pool cleaner facilitity inside ngx_ssl_create and do a little patch here, we can INDIRECTLY get the plcf through ngx_ssl_get_server_conf(ssl->ctx)
and compare it with the one getting from ngx_http_conf_get_module_loc_conf(cf, ngx_http_proxy_module)
, therefore we can make sure we get correct ssl and do the following check.
I know it's a bit clumsy, but the solution is good enough, since it's a common practice in Nginx to use pool cleaner this way. And it won't affect performance either, since it is working in config time.
Hopefully the comments above will make the idea more clear.
@@ -87,7 +87,8 @@ ngx_http_lua_ffi_get_ctx_ref(ngx_http_request_t *r, int *in_ssl_phase, | |||
return ctx->ctx_ref; | |||
} | |||
|
|||
*in_ssl_phase = ctx->context & (NGX_HTTP_LUA_CONTEXT_SSL_CERT | |||
*in_ssl_phase = ctx->context & (NGX_HTTP_LUA_CONTEXT_PROXY_SSL_VERIFY |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.



Here we reuse in_ssl_phase
, it originally was using for downstream ssl only. But proxy_ssl_verify_by_lua
directives are using for upstream ssl connection, ngx.ctx
in these two contexts are totally different, but it's reasonably to reuse it here, no matter downstream ssl or upstream ssl, they are ssl phases, just belonging to different connections
proxy_ssl_verify_by_lua directives
proxy_ssl_verify_by_lua directives are working after receiving server certificates, allowing us to control upstream ssl handshake dynamically with Lua
a series of related PRs
some of the docs hasn't finished yet, since the PR has not been merged, and some release infos can't be added, please review the codes first and docs may be updated later