-
Notifications
You must be signed in to change notification settings - Fork 2.1k
feature: tcpsock sslhandshake alpn #2460
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?
feature: tcpsock sslhandshake alpn #2460
Conversation
|
Modifications to the Lua API: openresty/lua-resty-core#513 |
|
The current issues here and openresty/lua-resty-core#513 's testing errors caused by code dependencies. I'm unsure how to proceed. cc @zhuizhuhaomeng Regarding the API modification, it is backward compatible. The new optional arugment is added at the end of |
|
you can change the git url in the .travis.yml |
|
After modifying the CI to use my lua-resty-core dev branch and fixing some errors there, I have passed the tests. https://github.com/openresty/lua-nginx-module/pull/2460/checks?check_run_id=53599698217
The test suite 4 shows identical results to the main branch commit. Some session cb yield-related tests failed in the 143 test file, which suggests this issue was not introduced by the current PR. I believe this PR is ready for substantive review. I'm unsure whether I or you should ultimately update the repository address in .travis.yaml back to the correct one during merging. I believe this may not have caused any additional side effects, so I'd like to know if it can be accepted and ultimately released in the 1.29 release cycle. If any points are identified that require modification, I will address them immediately. Thanks for your help @zhuizhuhaomeng |

This PR provides additional ALPN extension options for
tcpsock:sslhandshake, enabling cosocket to establish TLS handshakes and subsequent communication with servers that have specific ALPN negotiation requirements.Specifically, HTTP/2 requires TLS ALPN negotiation before establishing an HTTP/2 connection and sending requests via the PRI prologue. If the client cannot handshake with ALPN
h2, HTTP/2 over TLS cannot be implemented. In addition, there are other protocols that have this requirement.https://www.iana.org/assignments/tls-extensiontype-values/tls-extensiontype-values.xhtml#alpn-protocol-ids
The PR includes modifications to the FFI C functions and associated tests, which assert against debug logs in Nginx.
https://github.com/nginx/nginx/blob/78d1ab5a2c00839a36ff6bac661d9785fce3c1a4/src/http/modules/ngx_http_ssl_module.c#L455
As far as I know, I might also need to update the Lua API in
lua-resty-coreto accommodate these FFI modifications.However, I'm not certain which should happen first. Strictly speaking, without modifying that Lua API, we'll never pass the tests. But modifying the Lua API would make it depend on an FFI modification that doesn't yet exist. I'm not sure how to proceed, but I'll submit a PR there as well.
The proposed API change is as follows: it remains the old designs, with a new parameter
alpnadded at the end.It accepts such parameters and converts them to
ngx_str_t[1]before the Lua FFI call.It will be constructed as an
ngx_str_tto be passed to the FFI C function.I hereby granted the copyright of the changes in this pull request
to the authors of this lua-nginx-module project.