An "optimistic ACK attack" involves an attacker sending ACKs
for packets it hasn't received, causing the victim's
congestion controller to improperly send at a higher rate.
The standard defense against this attack is to skip the occasional
packet number, and to close the connection with an error if the
peer ACKs an unsent packet.
Implement this defense, increasing the gap between skipped
packet numbers as a connection's lifetime grows and correspondingly
the amount of work required on the part of the attacker.
Change-Id: I01f44f13367821b86af6535ffb69d380e2b4d7b7
Reviewed-on: https://go-review.googlesource.com/c/net/+/664298
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>
The sentPacket type tracks the state of a packet sent to the peer.
Packets can be in progress, acknowledged, or lost.
Track this state with an enum rather than a set of bools,
to avoid the possibility of nonsensical states such as a packet
being both acknowledged and lost.
This also simplifies a following change to add an "unsent" state
for intentionally skipped packet numbers.
Change-Id: I87c8fc399c72337c033ab7ec5ec8db2c56c732f9
Reviewed-on: https://go-review.googlesource.com/c/net/+/664297
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>
When discarding keys for a packet number space, we remove any
in-flight packets in that space from our count of bytes in flight.
Skip any packets in the space that were already acked or declared lost.
Also skip over acked/lost packets when discarding state in response
to a Reset message. This change shouldn't have any effect, since we
never declare packets acked/lost until we receive an ACK, and we
never handle a Reset after receiving an ACK.
Change-Id: I8842f03aa06e57a834a211f1284a48fe5d469db3
Reviewed-on: https://go-review.googlesource.com/c/net/+/664335
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>
Decoding QUIC packet numbers requires keeping track of the largest
packet number received so far from the peer. Our tests haven't
bothered doing that so far, so tests can't work with packet
numbers past 255.
Fix that so we can write tests that use more packets.
Change-Id: Icb795e5cf69794381c12a3a03b0da6bcf47a69c0
Reviewed-on: https://go-review.googlesource.com/c/net/+/664296
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>
The tests in qpack_decode_test.go require synctest helpers from
http3_test.go, but that file has a goexperiment.synctest build
constraint.
To make builds work when GOEXPERIMENT=nosynctest is specified the
synctest helpers are refactored into http3_synctest_test.go (with the
same build constraint) and the non-synctest related functionality is
kept in http3_test.go.
Change-Id: Iae339dc1895f27e7ac5ba985e204f4868c229a4d
Reviewed-on: https://go-review.googlesource.com/c/net/+/660535
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Damien Neil <dneil@google.com>
If for some reason the server sends an HTTP/1.1 response
starting with "HTTP/1.1 " (9 bytes), http2 transport interprets that as
a valid frame header for an expected payload of ~4MB, and fails with
non-descriptive messages like "unexpected EOF".
This could happen, for example, if ALPN is misconfigured on the server's
load balancer.
This change attempts to improve that feedback by noting in the error
messages whenever the frame header was exactly the "HTTP/1.1 " bytes.
Change-Id: I7bf9ed2ee7f299b939b9004386f5bfa30a4e9032
GitHub-Last-Rev: d6e410daa3
GitHub-Pull-Request: golang/net#224
Reviewed-on: https://go-review.googlesource.com/c/net/+/623155
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Sean Liao <sean@liao.dev>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Sean Liao <sean@liao.dev>
This change ensures that the <search> tag correctly closes an open <p> tag when encountered during parsing.
Changes:
- Added <search> to the list of elements that should close an open <p> tag in parse.go.
- Updated the second list in parse.go to ensure consistency.
- Updated html/atom/gen.go, table.go, and table_test.go accordingly.
- Modified parse_test.go to use strings.Builder instead of bytes.Buffer.
- Updated test error messages to follow Go’s conventions.
- Fixed an accidental colon in the comment in parse.go.
Change-Id: I5835da69f6bb0e14c483e55b7ae82915ae958dc1
Reviewed-on: https://go-review.googlesource.com/c/net/+/655457
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Rely on functions from the slices package where convenient. Drop custom max functions in favor of max builtin. Remove unused non-exported functions.
Reduce the number of bounds checks. Replace calls to strings.LastIndex by calls to strings.LastIndexByte.
goos: darwin
goarch: amd64
pkg: golang.org/x/net/publicsuffix
cpu: Intel(R) Core(TM) i7-6700HQ CPU @ 2.60GHz
│ old │ new │
│ sec/op │ sec/op vs base │
PublicSuffix-8 13.46µ ± 0% 13.23µ ± 0% -1.67% (p=0.000 n=20)
│ old │ new │
│ B/op │ B/op vs base │
PublicSuffix-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=20) ¹
¹ all samples are equal
│ old │ new │
│ allocs/op │ allocs/op vs base │
PublicSuffix-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=20) ¹
¹ all samples are equal
Change-Id: Id72967560884d98a5c0791ccea73dbb27d120c2c
GitHub-Last-Rev: 87567e7cb5
GitHub-Pull-Request: golang/net#233
Reviewed-on: https://go-review.googlesource.com/c/net/+/652236
Reviewed-by: Damien Neil <dneil@google.com>
Commit-Queue: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
The http2 package uses a precursor to the experimental
testing/synctest package, parsing runtime.Stack output
to determine when goroutines are idle.
When GOOS=js, some tests which use t.Log are flaky.
t.Log blocks in the syscall package writing to stdout.
The GOOS=js implementation of the syscall leaves the goroutine
blocked on a channel operation, which synctest interprets
as the goroutine being "durably blocked".
Fix the http2 synctest to treat any goroutine blocked in the
syscall package as not being durably blocked.
Making this fix reveals another bug when GOOS=js: Looping
while calling runtime.Gosched does not appear to permit
syscalls to make progress. Add a few time.Sleep(1) calls
while waiting for idleness to work around the problem.
While changing things in here, change http2's synctest
to not treat goroutines blocked on mutex operations as
durably blocked. This matches the behavior of testing/synctest.
(This would all be simpler if we just used testing/synctest,
but we don't want to make the http2 package depend on an
experimental API.)
Fixesgolang/go#67693
Change-Id: I889834e97e4a33f4ef232278b1a78af00d52d261
Reviewed-on: https://go-review.googlesource.com/c/net/+/653696
Reviewed-by: Jonathan Amsterdam <jba@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Damien Neil <dneil@google.com>
This is motivated by a few reasons. One, the upstream package has more
examples, and no one should be looking at this old package to learn how
to use it. Seeing an example might make it seem like the scope of the
documentation here is to provide examples, and that there aren't many
of them. Instead of trying to add more examples or maintain the current
one by porting the de-flake enhancement from CL 460999, delete the only
example here.
Second, running 'go fix ./...' causes the 'context' fix to rewrite the
import path of the example from "golang.org/x/net/context" to "context".
That is a false positive in the fix, and I would've liked it fix the
fix, but it only has the AST information at this time, not type info,
so the import path isn't currently available to the check. That means
it can't know when it's running on the golang.org/x/net/context package,
which is the one place it should skip the rewrite.
It seems simpler to just delete the example, and then it becomes
possible to use 'go fix ./...' safely on the entire x/net module.
For golang/go#49506.
Change-Id: I97eba33ca2e1f2960aef8340d8b561639a26ee48
Reviewed-on: https://go-review.googlesource.com/c/net/+/650156
Reviewed-by: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
On Darwin, the AF_FAMILY byte of a sockaddr for a netmask or genmask
can be ignored if unreasonable. In such cases, it is the family of the
DST address that should instead be used.
Additionally, fixing faulty test data. 192.168.86.0 is a Class C network
address, that should have a subnet mask of 255.255.255.0. What's more is
the data can also be flag as incorrect considering structure padding
rules alone.
Further more, you can validate that `route get` will never actually return a
netmask for a host query, even though it should be 255.255.255.255.
You can run the following to check:
route -n get -host 127.0.0.1
You will note the reply has no mention of netmask.
Depends on CL 646556 - https://go.dev/cl/646556Fixesgolang/go#71578.
Change-Id: Id95669b649a416a380d26c5cdba0e3d1c4bc1ffb
GitHub-Last-Rev: 20064b2797
GitHub-Pull-Request: golang/net#232
Reviewed-on: https://go-review.googlesource.com/c/net/+/647176
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Commit-Queue: Ian Lance Taylor <iant@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
Previously, we enforced minimum length requirements for sockaddr, but
the route command can legitimately parse shorter lengths. This change
treats any sockaddr with length less than the address offset as an
unspecified address (0.0.0.0 for IPv4 or :: for IPv6), as discern by
monitoring the route command.
To replicate the issue, prior to the fix, execute the following:
First:
route -n monitor
Next:
sudo route -n add -inet6 -ifscope en11 -net :: \
-netmask :: fe80::2d0:4cff:fe10:15d2
The route command that is actively monitoring will print something such
as:
RTM_ADD: Add Route: len 152, pid: 81198, seq 1, errno 0, ifscope 13, flags:<UP,GATEWAY,DONE,STATIC,IFSCOPE>
locks: inits:
sockaddrs: <DST,GATEWAY,NETMASK>
:: fe80::2d0:4cff:fe10:15d2 ::
Prior to the fix, if you had attempted parse the above message, PareRIB
would have returned errInvalidAddr which is clearly false.
Fixesgolang/go#71557
Change-Id: Iec86cc9b05a765b6e67e95a4e30ff31f66f3d17e
GitHub-Last-Rev: 396d8a27da
GitHub-Pull-Request: golang/net#231
Reviewed-on: https://go-review.googlesource.com/c/net/+/646556
Commit-Queue: Ian Lance Taylor <iant@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
sa_len of 0 should be valid, for Chapter 18 of UNIX® Network Programming
Volume 1, Third Edition: The Sockets Networking API, states:
The socket address structures are variable-length, but this code assumes
that each has an sa_len field specifying its length. There are two
complications that must be handled. First, the two masks, the network
mask and the cloning mask, can be returned in a socket address structure
with an sa_len of 0, but this really occupies the size of an unsigned
long. (Chapter 19 of TCPv2 discusses the cloning feature of the 4.4BSD
routing table). This value represents a mask of all zero bits, which we
printed as 0.0.0.0 for the network mask of the default route in our
earlier example.
There are other references in the book which also state sa_len of 0 is
valid.
Fixesgolang/go#70528
Change-Id: I9205a674f9cdf8091b1cc8b8a56609cd1cf4c670
GitHub-Last-Rev: df63086c54
GitHub-Pull-Request: golang/net#230
Reviewed-on: https://go-review.googlesource.com/c/net/+/646555
Auto-Submit: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
When a stream ends in the middle of a frame,
return a non-EOF error from Read or ReadByte.
When a stream ends at the end of a frame,
don't return io.EOF from the Read call that reads
the last byte of the frame.
(This complicates stream.Read slightly,
but means that code that reads frames consistently
never sees an io.EOF, but gets an error if it tries
to read past the end of a frame.)
For golang/go#70914
Change-Id: If1b852716fe5e3aa3503f6970e2e1fba2ebb5f48
Reviewed-on: https://go-review.googlesource.com/c/net/+/644116
Auto-Submit: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Basic support for encoding/decoding QPACK headers.
QPACK supports three forms of header compression:
Huffman-encoding of literal strings, a static table of
well-known header values, and a dynamic table of
header values negotiated between encoder and decoder
at runtime.
Right now, we support Huffman compression and the
static table, but not the dynamic table.
This is a supported mode for a QPACK encoder or
decoder, so we can leave dynamic table support
for after the rest of HTTP/3 is working.
For golang/go#70914
Change-Id: Ib694199b99c752a220d43f3a309169b16020b474
Reviewed-on: https://go-review.googlesource.com/c/net/+/642599
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Add the first rudiments of an HTTP/3 client.
The client currently opens a QUIC connection and creates a control
stream on it, and nothing else.
Add surrounding test infrastructure for examining the client's
behavior. Tests use the experimental testing/synctest package
and will only run when the Go version is at least Go 1.24 and
GOEXPERIMENT=synctest is set.
For golang/go#70914
Change-Id: I19803187a8e62c461f60d7a1d44c2a408377e342
Reviewed-on: https://go-review.googlesource.com/c/net/+/642516
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
