Skip to content

Add an SNI action to reject QUIC connections - #9348

Merged
maskit merged 13 commits into
apache:masterfrom
maskit:quiche_sni_disableh3
Jun 22, 2023
Merged

Add an SNI action to reject QUIC connections#9348
maskit merged 13 commits into
apache:masterfrom
maskit:quiche_sni_disableh3

Conversation

@maskit

@maskit maskit commented Jan 31, 2023

Copy link
Copy Markdown
Member

Adds quic SNI action to on/off QUIC availability per server name.

This depends on #9347.

@maskit maskit added the QUIC label Jan 31, 2023
@maskit maskit added this to the 10.0.0 milestone Jan 31, 2023
@maskit maskit self-assigned this Jan 31, 2023
@maskit
maskit changed the base branch from 10-Dev to master February 1, 2023 21:02
@maskit
maskit force-pushed the quiche_sni_disableh3 branch 2 times, most recently from 14963e2 to a3a8ae4 Compare April 21, 2023 00:06
@bryancall
bryancall requested a review from bneradt April 25, 2023 15:28
@maskit

maskit commented May 2, 2023

Copy link
Copy Markdown
Member Author

[approve ci]

@maskit

maskit commented May 2, 2023

Copy link
Copy Markdown
Member Author

Some of build issues are actually because of ATS's native QUIC implementation, which we don't maintain. #9670 should resolve those.

@brbzull0

brbzull0 commented May 9, 2023

Copy link
Copy Markdown
Contributor

[approve ci]

1 similar comment
@maskit

maskit commented May 9, 2023

Copy link
Copy Markdown
Member Author

[approve ci]

@maskit
maskit marked this pull request as ready for review May 11, 2023 19:46
@ezelkow1

Copy link
Copy Markdown
Member

[approve ci centos]

Comment thread doc/admin-guide/files/sni.yaml.en.rst Outdated
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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo, well not welll

@maskit

maskit commented Jun 13, 2023

Copy link
Copy Markdown
Member Author

[approve ci autest]

@ywkaras

ywkaras commented Jun 13, 2023

Copy link
Copy Markdown

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

@maskit
maskit force-pushed the quiche_sni_disableh3 branch from 7d15f3f to 72058c7 Compare June 14, 2023 17:18
@maskit

maskit commented Jun 14, 2023

Copy link
Copy Markdown
Member Author

[approve ci Fedora]

@maskit

maskit commented Jun 14, 2023

Copy link
Copy Markdown
Member Author

[approve ci fedora]

@maskit

maskit commented Jun 14, 2023

Copy link
Copy Markdown
Member Author

[approve ci]

@bneradt bneradt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Just a few word smithing suggestions.

Comment thread iocore/net/SNIActionPerformer.cc Outdated
Comment thread doc/admin-guide/files/sni.yaml.en.rst Outdated
Comment thread doc/admin-guide/files/sni.yaml.en.rst Outdated
Comment on lines +178 to +179
: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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@maskit
maskit requested review from bneradt and ywkaras June 15, 2023 23:10
@ywkaras

ywkaras commented Jun 19, 2023

Copy link
Copy Markdown

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

@bneradt

bneradt commented Jun 20, 2023

Copy link
Copy Markdown
Contributor

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

@maskit

maskit commented Jun 20, 2023

Copy link
Copy Markdown
Member Author

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

@maskit
maskit merged commit 6cfdc5d into apache:master Jun 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants