This is an automated email from the ASF dual-hosted git repository.
bneradt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new afbb1f5d6d Call SSL_get_negotiated_group not during read early data
(#12224)
afbb1f5d6d is described below
commit afbb1f5d6d60996db410b765817e4763eb27650c
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
---
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 ea5c9aae27..799cda55c0 100644
--- a/include/iocore/net/TLSBasicSupport.h
+++ b/include/iocore/net/TLSBasicSupport.h
@@ -83,6 +83,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 2298f354de..69ea874101 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 b7e5b0a511..2956237306 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 6737d734ae..0c3a5de50d 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>
@@ -217,3 +220,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
+}