moonchen opened a new pull request, #13371:
URL: https://github.com/apache/trafficserver/pull/13371
## Problem
The `tls_renegotiation.test.py` gold test added in #13306 fails on
BoringSSL. It passes on OpenSSL, so the ASF CI is green and the gap went
unnoticed.
The test's `ContainsExpression("trying to renegotiate from the client")`
asserts on a debug line emitted by `ssl_callback_info()` in `SSLUtils.cc`,
which lives behind the `state == SSL_ST_RENEGOTIATE` branch. On BoringSSL that
branch is *compiled* (`SSL_ST_RENEGOTIATE` is `#define`d) but never *taken*:
per BoringSSL's `ssl.h`, `SSL_get_state()`/`SSL_state()` "only ever return
`SSL_ST_INIT` or `SSL_ST_OK`", never `SSL_ST_RENEGOTIATE`. BoringSSL also
rejects a peer-initiated renegotiation inside the library before the callback
runs, so ATS never logs the line — the connection is refused safely, but the
detection assertion has nothing to match.
## Fix
Gate only the detection-line assertion behind `Condition.IsOpenSSL()`. The
crash-safety `ExcludesExpression` stays unconditional, so BoringSSL keeps that
coverage; only the detection-line check is now OpenSSL-only. This mirrors the
`Condition.IsOpenSSL()` pattern already used in `tls_async_handshake.test.py`.
`tls_renegotiation_allowed.test.py` needs no change — it only asserts on
client-side output.
## Follow-up (not in this PR)
On BoringSSL the `SSL_ST_RENEGOTIATE` branch in `ssl_callback_info()` is
effectively dead, so ATS's own renegotiation logging/abort never fires and
`allow_client_renegotiation=1` is a no-op there. Not a security issue —
BoringSSL enforces the refusal regardless — but the branch is misleading and
could be cleaned up or wired to a mechanism that actually fires.
--
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]
