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
+}

Reply via email to