Skip to content
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

Closed
lukeelmers opened this issue Sep 25, 2024 · 5 comments · Fixed by #204384
Closed

[9.0] Automatically enable http2 if running Kibana with TLS configured #194067

lukeelmers opened this issue Sep 25, 2024 · 5 comments · Fixed by #204384
Assignees
Labels
Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v9.0.0

Comments

@lukeelmers
Copy link
Member

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

@lukeelmers lukeelmers added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v9.0.0 labels Sep 25, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-core (Team:Core)

@lukeelmers
Copy link
Member Author

Also cc @pgayvallet as our http2 expert 🙂

@legrego
Copy link
Member

legrego commented Sep 26, 2024

This proposal makes sense to me. One caveat is clients running with only TLSv1.1.

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 TLSv1.1. I don't think we should automatically enable http2 in this scenario.

@rudolf
Copy link
Contributor

rudolf commented Oct 1, 2024

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.

@pgayvallet
Copy link
Contributor

pgayvallet commented Oct 7, 2024

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:

if (rawConfig.protocol === 'http2' && !rawConfig.http2.allowUnsecure) {
const err = ensureValidTLSConfigForH2C(rawConfig.ssl);
if (err) {
return err;
}
}

I don't see any other problem with that proposal, and think it's a great idea.

jesuswr added a commit that referenced this issue Jan 3, 2025
… 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]>
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this issue Jan 13, 2025
… 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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v9.0.0
Projects
None yet
6 participants