From 09873147aa7c24f6337bc4d7c6e878051a9b210c Mon Sep 17 00:00:00 2001 From: bneradt Date: Tue, 21 Jul 2026 13:56:30 -0500 Subject: [PATCH] Allow dynamic TLS record sizing The documented -1 value for proxy.config.ssl.max_record_size is incorrectly rejected by records validation, leaving dynamic TLS record sizing unreachable from records.yaml. This widens the accepted range to include the dynamic sentinel, clarifies the documented modes, and extends TLS wire-level coverage to verify the small-to-large record transition. Fixes: #13288 --- doc/admin-guide/files/records.yaml.en.rst | 5 +- src/records/RecordsConfig.cc | 2 +- tests/gold_tests/tls/tls_record_size.test.py | 72 ++++++++++--------- .../gold_tests/tls/tls_record_size_client.py | 50 +++++++++++-- 4 files changed, 86 insertions(+), 43 deletions(-) diff --git a/doc/admin-guide/files/records.yaml.en.rst b/doc/admin-guide/files/records.yaml.en.rst index 86e25df130d..628bab60514 100644 --- a/doc/admin-guide/files/records.yaml.en.rst +++ b/doc/admin-guide/files/records.yaml.en.rst @@ -4244,8 +4244,9 @@ SSL Termination This configuration specifies the maximum number of bytes to write into a SSL record when replying over a SSL session. In some circumstances this setting can improve response latency by reducing - buffering at the SSL layer. This setting can have a value between 0 - and 16383 (max TLS record size). + buffering at the SSL layer. This setting accepts ``-1`` for dynamic + sizing, ``0`` for the default behavior, or a fixed maximum between + ``1`` and ``16383`` bytes. The default of ``0`` means to always write all available data into a single SSL record. diff --git a/src/records/RecordsConfig.cc b/src/records/RecordsConfig.cc index 39fc437353f..9b709ed1d0f 100644 --- a/src/records/RecordsConfig.cc +++ b/src/records/RecordsConfig.cc @@ -1230,7 +1230,7 @@ static constexpr RecordElement RecordsConfig[] = , {RECT_CONFIG, "proxy.config.ssl.origin_session_cache.size", RECD_INT, "10240", RECU_RESTART_TS, RR_NULL, RECC_NULL, nullptr, RECA_NULL} , - {RECT_CONFIG, "proxy.config.ssl.max_record_size", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-16383]", RECA_NULL} + {RECT_CONFIG, "proxy.config.ssl.max_record_size", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_INT, "[-1-16383]", RECA_NULL} , {RECT_CONFIG, "proxy.config.ssl.hsts_max_age", RECD_INT, "-1", RECU_DYNAMIC, RR_NULL, RECC_STR, "^-?[0-9]+$", RECA_NULL} , diff --git a/tests/gold_tests/tls/tls_record_size.test.py b/tests/gold_tests/tls/tls_record_size.test.py index b40c33a6fa5..9b497079539 100644 --- a/tests/gold_tests/tls/tls_record_size.test.py +++ b/tests/gold_tests/tls/tls_record_size.test.py @@ -1,7 +1,7 @@ ''' -Exercise the TLS record-size clamp (proxy.config.ssl.max_record_size > 0): on a -large TLS download the body must arrive intact and every application-data record -on the wire must be clamped to the configured size. +Exercise fixed and dynamic TLS record sizing. On a large TLS download the body +must arrive intact and application-data records on the wire must follow the +configured sizing strategy. ''' # Licensed to the Apache Software Foundation (ASF) under one # or more contributor license agreements. See the NOTICE file @@ -19,30 +19,22 @@ # See the License for the specific language governing permissions and # limitations under the License. -# NOTE: only the positive (fixed-clamp) branch of the record-sizing logic is -# covered here. The documented dynamic mode (max_record_size == -1) cannot be -# enabled through records.yaml because the record's validity check is [0-16383], -# which rejects -1; that inconsistency is pre-existing, so the dynamic branch -# stays uncovered by design. - import os import sys Test.Summary = __doc__ -class TestRecordSizeClamp: - '''Verify max_record_size clamps every record of a large TLS download.''' - - # Comfortably larger than the clamp so many records pass through it. - _body_len: int = 1024 * 1024 - _max_record: int = 4096 +class TestRecordSize: + '''Verify fixed and dynamic TLS record sizing on large downloads.''' _server_counter: int = 0 _ts_counter: int = 0 - def __init__(self) -> None: + def __init__(self, max_record: int, body_len: int) -> None: '''Declare the test Processes.''' + self._max_record = max_record + self._body_len = body_len self._server = self._configure_server() self._ts = self._configure_trafficserver() @@ -51,15 +43,15 @@ def _configure_server(self) -> 'Process': :return: The origin server Process. ''' - server = Test.MakeOriginServer(f'server-{TestRecordSizeClamp._server_counter}') - TestRecordSizeClamp._server_counter += 1 + server = Test.MakeOriginServer(f'server-{TestRecordSize._server_counter}') + TestRecordSize._server_counter += 1 - body = "x" * TestRecordSizeClamp._body_len + body = "x" * self._body_len request_header = {"headers": "GET /obj HTTP/1.1\r\nHost: ex.test\r\n\r\n", "timestamp": "1469733493.993", "body": ""} response_header = { "headers": "HTTP/1.1 200 OK\r\nServer: microserver\r\nConnection: close\r\n" - f"Cache-Control: max-age=3600\r\nContent-Length: {TestRecordSizeClamp._body_len}\r\n\r\n", + f"Cache-Control: max-age=3600\r\nContent-Length: {self._body_len}\r\n\r\n", "timestamp": "1469733493.993", "body": body } @@ -67,12 +59,12 @@ def _configure_server(self) -> 'Process': return server def _configure_trafficserver(self) -> 'Process': - '''Configure Traffic Server with a positive max_record_size clamp. + '''Configure Traffic Server with the requested record-size strategy. :return: The Traffic Server Process. ''' - ts = Test.MakeATSProcess(f'ts-{TestRecordSizeClamp._ts_counter}', enable_tls=True) - TestRecordSizeClamp._ts_counter += 1 + ts = Test.MakeATSProcess(f'ts-{TestRecordSize._ts_counter}', enable_tls=True) + TestRecordSize._ts_counter += 1 ts.addDefaultSSLFiles() ts.Disk.ssl_multicert_yaml.AddLines( @@ -87,31 +79,45 @@ def _configure_trafficserver(self) -> 'Process': { 'proxy.config.ssl.server.cert.path': f'{ts.Variables.SSLDir}', 'proxy.config.ssl.server.private_key.path': f'{ts.Variables.SSLDir}', - # Positive cap -> the write path clamps each TLS record to this many bytes. - 'proxy.config.ssl.max_record_size': TestRecordSizeClamp._max_record, + 'proxy.config.ssl.max_record_size': self._max_record, }) + if self._max_record == -1: + ts.Disk.traffic_out.Content = Testers.ExcludesExpression( + r'proxy\.config\.ssl\.max_record_size.*Validity Check error', + 'The dynamic record-size sentinel should pass records validation') return ts def run(self) -> None: '''Configure and run the TestRun. - The client downloads the object and measures the TLS records on the wire, - asserting both that the body is intact and that no application-data record - exceeds the configured clamp. + The client downloads the object and measures the TLS records on the wire. ''' - tr = Test.AddTestRun("max_record_size>0 clamps records on a large TLS download") + if self._max_record == -1: + description = 'max_record_size=-1 dynamically sizes records on a large TLS download' + client_option = '--dynamic' + expected_output = 'PASS: TLS records ramp from small to large after the dynamic threshold' + else: + description = 'max_record_size>0 clamps records on a large TLS download' + client_option = f'--max-record {self._max_record}' + expected_output = 'PASS: every application-data record is within the configured clamp' + + tr = Test.AddTestRun(description) tr.Processes.Default.StartBefore(self._server) tr.Processes.Default.StartBefore(self._ts) tr.Processes.Default.Command = ( f'{sys.executable} {os.path.join(Test.TestDirectory, "tls_record_size_client.py")} ' f'-p {self._ts.Variables.ssl_port} --host ex.test --path /obj ' - f'--max-record {TestRecordSizeClamp._max_record} --expect-bytes {TestRecordSizeClamp._body_len}') + f'{client_option} --expect-bytes {self._body_len}') tr.Processes.Default.ReturnCode = 0 tr.Processes.Default.Streams.All += Testers.ContainsExpression( - "PASS: every application-data record is within the configured clamp", - "every TLS record must be clamped to the configured size") + expected_output, 'TLS records must follow the configured sizing strategy') tr.StillRunningAfter = self._ts tr.StillRunningAfter = self._server -TestRecordSizeClamp().run() +# The fixed-size test response is comfortably larger than its 4,096-byte clamp, +# ensuring that many records exercise the clamp. The dynamic-sizing test +# response must exceed its 1,000,000-byte threshold by enough data to demonstrate +# both phases. +TestRecordSize(4096, 1024 * 1024).run() +TestRecordSize(-1, 2 * 1024 * 1024).run() diff --git a/tests/gold_tests/tls/tls_record_size_client.py b/tests/gold_tests/tls/tls_record_size_client.py index a169944e5c4..b5a5ddce2c1 100644 --- a/tests/gold_tests/tls/tls_record_size_client.py +++ b/tests/gold_tests/tls/tls_record_size_client.py @@ -1,8 +1,8 @@ #!/usr/bin/env python3 ''' Download an object from ATS over TLS and inspect the TLS records on the wire: -confirm the body arrives intact AND that every application-data record is no -larger than the configured proxy.config.ssl.max_record_size (plus AEAD overhead). +confirm the body arrives intact AND that application-data records follow the +configured fixed or dynamic record-size strategy. A MemoryBIO drives the handshake so the raw ciphertext stream is visible; the 5-byte TLS record headers (type, version, length) are in cleartext, so record @@ -34,12 +34,16 @@ from collections.abc import Iterator TLS_APPLICATION_DATA = 23 +TLS12_GCM_OVERHEAD = 24 # A clamped plaintext record becomes ciphertext of plaintext + AEAD overhead # (TLS1.2 GCM: 8-byte explicit nonce + 16-byte tag = 24 bytes; the cipher is pinned # to AEAD below). 256 is a generous ceiling over that, far below an unclamped ~16 KB # record, so the clamp check stays decisive and cannot be tripped by the larger, # variable expansion of a CBC suite. RECORD_OVERHEAD = 256 +DYNAMIC_SMALL_RECORD = 1300 +DYNAMIC_MAX_RECORD = 16383 +DYNAMIC_BYTE_THRESHOLD = 1_000_000 def iter_record_lengths(buf: bytes | bytearray) -> Iterator[tuple[int, int]]: @@ -54,12 +58,42 @@ def iter_record_lengths(buf: bytes | bytearray) -> Iterator[tuple[int, int]]: i += 5 + length +def verify_dynamic_records(app_lengths: list[int]) -> bool: + '''Verify records ramp from single-segment to maximum-sized records.''' + small_limit = DYNAMIC_SMALL_RECORD + TLS12_GCM_OVERHEAD + max_limit = DYNAMIC_MAX_RECORD + TLS12_GCM_OVERHEAD + first_large = next((i for i, length in enumerate(app_lengths) if length > small_limit), None) + + if first_large is None: + print('FAIL: dynamic sizing never ramped up to large TLS records') + return False + + plaintext_before_ramp = sum(length - TLS12_GCM_OVERHEAD for length in app_lengths[:first_large]) + if plaintext_before_ramp < DYNAMIC_BYTE_THRESHOLD: + print( + f'FAIL: dynamic sizing ramped after only {plaintext_before_ramp} plaintext bytes; ' + f'expected at least {DYNAMIC_BYTE_THRESHOLD}') + return False + + max_record = max(app_lengths) + if max_record > max_limit: + print(f'FAIL: a dynamic application-data record ({max_record}) exceeds the maximum ({max_limit})') + return False + + print( + f'PASS: TLS records ramp from small to large after the dynamic threshold ' + f'(first_large={first_large}, plaintext_before_ramp={plaintext_before_ramp}, max_record_len={max_record})') + return True + + def main() -> int: parser = argparse.ArgumentParser(description='Measure ATS TLS record sizes on a download.') parser.add_argument('-p', '--port', type=int, required=True, help='ATS TLS port') parser.add_argument('--host', default='ex.test', help='Host header / SNI') parser.add_argument('--path', default='/obj', help='request path') - parser.add_argument('--max-record', type=int, required=True, help='configured proxy.config.ssl.max_record_size') + sizing = parser.add_mutually_exclusive_group(required=True) + sizing.add_argument('--max-record', type=int, help='positive proxy.config.ssl.max_record_size clamp') + sizing.add_argument('--dynamic', action='store_true', help='expect dynamic TLS record sizing') parser.add_argument('--expect-bytes', type=int, required=True, help='expected response body length') args = parser.parse_args() @@ -132,11 +166,8 @@ def feed() -> bytes: app_lengths = [length for content_type, length in iter_record_lengths(raw) if content_type == TLS_APPLICATION_DATA] max_record = max(app_lengths) if app_lengths else 0 - limit = args.max_record + RECORD_OVERHEAD - print( - f'app_data_records={len(app_lengths)} max_record_len={max_record} ' - f'limit={limit} body_len={body_len} expect={args.expect_bytes}') + print(f'app_data_records={len(app_lengths)} max_record_len={max_record} body_len={body_len} expect={args.expect_bytes}') if body_len != args.expect_bytes: print(f'FAIL: body length {body_len} != expected {args.expect_bytes}') @@ -144,6 +175,11 @@ def feed() -> bytes: if len(app_lengths) < 2: print('FAIL: too few application-data records to judge clamping') return 1 + if args.dynamic: + return 0 if verify_dynamic_records(app_lengths) else 1 + + assert args.max_record is not None + limit = args.max_record + RECORD_OVERHEAD if max_record > limit: print(f'FAIL: an application-data record ({max_record}) exceeds the clamp + overhead ({limit})') return 1