This is an automated email from the ASF dual-hosted git repository. cmcfarlen pushed a commit to branch 10.1.x in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 6de6ebb1d2985103d6a862b1bca09f4a58335b50 Author: Brian Neradt <[email protected]> AuthorDate: Wed May 7 16:18:36 2025 -0500 Call SSL_get_negotiated_group not during read early data (#12224) Recent versions of OpenSSL define SSL_CB_HANDSHAKE_DONE as follows: > Callback has been called because a handshake is finished. It also > occurs if the handshake is paused to allow the exchange of early data. Calling SSL_get_negotiated_group duing early data situations was causing a crash. This moves the calling of SSL_get_negotiated_group next to _record_tls_handshake_end_time where it will reliably not be called in the context of processing early data. Fixes: #12140 (cherry picked from commit afbb1f5d6d60996db410b765817e4763eb27650c) --- include/iocore/net/TLSBasicSupport.h | 1 + src/iocore/net/QUICNetVConnection.cc | 1 + src/iocore/net/SSLNetVConnection.cc | 2 +- src/iocore/net/SSLUtils.cc | 22 ---------------------- src/iocore/net/TLSBasicSupport.cc | 33 +++++++++++++++++++++++++++++++++ 5 files changed, 36 insertions(+), 23 deletions(-) diff --git a/include/iocore/net/TLSBasicSupport.h b/include/iocore/net/TLSBasicSupport.h index c269c51c0f..c814357ca4 100644 --- a/include/iocore/net/TLSBasicSupport.h +++ b/include/iocore/net/TLSBasicSupport.h @@ -79,6 +79,7 @@ protected: void _record_tls_handshake_begin_time(); void _record_tls_handshake_end_time(); + void _update_end_of_handshake_stats(); /** * Implementation should schedule either TS_EVENT_SSL_VERIFY_SERVER or TS_EVENT_SSL_VERIFY_CLIENT accordingly. diff --git a/src/iocore/net/QUICNetVConnection.cc b/src/iocore/net/QUICNetVConnection.cc index 27efe4a952..6a07b2cc32 100644 --- a/src/iocore/net/QUICNetVConnection.cc +++ b/src/iocore/net/QUICNetVConnection.cc @@ -250,6 +250,7 @@ QUICNetVConnection::_switch_to_established_state() { QUICConDebug("Enter state_connection_established"); this->_record_tls_handshake_end_time(); + this->_update_end_of_handshake_stats(); SET_HANDLER((NetVConnHandler)&QUICNetVConnection::state_established); this->_start_application(); this->_handshake_completed = true; diff --git a/src/iocore/net/SSLNetVConnection.cc b/src/iocore/net/SSLNetVConnection.cc index 0b719e460d..ff453b8bdd 100644 --- a/src/iocore/net/SSLNetVConnection.cc +++ b/src/iocore/net/SSLNetVConnection.cc @@ -1321,7 +1321,7 @@ SSLNetVConnection::sslServerHandShakeEvent(int &err) if (this->get_tls_handshake_begin_time()) { this->_record_tls_handshake_end_time(); - Metrics::Counter::increment(ssl_rsb.total_success_handshake_count_in); + this->_update_end_of_handshake_stats(); } if (this->get_tunnel_type() != SNIRoutingType::NONE) { diff --git a/src/iocore/net/SSLUtils.cc b/src/iocore/net/SSLUtils.cc index 40bbf7b470..a71180fa21 100644 --- a/src/iocore/net/SSLUtils.cc +++ b/src/iocore/net/SSLUtils.cc @@ -1077,28 +1077,6 @@ ssl_callback_info(const SSL *ssl, int where, int ret) } Metrics::Counter::increment(it->second); } - -#if defined(OPENSSL_IS_BORINGSSL) - uint16_t group_id = SSL_get_group_id(ssl); - if (group_id != 0) { - const char *group_name = SSL_get_group_name(group_id); - if (auto it = tls_group_map.find(group_name); it != tls_group_map.end()) { - Metrics::Counter::increment(it->second); - } else { - Warning("Unknown TLS Group"); - } - } -#elif defined(SSL_get_negotiated_group) - int nid = SSL_get_negotiated_group(const_cast<SSL *>(ssl)); - if (nid != NID_undef) { - if (auto it = tls_group_map.find(nid); it != tls_group_map.end()) { - Metrics::Counter::increment(it->second); - } else { - auto other = tls_group_map.find(SSL_GROUP_STAT_OTHER_KEY); - Metrics::Counter::increment(other->second); - } - } -#endif // OPENSSL_IS_BORINGSSL or SSL_get_negotiated_group } } diff --git a/src/iocore/net/TLSBasicSupport.cc b/src/iocore/net/TLSBasicSupport.cc index add717fb1c..ab97412ea4 100644 --- a/src/iocore/net/TLSBasicSupport.cc +++ b/src/iocore/net/TLSBasicSupport.cc @@ -24,6 +24,9 @@ #include "SSLStats.h" #include "iocore/net/TLSBasicSupport.h" +#if defined(OPENSSL_IS_BORINGSSL) +#include "tscore/Diags.h" // For Warning +#endif // OPENSSL_IS_BORINGSSL #include "tsutil/DbgCtl.h" #include <cinttypes> @@ -201,3 +204,33 @@ TLSBasicSupport::_record_tls_handshake_end_time() Dbg(dbg_ctl_ssl, "ssl handshake time:%" PRId64, ssl_handshake_time); Metrics::Counter::increment(ssl_rsb.total_handshake_time, ssl_handshake_time); } + +void +TLSBasicSupport::_update_end_of_handshake_stats() +{ + Metrics::Counter::increment(ssl_rsb.total_success_handshake_count_in); + +#if defined(OPENSSL_IS_BORINGSSL) + SSL *ssl = this->_get_ssl_object(); + uint16_t group_id = SSL_get_group_id(ssl); + if (group_id != 0) { + const char *group_name = SSL_get_group_name(group_id); + if (auto it = tls_group_map.find(group_name); it != tls_group_map.end()) { + Metrics::Counter::increment(it->second); + } else { + Warning("Unknown TLS Group"); + } + } +#elif defined(SSL_get_negotiated_group) + SSL *ssl = this->_get_ssl_object(); + int nid = SSL_get_negotiated_group(const_cast<SSL *>(ssl)); + if (nid != NID_undef) { + if (auto it = tls_group_map.find(nid); it != tls_group_map.end()) { + Metrics::Counter::increment(it->second); + } else { + auto other = tls_group_map.find(SSL_GROUP_STAT_OTHER_KEY); + Metrics::Counter::increment(other->second); + } + } +#endif // OPENSSL_IS_BORINGSSL or SSL_get_negotiated_group +}
