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: UDP sockets on windows error on receive due to ICMP TTL #68614

Open
raggi opened this issue Jul 26, 2024 · 6 comments
Open

net: UDP sockets on windows error on receive due to ICMP TTL #68614

raggi opened this issue Jul 26, 2024 · 6 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows
Milestone

Comments

@raggi
Copy link
Contributor

raggi commented Jul 26, 2024

Go version

go1.22.5

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/raggi/Library/Caches/go-build'
GOENV='/Users/raggi/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/raggi/go/pkg/mod'
GONOPROXY='ra66i.org/raggi,rag.pub/raggi'
GONOSUMDB='ra66i.org/raggi,rag.pub/raggi'
GOOS='darwin'
GOPATH='/Users/raggi/go'
GOPRIVATE='ra66i.org/raggi,rag.pub/raggi'
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/Users/raggi/.cache/tailscale-go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/Users/raggi/.cache/tailscale-go/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.22.5'
GCCGO='gccgo'
AR='ar'
CC='clang'
CXX='clang++'
CGO_ENABLED='1'
GOMOD='/dev/null'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/bb/dyr_1n6j575g8nq85nmnfbt00000gn/T/go-build2102863150=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

We have observed errors in RecvFrom as a result of ICMP replies to UDP sockets on Windows, which is an unexpected behavior resulting from a Windows socket behavior quirk.

What did you see happen?

Socket recv generated an error as a result of ICMP replies to earlier sent packets.

What did you expect to see?

Socket recv should not generate an error due to ICMP received.

Detail & Proposal

Background: a prior round of this issue was fixed in 3114bd6 which addressed one case of ICMP reply (https://www.betaarchive.com/wiki/index.php?title=Microsoft_KB_Archive/263823), but there are two. The Godot project ran into this issue as well, and describes the issue and fix here: godotengine/godot@397b01d

The net package only disables SIO_UDP_CONNRESET, but not SIO_UDP_NETRESET, as such there are still ICMP responses that can lead to wsarecvfrom: The connection has been broken due to keep-alive activity detecting a failure while the operation was in progress.

We should set SIO_UDP_NETRESET as well as SIO_UDP_CONNRESET in order to get behavior that is most similar to bsd sockets on the other major platforms, where asynchronous ICMP replies are ignored unless explicitly opted in to.

@seankhliao seankhliao added OS-Windows NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jul 26, 2024
@seankhliao
Copy link
Member

cc @golang/windows

andrew-d added a commit to tailscale/tailscale that referenced this issue Jul 26, 2024
By default, Windows sets the SIO_UDP_CONNRESET and SIO_UDP_NETRESET
options on created UDP sockets. These behaviours make the UDP socket
ICMP-aware; when the system gets an ICMP message (e.g. an "ICMP Port
Unreachable" message, in the case of SIO_UDP_CONNRESET), it will cause
the underlying UDP socket to throw an error. Confusingly, this can occur
even on reads, if the same UDP socket is used to write a packet that
triggers this response.

The Go runtime disabled the SIO_UDP_CONNRESET behavior in 3114bd6, but
did not change SIO_UDP_NETRESET–probably because that socket option
isn't documented particularly well.

Various other networking code seem to disable this behaviour, such as
the Godot game engine (godotengine/godot#22332) and the Eclipse TCF
agent (link below). Others appear to work around this by ignoring the
error returned (anacrolix/dht#16, among others).

For now, until it's clear whether this ends up in the upstream Go
implementation or not, let's also disable the SIO_UDP_NETRESET in a
similar manner to SIO_UDP_CONNRESET.

Eclipse TCF agent: https://gitlab.eclipse.org/eclipse/tcf/tcf.agent/-/blob/master/agent/tcf/framework/mdep.c

Updates golang/go#68614

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I70a2f19855f8dec1bfb82e63f6d14fc4a22ed5c3
andrew-d added a commit to tailscale/tailscale that referenced this issue Jul 26, 2024
By default, Windows sets the SIO_UDP_CONNRESET and SIO_UDP_NETRESET
options on created UDP sockets. These behaviours make the UDP socket
ICMP-aware; when the system gets an ICMP message (e.g. an "ICMP Port
Unreachable" message, in the case of SIO_UDP_CONNRESET), it will cause
the underlying UDP socket to throw an error. Confusingly, this can occur
even on reads, if the same UDP socket is used to write a packet that
triggers this response.

The Go runtime disabled the SIO_UDP_CONNRESET behavior in 3114bd6, but
did not change SIO_UDP_NETRESET–probably because that socket option
isn't documented particularly well.

Various other networking code seem to disable this behaviour, such as
the Godot game engine (godotengine/godot#22332) and the Eclipse TCF
agent (link below). Others appear to work around this by ignoring the
error returned (anacrolix/dht#16, among others).

For now, until it's clear whether this ends up in the upstream Go
implementation or not, let's also disable the SIO_UDP_NETRESET in a
similar manner to SIO_UDP_CONNRESET.

Eclipse TCF agent: https://gitlab.eclipse.org/eclipse/tcf/tcf.agent/-/blob/master/agent/tcf/framework/mdep.c

Updates #10976
Updates golang/go#68614

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I70a2f19855f8dec1bfb82e63f6d14fc4a22ed5c3
raggi added a commit to raggi/sys that referenced this issue Jul 26, 2024
In order to get BSD like behavior with regard to ICMP, it is necessary
to set SIO_UDP_NETRESET as well as SIO_UDP_CONNRESET.

For golang/go#68614
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/601038 mentions this issue: windows: add SIO_UDP_NETRESET for net/ to use

raggi added a commit to raggi/go that referenced this issue Jul 26, 2024
Windows UDP sockets have novel behavior in response to ICMP, where by
default RecvFrom will receive a read error as a result of an incoming
ICMP packet. This behavior is not portable or consistent with behavior
on other platforms, so for consistency it is disabled here.

This is similar to, but a different case from the prior change 3114bd6 /
https://golang.org/issue/5834 that disabled one of the two flags
influencing behavior in response to the reception of related ICMP.

Updates golang#5834
Updates golang#68614
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/601397 mentions this issue: internal/poll: disable SIO_UDP_NETRESET on Windows

@alexbrainman
Copy link
Member

The Godot project ran into this issue as well, and describes the issue and fix here: godotengine/godot@397b01d

Is there some official documentation for this?

I searched for SIO_UDP_NETRESET, but I could not find anything.

Or are we supposed to assume that https://github.com/godotengine/godot know how it works.

Thank you.

Alex

andrew-d added a commit to tailscale/tailscale that referenced this issue Aug 15, 2024
By default, Windows sets the SIO_UDP_CONNRESET and SIO_UDP_NETRESET
options on created UDP sockets. These behaviours make the UDP socket
ICMP-aware; when the system gets an ICMP message (e.g. an "ICMP Port
Unreachable" message, in the case of SIO_UDP_CONNRESET), it will cause
the underlying UDP socket to throw an error. Confusingly, this can occur
even on reads, if the same UDP socket is used to write a packet that
triggers this response.

The Go runtime disabled the SIO_UDP_CONNRESET behavior in 3114bd6, but
did not change SIO_UDP_NETRESET–probably because that socket option
isn't documented particularly well.

Various other networking code seem to disable this behaviour, such as
the Godot game engine (godotengine/godot#22332) and the Eclipse TCF
agent (link below). Others appear to work around this by ignoring the
error returned (anacrolix/dht#16, among others).

For now, until it's clear whether this ends up in the upstream Go
implementation or not, let's also disable the SIO_UDP_NETRESET in a
similar manner to SIO_UDP_CONNRESET.

Eclipse TCF agent: https://gitlab.eclipse.org/eclipse/tcf/tcf.agent/-/blob/master/agent/tcf/framework/mdep.c

Updates #10976
Updates golang/go#68614

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I70a2f19855f8dec1bfb82e63f6d14fc4a22ed5c3
Asutorufa pushed a commit to Asutorufa/tailscale that referenced this issue Aug 23, 2024
By default, Windows sets the SIO_UDP_CONNRESET and SIO_UDP_NETRESET
options on created UDP sockets. These behaviours make the UDP socket
ICMP-aware; when the system gets an ICMP message (e.g. an "ICMP Port
Unreachable" message, in the case of SIO_UDP_CONNRESET), it will cause
the underlying UDP socket to throw an error. Confusingly, this can occur
even on reads, if the same UDP socket is used to write a packet that
triggers this response.

The Go runtime disabled the SIO_UDP_CONNRESET behavior in 3114bd6, but
did not change SIO_UDP_NETRESET–probably because that socket option
isn't documented particularly well.

Various other networking code seem to disable this behaviour, such as
the Godot game engine (godotengine/godot#22332) and the Eclipse TCF
agent (link below). Others appear to work around this by ignoring the
error returned (anacrolix/dht#16, among others).

For now, until it's clear whether this ends up in the upstream Go
implementation or not, let's also disable the SIO_UDP_NETRESET in a
similar manner to SIO_UDP_CONNRESET.

Eclipse TCF agent: https://gitlab.eclipse.org/eclipse/tcf/tcf.agent/-/blob/master/agent/tcf/framework/mdep.c

Updates tailscale#10976
Updates golang/go#68614

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I70a2f19855f8dec1bfb82e63f6d14fc4a22ed5c3
raggi added a commit to raggi/sys that referenced this issue Aug 28, 2024
In order to get BSD like behavior with regard to ICMP, it is necessary
to set SIO_UDP_NETRESET as well as SIO_UDP_CONNRESET.

Updates golang/go#68614
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/609295 mentions this issue: windows: add SIO_UDP_NETRESET constant

@dmitshur dmitshur added this to the Backlog milestone Sep 4, 2024
gopherbot pushed a commit to golang/sys that referenced this issue Sep 4, 2024
In order to get BSD like behavior with regard to ICMP, it is necessary
to set SIO_UDP_NETRESET as well as SIO_UDP_CONNRESET.

Updates golang/go#68614

Change-Id: Ibdf5b6ea6bc08a9d3a0aeac9037864670cf765c0
Reviewed-on: https://go-review.googlesource.com/c/sys/+/609295
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Funda Secgin <fundasecgin38@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows
Projects
None yet
Development

No branches or pull requests

6 participants