Copilot commented on code in PR #13964:
URL: https://github.com/apache/trafficserver/pull/13964#discussion_r4447616047
##########
src/iocore/net/SSLNetVConnection.cc:
##########
@@ -2031,6 +2031,8 @@ SSLNetVConnection::_beReadyToTransferData()
if (this->write.vio.ntodo() <= 0) {
this->readSignalDone(VC_EVENT_WRITE_COMPLETE, nh);
}
+ } else if (ret == SSL_WAIT_FOR_HOOK) {
+ // reenable() resumes the handshake; rescheduling here requeues the VC and
spins process_ready_list().
Review Comment:
`sslClientHandShakeEvent()` also returns `SSL_WAIT_FOR_HOOK` when an
outbound PROXY header is only partially written or its write would block (lines
1641 and 1647), before invoking any hook. Suppressing rescheduling for those
returns drops the write retry/rearm even though no plugin will call
`reenable()`, which can stall the TLS connection until the connect timeout.
Distinguish those I/O waits from actual hook suspension by returning
`SSL_HANDSHAKE_WANT_WRITE` from both PROXY-write retry paths.
##########
tests/tools/uranium/services/curl.py:
##########
@@ -59,24 +59,50 @@ def supports(feature: str) -> bool:
result = subprocess.run(("curl", "--version"), capture_output=True,
text=True, check=False)
return feature.lower() in (result.stdout + result.stderr).lower()
+ @staticmethod
+ def _active_libraries() -> dict[str, str]:
+ """Return the libraries on curl's version line, mapped from name to
version.
+
+ A curl built with several TLS backends lists the inactive ones in
+ parentheses, so they are left out.
+ """
+
+ result = subprocess.run(("curl", "--version"), capture_output=True,
text=True, check=False)
+ components = result.stdout.partition("\n")[0].split()
+ return {
+ name: version
+ for name, _, version in (component.partition("/") for component in
components if not component.startswith("("))
+ }
+
@staticmethod
def accepts_seclevel() -> bool:
"""Return whether curl's TLS library accepts an @SECLEVEL directive in
``--ciphers``.
OpenSSL 1.1.0 and later, and its quictls fork, accept the directive.
BoringSSL rejects the whole cipher list, so curl fails before the
- TLS handshake. A curl built with several TLS backends lists the
- inactive ones in parentheses, so only the active backend counts.
+ TLS handshake.
"""
- result = subprocess.run(("curl", "--version"), capture_output=True,
text=True, check=False)
- for component in result.stdout.partition("\n")[0].split():
- name, _, version = component.partition("/")
- if name == "quictls":
- return True
- if name == "OpenSSL":
- return version_tuple(version) >= (1, 1, 0)
- return False
+ libraries = Curl._active_libraries()
+ if "quictls" in libraries:
+ return True
+ return "OpenSSL" in libraries and version_tuple(libraries["OpenSSL"])
>= (1, 1, 0)
+
+ @staticmethod
+ def supports_tls13() -> bool:
+ """Return whether curl can negotiate TLS 1.3 rather than fail
``--tlsv1.3`` with exit status 4.
+
+ OpenSSL added TLS 1.3 in 1.1.1. LibreSSL hides ``TLS1_3_VERSION``
+ before 3.4.0, so a curl built against an older release, such as the
+ one macOS ships, lacks it. Other backends are assumed capable.
+ """
+
+ libraries = Curl._active_libraries()
+ if "OpenSSL" in libraries:
+ return version_tuple(libraries["OpenSSL"]) >= (1, 1, 1)
+ if "LibreSSL" in libraries:
+ return version_tuple(libraries["LibreSSL"]) >= (3, 4, 0)
+ return True
Review Comment:
An active SecureTransport backend reaches this fallback and is reported as
TLS 1.3 capable, but SecureTransport supports only TLS 1.2 and earlier. For the
`SecureTransport (LibreSSL/3.3.6)` version line already represented in the unit
test, `test_tls_hooks_cert_resume` will therefore still run `--tlsv1.3` and
fail rather than skip the unsupported case. Return False for active
SecureTransport and update the corresponding unit-test expectation.
--
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]
