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

[raggi]

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.

[gabyhelp]

Related Issues and Documentation

• x/net/icmp: listen icmp in Windows not work properly #38427
• internal/poll: sending a zero-byte-payload UDP packet on Windows is a no-op #26668 (closed)
• net: TCP RST handling is unreliable on OSX #50254
• net: ReadFromUDP falls into busy waiting on empty UDP packet #59704 (closed)
• x/net/icmp: PacketConn.ReadFrom on an unprivileged PacketConn on darwin returns the entire IP packet #47369
• net: no checksum processing on loopback #23599 (closed)
• net: UDPConn.ReadFrom returns error for oversized datagram on Windows #14074 (closed)

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[seankhliao]

cc @golang/windows

[gopherbot]

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

[gopherbot]

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

[alexbrainman]

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

[gopherbot]

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

[qmuntal]

FYI: I asked the Microsoft Learn team to document SIO_UDP_NETRESET. If it ends up being added, then we can use it to fix this issue.

[qmuntal]

That was quick: https://learn.microsoft.com/en-us/windows/win32/winsock/winsock-ioctls#sio_udp_netreset.

[qmuntal]

@raggi do you feel like rebasing https://go-review.googlesource.com/c/go/+/601397 and adding some test there to demonstrate that SIO_UDP_NETRESET does something useful?

[raggi]

@qmuntal I'll work on the rebase right away. As far as adding a test goes, this requires infrastructure involvement, specifically I need a signal from the infrastructure to inform the test when it has sufficient privileges for raw packet write access, so that it can skip otherwise.

[raggi]

@qmuntal I notice that you recently adjusted how the related setting SIO_UDP_CONNRESET was configured, but I was unable to find a test covering that behavior. Are you able to point me at a test covering that?

[qmuntal]

@qmuntal I notice that you recently adjusted how the related setting SIO_UDP_CONNRESET was configured, but I was unable to find a test covering that behavior. Are you able to point me at a test covering that?

I'm afraid can't help there, I just moved SIO_UDP_CONNRESET from one file to another.

[raggi]

The net/ package as a whole has an unverified expectation of the PAL it implements, that UDP sockets do not error on this case, but has no mechanism today to verify that behavior on any platform. There are several possible ways to do so, which are all relatively invasive:

• Hook/mock the platform API/ABI to inject errors, however this is either platform specific which would be a shame as a portable solution would prevent this problem from being whack-a-mole on other ports.
• Adjust system level routing to trigger this issue, however this requires either some namespace type integration on the system or control over shared system resources in the test, which inevitably binds deeply to platform specific APIs as well, which are notably not covered by existing standard library APIs.
• Use raw packet interfaces which are at least commonly available on many platforms to inject the relevant packets. Unfortunately this still requires substantially elevated privileges to execute and so to avoid a change in requirements for running tests the test would need to be skipped when those privileges are not available.

Given this complexity and the lack of any existing standard of coverage for this behavior on any platform and sibling code, I think this should be done in a separate change at a separate time.