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 21:00:22 UTC

[GitHub] [trafficserver] maskit opened a new pull request, #9348: Add an SNI action to reject QUIC connections

maskit opened a new pull request, #9348:
URL: https://github.com/apache/trafficserver/pull/9348

   Adds `quic` SNI action to on/off QUIC availability per server name.
   
   This depends on #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] bneradt commented on a diff in pull request #9348: Add an SNI action to reject QUIC connections

Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on code in PR #9348:
URL: https://github.com/apache/trafficserver/pull/9348#discussion_r1230118626


##########
doc/admin-guide/files/sni.yaml.en.rst:
##########
@@ -45,7 +45,7 @@ the user needs to enter the fqdn in the configuration with a ``*.`` followed by
 For some settings, there is no guarantee that they will be applied to a connection under certain conditions.
 An established TLS connection may be reused for another server name if it’s used for HTTP/2. This also means that settings
 for server name A may affects requests for server name B as well. See https://daniel.haxx.se/blog/2016/08/18/http2-connection-coalescing/
-for a more detailed description of HTTP/2 connection coalescing.
+for a more detailed description of HTTP/2 connection coalescing. Similar thing can happen on a QUIC connection for HTTP/3 as well.

Review Comment:
   `Similar thing` -> `A similar thing`



##########
iocore/net/SNIActionPerformer.cc:
##########
@@ -28,6 +28,35 @@
 
 #include "P_SNIActionPerformer.h"
 
+#if TS_USE_QUIC == 1
+#include "P_QUICNetVConnection.h"
+#endif
+
+int
+ControlQUIC::SNIAction(TLSSNISupport *snis, const Context &ctx) const
+{
+#if TS_USE_QUIC == 1
+  if (enable_quic) {
+    return SSL_TLSEXT_ERR_OK;
+  }
+
+  // This action is only available for QUIC connections
+  auto *quic_vc = dynamic_cast<QUICNetVConnection *>(snis);
+  if (quic_vc == nullptr) {
+    return SSL_TLSEXT_ERR_OK;
+  }
+
+  if (is_debug_tag_set("ssl_sni")) {
+    const char *servername = quic_vc->get_server_name();
+    Debug("ssl_sni", "Rejecting handshake, fqdn [%s]", servername);

Review Comment:
   Probably good to say specifically that this is due to sni.yaml quic being disabled. Maybe:
   
   ```cpp
       Debug("ssl_sni", "Rejecting handshake due to QUIC being disabled for fqdn [%s]", servername);
   ```



##########
doc/admin-guide/files/sni.yaml.en.rst:
##########
@@ -174,6 +174,10 @@ http2_buffer_water_mark   Inbound   Specifies the high water mark for all HTTP/2
                                     By default this is :ts:cv:`proxy.config.http2.default_buffer_water_mark`.
                                     NOTE: Connection coalescing may prevent this taking effect.
 
+quic                      Inbound   Indicates whether QUIC connections should be accepted. The valid values are :code:`on` or
+                                    :code:`off`. Note that this is an additional setting to configure QUIC availability per server
+                                    name. You need to configure :ts:cv:`proxy.config.http.server_ports` to open ports for QUIC.

Review Comment:
   I suggest:
   
   ```
   Note that this is a more specific setting to configure QUIC availability per server
   name. More broadly, you will also need to configure :ts:cv:`proxy.config.http.server_ports` 
   to open ports for 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] maskit commented on pull request #9348: Add an SNI action to reject QUIC connections

Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit commented on PR #9348:
URL: https://github.com/apache/trafficserver/pull/9348#issuecomment-1531848515

   Some of build issues are actually because of ATS's native QUIC implementation, which we don't maintain. https://github.com/apache/trafficserver/pull/9670 should resolve those.


-- 
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 pull request #9348: Add an SNI action to reject QUIC connections

Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on PR #9348:
URL: https://github.com/apache/trafficserver/pull/9348#issuecomment-1598006833

   > @bneradt did you mean to continue block this? I'm OOO till the 22nd at least so don't wait for me.
   
   Thanks for checking. My comments aren't addressed yet, so the block still makes sense.


-- 
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] ywkaras commented on pull request #9348: Add an SNI action to reject QUIC connections

Posted by "ywkaras (via GitHub)" <gi...@apache.org>.
ywkaras commented on PR #9348:
URL: https://github.com/apache/trafficserver/pull/9348#issuecomment-1596409970

   @bneradt did you mean to continue block this?  I'm OOO till the 22nd at least so don't wait for me.


-- 
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 #9348: Add an SNI action to reject QUIC connections

Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit commented on PR #9348:
URL: https://github.com/apache/trafficserver/pull/9348#issuecomment-1589584715

   [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] ezelkow1 commented on pull request #9348: Add an SNI action to reject QUIC connections

Posted by "ezelkow1 (via GitHub)" <gi...@apache.org>.
ezelkow1 commented on PR #9348:
URL: https://github.com/apache/trafficserver/pull/9348#issuecomment-1554937557

   [approve ci centos]


-- 
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] ywkaras commented on pull request #9348: Add an SNI action to reject QUIC connections

Posted by "ywkaras (via GitHub)" <gi...@apache.org>.
ywkaras commented on PR #9348:
URL: https://github.com/apache/trafficserver/pull/9348#issuecomment-1590129956

   Rocky CI job fails due to a link error for test_UDPNet:  https://gist.github.com/ywkaras/59747baa5a9ede1b97ec9bac3b4b9d3e


-- 
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 #9348: Add an SNI action to reject QUIC connections

Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit commented on PR #9348:
URL: https://github.com/apache/trafficserver/pull/9348#issuecomment-1591839460

   [approve ci]


-- 
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 #9348: Add an SNI action to reject QUIC connections

Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit merged PR #9348:
URL: https://github.com/apache/trafficserver/pull/9348


-- 
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] ywkaras commented on a diff in pull request #9348: Add an SNI action to reject QUIC connections

Posted by "ywkaras (via GitHub)" <gi...@apache.org>.
ywkaras commented on code in PR #9348:
URL: https://github.com/apache/trafficserver/pull/9348#discussion_r1213390863


##########
doc/admin-guide/files/sni.yaml.en.rst:
##########
@@ -45,7 +45,7 @@ the user needs to enter the fqdn in the configuration with a ``*.`` followed by
 For some settings, there is no guarantee that they will be applied to a connection under certain conditions.
 An established TLS connection may be reused for another server name if it’s used for HTTP/2. This also means that settings
 for server name A may affects requests for server name B as well. See https://daniel.haxx.se/blog/2016/08/18/http2-connection-coalescing/
-for a more detailed description of HTTP/2 connection coalescing.
+for a more detailed description of HTTP/2 connection coalescing. Similar thing can happen on a QUIC connection for HTTP/3 as welll.

Review Comment:
   Typo, well not welll



-- 
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 #9348: Add an SNI action to reject QUIC connections

Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit commented on PR #9348:
URL: https://github.com/apache/trafficserver/pull/9348#issuecomment-1540359573

   [approve ci]


-- 
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 #9348: Add an SNI action to reject QUIC connections

Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit commented on PR #9348:
URL: https://github.com/apache/trafficserver/pull/9348#issuecomment-1591709783

   [approve ci Fedora]


-- 
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] ywkaras commented on pull request #9348: Add an SNI action to reject QUIC connections

Posted by "ywkaras (via GitHub)" <gi...@apache.org>.
ywkaras commented on PR #9348:
URL: https://github.com/apache/trafficserver/pull/9348#issuecomment-1590066729

   [approve ci rocky]


-- 
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 #9348: Add an SNI action to reject QUIC connections

Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit commented on PR #9348:
URL: https://github.com/apache/trafficserver/pull/9348#issuecomment-1591711182

   [approve ci fedora]


-- 
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 #9348: Add an SNI action to reject QUIC connections

Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit commented on PR #9348:
URL: https://github.com/apache/trafficserver/pull/9348#issuecomment-1599008738

   @bneradt I've already updated the texts as you suggested. Did I miss something?


-- 
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 #9348: Add an SNI action to reject QUIC connections

Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit commented on PR #9348:
URL: https://github.com/apache/trafficserver/pull/9348#issuecomment-1531656730

   [approve ci]


-- 
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] brbzull0 commented on pull request #9348: Add an SNI action to reject QUIC connections

Posted by "brbzull0 (via GitHub)" <gi...@apache.org>.
brbzull0 commented on PR #9348:
URL: https://github.com/apache/trafficserver/pull/9348#issuecomment-1540359539

   [approve ci]


-- 
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