From f0d6f173c919fdccf726f6984891d2c702c74111 Mon Sep 17 00:00:00 2001 From: Andrew Dunham Date: Thu, 4 Aug 2022 17:10:13 -0400 Subject: [PATCH] net/netcheck: try ICMP if UDP is blocked (#5056) Signed-off-by: Andrew Dunham --- cmd/tailscale/depaware.txt | 7 +- cmd/tailscaled/depaware.txt | 4 +- net/netcheck/netcheck.go | 101 ++++++++++++- net/netcheck/netcheck_test.go | 7 +- net/ping/ping.go | 253 +++++++++++++++++++++++++++++++ net/ping/ping_test.go | 255 ++++++++++++++++++++++++++++++++ tailcfg/tailcfg.go | 9 +- tailcfg/tailcfg_clone.go | 1 + tailcfg/tailcfg_test.go | 1 + tailcfg/tailcfg_view.go | 2 + wgengine/magicsock/magicsock.go | 1 + 11 files changed, 634 insertions(+), 7 deletions(-) create mode 100644 net/ping/ping.go create mode 100644 net/ping/ping_test.go diff --git a/cmd/tailscale/depaware.txt b/cmd/tailscale/depaware.txt index d30a71433..ba1040cf1 100644 --- a/cmd/tailscale/depaware.txt +++ b/cmd/tailscale/depaware.txt @@ -59,6 +59,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep tailscale.com/net/netns from tailscale.com/derp/derphttp+ tailscale.com/net/netutil from tailscale.com/client/tailscale+ tailscale.com/net/packet from tailscale.com/wgengine/filter + tailscale.com/net/ping from tailscale.com/net/netcheck tailscale.com/net/portmapper from tailscale.com/net/netcheck+ tailscale.com/net/stun from tailscale.com/net/netcheck tailscale.com/net/tlsdial from tailscale.com/derp/derphttp+ @@ -92,6 +93,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep W tailscale.com/util/endian from tailscale.com/net/netns tailscale.com/util/groupmember from tailscale.com/cmd/tailscale/cli tailscale.com/util/lineread from tailscale.com/net/interfaces+ + tailscale.com/util/mak from tailscale.com/net/netcheck tailscale.com/util/singleflight from tailscale.com/net/dnscache W 💣 tailscale.com/util/winutil from tailscale.com/hostinfo+ tailscale.com/version from tailscale.com/cmd/tailscale/cli+ @@ -108,12 +110,15 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep golang.org/x/crypto/nacl/box from tailscale.com/types/key golang.org/x/crypto/nacl/secretbox from golang.org/x/crypto/nacl/box golang.org/x/crypto/salsa20/salsa from golang.org/x/crypto/nacl/box+ - L golang.org/x/net/bpf from github.com/mdlayher/netlink+ + golang.org/x/net/bpf from github.com/mdlayher/netlink+ golang.org/x/net/dns/dnsmessage from net+ golang.org/x/net/http/httpguts from net/http+ golang.org/x/net/http/httpproxy from net/http golang.org/x/net/http2/hpack from net/http + 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 golang.org/x/net/icmp+ + golang.org/x/net/ipv6 from golang.org/x/net/icmp golang.org/x/net/proxy from tailscale.com/net/netns D golang.org/x/net/route from net+ golang.org/x/sync/errgroup from tailscale.com/derp+ diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt index 26674fe24..2eba5ceda 100644 --- a/cmd/tailscaled/depaware.txt +++ b/cmd/tailscaled/depaware.txt @@ -227,6 +227,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de 💣 tailscale.com/net/netstat from tailscale.com/ipn/ipnserver tailscale.com/net/netutil from tailscale.com/ipn/ipnlocal+ tailscale.com/net/packet from tailscale.com/net/tstun+ + tailscale.com/net/ping from tailscale.com/net/netcheck tailscale.com/net/portmapper from tailscale.com/net/netcheck+ tailscale.com/net/proxymux from tailscale.com/cmd/tailscaled tailscale.com/net/socks5 from tailscale.com/cmd/tailscaled @@ -321,8 +322,9 @@ 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/idna from golang.org/x/net/http/httpguts+ - golang.org/x/net/ipv4 from golang.zx2c4.com/wireguard/device + golang.org/x/net/ipv4 from golang.zx2c4.com/wireguard/device+ golang.org/x/net/ipv6 from golang.zx2c4.com/wireguard/device+ golang.org/x/net/proxy from tailscale.com/net/netns D golang.org/x/net/route from net+ diff --git a/net/netcheck/netcheck.go b/net/netcheck/netcheck.go index e11887484..ec9919f7f 100644 --- a/net/netcheck/netcheck.go +++ b/net/netcheck/netcheck.go @@ -29,6 +29,7 @@ import ( "tailscale.com/net/netaddr" "tailscale.com/net/neterror" "tailscale.com/net/netns" + "tailscale.com/net/ping" "tailscale.com/net/portmapper" "tailscale.com/net/stun" "tailscale.com/syncs" @@ -37,6 +38,7 @@ import ( "tailscale.com/types/nettype" "tailscale.com/types/opt" "tailscale.com/util/clientmetric" + "tailscale.com/util/mak" ) // Debugging and experimentation tweakables. @@ -54,6 +56,9 @@ const ( // switching to HTTP probing, on the assumption that outbound UDP // is blocked. stunProbeTimeout = 3 * time.Second + // icmpProbeTimeout is the maximum amount of time netcheck will spend + // probing with ICMP packets. + icmpProbeTimeout = 1 * time.Second // hairpinCheckTimeout is the amount of time we wait for a // hairpinned packet to come back. hairpinCheckTimeout = 100 * time.Millisecond @@ -79,6 +84,7 @@ type Report struct { IPv6CanSend bool // an IPv6 packet was able to be sent IPv4CanSend bool // an IPv4 packet was able to be sent OSHasIPv6 bool // could bind a socket to ::1 + ICMPv4 bool // an ICMPv4 round trip completed // MappingVariesByDestIP is whether STUN results depend which // STUN server you're talking to (on IPv4). @@ -905,7 +911,8 @@ func (c *Client) GetReport(ctx context.Context, dm *tailcfg.DERPMap) (_ *Report, } rs.stopTimers() - // Try HTTPS latency check if all STUN probes failed due to UDP presumably being blocked. + // Try HTTPS and ICMP latency check if all STUN probes failed due to + // UDP presumably being blocked. // TODO: this should be moved into the probePlan, using probeProto probeHTTPS. if !rs.anyUDP() && ctx.Err() == nil { var wg sync.WaitGroup @@ -916,6 +923,19 @@ func (c *Client) GetReport(ctx context.Context, dm *tailcfg.DERPMap) (_ *Report, } } if len(need) > 0 { + // Kick off ICMP in parallel to HTTPS checks; we don't + // reuse the same WaitGroup for those probes because we + // need to close the underlying Pinger after a timeout + // or when all ICMP probes are done, regardless of + // whether the HTTPS probes have finished. + wg.Add(1) + go func() { + defer wg.Done() + if err := c.measureAllICMPLatency(ctx, rs, need); err != nil { + c.logf("[v1] measureAllICMPLatency: %v", err) + } + }() + wg.Add(len(need)) c.logf("netcheck: UDP is blocked, trying HTTPS") } @@ -926,7 +946,11 @@ func (c *Client) GetReport(ctx context.Context, dm *tailcfg.DERPMap) (_ *Report, c.logf("[v1] netcheck: measuring HTTPS latency of %v (%d): %v", reg.RegionCode, reg.RegionID, err) } else { rs.mu.Lock() - rs.report.RegionLatency[reg.RegionID] = d + if l, ok := rs.report.RegionLatency[reg.RegionID]; !ok { + mak.Set(&rs.report.RegionLatency, reg.RegionID, d) + } else if l >= d { + rs.report.RegionLatency[reg.RegionID] = d + } // We set these IPv4 and IPv6 but they're not really used // and we don't necessarily set them both. If UDP is blocked // and both IPv4 and IPv6 are available over TCP, it's basically @@ -1086,12 +1110,85 @@ func (c *Client) measureHTTPSLatency(ctx context.Context, reg *tailcfg.DERPRegio return result.ServerProcessing, ip, nil } +func (c *Client) measureAllICMPLatency(ctx context.Context, rs *reportState, need []*tailcfg.DERPRegion) error { + if len(need) == 0 { + return nil + } + ctx, done := context.WithTimeout(ctx, icmpProbeTimeout) + defer done() + + p, err := ping.New(ctx, c.logf) + if err != nil { + return err + } + defer p.Close() + + c.logf("UDP is blocked, trying ICMP") + + var wg sync.WaitGroup + wg.Add(len(need)) + for _, reg := range need { + go func(reg *tailcfg.DERPRegion) { + defer wg.Done() + if d, err := c.measureICMPLatency(ctx, reg, p); err != nil { + c.logf("[v1] measuring ICMP latency of %v (%d): %v", reg.RegionCode, reg.RegionID, err) + } else { + c.logf("[v1] ICMP latency of %v (%d): %v", reg.RegionCode, reg.RegionID, d) + rs.mu.Lock() + if l, ok := rs.report.RegionLatency[reg.RegionID]; !ok { + mak.Set(&rs.report.RegionLatency, reg.RegionID, d) + } else if l >= d { + rs.report.RegionLatency[reg.RegionID] = d + } + + // We only send IPv4 ICMP right now + rs.report.IPv4 = true + rs.report.ICMPv4 = true + + rs.mu.Unlock() + } + }(reg) + } + + wg.Wait() + return nil +} + +func (c *Client) measureICMPLatency(ctx context.Context, reg *tailcfg.DERPRegion, p *ping.Pinger) (time.Duration, error) { + if len(reg.Nodes) == 0 { + return 0, fmt.Errorf("no nodes for region %d (%v)", reg.RegionID, reg.RegionCode) + } + + // Try pinging the first node in the region + node := reg.Nodes[0] + + // Get the IPAddr by asking for the UDP address that we would use for + // STUN and then using that IP. + // + // TODO(andrew-d): this is a bit ugly + nodeAddr := c.nodeAddr(ctx, node, probeIPv4) + if !nodeAddr.IsValid() { + return 0, fmt.Errorf("no address for node %v", node.Name) + } + addr := &net.IPAddr{ + IP: net.IP(nodeAddr.Addr().AsSlice()), + Zone: nodeAddr.Addr().Zone(), + } + + // Use the unique node.Name field as the packet data to reduce the + // likelihood that we get a mismatched echo response. + return p.Send(ctx, addr, []byte(node.Name)) +} + func (c *Client) logConciseReport(r *Report, dm *tailcfg.DERPMap) { c.logf("[v1] report: %v", logger.ArgWriter(func(w *bufio.Writer) { fmt.Fprintf(w, "udp=%v", r.UDP) if !r.IPv4 { fmt.Fprintf(w, " v4=%v", r.IPv4) } + if !r.UDP { + fmt.Fprintf(w, " icmpv4=%v", r.ICMPv4) + } fmt.Fprintf(w, " v6=%v", r.IPv6) fmt.Fprintf(w, " mapvarydest=%v", r.MappingVariesByDestIP) diff --git a/net/netcheck/netcheck_test.go b/net/netcheck/netcheck_test.go index 6dec50a25..f37aad0f2 100644 --- a/net/netcheck/netcheck_test.go +++ b/net/netcheck/netcheck_test.go @@ -524,7 +524,12 @@ func TestLogConciseReport(t *testing.T) { { name: "no_udp", r: &Report{}, - want: "udp=false v4=false v6=false mapvarydest= hair= portmap=? derp=0", + want: "udp=false v4=false icmpv4=false v6=false mapvarydest= hair= portmap=? derp=0", + }, + { + name: "no_udp_icmp", + r: &Report{ICMPv4: true, IPv4: true}, + want: "udp=false icmpv4=true v6=false mapvarydest= hair= portmap=? derp=0", }, { name: "ipv4_one_region", diff --git a/net/ping/ping.go b/net/ping/ping.go new file mode 100644 index 000000000..826d2d71f --- /dev/null +++ b/net/ping/ping.go @@ -0,0 +1,253 @@ +// Copyright (c) 2022 Tailscale Inc & AUTHORS All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Package ping allows sending ICMP echo requests to a host in order to +// determine network latency. +package ping + +import ( + "bytes" + "context" + "crypto/rand" + "encoding/binary" + "fmt" + "log" + "net" + "sync" + "time" + + "golang.org/x/net/icmp" + "golang.org/x/net/ipv4" + "tailscale.com/net/netns" + "tailscale.com/types/logger" +) + +type response struct { + t time.Time + err error +} + +type outstanding struct { + ch chan response + data []byte +} + +// Pinger represents a set of ICMP echo requests to be sent at a single time. +// +// A new instance should be created for each concurrent set of ping requests; +// this type should not be reused. +type Pinger struct { + c net.PacketConn + Logf logger.Logf + Verbose bool + timeNow func() time.Time + id uint16 // uint16 per RFC 792 + wg sync.WaitGroup + + // Following fields protected by mu + mu sync.Mutex + seq uint16 // uint16 per RFC 792 + pings map[uint16]outstanding +} + +// New creates a new Pinger. The Context provided will be used to create +// network listeners, and to set an absolute deadline (if any) on the net.Conn +func New(ctx context.Context, logf logger.Logf) (*Pinger, error) { + p, err := newUnstarted(ctx, logf) + if err != nil { + return nil, err + } + + // Start by setting the deadline from the context; note that this + // applies to all future I/O, so we only need to do it once. + deadline, ok := ctx.Deadline() + if ok { + if err := p.c.SetReadDeadline(deadline); err != nil { + return nil, err + } + } + + p.wg.Add(1) + go p.run(ctx) + return p, nil +} + +func newUnstarted(ctx context.Context, logf logger.Logf) (*Pinger, error) { + var id [2]byte + _, err := rand.Read(id[:]) + if err != nil { + return nil, err + } + + conn, err := netns.Listener(logf).ListenPacket(ctx, "ip4:icmp", "0.0.0.0") + if err != nil { + return nil, err + } + + return &Pinger{ + c: conn, + Logf: logf, + timeNow: time.Now, + id: binary.LittleEndian.Uint16(id[:]), + pings: make(map[uint16]outstanding), + }, nil +} + +func (p *Pinger) logf(format string, a ...any) { + if p.Logf != nil { + p.Logf(format, a...) + } else { + log.Printf(format, a...) + } +} + +func (p *Pinger) vlogf(format string, a ...any) { + if p.Verbose { + p.logf(format, a...) + } +} + +func (p *Pinger) Close() error { + err := p.c.Close() + p.wg.Wait() + return err +} + +func (p *Pinger) run(ctx context.Context) { + defer p.wg.Done() + buf := make([]byte, 1500) + +loop: + for { + select { + case <-ctx.Done(): + break loop + default: + } + + n, addr, err := p.c.ReadFrom(buf) + if err != nil { + // Ignore temporary errors; everything else is fatal + if netErr, ok := err.(net.Error); !ok || !netErr.Temporary() { + break + } + continue + } + + p.handleResponse(buf[:n], addr, p.timeNow()) + } + + p.cleanupOutstanding() +} + +func (p *Pinger) cleanupOutstanding() { + // Complete outstanding requests + p.mu.Lock() + defer p.mu.Unlock() + for _, o := range p.pings { + o.ch <- response{err: net.ErrClosed} + } +} + +func (p *Pinger) handleResponse(buf []byte, addr net.Addr, now time.Time) { + const ProtocolICMP = 1 + m, err := icmp.ParseMessage(ProtocolICMP, buf) + if err != nil { + p.vlogf("handleResponse: invalid packet: %v", err) + return + } + + if m.Type != ipv4.ICMPTypeEchoReply { + p.vlogf("handleResponse: wanted m.Type=%d; got %d", ipv4.ICMPTypeEchoReply, m.Type) + return + } + + resp, ok := m.Body.(*icmp.Echo) + if !ok || resp == nil { + p.vlogf("handleResponse: wanted body=*icmp.Echo; got %v", m.Body) + return + } + + // We assume we sent this if the ID in the response is ours. + if uint16(resp.ID) != p.id { + p.vlogf("handleResponse: wanted ID=%d; got %d", p.id, resp.ID) + return + } + + // Search for existing running echo request + var o outstanding + p.mu.Lock() + if o, ok = p.pings[uint16(resp.Seq)]; ok { + // Ensure that the data matches before we delete from our map, + // so a future correct packet will be handled correctly. + if bytes.Equal(resp.Data, o.data) { + delete(p.pings, uint16(resp.Seq)) + } else { + p.vlogf("handleResponse: got response for Seq %d with mismatched data", resp.Seq) + ok = false + } + } else { + p.vlogf("handleResponse: got response for unknown Seq %d", resp.Seq) + } + p.mu.Unlock() + + if ok { + o.ch <- response{t: now} + } +} + +// Send sends an ICMP Echo Request packet to the destination, waits for a +// response, and returns the duration between when the request was sent and +// when the reply was received. +// +// If provided, "data" is sent with the packet and is compared upon receiving a +// reply. +func (p *Pinger) Send(ctx context.Context, dest net.Addr, data []byte) (time.Duration, error) { + // Use sequential sequence numbers on the assumption that we will not + // wrap around when using a single Pinger instance + p.mu.Lock() + p.seq++ + seq := p.seq + p.mu.Unlock() + + m := icmp.Message{ + Type: ipv4.ICMPTypeEcho, + Code: 0, + Body: &icmp.Echo{ + ID: int(p.id), + Seq: int(seq), + Data: data, + }, + } + b, err := m.Marshal(nil) + if err != nil { + return 0, err + } + + // Register our response before sending since we could otherwise race a + // quick reply. + ch := make(chan response, 1) + p.mu.Lock() + p.pings[seq] = outstanding{ch: ch, data: data} + p.mu.Unlock() + + start := p.timeNow() + n, err := p.c.WriteTo(b, dest) + if err != nil { + return 0, err + } else if n != len(b) { + return 0, fmt.Errorf("conn.WriteTo: got %v; want %v", n, len(b)) + } + + select { + case resp := <-ch: + if resp.err != nil { + return 0, resp.err + } + return resp.t.Sub(start), nil + + case <-ctx.Done(): + return 0, ctx.Err() + } +} diff --git a/net/ping/ping_test.go b/net/ping/ping_test.go new file mode 100644 index 000000000..5c606db2a --- /dev/null +++ b/net/ping/ping_test.go @@ -0,0 +1,255 @@ +// Copyright (c) 2022 Tailscale Inc & AUTHORS All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package ping + +import ( + "context" + "errors" + "net" + "testing" + "time" + + "golang.org/x/net/icmp" + "golang.org/x/net/ipv4" + "tailscale.com/tstest" +) + +var ( + localhost = &net.IPAddr{IP: net.IPv4(127, 0, 0, 1)} + localhostUDP = &net.UDPAddr{IP: net.IPv4(127, 0, 0, 1), Port: 12345} +) + +func TestPinger(t *testing.T) { + clock := &tstest.Clock{} + + ctx := context.Background() + ctx, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() + + p, closeP := mockPinger(t, clock) + defer closeP() + + bodyData := []byte("data goes here") + + // Start a ping in the background + r := make(chan time.Duration, 1) + go func() { + dur, err := p.Send(ctx, localhostUDP, bodyData) + if err != nil { + t.Errorf("p.Send: %v", err) + r <- 0 + } else { + r <- dur + } + }() + + p.waitOutstanding(t, ctx, 1) + + // Fake a response from ourself + fakeResponse := mustMarshal(t, &icmp.Message{ + Type: ipv4.ICMPTypeEchoReply, + Code: 0, + Body: &icmp.Echo{ + ID: 1234, + Seq: 1, + Data: bodyData, + }, + }) + + const fakeDuration = 100 * time.Millisecond + p.handleResponse(fakeResponse, localhost, clock.Now().Add(fakeDuration)) + + select { + case dur := <-r: + want := fakeDuration + if dur != want { + t.Errorf("wanted ping response time = %d; got %d", want, dur) + } + case <-ctx.Done(): + t.Fatal("did not get response by timeout") + } +} + +func TestPingerTimeout(t *testing.T) { + ctx := context.Background() + ctx, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() + + clock := &tstest.Clock{} + p, closeP := mockPinger(t, clock) + defer closeP() + + // Send a ping in the background + r := make(chan error, 1) + go func() { + _, err := p.Send(ctx, localhostUDP, []byte("data goes here")) + r <- err + }() + + // Wait until we're blocking + p.waitOutstanding(t, ctx, 1) + + // Close everything down + p.cleanupOutstanding() + + // Should have got an error from the ping + err := <-r + if !errors.Is(err, net.ErrClosed) { + t.Errorf("wanted errors.Is(err, net.ErrClosed); got=%v", err) + } +} + +func TestPingerMismatch(t *testing.T) { + clock := &tstest.Clock{} + + ctx := context.Background() + ctx, cancel := context.WithTimeout(ctx, 1*time.Second) // intentionally short + defer cancel() + + p, closeP := mockPinger(t, clock) + defer closeP() + + bodyData := []byte("data goes here") + + // Start a ping in the background + r := make(chan time.Duration, 1) + go func() { + dur, err := p.Send(ctx, localhostUDP, bodyData) + if err != nil && !errors.Is(err, context.DeadlineExceeded) { + t.Errorf("p.Send: %v", err) + r <- 0 + } else { + r <- dur + } + }() + + p.waitOutstanding(t, ctx, 1) + + // "Receive" a bunch of intentionally malformed packets that should not + // result in the Send call above returning + badPackets := []struct { + name string + pkt *icmp.Message + }{ + { + name: "wrong type", + pkt: &icmp.Message{ + Type: ipv4.ICMPTypeDestinationUnreachable, + Code: 0, + Body: &icmp.DstUnreach{}, + }, + }, + { + name: "wrong id", + pkt: &icmp.Message{ + Type: ipv4.ICMPTypeEchoReply, + Code: 0, + Body: &icmp.Echo{ + ID: 9999, + Seq: 1, + Data: bodyData, + }, + }, + }, + { + name: "wrong seq", + pkt: &icmp.Message{ + Type: ipv4.ICMPTypeEchoReply, + Code: 0, + Body: &icmp.Echo{ + ID: 1234, + Seq: 5, + Data: bodyData, + }, + }, + }, + { + name: "bad body", + pkt: &icmp.Message{ + Type: ipv4.ICMPTypeEchoReply, + Code: 0, + Body: &icmp.Echo{ + ID: 1234, + Seq: 1, + + // Intentionally missing first byte + Data: bodyData[1:], + }, + }, + }, + } + + const fakeDuration = 100 * time.Millisecond + tm := clock.Now().Add(fakeDuration) + + for _, tt := range badPackets { + fakeResponse := mustMarshal(t, tt.pkt) + p.handleResponse(fakeResponse, localhost, tm) + } + + // Also "receive" a packet that does not unmarshal as an ICMP packet + p.handleResponse([]byte("foo"), localhost, tm) + + select { + case <-r: + t.Fatal("wanted timeout") + case <-ctx.Done(): + t.Logf("test correctly timed out") + } +} + +func mockPinger(t *testing.T, clock *tstest.Clock) (*Pinger, func()) { + // In tests, we use UDP so that we can test without being root; this + // doesn't matter becuase we mock out the ICMP reply below to be a real + // ICMP echo reply packet. + conn, err := net.ListenPacket("udp4", "127.0.0.1:0") + if err != nil { + t.Fatalf("net.ListenPacket: %v", err) + } + + p := &Pinger{ + c: conn, + Logf: t.Logf, + Verbose: true, + timeNow: clock.Now, + id: 1234, + pings: make(map[uint16]outstanding), + } + done := func() { + if err := p.Close(); err != nil { + t.Errorf("error on close: %v", err) + } + } + return p, done +} + +func mustMarshal(t *testing.T, m *icmp.Message) []byte { + t.Helper() + + b, err := m.Marshal(nil) + if err != nil { + t.Fatal(err) + } + return b +} + +func (p *Pinger) waitOutstanding(t *testing.T, ctx context.Context, count int) { + // This is a bit janky, but... we busy-loop to wait for the Send call + // to write to our map so we know that a response will be handled. + var haveMapEntry bool + for !haveMapEntry { + time.Sleep(10 * time.Millisecond) + select { + case <-ctx.Done(): + t.Error("no entry in ping map before timeout") + return + default: + } + + p.mu.Lock() + haveMapEntry = len(p.pings) == count + p.mu.Unlock() + } +} diff --git a/tailcfg/tailcfg.go b/tailcfg/tailcfg.go index f0a0241ee..12107a14e 100644 --- a/tailcfg/tailcfg.go +++ b/tailcfg/tailcfg.go @@ -510,6 +510,10 @@ type NetInfo struct { // WorkingUDP is whether the host has UDP internet connectivity. WorkingUDP opt.Bool + // WorkingICMPv4 is whether ICMPv4 works. + // Empty means not checked. + WorkingICMPv4 opt.Bool + // HavePortMap is whether we have an existing portmap open // (UPnP, PMP, or PCP). HavePortMap bool `json:",omitempty"` @@ -554,9 +558,9 @@ func (ni *NetInfo) String() string { if ni == nil { return "NetInfo(nil)" } - return fmt.Sprintf("NetInfo{varies=%v hairpin=%v ipv6=%v udp=%v derp=#%v portmap=%v link=%q}", + return fmt.Sprintf("NetInfo{varies=%v hairpin=%v ipv6=%v udp=%v icmpv4=%v derp=#%v portmap=%v link=%q}", ni.MappingVariesByDestIP, ni.HairPinning, ni.WorkingIPv6, - ni.WorkingUDP, ni.PreferredDERP, + ni.WorkingUDP, ni.WorkingICMPv4, ni.PreferredDERP, ni.portMapSummary(), ni.LinkType) } @@ -600,6 +604,7 @@ func (ni *NetInfo) BasicallyEqual(ni2 *NetInfo) bool { ni.WorkingIPv6 == ni2.WorkingIPv6 && ni.OSHasIPv6 == ni2.OSHasIPv6 && ni.WorkingUDP == ni2.WorkingUDP && + ni.WorkingICMPv4 == ni2.WorkingICMPv4 && ni.HavePortMap == ni2.HavePortMap && ni.UPnP == ni2.UPnP && ni.PMP == ni2.PMP && diff --git a/tailcfg/tailcfg_clone.go b/tailcfg/tailcfg_clone.go index 10201c7b6..5830a1f36 100644 --- a/tailcfg/tailcfg_clone.go +++ b/tailcfg/tailcfg_clone.go @@ -156,6 +156,7 @@ var _NetInfoCloneNeedsRegeneration = NetInfo(struct { WorkingIPv6 opt.Bool OSHasIPv6 opt.Bool WorkingUDP opt.Bool + WorkingICMPv4 opt.Bool HavePortMap bool UPnP opt.Bool PMP opt.Bool diff --git a/tailcfg/tailcfg_test.go b/tailcfg/tailcfg_test.go index 28ecc7dbc..6a900bcae 100644 --- a/tailcfg/tailcfg_test.go +++ b/tailcfg/tailcfg_test.go @@ -502,6 +502,7 @@ func TestNetInfoFields(t *testing.T) { "WorkingIPv6", "OSHasIPv6", "WorkingUDP", + "WorkingICMPv4", "HavePortMap", "UPnP", "PMP", diff --git a/tailcfg/tailcfg_view.go b/tailcfg/tailcfg_view.go index ec685b1ad..bf5a387d7 100644 --- a/tailcfg/tailcfg_view.go +++ b/tailcfg/tailcfg_view.go @@ -340,6 +340,7 @@ func (v NetInfoView) HairPinning() opt.Bool { return v.ж.HairPinning func (v NetInfoView) WorkingIPv6() opt.Bool { return v.ж.WorkingIPv6 } func (v NetInfoView) OSHasIPv6() opt.Bool { return v.ж.OSHasIPv6 } func (v NetInfoView) WorkingUDP() opt.Bool { return v.ж.WorkingUDP } +func (v NetInfoView) WorkingICMPv4() opt.Bool { return v.ж.WorkingICMPv4 } func (v NetInfoView) HavePortMap() bool { return v.ж.HavePortMap } func (v NetInfoView) UPnP() opt.Bool { return v.ж.UPnP } func (v NetInfoView) PMP() opt.Bool { return v.ж.PMP } @@ -357,6 +358,7 @@ var _NetInfoViewNeedsRegeneration = NetInfo(struct { WorkingIPv6 opt.Bool OSHasIPv6 opt.Bool WorkingUDP opt.Bool + WorkingICMPv4 opt.Bool HavePortMap bool UPnP opt.Bool PMP opt.Bool diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go index b7056fede..8e71570ee 100644 --- a/wgengine/magicsock/magicsock.go +++ b/wgengine/magicsock/magicsock.go @@ -757,6 +757,7 @@ func (c *Conn) updateNetInfo(ctx context.Context) (*netcheck.Report, error) { } ni.WorkingIPv6.Set(report.IPv6) ni.WorkingUDP.Set(report.UDP) + ni.WorkingICMPv4.Set(report.ICMPv4) ni.PreferredDERP = report.PreferredDERP if ni.PreferredDERP == 0 {