You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "maskit (via GitHub)" <gi...@apache.org> on 2023/01/31 20:47:20 UTC
[GitHub] [trafficserver] maskit opened a new pull request, #9347: Enables switching SSL certificates on QUIC with QUICHE
maskit opened a new pull request, #9347:
URL: https://github.com/apache/trafficserver/pull/9347
This enables using certificates loaded by QUICMultiCertConfigLoader (not only the default cert but also certs for specific servernames and IP addresses).
This also makes other SNI stuff work on QUIC.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bneradt commented on a diff in pull request #9347: Enables switching SSL certificates on QUIC with QUICHE
Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on code in PR #9347:
URL: https://github.com/apache/trafficserver/pull/9347#discussion_r1170165436
##########
iocore/net/QUICNetVConnection_quiche.cc:
##########
@@ -632,14 +665,83 @@ QUICNetVConnection::protocol_contains(std::string_view tag) const
return "";
}
+const char *
+QUICNetVConnection::get_server_name() const
+{
+ return get_sni_server_name();
+}
+
+bool
+QUICNetVConnection::support_sni() const
+{
+ return true;
+}
+
SSL *
QUICNetVConnection::_get_ssl_object() const
{
- return nullptr;
+ return this->_ssl;
}
ssl_curve_id
QUICNetVConnection::_get_tls_curve() const
{
- return 0;
+ if (getSSLSessionCacheHit()) {
+ return getSSLCurveNID();
+ } else {
+ return SSLGetCurveNID(this->_ssl);
+ }
+}
+
+void
+QUICNetVConnection::_fire_ssl_servername_event()
+{
+}
+
+const IpEndpoint &
+QUICNetVConnection::_getLocalEndpoint()
+{
+ return this->local_addr;
+}
+
+bool
+QUICNetVConnection::_isTryingRenegotiation() const
+{
+ // Renegotiation is not allowed on QUIC (TLS 1.3) connections.
+ // If handshake is completed when this function is called, that should be unallowed attempt of renegotiation.
+ if (this->getSSLHandShakeComplete()) {
+ return true;
+ } else {
+ return false;
+ }
Review Comment:
Might as well simplify to:
```
return this->getSSLHandShakeComplete();
```
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on pull request #9347: Enables switching SSL certificates on QUIC with QUICHE
Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit commented on PR #9347:
URL: https://github.com/apache/trafficserver/pull/9347#issuecomment-1468581999
This requires BoringSSL if we use Quiche implementation, and I think the autest failure is because of the build environment, which use OpenSSL(quictls).
@bneradt Can we use BoringSSL on the image for Quiche build?
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on pull request #9347: Enables switching SSL certificates on QUIC with QUICHE
Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit commented on PR #9347:
URL: https://github.com/apache/trafficserver/pull/9347#issuecomment-1487805721
[approve ci autest]
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on pull request #9347: Enables switching SSL certificates on QUIC with QUICHE
Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit commented on PR #9347:
URL: https://github.com/apache/trafficserver/pull/9347#issuecomment-1504084844
This breaks the original QUIC implementation, and autests for HTTP/3 would be ran only if ATS is built with Quiche.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on pull request #9347: Enables switching SSL certificates on QUIC with QUICHE
Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit commented on PR #9347:
URL: https://github.com/apache/trafficserver/pull/9347#issuecomment-1411053902
This depends on #9322, and most code change will be gone after merging it.
The most important change on this PR is use of `quiche_conn_new_with_tls`. This requires the both ATS and Quiche use the same version of BoringSSL.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit merged pull request #9347: Enables switching SSL certificates on QUIC with QUICHE
Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit merged PR #9347:
URL: https://github.com/apache/trafficserver/pull/9347
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on pull request #9347: Enables switching SSL certificates on QUIC with QUICHE
Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit commented on PR #9347:
URL: https://github.com/apache/trafficserver/pull/9347#issuecomment-1503796997
[approve ci autest]
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on pull request #9347: Enables switching SSL certificates on QUIC with QUICHE
Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit commented on PR #9347:
URL: https://github.com/apache/trafficserver/pull/9347#issuecomment-1496294497
[approve ci autest]
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on a diff in pull request #9347: Enables switching SSL certificates on QUIC with QUICHE
Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit commented on code in PR #9347:
URL: https://github.com/apache/trafficserver/pull/9347#discussion_r1170421568
##########
iocore/net/QUICNetVConnection_quiche.cc:
##########
@@ -632,14 +665,83 @@ QUICNetVConnection::protocol_contains(std::string_view tag) const
return "";
}
+const char *
+QUICNetVConnection::get_server_name() const
+{
+ return get_sni_server_name();
+}
+
+bool
+QUICNetVConnection::support_sni() const
+{
+ return true;
+}
+
SSL *
QUICNetVConnection::_get_ssl_object() const
{
- return nullptr;
+ return this->_ssl;
}
ssl_curve_id
QUICNetVConnection::_get_tls_curve() const
{
- return 0;
+ if (getSSLSessionCacheHit()) {
+ return getSSLCurveNID();
+ } else {
+ return SSLGetCurveNID(this->_ssl);
+ }
+}
+
+void
+QUICNetVConnection::_fire_ssl_servername_event()
+{
+}
+
+const IpEndpoint &
+QUICNetVConnection::_getLocalEndpoint()
+{
+ return this->local_addr;
+}
+
+bool
+QUICNetVConnection::_isTryingRenegotiation() const
+{
+ // Renegotiation is not allowed on QUIC (TLS 1.3) connections.
+ // If handshake is completed when this function is called, that should be unallowed attempt of renegotiation.
+ if (this->getSSLHandShakeComplete()) {
+ return true;
+ } else {
+ return false;
+ }
Review Comment:
Made the change.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org