Skip to content

Commit b83e1f0

Browse files
committed
SSL_CTX_get0_implemented_groups for TLS group metrics
Building on PR #11844 which added TLS group metrics, this change uses SSL_CTX_get0_implemented_groups() to dynamically discover all supported TLS groups at initialization, including KEMs (Key Encapsulation Mechanisms) like X25519MLKEM768 and SecP256r1MLKEM768 that don't have standard NIDs defined in older OpenSSL versions. This fixes issue #12622 where KEM groups were being reported as OTHER instead of their actual group names. The implementation adds a new conditional compilation path that uses string-based group maps (similar to BoringSSL) when SSL_CTX_get0_implemented_groups is available, falling back to the NID-based approach for older OpenSSL 3.x versions. Fixes: #12622
1 parent 577cbed commit b83e1f0

File tree

5 files changed

+92
-26
lines changed

5 files changed

+92
-26
lines changed

CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -477,6 +477,7 @@ check_symbol_exists(SSL_get_shared_curve "openssl/ssl.h" HAVE_SSL_GET_SHARED_CUR
477477
check_symbol_exists(SSL_get_curve_name "openssl/ssl.h" HAVE_SSL_GET_CURVE_NAME)
478478
check_symbol_exists(SSL_get0_group_name "openssl/ssl.h" HAVE_SSL_GET0_GROUP_NAME)
479479
check_symbol_exists(SSL_get_negotiated_group "openssl/ssl.h" HAVE_SSL_GET_NEGOTIATED_GROUP)
480+
check_symbol_exists(SSL_CTX_get0_implemented_groups "openssl/ssl.h" HAVE_SSL_CTX_GET0_IMPLEMENTED_GROUPS)
480481
check_symbol_exists(SSL_get_group_id "openssl/ssl.h" HAVE_SSL_GET_GROUP_ID)
481482
check_symbol_exists(SSL_get_group_name "openssl/ssl.h" HAVE_SSL_GET_GROUP_NAME)
482483
check_symbol_exists(SSL_group_to_name "openssl/ssl.h" HAVE_SSL_GROUP_TO_NAME)

include/tscore/ink_config.h.cmake.in

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,7 @@ const int DEFAULT_STACKSIZE = @DEFAULT_STACK_SIZE@;
177177
#cmakedefine01 HAVE_SSL_GET_CURVE_NAME
178178
#cmakedefine01 HAVE_SSL_GET0_GROUP_NAME
179179
#cmakedefine01 HAVE_SSL_GET_NEGOTIATED_GROUP
180+
#cmakedefine01 HAVE_SSL_CTX_GET0_IMPLEMENTED_GROUPS
180181
#cmakedefine01 HAVE_SSL_GET_GROUP_ID
181182
#cmakedefine01 HAVE_SSL_GET_GROUP_NAME
182183
#cmakedefine01 HAVE_SSL_GROUP_TO_NAME

src/iocore/net/SSLStats.cc

Lines changed: 54 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,10 @@
3434
SSLStatsBlock ssl_rsb;
3535
std::unordered_map<std::string, Metrics::Counter::AtomicType *> cipher_map;
3636

37-
#ifdef OPENSSL_IS_BORINGSSL
37+
#if defined(OPENSSL_IS_BORINGSSL) || HAVE_SSL_CTX_GET0_IMPLEMENTED_GROUPS
3838
std::unordered_map<std::string, Metrics::Counter::AtomicType *> tls_group_map;
3939
std::unordered_map<std::string, Metrics::Counter::AtomicType *> tls_group_handshake_time_map;
40-
#elif defined(SSL_get_negotiated_group)
40+
#elif HAVE_SSL_GET_NEGOTIATED_GROUP
4141
std::unordered_map<int, Metrics::Counter::AtomicType *> tls_group_map;
4242
std::unordered_map<int, Metrics::Counter::AtomicType *> tls_group_handshake_time_map;
4343
#endif
@@ -50,7 +50,7 @@ DbgCtl dbg_ctl_ssl{"ssl"};
5050
constexpr std::string_view UNKNOWN_CIPHER{"(NONE)"};
5151
#endif
5252

53-
#if defined(OPENSSL_IS_BORINGSSL) || defined(SSL_get_negotiated_group)
53+
#if defined(OPENSSL_IS_BORINGSSL) || HAVE_SSL_CTX_GET0_IMPLEMENTED_GROUPS || HAVE_SSL_GET_NEGOTIATED_GROUP
5454

5555
template <typename T>
5656
void
@@ -73,9 +73,10 @@ add_group_stat(T key, const std::string &name)
7373
Dbg(dbg_ctl_ssl, "registering SSL group handshake time metric '%s.handshake_time'", name.c_str());
7474
}
7575
}
76-
#endif // OPENSSL_IS_BORINGSSL or SSL_get_negotiated_group
76+
#endif // OPENSSL_IS_BORINGSSL or HAVE_SSL_CTX_GET0_IMPLEMENTED_GROUPS or HAVE_SSL_GET_NEGOTIATED_GROUP
7777

78-
#if not defined(OPENSSL_IS_BORINGSSL) and defined(SSL_get_negotiated_group) // OPENSSL 3.x
78+
#if not defined(OPENSSL_IS_BORINGSSL) and not HAVE_SSL_CTX_GET0_IMPLEMENTED_GROUPS and \
79+
HAVE_SSL_GET_NEGOTIATED_GROUP // OPENSSL 3.x without SSL_CTX_get0_implemented_groups
7980

8081
struct TLSGroup {
8182
int nid;
@@ -115,7 +116,7 @@ const TLSGroup TLS_GROUPS[] = {
115116
#endif
116117
};
117118

118-
#endif // OPENSSL 3.x
119+
#endif // OPENSSL 3.x without SSL_CTX_get0_implemented_groups
119120

120121
} // end anonymous namespace
121122

@@ -294,16 +295,18 @@ SSLInitializeStatistics()
294295
add_cipher_stat(cipher_name, stat_name);
295296
}
296297
#else
297-
// Get and register the SSL cipher stats. Note that we are using the default SSL context to obtain
298-
// the cipher list. This means that the set of ciphers is fixed by the build configuration and not
299-
// filtered by proxy.config.ssl.server.cipher_suite. This keeps the set of cipher suites stable across
300-
// configuration reloads and works for the case where we honor the client cipher preference.
301-
SSLMultiCertConfigLoader loader(nullptr);
302-
SSL_CTX *ctx = loader.default_server_ssl_ctx();
303-
SSL *ssl = SSL_new(ctx);
298+
// Acquire the loaded SSL certificate configuration to enumerate ciphers and groups.
299+
// This must be called AFTER SSLCertificateConfig::startup().
300+
SSLCertificateConfig::scoped_config lookup;
301+
if (!lookup || !lookup->ssl_default) {
302+
Dbg(dbg_ctl_ssl, "No SSL configuration, skipping cipher/group statistics initialization");
303+
return;
304+
}
305+
306+
SSL_CTX *ctx = lookup->ssl_default.get();
307+
SSL *ssl = SSL_new(ctx);
304308
STACK_OF(SSL_CIPHER) *ciphers = SSL_get_ciphers(ssl);
305309

306-
// BoringSSL has sk_SSL_CIPHER_num() return a size_t (well, sk_num() is)
307310
for (int index = 0; index < static_cast<int>(sk_SSL_CIPHER_num(ciphers)); index++) {
308311
SSL_CIPHER *cipher = const_cast<SSL_CIPHER *>(sk_SSL_CIPHER_value(ciphers, index));
309312
const char *cipherName = SSL_CIPHER_get_name(cipher);
@@ -312,14 +315,48 @@ SSLInitializeStatistics()
312315
add_cipher_stat(cipherName, statName);
313316
}
314317

318+
// TLS Group
319+
#if HAVE_SSL_CTX_GET0_IMPLEMENTED_GROUPS
320+
STACK_OF(OPENSSL_CSTRING) *group_names = sk_OPENSSL_CSTRING_new_null();
321+
if (group_names == nullptr) {
322+
Error("Failed to allocate stack for TLS group names");
323+
} else {
324+
constexpr int ALL_GROUPS = 1;
325+
DbgPrint(dbg_ctl_ssl, "Calling SSL_CTX_get0_implemented_groups on loaded SSL context");
326+
if (SSL_CTX_get0_implemented_groups(ctx, ALL_GROUPS, group_names) != 1) {
327+
Error("Failed to get implemented groups via SSL_CTX_get0_implemented_groups");
328+
}
329+
int const num_groups = sk_OPENSSL_CSTRING_num(group_names);
330+
DbgPrint(dbg_ctl_ssl, "SSL_CTX_get0_implemented_groups returned %d groups", num_groups);
331+
332+
for (int index = 0; index < num_groups; index++) {
333+
const char *name = sk_OPENSSL_CSTRING_value(group_names, index);
334+
if (name == nullptr) {
335+
Error("NULL group name returned for index %d in SSL_CTX_get0_implemented_groups", index);
336+
continue;
337+
}
338+
add_group_stat<std::string>(name, name);
339+
}
340+
341+
// Note: We don't free group_names as the strings are owned by OpenSSL's internal structures.
342+
sk_OPENSSL_CSTRING_free(group_names);
343+
}
344+
345+
// Add "OTHER" for groups not discovered.
346+
add_group_stat<std::string>("OTHER", "OTHER");
347+
#elif HAVE_SSL_GET_NEGOTIATED_GROUP
348+
// Use static NID table for group registration.
349+
for (auto group : TLS_GROUPS) {
350+
add_group_stat<int>(group.nid, group.name);
351+
}
352+
#endif // HAVE_SSL_CTX_GET0_IMPLEMENTED_GROUPS or HAVE_SSL_GET_NEGOTIATED_GROUP
353+
315354
SSL_free(ssl);
316-
SSLReleaseContext(ctx);
317355
#endif
318356

319357
// Add "OTHER" for ciphers not on the map
320358
add_cipher_stat(SSL_CIPHER_STAT_OTHER.c_str(), "proxy.process.ssl.cipher.user_agent." + SSL_CIPHER_STAT_OTHER);
321359

322-
// TLS Group
323360
#if defined(OPENSSL_IS_BORINGSSL)
324361
size_t list_size = SSL_get_all_group_names(nullptr, 0);
325362
std::vector<const char *> group_list(list_size);
@@ -328,9 +365,5 @@ SSLInitializeStatistics()
328365
for (const char *name : group_list) {
329366
add_group_stat<std::string>(name, name);
330367
}
331-
#elif defined(SSL_get_negotiated_group)
332-
for (auto group : TLS_GROUPS) {
333-
add_group_stat<int>(group.nid, group.name);
334-
}
335-
#endif // OPENSSL_IS_BORINGSSL or SSL_get_negotiated_group
368+
#endif // OPENSSL_IS_BORINGSSL
336369
}

src/iocore/net/SSLStats.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323

2424
#pragma once
2525

26+
#include "tscore/ink_config.h"
2627
#include "tsutil/Metrics.h"
2728

2829
#include <openssl/ssl.h>
@@ -114,16 +115,17 @@ struct SSLStatsBlock {
114115
extern SSLStatsBlock ssl_rsb;
115116
extern std::unordered_map<std::string, Metrics::Counter::AtomicType *> cipher_map;
116117

117-
#if defined(OPENSSL_IS_BORINGSSL)
118+
#if defined(OPENSSL_IS_BORINGSSL) || HAVE_SSL_CTX_GET0_IMPLEMENTED_GROUPS
118119
extern std::unordered_map<std::string, Metrics::Counter::AtomicType *> tls_group_map;
119120
extern std::unordered_map<std::string, Metrics::Counter::AtomicType *> tls_group_handshake_time_map;
120-
#elif defined(SSL_get_negotiated_group)
121+
#elif HAVE_SSL_GET_NEGOTIATED_GROUP
121122
extern std::unordered_map<int, Metrics::Counter::AtomicType *> tls_group_map;
122123
extern std::unordered_map<int, Metrics::Counter::AtomicType *> tls_group_handshake_time_map;
123124
constexpr int SSL_GROUP_STAT_OTHER_KEY = 0;
124125
#endif
125126

126127
// Initialize SSL statistics.
128+
// Must be called AFTER SSLCertificateConfig::startup() so SSL contexts are loaded.
127129
void SSLInitializeStatistics();
128130

129131
const std::string SSL_CIPHER_STAT_OTHER = "OTHER";

src/iocore/net/TLSBasicSupport.cc

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
limitations under the License.
2323
*/
2424

25+
#include "P_SSLUtils.h"
2526
#include "SSLStats.h"
2627
#include "iocore/net/TLSBasicSupport.h"
2728
#if defined(OPENSSL_IS_BORINGSSL)
@@ -249,7 +250,21 @@ TLSBasicSupport::_record_tls_handshake_end_time()
249250
Metrics::Counter::increment(it->second, ssl_handshake_time);
250251
}
251252
}
252-
#elif defined(SSL_get_negotiated_group)
253+
#elif HAVE_SSL_CTX_GET0_IMPLEMENTED_GROUPS
254+
SSL *ssl = this->_get_ssl_object();
255+
std::string_view group_name = SSLGetGroupName(ssl);
256+
if (!group_name.empty()) {
257+
std::string group_str(group_name);
258+
if (auto it = tls_group_handshake_time_map.find(group_str); it != tls_group_handshake_time_map.end()) {
259+
Metrics::Counter::increment(it->second, ssl_handshake_time);
260+
} else {
261+
auto other = tls_group_handshake_time_map.find("OTHER");
262+
if (other != tls_group_handshake_time_map.end()) {
263+
Metrics::Counter::increment(other->second, ssl_handshake_time);
264+
}
265+
}
266+
}
267+
#elif HAVE_SSL_GET_NEGOTIATED_GROUP
253268
SSL *ssl = this->_get_ssl_object();
254269
int nid = SSL_get_negotiated_group(const_cast<SSL *>(ssl));
255270
if (nid != NID_undef) {
@@ -281,7 +296,21 @@ TLSBasicSupport::_update_end_of_handshake_stats()
281296
Warning("Unknown TLS Group");
282297
}
283298
}
284-
#elif defined(SSL_get_negotiated_group)
299+
#elif HAVE_SSL_CTX_GET0_IMPLEMENTED_GROUPS
300+
SSL *ssl = this->_get_ssl_object();
301+
std::string_view group_name = SSLGetGroupName(ssl);
302+
if (!group_name.empty()) {
303+
std::string group_str(group_name);
304+
if (auto it = tls_group_map.find(group_str); it != tls_group_map.end()) {
305+
Metrics::Counter::increment(it->second);
306+
} else {
307+
auto other = tls_group_map.find("OTHER");
308+
if (other != tls_group_map.end()) {
309+
Metrics::Counter::increment(other->second);
310+
}
311+
}
312+
}
313+
#elif HAVE_SSL_GET_NEGOTIATED_GROUP
285314
SSL *ssl = this->_get_ssl_object();
286315
int nid = SSL_get_negotiated_group(const_cast<SSL *>(ssl));
287316
if (nid != NID_undef) {
@@ -292,5 +321,5 @@ TLSBasicSupport::_update_end_of_handshake_stats()
292321
Metrics::Counter::increment(other->second);
293322
}
294323
}
295-
#endif // OPENSSL_IS_BORINGSSL or SSL_get_negotiated_group
324+
#endif // OPENSSL_IS_BORINGSSL or HAVE_SSL_CTX_GET0_IMPLEMENTED_GROUPS or HAVE_SSL_GET_NEGOTIATED_GROUP
296325
}

0 commit comments

Comments
 (0)