maskit commented on code in PR #13729:
URL: https://github.com/apache/trafficserver/pull/13729#discussion_r4095820627
##########
src/iocore/net/SSLUtils.cc:
##########
@@ -1063,24 +1063,29 @@
SSLMultiCertConfigLoader::_set_handshake_callbacks(SSL_CTX *ctx)
#elif HAVE_SSL_CTX_SET_SELECT_CERTIFICATE_CB
SSL_CTX_set_select_certificate_cb(ctx, [](const SSL_CLIENT_HELLO
*client_hello) -> ssl_select_cert_result_t {
ssl_select_cert_result_t res;
- res = ssl_client_hello_callback(client_hello);
- if (res == ssl_select_cert_error) {
- return res;
- }
+ TLSEventSupport *es =
TLSEventSupport::getInstance(client_hello->ssl);
+
+ // BoringSSL runs this whole callback again once a paused hook reenables,
whereas OpenSSL calls
+ // only the callback that paused. After a pause in the cert hook, the
client hello and
+ // servername work is already done and must not run twice.
+ if (es == nullptr || !es->reached_cert_hooks()) {
+ res = ssl_client_hello_callback(client_hello);
+ if (res != ssl_select_cert_success) {
+ return res;
+ }
Review Comment:
The resumed call ending up in `HANDSHAKE_HOOKS_CLIENT_CERT` is not specific
to this change. OpenSSL re-invokes `cert_cb` after a `-1` pause as well, so
OpenSSL builds already re-run `selectCertificate()` and start the verify-client
hook list with `TS_EVENT_SSL_CERT` in the same situation. This PR brings
BoringSSL onto that shared path; fixing `ssl_cert_callback()` for both backends
is tracked in #13730.
##########
src/iocore/net/TLSEventSupport.cc:
##########
@@ -375,6 +375,20 @@ TLSEventSupport::calledHooks(TSEvent eventId) const
return retval;
}
+bool
+TLSEventSupport::reached_cert_hooks() const
+{
+ switch (this->sslHandshakeHookState) {
+ case SSLHandshakeHookState::HANDSHAKE_HOOKS_CERT:
+ case SSLHandshakeHookState::HANDSHAKE_HOOKS_CERT_INVOKE:
+ case SSLHandshakeHookState::HANDSHAKE_HOOKS_CLIENT_CERT:
+ case SSLHandshakeHookState::HANDSHAKE_HOOKS_CLIENT_CERT_INVOKE:
Review Comment:
The resumed call ending up in `HANDSHAKE_HOOKS_CLIENT_CERT` is not specific
to this change. OpenSSL re-invokes `cert_cb` after a `-1` pause as well, so
OpenSSL builds already re-run `selectCertificate()` and start the verify-client
hook list with `TS_EVENT_SSL_CERT` in the same situation. This PR brings
BoringSSL onto that shared path; fixing `ssl_cert_callback()` for both backends
is tracked in #13730.
--
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]
