diff --git a/CMakeLists.txt b/CMakeLists.txt index a03497767ec..b7ed086d395 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -214,7 +214,6 @@ set(ENABLE_TPROXY 'X' where X is a number to use as the IP_TRANSPARENT sockopt, anything else to enable." ) -option(ENABLE_OPENSSL_QUIC "Use OpenSSL native QUIC (default OFF)") option(ENABLE_QUICHE "Use quiche (default OFF)") option(ENABLE_EXAMPLE "Build example directory (default OFF)") @@ -313,8 +312,8 @@ if(NOT SSLLIB_IS_BORINGSSL AND NOT SSLLIB_IS_AWSLC AND OPENSSL_VERSION VERSION_GREATER_EQUAL "3.0.0" ) - set(SSLLIB_IS_OPENSSL3 TRUE) - add_compile_definitions(OPENSSL_API_COMPAT=10002 OPENSSL_IS_OPENSSL3) + set(SSLLIB_IS_AT_LEAST_OPENSSL3 TRUE) + add_compile_definitions(OPENSSL_API_COMPAT=10002 OPENSSL_IS_AT_LEAST_OPENSSL3) endif() check_openssl_has_quic_tls_cbs(SSLLIB_HAS_QUIC_TLS_CBS "${OPENSSL_INCLUDE_DIR}") @@ -329,6 +328,42 @@ endif() check_openssl_has_native_quic(SSLLIB_HAS_NATIVE_QUIC "${OPENSSL_INCLUDE_DIR}") +if(DEFINED ENABLE_OPENSSL_QUIC + AND NOT ENABLE_OPENSSL_QUIC STREQUAL "AUTO" + AND ENABLE_OPENSSL_QUIC +) + if(ENABLE_QUICHE) + message(FATAL_ERROR "ENABLE_OPENSSL_QUIC and ENABLE_QUICHE are mutually exclusive QUIC backends") + endif() + if(NOT SSLLIB_HAS_NATIVE_QUIC) + message(FATAL_ERROR "OpenSSL native QUIC support requires OpenSSL 3.5 or newer with OSSL_QUIC_server_method") + endif() + if(SSLLIB_IS_BORINGSSL + OR SSLLIB_IS_AWSLC + OR SSLLIB_IS_QUICTLS + ) + message(FATAL_ERROR "OpenSSL native QUIC support requires upstream OpenSSL 3.5 or newer") + endif() +endif() + +set(OPENSSL_QUIC_AVAILABLE ${SSLLIB_HAS_NATIVE_QUIC}) +if(SSLLIB_IS_BORINGSSL + OR SSLLIB_IS_AWSLC + OR SSLLIB_IS_QUICTLS + OR ENABLE_QUICHE +) + set(OPENSSL_QUIC_AVAILABLE FALSE) +endif() +auto_option( + OPENSSL_QUIC + FEATURE_VAR + TS_HAS_OPENSSL_QUIC + DESCRIPTION + "Use OpenSSL native QUIC" + VAR_DEPENDS + OPENSSL_QUIC_AVAILABLE +) + if(ENABLE_PROFILER) find_package(profiler REQUIRED) set(TS_HAS_PROFILER ${profiler_FOUND}) @@ -344,21 +379,7 @@ elseif(TS_HAS_MIMALLOC) link_libraries(mimalloc) endif() -if(ENABLE_OPENSSL_QUIC AND ENABLE_QUICHE) - message(FATAL_ERROR "ENABLE_OPENSSL_QUIC and ENABLE_QUICHE are mutually exclusive QUIC backends") -endif() - -if(ENABLE_OPENSSL_QUIC) - if(NOT SSLLIB_HAS_NATIVE_QUIC) - message(FATAL_ERROR "OpenSSL native QUIC support requires OpenSSL 3.5 or newer with OSSL_QUIC_server_method") - endif() - if(SSLLIB_IS_BORINGSSL - OR SSLLIB_IS_AWSLC - OR SSLLIB_IS_QUICTLS - ) - message(FATAL_ERROR "OpenSSL native QUIC support requires upstream OpenSSL 3.5 or newer") - endif() - set(TS_HAS_OPENSSL_QUIC TRUE) +if(TS_HAS_OPENSSL_QUIC) set(TS_USE_QUIC TRUE) message(STATUS "Using OpenSSL native QUIC") endif() @@ -625,12 +646,9 @@ check_symbol_exists(SSL_get_all_async_fds openssl/ssl.h TS_USE_TLS_ASYNC) check_symbol_exists(OSSL_PARAM_construct_end "openssl/params.h" HAVE_OSSL_PARAM_CONSTRUCT_END) check_symbol_exists(TLS1_3_VERSION "openssl/ssl.h" TS_USE_TLS13) check_symbol_exists(MD5_Init "openssl/md5.h" HAVE_MD5_INIT) -check_symbol_exists(ENGINE_load_dynamic "openssl/engine.h" HAVE_ENGINE_LOAD_DYNAMIC) -check_symbol_exists(ENGINE_get_default_RSA "openssl/engine.h" HAVE_ENGINE_GET_DEFAULT_RSA) -check_symbol_exists(ENGINE_load_private_key "openssl/engine.h" HAVE_ENGINE_LOAD_PRIVATE_KEY) check_symbol_exists(sysctlbyname "sys/sysctl.h" HAVE_SYSCTLBYNAME) -if(SSLLIB_IS_OPENSSL3) +if(SSLLIB_IS_AT_LEAST_OPENSSL3) check_symbol_exists(SSL_CTX_set_tlsext_ticket_key_evp_cb "openssl/ssl.h" TS_HAS_TLS_SESSION_TICKET) else() check_symbol_exists(SSL_CTX_set_tlsext_ticket_key_cb "openssl/ssl.h" TS_HAS_TLS_SESSION_TICKET) diff --git a/CMakePresets.json b/CMakePresets.json index a4eff644ea8..68bbb2d4e4a 100644 --- a/CMakePresets.json +++ b/CMakePresets.json @@ -201,7 +201,9 @@ "description": "CI Pipeline config for Fedora Linux compiled with c++20", "inherits": ["ci-fedora"], "cacheVariables": { - "CMAKE_CXX_STANDARD": "20" + "CMAKE_CXX_STANDARD": "20", + "CMAKE_C_COMPILER": "clang", + "CMAKE_CXX_COMPILER": "clang++" } }, { diff --git a/doc/admin-guide/files/records.yaml.en.rst b/doc/admin-guide/files/records.yaml.en.rst index 00f000a5e82..3bf27624ef2 100644 --- a/doc/admin-guide/files/records.yaml.en.rst +++ b/doc/admin-guide/files/records.yaml.en.rst @@ -5125,7 +5125,7 @@ HTTP/2 Configuration This is the threshold of sampling stream number to start checking the stream error rate. -.. ts:cv:: CONFIG proxy.config.http2.max_settings_per_frame INT 7 +.. ts:cv:: CONFIG proxy.config.http2.max_settings_per_frame INT 16 :reloadable: Specifies how many settings in an HTTP/2 SETTINGS frame |TS| accepts. @@ -5133,7 +5133,7 @@ HTTP/2 Configuration code of ENHANCE_YOUR_CALM. Any negative value configures no limit to the number of settings received. -.. ts:cv:: CONFIG proxy.config.http2.max_settings_per_minute INT 14 +.. ts:cv:: CONFIG proxy.config.http2.max_settings_per_minute INT 32 :reloadable: Specifies how many settings in HTTP/2 SETTINGS frames |TS| accept for a minute. @@ -5267,8 +5267,8 @@ removed in the future without prior notice. .. ts:cv:: CONFIG proxy.config.quic.instance_id INT 0 :reloadable: - A static key used for calculating Stateless Reset Token. All instances in a - cluster need to share the same value. + An instance identifier mixed into Stateless Reset Tokens. All instances in a + cluster that share token keys need to use the same value. .. ts:cv:: CONFIG proxy.config.quic.connection_table.size INT 65521 @@ -5283,6 +5283,20 @@ removed in the future without prior notice. Enables Stateless Retry. +.. ts:cv:: CONFIG proxy.config.quic.server.token_key.filename STRING NULL + :reloadable: + + The file containing the secret keys used to generate QUIC address-validation + and stateless-reset tokens. Relative paths are resolved from the |TS| + configuration directory. The file must contain one or more raw 32-byte keys. + The first key generates new tokens, while all keys validate address-validation + tokens to support key rotation. For example, generate a key with + ``head -c32 /dev/urandom > quic_token.key``. + + When this is not set, |TS| generates a random per-process key at startup. Set + the same key file on each server that must validate tokens generated by other + servers. Reload configuration after changing the key file. + .. ts:cv:: CONFIG proxy.config.quic.client.vn_exercise_enabled INT 0 :reloadable: diff --git a/doc/developer-guide/testing/autests.en.rst b/doc/developer-guide/testing/autests.en.rst index 04a02e57126..9d11b525035 100644 --- a/doc/developer-guide/testing/autests.en.rst +++ b/doc/developer-guide/testing/autests.en.rst @@ -356,6 +356,7 @@ The ``autest`` section configures the test environment: - **process_config**: Parameters passed to ``MakeATSProcess`` (e.g., ``enable_cache``) - **records_config**: Dictionary of records.config settings - **remap_config**: List of remap rules (string or dict format) + - **cache_config**: List of cache.config rules - **copy_to_config_dir**: List of files/directories to copy to ATS config directory - **log_validation**: Log validation rules for ``traffic_out`` and ``diags_log`` - **metric_checks**: List of metric name/value pairs to verify after traffic completes diff --git a/example/plugins/c-api/verify_cert/verify_cert.cc b/example/plugins/c-api/verify_cert/verify_cert.cc index f8746474bbe..a3f553057fe 100644 --- a/example/plugins/c-api/verify_cert/verify_cert.cc +++ b/example/plugins/c-api/verify_cert/verify_cert.cc @@ -66,7 +66,7 @@ CB_clientcert(TSCont /* contp */, TSEvent /* event */, void *edata) TSVConn ssl_vc = reinterpret_cast(edata); TSSslConnection sslobj = TSVConnSslConnectionGet(ssl_vc); SSL *ssl = reinterpret_cast(sslobj); -#ifdef OPENSSL_IS_OPENSSL3 +#ifdef OPENSSL_IS_AT_LEAST_OPENSSL3 X509 *cert = SSL_get1_peer_certificate(ssl); #else X509 *cert = SSL_get_peer_certificate(ssl); diff --git a/include/cripts/Connections.hpp b/include/cripts/Connections.hpp index 5d7eccbd08b..adee2a52964 100644 --- a/include/cripts/Connections.hpp +++ b/include/cripts/Connections.hpp @@ -365,7 +365,7 @@ class ConnBase auto conn = Connection(); if (mTLS) { -#ifdef OPENSSL_IS_OPENSSL3 +#ifdef OPENSSL_IS_AT_LEAST_OPENSSL3 return SSL_get1_peer_certificate(reinterpret_cast<::SSL *>(conn)); #else return SSL_get_peer_certificate(reinterpret_cast<::SSL *>(conn)); diff --git a/include/iocore/net/ConnectionTracker.h b/include/iocore/net/ConnectionTracker.h index d41d9ad86f0..360fda7b640 100644 --- a/include/iocore/net/ConnectionTracker.h +++ b/include/iocore/net/ConnectionTracker.h @@ -32,6 +32,7 @@ #include #include #include +#include #include "records/RecCore.h" #include "tscore/ink_platform.h" #include "tscore/ink_config.h" @@ -429,7 +430,7 @@ ConnectionTracker::Group::metric_name(const Key &key, std::string_view fqdn, std default: Warning("Invalid matching type to add to per_server.connections metrics"); } - return metric_prefix.empty() ? metric_name : metric_prefix + "." + metric_name; + return metric_prefix.empty() ? std::move(metric_name) : metric_prefix + "." + metric_name; } inline bool diff --git a/include/iocore/net/quic/QUICConfig.h b/include/iocore/net/quic/QUICConfig.h index b0b1ea25b2f..ee95c2773f1 100644 --- a/include/iocore/net/quic/QUICConfig.h +++ b/include/iocore/net/quic/QUICConfig.h @@ -34,6 +34,47 @@ #include "iocore/net/SSLTypes.h" #include "mgmt/config/ConfigContext.h" +#include +#include +#include + +class QUICTokenKeyConfigParams : public ConfigInfo +{ +public: + static constexpr size_t KEY_LENGTH = 32; + using Key = std::array; + + ~QUICTokenKeyConfigParams() override; + + /** Load one or more raw token keys from @a path. */ + bool load(const char *path, ConfigContext ctx = {}); + + /** Generate a random primary token key. */ + bool generate(ConfigContext ctx = {}); + + const std::vector &keys() const; + const std::string &filename() const; + +private: + std::vector m_keys; + std::string m_filename; +}; + +class QUICTokenKeyConfig +{ +public: + static void startup(); + static bool reconfigure(ConfigContext ctx = {}); + + static QUICTokenKeyConfigParams *acquire(); + static void release(QUICTokenKeyConfigParams *params); + + using scoped_config = ConfigProcessor::scoped_config; + +private: + static int _config_id; +}; + class QUICConfigParams : public ConfigInfo { public: diff --git a/include/iocore/net/quic/QUICTypes.h b/include/iocore/net/quic/QUICTypes.h index 951226b054e..f48b2c7c8bb 100644 --- a/include/iocore/net/quic/QUICTypes.h +++ b/include/iocore/net/quic/QUICTypes.h @@ -321,13 +321,20 @@ class QUICStatelessResetToken class QUICAddressValidationToken { public: + static constexpr size_t MAC_LENGTH = 32; + enum class Type : uint8_t { RESUMPTION, RETRY, }; - // FIXME Check token length - QUICAddressValidationToken(const uint8_t *buf, size_t len) : _token_len(len) { memcpy(this->_token, buf, len); } + QUICAddressValidationToken(const uint8_t *buf, size_t len) + { + if (buf != nullptr && len <= sizeof(_token)) { + memcpy(_token, buf, len); + _token_len = len; + } + } virtual ~QUICAddressValidationToken(){}; static Type @@ -354,7 +361,7 @@ class QUICAddressValidationToken // The size should be smaller than maximum size of Retry packet uint8_t _token[1200] = {0}; - unsigned int _token_len; + unsigned int _token_len = 0; }; class QUICResumptionToken : public QUICAddressValidationToken diff --git a/include/mgmt/config/ConfigReloadTrace.h b/include/mgmt/config/ConfigReloadTrace.h index 7d9dadbca07..6f5529a2bb3 100644 --- a/include/mgmt/config/ConfigReloadTrace.h +++ b/include/mgmt/config/ConfigReloadTrace.h @@ -25,11 +25,12 @@ #include #include +#include #include #include #include #include -#include +#include #include #include @@ -207,7 +208,7 @@ class ConfigReloadTask : public std::enable_shared_from_this using self_type = ConfigReloadTask; ConfigReloadTask() = default; ConfigReloadTask(std::string_view token, std::string_view description, bool main_task, ConfigReloadTaskPtr parent) - : _info(State::CREATED, token, description, main_task), _parent{parent} + : _info(State::CREATED, token, description, main_task), _parent{std::move(parent)} { if (_info.main_task) { _info.state = State::IN_PROGRESS; diff --git a/plugins/experimental/access_control/unit_tests/test_utils.cc b/plugins/experimental/access_control/unit_tests/test_utils.cc index a74678425be..90695cd322c 100644 --- a/plugins/experimental/access_control/unit_tests/test_utils.cc +++ b/plugins/experimental/access_control/unit_tests/test_utils.cc @@ -252,7 +252,7 @@ TEST_CASE("HMAC Digest: test various supported/unsupported types", "[MAC][access digests.push_back("ccf3230972bcf229fb3b16741495c74a72bbdd14"); #endif -#ifdef OPENSSL_IS_OPENSSL3 // MD4, RIPEMD160 are deprecated in OpenSSL 3 +#ifdef OPENSSL_IS_AT_LEAST_OPENSSL3 // MD4, RIPEMD160 are deprecated in OpenSSL 3 types.pop_front(); digests.pop_front(); types.pop_back(); diff --git a/plugins/experimental/filter_body/filter_body.cc b/plugins/experimental/filter_body/filter_body.cc index ca7471c71bf..224c78fec01 100644 --- a/plugins/experimental/filter_body/filter_body.cc +++ b/plugins/experimental/filter_body/filter_body.cc @@ -27,6 +27,7 @@ #include #include +#include #include #include #include @@ -901,7 +902,7 @@ parse_config(const char *filename) AddHeader add_hdr; add_hdr.name = hdr.first.as(); add_hdr.value = hdr.second.as(); - rule.add_headers.push_back(add_hdr); + rule.add_headers.push_back(std::move(add_hdr)); } } } @@ -953,7 +954,7 @@ parse_config(const char *filename) cond.patterns.push_back(pattern_node.as()); } } - rule.headers.push_back(cond); + rule.headers.push_back(std::move(cond)); } } diff --git a/plugins/experimental/jax_fingerprint/ja4/test.cc b/plugins/experimental/jax_fingerprint/ja4/test.cc index 35f8e79b53a..16456d19e7c 100644 --- a/plugins/experimental/jax_fingerprint/ja4/test.cc +++ b/plugins/experimental/jax_fingerprint/ja4/test.cc @@ -34,6 +34,7 @@ #include #include #include +#include #include namespace @@ -127,7 +128,7 @@ class MockDatasource : public ja4::Datasource void set_first_alpn(std::string first_alpn) { - this->_first_alpn = first_alpn; + this->_first_alpn = std::move(first_alpn); } void add_cipher(std::uint16_t cipher) diff --git a/plugins/experimental/jax_fingerprint/ja4h/test.cc b/plugins/experimental/jax_fingerprint/ja4h/test.cc index d2f84b1a8ba..3d1abb50224 100644 --- a/plugins/experimental/jax_fingerprint/ja4h/test.cc +++ b/plugins/experimental/jax_fingerprint/ja4h/test.cc @@ -27,6 +27,7 @@ #include #include +#include namespace { @@ -89,7 +90,7 @@ class MockDatasource : public Datasource void set_method(std::string method) { - this->_method = method; + this->_method = std::move(method); } void set_version(int version) @@ -99,7 +100,7 @@ class MockDatasource : public Datasource void set_fields(std::map fields) { - this->_fields = fields; + this->_fields = std::move(fields); } private: diff --git a/plugins/experimental/sslheaders/sslheaders.cc b/plugins/experimental/sslheaders/sslheaders.cc index 6f33711286f..b6613436c50 100644 --- a/plugins/experimental/sslheaders/sslheaders.cc +++ b/plugins/experimental/sslheaders/sslheaders.cc @@ -159,7 +159,7 @@ template class WrapX509 void _set() { -#ifdef OPENSSL_IS_OPENSSL3 +#ifdef OPENSSL_IS_AT_LEAST_OPENSSL3 _x509 = (IsClient ? SSL_get1_peer_certificate : SSL_get_certificate)(_ssl); #else _x509 = (IsClient ? SSL_get_peer_certificate : SSL_get_certificate)(_ssl); diff --git a/plugins/experimental/txn_box/plugin/src/Comparison.cc b/plugins/experimental/txn_box/plugin/src/Comparison.cc index 85013dd98a8..20d85d042e8 100644 --- a/plugins/experimental/txn_box/plugin/src/Comparison.cc +++ b/plugins/experimental/txn_box/plugin/src/Comparison.cc @@ -21,6 +21,7 @@ */ #include +#include #include #include @@ -1391,7 +1392,7 @@ ComboComparison::load(Config &cfg, YAML::Node const &cmp_node, TextView const &k } else if (value_node.IsSequence()) { cmps.reserve(cmp_node.size()); for (auto child : value_node) { - auto errata = self_type::load_case(cfg, cmps, child); + auto errata = self_type::load_case(cfg, cmps, std::move(child)); if (!errata.is_ok()) { errata.note("While parsing {} comparison at {}.", key, cmp_node.Mark()); return errata; diff --git a/plugins/experimental/txn_box/plugin/src/Config.cc b/plugins/experimental/txn_box/plugin/src/Config.cc index 3e5f9dca023..f90b1ee929e 100644 --- a/plugins/experimental/txn_box/plugin/src/Config.cc +++ b/plugins/experimental/txn_box/plugin/src/Config.cc @@ -23,6 +23,7 @@ #include #include #include +#include #include #include @@ -649,7 +650,7 @@ Config::parse_yaml(YAML::Node root, TextView path) if (root.IsSequence()) { for (auto child : root) { - errata.note((this->*drtv_loader)(child)); + errata.note((this->*drtv_loader)(std::move(child))); } if (!errata.is_ok()) { errata.note(R"(While loading list of top level directives for "{}" at {}.)", path, root.Mark()); diff --git a/plugins/experimental/txn_box/plugin/src/ip_space.cc b/plugins/experimental/txn_box/plugin/src/ip_space.cc index 6b472e6356e..bca213af703 100644 --- a/plugins/experimental/txn_box/plugin/src/ip_space.cc +++ b/plugins/experimental/txn_box/plugin/src/ip_space.cc @@ -698,7 +698,7 @@ Do_ip_space_define::load(Config &cfg, CfgStaticData const *, YAML::Node drtv_nod } } else if (cols_node.IsSequence()) { for (auto child : cols_node) { - auto errata = self->define_column(cfg, child); + auto errata = self->define_column(cfg, std::move(child)); if (!errata.is_ok()) { errata.note(R"(While parsing "{}" key at {}.)", COLUMNS_TAG, cols_node.Mark()); return errata; diff --git a/plugins/experimental/wasm/ats_context.cc b/plugins/experimental/wasm/ats_context.cc index c7c2b636f36..5670ea14605 100644 --- a/plugins/experimental/wasm/ats_context.cc +++ b/plugins/experimental/wasm/ats_context.cc @@ -719,7 +719,7 @@ Context::getProperty(std::string_view path, std::string *result) TSVConn client_conn = TSHttpSsnClientVConnGet(ssnp); TSSslConnection sslobj = TSVConnSslConnectionGet(client_conn); SSL *ssl = reinterpret_cast(sslobj); -#ifdef OPENSSL_IS_OPENSSL3 +#ifdef OPENSSL_IS_AT_LEAST_OPENSSL3 X509 *cert = SSL_get1_peer_certificate(ssl); #else X509 *cert = SSL_get_peer_certificate(ssl); @@ -787,7 +787,7 @@ Context::getProperty(std::string_view path, std::string *result) TSVConn client_conn = TSHttpSsnClientVConnGet(ssnp); TSSslConnection sslobj = TSVConnSslConnectionGet(client_conn); SSL *ssl = reinterpret_cast(sslobj); -#ifdef OPENSSL_IS_OPENSSL3 +#ifdef OPENSSL_IS_AT_LEAST_OPENSSL3 X509 *cert = SSL_get1_peer_certificate(ssl); #else X509 *cert = SSL_get_peer_certificate(ssl); @@ -825,7 +825,7 @@ Context::getProperty(std::string_view path, std::string *result) TSVConn client_conn = TSHttpSsnClientVConnGet(ssnp); TSSslConnection sslobj = TSVConnSslConnectionGet(client_conn); SSL *ssl = reinterpret_cast(sslobj); -#ifdef OPENSSL_IS_OPENSSL3 +#ifdef OPENSSL_IS_AT_LEAST_OPENSSL3 X509 *cert = SSL_get1_peer_certificate(ssl); #else X509 *cert = SSL_get_peer_certificate(ssl); @@ -863,7 +863,7 @@ Context::getProperty(std::string_view path, std::string *result) TSVConn client_conn = TSHttpSsnClientVConnGet(ssnp); TSSslConnection sslobj = TSVConnSslConnectionGet(client_conn); SSL *ssl = reinterpret_cast(sslobj); -#ifdef OPENSSL_IS_OPENSSL3 +#ifdef OPENSSL_IS_AT_LEAST_OPENSSL3 X509 *cert = SSL_get1_peer_certificate(ssl); #else X509 *cert = SSL_get_peer_certificate(ssl); @@ -952,7 +952,7 @@ Context::getProperty(std::string_view path, std::string *result) TSVConn client_conn = TSHttpSsnServerVConnGet(ssnp); TSSslConnection sslobj = TSVConnSslConnectionGet(client_conn); SSL *ssl = reinterpret_cast(sslobj); -#ifdef OPENSSL_IS_OPENSSL3 +#ifdef OPENSSL_IS_AT_LEAST_OPENSSL3 X509 *cert = SSL_get1_peer_certificate(ssl); #else X509 *cert = SSL_get_peer_certificate(ssl); @@ -990,7 +990,7 @@ Context::getProperty(std::string_view path, std::string *result) TSVConn client_conn = TSHttpSsnServerVConnGet(ssnp); TSSslConnection sslobj = TSVConnSslConnectionGet(client_conn); SSL *ssl = reinterpret_cast(sslobj); -#ifdef OPENSSL_IS_OPENSSL3 +#ifdef OPENSSL_IS_AT_LEAST_OPENSSL3 X509 *cert = SSL_get1_peer_certificate(ssl); #else X509 *cert = SSL_get_peer_certificate(ssl); @@ -1028,7 +1028,7 @@ Context::getProperty(std::string_view path, std::string *result) TSVConn client_conn = TSHttpSsnServerVConnGet(ssnp); TSSslConnection sslobj = TSVConnSslConnectionGet(client_conn); SSL *ssl = reinterpret_cast(sslobj); -#ifdef OPENSSL_IS_OPENSSL3 +#ifdef OPENSSL_IS_AT_LEAST_OPENSSL3 X509 *cert = SSL_get1_peer_certificate(ssl); #else X509 *cert = SSL_get_peer_certificate(ssl); diff --git a/plugins/healthchecks/healthchecks.cc b/plugins/healthchecks/healthchecks.cc index f31a43d0cc1..c5da85da9da 100644 --- a/plugins/healthchecks/healthchecks.cc +++ b/plugins/healthchecks/healthchecks.cc @@ -28,6 +28,8 @@ limitations under the License. #include #include #include +#include +#include /* ToDo: Linux specific */ #include @@ -42,9 +44,8 @@ static const char SEPARATORS[] = " \t\n"; static DbgCtl dbg_ctl{PLUGIN_NAME}; -#define MAX_PATH_LEN 4096 -#define MAX_BODY_LEN 16384 -#define FREELIST_TIMEOUT 300 +#define MAX_PATH_LEN 4096 +#define MAX_BODY_LEN 16384 /* Directories that we are watching for inotify IN_CREATE events. */ typedef struct HCDirEntry_t { @@ -53,66 +54,100 @@ typedef struct HCDirEntry_t { struct HCDirEntry_t *_next; /* Linked list */ } HCDirEntry; -/* Information about a status file. This is never modified (only replaced, see HCFileInfo_t) */ -typedef struct HCFileData_t { - int exists; /* Does this file exist */ - char body[MAX_BODY_LEN]; /* Body from fname. Empty string means file is missing */ - int b_len; /* Length of data */ - time_t remove; /* Used for deciding when the old object can be permanently removed */ - struct HCFileData_t *_next; /* Only used when these guys end up on the freelist */ -} HCFileData; - -/* The only thing that should change in this struct is data, atomically swapping ptrs */ -typedef struct HCFileInfo_t { - char fname[MAX_PATH_LEN]; /* Filename */ - char *basename; /* The "basename" of the file */ - unsigned basename_len = 0; /* The length of the basename */ - char path[PATH_NAME_MAX]; /* URL path for this HC */ - int p_len; /* Length of path */ - const char *ok; /* Header for an OK result */ - int o_len; /* Length of OK header */ - const char *miss; /* Header for miss results */ - int m_len; /* Length of miss header */ - std::atomic data; /* Holds the current data for this health check file */ - int wd; /* Watch descriptor */ - HCDirEntry *dir; /* Reference to the directory this file resides in */ - struct HCFileInfo_t *_next; /* Linked list */ -} HCFileInfo; +/* Information about a status file. This is never modified (only replaced, see HCFileInfo) */ +struct HCFileData { + int exists = 0; /* Does this file exist */ + int b_len = 0; /* Length of data */ + char body[MAX_BODY_LEN] = {}; /* Body from fname. Empty string means file is missing */ +}; + +using HCFileDataPtr = std::shared_ptr; + +/* The only thing that should change in this struct is data, which is replaced (never modified) by + the inotify thread. Readers take a reference to the current data via get_data(), which keeps + that snapshot alive for as long as the transaction needs it. */ +struct HCFileInfo { + char fname[MAX_PATH_LEN] = {}; /* Filename */ + char *basename = nullptr; /* The "basename" of the file */ + unsigned basename_len = 0; /* The length of the basename */ + char path[PATH_NAME_MAX] = {}; /* URL path for this HC */ + int p_len = 0; /* Length of path */ + const char *ok = nullptr; /* Header for an OK result */ + int o_len = 0; /* Length of OK header */ + const char *miss = nullptr; /* Header for miss results */ + int m_len = 0; /* Length of miss header */ + int wd = 0; /* Watch descriptor */ + HCDirEntry *dir = nullptr; /* Reference to the directory this file resides in */ + HCFileInfo *_next = nullptr; /* Linked list */ + + /* Take a reference to the current data for this health check file. */ + HCFileDataPtr + get_data() + { +#if defined(__cpp_lib_atomic_shared_ptr) && __cpp_lib_atomic_shared_ptr >= 201711L + return _data.load(std::memory_order_acquire); +#else + return std::atomic_load_explicit(&_data, std::memory_order_acquire); +#endif + } + + /* Replace the current data for this health check file. Snapshots handed out by get_data() stay + valid until their last reference is dropped. */ + void + set_data(HCFileDataPtr data) + { +#if defined(__cpp_lib_atomic_shared_ptr) && __cpp_lib_atomic_shared_ptr >= 201711L + _data.store(std::move(data), std::memory_order_release); +#else + std::atomic_store_explicit(&_data, std::move(data), std::memory_order_release); +#endif + } + +private: +#if defined(__cpp_lib_atomic_shared_ptr) && __cpp_lib_atomic_shared_ptr >= 201711L + std::atomic _data; /* Holds the current data for this health check file */ +#else + HCFileDataPtr _data; /* Holds the current data for this health check file */ +#endif +}; /* Global configuration */ HCFileInfo *g_config; /* State used for the intercept plugin. ToDo: Can this be improved ? */ -typedef struct HCState_t { - TSVConn net_vc; - TSVIO read_vio; - TSVIO write_vio; +struct HCState { + TSVConn net_vc = nullptr; + TSVIO read_vio = nullptr; + TSVIO write_vio = nullptr; - TSIOBuffer req_buffer; - TSIOBuffer resp_buffer; - TSIOBufferReader resp_reader; + TSIOBuffer req_buffer = nullptr; + TSIOBuffer resp_buffer = nullptr; + TSIOBufferReader resp_reader = nullptr; - int output_bytes; + int output_bytes = 0; - /* We actually need both here, so that our lock free switches works safely */ - HCFileInfo *info; - HCFileData *data; -} HCState; + /* We hold a reference to the data so that it cannot be replaced from under us mid transaction */ + HCFileInfo *info = nullptr; + HCFileDataPtr data; +}; /* Read / check the status files */ -static void -reload_status_file(HCFileInfo *info, HCFileData *data) +static HCFileDataPtr +load_status_file(HCFileInfo *info) { + auto data = std::make_shared(); FILE *fd; - memset(data, 0, sizeof(HCFileData)); if (nullptr != (fd = fopen(info->fname, "r"))) { data->exists = 1; - do { - data->b_len = fread(data->body, 1, MAX_BODY_LEN, fd); - } while (!feof(fd)); /* Only save the last 16KB of the file ... */ + size_t bytes_read; + while ((bytes_read = fread(data->body, 1, MAX_BODY_LEN, fd)) > 0) { + data->b_len = static_cast(bytes_read); + } fclose(fd); } + + return data; } /* Find a HCDirEntry from the linked list */ @@ -198,49 +233,16 @@ event_matches_config(struct inotify_event *event, HCFileInfo *finfo) static void * hc_thread(void *data ATS_UNUSED) { - int inotify_fd = inotify_init(); - HCFileData *fl_head = nullptr; - char buffer[INOTIFY_BUFLEN]; - struct timeval last_free, now; - - gettimeofday(&last_free, nullptr); + int inotify_fd = inotify_init(); + char buffer[INOTIFY_BUFLEN]; /* Setup watchers for the directories, these are a one time setup */ setup_watchers(inotify_fd); // This is a leak, but since we enter an infinite loop this is ok? while (true) { - HCFileData *fdata = fl_head, *fdata_prev = nullptr; - - gettimeofday(&now, nullptr); /* Read the inotify events, blocking until we get something */ int len = read(inotify_fd, buffer, INOTIFY_BUFLEN); - /* The fl_head is a linked list of previously released data entries. They - are ordered "by time", so once we find one that is scheduled for deletion, - we can also delete all entries after it in the linked list. */ - while (fdata) { - if (now.tv_sec > fdata->remove) { - /* Now drop off the "tail" from the freelist */ - if (fdata_prev) { - fdata_prev->_next = nullptr; - } else { - fl_head = nullptr; - } - - /* free() everything in the "tail" */ - do { - HCFileData *next = fdata->_next; - - Dbg(dbg_ctl, "Cleaning up entry from freelist"); - TSfree(fdata); - fdata = next; - } while (fdata); - break; /* Stop the loop, there's nothing else left to examine */ - } - fdata_prev = fdata; - fdata = fdata->_next; - } - if (len >= 0) { int i = 0; @@ -253,9 +255,6 @@ hc_thread(void *data ATS_UNUSED) finfo = finfo->_next; } if (finfo) { - auto *new_data = TSRalloc(); - HCFileData *old_data; - if (event->mask & (IN_CLOSE_WRITE | IN_ATTRIB)) { Dbg(dbg_ctl, "Modify file event (%d) on %s", event->mask, finfo->fname); } else if (event->mask & (IN_CREATE | IN_MOVED_TO)) { @@ -267,16 +266,12 @@ hc_thread(void *data ATS_UNUSED) } else { Dbg(dbg_ctl, "Unhandled event (%d) on %s", event->mask, finfo->fname); } - /* Load the new data and then swap this atomically */ - memset(new_data, 0, sizeof(HCFileData)); - reload_status_file(finfo, new_data); - Dbg(dbg_ctl, "Reloaded %s, len == %d, exists == %d", finfo->fname, new_data->b_len, new_data->exists); - old_data = finfo->data.exchange(new_data); + /* Load the new data and then publish it. The previous data is released once the last + transaction referencing it completes. */ + auto new_data = load_status_file(finfo); - /* Add the old data to the head of the freelist */ - old_data->remove = now.tv_sec + FREELIST_TIMEOUT; - old_data->_next = fl_head; - fl_head = old_data; + Dbg(dbg_ctl, "Reloaded %s, len == %d, exists == %d", finfo->fname, new_data->b_len, new_data->exists); + finfo->set_data(std::move(new_data)); } /* coverity[ -tainted_data_return] */ i += sizeof(struct inotify_event) + event->len; @@ -342,10 +337,9 @@ parse_configs(const char *fname) char *str, *save; char *ok = nullptr, *miss = nullptr, *mime = nullptr; - finfo = TSRalloc(); - memset(static_cast(finfo), 0, sizeof(HCFileInfo)); - if (fgets(buf, sizeof(buf) - 1, fd)) { + finfo = new HCFileInfo(); + str = strtok_r(buf, SEPARATORS, &save); int state = 0; while (nullptr != str) { @@ -388,9 +382,7 @@ parse_configs(const char *fname) Dbg(dbg_ctl, "Parsed: %s %s %s %s %s", finfo->path, finfo->fname, mime, ok, miss); finfo->ok = gen_header(ok, mime, &finfo->o_len); finfo->miss = gen_header(miss, mime, &finfo->m_len); - finfo->data = TSRalloc(); - memset(finfo->data, 0, sizeof(HCFileData)); - reload_status_file(finfo, finfo->data); + finfo->set_data(load_status_file(finfo)); /* Add it the linked list */ Dbg(dbg_ctl, "Adding path=%s to linked list", finfo->path); @@ -401,7 +393,7 @@ parse_configs(const char *fname) } prev_finfo = finfo; } else { - TSfree(finfo); + delete finfo; } } } @@ -434,7 +426,7 @@ cleanup(TSCont contp, HCState *my_state) my_state->net_vc = nullptr; } - TSfree(my_state); + delete my_state; TSContDestroy(contp); } @@ -567,11 +559,10 @@ health_check_origin(TSCont contp ATS_UNUSED, TSEvent event ATS_UNUSED, void *eda TSHttpTxnCntlSet(txnp, TS_HTTP_CNTL_SKIP_REMAPPING, true); /* not strictly necessary, but speed is everything these days */ /* This is us -- register our intercept */ - icontp = TSContCreate(hc_intercept, TSMutexCreate()); - my_state = TSRalloc(); - memset(my_state, 0, sizeof(*my_state)); + icontp = TSContCreate(hc_intercept, TSMutexCreate()); + my_state = new HCState(); my_state->info = info; - my_state->data = info->data; + my_state->data = info->get_data(); TSContDataSet(icontp, my_state); TSHttpTxnIntercept(icontp, txnp); } diff --git a/plugins/lua/ts_lua_client_request.cc b/plugins/lua/ts_lua_client_request.cc index 4e675ffafa2..1d6fb063b3a 100644 --- a/plugins/lua/ts_lua_client_request.cc +++ b/plugins/lua/ts_lua_client_request.cc @@ -1299,7 +1299,7 @@ ts_lua_client_request_client_cert_get_pem(lua_State *L) TSSslConnection ssl_conn = TSVConnSslConnectionGet(client_conn); if (ssl_conn) { SSL *ssl = reinterpret_cast(ssl_conn); -#ifdef OPENSSL_IS_OPENSSL3 +#ifdef OPENSSL_IS_AT_LEAST_OPENSSL3 X509 *cert = SSL_get1_peer_certificate(ssl); #else X509 *cert = SSL_get_peer_certificate(ssl); @@ -1335,7 +1335,7 @@ ts_lua_client_request_client_cert_get_subject(lua_State *L) TSSslConnection ssl_conn = TSVConnSslConnectionGet(client_conn); if (ssl_conn) { SSL *ssl = reinterpret_cast(ssl_conn); -#ifdef OPENSSL_IS_OPENSSL3 +#ifdef OPENSSL_IS_AT_LEAST_OPENSSL3 X509 *cert = SSL_get1_peer_certificate(ssl); #else X509 *cert = SSL_get_peer_certificate(ssl); @@ -1371,7 +1371,7 @@ ts_lua_client_request_client_cert_get_issuer(lua_State *L) TSSslConnection ssl_conn = TSVConnSslConnectionGet(client_conn); if (ssl_conn) { SSL *ssl = reinterpret_cast(ssl_conn); -#ifdef OPENSSL_IS_OPENSSL3 +#ifdef OPENSSL_IS_AT_LEAST_OPENSSL3 X509 *cert = SSL_get1_peer_certificate(ssl); #else X509 *cert = SSL_get_peer_certificate(ssl); @@ -1407,7 +1407,7 @@ ts_lua_client_request_client_cert_get_serial(lua_State *L) TSSslConnection ssl_conn = TSVConnSslConnectionGet(client_conn); if (ssl_conn) { SSL *ssl = reinterpret_cast(ssl_conn); -#ifdef OPENSSL_IS_OPENSSL3 +#ifdef OPENSSL_IS_AT_LEAST_OPENSSL3 X509 *cert = SSL_get1_peer_certificate(ssl); #else X509 *cert = SSL_get_peer_certificate(ssl); @@ -1443,7 +1443,7 @@ ts_lua_client_request_client_cert_get_signature(lua_State *L) TSSslConnection ssl_conn = TSVConnSslConnectionGet(client_conn); if (ssl_conn) { SSL *ssl = reinterpret_cast(ssl_conn); -#ifdef OPENSSL_IS_OPENSSL3 +#ifdef OPENSSL_IS_AT_LEAST_OPENSSL3 X509 *cert = SSL_get1_peer_certificate(ssl); #else X509 *cert = SSL_get_peer_certificate(ssl); @@ -1479,7 +1479,7 @@ ts_lua_client_request_client_cert_get_not_before(lua_State *L) TSSslConnection ssl_conn = TSVConnSslConnectionGet(client_conn); if (ssl_conn) { SSL *ssl = reinterpret_cast(ssl_conn); -#ifdef OPENSSL_IS_OPENSSL3 +#ifdef OPENSSL_IS_AT_LEAST_OPENSSL3 X509 *cert = SSL_get1_peer_certificate(ssl); #else X509 *cert = SSL_get_peer_certificate(ssl); @@ -1515,7 +1515,7 @@ ts_lua_client_request_client_cert_get_not_after(lua_State *L) TSSslConnection ssl_conn = TSVConnSslConnectionGet(client_conn); if (ssl_conn) { SSL *ssl = reinterpret_cast(ssl_conn); -#ifdef OPENSSL_IS_OPENSSL3 +#ifdef OPENSSL_IS_AT_LEAST_OPENSSL3 X509 *cert = SSL_get1_peer_certificate(ssl); #else X509 *cert = SSL_get_peer_certificate(ssl); @@ -1551,7 +1551,7 @@ ts_lua_client_request_client_cert_get_version(lua_State *L) TSSslConnection ssl_conn = TSVConnSslConnectionGet(client_conn); if (ssl_conn) { SSL *ssl = reinterpret_cast(ssl_conn); -#ifdef OPENSSL_IS_OPENSSL3 +#ifdef OPENSSL_IS_AT_LEAST_OPENSSL3 X509 *cert = SSL_get1_peer_certificate(ssl); #else X509 *cert = SSL_get_peer_certificate(ssl); @@ -1585,7 +1585,7 @@ ts_lua_client_request_client_cert_get_san_dns(lua_State *L) TSSslConnection ssl_conn = TSVConnSslConnectionGet(client_conn); if (ssl_conn) { SSL *ssl = reinterpret_cast(ssl_conn); -#ifdef OPENSSL_IS_OPENSSL3 +#ifdef OPENSSL_IS_AT_LEAST_OPENSSL3 X509 *cert = SSL_get1_peer_certificate(ssl); #else X509 *cert = SSL_get_peer_certificate(ssl); @@ -1626,7 +1626,7 @@ ts_lua_client_request_client_cert_get_san_ip(lua_State *L) TSSslConnection ssl_conn = TSVConnSslConnectionGet(client_conn); if (ssl_conn) { SSL *ssl = reinterpret_cast(ssl_conn); -#ifdef OPENSSL_IS_OPENSSL3 +#ifdef OPENSSL_IS_AT_LEAST_OPENSSL3 X509 *cert = SSL_get1_peer_certificate(ssl); #else X509 *cert = SSL_get_peer_certificate(ssl); @@ -1667,7 +1667,7 @@ ts_lua_client_request_client_cert_get_san_email(lua_State *L) TSSslConnection ssl_conn = TSVConnSslConnectionGet(client_conn); if (ssl_conn) { SSL *ssl = reinterpret_cast(ssl_conn); -#ifdef OPENSSL_IS_OPENSSL3 +#ifdef OPENSSL_IS_AT_LEAST_OPENSSL3 X509 *cert = SSL_get1_peer_certificate(ssl); #else X509 *cert = SSL_get_peer_certificate(ssl); @@ -1708,7 +1708,7 @@ ts_lua_client_request_client_cert_get_san_uri(lua_State *L) TSSslConnection ssl_conn = TSVConnSslConnectionGet(client_conn); if (ssl_conn) { SSL *ssl = reinterpret_cast(ssl_conn); -#ifdef OPENSSL_IS_OPENSSL3 +#ifdef OPENSSL_IS_AT_LEAST_OPENSSL3 X509 *cert = SSL_get1_peer_certificate(ssl); #else X509 *cert = SSL_get_peer_certificate(ssl); diff --git a/plugins/prefetch/path.cc b/plugins/prefetch/path.cc index a89e16ce522..7f500bab3ce 100644 --- a/plugins/prefetch/path.cc +++ b/plugins/prefetch/path.cc @@ -24,6 +24,7 @@ #include "path.h" #include +#include namespace { @@ -181,7 +182,7 @@ makeSafeRelativeFetchPath(const String ¤tPath, const String &relativePath, return false; } - fetchPath.path = normalizedCandidatePath; + fetchPath.path = std::move(normalizedCandidatePath); if (String::npos != queryStart) { fetchPath.hasQuery = true; fetchPath.query = relativePath.substr(queryStart + 1); diff --git a/plugins/stats_over_http/stats_over_http.cc b/plugins/stats_over_http/stats_over_http.cc index 2eb4b8b5d91..c5b208261e8 100644 --- a/plugins/stats_over_http/stats_over_http.cc +++ b/plugins/stats_over_http/stats_over_http.cc @@ -40,6 +40,7 @@ #include #include #include +#include #include #include #include @@ -744,7 +745,7 @@ parse_metric_v2(std::string_view name) } } - return {base_name, labels}; + return {std::move(base_name), std::move(labels)}; } static bool diff --git a/plugins/webp_transform/ImageTransform.cc b/plugins/webp_transform/ImageTransform.cc index e4ebc7f4d7c..530503748c6 100644 --- a/plugins/webp_transform/ImageTransform.cc +++ b/plugins/webp_transform/ImageTransform.cc @@ -434,13 +434,13 @@ class GlobalHookPlugin : public GlobalPlugin if (!content_length_usable) { TSHttpTxnServerRespNoStoreSet(static_cast(transaction.getAtsHandle()), 1); } - transaction.addPlugin(new ImageTransform(transaction, ctype, input_image_type, ImageEncoding::webp)); + transaction.addPlugin(new ImageTransform(transaction, std::move(ctype), input_image_type, ImageEncoding::webp)); } else if (webp_supported == false && transaction_convert_to_jpeg == true) { Dbg(webp_dbg_ctl, "Content type is webp. Converting to jpeg"); if (!content_length_usable) { TSHttpTxnServerRespNoStoreSet(static_cast(transaction.getAtsHandle()), 1); } - transaction.addPlugin(new ImageTransform(transaction, ctype, input_image_type, ImageEncoding::jpeg)); + transaction.addPlugin(new ImageTransform(transaction, std::move(ctype), input_image_type, ImageEncoding::jpeg)); } else { Dbg(webp_dbg_ctl, "Nothing to convert"); } diff --git a/src/api/InkAPI.cc b/src/api/InkAPI.cc index 9aabc670044..05fc77e2558 100644 --- a/src/api/InkAPI.cc +++ b/src/api/InkAPI.cc @@ -22,11 +22,12 @@ */ #include +#include #include #include #include #include -#include +#include #include "iocore/net/NetVConnection.h" #include "iocore/net/NetHandler.h" @@ -9205,7 +9206,8 @@ TSLogFieldRegister(std::string_view name, std::string_view symbol, TSLogType typ LogField *field = new LogField( name.data(), symbol.data(), static_cast(type), - [marshal_cb](void *sm, char *buf) -> int { return marshal_cb(reinterpret_cast(sm), buf); }, unmarshal_cb); + [marshal_cb = std::move(marshal_cb)](void *sm, char *buf) -> int { return marshal_cb(reinterpret_cast(sm), buf); }, + unmarshal_cb); Log::global_field_list.add(field, false); Log::field_symbol_hash.emplace(symbol.data(), field); diff --git a/src/iocore/cache/CacheDir.cc b/src/iocore/cache/CacheDir.cc index 99e9fba47b4..7368e57c4e1 100644 --- a/src/iocore/cache/CacheDir.cc +++ b/src/iocore/cache/CacheDir.cc @@ -35,6 +35,7 @@ #include #include +#include #ifdef LOOP_CHECK_MODE #define DIR_LOOP_THRESHOLD 1000 @@ -932,7 +933,7 @@ sync_cache_dir_on_shutdown() for (auto &[disk, indices] : drive_stripe_map) { Dbg(dbg_ctl_cache_dir_sync, "Disk %s: syncing %zu stripe(s)", disk->path, indices.size()); auto stripe_indices = indices; - threads.emplace_back([stripe_indices]() { + threads.emplace_back([stripe_indices = std::move(stripe_indices)]() { // Use a thread_local variable to give each OS thread a unique EThread* sentinel instead of 0xdeadbeef. thread_local char thread_sentinel; EThread *t = reinterpret_cast(&thread_sentinel); diff --git a/src/iocore/net/CMakeLists.txt b/src/iocore/net/CMakeLists.txt index 8ef85f25ef2..22910c16ebe 100644 --- a/src/iocore/net/CMakeLists.txt +++ b/src/iocore/net/CMakeLists.txt @@ -146,9 +146,12 @@ if(BUILD_TESTING) unit_tests/test_OCSPStapling.cc unit_tests/unit_test_main.cc ) - if(SSLLIB_IS_OPENSSL3) + if(SSLLIB_IS_AT_LEAST_OPENSSL3) target_sources(test_net PRIVATE unit_tests/test_SSLDHParams.cc) endif() + if(TS_USE_QUIC) + target_sources(test_net PRIVATE unit_tests/test_QUICTokenKeyConfig.cc) + endif() # Use link groups to solve circular dependency set(LINK_GROUP_LIBS ts::logging diff --git a/src/iocore/net/P_QUICNetVConnection.h b/src/iocore/net/P_QUICNetVConnection.h index bb8064ecb49..a43ed31c345 100644 --- a/src/iocore/net/P_QUICNetVConnection.h +++ b/src/iocore/net/P_QUICNetVConnection.h @@ -221,7 +221,9 @@ class QUICNetVConnection : public UnixNetVConnection, QUICConnectionId _initial_source_connection_id; // src cid used for Initial packet QUICConnectionId _quic_connection_id; // src cid in local +#if TS_HAS_QUICHE QUICConnectionTable *_ctable = nullptr; +#endif #if TS_HAS_OPENSSL_QUIC std::unordered_map _openssl_streams; @@ -240,10 +242,12 @@ class QUICNetVConnection : public UnixNetVConnection, void _close_packet_write_ready(Event *data); Event *_packet_write_ready = nullptr; - void _schedule_quiche_timeout(); - void _unschedule_quiche_timeout(); - void _close_quiche_timeout(Event *data); + void _schedule_quiche_timeout(); + void _unschedule_quiche_timeout(); + void _close_quiche_timeout(Event *data); +#if TS_HAS_QUICHE Event *_quiche_timeout = nullptr; +#endif void _schedule_closing_event(); diff --git a/src/iocore/net/P_SSLNetVConnection.h b/src/iocore/net/P_SSLNetVConnection.h index 28e0e99eb3b..161748796d1 100644 --- a/src/iocore/net/P_SSLNetVConnection.h +++ b/src/iocore/net/P_SSLNetVConnection.h @@ -258,7 +258,7 @@ class SSLNetVConnection : public UnixNetVConnection, bool peer_provided_cert() const override { -#ifdef OPENSSL_IS_OPENSSL3 +#ifdef OPENSSL_IS_AT_LEAST_OPENSSL3 X509 *cert = SSL_get1_peer_certificate(this->ssl); #else X509 *cert = SSL_get_peer_certificate(this->ssl); diff --git a/src/iocore/net/P_SSLUtils.h b/src/iocore/net/P_SSLUtils.h index af5728e4869..b3e327fbefa 100644 --- a/src/iocore/net/P_SSLUtils.h +++ b/src/iocore/net/P_SSLUtils.h @@ -25,7 +25,7 @@ #include "iocore/net/SSLTypes.h" #include "tscore/Diags.h" -#ifdef OPENSSL_IS_OPENSSL3 +#ifdef OPENSSL_IS_AT_LEAST_OPENSSL3 #include #include #endif @@ -114,7 +114,15 @@ namespace detail } }; -#ifdef OPENSSL_IS_OPENSSL3 + struct PKEYDeleter { + void + operator()(EVP_PKEY *p) + { + EVP_PKEY_free(p); + } + }; + +#ifdef OPENSSL_IS_AT_LEAST_OPENSSL3 struct PKEYCTXDeleter { void operator()(EVP_PKEY_CTX *pctx) @@ -156,7 +164,8 @@ struct ats_wildcard_matcher { using scoped_X509 = std::unique_ptr; using scoped_BIO = std::unique_ptr; -#ifdef OPENSSL_IS_OPENSSL3 +using scoped_PKEY = std::unique_ptr; +#ifdef OPENSSL_IS_AT_LEAST_OPENSSL3 using scoped_PKEY_CTX = std::unique_ptr; using scoped_Decoder_CTX = std::unique_ptr; #endif diff --git a/src/iocore/net/SSLClientCoordinator.cc b/src/iocore/net/SSLClientCoordinator.cc index e4af562498f..89b73f8cee0 100644 --- a/src/iocore/net/SSLClientCoordinator.cc +++ b/src/iocore/net/SSLClientCoordinator.cc @@ -25,6 +25,7 @@ #include "P_SSLConfig.h" #include "iocore/net/SSLSNIConfig.h" #include "mgmt/config/ConfigRegistry.h" +#include "records/RecHttp.h" #include "tscore/Filenames.h" #if TS_USE_QUIC == 1 #include "iocore/net/QUICMultiCertConfigLoader.h" @@ -40,7 +41,9 @@ SSLClientCoordinator::reconfigure(ConfigContext reconf_ctx) SNIConfig::reconfigure(reconf_ctx.add_dependent_ctx("SNIConfig")); SSLCertificateConfig::reconfigure(reconf_ctx.add_dependent_ctx("SSLCertificateConfig")); #if TS_USE_QUIC == 1 - QUICCertConfig::reconfigure(reconf_ctx.add_dependent_ctx("QUICCertConfig")); + if (HttpProxyPort::hasQUIC()) { + QUICCertConfig::reconfigure(reconf_ctx.add_dependent_ctx("QUICCertConfig")); + } #endif reconf_ctx.complete("SSL configs reloaded"); } diff --git a/src/iocore/net/SSLClientUtils.cc b/src/iocore/net/SSLClientUtils.cc index b675f8c560e..a91b86c5cfd 100644 --- a/src/iocore/net/SSLClientUtils.cc +++ b/src/iocore/net/SSLClientUtils.cc @@ -177,7 +177,7 @@ validate_server_certificate_hostname(NetVConnection *netvc, std::string_view hos char *matched_name = nullptr; bool const enforce_mode = netvc->options.verifyServerPolicy == YamlSNIConfig::Policy::ENFORCED; bool verified = false; -#ifdef OPENSSL_IS_OPENSSL3 +#ifdef OPENSSL_IS_AT_LEAST_OPENSSL3 X509 *cert = SSL_get1_peer_certificate(ssl); #else X509 *cert = SSL_get_peer_certificate(ssl); diff --git a/src/iocore/net/SSLKeyUtils.cc b/src/iocore/net/SSLKeyUtils.cc index 4bf14c5f473..590353d2d8b 100644 --- a/src/iocore/net/SSLKeyUtils.cc +++ b/src/iocore/net/SSLKeyUtils.cc @@ -23,13 +23,13 @@ #include "P_SSLUtils.h" #include -#ifdef OPENSSL_IS_OPENSSL3 +#ifdef OPENSSL_IS_AT_LEAST_OPENSSL3 #include #else #include #endif -#ifdef OPENSSL_IS_OPENSSL3 +#ifdef OPENSSL_IS_AT_LEAST_OPENSSL3 #include #include #include @@ -41,7 +41,7 @@ #include #endif -#ifdef OPENSSL_IS_OPENSSL3 +#ifdef OPENSSL_IS_AT_LEAST_OPENSSL3 EVP_PKEY * gen_dh_2048_256_pkey() @@ -187,4 +187,4 @@ set_ctx_dh(SSL_CTX *ctx, dh_key_t *pkey) return result; } -#endif // OPENSSL_IS_OPENSSL3 +#endif // OPENSSL_IS_AT_LEAST_OPENSSL3 diff --git a/src/iocore/net/SSLKeyUtils.h b/src/iocore/net/SSLKeyUtils.h index 64bea23e9ea..aab9a35e43d 100644 --- a/src/iocore/net/SSLKeyUtils.h +++ b/src/iocore/net/SSLKeyUtils.h @@ -21,14 +21,14 @@ #pragma once -#if OPENSSL_IS_OPENSSL3 +#if OPENSSL_IS_AT_LEAST_OPENSSL3 #include #else #include #endif #include -#ifdef OPENSSL_IS_OPENSSL3 +#ifdef OPENSSL_IS_AT_LEAST_OPENSSL3 using dh_key_t = EVP_PKEY; #else using dh_key_t = DH; diff --git a/src/iocore/net/SSLNetVConnection.cc b/src/iocore/net/SSLNetVConnection.cc index 4165cf23dfb..04691a3fc8e 100644 --- a/src/iocore/net/SSLNetVConnection.cc +++ b/src/iocore/net/SSLNetVConnection.cc @@ -1429,7 +1429,7 @@ SSLNetVConnection::sslServerHandShakeEvent(int &err) switch (ssl_error) { case SSL_ERROR_NONE: if (dbg_ctl_ssl.on()) { -#ifdef OPENSSL_IS_OPENSSL3 +#ifdef OPENSSL_IS_AT_LEAST_OPENSSL3 X509 *cert = SSL_get1_peer_certificate(ssl); #else X509 *cert = SSL_get_peer_certificate(ssl); @@ -1611,7 +1611,7 @@ SSLNetVConnection::sslClientHandShakeEvent(int &err) switch (ssl_error) { case SSL_ERROR_NONE: if (dbg_ctl_ssl.on()) { -#ifdef OPENSSL_IS_OPENSSL3 +#ifdef OPENSSL_IS_AT_LEAST_OPENSSL3 X509 *cert = SSL_get1_peer_certificate(ssl); #else X509 *cert = SSL_get_peer_certificate(ssl); diff --git a/src/iocore/net/SSLUtils.cc b/src/iocore/net/SSLUtils.cc index 0a6750b98ae..22b93de583f 100644 --- a/src/iocore/net/SSLUtils.cc +++ b/src/iocore/net/SSLUtils.cc @@ -57,14 +57,11 @@ #include #include #include -#ifdef OPENSSL_IS_OPENSSL3 +#ifdef OPENSSL_IS_AT_LEAST_OPENSSL3 #include #endif #include #include -#if HAVE_ENGINE_LOAD_DYNAMIC -#include -#endif #include #include #include @@ -870,10 +867,6 @@ void SSLPostConfigInitialize() { if (SSLConfigParams::engine_conf_file) { -#if HAVE_ENGINE_LOAD_DYNAMIC - ENGINE_load_dynamic(); -#endif - OPENSSL_load_builtin_modules(); if (CONF_modules_load_file(SSLConfigParams::engine_conf_file, nullptr, 0) <= 0) { char err_buf[256] = {0}; @@ -952,43 +945,27 @@ SSLMultiCertConfigLoader::default_server_ssl_ctx() static bool SSLPrivateKeyHandler(SSL_CTX *ctx, const char *keyPath, const char *secret_data, int secret_data_len) { - EVP_PKEY *pkey = nullptr; -#if HAVE_ENGINE_GET_DEFAULT_RSA && HAVE_ENGINE_LOAD_PRIVATE_KEY - ENGINE *e = ENGINE_get_default_RSA(); - if (e != nullptr) { - pkey = ENGINE_load_private_key(e, keyPath, nullptr, nullptr); - if (pkey) { - if (!SSL_CTX_use_PrivateKey(ctx, pkey)) { - Dbg(dbg_ctl_ssl_load, "failed to load server private key from engine"); - EVP_PKEY_free(pkey); - return false; - } - } + // SSL_CTX_use_PrivateKey() takes its own reference on the key, so this + // reference must be released on every exit. + scoped_BIO bio(BIO_new_mem_buf(secret_data, secret_data_len)); + + pem_password_cb *password_cb = SSL_CTX_get_default_passwd_cb(ctx); + void *u = SSL_CTX_get_default_passwd_cb_userdata(ctx); + + scoped_PKEY const pkey{PEM_read_bio_PrivateKey(bio.get(), nullptr, password_cb, u)}; + if (nullptr == pkey) { + Dbg(dbg_ctl_ssl_load, "failed to load server private key (%.*s) from %s", secret_data_len < 50 ? secret_data_len : 50, + secret_data, (!keyPath || keyPath[0] == '\0') ? "[empty key path]" : keyPath); + return false; } -#else - void *e = nullptr; -#endif - if (pkey == nullptr) { - scoped_BIO bio(BIO_new_mem_buf(secret_data, secret_data_len)); - - pem_password_cb *password_cb = SSL_CTX_get_default_passwd_cb(ctx); - void *u = SSL_CTX_get_default_passwd_cb_userdata(ctx); - pkey = PEM_read_bio_PrivateKey(bio.get(), nullptr, password_cb, u); - if (nullptr == pkey) { - Dbg(dbg_ctl_ssl_load, "failed to load server private key (%.*s) from %s", secret_data_len < 50 ? secret_data_len : 50, - secret_data, (!keyPath || keyPath[0] == '\0') ? "[empty key path]" : keyPath); - return false; - } - if (!SSL_CTX_use_PrivateKey(ctx, pkey)) { - Dbg(dbg_ctl_ssl_load, "failed to attach server private key loaded from %s", - (!keyPath || keyPath[0] == '\0') ? "[empty key path]" : keyPath); - EVP_PKEY_free(pkey); - return false; - } - if (e == nullptr && !SSL_CTX_check_private_key(ctx)) { - Dbg(dbg_ctl_ssl_load, "server private key does not match the certificate public key"); - return false; - } + if (!SSL_CTX_use_PrivateKey(ctx, pkey.get())) { + Dbg(dbg_ctl_ssl_load, "failed to attach server private key loaded from %s", + (!keyPath || keyPath[0] == '\0') ? "[empty key path]" : keyPath); + return false; + } + if (!SSL_CTX_check_private_key(ctx)) { + Dbg(dbg_ctl_ssl_load, "server private key does not match the certificate public key"); + return false; } return true; diff --git a/src/iocore/net/quic/QUICConfig.cc b/src/iocore/net/quic/QUICConfig.cc index 7d9fe28927a..7e25ae669b2 100644 --- a/src/iocore/net/quic/QUICConfig.cc +++ b/src/iocore/net/quic/QUICConfig.cc @@ -23,13 +23,19 @@ #include "iocore/net/quic/QUICConfig.h" +#include +#include #if TS_HAS_OPENSSL_QUIC #include #endif #include #include "mgmt/config/ConfigContextDiags.h" +#include "mgmt/config/ConfigRegistry.h" #include "records/RecHttp.h" +#include "tscore/Layout.h" +#include "tscore/MatcherUtils.h" +#include "tscore/ink_memory.h" #include "../P_SSLConfig.h" #include "../P_TLSKeyLogger.h" @@ -37,9 +43,145 @@ #include "iocore/net/quic/QUICGlobals.h" #include "iocore/net/quic/QUICTransportParameters.h" +int QUICTokenKeyConfig::_config_id = 0; int QUICConfig::_config_id = 0; int QUICConfigParams::_connection_table_size = 65521; +QUICTokenKeyConfigParams::~QUICTokenKeyConfigParams() +{ + if (!m_keys.empty()) { + OPENSSL_cleanse(m_keys.data(), m_keys.size() * sizeof(Key)); + } +} + +bool +QUICTokenKeyConfigParams::load(const char *path, ConfigContext ctx) +{ + int key_data_len = 0; + ats_scoped_str key_data{readIntoBuffer(path, __func__, &key_data_len)}; + + if (!key_data) { + CfgLoadFail(ctx, "Could not load QUIC token key from %s", path); + return false; + } + + if (key_data_len < 0) { + CfgLoadFail(ctx, "QUIC token key file %s is too large", path); + return false; + } + + size_t const key_data_size = static_cast(key_data_len); + + if (key_data_size < KEY_LENGTH || key_data_size % KEY_LENGTH != 0) { + CfgLoadFail(ctx, "QUIC token key file %s must contain one or more %zu-byte keys", path, KEY_LENGTH); + OPENSSL_cleanse(key_data.get(), key_data_size); + return false; + } + + m_keys.resize(key_data_size / KEY_LENGTH); + memcpy(m_keys.data(), key_data.get(), key_data_size); + OPENSSL_cleanse(key_data.get(), key_data_size); + m_filename = path; + return true; +} + +bool +QUICTokenKeyConfigParams::generate(ConfigContext ctx) +{ + m_keys.resize(1); + if (RAND_bytes(m_keys.front().data(), static_cast(m_keys.front().size())) != 1) { + CfgLoadFail(ctx, "Could not generate a random QUIC token key"); + OPENSSL_cleanse(m_keys.data(), m_keys.size() * sizeof(Key)); + m_keys.clear(); + return false; + } + return true; +} + +const std::vector & +QUICTokenKeyConfigParams::keys() const +{ + return m_keys; +} + +const std::string & +QUICTokenKeyConfigParams::filename() const +{ + return m_filename; +} + +void +QUICTokenKeyConfig::startup() +{ + config::ConfigRegistry::Get_Instance().register_record_config("quic_token_key", + [](ConfigContext ctx) { + CfgLoadLog(ctx, DL_Note, "QUIC token key loading ..."); + if (QUICTokenKeyConfig::reconfigure(ctx)) { + ctx.complete("QUIC token key reloaded"); + } else { + ctx.fail("Failed to reload QUIC token key"); + } + }, + {"proxy.config.quic.server.token_key.filename"}); + + if (!reconfigure()) { + Fatal("Failed to initialize QUIC token key"); + } +} + +bool +QUICTokenKeyConfig::reconfigure(ConfigContext ctx) +{ + std::string path; + if (auto rec_str = RecGetRecordStringAlloc("proxy.config.quic.server.token_key.filename"); rec_str && !rec_str->empty()) { + path = Layout::relative_to(Layout::get()->sysconfdir, *rec_str); + } + + if (path.empty()) { + bool already_random = false; + { + scoped_config current; + already_random = current && current->filename().empty(); + } + if (already_random) { + return true; + } + } + + auto *params = new QUICTokenKeyConfigParams; + if ((!path.empty() && !params->load(path.c_str(), ctx)) || (path.empty() && !params->generate(ctx))) { + delete params; + return false; + } + + bool unchanged = false; + { + scoped_config current; + unchanged = current && current->filename() == params->filename() && current->keys() == params->keys(); + } + if (unchanged) { + delete params; + return true; + } + + _config_id = configProcessor.set(_config_id, params); + return true; +} + +QUICTokenKeyConfigParams * +QUICTokenKeyConfig::acquire() +{ + return static_cast(configProcessor.get(_config_id)); +} + +void +QUICTokenKeyConfig::release(QUICTokenKeyConfigParams *params) +{ + if (_config_id > 0) { + configProcessor.release(_config_id, params); + } +} + SSL_CTX * quic_new_ssl_ctx() { @@ -471,6 +613,7 @@ QUICConfigParams::get_cc_algorithm() const void QUICConfig::startup() { + QUICTokenKeyConfig::startup(); reconfigure(); } diff --git a/src/iocore/net/quic/QUICTypes.cc b/src/iocore/net/quic/QUICTypes.cc index 419c586a54f..9de55694fbf 100644 --- a/src/iocore/net/quic/QUICTypes.cc +++ b/src/iocore/net/quic/QUICTypes.cc @@ -27,13 +27,82 @@ #include #include "iocore/net/quic/QUICTypes.h" +#include "iocore/net/quic/QUICConfig.h" #include "iocore/net/quic/QUICIntUtil.h" -#include "tscore/CryptoHash.h" #include +#include #include uint8_t QUICConnectionId::SCID_LEN = 0; +namespace +{ +bool +token_hmac(const QUICTokenKeyConfigParams::Key &key, const uint8_t *data, size_t data_len, + uint8_t (&digest)[QUICAddressValidationToken::MAC_LENGTH]) +{ + unsigned int digest_len = 0; + return HMAC(EVP_sha256(), key.data(), static_cast(key.size()), data, data_len, digest, &digest_len) != nullptr && + digest_len == sizeof(digest); +} + +bool +generate_token_hmac(const uint8_t *data, size_t data_len, uint8_t (&digest)[QUICAddressValidationToken::MAC_LENGTH]) +{ + QUICTokenKeyConfig::scoped_config key_config; + return key_config && !key_config->keys().empty() && token_hmac(key_config->keys().front(), data, data_len, digest); +} + +bool +validate_token_hmac(const uint8_t *data, size_t data_len, const uint8_t *expected) +{ + QUICTokenKeyConfig::scoped_config key_config; + uint8_t digest[QUICAddressValidationToken::MAC_LENGTH]; + bool valid = false; + + if (!key_config) { + return false; + } + + for (auto const &key : key_config->keys()) { + if (token_hmac(key, data, data_len, digest)) { + valid |= CRYPTO_memcmp(digest, expected, sizeof(digest)) == 0; + } + } + OPENSSL_cleanse(digest, sizeof(digest)); + return valid; +} + +size_t +retry_token_data(const IpEndpoint &src, QUICConnectionId original_dcid, QUICConnectionId scid, uint8_t *data, size_t data_size) +{ + ats_ip_nptop(src, reinterpret_cast(data), data_size); + size_t data_len = strlen(reinterpret_cast(data)); + size_t cid_len = 0; + + data[data_len++] = original_dcid.length(); + QUICTypeUtil::write_QUICConnectionId(original_dcid, data + data_len, &cid_len); + data_len += cid_len; + data[data_len++] = scid.length(); + QUICTypeUtil::write_QUICConnectionId(scid, data + data_len, &cid_len); + return data_len + cid_len; +} + +size_t +resumption_token_data(const IpEndpoint &src, QUICConnectionId cid, ink_hrtime expire_time, uint8_t *data, size_t data_size) +{ + ats_ip_nptop(src, reinterpret_cast(data), data_size); + size_t data_len = strlen(reinterpret_cast(data)); + size_t cid_len = 0; + size_t ignored = 0; + + QUICTypeUtil::write_QUICConnectionId(cid, data + data_len, &cid_len); + data_len += cid_len; + QUICIntUtil::write_uint_as_nbytes(expire_time >> 30, 4, data + data_len, &ignored); + return data_len + 4; +} +} // namespace + // TODO: move to somewhere in lib/ts/ int to_hex_str(char *dst, size_t dst_len, const uint8_t *src, size_t src_len) @@ -276,17 +345,18 @@ QUICTypeUtil::write_QUICMaxData(uint64_t max_data, uint8_t *buf, size_t *len) QUICStatelessResetToken::QUICStatelessResetToken(const QUICConnectionId &conn_id, uint32_t instance_id) { - uint64_t data = conn_id ^ instance_id; - CryptoHash _hash; - static constexpr char STATELESS_RESET_TOKEN_KEY[] = "stateless_token_reset_key"; - CryptoContext ctx; - ctx.update(STATELESS_RESET_TOKEN_KEY, strlen(STATELESS_RESET_TOKEN_KEY)); - ctx.update(reinterpret_cast(&data), 8); - ctx.finalize(_hash); + uint8_t data[QUICConnectionId::MAX_LENGTH + sizeof(instance_id)]; + uint8_t digest[QUICAddressValidationToken::MAC_LENGTH]; + size_t data_len = conn_id.length(); + size_t ignored = 0; - size_t dummy; - QUICIntUtil::write_uint_as_nbytes(_hash.u64[0], 8, _token, &dummy); - QUICIntUtil::write_uint_as_nbytes(_hash.u64[1], 8, _token + 8, &dummy); + memcpy(data, static_cast(conn_id), data_len); + QUICIntUtil::write_uint_as_nbytes(instance_id, sizeof(instance_id), data + data_len, &ignored); + data_len += sizeof(instance_id); + + ink_release_assert(generate_token_hmac(data, data_len, digest)); + memcpy(_token, digest, sizeof(_token)); + OPENSSL_cleanse(digest, sizeof(digest)); } uint64_t @@ -312,29 +382,19 @@ QUICStatelessResetToken::hex() const QUICResumptionToken::QUICResumptionToken(const IpEndpoint &src, QUICConnectionId cid, ink_hrtime expire_time) { - // TODO: read cookie secret from file like SSLTicketKeyConfig - static constexpr char stateless_retry_token_secret[] = "stateless_cookie_secret"; - size_t dummy; - - uint8_t data[1 + INET6_ADDRPORTSTRLEN + QUICConnectionId::MAX_LENGTH + 4] = {0}; - size_t data_len = 0; - ats_ip_nptop(src, reinterpret_cast(data), sizeof(data)); - data_len = strlen(reinterpret_cast(data)); - - size_t cid_len; - QUICTypeUtil::write_QUICConnectionId(cid, data + data_len, &cid_len); - data_len += cid_len; - - QUICIntUtil::write_uint_as_nbytes(expire_time >> 30, 4, data + data_len, &dummy); - data_len += 4; + size_t ignored = 0; + size_t cid_len = 0; + uint8_t data[INET6_ADDRPORTSTRLEN + QUICConnectionId::MAX_LENGTH + 4] = {0}; + size_t data_len = resumption_token_data(src, cid, expire_time, data, sizeof(data)); this->_token[0] = static_cast(Type::RESUMPTION); - HMAC(EVP_sha1(), stateless_retry_token_secret, sizeof(stateless_retry_token_secret), data, data_len, this->_token + 1, - &this->_token_len); - ink_assert(this->_token_len == 20); - this->_token_len += 1; + uint8_t digest[MAC_LENGTH]; + ink_release_assert(generate_token_hmac(data, data_len, digest)); + memcpy(this->_token + 1, digest, sizeof(digest)); + OPENSSL_cleanse(digest, sizeof(digest)); + this->_token_len = 1 + MAC_LENGTH; - QUICIntUtil::write_uint_as_nbytes(expire_time >> 30, 4, this->_token + this->_token_len, &dummy); + QUICIntUtil::write_uint_as_nbytes(expire_time >> 30, 4, this->_token + this->_token_len, &ignored); this->_token_len += 4; QUICTypeUtil::write_QUICConnectionId(cid, this->_token + this->_token_len, &cid_len); @@ -344,48 +404,49 @@ QUICResumptionToken::QUICResumptionToken(const IpEndpoint &src, QUICConnectionId bool QUICResumptionToken::is_valid(const IpEndpoint &src) const { - QUICResumptionToken x(src, this->cid(), this->expire_time() << 30); - return *this == x && this->expire_time() >= (ink_get_hrtime() >> 30); + if (this->_token_len < 1 + MAC_LENGTH + 4 || this->_token[0] != static_cast(Type::RESUMPTION) || + this->_token_len > 1 + MAC_LENGTH + 4 + QUICConnectionId::MAX_LENGTH) { + return false; + } + + auto token_cid = this->cid(); + auto token_expire_time = this->expire_time(); + uint8_t data[INET6_ADDRPORTSTRLEN + QUICConnectionId::MAX_LENGTH + 4] = {0}; + size_t data_len = resumption_token_data(src, token_cid, token_expire_time << 30, data, sizeof(data)); + return token_expire_time >= (ink_get_hrtime() >> 30) && validate_token_hmac(data, data_len, this->_token + 1); } const QUICConnectionId QUICResumptionToken::cid() const { - // Type uses 1 byte and output of EVP_sha1() should be 160 bits - return QUICTypeUtil::read_QUICConnectionId(this->_token + (1 + 20 + 4), this->_token_len - (1 + 20 + 4)); + constexpr size_t prefix_len = 1 + MAC_LENGTH + 4; + if (this->_token_len < prefix_len || this->_token_len > prefix_len + QUICConnectionId::MAX_LENGTH) { + return QUICConnectionId::ZERO(); + } + return QUICTypeUtil::read_QUICConnectionId(this->_token + prefix_len, this->_token_len - prefix_len); } ink_hrtime QUICResumptionToken::expire_time() const { - return QUICIntUtil::read_nbytes_as_uint(this->_token + (1 + 20), 4); + if (this->_token_len < 1 + MAC_LENGTH + 4) { + return 0; + } + return QUICIntUtil::read_nbytes_as_uint(this->_token + (1 + MAC_LENGTH), 4); } QUICRetryToken::QUICRetryToken(const IpEndpoint &src, QUICConnectionId original_dcid, QUICConnectionId scid) { - // TODO: read cookie secret from file like SSLTicketKeyConfig - static constexpr char stateless_retry_token_secret[] = "stateless_cookie_secret"; - - uint8_t data[1 + INET6_ADDRPORTSTRLEN + QUICConnectionId::MAX_LENGTH] = {0}; - size_t data_len = 0; - ats_ip_nptop(src, reinterpret_cast(data), sizeof(data)); - data_len = strlen(reinterpret_cast(data)); - - size_t cid_len; - *(data + data_len) = original_dcid.length(); - data_len += 1; - QUICTypeUtil::write_QUICConnectionId(original_dcid, data + data_len, &cid_len); - data_len += cid_len; - *(data + data_len) = scid.length(); - data_len += 1; - QUICTypeUtil::write_QUICConnectionId(scid, data + data_len, &cid_len); - data_len += cid_len; + uint8_t data[INET6_ADDRPORTSTRLEN + 2 + 2 * QUICConnectionId::MAX_LENGTH] = {0}; + size_t data_len = retry_token_data(src, original_dcid, scid, data, sizeof(data)); + size_t cid_len = 0; this->_token[0] = static_cast(Type::RETRY); - HMAC(EVP_sha1(), stateless_retry_token_secret, sizeof(stateless_retry_token_secret), data, data_len, this->_token + 1, - &this->_token_len); - ink_assert(this->_token_len == 20); - this->_token_len += 1; + uint8_t digest[MAC_LENGTH]; + ink_release_assert(generate_token_hmac(data, data_len, digest)); + memcpy(this->_token + 1, digest, sizeof(digest)); + OPENSSL_cleanse(digest, sizeof(digest)); + this->_token_len = 1 + MAC_LENGTH; *(this->_token + this->_token_len) = original_dcid.length(); this->_token_len += 1; @@ -400,24 +461,57 @@ QUICRetryToken::QUICRetryToken(const IpEndpoint &src, QUICConnectionId original_ bool QUICRetryToken::is_valid(const IpEndpoint &src) const { - return *this == QUICRetryToken(src, this->original_dcid(), this->scid()); + constexpr size_t fixed_len = 1 + MAC_LENGTH + 2; + if (this->_token_len < fixed_len || this->_token[0] != static_cast(Type::RETRY)) { + return false; + } + + size_t original_dcid_len = this->_token[1 + MAC_LENGTH]; + size_t scid_len_offset = 1 + MAC_LENGTH + 1 + original_dcid_len; + if (original_dcid_len > QUICConnectionId::MAX_LENGTH || scid_len_offset >= this->_token_len) { + return false; + } + + size_t scid_len = this->_token[scid_len_offset]; + if (scid_len > QUICConnectionId::MAX_LENGTH || scid_len_offset + 1 + scid_len != this->_token_len) { + return false; + } + + uint8_t data[INET6_ADDRPORTSTRLEN + 2 + 2 * QUICConnectionId::MAX_LENGTH] = {0}; + size_t data_len = retry_token_data(src, this->original_dcid(), this->scid(), data, sizeof(data)); + return validate_token_hmac(data, data_len, this->_token + 1); } const QUICConnectionId QUICRetryToken::original_dcid() const { - // Type uses 1 byte and output of EVP_sha1() should be 160 bits - auto len = *(this->_token + (1 + 20)); - auto start = this->_token + (1 + 20 + 1); + if (this->_token_len < 1 + MAC_LENGTH + 2) { + return QUICConnectionId::ZERO(); + } + auto len = this->_token[1 + MAC_LENGTH]; + if (len > QUICConnectionId::MAX_LENGTH || 1 + MAC_LENGTH + 1 + len >= this->_token_len) { + return QUICConnectionId::ZERO(); + } + auto start = this->_token + (1 + MAC_LENGTH + 1); return QUICTypeUtil::read_QUICConnectionId(start, len); } const QUICConnectionId QUICRetryToken::scid() const { - auto len = *(this->_token + (1 + 20)); - auto start = this->_token + (1 + 20 + 1 + len + 1); - len = *(this->_token + (1 + 20 + 1 + len)); + if (this->_token_len < 1 + MAC_LENGTH + 2) { + return QUICConnectionId::ZERO(); + } + auto original_dcid_len = this->_token[1 + MAC_LENGTH]; + auto scid_len_offset = 1 + MAC_LENGTH + 1 + original_dcid_len; + if (original_dcid_len > QUICConnectionId::MAX_LENGTH || scid_len_offset >= this->_token_len) { + return QUICConnectionId::ZERO(); + } + auto len = this->_token[scid_len_offset]; + if (len > QUICConnectionId::MAX_LENGTH || scid_len_offset + 1 + len != this->_token_len) { + return QUICConnectionId::ZERO(); + } + auto start = this->_token + scid_len_offset + 1; return QUICTypeUtil::read_QUICConnectionId(start, len); } diff --git a/src/iocore/net/unit_tests/test_QUICTokenKeyConfig.cc b/src/iocore/net/unit_tests/test_QUICTokenKeyConfig.cc new file mode 100644 index 00000000000..4e0a066c2a2 --- /dev/null +++ b/src/iocore/net/unit_tests/test_QUICTokenKeyConfig.cc @@ -0,0 +1,138 @@ +/** @file + + Tests for QUIC token key configuration. + + @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 "iocore/net/quic/QUICConfig.h" +#include "iocore/net/quic/QUICTypes.h" +#include "records/RecCore.h" + +#include + +#include +#include +#include +#include + +namespace +{ +class TokenKeyFile +{ +public: + explicit TokenKeyFile(const std::string &contents) + : _path(std::filesystem::temp_directory_path() / ("ats-quic-token-key-" + std::to_string(getpid()))) + { + write(contents); + } + + ~TokenKeyFile() + { + std::error_code ec; + std::filesystem::remove(_path, ec); + } + + void + write(const std::string &contents) const + { + std::ofstream output(_path, std::ios::binary | std::ios::trunc); + REQUIRE(output.is_open()); + output.write(contents.data(), contents.size()); + REQUIRE(output.good()); + } + + std::string + path() const + { + return _path.string(); + } + +private: + std::filesystem::path _path; +}; +} // namespace + +TEST_CASE("QUIC tokens use reloadable key files", "[quic][security]") +{ + std::string const key_a(QUICTokenKeyConfigParams::KEY_LENGTH, 'A'); + std::string const key_b(QUICTokenKeyConfigParams::KEY_LENGTH, 'B'); + TokenKeyFile key_file(key_a + key_b); + + REQUIRE(RecSetRecordString("proxy.config.quic.server.token_key.filename", key_file.path().c_str(), REC_SOURCE_EXPLICIT) == + REC_ERR_OKAY); + REQUIRE(QUICTokenKeyConfig::reconfigure()); + + IpEndpoint source; + IpEndpoint other_source; + REQUIRE(ats_ip_pton("192.0.2.1:443", &source.sa) == 0); + REQUIRE(ats_ip_pton("192.0.2.2:443", &other_source.sa) == 0); + + uint8_t const original_dcid_data[] = {0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08}; + uint8_t const scid_data[] = {0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18}; + QUICConnectionId original_dcid(original_dcid_data, sizeof(original_dcid_data)); + QUICConnectionId scid(scid_data, sizeof(scid_data)); + ink_hrtime const expire_time = ink_get_hrtime() + HRTIME_SECONDS(60); + + QUICRetryToken retry_with_a(source, original_dcid, scid); + QUICResumptionToken resumption_with_a(source, scid, expire_time); + QUICStatelessResetToken reset_with_a(scid, 1); + CHECK(retry_with_a.is_valid(source)); + CHECK_FALSE(retry_with_a.is_valid(other_source)); + CHECK(resumption_with_a.is_valid(source)); + CHECK_FALSE(resumption_with_a.is_valid(other_source)); + + uint8_t const malformed_data[] = {static_cast(QUICAddressValidationToken::Type::RETRY)}; + QUICRetryToken malformed_retry(malformed_data, sizeof(malformed_data)); + QUICResumptionToken malformed_resumption(malformed_data, sizeof(malformed_data)); + CHECK_FALSE(malformed_retry.is_valid(source)); + CHECK_FALSE(malformed_resumption.is_valid(source)); + + key_file.write(key_b + key_a); + REQUIRE(QUICTokenKeyConfig::reconfigure()); + + QUICRetryToken retry_with_b(source, original_dcid, scid); + QUICResumptionToken resumption_with_b(source, scid, expire_time); + QUICStatelessResetToken reset_with_b(scid, 1); + CHECK(retry_with_a.is_valid(source)); + CHECK(resumption_with_a.is_valid(source)); + CHECK(retry_with_a != retry_with_b); + CHECK(resumption_with_a != resumption_with_b); + CHECK(reset_with_a != reset_with_b); + + key_file.write(key_b); + REQUIRE(QUICTokenKeyConfig::reconfigure()); + CHECK_FALSE(retry_with_a.is_valid(source)); + CHECK_FALSE(resumption_with_a.is_valid(source)); + CHECK(retry_with_b.is_valid(source)); + CHECK(resumption_with_b.is_valid(source)); + + key_file.write(std::string(QUICTokenKeyConfigParams::KEY_LENGTH - 1, 'C')); + CHECK_FALSE(QUICTokenKeyConfig::reconfigure()); + CHECK(retry_with_b.is_valid(source)); + + REQUIRE(RecSetRecordString("proxy.config.quic.server.token_key.filename", "", REC_SOURCE_EXPLICIT) == REC_ERR_OKAY); + REQUIRE(QUICTokenKeyConfig::reconfigure()); + CHECK_FALSE(retry_with_b.is_valid(source)); + + QUICRetryToken random_retry(source, original_dcid, scid); + REQUIRE(QUICTokenKeyConfig::reconfigure()); + CHECK(random_retry.is_valid(source)); + CHECK(random_retry == QUICRetryToken(source, original_dcid, scid)); +} diff --git a/src/iocore/net/unit_tests/test_SSLDHParams.cc b/src/iocore/net/unit_tests/test_SSLDHParams.cc index 6a32a7458ba..3e75f5fe097 100644 --- a/src/iocore/net/unit_tests/test_SSLDHParams.cc +++ b/src/iocore/net/unit_tests/test_SSLDHParams.cc @@ -1,9 +1,14 @@ /** @file - Catch based unit tests for the DH-parameter handling behavior of - SSLMultiCertConfigLoader::init_server_ssl_ctx, which is the inknet - public boundary that transitively invokes ssl_context_enable_dhe - and (when a file is configured) load_dhparams_file. + Catch based unit tests for two pieces of inknet SSL_CTX setup, each + exercised through its public SSLMultiCertConfigLoader boundary: + + * The DH-parameter handling of init_server_ssl_ctx, which transitively + invokes ssl_context_enable_dhe and (when a file is configured) + load_dhparams_file. + + * The private key handling of load_certs, which transitively invokes the + file-static SSLPrivateKeyHandler. @section license License @@ -38,14 +43,25 @@ #include #include #include +#include #include +#include #include +#include #include namespace { +std::string +bio_to_string(BIO *bio) +{ + BUF_MEM *bm = nullptr; + REQUIRE(1 == BIO_get_mem_ptr(bio, &bm)); + return std::string{bm->data, bm->length}; +} + std::string make_valid_dh_pem() { @@ -62,31 +78,77 @@ make_valid_dh_pem() REQUIRE(EVP_PKEY_generate(pctx, &pkey) > 0); BIO *bio = BIO_new(BIO_s_mem()); + REQUIRE(bio != nullptr); REQUIRE(PEM_write_bio_Parameters(bio, pkey) == 1); - BUF_MEM *bm = nullptr; - BIO_get_mem_ptr(bio, &bm); - std::string out{bm->data, bm->length}; + std::string const out{bio_to_string(bio)}; BIO_free(bio); EVP_PKEY_free(pkey); EVP_PKEY_CTX_free(pctx); return out; } +// PEM-encodes pkey as a private key, optionally encrypting it with the given +// cipher and passphrase (cipher==nullptr leaves it unencrypted). +std::string +key_to_pem(EVP_PKEY *pkey, EVP_CIPHER const *cipher, char *pass) +{ + BIO *bio = BIO_new(BIO_s_mem()); + REQUIRE(bio != nullptr); + int passlen{pass ? static_cast(std::strlen(pass)) : 0}; + REQUIRE(PEM_write_bio_PrivateKey(bio, pkey, cipher, reinterpret_cast(pass), passlen, nullptr, nullptr) == 1); + std::string out{bio_to_string(bio)}; + BIO_free(bio); + return out; +} + std::string make_rsa_pem() { EVP_PKEY *pkey = EVP_RSA_gen(2048); REQUIRE(pkey != nullptr); - BIO *bio = BIO_new(BIO_s_mem()); - REQUIRE(PEM_write_bio_PrivateKey(bio, pkey, nullptr, nullptr, 0, nullptr, nullptr) == 1); - BUF_MEM *bm = nullptr; - BIO_get_mem_ptr(bio, &bm); - std::string out{bm->data, bm->length}; - BIO_free(bio); + std::string const out{key_to_pem(pkey, nullptr, nullptr)}; EVP_PKEY_free(pkey); return out; } +// A self-signed certificate paired with the matching 2048-bit RSA private key, +// both PEM-encoded. Each call produces a fresh, independent key pair. When a +// cipher is given the key PEM is encrypted under the passphrase. +struct CertAndKey { + std::string cert_pem; + std::string key_pem; +}; + +CertAndKey +make_cert_and_key(EVP_CIPHER const *cipher = nullptr, char *pass = nullptr) +{ + EVP_PKEY *pkey = EVP_RSA_gen(2048); + REQUIRE(pkey != nullptr); + + X509 *x509 = X509_new(); + REQUIRE(x509 != nullptr); + ASN1_INTEGER_set(X509_get_serialNumber(x509), 1); + X509_gmtime_adj(X509_getm_notBefore(x509), 0); + X509_gmtime_adj(X509_getm_notAfter(x509), 60L * 60L * 24L * 365L); + REQUIRE(X509_set_pubkey(x509, pkey) == 1); + + X509_NAME *name = X509_get_subject_name(x509); + X509_NAME_add_entry_by_txt(name, "CN", MBSTRING_ASC, reinterpret_cast("ats-test"), -1, -1, 0); + REQUIRE(X509_set_issuer_name(x509, name) == 1); + REQUIRE(X509_sign(x509, pkey, EVP_sha256()) > 0); + + BIO *cert_bio = BIO_new(BIO_s_mem()); + REQUIRE(cert_bio != nullptr); + REQUIRE(PEM_write_bio_X509(cert_bio, x509) == 1); + std::string const cert_pem{bio_to_string(cert_bio)}; + BIO_free(cert_bio); + X509_free(x509); + + std::string const key_pem{key_to_pem(pkey, cipher, pass)}; + EVP_PKEY_free(pkey); + return {cert_pem, key_pem}; +} + class TempFile { public: @@ -140,6 +202,55 @@ init_with_dhparams(char const *dhparams_file) return ok; } +// A fixed-passphrase callback, matching how SSLPrivateKeyHandler consults the +// SSL_CTX default password callback to decrypt an encrypted private key. +char test_passphrase[]{"ats-secret-pass"}; + +int +fixed_passphrase_cb(char *buf, int size, int /* rwflag */, void * /* u */) +{ + int len{static_cast(std::strlen(test_passphrase))}; + if (len > size) { + len = size; + } + std::memcpy(buf, test_passphrase, len); + return len; +} + +// Drives SSLPrivateKeyHandler via the public static load_certs boundary, +// holding the certificate fixed and valid so the only variable under test is +// the private key material. The certificate and key are read from real files, +// exactly as a production ssl_multicert entry would be, so that the file-load +// path (load_rsa_pkey_from_file) is genuinely exercised. +// +// An empty key_path selects the "key bundled in the certificate file" branch, +// where the file load is skipped and the key is read from the certificate +// secret. A non-null passwd_cb is installed as the SSL_CTX default password +// callback, exactly as init_server_ssl_ctx's dialog setup would do for an +// encrypted key. +bool +load_key_via_load_certs(char const *cert_path, char const *key_path, pem_password_cb *passwd_cb = nullptr) +{ + SSLConfigParams params; + SSLMultiCertConfigParams settings; + settings.cert = ats_strdup(cert_path); + + SSLMultiCertConfigLoader::CertLoadData data; + data.cert_names_list.emplace_back(cert_path); + data.key_list.emplace_back(key_path); + + SSL_CTX *ctx = SSL_CTX_new(TLS_server_method()); + REQUIRE(ctx != nullptr); + if (passwd_cb != nullptr) { + SSL_CTX_set_default_passwd_cb(ctx, passwd_cb); + } + + bool ok = SSLMultiCertConfigLoader::load_certs(ctx, data.cert_names_list, data.key_list, data, ¶ms, &settings); + + SSL_CTX_free(ctx); + return ok; +} + } // namespace TEST_CASE("ssl_context_enable_dhe: nullptr dhparams file falls back to built-in DH parameters") @@ -184,3 +295,49 @@ TEST_CASE("ssl_context_enable_dhe: truncated DH PEM (missing END marker) is reje TempFile truncated{pem.substr(0, end)}; CHECK_FALSE(init_with_dhparams(truncated.get_path())); } + +TEST_CASE("SSLPrivateKeyHandler: a key file matching the certificate is loaded") +{ + CertAndKey ck = make_cert_and_key(); + TempFile cert{ck.cert_pem}; + TempFile key{ck.key_pem}; + CHECK(load_key_via_load_certs(cert.get_path(), key.get_path())); +} + +TEST_CASE("SSLPrivateKeyHandler: an empty key path loads the key bundled in the certificate file") +{ + CertAndKey ck = make_cert_and_key(); + TempFile cert{ck.cert_pem + ck.key_pem}; + CHECK(load_key_via_load_certs(cert.get_path(), "")); +} + +TEST_CASE("SSLPrivateKeyHandler: a valid key file not matching the certificate is rejected") +{ + TempFile cert{make_cert_and_key().cert_pem}; + TempFile key{make_cert_and_key().key_pem}; + CHECK_FALSE(load_key_via_load_certs(cert.get_path(), key.get_path())); +} + +TEST_CASE("SSLPrivateKeyHandler: an unparseable key file is rejected") +{ + TempFile cert{make_cert_and_key().cert_pem}; + TempFile key{"-----BEGIN PRIVATE KEY-----\nnot base64\n-----END PRIVATE KEY-----\n"}; + CHECK_FALSE(load_key_via_load_certs(cert.get_path(), key.get_path())); +} + +TEST_CASE("SSLPrivateKeyHandler: an encrypted key file is decrypted via the SSL_CTX password callback") +{ + CertAndKey ck = make_cert_and_key(EVP_aes_256_cbc(), test_passphrase); + TempFile cert{ck.cert_pem}; + TempFile key{ck.key_pem}; + CHECK(load_key_via_load_certs(cert.get_path(), key.get_path(), fixed_passphrase_cb)); +} + +TEST_CASE("SSLPrivateKeyHandler: an encrypted key file with the wrong passphrase is rejected") +{ + char wrong_pass[]{"the-wrong-passphrase"}; + CertAndKey ck = make_cert_and_key(EVP_aes_256_cbc(), wrong_pass); + TempFile cert{ck.cert_pem}; + TempFile key{ck.key_pem}; + CHECK_FALSE(load_key_via_load_certs(cert.get_path(), key.get_path(), fixed_passphrase_cb)); +} diff --git a/src/iocore/net/unit_tests/test_SSLSNIConfig.cc b/src/iocore/net/unit_tests/test_SSLSNIConfig.cc index b9aa20f2668..066edfff08b 100644 --- a/src/iocore/net/unit_tests/test_SSLSNIConfig.cc +++ b/src/iocore/net/unit_tests/test_SSLSNIConfig.cc @@ -34,6 +34,7 @@ #include #include +#include #include "tscore/ink_inet.h" TEST_CASE("Test SSLSNIConfig") @@ -266,7 +267,7 @@ TEST_CASE("SNIConfig handles high-bit bytes while normalizing server names") item.inbound_port_ranges.emplace_back(1, ts::MAX_PORT_VALUE); SNIConfigParams params; - params.yaml_sni.items.push_back(item); + params.yaml_sni.items.push_back(std::move(item)); REQUIRE(params.load_sni_config()); std::string servername{"hIGH"}; diff --git a/src/iocore/net/unit_tests/unit_test_main.cc b/src/iocore/net/unit_tests/unit_test_main.cc index 25b355f0b64..41b96438d98 100644 --- a/src/iocore/net/unit_tests/unit_test_main.cc +++ b/src/iocore/net/unit_tests/unit_test_main.cc @@ -23,6 +23,7 @@ #include "iocore/eventsystem/EventSystem.h" #include "../P_SSLConfig.h" +#include "api/LifecycleAPIHooks.h" #include "records/RecordsConfig.h" #include "tscore/BaseLogFile.h" #include "tscore/Diags.h" @@ -55,6 +56,10 @@ class EventProcessorListener final : public Catch::EventListenerBase RecProcessInit(); LibRecordsConfigInit(); + // SSLSecret::loadSecret consults the global lifecycle hooks for the + // SSL_SECRET hook, so they must be allocated before any secret is loaded. + init_global_lifecycle_hooks(); + ink_event_system_init(EVENT_SYSTEM_MODULE_PUBLIC_VERSION); eventProcessor.start(test_threads); diff --git a/src/mgmt/config/ConfigReloadTrace.cc b/src/mgmt/config/ConfigReloadTrace.cc index bf26a6489f8..db322d441d7 100644 --- a/src/mgmt/config/ConfigReloadTrace.cc +++ b/src/mgmt/config/ConfigReloadTrace.cc @@ -27,6 +27,7 @@ #include "tsutil/ts_diag_levels.h" #include +#include #include "tsutil/Metrics.h" #include "tsutil/ts_time_parser.h" @@ -85,7 +86,7 @@ ConfigReloadTask::add_child(std::string_view description) // Read token directly - can't call get_token() as it would deadlock (tries to acquire shared_lock on same mutex) auto trace = std::make_shared(_info.token, description, false, shared_from_this()); _info.sub_tasks.push_back(trace); - return ConfigContext{trace, description}; + return ConfigContext{std::move(trace), description}; } ConfigReloadTask & @@ -469,7 +470,7 @@ ConfigReloadProgress::check_progress(int /* etype */, void * /* data */) } ConfigReloadProgress::ConfigReloadProgress(ConfigReloadTaskPtr reload) - : Continuation(new_ProxyMutex()), _reload{reload}, _every{get_configured_check_interval()} + : Continuation(new_ProxyMutex()), _reload{std::move(reload)}, _every{get_configured_check_interval()} { SET_HANDLER(&ConfigReloadProgress::check_progress); } diff --git a/src/mgmt/config/FileManager.cc b/src/mgmt/config/FileManager.cc index e3df9ea9b4a..744400625d6 100644 --- a/src/mgmt/config/FileManager.cc +++ b/src/mgmt/config/FileManager.cc @@ -252,6 +252,11 @@ FileManager::rereadConfig() ret.note(r); } + if (auto const &r = fileChanged("proxy.config.quic.server.token_key.filename", "proxy.config.quic.server.token_key.filename"); + !r) { + ret.note(r); + } + return ret; } diff --git a/src/mgmt/config/ReloadCoordinator.cc b/src/mgmt/config/ReloadCoordinator.cc index 202a7806af1..6add40d8f00 100644 --- a/src/mgmt/config/ReloadCoordinator.cc +++ b/src/mgmt/config/ReloadCoordinator.cc @@ -126,7 +126,7 @@ ReloadCoordinator::reserve_subtask(std::string_view config_key) auto task = std::make_shared(_current_task->get_token(), config_key, false, _current_task); task->set_config_key(config_key); - _current_task->add_sub_task(task); + _current_task->add_sub_task(std::move(task)); Dbg(dbg_ctl, "Reserved subtask for config '%.*s'", static_cast(config_key.size()), config_key.data()); } @@ -153,7 +153,7 @@ ReloadCoordinator::create_config_context(std::string_view config_key, std::strin if (existing->get_state() == ConfigReloadTask::State::CREATED) { // Activate the reserved subtask Dbg(dbg_ctl, "Activating reserved subtask for config '%.*s'", static_cast(config_key.size()), config_key.data()); - return ConfigContext{existing, description, filename}; + return ConfigContext{std::move(existing), description, filename}; } // Already handled — true duplicate Dbg(dbg_ctl, "Duplicate reload for config '%.*s' — subtask already exists, skipping", static_cast(config_key.size()), @@ -168,7 +168,7 @@ ReloadCoordinator::create_config_context(std::string_view config_key, std::strin task->set_config_key(config_key); _current_task->add_sub_task(task); - ConfigContext ctx{task, description, filename}; + ConfigContext ctx{std::move(task), description, filename}; return ctx; } diff --git a/src/mgmt/rpc/handlers/config/Configuration.cc b/src/mgmt/rpc/handlers/config/Configuration.cc index 9f901bbb62e..d2b498641d0 100644 --- a/src/mgmt/rpc/handlers/config/Configuration.cc +++ b/src/mgmt/rpc/handlers/config/Configuration.cc @@ -20,6 +20,7 @@ #include #include #include +#include #include "records/RecCore.h" #include "../../../../records/P_RecCore.h" @@ -308,7 +309,7 @@ reload_config(std::string_view const & /* id ATS_UNUSED */, YAML::Node const &pa continue; } - valid_configs.push_back({parent_key, key, it->second}); + valid_configs.push_back({std::move(parent_key), std::move(key), it->second}); } // If no valid configs, return early without creating a task diff --git a/src/proxy/ControlMatcher.cc b/src/proxy/ControlMatcher.cc index 4be7cf1880a..d24d539988b 100644 --- a/src/proxy/ControlMatcher.cc +++ b/src/proxy/ControlMatcher.cc @@ -313,7 +313,7 @@ UrlMatcher::NewEntry(matcher_line *line_info) // Fill in the parameter info cur_d = data_array + num_el; error = cur_d->Init(line_info); - if (error.failed()) { + if (!error.failed()) { url_str[num_el] = ats_strdup(pattern); url_value[num_el] = num_el; url_ht.emplace(url_str[num_el], url_value[num_el]); diff --git a/src/proxy/http2/HTTP2.cc b/src/proxy/http2/HTTP2.cc index e8ee95662cf..e0e0d62d099 100644 --- a/src/proxy/http2/HTTP2.cc +++ b/src/proxy/http2/HTTP2.cc @@ -488,8 +488,8 @@ uint32_t Http2::no_activity_timeout_out = 120; float Http2::stream_error_rate_threshold = 0.1; uint32_t Http2::stream_error_sampling_threshold = 10; -int32_t Http2::max_settings_per_frame = 7; -int32_t Http2::max_settings_per_minute = 14; +int32_t Http2::max_settings_per_frame = 16; +int32_t Http2::max_settings_per_minute = 32; int32_t Http2::max_settings_frames_per_minute = 14; int32_t Http2::max_ping_frames_per_minute = 60; int32_t Http2::max_priority_frames_per_minute = 120; diff --git a/src/proxy/logging/Log.cc b/src/proxy/logging/Log.cc index 96b7a56798e..2bcaba9815b 100644 --- a/src/proxy/logging/Log.cc +++ b/src/proxy/logging/Log.cc @@ -277,6 +277,7 @@ struct LoggingPreprocContinuation : public Continuation { mainEvent(int /* event ATS_UNUSED */, void * /* data ATS_UNUSED */) { Log::preproc_thread_main((void *)&m_idx); + delete this; return 0; } @@ -293,6 +294,7 @@ struct LoggingFlushContinuation : public Continuation { mainEvent(int /* event ATS_UNUSED */, void * /* data ATS_UNUSED */) { Log::flush_thread_main((void *)&m_idx); + delete this; return 0; } diff --git a/src/proxy/logging/LogField.cc b/src/proxy/logging/LogField.cc index 29efcc13dd3..e913eed3a92 100644 --- a/src/proxy/logging/LogField.cc +++ b/src/proxy/logging/LogField.cc @@ -311,8 +311,8 @@ LogField::LogField(const char *name, const char *symbol, Type type, CustomMarsha m_time_field(false), m_alias_map(nullptr), m_set_func(nullptr), - m_custom_marshal_func(custom_marshal), - m_custom_unmarshal_func(custom_unmarshal) + m_custom_marshal_func(std::move(custom_marshal)), + m_custom_unmarshal_func(std::move(custom_unmarshal)) { ink_assert(m_name != nullptr); ink_assert(m_symbol != nullptr); diff --git a/src/proxy/logging/LogObject.cc b/src/proxy/logging/LogObject.cc index dd14b763f66..b8744b9f3ab 100644 --- a/src/proxy/logging/LogObject.cc +++ b/src/proxy/logging/LogObject.cc @@ -395,7 +395,9 @@ LogObject::_checkout_write(size_t *write_offset, size_t bytes_needed) int idx = m_buffer_manager_idx++ % m_flush_threads; Dbg(dbg_ctl_log_logbuffer, "adding buffer %d to flush list after checkout", buffer->get_id()); m_buffer_manager[idx].add_to_flush_queue(buffer); - Log::preproc_notify[idx].signal(); + if (Log::preproc_notify != nullptr) { + Log::preproc_notify[idx].signal(); + } buffer = nullptr; } @@ -574,7 +576,9 @@ LogObject::flush_buffer(LogBuffer *buffer) int idx = m_buffer_manager_idx++ % m_flush_threads; Dbg(dbg_ctl_log_logbuffer, "adding buffer %d to flush list after checkout", buffer->get_id()); m_buffer_manager[idx].add_to_flush_queue(buffer); - Log::preproc_notify[idx].signal(); + if (Log::preproc_notify != nullptr) { + Log::preproc_notify[idx].signal(); + } } int diff --git a/src/proxy/unit_tests/CMakeLists.txt b/src/proxy/unit_tests/CMakeLists.txt index 0396792cd2d..be75c50e4cc 100644 --- a/src/proxy/unit_tests/CMakeLists.txt +++ b/src/proxy/unit_tests/CMakeLists.txt @@ -15,7 +15,9 @@ # ####################### -add_executable(test_proxy main.cc test_ControlBase.cc test_FetchSM.cc test_ParentHashConfig.cc stub.cc) +add_executable( + test_proxy main.cc test_ControlBase.cc test_ControlMatcher.cc test_FetchSM.cc test_ParentHashConfig.cc stub.cc +) target_link_libraries(test_proxy PRIVATE Catch2::Catch2WithMain ts::http ts::proxy ts::tscore ts::records ts::inkevent) diff --git a/src/proxy/unit_tests/test_ControlMatcher.cc b/src/proxy/unit_tests/test_ControlMatcher.cc new file mode 100644 index 00000000000..5fa08804349 --- /dev/null +++ b/src/proxy/unit_tests/test_ControlMatcher.cc @@ -0,0 +1,118 @@ +/** @file + + Unit tests for ControlMatcher. + + @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 "proxy/CacheControl.h" +#include "proxy/ControlMatcher.h" +#include "tscore/MatcherUtils.h" +#include "tscore/ink_memory.h" + +#include + +#include + +namespace +{ +class TestRequestData : public HttpRequestData +{ +public: + explicit TestRequestData(std::string_view url) : _url(url) {} + + char * + get_string() override + { + return ats_strndup(_url.data(), _url.size()); + } + +private: + std::string_view _url; +}; + +void +parse_line(char *text, matcher_line &line, int line_number) +{ + REQUIRE(parseConfigLine(text, &line, &http_dest_tags) == nullptr); + line.line_num = line_number; +} +} // namespace + +TEST_CASE("UrlMatcher inserts and matches exact URLs", "[ControlMatcher]") +{ + UrlMatcher matcher{"CacheControl", "cache.config"}; + char config[] = "url=http://example.com/exact action=never-cache"; + matcher_line line; + + matcher.AllocateSpace(1); + parse_line(config, line, 1); + + Result result = matcher.NewEntry(&line); + + REQUIRE_FALSE(result.failed()); + REQUIRE(matcher.num_el == 1); + + TestRequestData exact_request{"http://example.com/exact"}; + CacheControlResult exact_result; + + matcher.Match(&exact_request, &exact_result); + CHECK(exact_result.never_cache); + + TestRequestData other_request{"http://example.com/other"}; + CacheControlResult other_result; + + matcher.Match(&other_request, &other_result); + CHECK_FALSE(other_result.never_cache); +} + +TEST_CASE("UrlMatcher does not insert invalid records", "[ControlMatcher]") +{ + UrlMatcher matcher{"CacheControl", "cache.config"}; + char config[] = "url=http://example.com/exact action=invalid"; + matcher_line line; + + matcher.AllocateSpace(1); + parse_line(config, line, 1); + + Result result = matcher.NewEntry(&line); + + CHECK(result.failed()); + CHECK(matcher.num_el == 0); +} + +TEST_CASE("UrlMatcher rejects duplicate URLs", "[ControlMatcher]") +{ + UrlMatcher matcher{"CacheControl", "cache.config"}; + char first_config[] = "url=http://example.com/exact action=never-cache"; + char second_config[] = "url=http://example.com/exact action=standard-cache"; + matcher_line first_line; + matcher_line second_line; + + matcher.AllocateSpace(2); + parse_line(first_config, first_line, 1); + parse_line(second_config, second_line, 2); + + Result first_result = matcher.NewEntry(&first_line); + Result second_result = matcher.NewEntry(&second_line); + + CHECK_FALSE(first_result.failed()); + CHECK(second_result.failed()); + CHECK(matcher.num_el == 1); +} diff --git a/src/records/RecordsConfig.cc b/src/records/RecordsConfig.cc index 2065d87b09c..b653740edb4 100644 --- a/src/records/RecordsConfig.cc +++ b/src/records/RecordsConfig.cc @@ -1386,9 +1386,9 @@ static constexpr RecordElement RecordsConfig[] = , {RECT_CONFIG, "proxy.config.http2.stream_error_sampling_threshold", RECD_INT, "10", RECU_DYNAMIC, RR_NULL, RECC_STR, "^[0-9]+$", RECA_NULL} , - {RECT_CONFIG, "proxy.config.http2.max_settings_per_frame", RECD_INT, "7", RECU_DYNAMIC, RR_NULL, RECC_STR, "^-?[0-9]+$", RECA_NULL} + {RECT_CONFIG, "proxy.config.http2.max_settings_per_frame", RECD_INT, "16", RECU_DYNAMIC, RR_NULL, RECC_STR, "^-?[0-9]+$", RECA_NULL} , - {RECT_CONFIG, "proxy.config.http2.max_settings_per_minute", RECD_INT, "14", RECU_DYNAMIC, RR_NULL, RECC_STR, "^-?[0-9]+$", RECA_NULL} + {RECT_CONFIG, "proxy.config.http2.max_settings_per_minute", RECD_INT, "32", RECU_DYNAMIC, RR_NULL, RECC_STR, "^-?[0-9]+$", RECA_NULL} , {RECT_CONFIG, "proxy.config.http2.max_settings_frames_per_minute", RECD_INT, "14", RECU_DYNAMIC, RR_NULL, RECC_STR, "^-?[0-9]+$", RECA_NULL} , @@ -1445,6 +1445,8 @@ static constexpr RecordElement RecordsConfig[] = , {RECT_CONFIG, "proxy.config.quic.server.stateless_retry_enabled", RECD_INT, "0", RECU_RESTART_TS, RR_NULL, RECC_INT, "[0-1]", RECA_NULL} , + {RECT_CONFIG, "proxy.config.quic.server.token_key.filename", RECD_STRING, nullptr, RECU_DYNAMIC, RR_NULL, RECC_NULL, nullptr, RECA_NULL} + , {RECT_CONFIG, "proxy.config.quic.client.vn_exercise_enabled", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-1]", RECA_NULL} , {RECT_CONFIG, "proxy.config.quic.client.cm_exercise_enabled", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-1]", RECA_NULL} diff --git a/src/traffic_cache_tool/CacheDefs.h b/src/traffic_cache_tool/CacheDefs.h index 1d9a8c64644..5f77834936b 100644 --- a/src/traffic_cache_tool/CacheDefs.h +++ b/src/traffic_cache_tool/CacheDefs.h @@ -26,6 +26,7 @@ #include #include #include +#include #include "swoc/swoc_file.h" #include "swoc/Scalar.h" @@ -290,7 +291,7 @@ struct url_matcher { while (fileContent) { swoc::TextView line = fileContent.take_prefix_at('\n'); std::string reg_str(line.data(), line.size()); - str_vec.push_back(reg_str); + str_vec.push_back(std::move(reg_str)); count++; } patterns = (const char **)ats_malloc(count * sizeof(char *)); diff --git a/src/traffic_ctl/CtrlCommands.cc b/src/traffic_ctl/CtrlCommands.cc index ddb44957c4f..4cccf22c2d9 100644 --- a/src/traffic_ctl/CtrlCommands.cc +++ b/src/traffic_ctl/CtrlCommands.cc @@ -22,11 +22,12 @@ #include #include +#include #include #include #include +#include #include -#include #include #include @@ -278,7 +279,7 @@ ConfigCommand::config_status() {"error", DL_Error }, }; - std::string lowered{min_level}; + std::string lowered{std::move(min_level)}; std::transform(lowered.begin(), lowered.end(), lowered.begin(), [](unsigned char c) { return static_cast(std::tolower(c)); }); @@ -893,7 +894,7 @@ HostDBCommand::status_get() }; } - HostDBGetStatusRequest request{params}; + HostDBGetStatusRequest request{std::move(params)}; auto response = invoke_rpc(request); diff --git a/src/traffic_ctl/jsonrpc/ctrl_yaml_codecs.h b/src/traffic_ctl/jsonrpc/ctrl_yaml_codecs.h index 40daec816c3..fe1f15754e4 100644 --- a/src/traffic_ctl/jsonrpc/ctrl_yaml_codecs.h +++ b/src/traffic_ctl/jsonrpc/ctrl_yaml_codecs.h @@ -26,6 +26,8 @@ #include "CtrlRPCRequests.h" +#include + // traffic_ctl jsonrpc request/response YAML codec implementation. namespace YAML @@ -107,7 +109,7 @@ template <> struct convert { ConfigReloadResponse::LogEntry entry; entry.level = static_cast(log["level"].as(DL_Undefined)); entry.text = log["text"].as(); - info.logs.push_back(entry); + info.logs.push_back(std::move(entry)); } else { info.logs.push_back({DL_Undefined, log.as()}); } diff --git a/src/traffic_server/traffic_server.cc b/src/traffic_server/traffic_server.cc index 97f04ebcd46..15d7e908e1c 100644 --- a/src/traffic_server/traffic_server.cc +++ b/src/traffic_server/traffic_server.cc @@ -2402,7 +2402,9 @@ main(int /* argc ATS_UNUSED */, const char **argv) SSLConfigParams::load_ssl_file_cb = load_ssl_file_callback; sslNetProcessor.start(-1, stacksize); #if TS_USE_QUIC == 1 - quic_NetProcessor.start(-1, stacksize); + if (HttpProxyPort::hasQUIC()) { + quic_NetProcessor.start(-1, stacksize); + } #endif FileManager::instance().registerConfigPluginCallbacks([&]() { global_config_cbs->invoke(); }); cacheProcessor.afterInitCallbackSet(&CB_After_Cache_Init); diff --git a/src/tscore/CMakeLists.txt b/src/tscore/CMakeLists.txt index d3e70f7e5a5..f951dec2bb4 100644 --- a/src/tscore/CMakeLists.txt +++ b/src/tscore/CMakeLists.txt @@ -100,7 +100,7 @@ set_target_properties(tscore PROPERTIES POSITION_INDEPENDENT_CODE TRUE) if(SSLLIB_IS_BORINGSSL OR SSLLIB_IS_AWSLC) target_sources(tscore PRIVATE HKDF_boringssl.cc) -elseif(SSLLIB_IS_OPENSSL3) +elseif(SSLLIB_IS_AT_LEAST_OPENSSL3) target_sources(tscore PRIVATE HKDF_openssl3.cc) else() target_sources(tscore PRIVATE HKDF_openssl.cc) diff --git a/tests/gold_tests/autest-site/ats_replay.test.ext b/tests/gold_tests/autest-site/ats_replay.test.ext index 85516e92abe..ad338fdacfd 100644 --- a/tests/gold_tests/autest-site/ats_replay.test.ext +++ b/tests/gold_tests/autest-site/ats_replay.test.ext @@ -145,6 +145,13 @@ def configure_ats(obj: 'TestRun', server: 'Process', ats_config: dict, dns: Opti parent_line = parent_line.replace('{SERVER_HTTPS_PORT}', str(server.Variables.https_port)) ts.Disk.parent_config.AddLine(parent_line) + # Configure cache_config if specified. + cache_config = ats_config.get('cache_config', []) + for cache_line in cache_config: + cache_line = cache_line.replace('{SERVER_HTTP_PORT}', str(server.Variables.http_port)) + cache_line = cache_line.replace('{SERVER_HTTPS_PORT}', str(server.Variables.https_port)) + ts.Disk.cache_config.AddLine(cache_line) + # Configure logging.yaml if specified. logging_yaml = ats_config.get('logging_yaml') if logging_yaml != None: diff --git a/tests/gold_tests/cache/cache-exact-url.test.py b/tests/gold_tests/cache/cache-exact-url.test.py new file mode 100644 index 00000000000..42a7434016f --- /dev/null +++ b/tests/gold_tests/cache/cache-exact-url.test.py @@ -0,0 +1,24 @@ +''' +Test exact URL rules in cache.config. +''' +# 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. + +Test.Summary = ''' +Verify that exact URL cache.config rules match only the configured URL. +''' + +Test.ATSReplayTest(replay_file="replay/cache-exact-url.replay.yaml") diff --git a/tests/gold_tests/cache/replay/cache-exact-url.replay.yaml b/tests/gold_tests/cache/replay/cache-exact-url.replay.yaml new file mode 100644 index 00000000000..da0efda6714 --- /dev/null +++ b/tests/gold_tests/cache/replay/cache-exact-url.replay.yaml @@ -0,0 +1,126 @@ +# 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. + +meta: + version: "1.0" + +autest: + description: 'Verify exact URL cache.config matching' + + server: + name: 'server' + + client: + name: 'client' + + ats: + name: 'ts' + process_config: + enable_cache: true + records_config: + proxy.config.diags.debug.enabled: 1 + proxy.config.diags.debug.tags: 'cache_control|matcher' + remap_config: + - from: "http://example.com/" + to: "http://127.0.0.1:{SERVER_HTTP_PORT}/" + cache_config: + - "url=http://127.0.0.1:{SERVER_HTTP_PORT}/exact action=never-cache" + +sessions: +- transactions: + - client-request: + method: GET + url: /exact + version: '1.1' + headers: + fields: + - [Host, example.com] + - [uuid, exact-first] + + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Length, "0"] + - [Cache-Control, "max-age=300"] + + proxy-response: + status: 200 + + - client-request: + delay: 100ms + method: GET + url: /exact + version: '1.1' + headers: + fields: + - [Host, example.com] + - [uuid, exact-second] + + server-response: + status: 201 + reason: Created + headers: + fields: + - [Content-Length, "0"] + - [Cache-Control, "max-age=300"] + + proxy-response: + status: 201 + + - client-request: + method: GET + url: /other + version: '1.1' + headers: + fields: + - [Host, example.com] + - [uuid, other-first] + + server-response: + status: 202 + reason: Accepted + headers: + fields: + - [Content-Length, "0"] + - [Cache-Control, "max-age=300"] + + proxy-response: + status: 202 + + - client-request: + delay: 100ms + method: GET + url: /other + version: '1.1' + headers: + fields: + - [Host, example.com] + - [uuid, other-second] + + proxy-request: + expect: absent + + server-response: + status: 404 + reason: Not Found + headers: + fields: + - [Content-Length, "0"] + + proxy-response: + status: 202 diff --git a/tests/gold_tests/h2/clients/h2_extension_settings.py b/tests/gold_tests/h2/clients/h2_extension_settings.py new file mode 100644 index 00000000000..af8ea58c675 --- /dev/null +++ b/tests/gold_tests/h2/clients/h2_extension_settings.py @@ -0,0 +1,156 @@ +#!/usr/bin/env python3 + +# 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. +"""Send an HTTP/2 request after an extension-rich SETTINGS frame.""" + +import argparse +import socket +import ssl +import sys +from typing import Tuple + +import hpack + +H2_PREFACE = b"PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n" + +TYPE_DATA = 0x00 +TYPE_HEADERS = 0x01 +TYPE_RST_STREAM = 0x03 +TYPE_SETTINGS = 0x04 +TYPE_GOAWAY = 0x07 + +FLAG_ACK = 0x01 +FLAG_END_STREAM = 0x01 +FLAG_END_HEADERS = 0x04 + + +def make_frame(frame_type: int, flags: int = 0, stream_id: int = 0, payload: bytes = b"") -> bytes: + return len(payload).to_bytes(3, "big") + bytes([frame_type, flags]) + (stream_id & 0x7FFFFFFF).to_bytes(4, "big") + payload + + +def make_setting(setting_id: int, value: int) -> bytes: + return setting_id.to_bytes(2, "big") + value.to_bytes(4, "big") + + +def make_socket(port: int) -> ssl.SSLSocket: + socket.setdefaulttimeout(5) + + ctx = ssl.create_default_context() + ctx.check_hostname = False + ctx.verify_mode = ssl.CERT_NONE + ctx.set_alpn_protocols(["h2"]) + + raw_socket = socket.create_connection(("127.0.0.1", port)) + tls_socket = ctx.wrap_socket(raw_socket, server_hostname="localhost") + if tls_socket.selected_alpn_protocol() != "h2": + raise RuntimeError(f"failed to negotiate h2, got {tls_socket.selected_alpn_protocol()!r}") + return tls_socket + + +def recv_exact(sock: ssl.SSLSocket, size: int) -> bytes: + data = bytearray() + while len(data) < size: + chunk = sock.recv(size - len(data)) + if not chunk: + raise EOFError("socket closed") + data.extend(chunk) + return bytes(data) + + +def read_frame(sock: ssl.SSLSocket) -> Tuple[int, int, int, bytes]: + header = recv_exact(sock, 9) + length = int.from_bytes(header[0:3], "big") + frame_type = header[3] + flags = header[4] + stream_id = int.from_bytes(header[5:9], "big") & 0x7FFFFFFF + return frame_type, flags, stream_id, recv_exact(sock, length) + + +def make_settings_payload() -> bytes: + settings = [ + (0x01, 4096), + (0x02, 0), + (0x05, 16384), + (0x06, 131072), + (0x03, 100), + (0x04, 65535), + (0x2B61, 65535), + (0x2B62, 65535), + (0x2B63, 65535), + (0x2B64, 10), + (0x2B65, 10), + ] + return b"".join(make_setting(setting_id, value) for setting_id, value in settings) + + +def run(port: int) -> int: + encoder = hpack.Encoder() + decoder = hpack.Decoder() + request_headers = encoder.encode( + [ + (":method", "GET"), + (":scheme", "https"), + (":authority", "www.example.com"), + (":path", "/"), + ]) + + with make_socket(port) as sock: + sock.sendall(H2_PREFACE) + sock.sendall(make_frame(TYPE_SETTINGS, payload=make_settings_payload())) + sock.sendall(make_frame(TYPE_HEADERS, FLAG_END_HEADERS | FLAG_END_STREAM, 1, request_headers)) + + response_status = None + try: + while True: + frame_type, flags, stream_id, payload = read_frame(sock) + if frame_type == TYPE_SETTINGS and not (flags & FLAG_ACK): + sock.sendall(make_frame(TYPE_SETTINGS, FLAG_ACK)) + continue + if frame_type == TYPE_GOAWAY: + error_code = int.from_bytes(payload[4:8], "big") + print(f"Received GOAWAY with error code {error_code}", file=sys.stderr) + return 1 + if frame_type == TYPE_RST_STREAM and stream_id == 1: + error_code = int.from_bytes(payload[0:4], "big") + print(f"Received RST_STREAM with error code {error_code}", file=sys.stderr) + return 1 + if frame_type == TYPE_HEADERS and stream_id == 1: + if not (flags & FLAG_END_HEADERS): + print("Received an unexpected CONTINUATION sequence", file=sys.stderr) + return 1 + response_headers = decoder.decode(payload) + response_status = dict(response_headers).get(":status") + if stream_id == 1 and frame_type in (TYPE_HEADERS, TYPE_DATA) and (flags & FLAG_END_STREAM): + if response_status == "200": + print("Received 200 response") + return 0 + print(f"Received response status {response_status!r}", file=sys.stderr) + return 1 + except (EOFError, socket.timeout) as exc: + print(f"Connection ended before the response completed: {exc}", file=sys.stderr) + return 1 + + +def main() -> int: + parser = argparse.ArgumentParser() + parser.add_argument("port", type=int, help="ATS TLS port") + args = parser.parse_args() + return run(args.port) + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/gold_tests/h2/http2.test.py b/tests/gold_tests/h2/http2.test.py index bcccc3c1630..6d66e8da129 100644 --- a/tests/gold_tests/h2/http2.test.py +++ b/tests/gold_tests/h2/http2.test.py @@ -152,6 +152,7 @@ ts.Setup.CopyAs('h2client.py', Test.RunDirectory) ts.Setup.CopyAs('h2active_timeout.py', Test.RunDirectory) +ts.Setup.CopyAs('clients/h2_extension_settings.py', Test.RunDirectory) settings_limit_ts = Test.MakeATSProcess("ts_settings_limit", enable_tls=True, enable_cache=False) settings_limit_ts.addDefaultSSLFiles() @@ -263,3 +264,11 @@ tr.Processes.Default.StartBefore(settings_limit_ts) tr.Processes.Default.Streams.stdout += Testers.ContainsExpression( "Received GOAWAY with error code 11", "Received ENHANCE_YOUR_CALM GOAWAY.") + +# Test Case 11: Extension settings fit within the default SETTINGS limits. +tr = Test.AddTestRun("HTTP/2 extension settings") +tr.Processes.Default.Command = f'{sys.executable} h2_extension_settings.py {ts.Variables.ssl_port}' +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.Streams.stdout = Testers.ContainsExpression( + "Received 200 response", "The request following the extension settings should succeed.") +tr.StillRunningAfter = server diff --git a/tests/gold_tests/logging/log_pipe.test.py b/tests/gold_tests/logging/log_pipe.test.py index e550b75b5e3..fbfcc67b6f5 100644 --- a/tests/gold_tests/logging/log_pipe.test.py +++ b/tests/gold_tests/logging/log_pipe.test.py @@ -27,12 +27,16 @@ ts_counter = 1 -def get_ts(logging_config): +def get_ts(logging_config, disable_log_checks=False): """ Create a Traffic Server process. + + :param disable_log_checks: Whether to disable the default diagnostics log + checks on systems where the kernel may reject increasing the pipe + buffer size for an unprivileged Traffic Server process. """ global ts_counter - ts = Test.MakeATSProcess("ts{}".format(ts_counter)) + ts = Test.MakeATSProcess("ts{}".format(ts_counter), disable_log_checks=disable_log_checks) ts_counter += 1 ts.Disk.records_config.update( @@ -122,10 +126,15 @@ def get_ts(logging_config): mode: ascii_pipe format: custom pipe_buffer_size: {} - '''.format(pipe_name, pipe_size).split("\n")) + '''.format(pipe_name, pipe_size).split("\n"), + disable_log_checks=True) pipe_path = os.path.join(ts.Variables.LOGDIR, pipe_name) +ts.Disk.diags_log.Content += Testers.ExcludesExpression( + r"ERROR:(?! Set pipe size failed for pipe .*: Operation not permitted)", "The diagnostics should contain no unexpected errors.") +ts.Disk.diags_log.Content += Testers.ExcludesExpression("FATAL:", "The diagnostics should contain no fatal errors.") + ts.Disk.traffic_out.Content += Testers.ContainsExpression( "Created named pipe .*{}".format(pipe_name), "Verify that the named pipe was created") @@ -147,7 +156,8 @@ def get_ts(logging_config): "New buffer size for pipe.*{}".format(pipe_name), "Verify that the named pipe's size was adjusted") buffer_verifier = "pipe_buffer_is_larger_than.py" tr.Setup.Copy(buffer_verifier) -verify_buffer_size = tr.Processes.Process("verify_buffer_size", f"{sys.executable} {buffer_verifier} {pipe_path} {pipe_size}") +verify_buffer_size = tr.Processes.Process( + "verify_buffer_size", f"{sys.executable} {buffer_verifier} {pipe_path} {pipe_size} {ts.Disk.diags_log.AbsPath}") verify_buffer_size.Return = 0 verify_buffer_size.Streams.All += Testers.ContainsExpression("Success", "The buffer size verifier should report success.") diff --git a/tests/gold_tests/logging/log_plugin_init.test.py b/tests/gold_tests/logging/log_plugin_init.test.py new file mode 100644 index 00000000000..24fa59ac72e --- /dev/null +++ b/tests/gold_tests/logging/log_plugin_init.test.py @@ -0,0 +1,38 @@ +''' +Verify text logging during plugin initialization. +''' +# 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. + +import os + +Test.Summary = ''' +Verify plugins can fill a text log buffer before logging threads start. +''' + +ts = Test.MakeATSProcess('ts') +ts.Disk.records_config.update({'proxy.config.log.log_buffer_size': 9216}) +Test.PrepareTestPlugin(os.path.join(Test.Variables.AtsTestPluginsDir, 'test_log_interface.so'), ts, '--write-during-init') + +plugin_log = Test.Disk.File(os.path.join(ts.Variables.LOGDIR, 'test_log_interface.log'), exists=True) +plugin_log.Content = Testers.ContainsExpression( + 'Writing during plugin initialization', 'The pre-initialization log buffer should be flushed') + +tr = Test.AddTestRun('Start ATS with a plugin that fills a text log buffer during initialization') +tr.Processes.Default.Command = 'printf "traffic_server remained running"' +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.StartBefore(ts) +tr.StillRunningAfter = ts diff --git a/tests/gold_tests/logging/pipe_buffer_is_larger_than.py b/tests/gold_tests/logging/pipe_buffer_is_larger_than.py index 194dbfa1741..00fa7db43d0 100644 --- a/tests/gold_tests/logging/pipe_buffer_is_larger_than.py +++ b/tests/gold_tests/logging/pipe_buffer_is_larger_than.py @@ -19,7 +19,9 @@ import argparse import fcntl +import os import sys +import time F_SETPIPE_SZ = 1031 # Linux 2.6.35+ F_GETPIPE_SZ = 1032 # Linux 2.6.35+ @@ -32,10 +34,12 @@ def parse_args(): parser.add_argument('minimum_buffer_size', help='The minimu buffer size for the pipe to expect.') + parser.add_argument('diags_log', help='The diagnostics log in which a kernel permission error may be reported.') + return parser.parse_args() -def test_fifo(fifo, minimum_buffer_size): +def test_fifo(fifo, minimum_buffer_size, diags_log): try: fifo_fd = open(fifo, "rb+", buffering=0) buffer_size = fcntl.fcntl(fifo_fd, F_GETPIPE_SZ) @@ -43,9 +47,21 @@ def test_fifo(fifo, minimum_buffer_size): if buffer_size >= int(minimum_buffer_size): print("Success. Size is: {} which is larger than: {}".format(buffer_size, minimum_buffer_size)) return 0 - else: - print("Fail. Size is: {} which is smaller than: {}".format(buffer_size, minimum_buffer_size)) - return 1 + + # Diagnostic writes are asynchronous, so briefly wait for the + # explicit permission error before treating an unchanged size as a + # failure. + for _ in range(50): + if os.path.exists(diags_log): + with open(diags_log, encoding='utf-8') as diags: + diagnostics = diags.read() + if "Set pipe size failed" in diagnostics and "Operation not permitted" in diagnostics: + print("Success. The kernel denied increasing the pipe buffer for the unprivileged ATS user.") + return 0 + time.sleep(0.1) + + print("Fail. Size is: {} which is smaller than: {}".format(buffer_size, minimum_buffer_size)) + return 1 except Exception as e: print("Unable to open fifo, error: {}".format(str(e))) return 2 @@ -53,7 +69,7 @@ def test_fifo(fifo, minimum_buffer_size): def main(): args = parse_args() - return test_fifo(args.pipe_name, args.minimum_buffer_size) + return test_fifo(args.pipe_name, args.minimum_buffer_size, args.diags_log) if __name__ == '__main__': diff --git a/tests/gold_tests/pluginTest/healthchecks/healthchecks.test.py b/tests/gold_tests/pluginTest/healthchecks/healthchecks.test.py index bbf1a96f4cf..d4646f2b90a 100644 --- a/tests/gold_tests/pluginTest/healthchecks/healthchecks.test.py +++ b/tests/gold_tests/pluginTest/healthchecks/healthchecks.test.py @@ -44,6 +44,9 @@ def __init__(self) -> None: self._expect_acme_ssl_404() self._re_add_acme_ssl() self._expect_positive_healthchecks() + self._expect_full_buffer_acme_body() + self._rewrite_acme_while_serving() + self._expect_rewritten_acme_body() def _configure_global_ts(self) -> None: '''Configure a global Traffic Server instance for the test runs. @@ -147,6 +150,57 @@ def _re_add_acme_ssl(self) -> None: p.Command = 'sleep 1' p.ReturnCode = 0 + def _rewrite_acme_while_serving(self) -> None: + '''Rewrite the acme file repeatedly while healthcheck requests are in flight. + + The plugin replaces the health check file data underneath transactions which may still be + reading the previous data. This drives that replacement so that an ASan enabled build + catches the old data being released while it is still referenced. + :return: None + ''' + tr = Test.AddTestRun('Rewrite acme while healthchecks are being served') + acme_file = os.path.join(Test.RunDirectory, 'acme') + url = f'http://127.0.0.1:{self._ts.Variables.port}/acme' + + # Note that autest runs the command through string.Template, so shell variables cannot be + # used here. The loop is therefore unrolled. + commands = [] + for iteration in range(10): + commands.append(f'echo "{CONTENT} {iteration}" > {acme_file};') + commands.append('{curl} -s -o /dev/null ' + url + ' &') + commands.append('{curl} -s -o /dev/null ' + url + ' &') + commands.append('wait') + + tr.MakeCurlCommandMulti(' '.join(commands), ts=self._ts) + tr.Processes.Default.ReturnCode = 0 + + def _expect_full_buffer_acme_body(self) -> None: + '''Verify that a MAX_BODY_LEN-sized file is not reported as empty. + :return: None + ''' + tr = Test.AddTestRun('Expect a full-sized healthcheck response body') + acme_file = os.path.join(Test.RunDirectory, 'acme') + url = f'http://127.0.0.1:{self._ts.Variables.port}/acme' + command = (f'dd if=/dev/zero of={acme_file} bs=16384 count=1 2>/dev/null && sleep 1 && ' + '{curl} -s ' + url + ' | wc -c') + tr.MakeCurlCommandMulti(command, ts=self._ts) + p = tr.Processes.Default + p.ReturnCode = 0 + p.Streams.All += Testers.ContainsExpression('16384', 'Verify the response contains 16 KiB') + + def _expect_rewritten_acme_body(self) -> None: + '''Verify that the most recently written acme content is what gets served. + :return: None + ''' + tr = Test.AddTestRun('Expect the last written acme content in the response body') + acme_file = os.path.join(Test.RunDirectory, 'acme') + url = f'http://127.0.0.1:{self._ts.Variables.port}/acme' + command = f'echo "{CONTENT} final" > {acme_file} && sleep 1 && ' + '{curl} -v ' + url + tr.MakeCurlCommandMulti(command, ts=self._ts) + p = tr.Processes.Default + p.ReturnCode = 0 + p.Streams.All += Testers.ContainsExpression('HTTP/1.1 200', 'Verify 200 response for /acme') + p.Streams.All += Testers.ContainsExpression(f'{CONTENT} final', 'Verify the reloaded acme content is served') + # Instantiate the test TestFileChangeBehavior() diff --git a/tests/gold_tests/proxy_protocol/gold/access-cp.gold b/tests/gold_tests/proxy_protocol/gold/access-cp.gold index 28d355ac6af..c2f386d799a 100644 --- a/tests/gold_tests/proxy_protocol/gold/access-cp.gold +++ b/tests/gold_tests/proxy_protocol/gold/access-cp.gold @@ -1,8 +1,8 @@ +127.0.0.1 0 127.0.0.1 +127.0.0.1 0 127.0.0.1 127.0.0.1 127.0.0.1 127.0.0.1 127.0.0.1 127.0.0.1 127.0.0.1 127.0.0.1 127.0.0.1 127.0.0.1 127.0.0.1 127.0.0.1 127.0.0.1 198.51.100.1 198.51.100.1 127.0.0.1 -127.0.0.1 0 127.0.0.1 -127.0.0.1 0 127.0.0.1 198.51.100.1 198.51.100.1 127.0.0.1 diff --git a/tests/gold_tests/proxy_protocol/gold/access-nocp.gold b/tests/gold_tests/proxy_protocol/gold/access-nocp.gold index 5511d71605f..ccd785d680a 100644 --- a/tests/gold_tests/proxy_protocol/gold/access-nocp.gold +++ b/tests/gold_tests/proxy_protocol/gold/access-nocp.gold @@ -1,8 +1,8 @@ +127.0.0.1 0 127.0.0.1 +127.0.0.1 0 127.0.0.1 127.0.0.1 127.0.0.1 127.0.0.1 127.0.0.1 127.0.0.1 127.0.0.1 127.0.0.1 127.0.0.1 127.0.0.1 127.0.0.1 127.0.0.1 127.0.0.1 127.0.0.1 198.51.100.1 127.0.0.1 -127.0.0.1 0 127.0.0.1 -127.0.0.1 0 127.0.0.1 127.0.0.1 198.51.100.1 127.0.0.1 diff --git a/tests/gold_tests/proxy_protocol/proxy_protocol.test.py b/tests/gold_tests/proxy_protocol/proxy_protocol.test.py index d8fe27b0713..d753d11ca69 100644 --- a/tests/gold_tests/proxy_protocol/proxy_protocol.test.py +++ b/tests/gold_tests/proxy_protocol/proxy_protocol.test.py @@ -88,15 +88,22 @@ def checkAccessLog(self): """ check access log """ - Test.Disk.File(os.path.join(self.ts.Variables.LOGDIR, 'access.log'), exists=True, content=f"gold/access-{self.name}.gold") + log_file = os.path.join(self.ts.Variables.LOGDIR, 'access.log') + # Transactions can finish on different event threads, so their access + # log records are not guaranteed to be written in replay order. Test.AddAwaitFileContainsTestRun( f'Await PROXY protocol access log lines. {self.name}', - os.path.join(self.ts.Variables.LOGDIR, 'access.log'), - r'^127\.0\.0\.1 0 127\.0\.0\.1$', - 2, + log_file, + r'^(127\.0\.0\.1|198\.51\.100\.1) (0|127\.0\.0\.1|198\.51\.100\.1) 127\.0\.0\.1$', + 8, ) + tr = Test.AddTestRun(f'Verify PROXY protocol access log lines. {self.name}') + tr.Processes.Default.Command = f'LC_ALL=C sort < "{log_file}"' + tr.Processes.Default.Streams.stdout = f"gold/access-{self.name}.gold" + tr.Processes.Default.ReturnCode = 0 + def run(self): self.runTraffic() self.checkAccessLog() diff --git a/tests/gold_tests/records/gold/full_records.yaml b/tests/gold_tests/records/gold/full_records.yaml index fc02158c5ba..1123e2cbb12 100644 --- a/tests/gold_tests/records/gold/full_records.yaml +++ b/tests/gold_tests/records/gold/full_records.yaml @@ -303,8 +303,8 @@ records: max_ping_frames_per_minute: 60 max_priority_frames_per_minute: 120 max_settings_frames_per_minute: 14 - max_settings_per_frame: 7 - max_settings_per_minute: 14 + max_settings_per_frame: 16 + max_settings_per_minute: 32 min_avg_window_update: 2560.0 min_concurrent_streams_in: 10 no_activity_timeout_in: 120 diff --git a/tests/gold_tests/records/legacy_config/full_records.config b/tests/gold_tests/records/legacy_config/full_records.config index 8b1d13c822b..d7b640dae32 100644 --- a/tests/gold_tests/records/legacy_config/full_records.config +++ b/tests/gold_tests/records/legacy_config/full_records.config @@ -413,8 +413,8 @@ CONFIG proxy.config.http2.push_diary_size INT 256 CONFIG proxy.config.http2.zombie_debug_timeout_in INT 0 CONFIG proxy.config.http2.stream_error_rate_threshold FLOAT 0.1 CONFIG proxy.config.http2.stream_error_sampling_threshold INT 10 -CONFIG proxy.config.http2.max_settings_per_frame INT 7 -CONFIG proxy.config.http2.max_settings_per_minute INT 14 +CONFIG proxy.config.http2.max_settings_per_frame INT 16 +CONFIG proxy.config.http2.max_settings_per_minute INT 32 CONFIG proxy.config.http2.max_settings_frames_per_minute INT 14 CONFIG proxy.config.http2.max_ping_frames_per_minute INT 60 CONFIG proxy.config.http2.max_priority_frames_per_minute INT 120 diff --git a/tests/gold_tests/tls/ssl_multicert_loader.test.py b/tests/gold_tests/tls/ssl_multicert_loader.test.py index c424a92f0fc..1dfce76b652 100644 --- a/tests/gold_tests/tls/ssl_multicert_loader.test.py +++ b/tests/gold_tests/tls/ssl_multicert_loader.test.py @@ -88,6 +88,8 @@ tr3.Processes.Default.ReturnCode = 0 tr3.Processes.Default.Streams.stdout = Testers.ExcludesExpression("Could Not Connect", "Check response") tr3.Processes.Default.Streams.stderr = Testers.IncludesExpression(f"CN={sni_domain}", "Check response") +ts.Disk.diags_log.Content = Testers.ExcludesExpression( + r'\(quic\).*ssl_multicert', 'QUIC certificates should not load without a configured QUIC listener') ########################################################################## # Ensure ATS fails/exits when non-existent cert is specified diff --git a/tests/tools/plugins/test_log_interface.cc b/tests/tools/plugins/test_log_interface.cc index c61d54fbfbf..1378acc678a 100644 --- a/tests/tools/plugins/test_log_interface.cc +++ b/tests/tools/plugins/test_log_interface.cc @@ -73,7 +73,7 @@ global_handler(TSCont /* continuation ATS_UNUSED */, TSEvent event, void *data) } void -TSPluginInit(int /* argc ATS_UNUSED */, const char ** /* argv ATS_UNUSED */) +TSPluginInit(int argc, const char **argv) { TSPluginRegistrationInfo info; @@ -87,5 +87,14 @@ TSPluginInit(int /* argc ATS_UNUSED */, const char ** /* argv ATS_UNUSED */) } TSAssert(TS_SUCCESS == TSTextLogObjectCreate(plugin_name, TS_LOG_MODE_ADD_TIMESTAMP, &pluginlog)); + + if (argc > 1 && strcmp(argv[1], "--write-during-init") == 0) { + const std::string long_line(5000, 'i'); + + for (int i = 0; i < 2; ++i) { + TSAssert(TS_SUCCESS == TSTextLogObjectWrite(pluginlog, "Writing during plugin initialization: %s", long_line.c_str())); + } + } + TSHttpHookAdd(TS_HTTP_READ_REQUEST_HDR_HOOK, TSContCreate(global_handler, nullptr)); }