tailcfg, control/controlclient: TSMP & disco pings

tailcfg.PingResponse formalizes the TSMP & disco response message, and
controlclient is wired to send POST responses containing
tailcfg.PingResponse for TSMP and disco PingRequests.

Updates tailscale/corp#754

Signed-off-by: James Tucker <james@tailscale.com>

control/controlclient/direct.go

@@ -850,7 +850,7 @@ func (c *Direct) sendMapRequest(ctx context.Context, maxPolls int, cb func(*netm
 
 		if pr := resp.PingRequest; pr != nil && c.isUniquePingRequest(pr) {
 			metricMapResponsePings.Add(1)
-			go answerPing(c.logf, c.httpc, pr)
+			go answerPing(c.logf, c.httpc, pr, c.pinger)
 		}
 		if u := resp.PopBrowserURL; u != "" && u != sess.lastPopBrowserURL {
 			sess.lastPopBrowserURL = u
@@ -1181,29 +1181,47 @@ func (c *Direct) isUniquePingRequest(pr *tailcfg.PingRequest) bool {
 	return true
 }
 
-func answerPing(logf logger.Logf, c *http.Client, pr *tailcfg.PingRequest) {
+func answerPing(logf logger.Logf, c *http.Client, pr *tailcfg.PingRequest, pinger Pinger) {
 	if pr.URL == "" {
 		logf("invalid PingRequest with no URL")
 		return
 	}
+	if pr.Types == "" {
+		answerHeadPing(logf, c, pr)
+		return
+	}
+	for _, t := range strings.Split(pr.Types, ",") {
+		switch t {
+		case "TSMP", "disco":
+			go doPingerPing(logf, c, pr, pinger, t)
+		// TODO(tailscale/corp#754)
+		// case "host":
+		// case "peerapi":
+		default:
+			logf("unsupported ping request type: %q", t)
+		}
+	}
+}
+
+func answerHeadPing(logf logger.Logf, c *http.Client, pr *tailcfg.PingRequest) {
 	ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
 	defer cancel()
 
 	req, err := http.NewRequestWithContext(ctx, "HEAD", pr.URL, nil)
 	if err != nil {
-		logf("http.NewRequestWithContext(%q): %v", pr.URL, err)
+		logf("answerHeadPing: NewRequestWithContext: %v", err)
 		return
 	}
 	if pr.Log {
-		logf("answerPing: sending ping to %v ...", pr.URL)
+		logf("answerHeadPing: sending HEAD ping to %v ...", pr.URL)
 	}
 	t0 := time.Now()
 	_, err = c.Do(req)
 	d := time.Since(t0).Round(time.Millisecond)
 	if err != nil {
-		logf("answerPing error: %v to %v (after %v)", err, pr.URL, d)
+		logf("answerHeadPing error: %v to %v (after %v)", err, pr.URL, d)
 	} else if pr.Log {
-		logf("answerPing complete to %v (after %v)", pr.URL, d)
+		logf("answerHeadPing complete to %v (after %v)", pr.URL, d)
 	}
 }
 
@@ -1376,35 +1394,28 @@ func (c *Direct) DoNoiseRequest(req *http.Request) (*http.Response, error) {
 	return nc.Do(req)
 }
 
-// tsmpPing sends a Ping to pr.IP, and sends an http request back to pr.URL
+// doPingerPing sends a Ping to pr.IP using pinger, and sends an http request back to
-// with ping response data.
+// pr.URL with ping response data.
-func tsmpPing(logf logger.Logf, c *http.Client, pr *tailcfg.PingRequest, pinger Pinger) error {
+func doPingerPing(logf logger.Logf, c *http.Client, pr *tailcfg.PingRequest, pinger Pinger, pingType string) {
-	var err error
+	if pr.URL == "" || pr.IP.IsZero() || pinger == nil {
-	if pr.URL == "" {
+		logf("invalid ping request: missing url, ip or pinger")
-		return errors.New("invalid PingRequest with no URL")
+		return
 	}
-	if pr.IP.IsZero() {
+	start := time.Now()
-		return errors.New("PingRequest without IP")
+	pinger.Ping(pr.IP, pingType == "TSMP", func(res *ipnstate.PingResult) {
-	}
-	if !strings.Contains(pr.Types, "TSMP") {
-		return fmt.Errorf("PingRequest with no TSMP in Types, got %q", pr.Types)
-	}
-
-	now := time.Now()
-	pinger.Ping(pr.IP, true, func(res *ipnstate.PingResult) {
 		// Currently does not check for error since we just return if it fails.
-		err = postPingResult(now, logf, c, pr, res)
+		postPingResult(start, logf, c, pr, res.ToPingResponse(pingType))
 	})
-	return err
 }
 
-func postPingResult(now time.Time, logf logger.Logf, c *http.Client, pr *tailcfg.PingRequest, res *ipnstate.PingResult) error {
+func postPingResult(start time.Time, logf logger.Logf, c *http.Client, pr *tailcfg.PingRequest, res *tailcfg.PingResponse) error {
-	if res.Err != "" {
+	duration := time.Since(start)
-		return errors.New(res.Err)
-	}
-	duration := time.Since(now)
 	if pr.Log {
-		logf("TSMP ping to %v completed in %v seconds. pinger.Ping took %v seconds", pr.IP, res.LatencySeconds, duration.Seconds())
+		if res.Err == "" {
+			logf("ping to %v completed in %v. pinger.Ping took %v seconds", pr.IP, res.LatencySeconds, duration)
+		} else {
+			logf("ping to %v failed after %v: %v", pr.IP, duration, res.Err)
+		}
 	}
 	ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
 	defer cancel()
@@ -1414,20 +1425,20 @@ func postPingResult(now time.Time, logf logger.Logf, c *http.Client, pr *tailcfg
 		return err
 	}
 	// Send the results of the Ping, back to control URL.
-	req, err := http.NewRequestWithContext(ctx, "POST", pr.URL, bytes.NewBuffer(jsonPingRes))
+	req, err := http.NewRequestWithContext(ctx, "POST", pr.URL, bytes.NewReader(jsonPingRes))
 	if err != nil {
 		return fmt.Errorf("http.NewRequestWithContext(%q): %w", pr.URL, err)
 	}
 	if pr.Log {
-		logf("tsmpPing: sending ping results to %v ...", pr.URL)
+		logf("postPingResult: sending ping results to %v ...", pr.URL)
 	}
 	t0 := time.Now()
 	_, err = c.Do(req)
 	d := time.Since(t0).Round(time.Millisecond)
 	if err != nil {
-		return fmt.Errorf("tsmpPing error: %w to %v (after %v)", err, pr.URL, d)
+		return fmt.Errorf("postPingResult error: %w to %v (after %v)", err, pr.URL, d)
 	} else if pr.Log {
-		logf("tsmpPing complete to %v (after %v)", pr.URL, d)
+		logf("postPingResult complete to %v (after %v)", pr.URL, d)
 	}
 	return nil
 }

control/controlclient/direct_test.go

@@ -113,7 +113,8 @@ func TestTsmpPing(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	pingRes := &ipnstate.PingResult{
+	pingRes := &tailcfg.PingResponse{
+		Type:     "TSMP",
 		IP:       "123.456.7890",
 		Err:      "",
 		NodeName: "testnode",

ipn/ipnstate/ipnstate.go

@@ -480,6 +480,8 @@ func osEmoji(os string) string {
 
 // PingResult contains response information for the "tailscale ping" subcommand,
 // saying how Tailscale can reach a Tailscale IP or subnet-routed IP.
+// See tailcfg.PingResponse for a related response that is sent back to control
+// for remote diagnostic pings.
 type PingResult struct {
 	IP       string // ping destination
 	NodeIP   string // Tailscale IP of node handling IP (different for subnet routers)
@@ -513,6 +515,22 @@ 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 {
+	return &tailcfg.PingResponse{
+		Type:           pingType,
+		IP:             pr.IP,
+		NodeIP:         pr.NodeIP,
+		NodeName:       pr.NodeName,
+		Err:            pr.Err,
+		LatencySeconds: pr.LatencySeconds,
+		Endpoint:       pr.Endpoint,
+		DERPRegionID:   pr.DERPRegionID,
+		DERPRegionCode: pr.DERPRegionCode,
+		PeerAPIPort:    pr.PeerAPIPort,
+		IsLocalIP:      pr.IsLocalIP,
+	}
+}
+
 func SortPeers(peers []*PeerStatus) {
 	sort.Slice(peers, func(i, j int) bool { return sortKey(peers[i]) < sortKey(peers[j]) })
 }

tailcfg/tailcfg.go

@@ -67,7 +67,8 @@ type CapabilityVersion int
 //    28: 2022-03-09: client can communicate over Noise.
 //    29: 2022-03-21: MapResponse.PopBrowserURL
 //    30: 2022-03-22: client can request id tokens.
-const CurrentCapabilityVersion CapabilityVersion = 30
+//    31: 2022-04-15: PingRequest & PingResponse TSMP & disco support
+const CurrentCapabilityVersion CapabilityVersion = 31
 
 type StableID string
 
@@ -1194,8 +1195,8 @@ type DNSRecord struct {
 
 // 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
+// PingRequest with Types and IP, will send a ping to the IP and send a POST
-// POST request to the URL to prove that the ping succeeded.
+// request containing a PingResponse to the URL containing results.
 type PingRequest struct {
 	// URL is the URL to send a HEAD request to.
 	// It will be a unique URL each time. No auth headers are necessary.
@@ -1218,6 +1219,48 @@ type PingRequest struct {
 	IP netaddr.IP
 }
 
+// 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.
+
+	IP       string `json:",omitempty"` // ping destination
+	NodeIP   string `json:",omitempty"` // Tailscale IP of node handling IP (different for subnet routers)
+	NodeName string `json:",omitempty"` // DNS name base or (possibly not unique) hostname
+
+	// Err contains a short description of error conditions if the PingRequest
+	// could not be fulfilled for some reason.
+	// e.g. "100.1.2.3 is local Tailscale IP"
+	Err string `json:",omitempty"`
+
+	// LatencySeconds reports measurement of the round-trip time of a message to
+	// the requested target, if it could be determined. If LatencySeconds is
+	// omitted, Err should contain information as to the cause.
+	LatencySeconds float64 `json:",omitempty"`
+
+	// Endpoint is the ip:port if direct UDP was used.
+	// It is not currently set for TSMP pings.
+	Endpoint string `json:",omitempty"`
+
+	// DERPRegionID is non-zero DERP region ID if DERP was used.
+	// It is not currently set for TSMP pings.
+	DERPRegionID int `json:",omitempty"`
+
+	// DERPRegionCode is the three-letter region code
+	// corresponding to DERPRegionID.
+	// It is not currently set for TSMP pings.
+	DERPRegionCode string `json:",omitempty"`
+
+	// PeerAPIPort is set by TSMP ping responses for peers that
+	// are running a peerapi server. This is the port they're
+	// 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"`
+}
+
 type MapResponse struct {
 	// KeepAlive, if set, represents an empty message just to keep
 	// the connection alive. When true, all other fields except