diff --git a/wgengine/netstack/netstack.go b/wgengine/netstack/netstack.go index d029b6c19..efb328102 100644 --- a/wgengine/netstack/netstack.go +++ b/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 " : " 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 +} diff --git a/wgengine/netstack/netstack_userping.go b/wgengine/netstack/netstack_userping.go index ab95b5962..ee635bd87 100644 --- a/wgengine/netstack/netstack_userping.go +++ b/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. diff --git a/wgengine/netstack/netstack_userping_test.go b/wgengine/netstack/netstack_userping_test.go new file mode 100644 index 000000000..a179f7467 --- /dev/null +++ b/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) + } + }) + } +}