-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[9.0] Automatically enable http2 if running Kibana with TLS configured #194067
Comments
Pinging @elastic/kibana-core (Team:Core) |
Also cc @pgayvallet as our http2 expert 🙂 |
This proposal makes sense to me. One caveat is clients running with only h2 requires TLSv1.2 or better. While I expect most/all installations will have this configured, it's technically possible to tell Kibana to only support |
Given that there's http1 fallback I don't foresee this causing any issues, but we are essentially defaulting to enabling http2 so it feels like it would be worth highlighting this in our release notes. |
Just to clarify: h2 is the protocol, and http2 is the spec of having h2 over tls. http2 requires TLS1.2, because TLS Protocol negotiation (ALPN) is a TLS1.2+ feature, so http2-to-https fallback will not work if Kibana is configured to only run on TLS1.1. So Larry is right, we should not enable http2 automatically if TLS is configured to run only on tls1.1. I already implemented that check in the http2 support PR, so it should only be about reusing that logic: kibana/packages/core/http/core-http-server-internal/src/http_config.ts Lines 266 to 271 in fe83c0d
I don't see any other problem with that proposal, and think it's a great idea. |
… is not enabled or protocol is set to HTTP1 (#204384) ## Summary resolves #194067 Set HTTP2 as default if ssl is enabled. resolves #194065 Add deprecation log if ssl is not enabled or if protocol is set to http1 <img width="1665" alt="Screenshot 2024-12-17 at 17 06 50" src="https://github.com/user-attachments/assets/3bc7ff57-1079-4a27-90d2-88f3e09093d6" /> <img width="1727" alt="Screenshot 2024-12-17 at 17 06 22" src="https://github.com/user-attachments/assets/d5489705-6cd6-4e09-8327-fdd0f54292ea" /> ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) ### Identify risks Does this PR introduce any risks? For example, consider risks like hard to test bugs, performance regression, potential of data loss. Describe the risk, its severity, and mitigation for each identified risk. Invite stakeholders and evaluate how to proceed before merging. - [ ] [See some risk examples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx) - [ ] ... --------- Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Rudolf Meijering <[email protected]>
… is not enabled or protocol is set to HTTP1 (elastic#204384) ## Summary resolves elastic#194067 Set HTTP2 as default if ssl is enabled. resolves elastic#194065 Add deprecation log if ssl is not enabled or if protocol is set to http1 <img width="1665" alt="Screenshot 2024-12-17 at 17 06 50" src="https://github.com/user-attachments/assets/3bc7ff57-1079-4a27-90d2-88f3e09093d6" /> <img width="1727" alt="Screenshot 2024-12-17 at 17 06 22" src="https://github.com/user-attachments/assets/d5489705-6cd6-4e09-8327-fdd0f54292ea" /> ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) ### Identify risks Does this PR introduce any risks? For example, consider risks like hard to test bugs, performance regression, potential of data loss. Describe the risk, its severity, and mitigation for each identified risk. Invite stakeholders and evaluate how to proceed before merging. - [ ] [See some risk examples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx) - [ ] ... --------- Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Rudolf Meijering <[email protected]>
We want http2 to be the default experience for as many Kibana users as possible. What if, starting in 9.0, we were to automatically enable http2 (
server.protocol
) if we detected that TLS is enabled (server.ssl.*
)? Are there any risks/downsides?cc @timductive @rayafratkina @thomasneirynck @elastic/kibana-security
The text was updated successfully, but these errors were encountered: