bneradt commented on code in PR #13919:
URL: https://github.com/apache/trafficserver/pull/13919#discussion_r4389823115
##########
src/iocore/net/OpenSSLQUICNetVConnection.cc:
##########
@@ -250,7 +252,7 @@ QUICNetVConnection::state_handshake(int event, Event *data)
break;
}
- if (this->closed != 1 && SSL_is_init_finished(this->_ssl)) {
+ if (this->closed != 1 && SSL_is_init_finished(this->_ssl) &&
this->_run_client_hello_hooks()) {
Review Comment:
Agreed, and fixed. The hooks now run from the QUIC ClientHello callback, as
on TCP, rather than after the handshake.
`SSL_accept_connection()` hands the connection to ATS before OpenSSL
processes the ClientHello, so the callback can run the hooks on the QUIC NetVC.
It returns `SSL_CLIENT_HELLO_RETRY` in two cases: when the NetVC doesn't exist
yet, and while a hook has yet to reenable the connection. A hook that reenables
later doesn't have the remaining hooks run from `reenable()`, because OpenSSL
only exposes the ClientHello during the callback. Instead, `reenable()` resumes
the handshake, and the callback runs those hooks when OpenSSL calls it again. A
hook that reenables with `TS_EVENT_ERROR` makes the callback return
`SSL_CLIENT_HELLO_ERROR`, which fails the handshake.
`tests/uranium_tests/h3/test_h3_client_hello_fingerprint.py` now also covers
this:
- A hook delays reenabling. The handshake waits for it, and the
fingerprinting hook after it still reads the ClientHello.
- An abuse_shield JA3 denylist rejects an HTTP/3 ClientHello. The same
client is allowed before its JA3 is denylisted, as a positive control.
--
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]
