From 245a01ef9dcb0b61894522a514e9c1e063ff8b81 Mon Sep 17 00:00:00 2001 From: Masaori Koshiba Date: Tue, 5 Nov 2024 15:51:30 +0900 Subject: [PATCH 1/4] Add metrics for negotiated group with client --- src/iocore/net/SSLStats.cc | 59 ++++++++++++++++++++++++++++++++++++++ src/iocore/net/SSLStats.h | 1 + src/iocore/net/SSLUtils.cc | 15 ++++++++++ 3 files changed, 75 insertions(+) diff --git a/src/iocore/net/SSLStats.cc b/src/iocore/net/SSLStats.cc index 68fe3e72994..bab15f719a1 100644 --- a/src/iocore/net/SSLStats.cc +++ b/src/iocore/net/SSLStats.cc @@ -32,11 +32,51 @@ SSLStatsBlock ssl_rsb; std::unordered_map cipher_map; +std::unordered_map tls_group_map; namespace { DbgCtl dbg_ctl_ssl{"ssl"}; +#if defined(OPENSSL_IS_BORINGSSL) || defined(SSL_get_negotiated_group) + +struct TLSGroup { + int nid; + std::string name; +}; + +// NID and Group table. Some groups are not defined by some library. +constexpr TLSGroup TLS_GROUPS[] = { + {NID_X9_62_prime256v1, "P-256" }, + {NID_secp384r1, "P-384" }, + {NID_secp521r1, "P-521" }, + {NID_X25519, "X25519" }, +#ifdef NID_secp224r1 + {NID_secp224r1, "P-224" }, +#endif +#ifdef NID_X448 + {NID_X448, "X448" }, +#endif +#ifdef NID_ffdhe2048 + {NID_ffdhe2048, "ffdhe2048" }, +#endif +#ifdef NID_ffdhe3072 + {NID_ffdhe3072, "ffdhe3072" }, +#endif +#ifdef NID_ffdhe4096 + {NID_ffdhe4096, "ffdhe4096" }, +#endif +#ifdef NID_ffdhe6144 + {NID_ffdhe6144, "ffdhe6144" }, +#endif +#ifdef NID_ffdhe8192 + {NID_ffdhe8192, "ffdhe8192" }, +#endif +#ifdef NID_X25519MLKEM768 + {NID_X25519MLKEM768, "X25519MLKEM768"}, +#endif +}; +#endif // OPENSSL_IS_BORINGSSL or SSL_get_negotiated_group } // end anonymous namespace // ToDo: This gets called once per global sync, for now at least. @@ -85,6 +125,18 @@ add_cipher_stat(const char *cipherName, const std::string &statName) } } +static void +add_group_stat(int nid, const std::string &stat_name) +{ + // If not already registered ... + if (tls_group_map.find(nid) == tls_group_map.end()) { + Metrics::Counter::AtomicType *metric = Metrics::Counter::createPtr(stat_name); + + tls_group_map.emplace(nid, metric); + Dbg(dbg_ctl_ssl, "registering SSL group metric '%s'", stat_name.c_str()); + } +} + void SSLInitializeStatistics() { @@ -175,6 +227,13 @@ SSLInitializeStatistics() // Add "OTHER" for ciphers not on the map add_cipher_stat(SSL_CIPHER_STAT_OTHER.c_str(), "proxy.process.ssl.cipher.user_agent." + SSL_CIPHER_STAT_OTHER); +#if defined(OPENSSL_IS_BORINGSSL) || defined(SSL_get_negotiated_group) + // TLS Groups + for (auto group : TLS_GROUPS) { + add_group_stat(group.nid, "proxy.process.ssl.group.user_agent." + group.name); + } +#endif // OPENSSL_IS_BORINGSSL or SSL_get_negotiated_group + SSL_free(ssl); SSLReleaseContext(ctx); } diff --git a/src/iocore/net/SSLStats.h b/src/iocore/net/SSLStats.h index 6dbf070e422..3ebb14f835b 100644 --- a/src/iocore/net/SSLStats.h +++ b/src/iocore/net/SSLStats.h @@ -99,6 +99,7 @@ struct SSLStatsBlock { extern SSLStatsBlock ssl_rsb; extern std::unordered_map cipher_map; +extern std::unordered_map tls_group_map; // Initialize SSL statistics. void SSLInitializeStatistics(); diff --git a/src/iocore/net/SSLUtils.cc b/src/iocore/net/SSLUtils.cc index b519a0778fc..5564ff619d2 100644 --- a/src/iocore/net/SSLUtils.cc +++ b/src/iocore/net/SSLUtils.cc @@ -1086,6 +1086,20 @@ ssl_callback_info(const SSL *ssl, int where, int ret) } Metrics::Counter::increment(it->second); } + +#if defined(OPENSSL_IS_BORINGSSL) || defined(SSL_get_negotiated_group) + // TLS Group +#ifdef OPENSSL_IS_BORINGSSL + int nid = SSL_get_negotiated_group(ssl); +#elif defined(SSL_get_negotiated_group) + int nid = SSL_get_negotiated_group(const_cast(ssl)); +#endif + if (nid != NID_undef) { + if (auto it = tls_group_map.find(nid); it != tls_group_map.end()) { + Metrics::Counter::increment(it->second); + } + } +#endif // OPENSSL_IS_BORINGSSL or SSL_get_negotiated_group } } @@ -1543,6 +1557,7 @@ SSLMultiCertConfigLoader::_set_curves([[maybe_unused]] SSL_CTX *ctx) } } #endif + return true; } From 0ff768828b0d4b619e120f4014349b5be36821e0 Mon Sep 17 00:00:00 2001 From: Masaori Koshiba Date: Wed, 6 Nov 2024 15:17:32 +0900 Subject: [PATCH 2/4] Use SSL_get_all_group_names for BoringSSL --- src/iocore/net/SSLStats.cc | 79 ++++++++++++++++++++++++-------------- src/iocore/net/SSLStats.h | 8 ++++ src/iocore/net/SSLUtils.cc | 19 ++++++--- 3 files changed, 71 insertions(+), 35 deletions(-) diff --git a/src/iocore/net/SSLStats.cc b/src/iocore/net/SSLStats.cc index bab15f719a1..100935cba1c 100644 --- a/src/iocore/net/SSLStats.cc +++ b/src/iocore/net/SSLStats.cc @@ -32,7 +32,12 @@ SSLStatsBlock ssl_rsb; std::unordered_map cipher_map; -std::unordered_map tls_group_map; + +#ifdef OPENSSL_IS_BORINGSSL +std::unordered_map tls_group_map; +#elif defined(SSL_get_negotiated_group) +std::unordered_map tls_group_map; +#endif namespace { @@ -40,6 +45,23 @@ DbgCtl dbg_ctl_ssl{"ssl"}; #if defined(OPENSSL_IS_BORINGSSL) || defined(SSL_get_negotiated_group) +template +void +add_group_stat(T key, const std::string &name) +{ + // If not already registered ... + if (tls_group_map.find(key) == tls_group_map.end()) { + Metrics::Counter::AtomicType *metric = Metrics::Counter::createPtr("proxy.process.ssl.group.user_agent." + name); + + tls_group_map.emplace(key, metric); + Dbg(dbg_ctl_ssl, "registering SSL group metric '%s'", name.c_str()); + } +} + +#endif // OPENSSL_IS_BORINGSSL or SSL_get_negotiated_group + +#if not defined(OPENSSL_IS_BORINGSSL) and defined(SSL_get_negotiated_group) // OPENSSL 3.x + struct TLSGroup { int nid; std::string name; @@ -47,36 +69,39 @@ struct TLSGroup { // NID and Group table. Some groups are not defined by some library. constexpr TLSGroup TLS_GROUPS[] = { - {NID_X9_62_prime256v1, "P-256" }, - {NID_secp384r1, "P-384" }, - {NID_secp521r1, "P-521" }, - {NID_X25519, "X25519" }, + {SSL_GROUP_STAT_OTHER_KEY, "OTHER" }, + {NID_X9_62_prime256v1, "P-256" }, + {NID_secp384r1, "P-384" }, + {NID_secp521r1, "P-521" }, + {NID_X25519, "X25519" }, #ifdef NID_secp224r1 - {NID_secp224r1, "P-224" }, + {NID_secp224r1, "P-224" }, #endif #ifdef NID_X448 - {NID_X448, "X448" }, + {NID_X448, "X448" }, #endif #ifdef NID_ffdhe2048 - {NID_ffdhe2048, "ffdhe2048" }, + {NID_ffdhe2048, "ffdhe2048" }, #endif #ifdef NID_ffdhe3072 - {NID_ffdhe3072, "ffdhe3072" }, + {NID_ffdhe3072, "ffdhe3072" }, #endif #ifdef NID_ffdhe4096 - {NID_ffdhe4096, "ffdhe4096" }, + {NID_ffdhe4096, "ffdhe4096" }, #endif #ifdef NID_ffdhe6144 - {NID_ffdhe6144, "ffdhe6144" }, + {NID_ffdhe6144, "ffdhe6144" }, #endif #ifdef NID_ffdhe8192 - {NID_ffdhe8192, "ffdhe8192" }, + {NID_ffdhe8192, "ffdhe8192" }, #endif #ifdef NID_X25519MLKEM768 - {NID_X25519MLKEM768, "X25519MLKEM768"}, + {NID_X25519MLKEM768, "X25519MLKEM768"}, #endif }; -#endif // OPENSSL_IS_BORINGSSL or SSL_get_negotiated_group + +#endif // OPENSSL 3.x + } // end anonymous namespace // ToDo: This gets called once per global sync, for now at least. @@ -125,18 +150,6 @@ add_cipher_stat(const char *cipherName, const std::string &statName) } } -static void -add_group_stat(int nid, const std::string &stat_name) -{ - // If not already registered ... - if (tls_group_map.find(nid) == tls_group_map.end()) { - Metrics::Counter::AtomicType *metric = Metrics::Counter::createPtr(stat_name); - - tls_group_map.emplace(nid, metric); - Dbg(dbg_ctl_ssl, "registering SSL group metric '%s'", stat_name.c_str()); - } -} - void SSLInitializeStatistics() { @@ -227,10 +240,18 @@ SSLInitializeStatistics() // Add "OTHER" for ciphers not on the map add_cipher_stat(SSL_CIPHER_STAT_OTHER.c_str(), "proxy.process.ssl.cipher.user_agent." + SSL_CIPHER_STAT_OTHER); -#if defined(OPENSSL_IS_BORINGSSL) || defined(SSL_get_negotiated_group) - // TLS Groups + // TLS Group +#if defined(OPENSSL_IS_BORINGSSL) + size_t list_size = SSL_get_all_group_names(nullptr, 0); + std::vector group_list(list_size); + SSL_get_all_group_names(group_list.data(), group_list.size()); + + for (const char *name : group_list) { + add_group_stat(name, name); + } +#elif defined(SSL_get_negotiated_group) for (auto group : TLS_GROUPS) { - add_group_stat(group.nid, "proxy.process.ssl.group.user_agent." + group.name); + add_group_stat(group.nid, group.name); } #endif // OPENSSL_IS_BORINGSSL or SSL_get_negotiated_group diff --git a/src/iocore/net/SSLStats.h b/src/iocore/net/SSLStats.h index 3ebb14f835b..667e2bab066 100644 --- a/src/iocore/net/SSLStats.h +++ b/src/iocore/net/SSLStats.h @@ -30,6 +30,8 @@ #include "tsutil/Metrics.h" +#include + using ts::Metrics; // For some odd reason, these have to be initialized with nullptr, because the order @@ -99,7 +101,13 @@ struct SSLStatsBlock { extern SSLStatsBlock ssl_rsb; extern std::unordered_map cipher_map; + +#if defined(OPENSSL_IS_BORINGSSL) +extern std::unordered_map tls_group_map; +#elif defined(SSL_get_negotiated_group) extern std::unordered_map tls_group_map; +constexpr int SSL_GROUP_STAT_OTHER_KEY = 0; +#endif // Initialize SSL statistics. void SSLInitializeStatistics(); diff --git a/src/iocore/net/SSLUtils.cc b/src/iocore/net/SSLUtils.cc index 5564ff619d2..21684b8a97c 100644 --- a/src/iocore/net/SSLUtils.cc +++ b/src/iocore/net/SSLUtils.cc @@ -1087,16 +1087,24 @@ ssl_callback_info(const SSL *ssl, int where, int ret) Metrics::Counter::increment(it->second); } -#if defined(OPENSSL_IS_BORINGSSL) || defined(SSL_get_negotiated_group) - // TLS Group -#ifdef OPENSSL_IS_BORINGSSL - int nid = SSL_get_negotiated_group(ssl); +#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)); -#endif 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 @@ -1557,7 +1565,6 @@ SSLMultiCertConfigLoader::_set_curves([[maybe_unused]] SSL_CTX *ctx) } } #endif - return true; } From 809329ab62013dcbb6ee3592e2cb5a9aacf8bd9a Mon Sep 17 00:00:00 2001 From: Masaori Koshiba Date: Wed, 6 Nov 2024 20:35:35 +0900 Subject: [PATCH 3/4] Fix format --- src/iocore/net/SSLStats.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/iocore/net/SSLStats.h b/src/iocore/net/SSLStats.h index 667e2bab066..1630036a300 100644 --- a/src/iocore/net/SSLStats.h +++ b/src/iocore/net/SSLStats.h @@ -105,8 +105,8 @@ extern std::unordered_map cipher_ma #if defined(OPENSSL_IS_BORINGSSL) extern std::unordered_map tls_group_map; #elif defined(SSL_get_negotiated_group) -extern std::unordered_map tls_group_map; -constexpr int SSL_GROUP_STAT_OTHER_KEY = 0; +extern std::unordered_map tls_group_map; +constexpr int SSL_GROUP_STAT_OTHER_KEY = 0; #endif // Initialize SSL statistics. From 30aa89404f97336d163c506a7983e37425be0692 Mon Sep 17 00:00:00 2001 From: Masaori Koshiba Date: Thu, 7 Nov 2024 10:38:34 +0900 Subject: [PATCH 4/4] Fix for clang-analyzer build --- src/iocore/net/SSLStats.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/iocore/net/SSLStats.cc b/src/iocore/net/SSLStats.cc index 100935cba1c..8b296551f14 100644 --- a/src/iocore/net/SSLStats.cc +++ b/src/iocore/net/SSLStats.cc @@ -68,7 +68,7 @@ struct TLSGroup { }; // NID and Group table. Some groups are not defined by some library. -constexpr TLSGroup TLS_GROUPS[] = { +const TLSGroup TLS_GROUPS[] = { {SSL_GROUP_STAT_OTHER_KEY, "OTHER" }, {NID_X9_62_prime256v1, "P-256" }, {NID_secp384r1, "P-384" },