bneradt commented on code in PR #13044:
URL: https://github.com/apache/trafficserver/pull/13044#discussion_r3018766475
##########
src/iocore/net/SSLUtils.cc:
##########
@@ -401,13 +407,9 @@ ssl_cert_callback(SSL *ssl, [[maybe_unused]] void *arg)
setClientCertCACerts(ssl, sslnetvc->get_ca_cert_file(),
sslnetvc->get_ca_cert_dir());
}
- // Reset the ticket callback if needed
- SSL_CTX *ctx = SSL_get_SSL_CTX(ssl);
-#ifdef HAVE_SSL_CTX_SET_TLSEXT_TICKET_KEY_EVP_CB
- SSL_CTX_set_tlsext_ticket_key_evp_cb(ctx, ssl_callback_session_ticket);
-#else
- SSL_CTX_set_tlsext_ticket_key_cb(ctx, ssl_callback_session_ticket);
-#endif
+ if (!ssl_apply_sni_session_ticket_properties(ssl)) {
+ retval = 0;
+ }
Review Comment:
Thanks for checking into this. The logic has changed slightly, I think for
the better. Now the callback is set (along with other callbacks) in the
`SSLMultiCertConfigLoader::init_server_ssl_ctx` below. You can see the
`ssl_context_enable_ticket_callback` in that function further down in this file
in this patch. Currently line 1347.
I'm honestly not sure why the callback was set here in the first place. If
you look at the other stuff in this `ssl_cert_callback`, it's all cert related,
not "add a callback" functionality. It had to go out of its way to reach into
the SSL and get the SSL_CTX to set the callback on it. `init_server_ssl_ctx` is
the appropriate place to configure the ticket callback.
This discussion also applies to the master patch, btw. (In case that's
helpful.)
--
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]
