From 0909f575cfed0441be15fa260ee203d98ff02611 Mon Sep 17 00:00:00 2001 From: Brian Neradt Date: Thu, 30 Jul 2026 19:13:13 -0500 Subject: [PATCH 01/16] Stabilize proxy protocol access log test (#13462) Access log records for independent PROXY Protocol replay sessions can be finalized on different event threads. The test assumed replay order and failed intermittently when two otherwise correct records were reversed. This waits for all expected records and sorts them before comparing against the full gold output. It preserves content and count coverage without requiring transaction completion order. (cherry picked from commit df93dd75ef0da1ff035478735989d3996e1aea5b) --- .../gold_tests/proxy_protocol/gold/access-cp.gold | 4 ++-- .../proxy_protocol/gold/access-nocp.gold | 4 ++-- .../proxy_protocol/proxy_protocol.test.py | 15 +++++++++++---- 3 files changed, 15 insertions(+), 8 deletions(-) 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() From 53f07be0aeb05eb7ef2a77f36f8d7a5077b3a614 Mon Sep 17 00:00:00 2001 From: Brian Neradt Date: Fri, 31 Jul 2026 10:55:25 -0500 Subject: [PATCH 02/16] Raise HTTP/2 SETTINGS limits (#13444) HTTP/2 extensions can add enough SETTINGS parameters to exceed the defaults, causing Traffic Server to close otherwise valid connections before a request. This occurs with Meta forward proxy traffic that advertises WebTransport-over-HTTP/2 settings. This raises the per-frame limit to 16 and the per-minute limit to 32, retaining abuse protections while allowing protocol growth. This also adds an AuTest that sends an extension-rich SETTINGS frame and verifies that the subsequent request succeeds. Fixes: #13443 (cherry picked from commit 89fdedb504e13c7e2a9e8eb59d6e76627ed2c68c) --- doc/admin-guide/files/records.yaml.en.rst | 4 +- src/proxy/http2/HTTP2.cc | 4 +- src/records/RecordsConfig.cc | 4 +- .../h2/clients/h2_extension_settings.py | 156 ++++++++++++++++++ tests/gold_tests/h2/http2.test.py | 9 + .../gold_tests/records/gold/full_records.yaml | 4 +- .../records/legacy_config/full_records.config | 4 +- 7 files changed, 175 insertions(+), 10 deletions(-) create mode 100644 tests/gold_tests/h2/clients/h2_extension_settings.py diff --git a/doc/admin-guide/files/records.yaml.en.rst b/doc/admin-guide/files/records.yaml.en.rst index 00f000a5e82..89b9401abe6 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. 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/records/RecordsConfig.cc b/src/records/RecordsConfig.cc index 2065d87b09c..1afa50c42a0 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} , 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/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 From 99e8ceee9fef4bc4499e8f0eefd2fdaa46fd76e4 Mon Sep 17 00:00:00 2001 From: JosiahWI <41302989+JosiahWI@users.noreply.github.com> Date: Fri, 31 Jul 2026 10:59:45 -0500 Subject: [PATCH 03/16] Add tests for OpenSSL cert loading (#13350) This does not cover the ENGINE support, because that is a complicated test and the support will be removed for OpenSSL 4. (cherry picked from commit 56ddbbb34fbe7db767035190831920cfa8754a86) --- src/iocore/net/unit_tests/test_SSLDHParams.cc | 183 ++++++++++++++++-- src/iocore/net/unit_tests/unit_test_main.cc | 5 + 2 files changed, 175 insertions(+), 13 deletions(-) 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/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); From d14c803426dab1f15423204f4d91993bee1ddd8b Mon Sep 17 00:00:00 2001 From: Mo Chen Date: Fri, 31 Jul 2026 14:16:00 -0500 Subject: [PATCH 04/16] TLS: Fix EVP_PKEY leak in SSLPrivateKeyHandler (#13464) SSL_CTX_use_PrivateKey() takes its own reference on the key, so the reference from PEM_read_bio_PrivateKey() belongs to the caller. SSLPrivateKeyHandler() released it only when attaching the key failed, so a successful load leaked one EVP_PKEY per certificate. This repeats at startup, on every config reload, and on every secret or certificate update. Hold the key in a scoped_PKEY so every exit releases it. (cherry picked from commit 07aa06c79b829faa570acb52b453b3c0f26cff18) --- src/iocore/net/P_SSLUtils.h | 9 +++++++++ src/iocore/net/SSLUtils.cc | 15 ++++++++------- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/src/iocore/net/P_SSLUtils.h b/src/iocore/net/P_SSLUtils.h index af5728e4869..4fb3a39363a 100644 --- a/src/iocore/net/P_SSLUtils.h +++ b/src/iocore/net/P_SSLUtils.h @@ -114,6 +114,14 @@ namespace detail } }; + struct PKEYDeleter { + void + operator()(EVP_PKEY *p) + { + EVP_PKEY_free(p); + } + }; + #ifdef OPENSSL_IS_OPENSSL3 struct PKEYCTXDeleter { void @@ -156,6 +164,7 @@ struct ats_wildcard_matcher { using scoped_X509 = std::unique_ptr; using scoped_BIO = std::unique_ptr; +using scoped_PKEY = std::unique_ptr; #ifdef OPENSSL_IS_OPENSSL3 using scoped_PKEY_CTX = std::unique_ptr; using scoped_Decoder_CTX = std::unique_ptr; diff --git a/src/iocore/net/SSLUtils.cc b/src/iocore/net/SSLUtils.cc index 0a6750b98ae..3382b5369c3 100644 --- a/src/iocore/net/SSLUtils.cc +++ b/src/iocore/net/SSLUtils.cc @@ -952,15 +952,16 @@ 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; + // SSL_CTX_use_PrivateKey() takes its own reference on the key, so this + // reference must be released on every exit. + scoped_PKEY pkey; #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); + pkey.reset(ENGINE_load_private_key(e, keyPath, nullptr, nullptr)); if (pkey) { - if (!SSL_CTX_use_PrivateKey(ctx, pkey)) { + if (!SSL_CTX_use_PrivateKey(ctx, pkey.get())) { Dbg(dbg_ctl_ssl_load, "failed to load server private key from engine"); - EVP_PKEY_free(pkey); return false; } } @@ -973,16 +974,16 @@ SSLPrivateKeyHandler(SSL_CTX *ctx, const char *keyPath, const char *secret_data, 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); + + pkey.reset(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)) { + 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); - EVP_PKEY_free(pkey); return false; } if (e == nullptr && !SSL_CTX_check_private_key(ctx)) { From ca5f3a3e24cb48ecfcf2a8edeb142ab807df6407 Mon Sep 17 00:00:00 2001 From: Brian Neradt Date: Fri, 31 Jul 2026 17:04:45 -0500 Subject: [PATCH 05/16] Release logging thread continuations (#13466) Dedicated logging continuations remain allocated after their worker loops return during shutdown. Once shutdown reliably completes, LeakSanitizer can report the flush continuation as an 80-byte leak. Release both continuations after their loops finish. This matches the ownership model used by other dedicated event threads. (cherry picked from commit b080c8386cf5a10a5cada3ffef7f4b0d08c4861a) --- src/proxy/logging/Log.cc | 2 ++ 1 file changed, 2 insertions(+) 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; } From 9bafe093f5580eccf7f2a5d6222ae9faa68fbaae Mon Sep 17 00:00:00 2001 From: Brian Neradt Date: Fri, 31 Jul 2026 17:05:27 -0500 Subject: [PATCH 06/16] Make log pipe test tolerate restricted kernels (#13431) Some CI environments do not permit unprivileged processes to increase FIFO buffer sizes, even when the requested size is below the system maximum. The log_pipe AuTest treats this kernel policy as a product failure and cannot run in those environments. This makes the test accept only the corresponding `Operation not permitted` diagnostic when the FIFO remains unchanged, while continuing to reject other errors and fatal diagnostics. It also waits briefly for asynchronous diagnostic output before deciding whether the size check failed. (cherry picked from commit f063c0eb9a60b400bf0d5be8a494f808f286dfe8) --- tests/gold_tests/logging/log_pipe.test.py | 18 ++++++++++--- .../logging/pipe_buffer_is_larger_than.py | 26 +++++++++++++++---- 2 files changed, 35 insertions(+), 9 deletions(-) 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/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__': From f1b771cf16627e6d0e65cbb4913d16fccf9cf26d Mon Sep 17 00:00:00 2001 From: Brian Neradt Date: Fri, 31 Jul 2026 17:06:50 -0500 Subject: [PATCH 07/16] healthchecks: reference count status file data (#13432) Replaced file data was retired onto a freelist and freed after a timeout, but its deadline came from a timestamp taken before a blocking inotify read. That stale deadline let transactions retain references to data that was already freed. Files exactly 16 KiB long were also reported as empty because a final zero-byte read overwrote the saved length. This holds each immutable snapshot in an atomic shared_ptr. Every transaction pins its snapshot, so data lives exactly as long as it is referenced, without a freelist or request-path mutex. This also preserves the last successful file-read length and adds AuTest coverage for concurrent replacement and the 16 KiB boundary. Fixes: #8735 (cherry picked from commit 16bd59ab4ca21f5d0f1a15cfd9d5be22f106d5f0) --- plugins/healthchecks/healthchecks.cc | 201 +++++++++--------- .../healthchecks/healthchecks.test.py | 54 +++++ 2 files changed, 150 insertions(+), 105 deletions(-) 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/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() From 5633e149465d3e9d30410db22bbe64781d071eb1 Mon Sep 17 00:00:00 2001 From: JosiahWI <41302989+JosiahWI@users.noreply.github.com> Date: Fri, 31 Jul 2026 17:35:16 -0500 Subject: [PATCH 08/16] Rename `OPENSSL_IS_X` -> `OPENSSL_IS_AT_LEAST_X` (#13469) This is for consistency since there is now OpenSSL 4, and we need to add a macro definition for OpenSSL 4 as well. (cherry picked from commit 80c01c262ee51736d8d43f8dc859446550496dc9) --- CMakeLists.txt | 6 ++--- .../plugins/c-api/verify_cert/verify_cert.cc | 2 +- include/cripts/Connections.hpp | 2 +- .../access_control/unit_tests/test_utils.cc | 2 +- plugins/experimental/sslheaders/sslheaders.cc | 2 +- plugins/experimental/wasm/ats_context.cc | 14 +++++------ plugins/lua/ts_lua_client_request.cc | 24 +++++++++---------- src/iocore/net/CMakeLists.txt | 2 +- src/iocore/net/P_SSLNetVConnection.h | 2 +- src/iocore/net/P_SSLUtils.h | 6 ++--- src/iocore/net/SSLClientUtils.cc | 2 +- src/iocore/net/SSLKeyUtils.cc | 8 +++---- src/iocore/net/SSLKeyUtils.h | 4 ++-- src/iocore/net/SSLNetVConnection.cc | 4 ++-- src/iocore/net/SSLUtils.cc | 2 +- src/tscore/CMakeLists.txt | 2 +- 16 files changed, 42 insertions(+), 42 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index a03497767ec..0c50e3200ec 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -313,8 +313,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}") @@ -630,7 +630,7 @@ check_symbol_exists(ENGINE_get_default_RSA "openssl/engine.h" HAVE_ENGINE_GET_DE 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/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/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/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/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/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/src/iocore/net/CMakeLists.txt b/src/iocore/net/CMakeLists.txt index 8ef85f25ef2..9a9acc059d9 100644 --- a/src/iocore/net/CMakeLists.txt +++ b/src/iocore/net/CMakeLists.txt @@ -146,7 +146,7 @@ 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() # Use link groups to solve circular dependency 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 4fb3a39363a..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 @@ -122,7 +122,7 @@ namespace detail } }; -#ifdef OPENSSL_IS_OPENSSL3 +#ifdef OPENSSL_IS_AT_LEAST_OPENSSL3 struct PKEYCTXDeleter { void operator()(EVP_PKEY_CTX *pctx) @@ -165,7 +165,7 @@ struct ats_wildcard_matcher { using scoped_X509 = std::unique_ptr; using scoped_BIO = std::unique_ptr; using scoped_PKEY = std::unique_ptr; -#ifdef OPENSSL_IS_OPENSSL3 +#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/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 3382b5369c3..7da567f9717 100644 --- a/src/iocore/net/SSLUtils.cc +++ b/src/iocore/net/SSLUtils.cc @@ -57,7 +57,7 @@ #include #include #include -#ifdef OPENSSL_IS_OPENSSL3 +#ifdef OPENSSL_IS_AT_LEAST_OPENSSL3 #include #endif #include 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) From 944fb6d97d7b7759cde178dc0e0ab57f21ea8f3f Mon Sep 17 00:00:00 2001 From: Brian Neradt Date: Fri, 31 Jul 2026 18:15:38 -0500 Subject: [PATCH 09/16] Enable OpenSSL QUIC when available (#13468) OpenSSL 3.5 provides the QUIC implementation ATS needs to accept HTTP/3 connections from downstream clients. This affects client-to-ATS traffic, not ATS-to-origin traffic. Requiring an extra opt-in leaves capable builds without client-facing HTTP/3, but merely compiling QUIC should not make deployments without QUIC listeners load every TLS certificate twice. This makes native OpenSSL QUIC an automatic CMake option. It activates only with upstream OpenSSL's native server API, remains disabled when quiche is selected, and preserves explicit ON and OFF overrides. This also starts the QUIC processor and maintains its certificate table only when a QUIC listener is configured, avoiding the duplicate load when HTTP/3 is unused. (cherry picked from commit 3ccfb197eeae5e498282d04f53c86f6eaf63f22d) --- CMakeLists.txt | 53 +++++++++++++------ src/iocore/net/SSLClientCoordinator.cc | 5 +- src/traffic_server/traffic_server.cc | 4 +- .../tls/ssl_multicert_loader.test.py | 2 + 4 files changed, 46 insertions(+), 18 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 0c50e3200ec..795a714dc38 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)") @@ -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() 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/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/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 From 92f3f3c8f2d5f41d3ad6c20a71fc09cd25e33a17 Mon Sep 17 00:00:00 2001 From: Brian Neradt Date: Fri, 31 Jul 2026 18:16:05 -0500 Subject: [PATCH 10/16] Fix exact URL cache config matching (#13438) Exact URL entries in cache.config are dropped after successful parsing, so their cache actions never apply. Failed entries are inserted instead, which also defeats duplicate detection. Correct the success check and add unit and replay coverage for matching, nonmatching, invalid, and duplicate rules. Extend ATSReplayTest to accept cache.config entries with dynamic server ports. Fixes: #13421 (cherry picked from commit d05506e2c32081fb7c0d96355fab24ae974cf9f8) --- doc/developer-guide/testing/autests.en.rst | 1 + src/proxy/ControlMatcher.cc | 2 +- src/proxy/unit_tests/CMakeLists.txt | 4 +- src/proxy/unit_tests/test_ControlMatcher.cc | 118 ++++++++++++++++ .../autest-site/ats_replay.test.ext | 7 + .../gold_tests/cache/cache-exact-url.test.py | 24 ++++ .../cache/replay/cache-exact-url.replay.yaml | 126 ++++++++++++++++++ 7 files changed, 280 insertions(+), 2 deletions(-) create mode 100644 src/proxy/unit_tests/test_ControlMatcher.cc create mode 100644 tests/gold_tests/cache/cache-exact-url.test.py create mode 100644 tests/gold_tests/cache/replay/cache-exact-url.replay.yaml 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/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/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/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 From c4595b239e63d6b3af9f47aa382c74f982a2f742 Mon Sep 17 00:00:00 2001 From: Brian Neradt Date: Mon, 3 Aug 2026 14:08:56 -0500 Subject: [PATCH 11/16] Guard quiche-only QUIC connection state (#13478) Native OpenSSL QUIC builds do not use the connection table or quiche timeout event stored by QUICNetVConnection. Clang diagnoses those quiche-only private fields as unused, causing warnings-as-errors builds to fail. This guards the members with TS_HAS_QUICHE so native OpenSSL builds do not declare them while quiche builds retain the required state. (cherry picked from commit 5d878a4be2d6d8a22d11977e3811f5a4db948945) --- src/iocore/net/P_QUICNetVConnection.h | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) 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(); From 9960332e3369867a1affe6fc99b4bdaad1d281a6 Mon Sep 17 00:00:00 2001 From: Brian Neradt Date: Mon, 3 Aug 2026 17:39:15 -0500 Subject: [PATCH 12/16] Guard pre-init log wakeups (#13472) Plugins can fill a text log buffer during TSPluginInit before logging preprocessing threads and their notification objects exist. This causes a null dereference that crashes traffic_server during startup. This problem is addressed in this patch by queuing those buffers without signaling until the logging workers exist. Their initial queue scan then flushes the pending data. A regression test forces a rollover during plugin initialization and verifies ATS stays up and preserves the log entry. With the production fix removed, the regression test caused traffic_server to crash with SIGSEGV at address zero while signaling the preprocessing notification. With the fix applied, the identical test passes. (cherry picked from commit b279b9f5cad5f4bdbe13fe3b86bad45cbc8109c9) --- src/proxy/logging/LogObject.cc | 8 +++- .../logging/log_plugin_init.test.py | 38 +++++++++++++++++++ tests/tools/plugins/test_log_interface.cc | 11 +++++- 3 files changed, 54 insertions(+), 3 deletions(-) create mode 100644 tests/gold_tests/logging/log_plugin_init.test.py 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/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/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)); } From ebad97a1e37d094211e171a90c18cf7bb901f5be Mon Sep 17 00:00:00 2001 From: JosiahWI <41302989+JosiahWI@users.noreply.github.com> Date: Mon, 3 Aug 2026 18:19:22 -0500 Subject: [PATCH 13/16] Fix some copy instead of move Coverity CIDs (#13470) * Fix some copy instead of move Coverity CIDs CIDs fixed in this patch: 1664317 1664316 1664286 1664284 1663965 1660036 1658876 1658839 1658837 1658799 1654583 1654213 1654211 1654210 1654207 1654206 1645817 1645816 1645814 1645813 1645811 1645809 1645808 1645806 1645804 1645802 1645801 1645799 1645798 1645796 1645795 1644303 1644300 1644293 1644255 1644242 1644227 1644207 (cherry picked from commit 62016861cf314d6926a8c1375f726e53d7e35761) Partial cherry-pick: the hunks for src/config/ssl_multicert.cc, src/config/unit_tests/test_ssl_multicert.cc and src/records/unit_tests/test_ReloadDirectives.cc are omitted because those files do not exist on 10.2.x, as is the src/proxy/Plugin.cc hunk, which depends on the master-only plugin load summary. --- include/iocore/net/ConnectionTracker.h | 3 ++- include/mgmt/config/ConfigReloadTrace.h | 5 +++-- plugins/experimental/filter_body/filter_body.cc | 5 +++-- plugins/experimental/jax_fingerprint/ja4/test.cc | 3 ++- plugins/experimental/jax_fingerprint/ja4h/test.cc | 5 +++-- plugins/experimental/txn_box/plugin/src/Comparison.cc | 3 ++- plugins/experimental/txn_box/plugin/src/Config.cc | 3 ++- plugins/experimental/txn_box/plugin/src/ip_space.cc | 2 +- plugins/prefetch/path.cc | 3 ++- plugins/stats_over_http/stats_over_http.cc | 3 ++- plugins/webp_transform/ImageTransform.cc | 4 ++-- src/api/InkAPI.cc | 6 ++++-- src/iocore/cache/CacheDir.cc | 3 ++- src/iocore/net/unit_tests/test_SSLSNIConfig.cc | 3 ++- src/mgmt/config/ConfigReloadTrace.cc | 5 +++-- src/mgmt/config/ReloadCoordinator.cc | 6 +++--- src/mgmt/rpc/handlers/config/Configuration.cc | 3 ++- src/proxy/logging/LogField.cc | 4 ++-- src/traffic_cache_tool/CacheDefs.h | 3 ++- src/traffic_ctl/CtrlCommands.cc | 7 ++++--- src/traffic_ctl/jsonrpc/ctrl_yaml_codecs.h | 4 +++- 21 files changed, 51 insertions(+), 32 deletions(-) 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/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/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/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/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/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/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/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/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/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()}); } From f6d818be4f1ff35434fa9611780b825665bfdd22 Mon Sep 17 00:00:00 2001 From: JosiahWI <41302989+JosiahWI@users.noreply.github.com> Date: Mon, 3 Aug 2026 18:19:42 -0500 Subject: [PATCH 14/16] Remove OpenSSL ENGINE code (#13471) * Remove OpenSSL ENGINE code Commit a966bc4cce (#11219) accidentally disabled OpenSSL ENGINE support entirely. Although it was unintentional, it seems clear no one is using that API by this point (no one has reported it was broken), and the API is gone in recent OpenSSL versions. This patch removes the dead logic. (cherry picked from commit e7eb77a5c3acc2eecd651b1f6e60c74dc857970c) --- CMakeLists.txt | 3 -- src/iocore/net/SSLUtils.cc | 58 +++++++++++--------------------------- 2 files changed, 17 insertions(+), 44 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 795a714dc38..b7ed086d395 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -646,9 +646,6 @@ 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_AT_LEAST_OPENSSL3) diff --git a/src/iocore/net/SSLUtils.cc b/src/iocore/net/SSLUtils.cc index 7da567f9717..22b93de583f 100644 --- a/src/iocore/net/SSLUtils.cc +++ b/src/iocore/net/SSLUtils.cc @@ -62,9 +62,6 @@ #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}; @@ -954,42 +947,25 @@ SSLPrivateKeyHandler(SSL_CTX *ctx, const char *keyPath, const char *secret_data, { // SSL_CTX_use_PrivateKey() takes its own reference on the key, so this // reference must be released on every exit. - scoped_PKEY pkey; -#if HAVE_ENGINE_GET_DEFAULT_RSA && HAVE_ENGINE_LOAD_PRIVATE_KEY - ENGINE *e = ENGINE_get_default_RSA(); - if (e != nullptr) { - pkey.reset(ENGINE_load_private_key(e, keyPath, nullptr, nullptr)); - if (pkey) { - if (!SSL_CTX_use_PrivateKey(ctx, pkey.get())) { - Dbg(dbg_ctl_ssl_load, "failed to load server private key from engine"); - return false; - } - } - } -#else - void *e = nullptr; -#endif - if (pkey == nullptr) { - scoped_BIO bio(BIO_new_mem_buf(secret_data, secret_data_len)); + 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); + pem_password_cb *password_cb = SSL_CTX_get_default_passwd_cb(ctx); + void *u = SSL_CTX_get_default_passwd_cb_userdata(ctx); - pkey.reset(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.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 (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; - } + 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; + } + 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; From ceda35a0c717acb2e9d11ad34009653a9988f989 Mon Sep 17 00:00:00 2001 From: Brian Neradt Date: Mon, 3 Aug 2026 18:59:48 -0500 Subject: [PATCH 15/16] ci-fedora-cxx20 preset: use clang (#13477) Our autest fedora run exercises g++, let's get some recent clang coverage via the fedora CI run. (cherry picked from commit 6d36c38b9cf1ab9c520bd7e9cc73db47323affa2) --- CMakePresets.json | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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++" } }, { From 9868f752845d2e5be8a9456f9da2b31dd4cceff5 Mon Sep 17 00:00:00 2001 From: Brian Neradt Date: Mon, 3 Aug 2026 21:46:43 -0500 Subject: [PATCH 16/16] Make QUIC token secrets configurable (#13397) QUIC address-validation and stateless-reset tokens use fixed compile-time secrets, allowing anyone with the source to reproduce valid token MACs. This replaces the fixed values with reloadable 32-byte keys and a random per-process fallback. Multiple file keys allow rotation without immediately invalidating address-validation tokens, and HMAC-SHA256 protects all token types. This also rejects malformed tokens before parsing and adds coverage for file loading, key rotation, invalid files, and fallback-key stability. (cherry picked from commit db21b1b3d4b3f6905bf3a9abfab7c7616b62cd04) --- doc/admin-guide/files/records.yaml.en.rst | 18 +- include/iocore/net/quic/QUICConfig.h | 41 ++++ include/iocore/net/quic/QUICTypes.h | 13 +- src/iocore/net/CMakeLists.txt | 3 + src/iocore/net/quic/QUICConfig.cc | 143 +++++++++++ src/iocore/net/quic/QUICTypes.cc | 222 +++++++++++++----- .../net/unit_tests/test_QUICTokenKeyConfig.cc | 138 +++++++++++ src/mgmt/config/FileManager.cc | 5 + src/records/RecordsConfig.cc | 2 + 9 files changed, 516 insertions(+), 69 deletions(-) create mode 100644 src/iocore/net/unit_tests/test_QUICTokenKeyConfig.cc diff --git a/doc/admin-guide/files/records.yaml.en.rst b/doc/admin-guide/files/records.yaml.en.rst index 89b9401abe6..3bf27624ef2 100644 --- a/doc/admin-guide/files/records.yaml.en.rst +++ b/doc/admin-guide/files/records.yaml.en.rst @@ -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/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/src/iocore/net/CMakeLists.txt b/src/iocore/net/CMakeLists.txt index 9a9acc059d9..22910c16ebe 100644 --- a/src/iocore/net/CMakeLists.txt +++ b/src/iocore/net/CMakeLists.txt @@ -149,6 +149,9 @@ if(BUILD_TESTING) 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/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/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/records/RecordsConfig.cc b/src/records/RecordsConfig.cc index 1afa50c42a0..b653740edb4 100644 --- a/src/records/RecordsConfig.cc +++ b/src/records/RecordsConfig.cc @@ -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}