Instead of using strings.ToLower and == to check case insensitive
equality, just use strings.EqualFold, even when the strings are only
ASCII. This prevents us unnecessarily lowering extremely long strings,
which can be a somewhat expensive operation, even if we're only
attempting to compare equality with five characters.
Thanks to Guido Vranken for reporting this issue.
Fixesgolang/go#70906
Fixes CVE-2024-45338
Change-Id: I323b919f912d60dab6a87cadfdcac3e6b54cd128
Reviewed-on: https://go-review.googlesource.com/c/net/+/637536
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Gopher Robot <gobot@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Tatiana Bradley <tatianabradley@google.com>
When dropping packet protection keys for a number space:
Check to see if there is unused CRYPTO data received from the peer
in the space. If so, close the connection with an error. This can
only happen if the peer has sent us data with a gap in it. We
can never read the data that fills that gap (because we're dropping
the key it would be encrypted with), and this situation cannot
happen without the peer sending invalid TLS handshake data.
Drop any buffered CRYPTO data being sent to the peer.
Under normal operations, we may have data that was sent to the peer
but which we haven't received an ACK for yet. The peer has
received the data (or we wouldn't be dropping the number space)
and we will never see the ACK (because we're dropping the key it
would be encrypted with).
Fixesgolang/go#70704
Change-Id: I53380169cb59a2a6f87e69b38522ba81ad38c2b0
Reviewed-on: https://go-review.googlesource.com/c/net/+/634617
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Auto-Submit: Damien Neil <dneil@google.com>
CL 586249 unified frame read/write functions used by client
and server tests, but inadvertently broke some benchmarks.
Fix those benchmarks.
This mostly restores the previous behavior of the affected
benchmarks (for example, testing only to see that a DATA frame
contains an END_STREAM marker, ignoring the number of bytes
in the frame).
Fixesgolang/go#70647
Change-Id: I2b0099c3513ac8754d11c4e37b7d63277a0fb0b1
Reviewed-on: https://go-review.googlesource.com/c/net/+/633055
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Antonio Ojea <aojea@google.com>
gRPC has an unfortunate behavior of stictly rate limiting
the number of PING frames that it will receive. The default is
two PING frames every two hours when no requests are in flight;
two PING frames every five minutes when a request is in flight;
and the limit resets every time the gRPC endpoint sends a
HEADERS or DATA frame.
When sending a RST_STREAM frame, the Transport can bundle a PING
frame with it to confirm the server is responding. When canceling
several requests in succession, this can result in hitting the
gRPC ping limit.
Work around this gRPC behavior by sending at most one bundled
PING per HEADERS or DATA frame received. We already limit
ourselves to one PING in flight at a time; now, when we receive
a PING response, disable sending additional bundled PINGs
until we read a HEADERS/DATA frame.
This does not affect keep-alive pings.
Fixesgolang/go#70575.
Change-Id: I7c4003039bd2dc52106b2806ca31eeeee37b7e09
Reviewed-on: https://go-review.googlesource.com/c/net/+/632995
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Auto-Submit: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
When we create a new connection for a request, the request should
fail if the connection attempt fails.
There is a race condition which can cause this to not happen:
- net/http sends a request to a http2.Transport
- the http2.Transport returns ErrNoCachedConn
- net/http creates a new tls.Conn and passes it to the http2.Transport
- the http2.Transport adds the conn to its connection pool
- the connection immediately encounters an error
- the http2.Transport removes the conn from its connection pool
- net/http resends the request to the http2.Transport
- the http2.Transport returns ErrNoCachedConn, and the process repeates
If the request is sent to the http2.Transport before the connection
encounters an error, then the request fails. But otherwise, we get
stuck in an infinite loop of the http2.Transport asking for a new
connection, receiving one, and throwing it away.
To fix this, leave a dead connection in the pool for a short while
if it has never had a request sent to it. If a dead connection is
used for a new request, return an error and remove the connection
from the pool.
Change-Id: I64eb15a8f1512a6bda52db423072b945fab6f4b5
Reviewed-on: https://go-review.googlesource.com/c/net/+/625398
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
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>
Consider the case of an unresponsive client connection, where
the server has stopped responding. We send an infinite sequence of
requests to the connection in sequence, each with a timeout.
Each request counts against the concurrency limit for the
connection while active, but when a request times out we send
a RST_STREAM and free up the concurrency slot it was using.
We continue to try to send requests to the connection forever (or
until the kernel closes the underlying TCP connection, or until
ReadIdleTimeout/WriteByteTimeout results in us closing the connection).
Defend against this scenario by counting a canceled request
against the connection concurrency limit until we confirm the
server is responding. Specifically:
Track the number of in-flight request cancellations in cc.pendingResets.
This total counts against the connection concurrency limit.
When sending a RST_STREAM for a canceled request, increment
cc.pendingResets. Send a PING frame to the server, unless a PING
is already in flight.
When receiving a PING response, set cc.pendingResets to 0.
A hung connection will be used for at most
SETTINGS_MAX_CONCURRENT_STREAMS requests.
When StrictMaxConcurrentStreams is false, we will create a
new connection after reaching the concurrency limit for a hung one.
When StrictMaxConcurrentStreams is true, we will continue to
wait for the existing connection until some timeout closes it
or it becomes responsive again.
For golang/go#59690
Change-Id: I0151f9a594af14b32bcb6005a239fa19eb103704
Reviewed-on: https://go-review.googlesource.com/c/net/+/617655
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Replace Transport's limit of 5 1xx responses with a limit based
on the maximum header size: The total size of all 1xx response
headers must not exceed the limit we use on the size of the
final response headers.
(This differs slightly from the corresponding HTTP/1 change,
which imposes a limit on all 1xx response headers *plus* the
final response headers. The difference isn't substantial,
and this implementation fits better with the HTTP/2 framer.)
When the user is reading 1xx responses using a Got1xxResponse
client trace hook, disable the limit: Each 1xx response is
individually limited by the header size limit, but there
is no limit on the total number of responses. The user is
responsible for imposing a limit if they want one.
For golang/go#65035
Change-Id: I9c19dbf068e0f580789d952f63113b3d21ad86fc
Reviewed-on: https://go-review.googlesource.com/c/net/+/615295
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Very minor tweaks:
- Remove (c) pseudosymbol.
- Remove "All Rights Reserved."
- Change "Google Inc." (no longer exists) to "Google LLC".
[git-generate]
echo '
,s/\(c\) //
,s/ All rights reserved.//
,s/Google Inc./Google LLC/
w
q
' | sam -d LICENSE
Change-Id: Ibaa49e00dd08950a577e4343bfc574980d327995
Reviewed-on: https://go-review.googlesource.com/c/net/+/598579
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
CL 111835 changed Transport stream ID numbering to start at
stream 3 when AllowHTTP is set. This was based on a
misunderstanding:
When a connection upgrades an HTTP/1.1 request to HTTP/2,
the initial HTTP/1.1 request occupies stream 1.
However, Transport does not perform HTTP protocol upgrades.
When using a Transport to send unencrypted HTTP/2 requests,
the entire connection uses HTTP/2, the first request is
sent as HTTP/2, and there is no reason not to use stream 1
for this request.
Starting from stream 3 is mostly harmless,
but ClientConn.idleStateLocked assumes that client streams
start from 1. This causes it to misidentify new single-use
connections as having already sent a request (when AllowHTTP
is set), and therefore not suitable for use.
Revert to always starting stream IDs at 1.
Fixesgolang/go#67671
Change-Id: I97c89de4ae49623d916f9dbd200f8252d2fd4247
Reviewed-on: https://go-review.googlesource.com/c/net/+/591275
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Client and server tests both write frames to a test connection
and read frames back. Frame reads are usually paired with
test expectations.
Unify the API used for frame reads/writes in tests.
Introduce a testConnFramer type that implements a common set
of read/write methods, and embed it in both client and server
test types.
Change-Id: I6927c43459ba24f150a21c058a92797754f82bf1
Reviewed-on: https://go-review.googlesource.com/c/net/+/586249
Reviewed-by: Jonathan Amsterdam <jba@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
clientStream.requestedGzip is set from clientStream.writeRequest,
and examined by clientConn.readLoop. I'm not sure if there's
any possible way for an actual data race to happen here in
practice, since the read loop should only examine the field
after the request is sent by writeRequest, but it's enough
for the race detector to complain.
Set the field in ClientConn.roundTrip instead, before
the clientStream has become accessible to any other goroutines.
No test, but a following CL has race detector failures without
this change.
Change-Id: Id30f1b95bcfcc35c213440e0e47cce3feaaff06d
Reviewed-on: https://go-review.googlesource.com/c/net/+/586245
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
When run with -coverspec, tests track which portions of the
specification they cover and the TestSpecCoverage test produces
an error if any sections lack test coverage.
This is a lovely idea, and perhaps we should resurrect it at
some point, but there is currently exactly one coverage
annotation, dating back to the first commit of this package.
Change-Id: I5d2d8a1032b783d113ed0982f7e97a3bd1c07a33
Reviewed-on: https://go-review.googlesource.com/c/net/+/586243
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
