Session reuse #2
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Is there a specific reason why this never got merged? I would very much like to see this feature in luasec. |
|
To be honest, I waited to see if there was demand. Luasec was in 0.4 version, it was small and I wanted to keep it simple. |
Zash
added a commit
to Zash/luasec
that referenced
this pull request
Aug 12, 2021
Background:
The SSL Context has a reference to the Lua state, commonly called L, for
use when invoking callbacks during TLS handshaking.
Earlier there was a crash due to the Lua State having changed between
context creation and access to it during TLS handshake. This could
happen if the TLS context is created in one coroutine, which has
subsequently been garbage collected and its memory become invalid,
whereafter the TLS context is used to negotiate a TLS connection which
involves calling some callback.
Observation:
SIGSEGV in alpn_cb(), where arg/ctx -> L does not match the L earlier in
the stack, in meth_handshake().
#0 __strcmp_sse2 () at ../sysdeps/x86_64/strcmp.S:1447
lunarmodules#1 0x00005589415b1e1a in luaS_new (L=L@entry=0x5589444880a8, str=str@entry=0x7ff3f94b160e "SSL:ALPN:Registry") at lstring.c:246
lunarmodules#2 0x00005589415a47b2 in auxgetstr (L=0x5589444880a8, t=0x558944488188, k=k@entry=0x7ff3f94b160e "SSL:ALPN:Registry") at lapi.c:617
lunarmodules#3 0x00005589415a575b in lua_getfield (L=L@entry=0x5589444880a8, idx=idx@entry=-1001000, k=k@entry=0x7ff3f94b160e "SSL:ALPN:Registry") at lapi.c:657
lunarmodules#4 0x00007ff3f94af3c5 in alpn_cb (s=<optimized out>, out=0x7ffdda0dc260, outlen=0x7ffdda0dc25f "", in=0x558943d14570 "\vxmpp-client", inlen=12, arg=0x55894450d2a8) at src/context.c:660
lunarmodules#5 0x00007ff3f946751c in tls_handle_alpn (s=0x558943d6d540) at ../ssl/statem/statem_srvr.c:2147
lunarmodules#6 0x00007ff3f944ddd8 in tls_parse_all_extensions (s=s@entry=0x558943d6d540, context=context@entry=128, exts=<optimized out>, x=x@entry=0x0, chainidx=chainidx@entry=0, fin=fin@entry=1) at ../ssl/statem/extensions.c:761
lunarmodules#7 0x00007ff3f94677bc in tls_early_post_process_client_hello (s=0x558943d6d540) at ../ssl/statem/statem_srvr.c:1881
lunarmodules#8 tls_post_process_client_hello (s=0x558943d6d540, wst=<optimized out>) at ../ssl/statem/statem_srvr.c:2225
lunarmodules#9 0x00007ff3f945726b in read_state_machine (s=0x558943d6d540) at ../ssl/statem/statem.c:664
lunarmodules#10 state_machine (s=0x558943d6d540, server=1) at ../ssl/statem/statem.c:434
lunarmodules#11 0x00007ff3f9442f74 in SSL_do_handshake (s=0x558943d6d540) at ../ssl/ssl_lib.c:3615
lunarmodules#12 0x00007ff3f94ad44f in handshake (ssl=0x558944811648) at src/ssl.c:120
lunarmodules#13 meth_handshake (L=0x5589432a2268) at src/ssl.c:388
lunarmodules#14 0x00005589415a8e61 in luaD_precall (L=L@entry=0x5589432a2268, func=func@entry=0x558944223f20, nresults=2) at ldo.c:503
(gdb) frame 13
lunarmodules#13 meth_handshake (L=0x5589432a2268) at src/ssl.c:388
(gdb) p ctx
$1 = (p_context) 0x5589433ab508
This is passed as 'arg' to alpn_cb(), note different pointer
arg=0x55894450d2a8.
Theory:
When selecting a context during SNI negotiation, the selected context
would be a different one from the one where meth_handshake() has updated
the Lua state reference. Thus the reference can still be invalid and
cause a crash when invoving a callback afterwards, such as the ALPN one.
Solution:
Update the Lua state reference on the selected SSL context after SNI.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes that allow for session reuse
Sample scripts included.