diff --git a/client/tailscale/localclient.go b/client/tailscale/localclient.go index e0b9592b5..79650440c 100644 --- a/client/tailscale/localclient.go +++ b/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) diff --git a/cmd/tailscale/cli/ping.go b/cmd/tailscale/cli/ping.go index f6ca4183c..2c5dfed22 100644 --- a/cmd/tailscale/cli/ping.go +++ b/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) { diff --git a/control/controlclient/direct.go b/control/controlclient/direct.go index 28b3c97a6..571356e58 100644 --- a/control/controlclient/direct.go +++ b/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) diff --git a/disco/disco.go b/disco/disco.go index 0e7c3f7e5..46379b9d2 100644 --- a/disco/disco.go +++ b/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 { diff --git a/disco/disco_test.go b/disco/disco_test.go index 67bd1561a..1a56324a5 100644 --- a/disco/disco_test.go +++ b/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{ diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 03b088998..c8ca64058 100644 --- a/ipn/ipnlocal/local.go +++ b/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: diff --git a/ipn/localapi/localapi.go b/ipn/localapi/localapi.go index 9e4b41157..527519d5e 100644 --- a/ipn/localapi/localapi.go +++ b/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 diff --git a/wgengine/magicsock/endpoint.go b/wgengine/magicsock/endpoint.go index 374e430c1..6c1cc0c39 100644 --- a/wgengine/magicsock/endpoint.go +++ b/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() { diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go index 77f017295..a78003f06 100644 --- a/wgengine/magicsock/magicsock.go +++ b/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. diff --git a/wgengine/userspace.go b/wgengine/userspace.go index e29e20dae..6774045aa 100644 --- a/wgengine/userspace.go +++ b/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": diff --git a/wgengine/watchdog.go b/wgengine/watchdog.go index 19505be89..94dcef810 100644 --- a/wgengine/watchdog.go +++ b/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) }) diff --git a/wgengine/wgengine.go b/wgengine/wgengine.go index df591c9e0..b288e658a 100644 --- a/wgengine/wgengine.go +++ b/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.