From 4dbcde34a1e1cc1e46b0cbf26c5ece426b4bcc17 Mon Sep 17 00:00:00 2001 From: Brian Neradt Date: Fri, 18 Feb 2022 14:18:42 -0600 Subject: [PATCH] Fix test_QUIC unit test builds. (#8678) The test_QUIC unit tests were failing to build because they didn't link against a file with the TLSKeyLogger definition. This fixes the undefined references by breaking out TLSKeyLogger into a separate object that the unit tests can link with. (cherry picked from commit 2d70a007933176c80b09ddeaf164206794671442) --- iocore/net/Makefile.am | 2 + iocore/net/P_SSLUtils.h | 101 -------------------------- iocore/net/P_TLSKeyLogger.h | 129 ++++++++++++++++++++++++++++++++++ iocore/net/SSLClientUtils.cc | 1 + iocore/net/SSLConfig.cc | 2 +- iocore/net/SSLUtils.cc | 73 +------------------ iocore/net/TLSKeyLogger.cc | 98 ++++++++++++++++++++++++++ iocore/net/quic/Makefile.am | 1 + iocore/net/quic/QUICConfig.cc | 2 +- proxy/http3/Makefile.am | 1 + 10 files changed, 235 insertions(+), 175 deletions(-) create mode 100644 iocore/net/P_TLSKeyLogger.h create mode 100644 iocore/net/TLSKeyLogger.cc diff --git a/iocore/net/Makefile.am b/iocore/net/Makefile.am index dfeda5294f5..77332b32c45 100644 --- a/iocore/net/Makefile.am +++ b/iocore/net/Makefile.am @@ -157,6 +157,7 @@ libinknet_a_SOURCES = \ P_SSLUtils.h \ P_SSLClientCoordinator.h \ P_SSLClientUtils.h \ + P_TLSKeyLogger.h \ P_OCSPStapling.h \ P_UDPConnection.h \ P_UDPIOEvent.h \ @@ -191,6 +192,7 @@ libinknet_a_SOURCES = \ SSLUtils.cc \ OCSPStapling.cc \ TLSBasicSupport.cc \ + TLSKeyLogger.cc \ TLSSessionResumptionSupport.cc \ TLSSNISupport.cc \ UDPIOEvent.cc \ diff --git a/iocore/net/P_SSLUtils.h b/iocore/net/P_SSLUtils.h index 2091a449908..8cdab40296d 100644 --- a/iocore/net/P_SSLUtils.h +++ b/iocore/net/P_SSLUtils.h @@ -35,9 +35,7 @@ #include "P_SSLCertLookup.h" #include -#include #include -#include struct SSLConfigParams; class SSLNetVConnection; @@ -64,105 +62,6 @@ struct SSLLoadingContext { explicit SSLLoadingContext(SSL_CTX *c, SSLCertContextType ctx_type) : ctx(c), ctx_type(ctx_type) {} }; -/** A class for handling TLS secrets logging. */ -class TLSKeyLogger -{ -public: - TLSKeyLogger(const TLSKeyLogger &) = delete; - TLSKeyLogger &operator=(const TLSKeyLogger &) = delete; - - ~TLSKeyLogger() - { - std::unique_lock lock{_mutex}; - close_keylog_file(); - } - - /** A callback for TLS secret key logging. - * - * This is the callback registered with OpenSSL's SSL_CTX_set_keylog_callback - * to log TLS secrets if the user enabled that feature. For more information - * about this callback, see OpenSSL's documentation of - * SSL_CTX_set_keylog_callback. - * - * @param[in] ssl The SSL object associated with the connection. - * @param[in] line The line to place in the keylog file. - */ - static void - ssl_keylog_cb(const SSL *ssl, const char *line) - { - instance().log(line); - } - - /** Return whether TLS key logging is enabled. - * - * @return True if TLS session key logging is enabled, false otherwise. - */ - static bool - is_enabled() - { - return instance()._fd >= 0; - } - - /** Enable keylogging. - * - * @param[in] keylog_file The path to the file to log TLS secrets to. - */ - static void - enable_keylogging(const char *keylog_file) - { - instance().enable_keylogging_internal(keylog_file); - } - - /** Disable TLS secrets logging. */ - static void - disable_keylogging() - { - instance().disable_keylogging_internal(); - } - -private: - TLSKeyLogger() = default; - - /** Return the TLSKeyLogger singleton. - * - * We use a getter rather than a class static singleton member so that the - * construction of the singleton delayed until after TLS configuration is - * processed. - */ - static TLSKeyLogger & - instance() - { - static TLSKeyLogger instance; - return instance; - } - - /** Close the file descriptor for the key log file. - * - * @note This assumes that a unique lock has been acquired for _mutex. - */ - void close_keylog_file(); - - /** A TLS secret line to log to the keylog file. - * - * @param[in] line A line to log to the keylog file. - */ - void log(const char *line); - - /** Enable TLS keylogging in the instance singleton. */ - void enable_keylogging_internal(const char *keylog_file); - - /** Disable TLS keylogging in the instance singleton. */ - void disable_keylogging_internal(); - -private: - /** A file descriptor for the log file receiving the TLS secrets. */ - int _fd = -1; - - /** A mutex to coordinate dynamically changing TLS logging config changes and - * logging to the TLS log file. */ - std::shared_mutex _mutex; -}; - /** @brief Load SSL certificates from ssl_multicert.config and setup SSLCertLookup for SSLCertificateConfig */ diff --git a/iocore/net/P_TLSKeyLogger.h b/iocore/net/P_TLSKeyLogger.h new file mode 100644 index 00000000000..73b1b703b6f --- /dev/null +++ b/iocore/net/P_TLSKeyLogger.h @@ -0,0 +1,129 @@ +/** + + @section license License + + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ + +#pragma once + +#ifndef OPENSSL_IS_BORINGSSL +#include +#endif +#include + +#include +#include + +/** A class for handling TLS secrets logging. */ +class TLSKeyLogger +{ +public: + TLSKeyLogger(const TLSKeyLogger &) = delete; + TLSKeyLogger &operator=(const TLSKeyLogger &) = delete; + + ~TLSKeyLogger() + { + std::unique_lock lock{_mutex}; + close_keylog_file(); + } + + /** A callback for TLS secret key logging. + * + * This is the callback registered with OpenSSL's SSL_CTX_set_keylog_callback + * to log TLS secrets if the user enabled that feature. For more information + * about this callback, see OpenSSL's documentation of + * SSL_CTX_set_keylog_callback. + * + * @param[in] ssl The SSL object associated with the connection. + * @param[in] line The line to place in the keylog file. + */ + static void + ssl_keylog_cb(const SSL *ssl, const char *line) + { + instance().log(line); + } + + /** Return whether TLS key logging is enabled. + * + * @return True if TLS session key logging is enabled, false otherwise. + */ + static bool + is_enabled() + { + return instance()._fd >= 0; + } + + /** Enable keylogging. + * + * @param[in] keylog_file The path to the file to log TLS secrets to. + */ + static void + enable_keylogging(const char *keylog_file) + { + instance().enable_keylogging_internal(keylog_file); + } + + /** Disable TLS secrets logging. */ + static void + disable_keylogging() + { + instance().disable_keylogging_internal(); + } + +private: + TLSKeyLogger() = default; + + /** Return the TLSKeyLogger singleton. + * + * We use a getter rather than a class static singleton member so that the + * construction of the singleton delayed until after TLS configuration is + * processed. + */ + static TLSKeyLogger & + instance() + { + static TLSKeyLogger instance; + return instance; + } + + /** Close the file descriptor for the key log file. + * + * @note This assumes that a unique lock has been acquired for _mutex. + */ + void close_keylog_file(); + + /** A TLS secret line to log to the keylog file. + * + * @param[in] line A line to log to the keylog file. + */ + void log(const char *line); + + /** Enable TLS keylogging in the instance singleton. */ + void enable_keylogging_internal(const char *keylog_file); + + /** Disable TLS keylogging in the instance singleton. */ + void disable_keylogging_internal(); + +private: + /** A file descriptor for the log file receiving the TLS secrets. */ + int _fd = -1; + + /** A mutex to coordinate dynamically changing TLS logging config changes and + * logging to the TLS log file. */ + std::shared_mutex _mutex; +}; diff --git a/iocore/net/SSLClientUtils.cc b/iocore/net/SSLClientUtils.cc index 24bd22b5f37..5975584a59a 100644 --- a/iocore/net/SSLClientUtils.cc +++ b/iocore/net/SSLClientUtils.cc @@ -29,6 +29,7 @@ #include "P_SSLClientUtils.h" #include "P_SSLConfig.h" #include "P_SSLNetVConnection.h" +#include "P_TLSKeyLogger.h" #include "YamlSNIConfig.h" #include "SSLDiags.h" #include "SSLSessionCache.h" diff --git a/iocore/net/SSLConfig.cc b/iocore/net/SSLConfig.cc index 830ae1cffa7..0d996dd4a1a 100644 --- a/iocore/net/SSLConfig.cc +++ b/iocore/net/SSLConfig.cc @@ -50,7 +50,7 @@ #include "P_SSLSNI.h" #include "P_SSLCertLookup.h" #include "P_SSLSNI.h" -#include "P_SSLUtils.h" +#include "P_TLSKeyLogger.h" #include "SSLDiags.h" #include "SSLSessionCache.h" #include "SSLSessionTicket.h" diff --git a/iocore/net/SSLUtils.cc b/iocore/net/SSLUtils.cc index 07ae92cc86c..3424b72fe23 100644 --- a/iocore/net/SSLUtils.cc +++ b/iocore/net/SSLUtils.cc @@ -38,6 +38,7 @@ #include "P_OCSPStapling.h" #include "P_SSLSNI.h" #include "P_SSLConfig.h" +#include "P_TLSKeyLogger.h" #include "BoringSSLUtils.h" #include "ProxyProtocol.h" #include "SSLSessionCache.h" @@ -46,11 +47,7 @@ #include "SSLDiags.h" #include "SSLStats.h" -#include #include -#include -#include -#include #include #include #include @@ -104,74 +101,6 @@ static int ssl_vc_index = -1; static ink_mutex *mutex_buf = nullptr; static bool open_ssl_initialized = false; -// The caller of this function is responsible to acquire a unique_lock for -// _mutex. -void -TLSKeyLogger::close_keylog_file() -{ - if (_fd == -1) { - return; - } - if (close(_fd) == -1) { - Error("Could not close keylog file: %s", strerror(errno)); - } - _fd = -1; -} - -void -TLSKeyLogger::enable_keylogging_internal(const char *keylog_file) -{ -#if TS_HAS_TLS_KEYLOGGING - Debug("ssl_keylog", "Enabling TLS key logging to: %s.", keylog_file); - std::unique_lock lock{_mutex}; - if (keylog_file == nullptr) { - close_keylog_file(); - Debug("ssl_keylog", "Received a nullptr for keylog_file: disabling keylogging."); - return; - } - - _fd = open(keylog_file, O_WRONLY | O_APPEND | O_CREAT, S_IWUSR | S_IRUSR); - if (_fd == -1) { - Error("Could not open keylog file %s: %s", keylog_file, strerror(errno)); - return; - } - Note("Opened %s for TLS key logging.", keylog_file); -#else - Error("TLS keylogging is configured, but Traffic Server is not compiled with a version of OpenSSL that supports it."); - return; -#endif /* TS_HAS_TLS_KEYLOGGING */ -} - -void -TLSKeyLogger::disable_keylogging_internal() -{ - std::unique_lock lock{_mutex}; - if (is_enabled()) { - Note("Disabling TLS key logging."); - } - close_keylog_file(); - Debug("ssl_keylog", "TLS keylogging is disabled."); -} - -void -TLSKeyLogger::log(const char *line) -{ - std::shared_lock lock{_mutex}; - if (!is_enabled()) { - return; - } - - // writev() is guaranteed to be thread safe. - struct iovec vector[2]; - vector[0].iov_base = const_cast(reinterpret_cast(line)); - vector[0].iov_len = strlen(line); - vector[1].iov_base = const_cast(reinterpret_cast("\n")); - vector[1].iov_len = 1; - if (writev(_fd, vector, 2) <= 0) { - Error("Could not write TLS session key to key log file: %s", strerror(errno)); - } -} - /* Using pthread thread ID and mutex functions directly, instead of * ATS this_ethread / ProxyMutex, so that other linked libraries * may use pthreads and openssl without confusing us here. (TS-2271). diff --git a/iocore/net/TLSKeyLogger.cc b/iocore/net/TLSKeyLogger.cc new file mode 100644 index 00000000000..bdae514ec61 --- /dev/null +++ b/iocore/net/TLSKeyLogger.cc @@ -0,0 +1,98 @@ +/** @file + + @section license License + + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ + +#include "P_TLSKeyLogger.h" +#include "tscore/Diags.h" + +#include +#include +#include +#include +#include +#include + +// The caller of this function is responsible to acquire a unique_lock for +// _mutex. +void +TLSKeyLogger::close_keylog_file() +{ + if (_fd == -1) { + return; + } + if (close(_fd) == -1) { + Error("Could not close keylog file: %s", strerror(errno)); + } + _fd = -1; +} + +void +TLSKeyLogger::enable_keylogging_internal(const char *keylog_file) +{ +#if TS_HAS_TLS_KEYLOGGING + Debug("ssl_keylog", "Enabling TLS key logging to: %s.", keylog_file); + std::unique_lock lock{_mutex}; + if (keylog_file == nullptr) { + close_keylog_file(); + Debug("ssl_keylog", "Received a nullptr for keylog_file: disabling keylogging."); + return; + } + + _fd = open(keylog_file, O_WRONLY | O_APPEND | O_CREAT, S_IWUSR | S_IRUSR); + if (_fd == -1) { + Error("Could not open keylog file %s: %s", keylog_file, strerror(errno)); + return; + } + Note("Opened %s for TLS key logging.", keylog_file); +#else + Error("TLS keylogging is configured, but Traffic Server is not compiled with a version of OpenSSL that supports it."); + return; +#endif /* TS_HAS_TLS_KEYLOGGING */ +} + +void +TLSKeyLogger::disable_keylogging_internal() +{ + std::unique_lock lock{_mutex}; + if (is_enabled()) { + Note("Disabling TLS key logging."); + } + close_keylog_file(); + Debug("ssl_keylog", "TLS keylogging is disabled."); +} + +void +TLSKeyLogger::log(const char *line) +{ + std::shared_lock lock{_mutex}; + if (!is_enabled()) { + return; + } + + // writev() is guaranteed to be thread safe. + struct iovec vector[2]; + vector[0].iov_base = const_cast(reinterpret_cast(line)); + vector[0].iov_len = strlen(line); + vector[1].iov_base = const_cast(reinterpret_cast("\n")); + vector[1].iov_len = 1; + if (writev(_fd, vector, 2) <= 0) { + Error("Could not write TLS session key to key log file: %s", strerror(errno)); + } +} diff --git a/iocore/net/quic/Makefile.am b/iocore/net/quic/Makefile.am index ca72382de7a..ab32d25401b 100644 --- a/iocore/net/quic/Makefile.am +++ b/iocore/net/quic/Makefile.am @@ -148,6 +148,7 @@ test_LDADD = \ $(top_builddir)/src/tscore/libtscore.la \ $(top_builddir)/src/tscpp/util/libtscpputil.la \ $(top_builddir)/proxy/ParentSelectionStrategy.o \ + $(top_builddir)/iocore/net/TLSKeyLogger.o \ @HWLOC_LIBS@ @OPENSSL_LIBS@ @LIBPCRE@ @YAMLCPP_LIBS@ test_event_main_SOURCES = \ diff --git a/iocore/net/quic/QUICConfig.cc b/iocore/net/quic/QUICConfig.cc index 50e75d7b263..8e06255e427 100644 --- a/iocore/net/quic/QUICConfig.cc +++ b/iocore/net/quic/QUICConfig.cc @@ -28,7 +28,7 @@ #include #include "P_SSLConfig.h" -#include "P_SSLUtils.h" +#include "P_TLSKeyLogger.h" #include "QUICGlobals.h" #include "QUICTransportParameters.h" diff --git a/proxy/http3/Makefile.am b/proxy/http3/Makefile.am index 979f3a656e9..7a0ea023a81 100644 --- a/proxy/http3/Makefile.am +++ b/proxy/http3/Makefile.am @@ -71,6 +71,7 @@ test_CPPFLAGS = \ test_LDADD = \ $(top_builddir)/iocore/net/quic/libquic.a \ + $(top_builddir)/iocore/net/TLSKeyLogger.o \ $(top_builddir)/iocore/eventsystem/libinkevent.a \ $(top_builddir)/lib/records/librecords_p.a \ $(top_builddir)/mgmt/libmgmt_p.la \