lua-nginx-module icon indicating copy to clipboard operation
lua-nginx-module copied to clipboard

feature: ssl: support for TLS-PSK

Open vartiait opened this issue 8 years ago • 23 comments

I hereby granted the copyright of the changes in this pull request to the authors of this lua-nginx-module project.

Adds a support for TLS-PSK handshakes with following new directives to lua-nginx-module:

  • ssl_psk_by_lua_block { lua-script }
  • ssl_psk_by_lua_file
  • ssl_psk_identity_hint <tls_psk_identity_hint>
  • lua_ssl_psk_identity <tls_psk_identity>
  • lua_ssl_psk_key <tls_psk_key>

This is related to PR at lua-resty-core/pull/150

vartiait avatar Oct 04 '17 22:10 vartiait

@vartiait Thanks for the contribution! I wonder, however, whether we can reuse the existing ssl_certificiate_by_lua* for this.

@ghedo @lziest Will you please have a look at this? Thanks!

agentzh avatar Oct 05 '17 18:10 agentzh

@agentzh you're welcome!

I would like to keep the callbacks separate as OpenSSL callback for TLS-PSK (set with SSL_CTX_set_psk_server_callback) is not re-entrant whereas cert_cb is, but it's your call :)

By the way, have you noticed that some tests in t/129-ssl-socket.t seems to randomly fail due to a timeout when run by travis-ci?

vartiait avatar Oct 06 '17 07:10 vartiait

@vartiait Seems like I cannot rebase your branch to the latest master? See

agentzh@fedora64 ~/git/lua-nginx-module (PR/1167)$ git rebase master
First, rewinding head to replay your work on top of it...
Applying: feature: TLS-PSK handshake control.
Using index info to reconstruct a base tree...
M	config
M	src/ngx_http_lua_common.h
M	src/ngx_http_lua_control.c
M	src/ngx_http_lua_headers.c
M	src/ngx_http_lua_module.c
M	src/ngx_http_lua_socket_tcp.c
M	src/ngx_http_lua_ssl.h
M	src/ngx_http_lua_util.h
.git/rebase-apply/patch:453: trailing whitespace.
 *
.git/rebase-apply/patch:1021: trailing whitespace.
ngx_http_lua_ffi_ssl_set_psk_key(ngx_http_request_t *r,
.git/rebase-apply/patch:1165: trailing whitespace.
 *
warning: 3 lines add whitespace errors.
Falling back to patching base and 3-way merge...
Auto-merging src/ngx_http_lua_util.h
Auto-merging src/ngx_http_lua_ssl.h
Auto-merging src/ngx_http_lua_socket_tcp.c
Auto-merging src/ngx_http_lua_module.c
CONFLICT (content): Merge conflict in src/ngx_http_lua_module.c
Auto-merging src/ngx_http_lua_control.c
Auto-merging src/ngx_http_lua_common.h
Auto-merging config
error: Failed to merge in the changes.
Patch failed at 0001 feature: TLS-PSK handshake control.
The copy of the patch that failed is found in: .git/rebase-apply/patch

When you have resolved this problem, run "git rebase --continue".
If you prefer to skip this patch, run "git rebase --skip" instead.
To check out the original branch and stop rebasing, run "git rebase --abort".

Please do not use "git merge" in your branch and keep your git history strictly linear. Thank you!

agentzh avatar Oct 06 '17 20:10 agentzh

@vartiait Will you explain your use cases a bit? It seems that the use cases for this feature is very narrow.

agentzh avatar Oct 06 '17 22:10 agentzh

It seems to me that TLS-PSK is not a commonly used technology at all. AFAIK none of the web browsers on the market supports it. Additionally, NGINX, Apache, Python, Golang all lacks support to it in the standard TLS implementation as well. Maintaining such an obscure feature in the long term is probably not worth the effort, at least not until TLS-PSK become more adopted on the Internet.

I would rather spend more time on migrating to OpenSSL 1.1 and add client side certificate support to cosocket. Since the later can do everything TLS-PSK can and is more secure and widely used.

dndx avatar Oct 07 '17 00:10 dndx

@agentzh sorry for that, I rebased my branch now.

vartiait avatar Oct 07 '17 08:10 vartiait

@agentzh @dndx Yes, TLS-PSK is mainly used with IoT (Internet of Things) devices which are performance-constrained with limited CPU power.

There has apparently lately been also development of TLS-PSK support on NGINX side, but that approach is only for static PSK keys.

http://mailman.nginx.org/pipermail/nginx-devel/2017-August/010430.html http://mailman.nginx.org/pipermail/nginx-devel/2017-September/010471.html

My case in on a telco/mobile side, where 3GPP has specified TLS-PSK as an authentication option for CPU/power constrained devices in GAA/GBA architecture. Devices uses dynamic PSK keys for authentication and that's the reason I wrote a support for ssl_psk_by_lua as we are using OpenResty/NGINX as a platform to implement the GAA/GBA architecture.

3GPP specification TS 33.222 - Generic Authentication Architecture (GAA); Access to network application functions using Hypertext Transfer Protocol over Transport Layer Security (HTTPS):

http://www.etsi.org/deliver/etsi_ts/133200_133299/133222/14.00.00_60/ts_133222v140000p.pdf

The product where we are using OpenResty is called Radiator GBA/BSF Pack:

http://www.open.com.au/gba-bsf/

vartiait avatar Oct 07 '17 09:10 vartiait

@vartiait That sounds interesting. Thanks for the info!

agentzh avatar Oct 07 '17 23:10 agentzh

@vartiait I still think we should use ssl_certificate_by_lua* for that but we need a separate context constant so that we can disable those Lua APIs which may yield (like cosockets and sleep) in this context.

agentzh avatar Oct 07 '17 23:10 agentzh

@agentzh okay, wouldn't that imply also a change to ngx_http_lua_ssl_cert_by_chunk to prevent it from creating a new thread with ngx_http_lua_new_thread when called by ngx_http_lua_ssl_psk_server_handler?

vartiait avatar Oct 08 '17 10:10 vartiait

@agentzh ah, okay, never mind, I now read what ngx_http_lua_new_thread, lua_newthread, ngx_http_lua_run_thread and lua_resume do.

vartiait avatar Oct 08 '17 16:10 vartiait

@agentzh @dndx I added a flag entered_psk_handler to ngx_http_lua_ssl_ctx_t which is set in TLS-PSK callback ngx_http_lua_ssl_psk_server_handler and when that flag is set, the context will be set to NGX_HTTP_LUA_CONTEXT_SSL_PSK (instead of NGX_HTTP_LUA_CONTEXT_SSL_CERT) in ngx_http_lua_ssl_cert_by_chunk. What do you think?

vartiait avatar Oct 08 '17 20:10 vartiait

I removed separate ssl_psk_by_lua* handlers and updated TLS-PSK server callback to use ssl_certificate_by_lua*.

ssl_certificate_by_lua* handler will be called two times during TLS-PSK handshake, first time from ssl3_get_client_hello() (as with other ciphers) and the second time from ssl3_accept() when actually called to set the PSK key.

vartiait avatar Oct 09 '17 14:10 vartiait

@agentzh How could I get an exit code or a return code from ssl_certificate_by_lua* in order to return ngx.ERROR to terminate the handshake?

I tried to copy ctx->exit_code to cctx->exit_code in ngx_http_lua_ssl_cert_by_chunk, but that change broke few test cases in t/139-ssl-cert-by.t so I reverted it.

vartiait avatar Oct 09 '17 15:10 vartiait

@agentzh could you please comment the PR, I integrated TLS-PSK to ssl_certificate_by_lua*, are there some other tweaks needed?

vartiait avatar Oct 23 '17 10:10 vartiait

@vartiait I'll have a look as soon as I can manage. Thanks!

agentzh avatar Oct 24 '17 05:10 agentzh

What is the status here?

berndschoenbach avatar Feb 08 '18 11:02 berndschoenbach

This pull request is now in conflict :(

mergify[bot] avatar Jun 26 '20 00:06 mergify[bot]