wgengine/netstack: check userspace ping success on Windows

Hacky temporary workaround until we do #13654 correctly.

Updates #13654

Change-Id: I764eaedbb112fb3a34dddb89572fec1b2543fd4a
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

wgengine/netstack/netstack.go

@@ -5,6 +5,7 @@
 package netstack
 
 import (
+	"bytes"
 	"context"
 	"errors"
 	"expvar"
@@ -1909,3 +1910,35 @@ func (ns *Impl) ExpVar() expvar.Var {
 
 	return m
 }
+
+// windowsPingOutputIsSuccess reports whether the ping.exe output b contains a
+// success ping response for ip.
+//
+// See https://github.com/tailscale/tailscale/issues/13654
+//
+// TODO(bradfitz,nickkhyl): delete this and use the proper Windows APIs.
+func windowsPingOutputIsSuccess(ip netip.Addr, b []byte) bool {
+	// Look for a line that contains " <ip>: " and then three equal signs.
+	// As a special case, the 2nd equal sign may be a '<' character
+	// for sub-millisecond pings.
+	// This heuristic seems to match the ping.exe output in any language.
+	sub := fmt.Appendf(nil, " %s: ", ip)
+
+	eqSigns := func(bb []byte) (n int) {
+		for _, b := range bb {
+			if b == '=' || (b == '<' && n == 1) {
+				n++
+			}
+		}
+		return
+	}
+
+	for len(b) > 0 {
+		var line []byte
+		line, b, _ = bytes.Cut(b, []byte("\n"))
+		if _, rest, ok := bytes.Cut(line, sub); ok && eqSigns(rest) == 3 {
+			return true
+		}
+	}
+	return false
+}

wgengine/netstack/netstack_userping.go

@@ -6,6 +6,7 @@
 package netstack
 
 import (
+	"errors"
 	"net/netip"
 	"os"
 	"os/exec"
@@ -26,7 +27,13 @@ func (ns *Impl) sendOutboundUserPing(dstIP netip.Addr, timeout time.Duration) er
 	var err error
 	switch runtime.GOOS {
 	case "windows":
-		err = exec.Command("ping", "-n", "1", "-w", "3000", dstIP.String()).Run()
+		var out []byte
+		out, err = exec.Command("ping", "-n", "1", "-w", "3000", dstIP.String()).CombinedOutput()
+		if err == nil && !windowsPingOutputIsSuccess(dstIP, out) {
+			// TODO(bradfitz,nickkhyl): return the actual ICMP error we heard back to the caller?
+			// For now we just drop it.
+			err = errors.New("unsuccessful ICMP reply received")
+		}
 	case "freebsd":
 		// Note: 2000 ms is actually 1 second + 2,000
 		// milliseconds extra for 3 seconds total.

wgengine/netstack/netstack_userping_test.go

@@ -0,0 +1,77 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+
+package netstack
+
+import (
+	"net/netip"
+	"testing"
+)
+
+func TestWindowsPingOutputIsSuccess(t *testing.T) {
+	tests := []struct {
+		name string
+		ip   string
+		out  string
+		want bool
+	}{
+		{
+			name: "success",
+			ip:   "10.0.0.1",
+			want: true,
+			out: `Pinging 10.0.0.1 with 32 bytes of data:
+Reply from 10.0.0.1: bytes=32 time=7ms TTL=64
+
+Ping statistics for 10.0.0.1:
+	Packets: Sent = 1, Received = 1, Lost = 0 (0% loss),
+Approximate round trip times in milli-seconds:
+	Minimum = 7ms, Maximum = 7ms, Average = 7ms
+`,
+		},
+		{
+			name: "success_sub_millisecond",
+			ip:   "10.0.0.1",
+			want: true,
+			out: `Pinging 10.0.0.1 with 32 bytes of data:
+Reply from 10.0.0.1: bytes=32 time<1ms TTL=64
+
+Ping statistics for 10.0.0.1:
+	Packets: Sent = 1, Received = 1, Lost = 0 (0% loss),
+Approximate round trip times in milli-seconds:
+	Minimum = 7ms, Maximum = 7ms, Average = 7ms
+`,
+		},
+		{
+			name: "success_german",
+			ip:   "10.0.0.1",
+			want: true,
+			out: `Ping wird ausgeführt für 10.0.0.1 mit 32 Bytes Daten:
+Antwort von from 10.0.0.1: Bytes=32 Zeit=7ms TTL=64
+
+Ping-Statistik für 10.0.0.1:
+	Pakete: Gesendet = 4, Empfangen = 4, Verloren = 0 (0% Verlust),
+Ca. Zeitangaben in Millisek.:
+	Minimum = 7ms, Maximum = 7ms, Mittelwert = 7ms
+`,
+		},
+		{
+			name: "unreachable",
+			ip:   "10.0.0.6",
+			want: false,
+			out: `Pinging 10.0.0.6 with 32 bytes of data:
+Reply from 10.0.108.189: Destination host unreachable
+
+Ping statistics for 10.0.0.6:
+	Packets: Sent = 1, Received = 1, Lost = 0 (0% loss),
+`,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := windowsPingOutputIsSuccess(netip.MustParseAddr(tt.ip), []byte(tt.out))
+			if got != tt.want {
+				t.Errorf("got %v; want %v", got, tt.want)
+			}
+		})
+	}
+}