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

Automatically enable h2c when HTTP/2 is enabled without SSL #25856

Closed
wilkinsona opened this issue Apr 1, 2021 · 6 comments
Closed

Automatically enable h2c when HTTP/2 is enabled without SSL #25856

wilkinsona opened this issue Apr 1, 2021 · 6 comments
Assignees
Labels
status: noteworthy A noteworthy issue to call out in the release notes type: enhancement A general enhancement
Milestone

Comments

@wilkinsona
Copy link
Member

wilkinsona commented Apr 1, 2021

If you want to use HTTP/2 from end-to-end, an app that's running behind a proxy that's performing TLS termination will need to use h2c. We already support it, but there's no configuration property to enable it. Instead, you need to write some code and the complexity varies depending on the embedded container that you're using,

@wilkinsona wilkinsona added type: enhancement A general enhancement status: pending-design-work Needs design work before any code can be developed for: team-meeting An issue we'd like to discuss as a team to make progress labels Apr 1, 2021
@bric3
Copy link

bric3 commented Apr 1, 2021

That would be very valuable for application inside a mesh.

Additionally this could be interesting to support management endpoint as well, who need to be configured programmatically as well, see #25858

@bclozel
Copy link
Member

bclozel commented Apr 2, 2021

We've discussed that briefly with @scottfrederick and @snicoll. Here's a brief summary of our discussion.

Back when we introduced the HTTP/2 feature, we didn't want to push for the "h2c" use case because many key features of http/2 are about security. There were additional concerns about compatibility, since protocol upgrades over HTTP are not always well supported by proxies and server.

Now platform and service mesh products seem to enable the h2c use case in a different way and making this use case easier could help the community.

It seems that CloudFoundry will not rely on h2c but on http/2, since the routing component relies on TLS for route integrity, see cloudfoundry/routing-release/issues/200.

@onobc
Copy link
Contributor

onobc commented Apr 4, 2021

@wilkinsona I see the labels in the top-right corner for this issue. But... I am super interested and cobbled together a 1st pass proposal.

NOTE

  • It does not handle Jetty yet as its a bit more involved and I don't want to prematurely invest in this
  • It does not have tests yet (same as above)
  • It does not have docs yet (same as above)
  • The config props needs some thought (options listed in PR)

The spirit of the proposal is enabling the aforementioned code changes by respecting the new h2c property when building up "all things http2". This is the least intrusive approach. However, I understand that there may be a bigger picture to solve and there may be a desire to overhaul this area at a higher level (hence all the variant bright colored pills in the top right corner of the issue 😼 ) .

Proposal is here (I did not submit in this repo to avoid clutter in case you take a different route).

Once you take a scan please lmk what you think:

  • If you want to continue w/ this approach I will happily take it to completion.
  • If you want me to pivot the approach I can do that and take it completion as well.
  • Also, if team wants to handle it I respect that too. There is plenty of other stuff for me to keyboard on 😃

@philwebb philwebb removed the for: team-meeting An issue we'd like to discuss as a team to make progress label Apr 7, 2021
@philwebb philwebb added this to the 2.5.x milestone Apr 7, 2021
@wilkinsona wilkinsona self-assigned this Apr 12, 2021
@wilkinsona
Copy link
Member Author

Having explored a few different options for the configuration property, I've come to the conclusion that we probably do not need one. Instead, we can allow HTTP/2 to be enabled without SSL. In this configuration, h2c will be used. When both HTTP/2 and SSL are enabled, h2 will be used. Here is what that could look like. I'd like to get the rest of the team's opinions before merging.

@wilkinsona wilkinsona added the for: team-meeting An issue we'd like to discuss as a team to make progress label Apr 12, 2021
@philwebb philwebb added status: noteworthy A noteworthy issue to call out in the release notes and removed for: team-meeting An issue we'd like to discuss as a team to make progress status: pending-design-work Needs design work before any code can be developed labels Apr 12, 2021
@wilkinsona
Copy link
Member Author

Closed by 713c0fc.

@wilkinsona wilkinsona modified the milestones: 2.5.x, 2.5.0-RC1 Apr 12, 2021
@bric3
Copy link

bric3 commented Apr 12, 2021

Great to see this happening!

Will this work with the management endpoints as well?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: noteworthy A noteworthy issue to call out in the release notes type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

5 participants