Skip to content

Remove Jetty SNI check#2727

Closed
devinrsmith wants to merge 1 commit intodeephaven:mainfrom
devinrsmith:jetty-no-sni-check
Closed

Remove Jetty SNI check#2727
devinrsmith wants to merge 1 commit intodeephaven:mainfrom
devinrsmith:jetty-no-sni-check

Conversation

@devinrsmith
Copy link
Copy Markdown
Member

No description provided.

// configured trust stores. An SNI-less Jetty server setup potentially affords proxied deployments a simpler
// configuration model (essentially, the server does not need to know it is being proxied).
final boolean sniHostCheck = false;
httpConfig.addCustomizer(new SecureRequestCustomizer(sniHostCheck));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I believe this will conflict with #2548, which explicitly set it the opposite way.

I would still prefer that we fail "safe" instead of "unsafe" as this is doing, though would be happy to see it fail during the SNI handshake instead of during the HTTP conv as it is doing. If we want to support "unsafe" like this, my opinion is that it should default to safe and require explicit configuration to make it unsafe.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It's probably worthwhile to follow the same pattern here #2731 for configuring SNI - I'll wait until that has merged until pursuing this further.

@devinrsmith
Copy link
Copy Markdown
Member Author

Superseded by #3843

@github-actions github-actions Bot locked and limited conversation to collaborators May 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants