Browse Source

xcode: allow ICMP ping relay on macOS + iOS platforms (#12048)

Fixes tailscale/tailscale#10393
Fixes tailscale/corp#15412
Fixes tailscale/corp#19808

On Apple platforms, exit nodes and subnet routers have been unable to relay pings from Tailscale devices to non-Tailscale devices due to sandbox restrictions imposed on our network extensions by Apple. The sandbox prevented the code in netstack.go from spawning the `ping` process which we were using.

Replace that exec call with logic to send an ICMP echo request directly, which appears to work in userspace, and not trigger a sandbox violation in the syslog.

Signed-off-by: Andrea Gottardo <[email protected]>
Andrea Gottardo 1 year ago
parent
commit
e5f67f90a2

+ 2 - 1
cmd/tailscaled/depaware.txt

@@ -144,6 +144,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
    L    github.com/pierrec/lz4/v4/internal/xxh32                     from github.com/pierrec/lz4/v4/internal/lz4stream
   LD    github.com/pkg/sftp                                          from tailscale.com/ssh/tailssh
   LD    github.com/pkg/sftp/internal/encoding/ssh/filexfer           from github.com/pkg/sftp
+   D    github.com/prometheus-community/pro-bing                     from tailscale.com/wgengine/netstack
    L 💣 github.com/safchain/ethtool                                  from tailscale.com/net/netkernelconf+
    W 💣 github.com/tailscale/certstore                               from tailscale.com/control/controlclient
    W 💣 github.com/tailscale/go-winio                                from tailscale.com/safesocket
@@ -439,7 +440,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         golang.org/x/net/http2                                       from golang.org/x/net/http2/h2c+
         golang.org/x/net/http2/h2c                                   from tailscale.com/ipn/ipnlocal
         golang.org/x/net/http2/hpack                                 from golang.org/x/net/http2+
-        golang.org/x/net/icmp                                        from tailscale.com/net/ping
+        golang.org/x/net/icmp                                        from tailscale.com/net/ping+
         golang.org/x/net/idna                                        from golang.org/x/net/http/httpguts+
         golang.org/x/net/ipv4                                        from github.com/miekg/dns+
         golang.org/x/net/ipv6                                        from github.com/miekg/dns+

+ 2 - 1
go.mod

@@ -37,7 +37,7 @@ require (
 	github.com/google/go-cmp v0.6.0
 	github.com/google/go-containerregistry v0.18.0
 	github.com/google/nftables v0.2.1-0.20240414091927-5e242ec57806
-	github.com/google/uuid v1.5.0
+	github.com/google/uuid v1.6.0
 	github.com/goreleaser/nfpm/v2 v2.33.1
 	github.com/hdevalence/ed25519consensus v0.2.0
 	github.com/iancoleman/strcase v0.3.0
@@ -60,6 +60,7 @@ require (
 	github.com/peterbourgon/ff/v3 v3.4.0
 	github.com/pkg/errors v0.9.1
 	github.com/pkg/sftp v1.13.6
+	github.com/prometheus-community/pro-bing v0.4.0
 	github.com/prometheus/client_golang v1.18.0
 	github.com/prometheus/common v0.46.0
 	github.com/safchain/ethtool v0.3.0

+ 4 - 2
go.sum

@@ -468,8 +468,8 @@ github.com/google/rpmpack v0.5.0 h1:L16KZ3QvkFGpYhmp23iQip+mx1X39foEsqszjMNBm8A=
 github.com/google/rpmpack v0.5.0/go.mod h1:uqVAUVQLq8UY2hCDfmJ/+rtO3aw7qyhc90rCVEabEfI=
 github.com/google/uuid v1.1.1/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
 github.com/google/uuid v1.1.2/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
-github.com/google/uuid v1.5.0 h1:1p67kYwdtXjb0gL0BPiP1Av9wiZPo5A8z2cWkTZ+eyU=
-github.com/google/uuid v1.5.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
+github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0=
+github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
 github.com/googleapis/gax-go/v2 v2.0.4/go.mod h1:0Wqv26UfaUD9n4G6kQubkQ+KchISgw+vpHVxEJEs9eg=
 github.com/googleapis/gax-go/v2 v2.0.5/go.mod h1:DWXyrwAJ9X0FpwwEdw+IPEYBICEFu5mhpdKc/us6bOk=
 github.com/googleapis/google-cloud-go-testing v0.0.0-20200911160855-bcd43fbb19e8/go.mod h1:dvDLG8qkwmyD9a/MJJN3XJcT3xFxOKAvTZGvuZmac9g=
@@ -731,6 +731,8 @@ github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZb
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/polyfloyd/go-errorlint v1.4.1 h1:r8ru5FhXSn34YU1GJDOuoJv2LdsQkPmK325EOpPMJlM=
 github.com/polyfloyd/go-errorlint v1.4.1/go.mod h1:k6fU/+fQe38ednoZS51T7gSIGQW1y94d6TkSr35OzH8=
+github.com/prometheus-community/pro-bing v0.4.0 h1:YMbv+i08gQz97OZZBwLyvmmQEEzyfyrrjEaAchdy3R4=
+github.com/prometheus-community/pro-bing v0.4.0/go.mod h1:b7wRYZtCcPmt4Sz319BykUU241rWLe1VFXyiyWK/dH4=
 github.com/prometheus/client_golang v0.9.1/go.mod h1:7SWBe2y4D6OKWSNQJUaRYU/AaXPKyh/dDVn+NZz0KFw=
 github.com/prometheus/client_golang v1.0.0/go.mod h1:db9x61etRT2tGnBNRi70OPL5FsnadC4Ky3P0J6CfImo=
 github.com/prometheus/client_golang v1.7.1/go.mod h1:PY5Wy2awLA44sXw4AOSfFBetzPP4j5+D6mVACh+pe2M=

+ 3 - 47
wgengine/netstack/netstack.go

@@ -15,8 +15,6 @@ import (
 	"math"
 	"net"
 	"net/netip"
-	"os"
-	"os/exec"
 	"runtime"
 	"strconv"
 	"sync"
@@ -55,7 +53,6 @@ import (
 	"tailscale.com/types/nettype"
 	"tailscale.com/util/clientmetric"
 	"tailscale.com/version"
-	"tailscale.com/version/distro"
 	"tailscale.com/wgengine"
 	"tailscale.com/wgengine/filter"
 	"tailscale.com/wgengine/magicsock"
@@ -916,14 +913,8 @@ func (ns *Impl) shouldProcessInbound(p *packet.Parsed, t *tstun.Wrapper) bool {
 	return false
 }
 
-// setAmbientCapsRaw is non-nil on Linux for Synology, to run ping with
-// CAP_NET_RAW from tailscaled's binary.
-var setAmbientCapsRaw func(*exec.Cmd)
-
 var userPingSem = syncs.NewSemaphore(20) // 20 child ping processes at once
 
-var isSynology = runtime.GOOS == "linux" && distro.Get() == distro.Synology
-
 type userPingDirection int
 
 const (
@@ -944,6 +935,8 @@ const (
 // it bounds the number of pings going on at once. The idea is that
 // people only use ping occasionally to see if their internet's working
 // so this doesn't need to be great.
+// On Apple platforms, this function doesn't run the ping command. Instead,
+// it sends a non-privileged ping.
 //
 // The 'direction' parameter is used to determine where the response "pong"
 // packet should be written, if the ping succeeds. See the documentation on the
@@ -958,44 +951,7 @@ func (ns *Impl) userPing(dstIP netip.Addr, pingResPkt []byte, direction userPing
 	defer userPingSem.Release()
 
 	t0 := time.Now()
-	var err error
-	switch runtime.GOOS {
-	case "windows":
-		err = exec.Command("ping", "-n", "1", "-w", "3000", dstIP.String()).Run()
-	case "darwin", "freebsd":
-		// Note: 2000 ms is actually 1 second + 2,000
-		// milliseconds extra for 3 seconds total.
-		// See https://github.com/tailscale/tailscale/pull/3753 for details.
-		ping := "ping"
-		if dstIP.Is6() {
-			ping = "ping6"
-		}
-		err = exec.Command(ping, "-c", "1", "-W", "2000", dstIP.String()).Run()
-	case "openbsd":
-		ping := "ping"
-		if dstIP.Is6() {
-			ping = "ping6"
-		}
-		err = exec.Command(ping, "-c", "1", "-w", "3", dstIP.String()).Run()
-	case "android":
-		ping := "/system/bin/ping"
-		if dstIP.Is6() {
-			ping = "/system/bin/ping6"
-		}
-		err = exec.Command(ping, "-c", "1", "-w", "3", dstIP.String()).Run()
-	default:
-		ping := "ping"
-		if isSynology {
-			ping = "/bin/ping"
-		}
-		cmd := exec.Command(ping, "-c", "1", "-W", "3", dstIP.String())
-		if isSynology && os.Getuid() != 0 {
-			// On DSM7 we run as non-root and need to pass
-			// CAP_NET_RAW if our binary has it.
-			setAmbientCapsRaw(cmd)
-		}
-		err = cmd.Run()
-	}
+	err := ns.sendOutboundUserPing(dstIP, 3*time.Second)
 	d := time.Since(t0)
 	if err != nil {
 		if d < time.Second/2 {

+ 65 - 0
wgengine/netstack/netstack_userping.go

@@ -0,0 +1,65 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+
+//go:build !darwin && !ios
+
+package netstack
+
+import (
+	"net/netip"
+	"os"
+	"os/exec"
+	"runtime"
+	"time"
+
+	"tailscale.com/version/distro"
+)
+
+// setAmbientCapsRaw is non-nil on Linux for Synology, to run ping with
+// CAP_NET_RAW from tailscaled's binary.
+var setAmbientCapsRaw func(*exec.Cmd)
+
+var isSynology = runtime.GOOS == "linux" && distro.Get() == distro.Synology
+
+// sendOutboundUserPing sends a non-privileged ICMP (or ICMPv6) ping to dstIP with the given timeout.
+func (ns *Impl) sendOutboundUserPing(dstIP netip.Addr, timeout time.Duration) error {
+	var err error
+	switch runtime.GOOS {
+	case "windows":
+		err = exec.Command("ping", "-n", "1", "-w", "3000", dstIP.String()).Run()
+	case "freebsd":
+		// Note: 2000 ms is actually 1 second + 2,000
+		// milliseconds extra for 3 seconds total.
+		// See https://github.com/tailscale/tailscale/pull/3753 for details.
+		ping := "ping"
+		if dstIP.Is6() {
+			ping = "ping6"
+		}
+		err = exec.Command(ping, "-c", "1", "-W", "2000", dstIP.String()).Run()
+	case "openbsd":
+		ping := "ping"
+		if dstIP.Is6() {
+			ping = "ping6"
+		}
+		err = exec.Command(ping, "-c", "1", "-w", "3", dstIP.String()).Run()
+	case "android":
+		ping := "/system/bin/ping"
+		if dstIP.Is6() {
+			ping = "/system/bin/ping6"
+		}
+		err = exec.Command(ping, "-c", "1", "-w", "3", dstIP.String()).Run()
+	default:
+		ping := "ping"
+		if isSynology {
+			ping = "/bin/ping"
+		}
+		cmd := exec.Command(ping, "-c", "1", "-W", "3", dstIP.String())
+		if isSynology && os.Getuid() != 0 {
+			// On DSM7 we run as non-root and need to pass
+			// CAP_NET_RAW if our binary has it.
+			setAmbientCapsRaw(cmd)
+		}
+		err = cmd.Run()
+	}
+	return err
+}

+ 38 - 0
wgengine/netstack/netstack_userping_apple.go

@@ -0,0 +1,38 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+
+//go:build darwin || ios
+
+package netstack
+
+import (
+	"net/netip"
+	"time"
+
+	"github.com/prometheus-community/pro-bing"
+)
+
+// sendOutboundUserPing sends a non-privileged ICMP (or ICMPv6) ping to dstIP with the given timeout.
+func (ns *Impl) sendOutboundUserPing(dstIP netip.Addr, timeout time.Duration) error {
+	p, err := probing.NewPinger(dstIP.String())
+	if err != nil {
+		ns.logf("sendICMPPingToIP failed to create pinger: %v", err)
+		return err
+	}
+
+	p.Timeout = timeout
+	p.Count = 1
+	p.SetPrivileged(false)
+
+	p.OnSend = func(pkt *probing.Packet) {
+		ns.logf("sendICMPPingToIP: forwarding ping to %s:", p.Addr())
+	}
+	p.OnRecv = func(pkt *probing.Packet) {
+		ns.logf("sendICMPPingToIP: %d bytes pong from %s: icmp_seq=%d time=%v", pkt.Nbytes, pkt.IPAddr, pkt.Seq, pkt.Rtt)
+	}
+	p.OnFinish = func(stats *probing.Statistics) {
+		ns.logf("sendICMPPingToIP: done, %d replies received", stats.PacketsRecv)
+	}
+
+	return p.Run()
+}