Copilot commented on code in PR #13723:
URL: https://github.com/apache/trafficserver/pull/13723#discussion_r4084995431
##########
tests/gold_tests/tls/tls_sni_groups.test.py:
##########
@@ -74,16 +74,17 @@
tr = Test.AddTestRun("Test 0: x25519")
tr.Processes.Default.StartBefore(server)
tr.Processes.Default.StartBefore(Test.Processes.ts)
-tr.MakeCurlCommand(
- "-v --ciphers ECDHE-RSA-AES256-GCM-SHA384 --resolve
'bbb.com:{0}:127.0.0.1' -k https://bbb.com:{0}".format(
- ts.Variables.ssl_port),
- ts=ts)
+tr.Processes.Default.Command = (
+ 'printf "GET / HTTP/1.1\\r\\nHost: bbb.com\\r\\nConnection:
close\\r\\n\\r\\n" | '
+ 'openssl s_client -connect 127.0.0.1:{0} -servername bbb.com -tls1_2 '
+ '-cipher ECDHE-RSA-AES256-GCM-SHA384
-ign_eof'.format(ts.Variables.ssl_port))
Review Comment:
These runs now depend on the presence of an `openssl` command-line binary,
even in configurations where the build is using a different TLS implementation
(e.g., BoringSSL) or where OpenSSL is available only as a library. This can
make the test fail in minimal CI images or create version mismatches (notably
for the OpenSSL 3.5-gated hybrid group test). Consider adding an explicit
prerequisite/skip for the `openssl` executable (and, for the hybrid run,
verifying the `openssl` binary version is >= 3.5 as well), or otherwise
sourcing the `openssl` client from the same toolchain used for the build.
##########
tests/gold_tests/tls/tls_sni_groups.test.py:
##########
@@ -100,14 +101,16 @@
# Hybrid ECDH PQ key exchange TLS groups were added in OpenSSL 3.5
if Condition.HasOpenSSLVersion("3.5.0"):
tr = Test.AddTestRun("Test 2: X25519MLKEM768")
- tr.MakeCurlCommand(
- "-v --tls13-ciphers TLS_AES_256_GCM_SHA384 --resolve
'aaa.com:{0}:127.0.0.1' -k https://aaa.com:{0}".format(
- ts.Variables.ssl_port),
- ts=ts)
+ tr.Processes.Default.Command = (
+ 'printf "GET / HTTP/1.1\\r\\nHost: aaa.com\\r\\nConnection:
close\\r\\n\\r\\n" | '
+ 'openssl s_client -connect 127.0.0.1:{0} -servername aaa.com -tls1_3 '
+ '-ciphersuites TLS_AES_256_GCM_SHA384
-ign_eof'.format(ts.Variables.ssl_port))
Review Comment:
These runs now depend on the presence of an `openssl` command-line binary,
even in configurations where the build is using a different TLS implementation
(e.g., BoringSSL) or where OpenSSL is available only as a library. This can
make the test fail in minimal CI images or create version mismatches (notably
for the OpenSSL 3.5-gated hybrid group test). Consider adding an explicit
prerequisite/skip for the `openssl` executable (and, for the hybrid run,
verifying the `openssl` binary version is >= 3.5 as well), or otherwise
sourcing the `openssl` client from the same toolchain used for the build.
##########
tests/gold_tests/tls/tls_sni_groups.test.py:
##########
@@ -100,14 +101,16 @@
# Hybrid ECDH PQ key exchange TLS groups were added in OpenSSL 3.5
if Condition.HasOpenSSLVersion("3.5.0"):
tr = Test.AddTestRun("Test 2: X25519MLKEM768")
- tr.MakeCurlCommand(
- "-v --tls13-ciphers TLS_AES_256_GCM_SHA384 --resolve
'aaa.com:{0}:127.0.0.1' -k https://aaa.com:{0}".format(
- ts.Variables.ssl_port),
- ts=ts)
+ tr.Processes.Default.Command = (
+ 'printf "GET / HTTP/1.1\\r\\nHost: aaa.com\\r\\nConnection:
close\\r\\n\\r\\n" | '
+ 'openssl s_client -connect 127.0.0.1:{0} -servername aaa.com -tls1_3 '
+ '-ciphersuites TLS_AES_256_GCM_SHA384
-ign_eof'.format(ts.Variables.ssl_port))
tr.ReturnCode = 0
tr.StillRunningAfter = ts
ts.Disk.traffic_out.Content += Testers.ContainsExpression(
"Setting groups list from server_groups_list to X25519MLKEM768",
"Should log setting the server groups")
+ # A hybrid group has no legacy key type, so s_client may name it on the
TLS 1.3 group line
+ # rather than the temp key line.
tr.Processes.Default.Streams.All = Testers.IncludesExpression(
- f"SSL connection using TLSv1.3 / TLS_AES_256_GCM_SHA384 /
X25519MLKEM768",
- f"Curl should log using X25519MLKEM768 in the SSL connection")
+ r"((Server|Peer) Temp Key|Negotiated TLS1\.3 group): X25519MLKEM768",
Review Comment:
The expected label `Negotiated TLS1.3 group` is quite specific and may be
brittle across OpenSSL output variations (some tooling commonly prints
`TLSv1.3` elsewhere). To reduce false negatives, consider making the regex
tolerant to minor wording/version formatting differences (e.g., allowing
`TLS1.3` vs `TLSv1.3`, or optional whitespace) while still strictly asserting
the negotiated group name.
--
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]
