moonchen commented on code in PR #13306:
URL: https://github.com/apache/trafficserver/pull/13306#discussion_r3463678619


##########
tests/gold_tests/tls/tls_record_size.test.py:
##########
@@ -0,0 +1,116 @@
+'''
+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.
+'''
+#  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.
+
+# 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
+
+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
+
+    _server_counter: int = 0
+    _ts_counter: int = 0
+
+    def __init__(self) -> None:
+        '''Declare the test Processes.'''
+        self._server = self._configure_server()
+        self._ts = self._configure_trafficserver()
+
+    def _configure_server(self) -> 'Process':
+        '''Configure the origin server with a large response body.
+
+        :return: The origin server Process.
+        '''
+        server = 
Test.MakeOriginServer(f'server-{TestRecordSizeClamp._server_counter}')
+        TestRecordSizeClamp._server_counter += 1
+
+        body = "x" * TestRecordSizeClamp._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"
+                "Cache-Control: max-age=3600\r\nContent-Length: 
{0}\r\n\r\n".format(TestRecordSizeClamp._body_len),
+            "timestamp": "1469733493.993",
+            "body": body
+        }
+        server.addResponse("sessionlog.json", request_header, response_header)
+        return server
+
+    def _configure_trafficserver(self) -> 'Process':
+        '''Configure Traffic Server with a positive max_record_size clamp.
+
+        :return: The Traffic Server Process.
+        '''
+        ts = Test.MakeATSProcess(f'ts-{TestRecordSizeClamp._ts_counter}', 
enable_tls=True)
+        TestRecordSizeClamp._ts_counter += 1
+
+        ts.addDefaultSSLFiles()
+        ts.Disk.ssl_multicert_yaml.AddLines(
+            """
+ssl_multicert:
+  - dest_ip: "*"
+    ssl_cert_name: server.pem
+    ssl_key_name: server.key
+""".split("\n"))
+        ts.Disk.remap_config.AddLine('map / 
http://127.0.0.1:{0}'.format(self._server.Variables.Port))
+        ts.Disk.records_config.update(
+            {
+                'proxy.config.ssl.server.cert.path': 
'{0}'.format(ts.Variables.SSLDir),
+                'proxy.config.ssl.server.private_key.path': 
'{0}'.format(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,
+            })
+        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.
+        '''
+        tr = Test.AddTestRun("max_record_size>0 clamps records on a large TLS 
download")
+        tr.Processes.Default.StartBefore(self._server)
+        tr.Processes.Default.StartBefore(self._ts)
+        tr.Processes.Default.Command = (
+            f'python3 {os.path.join(Test.TestDirectory, 
"tls_record_size_client.py")} '

Review Comment:
   Done — switched all the helper-launch `Command` strings in the PR to 
`{sys.executable}`. Added a note to the Writing Autests section of AGENTS.md.



##########
tests/gold_tests/tls/tls_origin_post_abort.test.py:
##########
@@ -0,0 +1,99 @@
+'''
+When a TLS origin resets the connection while ATS is still sending it a POST
+request body, ATS must fail the transaction promptly -- the transport error 
must
+reach the request-body tunnel right away as VC_EVENT_ERROR, not sit until the
+outbound inactivity timeout rescues it.
+'''
+#  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 = __doc__
+
+
+class TestOriginPostAbort:
+    '''Verify a TLS origin RST mid-POST-body fails the transaction promptly.'''
+
+    _ts_counter: int = 0
+    _origin_counter: int = 0
+
+    def __init__(self) -> None:
+        '''Declare the test Processes.'''
+        Test.GetTcpPort("origin_port")
+        self._ts = self._configure_trafficserver()
+        self._origin = self._configure_origin()
+
+    def _configure_trafficserver(self) -> 'Process':
+        '''Configure Traffic Server with a TLS origin and a generous rescue 
timeout.
+
+        :return: The Traffic Server Process.
+        '''
+        ts = Test.MakeATSProcess(f'ts-{TestOriginPostAbort._ts_counter}', 
enable_cache=False)
+        TestOriginPostAbort._ts_counter += 1
+
+        ts.Disk.records_config.update(
+            {
+                'proxy.config.url_remap.remap_required': 1,
+                'proxy.config.http.connect_attempts_max_retries': 0,
+                'proxy.config.http.connect_attempts_timeout': 15,
+                # The rescue timeout: a prompt failure must beat this by a 
wide margin.
+                'proxy.config.http.transaction_no_activity_timeout_out': 15,
+                'proxy.config.http.transaction_no_activity_timeout_in': 30,
+                # Keep the kernel send buffer small so the request body cannot 
be
+                # absorbed by the kernel: ATS must still be mid-send at the 
RST.
+                'proxy.config.net.sock_send_buffer_size_out': 65536,
+                'proxy.config.ssl.client.verify.server.policy': 'DISABLED',
+                'proxy.config.diags.debug.enabled': 0,
+                'proxy.config.diags.debug.tags': 'http|ssl|ssl_io',
+            })
+        ts.Disk.remap_config.AddLine('map /post 
https://127.0.0.1:{0}'.format(Test.Variables.origin_port))
+
+        ts.Disk.traffic_out.Content = Testers.ExcludesExpression(
+            'received signal|failed assertion', 'ATS must not crash handling 
the mid-body origin abort')
+        return ts
+
+    def _configure_origin(self) -> 'Process':
+        '''Configure a raw-socket TLS origin that resets mid-request-body.
+
+        The scenario needs a TLS origin that accepts the request body and then 
RSTs
+        the connection mid-body (SO_LINGER 0). Proxy Verifier can only close 
cleanly
+        (close_notify), so it cannot express a mid-body reset; hence the small
+        raw-socket origin helper rather than an ATSReplayTest.
+
+        :return: The origin server Process.
+        '''
+        origin = Test.Processes.Process(
+            f'origin-{TestOriginPostAbort._origin_counter}', 'python3 {0} -p 
{1} -c {2} -d 1.0'.format(
+                os.path.join(Test.TestDirectory, 'tls_post_abort_origin.py'), 
Test.Variables.origin_port,
+                os.path.join(Test.Variables.AtsTestToolsDir, 'ssl', 
'server.pem')))

Review Comment:
   Done — converted every `.format()` call in the PR's files to f-strings, and 
added a note to the Writing Autests section of AGENTS.md.



##########
tests/gold_tests/tls/tls_record_size_client.py:
##########
@@ -0,0 +1,154 @@
+#!/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).
+
+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
+sizes can be measured without decrypting. TLS 1.2 is pinned so that handshake
+messages are their own record type (22) and only real application data is type 
23,
+and the cipher is restricted to AEAD (GCM) suites so the per-record overhead 
is a
+small fixed constant rather than the variable IV/MAC/padding of a CBC suite.
+'''
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+#  limitations under the License.
+
+import argparse
+import socket
+import ssl
+import sys
+
+TLS_APPLICATION_DATA = 23
+# 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
+
+
+def iter_record_lengths(buf):

Review Comment:
   Added: `iter_record_lengths(buf: bytes | bytearray) -> Iterator[tuple[int, 
int]]`, plus the `flush`/`feed` closures in the same function. Also annotated 
`main()` in `tls_renegotiation_client.py` for consistency, and added a line 
about annotating helper functions to the AGENTS.md note above.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to