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

net/http: support unencrypted HTTP/2 (h2c) #67816

Closed
Tracked by #67810
neild opened this issue Jun 4, 2024 · 15 comments
Closed
Tracked by #67810

net/http: support unencrypted HTTP/2 (h2c) #67816

neild opened this issue Jun 4, 2024 · 15 comments

Comments

@neild
Copy link
Contributor

neild commented Jun 4, 2024

This issue is part of a project to move x/net/http2 into std: #67810

This proposal depends on the version selection API proposed in #67814.

The net/http package does not directly support unencrypted HTTP/2, sometimes referred to as "h2c".

Users may send HTTP/2 requests over an unencrypted connection by setting http2.Transport.AllowHTTP and providing a DialTLS function which opens an unencrypted connection. Users may accept unencrypted HTTP/2 requests by using the golang.org/x/net/http2/h2c package.

Neither of these mechanisms is very clean: Returning an unencrypted connection from DialTLS is unfortunate, and the interactions between net/http, h2c, and golang.org/x/net/http2 are fairly complicated.

I propose adding support for unencrypted HTTP/2 as a first-class feature to net/http.

HTTP/2 will be enabled by a new http.Protocol value:

const (
	// HTTP2 is the HTTP/2 protocol over an unsecured TCP connection.
	UnencryptedHTTP2 Protocol
)

When Server.Protocol contains UnencryptedHTTP2, the server will accept HTTP/2 requests on its unencrypted port(s).

When Transport.Protocol contains UnencryptedHTTP2 but not HTTP1, the transport will use unencrypted HTTP/2 for requests for http:// URLs ("Starting HTTP/2 with prior knowledge", RFC 9113 Section 3.3).

The HTTP/2 http2.Transport.AllowHTTP setting controls whether the transport permits requests using the http:// scheme. When Protocols contains UnencryptedHTTP1, the transport will ignore this setting and always permit http:// requests.

Upgrade: h2c

RFC 7540 Section 3.2 defines a mechanism for upgrading an HTTP/1 request to HTTP/2. The client sends an HTTP/1 request with a Upgrade: h2c header and a Http2-Settings header containing the proposed HTTP/2 settings. The server may respond with 101 Switching Protocols and convert the connection to HTTP/2.

RFC 9113 Section 3.1 deprecates this upgrade mechanism, stating: "This usage was never widely deployed and is deprecated by this document."

(For encrypted connections, the protocol is negotiated using TLS ALPN and the Upgrade header is not used.)

The golang.org/x/net/http2/h2c package supports Upgrade: h2c on server connections.

We do not currently support Upgrade: h2c for client connections. (This means that the h2c package's support for Upgrade: h2c has no test coverage, since we don't have any ability to make a client connection to exercise it with.)

I propose that we only add support for unencrypted HTTP/2 with prior knowledge, not Upgrade: h2c. Connection upgrade is complicated, since it requires us to convert a connection from one protocol to another mid-request. It comes with surprising limitations that are difficult to describe. (The initial HTTP/1 request body on an upgraded connection must be sent in its entirety before the client can begin transmitting HTTP/2 frames.) Most requests for unencrypted HTTP/2 focus on cases where the user has full control of both endpoints and can use prior knowledge. The current lack of support for client-side upgrade means that any users depending on it are doing so with non-Go clients. And, of course, the mechanism is deprecated.

@gopherbot gopherbot added this to the Proposal milestone Jun 4, 2024
@ianlancetaylor ianlancetaylor moved this to Incoming in Proposals Jun 4, 2024
@Jorropo
Copy link
Member

Jorropo commented Jun 4, 2024

https://pkg.go.dev/golang.org/x/net/http2/h2c currently states:

The first request on an h2c connection is read entirely into memory before the Handler is called.

Would that be fixed ?

@neild
Copy link
Contributor Author

neild commented Jun 4, 2024

Since I'm proposing to only support HTTP/2 with prior knowledge, not Upgrade: h2c, that condition would not apply. (The h2c package reads the first request into memory for upgraded connections, but not for with-prior-knowledge ones.)

@neild
Copy link
Contributor Author

neild commented Jun 4, 2024

A few more details on with-prior-knowledge vs. upgrade:

There are two ways to start an unencrypted HTTP/2 connection.

One mechanism is called "HTTP/2 with prior knowledge", in which the client knows the server supports unencrypted HTTP/2. The client opens an unencrypted TCP connection and starts the HTTP/2 protocol, beginning by sending the HTTP/2 client preface. This is just HTTP/2, only over an unencrypted connection rather than TLS.

The other mechanism involves starting a connection as HTTP/1 and negotiating an upgrade to HTTP/2. The client sends an HTTP/1 request with an Upgrade: h2c header, and the server either accepts the upgrade (sending a 100 Switching Protocols response) or rejects it (sending a normal HTTP/1 response). A big problem with this approach is that the client needs to finish sending the initial request before it can start sending HTTP/2 frames. This is complex to implement (because we need to switch protocols mid-request) and makes the initial request behave in ways distinct from either HTTP/1 or HTTP/2 requests. Notably, the server may need to consume the request body before it can send the response body, because the server may rely on receiving window size updates.

This is why the h2c package reads the entire request body for Upgrade: h2c requests: Doing so ensures that the connection is clear for the HTTP/2 implementation.

We could do something more sophisticated. For example, perhaps we could accept Upgrade: h2c only if the request body size is below some threshold and reject the upgrade otherwise.

However, it isn't clear that this complexity is worth it. While the general idea of "use HTTP/2 if you can, fall back to HTTP/1 if you can't" seems reasonable, I don't know of any actual use cases. So far as I can tell, previous requests for unencrypted HTTP/2 have been for scenarios where the user controls both endpoints, or where the user needs to interact with an endpoint that only speaks HTTP/2.

I think the right choice is to support with-prior-knowledge only, at least to start with, especially since the Upgrade: h2c path is deprecated in the latest HTTP/2 RFCs.

@rsc
Copy link
Contributor

rsc commented Jun 12, 2024

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc rsc moved this from Incoming to Active in Proposals Jun 12, 2024
@rsc
Copy link
Contributor

rsc commented Jun 20, 2024

This seems reasonable modulo the answer to #67814.

@neild
Copy link
Contributor Author

neild commented Jun 24, 2024

If we adopt the suggestion in #67814 (comment), the API change here would be:

func (p *Protocols) UnencryptedHTTP2() bool
func (p *Protocols) SetUnencryptedHTTP2(ok bool)

@rsc
Copy link
Contributor

rsc commented Jul 25, 2024

Have all remaining concerns about this proposal been addressed?

The proposal is to first adopt #67814 and then add

func (p *Protocols) UnencryptedHTTP2() bool
func (p *Protocols) SetUnencryptedHTTP2(ok bool)

The default will be to not support unencrypted HTTP/2 unless explicitly opted in.

@rsc
Copy link
Contributor

rsc commented Jul 31, 2024

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

The proposal is to first adopt #67814 and then add

func (p *Protocols) UnencryptedHTTP2() bool
func (p *Protocols) SetUnencryptedHTTP2(ok bool)

The default will be to not support unencrypted HTTP/2 unless explicitly opted in.

@rsc
Copy link
Contributor

rsc commented Aug 7, 2024

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

The proposal is to first adopt #67814 and then add

func (p *Protocols) UnencryptedHTTP2() bool
func (p *Protocols) SetUnencryptedHTTP2(ok bool)

The default will be to not support unencrypted HTTP/2 unless explicitly opted in.

@rsc rsc changed the title proposal: net/http: support unencrypted HTTP/2 (h2c) net/http: support unencrypted HTTP/2 (h2c) Aug 7, 2024
@rsc rsc moved this from Likely Accept to Accepted in Proposals Aug 7, 2024
@rsc rsc modified the milestones: Proposal, Backlog Aug 7, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/625175 mentions this issue: http2: support unencrypted HTTP/2 handoff from net/http

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/622976 mentions this issue: net/http: add support for unencrypted HTTP/2

gopherbot pushed a commit to golang/net that referenced this issue Nov 5, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Allow net/http to pass unencrypted net.Conns to Server/Transport.
We don't have an existing way to pass a conn other than a *tls.Conn
into this package, so (ab)use TLSNextProto to pass unencrypted
connections:

The http2 package adds an "unencrypted_http2" entry to the
TLSNextProto maps. The net/http package calls this function
with a *tls.Conn wrapping a net.Conn with an UnencryptedNetConn
method returning the underlying, unencrypted net.Conn.

For golang/go#67816

Change-Id: I31f9c1ba31a17c82c8ed651382bd94193acf09b9
Reviewed-on: https://go-review.googlesource.com/c/net/+/625175
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
gopherbot pushed a commit that referenced this issue Nov 21, 2024
Add an UnencryptedHTTP2 protocol value.

Both Server and Transport implement "HTTP/2 with prior knowledge"
as described in RFC 9113, section 3.3. Neither supports the
deprecated HTTP/2 upgrade mechanism (RFC 7540, section 3.2 "h2c").

For Server, UnencryptedHTTP2 controls whether the server
will accept HTTP/2 connections on unencrypted ports.
When enabled, the server checks new connections for
the HTTP/2 preface and routes them appropriately.

For Transport, enabling UnencryptedHTTP2 and disabling HTTP1
causes http:// requests to be made over unencrypted HTTP/2
connections.

For #67816

Change-Id: I2763c4cdec1c2bc6bb8157edb93b94377de8a59b
Reviewed-on: https://go-review.googlesource.com/c/go/+/622976
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@neild neild closed this as completed Nov 21, 2024
@dmitshur dmitshur modified the milestones: Backlog, Go1.24 Nov 22, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/630976 mentions this issue: net/http: skip test which depends on h2_bundle.go update

@neild neild reopened this Nov 22, 2024
@neild
Copy link
Contributor Author

neild commented Nov 22, 2024

Reopening to remember to remove a t.Skip in net/http after the HTTP/2 bundle is updated.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/631035 mentions this issue: all: update golang.org/x/net [generated]

gopherbot pushed a commit that referenced this issue Nov 22, 2024
For #67816

Change-Id: I9ba3a245d6b18758944ca5e206a15892b2aa6028
Reviewed-on: https://go-review.googlesource.com/c/go/+/630976
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Auto-Submit: Damien Neil <dneil@google.com>
gopherbot pushed a commit that referenced this issue Nov 22, 2024
A part of the keeping Go's vendored dependencies and generated code
up to date.

This updates h2_bundle.go with unencrypted HTTP/2 support.

For #36905.
For #67816.

[git-generate]
cd src
go get golang.org/x/net@v0.31.0
go mod tidy
go mod vendor
cd cmd
go get golang.org/x/net@v0.31.0
go mod tidy
go mod vendor
go generate -run=bundle std

Change-Id: I2b77f651b990f260fbe7d551c7a819518f1c983f
Reviewed-on: https://go-review.googlesource.com/c/go/+/631035
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Damien Neil <dneil@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/631037 mentions this issue: net/http: re-enable TestTransportServerProtocols

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Accepted
Development

No branches or pull requests

5 participants