From ef1db9a6763ee8d9d9d5394f50eb7d51b68b7cf2 Mon Sep 17 00:00:00 2001 From: Owen Date: Wed, 5 Aug 2026 16:16:18 -0400 Subject: [PATCH] properly handle icmp over tunnel for automatic testing --- olm/exitnode.go | 2 +- peers/manager.go | 9 ++- peers/monitor/exitnode.go | 133 +++++++++++++++++++++++++++++++------- peers/monitor/monitor.go | 44 ++++++++++++- 4 files changed, 157 insertions(+), 31 deletions(-) diff --git a/olm/exitnode.go b/olm/exitnode.go index d0e3aeb..86f84fb 100644 --- a/olm/exitnode.go +++ b/olm/exitnode.go @@ -119,7 +119,7 @@ persistent_keepalive_interval=%d`, util.FixKey(cfg.PublicKey), allowedIP, resolv } if pm := o.getPeerManager(); pm != nil { - pm.SetExitNode(strings.Split(cfg.ServerIP, "/")[0]) + pm.SetExitNode(strings.Split(cfg.ServerIP, "/")[0], strings.Split(cfg.TunnelIP, "/")[0]) } logger.Info("Connected to exit node at %s", resolvedEndpoint) diff --git a/peers/manager.go b/peers/manager.go index 6a84a5e..573767a 100644 --- a/peers/manager.go +++ b/peers/manager.go @@ -127,12 +127,15 @@ func (pm *PeerManager) GetPeerMonitor() *monitor.PeerMonitor { return pm.peerMonitor } -// SetExitNode starts (or updates) ICMP connectivity monitoring of the given exit node -func (pm *PeerManager) SetExitNode(serverIP string) { +// SetExitNode starts (or updates) ICMP connectivity monitoring of the given exit node. +// tunnelIP is the secondary address assigned to us for this exit node, which the ping +// probe must be sourced from since the exit node's WireGuard peer entry only accepts +// traffic from that address. +func (pm *PeerManager) SetExitNode(serverIP, tunnelIP string) { pm.mu.RLock() defer pm.mu.RUnlock() if pm.peerMonitor != nil { - pm.peerMonitor.SetExitNode(serverIP) + pm.peerMonitor.SetExitNode(serverIP, tunnelIP) } } diff --git a/peers/monitor/exitnode.go b/peers/monitor/exitnode.go index 20ae8c3..dd0340c 100644 --- a/peers/monitor/exitnode.go +++ b/peers/monitor/exitnode.go @@ -3,8 +3,6 @@ package monitor import ( "bytes" "context" - "crypto/rand" - "encoding/binary" "fmt" "net/netip" "time" @@ -14,6 +12,7 @@ import ( xipv4 "golang.org/x/net/ipv4" "gvisor.dev/gvisor/pkg/tcpip" gipv4 "gvisor.dev/gvisor/pkg/tcpip/network/ipv4" + gstack "gvisor.dev/gvisor/pkg/tcpip/stack" gicmp "gvisor.dev/gvisor/pkg/tcpip/transport/icmp" "gvisor.dev/gvisor/pkg/waiter" ) @@ -24,25 +23,39 @@ const ( exitNodePingMaxAttempts = 3 ) -// SetExitNode starts (or, if the server address changed, restarts) ICMP -// connectivity monitoring of the exit node at serverIP. serverIP must be a -// bare IP address (no CIDR suffix). -func (pm *PeerMonitor) SetExitNode(serverIP string) { +// SetExitNode starts (or, if the exit node changed, restarts) ICMP +// connectivity monitoring of the exit node at serverIP. tunnelIP is the +// secondary address assigned to us for this exit node (ExitNodeConfig.TunnelIP) - +// the exit node's WireGuard peer entry only accepts traffic sourced from that +// address, so probes must be sourced from it rather than the site tunnel IP. +// Both serverIP and tunnelIP must be bare IP addresses (no CIDR suffix). +func (pm *PeerMonitor) SetExitNode(serverIP, tunnelIP string) { pm.exitNodeMu.Lock() - if pm.exitNodeCancel != nil && pm.exitNodeServerIP == serverIP { + if pm.exitNodeCancel != nil && pm.exitNodeServerIP == serverIP && pm.exitNodeTunnelIP == tunnelIP { pm.exitNodeMu.Unlock() return } + prevTunnelIP := pm.exitNodeTunnelIP if pm.exitNodeCancel != nil { pm.exitNodeCancel() } pm.exitNodeServerIP = serverIP + pm.exitNodeTunnelIP = tunnelIP ctx, cancel := context.WithCancel(context.Background()) pm.exitNodeCancel = cancel pm.exitNodeMu.Unlock() - logger.Info("Started exit node connectivity monitor for %s", serverIP) - go pm.runExitNodeMonitor(ctx, serverIP) + if prevTunnelIP != "" && prevTunnelIP != tunnelIP { + pm.removeExitNodeAddress(prevTunnelIP) + } + if tunnelIP != prevTunnelIP { + if err := pm.addExitNodeAddress(tunnelIP); err != nil { + logger.Error("Failed to register exit node tunnel address %s: %v", tunnelIP, err) + } + } + + logger.Info("Started exit node connectivity monitor for %s (via %s)", serverIP, tunnelIP) + go pm.runExitNodeMonitor(ctx, serverIP, tunnelIP) } // ClearExitNode stops ICMP monitoring of the exit node and clears its status @@ -53,9 +66,15 @@ func (pm *PeerMonitor) ClearExitNode() { pm.exitNodeCancel() pm.exitNodeCancel = nil } + tunnelIP := pm.exitNodeTunnelIP pm.exitNodeServerIP = "" + pm.exitNodeTunnelIP = "" pm.exitNodeMu.Unlock() + if tunnelIP != "" { + pm.removeExitNodeAddress(tunnelIP) + } + if pm.apiServer != nil { pm.apiServer.ClearExitNodeStatus() } @@ -63,19 +82,69 @@ func (pm *PeerMonitor) ClearExitNode() { logger.Info("Stopped exit node connectivity monitor") } +// addExitNodeAddress registers tunnelIP as a protocol address on the peer +// monitor's netstack NIC and adds a MiddleDevice rule so ICMP replies destined +// to it are intercepted and redirected into the netstack instead of being +// delivered to the host TUN device. +func (pm *PeerMonitor) addExitNodeAddress(tunnelIP string) error { + pm.mutex.Lock() + st := pm.stack + pm.mutex.Unlock() + + if st == nil { + return fmt.Errorf("netstack not initialized") + } + + addr, err := netip.ParseAddr(tunnelIP) + if err != nil { + return fmt.Errorf("invalid tunnel IP: %w", err) + } + + protoAddr := tcpip.ProtocolAddress{ + Protocol: gipv4.ProtocolNumber, + AddressWithPrefix: tcpip.AddrFrom4(addr.As4()).WithPrefix(), + } + if tcpipErr := st.AddProtocolAddress(1, protoAddr, gstack.AddressProperties{}); tcpipErr != nil { + return fmt.Errorf("failed to add protocol address: %s", tcpipErr) + } + + pm.middleDev.AddRule(addr, pm.handlePacket) + return nil +} + +// removeExitNodeAddress undoes addExitNodeAddress. +func (pm *PeerMonitor) removeExitNodeAddress(tunnelIP string) { + addr, err := netip.ParseAddr(tunnelIP) + if err != nil { + return + } + + pm.middleDev.RemoveRule(addr) + + pm.mutex.Lock() + st := pm.stack + pm.mutex.Unlock() + + if st != nil { + st.RemoveAddress(1, tcpip.AddrFrom4(addr.As4())) + } +} + // runExitNodeMonitor periodically pings the exit node and reports its status // to the API server until ctx is cancelled. -func (pm *PeerMonitor) runExitNodeMonitor(ctx context.Context, serverIP string) { +func (pm *PeerMonitor) runExitNodeMonitor(ctx context.Context, serverIP, tunnelIP string) { check := func() { var ( connected bool rtt time.Duration ) for attempt := 0; attempt < exitNodePingMaxAttempts; attempt++ { - if d, err := pm.pingExitNode(serverIP, exitNodePingTimeout); err == nil { + if d, err := pm.pingExitNode(serverIP, tunnelIP, exitNodePingTimeout); err == nil { connected = true rtt = d break + } else { + logger.Debug("Exit node ping attempt %d/%d to %s failed: %v", attempt+1, exitNodePingMaxAttempts, serverIP, err) } select { case <-ctx.Done(): @@ -102,15 +171,14 @@ func (pm *PeerMonitor) runExitNodeMonitor(ctx context.Context, serverIP string) } } -// pingExitNode sends a single ICMP echo request to dst and waits up to timeout -// for the matching reply. The request is built and read directly on the peer -// monitor's gvisor netstack, so it's injected into (and intercepted from) the -// WireGuard device via MiddleDevice - it never touches the host's real -// network stack, matching how the UDP peer tests above work. -func (pm *PeerMonitor) pingExitNode(dst string, timeout time.Duration) (time.Duration, error) { +// pingExitNode sends a single ICMP echo request from localTunnelIP to dst and +// waits up to timeout for the matching reply. The request is built and read +// directly on the peer monitor's gvisor netstack, so it's injected into (and +// intercepted from) the WireGuard device via MiddleDevice - it never touches +// the host's real network stack, matching how the UDP peer tests above work. +func (pm *PeerMonitor) pingExitNode(dst, localTunnelIP string, timeout time.Duration) (time.Duration, error) { pm.mutex.Lock() st := pm.stack - localIPStr := pm.localIP pm.mutex.Unlock() if st == nil { @@ -121,7 +189,7 @@ func (pm *PeerMonitor) pingExitNode(dst string, timeout time.Duration) (time.Dur if err != nil { return 0, fmt.Errorf("invalid destination address: %w", err) } - localAddr, err := netip.ParseAddr(localIPStr) + localAddr, err := netip.ParseAddr(localTunnelIP) if err != nil { return 0, fmt.Errorf("invalid local address: %w", err) } @@ -136,16 +204,31 @@ func (pm *PeerMonitor) pingExitNode(dst string, timeout time.Duration) (time.Dur if tcpipErr := ep.Bind(tcpip.FullAddress{NIC: 1, Addr: tcpip.AddrFromSlice(localAddr.AsSlice())}); tcpipErr != nil { return 0, fmt.Errorf("failed to bind ICMP endpoint: %s", tcpipErr) } + + // gvisor's ICMP endpoint overwrites whatever Identifier we put in the outgoing + // echo with its own bound "port" (assigned above by Bind), and demuxes incoming + // Echo Replies by that same value - so we must use it, not one we generate + // ourselves, both for the outgoing message and to register with handlePacket's + // filter below. + laddr, tcpipErr := ep.GetLocalAddress() + if tcpipErr != nil { + return 0, fmt.Errorf("failed to get local ICMP endpoint address: %s", tcpipErr) + } + echoID := int(laddr.Port) + + pm.portsLock.Lock() + pm.activeICMPIdents[laddr.Port] = true + pm.portsLock.Unlock() + defer func() { + pm.portsLock.Lock() + delete(pm.activeICMPIdents, laddr.Port) + pm.portsLock.Unlock() + }() + if tcpipErr := ep.Connect(tcpip.FullAddress{NIC: 1, Addr: tcpip.AddrFromSlice(dstAddr.AsSlice())}); tcpipErr != nil { return 0, fmt.Errorf("failed to connect ICMP endpoint: %s", tcpipErr) } - var idBuf [2]byte - if _, err := rand.Read(idBuf[:]); err != nil { - return 0, fmt.Errorf("failed to generate echo ID: %w", err) - } - echoID := int(binary.BigEndian.Uint16(idBuf[:])) - requestPing := icmp.Echo{ ID: echoID, Seq: 1, diff --git a/peers/monitor/monitor.go b/peers/monitor/monitor.go index edabd3c..2796b41 100644 --- a/peers/monitor/monitor.go +++ b/peers/monitor/monitor.go @@ -3,6 +3,7 @@ package monitor import ( "context" "crypto/rand" + "encoding/binary" "encoding/hex" "fmt" "net" @@ -113,7 +114,13 @@ type PeerMonitor struct { // injected directly into the WireGuard device via MiddleDevice. exitNodeMu sync.Mutex exitNodeServerIP string + exitNodeTunnelIP string exitNodeCancel context.CancelFunc + + // activeICMPIdents tracks the ICMP identifiers of our own in-flight exit-node + // ping probes (guarded by portsLock, alongside activePorts), so handlePacket + // only intercepts Echo Replies that are actually ours. + activeICMPIdents map[uint16]bool } // NewPeerMonitor creates a new peer monitor with the given callback @@ -134,6 +141,7 @@ func NewPeerMonitor(wsClient *websocket.Client, middleDev *middleDevice.MiddleDe localIP: localIP, publicDNS: publicDNS, activePorts: make(map[uint16]bool), + activeICMPIdents: make(map[uint16]bool), nsCtx: ctx, nsCancel: cancel, sharedBind: sharedBind, @@ -1346,6 +1354,24 @@ func (pm *PeerMonitor) initNetstack() error { return nil } +// icmpv4EchoReplyIdent returns the ICMP identifier of packet if it is an IPv4 +// ICMP Echo Reply (type 0), so it can be matched against our own in-flight +// exit-node ping probes before being pulled off the host's real traffic path. +func icmpv4EchoReplyIdent(packet []byte) (uint16, bool) { + if len(packet) < 20 || packet[0]>>4 != 4 { + return 0, false + } + ihl := int(packet[0]&0x0f) * 4 + if ihl < 20 || len(packet) < ihl+8 { + return 0, false + } + const icmpEchoReply = 0 + if packet[ihl] != icmpEchoReply { + return 0, false + } + return binary.BigEndian.Uint16(packet[ihl+4 : ihl+6]), true +} + // handlePacket is called by MiddleDevice when a packet arrives for our IP func (pm *PeerMonitor) handlePacket(packet []byte) bool { proto, ok := util.GetProtocol(packet) @@ -1354,8 +1380,22 @@ func (pm *PeerMonitor) handlePacket(packet []byte) bool { } switch proto { - case 1, 58: // ICMPv4, ICMPv6 - always ours, used only by the exit node ping probe - // no per-port filtering needed + case 1: // ICMPv4 - only intercept Echo Replies matching one of our own active + // exit-node ping probes, identified by the ICMP identifier field. Anything + // else (including real ICMP traffic to/from the host, e.g. `ping`) must be + // left alone so it reaches the host TUN normally. + ident, ok := icmpv4EchoReplyIdent(packet) + if !ok { + return false + } + + pm.portsLock.RLock() + active := pm.activeICMPIdents[ident] + pm.portsLock.RUnlock() + + if !active { + return false + } case 17: // UDP // Check destination port port, ok := util.GetDestPort(packet)