From 7bfd4f521d9078caba692f33a04b288f1c453ab7 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Tue, 7 Sep 2021 11:57:23 -0700 Subject: [PATCH] cmd/tailscale: fix "tailscale ip $self-host-hostname" And in the process, fix the related confusing error messages from pinging your own IP or hostname. Fixes #2803 Signed-off-by: Brad Fitzpatrick --- cmd/tailscale/cli/file.go | 2 +- cmd/tailscale/cli/ip.go | 9 ++++++++- cmd/tailscale/cli/ping.go | 34 ++++++++++++++++++++++++---------- ipn/ipnstate/ipnstate.go | 4 ++++ wgengine/pendopen.go | 4 ++-- wgengine/userspace.go | 28 ++++++++++++++++++++-------- 6 files changed, 59 insertions(+), 22 deletions(-) diff --git a/cmd/tailscale/cli/file.go b/cmd/tailscale/cli/file.go index 7a66612fa..7f6d9b5f0 100644 --- a/cmd/tailscale/cli/file.go +++ b/cmd/tailscale/cli/file.go @@ -91,7 +91,7 @@ func runCp(ctx context.Context, args []string) error { } else if hadBrackets && (err != nil || !ip.Is6()) { return errors.New("unexpected brackets around target") } - ip, err := tailscaleIPFromArg(ctx, target) + ip, _, err := tailscaleIPFromArg(ctx, target) if err != nil { return err } diff --git a/cmd/tailscale/cli/ip.go b/cmd/tailscale/cli/ip.go index 388636a6d..3acc683e5 100644 --- a/cmd/tailscale/cli/ip.go +++ b/cmd/tailscale/cli/ip.go @@ -57,7 +57,7 @@ func runIP(ctx context.Context, args []string) error { } ips := st.TailscaleIPs if of != "" { - ip, err := tailscaleIPFromArg(ctx, of) + ip, _, err := tailscaleIPFromArg(ctx, of) if err != nil { return err } @@ -101,5 +101,12 @@ func peerMatchingIP(st *ipnstate.Status, ipStr string) (ps *ipnstate.PeerStatus, } } } + if ps := st.Self; ps != nil { + for _, pip := range ps.TailscaleIPs { + if ip == pip { + return ps, true + } + } + } return nil, false } diff --git a/cmd/tailscale/cli/ping.go b/cmd/tailscale/cli/ping.go index 25470aa69..0d3c16caf 100644 --- a/cmd/tailscale/cli/ping.go +++ b/cmd/tailscale/cli/ping.go @@ -84,10 +84,14 @@ func runPing(ctx context.Context, args []string) error { go func() { pumpErr <- pump(ctx, bc, c) }() hostOrIP := args[0] - ip, err := tailscaleIPFromArg(ctx, hostOrIP) + ip, self, err := tailscaleIPFromArg(ctx, hostOrIP) if err != nil { return err } + if self { + fmt.Printf("%v is local Tailscale IP\n", ip) + return nil + } if pingArgs.verbose && ip != hostOrIP { log.Printf("lookup %q => %q", hostOrIP, ip) @@ -107,6 +111,10 @@ func runPing(ctx context.Context, args []string) error { case pr := <-prc: timer.Stop() if pr.Err != "" { + if pr.IsLocalIP { + fmt.Println(pr.Err) + return nil + } return errors.New(pr.Err) } latency := time.Duration(pr.LatencySeconds * float64(time.Second)).Round(time.Millisecond) @@ -147,33 +155,39 @@ func runPing(ctx context.Context, args []string) error { } } -func tailscaleIPFromArg(ctx context.Context, hostOrIP string) (ip string, err error) { +func tailscaleIPFromArg(ctx context.Context, hostOrIP string) (ip string, self bool, err error) { // If the argument is an IP address, use it directly without any resolution. if net.ParseIP(hostOrIP) != nil { - return hostOrIP, nil + return hostOrIP, false, nil } // Otherwise, try to resolve it first from the network peer list. st, err := tailscale.Status(ctx) if err != nil { - return "", err + return "", false, err + } + match := func(ps *ipnstate.PeerStatus) bool { + return strings.EqualFold(hostOrIP, dnsOrQuoteHostname(st, ps)) || hostOrIP == ps.DNSName } for _, ps := range st.Peer { - if hostOrIP == dnsOrQuoteHostname(st, ps) || hostOrIP == ps.DNSName { + if match(ps) { if len(ps.TailscaleIPs) == 0 { - return "", errors.New("node found but lacks an IP") + return "", false, errors.New("node found but lacks an IP") } - return ps.TailscaleIPs[0].String(), nil + return ps.TailscaleIPs[0].String(), false, nil } } + if match(st.Self) && len(st.Self.TailscaleIPs) > 0 { + return st.Self.TailscaleIPs[0].String(), true, nil + } // Finally, use DNS. var res net.Resolver if addrs, err := res.LookupHost(ctx, hostOrIP); err != nil { - return "", fmt.Errorf("error looking up IP of %q: %v", hostOrIP, err) + return "", false, fmt.Errorf("error looking up IP of %q: %v", hostOrIP, err) } else if len(addrs) == 0 { - return "", fmt.Errorf("no IPs found for %q", hostOrIP) + return "", false, fmt.Errorf("no IPs found for %q", hostOrIP) } else { - return addrs[0], nil + return addrs[0], false, nil } } diff --git a/ipn/ipnstate/ipnstate.go b/ipn/ipnstate/ipnstate.go index e77a60585..196659a3d 100644 --- a/ipn/ipnstate/ipnstate.go +++ b/ipn/ipnstate/ipnstate.go @@ -456,6 +456,10 @@ type PingResult struct { // running the server on. PeerAPIPort uint16 `json:",omitempty"` + // IsLocalIP is whether the ping request error is due to it being + // a ping to the local node. + IsLocalIP bool `json:",omitempty"` + // TODO(bradfitz): details like whether port mapping was used on either side? (Once supported) } diff --git a/wgengine/pendopen.go b/wgengine/pendopen.go index f473591b9..5d51ff3ae 100644 --- a/wgengine/pendopen.go +++ b/wgengine/pendopen.go @@ -117,7 +117,7 @@ func (e *userspaceEngine) trackOpenPostFilterOut(pp *packet.Parsed, t *tstun.Wra // like: // open-conn-track: timeout opening (100.115.73.60:52501 => 17.125.252.5:443); no associated peer node if runtime.GOOS == "ios" && flow.Dst.Port() == 443 && !tsaddr.IsTailscaleIP(flow.Dst.IP()) { - if _, err := e.peerForIP(flow.Dst.IP()); err != nil { + if _, _, err := e.peerForIP(flow.Dst.IP()); err != nil { return } } @@ -157,7 +157,7 @@ func (e *userspaceEngine) onOpenTimeout(flow flowtrack.Tuple) { } // Diagnose why it might've timed out. - n, err := e.peerForIP(flow.Dst.IP()) + n, _, err := e.peerForIP(flow.Dst.IP()) if err != nil { e.logf("open-conn-track: timeout opening %v; peerForIP: %v", flow, err) return diff --git a/wgengine/userspace.go b/wgengine/userspace.go index 7d41303ea..354444757 100644 --- a/wgengine/userspace.go +++ b/wgengine/userspace.go @@ -1244,7 +1244,7 @@ func (e *userspaceEngine) UpdateStatus(sb *ipnstate.StatusBuilder) { func (e *userspaceEngine) Ping(ip netaddr.IP, useTSMP bool, cb func(*ipnstate.PingResult)) { res := &ipnstate.PingResult{IP: ip.String()} - peer, err := e.peerForIP(ip) + peer, self, err := e.peerForIP(ip) if err != nil { e.logf("ping(%v): %v", ip, err) res.Err = err.Error() @@ -1257,6 +1257,13 @@ func (e *userspaceEngine) Ping(ip netaddr.IP, useTSMP bool, cb func(*ipnstate.Pi cb(res) return } + if self { + res.Err = fmt.Sprintf("%v is local Tailscale IP", ip) + res.IsLocalIP = true + cb(res) + return + } + pingType := "disco" if useTSMP { pingType = "TSMP" @@ -1400,12 +1407,12 @@ func (e *userspaceEngine) WhoIsIPPort(ipport netaddr.IPPort) (tsIP netaddr.IP, o // // peerForIP acquires both e.mu and e.wgLock, but neither at the same // time. -func (e *userspaceEngine) peerForIP(ip netaddr.IP) (n *tailcfg.Node, err error) { +func (e *userspaceEngine) peerForIP(ip netaddr.IP) (n *tailcfg.Node, isSelf bool, err error) { e.mu.Lock() nm := e.netMap e.mu.Unlock() if nm == nil { - return nil, errors.New("no network map") + return nil, false, errors.New("no network map") } // Check for exact matches before looking for subnet matches. @@ -1414,7 +1421,7 @@ func (e *userspaceEngine) peerForIP(ip netaddr.IP) (n *tailcfg.Node, err error) for _, p := range nm.Peers { for _, a := range p.Addresses { if a.IP() == ip && a.IsSingleIP() && tsaddr.IsTailscaleIP(ip) { - return p, nil + return p, false, nil } } for _, cidr := range p.AllowedIPs { @@ -1427,6 +1434,11 @@ func (e *userspaceEngine) peerForIP(ip netaddr.IP) (n *tailcfg.Node, err error) } } } + for _, a := range nm.Addresses { + if a.IP() == ip && a.IsSingleIP() && tsaddr.IsTailscaleIP(ip) { + return nm.SelfNode, true, nil + } + } e.wgLock.Lock() defer e.wgLock.Unlock() @@ -1450,17 +1462,17 @@ func (e *userspaceEngine) peerForIP(ip netaddr.IP) (n *tailcfg.Node, err error) if !bestKey.IsZero() { for _, p := range nm.Peers { if p.Key == bestKey { - return p, nil + return p, false, nil } } } if bestInNM == nil { - return nil, nil + return nil, false, nil } if bestInNMPrefix.Bits() == 0 { - return nil, errors.New("exit node found but not enabled") + return nil, false, errors.New("exit node found but not enabled") } - return nil, fmt.Errorf("node %q found, but not using its %v route", bestInNM.ComputedNameWithHost, bestInNMPrefix) + return nil, false, fmt.Errorf("node %q found, but not using its %v route", bestInNM.ComputedNameWithHost, bestInNMPrefix) } type closeOnErrorPool []func()