icmp: workaround for gvisor's fake ICMP echo

[Lanius-collaris]

Not working. I'm trying inserting code in other positions.

[ignoramous]

Thanks.

I wonder if this forceful default handling of ICMP is a bug on gvisor upstream despite having set a ICMP handler? It was working as I'd expect it to in previous (believe a year or so ago) gvisor version.

[Copilot]

Pull Request Overview

This PR introduces a workaround for gvisor's fake ICMP echo by modifying the ICMP packet types and recalculating checksums for both ICMPv6 and ICMPv4 before processing.

• Adds a new ICMPHackTarget and associated Action method for intercepting and modifying ICMP packets.
• Implements restore functions to revert the experimental packet type changes and adjusts the outbound handler to invoke the hack.

Comments suppressed due to low confidence (2)

intra/netstack/icmp.go:36

• [nitpick] Consider replacing the magic number 200 with a clearly named constant (e.g., ICMPv6ExperimentalType) for improved readability and maintenance.

icmp6Hdr.SetType(200)

intra/netstack/icmp.go:56

• [nitpick] Replace the magic number 253 with a named constant (e.g., ICMPv4ExperimentalType) to enhance clarity and consistency.

icmp4Hdr.SetType(253)

[Lanius-collaris]

It was working as I'd expect it to in previous (believe a year or so ago) gvisor version.

🤔Will current handler work if these lines are deleted?
https://github.com/google/gvisor/blob/3a9ba17351571e343e16caec2e44215d8adc400f/pkg/tcpip/network/ipv6/icmp.go#L694
https://github.com/google/gvisor/blob/3a9ba17351571e343e16caec2e44215d8adc400f/pkg/tcpip/network/ipv4/icmp.go#L449

[Lanius-collaris]

I wonder if this forceful default handling of ICMP is a bug on gvisor upstream despite having set a ICMP handler? It was working as I'd expect it to in previous (believe a year or so ago) gvisor version.

I have a theory that these lines were written for inbound ICMP echo requests (packet sockets can receive them).

@kevinGC
Could you answer ignoramous' question?

[kevinGC]

Sorry, what's the specific question? I'm missing some context and not sure what 'fake' means in the title here.

I don't remember any recent changes to ICMP handling. If you've set a custom handler for ICMP packets and the stack is still doing its own ICMP handling, I think that's because ICMP handling is tightly coupled to the ipv4 and ipv6 packages. Nobody has tried to split them AFAIK, so there's no easy way to turn off just ICMP.

Packet sockets should get ICMP messages regardless of the code in the ipv4 and ipv6 packages; it happens earlier in packaet processing (around nic.go:DeliverLinkPacket).

BTW @nybidari is the gVisor networking lead these days.

[ignoramous]

Sorry, what's the specific question? I'm missing some context and not sure what 'fake' means in the title here.

Netstack replies to ICMP messages on its own and does not wait on the Handler set via stack.SetTransportProtocolHandler (unlike for TCP and UDP, for which Netstack processes packets only if the Handler does not).

I should note that Netstack we use is setup with Spoofing and is Promiscuous (ref), in addition to being the default gateway (0.0.0.0 and :: routes); HandleLocal & NICForwarding opts are also set to false (ref).

Packet sockets should get ICMP messages regardless of the code in the ipv4 and ipv6 packages;

This happens even today. The Handler does get those packets, but Netstack replies to those packets before the Handler has had the chance.

Like @Lanius-collaris points out, ICMP is handled internally (here) before being delivered to the Handler (here?)? For us, it is enough if ICMP requests (echo, specifically) (which aren't tied to an existing UDP/TCP sesssion), like those usually coming from ping or equivalent progs, are sent to the Handler before Netstack looks at them.

[Lanius-collaris]

I have a theory that these lines were written for inbound ICMP echo requests (packet sockets can receive them).

Packet sockets should get ICMP messages regardless of the code in the ipv4 and ipv6 packages; it happens earlier in packaet processing (around nic.go:DeliverLinkPacket).

"packet sockets" here mean sockets created with socket(AF_PACKET, int socket_type, int protocol) on linux, not tcpip.Endpoint provided by gvisor.dev/gvisor/pkg/tcpip/transport/packet module.

If you've set a custom handler for ICMP packets and the stack is still doing its own ICMP handling, I think that's because ICMP handling is tightly coupled to the ipv4 and ipv6 packages. Nobody has tried to split them AFAIK, so there's no easy way to turn off just ICMP.

I have tried and succeeded. 😆 Stack.IPTables() is powerful.
Lanius-collaris/gvisor-playground@5403bf8

[Lanius-collaris]

I tried bumping gvisor for my gvisor-playground, found that new gvisor requires ForceReplaceTable() to work. All rules in iptables become useless if shouldSkipOrPopulateTables() returns true……

[ignoramous]

Are you saying, for the current changes to work, firestack would have to be pinned to its current gvisor@go version v0.0.0-20250816201027-ba3b9ca85f20?

firestack/go.mod

Line 27 in 96bf8e8

gvisor.dev/gvisorv0.0.0-20250816201027-ba3b9ca85f20

(I never really got the hang of the gvisor/netstack internals at all...)

[Lanius-collaris]

I tried bumping gvisor for my gvisor-playground, found that new gvisor requires ForceReplaceTable() to work. All rules in iptables become useless if shouldSkipOrPopulateTables() returns true……

Are you saying, for the current changes to work, firestack would have to be pinned to its current gvisor@go version v0.0.0-20250816201027-ba3b9ca85f20?

I mean that shouldSkipOrPopulateTables() return true ( https://github.com/google/gvisor/blob/fd370915f22e/pkg/tcpip/stack/iptables.go#L315 in some cases gvisor thinks modified iptables are not modified ) so 2cc55a2 doesn't work, but IPTables.ForceReplaceTable() solves this problem, IPTables.ReplaceTable() in gvisor v0.0.0-20231023213702-2691a8f9b1cf ( used by my gvisor-playground ) always works.

The comment about gvisor v0.0.0-20250816201027-ba3b9ca85f20 means that firestack's handler is not called after stack.SetTransportProtocolHandler(icmp.ProtocolNumber6, …)

[ignoramous]

Can we create a bool icmp6IPtablesHack (or if you can think of a better name), which when set to true uses these changes, but otherwise falls back to running existing code? If that's too much work, I can take it up, too.

[Lanius-collaris]

Can we create a bool icmp6IPtablesHack (or if you can think of a better name), which when set to true uses these changes, but otherwise falls back to running existing code? If that's too much work, I can take it up, too.

Even if gvisor changes the behavior ( in pkg/tcpip/network/ipv6/icmp.go and pkg/tcpip/network/ipv6/ipv6.go ), you still need to recompile firestack, and it's very easy to remove ICMPHackTarget, so I think a new option is not needed.

I will leave the maintenance to you.

[ignoramous]

Gotcha.

Even if gvisor changes the behavior ( in pkg/tcpip/network/ipv6/icmp.go and pkg/tcpip/network/ipv6/ipv6.go ), you still need to recompile firestack

Recompiling firestack is no big deal? Each commit batch has a compiled artifact pushed to GitHub Packages and Maven Central... and we've managed to upgrade the gvisor@go module from upstream without much fanfare (at least until now).

[Lanius-collaris]

Recompiling firestack is no big deal? Each commit batch has a compiled artifact pushed to GitHub Packages and Maven Central... and we've managed to upgrade the gvisor@go module from upstream without much fanfare (at least until now).

I means rethink app can't call settings.XXXX() to use stack.SetTransportProtocolHandler(icmp.ProtocolNumber6, …) for ICMPv6 without recompiling, do you want to keep two implementations?