moonchen commented on code in PR #13799:
URL: https://github.com/apache/trafficserver/pull/13799#discussion_r4252922196
##########
src/iocore/net/SSLUtils.cc:
##########
@@ -284,21 +305,26 @@ ssl_cert_callback(SSL *ssl, [[maybe_unused]] void *arg)
if (tcss) {
if (tes) {
- // Do the common certificate lookup only once. If we pause
- // and restart processing, do not execute the common logic again
- if (!tes->calledHooks(TS_EVENT_SSL_CERT)) {
+ bool const hooks_finished = tes->finished_cert_hooks();
+
+ // On retry, keep a context selected by a completed cert hook. A new
handshake
+ // must retain the existing certificate lookup behavior.
+ if ((!hooks_finished || is_renegotiation) &&
!tes->calledHooks(TS_EVENT_SSL_CERT)) {
Review Comment:
The mechanics are right: in `HANDSHAKE_HOOKS_DONE`,
`calledHooks(TS_EVENT_SSL_CERT)` is true, so an allowed renegotiation after a
verify-client hook skips `selectCertificate()`. The base revision skips it in
the same state, so this PR doesn't change it. OpenSSL 3 and BoringSSL also
refuse a client-initiated renegotiation before these callbacks run, because ATS
never sets `SSL_OP_ALLOW_CLIENT_RENEGOTIATION`, so only OpenSSL 1.1.1 builds
can reach it. I'll leave it for a follow-up.
--
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]
