brbzull0 opened a new pull request, #13630:
URL: https://github.com/apache/trafficserver/pull/13630
`SSLNetVCAttach()` (`src/iocore/net/SSLUtils.cc:1994`) is called from
exactly one
place, `SSLNetVConnection::SSLNetVConnection`
(`src/iocore/net/SSLNetVConnection.cc:145`). Nothing on the QUIC side
attaches,
so an `SSL` handle belonging to a QUIC connection has no `ssl_vc_index`
ex_data.
`SSLNetVCAccess()` then did:
```c
netvc = static_cast<SSLNetVConnection *>(SSL_get_ex_data(ssl, ssl_vc_index));
ink_assert(dynamic_cast<SSLNetVConnection *>(static_cast<NetVConnection
*>(SSL_get_ex_data(ssl, ssl_vc_index))));
return netvc;
```
The `dynamic_cast` was only evaluated inside `ink_assert`, so in a release
build
the unchecked value was returned as-is. Three callbacks that a QUIC handshake
also reaches then used it without a null check:
- `SNI_IpAllow::SNIAction()` -- `src/iocore/net/SNIActionPerformer.cc:418`
- `ssl_client_cert_callback()` -- `src/iocore/net/SSLClientUtils.cc:211`
- `ssl_verify_client_callback()` -- `src/iocore/net/SSLUtils.cc:200`
This makes the `dynamic_cast` the return value, so the accessor reports a
handle
that is not an `SSLNetVConnection` instead of handing back something
unchecked,
and adds the corresponding guard at those three call sites.
The cast is well defined: `SSLNetVConnection` derives from
`UnixNetVConnection`, which derives from `NetVConnection`, leftmost at both
levels, so `NetVConnection` is the primary base subobject at offset 0 and the
`void *` recovered from ex_data needs no adjustment. The `ink_assert` being
removed already performed this exact cast.
### Test
No new test. Reaching the null case requires a QUIC handshake to drive an
`SSL_CTX` callback shared with the TLS path, which the existing autest
harness
does not set up directly.
Existing coverage run against this change, 8/8 pass:
- `tls_sni_ip_allow` -- exercises `SNI_IpAllow::SNIAction()` directly
- `tls_client_verify`, `tls_client_verify2`, `tls_client_verify3` --
`ssl_verify_client_callback()`
- `tls_client_cert`, `tls_client_cert_override` --
`ssl_client_cert_callback()`
- `h3_sni_check`, `h3_proxy_verifier` -- the QUIC path
These confirm the non-null path through all three callbacks is unchanged.
--
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]
