From ae483d344618a65e372c0c255d30e64d0347157e Mon Sep 17 00:00:00 2001 From: James Tucker Date: Thu, 21 Apr 2022 18:49:01 -0700 Subject: [PATCH] wgengine, net/packet, cmd/tailscale: add ICMP echo Updates tailscale/corp#754 Signed-off-by: James Tucker --- cmd/tailscale/cli/ping.go | 27 ++++++--- control/controlclient/direct.go | 17 +++--- ipn/backend.go | 2 +- ipn/fake_test.go | 2 +- ipn/ipnlocal/local.go | 4 +- ipn/ipnstate/ipnstate.go | 2 +- ipn/message.go | 12 ++-- net/packet/icmp.go | 29 +++++++++ net/packet/packet.go | 23 ++++++++ net/tstun/wrap.go | 13 +++++ tailcfg/tailcfg.go | 19 +++++- wgengine/userspace.go | 100 ++++++++++++++++++++++++++++---- wgengine/watchdog.go | 4 +- wgengine/wgengine.go | 6 +- 14 files changed, 215 insertions(+), 45 deletions(-) create mode 100644 net/packet/icmp.go diff --git a/cmd/tailscale/cli/ping.go b/cmd/tailscale/cli/ping.go index 6a45fbdc9..fe0803678 100644 --- a/cmd/tailscale/cli/ping.go +++ b/cmd/tailscale/cli/ping.go @@ -18,6 +18,7 @@ "github.com/peterbourgon/ff/v3/ffcli" "tailscale.com/ipn" "tailscale.com/ipn/ipnstate" + "tailscale.com/tailcfg" ) var pingCmd = &ffcli.Command{ @@ -26,7 +27,7 @@ ShortHelp: "Ping a host at the Tailscale layer, see how it routed", LongHelp: strings.TrimSpace(` -The 'tailscale ping' command pings a peer node at the Tailscale layer +The 'tailscale ping' command pings a peer node from the Tailscale layer and reports which route it took for each response. The first ping or so will likely go over DERP (Tailscale's TCP relay protocol) while NAT traversal finds a direct path through. @@ -48,7 +49,8 @@ fs := newFlagSet("ping") fs.BoolVar(&pingArgs.verbose, "verbose", false, "verbose output") fs.BoolVar(&pingArgs.untilDirect, "until-direct", true, "stop once a direct path is established") - fs.BoolVar(&pingArgs.tsmp, "tsmp", false, "do a TSMP-level ping (through IP + wireguard, but not involving host OS stack)") + fs.BoolVar(&pingArgs.tsmp, "tsmp", false, "do a TSMP-level ping (through wireguard, but not either host OS stack)") + fs.BoolVar(&pingArgs.icmp, "icmp", false, "do a ICMP-level ping (through wireguard, but not the local host OS stack)") fs.IntVar(&pingArgs.num, "c", 10, "max number of pings to send") fs.DurationVar(&pingArgs.timeout, "timeout", 5*time.Second, "timeout before giving up on a ping") return fs @@ -60,9 +62,20 @@ untilDirect bool verbose bool tsmp bool + icmp bool timeout time.Duration } +func pingType() tailcfg.PingType { + if pingArgs.tsmp { + return tailcfg.PingTSMP + } + if pingArgs.icmp { + return tailcfg.PingICMP + } + return tailcfg.PingDisco +} + func runPing(ctx context.Context, args []string) error { st, err := localClient.Status(ctx) if err != nil { @@ -111,7 +124,7 @@ func runPing(ctx context.Context, args []string) error { anyPong := false for { n++ - bc.Ping(ip, pingArgs.tsmp) + bc.Ping(ip, pingType()) timer := time.NewTimer(pingArgs.timeout) select { case <-timer.C: @@ -132,10 +145,10 @@ func runPing(ctx context.Context, args []string) error { if pr.DERPRegionID != 0 { via = fmt.Sprintf("DERP(%s)", pr.DERPRegionCode) } - if pingArgs.tsmp { + if via == "" { // TODO(bradfitz): populate the rest of ipnstate.PingResult for TSMP queries? - // For now just say it came via TSMP. - via = "TSMP" + // For now just say which protocol it used. + via = string(pingType()) } anyPong = true extra := "" @@ -143,7 +156,7 @@ func runPing(ctx context.Context, args []string) error { extra = fmt.Sprintf(", %d", pr.PeerAPIPort) } printf("pong from %s (%s%s) via %v in %v\n", pr.NodeName, pr.NodeIP, extra, via, latency) - if pingArgs.tsmp { + if pingArgs.tsmp || pingArgs.icmp { return nil } if pr.Endpoint != "" && pingArgs.untilDirect { diff --git a/control/controlclient/direct.go b/control/controlclient/direct.go index 3a377f7f9..cf77b2a6e 100644 --- a/control/controlclient/direct.go +++ b/control/controlclient/direct.go @@ -126,9 +126,9 @@ type Options struct { // Pinger is a subset of the wgengine.Engine interface, containing just the Ping method. type Pinger interface { - // Ping is a request to start a discovery or TSMP ping with the peer handling - // the given IP and then call cb with its ping latency & method. - Ping(ip netaddr.IP, useTSMP bool, cb func(*ipnstate.PingResult)) + // Ping is a request to start a ping with the peer handling the given IP and + // then call cb with its ping latency & method. + Ping(ip netaddr.IP, pingType tailcfg.PingType, cb func(*ipnstate.PingResult)) } type Decompressor interface { @@ -1197,11 +1197,10 @@ func answerPing(logf logger.Logf, c *http.Client, pr *tailcfg.PingRequest, pinge return } for _, t := range strings.Split(pr.Types, ",") { - switch t { - case "TSMP", "disco": - go doPingerPing(logf, c, pr, pinger, t) + switch pt := tailcfg.PingType(t); pt { + case tailcfg.PingTSMP, tailcfg.PingDisco, tailcfg.PingICMP: + go doPingerPing(logf, c, pr, pinger, pt) // TODO(tailscale/corp#754) - // case "host": // case "peerapi": default: logf("unsupported ping request type: %q", t) @@ -1402,13 +1401,13 @@ func (c *Direct) DoNoiseRequest(req *http.Request) (*http.Response, error) { // doPingerPing sends a Ping to pr.IP using pinger, and sends an http request back to // pr.URL with ping response data. -func doPingerPing(logf logger.Logf, c *http.Client, pr *tailcfg.PingRequest, pinger Pinger, pingType string) { +func doPingerPing(logf logger.Logf, c *http.Client, pr *tailcfg.PingRequest, pinger Pinger, pingType tailcfg.PingType) { if pr.URL == "" || pr.IP.IsZero() || pinger == nil { logf("invalid ping request: missing url, ip or pinger") return } start := time.Now() - pinger.Ping(pr.IP, pingType == "TSMP", func(res *ipnstate.PingResult) { + pinger.Ping(pr.IP, pingType, func(res *ipnstate.PingResult) { // Currently does not check for error since we just return if it fails. postPingResult(start, logf, c, pr, res.ToPingResponse(pingType)) }) diff --git a/ipn/backend.go b/ipn/backend.go index dd597e716..fd2598c9f 100644 --- a/ipn/backend.go +++ b/ipn/backend.go @@ -248,5 +248,5 @@ type Backend interface { // Ping attempts to start connecting to the given IP and sends a Notify // with its PingResult. If the host is down, there might never // be a PingResult sent. The cmd/tailscale CLI client adds a timeout. - Ping(ip string, useTSMP bool) + Ping(ip string, pingType tailcfg.PingType) } diff --git a/ipn/fake_test.go b/ipn/fake_test.go index 3d08cfc46..0373e6604 100644 --- a/ipn/fake_test.go +++ b/ipn/fake_test.go @@ -100,7 +100,7 @@ func (b *FakeBackend) RequestEngineStatus() { } } -func (b *FakeBackend) Ping(ip string, useTSMP bool) { +func (b *FakeBackend) Ping(ip string, pingType tailcfg.PingType) { if b.notify != nil { b.notify(Notify{PingResult: &ipnstate.PingResult{}}) } diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 3fea28c49..f5356148e 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -1699,13 +1699,13 @@ func (b *LocalBackend) StartLoginInteractive() { } } -func (b *LocalBackend) Ping(ipStr string, useTSMP bool) { +func (b *LocalBackend) Ping(ipStr string, pingType tailcfg.PingType) { ip, err := netaddr.ParseIP(ipStr) if err != nil { b.logf("ignoring Ping request to invalid IP %q", ipStr) return } - b.e.Ping(ip, useTSMP, func(pr *ipnstate.PingResult) { + b.e.Ping(ip, pingType, func(pr *ipnstate.PingResult) { b.send(ipn.Notify{PingResult: pr}) }) } diff --git a/ipn/ipnstate/ipnstate.go b/ipn/ipnstate/ipnstate.go index b76890f29..7687da25c 100644 --- a/ipn/ipnstate/ipnstate.go +++ b/ipn/ipnstate/ipnstate.go @@ -515,7 +515,7 @@ type PingResult struct { // TODO(bradfitz): details like whether port mapping was used on either side? (Once supported) } -func (pr *PingResult) ToPingResponse(pingType string) *tailcfg.PingResponse { +func (pr *PingResult) ToPingResponse(pingType tailcfg.PingType) *tailcfg.PingResponse { return &tailcfg.PingResponse{ Type: pingType, IP: pr.IP, diff --git a/ipn/message.go b/ipn/message.go index 02cca9bd2..e35a2c0af 100644 --- a/ipn/message.go +++ b/ipn/message.go @@ -52,8 +52,8 @@ type SetPrefsArgs struct { } type PingArgs struct { - IP string - UseTSMP bool + IP string + Type tailcfg.PingType } // Command is a command message that is JSON encoded and sent by a @@ -171,7 +171,7 @@ func (bs *BackendServer) GotCommand(ctx context.Context, cmd *Command) error { bs.b.RequestEngineStatus() return nil } else if c := cmd.Ping; c != nil { - bs.b.Ping(c.IP, c.UseTSMP) + bs.b.Ping(c.IP, tailcfg.PingType(c.Type)) return nil } @@ -311,10 +311,10 @@ func (bc *BackendClient) RequestStatus() { bc.send(Command{AllowVersionSkew: true, RequestStatus: &NoArgs{}}) } -func (bc *BackendClient) Ping(ip string, useTSMP bool) { +func (bc *BackendClient) Ping(ip string, pingType tailcfg.PingType) { bc.send(Command{Ping: &PingArgs{ - IP: ip, - UseTSMP: useTSMP, + IP: ip, + Type: pingType, }}) } diff --git a/net/packet/icmp.go b/net/packet/icmp.go new file mode 100644 index 000000000..bac3dd13f --- /dev/null +++ b/net/packet/icmp.go @@ -0,0 +1,29 @@ +// 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 packet + +import ( + crand "crypto/rand" + + "encoding/binary" +) + +// ICMPEchoPayload generates a new random ID/Sequence pair, and returns a uint32 +// derived from them, along with the id, sequence and given payload in a buffer. +// It returns an error if the random source could not be read. +func ICMPEchoPayload(payload []byte) (idSeq uint32, buf []byte) { + buf = make([]byte, len(payload)+4) + + // make a completely random id/sequence combo, which is very unlikely to + // collide with a running ping sequence on the host system. Errors are + // ignored, that would result in collisions, but errors reading from the + // random device are rare, and will cause this process universe to soon end. + crand.Read(buf[:4]) + + idSeq = binary.LittleEndian.Uint32(buf) + copy(buf[4:], payload) + + return +} diff --git a/net/packet/packet.go b/net/packet/packet.go index a79bb1f79..48633b89d 100644 --- a/net/packet/packet.go +++ b/net/packet/packet.go @@ -434,6 +434,29 @@ func (q *Parsed) IsEchoResponse() bool { } } +// EchoIDSeq extracts the identifier/sequence bytes from an ICMP Echo response, +// and returns them as a uint32, used to lookup internally routed ICMP echo +// responses. This function is intentionally lightweight as it is called on +// every incoming ICMP packet. +func (q *Parsed) EchoIDSeq() uint32 { + switch q.IPProto { + case ipproto.ICMPv4: + offset := ip4HeaderLength + icmp4HeaderLength + if len(q.b) < offset+4 { + return 0 + } + return binary.LittleEndian.Uint32(q.b[offset:]) + case ipproto.ICMPv6: + offset := ip6HeaderLength + icmp6HeaderLength + if len(q.b) < offset+4 { + return 0 + } + return binary.LittleEndian.Uint32(q.b[offset:]) + default: + return 0 + } +} + func Hexdump(b []byte) string { out := new(strings.Builder) for i := 0; i < len(b); i += 16 { diff --git a/net/tstun/wrap.go b/net/tstun/wrap.go index 400553b6a..9d13ba7c2 100644 --- a/net/tstun/wrap.go +++ b/net/tstun/wrap.go @@ -151,6 +151,11 @@ type Wrapper struct { // OnTSMPPongReceived, if non-nil, is called whenever a TSMP pong arrives. OnTSMPPongReceived func(packet.TSMPPongReply) + // OnICMPEchoResponseReceived, if non-nil, is called whenever a ICMP echo response + // arrives. If the packet is to be handled internally this returns true, + // false otherwise. + OnICMPEchoResponseReceived func(*packet.Parsed) bool + // PeerAPIPort, if non-nil, returns the peerapi port that's // running for the given IP address. PeerAPIPort func(netaddr.IP) (port uint16, ok bool) @@ -575,6 +580,14 @@ func (t *Wrapper) filterIn(buf []byte) filter.Response { } } + if p.IsEchoResponse() { + if f := t.OnICMPEchoResponseReceived; f != nil && f(p) { + // Note: this looks dropped in metrics, even though it was + // handled internally. + return filter.DropSilently + } + } + // Issue 1526 workaround: if we see disco packets over // Tailscale from ourselves, then drop them, as that shouldn't // happen unless a networking stack is confused, as it seems diff --git a/tailcfg/tailcfg.go b/tailcfg/tailcfg.go index e4dd04d99..af8861082 100644 --- a/tailcfg/tailcfg.go +++ b/tailcfg/tailcfg.go @@ -1217,6 +1217,19 @@ type DNSRecord struct { Value string } +// PingType is a string representing the kind of ping to perform. +type PingType string + +const ( + // PingDisco performs a ping, without involving IP at either end. + PingDisco PingType = "disco" + // PingTSMP performs a ping, using the IP layer, but avoiding the OS IP stack. + PingTSMP PingType = "TSMP" + // PingICMP performs a ping between two tailscale nodes using ICMP that is + // received by the target systems IP stack. + PingICMP PingType = "ICMP" +) + // PingRequest with no IP and Types is a request to send an HTTP request to prove the // long-polling client is still connected. // PingRequest with Types and IP, will send a ping to the IP and send a POST @@ -1234,8 +1247,8 @@ type PingRequest struct { // For failure cases, the client will log regardless. Log bool `json:",omitempty"` - // Types is the types of ping that is initiated. Can be TSMP, ICMP or disco. - // Types will be comma separated, such as TSMP,disco. + // Types is the types of ping that are initiated. Can be any PingType, comma + // separated, e.g. "disco,TSMP" Types string // IP is the ping target. @@ -1246,7 +1259,7 @@ type PingRequest struct { // PingResponse provides result information for a TSMP or Disco PingRequest. // Typically populated from an ipnstate.PingResult used in `tailscale ping`. type PingResponse struct { - Type string // ping type, such as TSMP or disco. + Type PingType // ping type, such as TSMP or disco. IP string `json:",omitempty"` // ping destination NodeIP string `json:",omitempty"` // Tailscale IP of node handling IP (different for subnet routers) diff --git a/wgengine/userspace.go b/wgengine/userspace.go index 300abe4bb..e3a9157da 100644 --- a/wgengine/userspace.go +++ b/wgengine/userspace.go @@ -45,6 +45,7 @@ "tailscale.com/types/netmap" "tailscale.com/util/clientmetric" "tailscale.com/util/deephash" + "tailscale.com/util/mak" "tailscale.com/version" "tailscale.com/wgengine/filter" "tailscale.com/wgengine/magicsock" @@ -135,8 +136,15 @@ type userspaceEngine struct { endpoints []tailcfg.Endpoint pendOpen map[flowtrack.Tuple]*pendingOpenFlow // see pendopen.go networkMapCallbacks map[*someHandle]NetworkMapCallback - tsIPByIPPort map[netaddr.IPPort]netaddr.IP // allows registration of IP:ports as belonging to a certain Tailscale IP for whois lookups - pongCallback map[[8]byte]func(packet.TSMPPongReply) // for TSMP pong responses + tsIPByIPPort map[netaddr.IPPort]netaddr.IP // allows registration of IP:ports as belonging to a certain Tailscale IP for whois lookups + + // pongCallback is the map of response handlers waiting for disco or TSMP + // pong callbacks. The map key is a random slice of bytes. + pongCallback map[[8]byte]func(packet.TSMPPongReply) + // icmpEchoResponseCallback is the map of reponse handlers waiting for ICMP + // echo responses. The map key is a random uint32 that is the little endian + // value of the ICMP identifer and sequence number concatenated. + icmpEchoResponseCallback map[uint32]func() // Lock ordering: magicsock.Conn.mu, wgLock, then mu. } @@ -387,6 +395,20 @@ func NewUserspaceEngine(logf logger.Logf, conf Config) (_ Engine, reterr error) } } + e.tundev.OnICMPEchoResponseReceived = func(p *packet.Parsed) bool { + idSeq := p.EchoIDSeq() + e.mu.Lock() + defer e.mu.Unlock() + cb := e.icmpEchoResponseCallback[idSeq] + if cb == nil { + // We didn't swallow it, so let it flow to the host. + return false + } + e.logf("wgengine: got diagnostic ICMP response %02x", idSeq) + go cb() + return true + } + // wgdev takes ownership of tundev, will close it when closed. e.logf("Creating wireguard device...") e.wgdev = wgcfg.NewDevice(e.tundev, e.magicConn.Bind(), e.wgLogger.DeviceLogger) @@ -1267,7 +1289,7 @@ func (e *userspaceEngine) UpdateStatus(sb *ipnstate.StatusBuilder) { e.magicConn.UpdateStatus(sb) } -func (e *userspaceEngine) Ping(ip netaddr.IP, useTSMP bool, cb func(*ipnstate.PingResult)) { +func (e *userspaceEngine) Ping(ip netaddr.IP, pingType tailcfg.PingType, cb func(*ipnstate.PingResult)) { res := &ipnstate.PingResult{IP: ip.String()} pip, ok := e.PeerForIP(ip) if !ok { @@ -1284,15 +1306,14 @@ func (e *userspaceEngine) Ping(ip netaddr.IP, useTSMP bool, cb func(*ipnstate.Pi } peer := pip.Node - pingType := "disco" - if useTSMP { - pingType = "TSMP" - } e.logf("ping(%v): sending %v ping to %v %v ...", ip, pingType, peer.Key.ShortString(), peer.ComputedName) - if useTSMP { - e.sendTSMPPing(ip, peer, res, cb) - } else { + switch pingType { + case "disco": e.magicConn.Ping(peer, res, cb) + case "TSMP": + e.sendTSMPPing(ip, peer, res, cb) + case "ICMP": + e.sendICMPEchoRequest(ip, peer, res, cb) } } @@ -1313,6 +1334,55 @@ func (e *userspaceEngine) mySelfIPMatchingFamily(dst netaddr.IP) (src netaddr.IP return netaddr.IP{}, errors.New("no self address in netmap matching address family") } +func (e *userspaceEngine) sendICMPEchoRequest(destIP netaddr.IP, peer *tailcfg.Node, res *ipnstate.PingResult, cb func(*ipnstate.PingResult)) { + srcIP, err := e.mySelfIPMatchingFamily(destIP) + if err != nil { + res.Err = err.Error() + cb(res) + return + } + var icmph packet.Header + if srcIP.Is4() { + icmph = packet.ICMP4Header{ + IP4Header: packet.IP4Header{ + IPProto: ipproto.ICMPv4, + Src: srcIP, + Dst: destIP, + }, + Type: packet.ICMP4EchoRequest, + Code: packet.ICMP4NoCode, + } + } else { + icmph = packet.ICMP6Header{ + IP6Header: packet.IP6Header{ + IPProto: ipproto.ICMPv6, + Src: srcIP, + Dst: destIP, + }, + Type: packet.ICMP6EchoRequest, + Code: packet.ICMP6NoCode, + } + } + + idSeq, payload := packet.ICMPEchoPayload(nil) + + expireTimer := time.AfterFunc(10*time.Second, func() { + e.setICMPEchoResponseCallback(idSeq, nil) + }) + t0 := time.Now() + e.setICMPEchoResponseCallback(idSeq, func() { + expireTimer.Stop() + d := time.Since(t0) + res.LatencySeconds = d.Seconds() + res.NodeIP = destIP.String() + res.NodeName = peer.ComputedName + cb(res) + }) + + icmpPing := packet.Generate(icmph, payload) + e.tundev.InjectOutbound(icmpPing) +} + func (e *userspaceEngine) sendTSMPPing(ip netaddr.IP, peer *tailcfg.Node, res *ipnstate.PingResult, cb func(*ipnstate.PingResult)) { srcIP, err := e.mySelfIPMatchingFamily(ip) if err != nil { @@ -1373,6 +1443,16 @@ func (e *userspaceEngine) setTSMPPongCallback(data [8]byte, cb func(packet.TSMPP } } +func (e *userspaceEngine) setICMPEchoResponseCallback(idSeq uint32, cb func()) { + e.mu.Lock() + defer e.mu.Unlock() + if cb == nil { + delete(e.icmpEchoResponseCallback, idSeq) + } else { + mak.Set(&e.icmpEchoResponseCallback, idSeq, cb) + } +} + func (e *userspaceEngine) RegisterIPPortIdentity(ipport netaddr.IPPort, tsIP netaddr.IP) { e.mu.Lock() defer e.mu.Unlock() diff --git a/wgengine/watchdog.go b/wgengine/watchdog.go index 8b4972afb..523cfda8d 100644 --- a/wgengine/watchdog.go +++ b/wgengine/watchdog.go @@ -117,8 +117,8 @@ func (e *watchdogEngine) DiscoPublicKey() (k key.DiscoPublic) { e.watchdog("DiscoPublicKey", func() { k = e.wrap.DiscoPublicKey() }) return k } -func (e *watchdogEngine) Ping(ip netaddr.IP, useTSMP bool, cb func(*ipnstate.PingResult)) { - e.watchdog("Ping", func() { e.wrap.Ping(ip, useTSMP, cb) }) +func (e *watchdogEngine) Ping(ip netaddr.IP, pingType tailcfg.PingType, cb func(*ipnstate.PingResult)) { + e.watchdog("Ping", func() { e.wrap.Ping(ip, pingType, cb) }) } func (e *watchdogEngine) RegisterIPPortIdentity(ipp netaddr.IPPort, tsIP netaddr.IP) { e.watchdog("RegisterIPPortIdentity", func() { e.wrap.RegisterIPPortIdentity(ipp, tsIP) }) diff --git a/wgengine/wgengine.go b/wgengine/wgengine.go index 15ff35775..773c9bf13 100644 --- a/wgengine/wgengine.go +++ b/wgengine/wgengine.go @@ -154,9 +154,9 @@ type Engine interface { // status builder. UpdateStatus(*ipnstate.StatusBuilder) - // Ping is a request to start a discovery ping with the peer handling - // the given IP and then call cb with its ping latency & method. - Ping(ip netaddr.IP, useTSMP bool, cb func(*ipnstate.PingResult)) + // Ping is a request to start a ping with the peer handling the given IP and + // then call cb with its ping latency & method. + Ping(ip netaddr.IP, pingType tailcfg.PingType, cb func(*ipnstate.PingResult)) // RegisterIPPortIdentity registers a given node (identified by its // Tailscale IP) as temporarily having the given IP:port for whois lookups.