wgengine,ipn,cmd/tailscale: add size option to ping (#8739)

This adds the capability to pad disco ping message payloads to reach a
specified size. It also plumbs it through to the tailscale ping -size
flag.

Disco pings used for actual endpoint discovery do not use this yet.

Updates #311.

Signed-off-by: salman <salman@tailscale.com>
Co-authored-by: Val <valerie@tailscale.com>

client/tailscale/localclient.go

@@ -807,11 +807,25 @@ func (lc *LocalClient) ExpandSNIName(ctx context.Context, name string) (fqdn str
 	return "", false
 }
 
+// PingOpts contains options for the ping request.
+//
+// The zero value is valid, which means to use defaults.
+type PingOpts struct {
+	// Size is the length of the ping message in bytes. It's ignored if it's
+	// smaller than the minimum message size.
+	//
+	// For disco pings, it specifies the length of the packet's payload. That
+	// is, it includes the disco headers and message, but not the IP and UDP
+	// headers.
+	Size int
+}
+
 // Ping sends a ping of the provided type to the provided IP and waits
-// for its response.
-func (lc *LocalClient) Ping(ctx context.Context, ip netip.Addr, pingtype tailcfg.PingType) (*ipnstate.PingResult, error) {
+// for its response. The opts type specifies additional options.
+func (lc *LocalClient) PingWithOpts(ctx context.Context, ip netip.Addr, pingtype tailcfg.PingType, opts PingOpts) (*ipnstate.PingResult, error) {
 	v := url.Values{}
 	v.Set("ip", ip.String())
+	v.Set("size", strconv.Itoa(opts.Size))
 	v.Set("type", string(pingtype))
 	body, err := lc.send(ctx, "POST", "/localapi/v0/ping?"+v.Encode(), 200, nil)
 	if err != nil {
@@ -820,6 +834,12 @@ func (lc *LocalClient) Ping(ctx context.Context, ip netip.Addr, pingtype tailcfg
 	return decodeJSON[*ipnstate.PingResult](body)
 }
 
+// Ping sends a ping of the provided type to the provided IP and waits
+// for its response.
+func (lc *LocalClient) Ping(ctx context.Context, ip netip.Addr, pingtype tailcfg.PingType) (*ipnstate.PingResult, error) {
+	return lc.PingWithOpts(ctx, ip, pingtype, PingOpts{})
+}
+
 // NetworkLockStatus fetches information about the tailnet key authority, if one is configured.
 func (lc *LocalClient) NetworkLockStatus(ctx context.Context) (*ipnstate.NetworkLockStatus, error) {
 	body, err := lc.send(ctx, "GET", "/localapi/v0/tka/status", 200, nil)

cmd/tailscale/cli/ping.go

@@ -16,6 +16,7 @@ import (
 	"time"
 
 	"github.com/peterbourgon/ff/v3/ffcli"
+	"tailscale.com/client/tailscale"
 	"tailscale.com/ipn/ipnstate"
 	"tailscale.com/tailcfg"
 )
@@ -53,12 +54,14 @@ relay node.
 		fs.BoolVar(&pingArgs.peerAPI, "peerapi", false, "try hitting the peer's peerapi HTTP server")
 		fs.IntVar(&pingArgs.num, "c", 10, "max number of pings to send. 0 for infinity.")
 		fs.DurationVar(&pingArgs.timeout, "timeout", 5*time.Second, "timeout before giving up on a ping")
+		fs.IntVar(&pingArgs.size, "size", 0, "size of the ping message (disco pings only). 0 for minimum size.")
 		return fs
 	})(),
 }
 
 var pingArgs struct {
 	num         int
+	size        int
 	untilDirect bool
 	verbose     bool
 	tsmp        bool
@@ -115,7 +118,7 @@ func runPing(ctx context.Context, args []string) error {
 	for {
 		n++
 		ctx, cancel := context.WithTimeout(ctx, pingArgs.timeout)
-		pr, err := localClient.Ping(ctx, netip.MustParseAddr(ip), pingType())
+		pr, err := localClient.PingWithOpts(ctx, netip.MustParseAddr(ip), pingType(), tailscale.PingOpts{Size: pingArgs.size})
 		cancel()
 		if err != nil {
 			if errors.Is(err, context.DeadlineExceeded) {

control/controlclient/direct.go

@@ -171,7 +171,7 @@ type ControlDialPlanner interface {
 // Pinger is the LocalBackend.Ping method.
 type Pinger interface {
 	// Ping is a request to do a ping with the peer handling the given IP.
-	Ping(ctx context.Context, ip netip.Addr, pingType tailcfg.PingType) (*ipnstate.PingResult, error)
+	Ping(ctx context.Context, ip netip.Addr, pingType tailcfg.PingType, size int) (*ipnstate.PingResult, error)
 }
 
 type Decompressor interface {
@@ -1671,7 +1671,7 @@ func doPingerPing(logf logger.Logf, c *http.Client, pr *tailcfg.PingRequest, pin
 	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
 	defer cancel()
 
-	res, err := pinger.Ping(ctx, pr.IP, pingType)
+	res, err := pinger.Ping(ctx, pr.IP, pingType, 0)
 	if err != nil {
 		d := time.Since(start).Round(time.Millisecond)
 		logf("doPingerPing: ping error of type %q to %v after %v: %v", pingType, pr.IP, d, err)

disco/disco.go

@@ -94,6 +94,9 @@ type Message interface {
 	AppendMarshal([]byte) []byte
 }
 
+// MessageHeaderLen is the length of a message header, 2 bytes for type and version.
+const MessageHeaderLen = 2
+
 // appendMsgHeader appends two bytes (for t and ver) and then also
 // dataLen bytes to b, returning the appended slice in all. The
 // returned data slice is a subslice of all with just dataLen bytes of
@@ -117,15 +120,24 @@ type Ping struct {
 	// netmap data to reduce the discokey:nodekey relation from 1:N to
 	// 1:1.
 	NodeKey key.NodePublic
+
+	// Padding is the number of 0 bytes at the end of the
+	// message. (It's used to probe path MTU.)
+	Padding int
 }
 
+// PingLen is the length of a marshalled ping message, without the message
+// header or padding.
+const PingLen = 12 + key.NodePublicRawLen
+
 func (m *Ping) AppendMarshal(b []byte) []byte {
 	dataLen := 12
 	hasKey := !m.NodeKey.IsZero()
 	if hasKey {
 		dataLen += key.NodePublicRawLen
 	}
-	ret, d := appendMsgHeader(b, TypePing, v0, dataLen)
+
+	ret, d := appendMsgHeader(b, TypePing, v0, dataLen+m.Padding)
 	n := copy(d, m.TxID[:])
 	if hasKey {
 		m.NodeKey.AppendTo(d[:n])
@@ -138,11 +150,14 @@ func parsePing(ver uint8, p []byte) (m *Ping, err error) {
 		return nil, errShort
 	}
 	m = new(Ping)
+	m.Padding = len(p)
 	p = p[copy(m.TxID[:], p):]
+	m.Padding -= 12
 	// Deliberately lax on longer-than-expected messages, for future
 	// compatibility.
 	if len(p) >= key.NodePublicRawLen {
 		m.NodeKey = key.NodePublicFromRaw32(mem.B(p[:key.NodePublicRawLen]))
+		m.Padding -= key.NodePublicRawLen
 	}
 	return m, nil
 }
@@ -214,6 +229,8 @@ type Pong struct {
 	Src  netip.AddrPort // 18 bytes (16+2) on the wire; v4-mapped ipv6 for IPv4
 }
 
+// pongLen is the length of a marshalled pong message, without the message
+// header or padding.
 const pongLen = 12 + 16 + 2
 
 func (m *Pong) AppendMarshal(b []byte) []byte {

disco/disco_test.go

@@ -35,6 +35,23 @@ func TestMarshalAndParse(t *testing.T) {
 			},
 			want: "01 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 00 01 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 1e 1f",
 		},
+		{
+			name: "ping_with_padding",
+			m: &Ping{
+				TxID:    [12]byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12},
+				Padding: 3,
+			},
+			want: "01 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 00 00 00",
+		},
+		{
+			name: "ping_with_padding_and_nodekey_src",
+			m: &Ping{
+				TxID:    [12]byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12},
+				NodeKey: key.NodePublicFromRaw32(mem.B([]byte{1: 1, 2: 2, 30: 30, 31: 31})),
+				Padding: 3,
+			},
+			want: "01 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 00 01 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 1e 1f 00 00 00",
+		},
 		{
 			name: "pong",
 			m: &Pong{

ipn/ipnlocal/local.go

@@ -2400,7 +2400,7 @@ func (b *LocalBackend) StartLoginInteractive() {
 	}
 }
 
-func (b *LocalBackend) Ping(ctx context.Context, ip netip.Addr, pingType tailcfg.PingType) (*ipnstate.PingResult, error) {
+func (b *LocalBackend) Ping(ctx context.Context, ip netip.Addr, pingType tailcfg.PingType, size int) (*ipnstate.PingResult, error) {
 	if pingType == tailcfg.PingPeerAPI {
 		t0 := b.clock.Now()
 		node, base, err := b.pingPeerAPI(ctx, ip)
@@ -2423,7 +2423,7 @@ func (b *LocalBackend) Ping(ctx context.Context, ip netip.Addr, pingType tailcfg
 		return pr, nil
 	}
 	ch := make(chan *ipnstate.PingResult, 1)
-	b.e.Ping(ip, pingType, func(pr *ipnstate.PingResult) {
+	b.e.Ping(ip, pingType, size, func(pr *ipnstate.PingResult) {
 		select {
 		case ch <- pr:
 		default:

ipn/localapi/localapi.go

@@ -37,6 +37,7 @@ import (
 	"tailscale.com/net/netmon"
 	"tailscale.com/net/netutil"
 	"tailscale.com/net/portmapper"
+	"tailscale.com/net/tstun"
 	"tailscale.com/tailcfg"
 	"tailscale.com/tka"
 	"tailscale.com/tstime"
@@ -1346,7 +1347,24 @@ func (h *Handler) servePing(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, "missing 'type' parameter", 400)
 		return
 	}
-	res, err := h.b.Ping(ctx, ip, tailcfg.PingType(pingTypeStr))
+	size := 0
+	sizeStr := r.FormValue("size")
+	if sizeStr != "" {
+		size, err = strconv.Atoi(sizeStr)
+		if err != nil {
+			http.Error(w, "invalid 'size' parameter", 400)
+			return
+		}
+		if size != 0 && tailcfg.PingType(pingTypeStr) != tailcfg.PingDisco {
+			http.Error(w, "'size' parameter is only supported with disco pings", 400)
+			return
+		}
+		if size > int(tstun.DefaultMTU()) {
+			http.Error(w, fmt.Sprintf("maximum value for 'size' is %v", tstun.DefaultMTU()), 400)
+			return
+		}
+	}
+	res, err := h.b.Ping(ctx, ip, tailcfg.PingType(pingTypeStr), size)
 	if err != nil {
 		writeErrorJSON(w, err)
 		return

wgengine/magicsock/endpoint.go

@@ -19,10 +19,12 @@ import (
 	"sync/atomic"
 	"time"
 
+	"golang.org/x/crypto/poly1305"
 	"golang.org/x/exp/maps"
 	"tailscale.com/disco"
 	"tailscale.com/ipn/ipnstate"
 	"tailscale.com/net/stun"
+	"tailscale.com/net/tstun"
 	"tailscale.com/tailcfg"
 	"tailscale.com/tstime/mono"
 	"tailscale.com/types/key"
@@ -361,7 +363,7 @@ func (de *endpoint) heartbeat() {
 	udpAddr, _, _ := de.addrForSendLocked(now)
 	if udpAddr.IsValid() {
 		// We have a preferred path. Ping that every 2 seconds.
-		de.startDiscoPingLocked(udpAddr, now, pingHeartbeat)
+		de.startDiscoPingLocked(udpAddr, now, pingHeartbeat, 0)
 	}
 
 	if de.wantFullPingLocked(now) {
@@ -403,7 +405,7 @@ func (de *endpoint) noteActiveLocked() {
 
 // cliPing starts a ping for the "tailscale ping" command. res is value to call cb with,
 // already partially filled.
-func (de *endpoint) cliPing(res *ipnstate.PingResult, cb func(*ipnstate.PingResult)) {
+func (de *endpoint) cliPing(res *ipnstate.PingResult, size int, cb func(*ipnstate.PingResult)) {
 	de.mu.Lock()
 	defer de.mu.Unlock()
 
@@ -418,17 +420,17 @@ func (de *endpoint) cliPing(res *ipnstate.PingResult, cb func(*ipnstate.PingResu
 	now := mono.Now()
 	udpAddr, derpAddr, _ := de.addrForSendLocked(now)
 	if derpAddr.IsValid() {
-		de.startDiscoPingLocked(derpAddr, now, pingCLI)
+		de.startDiscoPingLocked(derpAddr, now, pingCLI, size)
 	}
 	if udpAddr.IsValid() && now.Before(de.trustBestAddrUntil) {
 		// Already have an active session, so just ping the address we're using.
 		// Otherwise "tailscale ping" results to a node on the local network
 		// can look like they're bouncing between, say 10.0.0.0/9 and the peer's
 		// IPv6 address, both 1ms away, and it's random who replies first.
-		de.startDiscoPingLocked(udpAddr, now, pingCLI)
+		de.startDiscoPingLocked(udpAddr, now, pingCLI, size)
 	} else {
 		for ep := range de.endpointState {
-			de.startDiscoPingLocked(ep, now, pingCLI)
+			de.startDiscoPingLocked(ep, now, pingCLI, size)
 		}
 	}
 	de.noteActiveLocked()
@@ -522,17 +524,31 @@ func (de *endpoint) removeSentDiscoPingLocked(txid stun.TxID, sp sentPing) {
 	delete(de.sentPing, txid)
 }
 
-// sendDiscoPing sends a ping with the provided txid to ep using de's discoKey.
+// discoPingSize is the size of a complete disco ping packet, without any padding.
+const discoPingSize = len(disco.Magic) + key.DiscoPublicRawLen + disco.NonceLen +
+	poly1305.TagSize + disco.MessageHeaderLen + disco.PingLen
+
+// sendDiscoPing sends a ping with the provided txid to ep using de's discoKey. size
+// is the desired disco message size, including all disco headers but excluding IP/UDP
+// headers.
 //
 // The caller (startPingLocked) should've already recorded the ping in
 // sentPing and set up the timer.
 //
 // The caller should use de.discoKey as the discoKey argument.
 // It is passed in so that sendDiscoPing doesn't need to lock de.mu.
-func (de *endpoint) sendDiscoPing(ep netip.AddrPort, discoKey key.DiscoPublic, txid stun.TxID, logLevel discoLogLevel) {
+func (de *endpoint) sendDiscoPing(ep netip.AddrPort, discoKey key.DiscoPublic, txid stun.TxID, size int, logLevel discoLogLevel) {
+	padding := 0
+	if size > int(tstun.DefaultMTU()) {
+		size = int(tstun.DefaultMTU())
+	}
+	if size-discoPingSize > 0 {
+		padding = size - discoPingSize
+	}
 	sent, _ := de.c.sendDiscoMessage(ep, de.publicKey, discoKey, &disco.Ping{
 		TxID:    [12]byte(txid),
 		NodeKey: de.c.publicKeyAtomic.Load(),
+		Padding: padding,
 	}, logLevel)
 	if !sent {
 		de.forgetDiscoPing(txid)
@@ -557,7 +573,8 @@ const (
 	pingCLI
 )
 
-func (de *endpoint) startDiscoPingLocked(ep netip.AddrPort, now mono.Time, purpose discoPingPurpose) {
+// startDiscoPingLocked sends a disco ping to ep in a separate goroutine.
+func (de *endpoint) startDiscoPingLocked(ep netip.AddrPort, now mono.Time, purpose discoPingPurpose, size int) {
 	if runtime.GOOS == "js" {
 		return
 	}
@@ -587,9 +604,10 @@ func (de *endpoint) startDiscoPingLocked(ep netip.AddrPort, now mono.Time, purpo
 	if purpose == pingHeartbeat {
 		logLevel = discoVerboseLog
 	}
-	go de.sendDiscoPing(ep, epDisco.key, txid, logLevel)
+	go de.sendDiscoPing(ep, epDisco.key, txid, size, logLevel)
 }
 
+// sendDiscoPingsLocked starts pinging all of ep's endpoints.
 func (de *endpoint) sendDiscoPingsLocked(now mono.Time, sendCallMeMaybe bool) {
 	de.lastFullPing = now
 	var sentAny bool
@@ -612,7 +630,7 @@ func (de *endpoint) sendDiscoPingsLocked(now mono.Time, sendCallMeMaybe bool) {
 			de.c.dlogf("[v1] magicsock: disco: send, starting discovery for %v (%v)", de.publicKey.ShortString(), de.discoShort())
 		}
 
-		de.startDiscoPingLocked(ep, now, pingDiscovery)
+		de.startDiscoPingLocked(ep, now, pingDiscovery, 0)
 	}
 	derpAddr := de.derpAddr
 	if sentAny && sendCallMeMaybe && derpAddr.IsValid() {

wgengine/magicsock/magicsock.go

@@ -720,7 +720,7 @@ func (c *Conn) LastRecvActivityOfNodeKey(nk key.NodePublic) string {
 }
 
 // Ping handles a "tailscale ping" CLI query.
-func (c *Conn) Ping(peer *tailcfg.Node, res *ipnstate.PingResult, cb func(*ipnstate.PingResult)) {
+func (c *Conn) Ping(peer *tailcfg.Node, res *ipnstate.PingResult, size int, cb func(*ipnstate.PingResult)) {
 	c.mu.Lock()
 	defer c.mu.Unlock()
 	if c.privateKey.IsZero() {
@@ -744,7 +744,7 @@ func (c *Conn) Ping(peer *tailcfg.Node, res *ipnstate.PingResult, cb func(*ipnst
 		cb(res)
 		return
 	}
-	ep.cliPing(res, cb)
+	ep.cliPing(res, size, cb)
 }
 
 // c.mu must be held
@@ -1262,7 +1262,7 @@ func (c *Conn) sendDiscoMessage(dst netip.AddrPort, dstKey key.NodePublic, dstDi
 			if !dstKey.IsZero() {
 				node = dstKey.ShortString()
 			}
-			c.dlogf("[v1] magicsock: disco: %v->%v (%v, %v) sent %v", c.discoShort, dstDisco.ShortString(), node, derpStr(dst.String()), disco.MessageSummary(m))
+			c.dlogf("[v1] magicsock: disco: %v->%v (%v, %v) sent %v len %v\n", c.discoShort, dstDisco.ShortString(), node, derpStr(dst.String()), disco.MessageSummary(m), len(pkt))
 		}
 		if isDERP {
 			metricSentDiscoDERP.Add(1)
@@ -1330,7 +1330,7 @@ func (c *Conn) handleDiscoMessage(msg []byte, src netip.AddrPort, derpNodeSrc ke
 		return
 	}
 	if debugDisco() {
-		c.logf("magicsock: disco: got disco-looking frame from %v via %s", sender.ShortString(), via)
+		c.logf("magicsock: disco: got disco-looking frame from %v via %s len %v", sender.ShortString(), via, len(msg))
 	}
 	if c.privateKey.IsZero() {
 		// Ignore disco messages when we're stopped.

wgengine/userspace.go

@@ -1211,7 +1211,7 @@ func (e *userspaceEngine) UpdateStatus(sb *ipnstate.StatusBuilder) {
 	e.magicConn.UpdateStatus(sb)
 }
 
-func (e *userspaceEngine) Ping(ip netip.Addr, pingType tailcfg.PingType, cb func(*ipnstate.PingResult)) {
+func (e *userspaceEngine) Ping(ip netip.Addr, pingType tailcfg.PingType, size int, cb func(*ipnstate.PingResult)) {
 	res := &ipnstate.PingResult{IP: ip.String()}
 	pip, ok := e.PeerForIP(ip)
 	if !ok {
@@ -1231,7 +1231,7 @@ func (e *userspaceEngine) Ping(ip netip.Addr, pingType tailcfg.PingType, cb func
 	e.logf("ping(%v): sending %v ping to %v %v ...", ip, pingType, peer.Key.ShortString(), peer.ComputedName)
 	switch pingType {
 	case "disco":
-		e.magicConn.Ping(peer, res, cb)
+		e.magicConn.Ping(peer, res, size, cb)
 	case "TSMP":
 		e.sendTSMPPing(ip, peer, res, cb)
 	case "ICMP":

wgengine/watchdog.go

@@ -158,8 +158,8 @@ func (e *watchdogEngine) DiscoPublicKey() (k key.DiscoPublic) {
 	e.watchdog("DiscoPublicKey", func() { k = e.wrap.DiscoPublicKey() })
 	return k
 }
-func (e *watchdogEngine) Ping(ip netip.Addr, pingType tailcfg.PingType, cb func(*ipnstate.PingResult)) {
-	e.watchdog("Ping", func() { e.wrap.Ping(ip, pingType, cb) })
+func (e *watchdogEngine) Ping(ip netip.Addr, pingType tailcfg.PingType, size int, cb func(*ipnstate.PingResult)) {
+	e.watchdog("Ping", func() { e.wrap.Ping(ip, pingType, size, cb) })
 }
 func (e *watchdogEngine) RegisterIPPortIdentity(ipp netip.AddrPort, tsIP netip.Addr) {
 	e.watchdog("RegisterIPPortIdentity", func() { e.wrap.RegisterIPPortIdentity(ipp, tsIP) })

wgengine/wgengine.go

@@ -150,9 +150,11 @@ type Engine interface {
 	// status builder.
 	UpdateStatus(*ipnstate.StatusBuilder)
 
-	// 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 netip.Addr, pingType tailcfg.PingType, cb func(*ipnstate.PingResult))
+	// Ping is a request to start a ping of the given message size to the peer
+	// handling the given IP, then call cb with its ping latency & method.
+	//
+	// If size is zero too small, it is ignored. See tailscale.PingOpts for details.
+	Ping(ip netip.Addr, pingType tailcfg.PingType, size int, cb func(*ipnstate.PingResult))
 
 	// RegisterIPPortIdentity registers a given node (identified by its
 	// Tailscale IP) as temporarily having the given IP:port for whois lookups.