From 888d1c5b79db9b66a4f48a1bcfad625fd6eb7aa7 Mon Sep 17 00:00:00 2001 From: Sebastian Lukas Date: Wed, 2 Oct 2024 12:13:49 +0200 Subject: [PATCH 1/6] Adding basic openssl tls key logging Signed-off-by: Sebastian Lukas --- lib/staging/tls/openssl_util.hpp | 5 ++++ lib/staging/tls/tests/gtest_main.cpp | 3 ++ lib/staging/tls/tls.cpp | 30 +++++++++++++++++++ lib/staging/tls/tls.hpp | 3 ++ modules/EvseV2G/EvseV2G.cpp | 3 ++ modules/EvseV2G/connection/tls_connection.cpp | 3 ++ 6 files changed, 47 insertions(+) diff --git a/lib/staging/tls/openssl_util.hpp b/lib/staging/tls/openssl_util.hpp index 08e65ae33..7c3f0afed 100644 --- a/lib/staging/tls/openssl_util.hpp +++ b/lib/staging/tls/openssl_util.hpp @@ -499,6 +499,7 @@ bool certificate_subject_public_key_sha_1(openssl::sha_1_digest_t& digest, const enum class log_level_t : std::uint8_t { debug, + info, warning, error, }; @@ -523,6 +524,10 @@ static inline void log_debug(const std::string& str) { log(log_level_t::debug, str); } +static inline void log_info(const std::string& str) { + log(log_level_t::info, str); +} + using log_handler_t = void (*)(log_level_t level, const std::string& err); /** diff --git a/lib/staging/tls/tests/gtest_main.cpp b/lib/staging/tls/tests/gtest_main.cpp index b73259c2a..02b6af7a3 100644 --- a/lib/staging/tls/tests/gtest_main.cpp +++ b/lib/staging/tls/tests/gtest_main.cpp @@ -17,6 +17,9 @@ void log_handler(openssl::log_level_t level, const std::string& str) { case openssl::log_level_t::debug: // std::cout << "DEBUG: " << str << std::endl; break; + case openssl::log_level_t::info: + std::cout << "INFO: " << str << std::endl; + break; case openssl::log_level_t::warning: std::cout << "WARN: " << str << std::endl; break; diff --git a/lib/staging/tls/tls.cpp b/lib/staging/tls/tls.cpp index 966baf873..96804ce00 100644 --- a/lib/staging/tls/tls.cpp +++ b/lib/staging/tls/tls.cpp @@ -13,6 +13,8 @@ #include #include #include +#include +#include #include #include #include @@ -63,6 +65,7 @@ template <> class default_delete { } // namespace std using ::openssl::log_error; +using ::openssl::log_info; using ::openssl::log_warning; namespace { @@ -692,6 +695,26 @@ std::uint32_t ServerConnection::m_count{0}; std::mutex ServerConnection::m_cv_mutex; std::condition_variable ServerConnection::m_cv; +namespace { + +std::unique_ptr keylog_file; + +void keylog_callback(const SSL* ssl, const char* line) { + std::string key_log_msg = "TLS Handshake keys: " + std::string(line); + log_info(key_log_msg); + + if (keylog_file) { + std::ofstream ofs; + ofs.open(keylog_file->string(), std::ofstream::out | std::ofstream::app); + ofs << line << std::endl; + ofs.close(); + } + + keylog_file.reset(); +} + +} // namespace + ServerConnection::ServerConnection(SslContext* ctx, int soc, const char* ip_in, const char* service_in, std::int32_t timeout_ms) : Connection(ctx, soc, ip_in, service_in, timeout_ms), m_tck_data{m_trusted_ca_keys, m_flags} { @@ -885,6 +908,13 @@ bool Server::init_ssl(const config_t& cfg) { // use the first server chain result = configure_ssl_ctx(ctx, cfg.ciphersuites, cfg.cipher_list, cfg.chains[0], true); if (result) { + + if (cfg.tls_key_logging) { + const auto file_path = std::filesystem::path(cfg.tls_key_logging_path) /= "tls_session_keys.log"; + keylog_file = std::make_unique(file_path); + SSL_CTX_set_keylog_callback(ctx, keylog_callback); + } + int mode = SSL_VERIFY_NONE; // TODO(james-ctc): verify may need to change based on TLS version diff --git a/lib/staging/tls/tls.hpp b/lib/staging/tls/tls.hpp index bc9a28a41..1178bcc70 100644 --- a/lib/staging/tls/tls.hpp +++ b/lib/staging/tls/tls.hpp @@ -381,6 +381,9 @@ class Server { ConfigItem service{nullptr}; //!< TLS port number as a string int socket{INVALID_SOCKET}; //!< use this specific socket - bypasses socket setup in init_socket() when set bool ipv6_only{true}; //!< listen on IPv6 only, when false listen on IPv4 only + + bool tls_key_logging{false}; //!< tls key logging is active when true + std::string tls_key_logging_path; //!< tls key logging file path }; using ConnectionPtr = std::unique_ptr; diff --git a/modules/EvseV2G/EvseV2G.cpp b/modules/EvseV2G/EvseV2G.cpp index d5567a796..be9d2f70e 100644 --- a/modules/EvseV2G/EvseV2G.cpp +++ b/modules/EvseV2G/EvseV2G.cpp @@ -16,6 +16,9 @@ void log_handler(openssl::log_level_t level, const std::string& str) { case openssl::log_level_t::debug: // ignore debug logs break; + case openssl::log_level_t::info: + EVLOG_info << str; + break; case openssl::log_level_t::warning: EVLOG_warning << str; break; diff --git a/modules/EvseV2G/connection/tls_connection.cpp b/modules/EvseV2G/connection/tls_connection.cpp index b13a0147f..9209296f9 100644 --- a/modules/EvseV2G/connection/tls_connection.cpp +++ b/modules/EvseV2G/connection/tls_connection.cpp @@ -139,6 +139,9 @@ bool build_config(tls::Server::config_t& config, struct v2g_context* ctx) { config.socket = ctx->tls_socket.fd; config.io_timeout_ms = static_cast(ctx->network_read_timeout_tls); + config.tls_key_logging = ctx->tls_key_logging; + config.tls_key_logging_path = ctx->tls_key_logging_path; + // information from libevse-security const auto cert_info = ctx->r_security->call_get_all_valid_certificates_info(LeafCertificateType::V2G, EncodingFormat::PEM, true); From 91dceab461a0e48c16bdd9832117888ae659693b Mon Sep 17 00:00:00 2001 From: Sebastian Lukas Date: Mon, 7 Oct 2024 11:21:45 +0200 Subject: [PATCH 2/6] Adding udp tls key logging server Signed-off-by: Sebastian Lukas --- config/config-sil-dc-tls.yaml | 1 + lib/staging/tls/tls.cpp | 91 ++++++++++++++++++- lib/staging/tls/tls.hpp | 20 ++++ modules/EvseV2G/connection/tls_connection.cpp | 1 + 4 files changed, 111 insertions(+), 2 deletions(-) diff --git a/config/config-sil-dc-tls.yaml b/config/config-sil-dc-tls.yaml index 7645821b2..d1f32668d 100644 --- a/config/config-sil-dc-tls.yaml +++ b/config/config-sil-dc-tls.yaml @@ -4,6 +4,7 @@ active_modules: config_module: device: auto tls_security: force + tls_key_logging: true connections: security: - module_id: evse_security diff --git a/lib/staging/tls/tls.cpp b/lib/staging/tls/tls.cpp index 96804ce00..7be6d7357 100644 --- a/lib/staging/tls/tls.cpp +++ b/lib/staging/tls/tls.cpp @@ -6,6 +6,7 @@ #include "extensions/trusted_ca_keys.hpp" #include "openssl_util.hpp" +#include #include #include #include @@ -17,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -698,19 +700,29 @@ std::condition_variable ServerConnection::m_cv; namespace { std::unique_ptr keylog_file; +std::unique_ptr keylog_server; void keylog_callback(const SSL* ssl, const char* line) { std::string key_log_msg = "TLS Handshake keys: " + std::string(line); log_info(key_log_msg); + if (keylog_server and keylog_server->get_fd() != -1) { + const auto result = keylog_server->send(line); + if (result not_eq strlen(line)) { + log_error("key_logging_server send() failed!"); + } + + keylog_server.reset(); + } + if (keylog_file) { std::ofstream ofs; ofs.open(keylog_file->string(), std::ofstream::out | std::ofstream::app); ofs << line << std::endl; ofs.close(); - } - keylog_file.reset(); + keylog_file.reset(); + } } } // namespace @@ -913,6 +925,7 @@ bool Server::init_ssl(const config_t& cfg) { const auto file_path = std::filesystem::path(cfg.tls_key_logging_path) /= "tls_session_keys.log"; keylog_file = std::make_unique(file_path); SSL_CTX_set_keylog_callback(ctx, keylog_callback); + this->m_tls_key_interface = cfg.host; } int mode = SSL_VERIFY_NONE; @@ -1092,6 +1105,12 @@ void Server::wait_for_connection(const ConnectionHandler& handler) { // new connection, pass to handler auto* ip = BIO_ADDR_hostname_string(peer.get(), 1); auto* service = BIO_ADDR_service_string(peer.get(), 1); + + if (m_tls_key_interface) { + const auto port = std::stoul(service); + keylog_server = std::make_unique(std::string(m_tls_key_interface), port); + } + auto connection = std::make_unique(m_context->ctx.get(), soc, ip, service, m_timeout_ms); handler(std::move(connection)); @@ -1375,4 +1394,72 @@ Client::override_t Client::default_overrides() { }; } +TlsKeyLoggingServer::TlsKeyLoggingServer(const std::string& interface_name, uint16_t port) { + static constexpr auto LINK_LOCAL_MULTICAST = "ff02::1"; + + fd = socket(AF_INET6, SOCK_DGRAM, 0); + if (fd < 0) { + log_error("Could not create socket"); + return; + } + + // source setup + + // find port between 49152-65535 + auto could_bind = false; + auto source_port = 49152; + for (; source_port < 65535; source_port++) { + sockaddr_in6 source_address = {AF_INET6, htons(source_port), 0, {}, 0}; + if (bind(fd, reinterpret_cast(&source_address), sizeof(sockaddr_in6)) == 0) { + could_bind = true; + break; + } + } + + if (!could_bind) { + log_error("Could not bind"); + close(fd); + return; + } + + log_info("UDP socket bound to source port: " + std::to_string(source_port)); + + const auto index = if_nametoindex(interface_name.c_str()); + auto mreq = ipv6_mreq{}; + mreq.ipv6mr_interface = index; + if (inet_pton(AF_INET6, LINK_LOCAL_MULTICAST, &mreq.ipv6mr_multiaddr) <= 0) { + close(fd); + log_error("Failed to setup multicast address"); + return; + } + if (setsockopt(fd, IPPROTO_IPV6, IPV6_ADD_MEMBERSHIP, &mreq, sizeof(mreq)) < 0) { + close(fd); + log_error("Could not add multicast group membership"); + return; + } + + if (setsockopt(fd, IPPROTO_IPV6, IPV6_MULTICAST_IF, &index, sizeof(index)) < 0) { + close(fd); + log_error("Could not set interface name:" + interface_name); + return; + } + + destination_address = {AF_INET6, htons(port), 0, {}, 0}; + if (inet_pton(AF_INET6, LINK_LOCAL_MULTICAST, &destination_address.sin6_addr) <= 0) { + close(fd); + log_error("Failed to setup server address, reset key_log_fd"); + } +} + +TlsKeyLoggingServer::~TlsKeyLoggingServer() { + if (fd != -1) { + close(fd); + } +} + +ssize_t TlsKeyLoggingServer::send(const char* line) { + return sendto(fd, line, strlen(line), 0, reinterpret_cast(&destination_address), + sizeof(destination_address)); +} + } // namespace tls diff --git a/lib/staging/tls/tls.hpp b/lib/staging/tls/tls.hpp index 1178bcc70..55df910de 100644 --- a/lib/staging/tls/tls.hpp +++ b/lib/staging/tls/tls.hpp @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -412,6 +413,8 @@ class Server { static int s_sig_int; //!< signal to use to wakeup serve() ConfigurationCallback m_init_callback{nullptr}; //!< callback to retrieve SSL configuration + ConfigItem m_tls_key_interface{nullptr}; + /** * \brief initialise the server socket * \param[in] cfg server configuration @@ -635,6 +638,23 @@ class Client { static override_t default_overrides(); }; +class TlsKeyLoggingServer { +public: + TlsKeyLoggingServer(const std::string& interface_name, uint16_t port); + ~TlsKeyLoggingServer(); + + ssize_t send(const char* line); + + auto get_fd() const { + return fd; + } + +private: + int fd{-1}; + uint8_t buffer[2048]; + sockaddr_in6 destination_address{}; +}; + } // namespace tls #endif // TLS_HPP_ diff --git a/modules/EvseV2G/connection/tls_connection.cpp b/modules/EvseV2G/connection/tls_connection.cpp index 9209296f9..25d374c0a 100644 --- a/modules/EvseV2G/connection/tls_connection.cpp +++ b/modules/EvseV2G/connection/tls_connection.cpp @@ -141,6 +141,7 @@ bool build_config(tls::Server::config_t& config, struct v2g_context* ctx) { config.tls_key_logging = ctx->tls_key_logging; config.tls_key_logging_path = ctx->tls_key_logging_path; + config.host = ctx->if_name; // information from libevse-security const auto cert_info = From 69e456c2aff3d281eca41637b2092852bc40d655 Mon Sep 17 00:00:00 2001 From: Sebastian Lukas Date: Tue, 8 Oct 2024 09:32:52 +0200 Subject: [PATCH 3/6] IsoMux adding log_handler + key logging feature Signed-off-by: Sebastian Lukas --- modules/IsoMux/IsoMux.cpp | 24 ++++++++++++++++++++ modules/IsoMux/connection/tls_connection.cpp | 2 ++ modules/IsoMux/v2g.hpp | 2 ++ 3 files changed, 28 insertions(+) diff --git a/modules/IsoMux/IsoMux.cpp b/modules/IsoMux/IsoMux.cpp index 521d292fb..f9a4669e7 100644 --- a/modules/IsoMux/IsoMux.cpp +++ b/modules/IsoMux/IsoMux.cpp @@ -6,6 +6,27 @@ #include "log.hpp" #include "sdp.hpp" +#include +namespace { +void log_handler(openssl::log_level_t level, const std::string& str) { + switch (level) { + case openssl::log_level_t::debug: + // ignore debug logs + break; + case openssl::log_level_t::info: + EVLOG_info << str; + break; + case openssl::log_level_t::warning: + EVLOG_warning << str; + break; + case openssl::log_level_t::error: + default: + EVLOG_error << str; + break; + } +} +} // namespace + struct v2g_context* v2g_ctx = nullptr; namespace module { @@ -21,6 +42,9 @@ void IsoMux::init() { v2g_ctx->proxy_port_iso20 = config.proxy_port_iso20; v2g_ctx->selected_iso20 = false; + v2g_ctx->tls_key_logging = config.tls_key_logging; + + (void)openssl::set_log_handler(log_handler); v2g_ctx->tls_server = &tls_server; invoke_init(*p_charger); diff --git a/modules/IsoMux/connection/tls_connection.cpp b/modules/IsoMux/connection/tls_connection.cpp index 3d3f7b1c0..d98abfc9f 100644 --- a/modules/IsoMux/connection/tls_connection.cpp +++ b/modules/IsoMux/connection/tls_connection.cpp @@ -122,6 +122,8 @@ bool build_config(tls::Server::config_t& config, struct v2g_context* ctx) { config.socket = ctx->tls_socket.fd; config.io_timeout_ms = static_cast(ctx->network_read_timeout_tls); + config.tls_key_logging = ctx->tls_key_logging; + // information from libevse-security const auto cert_info = ctx->r_security->call_get_leaf_certificate_info(LeafCertificateType::V2G, EncodingFormat::PEM, false); diff --git a/modules/IsoMux/v2g.hpp b/modules/IsoMux/v2g.hpp index 20ebdd371..58be0cdbc 100644 --- a/modules/IsoMux/v2g.hpp +++ b/modules/IsoMux/v2g.hpp @@ -146,6 +146,8 @@ struct v2g_context { } tls_socket; tls::Server* tls_server; + bool tls_key_logging; + enum V2gMsgTypeId current_v2g_msg; /* holds the last v2g msg type */ int state; /* holds the current state id */ std::atomic_bool is_connection_terminated; /* Is set to true if the connection is terminated (CP State A/F, shutdown From 4af647c9fc6258aa9bf19a94535941889e5af2ee Mon Sep 17 00:00:00 2001 From: Sebastian Lukas Date: Fri, 18 Oct 2024 11:35:57 +0200 Subject: [PATCH 4/6] Adding suggestions - Now we use SSL_set_ex_data and SSL_get_ex_data for the keylog file path and tls key server Signed-off-by: Sebastian Lukas --- lib/staging/tls/tls.cpp | 65 ++++++++++++++++++++++++++--------------- lib/staging/tls/tls.hpp | 46 +++++++++++++++++------------ 2 files changed, 69 insertions(+), 42 deletions(-) diff --git a/lib/staging/tls/tls.cpp b/lib/staging/tls/tls.cpp index 7be6d7357..de2b0712f 100644 --- a/lib/staging/tls/tls.cpp +++ b/lib/staging/tls/tls.cpp @@ -14,7 +14,6 @@ #include #include #include -#include #include #include #include @@ -699,36 +698,41 @@ std::condition_variable ServerConnection::m_cv; namespace { -std::unique_ptr keylog_file; -std::unique_ptr keylog_server; +int ssl_keylog_file_index{-1}; +int ssl_keylog_server_index{-1}; void keylog_callback(const SSL* ssl, const char* line) { - std::string key_log_msg = "TLS Handshake keys: " + std::string(line); + + auto keylog_server = static_cast(SSL_get_ex_data(ssl, ssl_keylog_server_index)); + + std::string key_log_msg = "TLS Handshake keys on port "; + key_log_msg += std::to_string(keylog_server->get_port()) + ": "; + key_log_msg += std::string(line); + log_info(key_log_msg); - if (keylog_server and keylog_server->get_fd() != -1) { + if (keylog_server->get_fd() != -1) { const auto result = keylog_server->send(line); if (result not_eq strlen(line)) { log_error("key_logging_server send() failed!"); } - - keylog_server.reset(); } - if (keylog_file) { + auto keylog_file_path = + static_cast(SSL_CTX_get_ex_data(SSL_get_SSL_CTX(ssl), ssl_keylog_file_index)); + + if (not keylog_file_path->empty()) { std::ofstream ofs; - ofs.open(keylog_file->string(), std::ofstream::out | std::ofstream::app); + ofs.open(keylog_file_path->string(), std::ofstream::out | std::ofstream::app); ofs << line << std::endl; ofs.close(); - - keylog_file.reset(); } } } // namespace ServerConnection::ServerConnection(SslContext* ctx, int soc, const char* ip_in, const char* service_in, - std::int32_t timeout_ms) : + std::int32_t timeout_ms, const ConfigItem& tls_key_interface) : Connection(ctx, soc, ip_in, service_in, timeout_ms), m_tck_data{m_trusted_ca_keys, m_flags} { { std::lock_guard lock(m_cv_mutex); @@ -738,6 +742,12 @@ ServerConnection::ServerConnection(SslContext* ctx, int soc, const char* ip_in, SSL_set_accept_state(m_context->ctx.get()); ServerStatusRequestV2::set_data(m_context->ctx.get(), &m_flags); ServerTrustedCaKeys::set_data(m_context->ctx.get(), &m_tck_data); + + if (tls_key_interface != nullptr) { + const auto port = std::stoul(service_in); + m_keylog_server = std::make_unique(std::string(tls_key_interface), port); + SSL_set_ex_data(m_context->ctx.get(), ssl_keylog_server_index, m_keylog_server.get()); + } } } @@ -922,10 +932,22 @@ bool Server::init_ssl(const config_t& cfg) { if (result) { if (cfg.tls_key_logging) { - const auto file_path = std::filesystem::path(cfg.tls_key_logging_path) /= "tls_session_keys.log"; - keylog_file = std::make_unique(file_path); - SSL_CTX_set_keylog_callback(ctx, keylog_callback); - this->m_tls_key_interface = cfg.host; + tls_key_log_file_path = std::filesystem::path(cfg.tls_key_logging_path) /= "tls_session_keys.log"; + + ssl_keylog_file_index = SSL_CTX_get_ex_new_index(0, std::string("").data(), nullptr, nullptr, nullptr); + ssl_keylog_server_index = SSL_get_ex_new_index(0, std::string("").data(), nullptr, nullptr, nullptr); + + if (ssl_keylog_file_index == -1 or ssl_keylog_server_index == -1) { + auto error_msg = std::string("_get_ex_new_index failed: ssl_keylog_file_index: "); + error_msg += std::to_string(ssl_keylog_file_index); + error_msg += ", ssl_keylog_server_index: " + std::to_string(ssl_keylog_server_index); + log_error(error_msg); + } else { + SSL_CTX_set_ex_data(ctx, ssl_keylog_file_index, &tls_key_log_file_path); + + SSL_CTX_set_keylog_callback(ctx, keylog_callback); + m_tls_key_interface = cfg.host; + } } int mode = SSL_VERIFY_NONE; @@ -1106,13 +1128,8 @@ void Server::wait_for_connection(const ConnectionHandler& handler) { auto* ip = BIO_ADDR_hostname_string(peer.get(), 1); auto* service = BIO_ADDR_service_string(peer.get(), 1); - if (m_tls_key_interface) { - const auto port = std::stoul(service); - keylog_server = std::make_unique(std::string(m_tls_key_interface), port); - } - - auto connection = - std::make_unique(m_context->ctx.get(), soc, ip, service, m_timeout_ms); + auto connection = std::make_unique(m_context->ctx.get(), soc, ip, service, + m_timeout_ms, m_tls_key_interface); handler(std::move(connection)); OPENSSL_free(ip); OPENSSL_free(service); @@ -1394,7 +1411,7 @@ Client::override_t Client::default_overrides() { }; } -TlsKeyLoggingServer::TlsKeyLoggingServer(const std::string& interface_name, uint16_t port) { +TlsKeyLoggingServer::TlsKeyLoggingServer(const std::string& interface_name, uint16_t port_) : port(port_) { static constexpr auto LINK_LOCAL_MULTICAST = "ff02::1"; fd = socket(AF_INET6, SOCK_DGRAM, 0); diff --git a/lib/staging/tls/tls.hpp b/lib/staging/tls/tls.hpp index 55df910de..8c87f00dd 100644 --- a/lib/staging/tls/tls.hpp +++ b/lib/staging/tls/tls.hpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -58,6 +59,28 @@ class ConfigItem { } }; +class TlsKeyLoggingServer { +public: + TlsKeyLoggingServer(const std::string& interface_name, uint16_t port_); + ~TlsKeyLoggingServer(); + + ssize_t send(const char* line); + + auto get_fd() const { + return fd; + } + + auto get_port() const { + return port; + } + +private: + int fd{-1}; + std::array buffer{}; + uint16_t port{0}; + sockaddr_in6 destination_address{}; +}; + // ---------------------------------------------------------------------------- // Connection represents a TLS connection @@ -259,8 +282,11 @@ class ServerConnection : public Connection { StatusFlags m_flags; //!< extension flags server_trusted_ca_keys_t m_tck_data; //!< extension per connection data + std::unique_ptr m_keylog_server{nullptr}; + public: - ServerConnection(SslContext* ctx, int soc, const char* ip_in, const char* service_in, std::int32_t timeout_ms); + ServerConnection(SslContext* ctx, int soc, const char* ip_in, const char* service_in, std::int32_t timeout_ms, + const ConfigItem& tls_key_interface); ServerConnection() = delete; ServerConnection(const ServerConnection&) = delete; ServerConnection(ServerConnection&&) = delete; @@ -414,6 +440,7 @@ class Server { ConfigurationCallback m_init_callback{nullptr}; //!< callback to retrieve SSL configuration ConfigItem m_tls_key_interface{nullptr}; + std::filesystem::path tls_key_log_file_path{}; /** * \brief initialise the server socket @@ -638,23 +665,6 @@ class Client { static override_t default_overrides(); }; -class TlsKeyLoggingServer { -public: - TlsKeyLoggingServer(const std::string& interface_name, uint16_t port); - ~TlsKeyLoggingServer(); - - ssize_t send(const char* line); - - auto get_fd() const { - return fd; - } - -private: - int fd{-1}; - uint8_t buffer[2048]; - sockaddr_in6 destination_address{}; -}; - } // namespace tls #endif // TLS_HPP_ From 60e8da7859f311d4759689265a847edcf18f5c5c Mon Sep 17 00:00:00 2001 From: James Chapman Date: Fri, 18 Oct 2024 11:47:58 +0100 Subject: [PATCH 5/6] fix: suggested improvement to TlsKeyLoggingServer constructor common error handling on error (removed early return) Signed-off-by: James Chapman --- lib/staging/tls/tls.cpp | 83 +++++++++++++++++++++++------------------ 1 file changed, 46 insertions(+), 37 deletions(-) diff --git a/lib/staging/tls/tls.cpp b/lib/staging/tls/tls.cpp index de2b0712f..45a7b47f0 100644 --- a/lib/staging/tls/tls.cpp +++ b/lib/staging/tls/tls.cpp @@ -1411,60 +1411,69 @@ Client::override_t Client::default_overrides() { }; } +// ---------------------------------------------------------------------------- +// TlsKeyLoggingServer + TlsKeyLoggingServer::TlsKeyLoggingServer(const std::string& interface_name, uint16_t port_) : port(port_) { static constexpr auto LINK_LOCAL_MULTICAST = "ff02::1"; + bool result{true}; fd = socket(AF_INET6, SOCK_DGRAM, 0); - if (fd < 0) { + if (fd == -1) { log_error("Could not create socket"); - return; + result = false; } - // source setup + if (result) { + // source setup + // find port between 49152-65535 + auto could_bind = false; + auto source_port = 49152; + for (; source_port < 65535; source_port++) { + sockaddr_in6 source_address = {AF_INET6, htons(source_port), 0, {}, 0}; + if (bind(fd, reinterpret_cast(&source_address), sizeof(sockaddr_in6)) == 0) { + could_bind = true; + break; + } + } - // find port between 49152-65535 - auto could_bind = false; - auto source_port = 49152; - for (; source_port < 65535; source_port++) { - sockaddr_in6 source_address = {AF_INET6, htons(source_port), 0, {}, 0}; - if (bind(fd, reinterpret_cast(&source_address), sizeof(sockaddr_in6)) == 0) { - could_bind = true; - break; + if (could_bind) { + log_info("UDP socket bound to source port: " + std::to_string(source_port)); + } else { + log_error("Could not bind"); + result = false; } } - if (!could_bind) { - log_error("Could not bind"); - close(fd); - return; - } + if (result) { + auto mreq = ipv6_mreq{}; + const auto index = if_nametoindex(interface_name.c_str()); + mreq.ipv6mr_interface = index; + if (inet_pton(AF_INET6, LINK_LOCAL_MULTICAST, &mreq.ipv6mr_multiaddr) <= 0) { + log_error("Failed to setup multicast address"); + result = false; + } - log_info("UDP socket bound to source port: " + std::to_string(source_port)); + if (setsockopt(fd, IPPROTO_IPV6, IPV6_ADD_MEMBERSHIP, &mreq, sizeof(mreq)) < 0) { + log_error("Could not add multicast group membership"); + result = false; + } - const auto index = if_nametoindex(interface_name.c_str()); - auto mreq = ipv6_mreq{}; - mreq.ipv6mr_interface = index; - if (inet_pton(AF_INET6, LINK_LOCAL_MULTICAST, &mreq.ipv6mr_multiaddr) <= 0) { - close(fd); - log_error("Failed to setup multicast address"); - return; - } - if (setsockopt(fd, IPPROTO_IPV6, IPV6_ADD_MEMBERSHIP, &mreq, sizeof(mreq)) < 0) { - close(fd); - log_error("Could not add multicast group membership"); - return; - } + if (setsockopt(fd, IPPROTO_IPV6, IPV6_MULTICAST_IF, &index, sizeof(index)) < 0) { + log_error("Could not set interface name:" + interface_name); + result = false; + } - if (setsockopt(fd, IPPROTO_IPV6, IPV6_MULTICAST_IF, &index, sizeof(index)) < 0) { - close(fd); - log_error("Could not set interface name:" + interface_name); - return; + destination_address = {AF_INET6, htons(port), 0, {}, 0}; + if (inet_pton(AF_INET6, LINK_LOCAL_MULTICAST, &destination_address.sin6_addr) <= 0) { + log_error("Failed to setup server address, reset key_log_fd"); + result = false; + } } - destination_address = {AF_INET6, htons(port), 0, {}, 0}; - if (inet_pton(AF_INET6, LINK_LOCAL_MULTICAST, &destination_address.sin6_addr) <= 0) { + if (!result && fd != -1) { close(fd); - log_error("Failed to setup server address, reset key_log_fd"); + fd = -1; } } From edada6b700b259db953661aed722998799212162 Mon Sep 17 00:00:00 2001 From: Sebastian Lukas Date: Tue, 22 Oct 2024 11:38:22 +0200 Subject: [PATCH 6/6] Removing not used buffer Signed-off-by: Sebastian Lukas --- lib/staging/tls/tls.hpp | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/staging/tls/tls.hpp b/lib/staging/tls/tls.hpp index 8c87f00dd..f48de624e 100644 --- a/lib/staging/tls/tls.hpp +++ b/lib/staging/tls/tls.hpp @@ -76,7 +76,6 @@ class TlsKeyLoggingServer { private: int fd{-1}; - std::array buffer{}; uint16_t port{0}; sockaddr_in6 destination_address{}; };