diff --git a/cmake/proxy-verifier.cmake b/cmake/proxy-verifier.cmake index bb44998b12d..789bef4f6c8 100644 --- a/cmake/proxy-verifier.cmake +++ b/cmake/proxy-verifier.cmake @@ -15,7 +15,7 @@ # ####################### -# This will download and extract proxy-verifier to git common directory and setup variables to point to it. +# This will download and extract proxy-verifier to PV_DEST_DIR and setup variables to point to it. # # Required variables: # PROXY_VERIFIER_VERSION @@ -23,6 +23,7 @@ # # Defines variables: # +# PV_DEST_DIR Directory the archive is downloaded to and extracted in # PROXY_VERIFIER_PATH Full path to the extracted proxy verifier for the build architecture # PROXY_VERIFIER_CLIENT Full path to client-verifier # PROXY_VERIFIER_SERVER Full path to server-verifier @@ -35,22 +36,29 @@ if(NOT PROXY_VERIFIER_HASH) message(FATAL_ERROR "PROXY_VERIFIER_HASH Required") endif() -# GIT_COMMON_DIR is set by the top-level CMakeLists.txt. -if(NOT GIT_COMMON_DIR) - message(FATAL_ERROR "GIT_COMMON_DIR not set. This should be set by the top-level CMakeLists.txt") +# Prefer the git common directory (set by the top-level CMakeLists.txt) so the download is shared by +# every worktree and build directory of the same clone. It isn't always available -- a source tree +# exported without .git, or a worktree whose common directory sits outside the paths visible to the +# build, such as when the worktree alone is mapped into a container. Fall back to the build +# directory, which always exists and is writable, rather than failing the configure. +if(GIT_COMMON_DIR) + set(PV_DEST_DIR "${GIT_COMMON_DIR}") +else() + set(PV_DEST_DIR "${CMAKE_BINARY_DIR}") + message(STATUS "GIT_COMMON_DIR not set, storing proxy-verifier in the build directory instead") endif() # Convert to absolute path (handles relative .git from regular non-worktree clones). -get_filename_component(GIT_COMMON_DIR "${GIT_COMMON_DIR}" ABSOLUTE BASE_DIR "${CMAKE_SOURCE_DIR}") +get_filename_component(PV_DEST_DIR "${PV_DEST_DIR}" ABSOLUTE BASE_DIR "${CMAKE_SOURCE_DIR}") -# Download proxy-verifier to git common directory. -set(PV_ARCHIVE ${GIT_COMMON_DIR}/proxy-verifier/proxy-verifier.tar.gz) +# Download proxy-verifier to the destination directory. +set(PV_ARCHIVE ${PV_DEST_DIR}/proxy-verifier/proxy-verifier.tar.gz) file( DOWNLOAD https://ci.trafficserver.apache.org/bintray/proxy-verifier-${PROXY_VERIFIER_VERSION}.tar.gz ${PV_ARCHIVE} EXPECTED_HASH ${PROXY_VERIFIER_HASH} SHOW_PROGRESS ) -file(ARCHIVE_EXTRACT INPUT ${PV_ARCHIVE} DESTINATION ${GIT_COMMON_DIR}) +file(ARCHIVE_EXTRACT INPUT ${PV_ARCHIVE} DESTINATION ${PV_DEST_DIR}) if(CMAKE_HOST_SYSTEM_NAME STREQUAL "Linux") if(CMAKE_HOST_SYSTEM_PROCESSOR STREQUAL "x86_64" @@ -78,7 +86,7 @@ else() message(FATAL_ERROR "Host ${CMAKE_HOST_SYSTEM_NAME} doesnt support running proxy verifier") endif() -set(PROXY_VERIFIER_PATH ${GIT_COMMON_DIR}/proxy-verifier-${PROXY_VERIFIER_VERSION}/${PV_SUBDIR}) +set(PROXY_VERIFIER_PATH ${PV_DEST_DIR}/proxy-verifier-${PROXY_VERIFIER_VERSION}/${PV_SUBDIR}) set(PROXY_VERIFIER_CLIENT ${PROXY_VERIFIER_PATH}/verifier-client) set(PROXY_VERIFIER_SERVER ${PROXY_VERIFIER_PATH}/verifier-server) diff --git a/doc/admin-guide/files/records.yaml.en.rst b/doc/admin-guide/files/records.yaml.en.rst index 3bf27624ef2..4934c8aaadb 100644 --- a/doc/admin-guide/files/records.yaml.en.rst +++ b/doc/admin-guide/files/records.yaml.en.rst @@ -1532,6 +1532,10 @@ Parent Proxy Configuration The amount of time allowed between connection retries to a parent cache that is unavailable. + Once this time has elapsed the parent is selected again as a retry candidate. It is + restored to the pool only if that retry actually succeeds; if the retry fails, the parent + remains unavailable and a further ``retry_time`` must elapse before it is tried again. + .. ts:cv:: CONFIG proxy.config.http.parent_proxy.max_trans_retries INT 2 Limits the number of simultaneous transactions that may retry a parent once the parents diff --git a/example/plugins/c-api/client_context_dump/client_context_dump.cc b/example/plugins/c-api/client_context_dump/client_context_dump.cc index f8572d775ce..6dcb94919bb 100644 --- a/example/plugins/c-api/client_context_dump/client_context_dump.cc +++ b/example/plugins/c-api/client_context_dump/client_context_dump.cc @@ -65,7 +65,7 @@ dump_context(const char *ca_path, const char *ck_path) // expiration date, serial number, common name, and subject alternative names const ASN1_TIME *not_after = X509_get_notAfter(cert); const ASN1_INTEGER *serial = X509_get_serialNumber(cert); - X509_NAME *subject_name = X509_get_subject_name(cert); + auto *subject_name = X509_get_subject_name(cert); // Subject name BIO *subject_bio = BIO_new(BIO_s_mem()); diff --git a/example/plugins/c-api/verify_cert/verify_cert.cc b/example/plugins/c-api/verify_cert/verify_cert.cc index a3f553057fe..4c3c75446ee 100644 --- a/example/plugins/c-api/verify_cert/verify_cert.cc +++ b/example/plugins/c-api/verify_cert/verify_cert.cc @@ -37,7 +37,7 @@ namespace DbgCtl dbg_ctl{PLUGIN_NAME}; static void -debug_certificate(const char *msg, X509_NAME *name) +debug_certificate(const char *msg, const X509_NAME *name) { BIO *bio; diff --git a/include/cripts/Certs.hpp b/include/cripts/Certs.hpp index f19fb8f41b0..9b402a070e9 100644 --- a/include/cripts/Certs.hpp +++ b/include/cripts/Certs.hpp @@ -107,10 +107,15 @@ class CertBase } } - void _load_name(X509_NAME *(*getter)(const X509 *)) const; - void _load_integer(ASN1_INTEGER *(*getter)(X509 *)) const; - void _load_long(long (*getter)(const X509 *)) const; - void _load_time(ASN1_TIME *(*getter)(const X509 *)) const; + using NameGetter = decltype(&X509_get_subject_name); + using IntegerGetter = decltype(&X509_get_serialNumber); + using LongGetter = decltype(&X509_get_version); + using TimeGetter = decltype(&X509_get_notBefore); + + void _load_name(NameGetter getter) const; + void _load_integer(IntegerGetter getter) const; + void _load_long(LongGetter getter) const; + void _load_time(TimeGetter getter) const; CertBase *_owner = nullptr; mutable std::unique_ptr _bio{nullptr, BIO_free}; diff --git a/plugins/certifier/certifier.cc b/plugins/certifier/certifier.cc index 563a39bd40f..94a490c2fb4 100644 --- a/plugins/certifier/certifier.cc +++ b/plugins/certifier/certifier.cc @@ -88,10 +88,11 @@ template <> struct default_delete { } // namespace std /// Name aliases for unique pts to openSSL objects -using scoped_X509 = std::unique_ptr; -using scoped_X509_REQ = std::unique_ptr; -using scoped_EVP_PKEY = std::unique_ptr; -using scoped_SSL_CTX = std::unique_ptr; +using scoped_X509 = std::unique_ptr; +using scoped_X509_REQ = std::unique_ptr; +using scoped_EVP_PKEY = std::unique_ptr; +using scoped_SSL_CTX = std::unique_ptr; +using scoped_X509_NAME = std::unique_ptr; class SslLRUList { @@ -401,12 +402,20 @@ mkcrt(const std::string &commonName, int serial) X509_gmtime_adj(X509_get_notAfter(cert.get()), static_cast(3650) * 24 * 3600); // Get handle to subject name - X509_NAME *n = X509_get_subject_name(cert.get()); + scoped_X509_NAME n{X509_NAME_dup(X509_get_subject_name(cert.get())), X509_NAME_free}; + if (n == nullptr) { + TSError("[%s] %s: failed to duplicate certificate subject", PLUGIN_NAME, __func__); + return nullptr; + } // Set common name field - if (X509_NAME_add_entry_by_txt(n, "CN", MBSTRING_ASC, (unsigned char *)commonName.c_str(), -1, -1, 0) != 1) { + if (X509_NAME_add_entry_by_txt(n.get(), "CN", MBSTRING_ASC, (unsigned char *)commonName.c_str(), -1, -1, 0) != 1) { TSError("[%s] %s: failed to add certificate subject CN", PLUGIN_NAME, __func__); return nullptr; } + if (X509_set_subject_name(cert.get(), n.get()) != 1) { + TSError("[%s] %s: failed to set certificate subject", PLUGIN_NAME, __func__); + return nullptr; + } // Set Traffic Server public key if (X509_set_pubkey(cert.get(), ca_pkey_scoped.get()) == 0) { diff --git a/plugins/experimental/cert_reporting_tool/cert_reporting_tool.cc b/plugins/experimental/cert_reporting_tool/cert_reporting_tool.cc index fd1d49f1486..3613508890a 100644 --- a/plugins/experimental/cert_reporting_tool/cert_reporting_tool.cc +++ b/plugins/experimental/cert_reporting_tool/cert_reporting_tool.cc @@ -65,7 +65,7 @@ dump_context(const char *ca_path, const char *ck_path) // expiration date, serial number, common name, and subject alternative names const ASN1_TIME *not_after = X509_get_notAfter(cert); const ASN1_INTEGER *serial = X509_get_serialNumber(cert); - X509_NAME *subject_name = X509_get_subject_name(cert); + const X509_NAME *subject_name = X509_get_subject_name(cert); // Subject name BIO *subject_bio = BIO_new(BIO_s_mem()); diff --git a/plugins/experimental/sslheaders/expand.cc b/plugins/experimental/sslheaders/expand.cc index f6a8a0c7548..cbba6594240 100644 --- a/plugins/experimental/sslheaders/expand.cc +++ b/plugins/experimental/sslheaders/expand.cc @@ -49,14 +49,14 @@ x509_expand_certificate(X509 *x509, BIO *bio) static void x509_expand_subject(X509 *x509, BIO *bio) { - X509_NAME *name = X509_get_subject_name(x509); + const X509_NAME *name = X509_get_subject_name(x509); X509_NAME_print_ex(bio, name, 0 /* indent */, XN_FLAG_ONELINE); } static void x509_expand_issuer(X509 *x509, BIO *bio) { - X509_NAME *name = X509_get_issuer_name(x509); + const X509_NAME *name = X509_get_issuer_name(x509); X509_NAME_print_ex(bio, name, 0 /* indent */, XN_FLAG_ONELINE); } @@ -72,8 +72,8 @@ x509_expand_signature(X509 *x509, BIO *bio) { const ASN1_BIT_STRING *sig; X509_get0_signature(&sig, nullptr, x509); - const char *ptr = reinterpret_cast(sig->data); - const char *end = ptr + sig->length; + const char *ptr = reinterpret_cast(ASN1_STRING_get0_data(sig)); + const char *end = ptr + ASN1_STRING_length(sig); // The canonical OpenSSL way to format the signature seems to be // X509_signature_dump(). However that separates each byte with a ':', which is diff --git a/plugins/experimental/txn_box/plugin/src/ts_util.cc b/plugins/experimental/txn_box/plugin/src/ts_util.cc index a1881768956..c63de2f2fd7 100644 --- a/plugins/experimental/txn_box/plugin/src/ts_util.cc +++ b/plugins/experimental/txn_box/plugin/src/ts_util.cc @@ -1111,8 +1111,10 @@ ssl_nid(swoc::TextView const &name) namespace { + using X509_NAME_ptr = decltype(X509_get_subject_name(nullptr)); + TextView - ssl_value_for(X509_NAME *name, int nid) + ssl_value_for(X509_NAME_ptr name, int nid) { if (int loc = X509_NAME_get_index_by_NID(name, nid, -1); loc >= 0) { if (auto entry = X509_NAME_get_entry(name, loc); entry != nullptr) { diff --git a/plugins/header_rewrite/header_rewrite.cc b/plugins/header_rewrite/header_rewrite.cc index b8c4f3c6dbf..80f720a935b 100644 --- a/plugins/header_rewrite/header_rewrite.cc +++ b/plugins/header_rewrite/header_rewrite.cc @@ -202,12 +202,12 @@ validate_rule_completion(RuleSet *rule, const std::string &fname, int lineno) bool RulesConfig::parse_config(const std::string &fname, TSHttpHookID default_hook, char *from_url, char *to_url) { - std::unique_ptr rule(nullptr); - std::string filename; - int lineno = 0; - ConditionGroup *group = nullptr; - std::stack group_stack; - std::stack if_stack; + std::unique_ptr rule(nullptr); + std::string filename; + int lineno = 0; + ConditionGroup *group = nullptr; + std::stack group_stack; + std::stack> if_stack; constexpr int MAX_IF_NESTING_DEPTH = 10; @@ -366,10 +366,8 @@ RulesConfig::parse_config(const std::string &fname, TSHttpHookID default_hook, c throw std::runtime_error("maximum if nesting depth exceeded"); } - auto *op_if = new OperatorIf(); - - if_stack.push(op_if); - group = op_if->get_group(); // Set group to the new OperatorIf's group + if_stack.push(std::make_unique()); + group = if_stack.top()->get_group(); // Set group to the new OperatorIf's group Dbg(dbg_ctl, "Started nested OperatorIf, depth: %zu", if_stack.size()); } else if (p.is_endif()) { @@ -377,21 +375,20 @@ RulesConfig::parse_config(const std::string &fname, TSHttpHookID default_hook, c throw std::runtime_error("endif without matching if"); } - OperatorIf *op_if = if_stack.top(); + auto op_if = std::move(if_stack.top()); if_stack.pop(); if (!if_stack.empty()) { auto *parent_sec = if_stack.top()->cur_section(); if (parent_sec->ops.oper) { - parent_sec->ops.oper->append(op_if); + parent_sec->ops.oper->append(op_if.release()); } else { - parent_sec->ops.oper.reset(op_if); + parent_sec->ops.oper = std::move(op_if); } group = if_stack.top()->get_group(); } else { - if (!rule->add_operator(op_if)) { - delete op_if; + if (!rule->add_operator(std::move(op_if))) { throw std::runtime_error("Failed to add nested OperatorIf to RuleSet"); } group = rule->get_group(); @@ -434,10 +431,6 @@ RulesConfig::parse_config(const std::string &fname, TSHttpHookID default_hook, c // Check for unmatched if statements if (!if_stack.empty()) { TSError("[%s] %zu unmatched 'if' statement(s) without 'endif' in file: %s", PLUGIN_NAME, if_stack.size(), fname.c_str()); - while (!if_stack.empty()) { - delete if_stack.top(); - if_stack.pop(); - } return false; } diff --git a/plugins/header_rewrite/operators.cc b/plugins/header_rewrite/operators.cc index 8013990285c..5d7bbab3860 100644 --- a/plugins/header_rewrite/operators.cc +++ b/plugins/header_rewrite/operators.cc @@ -23,6 +23,7 @@ #include #include #include +#include #include "records/RecCore.h" #include "ts/ts.h" @@ -1272,8 +1273,7 @@ OperatorRunPlugin::initialize(Parser &p) auto plugin_args = p.get_value(); if (plugin_name.empty()) { - TSError("[%s] missing plugin name", PLUGIN_NAME); - return; + throw std::runtime_error("run-plugin missing plugin name"); } std::vector tokens; @@ -1284,15 +1284,10 @@ OperatorRunPlugin::initialize(Parser &p) tokens.push_back(token); } - // Create argc and argv - int argc = tokens.size() + 2; - char **argv = new char *[argc]; - - argv[0] = p.from_url(); - argv[1] = p.to_url(); + std::vector argv{p.from_url(), p.to_url()}; - for (size_t i = 0; i < tokens.size(); ++i) { - argv[i + 2] = const_cast(tokens[i].c_str()); + for (auto const &argument : tokens) { + argv.push_back(const_cast(argument.c_str())); } std::string error; @@ -1304,14 +1299,12 @@ OperatorRunPlugin::initialize(Parser &p) elevate_access = RecGetRecordInt("proxy.config.plugin.load_elevated").value_or(0); ElevateAccess access(elevate_access ? ElevateAccess::FILE_PRIVILEGE : 0); - _plugin = plugin_factory.getRemapPlugin(swoc::file::path(plugin_name), argc, const_cast(argv), error, + _plugin = plugin_factory.getRemapPlugin(swoc::file::path(plugin_name), static_cast(argv.size()), argv.data(), error, isPluginDynamicReloadEnabled()); } // done elevating access - delete[] argv; - if (!_plugin) { - TSError("[%s] Unable to load plugin '%s': %s", PLUGIN_NAME, plugin_name.c_str(), error.c_str()); + throw std::runtime_error("run-plugin unable to load plugin '" + std::string{plugin_name} + "': " + error); } } @@ -1326,7 +1319,11 @@ OperatorRunPlugin::initialize_hooks() bool OperatorRunPlugin::exec(const Resources &res) const { - TSReleaseAssert(_plugin != nullptr); + // Rejected at config load (see initialize); guard anyway so a stray bad rule can't abort the server. + if (!_plugin) { + Dbg(pi_dbg_ctl, "OperatorRunPlugin::exec skipped, plugin was not loaded"); + return true; + } if (res._rri && res.state.txnp) { _plugin->doRemap(res.state.txnp, res._rri); @@ -1654,7 +1651,7 @@ OperatorIf::new_section(Parser::CondClause clause) bool OperatorIf::add_operator(Parser &p, const char *filename, int lineno) { - Operator *op = operator_factory(p.get_op()); + std::unique_ptr op{operator_factory(p.get_op())}; if (!op) { TSError("[%s] Unknown operator: %s, file: %s, line: %d", PLUGIN_NAME, p.get_op().c_str(), filename, lineno); @@ -1667,7 +1664,6 @@ OperatorIf::add_operator(Parser &p, const char *filename, int lineno) try { op->initialize(p); } catch (std::exception const &ex) { - delete op; TSError("[%s] Failed to initialize operator: %s, file: %s, line: %d, error: %s", PLUGIN_NAME, p.get_op().c_str(), filename, lineno, ex.what()); return false; @@ -1675,10 +1671,10 @@ OperatorIf::add_operator(Parser &p, const char *filename, int lineno) // Add to current section if (_cur_section->ops.oper) { - _cur_section->ops.oper->append(op); + _cur_section->ops.oper->append(op.release()); } else { - _cur_section->ops.oper.reset(op); - _cur_section->ops.oper_mods = op->get_oper_modifiers(); + _cur_section->ops.oper = std::move(op); + _cur_section->ops.oper_mods = _cur_section->ops.oper->get_oper_modifiers(); } return true; diff --git a/plugins/header_rewrite/ruleset.cc b/plugins/header_rewrite/ruleset.cc index acbeafc72f4..eae2ff0af15 100644 --- a/plugins/header_rewrite/ruleset.cc +++ b/plugins/header_rewrite/ruleset.cc @@ -19,6 +19,7 @@ // ruleset.cc: implementation of the ruleset class // // +#include #include #include "ruleset.h" @@ -89,14 +90,20 @@ RuleSet::make_condition(Parser &p, const char *filename, int lineno) bool RuleSet::add_operator(Parser &p, const char *filename, int lineno) { - Operator *op = operator_factory(p.get_op()); + std::unique_ptr op{operator_factory(p.get_op())}; - if (nullptr != op) { + if (op) { Dbg(pi_dbg_ctl, " Adding operator: %s(%s)=\"%s\"", p.get_op().c_str(), p.get_arg().c_str(), p.get_value().c_str()); op->set_config_location(filename, lineno); - op->initialize(p); + + try { + op->initialize(p); + } catch (std::exception const &ex) { + TSError("[%s] in %s:%d: failed to initialize operator %s: %s", PLUGIN_NAME, filename, lineno, p.get_op().c_str(), ex.what()); + return false; + } + if (!op->is_hook_valid(_hook)) { - delete op; Dbg(pi_dbg_ctl, "in %s:%d: can't use this operator in hook=%s: %s(%s)", filename, lineno, TSHttpHookNameLookup(_hook), p.get_op().c_str(), p.get_arg().c_str()); TSError("[%s] in %s:%d: can't use this operator in hook=%s: %s(%s)", PLUGIN_NAME, filename, lineno, @@ -107,9 +114,9 @@ RuleSet::add_operator(Parser &p, const char *filename, int lineno) auto *cur_sec = _op_if.cur_section(); if (!cur_sec->ops.oper) { - cur_sec->ops.oper.reset(op); + cur_sec->ops.oper = std::move(op); } else { - cur_sec->ops.oper->append(op); + cur_sec->ops.oper->append(op.release()); } cur_sec->ops.oper_mods = static_cast(cur_sec->ops.oper_mods | cur_sec->ops.oper->get_oper_modifiers()); @@ -136,14 +143,14 @@ RuleSet::get_all_resource_ids() const } bool -RuleSet::add_operator(Operator *op) +RuleSet::add_operator(std::unique_ptr op) { auto *cur_sec = _op_if.cur_section(); if (!cur_sec->ops.oper) { - cur_sec->ops.oper.reset(op); + cur_sec->ops.oper = std::move(op); } else { - cur_sec->ops.oper->append(op); + cur_sec->ops.oper->append(op.release()); } // Update some ruleset state based on this new operator diff --git a/plugins/header_rewrite/ruleset.h b/plugins/header_rewrite/ruleset.h index 78680bc9d7e..0ad40f7183a 100644 --- a/plugins/header_rewrite/ruleset.h +++ b/plugins/header_rewrite/ruleset.h @@ -49,7 +49,7 @@ class RuleSet Condition *make_condition(Parser &p, const char *filename, int lineno); ResourceIDs get_all_resource_ids() const; bool add_operator(Parser &p, const char *filename, int lineno); - bool add_operator(Operator *op); + bool add_operator(std::unique_ptr op); ConditionGroup * get_group() diff --git a/plugins/lua/ts_lua_client_cert_helpers.h b/plugins/lua/ts_lua_client_cert_helpers.h index c7f20b50ebc..a7098ed2c0a 100644 --- a/plugins/lua/ts_lua_client_cert_helpers.h +++ b/plugins/lua/ts_lua_client_cert_helpers.h @@ -18,7 +18,7 @@ // Helper functions for certificate data extraction static std::string -get_x509_name_string(X509_NAME *name) +get_x509_name_string(const X509_NAME *name) { if (!name) { return ""; @@ -157,12 +157,15 @@ get_x509_signature_string(X509 *cert) return ""; } - for (int i = 0; i < sig->length; i++) { - if (BIO_printf(bio, "%02x", sig->data[i]) <= 0) { + const unsigned char *sig_data = ASN1_STRING_get0_data(sig); + int sig_len = ASN1_STRING_length(sig); + + for (int i = 0; i < sig_len; i++) { + if (BIO_printf(bio, "%02x", sig_data[i]) <= 0) { BIO_free(bio); return ""; } - if (i < sig->length - 1) { + if (i < sig_len - 1) { if (BIO_printf(bio, ":") <= 0) { BIO_free(bio); return ""; diff --git a/src/api/InkAPI.cc b/src/api/InkAPI.cc index 05fc77e2558..1d4d3c9d32f 100644 --- a/src/api/InkAPI.cc +++ b/src/api/InkAPI.cc @@ -8319,9 +8319,9 @@ TSSslServerCertUpdate(const char *cert_path, const char *key_path) } // Extract common name - int pos = X509_NAME_get_index_by_NID(X509_get_subject_name(cert.get()), NID_commonName, -1); - X509_NAME_ENTRY *common_name = X509_NAME_get_entry(X509_get_subject_name(cert.get()), pos); - ASN1_STRING *common_name_asn1 = X509_NAME_ENTRY_get_data(common_name); + const int pos = X509_NAME_get_index_by_NID(X509_get_subject_name(cert.get()), NID_commonName, -1); + const X509_NAME_ENTRY *common_name = X509_NAME_get_entry(X509_get_subject_name(cert.get()), pos); + const ASN1_STRING *common_name_asn1 = X509_NAME_ENTRY_get_data(common_name); char *common_name_str = reinterpret_cast(const_cast(ASN1_STRING_get0_data(common_name_asn1))); if (ASN1_STRING_length(common_name_asn1) != static_cast(strlen(common_name_str))) { // Embedded null char diff --git a/src/cripts/Certs.cc b/src/cripts/Certs.cc index 8f893c14dc2..006effcb90c 100644 --- a/src/cripts/Certs.cc +++ b/src/cripts/Certs.cc @@ -54,8 +54,8 @@ CertBase::Signature::_load() const if (!_ready && _owner->_x509) { const ASN1_BIT_STRING *sig; X509_get0_signature(&sig, nullptr, _owner->_x509); - const char *ptr = reinterpret_cast(sig->data); - const char *end = ptr + sig->length; + const char *ptr = reinterpret_cast(ASN1_STRING_get0_data(sig)); + const char *end = ptr + ASN1_STRING_length(sig); super_type::_load(); for (; ptr < end; ++ptr) { @@ -78,7 +78,7 @@ CertBase::X509Value::_update_value() const } void -CertBase::X509Value::_load_name(X509_NAME *(*getter)(const X509 *)) const +CertBase::X509Value::_load_name(NameGetter getter) const { if (!_ready && _owner->_x509) { auto *name = getter(_owner->_x509); @@ -95,7 +95,7 @@ CertBase::X509Value::_load_name(X509_NAME *(*getter)(const X509 *)) const } void -CertBase::X509Value::_load_integer(ASN1_INTEGER *(*getter)(X509 *)) const +CertBase::X509Value::_load_integer(IntegerGetter getter) const { if (!_ready && _owner->_x509) { auto *value = getter(_owner->_x509); @@ -107,7 +107,7 @@ CertBase::X509Value::_load_integer(ASN1_INTEGER *(*getter)(X509 *)) const } void -CertBase::X509Value::_load_long(long (*getter)(const X509 *)) const +CertBase::X509Value::_load_long(LongGetter getter) const { if (!_ready && _owner->_x509) { auto value = getter(_owner->_x509); @@ -119,7 +119,7 @@ CertBase::X509Value::_load_long(long (*getter)(const X509 *)) const } void -CertBase::X509Value::_load_time(ASN1_TIME *(*getter)(const X509 *)) const +CertBase::X509Value::_load_time(TimeGetter getter) const { if (!_ready && _owner->_x509) { auto *time = getter(_owner->_x509); @@ -153,8 +153,8 @@ namespace _write_ip_address(const ASN1_OCTET_STRING *ip, BIO *_bio) { char buffer[INET6_ADDRSTRLEN]; - const unsigned char *raw = ip->data; - int len = ip->length; + const unsigned char *raw = ASN1_STRING_get0_data(ip); + int len = ASN1_STRING_length(ip); if (inet_ntop(len == 4 ? AF_INET : AF_INET6, raw, buffer, sizeof(buffer))) { BIO_printf(_bio, "%s", buffer); diff --git a/src/iocore/net/OCSPStapling.cc b/src/iocore/net/OCSPStapling.cc index e1c2e4ca3b5..715e68d2f90 100644 --- a/src/iocore/net/OCSPStapling.cc +++ b/src/iocore/net/OCSPStapling.cc @@ -22,6 +22,7 @@ #include "P_OCSPStapling.h" #include +#include #include #include @@ -38,6 +39,7 @@ #include "P_SSLUtils.h" #include "SSLStats.h" #include "proxy/FetchSM.h" +#include "tsutil/Bravo.h" // Macros for ASN1 and the code in TS_OCSP_* functions were borrowed from OpenSSL 3.1.0 (a92271e03a8d0dee507b6f1e7f49512568b2c7ad), // and were modified to make them compilable with BoringSSL and C++ compiler. @@ -282,18 +284,21 @@ namespace // Cached info stored in SSL_CTX ex_info struct certinfo { unsigned char idx[20] = {}; // Index in session cache SHA1 hash of certificate - TS_OCSP_CERTID *cid = nullptr; // Certificate ID for OCSP requests + TS_OCSP_CERTID *cid = nullptr; // Certificate ID for OCSP requests or nullptr if ID cannot be determined char *uri = nullptr; // Responder details char *certname = nullptr; char *user_agent = nullptr; - ink_mutex stapling_mutex; - unsigned char resp_der[MAX_STAPLING_DER] = {}; - unsigned int resp_derlen = 0; - bool is_prefetched = false; - bool is_expire = true; - time_t expire_time = 0; - - certinfo() { ink_mutex_init(&stapling_mutex); } + const bool is_prefetched; + + // OCSP response data, protected by resp_mutex. + // Readers take a shared lock; the updater takes an exclusive lock. + unsigned char resp_der[MAX_STAPLING_DER] = {}; + unsigned int resp_derlen = 0; + bool is_expire = true; + time_t expire_time = 0; + mutable ts::bravo::shared_mutex resp_mutex; + + explicit certinfo(bool is_prefetched) : is_prefetched(is_prefetched) {} ~certinfo() { if (cid) { @@ -304,7 +309,6 @@ struct certinfo { } ats_free(certname); ats_free(user_agent); - ink_mutex_destroy(&stapling_mutex); } certinfo(const certinfo &) = delete; @@ -489,7 +493,7 @@ TS_OCSP_cert_id_new(const EVP_MD *dgst, const X509_NAME *issuerName, const ASN1_ } /* Calculate the issuerKey hash, excluding tag and length */ - if (!EVP_Digest(issuerKey->data, issuerKey->length, md, &i, dgst, nullptr)) { + if (!EVP_Digest(ASN1_STRING_get0_data(issuerKey), ASN1_STRING_length(issuerKey), md, &i, dgst, nullptr)) { goto err; } @@ -513,9 +517,9 @@ TS_OCSP_cert_id_new(const EVP_MD *dgst, const X509_NAME *issuerName, const ASN1_ TS_OCSP_CERTID * TS_OCSP_cert_to_id(const EVP_MD *dgst, const X509 *subject, const X509 *issuer) { - const X509_NAME *iname; - const ASN1_INTEGER *serial; - ASN1_BIT_STRING *ikey; + const X509_NAME *iname; + const ASN1_INTEGER *serial; + const ASN1_BIT_STRING *ikey; if (!dgst) { dgst = EVP_sha1(); @@ -847,12 +851,13 @@ stapling_cache_response(TS_OCSP_RESPONSE *rsp, certinfo *cinf) return false; } - ink_mutex_acquire(&cinf->stapling_mutex); - memcpy(cinf->resp_der, resp_der, resp_derlen); - cinf->resp_derlen = resp_derlen; - cinf->is_expire = false; - cinf->expire_time = time(nullptr) + SSLConfigParams::ssl_ocsp_cache_timeout; - ink_mutex_release(&cinf->stapling_mutex); + { + std::lock_guard lock(cinf->resp_mutex); + memcpy(cinf->resp_der, resp_der, resp_derlen); + cinf->resp_derlen = resp_derlen; + cinf->is_expire = false; + cinf->expire_time = time(nullptr) + SSLConfigParams::ssl_ocsp_cache_timeout; + } Dbg(dbg_ctl_ssl_ocsp, "stapling_cache_response: success to cache response"); return true; @@ -881,7 +886,7 @@ ssl_stapling_init_cert(SSL_CTX *ctx, X509 *cert, const char *certname, const cha map = new certinfo_map; map_is_new = true; } - auto cinf_ptr = std::make_unique(); + auto cinf_ptr = std::make_unique(rsp_file != nullptr); certinfo *cinf = cinf_ptr.get(); // Initialize certinfo @@ -889,8 +894,6 @@ ssl_stapling_init_cert(SSL_CTX *ctx, X509 *cert, const char *certname, const cha if (SSLConfigParams::ssl_ocsp_user_agent != nullptr) { cinf->user_agent = ats_strdup(SSLConfigParams::ssl_ocsp_user_agent); } - cinf->is_prefetched = rsp_file ? true : false; - if (cinf->is_prefetched) { Dbg(dbg_ctl_ssl_ocsp, "using OCSP prefetched response file %s", rsp_file); FILE *fp = fopen(rsp_file, "r"); @@ -1330,11 +1333,14 @@ ocsp_update() if (map) { // Walk over all certs associated with this CTX for (auto &iter : *map) { - cinf = iter.second.get(); - ink_mutex_acquire(&cinf->stapling_mutex); + cinf = iter.second.get(); current_time = time(nullptr); - if (cinf->resp_derlen == 0 || cinf->is_expire || cinf->expire_time < current_time) { - ink_mutex_release(&cinf->stapling_mutex); + bool needs_refresh; + { + ts::bravo::shared_lock lock(cinf->resp_mutex); + needs_refresh = cinf->resp_derlen == 0 || cinf->is_expire || cinf->expire_time < current_time; + } + if (needs_refresh) { if (stapling_refresh_response(cinf, &resp)) { Dbg(dbg_ctl_ssl_ocsp, "Successfully refreshed OCSP for %s certificate. url=%s", cinf->certname, cinf->uri); Metrics::Counter::increment(ssl_rsb.ocsp_refreshed_cert); @@ -1342,8 +1348,6 @@ ocsp_update() Error("Failed to refresh OCSP for %s certificate. url=%s", cinf->certname, cinf->uri); Metrics::Counter::increment(ssl_rsb.ocsp_refresh_cert_failure); } - } else { - ink_mutex_release(&cinf->stapling_mutex); } } } @@ -1419,37 +1423,74 @@ ssl_callback_ocsp_stapling(SSL *ssl, void *) return SSL_TLSEXT_ERR_NOACK; } - ink_mutex_acquire(&cinf->stapling_mutex); - time_t current_time = time(nullptr); - if ((cinf->resp_derlen == 0 || cinf->is_expire) || (cinf->expire_time < current_time && !cinf->is_prefetched)) { - ink_mutex_release(&cinf->stapling_mutex); - SiteThrottledError("ssl_callback_ocsp_stapling: failed to get certificate status for %s", cinf->certname); - return SSL_TLSEXT_ERR_NOACK; - } else { #ifdef OPENSSL_IS_BORINGSSL + int set_ok; + { + ts::bravo::shared_lock lock(cinf->resp_mutex); + + time_t current_time = time(nullptr); + if (cinf->resp_derlen == 0 || cinf->is_expire || (cinf->expire_time < current_time && !cinf->is_prefetched)) { + SiteThrottledError("ssl_callback_ocsp_stapling: failed to get certificate status for %s", cinf->certname); + return SSL_TLSEXT_ERR_NOACK; + } + // SSL_set_ocsp_response copies the response, so hand it the cached buffer directly. - int set_ok = SSL_set_ocsp_response(ssl, cinf->resp_der, cinf->resp_derlen); - ink_mutex_release(&cinf->stapling_mutex); + set_ok = SSL_set_ocsp_response(ssl, cinf->resp_der, cinf->resp_derlen); + } #else - unsigned char *p = static_cast(OPENSSL_malloc(cinf->resp_derlen)); - if (p == nullptr) { - ink_mutex_release(&cinf->stapling_mutex); - Dbg(dbg_ctl_ssl_ocsp, "ssl_callback_ocsp_stapling: failed to allocate memory for %s", cinf->certname); - return SSL_TLSEXT_ERR_NOACK; + unsigned char *p = nullptr; + unsigned int resp_capacity = 0; + unsigned int resp_derlen; + + while (true) { + unsigned int required_capacity; + bool is_response_available; + { + ts::bravo::shared_lock lock(cinf->resp_mutex); + + time_t current_time = time(nullptr); + is_response_available = + cinf->resp_derlen != 0 && !cinf->is_expire && (cinf->expire_time >= current_time || cinf->is_prefetched); + + if (is_response_available) { + resp_derlen = cinf->resp_derlen; + if (resp_derlen <= resp_capacity) { + memcpy(p, cinf->resp_der, resp_derlen); + break; + } + required_capacity = resp_derlen; + } } - memcpy(p, cinf->resp_der, cinf->resp_derlen); - ink_mutex_release(&cinf->stapling_mutex); - // Takes ownership of p and frees it on success; on failure it does not. - int set_ok = SSL_set_tlsext_status_ocsp_resp(ssl, p, cinf->resp_derlen); - if (set_ok == 0) { + + if (!is_response_available) { OPENSSL_free(p); + SiteThrottledError("ssl_callback_ocsp_stapling: failed to get certificate status for %s", cinf->certname); + return SSL_TLSEXT_ERR_NOACK; } -#endif - if (set_ok == 0) { + + unsigned char *new_p = static_cast(OPENSSL_malloc(required_capacity)); + if (new_p == nullptr) { + OPENSSL_free(p); + Dbg(dbg_ctl_ssl_ocsp, "ssl_callback_ocsp_stapling: failed to allocate memory for %s", cinf->certname); return SSL_TLSEXT_ERR_NOACK; } - Dbg(dbg_ctl_ssl_ocsp, "ssl_callback_ocsp_stapling: successfully got certificate status for %s", cinf->certname); - Dbg(dbg_ctl_ssl_ocsp, "is_prefetched:%d uri:%s", cinf->is_prefetched, cinf->uri); - return SSL_TLSEXT_ERR_OK; + OPENSSL_free(p); + p = new_p; + resp_capacity = required_capacity; } + + // Takes ownership of p and frees it on success; on failure it does not. + int set_ok = SSL_set_tlsext_status_ocsp_resp(ssl, p, resp_derlen); + if (set_ok == 0) { + OPENSSL_free(p); + } +#endif + + if (set_ok == 0) { + return SSL_TLSEXT_ERR_NOACK; + } + + Dbg(dbg_ctl_ssl_ocsp, "ssl_callback_ocsp_stapling: successfully got certificate status for %s", cinf->certname); + Dbg(dbg_ctl_ssl_ocsp, "is_prefetched:%d uri:%s", cinf->is_prefetched, cinf->uri); + return SSL_TLSEXT_ERR_OK; } diff --git a/src/iocore/net/SSLNetVConnection.cc b/src/iocore/net/SSLNetVConnection.cc index 0423c0b37c3..f1e251bb7c2 100644 --- a/src/iocore/net/SSLNetVConnection.cc +++ b/src/iocore/net/SSLNetVConnection.cc @@ -167,7 +167,7 @@ SSLNetVConnection::_unbindSSLObject() } static void -debug_certificate_name(const char *msg, X509_NAME *name) +debug_certificate_name(const char *msg, const X509_NAME *name) { BIO *bio; diff --git a/src/iocore/net/SSLUtils.cc b/src/iocore/net/SSLUtils.cc index 22b93de583f..f528f1b3006 100644 --- a/src/iocore/net/SSLUtils.cc +++ b/src/iocore/net/SSLUtils.cc @@ -1023,7 +1023,7 @@ SSLMultiCertConfigLoader::check_server_cert_now(X509 *cert, const char *certname } /* CheckServerCertNow() */ static char * -asn1_strdup(ASN1_STRING *s) +asn1_strdup(const ASN1_STRING *s) { // Make sure we have an 8-bit encoding. ink_assert(ASN1_STRING_type(s) == V_ASN1_IA5STRING || ASN1_STRING_type(s) == V_ASN1_UTF8STRING || @@ -2347,10 +2347,9 @@ SSLMultiCertConfigLoader::load_certs_and_cross_reference_names( std::set name_set; // Grub through the names in the certs - X509_NAME *subject = nullptr; // Insert a key for the subject CN. - subject = X509_get_subject_name(cert); + auto *subject = X509_get_subject_name(cert); ats_scoped_str subj_name; if (subject) { int pos = -1; @@ -2360,9 +2359,9 @@ SSLMultiCertConfigLoader::load_certs_and_cross_reference_names( break; } - X509_NAME_ENTRY *e = X509_NAME_get_entry(subject, pos); - ASN1_STRING *cn = X509_NAME_ENTRY_get_data(e); - subj_name = asn1_strdup(cn); + const X509_NAME_ENTRY *e = X509_NAME_get_entry(subject, pos); + const ASN1_STRING *cn = X509_NAME_ENTRY_get_data(e); + subj_name = asn1_strdup(cn); Dbg(dbg_ctl_ssl_load, "subj '%s' in certificate %s %p", subj_name.get(), data.cert_names_list[i].c_str(), cert); name_set.insert(subj_name.get()); diff --git a/src/iocore/net/unit_tests/test_SSLDHParams.cc b/src/iocore/net/unit_tests/test_SSLDHParams.cc index 3e75f5fe097..8d0e44a4a0a 100644 --- a/src/iocore/net/unit_tests/test_SSLDHParams.cc +++ b/src/iocore/net/unit_tests/test_SSLDHParams.cc @@ -132,9 +132,12 @@ make_cert_and_key(EVP_CIPHER const *cipher = nullptr, char *pass = nullptr) 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 *name = X509_NAME_dup(X509_get_subject_name(x509)); + REQUIRE(name != nullptr); X509_NAME_add_entry_by_txt(name, "CN", MBSTRING_ASC, reinterpret_cast("ats-test"), -1, -1, 0); + REQUIRE(X509_set_subject_name(x509, name) == 1); REQUIRE(X509_set_issuer_name(x509, name) == 1); + X509_NAME_free(name); REQUIRE(X509_sign(x509, pkey, EVP_sha256()) > 0); BIO *cert_bio = BIO_new(BIO_s_mem()); diff --git a/src/proxy/http/HttpTransact.cc b/src/proxy/http/HttpTransact.cc index edf6b2c862e..c6d1b274ca0 100644 --- a/src/proxy/http/HttpTransact.cc +++ b/src/proxy/http/HttpTransact.cc @@ -3836,12 +3836,6 @@ HttpTransact::handle_response_from_parent(State *s) TxnDbg(dbg_ctl_http_trans, "(hrfp)"); HTTP_RELEASE_ASSERT(s->current.server == &s->parent_info); - // if this parent was retried from a markdown, then - // notify that the retry has completed. - if (s->parent_result.retry) { - markParentUp(s); - } - simple_or_unavailable_server_retry(s); s->parent_info.state = s->current.state; diff --git a/src/tscore/X509HostnameValidator.cc b/src/tscore/X509HostnameValidator.cc index 0efbcac99df..36cba94714e 100644 --- a/src/tscore/X509HostnameValidator.cc +++ b/src/tscore/X509HostnameValidator.cc @@ -206,12 +206,12 @@ do_check_string(ASN1_STRING *a, int cmp_type, equal_fn equal, const unsigned cha { bool retval = false; - if (!a->data || !a->length || cmp_type != a->type) { + if (!ASN1_STRING_get0_data(a) || !ASN1_STRING_length(a) || cmp_type != ASN1_STRING_type(a)) { return false; } - retval = equal(a->data, a->length, b, blen); + retval = equal(ASN1_STRING_get0_data(a), ASN1_STRING_length(a), b, blen); if (retval && peername) { - *peername = ats_strndup((char *)a->data, a->length); + *peername = ats_strndup(reinterpret_cast(ASN1_STRING_get0_data(a)), ASN1_STRING_length(a)); } return retval; } @@ -220,7 +220,6 @@ bool validate_hostname(X509 *x, std::string_view hostname, bool is_ip, char **peername) { GENERAL_NAMES *gens = nullptr; - X509_NAME *name = nullptr; int i; int alt_type; bool retval = false; @@ -269,19 +268,19 @@ validate_hostname(X509 *x, std::string_view hostname, bool is_ip, char **peernam } } // No SAN match -- check the subject - i = -1; - name = X509_get_subject_name(x); + i = -1; + auto *name = X509_get_subject_name(x); while ((i = X509_NAME_get_index_by_NID(name, NID_commonName, i)) >= 0) { - ASN1_STRING *str; - int astrlen; - unsigned char *astr; + const ASN1_STRING *str; + int astrlen; + unsigned char *astr; str = X509_NAME_ENTRY_get_data(X509_NAME_get_entry(name, i)); // Convert to UTF-8 astrlen = ASN1_STRING_to_UTF8(&astr, str); if (astrlen < 0) { - return -1; + return false; } retval = equal(astr, astrlen, hostname_data, hostname_len); if (retval && peername) { diff --git a/tests/gold_tests/chunked_encoding/chunked_encoding_h2.test.py b/tests/gold_tests/chunked_encoding/chunked_encoding_h2.test.py index 004267c2d60..0347e6c899a 100644 --- a/tests/gold_tests/chunked_encoding/chunked_encoding_h2.test.py +++ b/tests/gold_tests/chunked_encoding/chunked_encoding_h2.test.py @@ -16,6 +16,9 @@ # See the License for the specific language governing permissions and # limitations under the License. +import os +import sys + Test.Summary = ''' Test interaction of H2 and chunked encoding ''' @@ -36,12 +39,13 @@ # add ssl materials like key, certificates for the server ts.addDefaultSSLFiles() +origin_server = os.path.join(Test.TestDirectory, "chunked_encoding_h2_server.py") delay_server = Test.Processes.Process( - "delay-server", "bash -c '" + Test.TestDirectory + "/delay-server.sh {} outserver1'".format(Test.Variables.upstream_port)) + "delay-server", f'{sys.executable} "{origin_server}" 127.0.0.1 {Test.Variables.upstream_port} outserver1 delayed-chunked') server2 = Test.Processes.Process( - "server2", "bash -c '" + Test.TestDirectory + "/server2.sh {} outserver2'".format(Test.Variables.upstream_port2)) + "server2", f'{sys.executable} "{origin_server}" 127.0.0.1 {Test.Variables.upstream_port2} outserver2 content-length') server3 = Test.Processes.Process( - "server3", "bash -c '" + Test.TestDirectory + "/server3.sh {} outserver3'".format(Test.Variables.upstream_port3)) + "server3", f'{sys.executable} "{origin_server}" 127.0.0.1 {Test.Variables.upstream_port3} outserver3 chunked') ts.Disk.records_config.update( { @@ -58,9 +62,8 @@ ts.Disk.ssl_multicert_config.AddLine('dest_ip=* ssl_cert_name=server.pem ssl_key_name=server.key') -# Using netcat as a cheap origin server in case 1 so we can insert a delay in sending back the response. -# Replaced microserver for cases 2 and 3 as well because I was getting python exceptions when running -# microserver if chunked encoding headers were specified for the request headers +# Use a raw origin server in case 1 so the final chunk can be delayed. Use it +# for cases 2 and 3 as well because microserver rejects chunked request headers. # H2 GET request # chunked response without content-length @@ -70,7 +73,7 @@ tr.Processes.Default.Command = 'nghttp -vv https://127.0.0.1:{}/delay-chunked-response'.format(ts.Variables.ssl_port) tr.Processes.Default.ReturnCode = 0 tr.Processes.Default.StartBefore(Test.Processes.ts) -tr.Processes.Default.StartBefore(delay_server) +tr.Processes.Default.StartBefore(delay_server, ready=When.PortOpen(Test.Variables.upstream_port)) tr.Processes.Default.Streams.All = Testers.ExcludesExpression("RST_STREAM", "Delayed chunk close should not cause reset") tr.Processes.Default.Streams.All += Testers.ExcludesExpression("< content-length", "Should return chunked") tr.Processes.Default.Streams.All += Testers.ContainsExpression(":status: 200", "Should get successful response") @@ -81,7 +84,7 @@ # HTTP2 POST: www.example.com Host, chunked body server2_out = Test.Disk.File("outserver2") tr = Test.AddTestRun() -tr.Processes.Default.StartBefore(server2) +tr.Processes.Default.StartBefore(server2, ready=When.PortOpen(Test.Variables.upstream_port2)) tr.MakeCurlCommand( '--http2 -k https://127.0.0.1:{}/post-full --verbose -H "Transfer-encoding: chunked" -d "Knock knock"'.format( ts.Variables.ssl_port), @@ -97,7 +100,7 @@ # HTTP2 POST: chunked post body and chunked response server3_out = Test.Disk.File("outserver3") tr = Test.AddTestRun() -tr.Processes.Default.StartBefore(server3) +tr.Processes.Default.StartBefore(server3, ready=When.PortOpen(Test.Variables.upstream_port3)) tr.MakeCurlCommand( '--http2 -k https://127.0.0.1:{}/post-chunked --verbose -H "Transfer-encoding: chunked" -d "Knock knock"'.format( ts.Variables.ssl_port), diff --git a/tests/gold_tests/chunked_encoding/chunked_encoding_h2_server.py b/tests/gold_tests/chunked_encoding/chunked_encoding_h2_server.py new file mode 100644 index 00000000000..e1b003c9b0b --- /dev/null +++ b/tests/gold_tests/chunked_encoding/chunked_encoding_h2_server.py @@ -0,0 +1,115 @@ +#!/usr/bin/env python3 +"""Serve one raw HTTP request for the chunked HTTP/2 AuTest.""" + +# 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 argparse +from pathlib import Path +import socket +import sys +import time + + +def parse_args() -> argparse.Namespace: + """Parse the command-line arguments.""" + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("address", help="Address on which to listen.") + parser.add_argument("port", type=int, help="Port on which to listen.") + parser.add_argument("output", type=Path, help="File in which to record the request.") + parser.add_argument("response", choices=("delayed-chunked", "content-length", "chunked"), help="Response to send.") + return parser.parse_args() + + +def make_listening_socket(address: str, port: int) -> socket.socket: + """Create and return a listening TCP socket.""" + listener = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + listener.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) + listener.bind((address, port)) + listener.listen(1) + return listener + + +def receive_request(conn: socket.socket) -> bytes: + """Receive one HTTP request, including its declared body.""" + request = b"" + while b"\r\n\r\n" not in request: + data = conn.recv(4096) + if not data: + return request + request += data + + header, _, body = request.partition(b"\r\n\r\n") + content_length = 0 + is_chunked = False + for field in header.split(b"\r\n")[1:]: + name, separator, value = field.partition(b":") + if not separator: + continue + name = name.strip().lower() + value = value.strip().lower() + if name == b"content-length": + content_length = int(value) + elif name == b"transfer-encoding" and b"chunked" in value: + is_chunked = True + + if is_chunked: + while not (body.startswith(b"0\r\n\r\n") or b"\r\n0\r\n\r\n" in body): + data = conn.recv(4096) + if not data: + break + body += data + else: + while len(body) < content_length: + data = conn.recv(4096) + if not data: + break + body += data + + return header + b"\r\n\r\n" + body + + +def send_response(conn: socket.socket, response: str) -> None: + """Send the selected raw HTTP response.""" + if response == "delayed-chunked": + conn.sendall(b"HTTP/1.1 200\r\nTransfer-encoding: chunked\r\n\r\n") + conn.sendall(b"F\r\n123456789012345\r\n") + time.sleep(1) + conn.sendall(b"0\r\n\r\n") + elif response == "content-length": + conn.sendall(b"HTTP/1.1 200\r\nContent-length: 15\r\n\r\n123456789012345") + else: + conn.sendall(b"HTTP/1.1 200\r\nTransfer-encoding: chunked\r\n\r\nF\r\n123456789012345\r\n0\r\n\r\n") + + +def main() -> int: + """Ignore readiness probes, serve one request, and exit.""" + args = parse_args() + with make_listening_socket(args.address, args.port) as listener: + while True: + conn, _ = listener.accept() + with conn: + request = receive_request(conn) + if not request: + # When.PortOpen probes the listener without sending data. + continue + args.output.write_bytes(request) + send_response(conn, args.response) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/gold_tests/chunked_encoding/delay-server.sh b/tests/gold_tests/chunked_encoding/delay-server.sh deleted file mode 100755 index c4d4846ddc3..00000000000 --- a/tests/gold_tests/chunked_encoding/delay-server.sh +++ /dev/null @@ -1,43 +0,0 @@ -# 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. - -# A very simple cleartext server for one HTTP transaction. Does no validation of the Request message. -# Sends a fixed response message - -response () -{ - # Wait for end of Request message. - # - while (( 1 == 1 )) - do - if [[ -f $outfile ]] ; then - if tr '\r\n' '=!' < $outfile | grep '=!=!' > /dev/null - then - break; - fi - fi - sleep 1 - done - - # delay before finishing the chunk - printf "HTTP/1.1 200\r\nTransfer-encoding: chunked\r\n\r\n" - printf "F\r\n123456789012345\r\n" - sleep 1 - printf "0\r\n\r\n" - -} -outfile=$2 -response | nc -l $1 > "$outfile" diff --git a/tests/gold_tests/chunked_encoding/server3.sh b/tests/gold_tests/chunked_encoding/server3.sh deleted file mode 100755 index e08087faefa..00000000000 --- a/tests/gold_tests/chunked_encoding/server3.sh +++ /dev/null @@ -1,41 +0,0 @@ -# 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. - -# A very simple cleartext server for one HTTP transaction. Does no validation of the Request message. -# Sends a fixed response message - - -response () -{ - # Wait for end of Request message. - # - while (( 1 == 1 )) - do - if [[ -f $outfile ]] ; then - if tr '\r\n' '=!' < $outfile | grep '=!=!' > /dev/null - then - break; - fi - fi - sleep 1 - done - - printf "HTTP/1.1 200\r\nTransfer-encoding: chunked\r\n\r\n" - printf "F\r\n123456789012345\r\n0\r\n\r\n" - -} -outfile=$2 -response | nc -l $1 > "$outfile" diff --git a/tests/gold_tests/chunked_encoding/server2.sh b/tests/gold_tests/parent_proxy/parent_retry_availability.test.py old mode 100755 new mode 100644 similarity index 61% rename from tests/gold_tests/chunked_encoding/server2.sh rename to tests/gold_tests/parent_proxy/parent_retry_availability.test.py index 2fd88f60da4..51a7b1448d1 --- a/tests/gold_tests/chunked_encoding/server2.sh +++ b/tests/gold_tests/parent_proxy/parent_retry_availability.test.py @@ -1,3 +1,6 @@ +""" +Verify the retry path restores a parent only when the retry actually succeeds. +""" # 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 @@ -14,28 +17,10 @@ # See the License for the specific language governing permissions and # limitations under the License. -# A very simple cleartext server for one HTTP transaction. Does no validation of the Request message. -# Sends a fixed response message +Test.Summary = ''' +Both outcomes of a parent retry: a parent that is still up but not serving stays +marked down, and a parent that has recovered is restored to the pool. +''' - -response () -{ - # Wait for end of Request message. - # - while (( 1 == 1 )) - do - if [[ -f $outfile ]] ; then - if tr '\r\n' '=!' < $outfile | grep '=!=!' > /dev/null - then - break; - fi - fi - sleep 1 - done - - printf "HTTP/1.1 200\r\nContent-length: 15\r\n\r\n" - printf "123456789012345" - -} -outfile=$2 -response | nc -l $1 > "$outfile" +Test.ATSReplayTest(replay_file='replays/parent_retry_failure_stays_down.replay.yaml') +Test.ATSReplayTest(replay_file='replays/parent_retry_success_restores.replay.yaml') diff --git a/tests/gold_tests/parent_proxy/replays/parent_retry_failure_stays_down.replay.yaml b/tests/gold_tests/parent_proxy/replays/parent_retry_failure_stays_down.replay.yaml new file mode 100644 index 00000000000..d2c5fc5bd88 --- /dev/null +++ b/tests/gold_tests/parent_proxy/replays/parent_retry_failure_stays_down.replay.yaml @@ -0,0 +1,215 @@ +# 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. + +# +# A parent that accepts the connection but never sends a response ("up but not +# serving") must stay marked down when a retry probe also fails. +# +# A retried parent must only be restored once a retry actually succeeds. +# markParentUp() zeroes failedAt and failCount, and the markParentDown() that +# follows a failed probe takes its result->retry branch, which leaves +# new_fail_count at 0 and so never re-clears available. Restoring the parent +# before the probe's outcome is known therefore returns it to the pool with a +# clean counter after every retry_time window, no matter how long it stays +# degraded. That is why the restore belongs in the CONNECTION_ALIVE branch of +# HttpTransact::handle_response_from_parent and not at function entry. +# +# The verifier server plays the parent: server-response.delay (4s) exceeds the +# child's transaction_no_activity_timeout_out (2s), so ATS reads zero bytes and +# times out (INACTIVE_TIMEOUT), which enable_parent_timeout_markdowns=1 admits +# as a markdown-eligible failure. +# + +meta: + version: "1.0" + +autest: + description: 'A failed retry probe must not restore a degraded parent' + + server: + name: 'server-degraded' + # Every transaction is abandoned by ATS mid-flight (the response is still + # being held when the timeout fires), so the server may report an incomplete + # transaction cycle. The assertions live in the client statuses and the ATS + # diags log. + return_code: [0, 1] + + client: + name: 'client-degraded' + process_config: + # Each request waits out a 2s ATS timeout, and one waits a further 3s for + # retry_time. The default 5s poll timeout would abandon a transaction + # mid-flight and report no status violation at all. + other_args: '--poll-timeout 30000' + + # Resolve every hostname to loopback. The origin is never actually contacted + # (go_direct=false keeps every request on the parent), but ATS may still run + # an origin DNS lookup on the way to parent selection, and an unresolvable + # name would fail the request before selection runs. + dns: + name: 'dns-degraded' + + ats: + name: 'ts-degraded' + process_config: + enable_cache: false + + records_config: + proxy.config.diags.debug.enabled: 1 + proxy.config.diags.debug.tags: 'parent_select|http_trans' + # NOTE: no_dns_just_forward_to_parent is deliberately NOT set here. It + # routes the request through the parentExists() shortcut in + # HttpTransact::HandleRequest, which tests pRecord::available directly and + # so short-circuits to an error as soon as the parent is marked down -- + # before ParentRoundRobin::selectParent runs. The retry-window logic under + # test lives inside selectParent, so the request has to reach it. + # + # With the cache disabled every request is non-lookupable, and this + # setting (default 1) would then send each one direct to the origin + # instead of the parent -- "request not cacheable, so bypass parent". + proxy.config.http.uncacheable_requests_bypass_parent: 0 + # A read timeout only counts toward markdown when this is enabled. It is + # off by default upstream but enabled in the production config this + # reproduces. + proxy.config.http.parent_proxy.enable_parent_timeout_markdowns: 1 + # One timeout demotes the parent, keeping the test to four requests. + proxy.config.http.parent_proxy.fail_threshold: 1 + # Short enough that a single in-replay delay outlasts the down window. + proxy.config.http.parent_proxy.retry_time: 2 + # No intra-request retry looping; one attempt per request keeps the + # failure accounting one-to-one with the transactions below. + proxy.config.http.parent_proxy.total_connect_attempts: 1 + proxy.config.http.parent_proxy.per_parent_connect_attempts: 1 + # Keep the parent out of the HostStatus map so availability is governed + # purely by retry_time. + proxy.config.http.parent_proxy.self_detect: 0 + # Bound the wait on the silent parent. + proxy.config.http.transaction_no_activity_timeout_out: 2 + proxy.config.url_remap.remap_required: 0 + + # A single parent with go_direct=false: once it is unavailable there is + # nowhere else to go, so selection-time skipping is visible as a 502. + parent_config: + - 'dest_domain=. parent="127.0.0.1:{SERVER_HTTP_PORT}" go_direct=false parent_is_proxy=true' + + log_validation: + diags_log: + contains: + # The retry probe fails, so the parent must be re-marked down rather + # than left available. + - expression: 'Parent retry marked as down 127\.0\.0\.1:\d+' + description: 'The failed retry probe re-marks the parent down' + excludes: + # Note() emitted only by ParentSelectionStrategy::markParentUp. Every + # request in this replay fails, so no retry ever succeeds and the + # parent must never be restored. + - expression: 'http parent proxy 127\.0\.0\.1:\d+ restored with request' + description: 'A failed retry probe must not restore the parent' + +sessions: +- transactions: + # 1: the parent accepts but never responds. ATS times out (504) and the + # failure count reaches fail_threshold, so the parent is marked down as this + # request finishes. + - all: { headers: { fields: [[ uuid, degrade1 ]]}} + client-request: + method: GET + version: "1.1" + scheme: http + url: /degrade1 + headers: + fields: + - [ Host, example.com ] + - [ Content-Length, "0" ] + server-response: + delay: 4s + status: 200 + reason: OK + headers: + fields: + - [ Content-Length, "0" ] + proxy-response: + status: 504 + + # 2: still inside retry_time. The parent is skipped at selection and, with no + # other parent and go_direct=false, ATS returns 502 without touching the leg. + # This confirms the markdown from request 1 actually took effect. + - all: { headers: { fields: [[ uuid, downcheck ]]}} + client-request: + method: GET + version: "1.1" + scheme: http + url: /downcheck + headers: + fields: + - [ Host, example.com ] + - [ Content-Length, "0" ] + server-response: + status: 200 + reason: OK + headers: + fields: + - [ Content-Length, "0" ] + proxy-response: + status: 502 + + # 3: the retry probe. The client delay pushes this request past retry_time, so + # parent selection picks the parent as a retry candidate. The probe reaches the + # still-silent parent and times out (504). The parent must not be restored: + # markParentUp belongs to the CONNECTION_ALIVE branch, which this never takes. + - all: { headers: { fields: [[ uuid, retryprobe ]]}} + client-request: + method: GET + version: "1.1" + scheme: http + url: /retryprobe + delay: 3s + headers: + fields: + - [ Host, example.com ] + - [ Content-Length, "0" ] + server-response: + delay: 4s + status: 200 + reason: OK + headers: + fields: + - [ Content-Length, "0" ] + proxy-response: + status: 504 + + # 4: the retry probe failed, so the parent must still be down. Selection skips + # it and ATS returns 502 without touching the leg. A 504 here would mean the + # failed probe had restored the parent and it was attempted again. + - all: { headers: { fields: [[ uuid, aftercheck ]]}} + client-request: + method: GET + version: "1.1" + scheme: http + url: /aftercheck + headers: + fields: + - [ Host, example.com ] + - [ Content-Length, "0" ] + server-response: + delay: 4s + status: 200 + reason: OK + headers: + fields: + - [ Content-Length, "0" ] + proxy-response: + status: 502 diff --git a/tests/gold_tests/parent_proxy/replays/parent_retry_success_restores.replay.yaml b/tests/gold_tests/parent_proxy/replays/parent_retry_success_restores.replay.yaml new file mode 100644 index 00000000000..91a8942d53e --- /dev/null +++ b/tests/gold_tests/parent_proxy/replays/parent_retry_success_restores.replay.yaml @@ -0,0 +1,177 @@ +# 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. + +# +# A parent that recovers must be restored to the pool by a successful retry. +# +# This is the positive counterpart to parent_retry_failure_stays_down.replay.yaml, +# which covers the case where the retry probe also fails. Together they pin both +# outcomes of the retry path: restore only on success, stay down on failure. +# +# The parent stops being silent partway through the replay, so the retry probe +# sent after retry_time elapses actually succeeds. markParentUp() must then run +# from the CONNECTION_ALIVE branch of handle_response_from_parent, clearing the +# failure count and returning the parent to the pool. +# + +meta: + version: "1.0" + +autest: + description: 'A successful retry restores a parent that had been marked down' + + server: + name: 'server-recovered' + # The first transaction is abandoned by ATS mid-flight (the response is + # still being held when the timeout fires), so the server may report an + # incomplete transaction cycle. + return_code: [0, 1] + + client: + name: 'client-recovered' + process_config: + # The first request waits out a 2s ATS timeout and the retry probe waits a + # further 3s for retry_time, both well past the 5s default. + other_args: '--poll-timeout 30000' + + # Resolve every hostname to loopback, as in the failing-retry scenario. + dns: + name: 'dns-recovered' + + ats: + name: 'ts-recovered' + process_config: + enable_cache: false + + records_config: + proxy.config.diags.debug.enabled: 1 + proxy.config.diags.debug.tags: 'parent_select|http_trans' + # See parent_retry_failure_stays_down.replay.yaml for why + # no_dns_just_forward_to_parent must stay unset and why this must be 0. + proxy.config.http.uncacheable_requests_bypass_parent: 0 + proxy.config.http.parent_proxy.enable_parent_timeout_markdowns: 1 + proxy.config.http.parent_proxy.fail_threshold: 1 + proxy.config.http.parent_proxy.retry_time: 2 + proxy.config.http.parent_proxy.total_connect_attempts: 1 + proxy.config.http.parent_proxy.per_parent_connect_attempts: 1 + proxy.config.http.parent_proxy.self_detect: 0 + proxy.config.http.transaction_no_activity_timeout_out: 2 + proxy.config.url_remap.remap_required: 0 + + parent_config: + - 'dest_domain=. parent="127.0.0.1:{SERVER_HTTP_PORT}" go_direct=false parent_is_proxy=true' + + log_validation: + diags_log: + contains: + # Note() from ParentSelectionStrategy::markParentUp, emitted only when + # a retry succeeds against a parent with a non-zero failure count. + - expression: 'http parent proxy 127\.0\.0\.1:\d+ restored with request' + description: 'A successful retry restores the parent' + excludes: + # The retry succeeds, so it must never be re-marked down. + - expression: 'Parent retry marked as down' + description: 'A successful retry must not re-mark the parent down' + +sessions: +- transactions: + # 1: the parent accepts but never responds. ATS times out (504) and marks it + # down, since fail_threshold is 1. + - all: { headers: { fields: [[ uuid, degrade1 ]]}} + client-request: + method: GET + version: "1.1" + scheme: http + url: /degrade1 + headers: + fields: + - [ Host, example.com ] + - [ Content-Length, "0" ] + server-response: + delay: 4s + status: 200 + reason: OK + headers: + fields: + - [ Content-Length, "0" ] + proxy-response: + status: 504 + + # 2: still inside retry_time, so the parent is skipped at selection and ATS + # returns 502 without touching the leg. + - all: { headers: { fields: [[ uuid, downcheck ]]}} + client-request: + method: GET + version: "1.1" + scheme: http + url: /downcheck + headers: + fields: + - [ Host, example.com ] + - [ Content-Length, "0" ] + server-response: + status: 200 + reason: OK + headers: + fields: + - [ Content-Length, "0" ] + proxy-response: + status: 502 + + # 3: the retry probe. The client delay pushes it past retry_time so the parent + # is selected as a retry candidate, and this time the parent responds promptly + # (no server delay). The 200 proves the retry succeeded and the parent was + # restored from the CONNECTION_ALIVE branch. + - all: { headers: { fields: [[ uuid, retryprobe ]]}} + client-request: + method: GET + version: "1.1" + scheme: http + url: /retryprobe + delay: 3s + headers: + fields: + - [ Host, example.com ] + - [ Content-Length, "0" ] + server-response: + status: 200 + reason: OK + headers: + fields: + - [ Content-Length, "0" ] + proxy-response: + status: 200 + + # 4: the parent is available again, so this request goes straight through + # without waiting for another retry_time window. + - all: { headers: { fields: [[ uuid, aftercheck ]]}} + client-request: + method: GET + version: "1.1" + scheme: http + url: /aftercheck + headers: + fields: + - [ Host, example.com ] + - [ Content-Length, "0" ] + server-response: + status: 200 + reason: OK + headers: + fields: + - [ Content-Length, "0" ] + proxy-response: + status: 200 diff --git a/tests/gold_tests/pluginTest/header_rewrite/header_rewrite_bad_run_plugin.test.py b/tests/gold_tests/pluginTest/header_rewrite/header_rewrite_bad_run_plugin.test.py new file mode 100644 index 00000000000..c5371f5c9e9 --- /dev/null +++ b/tests/gold_tests/pluginTest/header_rewrite/header_rewrite_bad_run_plugin.test.py @@ -0,0 +1,156 @@ +# 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. +''' +Verify header_rewrite rejects a run-plugin operator whose target plugin fails to +load. The failure must be caught at config load time, not aborted at request time. +''' + +Test.Summary = ''' +header_rewrite must reject a run-plugin whose target plugin fails to load, at +config load time, rather than aborting the server on the first request. +''' + +# Reproduce the reported crash: run-plugin against a plugin whose instance-init fails +# (conf_remap + a missing file) hands header_rewrite a null instance, which old code aborted on. +Test.SkipUnless( + Condition.PluginExists('header_rewrite.so'), + Condition.PluginExists('conf_remap.so'), +) + + +class TestBadRunPlugin: + '''Verify failed run-plugin initialization is rejected safely.''' + + ERROR_MARKER: str = 'run-plugin unable to load' + BAD_RULE_LINES: list[str] = [ + 'cond %{REMAP_PSEUDO_HOOK}', + ' run-plugin conf_remap.so no_such_conf_remap_file.yaml', + ] + NESTED_BAD_RULE_LINES: list[str] = [ + 'cond %{REMAP_PSEUDO_HOOK}', + ' if', + ' cond %{TRUE}', + ' run-plugin conf_remap.so no_such_conf_remap_file.yaml', + ' endif', + ] + + def __init__(self) -> None: + '''Configure startup and reload rejection scenarios.''' + self._configure_startup_rejection() + self._server = self._configure_origin_server() + self._ts = self._configure_traffic_server() + self._configure_baseline_request() + self._configure_bad_remap_install() + self._configure_failed_reload() + self._configure_post_reload_request() + self._ts.Disk.diags_log.Content = Testers.IncludesExpression( + self.ERROR_MARKER, 'the rejected reload should log the run-plugin failure') + + def _configure_startup_rejection(self) -> None: + '''Verify a bad top-level run-plugin fails startup cleanly.''' + ts = Test.MakeATSProcess("ts-startup", disable_log_checks=True) + ts.Disk.records_config.update({ + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'header_rewrite', + }) + ts.Disk.MakeConfigFile('bad_run_plugin.conf').AddLines(self.BAD_RULE_LINES) + ts.Disk.remap_config.AddLine( + 'map http://startup.example.com/ http://127.0.0.1/ ' + '@plugin=header_rewrite.so @pparam=bad_run_plugin.conf') + + # Invalid remap.config triggers a controlled exit rather than SIGABRT. + ts.ReturnCode = 33 + ts.Ready = 0 + ts.Disk.diags_log.Content = Testers.IncludesExpression( + self.ERROR_MARKER, 'header_rewrite must report the failed run-plugin load') + ts.Disk.traffic_out.Content = Testers.ExcludesExpression( + 'Traffic Server is fully initialized', 'ATS must not initialize with a bad run-plugin config') + + tr = Test.AddTestRun("Bad run-plugin config fails startup instead of crashing") + tr.Processes.Default.Command = 'echo verifying startup rejection' + tr.Processes.Default.ReturnCode = 0 + tr.Processes.Default.StartBefore(ts) + + def _configure_origin_server(self) -> 'Process': + '''Configure the origin used to verify reload behavior.''' + server = Test.MakeOriginServer("server") + request_header = { + "headers": "GET / HTTP/1.1\r\nHost: reload.example.com\r\n\r\n", + "timestamp": "1469733493.993", + "body": "" + } + response_header = {"headers": "HTTP/1.1 200 OK\r\nConnection: close\r\n\r\n", "timestamp": "1469733493.993", "body": ""} + server.addResponse("sessionfile.log", request_header, response_header) + return server + + def _configure_traffic_server(self) -> 'Process': + '''Configure ATS with a valid initial remap table.''' + ts = Test.MakeATSProcess("ts-reload", disable_log_checks=True) + ts.Disk.records_config.update({ + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'header_rewrite', + }) + ts.Disk.MakeConfigFile('nested_bad_run_plugin.conf').AddLines(self.NESTED_BAD_RULE_LINES) + ts.Disk.remap_config.AddLine(f'map http://reload.example.com http://127.0.0.1:{self._server.Variables.Port}') + return ts + + def _configure_curl_run(self, name: str, expectation: str) -> 'TestRun': + '''Configure a request that verifies ATS still serves traffic.''' + tr = Test.AddTestRun(name) + tr.MakeCurlCommand( + f'--proxy 127.0.0.1:{self._ts.Variables.port} "http://reload.example.com" ' + '-H "Proxy-Connection: keep-alive" --verbose', + ts=self._ts) + tr.Processes.Default.ReturnCode = 0 + tr.Processes.Default.Streams.stderr = Testers.IncludesExpression('200 OK', expectation) + tr.StillRunningAfter = self._ts + tr.StillRunningAfter = self._server + return tr + + def _configure_baseline_request(self) -> None: + '''Verify the valid initial configuration serves requests.''' + tr = self._configure_curl_run("Baseline request is served before reload", 'baseline request should be served') + tr.Processes.Default.StartBefore(self._server) + tr.Processes.Default.StartBefore(self._ts) + + def _configure_bad_remap_install(self) -> None: + '''Replace remap.config with one containing a bad nested run-plugin.''' + tr = Test.AddTestRun("Install a remap.config with a bad run-plugin") + remap_path = self._ts.Disk.remap_config.AbsPath + tr.Disk.File(remap_path, id="remap_bad", typename="ats:config") + tr.Disk.remap_bad.AddLine( + f'map http://reload.example.com http://127.0.0.1:{self._server.Variables.Port} ' + '@plugin=header_rewrite.so @pparam=nested_bad_run_plugin.conf') + tr.Processes.Default.Command = 'echo installed bad remap.config' + tr.Processes.Default.ReturnCode = 0 + tr.Processes.Default.Env = self._ts.Env + tr.StillRunningAfter = self._ts + tr.StillRunningAfter = self._server + + def _configure_failed_reload(self) -> None: + '''Verify the bad remap table is rejected without stopping ATS.''' + tr = Test.AddConfigReload( + self._ts, expect="fail", delay_start=2, description="Reload with bad run-plugin must be rejected, not fatal") + tr.StillRunningAfter = self._ts + tr.StillRunningAfter = self._server + + def _configure_post_reload_request(self) -> None: + '''Verify the rejected reload leaves the old configuration active.''' + self._configure_curl_run( + "Server still serves the old config after the rejected reload", 'old config should still serve after a rejected reload') + + +TestBadRunPlugin() diff --git a/tests/gold_tests/pluginTest/stale_response/stale_response_max_memory.replay.yaml b/tests/gold_tests/pluginTest/stale_response/stale_response_max_memory.replay.yaml index 3b199dd6285..af031c9e7b7 100644 --- a/tests/gold_tests/pluginTest/stale_response/stale_response_max_memory.replay.yaml +++ b/tests/gold_tests/pluginTest/stale_response/stale_response_max_memory.replay.yaml @@ -77,6 +77,11 @@ sessions: - [ Connection, close ] - [ Cache-Control, "max-age=1" ] - [ X-Response, oversized-origin-response ] + # The preceding fields serialize to 215 bytes. X-Padding adds 42 wire + # bytes (name, separator, 29-byte value, and CRLF), bringing the header + # to the 256-byte limit plus the one-byte overflow sentinel. This makes + # the memory rejection independent of body segmentation. + - [ X-Padding, aaaaaaaaaaaaaaaaaaaaaaaaaaaaa ] content: size: 512 diff --git a/tests/gold_tests/qmux/go_qmux_client/go.mod b/tests/gold_tests/qmux/go_qmux_client/go.mod index 531eb56c069..e9161bbcdb0 100644 --- a/tests/gold_tests/qmux/go_qmux_client/go.mod +++ b/tests/gold_tests/qmux/go_qmux_client/go.mod @@ -9,7 +9,7 @@ require ( ) require ( - golang.org/x/crypto v0.50.0 // indirect - golang.org/x/net v0.53.0 // indirect - golang.org/x/sys v0.43.0 // indirect + golang.org/x/crypto v0.52.0 // indirect + golang.org/x/net v0.55.0 // indirect + golang.org/x/sys v0.45.0 // indirect ) diff --git a/tests/gold_tests/qmux/go_qmux_client/go.sum b/tests/gold_tests/qmux/go_qmux_client/go.sum index b10735cd87f..7527a339a24 100644 --- a/tests/gold_tests/qmux/go_qmux_client/go.sum +++ b/tests/gold_tests/qmux/go_qmux_client/go.sum @@ -16,11 +16,11 @@ github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= go.uber.org/mock v0.5.2 h1:LbtPTcP8A5k9WPXj54PPPbjcI4Y6lhyOZXn+VS7wNko= go.uber.org/mock v0.5.2/go.mod h1:wLlUxC2vVTPTaE3UD51E0BGOAElKrILxhVSDYQLld5o= -golang.org/x/crypto v0.50.0 h1:zO47/JPrL6vsNkINmLoo/PH1gcxpls50DNogFvB5ZGI= -golang.org/x/crypto v0.50.0/go.mod h1:3muZ7vA7PBCE6xgPX7nkzzjiUq87kRItoJQM1Yo8S+Q= -golang.org/x/net v0.53.0 h1:d+qAbo5L0orcWAr0a9JweQpjXF19LMXJE8Ey7hwOdUA= -golang.org/x/net v0.53.0/go.mod h1:JvMuJH7rrdiCfbeHoo3fCQU24Lf5JJwT9W3sJFulfgs= -golang.org/x/sys v0.43.0 h1:Rlag2XtaFTxp19wS8MXlJwTvoh8ArU6ezoyFsMyCTNI= -golang.org/x/sys v0.43.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/crypto v0.52.0 h1:RMs7fP2rXdep0CftQlK8Uf+kibLm7qkCcradZWYz988= +golang.org/x/crypto v0.52.0/go.mod h1:1QgfPxDqh0T2M/elOJtp9RvuR95kVjir0e6/BvEmGbc= +golang.org/x/net v0.55.0 h1:bcvxaJn3e1U6InsFWt1JUq1aSjnRxLzT2rtD2KfkDF8= +golang.org/x/net v0.55.0/go.mod h1:L5U2KuzuOe1lY7Z+aWVIKK6qEeJXnXV9yzGA+WCHJww= +golang.org/x/sys v0.45.0 h1:dO4czNzziLiiXplLQgBCEpCvXQ3dnkn0SdaZSYdQ+FY= +golang.org/x/sys v0.45.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/tests/gold_tests/tls/tls_engine_abort.test.py b/tests/gold_tests/tls/tls_engine_abort.test.py index f2756e55ce3..06ffbfe56ad 100644 --- a/tests/gold_tests/tls/tls_engine_abort.test.py +++ b/tests/gold_tests/tls/tls_engine_abort.test.py @@ -42,7 +42,8 @@ # A wide pause window (well beyond the abort client's 0.4s handshake attempt) # so the abort reliably lands while the async job is still in flight. -Test.PrepareTestPlugin(async_handshake, ts, '-delay-ms=2000') +if os.path.isfile(async_handshake): + Test.PrepareTestPlugin(async_handshake, ts, '-delay-ms=2000') server.addResponse( "sessionlog.json", { diff --git a/tests/tools/plugins/ssl_client_verify_test.cc b/tests/tools/plugins/ssl_client_verify_test.cc index 7f3ae7c3759..a4243818e05 100644 --- a/tests/tools/plugins/ssl_client_verify_test.cc +++ b/tests/tools/plugins/ssl_client_verify_test.cc @@ -58,7 +58,7 @@ check_names(X509 *cert) bool retval = false; // Check the common name - X509_NAME *subject = X509_get_subject_name(cert); + auto *subject = X509_get_subject_name(cert); if (subject) { int pos = -1; for (; !retval;) { @@ -67,10 +67,10 @@ check_names(X509 *cert) break; } - X509_NAME_ENTRY *e = X509_NAME_get_entry(subject, pos); - ASN1_STRING *cn = X509_NAME_ENTRY_get_data(e); - char *subj_name = strndup(reinterpret_cast(ASN1_STRING_get0_data(cn)), ASN1_STRING_length(cn)); - retval = check_name(subj_name); + auto *e = X509_NAME_get_entry(subject, pos); + auto *cn = X509_NAME_ENTRY_get_data(e); + char *subj_name = strndup(reinterpret_cast(ASN1_STRING_get0_data(cn)), ASN1_STRING_length(cn)); + retval = check_name(subj_name); free(subj_name); } }