You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by GitBox <gi...@apache.org> on 2021/05/11 15:17:29 UTC
[GitHub] [trafficserver] duke8253 opened a new pull request #7808: Fix origin session related crashes
duke8253 opened a new pull request #7808:
URL: https://github.com/apache/trafficserver/pull/7808
After testing origin session reuse #7479, we've noticed a few crashes, this PR fixes these crashes.
```
#0 0x0000000000000000 in ?? ()
#1 0x00002b6a937248e3 in ssl_security (s=s@entry=0x2b70832ca000, op=op@entry=131077, bits=<optimized out>, nid=<optimized out>, other=other@entry=0x2b6a97c50cce) at ssl/ssl_cert.c:964
#2 0x00002b6a937550d0 in tls_curve_allowed (s=s@entry=0x2b70832ca000, curve=curve@entry=29, op=op@entry=131077) at ssl/t1_lib.c:263
#3 0x00002b6a93755230 in tls1_shared_group (nmatch=0, s=0x2b70832ca000) at ssl/t1_lib.c:305
#4 tls1_shared_group (s=0x2b70832ca000, nmatch=<optimized out>) at ssl/t1_lib.c:283
#5 0x00002b6a93721147 in ssl3_ctrl (s=0x2b70832ca000, cmd=<optimized out>, larg=0, parg=0x0) at ssl/s3_lib.c:3633
#6 0x0000000000781022 in SSLGetCurveNID (ssl=ssl@entry=0x2b70832ca000) at ../../../../../../_vcs/trafficserver9.1/iocore/net/SSLUtils.cc:2421
#7 0x000000000077c7f4 in SSLSessionBucket::insertSession(SSLSessionID const&, ssl_session_st*, ssl_st*) () at ../../../../../../_vcs/trafficserver9.1/iocore/net/SSLSessionCache.cc:160
#8 0x000000000077ccc9 in SSLSessionCache::insertSession(SSLSessionID const&, ssl_session_st*, ssl_st*) () at ../../../../../../_vcs/trafficserver9.1/iocore/net/SSLSessionCache.cc:113
#9 0x000000000077f7fb in ssl_new_cached_session(ssl_st*, ssl_session_st*) () at ../../../../../../_vcs/trafficserver9.1/iocore/net/SSLUtils.cc:213
#10 0x00002b6a9372cdda in ssl_update_cache (s=s@entry=0x2b70832ca000, mode=mode@entry=2) at ssl/ssl_lib.c:3507
#11 0x00002b6a9374a9e3 in tls_finish_handshake (s=0x2b70832ca000, wst=<optimized out>, clearbufs=<optimized out>, stop=1) at ssl/statem/statem_lib.c:1088
#12 0x00002b6a9374071e in write_state_machine (s=0x2b70832ca000) at ssl/statem/statem.c:810
#13 state_machine (s=0x2b70832ca000, server=1) at ssl/statem/statem.c:443
#14 0x00002b6a9372cf64 in SSL_do_handshake (s=0x2b70832ca000) at ssl/ssl_lib.c:3672
#15 0x0000000000786160 in SSLAccept(ssl_st*) () at ../../../../../../_vcs/trafficserver9.1/iocore/net/SSLUtils.cc:1916
#16 0x000000000076cb56 in SSLNetVConnection::sslServerHandShakeEvent(int&) () at ../../../../../../_vcs/trafficserver9.1/iocore/net/SSLNetVConnection.cc:1253
#17 0x0000000000770455 in SSLNetVConnection::sslStartHandShake(int, int&) () at ../../../../../../_vcs/trafficserver9.1/iocore/net/SSLNetVConnection.cc:1060
#18 0x000000000076f2f2 in SSLNetVConnection::net_read_io(NetHandler*, EThread*) () at ../../../../../../_vcs/trafficserver9.1/iocore/net/SSLNetVConnection.cc:569
#19 0x00000000007937fe in NetHandler::process_ready_list (this=this@entry=0x2b6a98934f30) at ../../../../../../_vcs/trafficserver9.1/iocore/net/UnixNet.cc:415
#20 0x0000000000793c58 in NetHandler::waitForActivity(long) () at ../../../../../../_vcs/trafficserver9.1/iocore/net/UnixNet.cc:549
#21 0x00000000007e8461 in EThread::execute_regular (this=this@entry=0x2b6a98930e40) at ../../../../../../_vcs/trafficserver9.1/iocore/eventsystem/I_PriorityEventQueue.h:115
#22 0x00000000007e85e2 in execute (this=0x2b6a98930e40) at ../../../../../../_vcs/trafficserver9.1/iocore/eventsystem/UnixEThread.cc:364
#23 EThread::execute (this=0x2b6a98930e40) at ../../../../../../_vcs/trafficserver9.1/iocore/eventsystem/UnixEThread.cc:342
#24 0x00000000007e6aba in spawn_thread_internal (a=0x2b6a9639df80) at ../../../../../../_vcs/trafficserver9.1/iocore/eventsystem/Thread.cc:92
#25 0x00002b6a945e1ea5 in start_thread () from /lib64/libpthread.so.0
#26 0x00002b6a953189fd in clone () from /lib64/libc.so.6
```
```
#0 OPENSSL_sk_free (st=0x1) at crypto/stack/stack.c:376
#1 OPENSSL_sk_free (st=0x1) at crypto/stack/stack.c:372
#2 0x00002ad1808fc4f4 in sk_void_free (sk=<optimized out>) at include/openssl/crypto.h:89
#3 CRYPTO_free_ex_data (class_index=class_index@entry=2, obj=obj@entry=0x2ad3fa26a780, ad=ad@entry=0x2ad3fa26a980) at crypto/ex_data.c:361
#4 0x00002ad180539738 in SSL_SESSION_free (ss=0x2ad3fa26a780) at ssl/ssl_sess.c:759
#5 SSL_SESSION_free (ss=0x2ad3fa26a780) at ssl/ssl_sess.c:747
#6 0x00002ad180534516 in SSL_free (s=0x2ad512c13c00) at ssl/ssl_lib.c:1179
#7 SSL_free (s=0x2ad512c13c00) at ssl/ssl_lib.c:1146
#8 0x000000000076b024 in SSLNetVConnection::clear (this=0x2ad419e54600) at ../../../../../../_vcs/trafficserver9.1/iocore/net/SSLNetVConnection.cc:939
#9 0x0000000000769340 in free (t=0x2ad1866b0dc0, this=0x2ad419e54600) at ../../../../../../_vcs/trafficserver9.1/iocore/net/SSLNetVConnection.cc:984
#10 SSLNetVConnection::free (this=0x2ad419e54600, t=0x2ad1866b0dc0) at ../../../../../../_vcs/trafficserver9.1/iocore/net/SSLNetVConnection.cc:959
#11 0x000000000079352f in NetHandler::free_netevent (this=0x2ad1866b4eb0, ne=ne@entry=0x2ad419e547b0) at ../../../../../../_vcs/trafficserver9.1/iocore/net/UnixNet.cc:367
#12 0x00000000007a1ace in write_signal_and_update (event=<optimized out>, vc=0x2ad419e54600) at ../../../../../../_vcs/trafficserver9.1/iocore/net/UnixNetVConnection.cc:139
#13 read_signal_and_update(int, UnixNetVConnection*) () at ../../../../../../_vcs/trafficserver9.1/iocore/net/UnixNetVConnection.cc:113
#14 0x000000000076fa4e in SSLNetVConnection::net_read_io(NetHandler*, EThread*) () at ../../../../../../_vcs/trafficserver9.1/iocore/net/SSLNetVConnection.cc:681
#15 0x00000000007937fe in NetHandler::process_ready_list (this=this@entry=0x2ad1866b4eb0) at ../../../../../../_vcs/trafficserver9.1/iocore/net/UnixNet.cc:415
#16 0x0000000000793c58 in NetHandler::waitForActivity(long) () at ../../../../../../_vcs/trafficserver9.1/iocore/net/UnixNet.cc:549
#17 0x00000000007e8461 in EThread::execute_regular (this=this@entry=0x2ad1866b0dc0) at ../../../../../../_vcs/trafficserver9.1/iocore/eventsystem/I_PriorityEventQueue.h:115
#18 0x00000000007e85e2 in execute (this=0x2ad1866b0dc0) at ../../../../../../_vcs/trafficserver9.1/iocore/eventsystem/UnixEThread.cc:364
#19 EThread::execute (this=0x2ad1866b0dc0) at ../../../../../../_vcs/trafficserver9.1/iocore/eventsystem/UnixEThread.cc:342
#20 0x00000000007e6aba in spawn_thread_internal (a=0x2ad1831a7b80) at ../../../../../../_vcs/trafficserver9.1/iocore/eventsystem/Thread.cc:92
#21 0x00002ad1813e7ea5 in start_thread () from /lib64/libpthread.so.0
#22 0x00002ad18211e9fd in clone () from /lib64/libc.so.6
```
```
#0 0x00002ab0bb6a1e75 in CRYPTO_UP_REF (lock=0xe8ee894818244c8d, ret=<synthetic pointer>, val=0x75d500 <verify_callback(int, x509_store_ctx_st*)+112>) at include/internal/refcount.h:34
#1 X509_up_ref (x=0x75d440 <ssl_client_cert_callback(SSL*, void*)>) at crypto/x509/x509_set.c:103
#2 0x00002ab0bb26398e in ssl_session_dup (src=src@entry=0x2ab45544a900, ticket=ticket@entry=0) at ssl/ssl_sess.c:149
#3 0x00002ab0bb2752c4 in tls_process_new_session_ticket (s=0x2ab455268400, pkt=0x2ab0c690ae30) at ssl/statem/statem_clnt.c:2619
#4 0x00002ab0bb276e55 in ossl_statem_client_process_message (s=0x2ab455268400, pkt=<optimized out>) at ssl/statem/statem_clnt.c:1060
#5 0x00002ab0bb270a23 in read_state_machine (s=0x2ab455268400) at ssl/statem/statem.c:636
#6 state_machine (s=0x2ab455268400, server=0) at ssl/statem/statem.c:434
#7 0x00002ab0bb249a19 in ssl3_read_bytes (s=0x2ab455268400, type=23, recvd_type=0x0, buf=0x2ab43d8ee000 <Address 0x2ab43d8ee000 out of bounds>, len=8192, peek=0,
readbytes=0x2ab0c690aff8) at ssl/record/rec_layer_s3.c:1658
#8 0x00002ab0bb2503e5 in ssl3_read_internal (s=0x2ab455268400, buf=0x2ab43d8ee000, len=8192, peek=0, readbytes=0x2ab0c690aff8) at ssl/s3_lib.c:4464
#9 0x00002ab0bb25acfa in ssl_read_internal (readbytes=0x2ab0c690aff8, num=8192, buf=0x2ab43d8ee000, s=0x2ab455268400) at ssl/ssl_lib.c:1769
#10 ssl_read_internal (s=0x2ab455268400, buf=0x2ab43d8ee000, num=8192, readbytes=0x2ab0c690aff8) at ssl/ssl_lib.c:1732
#11 0x00002ab0bb25ade5 in SSL_read (s=s@entry=0x2ab455268400, buf=buf@entry=0x2ab43d8ee000, num=num@entry=8192) at ssl/ssl_lib.c:1783
#12 0x00000000007855fb in SSLReadBuffer(ssl_st*, void*, long, long&) () at ../../../../../../_vcs/trafficserver9.1/iocore/net/SSLUtils.cc:1844
#13 0x000000000076a455 in ssl_read_from_net(SSLNetVConnection*, EThread*, long&) () at ../../../../../../_vcs/trafficserver9.1/iocore/net/SSLNetVConnection.cc:281
#14 0x000000000076f453 in SSLNetVConnection::net_read_io(NetHandler*, EThread*) () at ../../../../../../_vcs/trafficserver9.1/iocore/net/SSLNetVConnection.cc:671
#15 0x00000000007937fe in NetHandler::process_ready_list (this=this@entry=0x2ab0c23780b0) at ../../../../../../_vcs/trafficserver9.1/iocore/net/UnixNet.cc:415
#16 0x0000000000793c58 in NetHandler::waitForActivity(long) () at ../../../../../../_vcs/trafficserver9.1/iocore/net/UnixNet.cc:549
#17 0x00000000007e8461 in EThread::execute_regular (this=this@entry=0x2ab0c2373fc0) at ../../../../../../_vcs/trafficserver9.1/iocore/eventsystem/I_PriorityEventQueue.h:115
#18 0x00000000007e85e2 in execute (this=0x2ab0c2373fc0) at ../../../../../../_vcs/trafficserver9.1/iocore/eventsystem/UnixEThread.cc:364
#19 EThread::execute (this=0x2ab0c2373fc0) at ../../../../../../_vcs/trafficserver9.1/iocore/eventsystem/UnixEThread.cc:342
#20 0x00000000007e6aba in spawn_thread_internal (a=0x2ab0bdda7e80) at ../../../../../../_vcs/trafficserver9.1/iocore/eventsystem/Thread.cc:92
#21 0x00002ab0bc111ea5 in start_thread () from /lib64/libpthread.so.0
#22 0x00002ab0bce489fd in clone () from /lib64/libc.so.6
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] duke8253 merged pull request #7808: Fix origin session related crashes
Posted by GitBox <gi...@apache.org>.
duke8253 merged pull request #7808:
URL: https://github.com/apache/trafficserver/pull/7808
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] duke8253 commented on a change in pull request #7808: Fix origin session related crashes
Posted by GitBox <gi...@apache.org>.
duke8253 commented on a change in pull request #7808:
URL: https://github.com/apache/trafficserver/pull/7808#discussion_r630674203
##########
File path: iocore/net/SSLUtils.cc
##########
@@ -2034,7 +2034,12 @@ SSLConnect(SSL *ssl)
Debug("ssl.origin_session_cache", "origin session cache lookup key = %s", lookup_key.c_str());
- sess = origin_sess_cache->get_session(lookup_key);
+ TLSSessionResumptionSupport *srs = TLSSessionResumptionSupport::getInstance(ssl);
Review comment:
I agree, was trying to stay away from this, but looks like we'll have to do things like this for now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on a change in pull request #7808: Fix origin session related crashes
Posted by GitBox <gi...@apache.org>.
maskit commented on a change in pull request #7808:
URL: https://github.com/apache/trafficserver/pull/7808#discussion_r630665002
##########
File path: iocore/net/TLSSessionResumptionSupport.cc
##########
@@ -172,6 +172,30 @@ TLSSessionResumptionSupport::getSession(SSL *ssl, const unsigned char *id, int l
return session;
}
+SSL_SESSION *
+TLSSessionResumptionSupport::getOriginSession(SSL *ssl, const std::string &lookup_key)
Review comment:
I'm OK with adding this for now, but I wonder if we can have only one `getSession` and lookup different caches inside it depends on the connection direction (SSL_is_server). I t think we never call `getOriginSession` on client sessions (inbound connections), and vice versa.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on a change in pull request #7808: Fix origin session related crashes
Posted by GitBox <gi...@apache.org>.
maskit commented on a change in pull request #7808:
URL: https://github.com/apache/trafficserver/pull/7808#discussion_r630661453
##########
File path: iocore/net/SSLUtils.cc
##########
@@ -2034,7 +2034,12 @@ SSLConnect(SSL *ssl)
Debug("ssl.origin_session_cache", "origin session cache lookup key = %s", lookup_key.c_str());
- sess = origin_sess_cache->get_session(lookup_key);
+ TLSSessionResumptionSupport *srs = TLSSessionResumptionSupport::getInstance(ssl);
Review comment:
Adding these lines here is actually a bit of sad thing, because `SSLConnect` was originally independent from NetVC. You could use it for any SSL connections. `SSLReadBuffer` too, before supporting early data. I think that's the reason those functions are in SSLUtils.
Now those two functions depend on NetVC, and there is no longer a reason to have those in SSLUtils as non-member functions. Although I don't think we should do this on this PR, we should probably make the two functions (and SSLWriteBuffer) member functions of SSLNetVC and move them to P_SSLNetVConnection.h. Then we don't need to call `getInstance` to call `getOriginSession`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on a change in pull request #7808: Fix origin session related crashes
Posted by GitBox <gi...@apache.org>.
maskit commented on a change in pull request #7808:
URL: https://github.com/apache/trafficserver/pull/7808#discussion_r630665002
##########
File path: iocore/net/TLSSessionResumptionSupport.cc
##########
@@ -172,6 +172,30 @@ TLSSessionResumptionSupport::getSession(SSL *ssl, const unsigned char *id, int l
return session;
}
+SSL_SESSION *
+TLSSessionResumptionSupport::getOriginSession(SSL *ssl, const std::string &lookup_key)
Review comment:
I'm OK with adding this for now, but I wonder if we can have only one `getSession` and lookup different caches inside it depends on the connection direction (SSL_is_server). I don't think we never call `getOriginSession` on client sessions (inbound connections), and vice versa.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org