Skip to content

netstack: allow defaultHandler respond ICMPv4Echo in promiscuous mode #11609

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Amaindex
Copy link

@Amaindex Amaindex commented Apr 2, 2025

Motivation

The gVisor network stack is extensively employed in user-space tunneling software, often operating in promiscuous mode. In this configuration, the stack directly responds to all ICMPv4 Echo Request packets, irrespective of whether a transport defaultHandler has already processed them. This behavior is often unintended in certain scenarios, as evidenced by issues such as:

In these scenarios, users may prefer to utilize SetTransportProtocolHandler to configure a custom defaultHandler for tailored processing of ICMPv4 Echo packets.

In issue #8657, @kevinGC proposed a potential solution:

// If a customer ICMPv4 protocol handler has been set, use that in favor of default handling.
if p == header.ICMPv4ProtocolNumber {
    if _, ok := e.protocol.stack.transportProtocols[header.ICMPv4ProtocolNumber]; !ok {
        // handle the "normal" way
    }
}

However, this approach appears somewhat aggressive, as it could impair applications that depend on the existing gVisor stack behavior with ICMPv4 protocol handlers, such as runsc itself. These programs would require code adjustments to accommodate this change, as shown below:

// gvisor/runsc/boot/loader.go
func newEmptySandboxNetworkStack(clock tcpip.Clock, allowPacketEndpointWrite bool) (*netstack.Stack, error) {
    netProtos := []stack.NetworkProtocolFactory{ipv4.NewProtocol, ipv6.NewProtocol, arp.NewProtocol}
    transProtos := []stack.TransportProtocolFactory{
        tcp.NewProtocol,
        udp.NewProtocol,
        // icmp.NewProtocol4, runsc would need to remove this to allow stack ICMPv4Echo replies.
        icmp.NewProtocol6,
    }
    // ...
}

This patch introduces an alternative by enabling the stack to refrain from directly responding to ICMPv4 Echo packets delivered locally due to promiscuous mode, thereby allowing the defaultHandler to handle them independently.

This proposal is presented as an initial step for discussion, and insights from experts on potential refinements or superior alternatives are warmly welcomed.

Testing and adjustments for ICMPv6 will be addressed once the approach is finalized.

Patch Details

In ipv4.go:handleValidatedPacket, the packet is evaluated based on AcquireAssignedAddress to determine local delivery or forwarding:

func (e *endpoint) handleValidatedPacket(h header.IPv4, pkt *stack.PacketBuffer, inNICName string) {
    // ...
    if addressEndpoint := e.AcquireAssignedAddress(dstAddr, e.nic.Promiscuous(), stack.CanBePrimaryEndpoint, true /* readOnly */); addressEndpoint != nil {
        subnet := addressEndpoint.AddressWithPrefix().Subnet()
        pkt.NetworkPacketInfo.LocalAddressBroadcast = subnet.IsBroadcast(dstAddr) || dstAddr == header.IPv4Broadcast
        e.deliverPacketLocally(h, pkt, inNICName)
    } else if e.Forwarding() {
        e.handleForwardingError(e.forwardUnicastPacket(pkt))
    } else {
        stats.ip.InvalidDestinationAddressesReceived.Increment()
    }
}

In promiscuous mode, packets destined for unknown addresses are assigned a temporary address and delivered locally:

/*
handleValidatedPacket
 \----AcquireAssignedAddress
       \----AcquireAssignedAddressOrMatching
             \----addAndAcquireAddressLocked
*/
func (a *AddressableEndpointState) AcquireAssignedAddressOrMatching(localAddr tcpip.Address, f func(AddressEndpoint) bool, allowTemp bool, tempPEB PrimaryEndpointBehavior, readOnly bool) AddressEndpoint {
    // ...
    if !allowTemp { // e.nic.Promiscuous()
        return nil
    }
    // ...
    ep, err := a.addAndAcquireAddressLocked(addr, AddressProperties{PEB: tempPEB}, Temporary)
    // ...
}

By leveraging the Temporary field in AddressProperties, we can identify packets delivered locally due to promiscuous mode. A new field, LocalAddressTemporary, is added to NetworkPacketInfo to record this status:

// pkg/tcpip/stack/addressable_endpoint_state.go
func (a *AddressableEndpointState) AcquireAssignedAddressOrMatching(localAddr tcpip.Address, f func(AddressEndpoint) bool, allowTemp bool, tempPEB PrimaryEndpointBehavior, readOnly bool) AddressEndpoint {
    // ...
    if !allowTemp { // e.nic.Promiscuous()
        return nil
    }
    // ...
    ep, err := a.addAndAcquireAddressLocked(addr, AddressProperties{PEB: tempPEB, Temporary: true}, Temporary) // set AddressProperties.Temporary
    // ...
}

// pkg/tcpip/network/ipv4/ipv4.go
func (e *endpoint) handleValidatedPacket(h header.IPv4, pkt *stack.PacketBuffer, inNICName string) {
    // ...
    if addressEndpoint := e.AcquireAssignedAddress(dstAddr, e.nic.Promiscuous(), stack.CanBePrimaryEndpoint, true /* readOnly */); addressEndpoint != nil {
        pkt.NetworkPacketInfo.LocalAddressTemporary = addressEndpoint.Temporary() // packets delivered locally due to promiscuous mode 
        subnet := addressEndpoint.AddressWithPrefix().Subnet()
        pkt.NetworkPacketInfo.LocalAddressBroadcast = subnet.IsBroadcast(dstAddr) || dstAddr == header.IPv4Broadcast
        e.deliverPacketLocally(h, pkt, inNICName)
    } else if e.Forwarding() {
        e.handleForwardingError(e.forwardUnicastPacket(pkt))
    } else {
        stats.ip.InvalidDestinationAddressesReceived.Increment()
    }
}

Finally, in icmp.go:handleICMP, direct replies to such packets are skipped:

func (e *endpoint) handleICMP(pkt *stack.PacketBuffer) {
    // ...
    switch h.Type() {
    case header.ICMPv4Echo:
        // ...
        localAddressTemporary := pkt.NetworkPacketInfo.LocalAddressTemporary
        localAddressBroadcast := pkt.NetworkPacketInfo.LocalAddressBroadcast

        // It's possible that a raw socket or custom defaultHandler expects to
	// receive this packet.
        e.dispatcher.DeliverTransportPacket(header.ICMPv4ProtocolNumber, pkt)
        pkt = nil

        // Skip direct ICMP echo reply if the packet was received with a temporary
	// address, allowing custom handlers to take over.
        if localAddressTemporary {
            return
        }
        // ...
    }
}

Quick Testing

To validate this patch, a simple test program and procedure are provided below:

package main

import (
    "fmt"
    "net"
    "gvisor.dev/gvisor/pkg/tcpip"
    "gvisor.dev/gvisor/pkg/tcpip/header"
    "gvisor.dev/gvisor/pkg/tcpip/link/fdbased"
    "gvisor.dev/gvisor/pkg/tcpip/link/tun"
    "gvisor.dev/gvisor/pkg/tcpip/network/ipv4"
    "gvisor.dev/gvisor/pkg/tcpip/network/ipv6"
    "gvisor.dev/gvisor/pkg/tcpip/stack"
    "gvisor.dev/gvisor/pkg/tcpip/transport/icmp"
    "gvisor.dev/gvisor/pkg/tcpip/transport/tcp"
    "gvisor.dev/gvisor/pkg/tcpip/transport/udp"
)

func main() {
    fd, _ := tun.Open("tun0")
    ep, _ := fdbased.New(&fdbased.Options{
        FDs:                   []int{fd},
        MTU:                   1500,
        EthernetHeader:        false,
        PacketDispatchMode:    fdbased.Readv,
        MaxSyscallHeaderBytes: 0x00,
    })
    s := stack.New(stack.Options{
        NetworkProtocols: []stack.NetworkProtocolFactory{
            ipv4.NewProtocol,
            ipv6.NewProtocol,
        },
        TransportProtocols: []stack.TransportProtocolFactory{
            tcp.NewProtocol,
            udp.NewProtocol,
            icmp.NewProtocol4,
            icmp.NewProtocol6,
        },
    })

    nicID := s.NextNICID()
    s.CreateNICWithOptions(nicID, ep,
        stack.NICOptions{
            Disabled: false,
            QDisc:    nil,
        },
    )
    s.SetPromiscuousMode(nicID, true)
    s.SetSpoofing(nicID, true)
    s.SetRouteTable([]tcpip.Route{
        {Destination: header.IPv4EmptySubnet, NIC: nicID},
        {Destination: header.IPv6EmptySubnet, NIC: nicID},
    })
    s.SetTransportProtocolHandler(icmp.ProtocolNumber4, func(id stack.TransportEndpointID, pkt *stack.PacketBuffer) bool {
        h := header.ICMPv4(pkt.TransportHeader().Slice())
        fmt.Printf("icmpv4: %s->%s, hType: %v, ", id.RemoteAddress, id.LocalAddress, h.Type())
        if !pkt.NetworkPacketInfo.LocalAddressTemporary {
            fmt.Println("packet to permanent address, processed by stack")
            return true
        }
        fmt.Println("packet to temporary address, need to process by user")
        return true
    })
    protocolAddr := tcpip.ProtocolAddress{
        Protocol: ipv4.ProtocolNumber,
        AddressWithPrefix: tcpip.AddressWithPrefix{
            Address:   tcpip.AddrFromSlice(net.IPv4(11, 0, 0, 1).To4()),
            PrefixLen: 8,
        },
    }
    s.AddProtocolAddress(nicID, protocolAddr, stack.AddressProperties{PEB: stack.CanBePrimaryEndpoint, Temporary: false})

    fmt.Println("stack started ...")
    select {}
}

Test procedure:

$ ip tuntap add mode tun dev tun0
ip link set dev tun0 up 

$ go run main.go  
stack started ...

icmpv4: 10.161.22.19->11.0.0.1, hType: 8, packet to permanent address, processed by stack
icmpv4: 10.161.22.19->11.0.0.1, hType: 8, packet to permanent address, processed by stack
icmpv4: 10.161.22.19->11.0.0.1, hType: 8, packet to permanent address, processed by stack

icmpv4: 10.161.22.19->11.0.0.2, hType: 8, packet to temporary address, need to process by user
icmpv4: 10.161.22.19->11.0.0.2, hType: 8, packet to temporary address, need to process by user
icmpv4: 10.161.22.19->11.0.0.2, hType: 8, packet to temporary address, need to process by user
$ ping 11.0.0.1
PING 11.0.0.1 (11.0.0.1) 56(84) bytes of data.
64 bytes from 11.0.0.1: icmp_seq=1 ttl=64 time=0.441 ms
64 bytes from 11.0.0.1: icmp_seq=2 ttl=64 time=0.425 ms
64 bytes from 11.0.0.1: icmp_seq=3 ttl=64 time=0.437 ms
^C
--- 11.0.0.1 ping statistics ---
3 packets transmitted, 3 received, 0% packet loss, time 2012ms
rtt min/avg/max/mdev = 0.425/0.434/0.441/0.006 ms

$ ping 11.0.0.2
PING 11.0.0.2 (11.0.0.2) 56(84) bytes of data.
^C
--- 11.0.0.2 ping statistics ---
3 packets transmitted, 0 received, 100% packet loss, time 2052ms

In promiscuous mode, ICMPv4Echo packets are replied to directly by the
network stack, even if custom transport defaultHandler processes them.
This change adds a LocalAddressTemporary field to NetworkPacketInfo to
identify packets received with temporary addresses due to promiscuous
mode, and skips the direct ICMP reply for these. This allows custom
handlers to independently process such packets.

- Added LocalAddressTemporary field to NetworkPacketInfo.
- Set Temporary property when adding temporary address.
- Set LocalAddressTemporary in addressEndpoint check.
- Skip direct ICMPv4Echo reply for packets with temporary addresses.

For google#8657
Copy link

google-cla bot commented Apr 2, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@nybidari nybidari self-requested a review April 8, 2025 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant